Cosmos: id not populated on Attach

[TheFanatr]

When trying to delete an entity created using a different database context instance than the one currently in use, an exception is thrown.

Exception message: Unable to track an entity of type 'Item' because alternate key property 'id' is null. If the alternate key is not used in a relationship, then consider using a unique index instead. Unique indexes may contain nulls, while alternate keys must not.
Stack trace: Exception thrown: 'System.InvalidOperationException' in System.Private.CoreLib.dll
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.NullableKeyIdentityMap`1.Add(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState(InternalEntityEntry entry, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState[TEntity](TEntity entity, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.Attach[TEntity](TEntity entity)
   at EFCoreAzureCosmosDBExperiments.Program.TestCrossContextNonTrackedRDAsync() in C:\Users\alexf\Source\Repos\EFCoreAzureCosmosDBExperiments\EFCoreAzureCosmosDBExperiments\Program.cs:line 27
   at EFCoreAzureCosmosDBExperiments.Program.Main() in C:\Users\alexf\Source\Repos\EFCoreAzureCosmosDBExperiments\EFCoreAzureCosmosDBExperiments\Program.cs:line 17

Steps to reproduce

• Create a new console application running the latest public release of .NET Core 3.0, which, as of writing, is Preview 3, and replace the source code with the files contained in the following ZIP archive. The project file is also provided for reference.
  EFCoreAzureCosmosDBExperiments.zip
• Modify the CosmosContext class to connect to whatever instance or emulator of Azure Cosmos DB you have running.
• Run the program and follow the on-screen instructions to run examples where the exception is thrown, and where the program operates correctly.

Further technical details

EF Core version: 3.0.0-preview3.19153.1
Database Provider: Microsoft.EntityFrameworkCore.Cosmos
Operating system: Windows 10 1908 Build 17763.379
IDE: Visual Studio 2019 16.0.0

[TheFanatr]

Note that the comment on line 55 of Program.cs, stating that line 56 is needed to observe the incorrect behaviour, is innacurrate; it is leftover from when I was trying to fix the issue. The issue is still present without line 56.

[TheFanatr]

What's interesting is that it seems that the exception will occur even if just attaching the entity to the context.

[TheFanatr]

Just completed a sanity check against a demonstration from a blog post online, and the same issue arises. I should probably also ask if it is legitimately invalid to simply introduce a model instance out of the blue and attach it to a context with the intent of removing it.

[TheFanatr]

Apparently, the issue is that because the ID property I have on the model is a Guid, while the primary key for any document by default is a string approximately equal to $"{Descriminator}|{ID}", as far as I can tell, whenever a previously queried entity is attached to a new context, the process of inferring the primary key, serialized as id in the JSON for the document, is failing. Attempting to manually set this property via the use of shadow properties, seems to not be working because the entity needs to be attached first, which causes an exception.

[TheFanatr]

Same behaviour is experienced with latest nightly build.

[TheFanatr]

Apparently, if the entity is simply reloaded via using DbContext.Entry<TEntity>(...) to get an EntityEntry<TEntity> and calling ReloadAsync(), then it can be removed by the context instance. This should be made more obvious in documentation if it is intended behaviour.

[TheFanatr]

This is still an issue in the latest build from the dotnetfeed.blob.core.windows.net Entity Framework Core feed. The only mitigation, now that the Reload workaround stopped working, is to have a string property called id case sensitive. This is very frustrating.

[AndriySvyryd]

@TheFanatr Instead of calling Attach/Remove you'll need to get the entry and set the store id explicitly:

var itemEntry = context.Entry(createdItem);
itemEntry.Property<string>("id").CurrentValue = "Item|" + createdItem.ID;
itemEntry.State = EntityState.Deleted;

We'll investigate whether this value can be generated automatically for non-add scenarios in the future.

[TheFanatr]

@AndriySvyryd Thank you for providing this workaround, however I am still a little frustrated. The value used to be generated correctly for deletion as long as itemEntry.ReloadAsync() was called before the entity was marked for removal (using the .NET Core 3.0 Preview 5 release), which is no longer the case, and also the value is currently always generated correctly and deletion can occur without needing to directly use any of the EntityEntry<TEntity> APIs as long as there is a string property called id, but then the initial value needs to be generated manually as follows and the value is incorrectly stored to the database without the discriminator as seen in the image below.

public string id { get; set; } = Guid.NewGuid().ToString();

The incorrect storage can be fixed by concatenating the discriminator to the beginning of the string but then that causes other issues and inconveniences, and I have found no way to get any other property name and type combination to map to id in the JSON, even when using the ModelBuilder instance provided to OnModelCreating on the CosmosContext to set the property up using the ToJsonProperty method nor by manually adding the Cosmos:PropertyName annotation to the property, the property configured as a key, and the property configured as an alternate key.

It feels as though this functionality is not yet implemented, but no exceptions are being thrown. Am I using these features incorrectly?

I should also mention that I was aware of the provided workaround beforehand but considered it to be inadequate as I believed I was simply using the APIs incorrectly, but if it is currently the most appropriate solution then so be it.

[AndriySvyryd]

@TheFanatr I'll also investigate why Reload stopped working.
The id property is in shadow state by default, meaning its value is stored in the context and is lost when you attach the entity to a different context, by adding and id property to the CLR type you are providing a place to store the value between contexts. Currently the id name is hardcoded, but we'll consider adding ways of mapping it to a different property after 3.0

What problems did you run into when setting id to the Guid value without discriminator? As long as you do it consistently it should also work, Cosmos just needs it to be unique across the collection.

[AndriySvyryd]

Related: #16186

[AndriySvyryd]

For Attach to work we need to generate key values: #13575

[TheFanatr]

I have a fix implemented on a local copy of the master branch via inheriting StateManager from a class and overriding StateChanging to include a call to ValueGenerationManager.Generate(entry) and adding the aforementioned derived class's type to the internal service collection via modifying the CosmosServiceCollectionExtensions.AddEntityFrameworkCosmos to include a EntityFrameworkServicesBuilder.TryAdd call that replaces the default IStateManager service.

[TheFanatr]

This is messy because it does not account for other values that are generated, and more specifically, making sure that those generators don't run and replace existing data. But as for generating the shadow properties required for the Cosmos DB provider to work correctly, this suffices for me as it also fixes the problem of Reload not working.

[TheFanatr]

I've never touched the source to Entity Framework Core in my life so this took some long hours (15+) of debugging; maybe it should be more clear somewhere that this provider is not to be used in it's current state. To further back that up, I've now found that loading reference navigation properties does not work and will post an issue soon enough.

[AndriySvyryd]

@TheFanatr Reload should be working in the nightly builds. The current limitations of the Cosmos provider are listed on this page

[TheFanatr]

I did try with the nightly builds on the 30th, and it did not work. I set up a reproduction project and just debugged it myself with the Entity Framework Core source from master. I can post it if you would like.

[TheFanatr]

The failure mode was an exception saying that the model was not finalized, but I know that it was and even finalized it manually to see if it could be fixed that way.

[AndriySvyryd]

@TheFanatr If Reload doesn't work then please create a new issue with a small repro project that uses an officially built nightly package

[TheFanatr]

Ok, sounds good. Just tested at 1:38 PM with latest master alpha release 5.0.0-alpha1.19551.4. I'll post the issue soon.