Improved JsonApiClient and relationships into openapi-required-and-nullable

appveyor.yml

@@ -14,6 +14,7 @@ environment:
 
 branches:
   only:
+  - openapi-required-and-nullable-properties # TODO: remove
   - master
   - openapi
   - develop

src/JsonApiDotNetCore.OpenApi.Client/IJsonApiClient.cs

@@ -5,14 +5,19 @@ namespace JsonApiDotNetCore.OpenApi.Client;
 public interface IJsonApiClient
 {
     /// <summary>
-    /// Ensures correct serialization of attributes in a POST/PATCH Resource request body. In JSON:API, an omitted attribute indicates to ignore it, while an
-    /// attribute that is set to "null" means to clear it. This poses a problem because the serializer cannot distinguish between "you have explicitly set
-    /// this .NET property to null" vs "you didn't touch it, so it is null by default" when converting an instance to JSON. Therefore, calling this method
-    /// treats all attributes that contain their default value (<c>null</c> for reference types, <c>0</c> for integers, <c>false</c> for booleans, etc) as
-    /// omitted unless explicitly listed to include them using <paramref name="alwaysIncludedAttributeSelectors" />.
+    /// <para>
+    /// Calling this method ensures that attributes containing a default value (<c>null</c> for reference types, <c>0</c> for integers, <c>false</c> for
+    /// booleans, etc) are omitted during serialization, except for those explicitly marked for inclusion in
+    /// <paramref name="alwaysIncludedAttributeSelectors" />.
+    /// </para>
+    /// <para>
+    /// This is sometimes required to ensure correct serialization of attributes during a POST/PATCH request. In JSON:API, an omitted attribute indicates to
+    /// ignore it, while an attribute that is set to "null" means to clear it. This poses a problem because the serializer cannot distinguish between "you
+    /// have explicitly set this .NET property to null" vs "you didn't touch it, so it is null by default" when converting an instance to JSON.
+    /// </para>
     /// </summary>
     /// <param name="requestDocument">
-    /// The request document instance for which this registration applies.
+    /// The request document instance for which default values should be omitted.
     /// </param>
     /// <param name="alwaysIncludedAttributeSelectors">
     /// Optional. A list of expressions to indicate which properties to unconditionally include in the JSON request body. For example:
@@ -30,7 +35,7 @@ public interface IJsonApiClient
     /// An <see cref="IDisposable" /> to clear the current registration. For efficient memory usage, it is recommended to wrap calls to this method in a
     /// <c>using</c> statement, so the registrations are cleaned up after executing the request.
     /// </returns>
-    IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
+    IDisposable OmitDefaultValuesForAttributesInRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
         params Expression<Func<TAttributesObject, object?>>[] alwaysIncludedAttributeSelectors)
         where TRequestDocument : class;
 }

src/JsonApiDotNetCore.OpenApi.Client/JsonApiClient.cs

@@ -3,6 +3,7 @@
 using JetBrains.Annotations;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Serialization;
+using SysNotNull = System.Diagnostics.CodeAnalysis.NotNullAttribute;
 
 namespace JsonApiDotNetCore.OpenApi.Client;
 
@@ -23,7 +24,7 @@ protected void SetSerializerSettingsForJsonApi(JsonSerializerSettings settings)
     }
 
     /// <inheritdoc />
-    public IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
+    public IDisposable OmitDefaultValuesForAttributesInRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
         params Expression<Func<TAttributesObject, object?>>[] alwaysIncludedAttributeSelectors)
         where TRequestDocument : class
     {
@@ -43,9 +44,10 @@ public IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttri
             }
         }
 
-        _jsonApiJsonConverter.RegisterRequestDocument(requestDocument, new AttributeNamesContainer(attributeNames, typeof(TAttributesObject)));
+        _jsonApiJsonConverter.RegisterRequestDocumentForAttributesOmission(requestDocument,
+            new AttributesObjectInfo(attributeNames, typeof(TAttributesObject)));
 
-        return new AttributesRegistrationScope(_jsonApiJsonConverter, requestDocument);
+        return new RequestDocumentRegistrationScope(_jsonApiJsonConverter, requestDocument);
     }
 
     private static Expression RemoveConvert(Expression expression)
@@ -67,38 +69,38 @@ private static Expression RemoveConvert(Expression expression)
 
     private sealed class JsonApiJsonConverter : JsonConverter
     {
-        private readonly Dictionary<object, AttributeNamesContainer> _alwaysIncludedAttributesPerRequestDocumentInstance = new();
-        private readonly Dictionary<Type, ISet<object>> _requestDocumentInstancesPerRequestDocumentType = new();
-        private bool _isSerializing;
+        private readonly Dictionary<object, AttributesObjectInfo> _attributesObjectInfoByRequestDocument = new();
+        private readonly Dictionary<Type, ISet<object>> _requestDocumentsByType = new();
+        private SerializationScope? _serializationScope;
 
         public override bool CanRead => false;
 
-        public void RegisterRequestDocument(object requestDocument, AttributeNamesContainer attributes)
+        public void RegisterRequestDocumentForAttributesOmission(object requestDocument, AttributesObjectInfo attributesObjectInfo)
         {
-            _alwaysIncludedAttributesPerRequestDocumentInstance[requestDocument] = attributes;
+            _attributesObjectInfoByRequestDocument[requestDocument] = attributesObjectInfo;
 
             Type requestDocumentType = requestDocument.GetType();
 
-            if (!_requestDocumentInstancesPerRequestDocumentType.ContainsKey(requestDocumentType))
+            if (!_requestDocumentsByType.ContainsKey(requestDocumentType))
             {
-                _requestDocumentInstancesPerRequestDocumentType[requestDocumentType] = new HashSet<object>();
+                _requestDocumentsByType[requestDocumentType] = new HashSet<object>();
             }
 
-            _requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Add(requestDocument);
+            _requestDocumentsByType[requestDocumentType].Add(requestDocument);
         }
 
-        public void RemoveAttributeRegistration(object requestDocument)
+        public void RemoveRegistration(object requestDocument)
         {
-            if (_alwaysIncludedAttributesPerRequestDocumentInstance.ContainsKey(requestDocument))
+            if (_attributesObjectInfoByRequestDocument.ContainsKey(requestDocument))
             {
-                _alwaysIncludedAttributesPerRequestDocumentInstance.Remove(requestDocument);
+                _attributesObjectInfoByRequestDocument.Remove(requestDocument);
 
                 Type requestDocumentType = requestDocument.GetType();
-                _requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Remove(requestDocument);
+                _requestDocumentsByType[requestDocumentType].Remove(requestDocument);
 
-                if (!_requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Any())
+                if (!_requestDocumentsByType[requestDocumentType].Any())
                 {
-                    _requestDocumentInstancesPerRequestDocumentType.Remove(requestDocumentType);
+                    _requestDocumentsByType.Remove(requestDocumentType);
                 }
             }
         }
@@ -107,71 +109,195 @@ public override bool CanConvert(Type objectType)
         {
             ArgumentGuard.NotNull(objectType);
 
-            return !_isSerializing && _requestDocumentInstancesPerRequestDocumentType.ContainsKey(objectType);
+            if (_serializationScope == null)
+            {
+                return _requestDocumentsByType.ContainsKey(objectType);
+            }
+
+            return _serializationScope.ShouldConvertAsAttributesObject(objectType);
         }
 
         public override object ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
         {
-            throw new Exception("This code should not be reachable.");
+            throw new UnreachableCodeException();
         }
 
         public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
         {
             ArgumentGuard.NotNull(writer);
+            ArgumentGuard.NotNull(value);
             ArgumentGuard.NotNull(serializer);
 
-            if (value != null)
+            if (_serializationScope == null)
             {
-                if (_alwaysIncludedAttributesPerRequestDocumentInstance.ContainsKey(value))
-                {
-                    AttributeNamesContainer attributeNamesContainer = _alwaysIncludedAttributesPerRequestDocumentInstance[value];
-                    serializer.ContractResolver = new JsonApiDocumentContractResolver(attributeNamesContainer);
-                }
+                AssertObjectIsRequestDocument(value);
 
-                try
-                {
-                    _isSerializing = true;
-                    serializer.Serialize(writer, value);
-                }
-                finally
+                SerializeRequestDocument(writer, value, serializer);
+            }
+            else
+            {
+                AttributesObjectInfo? attributesObjectInfo = _serializationScope.AttributesObjectInScope;
+
+                AssertObjectMatchesSerializationScope(attributesObjectInfo, value);
+
+                SerializeAttributesObject(attributesObjectInfo, writer, value, serializer);
+            }
+        }
+
+        private void AssertObjectIsRequestDocument(object value)
+        {
+            Type objectType = value.GetType();
+
+            if (!_requestDocumentsByType.ContainsKey(objectType))
+            {
+                throw new UnreachableCodeException();
+            }
+        }
+
+        private void SerializeRequestDocument(JsonWriter writer, object value, JsonSerializer serializer)
+        {
+            _serializationScope = new SerializationScope();
+
+            if (_attributesObjectInfoByRequestDocument.TryGetValue(value, out AttributesObjectInfo? attributesObjectInfo))
+            {
+                _serializationScope.AttributesObjectInScope = attributesObjectInfo;
+            }
+
+            try
+            {
+                serializer.Serialize(writer, value);
+            }
+            finally
+            {
+                _serializationScope = null;
+            }
+        }
+
+        private static void AssertObjectMatchesSerializationScope([SysNotNull] AttributesObjectInfo? attributesObjectInfo, object value)
+        {
+            Type objectType = value.GetType();
+
+            if (attributesObjectInfo == null || !attributesObjectInfo.MatchesType(objectType))
+            {
+                throw new UnreachableCodeException();
+            }
+        }
+
+        private static void SerializeAttributesObject(AttributesObjectInfo alwaysIncludedAttributes, JsonWriter writer, object value, JsonSerializer serializer)
+        {
+            AssertRequiredPropertiesAreNotExcluded(value, alwaysIncludedAttributes, writer);
+
+            serializer.ContractResolver = new JsonApiAttributeContractResolver(alwaysIncludedAttributes);
+            serializer.Serialize(writer, value);
+        }
+
+        private static void AssertRequiredPropertiesAreNotExcluded(object value, AttributesObjectInfo alwaysIncludedAttributes, JsonWriter jsonWriter)
+        {
+            PropertyInfo[] propertyInfos = value.GetType().GetProperties();
+
+            foreach (PropertyInfo attributesPropertyInfo in propertyInfos)
+            {
+                bool isExplicitlyIncluded = alwaysIncludedAttributes.IsAttributeMarkedForInclusion(attributesPropertyInfo.Name);
+
+                if (isExplicitlyIncluded)
                 {
-                    _isSerializing = false;
+                    return;
                 }
+
+                AssertRequiredPropertyIsNotIgnored(value, attributesPropertyInfo, jsonWriter.Path);
+            }
+        }
+
+        private static void AssertRequiredPropertyIsNotIgnored(object value, PropertyInfo attribute, string path)
+        {
+            JsonPropertyAttribute jsonPropertyForAttribute = attribute.GetCustomAttributes<JsonPropertyAttribute>().Single();
+
+            if (jsonPropertyForAttribute.Required != Required.Always)
+            {
+                return;
+            }
+
+            bool isPropertyIgnored = DefaultValueEqualsCurrentValue(attribute, value);
+
+            if (isPropertyIgnored)
+            {
+                throw new JsonSerializationException(
+                    $"Ignored property '{jsonPropertyForAttribute.PropertyName}' must have a value because it is required. Path '{path}'.");
             }
         }
+
+        private static bool DefaultValueEqualsCurrentValue(PropertyInfo propertyInfo, object instance)
+        {
+            object? currentValue = propertyInfo.GetValue(instance);
+            object? defaultValue = GetDefaultValue(propertyInfo.PropertyType);
+
+            if (defaultValue == null)
+            {
+                return currentValue == null;
+            }
+
+            return defaultValue.Equals(currentValue);
+        }
+
+        private static object? GetDefaultValue(Type type)
+        {
+            return type.IsValueType ? Activator.CreateInstance(type) : null;
+        }
+    }
+
+    private sealed class SerializationScope
+    {
+        private bool _isFirstAttemptToConvertAttributes = true;
+        public AttributesObjectInfo? AttributesObjectInScope { get; set; }
+
+        public bool ShouldConvertAsAttributesObject(Type type)
+        {
+            if (!_isFirstAttemptToConvertAttributes || AttributesObjectInScope == null)
+            {
+                return false;
+            }
+
+            if (!AttributesObjectInScope.MatchesType(type))
+            {
+                return false;
+            }
+
+            _isFirstAttemptToConvertAttributes = false;
+            return true;
+        }
     }
 
-    private sealed class AttributeNamesContainer
+    private sealed class AttributesObjectInfo
     {
-        private readonly ISet<string> _attributeNames;
-        private readonly Type _containerType;
+        private readonly ISet<string> _attributesMarkedForInclusion;
+        private readonly Type _attributesObjectType;
 
-        public AttributeNamesContainer(ISet<string> attributeNames, Type containerType)
+        public AttributesObjectInfo(ISet<string> attributesMarkedForInclusion, Type attributesObjectType)
         {
-            ArgumentGuard.NotNull(attributeNames);
-            ArgumentGuard.NotNull(containerType);
+            ArgumentGuard.NotNull(attributesMarkedForInclusion);
+            ArgumentGuard.NotNull(attributesObjectType);
 
-            _attributeNames = attributeNames;
-            _containerType = containerType;
+            _attributesMarkedForInclusion = attributesMarkedForInclusion;
+            _attributesObjectType = attributesObjectType;
         }
 
-        public bool ContainsAttribute(string name)
+        public bool IsAttributeMarkedForInclusion(string name)
         {
-            return _attributeNames.Contains(name);
+            return _attributesMarkedForInclusion.Contains(name);
         }
 
-        public bool ContainerMatchesType(Type type)
+        public bool MatchesType(Type type)
         {
-            return _containerType == type;
+            return _attributesObjectType == type;
         }
     }
 
-    private sealed class AttributesRegistrationScope : IDisposable
+    private sealed class RequestDocumentRegistrationScope : IDisposable
     {
         private readonly JsonApiJsonConverter _jsonApiJsonConverter;
         private readonly object _requestDocument;
 
-        public AttributesRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, object requestDocument)
+        public RequestDocumentRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, object requestDocument)
         {
             ArgumentGuard.NotNull(jsonApiJsonConverter);
             ArgumentGuard.NotNull(requestDocument);
@@ -182,28 +308,28 @@ public AttributesRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, ob
 
         public void Dispose()
         {
-            _jsonApiJsonConverter.RemoveAttributeRegistration(_requestDocument);
+            _jsonApiJsonConverter.RemoveRegistration(_requestDocument);
         }
     }
 
-    private sealed class JsonApiDocumentContractResolver : DefaultContractResolver
+    private sealed class JsonApiAttributeContractResolver : DefaultContractResolver
     {
-        private readonly AttributeNamesContainer _attributeNamesContainer;
+        private readonly AttributesObjectInfo _attributesObjectInfo;
 
-        public JsonApiDocumentContractResolver(AttributeNamesContainer attributeNamesContainer)
+        public JsonApiAttributeContractResolver(AttributesObjectInfo attributesObjectInfo)
         {
-            ArgumentGuard.NotNull(attributeNamesContainer);
+            ArgumentGuard.NotNull(attributesObjectInfo);
 
-            _attributeNamesContainer = attributeNamesContainer;
+            _attributesObjectInfo = attributesObjectInfo;
         }
 
         protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
         {
             JsonProperty property = base.CreateProperty(member, memberSerialization);
 
-            if (_attributeNamesContainer.ContainerMatchesType(property.DeclaringType!))
+            if (_attributesObjectInfo.MatchesType(property.DeclaringType!))
             {
-                if (_attributeNamesContainer.ContainsAttribute(property.UnderlyingName!))
+                if (_attributesObjectInfo.IsAttributeMarkedForInclusion(property.UnderlyingName!))
                 {
                     property.NullValueHandling = NullValueHandling.Include;
                     property.DefaultValueHandling = DefaultValueHandling.Include;

src/JsonApiDotNetCore.OpenApi.Client/UnreachableCodeException.cs

@@ -0,0 +1,9 @@
+namespace JsonApiDotNetCore.OpenApi.Client;
+
+internal sealed class UnreachableCodeException : Exception
+{
+    public UnreachableCodeException()
+        : base("This code should not be reachable.")
+    {
+    }
+}