Improve handling of null values in JSON properties mapped to primitive collections

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs

@@ -3,6 +3,7 @@
 
 using System.Collections;
 using System.Runtime.CompilerServices;
+using System.Text;
 using System.Text.Json;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Query.Internal;
@@ -79,6 +80,9 @@ private static readonly MethodInfo MaterializeJsonNullableValueStructuralTypeMet
         private static readonly MethodInfo MaterializeJsonEntityCollectionMethodInfo
             = typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(MaterializeJsonEntityCollection))!;
 
+        private static readonly MethodInfo ReadPrimitiveCollectionFromJsonMethodInfo
+            = typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ReadPrimitiveCollectionFromJson))!;
+
         private static readonly MethodInfo InverseCollectionFixupMethod
             = typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(InverseCollectionFixup))!;
 
@@ -956,6 +960,38 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
             dataReaderContext.HasNext = 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>
+        [EntityFrameworkInternal]
+        public static object? ReadPrimitiveCollectionFromJson(
+            string? json,
+            JsonValueReaderWriter readerWriter,
+            bool nullable,
+            string propertyName)
+        {
+            if (json == null)
+            {
+                return null;
+            }
+
+            // A primitive collection mapped to a column is read by parsing the JSON string with the collection's
+            // JsonValueReaderWriter (which doesn't handle the 'null' token). The stored value may be a JSON 'null'
+            // token (e.g. the literal string "null"), which must be materialized as null for an optional property,
+            // rather than letting the reader/writer throw. See issues #34881 and #38454.
+            var manager = new Utf8JsonReaderManager(new JsonReaderData(Encoding.UTF8.GetBytes(json)), null);
+            manager.MoveNext();
+
+            return manager.CurrentReader.TokenType == JsonTokenType.Null
+                ? nullable
+                    ? null
+                    : throw new InvalidOperationException(RelationalStrings.NullValueInRequiredJsonProperty(propertyName))
+                : readerWriter.FromJson(ref manager);
+        }
+
         /// <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

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

@@ -10,6 +10,7 @@
 using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Storage.Json;
+using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
 using static System.Linq.Expressions.Expression;
 
 namespace Microsoft.EntityFrameworkCore.Query;
@@ -3000,7 +3001,66 @@ Expression valueExpression
             var converter = typeMapping.Converter;
 
             var converterExpression = default(Expression);
-            if (converter != null)
+            var primitiveCollectionJsonHandled = false;
+
+            // #34881/#38454: A primitive collection mapped to a column uses CollectionToJsonStringConverter, which parses the
+            // provider JSON string via the collection's JsonValueReaderWriter. That reader/writer doesn't handle a JSON 'null'
+            // token, so we must peek the first token here and short-circuit to null (or throw for a required property) before
+            // invoking the converter, rather than letting the reader/writer throw a cryptic "Invalid token type: 'Null'".
+            if (property is IProperty { IsPrimitiveCollection: true } primitiveCollectionProperty
+                && converter is not null
+                && converter.GetType() is { IsGenericType: true } converterClrType
+                && converterClrType.GetGenericTypeDefinition() == typeof(CollectionToJsonStringConverter<>))
+            {
+                var liftableConstantParameter = Parameter(typeof(MaterializerLiftableConstantContext), "c");
+                var jsonReaderWriterExpression = _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
+                    primitiveCollectionProperty.GetJsonValueReaderWriter() ?? primitiveCollectionProperty.GetTypeMapping().JsonValueReaderWriter!,
+                    Lambda<Func<MaterializerLiftableConstantContext, object>>(
+                        Coalesce(
+                            Call(
+                                LiftableConstantExpressionHelpers.BuildMemberAccessForProperty(
+                                    primitiveCollectionProperty, liftableConstantParameter),
+                                PropertyGetJsonValueReaderWriterMethod),
+                            Property(
+                                Call(
+                                    LiftableConstantExpressionHelpers.BuildMemberAccessForProperty(
+                                        primitiveCollectionProperty, liftableConstantParameter),
+                                    PropertyGetTypeMappingMethod),
+                                nameof(CoreTypeMapping.JsonValueReaderWriter))),
+                        liftableConstantParameter),
+                    primitiveCollectionProperty.Name + "JsonReaderWriter",
+                    typeof(JsonValueReaderWriter));
+
+                if (valueExpression.Type != typeof(string))
+                {
+                    valueExpression = Convert(valueExpression, typeof(string));
+                }
+
+                Expression readExpression = Call(
+                    ReadPrimitiveCollectionFromJsonMethodInfo,
+                    valueExpression,
+                    jsonReaderWriterExpression,
+                    Constant(nullable),
+                    Constant(primitiveCollectionProperty.Name));
+
+                if (readExpression.Type != type)
+                {
+                    readExpression = Convert(readExpression, type);
+                }
+
+                if (nullable)
+                {
+                    // The column itself may be SQL NULL (DbNull), distinct from a JSON 'null' token in a non-null string.
+                    readExpression = Condition(
+                        Call(dbDataReader, IsDbNullMethod, indexExpression),
+                        Default(type),
+                        readExpression);
+                }
+
+                valueExpression = readExpression;
+                primitiveCollectionJsonHandled = true;
+            }
+            else if (converter != null)
             {
                 // if IProperty is available, we can reliably get the converter from the model and then incorporate FromProvider(Typed) delegate
                 // into the expression. This way we have consistent behavior between precompiled and normal queries (same code path)
@@ -3074,12 +3134,14 @@ Expression valueExpression
                 }
             }
 
-            if (valueExpression.Type != type)
+            if (!primitiveCollectionJsonHandled
+                && valueExpression.Type != type)
             {
                 valueExpression = Convert(valueExpression, type);
             }
 
-            if (nullable)
+            if (!primitiveCollectionJsonHandled
+                && nullable)
             {
                 Expression replaceExpression;
                 if (converter?.ConvertsNulls == true)
@@ -3277,6 +3339,31 @@ private Expression CreateReadJsonPropertyValueExpression(
                     nullExpression,
                     resultExpression);
             }
+            else if (property.ClrType != typeof(string)
+                && property.ClrType.TryGetElementType(typeof(IEnumerable<>)) is not null)
+            {
+                // A required primitive collection nested in a JSON document can't be materialized from a JSON 'null'
+                // token. Throw a clear, property-named error instead of the cryptic reader/writer "Invalid token type".
+                if (resultExpression.Type != property.ClrType)
+                {
+                    resultExpression = Convert(resultExpression, property.ClrType);
+                }
+
+                resultExpression = Condition(
+                    Equal(
+                        Property(
+                            Field(
+                                jsonReaderManagerParameter,
+                                Utf8JsonReaderManagerCurrentReaderField),
+                            Utf8JsonReaderTokenTypeProperty),
+                        Constant(JsonTokenType.Null)),
+                    Throw(
+                        New(
+                            typeof(InvalidOperationException).GetConstructor([typeof(string)])!,
+                            Constant(RelationalStrings.NullValueInRequiredJsonProperty(property.Name))),
+                        property.ClrType),
+                    resultExpression);
+            }
 
             if (_detailedErrorsEnabled)
             {