IPROTO-305 MessageMarshaller map support

* undeprecate MessageMarshaller * add compliance tests for map against protoc-generated marshallers
infinispan · Mar 8, 2024 · 88f49fa · 88f49fa
1 parent f9f593f
commit 88f49fa
Show file tree

Hide file tree

Showing 17 changed files with 428 additions and 97 deletions.
diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml
@@ -14,6 +14,9 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: Install Protoc
+        uses: arduino/setup-protoc@v3
+
       - name: Setup Java
         uses: actions/setup-java@v4
         with:
@@ -36,6 +39,9 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: Install Protoc
+        uses: arduino/setup-protoc@v3
+
       - name: Setup Java
         uses: actions/setup-java@v4
         with:

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -30,6 +30,9 @@ jobs:
           mvn -B versions:set -DnewVersion=${{ github.event.inputs.version }} -DprocessAllModules=true
           mvn -B versions:set-property -Dproperty=version.infinispan -DnewVersion=${{ github.event.inputs.version }}
 
+      - name: Install Protoc
+        uses: arduino/setup-protoc@v3
+
       - name: Set up Java for publishing to OSSRH
         uses: actions/setup-java@v4
         with:

diff --git a/.github/workflows/test_report.yaml b/.github/workflows/test_report.yaml
@@ -8,13 +8,6 @@ jobs:
   report:
     runs-on: ubuntu-latest
     steps:
-      - uses: dorny/test-reporter@v1
-        with:
-          artifact: jdk11-test-results
-          name: JDK 11 Tests
-          path: '**/*.xml'
-          reporter: java-junit
-
       - uses: dorny/test-reporter@v1
         with:
           artifact: jdk17-test-results

diff --git a/core/src/main/java/org/infinispan/protostream/MessageMarshaller.java b/core/src/main/java/org/infinispan/protostream/MessageMarshaller.java
@@ -5,17 +5,15 @@
 import java.time.Instant;
 import java.util.Collection;
 import java.util.Date;
+import java.util.Map;
 
 /**
  * Contract to be implemented by manually written marshallers for Protobuf message (entity) types. The marshaller
  * implementation must be stateless and thread-safe.
  *
  * @author [email protected]
  * @since 1.0
- * @deprecated since 4.3.1.Final. Will be removed in version 5. Please use annotation based marshallers instead. See
- * {@link org.infinispan.protostream.annotations.AutoProtoSchemaBuilder}
  */
-@Deprecated
 public interface MessageMarshaller<T> extends BaseMarshaller<T> {
 
    /**
@@ -85,6 +83,8 @@ interface ProtoStreamReader {
       <E, C extends Collection<? super E>> C readCollection(String fieldName, C collection, Class<E> elementClass) throws IOException;
 
       <E> E[] readArray(String fieldName, Class<? extends E> elementClass) throws IOException;
+
+      <K, V, M extends Map<? super K, ? super V>> M readMap(String fieldName, M map, Class<K> keyClass, Class<V> valueClass) throws IOException;
    }
 
    /**
@@ -141,15 +141,6 @@ interface ProtoStreamWriter {
 
       <E> void writeObject(String fieldName, E value, Class<? extends E> clazz) throws IOException;
 
-      /**
-       * Writes an enum value. The third argument (the {@code class} was never used internally) so this variant is now
-       * deprecated.
-       *
-       * @deprecated replaced by {@link ProtoStreamWriter#writeEnum(String fieldName, Enum value)}
-       */
-      @Deprecated
-      <E extends Enum<E>> void writeEnum(String fieldName, E value, Class<E> clazz) throws IOException;
-
       /**
        * Writes an enum value.
        *
@@ -161,5 +152,7 @@ interface ProtoStreamWriter {
       <E> void writeCollection(String fieldName, Collection<? super E> collection, Class<E> elementClass) throws IOException;
 
       <E> void writeArray(String fieldName, E[] array, Class<? extends E> elementClass) throws IOException;
+
+      <K, V> void writeMap(String fieldName, Map<? super K, ? super V> map, Class<K> keyClass, Class<V> valueClass) throws IOException;
    }
 }
diff --git a/core/src/main/java/org/infinispan/protostream/descriptors/MapDescriptor.java b/core/src/main/java/org/infinispan/protostream/descriptors/MapDescriptor.java
@@ -27,6 +27,18 @@ public JavaType getKeyJavaType() {
       return getKeyType().getJavaType();
    }
 
+   public int getWireTag() {
+      return WireType.makeTag(number, WireType.LENGTH_DELIMITED);
+   }
+
+   public int getKeyWireTag() {
+      return WireType.makeTag(1, keyType.getWireType());
+   }
+
+   public int getValueWireTag() {
+      return WireType.makeTag(2, type.getWireType());
+   }
+
    @Override
    public Label getLabel() {
       return Label.OPTIONAL;

diff --git a/core/src/main/java/org/infinispan/protostream/impl/ProtoStreamReaderImpl.java b/core/src/main/java/org/infinispan/protostream/impl/ProtoStreamReaderImpl.java
@@ -11,6 +11,7 @@
 import java.util.Date;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Map;
 
 import org.infinispan.protostream.ImmutableSerializationContext;
 import org.infinispan.protostream.MessageContext;
@@ -21,6 +22,7 @@
 import org.infinispan.protostream.descriptors.Descriptor;
 import org.infinispan.protostream.descriptors.FieldDescriptor;
 import org.infinispan.protostream.descriptors.JavaType;
+import org.infinispan.protostream.descriptors.MapDescriptor;
 import org.infinispan.protostream.descriptors.Type;
 import org.infinispan.protostream.descriptors.WireType;
 import org.jboss.logging.Logger;
@@ -149,25 +151,13 @@ private Object convertWireTypeToJavaType(Type type, Object o) {
          case STRING:
             o = new String((byte[]) o, StandardCharsets.UTF_8);
             break;
-         case BYTES:
-            o = (byte[]) o;
+         case BYTES, INT64, UINT64, FIXED64, SFIXED64, SINT64, FIXED32, SFIXED32:
             break;
          case INT32:
          case UINT32:
          case SINT32:
             o = ((Long) o).intValue();
             break;
-         case FIXED32:
-         case SFIXED32:
-            o = (Integer) o;
-            break;
-         case INT64:
-         case UINT64:
-         case FIXED64:
-         case SFIXED64:
-         case SINT64:
-            o = (Long) o;
-            break;
          case BOOL:
             o = ((Long) o) != 0;
             break;
@@ -336,7 +326,7 @@ private <A> A readNestedObject(FieldDescriptor fd, Class<A> clazz, ProtobufTagMa
       TagReader in = ctx.getReader();
       A a;
       if (fd.getType() == Type.GROUP) {
-         a = marshallerDelegate.unmarshall(ctx ,fd);
+         a = marshallerDelegate.unmarshall(ctx, fd);
          in.checkLastTagWas(WireType.makeTag(fd.getNumber(), WireType.WIRETYPE_END_GROUP));
       } else if (fd.getType() == Type.MESSAGE) {
          if (length < 0) {
@@ -424,56 +414,24 @@ private void readPrimitiveCollection(FieldDescriptor fd, Collection<? super Obje
             break;
          }
          if (tag == expectedTag) {
-            Object value;
-            switch (type) {
-               case DOUBLE:
-                  value = messageContext.in.readDouble();
-                  break;
-               case FLOAT:
-                  value = messageContext.in.readFloat();
-                  break;
-               case BOOL:
-                  value = messageContext.in.readBool();
-                  break;
-               case STRING:
-                  value = messageContext.in.readString();
-                  break;
-               case BYTES:
-                  value = messageContext.in.readByteArray();
-                  break;
-               case INT64:
-                  value = messageContext.in.readInt64();
-                  break;
-               case UINT64:
-                  value = messageContext.in.readUInt64();
-                  break;
-               case FIXED64:
-                  value = messageContext.in.readFixed64();
-                  break;
-               case SFIXED64:
-                  value = messageContext.in.readSFixed64();
-                  break;
-               case SINT64:
-                  value = messageContext.in.readSInt64();
-                  break;
-               case INT32:
-                  value = messageContext.in.readInt32();
-                  break;
-               case FIXED32:
-                  value = messageContext.in.readFixed32();
-                  break;
-               case UINT32:
-                  value = messageContext.in.readUInt32();
-                  break;
-               case SFIXED32:
-                  value = messageContext.in.readSFixed32();
-                  break;
-               case SINT32:
-                  value = messageContext.in.readSInt32();
-                  break;
-               default:
-                  throw new IllegalStateException("Unexpected field type : " + type);
-            }
+            Object value = switch (type) {
+               case DOUBLE -> messageContext.in.readDouble();
+               case FLOAT -> messageContext.in.readFloat();
+               case BOOL -> messageContext.in.readBool();
+               case STRING -> messageContext.in.readString();
+               case BYTES -> messageContext.in.readByteArray();
+               case INT64 -> messageContext.in.readInt64();
+               case UINT64 -> messageContext.in.readUInt64();
+               case FIXED64 -> messageContext.in.readFixed64();
+               case SFIXED64 -> messageContext.in.readSFixed64();
+               case SINT64 -> messageContext.in.readSInt64();
+               case INT32 -> messageContext.in.readInt32();
+               case FIXED32 -> messageContext.in.readFixed32();
+               case UINT32 -> messageContext.in.readUInt32();
+               case SFIXED32 -> messageContext.in.readSFixed32();
+               case SINT32 -> messageContext.in.readSInt32();
+               default -> throw new IllegalStateException("Unexpected field type : " + type);
+            };
             collection.add(value);
          } else {
             messageContext.unknownFieldSet.readSingleField(tag, messageContext.in);
@@ -509,4 +467,79 @@ private void checkFieldRead(FieldDescriptor fd, boolean expectRepeated) {
          log.fieldReadOutOfSequence(fd.getFullName());
       }
    }
+
+   @Override
+   public <K, V, M extends Map<? super K, ? super V>> M readMap(String fieldName, M map, Class<K> keyClass, Class<V> valueClass) throws IOException {
+      final MapDescriptor md = (MapDescriptor) messageContext.getFieldByName(fieldName);
+      TagReaderImpl in = (TagReaderImpl) ctx.getReader();
+      final int expectedTag = md.getWireTag();
+      while (true) {
+         Object o = messageContext.unknownFieldSet.consumeTag(expectedTag);
+         if (o == null) {
+            break;
+         }
+         readMapEntry(map, valueClass, TagReaderImpl.newNestedInstance(in, (byte[]) o), md);
+      }
+      while (true) {
+         int tag = in.readTag();
+         if (tag == 0) {
+            break;
+         }
+         if (tag != expectedTag) {
+            messageContext.unknownFieldSet.readSingleField(tag, messageContext.in);
+            break;
+         }
+         int len = in.readUInt32();
+         int limit = in.pushLimit(len);
+         readMapEntry(map, valueClass, in, md);
+         in.popLimit(limit);
+      }
+      return map;
+   }
+
+   private <K, V, M extends Map<? super K, ? super V>> void readMapEntry(M map, Class<V> valueClass, TagReaderImpl in, MapDescriptor md) throws IOException {
+      int ktag = in.readTag();
+      if (ktag != md.getKeyWireTag()) {
+         throw new IllegalStateException();
+      }
+      Object k = switch (md.getKeyType()) {
+         case BOOL -> in.readBool();
+         case INT32 -> in.readInt32();
+         case INT64 -> in.readInt64();
+         case FIXED32 -> in.readFixed32();
+         case FIXED64 -> in.readFixed64();
+         case SINT32 -> in.readSInt32();
+         case SINT64 -> in.readSInt64();
+         case SFIXED32 -> in.readSFixed32();
+         case SFIXED64 -> in.readSFixed64();
+         case UINT32 -> in.readUInt32();
+         case UINT64 -> in.readUInt64();
+         case STRING -> in.readString();
+         default ->
+               throw new IllegalArgumentException("The Protobuf declared field type is not compatible with the written type : " + md.getFullName());
+      };
+      int vtag = in.readTag();
+      if (vtag != md.getValueWireTag()) {
+         throw new IllegalStateException();
+      }
+      Object v = switch (md.getType()) {
+         case BOOL -> in.readBool();
+         case INT32 -> in.readInt32();
+         case INT64 -> in.readInt64();
+         case FIXED32 -> in.readFixed32();
+         case FIXED64 -> in.readFixed64();
+         case SINT32 -> in.readSInt32();
+         case SINT64 -> in.readSInt64();
+         case SFIXED32 -> in.readSFixed32();
+         case SFIXED64 -> in.readSFixed64();
+         case UINT32 -> in.readUInt32();
+         case UINT64 -> in.readUInt64();
+         case STRING -> in.readString();
+         case MESSAGE -> readNestedObject(md, valueClass, in, -1);
+         default ->
+               throw new IllegalArgumentException("The Protobuf declared field type is not compatible with the written type : " + md.getFullName());
+      };
+      map.put((K) k, (V) v);
+      in.checkLastTagWas(0);
+   }
 }