[Bug]: Attach with ValueGenerator composite key not working in case of Relations (FK)

[mseada94]

Setup

• Two entities (Book, Author) have one-to-many relation with a composite foreign key. (One Author Has Many Books)
• Single entity (Student) has no relations.
• One of the key properties is generated using custom ValueGenerator<Guid>. (TenantId)

Error and notes

• When trying to attach the book entity with the TenantId property unset, The value will be generated correctly but the entity will be attached as a new Added entity.
• This behavior is not the same when the entity has no relations like the Student entity in this sample
• When trying to attach the entity (student) with the TenantId property unset, The value will be generated correctly and the entity will be attached as Unchanged entity.

Code Sample (Console App) >> attach-demo branch
https://github.com/mohamed-seada-1994/EFCore-ValueGenerator-Bug-Sample/tree/attach-demo

Note: this sample use the workaround mentioned in #26332

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: NET 5.0
Operating system: Win10
IDE: Visual Studio 2019 16.11.5

[mseada94]

I test EF Core 6.0.0-rc.2.21480.5, and both cases wouldn't identify the entities, the state in both cases is Added.
I don't think this should be the right behavior.
If a key property was a value-generated shadow property, It will be impossible to use Attach or Update.

Code Sample (Console App) >> attach-demo-net6
https://github.com/mohamed-seada-1994/EFCore-ValueGenerator-Bug-Sample/tree/attach-demo-net6

[ajcvickers]

Note for triage: the fundamental question here is whether the generation of a stable value should be enough to consider the entity as new or not. For example, in the following code, the state of the attached entity is Added.

public class Book
{
    public Guid Id { get; set; }
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseSqlServer(Your.ConnectionString);

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Book>().Property(e => e.Id).HasValueGenerator<TenantIdGenerator>();
    }
}

public class TenantIdGenerator : ValueGenerator<Guid>
{
    public override Guid Next(EntityEntry entry) => Guid.Parse("98D06A82-C691-4988-EA39-08D98E2C8D8F");
    public override bool GeneratesTemporaryValues => false;
    public override bool GeneratesStableValues => true;
}

public class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            var book = new Book();

            context.Attach(book);

            Console.WriteLine($"State = {context.Entry(book).State}");
        }
    }
}

[mseada94]

I consider it a bug, as EF core 5 has different behaviour for two similar situations.

When I test it in EF Core 6 and both the entities had Added state, I am not sure if this was a fix or unintended side effect that needs to be fixed.

If it was intented, is there any way to attach the existing entities with a value generated key?

[ajcvickers]

@mohamed-seada-1994 If the key value is not set, then the entity is considered new and will get the Added state.

[mseada94]

@ajcvickers
For normal property, I could set the value of the key before calling attach or update.
But what about auto-generated shadow property as key?

The use case I want to cover involves a shadow property and updating an entity using update.
How could I set the value of the shadow property and get the entity attached correctly?
I expect:

• When calling update, the state should be Modified not Added.
• When calling attach, the state should be UnChanged not Added.

The problem I have with update is caused because EFCore is not able to attach the entity with unset or Shadow auto-generated value.

For non-shadow property, the entity is considered Added, and then the property value change as required by the value-generator.
I think the value should be generated before attempting to detect the entity state.

[eation]

@ajcvickers @mohamed-seada-1994
Maybe it's a bug.

InternalEntityEntry.cs#L1513-L1542 This HasDefaultValue method return true in final, but not everything has a default value.

InternalEntityEntry.cs#L198-L218 And this will return false when newState is Modified. It's false, so on this InternalEntityEntry.cs#L134-L137, it was not generated any value.

ValueGenerationManager.cs#L91-L117 On line 97, not everything has a default value, so it will be skip some ValueGenerator property, skip generated value.

[ajcvickers]

@eation All of that seems by-design, except possibly changing the handling of stable generated values.

@mohamed-seada-1994 If you have a shadow property that is part of a primary key, then it needs to be set explicitly to the correct value before attaching. This is one reason (of many) not to put PKs into shadow state. If you're seeing some behavior that involves a FK that is not also part of a PK, then please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

[ajcvickers]

Note from triage: generation of a stable value should not be enough to treat an entity as new.

[mseada94]

@ajcvickers
I Add User, UserClaim Entities where FK is simple and not part of the PK.

EfCore 5:
It works fine like Sample with no FK.
https://github.com/mohamed-seada-1994/EFCore-ValueGenerator-Bug-Sample/tree/attach-demo

EFCore 6:
All 3 samples consider entity new with Added State
https://github.com/mohamed-seada-1994/EFCore-ValueGenerator-Bug-Sample/tree/attach-demo-net6

[mseada94]

@ajcvickers Is there any workaround I could use to set/generate shadow property explicitly before update/attach?

[eation]

@mohamed-seada-1994 I was make a simple fix for #6999 on this fork, it work for this test ConcurrencyValueGeneratorTest current in efcore5,but need more test in efcore6, maybe u can test it.
ps: Attach(entity) has a bug, it not tracked any entity, ChangingCount or ChangedCount is 0, so u can replace with Add() or Update(). To set/generate a key property, need to set [DatabaseGenerated(DatabaseGeneratedOption.None)] dataannotations and then use custom ValueGenerator to avoid Identity Insert OFF error at this time.

@ajcvickers

[ajcvickers]

@mohamed-seada-1994 TenantId is part of the PK. It has a generated value. This means the state of the entity should be Added. EF Core 6.0 has the correct behavior here. Other than what has been discussed with stable values above, this is behaving as designed.

I should point out again that using value generators to generate primary key values that are not new (i.e. are already stored in the database) is not something EF Core was designed for. It's not something I have ever seen anyone do before this issue. Stable keys potentially allows for this, again as discussed above. The best workaround is to not used a value generator to set values that already exist in the database.

[mseada94]

@eation I think you solution is related to generate values on update before saving the changes, It's not going to work in case of autogenerated PK before attach, update.

---

@ajcvickers ValueGenerator is designed to work on Add not on Update.
Entity with Shadow PK Attach and Update behaviors were not considered on design, and should be avoided if possible.

The use case I have requires using Shadow Property as PK.

GeneratesStableValues option is a feature that allows for Shadow property to be used as primary key, this feature should support a stable solution to all possible operations not just adding such as (searching, updating) this added entity.
This require a way to set the value of the shadow property with 2 possible situations:

1. ValueGenerator generate a consistence value based on the current context (could be a scoped option or constant) so it could be used with search and update.
2. ValueGenerator generate a random value or sequence which require an explicit way to set the Shadow Key before search and update.

---

I found this workaround to set the value of the shadow property before Attach or Update, which work fine for EFCore 5 and EFCore 6.

var generatedTenantId = Guid.Parse("98D06A82-C691-4988-EA39-08D98E2C8D8F");

var entry = context.Entry(book);
entry.Property("TenantId").OriginalValue = generatedTenantId;
entry.Property("TenantId").CurrentValue = generatedTenantId;
entry.Property("TenantId").IsModified = false;
                
book.Title = "Modified!";
var attachedEntry = context.Books.Update(book); // attachedEntry.Status >> "Modified"
context.SaveChanges();

With this workaround I have to set the shadow property on before calling Update, Attach.
The shadow property value should be resolved from Request Context as a scoped option, Is there a way to enforce this behavior implicitly using ValueGenerator or custom DbContext Property without changing the caller code?