From 6bf4a8cd238bfcefb2784bdb720863386c571f5a Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Sun, 17 Mar 2024 00:40:10 +0100 Subject: [PATCH] Move back [FromBody] and [Required] to derived controllers, reverting most of the previous PR (#1503). It turns out that ASP.NET ModelState doesn't look at attributes on base parameters --- .../Controllers/BaseJsonApiController.cs | 24 +++++++++---------- .../BaseJsonApiOperationsController.cs | 4 +--- .../Controllers/JsonApiController.cs | 24 ++++++++++--------- .../JsonApiOperationsController.cs | 3 ++- .../ObfuscatedIdentifiableController.cs | 22 +++++++++-------- 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index 1d518c6194..5fee532b36 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -1,4 +1,3 @@ -using System.ComponentModel.DataAnnotations; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Middleware; @@ -107,7 +106,7 @@ public virtual async Task GetAsync(CancellationToken cancellation /// GET /articles/1 HTTP/1.1 /// ]]> /// - public virtual async Task GetAsync([Required] TId id, CancellationToken cancellationToken) + public virtual async Task GetAsync(TId id, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -132,7 +131,7 @@ public virtual async Task GetAsync([Required] TId id, Cancellatio /// GET /articles/1/revisions HTTP/1.1 /// ]]> /// - public virtual async Task GetSecondaryAsync([Required] TId id, [Required] string relationshipName, CancellationToken cancellationToken) + public virtual async Task GetSecondaryAsync(TId id, string relationshipName, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -161,7 +160,7 @@ public virtual async Task GetSecondaryAsync([Required] TId id, [R /// GET /articles/1/relationships/revisions HTTP/1.1 /// ]]> /// - public virtual async Task GetRelationshipAsync([Required] TId id, [Required] string relationshipName, CancellationToken cancellationToken) + public virtual async Task GetRelationshipAsync(TId id, string relationshipName, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -186,7 +185,7 @@ public virtual async Task GetRelationshipAsync([Required] TId id, /// POST /articles HTTP/1.1 /// ]]> /// - public virtual async Task PostAsync([FromBody] [Required] TResource resource, CancellationToken cancellationToken) + public virtual async Task PostAsync(TResource resource, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -236,8 +235,8 @@ public virtual async Task PostAsync([FromBody] [Required] TResour /// /// Propagates notification that request handling should be canceled. /// - public virtual async Task PostRelationshipAsync([Required] TId id, [Required] string relationshipName, - [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) + public virtual async Task PostRelationshipAsync(TId id, string relationshipName, ISet rightResourceIds, + CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -265,7 +264,7 @@ public virtual async Task PostRelationshipAsync([Required] TId id /// PATCH /articles/1 HTTP/1.1 /// ]]> /// - public virtual async Task PatchAsync([Required] TId id, [FromBody] [Required] TResource resource, CancellationToken cancellationToken) + public virtual async Task PatchAsync(TId id, TResource resource, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -311,8 +310,7 @@ public virtual async Task PatchAsync([Required] TId id, [FromBody /// /// Propagates notification that request handling should be canceled. /// - public virtual async Task PatchRelationshipAsync([Required] TId id, [Required] string relationshipName, [FromBody] object? rightValue, - CancellationToken cancellationToken) + public virtual async Task PatchRelationshipAsync(TId id, string relationshipName, object? rightValue, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -338,7 +336,7 @@ public virtual async Task PatchRelationshipAsync([Required] TId i /// DELETE /articles/1 HTTP/1.1 /// ]]> /// - public virtual async Task DeleteAsync([Required] TId id, CancellationToken cancellationToken) + public virtual async Task DeleteAsync(TId id, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { @@ -372,8 +370,8 @@ public virtual async Task DeleteAsync([Required] TId id, Cancella /// /// Propagates notification that request handling should be canceled. /// - public virtual async Task DeleteRelationshipAsync([Required] TId id, [Required] string relationshipName, - [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) + public virtual async Task DeleteRelationshipAsync(TId id, string relationshipName, ISet rightResourceIds, + CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiOperationsController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiOperationsController.cs index f16a2a6686..1e7da80bd1 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiOperationsController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiOperationsController.cs @@ -1,4 +1,3 @@ -using System.ComponentModel.DataAnnotations; using JetBrains.Annotations; using JsonApiDotNetCore.AtomicOperations; using JsonApiDotNetCore.Configuration; @@ -103,8 +102,7 @@ protected BaseJsonApiOperationsController(IJsonApiOptions options, IResourceGrap /// } /// ]]> /// - public virtual async Task PostOperationsAsync([FromBody] [Required] IList operations, - CancellationToken cancellationToken) + public virtual async Task PostOperationsAsync(IList operations, CancellationToken cancellationToken) { _traceWriter.LogMethodStart(new { diff --git a/src/JsonApiDotNetCore/Controllers/JsonApiController.cs b/src/JsonApiDotNetCore/Controllers/JsonApiController.cs index 82719bfeac..0c56500797 100644 --- a/src/JsonApiDotNetCore/Controllers/JsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/JsonApiController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel.DataAnnotations; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Services; @@ -49,7 +50,7 @@ public override Task GetAsync(CancellationToken cancellationToken /// [HttpGet("{id}")] [HttpHead("{id}")] - public override Task GetAsync(TId id, CancellationToken cancellationToken) + public override Task GetAsync([Required] TId id, CancellationToken cancellationToken) { return base.GetAsync(id, cancellationToken); } @@ -57,7 +58,7 @@ public override Task GetAsync(TId id, CancellationToken cancellat /// [HttpGet("{id}/{relationshipName}")] [HttpHead("{id}/{relationshipName}")] - public override Task GetSecondaryAsync(TId id, string relationshipName, CancellationToken cancellationToken) + public override Task GetSecondaryAsync([Required] TId id, [Required] string relationshipName, CancellationToken cancellationToken) { return base.GetSecondaryAsync(id, relationshipName, cancellationToken); } @@ -65,36 +66,37 @@ public override Task GetSecondaryAsync(TId id, string relationshi /// [HttpGet("{id}/relationships/{relationshipName}")] [HttpHead("{id}/relationships/{relationshipName}")] - public override Task GetRelationshipAsync(TId id, string relationshipName, CancellationToken cancellationToken) + public override Task GetRelationshipAsync([Required] TId id, [Required] string relationshipName, CancellationToken cancellationToken) { return base.GetRelationshipAsync(id, relationshipName, cancellationToken); } /// [HttpPost] - public override Task PostAsync(TResource resource, CancellationToken cancellationToken) + public override Task PostAsync([FromBody] [Required] TResource resource, CancellationToken cancellationToken) { return base.PostAsync(resource, cancellationToken); } /// [HttpPost("{id}/relationships/{relationshipName}")] - public override Task PostRelationshipAsync(TId id, string relationshipName, ISet rightResourceIds, - CancellationToken cancellationToken) + public override Task PostRelationshipAsync([Required] TId id, [Required] string relationshipName, + [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) { return base.PostRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken); } /// [HttpPatch("{id}")] - public override Task PatchAsync(TId id, TResource resource, CancellationToken cancellationToken) + public override Task PatchAsync([Required] TId id, [FromBody] [Required] TResource resource, CancellationToken cancellationToken) { return base.PatchAsync(id, resource, cancellationToken); } /// [HttpPatch("{id}/relationships/{relationshipName}")] - public override Task PatchRelationshipAsync(TId id, string relationshipName, [FromBody] object? rightValue, + // 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 Task PatchRelationshipAsync([Required] TId id, [Required] string relationshipName, [FromBody] [Required] object? rightValue, CancellationToken cancellationToken) { return base.PatchRelationshipAsync(id, relationshipName, rightValue, cancellationToken); @@ -102,15 +104,15 @@ public override Task PatchRelationshipAsync(TId id, string relati /// [HttpDelete("{id}")] - public override Task DeleteAsync(TId id, CancellationToken cancellationToken) + public override Task DeleteAsync([Required] TId id, CancellationToken cancellationToken) { return base.DeleteAsync(id, cancellationToken); } /// [HttpDelete("{id}/relationships/{relationshipName}")] - public override Task DeleteRelationshipAsync(TId id, string relationshipName, ISet rightResourceIds, - CancellationToken cancellationToken) + public override Task DeleteRelationshipAsync([Required] TId id, [Required] string relationshipName, + [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) { return base.DeleteRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken); } diff --git a/src/JsonApiDotNetCore/Controllers/JsonApiOperationsController.cs b/src/JsonApiDotNetCore/Controllers/JsonApiOperationsController.cs index bacb5b9342..e2a0417ad2 100644 --- a/src/JsonApiDotNetCore/Controllers/JsonApiOperationsController.cs +++ b/src/JsonApiDotNetCore/Controllers/JsonApiOperationsController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel.DataAnnotations; using JsonApiDotNetCore.AtomicOperations; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Middleware; @@ -17,7 +18,7 @@ public abstract class JsonApiOperationsController( { /// [HttpPost] - public override Task PostOperationsAsync(IList operations, CancellationToken cancellationToken) + public override Task PostOperationsAsync([FromBody] [Required] IList operations, CancellationToken cancellationToken) { return base.PostOperationsAsync(operations, cancellationToken); } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/IdObfuscation/ObfuscatedIdentifiableController.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/IdObfuscation/ObfuscatedIdentifiableController.cs index 46391aab7c..1c93a9c08c 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/IdObfuscation/ObfuscatedIdentifiableController.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/IdObfuscation/ObfuscatedIdentifiableController.cs @@ -22,21 +22,21 @@ public override Task GetAsync(CancellationToken cancellationToken } [HttpGet("{id}")] - public Task GetAsync(string id, CancellationToken cancellationToken) + public Task GetAsync([Required] string id, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.GetAsync(idValue, cancellationToken); } [HttpGet("{id}/{relationshipName}")] - public Task GetSecondaryAsync(string id, string relationshipName, CancellationToken cancellationToken) + public Task GetSecondaryAsync([Required] string id, [Required] string relationshipName, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.GetSecondaryAsync(idValue, relationshipName, cancellationToken); } [HttpGet("{id}/relationships/{relationshipName}")] - public Task GetRelationshipAsync(string id, string relationshipName, CancellationToken cancellationToken) + public Task GetRelationshipAsync([Required] string id, [Required] string relationshipName, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.GetRelationshipAsync(idValue, relationshipName, cancellationToken); @@ -49,37 +49,39 @@ public override Task PostAsync([FromBody] [Required] TResource re } [HttpPost("{id}/relationships/{relationshipName}")] - public Task PostRelationshipAsync(string id, string relationshipName, [FromBody] [Required] ISet rightResourceIds, - CancellationToken cancellationToken) + public Task PostRelationshipAsync([Required] string id, [Required] string relationshipName, + [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.PostRelationshipAsync(idValue, relationshipName, rightResourceIds, cancellationToken); } [HttpPatch("{id}")] - public Task PatchAsync(string id, [FromBody] [Required] TResource resource, CancellationToken cancellationToken) + public Task PatchAsync([Required] string id, [FromBody] [Required] TResource resource, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.PatchAsync(idValue, resource, cancellationToken); } [HttpPatch("{id}/relationships/{relationshipName}")] - public Task PatchRelationshipAsync(string id, string relationshipName, [FromBody] object? rightValue, CancellationToken cancellationToken) + // 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 Task PatchRelationshipAsync([Required] string id, [Required] string relationshipName, [FromBody] [Required] object? rightValue, + CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.PatchRelationshipAsync(idValue, relationshipName, rightValue, cancellationToken); } [HttpDelete("{id}")] - public Task DeleteAsync(string id, CancellationToken cancellationToken) + public Task DeleteAsync([Required] string id, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.DeleteAsync(idValue, cancellationToken); } [HttpDelete("{id}/relationships/{relationshipName}")] - public Task DeleteRelationshipAsync(string id, string relationshipName, [FromBody] [Required] ISet rightResourceIds, - CancellationToken cancellationToken) + public Task DeleteRelationshipAsync([Required] string id, [Required] string relationshipName, + [FromBody] [Required] ISet rightResourceIds, CancellationToken cancellationToken) { int idValue = _codec.Decode(id); return base.DeleteRelationshipAsync(idValue, relationshipName, rightResourceIds, cancellationToken);