Skip to content

Fixed: Produce error on invalid ID in request body #1593

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 4 commits into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Fix usage of whitespace IDs when TId is string
  • Loading branch information
bkoelman committed Jul 14, 2024
commit bce1d8f4814abc5ed9c37d9ab2a003bf78f2bf7f
26 changes: 14 additions & 12 deletions src/JsonApiDotNetCore/Controllers/JsonApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ public override async Task<IActionResult> GetAsync(CancellationToken cancellatio
/// <inheritdoc />
[HttpGet("{id}")]
[HttpHead("{id}")]
public override async Task<IActionResult> GetAsync([Required] [DisallowNull] TId id, CancellationToken cancellationToken)
public override async Task<IActionResult> GetAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id, CancellationToken cancellationToken)
{
return await base.GetAsync(id, cancellationToken);
}

/// <inheritdoc />
[HttpGet("{id}/{relationshipName}")]
[HttpHead("{id}/{relationshipName}")]
public override async Task<IActionResult> GetSecondaryAsync([Required] [DisallowNull] TId id, [Required] string relationshipName,
public override async Task<IActionResult> GetSecondaryAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id, [Required] string relationshipName,
CancellationToken cancellationToken)
{
return await base.GetSecondaryAsync(id, relationshipName, cancellationToken);
Expand All @@ -68,8 +68,8 @@ public override async Task<IActionResult> GetSecondaryAsync([Required] [Disallow
/// <inheritdoc />
[HttpGet("{id}/relationships/{relationshipName}")]
[HttpHead("{id}/relationships/{relationshipName}")]
public override async Task<IActionResult> GetRelationshipAsync([Required] [DisallowNull] TId id, [Required] string relationshipName,
CancellationToken cancellationToken)
public override async Task<IActionResult> GetRelationshipAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id,
[Required] string relationshipName, CancellationToken cancellationToken)
{
return await base.GetRelationshipAsync(id, relationshipName, cancellationToken);
}
Expand All @@ -83,39 +83,41 @@ public override async Task<IActionResult> PostAsync([Required] TResource resourc

/// <inheritdoc />
[HttpPost("{id}/relationships/{relationshipName}")]
public override async Task<IActionResult> PostRelationshipAsync([Required] [DisallowNull] TId id, [Required] string relationshipName,
[Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
public override async Task<IActionResult> PostRelationshipAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id,
[Required] string relationshipName, [Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
{
return await base.PostRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken);
}

/// <inheritdoc />
[HttpPatch("{id}")]
public override async Task<IActionResult> PatchAsync([Required] [DisallowNull] TId id, [Required] TResource resource, CancellationToken cancellationToken)
public override async Task<IActionResult> PatchAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id, [Required] TResource resource,
CancellationToken cancellationToken)
{
return await base.PatchAsync(id, resource, cancellationToken);
}

/// <inheritdoc />
[HttpPatch("{id}/relationships/{relationshipName}")]
// `AllowEmptyStrings = true` in `[Required]` prevents the model binder from producing a validation error on whitespace when TId is string.
// Parameter `[Required] object? rightValue` makes Swashbuckle generate the OpenAPI request body as required. We don't actually validate ModelState, so it doesn't hurt.
public override async Task<IActionResult> PatchRelationshipAsync([Required] [DisallowNull] TId id, [Required] string relationshipName,
[Required] object? rightValue, CancellationToken cancellationToken)
public override async Task<IActionResult> PatchRelationshipAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id,
[Required] string relationshipName, [Required] object? rightValue, CancellationToken cancellationToken)
{
return await base.PatchRelationshipAsync(id, relationshipName, rightValue, cancellationToken);
}

/// <inheritdoc />
[HttpDelete("{id}")]
public override async Task<IActionResult> DeleteAsync([Required] [DisallowNull] TId id, CancellationToken cancellationToken)
public override async Task<IActionResult> DeleteAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id, CancellationToken cancellationToken)
{
return await base.DeleteAsync(id, cancellationToken);
}

/// <inheritdoc />
[HttpDelete("{id}/relationships/{relationshipName}")]
public override async Task<IActionResult> DeleteRelationshipAsync([Required] [DisallowNull] TId id, [Required] string relationshipName,
[Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
public override async Task<IActionResult> DeleteRelationshipAsync([Required(AllowEmptyStrings = true)] [DisallowNull] TId id,
[Required] string relationshipName, [Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
{
return await base.DeleteRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,9 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
Map? gameInDatabase = await dbContext.Maps.FirstWithIdOrDefaultAsync(existingMap.Id);
Map? mapInDatabase = await dbContext.Maps.FirstWithIdOrDefaultAsync(existingMap.Id);

gameInDatabase.Should().BeNull();
mapInDatabase.Should().BeNull();
});
}
}
3 changes: 3 additions & 0 deletions test/JsonApiDotNetCoreTests/IntegrationTests/ZeroKeys/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ public sealed class Game : Identifiable<int?>
[Attr]
public Guid SessionToken => Guid.NewGuid();

[HasOne]
public Player? Host { get; set; }

[HasMany]
public ICollection<Player> ActivePlayers { get; set; } = new List<Player>();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using JetBrains.Annotations;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;

namespace JsonApiDotNetCoreTests.IntegrationTests.ZeroKeys;

[UsedImplicitly(ImplicitUseTargetFlags.Members)]
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ZeroKeys")]
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ZeroKeys", ClientIdGeneration = ClientIdGenerationMode.Allowed)]
public sealed class Player : Identifiable<string?>
{
[Attr]
Expand Down
Loading
Loading