From a050acd5ba768dec2e466771e2e07662a601ba5c Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 15:46:35 -0500 Subject: [PATCH 1/7] feat: Error should accept objects for source --- src/JsonApiDotNetCore/Internal/Error.cs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/JsonApiDotNetCore/Internal/Error.cs b/src/JsonApiDotNetCore/Internal/Error.cs index 999611d79e..71852e28ea 100644 --- a/src/JsonApiDotNetCore/Internal/Error.cs +++ b/src/JsonApiDotNetCore/Internal/Error.cs @@ -9,9 +9,9 @@ public class Error { public Error() { } - + [Obsolete("Use Error constructors with int typed status")] - public Error(string status, string title, ErrorMeta meta = null, string source = null) + public Error(string status, string title, ErrorMeta meta = null, object source = null) { Status = status; Title = title; @@ -19,7 +19,7 @@ public Error(string status, string title, ErrorMeta meta = null, string source = Source = source; } - public Error(int status, string title, ErrorMeta meta = null, string source = null) + public Error(int status, string title, ErrorMeta meta = null, object source = null) { Status = status.ToString(); Title = title; @@ -28,7 +28,7 @@ public Error(int status, string title, ErrorMeta meta = null, string source = nu } [Obsolete("Use Error constructors with int typed status")] - public Error(string status, string title, string detail, ErrorMeta meta = null, string source = null) + public Error(string status, string title, string detail, ErrorMeta meta = null, object source = null) { Status = status; Title = title; @@ -37,7 +37,7 @@ public Error(string status, string title, string detail, ErrorMeta meta = null, Source = source; } - public Error(int status, string title, string detail, ErrorMeta meta = null, string source = null) + public Error(int status, string title, string detail, ErrorMeta meta = null, object source = null) { Status = status.ToString(); Title = title; @@ -45,13 +45,13 @@ public Error(int status, string title, string detail, ErrorMeta meta = null, str Meta = meta; Source = source; } - + [JsonProperty("title")] public string Title { get; set; } [JsonProperty("detail")] public string Detail { get; set; } - + [JsonProperty("status")] public string Status { get; set; } @@ -59,7 +59,7 @@ public Error(int status, string title, string detail, ErrorMeta meta = null, str public int StatusCode => int.Parse(Status); [JsonProperty("source")] - public string Source { get; set; } + public object Source { get; set; } [JsonProperty("meta")] public ErrorMeta Meta { get; set; } @@ -73,8 +73,8 @@ public class ErrorMeta [JsonProperty("stackTrace")] public string[] StackTrace { get; set; } - public static ErrorMeta FromException(Exception e) - => new ErrorMeta { + public static ErrorMeta FromException(Exception e) + => new ErrorMeta { StackTrace = e.Demystify().ToString().Split(new[] { "\n"}, int.MaxValue, StringSplitOptions.RemoveEmptyEntries) }; } From 79fb2856af7f9b475396f7dbcff31eb37be660a4 Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 17:20:06 -0500 Subject: [PATCH 2/7] feat: add 'GetPublicAttributeName' to ContextGraph --- src/JsonApiDotNetCore/Internal/ContextGraph.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index c0c7f2274b..6a39a292de 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -7,7 +7,7 @@ namespace JsonApiDotNetCore.Internal public interface IContextGraph { /// - /// Gets the value of the navigation property, defined by the relationshipName, + /// Gets the value of the navigation property, defined by the relationshipName, /// on the provided instance. /// /// The resource instance @@ -42,6 +42,12 @@ public interface IContextGraph /// ContextEntity GetContextEntity(Type entityType); + /// + /// Get the public attribute name for a type based on the internal attribute name. + /// + /// The internal attribute name for a . + string GetPublicAttributeName(string internalAttributeName); + /// /// Was built against an EntityFrameworkCore DbContext ? /// @@ -111,5 +117,13 @@ public string GetRelationshipName(string relationshipName) .SingleOrDefault(r => r.Is(relationshipName)) ?.InternalRelationshipName; } - } + + public string GetPublicAttributeName(string internalAttributeName) + { + return GetContextEntity(typeof(TParent)) + .Attributes + .Single(a => a.InternalAttributeName == internalAttributeName) + .PublicAttributeName; + } + } } From 30c531d490cf005722362c2eef408fe602867ec9 Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 17:31:01 -0500 Subject: [PATCH 3/7] wip: new 422 validation errors with source.pointer --- .../Controllers/BaseJsonApiController.cs | 8 ++++---- .../Extensions/ModelStateExtensions.cs | 13 +++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index fd6ec8947a..de6aab46e1 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -8,7 +8,7 @@ namespace JsonApiDotNetCore.Controllers { - public class BaseJsonApiController + public class BaseJsonApiController : BaseJsonApiController where T : class, IIdentifiable { @@ -47,7 +47,7 @@ public class BaseJsonApiController private readonly ICreateService _create; private readonly IUpdateService _update; private readonly IUpdateRelationshipService _updateRelationships; - private readonly IDeleteService _delete; + private readonly IDeleteService _delete; private readonly IJsonApiContext _jsonApiContext; public BaseJsonApiController( @@ -156,7 +156,7 @@ public virtual async Task PostAsync([FromBody] T entity) return Forbidden(); if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection()); + return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); entity = await _create.CreateAsync(entity); @@ -170,7 +170,7 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection()); + return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index d67f7e66c4..0d5701013d 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -6,7 +6,7 @@ namespace JsonApiDotNetCore.Extensions { public static class ModelStateExtensions { - public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState) + public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState, IContextGraph contextGraph) { ErrorCollection collection = new ErrorCollection(); foreach (var entry in modelState) @@ -16,10 +16,19 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary foreach (var modelError in entry.Value.Errors) { + var attrName = entry.Key; + if (modelError.Exception is JsonApiException jex) collection.Errors.AddRange(jex.GetError().Errors); else - collection.Errors.Add(new Error(400, entry.Key, modelError.ErrorMessage, modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null)); + collection.Errors.Add(new Error( + status: 422, + title: entry.Key, + detail: modelError.ErrorMessage, + meta: modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null, + source: new { + pointer = $"/data/attributes/{attrName}" + })); } } From 0e170593cc992f2749ffd634ef31e9e5e56c0665 Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 17:46:34 -0500 Subject: [PATCH 4/7] feat: validations send source.pointer --- src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs | 4 ++-- src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index de6aab46e1..407899051d 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -156,7 +156,7 @@ public virtual async Task PostAsync([FromBody] T entity) return Forbidden(); if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); + return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); entity = await _create.CreateAsync(entity); @@ -170,7 +170,7 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); + return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index 0d5701013d..47f56a2570 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -6,7 +6,7 @@ namespace JsonApiDotNetCore.Extensions { public static class ModelStateExtensions { - public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState, IContextGraph contextGraph) + public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState, IContextGraph contextGraph) { ErrorCollection collection = new ErrorCollection(); foreach (var entry in modelState) @@ -16,7 +16,7 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary foreach (var modelError in entry.Value.Errors) { - var attrName = entry.Key; + var attrName =contextGraph.GetPublicAttributeName(entry.Key); if (modelError.Exception is JsonApiException jex) collection.Errors.AddRange(jex.GetError().Errors); From b841433be399374feb70390e9ca5a990a883f0a3 Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 18:51:20 -0500 Subject: [PATCH 5/7] fix: handle 'GetPublicAttributeName' null and mock for tests --- .../Extensions/ModelStateExtensions.cs | 6 +- .../Internal/ContextGraph.cs | 2 +- .../BaseJsonApiController_Tests.cs | 696 +++++++++--------- 3 files changed, 356 insertions(+), 348 deletions(-) diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index 47f56a2570..3a742617db 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -14,10 +14,10 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDiction if (entry.Value.Errors.Any() == false) continue; + var attrName = contextGraph.GetPublicAttributeName(entry.Key); + foreach (var modelError in entry.Value.Errors) { - var attrName =contextGraph.GetPublicAttributeName(entry.Key); - if (modelError.Exception is JsonApiException jex) collection.Errors.AddRange(jex.GetError().Errors); else @@ -26,7 +26,7 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDiction title: entry.Key, detail: modelError.ErrorMessage, meta: modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null, - source: new { + source: attrName == null ? null : new { pointer = $"/data/attributes/{attrName}" })); } diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index 6a39a292de..4b6a310527 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -122,7 +122,7 @@ public string GetPublicAttributeName(string internalAttributeName) { return GetContextEntity(typeof(TParent)) .Attributes - .Single(a => a.InternalAttributeName == internalAttributeName) + .SingleOrDefault(a => a.InternalAttributeName == internalAttributeName)? .PublicAttributeName; } } diff --git a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs index 873b3f50d2..c0f0c0cbe6 100644 --- a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs +++ b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs @@ -1,344 +1,352 @@ -using JsonApiDotNetCore.Controllers; -using JsonApiDotNetCore.Models; -using JsonApiDotNetCore.Services; -using Moq; -using Xunit; -using System.Threading.Tasks; -using JsonApiDotNetCore.Configuration; -using JsonApiDotNetCore.Internal; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; - -namespace UnitTests -{ - public class BaseJsonApiController_Tests - { - public class Resource : Identifiable { } - private Mock _jsonApiContextMock = new Mock(); - - [Fact] - public async Task GetAsync_Calls_Service() - { - // arrange - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getAll: serviceMock.Object); - - // act - await controller.GetAsync(); - - // assert - serviceMock.Verify(m => m.GetAsync(), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task GetAsync_Throws_405_If_No_Service() - { - // arrange - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.GetAsync()); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task GetAsyncById_Calls_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getById: serviceMock.Object); - - // act - await controller.GetAsync(id); - - // assert - serviceMock.Verify(m => m.GetAsync(id), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task GetAsyncById_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getById: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.GetAsync(id)); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task GetRelationshipsAsync_Calls_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationships: serviceMock.Object); - - // act - await controller.GetRelationshipsAsync(id, string.Empty); - - // assert - serviceMock.Verify(m => m.GetRelationshipsAsync(id, string.Empty), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task GetRelationshipsAsync_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationships: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.GetRelationshipsAsync(id, string.Empty)); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task GetRelationshipAsync_Calls_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationship: serviceMock.Object); - - // act - await controller.GetRelationshipAsync(id, string.Empty); - - // assert - serviceMock.Verify(m => m.GetRelationshipAsync(id, string.Empty), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task GetRelationshipAsync_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationship: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.GetRelationshipAsync(id, string.Empty)); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task PatchAsync_Calls_Service() - { - // arrange - const int id = 0; - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); - - // act - await controller.PatchAsync(id, resource); - - // assert - serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task PatchAsync_ModelStateInvalid_ValidateModelStateDisbled() - { - // arrange - const int id = 0; - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); - - // act - var response = await controller.PatchAsync(id, resource); - - // assert - serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Once); - VerifyApplyContext(); - Assert.IsNotType(response); - } - - [Fact] - public async Task PatchAsync_ModelStateInvalid_ValidateModelStateEnabled() - { - // arrange - const int id = 0; - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions{ValidateModelState = true}); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); - controller.ModelState.AddModelError("Id", "Failed Validation"); - - // act - var response = await controller.PatchAsync(id, resource); - - // assert - serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Never); - Assert.IsType(response); - Assert.IsType(((BadRequestObjectResult) response).Value); - } - - [Fact] - public async Task PatchAsync_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.PatchAsync(id, It.IsAny())); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task PostAsync_Calls_Service() - { - // arrange - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); - serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); - controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext {HttpContext = new DefaultHttpContext()}; - - // act - await controller.PostAsync(resource); - - // assert - serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task PostAsync_ModelStateInvalid_ValidateModelStateDisabled() - { - // arrange - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); - serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); - controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext { HttpContext = new DefaultHttpContext() }; - - // act - var response = await controller.PostAsync(resource); - - // assert - serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); - VerifyApplyContext(); - Assert.IsNotType(response); - } - - [Fact] - public async Task PostAsync_ModelStateInvalid_ValidateModelStateEnabled() - { - // arrange - var resource = new Resource(); - var serviceMock = new Mock>(); - _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = true }); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); - controller.ModelState.AddModelError("Id", "Failed Validation"); - - // act - var response = await controller.PostAsync(resource); - - // assert - serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Never); - Assert.IsType(response); - Assert.IsType(((BadRequestObjectResult)response).Value); - } - - [Fact] - public async Task PatchRelationshipsAsync_Calls_Service() - { - // arrange - const int id = 0; - var resource = new Resource(); - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, updateRelationships: serviceMock.Object); - - // act - await controller.PatchRelationshipsAsync(id, string.Empty, null); - - // assert - serviceMock.Verify(m => m.UpdateRelationshipsAsync(id, string.Empty, null), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task PatchRelationshipsAsync_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, updateRelationships: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.PatchRelationshipsAsync(id, string.Empty, null)); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - [Fact] - public async Task DeleteAsync_Calls_Service() - { - // arrange - const int id = 0; - var resource = new Resource(); - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, delete: serviceMock.Object); - - // act - await controller.DeleteAsync(id); - - // assert - serviceMock.Verify(m => m.DeleteAsync(id), Times.Once); - VerifyApplyContext(); - } - - [Fact] - public async Task DeleteAsync_Throws_405_If_No_Service() - { - // arrange - const int id = 0; - var serviceMock = new Mock>(); - var controller = new BaseJsonApiController(_jsonApiContextMock.Object, delete: null); - - // act - var exception = await Assert.ThrowsAsync(() => controller.DeleteAsync(id)); - - // assert - Assert.Equal(405, exception.GetStatusCode()); - } - - private void VerifyApplyContext() - => _jsonApiContextMock.Verify(m => m.ApplyContext(It.IsAny>()), Times.Once); - } -} +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Models; +using JsonApiDotNetCore.Services; +using Moq; +using Xunit; +using System.Threading.Tasks; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Internal; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; + +namespace UnitTests +{ + public class BaseJsonApiController_Tests + { + public class Resource : Identifiable + { + [Attr("test-attribute")] public string TestAttribute { get; set; } + } + private Mock _jsonApiContextMock = new Mock(); + private Mock _contextGraphMock = new Mock(); + + [Fact] + public async Task GetAsync_Calls_Service() + { + // arrange + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getAll: serviceMock.Object); + + // act + await controller.GetAsync(); + + // assert + serviceMock.Verify(m => m.GetAsync(), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task GetAsync_Throws_405_If_No_Service() + { + // arrange + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.GetAsync()); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task GetAsyncById_Calls_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getById: serviceMock.Object); + + // act + await controller.GetAsync(id); + + // assert + serviceMock.Verify(m => m.GetAsync(id), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task GetAsyncById_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getById: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.GetAsync(id)); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task GetRelationshipsAsync_Calls_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationships: serviceMock.Object); + + // act + await controller.GetRelationshipsAsync(id, string.Empty); + + // assert + serviceMock.Verify(m => m.GetRelationshipsAsync(id, string.Empty), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task GetRelationshipsAsync_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationships: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.GetRelationshipsAsync(id, string.Empty)); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task GetRelationshipAsync_Calls_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationship: serviceMock.Object); + + // act + await controller.GetRelationshipAsync(id, string.Empty); + + // assert + serviceMock.Verify(m => m.GetRelationshipAsync(id, string.Empty), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task GetRelationshipAsync_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, getRelationship: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.GetRelationshipAsync(id, string.Empty)); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task PatchAsync_Calls_Service() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); + + // act + await controller.PatchAsync(id, resource); + + // assert + serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task PatchAsync_ModelStateInvalid_ValidateModelStateDisbled() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); + + // act + var response = await controller.PatchAsync(id, resource); + + // assert + serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Once); + VerifyApplyContext(); + Assert.IsNotType(response); + } + + [Fact] + public async Task PatchAsync_ModelStateInvalid_ValidateModelStateEnabled() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.SetupGet(a => a.ContextGraph).Returns(_contextGraphMock.Object); + _contextGraphMock.Setup(a => a.GetPublicAttributeName("TestAttribute")).Returns("test-attribute"); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions{ValidateModelState = true}); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); + controller.ModelState.AddModelError("TestAttribute", "Failed Validation"); + + // act + var response = await controller.PatchAsync(id, resource); + + // assert + serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Never); + Assert.IsType(response); + Assert.IsType(((BadRequestObjectResult) response).Value); + } + + [Fact] + public async Task PatchAsync_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.PatchAsync(id, It.IsAny())); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task PostAsync_Calls_Service() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); + controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext {HttpContext = new DefaultHttpContext()}; + + // act + await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task PostAsync_ModelStateInvalid_ValidateModelStateDisabled() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); + controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext { HttpContext = new DefaultHttpContext() }; + + // act + var response = await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); + VerifyApplyContext(); + Assert.IsNotType(response); + } + + [Fact] + public async Task PostAsync_ModelStateInvalid_ValidateModelStateEnabled() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.SetupGet(a => a.ContextGraph).Returns(_contextGraphMock.Object); + _contextGraphMock.Setup(a => a.GetPublicAttributeName("TestAttribute")).Returns("test-attribute"); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = true }); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + controller.ModelState.AddModelError("TestAttribute", "Failed Validation"); + + // act + var response = await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Never); + Assert.IsType(response); + Assert.IsType(((BadRequestObjectResult)response).Value); + } + + [Fact] + public async Task PatchRelationshipsAsync_Calls_Service() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, updateRelationships: serviceMock.Object); + + // act + await controller.PatchRelationshipsAsync(id, string.Empty, null); + + // assert + serviceMock.Verify(m => m.UpdateRelationshipsAsync(id, string.Empty, null), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task PatchRelationshipsAsync_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, updateRelationships: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.PatchRelationshipsAsync(id, string.Empty, null)); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + [Fact] + public async Task DeleteAsync_Calls_Service() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, delete: serviceMock.Object); + + // act + await controller.DeleteAsync(id); + + // assert + serviceMock.Verify(m => m.DeleteAsync(id), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task DeleteAsync_Throws_405_If_No_Service() + { + // arrange + const int id = 0; + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, delete: null); + + // act + var exception = await Assert.ThrowsAsync(() => controller.DeleteAsync(id)); + + // assert + Assert.Equal(405, exception.GetStatusCode()); + } + + private void VerifyApplyContext() + => _jsonApiContextMock.Verify(m => m.ApplyContext(It.IsAny>()), Times.Once); + } +} From 524232e1eb9d670b40f14d6fad1097a1ee2a2f06 Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 22:22:04 -0500 Subject: [PATCH 6/7] feat: throw 422 status code when validation fails --- .../Controllers/BaseJsonApiController.cs | 5 +++-- test/UnitTests/Controllers/BaseJsonApiController_Tests.cs | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index 407899051d..aec2d16cfe 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -156,7 +156,7 @@ public virtual async Task PostAsync([FromBody] T entity) return Forbidden(); if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); + return UnprocessableEntity(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); entity = await _create.CreateAsync(entity); @@ -169,8 +169,9 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); + if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) - return BadRequest(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); + return UnprocessableEntity(ModelState.ConvertToErrorCollection(_jsonApiContext.ContextGraph)); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs index c0f0c0cbe6..7327bb18a3 100644 --- a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs +++ b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs @@ -200,8 +200,8 @@ public async Task PatchAsync_ModelStateInvalid_ValidateModelStateEnabled() // assert serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Never); - Assert.IsType(response); - Assert.IsType(((BadRequestObjectResult) response).Value); + Assert.IsType(response); + Assert.IsType(((UnprocessableEntityObjectResult) response).Value); } [Fact] @@ -278,8 +278,8 @@ public async Task PostAsync_ModelStateInvalid_ValidateModelStateEnabled() // assert serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Never); - Assert.IsType(response); - Assert.IsType(((BadRequestObjectResult)response).Value); + Assert.IsType(response); + Assert.IsType(((UnprocessableEntityObjectResult)response).Value); } [Fact] From 7e22eebc583d33c4ba7b4a91a493619bd9bc47ec Mon Sep 17 00:00:00 2001 From: Ryan Tablada Date: Tue, 7 Aug 2018 23:55:08 -0500 Subject: [PATCH 7/7] feat: obsolete method ConvertToErrorCollection --- .../Extensions/ModelStateExtensions.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index 3a742617db..43c33c21f2 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -1,3 +1,4 @@ +using System; using JsonApiDotNetCore.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.EntityFrameworkCore.Internal; @@ -6,6 +7,26 @@ namespace JsonApiDotNetCore.Extensions { public static class ModelStateExtensions { + [Obsolete("Use Generic Method ConvertToErrorCollection(IContextGraph contextGraph) instead for full validation errors")] + public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState) + { + ErrorCollection collection = new ErrorCollection(); + foreach (var entry in modelState) + { + if (entry.Value.Errors.Any() == false) + continue; + + foreach (var modelError in entry.Value.Errors) + { + if (modelError.Exception is JsonApiException jex) + collection.Errors.AddRange(jex.GetError().Errors); + else + collection.Errors.Add(new Error(400, entry.Key, modelError.ErrorMessage, modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null)); + } + } + + return collection; + } public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState, IContextGraph contextGraph) { ErrorCollection collection = new ErrorCollection();