Review tweaks

Author: ajcvickers

src/EFCore.Sqlite.Core/Extensions/SqliteComplexTypePrimitiveCollectionBuilderExtensions.cs

@@ -0,0 +1,49 @@
+// 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;
+
+/// <summary>
+///     SQLite-specific extension methods for <see cref="ComplexTypePrimitiveCollectionBuilder" />.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-modeling">Modeling entity types and relationships</see>, and
+///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+/// </remarks>
+public static class SqliteComplexTypePrimitiveCollectionBuilderExtensions
+{
+    /// <summary>
+    ///     Configures the SRID of the column that the property maps to when targeting SQLite.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-spatial">Spatial data</see>, and
+    ///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+    /// </remarks>
+    /// <param name="primitiveCollectionBuilder">The builder for the property being configured.</param>
+    /// <param name="srid">The SRID.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static ComplexTypePrimitiveCollectionBuilder HasSrid(
+        this ComplexTypePrimitiveCollectionBuilder primitiveCollectionBuilder,
+        int srid)
+    {
+        primitiveCollectionBuilder.Metadata.SetSrid(srid);
+
+        return primitiveCollectionBuilder;
+    }
+
+    /// <summary>
+    ///     Configures the SRID of the column that the property maps to when targeting SQLite.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-spatial">Spatial data</see>, and
+    ///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+    /// </remarks>
+    /// <param name="primitiveCollectionBuilder">The builder for the property being configured.</param>
+    /// <param name="srid">The SRID.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static ComplexTypePrimitiveCollectionBuilder<TProperty> HasSrid<TProperty>(
+        this ComplexTypePrimitiveCollectionBuilder<TProperty> primitiveCollectionBuilder,
+        int srid)
+        => (ComplexTypePrimitiveCollectionBuilder<TProperty>)HasSrid(
+            (ComplexTypePrimitiveCollectionBuilder)primitiveCollectionBuilder, srid);
+}

src/EFCore.Sqlite.Core/Extensions/SqliteComplexTypePropertyBuilderExtensions.cs

@@ -0,0 +1,46 @@
+// 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;
+
+/// <summary>
+///     SQLite-specific extension methods for <see cref="ComplexTypePropertyBuilder" />.
+/// </summary>
+/// <remarks>
+///     See <see href="https://aka.ms/efcore-docs-modeling">Modeling entity types and relationships</see>, and
+///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+/// </remarks>
+public static class SqliteComplexTypePropertyBuilderExtensions
+{
+    /// <summary>
+    ///     Configures the SRID of the column that the property maps to when targeting SQLite.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-spatial">Spatial data</see>, and
+    ///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+    /// </remarks>
+    /// <param name="propertyBuilder">The builder for the property being configured.</param>
+    /// <param name="srid">The SRID.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static ComplexTypePropertyBuilder HasSrid(this ComplexTypePropertyBuilder propertyBuilder, int srid)
+    {
+        propertyBuilder.Metadata.SetSrid(srid);
+
+        return propertyBuilder;
+    }
+
+    /// <summary>
+    ///     Configures the SRID of the column that the property maps to when targeting SQLite.
+    /// </summary>
+    /// <remarks>
+    ///     See <see href="https://aka.ms/efcore-docs-spatial">Spatial data</see>, and
+    ///     <see href="https://aka.ms/efcore-docs-sqlite">Accessing SQLite databases with EF Core</see> for more information and examples.
+    /// </remarks>
+    /// <param name="propertyBuilder">The builder for the property being configured.</param>
+    /// <param name="srid">The SRID.</param>
+    /// <returns>The same builder instance so that multiple calls can be chained.</returns>
+    public static ComplexTypePropertyBuilder<TProperty> HasSrid<TProperty>(
+        this ComplexTypePropertyBuilder<TProperty> propertyBuilder,
+        int srid)
+        => (ComplexTypePropertyBuilder<TProperty>)HasSrid((ComplexTypePropertyBuilder)propertyBuilder, srid);
+}

src/EFCore/Metadata/Builders/IConventionPropertyBuilder.cs

@@ -545,13 +545,13 @@ bool CanSetProviderValueComparer(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    IConventionElementTypeBuilder? ElementType(bool fromDataAnnotation = false);
+    IConventionElementTypeBuilder? ElementType(bool elementType, bool fromDataAnnotation = false);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    bool CanSetElementType(bool fromDataAnnotation = false);
+    bool CanSetElementType(bool elementType, bool fromDataAnnotation = false);
 }

src/EFCore/Metadata/Conventions/PropertyDiscoveryConvention.cs

@@ -94,7 +94,7 @@ private void Process(IConventionEntityTypeBuilder entityTypeBuilder)
             var propertyBuilder = entityTypeBuilder.Property(propertyInfo);
             if (mapping?.ElementTypeMapping != null)
             {
-                propertyBuilder?.ElementType();
+                propertyBuilder?.ElementType(true);
             }
         }
     }

src/EFCore/Metadata/IConventionProperty.cs

@@ -466,7 +466,7 @@ bool IsImplicitlyCreated()
     /// <param name="primitiveCollection">If <see langword="true"/>, then this is a collection of primitive elements.</param>
     /// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
     /// <returns>The configuration for the elements.</returns>
-    IElementType? IsPrimitiveCollection(bool primitiveCollection, bool fromDataAnnotation = false);
+    IElementType? ElementType(bool primitiveCollection, bool fromDataAnnotation = false);
 
     /// <summary>
     ///     Returns the configuration source for <see cref="IReadOnlyProperty.GetElementType" />.

src/EFCore/Metadata/IMutableProperty.cs

@@ -276,8 +276,8 @@ void SetProviderValueComparer(
     /// <summary>
     ///     Sets the configuration for elements of the primitive collection represented by this property.
     /// </summary>
-    /// <param name="primitiveCollection">If <see langword="true"/>, then this is a collection of primitive elements.</param>
-    void IsPrimitiveCollection(bool primitiveCollection);
+    /// <param name="elementType">If <see langword="true"/>, then this is a collection of primitive elements.</param>
+    void ElementType(bool elementType);
 
     /// <inheritdoc />
     bool IReadOnlyProperty.IsNullable

src/EFCore/Metadata/Internal/InternalPropertyBuilder.cs

@@ -564,7 +564,7 @@ public virtual bool CanSetConversion(Type? providerClrType, ConfigurationSource?
     {
         if (CanSetConverter(converterType, configurationSource))
         {
-            Metadata.IsPrimitiveCollection(false, configurationSource);
+            Metadata.ElementType(false, configurationSource);
             Metadata.SetProviderClrType(null, configurationSource);
             Metadata.SetValueConverter(converterType, configurationSource);
 
@@ -776,11 +776,11 @@ public virtual bool CanSetProviderValueComparer(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual InternalElementTypeBuilder? ElementType(ConfigurationSource configurationSource)
+    public virtual InternalElementTypeBuilder? ElementType(bool elementType, ConfigurationSource configurationSource)
     {
-        if (CanSetElementType(configurationSource))
+        if (CanSetElementType(elementType, configurationSource))
         {
-            Metadata.IsPrimitiveCollection(true, configurationSource);
+            Metadata.ElementType(elementType, configurationSource);
             Metadata.SetValueConverter((Type?)null, configurationSource);
             return new InternalElementTypeBuilder((ElementType)Metadata.GetElementType()!, ModelBuilder);
         }
@@ -794,8 +794,9 @@ public virtual bool CanSetProviderValueComparer(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool CanSetElementType(ConfigurationSource? configurationSource)
-        => configurationSource.Overrides(Metadata.GetElementTypeConfigurationSource());
+    public virtual bool CanSetElementType(bool elementType, ConfigurationSource? configurationSource)
+        => configurationSource.Overrides(Metadata.GetElementTypeConfigurationSource())
+            && (elementType != (Metadata.GetElementType() != null));
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -905,10 +906,7 @@ public virtual bool CanSetElementType(ConfigurationSource? configurationSource)
         }
 
         var oldElementType = (ElementType?)Metadata.GetElementType();
-        var oldElementTypeConfigurationSource = Metadata.GetElementTypeConfigurationSource();
-        if (oldElementType != null
-            && oldElementTypeConfigurationSource.HasValue
-            && newPropertyBuilder.CanSetElementType(oldElementTypeConfigurationSource))
+        if (oldElementType != null)
         {
             var newElementType = (ElementType?)newPropertyBuilder.Metadata.GetElementType();
             if (newElementType != null)
@@ -1525,16 +1523,15 @@ bool IConventionPropertyBuilder.CanSetProviderValueComparer(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    IConventionElementTypeBuilder? IConventionPropertyBuilder.ElementType(bool fromDataAnnotation)
-        => ElementType(fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
+    IConventionElementTypeBuilder? IConventionPropertyBuilder.ElementType(bool elementType, bool fromDataAnnotation)
+        => ElementType(elementType, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual bool CanSetElementType(bool fromDataAnnotation = false)
-        => (fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention)
-            .Overrides(Metadata.GetElementTypeConfigurationSource());
+    bool IConventionPropertyBuilder.CanSetElementType(bool elementType, bool fromDataAnnotation)
+        => CanSetElementType(elementType, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 }

src/EFCore/Metadata/Internal/InternalTypeBaseBuilder.cs

@@ -333,7 +333,7 @@ public static bool IsCompatible(MemberInfo? newMemberInfo, PropertyBase existing
     {
         var builder = Property(propertyType, propertyName, memberInfo, typeConfigurationSource, configurationSource);
 
-        builder?.ElementType(configurationSource!.Value);
+        builder?.ElementType(true, configurationSource!.Value);
 
         return builder;
     }

src/EFCore/Metadata/Internal/Property.cs

@@ -1214,26 +1214,26 @@ public virtual CoreTypeMapping? TypeMapping
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual IElementType? IsPrimitiveCollection(
-        bool primitiveCollection,
+    public virtual IElementType? ElementType(
+        bool elementType,
         ConfigurationSource configurationSource)
     {
         var existingElementType = GetElementType();
         if (existingElementType == null
-            && primitiveCollection)
+            && elementType)
         {
             var elementClrType = ClrType.TryGetElementType(typeof(IEnumerable<>));
             if (elementClrType == null)
             {
                 throw new InvalidOperationException(CoreStrings.NotCollection(ClrType.ShortDisplayName(), Name));
             }
-            var elementType = new ElementType(elementClrType, this, configurationSource);
-            SetAnnotation(CoreAnnotationNames.ElementType, elementType, configurationSource);
-            OnElementTypeSet(elementType, null);
-            return elementType;
+            var newElementType = new ElementType(elementClrType, this, configurationSource);
+            SetAnnotation(CoreAnnotationNames.ElementType, newElementType, configurationSource);
+            OnElementTypeSet(newElementType, null);
+            return newElementType;
         }
 
-        if (existingElementType != null && !primitiveCollection)
+        if (existingElementType != null && !elementType)
         {
             ((ElementType)existingElementType).SetRemovedFromModel();
             RemoveAnnotation(CoreAnnotationNames.ElementType);
@@ -2007,9 +2007,9 @@ void IMutableProperty.SetJsonValueReaderWriterType(Type? readerWriterType)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [DebuggerStepThrough]
-    IElementType? IConventionProperty.IsPrimitiveCollection(bool primitiveCollection, bool fromDataAnnotation)
-        => IsPrimitiveCollection(
-            primitiveCollection,
+    IElementType? IConventionProperty.ElementType(bool elementType, bool fromDataAnnotation)
+        => ElementType(
+            elementType,
             fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
 
     /// <summary>
@@ -2019,6 +2019,6 @@ void IMutableProperty.SetJsonValueReaderWriterType(Type? readerWriterType)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [DebuggerStepThrough]
-    void IMutableProperty.IsPrimitiveCollection(bool primitiveCollection)
-        => IsPrimitiveCollection(primitiveCollection, ConfigurationSource.Explicit);
+    void IMutableProperty.ElementType(bool elementType)
+        => ElementType(elementType, ConfigurationSource.Explicit);
 }

test/EFCore.Cosmos.FunctionalTests/CosmosApiConsistencyTest.cs

@@ -68,16 +68,6 @@ public override
                         typeof(CosmosPropertyBuilderExtensions),
                         null
                     )
-                },
-                {
-                    typeof(IReadOnlyElementType),
-                    (
-                        null,
-                        null,
-                        null,
-                        typeof(CosmosPrimitiveCollectionBuilderExtensions),
-                        null
-                    )
                 }
             };
     }