Change scaffolding PG arrays to List<T> instead of T[]

Closes #2758
npgsql · Nov 8, 2023 · 289d0b3 · 289d0b3
1 parent 0e40415
commit 289d0b3
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 12 deletions.
diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs
@@ -717,9 +717,8 @@ is PropertyInfo globalEnumTypeMappingsProperty
                 if (modelType is null)
                 {
                     // There's no model type - we're scaffolding purely from the store type.
-                    // TODO: Consider returning List<T> by default for scaffolding, more useful, #2758
                     elementType = relationalElementMapping.ClrType;
-                    modelType = concreteCollectionType = elementType.MakeArrayType();
+                    modelType = concreteCollectionType = typeof(List<>).MakeGenericType(elementType);
                 }
                 else
                 {
@@ -747,9 +746,8 @@ is PropertyInfo globalEnumTypeMappingsProperty
                 // TODO: Why exclude if there's an element converter??
                 && (relationalElementMapping.Converter is null || modelType is null || modelType.IsArrayOrGenericList()))
             {
-                // TODO: Consider returning List<T> by default for scaffolding, more useful, #2758
                 return new NpgsqlMultirangeTypeMapping(
-                    storeType, modelType ?? relationalElementMapping.ClrType.MakeArrayType(), rangeMapping);
+                    storeType, modelType ?? typeof(List<>).MakeGenericType(relationalElementMapping.ClrType), rangeMapping);
             }
 
             // TODO: This needs to move to the NodaTime plugin, but there's no FindCollectionMapping extension yet for plugins

diff --git a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs
@@ -12,9 +12,9 @@ public class NpgsqlTypeMappingSourceTest
 {
     [Theory]
     [InlineData("integer", typeof(int), null, null, null, false)]
-    [InlineData("integer[]", typeof(int[]), null, null, null, false)]
+    [InlineData("integer[]", typeof(List<int>), null, null, null, false)]
     [InlineData("int", typeof(int), null, null, null, false)]
-    [InlineData("int[]", typeof(int[]), null, null, null, false)]
+    [InlineData("int[]", typeof(List<int>), null, null, null, false)]
     [InlineData("numeric", typeof(decimal), null, null, null, false)]
     [InlineData("numeric(10,2)", typeof(decimal), null, 10, 2, false)]
     [InlineData("text", typeof(string), null, null, null, false)]
@@ -32,7 +32,7 @@ public class NpgsqlTypeMappingSourceTest
     [InlineData("int4range", typeof(NpgsqlRange<int>), null, null, null, false)]
     [InlineData("floatrange", typeof(NpgsqlRange<float>), null, null, null, false)]
     [InlineData("dummyrange", typeof(NpgsqlRange<DummyType>), null, null, null, false)]
-    [InlineData("int4multirange", typeof(NpgsqlRange<int>[]), null, null, null, false)]
+    [InlineData("int4multirange", typeof(List<NpgsqlRange<int>>), null, null, null, false)]
     [InlineData("geometry", typeof(Geometry), null, null, null, false)]
     [InlineData("geometry(Polygon)", typeof(Polygon), null, null, null, false)]
     [InlineData("geography(Point, 4326)", typeof(Point), null, null, null, false)]
@@ -70,7 +70,7 @@ public void Varchar32_Array()
         var mapping = CreateTypeMappingSource().FindMapping("varchar(32)[]");
 
         var arrayMapping = Assert.IsAssignableFrom<NpgsqlArrayTypeMapping>(mapping);
-        Assert.Same(typeof(string[]), arrayMapping.ClrType);
+        Assert.Same(typeof(List<string>), arrayMapping.ClrType);
         Assert.Equal("varchar(32)[]", arrayMapping.StoreType);
         Assert.Null(arrayMapping.Size);
 
@@ -93,7 +93,7 @@ public void Timestamp_without_time_zone_5()
     public void Timestamp_without_time_zone_Array_5()
     {
         var arrayMapping = Assert.IsAssignableFrom<NpgsqlArrayTypeMapping>(CreateTypeMappingSource().FindMapping("timestamp(5) without time zone[]"));
-        Assert.Same(typeof(DateTime[]), arrayMapping.ClrType);
+        Assert.Same(typeof(List<DateTime>), arrayMapping.ClrType);
         Assert.Equal("timestamp(5) without time zone[]", arrayMapping.StoreType);
 
         var elementMapping = arrayMapping.ElementTypeMapping;
@@ -225,7 +225,7 @@ public void Array_over_type_mapping_with_value_converter_by_clr_type_list()
 
     [Fact]
     public void Array_over_type_mapping_with_value_converter_by_store_type()
-        => Array_over_type_mapping_with_value_converter(CreateTypeMappingSource().FindMapping("ltree[]"), typeof(LTree[]));
+        => Array_over_type_mapping_with_value_converter(CreateTypeMappingSource().FindMapping("ltree[]"), typeof(List<LTree>));
 
     private void Array_over_type_mapping_with_value_converter(CoreTypeMapping mapping, Type expectedType)
     {
@@ -276,15 +276,15 @@ public void Multirange_by_store_type_across_pg_versions()
         var mapping13 = CreateTypeMappingSource(postgresVersion: new(13, 0)).FindMapping("int4multirange");
         var mappingDefault = CreateTypeMappingSource().FindMapping("int4multirange")!;
 
-        Assert.Same(typeof(NpgsqlRange<int>[]), mapping14.ClrType);
+        Assert.Same(typeof(List<NpgsqlRange<int>>), mapping14.ClrType);
         Assert.Null(mapping13);
 
         // See #2351 - we didn't put multiranges behind a version opt-in in 6.0, although the default PG version is still 12; this causes
         // anyone with arrays of ranges to fail if they upgrade to 6.0 with pre-14 PG.
         // Changing this in a patch would break people already using 6.0 with PG14, so multiranges are on by default unless users explicitly
         // specify < 14.
         // Once 14 is made the default version, this stuff can be removed.
-        Assert.Same(typeof(NpgsqlRange<int>[]), mappingDefault.ClrType);
+        Assert.Same(typeof(List<NpgsqlRange<int>>), mappingDefault.ClrType);
     }
 
     #region Support