Fix NpgsqlArrayConverter for one-directional conversions

Fixes #3050

Author: roji

src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs

@@ -131,25 +131,15 @@ private static Expression<Func<TInput, TOutput>> ArrayConversionExpression<TInpu
                 indexer = i => ArrayAccess(input, i);
                 break;
 
-            // Input is typed as an IList - we can get its length and index into it
+            // Input is typed as an ICollection - we can get its length, but we can't index into it
             case { IsGenericType: true } when inputInterfaces.Append(input.Type)
-                .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>)):
+                .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>)):
             {
                 getInputLength = Property(
                     input,
                     input.Type.GetProperty("Count")
-                    // If TInput is an interface (IList<T>), its Count property needs to be found on ICollection<T>
+                    // If TInput is an interface (ICollection<T>), its Count property needs to be found on ICollection<T>
                     ?? typeof(ICollection<>).MakeGenericType(input.Type.GetGenericArguments()[0]).GetProperty("Count")!);
-                indexer = i => Property(input, input.Type.FindIndexerProperty()!, i);
-                break;
-            }
-
-            // Input is typed as an ICollection - we can get its length, but we can't index into it
-            case { IsGenericType: true } when inputInterfaces.Append(input.Type)
-                .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>)):
-            {
-                getInputLength = Property(
-                    input, typeof(ICollection<>).MakeGenericType(input.Type.GetGenericArguments()[0]).GetProperty("Count")!);
                 indexer = null;
                 break;
             }
@@ -176,6 +166,31 @@ private static Expression<Func<TInput, TOutput>> ArrayConversionExpression<TInpu
                 throw new NotSupportedException($"Array value converter input type must be an IEnumerable, but is {typeof(TInput)}");
         }
 
+        Expression? instantiateOutput = typeof(TConcreteOutput) switch
+        {
+            var t when t.IsArray => NewArrayBounds(outputElementType, lengthVariable),
+            var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorInfo ctorWithLength => New(ctorWithLength, lengthVariable),
+            var t when typeof(TConcreteOutput).GetConstructor([]) is not null => New(typeof(TConcreteOutput)),
+
+            _ => null
+        };
+
+        if (instantiateOutput is null)
+        {
+            // If the output type can't be instantiated (no public parameterless constructor), we can't value convert to it.
+            // If we simply throw and prevent the array converter from being instantiated, that would block scenarios where the user is
+            // only *writing* an uninstantiable type, but never reading it (see #3050). To allow for such "one-directional" value converters,
+            // we instead return a lambda that throws, so that if converter is actually ever used to read, it will throw at that point.
+            // See test Parameter_collection_Dictionary_Valuees_with_value_converter_Contains.
+            return Lambda<Func<TInput, TOutput>>(
+                Throw(
+                    New(
+                        typeof(InvalidOperationException).GetConstructor([typeof(string)])!,
+                        Constant($"Type {typeof(TConcreteOutput)} cannot be instantiated as it does not have a public parameterless constructor.")),
+                    typeof(TOutput)),
+                input);
+        }
+
         expressions.AddRange(
         [
             // Get the length of the input array or list
@@ -184,14 +199,7 @@ private static Expression<Func<TInput, TOutput>> ArrayConversionExpression<TInpu
 
             // Allocate an output array or list
             // var result = new int[length];
-            Assign(
-                output,
-                typeof(TConcreteOutput) switch
-                {
-                    var t when t.IsArray => NewArrayBounds(outputElementType, lengthVariable),
-                    var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorInfo ctorWithLength => New(ctorWithLength, lengthVariable),
-                    _ => New(typeof(TConcreteOutput))
-                })
+            Assign(output, instantiateOutput)
         ]);
 
         if (indexer is not null)
@@ -200,7 +208,7 @@ var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorI
             // the element converter on each element.
             // for (var i = 0; i < length; i++)
             // {
-            //     result[i] = input[i];
+            //     result[i] = convert(input[i]);
             // }
             var counter = Parameter(typeof(int), "i");
 
@@ -232,7 +240,7 @@ elementConversionExpression is null
             // counter = 0;
             // while (enumerator.MoveNext())
             // {
-            //     output[counter] = enumerator.Current;
+            //     output[counter] = convert(enumerator.Current);
             //     counter++;
             // }
             var enumerableType = typeof(IEnumerable<>).MakeGenericType(inputElementType);

test/EFCore.PG.FunctionalTests/Query/PrimitiveCollectionsQueryNpgsqlTest.cs

@@ -533,6 +533,31 @@ public virtual async Task Parameter_collection_HashSet_with_value_converter_Cont
 """);
     }
 
+    [ConditionalFact]
+    public virtual async Task Parameter_collection_Dictionary_Values_with_value_converter_Contains()
+    {
+        Dictionary<int, MyEnum> enums = new()
+        {
+            [0] = MyEnum.Value1,
+            [1] = MyEnum.Value4
+        };
+
+        // Dictionary<>.ValuesCollection doesn't have a public parameterless constructor, so NpgsqlArrayConverter can't convert to it
+        // (see #3050). We still allow NpgsqlArrayConverter to be built to allow one-directional conversion: in the query below,
+        // we only need to write enum.Values as a parameter (never read it).
+        await AssertQuery(ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => enums.Values.Contains(c.Enum)));
+
+        AssertSql(
+            """
+@enums_Values={ '0'
+'3' } (DbType = Object)
+
+SELECT p."Id", p."Bool", p."Bools", p."DateTime", p."DateTimes", p."Enum", p."Enums", p."Int", p."Ints", p."NullableInt", p."NullableInts", p."NullableString", p."NullableStrings", p."NullableWrappedId", p."NullableWrappedIdWithNullableComparer", p."String", p."Strings", p."WrappedId"
+FROM "PrimitiveCollectionsEntity" AS p
+WHERE p."Enum" = ANY (@enums_Values)
+""");
+    }
+
     public override async Task Parameter_collection_ImmutableArray_of_ints_Contains_int()
     {
         await base.Parameter_collection_ImmutableArray_of_ints_Contains_int();