From 5dc8bd76c508d7482b5b2792892e2d39a8ef6d32 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Sun, 10 Nov 2019 13:25:12 +0100 Subject: [PATCH 1/5] chore: improve testability by exposing SingleData and ManyData on Document --- .../Models/JsonApiDocuments/ExposableData.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs b/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs index bb2f9a2800..17d11e1366 100644 --- a/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs +++ b/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs @@ -31,12 +31,12 @@ public bool ShouldSerializeData() /// /// Internally used for "single" primary data. /// - internal T SingleData { get; private set; } + public T SingleData { get; private set; } /// /// Internally used for "many" primary data. /// - internal List ManyData { get; private set; } + public List ManyData { get; private set; } /// /// Internally used to indicate if the document's primary data is From cd95cfae202459c1e5e04b34e0d23955b96790d6 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Sun, 10 Nov 2019 14:04:58 +0100 Subject: [PATCH 2/5] fix: #620 --- .../Configuration/JsonApiOptions.cs | 4 +- .../Controllers/BaseJsonApiController.cs | 16 +++---- .../IApplicationBuilderExtensions.cs | 12 ++---- .../Extensions/ModelStateExtensions.cs | 4 +- .../Formatters/JsonApiWriter.cs | 43 +++++++++++-------- 5 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs index 8530cd8fa2..3fd4946c03 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs @@ -32,12 +32,12 @@ public class JsonApiOptions : IJsonApiOptions /// /// Whether or not stack traces should be serialized in Error objects /// - public static bool DisableErrorStackTraces { get; set; } + public static bool DisableErrorStackTraces { get; set; } = true; /// /// Whether or not source URLs should be serialized in Error objects /// - public static bool DisableErrorSource { get; set; } + public static bool DisableErrorSource { get; set; } = true; /// /// Whether or not ResourceHooks are enabled. diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index e0b781b283..ef2c9d3803 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -141,7 +141,7 @@ public virtual async Task PostAsync([FromBody] T entity) return Forbidden(); if (_jsonApiOptions.ValidateModelState && !ModelState.IsValid) - return UnprocessableEntity(ModelState.ConvertToErrorCollection(GetAssociatedResource())); + return UnprocessableEntity(ModelState.ConvertToErrorCollection()); entity = await _create.CreateAsync(entity); @@ -155,7 +155,7 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) return UnprocessableEntity(); if (_jsonApiOptions.ValidateModelState && !ModelState.IsValid) - return UnprocessableEntity(ModelState.ConvertToErrorCollection(GetAssociatedResource())); + return UnprocessableEntity(ModelState.ConvertToErrorCollection()); var updatedEntity = await _update.UpdateAsync(id, entity); @@ -181,12 +181,12 @@ public virtual async Task DeleteAsync(TId id) return NoContent(); } - internal Type GetAssociatedResource() - { - return GetType().GetMethod(nameof(GetAssociatedResource), BindingFlags.Instance | BindingFlags.NonPublic) - .DeclaringType - .GetGenericArguments()[0]; - } + //internal Type GetAssociatedResource() + //{ + // return GetType().GetMethod(nameof(GetAssociatedResource), BindingFlags.Instance | BindingFlags.NonPublic) + // .DeclaringType + // .GetGenericArguments()[0]; + //} } public class BaseJsonApiController : BaseJsonApiController diff --git a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs index 5f4aeb53dd..9e5591d30b 100644 --- a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; namespace JsonApiDotNetCore.Extensions @@ -20,7 +21,6 @@ public static class IApplicationBuilderExtensions /// public static void UseJsonApi(this IApplicationBuilder app) { - DisableDetailedErrorsIfProduction(app); LogResourceGraphValidations(app); using (var scope = app.ApplicationServices.CreateScope()) { @@ -38,14 +38,10 @@ public static void UseJsonApi(this IApplicationBuilder app) app.UseEndpoints(endpoints => endpoints.MapControllers()); } - private static void DisableDetailedErrorsIfProduction(IApplicationBuilder app) + public static void EnableDetailedErrors(this IApplicationBuilder app) { - var webHostEnvironment = (IWebHostEnvironment) app.ApplicationServices.GetService(typeof(IWebHostEnvironment)); - if (webHostEnvironment.EnvironmentName == "Production") - { - JsonApiOptions.DisableErrorStackTraces = true; - JsonApiOptions.DisableErrorSource = true; - } + JsonApiOptions.DisableErrorStackTraces = false; + JsonApiOptions.DisableErrorSource = false; } private static void LogResourceGraphValidations(IApplicationBuilder app) diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index 262e81678e..7c2359eb1c 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -9,7 +9,7 @@ namespace JsonApiDotNetCore.Extensions { public static class ModelStateExtensions { - public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState, Type resourceType) + public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState) where T : class, IIdentifiable { ErrorCollection collection = new ErrorCollection(); foreach (var entry in modelState) @@ -19,7 +19,7 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDiction continue; } - var targetedProperty = resourceType.GetProperty(entry.Key); + var targetedProperty = typeof(T).GetProperty(entry.Key); var attrName = targetedProperty.GetCustomAttribute().PublicAttributeName; foreach (var modelError in entry.Value.Errors) diff --git a/src/JsonApiDotNetCore/Formatters/JsonApiWriter.cs b/src/JsonApiDotNetCore/Formatters/JsonApiWriter.cs index d1500e6ae5..aa8b779992 100644 --- a/src/JsonApiDotNetCore/Formatters/JsonApiWriter.cs +++ b/src/JsonApiDotNetCore/Formatters/JsonApiWriter.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Serialization.Server; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.Extensions.Logging; using Newtonsoft.Json; @@ -32,33 +33,39 @@ public async Task WriteAsync(OutputFormatterWriteContext context) throw new ArgumentNullException(nameof(context)); var response = context.HttpContext.Response; - using (var writer = context.WriterFactory(response.Body, Encoding.UTF8)) + using var writer = context.WriterFactory(response.Body, Encoding.UTF8); + string responseContent; + + if (_serializer == null) + { + responseContent = JsonConvert.SerializeObject(context.Object); + } + else { response.ContentType = Constants.ContentType; - string responseContent; - if (_serializer == null) + try { - responseContent = JsonConvert.SerializeObject(context.Object); - } - else - { - try + if (context.Object is ProblemDetails pd) { - responseContent = _serializer.Serialize(context.Object); - } - catch (Exception e) - { - _logger?.LogError(new EventId(), e, "An error ocurred while formatting the response"); var errors = new ErrorCollection(); - errors.Add(new Error(400, e.Message, ErrorMeta.FromException(e))); + errors.Add(new Error(pd.Status.Value, pd.Title, pd.Detail)); responseContent = _serializer.Serialize(errors); - response.StatusCode = 400; + } else + { + responseContent = _serializer.Serialize(context.Object); } } - - await writer.WriteAsync(responseContent); - await writer.FlushAsync(); + catch (Exception e) + { + _logger?.LogError(new EventId(), e, "An error ocurred while formatting the response"); + var errors = new ErrorCollection(); + errors.Add(new Error(500, e.Message, ErrorMeta.FromException(e))); + responseContent = _serializer.Serialize(errors); + response.StatusCode = 500; + } } + await writer.WriteAsync(responseContent); + await writer.FlushAsync(); } } } From 0a640663f508efcf2d425fff0c70330c35b72524 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 11 Nov 2019 10:52:30 +0100 Subject: [PATCH 3/5] chore: fix problem in which custom outputter is not executed --- .../Controllers/BaseJsonApiController.cs | 29 +++++++++++-------- .../Models/JsonApiDocuments/ExposableData.cs | 2 ++ .../Acceptance/Spec/FetchingDataTests.cs | 26 +++++++++++++++++ 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index ef2c9d3803..c241c5b81d 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -1,5 +1,3 @@ -using System; -using System.Reflection; using System.Threading.Tasks; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Extensions; @@ -106,7 +104,11 @@ public virtual async Task GetAsync(TId id) if (_getById == null) throw Exceptions.UnSupportedRequestMethod; var entity = await _getById.GetAsync(id); if (entity == null) - return NotFound(); + { + // remove the null argument as soon as this has been resolved: + // https://github.com/aspnet/AspNetCore/issues/16969 + return NotFound(null); + } return Ok(entity); } @@ -117,7 +119,11 @@ public virtual async Task GetRelationshipsAsync(TId id, string re throw Exceptions.UnSupportedRequestMethod; var relationship = await _getRelationships.GetRelationshipsAsync(id, relationshipName); if (relationship == null) - return NotFound(); + { + // remove the null argument as soon as this has been resolved: + // https://github.com/aspnet/AspNetCore/issues/16969 + return NotFound(null); + } return Ok(relationship); } @@ -160,7 +166,12 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) var updatedEntity = await _update.UpdateAsync(id, entity); if (updatedEntity == null) - return NotFound(); + { + // remove the null argument as soon as this has been resolved: + // https://github.com/aspnet/AspNetCore/issues/16969 + return NotFound(null); + } + return Ok(updatedEntity); } @@ -180,14 +191,8 @@ public virtual async Task DeleteAsync(TId id) return NotFound(); return NoContent(); } - - //internal Type GetAssociatedResource() - //{ - // return GetType().GetMethod(nameof(GetAssociatedResource), BindingFlags.Instance | BindingFlags.NonPublic) - // .DeclaringType - // .GetGenericArguments()[0]; - //} } + public class BaseJsonApiController : BaseJsonApiController where T : class, IIdentifiable diff --git a/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs b/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs index 17d11e1366..db4b634bd5 100644 --- a/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs +++ b/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs @@ -31,11 +31,13 @@ public bool ShouldSerializeData() /// /// Internally used for "single" primary data. /// + [JsonIgnore] public T SingleData { get; private set; } /// /// Internally used for "many" primary data. /// + [JsonIgnore] public List ManyData { get; private set; } /// diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs index e863104596..c58fc94663 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs @@ -123,5 +123,31 @@ public async Task GetResources_NoDefaultPageSize_ReturnsResources() // Assert Assert.True(result.Data.Count >= 20); } + + [Fact] + public async Task GetSingleResource_ResourceDoesNotExist_ReturnsNotFoundWithNullData() + { + // Arrange + var context = _fixture.GetService(); + context.TodoItems.RemoveRange(context.TodoItems); + await context.SaveChangesAsync(); + + var builder = new WebHostBuilder() + .UseStartup(); + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todoItems/123"; + var server = new TestServer(builder); + var client = server.CreateClient(); + var request = new HttpRequestMessage(httpMethod, route); + + // Act + var response = await client.SendAsync(request); + var body = await response.Content.ReadAsStringAsync(); + var document = JsonConvert.DeserializeObject(body); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + Assert.Null(document.Data); + } } } From 9c62a49847700da6bab06937d1e65256c3782e8d Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 11 Nov 2019 10:54:10 +0100 Subject: [PATCH 4/5] chore: enable detailed errors --- src/Examples/JsonApiDotNetCoreExample/Startups/Startup.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Examples/JsonApiDotNetCoreExample/Startups/Startup.cs b/src/Examples/JsonApiDotNetCoreExample/Startups/Startup.cs index a2d11f06f5..5f9c4c979c 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Startups/Startup.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Startups/Startup.cs @@ -58,6 +58,7 @@ public virtual void Configure( AppDbContext context) { context.Database.EnsureCreated(); + app.EnableDetailedErrors(); app.UseJsonApi(); } From 58277be61890cc120ae85d4fa2174f222b7bc2c4 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Mon, 11 Nov 2019 11:38:03 +0100 Subject: [PATCH 5/5] docs: add comment --- .../Extensions/IApplicationBuilderExtensions.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs index 9e5591d30b..9f0a665c3d 100644 --- a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs @@ -38,6 +38,10 @@ public static void UseJsonApi(this IApplicationBuilder app) app.UseEndpoints(endpoints => endpoints.MapControllers()); } + /// + /// Configures your application to return stack traces in error results. + /// + /// public static void EnableDetailedErrors(this IApplicationBuilder app) { JsonApiOptions.DisableErrorStackTraces = false;