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

Map directly to JSON id for types converted to string #34564

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Map directly to JSON id for types converted to string #34564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc76f7e
Map directly to JSON id for types converted to string
ajcvickers Aug 28, 2024
662b616
Updates
ajcvickers Aug 30, 2024
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
20 changes: 13 additions & 7 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 @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entityType.Builder.RemoveUnusedImplicitProperties([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
Open in desktop
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
Open in desktop
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