Fix crash on operations requests when trace logging is turned on

json-api-dotnet · Nov 16, 2023 · ba430da · ba430da
1 parent 24b9546
commit ba430da
Show file tree

Hide file tree

Showing 10 changed files with 485 additions and 62 deletions.
diff --git a/src/JsonApiDotNetCore/Middleware/TraceLogWriter.cs b/src/JsonApiDotNetCore/Middleware/TraceLogWriter.cs
@@ -4,6 +4,9 @@
 using System.Text.Encodings.Web;
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using JsonApiDotNetCore.Configuration;
+using JsonApiDotNetCore.Resources;
+using JsonApiDotNetCore.Resources.Annotations;
 using Microsoft.Extensions.Logging;
 
 namespace JsonApiDotNetCore.Middleware;
@@ -14,8 +17,69 @@ internal abstract class TraceLogWriter
     {
         WriteIndented = true,
         Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
-        ReferenceHandler = ReferenceHandler.Preserve
+        ReferenceHandler = ReferenceHandler.IgnoreCycles,
+        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
+        Converters =
+        {
+            new JsonStringEnumConverter(),
+            new ResourceTypeInTraceJsonConverter(),
+            new ResourceFieldInTraceJsonConverterFactory(),
+            new IdentifiableInTraceJsonConverter()
+        }
     };
+
+    private sealed class ResourceTypeInTraceJsonConverter : JsonConverter<ResourceType>
+    {
+        public override ResourceType Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            throw new NotSupportedException();
+        }
+
+        public override void Write(Utf8JsonWriter writer, ResourceType value, JsonSerializerOptions options)
+        {
+            writer.WriteStringValue(value.PublicName);
+        }
+    }
+
+    private sealed class ResourceFieldInTraceJsonConverterFactory : JsonConverterFactory
+    {
+        public override bool CanConvert(Type typeToConvert)
+        {
+            return typeToConvert.IsAssignableTo(typeof(ResourceFieldAttribute));
+        }
+
+        public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
+        {
+            return new ResourceFieldInTraceJsonConverter();
+        }
+
+        private sealed class ResourceFieldInTraceJsonConverter : JsonConverter<ResourceFieldAttribute>
+        {
+            public override ResourceFieldAttribute Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+            {
+                throw new NotSupportedException();
+            }
+
+            public override void Write(Utf8JsonWriter writer, ResourceFieldAttribute value, JsonSerializerOptions options)
+            {
+                writer.WriteStringValue(value.PublicName);
+            }
+        }
+    }
+
+    private sealed class IdentifiableInTraceJsonConverter : JsonConverter<IIdentifiable>
+    {
+        public override IIdentifiable Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            throw new NotSupportedException();
+        }
+
+        public override void Write(Utf8JsonWriter writer, IIdentifiable value, JsonSerializerOptions options)
+        {
+            Type runtimeType = value.GetType();
+            JsonSerializer.Serialize(writer, value, runtimeType, options);
+        }
+    }
 }
 
 internal sealed class TraceLogWriter<T> : TraceLogWriter
@@ -139,7 +203,7 @@ private static string SerializeObject(object value)
         {
             return JsonSerializer.Serialize(value, SerializerOptions);
         }
-        catch (JsonException)
+        catch (Exception exception) when (exception is JsonException or NotSupportedException)
         {
             // Never crash as a result of logging, this is best-effort only.
             return "object";

diff --git a/src/JsonApiDotNetCore/Queries/QueryLayer.cs b/src/JsonApiDotNetCore/Queries/QueryLayer.cs
@@ -42,7 +42,7 @@ internal void WriteLayer(IndentingStringWriter writer, string? prefix)
 
         using (writer.Indent())
         {
-            if (Include != null)
+            if (Include != null && Include.Elements.Any())
             {
                 writer.WriteLine($"{nameof(Include)}: {Include}");
             }

diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Mixed/AtomicLoggingTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Mixed/AtomicLoggingTests.cs
@@ -80,9 +80,10 @@ public async Task Logs_at_error_level_on_unhandled_exception()
         error.Source.ShouldNotBeNull();
         error.Source.Pointer.Should().Be("/atomic:operations[0]");
 
-        loggerFactory.Logger.Messages.ShouldNotBeEmpty();
+        IReadOnlyList<FakeLogMessage> logMessages = loggerFactory.Logger.GetMessages();
+        logMessages.ShouldNotBeEmpty();
 
-        loggerFactory.Logger.Messages.Should().ContainSingle(message => message.LogLevel == LogLevel.Error &&
+        logMessages.Should().ContainSingle(message => message.LogLevel == LogLevel.Error &&
             message.Text.Contains("Simulated failure.", StringComparison.Ordinal));
     }
 
@@ -117,9 +118,10 @@ public async Task Logs_at_info_level_on_invalid_request_body()
 
         responseDocument.Errors.ShouldHaveCount(1);
 
-        loggerFactory.Logger.Messages.ShouldNotBeEmpty();
+        IReadOnlyList<FakeLogMessage> logMessages = loggerFactory.Logger.GetMessages();
+        logMessages.ShouldNotBeEmpty();
 
-        loggerFactory.Logger.Messages.Should().ContainSingle(message => message.LogLevel == LogLevel.Information &&
+        logMessages.Should().ContainSingle(message => message.LogLevel == LogLevel.Information &&
             message.Text.Contains("Failed to deserialize request body", StringComparison.Ordinal));
     }