Skip to content

Fixed: Do not allow the use of 'lid' when client-generated IDs are required #1581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading