Skip to content

Commit

Permalink
Map directly to JSON id for types converted to string (#34564)
Browse files Browse the repository at this point in the history 
* Map directly to JSON id for types converted to string

Fixes #34554

I also needed to fix #34511 to make this work. This was happening because we were removing the by-convention property added by property discovery. We only need to remove it when it's the computed property as well, which is handled by the other case.

* Updates
  • Loading branch information
@ajcvickers
ajcvickers authored Aug 30, 2024
1 parent 750f432 commit f0965d6
Show file tree
Hide file tree
Showing 11 changed files with 390 additions and 172 deletions.
20 changes: 13 additions & 7 deletions src/EFCore.Cosmos/Metadata/Conventions/CosmosJsonIdConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,16 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
{
// 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.
if (jsonIdProperty != null)
if (jsonIdProperty is not null)
{
jsonIdProperty.Builder.ToJsonProperty(null);
entityType.Builder.HasNoProperty(jsonIdProperty);
entityType.Builder.RemoveUnusedImplicitProperties([jsonIdProperty]);
}

if (computedIdProperty != null
if (computedIdProperty is not null
&& computedIdProperty != jsonIdProperty)
{
entityType.Builder.HasNoProperty(computedIdProperty);
}
entityType.Builder.RemoveUnusedImplicitProperties([computedIdProperty]); }

return;
}
Expand All @@ -115,10 +114,17 @@ private void ProcessEntityType(IConventionEntityType entityType, IConventionCont
// - IDiscriminatorPropertySetConvention
// - IEntityTypeBaseTypeChangedConvention
var idDefinition = DefinitionFactory.Create((IEntityType)entityType)!;
var keyProperty = (IConventionProperty?)idDefinition.Properties.FirstOrDefault();
if (idDefinition is { IncludesDiscriminator: false, Properties.Count: 1 })
{
var clrType = keyProperty!.GetValueConverter()?.ProviderClrType ?? keyProperty.ClrType;
// If the property maps to a string in the JSON document, then we can use it directly, even if a value converter
// is applied. On the other hand, if it maps to a numeric or bool, then we need to duplicate this to preserve the
// non-string value for queries.
var keyProperty = (IConventionProperty)idDefinition.Properties.First();
var mapping = Dependencies.TypeMappingSource.FindMapping((IProperty)keyProperty);
var clrType = mapping?.Converter?.ProviderClrType
?? mapping?.ClrType
?? keyProperty!.ClrType;

if (clrType == typeof(string))
{
// We are at the point where we are going to map the `id` directly to the PK.
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,12 @@ public async Task Entities_with_null_PK_can_be_added_with_normal_use_of_DbContex

var entry = await context.AddAsync(item);

var id = entry.Property("__id").CurrentValue;
var id = entry.Property("Id").CurrentValue;

Assert.NotNull(item.Id);
Assert.NotNull(id);

Assert.Equal($"{item.Id}", id);
Assert.Equal(item.Id, id);
Assert.Equal(EntityState.Added, entry.State);
}

Expand Down
180 changes: 180 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/ModelBuilding/CosmosModelBuilderGenericTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,186 @@ public virtual void No_alternate_key_is_created_if_id_is_hierarchical_partition_
Assert.DoesNotContain(entity.GetKeys(), k => k != entity.FindPrimaryKey());
}

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

modelBuilder.Entity<SingleStringKey>();

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleStringKey))!;

Assert.Equal(
[nameof(SingleStringKey.Id)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleStringKey.Id), "$type", nameof(SingleStringKey.Name), nameof(SingleStringKey.P1),
nameof(SingleStringKey.P2), nameof(SingleStringKey.P3), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

[ConditionalFact] // Issue #34511
public virtual void Single_string_primary_key_with_single_partition_key_maps_to_JSON_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<SingleStringKey>().HasPartitionKey(e => e.P1);

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleStringKey))!;

Assert.Equal(
[nameof(SingleStringKey.Id), nameof(SingleStringKey.P1)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleStringKey.Id), nameof(SingleStringKey.P1), "$type", nameof(SingleStringKey.Name),
nameof(SingleStringKey.P2), nameof(SingleStringKey.P3), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

[ConditionalFact] // Issue #34511
public virtual void Single_string_primary_key_with_hierarchical_partition_key_maps_to_JSON_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<SingleStringKey>().HasPartitionKey(e => new { e.P1, e.P2, e.P3 });

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleStringKey))!;

Assert.Equal(
[nameof(SingleStringKey.Id), nameof(SingleStringKey.P1), nameof(SingleStringKey.P2), nameof(SingleStringKey.P3)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleStringKey.Id), nameof(SingleStringKey.P1), nameof(SingleStringKey.P2), nameof(SingleStringKey.P3),
"$type", nameof(SingleStringKey.Name), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

protected class SingleStringKey
{
public string Id { get; set; } = null!;
public string? Name { get; set; }
public string P1 { get; set; } = null!;
public string P2 { get; set; } = null!;
public string P3 { get; set; } = null!;
}

[ConditionalFact] // Issue #34554
public virtual void Single_GUID_primary_key_maps_to_JSON_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<SingleGuidKey>();

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleGuidKey))!;

Assert.Equal(
[nameof(SingleGuidKey.Id)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleGuidKey.Id), "$type", nameof(SingleGuidKey.Name), nameof(SingleGuidKey.P1),
nameof(SingleGuidKey.P2), nameof(SingleGuidKey.P3), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

[ConditionalFact] // Issue #34554
public virtual void Single_GUID_primary_key_with_single_partition_key_maps_to_JSON_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<SingleGuidKey>().HasPartitionKey(e => e.P1);

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleGuidKey))!;

Assert.Equal(
[nameof(SingleGuidKey.Id), nameof(SingleGuidKey.P1)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleGuidKey.Id), nameof(SingleGuidKey.P1), "$type", nameof(SingleGuidKey.Name),
nameof(SingleGuidKey.P2), nameof(SingleGuidKey.P3), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

[ConditionalFact] // Issue #34554
public virtual void Single_GUID_primary_key_with_hierarchical_partition_key_maps_to_JSON_id()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<SingleGuidKey>().HasPartitionKey(e => new { e.P1, e.P2, e.P3 });

var model = modelBuilder.FinalizeModel();

var entityType = model.FindEntityType(typeof(SingleGuidKey))!;

Assert.Equal(
[nameof(SingleGuidKey.Id), nameof(SingleGuidKey.P1), nameof(SingleGuidKey.P2), nameof(SingleGuidKey.P3)],
entityType.FindPrimaryKey()!.Properties.Select(p => p.Name));

Assert.Equal(
[
nameof(SingleGuidKey.Id), nameof(SingleGuidKey.P1), nameof(SingleGuidKey.P2), nameof(SingleGuidKey.P3),
"$type", nameof(SingleGuidKey.Name), "__jObject"
],
entityType.GetProperties().Select(p => p.Name));

Assert.Equal(1, entityType.GetKeys().Count());
Assert.Null(entityType.FindProperty("__id"));
Assert.Equal("id", entityType.FindProperty("Id")!.GetJsonPropertyName());
}

protected class SingleGuidKey
{
public Guid Id { get; set; }
public string? Name { get; set; }
public string P1 { get; set; } = null!;
public string P2 { get; set; } = null!;
public string P3 { get; set; } = null!;
}

protected override TestModelBuilder CreateModelBuilder(Action<ModelConfigurationBuilder>? configure = null)
=> new GenericTestModelBuilder(Fixture, configure);
}
Expand Down
Loading

0 comments on commit f0965d6

Please sign in to comment.