Fix/datetime filter fix

[VitaliyChaban]

fix error:
Cannot write DateTime with Kind=Unspecified to PostgreSQL type 'timestamp with time zone', only UTC is supported

also:
Fixes #1223
Fixes #1033
Fixes #378

[VitaliyChaban]

Yes, I accept the agreement. What do I need to do for this?

@microsoft-github-policy-service agree company="DexTechnology"

[VitaliyChaban]

Can someone review the changes and approve the pull request?
There are very few changes, just one line not counting the auto-test. In terms of functionality, it only affects the date filtering with time and nothing else.

[VitaliyChaban]

I added a more accurate setter for 'Kind' depending on TimeZoneInfo in the settings.
All tests are passing, and there are new tests.

private static DateTimeKind GetTargetDateTimeKind(TimeZoneInfo timeZone)
    {
        if (timeZone.Equals(TimeZoneInfo.Local))
            return DateTimeKind.Local;

        if (timeZone.Equals(TimeZoneInfo.Utc))
            return DateTimeKind.Utc;

        return DateTimeKind.Unspecified;
    }

DateTimeOffset dateTimeOffsetValue = (DateTimeOffset)value;
TimeZoneInfo timeZone = timeZoneInfo ?? TimeZoneInfo.Local;

dateTimeOffsetValue = TimeZoneInfo.ConvertTime(dateTimeOffsetValue, timeZone);

var dateTimeKind = GetTargetDateTimeKind(timeZone);

return DateTime.SpecifyKind(dateTimeOffsetValue.DateTime, dateTimeKind);

[VitaliyChaban]

@xuzhg

Could you please let me know how often you conduct releases and when we can expect a new version with this fix? Will we have to wait for a scheduled release for some time, or can a version with these changes be released earlier (perhaps as a release candidate or something similar)?

[6wanted9]

Hello! Any updates on this issue?

[6wanted9]

@habbes @xuzhg take a look, please

[6wanted9]

Guys, take a look please @xuzhg @marabooy @KenitoInc @habbes @mikepizzo @ElizabethOkerio @gathogojr

[VitaliyChaban]

Most likely, the error was that the local time on your server is equivalent to UTC.
I will take this into account in the unit tests.

[VitaliyChaban]

@WanjohiSammy
Could you please run the tests on the server again?
I have accounted for the case where the server's local time is equivalent to UTC in the tests.

[WanjohiSammy]

Getting the error:

Failed Microsoft.AspNetCore.OData.Tests.Edm.EdmPrimitiveHelperTests.ConvertDateTimeValue_ImplicitKind(valueToConvert: 2014-12-12T01:02:03.0000000+08:00) [1 ms]
  Error Message:
   Assert.Equal() Failure
Expected: Local
Actual:   Utc

  Failed Microsoft.AspNetCore.OData.Tests.Edm.EdmPrimitiveHelperTests.ConvertDateTimeValue_ImplicitKind(valueToConvert: 2014-12-12T01:02:03.0000000-08:00) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected: Local
Actual:   Utc

  Failed Microsoft.AspNetCore.OData.Tests.Edm.EdmPrimitiveHelperTests.ConvertDateTimeValue_ImplicitKind(valueToConvert: 2014-12-12T01:02:03.0000000+00:00) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected: Local
Actual:   Utc

[VitaliyChaban]

@WanjohiSammy
I fixed it.
The behavior for implicitly specifying TimeZoneInfo.Local should be the same as for explicitly specifying it.

Sorry for bothering you with testing on the test bench)
I wasn't able to reproduce the case where the system's local time = UTC right away

[gathogojr]

@VitaliyChaban Do you think you can include a unit test or E2E test for the scenario described here that confirms that the date from the URL is parsed into the expected value?

[VitaliyChaban]

@VitaliyChaban Do you think you can include a unit test or E2E test for the scenario described here that confirms that the date from the URL is parsed into the expected value?

I’m not very familiar with the context of E2E tests, so I’m not sure exactly what you mean.

The issue you’re describing should be fixed after my changes if you specify TimeZone = TimeZoneInfo.Utc in ODataSettings and in the query string use a dateTime with 'Z' at the end: '2024-04-17T09:45:00%2B02:00Z'

In that case, the original dateTime will be passed to EF, but with kind=utc, as expected.

[gathogojr]

@VitaliyChaban Do you think you can include a unit test or E2E test for the scenario described here that confirms that the date from the URL is parsed into the expected value?

I’m not very familiar with the context of E2E tests, so I’m not sure exactly what you mean.

The issue you’re describing should be fixed after my changes if you specify TimeZone = TimeZoneInfo.Utc in ODataSettings and in the query string use a dateTime with 'Z' at the end: '2024-04-17T09:45:00%2B02:00Z'

In that case, the original dateTime will be passed to EF, but with kind=utc, as expected.

Here's how the poster described the issue:

Sample GET url http://localhost:51994/time-tracks?$filter=((dateFrom+eq+2024-04-17T09:45:00%2B02:00))&$orderby=dateFrom+desc&$skip=0&$top=25 contains time 2024-04-17T9:45:00.0000000+02:00

Produces SQL query that contains "2024-04-17T11:45:00.0000000+02:00". So it get's treated as if the date in the filter was UTC time.

The E2E test I had in mind is one that has a dependency on EF. One that would help us verify that your fix truly resolves the issue. You can check through the E2E tests in the repo to see if there are E2E tests with a dependency on EF that can guide you on how to proceed.

[WanjohiSammy]

@VitaliyChaban Build is failing. Could you test this locally.

[xUnit.net 00:00:33.52]                                        ↓ (pos 641)
[xUnit.net 00:00:33.52]       Expected: ···" /><Property Name="PublishDay" Type="Edm.Date" /><Property N···
[xUnit.net 00:00:33.52]       Actual:   ···" /><Property Name="LastAction" Type="Edm.DateTimeOffset" Nul···
[xUnit.net 00:00:33.52]                                        ↑ (pos 641)

[VitaliyChaban]

@WanjohiSammy
I just revert it.
When saving a new field to the database, it is necessary to check it in other tests, and it's difficult to do this quickly during the workday, as some of your tests use comparison with a constant string, while the database may slightly format the final value during serialization.

@gathogojr
In any case, everything related to the conversion of the input dateTime (including kind and correct offset) is already covered by tests.

It seems that further testing with EF goes beyond the issue I wanted to fix. Moreover, the SQL query formation might even differ depending on the specific database being used.