Rely on supported AllOf approach for creating nullable reference schemas

appveyor.yml

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

src/JsonApiDotNetCore.OpenApi.Client/ApiException.cs

@@ -15,7 +15,7 @@ public sealed class ApiException : Exception
 
     public IReadOnlyDictionary<string, IEnumerable<string>> Headers { get; }
 
-    public ApiException(string message, int statusCode, string? response, IReadOnlyDictionary<string, IEnumerable<string>> headers, Exception innerException)
+    public ApiException(string message, int statusCode, string? response, IReadOnlyDictionary<string, IEnumerable<string>> headers, Exception? innerException)
         : base($"{message}\n\nStatus: {statusCode}\nResponse: \n{response ?? "(null)"}", innerException)
     {
         StatusCode = statusCode;

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 WithPartialAttributeSerialization<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 WithPartialAttributeSerialization<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
         params Expression<Func<TAttributesObject, object?>>[] alwaysIncludedAttributeSelectors)
         where TRequestDocument : class
     {
@@ -39,13 +40,14 @@ public IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttri
             }
             else
             {
-                throw new ArgumentException($"The expression '{selector}' should select a single property. For example: 'article => article.Title'.");
+                throw new ArgumentException(
+                    $"The expression '{nameof(alwaysIncludedAttributeSelectors)}' should select a single property. For example: 'article => article.Title'.");
             }
         }
 
-        _jsonApiJsonConverter.RegisterRequestDocument(requestDocument, new AttributeNamesContainer(attributeNames, typeof(TAttributesObject)));
+        _jsonApiJsonConverter.RegisterDocument(requestDocument, new AttributeNamesContainer(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, AttributeNamesContainer> _alwaysIncludedAttributesByRequestDocument = 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 RegisterDocument(object requestDocument, AttributeNamesContainer alwaysIncludedAttributes)
         {
-            _alwaysIncludedAttributesPerRequestDocumentInstance[requestDocument] = attributes;
+            _alwaysIncludedAttributesByRequestDocument[requestDocument] = alwaysIncludedAttributes;
 
             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 (_alwaysIncludedAttributesByRequestDocument.ContainsKey(requestDocument))
             {
-                _alwaysIncludedAttributesPerRequestDocumentInstance.Remove(requestDocument);
+                _alwaysIncludedAttributesByRequestDocument.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
+            {
+                AttributeNamesContainer? 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 (_alwaysIncludedAttributesByRequestDocument.TryGetValue(value, out AttributeNamesContainer? attributesObjectInfo))
+            {
+                _serializationScope.AttributesObjectInScope = attributesObjectInfo;
+            }
+
+            try
+            {
+                serializer.Serialize(writer, value);
+            }
+            finally
+            {
+                _serializationScope = null;
+            }
+        }
+
+        private static void AssertObjectMatchesSerializationScope([SysNotNull] AttributeNamesContainer? attributesObjectInfo, object value)
+        {
+            Type objectType = value.GetType();
+
+            if (attributesObjectInfo == null || !attributesObjectInfo.MatchesAttributesObjectType(objectType))
+            {
+                throw new UnreachableCodeException();
+            }
+        }
+
+        private static void SerializeAttributesObject(AttributeNamesContainer 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, AttributeNamesContainer alwaysIncludedAttributes, JsonWriter jsonWriter)
+        {
+            PropertyInfo[] propertyInfos = value.GetType().GetProperties();
+
+            foreach (PropertyInfo attributesPropertyInfo in propertyInfos)
+            {
+                bool isExplicitlyIncluded = alwaysIncludedAttributes.ContainsAttribute(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 is not (Required.Always or Required.AllowNull))
+            {
+                return;
+            }
+
+            bool isPropertyIgnored = DefaultValueEqualsCurrentValue(attribute, value);
+
+            if (isPropertyIgnored)
+            {
+                throw new InvalidOperationException($"The following property should not be omitted: {path}.{jsonPropertyForAttribute.PropertyName}.");
+            }
+        }
+
+        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 AttributeNamesContainer? AttributesObjectInScope { get; set; }
+
+        public bool ShouldConvertAsAttributesObject(Type type)
+        {
+            if (!_isFirstAttemptToConvertAttributes || AttributesObjectInScope == null)
+            {
+                return false;
             }
+
+            if (!AttributesObjectInScope.MatchesAttributesObjectType(type))
+            {
+                return false;
+            }
+
+            _isFirstAttemptToConvertAttributes = false;
+            return true;
         }
     }
 
     private sealed class AttributeNamesContainer
     {
         private readonly ISet<string> _attributeNames;
-        private readonly Type _containerType;
+        private readonly Type _attributesObjectType;
 
-        public AttributeNamesContainer(ISet<string> attributeNames, Type containerType)
+        public AttributeNamesContainer(ISet<string> attributeNames, Type attributesObjectType)
         {
             ArgumentGuard.NotNull(attributeNames);
-            ArgumentGuard.NotNull(containerType);
+            ArgumentGuard.NotNull(attributesObjectType);
 
             _attributeNames = attributeNames;
-            _containerType = containerType;
+            _attributesObjectType = attributesObjectType;
         }
 
         public bool ContainsAttribute(string name)
         {
             return _attributeNames.Contains(name);
         }
 
-        public bool ContainerMatchesType(Type type)
+        public bool MatchesAttributesObjectType(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,30 @@ 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 AttributeNamesContainer _alwaysIncludedAttributes;
 
-        public JsonApiDocumentContractResolver(AttributeNamesContainer attributeNamesContainer)
+        public JsonApiAttributeContractResolver(AttributeNamesContainer alwaysIncludedAttributes)
         {
-            ArgumentGuard.NotNull(attributeNamesContainer);
+            ArgumentGuard.NotNull(alwaysIncludedAttributes);
 
-            _attributeNamesContainer = attributeNamesContainer;
+            _alwaysIncludedAttributes = alwaysIncludedAttributes;
         }
 
         protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
         {
             JsonProperty property = base.CreateProperty(member, memberSerialization);
 
-            if (_attributeNamesContainer.ContainerMatchesType(property.DeclaringType!))
+            bool canOmitAttribute = property.Required != Required.Always;
+
+            if (canOmitAttribute && _alwaysIncludedAttributes.MatchesAttributesObjectType(property.DeclaringType!))
             {
-                if (_attributeNamesContainer.ContainsAttribute(property.UnderlyingName!))
+                if (_alwaysIncludedAttributes.ContainsAttribute(property.UnderlyingName!))
                 {
                     property.NullValueHandling = NullValueHandling.Include;
                     property.DefaultValueHandling = DefaultValueHandling.Include;