IPROTO-273 Elements in collections are not properly limited

infinispan · Apr 15, 2024 · 11bff4f · 11bff4f
1 parent 15470cf
commit 11bff4f
Show file tree

Hide file tree

Showing 8 changed files with 586 additions and 195 deletions.
diff --git a/core/src/main/java/org/infinispan/protostream/WrappedMessage.java b/core/src/main/java/org/infinispan/protostream/WrappedMessage.java
diff --git a/core/src/main/java/org/infinispan/protostream/config/Configuration.java b/core/src/main/java/org/infinispan/protostream/config/Configuration.java
@@ -43,6 +43,8 @@ public interface Configuration {
     */
    int maxNestedMessageDepth();
 
+   boolean wrapCollectionElements();
+
    AnnotationsConfig annotationsConfig();
 
    interface AnnotationsConfig {
@@ -82,6 +84,20 @@ interface Builder {
 
       Builder maxNestedMessageDepth(int maxNestedMessageDepth);
 
+      /**
+       * Wraps all the elements in a collection or array into a wrapped message.
+       * <p>
+       * WARNING: enabling this option will change the binary format in an incompatible way. All readers/writers must
+       * have this option enabled or disabled in order to be able to parse the messages. Use with caution.
+       * <p>
+       * This option is required to fix a bug (IPROTO-273) where collections (or arrays) of non-primitive classes are
+       * unable to be read.
+       *
+       * @param wrapCollectionElements {@code true} to enable wrap the elements, {@code false} otherwise.
+       * @return This instance.
+       */
+      Builder wrapCollectionElements(boolean wrapCollectionElements);
+
       AnnotationsConfig.Builder annotationsConfig();
 
       Configuration build();

diff --git a/core/src/main/java/org/infinispan/protostream/config/impl/ConfigurationImpl.java b/core/src/main/java/org/infinispan/protostream/config/impl/ConfigurationImpl.java
@@ -1,28 +1,28 @@
 package org.infinispan.protostream.config.impl;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import org.infinispan.protostream.config.AnnotationConfiguration;
 import org.infinispan.protostream.config.Configuration;
 import org.infinispan.protostream.descriptors.AnnotationElement;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * @author [email protected]
  * @since 2.0
  */
 public final class ConfigurationImpl implements Configuration {
    private final boolean logOutOfSequenceReads;
    private final boolean logOutOfSequenceWrites;
-   private final boolean lenient;
    private final AnnotationsConfigImpl annotationsConfig;
    private final int maxNestedMessageDepth;
+   private final boolean wrapCollectionElements;
 
    private ConfigurationImpl(BuilderImpl builder, Map<String, AnnotationConfigurationImpl> annotations) {
       this.logOutOfSequenceReads = builder.logOutOfSequenceReads;
       this.logOutOfSequenceWrites = builder.logOutOfSequenceWrites;
-      this.lenient = builder.lenient;
       this.maxNestedMessageDepth = builder.maxNestedMessageDepth;
+      this.wrapCollectionElements = builder.wrapCollectionElements;
       this.annotationsConfig = new AnnotationsConfigImpl(annotations, builder.logUndefinedAnnotations);
    }
 
@@ -41,6 +41,11 @@ public int maxNestedMessageDepth() {
       return maxNestedMessageDepth;
    }
 
+   @Override
+   public boolean wrapCollectionElements() {
+      return wrapCollectionElements;
+   }
+
    @Override
    public AnnotationsConfig annotationsConfig() {
       return annotationsConfig;
@@ -85,10 +90,10 @@ public String toString() {
    public static final class BuilderImpl implements Builder {
       private boolean logOutOfSequenceReads = true;
       private boolean logOutOfSequenceWrites = true;
-      private boolean lenient = true;
       private int maxNestedMessageDepth = Configuration.DEFAULT_MAX_NESTED_DEPTH;
       private AnnotationsConfigBuilderImpl annotationsConfigBuilder = null;
       private Boolean logUndefinedAnnotations;
+      private boolean wrapCollectionElements;
 
       final class AnnotationsConfigBuilderImpl implements AnnotationsConfig.Builder {
 
@@ -138,7 +143,6 @@ public Builder setLogOutOfSequenceWrites(boolean logOutOfSequenceWrites) {
 
       @Override
       public Builder setLenient(boolean lenient) {
-         this.lenient = lenient;
          return this;
       }
 
@@ -148,6 +152,12 @@ public Builder maxNestedMessageDepth(int maxNestedMessageDepth) {
          return this;
       }
 
+      @Override
+      public Builder wrapCollectionElements(boolean wrapCollectionElements) {
+         this.wrapCollectionElements = wrapCollectionElements;
+         return this;
+      }
+
       @Override
       public AnnotationsConfig.Builder annotationsConfig() {
          if (annotationsConfigBuilder == null) {

diff --git a/types/pom.xml b/types/pom.xml
@@ -59,6 +59,37 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-slf4j-impl</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-jul</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <scope>test</scope>
+        </dependency>
+
     </dependencies>
 
     <build>

diff --git a/types/src/test/java/org/infinispan/protostream/types/java/Book.java b/types/src/test/java/org/infinispan/protostream/types/java/Book.java
@@ -0,0 +1,60 @@
+package org.infinispan.protostream.types.java;
+
+import org.infinispan.protostream.annotations.ProtoFactory;
+import org.infinispan.protostream.annotations.ProtoField;
+
+import java.util.Objects;
+
+public class Book implements Comparable<Book> {
+
+    @ProtoField(number = 1)
+    String title;
+
+    @ProtoField(number = 2)
+    String description;
+
+    @ProtoField(number = 3, defaultValue = "2023")
+    int publicationYear;
+
+    @ProtoFactory
+    public Book(String title, String description, int publicationYear) {
+        this.title = title;
+        this.description = description;
+        this.publicationYear = publicationYear;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Book book = (Book) o;
+        return publicationYear == book.publicationYear && Objects.equals(title, book.title) && Objects.equals(description, book.description);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(title, description, publicationYear);
+    }
+
+    @Override
+    public String toString() {
+        return "Book{" +
+                "title='" + title + '\'' +
+                ", description='" + description + '\'' +
+                ", publicationYear=" + publicationYear +
+                '}';
+    }
+
+    @Override
+    public int compareTo(Book o) {
+        int cmp = Integer.compare(this.publicationYear, o.publicationYear);
+        if (cmp != 0) {
+            return cmp;
+        }
+        cmp = title.compareTo(o.title);
+        if (cmp != 0) {
+            return cmp;
+        }
+        return description.compareTo(o.description);
+    }
+}
diff --git a/types/src/test/java/org/infinispan/protostream/types/java/BookSchema.java b/types/src/test/java/org/infinispan/protostream/types/java/BookSchema.java
@@ -0,0 +1,14 @@
+package org.infinispan.protostream.types.java;
+
+import org.infinispan.protostream.GeneratedSchema;
+import org.infinispan.protostream.annotations.AutoProtoSchemaBuilder;
+
+@AutoProtoSchemaBuilder(
+        includeClasses = {
+                Book.class
+        },
+        schemaFileName = "book.proto",
+        schemaFilePath = "proto/",
+        schemaPackageName = "library")
+public interface BookSchema extends GeneratedSchema {
+}