From 144614d637f86213c65a3b767af99e017707e255 Mon Sep 17 00:00:00 2001 From: "Day, Jeremy(jday)" Date: Thu, 27 Jan 2022 14:09:10 -0800 Subject: [PATCH] Enhances detection of textual content-types ensuring appropriate protobuf field usage. Moved content-type introspection in a seprate support class. Added unit tests to ensure introspection is correct. Addressed review comments Signed-off-by: Day, Jeremy(jday) --- .../cloudevents/protobuf/ProtoSerializer.java | 18 +++---- .../io/cloudevents/protobuf/ProtoSupport.java | 47 ++++++++++++++++++ .../protobuf/ProtoSupportTest.java | 49 +++++++++++++++++++ 3 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSupport.java create mode 100644 formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoSupportTest.java diff --git a/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSerializer.java b/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSerializer.java index 1fbb82ad6..bc240d84d 100644 --- a/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSerializer.java +++ b/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSerializer.java @@ -77,15 +77,6 @@ public static CloudEvent toProto(io.cloudevents.CloudEvent ce) throws InvalidPro } } - // The proto spec says all text data should go into the text field. It is really difficult to figure out every case - // of text data based on the media type though, so I am just going to check for some common cases. - private static boolean isTextType(String type) { - if (type == null) { - return false; - } - return type.startsWith("text/") || "application/json".equals(type) || "application/xml".equals(type); - } - /** * Defines a {@link CloudEventContextWriter} that will allow setting the attributes within a Protobuf object. */ @@ -272,9 +263,16 @@ public CloudEvent end(CloudEventData data) throws CloudEventRWException { throw CloudEventRWException.newDataConversion(e, "byte[]", "com.google.protobuf.Any"); } protoBuilder.setProtoData(dataAsAny); - } else if (isTextType(dataContentType)) { + } else if (ProtoSupport.isTextContent(dataContentType)) { + /** + * The protobuf format specification states that textual data must + * be carried in the 'text_data' field. + */ protoBuilder.setTextDataBytes(ByteString.copyFrom(data.toBytes())); } else { + /** + * All other content is assumed to be binary. + */ ByteString byteString = ByteString.copyFrom(data.toBytes()); protoBuilder.setBinaryData(byteString); } diff --git a/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSupport.java b/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSupport.java new file mode 100644 index 000000000..a2c079359 --- /dev/null +++ b/formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSupport.java @@ -0,0 +1,47 @@ +/* + * Copyright 2018-Present The CloudEvents Authors + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package io.cloudevents.protobuf; + +/** + * General support functions. + */ + +final class ProtoSupport { + + // Prevent Instantiation + private ProtoSupport() { + } + + /** + * Determine if the given content type indicates that + * content is textual. + */ + static boolean isTextContent(String contentType) { + + if (contentType == null) { + return false; + } + + return contentType.startsWith("text/") + || "application/json".equals(contentType) + || "application/xml".equals(contentType) + || contentType.endsWith("+json") + || contentType.endsWith("+xml") + ; + } +} diff --git a/formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoSupportTest.java b/formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoSupportTest.java new file mode 100644 index 000000000..0e28bfe51 --- /dev/null +++ b/formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoSupportTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2018-Present The CloudEvents Authors + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package io.cloudevents.protobuf; + +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.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ProtoSupportTest { + + @ParameterizedTest + @MethodSource("textContentArguments") + public void serialize(String contentType, boolean isText) throws IOException { + + assertThat(isText).isEqualTo(ProtoSupport.isTextContent(contentType)); + } + + // Test Data set for contentType text determination. + public static Stream textContentArguments() { + return Stream.of( + Arguments.of("application/json", true), + Arguments.of("application/xml", true), + Arguments.of("text/plain", true), + Arguments.of("application/protobuf", false), + Arguments.of("application/fubar+xml", true), + Arguments.of("application/fubar+json", true) + ); + } +}