Unified error messages about data presense and its value: null/object/array

Author: Bart Koelman

src/JsonApiDotNetCore/Errors/ToManyRelationshipRequiredException.cs

@@ -61,10 +61,8 @@ private static void AssertNoHref(AtomicOperationObject atomicOperationObject, Re
         {
             if (atomicOperationObject.Href != null)
             {
-                using (state.Position.PushElement("href"))
-                {
-                    throw new ModelConversionException(state.Position, "The 'href' element is not supported.", null);
-                }
+                using IDisposable _ = state.Position.PushElement("href");
+                throw new ModelConversionException(state.Position, "The 'href' element is not supported.", null);
             }
         }
 
@@ -76,10 +74,8 @@ private WriteOperationKind ConvertOperationCode(AtomicOperationObject atomicOper
                 {
                     if (atomicOperationObject.Ref is { Relationship: null })
                     {
-                        using (state.Position.PushElement("ref"))
-                        {
-                            throw new ModelConversionException(state.Position, "The 'relationship' element is required.", null);
-                        }
+                        using IDisposable _ = state.Position.PushElement("ref");
+                        throw new ModelConversionException(state.Position, "The 'relationship' element is required.", null);
                     }
 
                     return atomicOperationObject.Ref == null ? WriteOperationKind.CreateResource : WriteOperationKind.AddToRelationship;

src/JsonApiDotNetCore/Serialization/RequestAdapters/AtomicOperationObjectAdapter.cs

@@ -22,7 +22,6 @@ public AtomicReferenceResult Convert(AtomicReference atomicReference, ResourceId
             ArgumentGuard.NotNull(state, nameof(state));
 
             using IDisposable _ = state.Position.PushElement("ref");
-
             (IIdentifiable resource, ResourceContext resourceContext) = ConvertResourceIdentity(atomicReference, requirements, state);
 
             RelationshipAttribute relationship = atomicReference.Relationship != null
@@ -35,7 +34,6 @@ public AtomicReferenceResult Convert(AtomicReference atomicReference, ResourceId
         private RelationshipAttribute ConvertRelationship(string relationshipName, ResourceContext resourceContext, RequestAdapterState state)
         {
             using IDisposable _ = state.Position.PushElement("relationship");
-
             RelationshipAttribute relationship = resourceContext.TryGetRelationshipByPublicName(relationshipName);
 
             AssertIsKnownRelationship(relationship, relationshipName, resourceContext, state);

src/JsonApiDotNetCore/Serialization/RequestAdapters/AtomicReferenceAdapter.cs

@@ -0,0 +1,55 @@
+using JetBrains.Annotations;
+using JsonApiDotNetCore.Serialization.Objects;
+
+namespace JsonApiDotNetCore.Serialization.RequestAdapters
+{
+    /// <summary>
+    /// Contains shared assertions for derived types.
+    /// </summary>
+    public abstract class BaseDataAdapter
+    {
+        [AssertionMethod]
+        protected static void AssertHasData<T>(SingleOrManyData<T> data, RequestAdapterState state)
+            where T : class, IResourceIdentity
+        {
+            if (!data.IsAssigned)
+            {
+                throw new ModelConversionException(state.Position, "The 'data' element is required.", null);
+            }
+        }
+
+        [AssertionMethod]
+        protected static void AssertHasSingleValue<T>(SingleOrManyData<T> data, bool allowNull, RequestAdapterState state)
+            where T : class, IResourceIdentity
+        {
+            if (data.SingleValue == null)
+            {
+                if (!allowNull)
+                {
+                    throw new ModelConversionException(state.Position,
+                        data.ManyValue == null
+                            ? "Expected an object in 'data' element, instead of 'null'."
+                            : "Expected an object in 'data' element, instead of an array.", null);
+                }
+
+                if (data.ManyValue != null)
+                {
+                    throw new ModelConversionException(state.Position, "Expected an object or 'null' in 'data' element, instead of an array.", null);
+                }
+            }
+        }
+
+        [AssertionMethod]
+        protected static void AssertHasManyValue<T>(SingleOrManyData<T> data, RequestAdapterState state)
+            where T : class, IResourceIdentity
+        {
+            if (data.ManyValue == null)
+            {
+                throw new ModelConversionException(state.Position,
+                    data.SingleValue == null
+                        ? "Expected an array in 'data' element, instead of 'null'."
+                        : "Expected an array in 'data' element, instead of an object.", null);
+            }
+        }
+    }
+}

src/JsonApiDotNetCore/Serialization/RequestAdapters/BaseDataAdapter.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Resources;
@@ -26,12 +27,10 @@ public IList<OperationContainer> Convert(Document document, RequestAdapterState
             ArgumentGuard.NotNull(state, nameof(state));
             AssertHasOperations(document.Operations, state);
 
-            using (state.Position.PushElement("atomic:operations"))
-            {
-                AssertMaxOperationsNotExceeded(document.Operations, state);
+            using IDisposable _ = state.Position.PushElement("atomic:operations");
+            AssertMaxOperationsNotExceeded(document.Operations, state);
 
-                return ConvertOperations(document.Operations, state);
-            }
+            return ConvertOperations(document.Operations, state);
         }
 
         private static void AssertHasOperations(IEnumerable<AtomicOperationObject> atomicOperationObjects, RequestAdapterState state)
@@ -59,13 +58,12 @@ private IList<OperationContainer> ConvertOperations(IEnumerable<AtomicOperationO
 
             foreach (AtomicOperationObject atomicOperationObject in atomicOperationObjects)
             {
-                using (state.Position.PushArrayIndex(operationIndex))
-                {
-                    OperationContainer operation = _atomicOperationObjectAdapter.Convert(atomicOperationObject, state);
-                    operations.Add(operation);
+                using IDisposable _ = state.Position.PushArrayIndex(operationIndex);
+
+                OperationContainer operation = _atomicOperationObjectAdapter.Convert(atomicOperationObject, state);
+                operations.Add(operation);
 
-                    operationIndex++;
-                }
+                operationIndex++;
             }
 
             return operations;

src/JsonApiDotNetCore/Serialization/RequestAdapters/OperationsDocumentAdapter.cs

@@ -9,8 +9,8 @@
 
 namespace JsonApiDotNetCore.Serialization.RequestAdapters
 {
-    /// <inheritdoc />
-    public sealed class RelationshipDataAdapter : IRelationshipDataAdapter
+    /// <inheritdoc cref="IRelationshipDataAdapter" />
+    public sealed class RelationshipDataAdapter : BaseDataAdapter, IRelationshipDataAdapter
     {
         private static readonly CollectionConverter CollectionConverter = new();
 
@@ -70,9 +70,9 @@ public object Convert(SingleOrManyData<ResourceIdentifierObject> data, Relations
         {
             ArgumentGuard.NotNull(relationship, nameof(relationship));
             ArgumentGuard.NotNull(state, nameof(state));
+            AssertHasData(data, state);
 
             using IDisposable _ = state.Position.PushElement("data");
-
             ResourceContext rightResourceContext = _resourceGraph.GetResourceContext(relationship.RightType);
 
             var requirements = new ResourceIdentityRequirements
@@ -83,31 +83,22 @@ public object Convert(SingleOrManyData<ResourceIdentifierObject> data, Relations
             };
 
             return relationship is HasOneAttribute
-                ? ConvertToOneRelationshipData(data, relationship, requirements, state)
+                ? ConvertToOneRelationshipData(data, requirements, state)
                 : ConvertToManyRelationshipData(data, relationship, requirements, useToManyElementType, state);
         }
 
-        private IIdentifiable ConvertToOneRelationshipData(SingleOrManyData<ResourceIdentifierObject> data, RelationshipAttribute relationship,
-            ResourceIdentityRequirements requirements, RequestAdapterState state)
+        private IIdentifiable ConvertToOneRelationshipData(SingleOrManyData<ResourceIdentifierObject> data, ResourceIdentityRequirements requirements,
+            RequestAdapterState state)
         {
-            AssertHasSingleValue(data, relationship, state);
+            AssertHasSingleValue(data, true, state);
 
             return data.SingleValue != null ? _resourceIdentifierObjectAdapter.Convert(data.SingleValue, requirements, state) : null;
         }
 
-        private static void AssertHasSingleValue(SingleOrManyData<ResourceIdentifierObject> data, RelationshipAttribute relationship, RequestAdapterState state)
-        {
-            if (!data.IsAssigned || data.ManyValue != null)
-            {
-                throw new DeserializationException(state.Position, "Expected single data element for to-one relationship.",
-                    $"Expected single data element for '{relationship.PublicName}' relationship.");
-            }
-        }
-
         private IEnumerable ConvertToManyRelationshipData(SingleOrManyData<ResourceIdentifierObject> data, RelationshipAttribute relationship,
             ResourceIdentityRequirements requirements, bool useToManyElementType, RequestAdapterState state)
         {
-            AssertHasManyValue(data, relationship, state);
+            AssertHasManyValue(data, state);
 
             int arrayIndex = 0;
             var rightResources = new List<IIdentifiable>();
@@ -131,14 +122,5 @@ private IEnumerable ConvertToManyRelationshipData(SingleOrManyData<ResourceIdent
             resourceSet.AddRange(rightResources);
             return resourceSet;
         }
-
-        private static void AssertHasManyValue(SingleOrManyData<ResourceIdentifierObject> data, RelationshipAttribute relationship, RequestAdapterState state)
-        {
-            if (data.ManyValue == null)
-            {
-                throw new DeserializationException(state.Position, "Expected data[] element for to-many relationship.",
-                    $"Expected data[] element for '{relationship.PublicName}' relationship.");
-            }
-        }
     }
 }

src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs

@@ -1,11 +1,12 @@
+using System;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Resources;
 using JsonApiDotNetCore.Serialization.Objects;
 
 namespace JsonApiDotNetCore.Serialization.RequestAdapters
 {
-    /// <inheritdoc />
-    public class ResourceDataAdapter : IResourceDataAdapter
+    /// <inheritdoc cref="IResourceDataAdapter" />
+    public class ResourceDataAdapter : BaseDataAdapter, IResourceDataAdapter
     {
         private readonly IResourceDefinitionAccessor _resourceDefinitionAccessor;
         private readonly IResourceObjectAdapter _resourceObjectAdapter;
@@ -24,36 +25,19 @@ public IIdentifiable Convert(SingleOrManyData<ResourceObject> data, ResourceIden
         {
             ArgumentGuard.NotNull(requirements, nameof(requirements));
             ArgumentGuard.NotNull(state, nameof(state));
-            AssertHasData(data, state);
 
-            using (state.Position.PushElement("data"))
-            {
-                AssertNoManyValue(data, state);
+            AssertHasData(data, state);
 
-                (IIdentifiable resource, ResourceContext _) = ConvertResourceObject(data, requirements, state);
+            using IDisposable _ = state.Position.PushElement("data");
+            AssertHasSingleValue(data, false, state);
 
-                // Ensure that IResourceDefinition extensibility point sees the current operation, it case it injects IJsonApiRequest.
-                state.RefreshInjectables();
+            (IIdentifiable resource, ResourceContext _) = ConvertResourceObject(data, requirements, state);
 
-                _resourceDefinitionAccessor.OnDeserialize(resource);
-                return resource;
-            }
-        }
+            // Ensure that IResourceDefinition extensibility point sees the current operation, it case it injects IJsonApiRequest.
+            state.RefreshInjectables();
 
-        private static void AssertHasData(SingleOrManyData<ResourceObject> data, RequestAdapterState state)
-        {
-            if (data.Value == null)
-            {
-                throw new DeserializationException(state.Position, "The 'data' element is required.", null);
-            }
-        }
-
-        private static void AssertNoManyValue(SingleOrManyData<ResourceObject> data, RequestAdapterState state)
-        {
-            if (data.ManyValue != null)
-            {
-                throw new DeserializationException(state.Position, "Expected 'data' object instead of array.", null);
-            }
+            _resourceDefinitionAccessor.OnDeserialize(resource);
+            return resource;
         }
 
         protected virtual (IIdentifiable resource, ResourceContext resourceContext) ConvertResourceObject(SingleOrManyData<ResourceObject> data,

src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceDataAdapter.cs

@@ -57,7 +57,6 @@ private void ConvertAttribute(IIdentifiable resource, string attributeName, obje
             RequestAdapterState state)
         {
             using IDisposable _ = state.Position.PushElement(attributeName);
-
             AttrAttribute attr = resourceContext.TryGetAttributeByPublicName(attributeName);
 
             if (attr == null && _options.AllowUnknownFieldsInRequestBody)
@@ -142,7 +141,6 @@ private void ConvertRelationship(string relationshipName, SingleOrManyData<Resou
             ResourceContext resourceContext, RequestAdapterState state)
         {
             using IDisposable _ = state.Position.PushElement(relationshipName);
-
             RelationshipAttribute relationship = resourceContext.TryGetRelationshipByPublicName(relationshipName);
 
             if (relationship == null && _options.AllowUnknownFieldsInRequestBody)