Fix/#620 (#621)

maurei · wisepotato · commit a801d958d964 · 2019-11-11T11:45:30.000+01:00
* chore: improve testability by exposing SingleData and ManyData on Document * fix: #620 * chore: fix problem in which custom outputter is not executed * chore: enable detailed errors * docs: add comment
diff --git 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();
         }
 
diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs
@@ -32,12 +32,12 @@ public class JsonApiOptions : IJsonApiOptions
         /// <summary>
         /// Whether or not stack traces should be serialized in Error objects
         /// </summary>
-        public static bool DisableErrorStackTraces { get; set; }
+        public static bool DisableErrorStackTraces { get; set; } = true;
 
         /// <summary>
         /// Whether or not source URLs should be serialized in Error objects
         /// </summary>
-        public static bool DisableErrorSource { get; set; }
+        public static bool DisableErrorSource { get; set; } = true;
 
         /// <summary>
         /// Whether or not ResourceHooks are enabled. 
diff --git 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<IActionResult> 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<IActionResult> 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);
         }
@@ -141,7 +147,7 @@ public virtual async Task<IActionResult> PostAsync([FromBody] T entity)
                 return Forbidden();
 
             if (_jsonApiOptions.ValidateModelState && !ModelState.IsValid)
-                return UnprocessableEntity(ModelState.ConvertToErrorCollection<T>(GetAssociatedResource()));
+                return UnprocessableEntity(ModelState.ConvertToErrorCollection<T>());
 
             entity = await _create.CreateAsync(entity);
 
@@ -155,12 +161,17 @@ public virtual async Task<IActionResult> PatchAsync(TId id, [FromBody] T entity)
                 return UnprocessableEntity();
 
             if (_jsonApiOptions.ValidateModelState && !ModelState.IsValid)
-                return UnprocessableEntity(ModelState.ConvertToErrorCollection<T>(GetAssociatedResource()));
+                return UnprocessableEntity(ModelState.ConvertToErrorCollection<T>());
 
             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<IActionResult> 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<T>
     : BaseJsonApiController<T, int>
     where T : class, IIdentifiable<int>
diff --git 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
         /// <returns></returns>
         public static void UseJsonApi(this IApplicationBuilder app)
         {
-            DisableDetailedErrorsIfProduction(app);
             LogResourceGraphValidations(app);
             using (var scope = app.ApplicationServices.CreateScope())
             {
@@ -38,14 +38,14 @@ public static void UseJsonApi(this IApplicationBuilder app)
             app.UseEndpoints(endpoints => endpoints.MapControllers());
         }
 
-        private static void DisableDetailedErrorsIfProduction(IApplicationBuilder app)
+        /// <summary>
+        /// Configures your application to return stack traces in error results.
+        /// </summary>
+        /// <param name="app"></param>
+        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
@@ -9,7 +9,7 @@ namespace JsonApiDotNetCore.Extensions
 {
     public static class ModelStateExtensions
     {
-        public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDictionary modelState, Type resourceType)
+        public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDictionary modelState) where T : class, IIdentifiable
         {
             ErrorCollection collection = new ErrorCollection();
             foreach (var entry in modelState)
@@ -19,7 +19,7 @@ public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDiction
                     continue;
                 }
 
-                var targetedProperty = resourceType.GetProperty(entry.Key);
+                var targetedProperty = typeof(T).GetProperty(entry.Key);
                 var attrName = targetedProperty.GetCustomAttribute<AttrAttribute>().PublicAttributeName;
 
                 foreach (var modelError in entry.Value.Errors)
diff --git 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();
         }
     }
 }
diff --git a/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs b/src/JsonApiDotNetCore/Models/JsonApiDocuments/ExposableData.cs
@@ -31,12 +31,14 @@ public bool ShouldSerializeData()
         /// <summary>
         /// Internally used for "single" primary data.
         /// </summary>
-        internal T SingleData { get; private set; }
+        [JsonIgnore]
+        public T SingleData { get; private set; }
 
         /// <summary>
         /// Internally used for "many" primary data.
         /// </summary>
-        internal List<T> ManyData { get; private set; }
+        [JsonIgnore]
+        public List<T> ManyData { get; private set; }
 
         /// <summary>
         /// Internally used to indicate if the document's primary data is
diff --git 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<AppDbContext>();
+            context.TodoItems.RemoveRange(context.TodoItems);
+            await context.SaveChangesAsync();
+
+            var builder = new WebHostBuilder()
+                .UseStartup<NoDefaultPageSizeStartup>();
+            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<Document>(body);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+            Assert.Null(document.Data);
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,7 @@ public virtual void Configure(`
`58`	`58`	`AppDbContext context)`
`59`	`59`	`{`
`60`	`60`	`context.Database.EnsureCreated();`
	`61`	`+ app.EnableDetailedErrors();`
`61`	`62`	`app.UseJsonApi();`
`62`	`63`	`}`
`63`	`64`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ namespace JsonApiDotNetCore.Extensions`
`9`	`9`	`{`
`10`	`10`	`public static class ModelStateExtensions`
`11`	`11`	`{`
`12`		`- public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDictionary modelState, Type resourceType)`
	`12`	`+ public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDictionary modelState) where T : class, IIdentifiable`
`13`	`13`	`{`
`14`	`14`	`ErrorCollection collection = new ErrorCollection();`
`15`	`15`	`foreach (var entry in modelState)`
`@@ -19,7 +19,7 @@ public static ErrorCollection ConvertToErrorCollection<T>(this ModelStateDiction`
`19`	`19`	`continue;`
`20`	`20`	`}`
`21`	`21`
`22`		`- var targetedProperty = resourceType.GetProperty(entry.Key);`
	`22`	`+ var targetedProperty = typeof(T).GetProperty(entry.Key);`
`23`	`23`	`var attrName = targetedProperty.GetCustomAttribute<AttrAttribute>().PublicAttributeName;`
`24`	`24`
`25`	`25`	`foreach (var modelError in entry.Value.Errors)`