Fix range mapping

src/EFCore.PG/Infrastructure/Internal/INpgsqlOptions.cs

@@ -25,6 +25,6 @@ public interface INpgsqlOptions : ISingletonOptions
         /// The collection of range mappings.
         /// </summary>
         [NotNull]
-        IReadOnlyList<RangeMappingInfo> RangeMappings { get; }
+        IReadOnlyList<UserRangeDefinition> UserRangeDefinitions { get; }
     }
 }

src/EFCore.PG/Infrastructure/Internal/NpgsqlOptionsExtension.cs

@@ -14,7 +14,7 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal
     /// </summary>
     public class NpgsqlOptionsExtension : RelationalOptionsExtension
     {
-        [NotNull] readonly List<RangeMappingInfo> _rangeMappings;
+        [NotNull] readonly List<UserRangeDefinition> _userRangeDefinitions;
 
         /// <summary>
         /// The name of the database for administrative operations.
@@ -32,7 +32,7 @@ public class NpgsqlOptionsExtension : RelationalOptionsExtension
         /// The list of range mappings specified by the user.
         /// </summary>
         [NotNull]
-        public IReadOnlyList<RangeMappingInfo> RangeMappings => _rangeMappings;
+        public IReadOnlyList<UserRangeDefinition> UserRangeDefinitions => _userRangeDefinitions;
 
         /// <summary>
         /// The specified <see cref="ProvideClientCertificatesCallback"/>.
@@ -55,7 +55,7 @@ public class NpgsqlOptionsExtension : RelationalOptionsExtension
         /// Initializes an instance of <see cref="NpgsqlOptionsExtension"/> with the default settings.
         /// </summary>
         public NpgsqlOptionsExtension()
-            => _rangeMappings = new List<RangeMappingInfo>();
+            => _userRangeDefinitions = new List<UserRangeDefinition>();
 
         // NB: When adding new options, make sure to update the copy ctor below.
         /// <summary>
@@ -65,7 +65,7 @@ public NpgsqlOptionsExtension()
         public NpgsqlOptionsExtension([NotNull] NpgsqlOptionsExtension copyFrom) : base(copyFrom)
         {
             AdminDatabase = copyFrom.AdminDatabase;
-            _rangeMappings = new List<RangeMappingInfo>(copyFrom._rangeMappings);
+            _userRangeDefinitions = new List<UserRangeDefinition>(copyFrom._userRangeDefinitions);
             PostgresVersion = copyFrom.PostgresVersion;
             ProvideClientCertificatesCallback = copyFrom.ProvideClientCertificatesCallback;
             RemoteCertificateValidationCallback = copyFrom.RemoteCertificateValidationCallback;
@@ -94,11 +94,11 @@ public override bool ApplyServices(IServiceCollection services)
         /// Returns a copy of the current instance configured with the specified range mapping.
         /// </summary>
         [NotNull]
-        public virtual NpgsqlOptionsExtension WithRangeMapping<TSubtype>(string rangeName, string subtypeName)
+        public virtual NpgsqlOptionsExtension WithUserRangeDefinition<TSubtype>(string rangeName, string subtypeName)
         {
             var clone = (NpgsqlOptionsExtension)Clone();
 
-            clone._rangeMappings.Add(new RangeMappingInfo(rangeName, typeof(TSubtype), subtypeName));
+            clone._userRangeDefinitions.Add(new UserRangeDefinition(rangeName, typeof(TSubtype), subtypeName));
 
             return clone;
         }
@@ -107,11 +107,11 @@ public virtual NpgsqlOptionsExtension WithRangeMapping<TSubtype>(string rangeNam
         /// Returns a copy of the current instance configured with the specified range mapping.
         /// </summary>
         [NotNull]
-        public virtual NpgsqlOptionsExtension WithRangeMapping(string rangeName, Type subtypeClrType, string subtypeName)
+        public virtual NpgsqlOptionsExtension WithUserRangeDefinition(string rangeName, Type subtypeClrType, string subtypeName)
         {
             var clone = (NpgsqlOptionsExtension)Clone();
 
-            clone._rangeMappings.Add(new RangeMappingInfo(rangeName, subtypeClrType, subtypeName));
+            clone._userRangeDefinitions.Add(new UserRangeDefinition(rangeName, subtypeClrType, subtypeName));
 
             return clone;
         }
@@ -194,7 +194,7 @@ public virtual NpgsqlOptionsExtension WithRemoteCertificateValidationCallback([C
         #endregion Authentication
     }
 
-    public readonly struct RangeMappingInfo
+    public class UserRangeDefinition
     {
         /// <summary>The name of the PostgreSQL range type to be mapped.</summary>
         public string RangeName { get; }
@@ -209,7 +209,7 @@ public readonly struct RangeMappingInfo
         /// </summary>
         public string SubtypeName { get; }
 
-        public RangeMappingInfo(string rangeName, Type subtypeClrType, string subtypeName)
+        public UserRangeDefinition(string rangeName, Type subtypeClrType, string subtypeName)
         {
             RangeName = rangeName;
             SubtypeClrType = subtypeClrType;

src/EFCore.PG/Infrastructure/NpgsqlDbContextOptionsBuilder.cs

@@ -54,7 +54,7 @@ public virtual void SetPostgresVersion([CanBeNull] Version postgresVersion)
         /// <code>NpgsqlTypeMappingSource.MapRange{float}("floatrange");</code>
         /// </example>
         public virtual void MapRange<TSubtype>([NotNull] string rangeName, string subtypeName = null)
-            => WithOption(e => e.WithRangeMapping(rangeName, typeof(TSubtype), subtypeName));
+            => WithOption(e => e.WithUserRangeDefinition(rangeName, typeof(TSubtype), subtypeName));
 
         /// <summary>
         /// Maps a user-defined PostgreSQL range type for use.
@@ -73,7 +73,7 @@ public virtual void MapRange<TSubtype>([NotNull] string rangeName, string subtyp
         /// <code>NpgsqlTypeMappingSource.MapRange("floatrange", typeof(float));</code>
         /// </example>
         public virtual void MapRange([NotNull] string rangeName, [NotNull] Type subtypeClrType, string subtypeName = null)
-            => WithOption(e => e.WithRangeMapping(rangeName, subtypeClrType, subtypeName));
+            => WithOption(e => e.WithUserRangeDefinition(rangeName, subtypeClrType, subtypeName));
 
         /// <summary>
         /// Appends NULLS FIRST to all ORDER BY clauses. This is important for the tests which were written

src/EFCore.PG/Internal/NpgsqlOptions.cs

@@ -20,10 +20,10 @@ public class NpgsqlOptions : INpgsqlOptions
 
         /// <inheritdoc />
         [NotNull]
-        public virtual IReadOnlyList<RangeMappingInfo> RangeMappings { get; private set; }
+        public virtual IReadOnlyList<UserRangeDefinition> UserRangeDefinitions { get; private set; }
 
         public NpgsqlOptions()
-            => RangeMappings = new RangeMappingInfo[0];
+            => UserRangeDefinitions = new UserRangeDefinition[0];
 
         /// <inheritdoc />
         public void Initialize(IDbContextOptions options)
@@ -32,7 +32,7 @@ public void Initialize(IDbContextOptions options)
 
             PostgresVersion = npgsqlOptions.PostgresVersion;
             ReverseNullOrderingEnabled = npgsqlOptions.ReverseNullOrdering;
-            RangeMappings = npgsqlOptions.RangeMappings;
+            UserRangeDefinitions = npgsqlOptions.UserRangeDefinitions;
         }
 
         /// <inheritdoc />

src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs

@@ -22,6 +22,8 @@ public class NpgsqlTypeMappingSource : RelationalTypeMappingSource
         public ConcurrentDictionary<string, RelationalTypeMapping[]> StoreTypeMappings { get; }
         public ConcurrentDictionary<Type, RelationalTypeMapping> ClrTypeMappings { get; }
 
+        readonly IReadOnlyList<UserRangeDefinition> _userRangeDefinitions;
+
         #region Mappings
 
         // Numeric types
@@ -238,24 +240,7 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc
 
             LoadUserDefinedTypeMappings();
 
-            if (npgsqlOptions == null)
-                return;
-
-            foreach (var (rangeName, subtypeClrType, subtypeName) in npgsqlOptions.RangeMappings)
-            {
-                var subtypeMapping = subtypeName == null
-                    ? ClrTypeMappings.TryGetValue(subtypeClrType, out var mapping)
-                        ? mapping
-                        : throw new Exception($"Could not map range {rangeName}, no mapping was found for subtype CLR type {subtypeClrType}")
-                    : StoreTypeMappings.TryGetValue(subtypeName, out var mappings)
-                        ? mappings[0]
-                        : throw new Exception($"Could not map range {rangeName}, no mapping was found for subtype {subtypeName}");
-
-                var rangeClrType = typeof(NpgsqlRange<>).MakeGenericType(subtypeClrType);
-                var rangeMapping = new NpgsqlRangeTypeMapping(rangeName, rangeClrType, subtypeMapping);
-                StoreTypeMappings[rangeName] = new RelationalTypeMapping[] { rangeMapping };
-                ClrTypeMappings[rangeClrType] = rangeMapping;
-            }
+            _userRangeDefinitions = npgsqlOptions?.UserRangeDefinitions ?? new UserRangeDefinition[0];
         }
 
         /// <summary>
@@ -300,8 +285,10 @@ protected override RelationalTypeMapping FindMapping(in RelationalTypeMappingInf
             base.FindMapping(mappingInfo) ??
             // Then, any mappings that have already been set up
             FindExistingMapping(mappingInfo) ??
-            // Finally, try any array mappings which have not yet been set up
-            FindArrayMapping(mappingInfo);
+            // Try any array mappings which have not yet been set up
+            FindArrayMapping(mappingInfo) ??
+            // Try any user-defined range mappings which have not yet been set up
+            FindUserRangeMapping(mappingInfo);
 
         protected virtual RelationalTypeMapping FindExistingMapping(in RelationalTypeMappingInfo mappingInfo)
         {
@@ -387,7 +374,7 @@ protected virtual RelationalTypeMapping FindExistingMapping(in RelationalTypeMap
             return mapping;
         }
 
-        RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo)
+        protected virtual RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo)
         {
             // PostgreSQL array type names are the element plus []
             var storeType = mappingInfo.StoreTypeName;
@@ -445,5 +432,64 @@ RelationalTypeMapping FindArrayMapping(in RelationalTypeMappingInfo mappingInfo)
 
             return null;
         }
+
+        protected virtual RelationalTypeMapping FindUserRangeMapping(in RelationalTypeMappingInfo mappingInfo)
+        {
+            UserRangeDefinition rangeDefinition = null;
+            var rangeStoreType = mappingInfo.StoreTypeName;
+            var rangeClrType = mappingInfo.ClrType;
+
+            // If the incoming MappingInfo contains a ClrType, make sure it's an NpgsqlRange<T>, otherwise bail
+            if (rangeClrType != null &&
+                (!rangeClrType.IsGenericType || rangeClrType.GetGenericTypeDefinition() != typeof(NpgsqlRange<>)))
+            {
+                return null;
+            }
+
+            // Try to find a user range definition (defined by the user on their context options), based on the
+            // incoming MappingInfo's StoreType or ClrType
+            if (rangeStoreType != null)
+            {
+                rangeDefinition = _userRangeDefinitions.SingleOrDefault(m => m.RangeName == rangeStoreType);
+
+                if (rangeDefinition == null)
+                    return null;
+
+                if (rangeClrType == null)
+                {
+                    // The incoming MappingInfo does not contain a ClrType, only a StoreType (i.e. scaffolding).
+                    // Construct the range ClrType from the range definition's subtype ClrType
+                    rangeClrType = typeof(NpgsqlRange<>).MakeGenericType(rangeDefinition.SubtypeClrType);
+                }
+                else if (rangeClrType != typeof(NpgsqlRange<>).MakeGenericType(rangeDefinition.SubtypeClrType))
+                {
+                    // If the incoming MappingInfo also contains a ClrType (in addition to the StoreType), make sure it
+                    // corresponds to the subtype ClrType on the range definition
+                    return null;
+                }
+            }
+            else if (rangeClrType != null)
+                rangeDefinition = _userRangeDefinitions.SingleOrDefault(m => m.SubtypeClrType == rangeClrType.GetGenericArguments()[0]);
+
+            if (rangeDefinition == null)
+                return null;
+
+            // We now have a user-defined range definition from the context options. Use it to get the subtype's
+            // mapping
+            var subtypeMapping = (RelationalTypeMapping)(rangeDefinition.SubtypeName == null
+                ? FindMapping(rangeDefinition.SubtypeClrType)
+                : FindMapping(rangeDefinition.SubtypeName));
+
+            if (subtypeMapping == null)
+                throw new Exception($"Could not map range {rangeDefinition.RangeName}, no mapping was found its subtype");
+
+            // Finally, construct a range mapping and add it to our lookup dictionaries - next time it will be found as
+            // an existing mapping
+            var rangeMapping = new NpgsqlRangeTypeMapping(rangeDefinition.RangeName, rangeClrType, subtypeMapping);
+            StoreTypeMappings[rangeDefinition.RangeName] = new RelationalTypeMapping[] { rangeMapping };
+            ClrTypeMappings[rangeClrType] = rangeMapping;
+
+            return rangeMapping;
+        }
     }
 }

test/EFCore.PG.Tests/EFCore.PG.Tests.csproj

@@ -5,6 +5,7 @@
     <TargetFrameworks Condition="'$(OS)' != 'Windows_NT' OR '$(CoreOnly)' == 'True'">netcoreapp2.1</TargetFrameworks>
     <AssemblyName>Npgsql.EntityFrameworkCore.PostgreSQL.Tests</AssemblyName>
     <RootNamespace>Npgsql.EntityFrameworkCore.PostgreSQL</RootNamespace>
+    <LangVersion>latest</LangVersion>
   </PropertyGroup>
 
   <ItemGroup>

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs

@@ -0,0 +1,111 @@
+using System;
+using System.Data;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Storage;
+using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
+using NpgsqlTypes;
+using Xunit;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage
+{
+    public class NpgsqlTypeMappingSourceTest
+    {
+        [Theory]
+        [InlineData("integer", typeof(int))]
+        [InlineData("integer[]", typeof(int[]))]
+        [InlineData("dummy", typeof(DummyType))]
+        [InlineData("int4range", typeof(NpgsqlRange<int>))]
+        [InlineData("floatrange", typeof(NpgsqlRange<float>))]
+        [InlineData("dummyrange", typeof(NpgsqlRange<DummyType>))]
+        public void By_StoreType(string storeType, Type expectedClrType)
+            => Assert.Same(expectedClrType, Source.FindMapping(storeType).ClrType);
+
+        [Theory]
+        [InlineData(typeof(int), "integer")]
+        [InlineData(typeof(int[]), "integer[]")]
+        [InlineData(typeof(DummyType), "dummy")]
+        [InlineData(typeof(NpgsqlRange<int>), "int4range")]
+        [InlineData(typeof(NpgsqlRange<float>), "floatrange")]
+        [InlineData(typeof(NpgsqlRange<DummyType>), "dummyrange")]
+        public void By_ClrType(Type clrType, string expectedStoreType)
+            => Assert.Equal(expectedStoreType, ((RelationalTypeMapping)Source.FindMapping(clrType)).StoreType);
+
+        [Theory]
+        [InlineData("integer", typeof(int))]
+        [InlineData("integer[]", typeof(int[]))]
+        [InlineData("dummy", typeof(DummyType))]
+        [InlineData("int4range", typeof(NpgsqlRange<int>))]
+        [InlineData("floatrange", typeof(NpgsqlRange<float>))]
+        [InlineData("dummyrange", typeof(NpgsqlRange<DummyType>))]
+        public void By_StoreType_with_ClrType(string storeType, Type clrType)
+            => Assert.Equal(storeType, Source.FindMapping(clrType, storeType).StoreType);
+
+        [Theory]
+        [InlineData("integer", typeof(UnknownType))]
+        //[InlineData("integer[]", typeof(UnknownType))] TODO Implement
+        [InlineData("dummy", typeof(UnknownType))]
+        [InlineData("int4range", typeof(UnknownType))]
+        [InlineData("floatrange", typeof(UnknownType))]
+        [InlineData("dummyrange", typeof(UnknownType))]
+        public void By_StoreType_with_wrong_ClrType(string storeType, Type wrongClrType)
+            => Assert.Null(Source.FindMapping(wrongClrType, storeType));
+
+        // Happens when using domain/aliases: we don't know about the domain but continue with the mapping based on the ClrType
+        [Fact]
+        public void Unknown_StoreType_with_known_ClrType()
+            => Assert.Equal("integer", Source.FindMapping(typeof(int), "some_domain").StoreType);
+
+        #region Support
+
+        public NpgsqlTypeMappingSourceTest()
+        {
+            var builder = new DbContextOptionsBuilder();
+            new NpgsqlDbContextOptionsBuilder(builder).MapRange("floatrange", typeof(float));
+            new NpgsqlDbContextOptionsBuilder(builder).MapRange("dummyrange", typeof(DummyType), "dummy");
+            var options = new NpgsqlOptions();
+            options.Initialize(builder.Options);
+
+            Source = new NpgsqlTypeMappingSource(
+                new TypeMappingSourceDependencies(
+                    new ValueConverterSelector(new ValueConverterSelectorDependencies()),
+                    Array.Empty<ITypeMappingSourcePlugin>()),
+                new RelationalTypeMappingSourceDependencies(
+                    new[] { new DummyTypeMappingSourcePlugin() }),
+                options);
+        }
+
+        NpgsqlTypeMappingSource Source { get; }
+
+        class DummyTypeMappingSourcePlugin : IRelationalTypeMappingSourcePlugin
+        {
+            public RelationalTypeMapping FindMapping(in RelationalTypeMappingInfo mappingInfo)
+                => mappingInfo.StoreTypeName != null
+                   ? mappingInfo.StoreTypeName == "dummy" && (mappingInfo.ClrType == null || mappingInfo.ClrType == typeof(DummyType))
+                       ? _dummyMapping
+                       : null
+                   : mappingInfo.ClrType == typeof(DummyType)
+                       ? _dummyMapping
+                       : null;
+
+            DummyMapping _dummyMapping = new DummyMapping();
+
+            class DummyMapping : RelationalTypeMapping
+            {
+                // TODO: The DbType is a hack, we currently require of range subtype mapping that they other expose an NpgsqlDbType
+                // or a DbType (from which NpgsqlDbType is computed), since RangeTypeMapping sends an NpgsqlDbType.
+                // This means we currently don't support ranges over types without NpgsqlDbType, which are accessible via
+                // NpgsqlParameter.DataTypeName
+                public DummyMapping() : base("dummy", typeof(DummyType), System.Data.DbType.Guid) {}
+            }
+        }
+
+        class DummyType {}
+
+        class UnknownType {}
+
+        #endregion Support
+    }
+}