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

CosmosDb OwnsMany is broken when multiple child entities are saved without int Id values specified #33573

Closed
markphillips100 opened this issue Apr 19, 2024 · 14 comments

Comments

@markphillips100
Copy link

I created bug #32410 a while back which appeared to be resolved, or at least it was in the alpha nightly release I tested (9.0.0-alpha.1.23417.8).

Using the same sample I created for that issue I tested EFCore versions 8.0.2, 8.0.3, and 8.0.4 and all are not working as expected, well partly.

If only one child entity is saved in the parent's child collection without specifying the Id or the foreign key then all is well with the alpha version and all 3 of previously mentioned v8.0.x runtimes.

So this data persists fine and generates an int Id value in the backend for the only child:

{
  "familyId": "df22d34b-e2fa-49e6-b26f-3af9fd1ef23e",
  "lastName": "string",
  "children": [
    {
      "familyName": "string",
      "firstName": "string",
      "gender": "string"
    }
  ]
}

If however, 2 or more children are in the collection to be saved at the backend, only the alpha version works. All the other versions fail with the following exception:

System.InvalidOperationException: The instance of entity type 'Child' cannot be tracked because another instance with the same key value for {'FamilyId', 'Id'} is already being tracked. When replacing owned entities, modify the properties without changing the instance or detach the previous owned entity entry first. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.ThrowIdentityConflict(InternalEntityEntry entry)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry, Boolean updateDuplicate)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry)
   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, Boolean modifyProperties)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityStateAsync(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey, Nullable`1 fallbackState, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintActionAsync(EntityEntryGraphNode`1 node, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraphAsync[TState](EntityEntryGraphNode`1 node, Func`3 handleNode, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraphAsync[TState](EntityEntryGraphNode`1 node, Func`3 handleNode, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraphAsync(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.AddAsync[TEntity](TEntity entity, CancellationToken cancellationToken)
   at EFCoreCosmosSample.Infrastructure.Repositories.FamilyRepository.AddAsync(Family entity, CancellationToken cancellationToken) in D:\Dev\github\efcore-cosmos-sample\EFCoreCosmosSample.Infrastructure\Repositories\FamilyRepository.cs:line 28
   at EFCoreCosmosSample.Core.Services.FamilyService.AddFamily(Family family) in D:\Dev\github\efcore-cosmos-sample\EFCoreCosmosSample.Core\Services\FamilyService.cs:line 16
   at EFCoreCosmosSample.Api.Controllers.FamilyController.Add(Family family) in D:\Dev\github\efcore-cosmos-sample\EFCoreCosmosSample.Api\Controllers\FamilyController.cs:line 35
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

So it is not possible to save this document:

{
  "familyId": "df22d34b-e2fa-49e6-b26f-3af9fd1ef23e",
  "lastName": "string",
  "children": [
    {
      "familyName": "string",
      "firstName": "string",
      "gender": "string"
    },
    {
      "familyName": "string2",
      "firstName": "string2",
      "gender": "string2"
    }
  ]
}
@markphillips100
Copy link
Author

Apologies for taking so long to even try out the 8.0.2 release to ensure working. Diverted elsewhere for a while.

@markphillips100
Copy link
Author

Another observation is that the alpha does not persist the Id property at all in the backend (my expected behaviour). The 8.0.2/3/4 releases all persist the value 0 for the Id, hence I guess why they throw an exception due to all having 0 as their Id for all children that have none specified.

@markphillips100
Copy link
Author

Ah so looking at the PR code, and here it was lost on me that I would need to apply a switch to enable the functionality.

I applied AppContext.SetSwitch("Microsoft.EntityFrameworkCore.Issue31664", isEnabled: true); in the ConfigureServices and this seems to resolve the issue I was seeing.

I'll do some more testing (with 8.0.4) just to be sure.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Apr 23, 2024

@markphillips100 I'm unable to repro the behavior you are seeing. With the model in https://github.com/markphillips100/efcore-cosmos-sample/tree/dotnet8 and not setting any AppContext switches I'm able to save multiple children and Id is not persisted.

However if you are satisfied with the current behavior in 9.0.0-preview.3 and 8.0.4 with the AppContext switch set, then I'll consider this issue resolved.

@AndriySvyryd AndriySvyryd closed this as not planned Won't fix, can't repro, duplicate, stale Apr 23, 2024
@AndriySvyryd AndriySvyryd removed their assignment Apr 23, 2024
@markphillips100
Copy link
Author

@AndriySvyryd did you change the PackageReference versions in the repo before attepting to reproduce? It is currently set to use 8.0.0 so you must set to either 8.0.2, 8.0.3, or 8.0.4.

@markphillips100
Copy link
Author

@AndriySvyryd of course I am not happy with using the alpha release where the functionality worked and I never tried 9.0.0-preview.3 as you suggested.

@markphillips100
Copy link
Author

@ajcvickers @AndriySvyryd please reopen as this is not resolved.

@markphillips100
Copy link
Author

I've updated the sample to use 8.0.4 so just pull the repo and use branch dotnet8. This one will break if you post many children without specifying the child Id in each.

To get it working, uncomment line here.

@markphillips100
Copy link
Author

Basically I'd like to know if it is expected to include the switch to obtain the behaviour I require from 8.0.2 onwards. I'm okay with using it but just need the clarification.

@AndriySvyryd
Copy link
Member

I've updated the sample to use 8.0.4 so just pull the repo and use branch dotnet8. This one will break if you post many children without specifying the child Id in each.

Right, you added the Id property to Child, so it's no longer a shadow property in the model and hence doesn't get a value generated by default and is persisted.

@markphillips100
Copy link
Author

@AndriySvyryd right, I just noticed that. This was my bad as I brought it inline with my real use case which does have the int Id. Aspnet core's Identity has a type IdentityUserClaim<> that has this Id and along with the behaviour change that occurred from efcore v8, this is what caused the initial issue (and ended up deleting records from the database during updates).

So can you just clarify please that I must use the switch Microsoft.EntityFrameworkCore.Issue31664 when:

  1. I use a type that has an int Id on an owned entity collection, and
  2. I do not want the int id to be persisted (or exception due to no value assignment) in the nested document structure.

@AndriySvyryd
Copy link
Member

@markphillips100 To get the same behavior across all versions, instead of using the switch you can explicitly mark the Id property as generated on add and not-persisted.

        familyEntity
            .OwnsMany(f => f.Children, b =>
            {
                b.Property(c => c.Id).ValueGeneratedOnAdd().ToJsonProperty("");
                b.WithOwner()
                 .HasForeignKey(x => x.FamilyId);
            });

@markphillips100
Copy link
Author

Thank you. I shall give that a try. Ideally not using the switch would be best.

@markphillips100
Copy link
Author

@AndriySvyryd that works perfectly so thank you very much.

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

No branches or pull requests

4 participants