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

EF core 5.0 doesn't detect change to DateTimeOffset's offset #24567

Closed
clement911 opened this issue Apr 2, 2021 · 6 comments · Fixed by #24606
Closed

EF core 5.0 doesn't detect change to DateTimeOffset's offset #24567

clement911 opened this issue Apr 2, 2021 · 6 comments · Fixed by #24606
Assignees
Labels
area-type-mapping closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@clement911
Copy link
Contributor

I see the following bug with EF 5.0

var entity = dbCtx.Find<MyEntity>(123);//Load from DB context
entity.MyDateTimeOffset = entity.MyDateTimeOffset.ToOffset(entity.MyDateTimeOffset.Add(TimeSpan.FromHours(2)));//Switch the offset by 2 hours
ctx.Entry(order).State; //Return Unchanged but expecting Modified

I believe this happens because IEquatable<DateTimeOffset> considers that two DateTimeOffsets are equal if they represent the same absolute time.
However, from a DB perspective, the code above should detect the change and persist the new offset in the column (which is of type datetimeoffset in Sql Azure)

@dengere
Copy link

dengere commented Apr 4, 2021

I can't reproduce it,
it could be something missed. entity.MyDateTimeOffset.Add(TimeSpan.FromHours(2)) produce new DateTimeOffset object. ToOffset acccept only TimeSpan, and it can not convert from DateTimeOffset to TimeSpan.

@clement911
Copy link
Contributor Author

Here is a more comprehensive repro, using the InMemoryDatabase provider.

Program.cs

using Microsoft.EntityFrameworkCore;
using System;
using System.ComponentModel.DataAnnotations;

namespace ReproEFOffset
{
    public class MyEntity
    {
        [Key]
        public int Id { get; set; }

        public DateTimeOffset UpdatedAt { get; set; }
    }

    public class MyContext : DbContext
    {
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);
            modelBuilder.Entity<MyEntity>().HasKey(i => i.Id);
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            base.OnConfiguring(optionsBuilder);
            optionsBuilder.UseInMemoryDatabase("MyDb");
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var ctx = new MyContext();
            var entity = new MyEntity { Id = 1, UpdatedAt = DateTimeOffset.UtcNow };
            ctx.Add(entity);
            ctx.SaveChanges();

            entity.UpdatedAt = entity.UpdatedAt.ToOffset(TimeSpan.FromHours(-6));
            var entityState = ctx.Entry(entity).State;
            Console.WriteLine(entityState);//Displays Unchanged but expecting Modified
        }
    }
}

.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="5.0.4" />
  </ItemGroup>

</Project>

@roji roji added the type-bug label Apr 6, 2021
@roji roji self-assigned this Apr 6, 2021
@roji
Copy link
Member

roji commented Apr 6, 2021

I can confirm this repros - and as @clement911 wrote this is due to the equality behavior .NET defines for DateTimeOffset, which doesn't take timezone differences into account.

@clement911
Copy link
Contributor Author

Thanks @roji , can this be prioritized for the next release?

@ajcvickers ajcvickers added this to the 6.0.0 milestone Apr 6, 2021
roji added a commit that referenced this issue Apr 7, 2021
roji added a commit that referenced this issue Apr 7, 2021
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 7, 2021
@roji
Copy link
Member

roji commented Apr 7, 2021

@clement911 I've submitted a fix for this. In the meantime you can set up your own value comparer with the right behavior as described in the docs:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Blog>().Property(b => b.DateTimeOffset)
        .Metadata
        .SetValueComparer(new ValueComparer<DateTimeOffset>(
            (dto1, dto2) => dto1 == dto2 && dto1.Offset == dto2.Offset,
            dto => dto.GetHashCode()));
}

@clement911
Copy link
Contributor Author

Awesome. Thanks for the quick fix!

roji added a commit that referenced this issue Apr 8, 2021
@ghost ghost closed this as completed in #24606 Apr 8, 2021
ghost pushed a commit that referenced this issue Apr 8, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview4 Apr 26, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview4, 6.0.0 Nov 8, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-type-mapping closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants