Skip to content

Commit d9003e0

Browse files
AndriySvyrydAndriySvyryd
committed
Cleanup CosmosJsonIdConvention
Fixes #35325
1 parent b8ddc5b commit d9003e0

File tree

3 files changed

+98
-42
lines changed

3 files changed

+98
-42
lines changed

‎src/EFCore.Cosmos/Metadata/Conventions/CosmosJsonIdConvention.cs

+14-27
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,10 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
7676
var computedIdProperty = entityType.FindDeclaredProperty(DefaultIdPropertyName);
7777

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

9898
return;
9999
}
@@ -102,17 +102,9 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
102102
// key is represented by a single string property, and the discriminator is not being included in the JSON `id`.
103103
// If these conditions are not met, or if the user has opted-in, then we will create a computed property that transforms
104104
// the appropriate values into a single string for the JSON `id` property.
105-
106-
// The line below requires: IModelAnnotationChangedConvention, IPropertyAnnotationChangedConvention
107105
var alwaysCreateId = entityType.GetHasShadowId();
108106
if (alwaysCreateId != true)
109107
{
110-
// The line below requires:
111-
// - IModelAnnotationChangedConvention, IPropertyAnnotationChangedConvention
112-
// - IKeyAddedConvention, IKeyRemovedConvention
113-
// - IPropertyAddedConvention, IPropertyRemovedConvention
114-
// - IDiscriminatorPropertySetConvention
115-
// - IEntityTypeBaseTypeChangedConvention
116108
var idDefinition = DefinitionFactory.Create((IEntityType)entityType)!;
117109
if (idDefinition is { IncludesDiscriminator: false, Properties.Count: 1 })
118110
{
@@ -125,15 +117,16 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
125117
?? mapping?.ClrType
126118
?? keyProperty!.ClrType;
127119

128-
if (clrType == typeof(string))
120+
if (clrType == typeof(string)
121+
&& keyProperty.Builder.CanSetJsonProperty(IdPropertyJsonName))
129122
{
130123
// We are at the point where we are going to map the `id` directly to the PK.
131124
// However, if a previous run of this convention create the computed property, then we need to remove that
132125
// mapping since it is now not needed.
133-
if (computedIdProperty != null)
126+
if (computedIdProperty != null
127+
&& entityType.Builder.HasNoProperty(computedIdProperty) == null)
134128
{
135129
computedIdProperty.Builder.ToJsonProperty(null);
136-
entityType.Builder.HasNoProperty(computedIdProperty);
137130
}
138131

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

148141
// Finally, actually map the primary key directly to the JSON `id`.
149-
if (keyProperty.GetJsonPropertyName() != IdPropertyJsonName)
150-
{
151-
keyProperty.Builder.ToJsonProperty(IdPropertyJsonName);
152-
}
142+
keyProperty.Builder.ToJsonProperty(IdPropertyJsonName);
153143

154144
return;
155145
}
@@ -183,13 +173,10 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
183173
return;
184174
}
185175

186-
if (computedIdPropertyBuilder.Metadata.GetJsonPropertyName() != IdPropertyJsonName)
187-
{
188-
computedIdPropertyBuilder = computedIdPropertyBuilder.ToJsonProperty(IdPropertyJsonName)
189-
?? computedIdPropertyBuilder;
190-
}
191-
192176
// Don't chain, because each of these could return null if the property has been explicitly configured with some other value.
177+
computedIdPropertyBuilder = computedIdPropertyBuilder.ToJsonProperty(IdPropertyJsonName)
178+
?? computedIdPropertyBuilder;
179+
193180
computedIdPropertyBuilder = computedIdPropertyBuilder.IsRequired(true)
194181
?? computedIdPropertyBuilder;
195182

‎test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs

+29-6
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ public virtual void Hierarchical_partition_key_is_added_to_the_alternate_key_if_
230230
{
231231
var modelBuilder = CreateModelBuilder();
232232

233-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
233+
modelBuilder.Entity<Customer>().HasShadowId();
234234
modelBuilder.Entity<Customer>().HasKey(CosmosJsonIdConvention.DefaultIdPropertyName);
235235

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

260+
[ConditionalFact]
261+
public virtual void Id_property_created_if_key_not_mapped_to_id()
262+
{
263+
var modelBuilder = CreateModelBuilder();
264+
265+
modelBuilder.Entity<Customer>()
266+
.Property(c => c.Name)
267+
.ToJsonProperty("Name");
268+
modelBuilder.Entity<Customer>()
269+
.Ignore(b => b.Details)
270+
.Ignore(b => b.Orders)
271+
.HasKey(c => c.Name);
272+
273+
var model = modelBuilder.FinalizeModel();
274+
275+
var entity = model.FindEntityType(typeof(Customer))!;
276+
277+
Assert.Equal(CosmosJsonIdConvention.IdPropertyJsonName,
278+
entity.FindProperty(CosmosJsonIdConvention.DefaultIdPropertyName)!.GetJsonPropertyName());
279+
280+
Assert.Equal(1, entity.GetKeys().Count());
281+
}
282+
260283
[ConditionalFact]
261284
public virtual void No_id_property_created_if_another_property_mapped_to_id()
262285
{
@@ -311,7 +334,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id()
311334
{
312335
var modelBuilder = CreateModelBuilder();
313336

314-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
337+
modelBuilder.Entity<Customer>().HasShadowId();
315338
modelBuilder.Entity<Customer>().HasKey(CosmosJsonIdConvention.DefaultIdPropertyName);
316339

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

338-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
361+
modelBuilder.Entity<Customer>().HasShadowId();
339362
modelBuilder.Entity<Customer>().HasKey(nameof(Customer.AlternateKey), CosmosJsonIdConvention.DefaultIdPropertyName);
340363

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

362-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
385+
modelBuilder.Entity<Customer>().HasShadowId();
363386

364387
modelBuilder.Entity<Customer>().HasKey(
365388
nameof(Customer.AlternateKey),
@@ -404,7 +427,7 @@ public virtual void No_alternate_key_is_created_if_primary_key_contains_id_and_h
404427
{
405428
var modelBuilder = CreateModelBuilder();
406429

407-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
430+
modelBuilder.Entity<Customer>().HasShadowId();
408431

409432
modelBuilder.Entity<Customer>().HasKey(
410433
nameof(Customer.Title),
@@ -449,7 +472,7 @@ public virtual void Hierarchical_partition_key_is_added_to_the_alternate_key_if_
449472
{
450473
var modelBuilder = CreateModelBuilder();
451474

452-
modelBuilder.Entity<Customer>().AlwaysHasShadowId();
475+
modelBuilder.Entity<Customer>().HasShadowId();
453476

454477
modelBuilder.Entity<Customer>().HasKey(
455478
nameof(Customer.Title),

‎test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosTestModelBuilderExtensions.cs

+55-9
Original file line numberDiff line numberDiff line change
@@ -7,37 +7,49 @@ namespace Microsoft.EntityFrameworkCore.ModelBuilding;
77

88
public static class CosmosTestModelBuilderExtensions
99
{
10-
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasPartitionKey<TEntity, TProperty>(
10+
public static ModelBuilderTest.TestModelBuilder HasShadowIds(
11+
this ModelBuilderTest.TestModelBuilder builder,
12+
bool? alwaysCreate = true)
13+
{
14+
if (builder is IInfrastructure<ModelBuilder> nonGenericBuilder)
15+
{
16+
nonGenericBuilder.Instance.HasShadowIds(alwaysCreate);
17+
}
18+
19+
return builder;
20+
}
21+
22+
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasShadowId<TEntity>(
1123
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
12-
Expression<Func<TEntity, TProperty>> propertyExpression)
24+
bool? alwaysCreate = true)
1325
where TEntity : class
1426
{
1527
switch (builder)
1628
{
1729
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
18-
genericBuilder.Instance.HasPartitionKey(propertyExpression);
30+
genericBuilder.Instance.HasShadowId(alwaysCreate);
1931
break;
2032
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
21-
var names = propertyExpression.GetMemberAccessList().Select(e => e.GetSimpleMemberName()).ToList();
22-
nonGenericBuilder.Instance.HasPartitionKey(names.FirstOrDefault(), names.Count > 1 ? names.Skip(1).ToArray() : []);
33+
nonGenericBuilder.Instance.HasShadowId(alwaysCreate);
2334
break;
2435
}
2536

2637
return builder;
2738
}
2839

29-
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> AlwaysHasShadowId<TEntity>(
40+
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> HasPartitionKey<TEntity, TProperty>(
3041
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
31-
bool? alwaysCreate = true)
42+
Expression<Func<TEntity, TProperty>> propertyExpression)
3243
where TEntity : class
3344
{
3445
switch (builder)
3546
{
3647
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
37-
genericBuilder.Instance.HasShadowId(alwaysCreate);
48+
genericBuilder.Instance.HasPartitionKey(propertyExpression);
3849
break;
3950
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
40-
nonGenericBuilder.Instance.HasShadowId(alwaysCreate);
51+
var names = propertyExpression.GetMemberAccessList().Select(e => e.GetSimpleMemberName()).ToList();
52+
nonGenericBuilder.Instance.HasPartitionKey(names.FirstOrDefault(), names.Count > 1 ? names.Skip(1).ToArray() : []);
4153
break;
4254
}
4355

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

78+
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> ToContainer<TEntity>(
79+
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder,
80+
string name)
81+
where TEntity : class
82+
{
83+
switch (builder)
84+
{
85+
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
86+
genericBuilder.Instance.ToContainer(name);
87+
break;
88+
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
89+
nonGenericBuilder.Instance.ToContainer(name);
90+
break;
91+
}
92+
93+
return builder;
94+
}
95+
public static ModelBuilderTest.TestEntityTypeBuilder<TEntity> UseETagConcurrency<TEntity>(
96+
this ModelBuilderTest.TestEntityTypeBuilder<TEntity> builder)
97+
where TEntity : class
98+
{
99+
switch (builder)
100+
{
101+
case IInfrastructure<EntityTypeBuilder<TEntity>> genericBuilder:
102+
genericBuilder.Instance.UseETagConcurrency();
103+
break;
104+
case IInfrastructure<EntityTypeBuilder> nonGenericBuilder:
105+
nonGenericBuilder.Instance.UseETagConcurrency();
106+
break;
107+
}
108+
109+
return builder;
110+
}
111+
66112
public static ModelBuilderTest.TestPropertyBuilder<TProperty> ToJsonProperty<TProperty>(
67113
this ModelBuilderTest.TestPropertyBuilder<TProperty> builder,
68114
string name)

0 commit comments

Comments
 (0)