From 4fbdcf94b73fed50f2ab911c07293f1019427316 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Thu, 11 Aug 2022 14:49:22 -0700 Subject: [PATCH] Addressing API review feedback regarding json columns - renamed JsonColumnName to ContainerColumnName, - renamed JsonColumnTypeMapping to ContainerColumnTypeMapping, - changed RelationalModelValidator.ValidateJsonEntityProperties to protected method, - moved JsonPropertyName methods on navigation into entity --- .../Design/AnnotationCodeGenerator.cs | 12 +- .../RelationalEntityTypeBuilderExtensions.cs | 39 ++++++ .../RelationalEntityTypeExtensions.cs | 131 ++++++++++++------ .../RelationalNavigationBuilderExtensions.cs | 52 ------- .../RelationalNavigationExtensions.cs | 62 --------- ...ationalOwnedNavigationBuilderExtensions.cs | 6 +- .../RelationalModelValidator.cs | 6 +- .../RelationalMapToJsonConvention.cs | 6 +- ...tionJsonPropertyNameAttributeConvention.cs | 2 +- .../Metadata/Internal/RelationalModel.cs | 20 +-- .../Metadata/RelationalAnnotationNames.cs | 8 +- .../Update/ModificationCommand.cs | 6 +- .../Builders/OwnedNavigationBuilder.cs | 26 ---- .../Design/CSharpMigrationsGeneratorTest.cs | 8 +- .../Migrations/ModelSnapshotSqlServerTest.cs | 9 +- .../SqlServerModelBuilderTestBase.cs | 26 ++-- 16 files changed, 184 insertions(+), 235 deletions(-) delete mode 100644 src/EFCore.Relational/Extensions/RelationalNavigationBuilderExtensions.cs delete mode 100644 src/EFCore.Relational/Extensions/RelationalNavigationExtensions.cs diff --git a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs index 84714758081..861376d9473 100644 --- a/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/AnnotationCodeGenerator.cs @@ -36,7 +36,7 @@ public class AnnotationCodeGenerator : IAnnotationCodeGenerator RelationalAnnotationNames.UpdateStoredProcedure, RelationalAnnotationNames.MappingFragments, RelationalAnnotationNames.RelationalOverrides, - RelationalAnnotationNames.JsonColumnTypeMapping + RelationalAnnotationNames.ContainerColumnTypeMapping }; /// @@ -200,17 +200,17 @@ public virtual IReadOnlyList GenerateFluentApiCalls( } } - if (annotations.TryGetValue(RelationalAnnotationNames.JsonColumnName, out var jsonColumnNameAnnotation) - && jsonColumnNameAnnotation != null && jsonColumnNameAnnotation.Value is string jsonColumnName + if (annotations.TryGetValue(RelationalAnnotationNames.ContainerColumnName, out var containerColumnNameAnnotation) + && containerColumnNameAnnotation != null && containerColumnNameAnnotation.Value is string containerColumnName && entityType.IsOwned()) { methodCallCodeFragments.Add( new MethodCallCodeFragment( nameof(RelationalOwnedNavigationBuilderExtensions.ToJson), - jsonColumnName)); + containerColumnName)); - annotations.Remove(RelationalAnnotationNames.JsonColumnName); - annotations.Remove(RelationalAnnotationNames.JsonColumnTypeMapping); + annotations.Remove(RelationalAnnotationNames.ContainerColumnName); + annotations.Remove(RelationalAnnotationNames.ContainerColumnTypeMapping); } methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(entityType, annotations, GenerateFluentApi)); diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs index 84cbfd3b1ed..f06804f0dd2 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs @@ -1146,4 +1146,43 @@ public static bool CanHaveTrigger( tableName, tableSchema, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + + /// + /// Configures the entity mapped to a JSON column, mapping it to a specific JSON property, + /// rather than using the navigation name leading to it. + /// + /// The builder for the entity type being configured. + /// JSON property name to be used. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// The same builder instance if the configuration was applied, + /// otherwise. + /// + public static IConventionEntityTypeBuilder? HasJsonPropertyName( + this IConventionEntityTypeBuilder entityTypeBuilder, + string? name, + bool fromDataAnnotation = false) + { + if (!entityTypeBuilder.CanSetJsonPropertyName(name, fromDataAnnotation)) + { + return null; + } + + entityTypeBuilder.Metadata.SetJsonPropertyName(name, fromDataAnnotation); + + return entityTypeBuilder; + } + + /// + /// Returns a value indicating whether the given value can be used as a JSON property name for a given navigation. + /// + /// The builder for the entity type being configured. + /// JSON property name to be used. + /// Indicates whether the configuration was specified using a data annotation. + /// if the given value can be set as JSON property name for this navigation. + public static bool CanSetJsonPropertyName( + this IConventionEntityTypeBuilder entityTypeBuilder, + string? name, + bool fromDataAnnotation = false) + => entityTypeBuilder.CanSetAnnotation(RelationalAnnotationNames.JsonPropertyName, name, fromDataAnnotation); } diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs index 150739d1642..b2fcfff28ab 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs @@ -1847,87 +1847,136 @@ public static IEnumerable GetDeclaredTriggers(this IEntityType entityT /// The entity type. /// A value indicating whether the associated entity type is mapped to a JSON column. public static bool IsMappedToJson(this IReadOnlyEntityType entityType) - => !string.IsNullOrEmpty(entityType.GetJsonColumnName()); + => !string.IsNullOrEmpty(entityType.GetContainerColumnName()); /// - /// Sets the name of the JSON column to which the entity type is mapped. + /// Sets the name of the container column to which the entity type is mapped. /// - /// The entity type to set the JSON column name for. + /// The entity type to set the container column name for. /// The name to set. - public static void SetJsonColumnName(this IMutableEntityType entityType, string? columnName) - => entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.JsonColumnName, columnName); + public static void SetContainerColumnName(this IMutableEntityType entityType, string? columnName) + => entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.ContainerColumnName, columnName); /// - /// Sets the name of the JSON column to which the entity type is mapped. + /// Sets the name of the container column to which the entity type is mapped. /// - /// The entity type to set the JSON column name for. + /// The entity type to set the container column name for. /// The name to set. /// Indicates whether the configuration was specified using a data annotation. /// The configured value. - public static string? SetJsonColumnName( + public static string? SetContainerColumnName( this IConventionEntityType entityType, string? columnName, bool fromDataAnnotation = false) - => (string?)entityType.SetAnnotation(RelationalAnnotationNames.JsonColumnName, columnName, fromDataAnnotation)?.Value; + => (string?)entityType.SetAnnotation(RelationalAnnotationNames.ContainerColumnName, columnName, fromDataAnnotation)?.Value; /// - /// Gets the for the JSON column name. + /// Gets the for the container column name. /// - /// The entity type to set the JSON column name for. - /// The for the JSON column name. - public static ConfigurationSource? GetJsonColumnNameConfigurationSource(this IConventionEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.JsonColumnName) + /// The entity type to set the container column name for. + /// The for the container column name. + public static ConfigurationSource? GetContainerColumnNameConfigurationSource(this IConventionEntityType entityType) + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnName) ?.GetConfigurationSource(); /// - /// Gets the JSON column name to which the entity type is mapped. + /// Gets the container column name to which the entity type is mapped. /// - /// The entity type to get the JSON column name for. - /// The JSON column name to which the entity type is mapped. - public static string? GetJsonColumnName(this IReadOnlyEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.JsonColumnName)?.Value is string jsonColumnName - ? jsonColumnName - : (entityType.FindOwnership()?.PrincipalEntityType.GetJsonColumnName()); + /// The entity type to get the container column name for. + /// The container column name to which the entity type is mapped. + public static string? GetContainerColumnName(this IReadOnlyEntityType entityType) + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnName)?.Value is string columnName + ? columnName + : (entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnName()); /// - /// Sets the type mapping for the JSON column to which the entity type is mapped. + /// Sets the type mapping for the container column to which the entity type is mapped. /// - /// The entity type to set the JSON column type mapping for. + /// The entity type to set the container column type mapping for. /// The type mapping to set. - public static void SetJsonColumnTypeMapping(this IMutableEntityType entityType, RelationalTypeMapping typeMapping) - => entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.JsonColumnTypeMapping, typeMapping); + public static void SetContainerColumnTypeMapping(this IMutableEntityType entityType, RelationalTypeMapping typeMapping) + => entityType.SetOrRemoveAnnotation(RelationalAnnotationNames.ContainerColumnTypeMapping, typeMapping); /// - /// Sets the type mapping for the JSON column to which the entity type is mapped. + /// Sets the type mapping for the container column to which the entity type is mapped. /// - /// The entity type to set the JSON column type mapping for. + /// The entity type to set the container column type mapping for. /// The type mapping to set. /// Indicates whether the configuration was specified using a data annotation. /// The configured value. - public static RelationalTypeMapping? SetJsonColumnTypeMapping( + public static RelationalTypeMapping? SetContainerColumnTypeMapping( this IConventionEntityType entityType, RelationalTypeMapping? typeMapping, bool fromDataAnnotation = false) - => (RelationalTypeMapping?)entityType.SetAnnotation(RelationalAnnotationNames.JsonColumnTypeMapping, typeMapping, fromDataAnnotation)?.Value; + => (RelationalTypeMapping?)entityType.SetAnnotation(RelationalAnnotationNames.ContainerColumnTypeMapping, typeMapping, fromDataAnnotation)?.Value; /// - /// Gets the for the JSON column type mapping. + /// Gets the for the container column type mapping. /// - /// The entity type to set the JSON column type mapping for. - /// The for the JSON column type mapping. - public static ConfigurationSource? GetJsonColumnTypeMappingConfigurationSource(this IConventionEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.JsonColumnTypeMapping) + /// The entity type to set the container column type mapping for. + /// The for the container column type mapping. + public static ConfigurationSource? GetContainerColumnTypeMappingConfigurationSource(this IConventionEntityType entityType) + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnTypeMapping) ?.GetConfigurationSource(); /// - /// Gets the JSON column type mapping to which the entity type is mapped. + /// Gets the container column type mapping to which the entity type is mapped. /// - /// The entity type to get the JSON column type mapping for. - /// The JSON column type mapping to which the entity type is mapped. - public static RelationalTypeMapping? GetJsonColumnTypeMapping(this IReadOnlyEntityType entityType) - => entityType.FindAnnotation(RelationalAnnotationNames.JsonColumnTypeMapping)?.Value is RelationalTypeMapping jsonColumnTypeMapping - ? jsonColumnTypeMapping - : (entityType.FindOwnership()?.PrincipalEntityType.GetJsonColumnTypeMapping()); + /// The entity type to get the container column type mapping for. + /// The container column type mapping to which the entity type is mapped. + public static RelationalTypeMapping? GetContainerColumnTypeMapping(this IReadOnlyEntityType entityType) + => entityType.FindAnnotation(RelationalAnnotationNames.ContainerColumnTypeMapping)?.Value is RelationalTypeMapping typeMapping + ? typeMapping + : (entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnTypeMapping()); + + /// + /// Sets the value of JSON property name used for the given entity mapped to a JSON column. + /// + /// The entity type. + /// The name to be used. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + public static string? SetJsonPropertyName( + this IConventionEntityType entityType, + string? name, + bool fromDataAnnotation = false) + => (string?)entityType.SetOrRemoveAnnotation( + RelationalAnnotationNames.JsonPropertyName, + Check.NullButNotEmpty(name, nameof(name)), + fromDataAnnotation)?.Value; + + /// + /// Gets the value of JSON property name used for the given entity mapped to a JSON column. + /// + /// + /// Unless configured explicitly, navigation name is used. + /// + /// The entity type. + /// + /// The value for the JSON property used to store this entity type. + /// is returned for entities that are not mapped to a JSON column. + /// + public static string? GetJsonPropertyName(this IReadOnlyEntityType entityType) + => (string?)entityType.FindAnnotation(RelationalAnnotationNames.JsonPropertyName)?.Value + ?? (!entityType.IsMappedToJson() ? null : entityType.FindOwnership()!.GetNavigation(pointsToPrincipal: false)!.Name); + + /// + /// Sets the value of JSON property name used for the given entity mapped to a JSON column. + /// + /// The entity type. + /// The name to be used. + public static void SetJsonPropertyName(this IMutableEntityType entityType, string? name) + => entityType.SetOrRemoveAnnotation( + RelationalAnnotationNames.JsonPropertyName, + Check.NullButNotEmpty(name, nameof(name))); + + /// + /// Gets the for the JSON property name for a given entity Type. + /// + /// The entity type. + /// The for the JSON property name for a given navigation. + public static ConfigurationSource? GetJsonPropertyNameConfigurationSource(this IConventionEntityType entityType) + => entityType.FindAnnotation(RelationalAnnotationNames.JsonPropertyName)?.GetConfigurationSource(); #endregion } diff --git a/src/EFCore.Relational/Extensions/RelationalNavigationBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalNavigationBuilderExtensions.cs deleted file mode 100644 index 0435837e2ba..00000000000 --- a/src/EFCore.Relational/Extensions/RelationalNavigationBuilderExtensions.cs +++ /dev/null @@ -1,52 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.EntityFrameworkCore; - -/// -/// Relational database specific extension methods for . -/// -/// -/// See Modeling entity types and relationships for more information and examples. -/// -public static class RelationalNavigationBuilderExtensions -{ - /// - /// Configures the navigation of an entity mapped to a JSON column, mapping the navigation to a specific JSON property, - /// rather than using the navigation name. - /// - /// The builder for the navigation being configured. - /// JSON property name to be used. - /// Indicates whether the configuration was specified using a data annotation. - /// - /// The same builder instance if the configuration was applied, - /// otherwise. - /// - public static IConventionNavigationBuilder? HasJsonPropertyName( - this IConventionNavigationBuilder navigationBuilder, - string? name, - bool fromDataAnnotation = false) - { - if (!navigationBuilder.CanSetJsonPropertyName(name, fromDataAnnotation)) - { - return null; - } - - navigationBuilder.Metadata.SetJsonPropertyName(name, fromDataAnnotation); - - return navigationBuilder; - } - - /// - /// Returns a value indicating whether the given value can be used as a JSON property name for a given navigation. - /// - /// The builder for the navigation being configured. - /// JSON property name to be used. - /// Indicates whether the configuration was specified using a data annotation. - /// if the given value can be set as JSON property name for this navigation. - public static bool CanSetJsonPropertyName( - this IConventionNavigationBuilder navigationBuilder, - string? name, - bool fromDataAnnotation = false) - => navigationBuilder.CanSetAnnotation(RelationalAnnotationNames.JsonPropertyName, name, fromDataAnnotation); -} diff --git a/src/EFCore.Relational/Extensions/RelationalNavigationExtensions.cs b/src/EFCore.Relational/Extensions/RelationalNavigationExtensions.cs deleted file mode 100644 index 3092c2d6ddd..00000000000 --- a/src/EFCore.Relational/Extensions/RelationalNavigationExtensions.cs +++ /dev/null @@ -1,62 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.EntityFrameworkCore; - -/// -/// Navigation extension methods for relational database metadata. -/// -/// -/// See Modeling entity types and relationships for more information and examples. -/// -public static class RelationalNavigationExtensions -{ - /// - /// Gets the value of JSON property name used for the given navigation of an entity mapped to a JSON column. - /// - /// - /// Unless configured explicitly, navigation name is used. - /// - /// The navigation. - /// - /// The value for the JSON property used to store the value of this navigation. - /// is returned for navigations of entities that are not mapped to a JSON column. - /// - public static string? GetJsonPropertyName(this IReadOnlyNavigationBase navigation) - => (string?)navigation.FindAnnotation(RelationalAnnotationNames.JsonPropertyName)?.Value - ?? (!navigation.DeclaringEntityType.IsMappedToJson() ? null : navigation.Name); - - /// - /// Sets the value of JSON property name used for the given navigation of an entity mapped to a JSON column. - /// - /// The navigation. - /// The name to be used. - public static void SetJsonPropertyName(this IMutableNavigationBase navigation, string? name) - => navigation.SetOrRemoveAnnotation( - RelationalAnnotationNames.JsonPropertyName, - Check.NullButNotEmpty(name, nameof(name))); - - /// - /// Sets the value of JSON property name used for the given navigation of an entity mapped to a JSON column. - /// - /// The navigation. - /// The name to be used. - /// Indicates whether the configuration was specified using a data annotation. - /// The configured value. - public static string? SetJsonPropertyName( - this IConventionNavigationBase navigation, - string? name, - bool fromDataAnnotation = false) - => (string?)navigation.SetOrRemoveAnnotation( - RelationalAnnotationNames.JsonPropertyName, - Check.NullButNotEmpty(name, nameof(name)), - fromDataAnnotation)?.Value; - - /// - /// Gets the for the JSON property name for a given navigation. - /// - /// The navigation. - /// The for the JSON property name for a given navigation. - public static ConfigurationSource? GetJsonPropertyNameConfigurationSource(this IConventionNavigationBase navigation) - => navigation.FindAnnotation(RelationalAnnotationNames.JsonPropertyName)?.GetConfigurationSource(); -} diff --git a/src/EFCore.Relational/Extensions/RelationalOwnedNavigationBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalOwnedNavigationBuilderExtensions.cs index 576d62aed1a..0f0329748a5 100644 --- a/src/EFCore.Relational/Extensions/RelationalOwnedNavigationBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalOwnedNavigationBuilderExtensions.cs @@ -69,7 +69,7 @@ public static OwnedNavigationBuilder ToJson x.FindOwnership() is IForeignKey ownership && !ownership.PrincipalEntityType.IsOwned()) - .GroupBy(x => x.GetJsonColumnName()) + .GroupBy(x => x.GetContainerColumnName()) .Where(x => x.Key is not null) .Select(g => new { g.Key, Count = g.Count() }) .Where(x => x.Count > 1) @@ -2544,7 +2544,7 @@ protected virtual void ValidateJsonEntityKey( /// /// The store object. /// The entity type containing the properties to validate. - public virtual void ValidateJsonEntityProperties( + protected virtual void ValidateJsonEntityProperties( in StoreObjectIdentifier storeObject, IEntityType jsonEntityType) { @@ -2573,7 +2573,7 @@ public virtual void ValidateJsonEntityProperties( foreach (var navigation in jsonEntityType.GetDeclaredNavigations()) { - var jsonPropertyName = navigation.GetJsonPropertyName()!; + var jsonPropertyName = navigation.TargetEntityType.GetJsonPropertyName()!; if (!jsonPropertyNames.Contains(jsonPropertyName)) { jsonPropertyNames.Add(jsonPropertyName); diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs index a464847e592..a699f3d4fce 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs @@ -44,7 +44,7 @@ public virtual void ProcessEntityTypeAnnotationChanged( IConventionAnnotation? oldAnnotation, IConventionContext context) { - if (name != RelationalAnnotationNames.JsonColumnName) + if (name != RelationalAnnotationNames.ContainerColumnName) { return; } @@ -55,11 +55,11 @@ public virtual void ProcessEntityTypeAnnotationChanged( var jsonColumnTypeMapping = ((IRelationalTypeMappingSource)Dependencies.TypeMappingSource).FindMapping( typeof(JsonElement))!; - entityTypeBuilder.Metadata.SetJsonColumnTypeMapping(jsonColumnTypeMapping); + entityTypeBuilder.Metadata.SetContainerColumnTypeMapping(jsonColumnTypeMapping); } else { - entityTypeBuilder.Metadata.SetJsonColumnTypeMapping(null); + entityTypeBuilder.Metadata.SetContainerColumnTypeMapping(null); } } diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalNavigationJsonPropertyNameAttributeConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalNavigationJsonPropertyNameAttributeConvention.cs index 7ae5957dc14..17f55836472 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalNavigationJsonPropertyNameAttributeConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalNavigationJsonPropertyNameAttributeConvention.cs @@ -42,7 +42,7 @@ public override void ProcessNavigationAdded( { if (!string.IsNullOrWhiteSpace(attribute.Name)) { - navigationBuilder.HasJsonPropertyName(attribute.Name, fromDataAnnotation: true); + navigationBuilder.Metadata.TargetEntityType.Builder.HasJsonPropertyName(attribute.Name, fromDataAnnotation: true); } } } diff --git a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs index 63b40a7f339..c286ff5474a 100644 --- a/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs +++ b/src/EFCore.Relational/Metadata/Internal/RelationalModel.cs @@ -457,17 +457,17 @@ private static void CreateTableMapping( IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal }; - var jsonColumnName = mappedType.GetJsonColumnName(); - if (!string.IsNullOrEmpty(jsonColumnName)) + var containerColumnName = mappedType.GetContainerColumnName(); + if (!string.IsNullOrEmpty(containerColumnName)) { var ownership = mappedType.GetForeignKeys().Single(fk => fk.IsOwnership); if (!ownership.PrincipalEntityType.IsMappedToJson()) { - Debug.Assert(table.FindColumn(jsonColumnName) == null); + Debug.Assert(table.FindColumn(containerColumnName) == null); var jsonColumnTypeMapping = relationalTypeMappingSource.FindMapping(typeof(JsonElement))!; - var jsonColumn = new JsonColumn(jsonColumnName, jsonColumnTypeMapping.StoreType, table, jsonColumnTypeMapping.ProviderValueComparer); - table.Columns.Add(jsonColumnName, jsonColumn); + var jsonColumn = new JsonColumn(containerColumnName, jsonColumnTypeMapping.StoreType, table, jsonColumnTypeMapping.ProviderValueComparer); + table.Columns.Add(containerColumnName, jsonColumn); jsonColumn.IsNullable = !ownership.IsRequired || !ownership.IsUnique; if (ownership.PrincipalEntityType.BaseType != null) @@ -614,17 +614,17 @@ private static void CreateViewMapping( IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal }; - var jsonColumnName = mappedType.GetJsonColumnName(); - if (!string.IsNullOrEmpty(jsonColumnName)) + var containerColumnName = mappedType.GetContainerColumnName(); + if (!string.IsNullOrEmpty(containerColumnName)) { var ownership = mappedType.GetForeignKeys().Single(fk => fk.IsOwnership); if (!ownership.PrincipalEntityType.IsMappedToJson()) { - Debug.Assert(view.FindColumn(jsonColumnName) == null); + Debug.Assert(view.FindColumn(containerColumnName) == null); var jsonColumnTypeMapping = relationalTypeMappingSource.FindMapping(typeof(JsonElement))!; - var jsonColumn = new JsonViewColumn(jsonColumnName, jsonColumnTypeMapping.StoreType, view); - view.Columns.Add(jsonColumnName, jsonColumn); + var jsonColumn = new JsonViewColumn(containerColumnName, jsonColumnTypeMapping.StoreType, view); + view.Columns.Add(containerColumnName, jsonColumn); jsonColumn.IsNullable = !ownership.IsRequired || !ownership.IsUnique; if (ownership.PrincipalEntityType.BaseType != null) diff --git a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs index 3e9f70f306b..abc44e5ebef 100644 --- a/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs +++ b/src/EFCore.Relational/Metadata/RelationalAnnotationNames.cs @@ -320,14 +320,14 @@ public static class RelationalAnnotationNames public const string FieldValueGetter = Prefix + "FieldValueGetter"; /// - /// The name for the annotation specifying JSON column name to which the object is mapped. + /// The name for the annotation specifying container column name to which the object is mapped. /// - public const string JsonColumnName = Prefix + "JsonColumnName"; + public const string ContainerColumnName = Prefix + "ContainerColumnName"; /// - /// The name for the annotation specifying JSON column type mapping. + /// The name for the annotation specifying container column type mapping. /// - public const string JsonColumnTypeMapping = Prefix + "JsonColumnTypeMapping"; + public const string ContainerColumnTypeMapping = Prefix + "ContainerColumnTypeMapping"; /// /// The JSON property name for the element that the property/navigation maps to. diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 578f98ef130..f356135f499 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -289,8 +289,8 @@ private List GenerateColumnModifications() // for JSON entry, traverse to the entry for root JSON entity // and build entire JSON structure based on it // this will be the column modification command - var jsonColumnName = entry.EntityType.GetJsonColumnName()!; - var jsonColumnTypeMapping = entry.EntityType.GetJsonColumnTypeMapping()!; + var jsonColumnName = entry.EntityType.GetContainerColumnName()!; + var jsonColumnTypeMapping = entry.EntityType.GetContainerColumnTypeMapping()!; var currentEntry = entry; var currentOwnership = currentEntry.EntityType.FindOwnership()!; @@ -493,7 +493,7 @@ private JsonNode CreateJson(object? navigationValue, IUpdateEntry parentEntry, I foreach (var navigation in entityType.GetNavigations()) { - var jsonPropertyName = navigation.GetJsonPropertyName()!; + var jsonPropertyName = navigation.TargetEntityType.GetJsonPropertyName()!; var ownedNavigationValue = entry.GetCurrentValue(navigation)!; var navigationJson = CreateJson( ownedNavigationValue, diff --git a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs index c87b9e78c6e..aba20203604 100644 --- a/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs +++ b/src/EFCore/Metadata/Builders/OwnedNavigationBuilder.cs @@ -170,32 +170,6 @@ public virtual PropertyBuilder Property(string propertyNam typeof(TProperty), Check.NotEmpty(propertyName, nameof(propertyName)), ConfigurationSource.Explicit)!.Metadata)); - /// - /// Returns an object that can be used to configure a property of the entity type. - /// If no property with the given name exists, then a new property will be added. - /// - /// - /// When adding a new property, if a property with the same name exists in the entity class - /// then it will be added to the model. If no property exists in the entity class, then - /// a new shadow state property will be added. A shadow state property is one that does not have a - /// corresponding property in the entity class. The current value for the property is stored in - /// the rather than being stored in instances of the entity class. - /// - /// The type of the property to be configured. - /// The name of the property to be configured. - /// Indicates whether the type configuration source should be set. - /// An object that can be used to configure the property. - public virtual PropertyBuilder Property( - Type propertyType, - string propertyName, - bool setTypeConfigurationSource = true) - => new( - DependentEntityType.Builder.Property( - Check.NotNull(propertyType, nameof(propertyType)), - Check.NotEmpty(propertyName, nameof(propertyName)), - setTypeConfigurationSource ? ConfigurationSource.Explicit : null, - ConfigurationSource.Explicit)!.Metadata); - /// /// Returns an object that can be used to configure a property of the owned entity type. /// If no property with the given name exists, then a new property will be added. diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs index a85e35998f2..fbc87aa5b63 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs @@ -90,8 +90,8 @@ public void Test_new_annotations_handled_for_entity_types() RelationalAnnotationNames.ModelDependencies, RelationalAnnotationNames.FieldValueGetter, RelationalAnnotationNames.JsonPropertyName, - RelationalAnnotationNames.JsonColumnName, // Appears on entity type but requires specific model (i.e. owned types that can map to json, otherwise validation throws) - RelationalAnnotationNames.JsonColumnTypeMapping, + RelationalAnnotationNames.ContainerColumnName, // Appears on entity type but requires specific model (i.e. owned types that can map to json, otherwise validation throws) + RelationalAnnotationNames.ContainerColumnTypeMapping, }; // Add a line here if the code generator is supposed to handle this annotation @@ -243,8 +243,8 @@ public void Test_new_annotations_handled_for_properties() RelationalAnnotationNames.ModelDependencies, RelationalAnnotationNames.Triggers, RelationalAnnotationNames.FieldValueGetter, - RelationalAnnotationNames.JsonColumnName, - RelationalAnnotationNames.JsonColumnTypeMapping, + RelationalAnnotationNames.ContainerColumnName, + RelationalAnnotationNames.ContainerColumnTypeMapping, RelationalAnnotationNames.JsonPropertyName, }; diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index f258f206c84..bc0392316eb 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -3661,12 +3661,13 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() b3.ToTable(""EntityWithOneProperty""); + b3.HasAnnotation(""Relational:JsonPropertyName"", ""JsonProps""); + b3.WithOwner() .HasForeignKey(""EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId""); }); - b2.Navigation(""Properties"") - .HasAnnotation(""Relational:JsonPropertyName"", ""JsonProps""); + b2.Navigation(""Properties""); }); b1.Navigation(""EntityWithOneProperty""); @@ -3697,7 +3698,7 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() Assert.Equal("NotKey", RelationalPropertyExtensions.GetJsonPropertyName(ownedProperties1[1])); Assert.Equal(nameof(EntityWithOneProperty), ownedType1.GetTableName()); - Assert.Equal("EntityWithTwoProperties", ownedType1.GetJsonColumnName()); + Assert.Equal("EntityWithTwoProperties", ownedType1.GetContainerColumnName()); var ownership2 = ownedType1.FindNavigation(nameof(EntityWithStringKey)).ForeignKey; Assert.Equal("EntityWithTwoPropertiesEntityWithOnePropertyId", ownership2.Properties[0].Name); @@ -3712,7 +3713,7 @@ public virtual void Owned_types_mapped_to_json_are_stored_in_snapshot() Assert.Equal("EntityWithTwoPropertiesEntityWithOnePropertyId", ownedProperties2[0].Name); var navigation3 = ownedType2.FindNavigation(nameof(EntityWithStringKey.Properties)); - Assert.Equal("JsonProps", navigation3.GetJsonPropertyName()); + Assert.Equal("JsonProps", navigation3.TargetEntityType.GetJsonPropertyName()); var ownership3 = navigation3.ForeignKey; Assert.Equal("EntityWithStringKeyEntityWithTwoPropertiesEntityWithOnePropertyId", ownership3.Properties[0].Name); Assert.Equal(nameof(EntityWithStringKey.Properties), ownership3.PrincipalToDependent.Name); diff --git a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderTestBase.cs b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderTestBase.cs index 9b4743ef6dd..f47fb74836d 100644 --- a/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderTestBase.cs +++ b/test/EFCore.SqlServer.Tests/ModelBuilding/SqlServerModelBuilderTestBase.cs @@ -1445,12 +1445,12 @@ public virtual void Json_entity_and_normal_owned_can_exist_side_by_side_on_same_ Assert.Equal(4, ownedEntities.Count()); Assert.Equal(2, ownedEntities.Where(e => e.IsMappedToJson()).Count()); Assert.Equal(2, ownedEntities.Where(e => e.IsOwned() && !e.IsMappedToJson()).Count()); - var reference = ownedEntities.Where(e => e.GetJsonColumnName() == "reference").Single(); + var reference = ownedEntities.Where(e => e.GetContainerColumnName() == "reference").Single(); Assert.Equal("Date", reference.GetProperty("Date").GetJsonPropertyName()); Assert.Equal("Fraction", reference.GetProperty("Fraction").GetJsonPropertyName()); Assert.Equal("Enum", reference.GetProperty("Enum").GetJsonPropertyName()); - var collection = ownedEntities.Where(e => e.GetJsonColumnName() == "collection").Single(); + var collection = ownedEntities.Where(e => e.GetContainerColumnName() == "collection").Single(); Assert.Equal("Date", collection.GetProperty("Date").GetJsonPropertyName()); Assert.Equal("Fraction", collection.GetProperty("Fraction").GetJsonPropertyName()); Assert.Equal("Enum", collection.GetProperty("Enum").GetJsonPropertyName()); @@ -1490,7 +1490,7 @@ public virtual void Json_entity_with_tph_inheritance() Assert.Equal("Enum", ownedEntity.GetProperty("Enum").GetJsonPropertyName()); } - var jsonColumnNames = ownedEntities.Select(x => x.GetJsonColumnName()).OrderBy(x => x).ToList(); + var jsonColumnNames = ownedEntities.Select(x => x.GetContainerColumnName()).OrderBy(x => x).ToList(); Assert.Equal("collection_on_base", jsonColumnNames[0]); Assert.Equal("collection_on_derived", jsonColumnNames[1]); Assert.Equal("reference_on_base", jsonColumnNames[2]); @@ -1549,10 +1549,10 @@ public virtual void Json_entity_with_nested_structure_same_property_names() Assert.Equal("Date", outerOwnedEntity.GetProperty("Date").GetJsonPropertyName()); Assert.Equal("Fraction", outerOwnedEntity.GetProperty("Fraction").GetJsonPropertyName()); Assert.Equal("Enum", outerOwnedEntity.GetProperty("Enum").GetJsonPropertyName()); - Assert.Equal("Reference1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference1").GetJsonPropertyName()); - Assert.Equal("Reference2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference2").GetJsonPropertyName()); - Assert.Equal("Collection1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection1").GetJsonPropertyName()); - Assert.Equal("Collection2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection2").GetJsonPropertyName()); + Assert.Equal("Reference1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference1").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("Reference2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference2").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("Collection1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection1").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("Collection2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection2").TargetEntityType.GetJsonPropertyName()); } var ownedEntities = model.FindEntityTypes(typeof(OwnedEntity)); @@ -1707,7 +1707,7 @@ public virtual void Entity_mapped_to_json_and_unwound_afterwards_properly_cleans foreach (var outerOwnedEntity in outerOwnedEntities) { Assert.False(outerOwnedEntity.IsMappedToJson()); - Assert.Null(outerOwnedEntity.GetJsonColumnTypeMapping()); + Assert.Null(outerOwnedEntity.GetContainerColumnTypeMapping()); var myEnum = outerOwnedEntity.GetDeclaredProperties().Where(p => p.ClrType.IsEnum).Single(); var typeMapping = myEnum.FindRelationalTypeMapping()!; @@ -1720,7 +1720,7 @@ public virtual void Entity_mapped_to_json_and_unwound_afterwards_properly_cleans foreach (var ownedEntity in ownedEntities) { Assert.False(ownedEntity.IsMappedToJson()); - Assert.Null(ownedEntity.GetJsonColumnTypeMapping()); + Assert.Null(ownedEntity.GetContainerColumnTypeMapping()); var myEnum = ownedEntity.GetDeclaredProperties().Where(p => p.ClrType.IsEnum).Single(); var typeMapping = myEnum.FindRelationalTypeMapping()!; Assert.True(typeMapping.Converter is EnumToNumberConverter); @@ -1842,10 +1842,10 @@ public virtual void Json_entity_with_custom_property_names() Assert.Equal("OuterDate", outerOwnedEntity.GetProperty("Date").GetJsonPropertyName()); Assert.Equal("OuterFraction", outerOwnedEntity.GetProperty("Fraction").GetJsonPropertyName()); Assert.Equal("OuterEnum", outerOwnedEntity.GetProperty("Enum").GetJsonPropertyName()); - Assert.Equal("RenamedReference1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference1").GetJsonPropertyName()); - Assert.Equal("RenamedReference2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference2").GetJsonPropertyName()); - Assert.Equal("RenamedCollection1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection1").GetJsonPropertyName()); - Assert.Equal("RenamedCollection2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection2").GetJsonPropertyName()); + Assert.Equal("RenamedReference1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference1").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("RenamedReference2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Reference2").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("RenamedCollection1", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection1").TargetEntityType.GetJsonPropertyName()); + Assert.Equal("RenamedCollection2", outerOwnedEntity.GetNavigations().Single(n => n.Name == "Collection2").TargetEntityType.GetJsonPropertyName()); } var ownedEntities = model.FindEntityTypes(typeof(OwnedEntity));