From 5e3bfc890f275f81424f3ed68e5c3c55a16fc24a Mon Sep 17 00:00:00 2001 From: Jem Day Date: Wed, 24 Mar 2021 08:58:33 -0700 Subject: [PATCH] Specification Compliant handling of numeric context attributes (#358) * - Added tests case to verify expected handling of numeric context attributes - Updated serializer. Signed-off-by: Day, Jeremy(jday) * - Added @deprecated marker for CloudEventContextWriter.set(name, Number) - Added use of new method for JSON serializer. Cleanup of deprecated implementations can occur independantly. Signed-off-by: Day, Jeremy(jday) * Addressed Review Comments - Now throws exception when non specification compliant numeric attribute values are received during deserialization. - Added test cases to verify deserialization exceptions. Signed-off-by: Day, Jeremy(jday) * Address Review Comments Signed-off-by: Day, Jeremy(jday) * Address Review Comment Signed-off-by: Day, Jeremy(jday) --- .../rw/CloudEventContextWriter.java | 18 ++++++++++ .../cloudevents/rw/CloudEventRWException.java | 11 +++++++ .../core/v03/CloudEventBuilder.java | 20 +++++++++++ .../core/v1/CloudEventBuilder.java | 19 +++++++++++ .../java/io/cloudevents/core/test/Data.java | 11 +++++++ .../jackson/CloudEventDeserializer.java | 12 ++++++- .../jackson/CloudEventSerializer.java | 19 +++++++++-- .../cloudevents/jackson/JsonFormatTest.java | 33 +++++++++++++++++-- .../resources/v03/fail_numeric_decimal.json | 7 ++++ .../test/resources/v03/fail_numeric_long.json | 7 ++++ .../resources/v1/fail_numeric_decimal.json | 7 ++++ .../test/resources/v1/fail_numeric_long.json | 7 ++++ .../src/test/resources/v1/numeric_ext.json | 10 ++++++ 13 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 formats/json-jackson/src/test/resources/v03/fail_numeric_decimal.json create mode 100644 formats/json-jackson/src/test/resources/v03/fail_numeric_long.json create mode 100644 formats/json-jackson/src/test/resources/v1/fail_numeric_decimal.json create mode 100644 formats/json-jackson/src/test/resources/v1/fail_numeric_long.json create mode 100644 formats/json-jackson/src/test/resources/v1/numeric_ext.json diff --git a/api/src/main/java/io/cloudevents/rw/CloudEventContextWriter.java b/api/src/main/java/io/cloudevents/rw/CloudEventContextWriter.java index 6643efb0..e0b6101d 100644 --- a/api/src/main/java/io/cloudevents/rw/CloudEventContextWriter.java +++ b/api/src/main/java/io/cloudevents/rw/CloudEventContextWriter.java @@ -83,11 +83,29 @@ public interface CloudEventContextWriter { * @return self * @throws CloudEventRWException if anything goes wrong while writing this extension. * @throws IllegalArgumentException if you're trying to set the specversion attribute. + * + * @deprecated CloudEvent specification only permits {@link Integer} type as a + * numeric value. */ default CloudEventContextWriter withContextAttribute(String name, Number value) throws CloudEventRWException { return withContextAttribute(name, value.toString()); } + /** + * Set attribute with type {@link Integer}. + * This setter should not be invoked for specversion, because the writer should + * already know the specversion or because it doesn't need it to correctly write the value. + * + * @param name name of the attribute + * @param value value of the attribute + * @return self + * @throws CloudEventRWException if anything goes wrong while writing this extension. + * @throws IllegalArgumentException if you're trying to set the specversion attribute. + */ + default CloudEventContextWriter withContextAttribute(String name, Integer value) throws CloudEventRWException { + return withContextAttribute(name, value.toString()); + } + /** * Set attribute with type {@link Boolean} attribute. * This setter should not be invoked for specversion, because the writer should diff --git a/api/src/main/java/io/cloudevents/rw/CloudEventRWException.java b/api/src/main/java/io/cloudevents/rw/CloudEventRWException.java index bd8d50f7..3648ffc5 100644 --- a/api/src/main/java/io/cloudevents/rw/CloudEventRWException.java +++ b/api/src/main/java/io/cloudevents/rw/CloudEventRWException.java @@ -137,6 +137,17 @@ public class CloudEventRWException extends RuntimeException { ); } + public static CloudEventRWException newInvalidAttributeType(String attributeName,Object value){ + return new CloudEventRWException( + CloudEventRWExceptionKind.INVALID_ATTRIBUTE_TYPE, + "Invalid attribute/extension type for \"" + + attributeName + + "\": Type" + value.getClass().getCanonicalName() + + ". Value: " + value + + ); + } + /** * @param attributeName the invalid attribute name * @param value the value of the attribute diff --git a/core/src/main/java/io/cloudevents/core/v03/CloudEventBuilder.java b/core/src/main/java/io/cloudevents/core/v03/CloudEventBuilder.java index 3ee4f465..834b3983 100644 --- a/core/src/main/java/io/cloudevents/core/v03/CloudEventBuilder.java +++ b/core/src/main/java/io/cloudevents/core/v03/CloudEventBuilder.java @@ -246,6 +246,26 @@ public final class CloudEventBuilder extends BaseCloudEventBuilder { writer.withContextAttribute(extensionName, extensionValue.booleanValue()); break; case NUMBER: - writer.withContextAttribute(extensionName, extensionValue.numberValue()); + + final Number numericValue = extensionValue.numberValue(); + + // Only 'Int' values are supported by the specification + + if (numericValue instanceof Integer){ + writer.withContextAttribute(extensionName, (Integer) numericValue); + } else{ + throw CloudEventRWException.newInvalidAttributeType(extensionName,numericValue); + } + break; case STRING: writer.withContextAttribute(extensionName, extensionValue.textValue()); diff --git a/formats/json-jackson/src/main/java/io/cloudevents/jackson/CloudEventSerializer.java b/formats/json-jackson/src/main/java/io/cloudevents/jackson/CloudEventSerializer.java index 2da628e5..50f0931d 100644 --- a/formats/json-jackson/src/main/java/io/cloudevents/jackson/CloudEventSerializer.java +++ b/formats/json-jackson/src/main/java/io/cloudevents/jackson/CloudEventSerializer.java @@ -65,10 +65,23 @@ class CloudEventSerializer extends StdSerializer { } @Override - public CloudEventContextWriter withContextAttribute(String name, Number value) throws CloudEventRWException { + public CloudEventContextWriter withContextAttribute(String name, Number value) throws CloudEventRWException + { + // Only Integer types are supported by the specification + if (value instanceof Integer) { + this.withContextAttribute(name, (Integer) value); + } else { + // Default to string representation for other numeric values + this.withContextAttribute(name, value.toString()); + } + return this; + } + + @Override + public CloudEventContextWriter withContextAttribute(String name, Integer value) throws CloudEventRWException + { try { - gen.writeFieldName(name); - provider.findValueSerializer(value.getClass()).serialize(value, gen, provider); + gen.writeNumberField(name, value.intValue()); return this; } catch (IOException e) { throw new RuntimeException(e); diff --git a/formats/json-jackson/src/test/java/io/cloudevents/jackson/JsonFormatTest.java b/formats/json-jackson/src/test/java/io/cloudevents/jackson/JsonFormatTest.java index e8f049ac..de99403d 100644 --- a/formats/json-jackson/src/test/java/io/cloudevents/jackson/JsonFormatTest.java +++ b/formats/json-jackson/src/test/java/io/cloudevents/jackson/JsonFormatTest.java @@ -24,14 +24,17 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory; import io.cloudevents.CloudEvent; import io.cloudevents.SpecVersion; import io.cloudevents.core.builder.CloudEventBuilder; +import io.cloudevents.core.format.EventDeserializationException; import io.cloudevents.core.provider.EventFormatProvider; import io.cloudevents.rw.CloudEventRWException; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import java.io.IOException; +import java.math.BigInteger; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -40,8 +43,7 @@ import java.util.Objects; import java.util.stream.Stream; import static io.cloudevents.core.test.Data.*; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.*; class JsonFormatTest { @@ -120,6 +122,21 @@ class JsonFormatTest { .hasMessageContaining(CloudEventRWException.newInvalidSpecVersion("9000.1").getMessage()); } + @ParameterizedTest + @MethodSource("badJsonContent") + /** + * JSON content that should fail deserialization + * as it represents content that is not CE + * specification compliant. + */ + void verifyDeserializeError(String inputFile){ + + byte[] input = loadFile(inputFile); + + assertThatExceptionOfType(EventDeserializationException.class).isThrownBy(() -> getFormat().deserialize(input)); + + } + public static Stream serializeTestArgumentsDefault() { return Stream.of( Arguments.of(V03_MIN, "v03/min.json"), @@ -133,7 +150,8 @@ class JsonFormatTest { Arguments.of(V1_WITH_JSON_DATA_WITH_EXT, "v1/json_data_with_ext.json"), Arguments.of(V1_WITH_XML_DATA, "v1/base64_xml_data.json"), Arguments.of(V1_WITH_TEXT_DATA, "v1/base64_text_data.json"), - Arguments.of(V1_WITH_BINARY_EXT, "v1/binary_attr.json") + Arguments.of(V1_WITH_BINARY_EXT, "v1/binary_attr.json"), + Arguments.of(V1_WITH_NUMERIC_EXT,"v1/numeric_ext.json") ); } @@ -200,6 +218,15 @@ class JsonFormatTest { ); } + public static Stream badJsonContent() { + return Stream.of( + "v03/fail_numeric_decimal.json", + "v03/fail_numeric_long.json", + "v1/fail_numeric_decimal.json", + "v1/fail_numeric_long.json" + ); + } + private JsonFormat getFormat() { return (JsonFormat) EventFormatProvider.getInstance().resolveFormat(JsonFormat.CONTENT_TYPE); } diff --git a/formats/json-jackson/src/test/resources/v03/fail_numeric_decimal.json b/formats/json-jackson/src/test/resources/v03/fail_numeric_decimal.json new file mode 100644 index 00000000..8fa884b9 --- /dev/null +++ b/formats/json-jackson/src/test/resources/v03/fail_numeric_decimal.json @@ -0,0 +1,7 @@ +{ + "specversion": "1.0", + "id": "1", + "type": "mock.test", + "source": "http://localhost/source", + "decimal": 42.42 +} diff --git a/formats/json-jackson/src/test/resources/v03/fail_numeric_long.json b/formats/json-jackson/src/test/resources/v03/fail_numeric_long.json new file mode 100644 index 00000000..fff47bbe --- /dev/null +++ b/formats/json-jackson/src/test/resources/v03/fail_numeric_long.json @@ -0,0 +1,7 @@ +{ + "specversion": "1.0", + "id": "1", + "type": "mock.test", + "source": "http://localhost/source", + "long": 4247483647 +} diff --git a/formats/json-jackson/src/test/resources/v1/fail_numeric_decimal.json b/formats/json-jackson/src/test/resources/v1/fail_numeric_decimal.json new file mode 100644 index 00000000..8fa884b9 --- /dev/null +++ b/formats/json-jackson/src/test/resources/v1/fail_numeric_decimal.json @@ -0,0 +1,7 @@ +{ + "specversion": "1.0", + "id": "1", + "type": "mock.test", + "source": "http://localhost/source", + "decimal": 42.42 +} diff --git a/formats/json-jackson/src/test/resources/v1/fail_numeric_long.json b/formats/json-jackson/src/test/resources/v1/fail_numeric_long.json new file mode 100644 index 00000000..fff47bbe --- /dev/null +++ b/formats/json-jackson/src/test/resources/v1/fail_numeric_long.json @@ -0,0 +1,7 @@ +{ + "specversion": "1.0", + "id": "1", + "type": "mock.test", + "source": "http://localhost/source", + "long": 4247483647 +} diff --git a/formats/json-jackson/src/test/resources/v1/numeric_ext.json b/formats/json-jackson/src/test/resources/v1/numeric_ext.json new file mode 100644 index 00000000..9a3074eb --- /dev/null +++ b/formats/json-jackson/src/test/resources/v1/numeric_ext.json @@ -0,0 +1,10 @@ +{ + "specversion": "1.0", + "id": "1", + "type": "mock.test", + "source": "http://localhost/source", + "integer": 42, + "decimal": "42.42", + "float": "4.2", + "long": "4200" +}