2

Question updated to reflect more detail and a simplified reproduction case as I have dug deeper with the help of comments.

I have a custom ValueGenerator as follows:

public class UtcDateTimeGenerator : ValueGenerator<DateTime>
{
    public override DateTime Next(EntityEntry entry)
    {
        // This method never seems to be called
        return DateTime.UtcNow;
    }

    protected override object NextValue(EntityEntry entry)
    {
        // This one is called.
        return DateTime.UtcNow;
    }

    public override bool GeneratesTemporaryValues => false;
}

My Entity:

public abstract class AggregateRoot<TId>
    where TId : IComparable
{
    public TId Id { get; set; }
}

public abstract class AuditedAggregateRoot<TId> : AggregateRoot<TId>
    where TId : IComparable
{
    public DateTime Created { get; internal set; }
    public DateTime LastModified { get; internal set; }
}

public class Player : AuditedAggregateRoot<Guid>
{
    public string ExternalId { get; internal set; }
    public string DisplayName { get; internal set; }
    public string Email { get; internal set; }
    public DateTime LastLogin { get; internal set; }

    /// <summary>
    /// Required for Entity Framework Core
    /// </summary>
    private Player()
    {
    }

    public Player([NotNull]string externalId, [NotNull]string displayName, [NotNull]string email)
    {
        ExternalId = externalId;
        DisplayName = displayName;
        Email = email;
    }
}

My entity's IEntityTypeConfiguration:

public abstract class AggregateRootTypeConfiguration<TEntity, TKey> : IEntityTypeConfiguration<TEntity>
    where TEntity : AggregateRoot<TKey>
    where TKey : IComparable
{
    public virtual void Configure(EntityTypeBuilder<TEntity> builder)
    {
        builder.HasKey(x => x.Id);
        builder.Property(x => x.Id)
            .HasValueGenerator<SequentialGuidValueGenerator>()
            .ValueGeneratedOnAdd()
            .IsRequired();
    }
}

public class AuditedAggregateRootTypeConfiguration<TEntity, TKey> : AggregateRootTypeConfiguration<TEntity, TKey>
    where TEntity : AuditedAggregateRoot<TKey>
    where TKey : IComparable
{
    public override void Configure(EntityTypeBuilder<TEntity> builder)
    {
        base.Configure(builder);
        builder.Property(x => x.Created)
            .HasValueGenerator<UtcDateTimeGenerator>()
            .ValueGeneratedOnAdd()
            .IsRequired();

        builder.Property(x => x.LastModified)
            .HasValueGenerator<UtcDateTimeGenerator>()
            .ValueGeneratedOnAddOrUpdate()
            .IsRequired();
    }
}

public class PlayerEntityTypeConfiguration : AuditedAggregateRootTypeConfiguration<Player, Guid>
{
    public override void Configure(EntityTypeBuilder<Player> builder)
    {
        base.Configure(builder);
        builder.Property(x => x.Email)
            .HasMaxLength(254)
            .IsRequired();

        builder.Property(x => x.ExternalId)
            .HasMaxLength(200)
            .IsRequired();

        builder.Property(x => x.DisplayName)
            .HasMaxLength(200)
            .IsRequired();

        builder.HasIndex(x => x.ExternalId).IsUnique();
        builder.HasIndex(x => x.Email).IsUnique();
        builder.HasIndex(x => x.DisplayName).IsUnique();
    }
}

DbContext:

public class MyDbContext : DbContext
{
    public EmpiresDbContext(DbContextOptions<EmpiresDbContext> dbContextOptions) : base(dbContextOptions)
    {
    }

    public DbSet<Player> Players { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.ApplyConfigurationsFromAssembly(typeof(EmpiresDbContext).Assembly);
        base.OnModelCreating(modelBuilder);
    }
    
    /*
     * Overrides below to fix EF Core not calling ValueGenerator's on update
     * See: https://github.com/dotnet/efcore/issues/19765#issuecomment-617679987
     */
    
    public override int SaveChanges(bool acceptAllChangesOnSuccess)
    {
        GenerateOnUpdate();
        return base.SaveChanges(acceptAllChangesOnSuccess);
    }

    public override Task<int> SaveChangesAsync(
        bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default)
    {
        GenerateOnUpdate();
        return base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);
    }

    private void GenerateOnUpdate()
    {
        foreach (var entityEntry in ChangeTracker.Entries())
        {
            foreach (var propertyEntry in entityEntry.Properties)
            {
                var property = propertyEntry.Metadata;
                var valueGeneratorFactory =
                    property.GetValueGeneratorFactory();
                var generatedOnUpdate = (property.ValueGenerated & ValueGenerated.OnUpdate)
                                        == ValueGenerated.OnUpdate;
                if (!generatedOnUpdate || valueGeneratorFactory == null)
                {
                    continue;
                }

                var valueGenerator = valueGeneratorFactory.Invoke(
                    property,
                    entityEntry.Metadata);
                propertyEntry.CurrentValue = valueGenerator.Next(entityEntry);
            }
        }
    }
}

I'm injecting MyDbContext into the service and then, in the CreatePlayer method:

var player = new Player(externalId, displayName, email);
_dbContext.Players.Add(player);
await _dbContext.SaveChangesAsync();

However, when await _dbContext.SaveChangesAsync(); is called, I get an exception:

SqlException: Cannot insert the value NULL into column 'LastModified'

I have checked with a break-point that the propertyEntry.CurrentValue = valueGenerator.Next(entityEntry); line in the Dbcontext is being hit and that it is assigning a correct value to the LastModified property.

With a SQL trace I can see the SQL that is generated is as follows:

exec sp_executesql N'SET NOCOUNT ON;
INSERT INTO [Players] ([Id], [Created], [DisplayName], [Email], [ExternalId], [LastLogin])
VALUES (@p0, @p1, @p2, @p3, @p4, @p5);
SELECT [LastModified]
FROM [Players]
WHERE @@ROWCOUNT = 1 AND [Id] = @p0;

',N'@p0 uniqueidentifier,@p1 datetime2(7),@p2 nvarchar(200),@p3 nvarchar(254),@p4 nvarchar(4000),@p5 datetime2(7)',@p0='C17E4EC8-CDD6-458A-8CEF-08D8C5D6F63A',@p1='2021-01-31 11:00:27.1068666',@p2=N'Kyr',@p3=N'test@test.test',@p4=N'my-iDp-id-removed-for-security',@p5='0001-01-01 00:00:00'

As you can see above, the LastModified column isn't even referenced.

Hades
  • 1,975
  • 1
  • 23
  • 39
  • Please [edit] your question to include the full source code you have as a [mcve], which can be compiled and tested by others. – Progman Jan 30 '21 at 18:17
  • Could you share the code from where you are calling `Insert()` and `SaveAsync()`? – atiyar Jan 31 '21 at 00:10
  • Some additional code added above as requested. – Hades Jan 31 '21 at 00:15
  • After some more digging and simplifications that have been suggested I've updated the question above to reflect the issue better. It's actually the "LastModified" property that's being reported as null, not "Created" (I misread the error) but I have things in place that should be generating that properties value (updated in question) – Hades Jan 31 '21 at 13:42
  • I have the same issue exactly, did you find a solution? – lonix Nov 12 '21 at 10:46
  • The simple answer is that EF doesn't use value generators on update... but nothing in the docs or their API makes this clear. I stopped using EF for projects and started using Dapper instead. EF just has too many hidden issues and niggles like this. With Dapper I can be in control of exactly what's happening and it allows for a better Onion Architecture/DDD solution setup too. – Hades Nov 12 '21 at 15:08

1 Answers1

0

Your ValueGenerator works perfectly fine with me.

To narrow down the cause of the issue, take a dependency of your DbContext directly in the service class, temporarily -

public class PlayerService : IPlayerService
{
    private EmpiresDbContext _dbContext;
    
    public PlayerService(EmpiresDbContext context)
    {
        _dbContext = context;
    }
}

and see if the following code works -

var player = new Player(externalId, displayName, email);
try
{
    _dbContext.Players.Add(player);
    await _dbContext.SaveChangesAsync();
}
catch(DbUpdateException ex)
{
    var message = ex.Message;
    throw;
}

Two suggestions (not directly related to your issue) -

  1. Remove all the .ConfigureAwait(false) from your code. Check this answer to see why.
  2. Do not use transaction for a single insert operation like this; it'll cost performance for no benefit.

EDIT :
Going through the code in your updated post, I see you are using the .ValueGeneratedOnAddOrUpdate() configuration with LastModified field, and as far as I can remember, this configuration has a known issue. You might even find issues posted on EF Core's GitHub repo on this.

I'd suggest to use the .ValueGeneratedOnAdd() configuration, as with Created field, and then handle the update scenarios manually.

atiyar
  • 7,762
  • 6
  • 34
  • 75
  • After some more digging and simplifications as you suggested I've updated the question above to reflect the issue better. It's actually the "LastModified" property that's being reported as null, not "Created" (I misread the error) but I have things in place that should be generating that properties value (updated in question) – Hades Jan 31 '21 at 13:42