IPROTO-343 Allow null values for primitive types with PROTO3 syntax

infinispan · Jul 2, 2024 · 56015c1 · 56015c1
1 parent 77ab992
commit 56015c1
Show file tree

Hide file tree

Showing 11 changed files with 174 additions and 10 deletions.
diff --git a/core/src/main/java/org/infinispan/protostream/annotations/ProtoSchema.java b/core/src/main/java/org/infinispan/protostream/annotations/ProtoSchema.java
@@ -120,4 +120,16 @@
     * Specifies the protobuf syntax used by the generated schema. Defaults to {@link ProtoSyntax#PROTO2}
     */
    ProtoSyntax syntax() default ProtoSyntax.PROTO2;
+
+   /**
+    * Specifies whether generated marshallers should allow missing fields of boxed primitives and String to be
+    * initialized as null when the {@link ProtoField#defaultValue()} is defined as an empty string.
+    * <p>
+    * This has no affect when utilising ProtoSyntax.PROTO2, see IPROTO-349 for more details.
+    * <p>
+    * WARNING. Enabling this behaviour goes against the protobuf specification and could cause issues if your application
+    * utilises other languages and/or protobuf libraries. We only recommend enabling this feature if ProtoStream is the
+    * only protobuf library in use.
+    */
+   boolean allowNullFields() default false;
 }
diff --git a/core/src/main/java/org/infinispan/protostream/annotations/impl/BaseProtoSchemaGenerator.java b/core/src/main/java/org/infinispan/protostream/annotations/impl/BaseProtoSchemaGenerator.java
@@ -83,9 +83,11 @@ public abstract class BaseProtoSchemaGenerator {
 
    private final Map<String, ProtoTypeMetadata> metadataByTypeName = new HashMap<>();
    private final ProtoSyntax syntax;
+   private final boolean allowNullFields;
 
    protected BaseProtoSchemaGenerator(XTypeFactory typeFactory, SerializationContext serializationContext,
-                                      String generator, String fileName, String packageName, Set<XClass> classes, boolean autoImportClasses, ProtoSyntax syntax) {
+                                      String generator, String fileName, String packageName, Set<XClass> classes,
+                                      boolean autoImportClasses, ProtoSyntax syntax, boolean allowNullFields) {
       if (fileName == null) {
          throw new ProtoSchemaBuilderException("fileName cannot be null");
       }
@@ -98,6 +100,11 @@ protected BaseProtoSchemaGenerator(XTypeFactory typeFactory, SerializationContex
       this.classes = classes;
       this.autoImportClasses = autoImportClasses;
       this.syntax = syntax;
+      this.allowNullFields = allowNullFields;
+   }
+
+   public boolean allowNullFields() {
+      return allowNullFields;
    }
 
    public ProtoSyntax syntax() {

diff --git a/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoMessageTypeMetadata.java b/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoMessageTypeMetadata.java
@@ -834,7 +834,8 @@ protected XClass getJavaTypeFromAnnotation(ProtoField annotation) {
     * type.
     */
    private Object getDefaultValue(XClass clazz, String fieldName, XClass fieldType, Type protobufType, String value, boolean isRepeated) {
-      if (protoSchemaGenerator.syntax() == ProtoSyntax.PROTO2) {
+      if (protoSchemaGenerator.syntax() == ProtoSyntax.PROTO2 ||
+            (protoSchemaGenerator.allowNullFields() && !fieldType.isPrimitive())) {
          if (value == null || value.isEmpty()) {
             return null;
          }

diff --git a/...infinispan/protostream/integrationtests/processor/marshaller/GeneratedMarshallerTest.java b/...infinispan/protostream/integrationtests/processor/marshaller/GeneratedMarshallerTest.java
@@ -2,6 +2,7 @@
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
@@ -13,10 +14,13 @@
 import org.infinispan.protostream.ProtobufUtil;
 import org.infinispan.protostream.SerializationContext;
 import org.infinispan.protostream.exception.ProtoStreamException;
+import org.infinispan.protostream.integrationtests.processor.marshaller.model.FootballSchema;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.FootballSchemaImpl;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.FootballTeam;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.MapSchema;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.ModelWithMap;
+import org.infinispan.protostream.integrationtests.processor.marshaller.model.NoNullsModel;
+import org.infinispan.protostream.integrationtests.processor.marshaller.model.NullsAllowedModel;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.Player;
 import org.infinispan.protostream.integrationtests.processor.marshaller.model.SimpleEnum;
 import org.junit.Test;
@@ -36,7 +40,7 @@ public void testGenericMessage() {
       FootballTeam footBallTeam = new FootballTeam();
       footBallTeam.setName("New-Team");
 
-      Player player = new Player("fax4ever", footBallTeam);
+      Player player = new Player("fax4ever", footBallTeam, 9, 10);
       footBallTeam.setPlayers(Collections.singletonList(player));
 
       assertThatThrownBy(() -> ProtobufUtil.toWrappedByteArray(ctx, player))
@@ -66,4 +70,43 @@ public void testMaps() throws IOException {
       assertEquals(maps.getSimpleMap(), copy.getSimpleMap());
       assertEquals(maps.getEnumMap(), copy.getEnumMap());
    }
+
+   @Test
+   public void proto2NullFields() throws IOException {
+      var ctx = ProtobufUtil.newSerializationContext();
+      FootballSchema.INSTANCE.registerSchema(ctx);
+      FootballSchema.INSTANCE.registerMarshallers(ctx);
+
+      var bytes = ProtobufUtil.toWrappedByteArray(ctx, new Player(null, null, null, 0));
+      Player player = ProtobufUtil.fromWrappedByteArray(ctx, bytes);
+      assertNull(player.getName());
+      assertNull(player.getShirtNumber());
+      assertEquals(0, player.getMatchRating());
+   }
+
+   @Test
+   public void proto3AllowNullFields() throws IOException {
+      var ctx = ProtobufUtil.newSerializationContext();
+      NullsAllowedModel.SCHEMA.registerSchema(ctx);
+      NullsAllowedModel.SCHEMA.registerMarshallers(ctx);
+
+      var bytes = ProtobufUtil.toWrappedByteArray(ctx, new NullsAllowedModel(null, null, 0));
+      NullsAllowedModel model = ProtobufUtil.fromWrappedByteArray(ctx, bytes);
+      assertNull(model.string);
+      assertNull(model.boxedInt);
+      assertEquals(0, model.primitiveInt);
+   }
+
+   @Test
+   public void proto3NoNullFields() throws IOException {
+      var ctx = ProtobufUtil.newSerializationContext();
+      NoNullsModel.SCHEMA.registerSchema(ctx);
+      NoNullsModel.SCHEMA.registerMarshallers(ctx);
+
+      var bytes = ProtobufUtil.toWrappedByteArray(ctx, new NoNullsModel(null, null, 0));
+      NoNullsModel model = ProtobufUtil.fromWrappedByteArray(ctx, bytes);
+      assertEquals("", model.string);
+      assertEquals((Integer) 0, model.boxedInt);
+      assertEquals(0, model.primitiveInt);
+   }
 }
diff --git a/...rg/infinispan/protostream/integrationtests/processor/marshaller/model/FootballSchema.java b/...rg/infinispan/protostream/integrationtests/processor/marshaller/model/FootballSchema.java
@@ -6,5 +6,5 @@
 @AutoProtoSchemaBuilder(includeClasses = { FootballTeam.class, Player.class }, schemaPackageName = "org.football",
       schemaFileName = "football.proto", schemaFilePath = "proto")
 public interface FootballSchema extends GeneratedSchema {
-
+   GeneratedSchema INSTANCE = new FootballSchemaImpl();
 }
diff --git a/.../org/infinispan/protostream/integrationtests/processor/marshaller/model/NoNullsModel.java b/.../org/infinispan/protostream/integrationtests/processor/marshaller/model/NoNullsModel.java
@@ -0,0 +1,39 @@
+package org.infinispan.protostream.integrationtests.processor.marshaller.model;
+
+import org.infinispan.protostream.GeneratedSchema;
+import org.infinispan.protostream.annotations.ProtoFactory;
+import org.infinispan.protostream.annotations.ProtoField;
+import org.infinispan.protostream.annotations.ProtoSchema;
+import org.infinispan.protostream.annotations.ProtoSyntax;
+
+
+public class NoNullsModel {
+
+   public static final GeneratedSchema SCHEMA = new NoNullsSchemaImpl();
+
+   @ProtoField(1)
+   public String string;
+
+   @ProtoField(2)
+   public Integer boxedInt;
+
+   @ProtoField(3)
+   public int primitiveInt;
+
+   @ProtoFactory
+   public NoNullsModel(String string, Integer boxedInt, int primitiveInt) {
+      this.string = string;
+      this.boxedInt = boxedInt;
+      this.primitiveInt = primitiveInt;
+   }
+
+   @ProtoSchema(
+         includeClasses = NoNullsModel.class,
+         schemaPackageName = "nonulls",
+         schemaFilePath = "proto",
+         schemaFileName = "nonulls.proto",
+         syntax = ProtoSyntax.PROTO3
+   )
+   interface NoNullsSchema extends GeneratedSchema {
+   }
+}
diff --git a/...infinispan/protostream/integrationtests/processor/marshaller/model/NullsAllowedModel.java b/...infinispan/protostream/integrationtests/processor/marshaller/model/NullsAllowedModel.java
@@ -0,0 +1,40 @@
+package org.infinispan.protostream.integrationtests.processor.marshaller.model;
+
+import org.infinispan.protostream.GeneratedSchema;
+import org.infinispan.protostream.annotations.ProtoFactory;
+import org.infinispan.protostream.annotations.ProtoField;
+import org.infinispan.protostream.annotations.ProtoSchema;
+import org.infinispan.protostream.annotations.ProtoSyntax;
+
+
+public class NullsAllowedModel {
+
+   public static final GeneratedSchema SCHEMA = new NullsAllowedSchemaImpl();
+
+   @ProtoField(1)
+   public String string;
+
+   @ProtoField(2)
+   public Integer boxedInt;
+
+   @ProtoField(value = 3, defaultValue = "0")
+   public int primitiveInt;
+
+   @ProtoFactory
+   public NullsAllowedModel(String string, Integer boxedInt, int primitiveInt) {
+      this.string = string;
+      this.boxedInt = boxedInt;
+      this.primitiveInt = primitiveInt;
+   }
+
+   @ProtoSchema(
+         allowNullFields = true,
+         includeClasses = NullsAllowedModel.class,
+         schemaPackageName = "allownulls",
+         schemaFilePath = "proto",
+         schemaFileName = "allownulls.proto",
+         syntax = ProtoSyntax.PROTO3
+   )
+   interface NullsAllowedSchema extends GeneratedSchema {
+   }
+}
diff --git a/...t/java/org/infinispan/protostream/integrationtests/processor/marshaller/model/Player.java b/...t/java/org/infinispan/protostream/integrationtests/processor/marshaller/model/Player.java
@@ -7,11 +7,15 @@ public class Player {
 
    private String name;
    private FootballTeam footballTeam;
+   private Integer shirtNumber;
+   private int matchRating;
 
    @ProtoFactory
-   public Player(String name, FootballTeam footballTeam) {
+   public Player(String name, FootballTeam footballTeam, Integer shirtNumber, int matchRating) {
       this.name = name;
       this.footballTeam = footballTeam;
+      this.shirtNumber = shirtNumber;
+      this.matchRating = matchRating;
    }
 
    @ProtoField(value = 1)
@@ -23,4 +27,14 @@ public String getName() {
    public FootballTeam getFootballTeam() {
       return footballTeam;
    }
+
+   @ProtoField(3)
+   public Integer getShirtNumber() {
+      return shirtNumber;
+   }
+
+   @ProtoField(value = 4, defaultValue = "0")
+   public int getMatchRating() {
+      return matchRating;
+   }
 }
diff --git a/...pan/protostream/annotations/impl/processor/AutoProtoSchemaBuilderAnnotationProcessor.java b/...pan/protostream/annotations/impl/processor/AutoProtoSchemaBuilderAnnotationProcessor.java
@@ -261,8 +261,7 @@ private void processPackage(RoundEnvironment roundEnv, SerializationContext serC
 
       CompileTimeProtoSchemaGenerator protoSchemaGenerator = new CompileTimeProtoSchemaGenerator(typeFactory, generatedFilesWriter, serCtx,
             initializerPackageName, protobufFileName, protobufPackageName, dependencies.marshalledClasses, xclasses, builderAnnotation.autoImportClasses(),
-            builderAnnotation.syntax(),
-            classScanner);
+            builderAnnotation.syntax(), builderAnnotation.allowNullFields(), classScanner);
       String schemaSrc = protoSchemaGenerator.generateAndRegister();
 
       writeSerializationContextInitializer(packageElement, packageElement.getQualifiedName().toString(), builderAnnotation,
@@ -306,7 +305,8 @@ private void processClass(RoundEnvironment roundEnv, SerializationContext serCtx
       Set<XClass> xclasses = classScanner.getXClasses();
 
       CompileTimeProtoSchemaGenerator protoSchemaGenerator = new CompileTimeProtoSchemaGenerator(typeFactory, generatedFilesWriter, serCtx,
-            typeElement.getQualifiedName().toString(), protobufFileName, protobufPackageName, dependencies.marshalledClasses, xclasses, annotation.autoImportClasses(), annotation.syntax(), classScanner);
+            typeElement.getQualifiedName().toString(), protobufFileName, protobufPackageName, dependencies.marshalledClasses,
+            xclasses, annotation.autoImportClasses(), annotation.syntax(), annotation.allowNullFields(), classScanner);
 
       String schemaSrc = protoSchemaGenerator.generateAndRegister();
 

diff --git a/...rg/infinispan/protostream/annotations/impl/processor/CompileTimeProtoSchemaGenerator.java b/...rg/infinispan/protostream/annotations/impl/processor/CompileTimeProtoSchemaGenerator.java
@@ -39,8 +39,9 @@ final class CompileTimeProtoSchemaGenerator extends BaseProtoSchemaGenerator {
    CompileTimeProtoSchemaGenerator(XTypeFactory typeFactory, GeneratedFilesWriter generatedFilesWriter,
                                    SerializationContext serializationContext, String generator,
                                    String fileName, String packageName, Map<XClass, CompileTimeDependency> dependencies,
-                                   Set<XClass> classes, boolean autoImportClasses, ProtoSyntax syntax, AnnotatedClassScanner classScanner) {
-      super(typeFactory, serializationContext, generator, fileName, packageName, classes, autoImportClasses, syntax);
+                                   Set<XClass> classes, boolean autoImportClasses, ProtoSyntax syntax, boolean allowNullFields,
+                                   AnnotatedClassScanner classScanner) {
+      super(typeFactory, serializationContext, generator, fileName, packageName, classes, autoImportClasses, syntax, allowNullFields);
       this.dependencies = dependencies;
       this.marshallerSourceCodeGenerator = new MarshallerSourceCodeGenerator(generatedFilesWriter, typeFactory, packageName);
       this.classScanner = classScanner;

diff --git a/...ain/java/org/infinispan/protostream/annotations/impl/processor/ProtoSchemaAnnotation.java b/...ain/java/org/infinispan/protostream/annotations/impl/processor/ProtoSchemaAnnotation.java
@@ -20,6 +20,7 @@ class ProtoSchemaAnnotation {
    private final String schemaPackageName;
    private final boolean service;
    private final ProtoSyntax syntax;
+   private boolean allowNullFields;
    private final String[] value;
    private final Annotation annotation;
    private final String annotationName;
@@ -36,6 +37,7 @@ public ProtoSchemaAnnotation(AutoProtoSchemaBuilder annotation) {
       schemaPackageName = annotation.schemaPackageName();
       service = annotation.service();
       syntax = annotation.syntax();
+      allowNullFields = false;
       value = annotation.value();
    }
 
@@ -51,6 +53,7 @@ public ProtoSchemaAnnotation(ProtoSchema annotation) {
       schemaPackageName = annotation.schemaPackageName();
       service = annotation.service();
       syntax = annotation.syntax();
+      allowNullFields = annotation.allowNullFields();
       value = annotation.value();
    }
 
@@ -114,6 +117,10 @@ public ProtoSyntax syntax() {
       return syntax;
    }
 
+   public boolean allowNullFields() {
+      return allowNullFields;
+   }
+
    public String[] value() {
       return value;
    }