From 16cf1f561b652bf3fa5479e1f96eb5cfb8e1a292 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 19:20:16 +0000 Subject: [PATCH 1/4] Initial plan From 2bb3783d3d513edc95a228edfb4782f5310f7e1d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 19:45:49 +0000 Subject: [PATCH 2/4] Fix snapshot generation for complex collection properties Skip generating HasMaxLength and HasPrecision annotations for properties in complex collections since ComplexCollectionTypePropertyBuilder doesn't support these methods. Also skip IsConcurrencyToken and ValueGenerated* methods for the same reason. Fixes #37256 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Design/CSharpSnapshotGenerator.cs | 63 +++++++++++-- ...rpMigrationsGeneratorTest.ModelSnapshot.cs | 89 +++++++++++++++++++ 2 files changed, 145 insertions(+), 7 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index 04a6940a970..a3dd472f403 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -421,10 +421,24 @@ protected virtual void GenerateProperties( string entityTypeBuilderName, IEnumerable properties, IndentedStringBuilder stringBuilder) + => GenerateProperties(entityTypeBuilderName, properties, stringBuilder, isInComplexCollection: false); + + /// + /// Generates code for objects. + /// + /// The name of the builder variable. + /// The properties. + /// The builder code is added to. + /// Whether the properties are in a complex collection. + protected virtual void GenerateProperties( + string entityTypeBuilderName, + IEnumerable properties, + IndentedStringBuilder stringBuilder, + bool isInComplexCollection) { foreach (var property in properties) { - GenerateProperty(entityTypeBuilderName, property, stringBuilder); + GenerateProperty(entityTypeBuilderName, property, stringBuilder, isInComplexCollection); } } @@ -438,6 +452,20 @@ protected virtual void GenerateProperty( string entityTypeBuilderName, IProperty property, IndentedStringBuilder stringBuilder) + => GenerateProperty(entityTypeBuilderName, property, stringBuilder, isInComplexCollection: false); + + /// + /// Generates code for an . + /// + /// The name of the builder variable. + /// The property. + /// The builder code is added to. + /// Whether the property is in a complex collection. + protected virtual void GenerateProperty( + string entityTypeBuilderName, + IProperty property, + IndentedStringBuilder stringBuilder, + bool isInComplexCollection) { var clrType = (FindValueConverter(property)?.ProviderClrType ?? property.ClrType) .MakeNullable(property.IsNullable); @@ -452,7 +480,8 @@ protected virtual void GenerateProperty( // Note that GenerateAnnotations below does the corresponding decrement stringBuilder.IncrementIndent(); - if (property.IsConcurrencyToken) + // ComplexCollectionTypePropertyBuilder doesn't have IsConcurrencyToken method + if (!isInComplexCollection && property.IsConcurrencyToken) { stringBuilder .AppendLine() @@ -466,7 +495,8 @@ protected virtual void GenerateProperty( .Append(".IsRequired()"); } - if (property.ValueGenerated != ValueGenerated.Never) + // ComplexCollectionTypePropertyBuilder doesn't have ValueGenerated* methods + if (!isInComplexCollection && property.ValueGenerated != ValueGenerated.Never) { stringBuilder .AppendLine() @@ -480,7 +510,7 @@ protected virtual void GenerateProperty( : ".ValueGeneratedOnAddOrUpdate()"); } - GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder); + GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder, isInComplexCollection); } /// @@ -493,13 +523,32 @@ protected virtual void GeneratePropertyAnnotations( string propertyBuilderName, IProperty property, IndentedStringBuilder stringBuilder) + => GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder, isInComplexCollection: false); + + /// + /// Generates code for the annotations on an . + /// + /// The name of the builder variable. + /// The property. + /// The builder code is added to. + /// Whether the property is in a complex collection. + protected virtual void GeneratePropertyAnnotations( + string propertyBuilderName, + IProperty property, + IndentedStringBuilder stringBuilder, + bool isInComplexCollection) { var annotations = Dependencies.AnnotationCodeGenerator .FilterIgnoredAnnotations(property.GetAnnotations()) .ToDictionary(a => a.Name, a => a); - GenerateFluentApiForMaxLength(property, stringBuilder); - GenerateFluentApiForPrecisionAndScale(property, stringBuilder); + // ComplexCollectionTypePropertyBuilder doesn't have HasMaxLength or HasPrecision methods + if (!isInComplexCollection) + { + GenerateFluentApiForMaxLength(property, stringBuilder); + GenerateFluentApiForPrecisionAndScale(property, stringBuilder); + } + GenerateFluentApiForIsUnicode(property, stringBuilder); if (!annotations.ContainsKey(RelationalAnnotationNames.ColumnType) @@ -594,7 +643,7 @@ protected virtual void GenerateComplexProperty( .AppendLine(".IsRequired();"); } - GenerateProperties(complexTypeBuilderName, complexType.GetDeclaredProperties(), stringBuilder); + GenerateProperties(complexTypeBuilderName, complexType.GetDeclaredProperties(), stringBuilder, complexProperty.IsCollection); GenerateComplexProperties(complexTypeBuilderName, complexType.GetDeclaredComplexProperties(), stringBuilder); diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs index dade6edb6ff..304f80f67bb 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs @@ -6259,6 +6259,95 @@ public virtual void Complex_types_mapped_to_json_are_stored_in_snapshot() Assert.Equal(typeof(List>), propertiesComplexCollection.ClrType); }); + [ConditionalFact] + public virtual void Complex_collection_property_annotations_not_supported_by_builder_are_ignored_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.ComplexProperty( + x => x.EntityWithStringKey, bbb => + { + bbb.ComplexCollection(x => x.Properties, bbbb => + { + bbbb.HasJsonPropertyName("JsonProps"); + // Set MaxLength directly on the model to simulate convention behavior + // This should NOT appear in snapshot because ComplexCollectionTypePropertyBuilder + // doesn't support HasMaxLength + bbbb.Metadata.ComplexType.FindProperty("Name")!.SetMaxLength(100); + }); + }); + }); + }); + }, + 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"); + + b1.Property("Id"); + + b1.ComplexProperty(typeof(Dictionary), "EntityWithStringKey", "Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+EntityWithOneProperty.EntityWithTwoProperties#EntityWithTwoProperties.EntityWithStringKey#EntityWithStringKey", b2 => + { + b2.Property("Id"); + + b2.ComplexCollection(typeof(List>), "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)); + var complexProperty1 = entityWithOneProperty.FindComplexProperty(nameof(EntityWithOneProperty.EntityWithTwoProperties)); + var complexType1 = complexProperty1.ComplexType; + var entityWithStringKeyComplexProperty = + complexType1.FindComplexProperty(nameof(EntityWithTwoProperties.EntityWithStringKey)); + var entityWithStringKeyComplexType = entityWithStringKeyComplexProperty.ComplexType; + + var propertiesComplexCollection = + entityWithStringKeyComplexType.FindComplexProperty(nameof(EntityWithStringKey.Properties)); + Assert.True(propertiesComplexCollection.IsCollection); + + // MaxLength is NOT in the snapshot, so it won't be set on the model created from snapshot + // This verifies that the snapshot doesn't contain HasMaxLength which would cause a compile error + var nameProperty = propertiesComplexCollection.ComplexType.FindProperty("Name"); + Assert.Null(nameProperty.GetMaxLength()); + }); + #endregion #region HasKey From 2aa95cc83ec1b45c25001bb3394d369f8627e048 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 19:58:15 +0000 Subject: [PATCH 3/4] Add ModelDiffer test for complex collection property annotations Add test to verify that no operations are generated when the model contains annotations (like MaxLength) that are ignored in the snapshot for complex collection properties. Addresses review feedback from @AndriySvyryd Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/MigrationsModelDifferTest.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index 4553967aeb3..25e670313ca 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -9922,6 +9922,46 @@ public virtual void Noop_on_complex_properties() target => { }, Assert.Empty); + [ConditionalFact] + public void Noop_on_complex_collection_property_annotations_not_in_snapshot() + => Execute( + builder => + { + builder.Entity( + "Entity", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.HasKey("Id"); + + e.ComplexCollection, MyJsonComplex>( + "ComplexCollection", cp => + { + cp.ToJson(); + cp.Property(x => x.Value); + cp.Property(x => x.Date); + }); + }); + }, + source => + { + // Simulate convention setting MaxLength on string property in complex collection + // This annotation is NOT emitted in the snapshot because ComplexCollectionTypePropertyBuilder + // doesn't support HasMaxLength + var entity = source.Model.FindEntityType("Entity"); + var complexProperty = entity!.FindComplexProperty("ComplexCollection")!; + var valueProperty = complexProperty.ComplexType.FindProperty("Value")!; + ((IMutableProperty)valueProperty).SetMaxLength(255); + }, + target => + { + // Target model also has MaxLength set (same as source) + var entity = target.Model.FindEntityType("Entity"); + var complexProperty = entity!.FindComplexProperty("ComplexCollection")!; + var valueProperty = complexProperty.ComplexType.FindProperty("Value")!; + ((IMutableProperty)valueProperty).SetMaxLength(255); + }, + Assert.Empty); + protected class MyJsonComplex { public string Value { get; set; } From 877fe049a2ace9792f4265bf15a69ff7e7a9cc37 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Dec 2025 00:13:19 +0000 Subject: [PATCH 4/4] Refactor complex collection check to use property.DeclaringType - Remove method overloads with isInComplexCollection parameter - Check property.DeclaringType directly to determine if in complex collection - Update test to also configure IsConcurrencyToken, ValueGenerated, precision and scale - Empty target lambda in ModelDiffer test as snapshot doesn't have extra config Addresses review feedback from @AndriySvyryd Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Design/CSharpSnapshotGenerator.cs | 54 ++++--------------- ...rpMigrationsGeneratorTest.ModelSnapshot.cs | 14 +++-- .../Internal/MigrationsModelDifferTest.cs | 9 +--- 3 files changed, 20 insertions(+), 57 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index a3dd472f403..a933fe05781 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -421,24 +421,10 @@ protected virtual void GenerateProperties( string entityTypeBuilderName, IEnumerable properties, IndentedStringBuilder stringBuilder) - => GenerateProperties(entityTypeBuilderName, properties, stringBuilder, isInComplexCollection: false); - - /// - /// Generates code for objects. - /// - /// The name of the builder variable. - /// The properties. - /// The builder code is added to. - /// Whether the properties are in a complex collection. - protected virtual void GenerateProperties( - string entityTypeBuilderName, - IEnumerable properties, - IndentedStringBuilder stringBuilder, - bool isInComplexCollection) { foreach (var property in properties) { - GenerateProperty(entityTypeBuilderName, property, stringBuilder, isInComplexCollection); + GenerateProperty(entityTypeBuilderName, property, stringBuilder); } } @@ -452,20 +438,6 @@ protected virtual void GenerateProperty( string entityTypeBuilderName, IProperty property, IndentedStringBuilder stringBuilder) - => GenerateProperty(entityTypeBuilderName, property, stringBuilder, isInComplexCollection: false); - - /// - /// Generates code for an . - /// - /// The name of the builder variable. - /// The property. - /// The builder code is added to. - /// Whether the property is in a complex collection. - protected virtual void GenerateProperty( - string entityTypeBuilderName, - IProperty property, - IndentedStringBuilder stringBuilder, - bool isInComplexCollection) { var clrType = (FindValueConverter(property)?.ProviderClrType ?? property.ClrType) .MakeNullable(property.IsNullable); @@ -481,6 +453,9 @@ protected virtual void GenerateProperty( stringBuilder.IncrementIndent(); // ComplexCollectionTypePropertyBuilder doesn't have IsConcurrencyToken method + var isInComplexCollection = property.DeclaringType is IComplexType complexType + && complexType.ComplexProperty.IsCollection; + if (!isInComplexCollection && property.IsConcurrencyToken) { stringBuilder @@ -510,7 +485,7 @@ protected virtual void GenerateProperty( : ".ValueGeneratedOnAddOrUpdate()"); } - GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder, isInComplexCollection); + GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder); } /// @@ -523,26 +498,15 @@ protected virtual void GeneratePropertyAnnotations( string propertyBuilderName, IProperty property, IndentedStringBuilder stringBuilder) - => GeneratePropertyAnnotations(propertyBuilderName, property, stringBuilder, isInComplexCollection: false); - - /// - /// Generates code for the annotations on an . - /// - /// The name of the builder variable. - /// The property. - /// The builder code is added to. - /// Whether the property is in a complex collection. - protected virtual void GeneratePropertyAnnotations( - string propertyBuilderName, - IProperty property, - IndentedStringBuilder stringBuilder, - bool isInComplexCollection) { var annotations = Dependencies.AnnotationCodeGenerator .FilterIgnoredAnnotations(property.GetAnnotations()) .ToDictionary(a => a.Name, a => a); // ComplexCollectionTypePropertyBuilder doesn't have HasMaxLength or HasPrecision methods + var isInComplexCollection = property.DeclaringType is IComplexType complexType + && complexType.ComplexProperty.IsCollection; + if (!isInComplexCollection) { GenerateFluentApiForMaxLength(property, stringBuilder); @@ -643,7 +607,7 @@ protected virtual void GenerateComplexProperty( .AppendLine(".IsRequired();"); } - GenerateProperties(complexTypeBuilderName, complexType.GetDeclaredProperties(), stringBuilder, complexProperty.IsCollection); + GenerateProperties(complexTypeBuilderName, complexType.GetDeclaredProperties(), stringBuilder); GenerateComplexProperties(complexTypeBuilderName, complexType.GetDeclaredComplexProperties(), stringBuilder); diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs index 304f80f67bb..26af035e75d 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs @@ -6278,10 +6278,16 @@ public virtual void Complex_collection_property_annotations_not_supported_by_bui bbb.ComplexCollection(x => x.Properties, bbbb => { bbbb.HasJsonPropertyName("JsonProps"); - // Set MaxLength directly on the model to simulate convention behavior - // This should NOT appear in snapshot because ComplexCollectionTypePropertyBuilder - // doesn't support HasMaxLength - bbbb.Metadata.ComplexType.FindProperty("Name")!.SetMaxLength(100); + // Set annotations directly on the model to simulate convention behavior + // These should NOT appear in snapshot because ComplexCollectionTypePropertyBuilder + // doesn't support these methods + var complexType = bbbb.Metadata.ComplexType; + var nameProperty = (IMutableProperty)complexType.FindProperty("Name")!; + nameProperty.SetMaxLength(100); + nameProperty.SetPrecision(10); + nameProperty.SetScale(2); + nameProperty.IsConcurrencyToken = true; + nameProperty.ValueGenerated = ValueGenerated.OnAdd; }); }); }); diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index 25e670313ca..1472464bcc2 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -9952,14 +9952,7 @@ public void Noop_on_complex_collection_property_annotations_not_in_snapshot() var valueProperty = complexProperty.ComplexType.FindProperty("Value")!; ((IMutableProperty)valueProperty).SetMaxLength(255); }, - target => - { - // Target model also has MaxLength set (same as source) - var entity = target.Model.FindEntityType("Entity"); - var complexProperty = entity!.FindComplexProperty("ComplexCollection")!; - var valueProperty = complexProperty.ComplexType.FindProperty("Value")!; - ((IMutableProperty)valueProperty).SetMaxLength(255); - }, + target => { }, Assert.Empty); protected class MyJsonComplex