Azure Cosmos DB provider should default to implicit ownership
JeremyLikness opened this issue · 8 comments
The typical case for document databases is to store a root entity and serialize its children, including single and collection navigations, as sub-properties rather than separately tracked entities. The EF Core provider for Azure Cosmos DB currently has two issues with this convention. First, complex entities in the graph are assumed to be independently tracked and therefore modeling the default behavior requires explicitly configuring ownership. For larger graphs this has a negative impact on maintainability. Second, complex entities shouldn't have to be tracked independently. They should be considered serializable types owned by the root entity that are serialized directly.
As a solution, I'd like to see:
- When I configure an entity for Cosmos DB as DbSet any complex types or collections in the graph are assumed to be owned by the parent entity. Let me opt out by configuration but don't force me to opt in.
- Instead of forcing complex types to be tracked as separate entities, treat them the same way as primitives, i.e. serialize and deserialize directly instead of tracking separately. This way their properties are still indexed and queryable.
Example from Cosmos DB getting started app:
namespace todo
{
public class Family
{
[JsonPropertyName("id")]
public string Id { get; set; }
public string LastName { get; set; }
public IList<Parent> Parents { get; set; }
public IList<Child> Children { get; set; }
public Address Address { get; set; }
public bool IsRegistered { get; set; }
public override string ToString()
{
return JsonSerializer.Serialize(this);
}
}
public class Parent
{
public string FamilyName { get; set; }
public string FirstName { get; set; }
}
public class Child
{
public string FamilyName { get; set; }
public string FirstName { get; set; }
public string Gender { get; set; }
public int Grade { get; set; }
public IList<Pet> Pets { get; set; }
}
public class Pet
{
public string GivenName { get; set; }
}
public class Address
{
public string State { get; set; }
public string County { get; set; }
public string City { get; set; }
}
}Currently I have to do this configuration, whereas with the SDK v4 it "just works":
builder.Entity<Family>()
.HasPartitionKey(nameof(Family.LastName))
.OwnsMany(f => f.Parents);
builder.Entity<Family>()
.OwnsMany(f => f.Children)
.OwnsMany(c => c.Pets);
builder.Entity<Family>()
.OwnsOne(f => f.Address);OwnsMany is fine at one level but gets complicated when there are multiple nested complex types. Using a value converter makes me lose the ability to query on the summary. It would be great if we could just configure the Family as a DbSet and only have to configure the partition key.
Related - #6787 to allow configuring owned types without using deeply nested lambdas.
If I am reading the Merge correctly it only addresses issue 1.
Is Issue 2 being tracked separately?
Instead of forcing complex types to be tracked as separate entities, treat them the same way as primitives, i.e. serialize and deserialize directly instead of tracking separately. This way their properties are still indexed and queryable.
We are currently having to work around this by adding and serialising Guid keys on all child entities in complex types so that EF can track them properly so this second issue was of real interest to me and I would not like to see it get lost. Especially since it looked like it was going to get fixed in EFCore6.
@ACROV You are reading correctly. #17306 will allow value converters that output raw JSON, though it means that all complex types will become a black box for query.
You shouldn't need to create any keys for owned entities, EF Core creates shadow keys by convention that are not persisted, if there's an issue with them please file it separately.
@AndriySvyryd, @ajcvickers, @JeremyLikness - Apologies in advance for my limited EF Core for Cosmos understanding.. if you can reach me half way...
You shouldn't need to create any keys for owned entities, EF Core creates shadow keys by convention that are not persisted, if there's an issue with them please file it separately.
The problem i think we ran across is that we do not keep a persistent DB context when using Blazor WASM (Client only) app.
Using the DBContext - we get all the data needed.. and use it disconnected from the DBContext.. then if we need to update data, we new up a new DBContext() and use the Update() command to update the entity. This I believe is recommended way to do things in Blazor WASM client only (not keeping the DbContext alive).
So in this case.. should we be adding an "Id" to every owned class going forward?
Or do you think we should be doing below as the recommended way to update Entities with owned content nested?
var x= context.Add(this.SomeEntityWithNestedOwnedList);
x.State = EntityState.Modified;
@ajcvickers - in this post you mentioned one could add a private CLR property to the owned entity. #19856
The best way to deal with this is to add a CLR property for the key (which can be private) to the owned entity type. The key values are then preserved while the entities are not being tracked, and re-attaching the entities works correctly. We should document this.
can you expand on this - what does it look like? We tried but did not work for us.
Or do you think we should be doing below as the recommended way to update Entities with owned content nested?
If that works, then yes. In most cases EF will generate the values for the shadow keys on owned entity types. So, unless something is failing you don't need to preserve them.
I know it's a small thing but my brain kind of hurts... when needing to update an entity
to code "Add"
and then on another line mark it as "Modified".
Makes me feel like I'm going off course on a work round, and from readability standpoint reading Add feels wrong when really updating (even if that's what its doing under the covers).
Would it be possible to introduce a new method on Microsoft.EntityFrameworkCore.Cosmos SDK DB Context... and name that method UpdateDisconnectedEntity() or something like that?
Or add the "Add" and "Mark as Modified" somewhere in documentation for Microsoft.EntityFrameworkCore.Cosmos for BlazorWASM.
@JeremyLikness - thoughts? since you work with Blazor WASM.