Make ProtoCloudEventData consistent with spec.

docs/protobuf.md

@@ -11,20 +11,22 @@ This module provides the Protocol Buffer (protobuf) `EventFormat` implementation
 Protobuf runtime and classes generated from the CloudEvents
 [proto spec](https://github.com/cloudevents/spec/blob/v1.0.1/spec.proto).
 
+# Setup
 For Maven based projects, use the following dependency:
 
 ```xml
 <dependency>
     <groupId>io.cloudevents</groupId>
     <artifactId>cloudevents-protobuf</artifactId>
-    <version>2.3.0</version>
+    <version>x.y.z</version>
 </dependency>
 ```
 
+No further configuration is required is use the module.
+
 ## Using the Protobuf Event Format
 
-You don't need to perform any operation to configure the module, more than
-adding the dependency to your project:
+### Event serialization
 
 ```java
 import io.cloudevents.CloudEvent;
@@ -44,5 +46,51 @@ byte[]serialized = EventFormatProvider
     .serialize(event);
 ```
 
-The `EventFormatProvider` will resolve automatically the `ProtobufFormat` using the
+The `EventFormatProvider` will automatically resolve the `ProtobufFormat` using the
 `ServiceLoader` APIs.
+
+## Passing Protobuf messages as CloudEvent data.
+
+The `ProtoCloudEventData` capability provides a convenience mechanism to handle Protobuf message object data.
+
+### Building
+
+```java
+// Build my business event message.
+com.google.protobuf.Message myMessage = ..... ;
+
+// Wrap the protobuf message as CloudEventData.
+CloudEventData ceData = ProtoCloudEventData.wrap(myMessage);
+
+// Build the CloudEvent
+CloudEvent event = CloudEventBuilder.v1()
+    .withId("hello")
+    .withType("example.protodata")
+    .withSource(URI.create("http://localhost"))
+    .withData(ceData)
+    .build();
+```
+
+### Reading
+
+If the `ProtobufFormat` is used to deserialize a CloudEvent that contains a protobuf message object as data you can use
+the `ProtoCloudEventData` to access it as an 'Any' directly.
+
+```java
+
+// Deserialize the event.
+CloudEvent myEvent = eventFormat.deserialize(raw);
+
+// Get the Data
+CloudEventData eventData = myEvent.getData();
+
+if (ceData instanceOf ProtoCloudEventData) {
+
+    // Obtain the protobuf 'any'
+    Any anAny = ((ProtoCloudEventData) eventData).getAny();
+
+    ...
+}
+
+```
+

formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoCloudEventData.java

@@ -16,6 +16,7 @@
  */
 package io.cloudevents.protobuf;
 
+import com.google.protobuf.Any;
 import com.google.protobuf.Message;
 import io.cloudevents.CloudEventData;
 
@@ -26,13 +27,7 @@
 public interface ProtoCloudEventData extends CloudEventData {
 
     /**
-     * Gets the protobuf {@link Message} representation of this data.
-     * @return The data as a {@link Message}
-     */
-    Message getMessage();
-
-    /**
-     * Convenience helper to wrap a Protobuf Message as
+     * Convenience helper to wrap a Protobuf {@link Message} as
      * CloudEventData.
      *
      * @param protoMessage The message to wrap
@@ -41,4 +36,11 @@ public interface ProtoCloudEventData extends CloudEventData {
     static CloudEventData wrap(Message protoMessage) {
         return new ProtoDataWrapper(protoMessage);
     }
+
+    /**
+     * Gets the protobuf {@link Any} representation of this data.
+     *
+     * @return The data as an {@link Any}
+     */
+    Any getAny();
 }

formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoDataWrapper.java

@@ -19,24 +19,31 @@
 import com.google.protobuf.Any;
 import com.google.protobuf.Message;
 
-import java.util.Arrays;
+import java.util.Objects;
 
 class ProtoDataWrapper implements ProtoCloudEventData {
 
-    private final Message protoMessage;
+    private final Any protoAny;
 
     ProtoDataWrapper(Message protoMessage) {
-        this.protoMessage = protoMessage;
+
+        Objects.requireNonNull(protoMessage);
+
+        if (protoMessage instanceof Any) {
+            protoAny = (Any) protoMessage;
+        } else {
+            protoAny = Any.pack(protoMessage);
+        }
     }
 
     @Override
-    public Message getMessage() {
-        return protoMessage;
+    public Any getAny() {
+        return protoAny;
     }
 
     @Override
     public byte[] toBytes() {
-        return protoMessage.toByteArray();
+        return protoAny.toByteArray();
     }
 
     @Override
@@ -53,17 +60,21 @@ public boolean equals(Object obj) {
         // Now compare the actual data
         ProtoDataWrapper rhs = (ProtoDataWrapper) obj;
 
-        if (this.getMessage() == rhs.getMessage()){
-            return true;
-        }
+        final Any lhsAny = getAny();
+        final Any rhsAny = rhs.getAny();
 
         // This is split out for readability.
-        //  Compare the content in terms onf an 'Any'.
-        //  - Verify the types match
-        //  - Verify the values match.
+        //  1. Sanity compare the 'Any' references.
+        //  2. Compare the content in terms onf an 'Any'.
+        //     - Verify the types match
+        //     - Verify the values match.
 
-        final Any lhsAny = getAsAny(this.getMessage());
-        final Any rhsAny = getAsAny(rhs.getMessage());
+        // NULL checks not required as object cannot be built
+        // with a null.
+
+        if (lhsAny == rhsAny) {
+            return true;
+        }
 
         final boolean typesMatch = (ProtoSupport.extractMessageType(lhsAny).equals(ProtoSupport.extractMessageType(rhsAny)));
 
@@ -72,15 +83,7 @@ public boolean equals(Object obj) {
         } else {
             return false;
         }
-    }
-
-    private Any getAsAny(Message m) {
-
-        if (m instanceof Any) {
-            return (Any) m;
-        }
-
-        return Any.pack(m);
 
     }
+
 }

formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoDeserializer.java

@@ -16,18 +16,14 @@
  */
 package io.cloudevents.protobuf;
 
-import com.google.protobuf.Message;
 import io.cloudevents.CloudEventData;
 import io.cloudevents.SpecVersion;
 import io.cloudevents.core.data.BytesCloudEventData;
 import io.cloudevents.core.v1.CloudEventV1;
-import io.cloudevents.rw.CloudEventDataMapper;
-import io.cloudevents.rw.CloudEventRWException;
-import io.cloudevents.rw.CloudEventReader;
-import io.cloudevents.rw.CloudEventWriter;
-import io.cloudevents.rw.CloudEventWriterFactory;
+import io.cloudevents.rw.*;
 import io.cloudevents.v1.proto.CloudEvent;
 import io.cloudevents.v1.proto.CloudEvent.CloudEventAttributeValue;
+
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;

formats/protobuf/src/main/java/io/cloudevents/protobuf/ProtoSerializer.java

@@ -16,8 +16,11 @@
  */
 package io.cloudevents.protobuf;
 
-import com.google.protobuf.*;
+import com.google.protobuf.Any;
+import com.google.protobuf.ByteString;
 import com.google.protobuf.Descriptors.FieldDescriptor;
+import com.google.protobuf.InvalidProtocolBufferException;
+import com.google.protobuf.Timestamp;
 import io.cloudevents.CloudEventData;
 import io.cloudevents.SpecVersion;
 import io.cloudevents.core.CloudEventUtils;
@@ -27,7 +30,6 @@
 import io.cloudevents.rw.CloudEventRWException;
 import io.cloudevents.rw.CloudEventWriter;
 import io.cloudevents.v1.proto.CloudEvent;
-import io.cloudevents.v1.proto.CloudEvent.*;
 
 import java.net.URI;
 import java.time.Instant;
@@ -244,16 +246,20 @@ public CloudEvent end(CloudEventData data) throws CloudEventRWException {
 
                 // If it's a proto message we can handle that directly.
                 if (data instanceof ProtoCloudEventData) {
+
                     final ProtoCloudEventData protoData = (ProtoCloudEventData) data;
-                    final Message m = protoData.getMessage();
-                    if (m != null) {
-                        // If it's already an 'Any' don't re-pack it.
-                        if (m instanceof Any) {
-                            protoBuilder.setProtoData((Any) m);
-                        }else {
-                            protoBuilder.setProtoData(Any.pack(m));
-                        }
+                    final Any anAny = protoData.getAny();
+
+                    // Even though our local implementation cannot be instantiated
+                    // with NULL data nothing stops somebody from having their own
+                    // variant that isn't as 'safe'.
+
+                    if (anAny != null) {
+                        protoBuilder.setProtoData(anAny);
+                    } else {
+                        throw CloudEventRWException.newOther("ProtoCloudEventData: getAny() was NULL");
                     }
+
                 } else {
                     if (Objects.equals(dataContentType, PROTO_DATA_CONTENT_TYPE)) {
                         // This will throw if the data provided is not an Any. The protobuf CloudEvent spec requires proto data to be stored as

formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoDataWrapperTest.java

@@ -45,12 +45,12 @@ public void testBasic() {
         ProtoDataWrapper pdw = new ProtoDataWrapper(quote1);
 
         assertThat(pdw).isNotNull();
-        assertThat(pdw.getMessage()).isNotNull();
+        assertThat(pdw.getAny()).isNotNull();
         assertThat(pdw.toBytes()).withFailMessage("toBytes was NULL").isNotNull();
         assertThat(pdw.toBytes()).withFailMessage("toBytes[] returned empty array").hasSizeGreaterThan(0);
 
-        // This is current behavior and will probably change in the next version.
-        assertThat(pdw.getMessage()).isInstanceOf(io.cloudevents.test.v1.proto.Test.Quote.class);
+        // Ensure it's a Quote.
+        assertThat(pdw.getAny().is(io.cloudevents.test.v1.proto.Test.Quote.class)).isTrue();
     }
 
     @Test
@@ -91,7 +91,7 @@ public void testBytes() {
         final byte[] actData = pdw.toBytes();
 
         // Verify
-        Arrays.equals(expData, actData);
+        assertThat(Arrays.equals(actData, expData)).isTrue();
 
     }
 

formats/protobuf/src/test/java/io/cloudevents/protobuf/ProtoMessageDataTest.java

@@ -18,16 +18,16 @@
 
 import com.google.protobuf.Any;
 import com.google.protobuf.InvalidProtocolBufferException;
-import com.google.protobuf.Message;
 import io.cloudevents.CloudEvent;
 import io.cloudevents.CloudEventData;
 import io.cloudevents.core.builder.CloudEventBuilder;
 import io.cloudevents.core.format.EventFormat;
+import io.cloudevents.test.v1.proto.Test.Decimal;
+import io.cloudevents.test.v1.proto.Test.Quote;
+import org.junit.jupiter.api.Test;
+
 import java.math.BigDecimal;
 import java.net.URI;
-import org.junit.jupiter.api.Test;
-import io.cloudevents.test.v1.proto.Test.Quote;
-import io.cloudevents.test.v1.proto.Test.Decimal;
 
 import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -49,7 +49,7 @@ public void verifyDataWrapper() {
         assertThat(ced).isInstanceOf(ProtoCloudEventData.class);
 
         ProtoCloudEventData pced = (ProtoCloudEventData) ced;
-        assertThat(pced.getMessage()).isNotNull();
+        assertThat(pced.getAny()).isNotNull();
     }
 
     @Test
@@ -82,12 +82,11 @@ public void verifyMessage() throws InvalidProtocolBufferException {
         assertThat(eventData).isNotNull();
         assertThat(eventData).isInstanceOf(ProtoCloudEventData.class);
 
-        Message newMessage = ((ProtoCloudEventData) eventData).getMessage();
-        assertThat(newMessage).isNotNull();
-        assertThat(newMessage).isInstanceOf(Any.class);
+        Any newAny = ((ProtoCloudEventData) eventData).getAny();
+        assertThat(newAny).isNotNull();
 
         // Hydrate the data - maybe there's a cleaner way to do this.
-        Quote newQuote = ((Any) newMessage).unpack(Quote.class);
+        Quote newQuote = newAny.unpack(Quote.class);
         assertThat(newQuote).ignoringRepeatedFieldOrder().isEqualTo(pyplQuote);
     }