Unified error messages about mismatches in 'id' and 'lid' values

src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs

@@ -3,7 +3,6 @@
 using System.Collections.Generic;
 using System.Linq;
 using JsonApiDotNetCore.Configuration;
-using JsonApiDotNetCore.Middleware;
 using JsonApiDotNetCore.Resources;
 using JsonApiDotNetCore.Resources.Annotations;
 using JsonApiDotNetCore.Serialization.Objects;

src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceIdentityAdapter.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Net;
 using JsonApiDotNetCore.Configuration;
-using JsonApiDotNetCore.Errors;
 using JsonApiDotNetCore.Middleware;
 using JsonApiDotNetCore.Resources;
 using JsonApiDotNetCore.Resources.Annotations;
@@ -34,75 +33,7 @@ protected ResourceIdentityAdapter(IResourceGraph resourceGraph, IResourceFactory
             ArgumentGuard.NotNull(state, nameof(state));
 
             ResourceContext resourceContext = ConvertType(identity, requirements, state);
-
-            if (state.Request.Kind != EndpointKind.AtomicOperations)
-            {
-                AssertHasNoLid(identity, state);
-            }
-
-            AssertNoIdWithLid(identity, state);
-
-            if (requirements.IdConstraint == JsonElementConstraint.Required)
-            {
-                AssertHasIdOrLid(identity, state);
-            }
-            else if (requirements.IdConstraint == JsonElementConstraint.Forbidden)
-            {
-                AssertHasNoId(identity, state);
-            }
-
-            if (requirements.IdValue != null && identity.Id != null && identity.Id != requirements.IdValue)
-            {
-                using (state.Position.PushElement("id"))
-                {
-                    if (state.Request.Kind == EndpointKind.AtomicOperations)
-                    {
-                        throw new DeserializationException(state.Position, "Resource ID mismatch between 'ref.id' and 'data.id' element.",
-                            $"Expected resource with ID '{requirements.IdValue}' in 'data.id', instead of '{identity.Id}'.");
-                    }
-
-                    throw new JsonApiException(new ErrorObject(HttpStatusCode.Conflict)
-                    {
-                        Title = "Resource ID mismatch between request body and endpoint URL.",
-                        Detail = $"Expected resource ID '{requirements.IdValue}', instead of '{identity.Id}'.",
-                        Source = new ErrorSource
-                        {
-                            Pointer = state.Position.ToSourcePointer()
-                        }
-                    });
-                }
-            }
-
-            if (requirements.LidValue != null && identity.Lid != null && identity.Lid != requirements.LidValue)
-            {
-                using (state.Position.PushElement("lid"))
-                {
-                    throw new DeserializationException(state.Position, "Resource local ID mismatch between 'ref.lid' and 'data.lid' element.",
-                        $"Expected resource with local ID '{requirements.LidValue}' in 'data.lid', instead of '{identity.Lid}'.");
-                }
-            }
-
-            if (requirements.IdValue != null && identity.Lid != null)
-            {
-                using (state.Position.PushElement("lid"))
-                {
-                    throw new DeserializationException(state.Position, "Resource identity mismatch between 'ref.id' and 'data.lid' element.",
-                        $"Expected resource with ID '{requirements.IdValue}' in 'data.id', instead of '{identity.Lid}' in 'data.lid'.");
-                }
-            }
-
-            if (requirements.LidValue != null && identity.Id != null)
-            {
-                using (state.Position.PushElement("id"))
-                {
-                    throw new DeserializationException(state.Position, "Resource identity mismatch between 'ref.lid' and 'data.id' element.",
-                        $"Expected resource with local ID '{requirements.LidValue}' in 'data.lid', instead of '{identity.Id}' in 'data.id'.");
-                }
-            }
-
-            IIdentifiable resource = _resourceFactory.CreateInstance(resourceContext.ResourceType);
-            AssignStringId(identity, resource, state);
-            resource.LocalId = identity.Lid;
+            IIdentifiable resource = CreateResource(identity, requirements, resourceContext.ResourceType, state);
 
             return (resource, resourceContext);
         }
@@ -148,6 +79,34 @@ private static void AssertIsCompatibleResourceType(ResourceContext actual, Resou
             }
         }
 
+        private IIdentifiable CreateResource(IResourceIdentity identity, ResourceIdentityRequirements requirements, Type resourceType,
+            RequestAdapterState state)
+        {
+            if (state.Request.Kind != EndpointKind.AtomicOperations)
+            {
+                AssertHasNoLid(identity, state);
+            }
+
+            AssertNoIdWithLid(identity, state);
+
+            if (requirements.IdConstraint == JsonElementConstraint.Required)
+            {
+                AssertHasIdOrLid(identity, requirements, state);
+            }
+            else if (requirements.IdConstraint == JsonElementConstraint.Forbidden)
+            {
+                AssertHasNoId(identity, state);
+            }
+
+            AssertSameIdValue(identity, requirements.IdValue, state);
+            AssertSameLidValue(identity, requirements.LidValue, state);
+
+            IIdentifiable resource = _resourceFactory.CreateInstance(resourceType);
+            AssignStringId(identity, resource, state);
+            resource.LocalId = identity.Lid;
+            return resource;
+        }
+
         private static void AssertHasNoLid(IResourceIdentity identity, RequestAdapterState state)
         {
             if (identity.Lid != null)
@@ -165,14 +124,25 @@ private static void AssertNoIdWithLid(IResourceIdentity identity, RequestAdapter
             }
         }
 
-        private static void AssertHasIdOrLid(IResourceIdentity identity, RequestAdapterState state)
+        private static void AssertHasIdOrLid(IResourceIdentity identity, ResourceIdentityRequirements requirements, RequestAdapterState state)
         {
-            if (identity.Id == null && identity.Lid == null)
+            string message = null;
+
+            if (requirements.IdValue != null && identity.Id == null)
+            {
+                message = "The 'id' element is required.";
+            }
+            else if (requirements.LidValue != null && identity.Lid == null)
             {
-                string message = state.Request.Kind == EndpointKind.AtomicOperations
-                    ? "The 'id' or 'lid' element is required."
-                    : "The 'id' element is required.";
+                message = "The 'lid' element is required.";
+            }
+            else if (identity.Id == null && identity.Lid == null)
+            {
+                message = state.Request.Kind == EndpointKind.AtomicOperations ? "The 'id' or 'lid' element is required." : "The 'id' element is required.";
+            }
 
+            if (message != null)
+            {
                 throw new ModelConversionException(state.Position, message, null);
             }
         }
@@ -186,18 +156,39 @@ private static void AssertHasNoId(IResourceIdentity identity, RequestAdapterStat
             }
         }
 
-        private void AssignStringId(IResourceIdentity identity, IIdentifiable resource, RequestAdapterState state)
+        private static void AssertSameIdValue(IResourceIdentity identity, string expected, RequestAdapterState state)
         {
-            if (identity.Id != null)
+            if (expected != null && identity.Id != expected)
             {
                 using IDisposable _ = state.Position.PushElement("id");
 
+                throw new ModelConversionException(state.Position, "Conflicting 'id' values found.", $"Expected '{expected}' instead of '{identity.Id}'.",
+                    HttpStatusCode.Conflict);
+            }
+        }
+
+        private static void AssertSameLidValue(IResourceIdentity identity, string expected, RequestAdapterState state)
+        {
+            if (expected != null && identity.Lid != expected)
+            {
+                using IDisposable _ = state.Position.PushElement("lid");
+
+                throw new ModelConversionException(state.Position, "Conflicting 'lid' values found.", $"Expected '{expected}' instead of '{identity.Lid}'.",
+                    HttpStatusCode.Conflict);
+            }
+        }
+
+        private void AssignStringId(IResourceIdentity identity, IIdentifiable resource, RequestAdapterState state)
+        {
+            if (identity.Id != null)
+            {
                 try
                 {
                     resource.StringId = identity.Id;
                 }
                 catch (FormatException exception)
                 {
+                    using IDisposable _ = state.Position.PushElement("id");
                     throw new DeserializationException(state.Position, null, exception.Message);
                 }
             }

test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Updating/Resources/AtomicUpdateResourceTests.cs

@@ -1136,14 +1136,14 @@ public async Task Cannot_update_on_resource_ID_mismatch_between_ref_and_data()
             (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
 
             // Assert
-            httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity);
+            httpResponse.Should().HaveStatusCode(HttpStatusCode.Conflict);
 
             responseDocument.Errors.Should().HaveCount(1);
 
             ErrorObject error = responseDocument.Errors[0];
-            error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
-            error.Title.Should().Be("Failed to deserialize request body: Resource ID mismatch between 'ref.id' and 'data.id' element.");
-            error.Detail.Should().Be($"Expected resource with ID '{performerId1}' in 'data.id', instead of '{performerId2}'.");
+            error.StatusCode.Should().Be(HttpStatusCode.Conflict);
+            error.Title.Should().Be("Failed to deserialize request body: Conflicting 'id' values found.");
+            error.Detail.Should().Be($"Expected '{performerId1}' instead of '{performerId2}'.");
             error.Source.Pointer.Should().Be("/atomic:operations[0]/data/id");
         }
 
@@ -1184,14 +1184,14 @@ public async Task Cannot_update_on_resource_local_ID_mismatch_between_ref_and_da
             (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
 
             // Assert
-            httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity);
+            httpResponse.Should().HaveStatusCode(HttpStatusCode.Conflict);
 
             responseDocument.Errors.Should().HaveCount(1);
 
             ErrorObject error = responseDocument.Errors[0];
-            error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
-            error.Title.Should().Be("Failed to deserialize request body: Resource local ID mismatch between 'ref.lid' and 'data.lid' element.");
-            error.Detail.Should().Be("Expected resource with local ID 'local-1' in 'data.lid', instead of 'local-2'.");
+            error.StatusCode.Should().Be(HttpStatusCode.Conflict);
+            error.Title.Should().Be("Failed to deserialize request body: Conflicting 'lid' values found.");
+            error.Detail.Should().Be("Expected 'local-1' instead of 'local-2'.");
             error.Source.Pointer.Should().Be("/atomic:operations[0]/data/lid");
         }
 
@@ -1240,9 +1240,9 @@ public async Task Cannot_update_on_mixture_of_ID_and_local_ID_between_ref_and_da
 
             ErrorObject error = responseDocument.Errors[0];
             error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
-            error.Title.Should().Be("Failed to deserialize request body: Resource identity mismatch between 'ref.id' and 'data.lid' element.");
-            error.Detail.Should().Be($"Expected resource with ID '{performerId}' in 'data.id', instead of 'local-1' in 'data.lid'.");
-            error.Source.Pointer.Should().Be("/atomic:operations[0]/data/lid");
+            error.Title.Should().Be("Failed to deserialize request body: The 'id' element is required.");
+            error.Detail.Should().BeNull();
+            error.Source.Pointer.Should().Be("/atomic:operations[0]/data");
         }
 
         [Fact]
@@ -1290,9 +1290,9 @@ public async Task Cannot_update_on_mixture_of_local_ID_and_ID_between_ref_and_da
 
             ErrorObject error = responseDocument.Errors[0];
             error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
-            error.Title.Should().Be("Failed to deserialize request body: Resource identity mismatch between 'ref.lid' and 'data.id' element.");
-            error.Detail.Should().Be($"Expected resource with local ID 'local-1' in 'data.lid', instead of '{performerId}' in 'data.id'.");
-            error.Source.Pointer.Should().Be("/atomic:operations[0]/data/id");
+            error.Title.Should().Be("Failed to deserialize request body: The 'lid' element is required.");
+            error.Detail.Should().BeNull();
+            error.Source.Pointer.Should().Be("/atomic:operations[0]/data");
         }
 
         [Fact]

test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateResourceTests.cs

@@ -998,8 +998,8 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
 
             ErrorObject error = responseDocument.Errors[0];
             error.StatusCode.Should().Be(HttpStatusCode.Conflict);
-            error.Title.Should().Be("Resource ID mismatch between request body and endpoint URL.");
-            error.Detail.Should().Be($"Expected resource ID '{existingWorkItems[1].StringId}', instead of '{existingWorkItems[0].StringId}'.");
+            error.Title.Should().Be("Failed to deserialize request body: Conflicting 'id' values found.");
+            error.Detail.Should().Be($"Expected '{existingWorkItems[1].StringId}' instead of '{existingWorkItems[0].StringId}'.");
             error.Source.Pointer.Should().Be("/data/id");
         }