Skip to content

Commit

Permalink
Merge pull request #1581 from json-api-dotnet/block-lid-on-required-c…
Browse files Browse the repository at this point in the history
…lient-id

Fixed: Do not allow the use of 'lid' when client-generated IDs are required
  • Loading branch information
bkoelman authored Jun 30, 2024
2 parents 491b003 + 7302906 commit 479066f
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ private ResourceIdentityRequirements CreateRefRequirements(RequestAdapterState s
return new ResourceIdentityRequirements
{
EvaluateIdConstraint = resourceType =>
ResourceIdentityRequirements.DoEvaluateIdConstraint(resourceType, state.Request.WriteOperation, _options.ClientIdGeneration)
ResourceIdentityRequirements.DoEvaluateIdConstraint(resourceType, state.Request.WriteOperation, _options.ClientIdGeneration),
EvaluateAllowLid = resourceType =>
ResourceIdentityRequirements.DoEvaluateAllowLid(resourceType, state.Request.WriteOperation, _options.ClientIdGeneration)
};
}

Expand All @@ -135,6 +137,7 @@ private static ResourceIdentityRequirements CreateDataRequirements(AtomicReferen
{
ResourceType = refResult.ResourceType,
EvaluateIdConstraint = refRequirements.EvaluateIdConstraint,
EvaluateAllowLid = refRequirements.EvaluateAllowLid,
IdValue = refResult.Resource.StringId,
LidValue = refResult.Resource.LocalId,
RelationshipName = refResult.Relationship?.PublicName
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections;
using JsonApiDotNetCore.Middleware;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;
using JsonApiDotNetCore.Serialization.Objects;
Expand Down Expand Up @@ -71,6 +72,7 @@ private static SingleOrManyData<ResourceIdentifierObject> ToIdentifierData(Singl
{
ResourceType = relationship.RightType,
EvaluateIdConstraint = _ => JsonElementConstraint.Required,
EvaluateAllowLid = _ => state.Request.Kind == EndpointKind.AtomicOperations,
RelationshipName = relationship.PublicName
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,20 @@ private static void AssertIsCompatibleResourceType(ResourceType actual, Resource
private IIdentifiable CreateResource(ResourceIdentity identity, ResourceIdentityRequirements requirements, ResourceType resourceType,
RequestAdapterState state)
{
if (state.Request.Kind != EndpointKind.AtomicOperations)
AssertNoIdWithLid(identity, state);

bool allowLid = requirements.EvaluateAllowLid?.Invoke(resourceType) ?? false;

if (!allowLid)
{
AssertHasNoLid(identity, state);
}

AssertNoIdWithLid(identity, state);

JsonElementConstraint? idConstraint = requirements.EvaluateIdConstraint?.Invoke(resourceType);

if (idConstraint == JsonElementConstraint.Required)
{
AssertHasIdOrLid(identity, requirements, state);
AssertHasIdOrLid(identity, requirements, allowLid, state);
}
else if (idConstraint == JsonElementConstraint.Forbidden)
{
Expand All @@ -128,7 +130,10 @@ private static void AssertHasNoLid(ResourceIdentity identity, RequestAdapterStat
if (identity.Lid != null)
{
using IDisposable _ = state.Position.PushElement("lid");
throw new ModelConversionException(state.Position, "The 'lid' element is not supported at this endpoint.", null);

throw state.Request.Kind == EndpointKind.AtomicOperations
? new ModelConversionException(state.Position, "The 'lid' element cannot be used because a client-generated ID is required.", null)
: new ModelConversionException(state.Position, "The 'lid' element is not supported at this endpoint.", null);
}
}

Expand All @@ -140,7 +145,7 @@ private static void AssertNoIdWithLid(ResourceIdentity identity, RequestAdapterS
}
}

private static void AssertHasIdOrLid(ResourceIdentity identity, ResourceIdentityRequirements requirements, RequestAdapterState state)
private static void AssertHasIdOrLid(ResourceIdentity identity, ResourceIdentityRequirements requirements, bool allowLid, RequestAdapterState state)
{
string? message = null;

Expand All @@ -154,7 +159,7 @@ private static void AssertHasIdOrLid(ResourceIdentity identity, ResourceIdentity
}
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.";
message = allowLid ? "The 'id' or 'lid' element is required." : "The 'id' element is required.";
}

if (message != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public sealed class ResourceIdentityRequirements
/// </summary>
public Func<ResourceType, JsonElementConstraint?>? EvaluateIdConstraint { get; init; }

/// <summary>
/// When not null, provides a callback to indicate whether the "lid" element can be used instead of the "id" element. Defaults to <c>false</c>.
/// </summary>
public Func<ResourceType, bool>? EvaluateAllowLid { get; init; }

/// <summary>
/// When not null, indicates what the value of the "id" element must be.
/// </summary>
Expand Down Expand Up @@ -50,4 +55,15 @@ public sealed class ResourceIdentityRequirements
}
: JsonElementConstraint.Required;
}

internal static bool DoEvaluateAllowLid(ResourceType resourceType, WriteOperationKind? writeOperation, ClientIdGenerationMode globalClientIdGeneration)
{
if (writeOperation == null)
{
return false;
}

ClientIdGenerationMode clientIdGeneration = resourceType.ClientIdGeneration ?? globalClientIdGeneration;
return !(writeOperation == WriteOperationKind.CreateResource && clientIdGeneration == ClientIdGenerationMode.Required);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public async Task Cannot_create_resource_for_missing_client_generated_ID()

ErrorObject error = responseDocument.Errors[0];
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
error.Title.Should().Be("Failed to deserialize request body: The 'id' or 'lid' element is required.");
error.Title.Should().Be("Failed to deserialize request body: The 'id' element is required.");
error.Detail.Should().BeNull();
error.Source.ShouldNotBeNull();
error.Source.Pointer.Should().Be("/atomic:operations[0]/data");
Expand Down Expand Up @@ -281,6 +281,45 @@ public async Task Cannot_create_resource_for_incompatible_ID()
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
}

[Fact]
public async Task Cannot_create_resource_with_local_ID()
{
// Arrange
var requestBody = new
{
atomic__operations = new[]
{
new
{
op = "add",
data = new
{
type = "textLanguages",
lid = "new-server-id"
}
}
}
};

const string route = "/operations";

// Act
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);

// Assert
httpResponse.ShouldHaveStatusCode(HttpStatusCode.UnprocessableEntity);

responseDocument.Errors.ShouldHaveCount(1);

ErrorObject error = responseDocument.Errors[0];
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
error.Title.Should().Be("Failed to deserialize request body: The 'lid' element cannot be used because a client-generated ID is required.");
error.Detail.Should().BeNull();
error.Source.ShouldNotBeNull();
error.Source.Pointer.Should().Be("/atomic:operations[0]/data/lid");
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
}

[Fact]
public async Task Cannot_create_resource_for_ID_and_local_ID()
{
Expand Down

0 comments on commit 479066f

Please sign in to comment.