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

DateTime in filter expression gets parsed as local DateTime when using "Z" designator #1528

Closed
max-maurice opened this issue Apr 10, 2024 · 3 comments
Labels

Comments

@max-maurice
Copy link

DESCRIPTION

A filter expression containing a DateTime with "Z" instead of "+00:00" gets parsed as local time instead of UTC.

STEPS TO REPRODUCE

Using a model with a datetime property like this:

public partial class Item : IIdentifiable<Guid>
{
  // ...

  [Column(TypeName = "datetime")]
  [Attr(PublicName = "changedAt")]
  public DateTime? ChangedAt { get; set; }
}

Create test data:

item.ChangedAt = new DateTime(2024, 4, 8, 10, 0, 5, DateTimeKind.Local); // Local is CEST, which is +2h on April, 8th
  1. Send a request to /items?filter=equals(changedAt,'2024-04-08T08:00:05Z')
  2. JADNC returns no results (wrong)
  3. Send a request to /items?filter=equals(changedAt,'2024-04-08T08:00:05+00:00')
  4. JADNC returns 1 result (correct)
  5. Send a request to /items?filter=equals(changedAt,'2024-04-08T10:00:05+02:00')
  6. JADNC returns 1 result (correct)

Using DateTimeOffset instead of DateTime or specifying DateTimeKind.Utc instead of Local changes the behavior, but the results are still inconsistent.

EXPECTED BEHAVIOR

The "Z" zone designator should be treated like a "+00:00" offset.

ACTUAL BEHAVIOR

The "Z" designator seems to be ignored and the datetime treated as local time.

VERSIONS USED

  • JsonApiDotNetCore version: 5.5.1
  • ASP.NET Core version: 7.0.15
  • Entity Framework Core version: 7.0.15
  • Database provider: SqlServer
@bkoelman
Copy link
Member

bkoelman commented Apr 11, 2024

From what I can tell, JsonApiDotNetCore parses the "Z" designator correctly:

image

It's then up to the database driver to convert that to SQL. And that's where it gets tricky.

For example, Npgsql took a breaking change in 2020 and decided to fail on any DateTime or DateTimeOffset that isn't UTC because of all the non-intuitive behavior during conversions. We have several integration tests (1, 2, 3, 4) to verify that a local or UTC DateTime roundtrips properly, with PostgreSQL at least.

For SQL Server, there's a workaround described here, that deals with the fact that the underlying column type is unable to store whether the value is local or UTC. The consensus from the EF Core team is described at dotnet/efcore#4711 (comment), which comes down to: when using SQL Server, use DateTimeOffset instead of DateTime.

Would using DateTimeOffset (with the appropriate DATETIMEOFFSET underlying column type) work for you?

@bkoelman
Copy link
Member

bkoelman commented Apr 11, 2024

In case you're interested, there's a fun video at https://www.youtube.com/watch?v=ZLJLfImuFqM&list=PLdo4fOcmZ0oX-DBuRG4u58ZTAJgBAeQ-t discussing dates, offsets, and time zones. It has some interesting details, for example: did you know that DateTimeKind actually has four values internally?

@max-maurice
Copy link
Author

Thanks Bart, you're right! It's not JsonApiDotNetCore, but my implementation of a ValueConverter. I had

public class LocalDateTimeConverter : ValueConverter<DateTime, DateTime>
{
  public LocalDateTimeConverter()
      : base(v => v /* <- WRONG! */, v => DateTime.SpecifyKind(v, DateTimeKind.Local))
  {
  }
}

where the first ctor parameter was missing the conversion .ToLocalTime() and should read v => v.ToLocalTime().

Sorry for the troubles!

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

No branches or pull requests

2 participants