From 5f618d28039b3f271a8fd9059c05d661a47e6f04 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 29 Sep 2021 22:24:15 +0200 Subject: [PATCH] Unified error messages about mismatches in 'id' and 'lid' values --- .../Errors/ResourceIdMismatchException.cs | 22 --- .../RelationshipDataAdapter.cs | 1 - .../ResourceIdentityAdapter.cs | 145 ++++++++---------- .../Resources/AtomicUpdateResourceTests.cs | 28 ++-- .../Updating/Resources/UpdateResourceTests.cs | 4 +- 5 files changed, 84 insertions(+), 116 deletions(-) delete mode 100644 src/JsonApiDotNetCore/Errors/ResourceIdMismatchException.cs diff --git a/src/JsonApiDotNetCore/Errors/ResourceIdMismatchException.cs b/src/JsonApiDotNetCore/Errors/ResourceIdMismatchException.cs deleted file mode 100644 index fdfd6b6fe9..0000000000 --- a/src/JsonApiDotNetCore/Errors/ResourceIdMismatchException.cs +++ /dev/null @@ -1,22 +0,0 @@ -using System.Net; -using JetBrains.Annotations; -using JsonApiDotNetCore.Serialization.Objects; - -namespace JsonApiDotNetCore.Errors -{ - /// - /// The error that is thrown when the resource ID in the request body does not match the ID in the current endpoint URL. - /// - [PublicAPI] - public sealed class ResourceIdMismatchException : JsonApiException - { - public ResourceIdMismatchException(string bodyId, string endpointId, string requestPath) - : base(new ErrorObject(HttpStatusCode.Conflict) - { - Title = "Resource ID mismatch between request body and endpoint URL.", - Detail = $"Expected resource ID '{endpointId}' in PATCH request body at endpoint '{requestPath}', instead of '{bodyId}'." - }) - { - } - } -} diff --git a/src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs b/src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs index 73b484e2e7..54c99424c5 100644 --- a/src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs +++ b/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; diff --git a/src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceIdentityAdapter.cs b/src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceIdentityAdapter.cs index 2df546d9a1..9e6c52b16b 100644 --- a/src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceIdentityAdapter.cs +++ b/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); } } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Updating/Resources/AtomicUpdateResourceTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Updating/Resources/AtomicUpdateResourceTests.cs index 3603744066..7200e0d9b8 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Updating/Resources/AtomicUpdateResourceTests.cs +++ b/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(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(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] diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateResourceTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateResourceTests.cs index 5e840f18c3..4a76ad472d 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateResourceTests.cs +++ b/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"); }