Code review changes

dotnet · Aug 6, 2023 · f765332 · f765332
1 parent 8b69f8d
commit f765332
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 12 deletions.
diff --git a/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs b/src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs
@@ -278,9 +278,9 @@ private static void TryUniquifyColumnNames(
             }
         }
 
-        foreach (var complexPropery in type.GetDeclaredComplexProperties())
+        foreach (var complexProperty in type.GetDeclaredComplexProperties())
         {
-            TryUniquifyColumnNames(complexPropery.ComplexType, columns, storeObject, maxLength);
+            TryUniquifyColumnNames(complexProperty.ComplexType, columns, storeObject, maxLength);
         }
     }
 

diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs
@@ -1164,6 +1164,28 @@ protected virtual void ValidateData(IModel model, IDiagnosticsLogger<DbLoggerCat
                     }
                 }
 
+                foreach (var complexProperty in entityType.GetComplexProperties())
+                {
+                    if (seedDatum.TryGetValue(complexProperty.Name, out var value)
+                        && ((complexProperty.IsCollection && value is IEnumerable collection && collection.Any())
+                            || (!complexProperty.IsCollection && value != complexProperty.Sentinel)))
+                    {
+                        if (sensitiveDataLogged)
+                        {
+                            throw new InvalidOperationException(
+                                CoreStrings.SeedDatumComplexPropertySensitive(
+                                    entityType.DisplayName(),
+                                    string.Join(", ", key.Properties.Select((p, i) => p.Name + ":" + keyValues[i])),
+                                    complexProperty.Name));
+                        }
+
+                        throw new InvalidOperationException(
+                            CoreStrings.SeedDatumComplexProperty(
+                                entityType.DisplayName(),
+                                complexProperty.Name));
+                    }
+                }
+
                 if (identityMap == null)
                 {
                     if (!identityMaps.TryGetValue(key, out identityMap))

diff --git a/src/EFCore/Metadata/Internal/EntityType.cs b/src/EFCore/Metadata/Internal/EntityType.cs
@@ -2701,6 +2701,7 @@ public virtual IEnumerable<Trigger> GetDeclaredTriggers()
             propertiesList ??= GetProperties()
                 .Concat<IPropertyBase>(GetNavigations())
                 .Concat(GetSkipNavigations())
+                .Concat(GetComplexProperties())
                 .ToList();
             if (ClrType.IsAssignableFrom(type))
             {

diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs
diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx
@@ -283,16 +283,16 @@
     <value>The collection complex property '{property}' cannot be added to the type '{type}' because its CLR type '{clrType}' does not implement 'IEnumerable&lt;{targetType}&gt;'. Collection complex property must implement IEnumerable&lt;&gt; of the complex type.</value>
   </data>
   <data name="ComplexPropertyCollection" xml:space="preserve">
-    <value>Adding the collection complex property '{type}.{property}' isn't supported.</value>
+    <value>Adding the collection complex property '{type}.{property}' isn't supported. See https://github.com/dotnet/efcore/issues/31237 for more information.</value>
   </data>
   <data name="ComplexPropertyIndexer" xml:space="preserve">
-    <value>Adding the complex property '{type}.{property}' as an indexer property isn't supported.</value>
+    <value>Adding the complex property '{type}.{property}' as an indexer property isn't supported. See https://github.com/dotnet/efcore/issues/31244 for more information.</value>
   </data>
   <data name="ComplexPropertyOptional" xml:space="preserve">
-    <value>Configuring the complex property '{type}.{property}' as optional is not supported, call 'IsRequired()'.</value>
+    <value>Configuring the complex property '{type}.{property}' as optional is not supported, call 'IsRequired()'. See https://github.com/dotnet/efcore/issues/31376 for more information.</value>
   </data>
   <data name="ComplexPropertyShadow" xml:space="preserve">
-    <value>Configuring the complex property '{type}.{property}' in shadow state isn't supported.</value>
+    <value>Configuring the complex property '{type}.{property}' in shadow state isn't supported.  See https://github.com/dotnet/efcore/issues/31243 for more information.</value>
   </data>
   <data name="ComplexPropertyWrongClrType" xml:space="preserve">
     <value>The complex property '{property}' cannot be added to the type '{type}' because its CLR type '{clrType}' does not match the expected CLR type '{targetType}'.</value>
@@ -1398,6 +1398,12 @@
   <data name="SavepointsNotSupported" xml:space="preserve">
     <value>Savepoints are not supported by the database provider in use.</value>
   </data>
+  <data name="SeedDatumComplexProperty" xml:space="preserve">
+    <value>The seed entity for entity type '{entityType}' cannot be added because it has the complex property '{property}' set. Complex properties are currently not supported in seeding. See https://github.com/dotnet/efcore/issues/31254 for more information. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the involved property values.</value>
+  </data>
+  <data name="SeedDatumComplexPropertySensitive" xml:space="preserve">
+    <value>The seed entity for entity type '{entityType}' with the key value '{keyValue}' cannot be added because it has the complex property '{property}' set. Complex properties are currently not supported in seeding. See https://github.com/dotnet/efcore/issues/31254 for more information.</value>
+  </data>
   <data name="SeedDatumDefaultValue" xml:space="preserve">
     <value>The seed entity for entity type '{entityType}' cannot be added because a default value was provided for the required property '{property}'. Please provide a value different from '{defaultValue}'.</value>
   </data>
@@ -1564,7 +1570,7 @@
     <value>The value for property '{1_entityType}.{0_property}' cannot be set to null because its type is '{propertyType}' which is not a nullable type.</value>
   </data>
   <data name="ValueComplexType" xml:space="preserve">
-    <value>Adding the complex property '{type}.{property}' isn't supported because it's of a value type '{propertyType}'.</value>
+    <value>Adding the complex property '{type}.{property}' isn't supported because it's of a value type '{propertyType}'. See https://github.com/dotnet/efcore/issues/9906 for more information.</value>
   </data>
   <data name="VisitIsNotAllowed" xml:space="preserve">
     <value>Calling '{visitMethodName}' is not allowed. Visit the expression manually for the relevant part in the visitor.</value>

diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs
@@ -1491,6 +1491,33 @@ public virtual void Detects_collection_navigations_in_seeds(bool sensitiveDataLo
             sensitiveDataLoggingEnabled);
     }
 
+    [ConditionalTheory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public virtual void Detects_complex_properties_in_seeds(bool sensitiveDataLoggingEnabled)
+    {
+        var modelBuilder = CreateConventionModelBuilder(sensitiveDataLoggingEnabled: sensitiveDataLoggingEnabled);
+        modelBuilder.Entity<SampleEntity>(
+            e =>
+            {
+                e.HasData(
+                    new SampleEntity { Id = 1, ReferencedEntity = new ReferencedEntity { Id = 2 } });
+                e.ComplexProperty(s => s.ReferencedEntity).IsRequired();
+            });
+
+        VerifyError(
+            sensitiveDataLoggingEnabled
+                ? CoreStrings.SeedDatumComplexPropertySensitive(
+                    nameof(SampleEntity),
+                    $"{nameof(SampleEntity.Id)}:1",
+                    nameof(SampleEntity.ReferencedEntity))
+                : CoreStrings.SeedDatumComplexProperty(
+                    nameof(SampleEntity),
+                    nameof(SampleEntity.ReferencedEntity)),
+            modelBuilder,
+            sensitiveDataLoggingEnabled);
+    }
+
     [ConditionalFact]
     public virtual void Throws_on_two_properties_sharing_a_field()
     {

diff --git a/test/EFCore.Tests/ModelBuilding/ComplexTypeTestBase.cs b/test/EFCore.Tests/ModelBuilding/ComplexTypeTestBase.cs
@@ -1518,6 +1518,7 @@ public virtual void Complex_properties_not_discovered_by_convention()
                 .Entity<ComplexProperties>()
                 .ComplexProperty(e => e.Customer);
 
+            // TODO: Issue #14661
             //modelBuilder
             //    .Entity<ValueComplexProperties>()
             //    .Ignore(e => e.Tuple)
@@ -1602,6 +1603,7 @@ public virtual void Throws_for_tuple()
                     Assert.Throws<InvalidOperationException>(modelBuilder.FinalizeModel).Message);
 
             // Uncomment when value types are supported.
+            // TODO: Issue #14661
             //var model = modelBuilder.FinalizeModel();
 
             //var valueType = model.FindEntityType(typeof(ValueComplexProperties));