Skip to content

Commit

Permalink
Unified error messages about mismatches in 'id' and 'lid' values
Browse files Browse the repository at this point in the history
  • Loading branch information
Bart Koelman committed Sep 29, 2021
1 parent 05a639d commit 5f618d2
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 116 deletions.
22 changes: 0 additions & 22 deletions src/JsonApiDotNetCore/Errors/ResourceIdMismatchException.cs
View file

This file was deleted.

1 change: 0 additions & 1 deletion src/JsonApiDotNetCore/Serialization/RequestAdapters/RelationshipDataAdapter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
145 changes: 68 additions & 77 deletions src/JsonApiDotNetCore/Serialization/RequestAdapters/ResourceIdentityAdapter.cs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down
28 changes: 14 additions & 14 deletions ...reTests/IntegrationTests/AtomicOperations/Updating/Resources/AtomicUpdateResourceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions ...onApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateResourceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down

0 comments on commit 5f618d2

Please sign in to comment.