From 66171d9ddd7646c43655cddb69dfa8df9d9c881e Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 8 Nov 2023 21:47:51 +0100 Subject: [PATCH] Change scaffolding PG arrays to List instead of T[] (#2928) Closes #2758 --- .../Storage/Internal/NpgsqlTypeMappingSource.cs | 6 ++---- .../NpgsqlNodaTimeTypeMappingTest.cs | 4 ++-- .../Storage/NpgsqlTypeMappingSourceTest.cs | 16 ++++++++-------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index f090e4dc0..37054d116 100644 --- 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 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 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.NodaTime.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs b/test/EFCore.PG.NodaTime.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs index db7730c2f..f26d05129 100644 --- a/test/EFCore.PG.NodaTime.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs +++ b/test/EFCore.PG.NodaTime.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs @@ -90,7 +90,7 @@ public void GenerateSqlLiteral_returns_tsrange_literal() public void Array_of_NpgsqlRange_of_LocalDateTime_is_properly_mapped() { Assert.Equal("tsmultirange", GetMapping(typeof(NpgsqlRange[])).StoreType); - Assert.Same(typeof(NpgsqlRange[]), GetMapping("tsmultirange").ClrType); + Assert.Same(typeof(List>), GetMapping("tsmultirange").ClrType); } [Fact] @@ -503,7 +503,7 @@ public void GenerateCodeLiteral_returns_LocalTime_literal() public void LocalTime_array_is_properly_mapped() { Assert.Equal("time[]", GetMapping(typeof(LocalTime[])).StoreType); - Assert.Same(typeof(LocalTime[]), GetMapping("time[]").ClrType); + Assert.Same(typeof(List), GetMapping("time[]").ClrType); } [Fact] diff --git a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs index 61215f87a..f46113650 100644 --- 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), null, null, null, false)] [InlineData("int", typeof(int), null, null, null, false)] - [InlineData("int[]", typeof(int[]), null, null, null, false)] + [InlineData("int[]", typeof(List), 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), null, null, null, false)] [InlineData("floatrange", typeof(NpgsqlRange), null, null, null, false)] [InlineData("dummyrange", typeof(NpgsqlRange), null, null, null, false)] - [InlineData("int4multirange", typeof(NpgsqlRange[]), null, null, null, false)] + [InlineData("int4multirange", typeof(List>), 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(mapping); - Assert.Same(typeof(string[]), arrayMapping.ClrType); + Assert.Same(typeof(List), 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(CreateTypeMappingSource().FindMapping("timestamp(5) without time zone[]")); - Assert.Same(typeof(DateTime[]), arrayMapping.ClrType); + Assert.Same(typeof(List), 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)); private void Array_over_type_mapping_with_value_converter(CoreTypeMapping mapping, Type expectedType) { @@ -276,7 +276,7 @@ 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[]), mapping14.ClrType); + Assert.Same(typeof(List>), 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 @@ -284,7 +284,7 @@ public void Multirange_by_store_type_across_pg_versions() // 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[]), mappingDefault.ClrType); + Assert.Same(typeof(List>), mappingDefault.ClrType); } #region Support