fix: Minor tweaks to the System.Text.Json JsonEventFormatter for compliance

- Make "JSON media type detection" configurable, and default to */json and */*+json
- Default to JSON content type (including making it present in the serialized event)
- Make a "binary vs non-binary" distinction as the first part of serialization (instead of as a fallback)
- Deserialize string data values regardless of content type
- Prohibit non-string data values for non-JSON content types

Signed-off-by: Jon Skeet <[email protected]>

Author: jskeet

src/CloudNative.CloudEvents.SystemTextJson/JsonEventFormatter.cs

@@ -29,34 +29,44 @@ namespace CloudNative.CloudEvents.SystemTextJson
     /// nor "data_base64" property is populated in a structured mode message.
     /// </description></item>
     /// <item><description>
-    /// If the data content type is absent or has a media type of "application/json", the data is encoded as JSON,
+    /// If the data value is a byte array, it is serialized either directly as binary data
+    /// (for binary mode messages) or as base64 data (for structured mode messages).
+    /// </description></item>
+    /// <item><description>
+    /// Otherwise, if the data content type is absent or has a media type indicating JSON, the data is encoded as JSON,
     /// using the <see cref="JsonSerializerOptions"/> passed into the constructor, or the default options.
     /// </description></item>
     /// <item><description>
     /// Otherwise, if the data content type has a media type beginning with "text/" and the data value is a string,
     /// the data is serialized as a string.
     /// </description></item>
     /// <item><description>
-    /// Otherwise, if the data value is a byte array, it is serialized either directly as binary data
-    /// (for binary mode messages) or as base64 data (for structured mode messages).
-    /// </description></item>
-    /// <item><description>
     /// Otherwise, the encoding operation fails.
     /// </description></item>
     /// </list>
     /// <para>
-    /// When decoding CloudEvent data, this implementation uses the following rules:
-    /// </para>
-    /// <para>
-    /// In a structured mode message, any data is either binary data within the "data_base64" property value,
-    /// or is a JSON token as the "data" property value. Binary data is represented as a byte array.
-    /// A JSON token is decoded as a string if is just a string value and the data content type is specified
-    /// and has a media type beginning with "text/". A JSON token representing the null value always
-    /// leads to a null data result. In any other situation, the JSON token is preserved as a <see cref="JsonElement"/>
-    /// that can be used for further deserialization (e.g. to a specific CLR type). This behavior can be modified
-    /// by overriding <see cref="DecodeStructuredModeDataBase64Property(JsonElement, CloudEvent)"/> and
-    /// <see cref="DecodeStructuredModeDataProperty(JsonElement, CloudEvent)"/>.
+    /// When decoding structured mode CloudEvent data, this implementation uses the following rules,
+    /// which can be modified by overriding <see cref="DecodeStructuredModeDataBase64Property(JsonElement, CloudEvent)"/>
+    /// and <see cref="DecodeStructuredModeDataProperty(JsonElement, CloudEvent)"/>.
     /// </para>
+    /// <list type="bullet">
+    /// <item><description>
+    /// If the "data_base64" property is present, its value is decoded as a byte array.
+    /// </description></item>
+    /// <item><description>
+    /// If the "data" property is present (and non-null) and the content type is absent or indicates a JSON media type,
+    /// the JSON token present in the property is preserved as a <see cref="JsonElement"/> that can be used for further
+    /// deserialization (e.g. to a specific CLR type).
+    /// </description></item>
+    /// <item><description>
+    /// If the "data" property has a string value and a non-JSON content type has been specified, the data is
+    /// deserialized as a string.
+    /// </description></item>
+    /// <item><description>
+    /// If the "data" property has a non-null, non-string value and a non-JSON content type has been specified,
+    /// the deserialization operation fails.
+    /// </description></item>
+    /// </list>
     /// <para>
     /// In a binary mode message, the data is parsed based on the content type of the message. When the content
     /// type is absent or has a media type of "application/json", the data is parsed as JSON, with the result as
@@ -310,6 +320,9 @@ private void PopulateDataFromStructuredEvent(CloudEvent cloudEvent, JsonElement
             }
             else
             {
+                // If no content type has been specified, default to application/json
+                cloudEvent.DataContentType ??= JsonMediaType;
+                
                 DecodeStructuredModeDataProperty(dataElement, cloudEvent);
             }
         }
@@ -347,8 +360,9 @@ protected virtual void DecodeStructuredModeDataBase64Property(JsonElement dataBa
         /// </summary>
         /// <remarks>
         /// <para>
-        /// This implementation converts JSON string tokens to strings when the content type suggests
-        /// that's appropriate, but otherwise returns the token directly.
+        /// This implementation will populate the Data property with the verbatim <see cref="JsonElement"/> if
+        /// the content type is deemed to be JSON according to <see cref="IsJsonMediaType(string)"/>. Otherwise,
+        /// it validates that the token is a string, and the Data property is populated with that string.
         /// </para>
         /// <para>
         /// Override this method to provide more specialized conversions.
@@ -358,12 +372,24 @@ protected virtual void DecodeStructuredModeDataBase64Property(JsonElement dataBa
         /// not have a null token type.</param>
         /// <param name="cloudEvent">The event being decoded. This should not be modified except to
         /// populate the <see cref="CloudEvent.Data"/> property, but may be used to provide extra
-        /// information such as the data content type. Will not be null.</param>
+        /// information such as the data content type. Will not be null, and the <see cref="CloudEvent.DataContentType"/>
+        /// property will be non-null.</param>
         /// <returns>The data to populate in the <see cref="CloudEvent.Data"/> property.</returns>
-        protected virtual void DecodeStructuredModeDataProperty(JsonElement dataElement, CloudEvent cloudEvent) =>
-            cloudEvent.Data = dataElement.ValueKind == JsonValueKind.String && cloudEvent.DataContentType?.StartsWith("text/") == true
-                ? dataElement.GetString()
-                : (object) dataElement.Clone(); // Deliberately cast to object to provide the conditional operator expression type.
+        protected virtual void DecodeStructuredModeDataProperty(JsonElement dataElement, CloudEvent cloudEvent)
+        {
+            if (IsJsonMediaType(cloudEvent.DataContentType!))
+            {
+                cloudEvent.Data = dataElement.Clone();
+            }
+            else
+            {
+                if (dataElement.ValueKind != JsonValueKind.String)
+                {
+                    throw new ArgumentException("CloudEvents with a non-JSON datacontenttype can only have string data values.");
+                }
+                cloudEvent.Data = dataElement.GetString();
+            }
+        }
 
         /// <inheritdoc />
         public override ReadOnlyMemory<byte> EncodeBatchModeMessage(IEnumerable<CloudEvent> cloudEvents, out ContentType contentType)
@@ -426,12 +452,16 @@ private void WriteCloudEventForBatchOrStructuredMode(Utf8JsonWriter writer, Clou
                     default:
                         writer.WriteStringValue(attribute.Type.Format(value));
                         break;
-
                 }
             }
 
             if (cloudEvent.Data is object)
             {
+                if (cloudEvent.DataContentType is null)
+                {
+                    writer.WritePropertyName(cloudEvent.SpecVersion.DataContentTypeAttribute.Name);
+                    writer.WriteStringValue(JsonMediaType);
+                }
                 EncodeStructuredModeData(cloudEvent, writer);
             }
             writer.WriteEndObject();
@@ -452,26 +482,31 @@ private void WriteCloudEventForBatchOrStructuredMode(Utf8JsonWriter writer, Clou
         /// <param name="writer"/>The writer to serialize the data to. Will not be null.</param>
         protected virtual void EncodeStructuredModeData(CloudEvent cloudEvent, Utf8JsonWriter writer)
         {
-            ContentType dataContentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
-            if (dataContentType.MediaType == JsonMediaType)
-            {
-                writer.WritePropertyName(DataPropertyName);
-                JsonSerializer.Serialize(writer, cloudEvent.Data, SerializerOptions);
-            }
-            else if (cloudEvent.Data is string text && dataContentType.MediaType.StartsWith("text/"))
-            {
-                writer.WritePropertyName(DataPropertyName);
-                writer.WriteStringValue(text);
-            }
-            else if (cloudEvent.Data is byte[] binary)
+            // Binary data is encoded using the data_base64 property, regardless of content type.
+            // TODO: Support other forms of binary data, e.g. ReadOnlyMemory<byte>
+            if (cloudEvent.Data is byte[] binary)
             {
                 writer.WritePropertyName(DataBase64PropertyName);
                 writer.WriteStringValue(Convert.ToBase64String(binary));
             }
             else
             {
-                // We assume CloudEvent.Data is not null due to the way this is called.
-                throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data!.GetType()} with content type '{cloudEvent.DataContentType}'");
+                ContentType dataContentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
+                if (IsJsonMediaType(dataContentType.MediaType))
+                {
+                    writer.WritePropertyName(DataPropertyName);
+                    JsonSerializer.Serialize(writer, cloudEvent.Data, SerializerOptions);
+                }
+                else if (cloudEvent.Data is string text && dataContentType.MediaType.StartsWith("text/"))
+                {
+                    writer.WritePropertyName(DataPropertyName);
+                    writer.WriteStringValue(text);
+                }
+                else
+                {
+                    // We assume CloudEvent.Data is not null due to the way this is called.
+                    throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data!.GetType()} with content type '{cloudEvent.DataContentType}'");
+                }
             }
         }
 
@@ -484,8 +519,14 @@ public override ReadOnlyMemory<byte> EncodeBinaryModeEventData(CloudEvent cloudE
             {
                 return Array.Empty<byte>();
             }
+            // Binary data is left alone, regardless of the content type.
+            // TODO: Support other forms of binary data, e.g. ReadOnlyMemory<byte>
+            if (cloudEvent.Data is byte[] bytes)
+            {
+                return bytes;
+            }
             ContentType contentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
-            if (contentType.MediaType == JsonMediaType)
+            if (IsJsonMediaType(contentType.MediaType))
             {
                 var encoding = MimeUtilities.GetEncoding(contentType);
                 if (encoding is UTF8Encoding)
@@ -501,10 +542,6 @@ public override ReadOnlyMemory<byte> EncodeBinaryModeEventData(CloudEvent cloudE
             {
                 return MimeUtilities.GetEncoding(contentType).GetBytes(text);
             }
-            if (cloudEvent.Data is byte[] bytes)
-            {
-                return bytes;
-            }
             throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data.GetType()} with content type '{cloudEvent.DataContentType}'");
         }
 
@@ -541,6 +578,15 @@ public override void DecodeBinaryModeEventData(ReadOnlyMemory<byte> body, CloudE
                 cloudEvent.Data = body.ToArray();
             }
         }
+
+        /// <summary>
+        /// Determines whether the given media type should be handled as JSON.
+        /// The default implementation treats anything ending with "/json" or "+json"
+        /// as JSON.
+        /// </summary>
+        /// <param name="mediaType">The media type to check for JSON. Will not be null.</param>
+        /// <returns>Whether or not <paramref name="mediaType"/> indicates JSON data.</returns>
+        protected virtual bool IsJsonMediaType(string mediaType) => mediaType.EndsWith("/json") || mediaType.EndsWith("+json");
     }
 
     /// <summary>

test/CloudNative.CloudEvents.UnitTests/SystemTextJson/JsonElementAsserter.cs

@@ -39,6 +39,7 @@ public void AssertProperties(JsonElement obj, bool assertCount)
                         JsonValueKind.String => property.GetString(),
                         JsonValueKind.Number => property.GetInt32(),
                         JsonValueKind.Null => (object?) null,
+                        JsonValueKind.Object => JsonSerializer.Deserialize(property.GetRawText(), expectation.value.GetType()),
                         _ => throw new Exception($"Unhandled value kind: {property.ValueKind}")
                     };