From 594b533339cc91a8b43a545ccc538f071caf90c4 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Fri, 17 Sep 2021 14:45:10 +0200 Subject: [PATCH] Review feedback: use base class instead of static helper --- .../Serialization/JsonConverterSupport.cs | 37 ------------------- .../JsonConverters/ResourceObjectConverter.cs | 25 ++++++------- .../SingleOrManyDataConverterFactory.cs | 6 +-- .../WriteOnlyDocumentConverter.cs | 19 +++++----- .../WriteOnlyRelationshipObjectConverter.cs | 9 ++--- .../Serialization/JsonObjectConverter.cs | 35 ++++++++++++++++++ 6 files changed, 63 insertions(+), 68 deletions(-) delete mode 100644 src/JsonApiDotNetCore/Serialization/JsonConverterSupport.cs create mode 100644 src/JsonApiDotNetCore/Serialization/JsonObjectConverter.cs diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverterSupport.cs b/src/JsonApiDotNetCore/Serialization/JsonConverterSupport.cs deleted file mode 100644 index 0b26c674f0..0000000000 --- a/src/JsonApiDotNetCore/Serialization/JsonConverterSupport.cs +++ /dev/null @@ -1,37 +0,0 @@ -using System.Text.Json; -using System.Text.Json.Serialization; - -#pragma warning disable AV1008 // Class should not be static - -namespace JsonApiDotNetCore.Serialization -{ - internal static class JsonConverterSupport - { - public static T ReadSubTree(ref Utf8JsonReader reader, JsonSerializerOptions options) - { - if (typeof(T) != typeof(object) && options?.GetConverter(typeof(T)) is JsonConverter converter) - { - return converter.Read(ref reader, typeof(T), options); - } - - return JsonSerializer.Deserialize(ref reader, options); - } - - public static void WriteSubTree(Utf8JsonWriter writer, T value, JsonSerializerOptions options) - { - if (typeof(T) != typeof(object) && options?.GetConverter(typeof(T)) is JsonConverter converter) - { - converter.Write(writer, value, options); - } - else - { - JsonSerializer.Serialize(writer, value, options); - } - } - - public static JsonException GetEndOfStreamError() - { - return new("Unexpected end of JSON stream."); - } - } -} diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs b/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs index 4414fe1b41..4f0758fff0 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonConverters/ResourceObjectConverter.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Reflection; using System.Text.Json; -using System.Text.Json.Serialization; using JetBrains.Annotations; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Resources; @@ -15,7 +14,7 @@ namespace JsonApiDotNetCore.Serialization.JsonConverters /// Converts to/from JSON. /// [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)] - public sealed class ResourceObjectConverter : JsonConverter + public sealed class ResourceObjectConverter : JsonObjectConverter { private static readonly JsonEncodedText TypeText = JsonEncodedText.Encode("type"); private static readonly JsonEncodedText IdText = JsonEncodedText.Encode("id"); @@ -72,7 +71,7 @@ public override ResourceObject Read(ref Utf8JsonReader reader, Type typeToConver { // Newtonsoft.Json used to auto-convert number to strings, while System.Text.Json does not. This is so likely // to hit users during upgrade that we special-case for this and produce a helpful error message. - var jsonElement = JsonConverterSupport.ReadSubTree(ref reader, options); + var jsonElement = ReadSubTree(ref reader, options); throw new JsonException($"Failed to convert ID '{jsonElement}' of type '{jsonElement.ValueKind}' to type 'String'."); } @@ -99,17 +98,17 @@ public override ResourceObject Read(ref Utf8JsonReader reader, Type typeToConver } case "relationships": { - resourceObject.Relationships = JsonConverterSupport.ReadSubTree>(ref reader, options); + resourceObject.Relationships = ReadSubTree>(ref reader, options); break; } case "links": { - resourceObject.Links = JsonConverterSupport.ReadSubTree(ref reader, options); + resourceObject.Links = ReadSubTree(ref reader, options); break; } case "meta": { - resourceObject.Meta = JsonConverterSupport.ReadSubTree>(ref reader, options); + resourceObject.Meta = ReadSubTree>(ref reader, options); break; } default: @@ -124,7 +123,7 @@ public override ResourceObject Read(ref Utf8JsonReader reader, Type typeToConver } } - throw JsonConverterSupport.GetEndOfStreamError(); + throw GetEndOfStreamError(); } private static string TryPeekType(ref Utf8JsonReader reader) @@ -196,7 +195,7 @@ private static IDictionary ReadAttributes(ref Utf8JsonReader rea // Inside a JsonConverter there is no way to know where in the JSON object tree we are. And the serializer // is unable to provide the correct position either. So we avoid an exception and postpone producing an error // response to the post-processing phase, by setting a sentinel value. - var jsonElement = JsonConverterSupport.ReadSubTree(ref reader, options); + var jsonElement = ReadSubTree(ref reader, options); attributeValue = new JsonInvalidAttributeInfo(attributeName, property.PropertyType, jsonElement.ToString(), jsonElement.ValueKind); @@ -215,7 +214,7 @@ private static IDictionary ReadAttributes(ref Utf8JsonReader rea } } - throw JsonConverterSupport.GetEndOfStreamError(); + throw GetEndOfStreamError(); } /// @@ -240,25 +239,25 @@ public override void Write(Utf8JsonWriter writer, ResourceObject value, JsonSeri if (!value.Attributes.IsNullOrEmpty()) { writer.WritePropertyName(AttributesText); - JsonConverterSupport.WriteSubTree(writer, value.Attributes, options); + WriteSubTree(writer, value.Attributes, options); } if (!value.Relationships.IsNullOrEmpty()) { writer.WritePropertyName(RelationshipsText); - JsonConverterSupport.WriteSubTree(writer, value.Relationships, options); + WriteSubTree(writer, value.Relationships, options); } if (value.Links != null && value.Links.HasValue()) { writer.WritePropertyName(LinksText); - JsonConverterSupport.WriteSubTree(writer, value.Links, options); + WriteSubTree(writer, value.Links, options); } if (!value.Meta.IsNullOrEmpty()) { writer.WritePropertyName(MetaText); - JsonConverterSupport.WriteSubTree(writer, value.Meta, options); + WriteSubTree(writer, value.Meta, options); } writer.WriteEndObject(); diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverters/SingleOrManyDataConverterFactory.cs b/src/JsonApiDotNetCore/Serialization/JsonConverters/SingleOrManyDataConverterFactory.cs index e23abaf0ed..0ca65c237e 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonConverters/SingleOrManyDataConverterFactory.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonConverters/SingleOrManyDataConverterFactory.cs @@ -28,7 +28,7 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer return (JsonConverter)Activator.CreateInstance(converterType, BindingFlags.Instance | BindingFlags.Public, null, null, null); } - private sealed class SingleOrManyDataConverter : JsonConverter> + private sealed class SingleOrManyDataConverter : JsonObjectConverter> where T : class, IResourceIdentity { public override SingleOrManyData Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions serializerOptions) @@ -48,7 +48,7 @@ public override SingleOrManyData Read(ref Utf8JsonReader reader, Type typeToC } case JsonTokenType.StartObject: { - var resourceObject = JsonConverterSupport.ReadSubTree(ref reader, serializerOptions); + var resourceObject = ReadSubTree(ref reader, serializerOptions); objects.Add(resourceObject); break; } @@ -67,7 +67,7 @@ public override SingleOrManyData Read(ref Utf8JsonReader reader, Type typeToC public override void Write(Utf8JsonWriter writer, SingleOrManyData value, JsonSerializerOptions options) { - JsonConverterSupport.WriteSubTree(writer, value.Value, options); + WriteSubTree(writer, value.Value, options); } } } diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyDocumentConverter.cs b/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyDocumentConverter.cs index 7f545dcccd..65d8f36d12 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyDocumentConverter.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyDocumentConverter.cs @@ -1,6 +1,5 @@ using System; using System.Text.Json; -using System.Text.Json.Serialization; using JetBrains.Annotations; using JsonApiDotNetCore.Serialization.Objects; @@ -10,7 +9,7 @@ namespace JsonApiDotNetCore.Serialization.JsonConverters /// Converts to JSON. /// [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)] - public sealed class WriteOnlyDocumentConverter : JsonConverter + public sealed class WriteOnlyDocumentConverter : JsonObjectConverter { private static readonly JsonEncodedText JsonApiText = JsonEncodedText.Encode("jsonapi"); private static readonly JsonEncodedText LinksText = JsonEncodedText.Encode("links"); @@ -36,49 +35,49 @@ public override void Write(Utf8JsonWriter writer, Document value, JsonSerializer if (value.JsonApi != null) { writer.WritePropertyName(JsonApiText); - JsonConverterSupport.WriteSubTree(writer, value.JsonApi, options); + WriteSubTree(writer, value.JsonApi, options); } if (value.Links != null && value.Links.HasValue()) { writer.WritePropertyName(LinksText); - JsonConverterSupport.WriteSubTree(writer, value.Links, options); + WriteSubTree(writer, value.Links, options); } if (value.Data.IsAssigned) { writer.WritePropertyName(DataText); - JsonConverterSupport.WriteSubTree(writer, value.Data, options); + WriteSubTree(writer, value.Data, options); } if (!value.Operations.IsNullOrEmpty()) { writer.WritePropertyName(AtomicOperationsText); - JsonConverterSupport.WriteSubTree(writer, value.Operations, options); + WriteSubTree(writer, value.Operations, options); } if (!value.Results.IsNullOrEmpty()) { writer.WritePropertyName(AtomicResultsText); - JsonConverterSupport.WriteSubTree(writer, value.Results, options); + WriteSubTree(writer, value.Results, options); } if (!value.Errors.IsNullOrEmpty()) { writer.WritePropertyName(ErrorsText); - JsonConverterSupport.WriteSubTree(writer, value.Errors, options); + WriteSubTree(writer, value.Errors, options); } if (value.Included != null) { writer.WritePropertyName(IncludedText); - JsonConverterSupport.WriteSubTree(writer, value.Included, options); + WriteSubTree(writer, value.Included, options); } if (!value.Meta.IsNullOrEmpty()) { writer.WritePropertyName(MetaText); - JsonConverterSupport.WriteSubTree(writer, value.Meta, options); + WriteSubTree(writer, value.Meta, options); } writer.WriteEndObject(); diff --git a/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyRelationshipObjectConverter.cs b/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyRelationshipObjectConverter.cs index 43c7f4db85..d80fcee5bd 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyRelationshipObjectConverter.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonConverters/WriteOnlyRelationshipObjectConverter.cs @@ -1,6 +1,5 @@ using System; using System.Text.Json; -using System.Text.Json.Serialization; using JetBrains.Annotations; using JsonApiDotNetCore.Serialization.Objects; @@ -10,7 +9,7 @@ namespace JsonApiDotNetCore.Serialization.JsonConverters /// Converts to JSON. /// [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)] - public sealed class WriteOnlyRelationshipObjectConverter : JsonConverter + public sealed class WriteOnlyRelationshipObjectConverter : JsonObjectConverter { private static readonly JsonEncodedText DataText = JsonEncodedText.Encode("data"); private static readonly JsonEncodedText LinksText = JsonEncodedText.Encode("links"); @@ -31,19 +30,19 @@ public override void Write(Utf8JsonWriter writer, RelationshipObject value, Json if (value.Links != null && value.Links.HasValue()) { writer.WritePropertyName(LinksText); - JsonConverterSupport.WriteSubTree(writer, value.Links, options); + WriteSubTree(writer, value.Links, options); } if (value.Data.IsAssigned) { writer.WritePropertyName(DataText); - JsonConverterSupport.WriteSubTree(writer, value.Data, options); + WriteSubTree(writer, value.Data, options); } if (!value.Meta.IsNullOrEmpty()) { writer.WritePropertyName(MetaText); - JsonConverterSupport.WriteSubTree(writer, value.Meta, options); + WriteSubTree(writer, value.Meta, options); } writer.WriteEndObject(); diff --git a/src/JsonApiDotNetCore/Serialization/JsonObjectConverter.cs b/src/JsonApiDotNetCore/Serialization/JsonObjectConverter.cs new file mode 100644 index 0000000000..8e1e5f44d3 --- /dev/null +++ b/src/JsonApiDotNetCore/Serialization/JsonObjectConverter.cs @@ -0,0 +1,35 @@ +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace JsonApiDotNetCore.Serialization +{ + public abstract class JsonObjectConverter : JsonConverter + { + protected static TValue ReadSubTree(ref Utf8JsonReader reader, JsonSerializerOptions options) + { + if (typeof(TValue) != typeof(object) && options?.GetConverter(typeof(TValue)) is JsonConverter converter) + { + return converter.Read(ref reader, typeof(TValue), options); + } + + return JsonSerializer.Deserialize(ref reader, options); + } + + protected static void WriteSubTree(Utf8JsonWriter writer, TValue value, JsonSerializerOptions options) + { + if (typeof(TValue) != typeof(object) && options?.GetConverter(typeof(TValue)) is JsonConverter converter) + { + converter.Write(writer, value, options); + } + else + { + JsonSerializer.Serialize(writer, value, options); + } + } + + protected static JsonException GetEndOfStreamError() + { + return new JsonException("Unexpected end of JSON stream."); + } + } +}