Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoiding return DateTime.Kind=Unspecified when converting from DateTimeOffset to DateTime (issues 378, 384 and others) #1033

Open
wants to merge 1 commit into
base: release-8.x
Choose a base branch
from

Conversation

darkeagle76
Copy link

@darkeagle76 darkeagle76 commented Aug 29, 2023

fixes #378 , fixes #384
With this modification we would avoid getting a DateTime value (derived from a conversion from a DateTimeOffset value) that has Kind=Unspecified. The problem is mainly evident when passing Dates as parameters of the ODATA $filter option. With this change the conversion will aways return an UTC datetime when it is originated from a DateTimeOffset value.

I already provided additional details here: but my original comment remained without answer.

Thanks in advance for considering my request.
D

…meOffset to DateTime

With this modification we would avoid getting a DateTime value (derived from a conversion from a DateTimeOffset value) that has Kind=Unspecified.
The problem is mainly evident when passing Dates as parameters of the ODATA $filter option.
With this change the conversion will aways return an UTC datetime when it is originated from a DateTimeOffset value.
@darkeagle76
Copy link
Author

With this modification we would avoid getting a DateTime value (derived from a conversion from a DateTimeOffset value) that has Kind=Unspecified. The problem is mainly evident when passing Dates as parameters of the ODATA $filter option. With this change the conversion will aways return an UTC datetime when it is originated from a DateTimeOffset value.

I already provided additional details here: but my original comment remained without answer.

Thanks in advance for considering my request. D

@microsoft-github-policy-service agree

@darkeagle76 darkeagle76 changed the title Avoiding return DateTime.Kind=Unspecified when converting from DateTimeOffset to DateTime Avoiding return DateTime.Kind=Unspecified when converting from DateTimeOffset to DateTime (issues [378:](https://github.com/OData/AspNetCoreOData/issues/378), [384:](https://github.com/OData/AspNetCoreOData/issues/384) and others) Sep 7, 2023
@darkeagle76 darkeagle76 changed the title Avoiding return DateTime.Kind=Unspecified when converting from DateTimeOffset to DateTime (issues [378:](https://github.com/OData/AspNetCoreOData/issues/378), [384:](https://github.com/OData/AspNetCoreOData/issues/384) and others) Avoiding return DateTime.Kind=Unspecified when converting from DateTimeOffset to DateTime (issues 378, 384 and others) Sep 7, 2023
@gathogojr
Copy link
Contributor

Thank you @darkeagle76 for your contribution. The build was triggered and tests are failing after your change. Please check that out
In addition, please add tests to validate your change.

@KenitoInc KenitoInc self-assigned this Oct 26, 2023
@jusbuc2k
Copy link

jusbuc2k commented Aug 1, 2024

Is there a reason this hasn't been addressed? This problem breaks any usage of odata with newer Postgres/EFcore with date filtering.

Perhaps if there's concern about breaking existing usages, we just need another option on query settings like this that can default to the old way:

            options.TimeZone = TimeZoneInfo.Utc;
            options.DateTimeOffsetToDateTimeTransform = (DateTimeOffset dto) => dto.UtcDateTime;

@VitaliyChaban
Copy link

VitaliyChaban commented Sep 20, 2024

I don’t know what might have broken due to these changes, but I can definitely say that WITHOUT these changes, it is impossible to work with OData on the new frameworks if time filtering is required.

I don't think that's a valid reason to reject this fix.

Expected: 2014-12-11T17:02:03.0000000
Actual: 2014-12-12T01:02:03.0000000Z

There is clearly a problem with the expected result, not with the actual one

If you're expecting local time here, you can pass it like this:

return DateTime.SpecifyKind(dateTimeOffsetValue, DateTimeKind.Local).ToUniversalTime(); or
return DateTime.SpecifyKind(dateTimeOffsetValue, DateTimeKind.Utc);
But the 'Z' should be at the end and sent to the database, because any other format leads to the aforementioned error.

I believe the issue lies in your tests, not in the proposed fixes.

There is a compromise option as suggested above—just pass a function in the settings that will modify the dateTime if needed.

At the moment, the only solution to the problem is to abandon the official NuGet package and use the fork from darkeagle76.

@VitaliyChaban
Copy link

I added a fix so that your tests pass, and I also added a new test that checks for the explicit indication of Kind=UTC.

Can we release this fix? At least as a preview version or something like that. We need this fix.

#1316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time zone settings not propagated to query Dates in $filter parsed as DateTime with DateTimeKind.Unspecified
6 participants