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.

Sign up for GitHub

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

Cosmos: Create shadow property for id if key cannot be used #35347

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Cosmos: Create shadow property for id if key cannot be used #35347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 14 additions & 27 deletions src/EFCore.Cosmos/Metadata/Conventions/CosmosJsonIdConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,10 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
var computedIdProperty = entityType.FindDeclaredProperty(DefaultIdPropertyName);

var primaryKey = entityType.FindPrimaryKey();
if (entityType.BaseType != null // Requires: IEntityTypeBaseTypeChangedConvention
|| !entityType.IsDocumentRoot() // Requires: IEntityTypeAnnotationChangedConvention (ContainerName)
|| entityType.GetForeignKeys()
.Any(fk => fk.IsOwnership) // Requires: IForeignKeyOwnershipChangedConvention, IForeignKeyRemovedConvention
|| primaryKey == null) // Requires: IKeyAddedConvention, IKeyRemovedConvention
if (entityType.BaseType != null
|| !entityType.IsDocumentRoot()
|| entityType.GetForeignKeys().Any(fk => fk.IsOwnership)
|| primaryKey == null)
{
// If the entity type is not a keyed, root document in the container, then it doesn't have an `id` mapping, so
// undo anything that was done by previous execution of this convention.
Expand All @@ -93,7 +92,8 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
if (computedIdProperty is not null
&& computedIdProperty != jsonIdProperty)
{
entityType.Builder.RemoveUnusedImplicitProperties([computedIdProperty]); }
entityType.Builder.RemoveUnusedImplicitProperties([computedIdProperty]);
}

return;
}
Expand All @@ -102,17 +102,9 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
// key is represented by a single string property, and the discriminator is not being included in the JSON `id`.
// If these conditions are not met, or if the user has opted-in, then we will create a computed property that transforms
// the appropriate values into a single string for the JSON `id` property.

// The line below requires: IModelAnnotationChangedConvention, IPropertyAnnotationChangedConvention
var alwaysCreateId = entityType.GetHasShadowId();
if (alwaysCreateId != true)
{
// The line below requires:
// - IModelAnnotationChangedConvention, IPropertyAnnotationChangedConvention
// - IKeyAddedConvention, IKeyRemovedConvention
// - IPropertyAddedConvention, IPropertyRemovedConvention
// - IDiscriminatorPropertySetConvention
// - IEntityTypeBaseTypeChangedConvention
var idDefinition = DefinitionFactory.Create((IEntityType)entityType)!;
if (idDefinition is { IncludesDiscriminator: false, Properties.Count: 1 })
{
Expand All @@ -125,15 +117,16 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
?? mapping?.ClrType
?? keyProperty!.ClrType;

if (clrType == typeof(string))
if (clrType == typeof(string)
&& keyProperty.Builder.CanSetJsonProperty(IdPropertyJsonName))
{
// We are at the point where we are going to map the `id` directly to the PK.
// However, if a previous run of this convention create the computed property, then we need to remove that
// mapping since it is now not needed.
if (computedIdProperty != null)
if (computedIdProperty != null
&& entityType.Builder.HasNoProperty(computedIdProperty) == null)
{
computedIdProperty.Builder.ToJsonProperty(null);
entityType.Builder.HasNoProperty(computedIdProperty);
}

// If there was previously a different property mapped to `id`, but not one of our computed properties,
Expand All @@ -146,10 +139,7 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
}

// Finally, actually map the primary key directly to the JSON `id`.
if (keyProperty.GetJsonPropertyName() != IdPropertyJsonName)
{
keyProperty.Builder.ToJsonProperty(IdPropertyJsonName);
}
keyProperty.Builder.ToJsonProperty(IdPropertyJsonName);

return;
}
Expand Down Expand Up @@ -183,13 +173,10 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
return;
}

if (computedIdPropertyBuilder.Metadata.GetJsonPropertyName() != IdPropertyJsonName)
{
computedIdPropertyBuilder = computedIdPropertyBuilder.ToJsonProperty(IdPropertyJsonName)
?? computedIdPropertyBuilder;
}

// Don't chain, because each of these could return null if the property has been explicitly configured with some other value.
computedIdPropertyBuilder = computedIdPropertyBuilder.ToJsonProperty(IdPropertyJsonName)
?? computedIdPropertyBuilder;

computedIdPropertyBuilder = computedIdPropertyBuilder.IsRequired(true)
?? computedIdPropertyBuilder;

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Cosmos/Metadata/Conventions/CosmosPartitionKeyInPrimaryKeyConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private static void ProcessIdProperty(IConventionEntityTypeBuilder entityTypeBui
if (keyContainsPartitionProperties)
{
primaryKey.DeclaringEntityType.Builder.HasNoKey(primaryKey);
entityTypeBuilder.HasKey(primaryKeyProperties);
entityTypeBuilder.PrimaryKey(primaryKeyProperties);
}
}
}
Expand Down
35 changes: 29 additions & 6 deletions test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public virtual void Hierarchical_partition_key_is_added_to_the_alternate_key_if_
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();
modelBuilder.Entity<Customer>().HasKey(CosmosJsonIdConvention.DefaultIdPropertyName);

modelBuilder.Entity<Customer>()
Expand All @@ -257,6 +257,29 @@ public virtual void Hierarchical_partition_key_is_added_to_the_alternate_key_if_
entity.FindPrimaryKey()!.Properties.Select(p => p.Name));
}

[ConditionalFact]
public virtual void Id_property_created_if_key_not_mapped_to_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>()
.Property(c => c.Name)
.ToJsonProperty("Name");
modelBuilder.Entity<Customer>()
.Ignore(b => b.Details)
.Ignore(b => b.Orders)
.HasKey(c => c.Name);

var model = modelBuilder.FinalizeModel();

var entity = model.FindEntityType(typeof(Customer))!;

Assert.Equal(CosmosJsonIdConvention.IdPropertyJsonName,
entity.FindProperty(CosmosJsonIdConvention.DefaultIdPropertyName)!.GetJsonPropertyName());

Assert.Equal(1, entity.GetKeys().Count());
}

[ConditionalFact]
public virtual void No_id_property_created_if_another_property_mapped_to_id()
{
Expand Down Expand Up @@ -311,7 +334,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();
modelBuilder.Entity<Customer>().HasKey(CosmosJsonIdConvention.DefaultIdPropertyName);

modelBuilder.Entity<Customer>()
Expand All @@ -335,7 +358,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id_and_p
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();
modelBuilder.Entity<Customer>().HasKey(nameof(Customer.AlternateKey), CosmosJsonIdConvention.DefaultIdPropertyName);

modelBuilder.Entity<Customer>()
Expand All @@ -359,7 +382,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id_and_h
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();

modelBuilder.Entity<Customer>().HasKey(
nameof(Customer.AlternateKey),
Expand Down Expand Up @@ -404,7 +427,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id_and_h
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();

modelBuilder.Entity<Customer>().HasKey(
nameof(Customer.Title),
Expand Down Expand Up @@ -449,7 +472,7 @@ public virtual void Hierarchical_partition_key_is_added_to_the_alternate_key_if_
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Customer>().AlwaysHasShadowId();
modelBuilder.Entity<Customer>().HasShadowId();

modelBuilder.Entity<Customer>().HasKey(
nameof(Customer.Title),
Expand Down
64 changes: 55 additions & 9 deletions test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosTestModelBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,49 @@ namespace Microsoft.EntityFrameworkCore.ModelBuilding;

public static class CosmosTestModelBuilderExtensions
{
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasPartitionKey<TEntity, TProperty>(
public static ModelBuilderTest.TestModelBuilder HasShadowIds(
this ModelBuilderTest.TestModelBuilder builder,
bool? alwaysCreate = true)
{
if (builder is IInfrastructure<ModelBuilder> nonGenericBuilder)
{
nonGenericBuilder.Instance.HasShadowIds(alwaysCreate);
}

return builder;
}

public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasShadowId<TEntity>(
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
Expression<Func<TEntity, TProperty>> propertyExpression)
bool? alwaysCreate = true)
where TEntity : class
{
switch (builder)
{
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
genericBuilder.Instance.HasPartitionKey(propertyExpression);
genericBuilder.Instance.HasShadowId(alwaysCreate);
break;
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
var names = propertyExpression.GetMemberAccessList().Select(e => e.GetSimpleMemberName()).ToList();
nonGenericBuilder.Instance.HasPartitionKey(names.FirstOrDefault(), names.Count > 1 ? names.Skip(1).ToArray() : []);
nonGenericBuilder.Instance.HasShadowId(alwaysCreate);
break;
}

return builder;
}

public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> AlwaysHasShadowId<TEntity>(
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasPartitionKey<TEntity, TProperty>(
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
bool? alwaysCreate = true)
Expression<Func<TEntity, TProperty>> propertyExpression)
where TEntity : class
{
switch (builder)
{
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
genericBuilder.Instance.HasShadowId(alwaysCreate);
genericBuilder.Instance.HasPartitionKey(propertyExpression);
break;
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
nonGenericBuilder.Instance.HasShadowId(alwaysCreate);
var names = propertyExpression.GetMemberAccessList().Select(e => e.GetSimpleMemberName()).ToList();
nonGenericBuilder.Instance.HasPartitionKey(names.FirstOrDefault(), names.Count > 1 ? names.Skip(1).ToArray() : []);
break;
}

Expand All @@ -63,6 +75,40 @@ public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasPartitionKey<TE
return builder;
}

public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> ToContainer<TEntity>(
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
string name)
where TEntity : class
{
switch (builder)
{
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
genericBuilder.Instance.ToContainer(name);
break;
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
nonGenericBuilder.Instance.ToContainer(name);
break;
}

return builder;
}
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> UseETagConcurrency<TEntity>(
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder)
where TEntity : class
{
switch (builder)
{
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
genericBuilder.Instance.UseETagConcurrency();
break;
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
nonGenericBuilder.Instance.UseETagConcurrency();
break;
}

return builder;
}

public static ModelBuilderTest.TestPropertyBuilder<TProperty> ToJsonProperty<TProperty>(
this ModelBuilderTest.TestPropertyBuilder<TProperty> builder,
string name)
Expand Down