From bf874ba4bb94bb818baf95021c40002104bae9cf Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 6 Aug 2025 17:35:59 -0700 Subject: [PATCH] Handle complex collections in the model snapshot Fix value type complex properties in the model snapshot Add missing extension methods for ComplexCollectionTypePropertyBuilder Fixes #36494 Part of #31237 --- .../Design/CSharpSnapshotGenerator.cs | 14 +- .../Design/AnnotationCodeGenerator.cs | 46 ++++ ...rpMigrationsGeneratorTest.ModelSnapshot.cs | 197 +++++++++++++++--- 3 files changed, 225 insertions(+), 32 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 6eaa03404eb..9c178755547 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -503,7 +503,8 @@ protected virtual void GeneratePropertyAnnotations( GenerateFluentApiForPrecisionAndScale(property, stringBuilder); GenerateFluentApiForIsUnicode(property, stringBuilder); - if (!annotations.ContainsKey(RelationalAnnotationNames.ColumnType)) + if (!annotations.ContainsKey(RelationalAnnotationNames.ColumnType) + && !property.DeclaringType.IsMappedToJson()) { annotations[RelationalAnnotationNames.ColumnType] = new Annotation( RelationalAnnotationNames.ColumnType, @@ -568,7 +569,8 @@ protected virtual void GenerateComplexProperty( stringBuilder .AppendLine() .Append(builderName) - .Append($".ComplexProperty<{Code.Reference(Model.DefaultPropertyBagType)}>(") + .Append(complexProperty.IsCollection ? ".ComplexCollection(" : ".ComplexProperty(") + .Append($"typeof({Code.Reference(Model.DefaultPropertyBagType)}), ") .Append($"{Code.Literal(complexProperty.Name)}, {Code.Literal(complexType.Name)}, ") .Append(complexTypeBuilderName) .AppendLine(" =>"); @@ -579,7 +581,7 @@ protected virtual void GenerateComplexProperty( using (stringBuilder.Indent()) { - if (complexProperty.IsNullable != complexProperty.ClrType.IsNullableType()) + if (!complexProperty.IsNullable) { stringBuilder .AppendLine() @@ -680,8 +682,8 @@ protected virtual void GenerateComplexPropertyAnnotations( } var propertyAnnotations = Dependencies.AnnotationCodeGenerator - .FilterIgnoredAnnotations(property.GetAnnotations()) - .ToDictionary(a => a.Name, a => a); + .FilterIgnoredAnnotations(property.GetAnnotations()) + .ToDictionary(a => a.Name, a => a); var typeAnnotations = Dependencies.AnnotationCodeGenerator .FilterIgnoredAnnotations(property.ComplexType.GetAnnotations()) @@ -692,7 +694,7 @@ protected virtual void GenerateComplexPropertyAnnotations( inChainedCall: false, hasAnnotationMethodInfo: HasPropertyAnnotationMethodInfo); GenerateAnnotations( - propertyBuilderName, property, stringBuilder, typeAnnotations, + propertyBuilderName, property.ComplexType, stringBuilder, typeAnnotations, inChainedCall: false, hasAnnotationMethodInfo: HasTypeAnnotationMethodInfo); } diff --git a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs index 189b2ee6609..f69dae85b38 100644 --- a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs @@ -3,6 +3,7 @@ using System.ComponentModel.DataAnnotations; using System.Diagnostics.CodeAnalysis; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; #pragma warning disable EF1001 // Accessing annotation names (internal) @@ -253,6 +254,18 @@ public virtual IReadOnlyList GenerateFluentApiCalls( annotations.Remove(RelationalAnnotationNames.ContainerColumnType); } + if (annotations.TryGetValue(RelationalAnnotationNames.JsonPropertyName, out var jsonPropertyNameAnnotation) + && jsonPropertyNameAnnotation is { Value: string jsonPropertyName } + && entityType.IsOwned()) + { + methodCallCodeFragments.Add( + new MethodCallCodeFragment( + nameof(RelationalOwnedNavigationBuilderExtensions.HasJsonPropertyName), + jsonPropertyName)); + + annotations.Remove(RelationalAnnotationNames.JsonPropertyName); + } + methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(entityType, annotations, GenerateFluentApi)); return methodCallCodeFragments; @@ -265,6 +278,31 @@ public virtual IReadOnlyList GenerateFluentApiCalls( { var methodCallCodeFragments = new List(); + if (annotations.TryGetValue(RelationalAnnotationNames.ContainerColumnName, out var containerColumnNameAnnotation) + && containerColumnNameAnnotation is { Value: string containerColumnName }) + { + methodCallCodeFragments.Add( + new MethodCallCodeFragment( + nameof(RelationalComplexPropertyBuilderExtensions.ToJson), + containerColumnName)); + + annotations.Remove(RelationalAnnotationNames.ContainerColumnName); +#pragma warning disable CS0618 + annotations.Remove(RelationalAnnotationNames.ContainerColumnTypeMapping); +#pragma warning restore CS0618 + } + + if (annotations.TryGetValue(RelationalAnnotationNames.ContainerColumnType, out var containerColumnTypeAnnotation) + && containerColumnTypeAnnotation is { Value: string containerColumnType }) + { + methodCallCodeFragments.Add( + new MethodCallCodeFragment( + nameof(RelationalComplexPropertyBuilderExtensions.HasColumnType), + containerColumnType)); + + annotations.Remove(RelationalAnnotationNames.ContainerColumnType); + } + methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(complexType, annotations, GenerateFluentApi)); return methodCallCodeFragments; @@ -336,6 +374,10 @@ public virtual IReadOnlyList GenerateFluentApiCalls( : new MethodCallCodeFragment(nameof(RelationalPropertyBuilderExtensions.IsFixedLength), false)); } + GenerateSimpleFluentApiCall( + annotations, + RelationalAnnotationNames.JsonPropertyName, nameof(RelationalPropertyBuilderExtensions.HasJsonPropertyName), methodCallCodeFragments); + GenerateSimpleFluentApiCall( annotations, RelationalAnnotationNames.Comment, nameof(RelationalPropertyBuilderExtensions.HasComment), methodCallCodeFragments); @@ -356,6 +398,10 @@ public virtual IReadOnlyList GenerateFluentApiCalls( { var methodCallCodeFragments = new List(); + GenerateSimpleFluentApiCall( + annotations, + RelationalAnnotationNames.JsonPropertyName, nameof(RelationalComplexPropertyBuilderExtensions.HasJsonPropertyName), methodCallCodeFragments); + methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(complexProperty, annotations, GenerateFluentApi)); return methodCallCodeFragments; diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs index f0f4f0ead36..4d8eebc8134 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs @@ -123,7 +123,7 @@ protected override void BuildModel(ModelBuilder modelBuilder) } [ConditionalFact] - public void Snapshot_with_default_values_are_round_tripped() + public void Snapshot_default_values_are_round_tripped() { var generator = CreateMigrationsCodeGenerator(); @@ -367,6 +367,8 @@ private class EntityWithTwoProperties public int Id { get; set; } public int AlternateId { get; set; } + [NotMapped] + public Coordinates Coordinates { get; set; } public EntityWithOneProperty EntityWithOneProperty { get; set; } [NotMapped] @@ -4352,12 +4354,10 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() { b.OwnsOne("Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithTwoProperties", "EntityWithTwoProperties", b1 => { - b1.Property("EntityWithOnePropertyId") - .HasColumnType("int"); + b1.Property("EntityWithOnePropertyId"); b1.Property("AlternateId") - .HasColumnType("int") - .HasAnnotation("Relational:JsonPropertyName", "NotKey"); + .HasJsonPropertyName("NotKey"); b1.HasKey("EntityWithOnePropertyId"); @@ -4370,8 +4370,7 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() b1.OwnsOne("Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithStringKey", "EntityWithStringKey", b2 => { - b2.Property("EntityWithTwoPropertiesEntityWithOnePropertyId") - .HasColumnType("int"); + b2.Property("EntityWithTwoPropertiesEntityWithOnePropertyId"); b2.HasKey("EntityWithTwoPropertiesEntityWithOnePropertyId"); @@ -4382,24 +4381,20 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() b2.OwnsMany("Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithStringProperty", "Properties", b3 => { - b3.Property("EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId") - .HasColumnType("int"); + b3.Property("EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId"); b3.Property("__synthesizedOrdinal") - .ValueGeneratedOnAdd() - .HasColumnType("int"); + .ValueGeneratedOnAdd(); - b3.Property("Id") - .HasColumnType("int"); + b3.Property("Id"); - b3.Property("Name") - .HasColumnType("nvarchar(max)"); + b3.Property("Name"); b3.HasKey("EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId", "__synthesizedOrdinal"); b3.ToTable("EntityWithOneProperty", "DefaultSchema"); - b3.HasAnnotation("Relational:JsonPropertyName", "JsonProps"); + b3.HasJsonPropertyName("JsonProps"); b3.WithOwner() .HasForeignKey("EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId"); @@ -5992,10 +5987,10 @@ public virtual void PrimitiveCollection_is_stored_in_snapshot() .HasColumnOrder(1) .HasComputedColumnSql("ListSql") .IsFixedLength() + .HasJsonPropertyName("ListJson") .HasComment("ListComment") .UseCollation("ListCollation") - .HasAnnotation("AnnotationName", "AnnotationValue") - .HasAnnotation("Relational:JsonPropertyName", "ListJson"); + .HasAnnotation("AnnotationName", "AnnotationValue"); SqlServerPrimitiveCollectionBuilderExtensions.IsSparse(b.PrimitiveCollection("List")); @@ -6029,11 +6024,15 @@ public virtual void Complex_properties_are_stored_in_snapshot() eb.PrimitiveCollection>("List") .HasColumnType("nvarchar(max)") .IsSparse(); + eb.ComplexProperty(e => e.Coordinates, cb => + { + cb.Property(c => c.Latitude).HasColumnName("Coordinate_X"); + cb.Property(c => c.Longitude).HasColumnName("Coordinate_Y"); + }); eb.ComplexProperty(e => e.EntityWithStringKey, cb => { cb.Ignore(e => e.Properties); - cb.Property(e => e.Id).IsRequired(); - cb.HasDiscriminator(); + cb.HasDiscriminator("Id"); }); eb.HasPropertyAnnotation("PropertyAnnotation", 1); eb.HasTypeAnnotation("TypeAnnotation", 2); @@ -6051,7 +6050,7 @@ public virtual void Complex_properties_are_stored_in_snapshot() SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); - b.ComplexProperty>("EntityWithTwoProperties", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties", b1 => + b.ComplexProperty(typeof(Dictionary), "EntityWithTwoProperties", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties", b1 => { b1.IsRequired(); @@ -6069,17 +6068,26 @@ public virtual void Complex_properties_are_stored_in_snapshot() SqlServerComplexTypePrimitiveCollectionBuilderExtensions.IsSparse(b1.PrimitiveCollection("List")); - b1.ComplexProperty>("EntityWithStringKey", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.EntityWithStringKey#EntityWithStringKey", b2 => + b1.ComplexProperty(typeof(Dictionary), "Coordinates", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.Coordinates#Coordinates", b2 => { - b2.Property("Discriminator") - .IsRequired() - .HasColumnType("nvarchar(max)"); + b2.IsRequired(); + b2.Property("Latitude") + .HasColumnType("decimal(18,2)") + .HasColumnName("Coordinate_X"); + + b2.Property("Longitude") + .HasColumnType("decimal(18,2)") + .HasColumnName("Coordinate_Y"); + }); + + b1.ComplexProperty(typeof(Dictionary), "EntityWithStringKey", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.EntityWithStringKey#EntityWithStringKey", b2 => + { b2.Property("Id") .IsRequired() .HasColumnType("nvarchar(max)"); - b2.HasDiscriminator().HasValue("EntityWithStringKey"); + b2.HasDiscriminator("Id").HasValue("EntityWithStringKey"); }); b1.HasPropertyAnnotation("PropertyAnnotation", 1); @@ -6111,6 +6119,21 @@ public virtual void Complex_properties_are_stored_in_snapshot() Assert.Equal(1, complexProperty["PropertyAnnotation"]); Assert.Equal(2, complexProperty.ComplexType["TypeAnnotation"]); + var coordinateComplexProperty = complexType.FindComplexProperty(nameof(EntityWithTwoProperties.Coordinates)); + Assert.False(coordinateComplexProperty.IsCollection); + Assert.False(coordinateComplexProperty.IsNullable); + var coordinateComplexType = coordinateComplexProperty.ComplexType; + Assert.Equal( + "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.Coordinates#Coordinates", + coordinateComplexType.Name); + Assert.Equal( + "EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.Coordinates#Coordinates", + coordinateComplexType.DisplayName()); + var coordinateXProperty = coordinateComplexType.FindProperty(nameof(Coordinates.Latitude)); + Assert.Equal("Coordinate_X", coordinateXProperty.GetColumnName()); + var coordinateYProperty = coordinateComplexType.FindProperty(nameof(Coordinates.Longitude)); + Assert.Equal("Coordinate_Y", coordinateYProperty.GetColumnName()); + var nestedComplexProperty = complexType.FindComplexProperty(nameof(EntityWithTwoProperties.EntityWithStringKey)); Assert.False(nestedComplexProperty.IsCollection); Assert.True(nestedComplexProperty.IsNullable); @@ -6127,6 +6150,128 @@ public virtual void Complex_properties_are_stored_in_snapshot() }, validate: true); + public readonly struct Coordinates(decimal latitude, decimal longitude) + { + public decimal Latitude { get; } = latitude; + public decimal Longitude { get; } = longitude; + } + + [ConditionalFact] + public virtual void Complex_types_mapped_to_json_are_stored_in_snapshot() + => Test( + builder => + { + builder.Entity( + b => + { + b.HasKey(x => x.Id).HasName("PK_Custom"); + + b.ComplexProperty( + x => x.EntityWithTwoProperties, bb => + { + bb.ToJson("TwoProps").HasColumnType("json"); + bb.Property(x => x.AlternateId).HasJsonPropertyName("NotKey"); + bb.ComplexProperty( + x => x.EntityWithStringKey, bbb => + { + bbb.ComplexCollection(x => x.Properties, bbbb => bbbb.HasJsonPropertyName("JsonProps")); + }); + bb.ComplexProperty(x => x.Coordinates, bbb => + { + bbb.Property(c => c.Latitude).HasJsonPropertyName("Lat"); + bbb.Property(c => c.Longitude).HasJsonPropertyName("Lon"); + }); + }); + }); + }, + AddBoilerPlate( + GetHeading() + + """ + modelBuilder.Entity("Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int"); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); + + b.ComplexProperty(typeof(Dictionary), "EntityWithTwoProperties", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties", b1 => + { + b1.Property("AlternateId") + .HasJsonPropertyName("NotKey"); + + b1.Property("Id"); + + b1.ComplexProperty(typeof(Dictionary), "Coordinates", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.Coordinates#Coordinates", b2 => + { + b2.IsRequired(); + + b2.Property("Latitude") + .HasJsonPropertyName("Lat"); + + b2.Property("Longitude") + .HasJsonPropertyName("Lon"); + }); + + b1.ComplexProperty(typeof(Dictionary), "EntityWithStringKey", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.EntityWithStringKey#EntityWithStringKey", b2 => + { + b2.Property("Id"); + + b2.ComplexCollection(typeof(Dictionary), "Properties", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.EntityWithStringKey#EntityWithStringKey.Properties#EntityWithStringProperty", b3 => + { + b3.Property("Id"); + + b3.Property("Name"); + + b3.HasJsonPropertyName("JsonProps"); + }); + }); + + b1 + .ToJson("TwoProps") + .HasColumnType("json"); + }); + + b.HasKey("Id") + .HasName("PK_Custom"); + + b.ToTable("EntityWithOneProperty", "DefaultSchema"); + }); +""", usingSystem: false, usingCollections: true), + o => + { + var entityWithOneProperty = o.FindEntityType(typeof(EntityWithOneProperty)); + Assert.Equal("PK_Custom", entityWithOneProperty.GetKeys().Single().GetName()); + + var complexProperty1 = entityWithOneProperty.FindComplexProperty(nameof(EntityWithOneProperty.EntityWithTwoProperties)); + Assert.False(complexProperty1.IsCollection); + Assert.True(complexProperty1.IsNullable); + var complexType1 = complexProperty1.ComplexType; + Assert.Equal("TwoProps", complexType1.GetContainerColumnName()); + Assert.Equal("json", complexType1.GetContainerColumnType()); + + var alternateIdProperty = complexType1.FindProperty(nameof(EntityWithTwoProperties.AlternateId)); + Assert.Equal("NotKey", alternateIdProperty.GetJsonPropertyName()); + + var coordinatesComplexProperty = complexType1.FindComplexProperty(nameof(EntityWithTwoProperties.Coordinates)); + Assert.False(coordinatesComplexProperty.IsCollection); + Assert.False(coordinatesComplexProperty.IsNullable); + var coordinatesComplexType = coordinatesComplexProperty.ComplexType; + var latitudeProperty = coordinatesComplexType.FindProperty(nameof(Coordinates.Latitude)); + Assert.Equal("Lat", latitudeProperty.GetJsonPropertyName()); + var longitudeProperty = coordinatesComplexType.FindProperty(nameof(Coordinates.Longitude)); + Assert.Equal("Lon", longitudeProperty.GetJsonPropertyName()); + + var entityWithStringKeyComplexProperty = complexType1.FindComplexProperty(nameof(EntityWithTwoProperties.EntityWithStringKey)); + Assert.False(entityWithStringKeyComplexProperty.IsCollection); + Assert.True(entityWithStringKeyComplexProperty.IsNullable); + var entityWithStringKeyComplexType = entityWithStringKeyComplexProperty.ComplexType; + + var propertiesComplexCollection = entityWithStringKeyComplexType.FindComplexProperty(nameof(EntityWithStringKey.Properties)); + Assert.True(propertiesComplexCollection.IsCollection); + Assert.Equal("JsonProps", propertiesComplexCollection.GetJsonPropertyName()); + }); + #endregion #region HasKey