Skip to content

Commit d4f1d69

Browse files
authored
Merge pull request #1231 from json-api-dotnet/attributes-object-schema-property-tests
Improved JsonApiClient and relationships into `openapi-required-and-nullable`
2 parents daf25a2 + affc187 commit d4f1d69

26 files changed

+5876
-349
lines changed

appveyor.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ environment:
1414

1515
branches:
1616
only:
17+
- openapi-required-and-nullable-properties # TODO: remove
1718
- master
1819
- openapi
1920
- develop

src/JsonApiDotNetCore.OpenApi.Client/IJsonApiClient.cs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,19 @@ namespace JsonApiDotNetCore.OpenApi.Client;
55
public interface IJsonApiClient
66
{
77
/// <summary>
8-
/// Ensures correct serialization of attributes in a POST/PATCH Resource request body. In JSON:API, an omitted attribute indicates to ignore it, while an
9-
/// attribute that is set to "null" means to clear it. This poses a problem because the serializer cannot distinguish between "you have explicitly set
10-
/// 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
11-
/// 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
12-
/// omitted unless explicitly listed to include them using <paramref name="alwaysIncludedAttributeSelectors" />.
8+
/// <para>
9+
/// 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
10+
/// booleans, etc) are omitted during serialization, except for those explicitly marked for inclusion in
11+
/// <paramref name="alwaysIncludedAttributeSelectors" />.
12+
/// </para>
13+
/// <para>
14+
/// This is sometimes required to ensure correct serialization of attributes during a POST/PATCH request. In JSON:API, an omitted attribute indicates to
15+
/// 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
16+
/// 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.
17+
/// </para>
1318
/// </summary>
1419
/// <param name="requestDocument">
15-
/// The request document instance for which this registration applies.
20+
/// The request document instance for which default values should be omitted.
1621
/// </param>
1722
/// <param name="alwaysIncludedAttributeSelectors">
1823
/// 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
3035
/// An <see cref="IDisposable" /> to clear the current registration. For efficient memory usage, it is recommended to wrap calls to this method in a
3136
/// <c>using</c> statement, so the registrations are cleaned up after executing the request.
3237
/// </returns>
33-
IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
38+
IDisposable OmitDefaultValuesForAttributesInRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
3439
params Expression<Func<TAttributesObject, object?>>[] alwaysIncludedAttributeSelectors)
3540
where TRequestDocument : class;
3641
}

src/JsonApiDotNetCore.OpenApi.Client/JsonApiClient.cs

Lines changed: 180 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using JetBrains.Annotations;
44
using Newtonsoft.Json;
55
using Newtonsoft.Json.Serialization;
6+
using SysNotNull = System.Diagnostics.CodeAnalysis.NotNullAttribute;
67

78
namespace JsonApiDotNetCore.OpenApi.Client;
89

@@ -23,7 +24,7 @@ protected void SetSerializerSettingsForJsonApi(JsonSerializerSettings settings)
2324
}
2425

2526
/// <inheritdoc />
26-
public IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
27+
public IDisposable OmitDefaultValuesForAttributesInRequestDocument<TRequestDocument, TAttributesObject>(TRequestDocument requestDocument,
2728
params Expression<Func<TAttributesObject, object?>>[] alwaysIncludedAttributeSelectors)
2829
where TRequestDocument : class
2930
{
@@ -43,9 +44,10 @@ public IDisposable RegisterAttributesForRequestDocument<TRequestDocument, TAttri
4344
}
4445
}
4546

46-
_jsonApiJsonConverter.RegisterRequestDocument(requestDocument, new AttributeNamesContainer(attributeNames, typeof(TAttributesObject)));
47+
_jsonApiJsonConverter.RegisterRequestDocumentForAttributesOmission(requestDocument,
48+
new AttributesObjectInfo(attributeNames, typeof(TAttributesObject)));
4749

48-
return new AttributesRegistrationScope(_jsonApiJsonConverter, requestDocument);
50+
return new RequestDocumentRegistrationScope(_jsonApiJsonConverter, requestDocument);
4951
}
5052

5153
private static Expression RemoveConvert(Expression expression)
@@ -67,38 +69,38 @@ private static Expression RemoveConvert(Expression expression)
6769

6870
private sealed class JsonApiJsonConverter : JsonConverter
6971
{
70-
private readonly Dictionary<object, AttributeNamesContainer> _alwaysIncludedAttributesPerRequestDocumentInstance = new();
71-
private readonly Dictionary<Type, ISet<object>> _requestDocumentInstancesPerRequestDocumentType = new();
72-
private bool _isSerializing;
72+
private readonly Dictionary<object, AttributesObjectInfo> _attributesObjectInfoByRequestDocument = new();
73+
private readonly Dictionary<Type, ISet<object>> _requestDocumentsByType = new();
74+
private SerializationScope? _serializationScope;
7375

7476
public override bool CanRead => false;
7577

76-
public void RegisterRequestDocument(object requestDocument, AttributeNamesContainer attributes)
78+
public void RegisterRequestDocumentForAttributesOmission(object requestDocument, AttributesObjectInfo attributesObjectInfo)
7779
{
78-
_alwaysIncludedAttributesPerRequestDocumentInstance[requestDocument] = attributes;
80+
_attributesObjectInfoByRequestDocument[requestDocument] = attributesObjectInfo;
7981

8082
Type requestDocumentType = requestDocument.GetType();
8183

82-
if (!_requestDocumentInstancesPerRequestDocumentType.ContainsKey(requestDocumentType))
84+
if (!_requestDocumentsByType.ContainsKey(requestDocumentType))
8385
{
84-
_requestDocumentInstancesPerRequestDocumentType[requestDocumentType] = new HashSet<object>();
86+
_requestDocumentsByType[requestDocumentType] = new HashSet<object>();
8587
}
8688

87-
_requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Add(requestDocument);
89+
_requestDocumentsByType[requestDocumentType].Add(requestDocument);
8890
}
8991

90-
public void RemoveAttributeRegistration(object requestDocument)
92+
public void RemoveRegistration(object requestDocument)
9193
{
92-
if (_alwaysIncludedAttributesPerRequestDocumentInstance.ContainsKey(requestDocument))
94+
if (_attributesObjectInfoByRequestDocument.ContainsKey(requestDocument))
9395
{
94-
_alwaysIncludedAttributesPerRequestDocumentInstance.Remove(requestDocument);
96+
_attributesObjectInfoByRequestDocument.Remove(requestDocument);
9597

9698
Type requestDocumentType = requestDocument.GetType();
97-
_requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Remove(requestDocument);
99+
_requestDocumentsByType[requestDocumentType].Remove(requestDocument);
98100

99-
if (!_requestDocumentInstancesPerRequestDocumentType[requestDocumentType].Any())
101+
if (!_requestDocumentsByType[requestDocumentType].Any())
100102
{
101-
_requestDocumentInstancesPerRequestDocumentType.Remove(requestDocumentType);
103+
_requestDocumentsByType.Remove(requestDocumentType);
102104
}
103105
}
104106
}
@@ -107,71 +109,195 @@ public override bool CanConvert(Type objectType)
107109
{
108110
ArgumentGuard.NotNull(objectType);
109111

110-
return !_isSerializing && _requestDocumentInstancesPerRequestDocumentType.ContainsKey(objectType);
112+
if (_serializationScope == null)
113+
{
114+
return _requestDocumentsByType.ContainsKey(objectType);
115+
}
116+
117+
return _serializationScope.ShouldConvertAsAttributesObject(objectType);
111118
}
112119

113120
public override object ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
114121
{
115-
throw new Exception("This code should not be reachable.");
122+
throw new UnreachableCodeException();
116123
}
117124

118125
public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
119126
{
120127
ArgumentGuard.NotNull(writer);
128+
ArgumentGuard.NotNull(value);
121129
ArgumentGuard.NotNull(serializer);
122130

123-
if (value != null)
131+
if (_serializationScope == null)
124132
{
125-
if (_alwaysIncludedAttributesPerRequestDocumentInstance.ContainsKey(value))
126-
{
127-
AttributeNamesContainer attributeNamesContainer = _alwaysIncludedAttributesPerRequestDocumentInstance[value];
128-
serializer.ContractResolver = new JsonApiDocumentContractResolver(attributeNamesContainer);
129-
}
133+
AssertObjectIsRequestDocument(value);
130134

131-
try
132-
{
133-
_isSerializing = true;
134-
serializer.Serialize(writer, value);
135-
}
136-
finally
135+
SerializeRequestDocument(writer, value, serializer);
136+
}
137+
else
138+
{
139+
AttributesObjectInfo? attributesObjectInfo = _serializationScope.AttributesObjectInScope;
140+
141+
AssertObjectMatchesSerializationScope(attributesObjectInfo, value);
142+
143+
SerializeAttributesObject(attributesObjectInfo, writer, value, serializer);
144+
}
145+
}
146+
147+
private void AssertObjectIsRequestDocument(object value)
148+
{
149+
Type objectType = value.GetType();
150+
151+
if (!_requestDocumentsByType.ContainsKey(objectType))
152+
{
153+
throw new UnreachableCodeException();
154+
}
155+
}
156+
157+
private void SerializeRequestDocument(JsonWriter writer, object value, JsonSerializer serializer)
158+
{
159+
_serializationScope = new SerializationScope();
160+
161+
if (_attributesObjectInfoByRequestDocument.TryGetValue(value, out AttributesObjectInfo? attributesObjectInfo))
162+
{
163+
_serializationScope.AttributesObjectInScope = attributesObjectInfo;
164+
}
165+
166+
try
167+
{
168+
serializer.Serialize(writer, value);
169+
}
170+
finally
171+
{
172+
_serializationScope = null;
173+
}
174+
}
175+
176+
private static void AssertObjectMatchesSerializationScope([SysNotNull] AttributesObjectInfo? attributesObjectInfo, object value)
177+
{
178+
Type objectType = value.GetType();
179+
180+
if (attributesObjectInfo == null || !attributesObjectInfo.MatchesType(objectType))
181+
{
182+
throw new UnreachableCodeException();
183+
}
184+
}
185+
186+
private static void SerializeAttributesObject(AttributesObjectInfo alwaysIncludedAttributes, JsonWriter writer, object value, JsonSerializer serializer)
187+
{
188+
AssertRequiredPropertiesAreNotExcluded(value, alwaysIncludedAttributes, writer);
189+
190+
serializer.ContractResolver = new JsonApiAttributeContractResolver(alwaysIncludedAttributes);
191+
serializer.Serialize(writer, value);
192+
}
193+
194+
private static void AssertRequiredPropertiesAreNotExcluded(object value, AttributesObjectInfo alwaysIncludedAttributes, JsonWriter jsonWriter)
195+
{
196+
PropertyInfo[] propertyInfos = value.GetType().GetProperties();
197+
198+
foreach (PropertyInfo attributesPropertyInfo in propertyInfos)
199+
{
200+
bool isExplicitlyIncluded = alwaysIncludedAttributes.IsAttributeMarkedForInclusion(attributesPropertyInfo.Name);
201+
202+
if (isExplicitlyIncluded)
137203
{
138-
_isSerializing = false;
204+
return;
139205
}
206+
207+
AssertRequiredPropertyIsNotIgnored(value, attributesPropertyInfo, jsonWriter.Path);
208+
}
209+
}
210+
211+
private static void AssertRequiredPropertyIsNotIgnored(object value, PropertyInfo attribute, string path)
212+
{
213+
JsonPropertyAttribute jsonPropertyForAttribute = attribute.GetCustomAttributes<JsonPropertyAttribute>().Single();
214+
215+
if (jsonPropertyForAttribute.Required != Required.Always)
216+
{
217+
return;
218+
}
219+
220+
bool isPropertyIgnored = DefaultValueEqualsCurrentValue(attribute, value);
221+
222+
if (isPropertyIgnored)
223+
{
224+
throw new JsonSerializationException(
225+
$"Ignored property '{jsonPropertyForAttribute.PropertyName}' must have a value because it is required. Path '{path}'.");
140226
}
141227
}
228+
229+
private static bool DefaultValueEqualsCurrentValue(PropertyInfo propertyInfo, object instance)
230+
{
231+
object? currentValue = propertyInfo.GetValue(instance);
232+
object? defaultValue = GetDefaultValue(propertyInfo.PropertyType);
233+
234+
if (defaultValue == null)
235+
{
236+
return currentValue == null;
237+
}
238+
239+
return defaultValue.Equals(currentValue);
240+
}
241+
242+
private static object? GetDefaultValue(Type type)
243+
{
244+
return type.IsValueType ? Activator.CreateInstance(type) : null;
245+
}
246+
}
247+
248+
private sealed class SerializationScope
249+
{
250+
private bool _isFirstAttemptToConvertAttributes = true;
251+
public AttributesObjectInfo? AttributesObjectInScope { get; set; }
252+
253+
public bool ShouldConvertAsAttributesObject(Type type)
254+
{
255+
if (!_isFirstAttemptToConvertAttributes || AttributesObjectInScope == null)
256+
{
257+
return false;
258+
}
259+
260+
if (!AttributesObjectInScope.MatchesType(type))
261+
{
262+
return false;
263+
}
264+
265+
_isFirstAttemptToConvertAttributes = false;
266+
return true;
267+
}
142268
}
143269

144-
private sealed class AttributeNamesContainer
270+
private sealed class AttributesObjectInfo
145271
{
146-
private readonly ISet<string> _attributeNames;
147-
private readonly Type _containerType;
272+
private readonly ISet<string> _attributesMarkedForInclusion;
273+
private readonly Type _attributesObjectType;
148274

149-
public AttributeNamesContainer(ISet<string> attributeNames, Type containerType)
275+
public AttributesObjectInfo(ISet<string> attributesMarkedForInclusion, Type attributesObjectType)
150276
{
151-
ArgumentGuard.NotNull(attributeNames);
152-
ArgumentGuard.NotNull(containerType);
277+
ArgumentGuard.NotNull(attributesMarkedForInclusion);
278+
ArgumentGuard.NotNull(attributesObjectType);
153279

154-
_attributeNames = attributeNames;
155-
_containerType = containerType;
280+
_attributesMarkedForInclusion = attributesMarkedForInclusion;
281+
_attributesObjectType = attributesObjectType;
156282
}
157283

158-
public bool ContainsAttribute(string name)
284+
public bool IsAttributeMarkedForInclusion(string name)
159285
{
160-
return _attributeNames.Contains(name);
286+
return _attributesMarkedForInclusion.Contains(name);
161287
}
162288

163-
public bool ContainerMatchesType(Type type)
289+
public bool MatchesType(Type type)
164290
{
165-
return _containerType == type;
291+
return _attributesObjectType == type;
166292
}
167293
}
168294

169-
private sealed class AttributesRegistrationScope : IDisposable
295+
private sealed class RequestDocumentRegistrationScope : IDisposable
170296
{
171297
private readonly JsonApiJsonConverter _jsonApiJsonConverter;
172298
private readonly object _requestDocument;
173299

174-
public AttributesRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, object requestDocument)
300+
public RequestDocumentRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, object requestDocument)
175301
{
176302
ArgumentGuard.NotNull(jsonApiJsonConverter);
177303
ArgumentGuard.NotNull(requestDocument);
@@ -182,28 +308,28 @@ public AttributesRegistrationScope(JsonApiJsonConverter jsonApiJsonConverter, ob
182308

183309
public void Dispose()
184310
{
185-
_jsonApiJsonConverter.RemoveAttributeRegistration(_requestDocument);
311+
_jsonApiJsonConverter.RemoveRegistration(_requestDocument);
186312
}
187313
}
188314

189-
private sealed class JsonApiDocumentContractResolver : DefaultContractResolver
315+
private sealed class JsonApiAttributeContractResolver : DefaultContractResolver
190316
{
191-
private readonly AttributeNamesContainer _attributeNamesContainer;
317+
private readonly AttributesObjectInfo _attributesObjectInfo;
192318

193-
public JsonApiDocumentContractResolver(AttributeNamesContainer attributeNamesContainer)
319+
public JsonApiAttributeContractResolver(AttributesObjectInfo attributesObjectInfo)
194320
{
195-
ArgumentGuard.NotNull(attributeNamesContainer);
321+
ArgumentGuard.NotNull(attributesObjectInfo);
196322

197-
_attributeNamesContainer = attributeNamesContainer;
323+
_attributesObjectInfo = attributesObjectInfo;
198324
}
199325

200326
protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
201327
{
202328
JsonProperty property = base.CreateProperty(member, memberSerialization);
203329

204-
if (_attributeNamesContainer.ContainerMatchesType(property.DeclaringType!))
330+
if (_attributesObjectInfo.MatchesType(property.DeclaringType!))
205331
{
206-
if (_attributeNamesContainer.ContainsAttribute(property.UnderlyingName!))
332+
if (_attributesObjectInfo.IsAttributeMarkedForInclusion(property.UnderlyingName!))
207333
{
208334
property.NullValueHandling = NullValueHandling.Include;
209335
property.DefaultValueHandling = DefaultValueHandling.Include;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
namespace JsonApiDotNetCore.OpenApi.Client;
2+
3+
internal sealed class UnreachableCodeException : Exception
4+
{
5+
public UnreachableCodeException()
6+
: base("This code should not be reachable.")
7+
{
8+
}
9+
}

0 commit comments

Comments
 (0)