fix empty union codegen & skip option (#484)

linkedin · Apr 28, 2023 · d4d22f3 · d4d22f3
1 parent e578c6b
commit d4d22f3
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 54 deletions.
diff --git a/avro-builder/builder/src/main/java/com/linkedin/avroutil1/builder/SchemaBuilder.java b/avro-builder/builder/src/main/java/com/linkedin/avroutil1/builder/SchemaBuilder.java
@@ -209,9 +209,9 @@ public static void main(String[] args) throws Exception {
       handleUtf8EncodingInPutByIndex = Boolean.TRUE.equals(Boolean.parseBoolean(value));
     }
 
-    boolean skipCodegenIfSchemaOnClasspath = true;
+    boolean skipCodegenIfSchemaOnClasspath = false;
     if (options.has(skipCodegenIfSchemaOnClasspathOpt)) {
-      String value = options.valueOf(enableUtf8EncodingInPutByIndex);
+      String value = options.valueOf(skipCodegenIfSchemaOnClasspathOpt);
       skipCodegenIfSchemaOnClasspath = Boolean.TRUE.equals(Boolean.parseBoolean(value));
     }
 

diff --git a/...rc/main/java/com/linkedin/avroutil1/builder/operations/codegen/own/AvroUtilCodeGenOp.java b/...rc/main/java/com/linkedin/avroutil1/builder/operations/codegen/own/AvroUtilCodeGenOp.java
@@ -297,10 +297,7 @@ public void run(OperationContext opContext) throws Exception {
             }
           }
         } catch (Exception e) {
-          errorCount++;
-          //TODO - error-out
-          LOGGER.error(
-              "failed to generate class for " + fullname, e);
+          throw new RuntimeException("failed to generate class for " + fullname, e);
         }
       }
       writeJavaFilesToDisk(generatedSpecificClasses, config.getOutputSpecificRecordClassesRoot());

diff --git a/avro-codegen/build.gradle b/avro-codegen/build.gradle
@@ -17,6 +17,7 @@ dependencies {
     implementation "com.squareup:javapoet:1.13.0"
     implementation "commons-io:commons-io:2.6"
     implementation "org.apache.logging.log4j:log4j-api:2.17.1"
+    implementation "org.slf4j:slf4j-api:1.7.14"
     implementation "com.beust:jcommander:1.78"
     implementation "org.apache.ant:ant:1.10.7"
     testImplementation "com.fasterxml.jackson.core:jackson-core:2.10.2"

diff --git a/avro-codegen/src/main/java/com/linkedin/avroutil1/codegen/SpecificRecordClassGenerator.java b/avro-codegen/src/main/java/com/linkedin/avroutil1/codegen/SpecificRecordClassGenerator.java
@@ -50,6 +50,8 @@
 import java.util.Map;
 import java.util.Queue;
 import java.util.StringJoiner;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import java.util.stream.Collectors;
 import javax.lang.model.element.Modifier;
 import javax.tools.JavaFileObject;
@@ -59,6 +61,7 @@
  * generates java classes out of avro schemas.
  */
 public class SpecificRecordClassGenerator {
+  private static final Logger LOGGER = LoggerFactory.getLogger(SpecificRecordClassGenerator.class);
 
   private int sizeValCounter = -1;
 
@@ -1140,24 +1143,29 @@ private String getSerializedCustomDecodeBlock(SpecificRecordGenerationConfig con
 
       case UNION:
         int numberOfUnionMembers = ((AvroUnionSchema) fieldSchema).getTypes().size();
-        codeBlockBuilder.beginControlFlow("switch(in.readIndex())");
-        for (int i = 0; i < numberOfUnionMembers; i++) {
-
-          SchemaOrRef unionMember = ((AvroUnionSchema) fieldSchema).getTypes().get(i);
-          codeBlockBuilder.addStatement("case $L: ", i);
-          codeBlockBuilder.addStatement(
-              getSerializedCustomDecodeBlock(config, unionMember.getSchema(), unionMember.getSchema().type(), fieldName,
-                  schemaFieldName, arrayOption + ".getTypes().get(" + i + ")" ));
-          if (unionMember.getSchema().type().equals(AvroType.NULL)) {
-            codeBlockBuilder.addStatement("$L = null", fieldName);
+        if (numberOfUnionMembers > 0) {
+          codeBlockBuilder.beginControlFlow("switch(in.readIndex())");
+          for (int i = 0; i < numberOfUnionMembers; i++) {
+
+            SchemaOrRef unionMember = ((AvroUnionSchema) fieldSchema).getTypes().get(i);
+            codeBlockBuilder.addStatement("case $L: ", i);
+            codeBlockBuilder.addStatement(
+                getSerializedCustomDecodeBlock(config, unionMember.getSchema(), unionMember.getSchema().type(),
+                    fieldName, schemaFieldName, arrayOption + ".getTypes().get(" + i + ")"));
+            if (unionMember.getSchema().type().equals(AvroType.NULL)) {
+              codeBlockBuilder.addStatement("$L = null", fieldName);
+            }
+            codeBlockBuilder.addStatement("break");
           }
-          codeBlockBuilder.addStatement("break");
-        }
-        codeBlockBuilder.addStatement("default:")
-            .addStatement("throw new $T($S)", IndexOutOfBoundsException.class, "Union IndexOutOfBounds")
-            .endControlFlow();
+          codeBlockBuilder.addStatement("default:")
+              .addStatement("throw new $T($S)", IndexOutOfBoundsException.class, "Union IndexOutOfBounds")
+              .endControlFlow();
 
-        serializedCodeBlock = codeBlockBuilder.build().toString();
+          serializedCodeBlock = codeBlockBuilder.build().toString();
+        } else {
+          LOGGER.warn("Unions with Zero types are not recommended and poorly supported. "
+              + "Please consider using a nullable field instead. Field name: " + fieldName);
+        }
         break;
       case RECORD:
         TypeName className = SpecificRecordGeneratorUtil.getTypeName(fieldSchema, fieldType, true,
@@ -1322,42 +1330,43 @@ arrayItemAvroType, getElementVarName()))
         break;
       case UNION:
         int numberOfUnionMembers = ((AvroUnionSchema) fieldSchema).getTypes().size();
-
-        for (int i = 0; i < numberOfUnionMembers; i++) {
-          AvroSchema unionMemberSchema = ((AvroUnionSchema) fieldSchema).getTypes().get(i).getSchema();
-          Class<?> unionMemberType =
-              SpecificRecordGeneratorUtil.getJavaClassForAvroTypeIfApplicable(unionMemberSchema.type(),
-                  config.getDefaultFieldStringRepresentation(), true);
-          TypeName unionMemberTypeName =
-              SpecificRecordGeneratorUtil.getTypeName(unionMemberSchema, unionMemberSchema.type(), false,
-                  config.getDefaultFieldStringRepresentation());
-
-          if (i == 0) {
-            if (unionMemberSchema.type().equals(AvroType.NULL)) {
-              codeBlockBuilder.beginControlFlow("if ($L == null) ", fieldName);
-            } else {
-              codeBlockBuilder.beginControlFlow("if ($L instanceof $T) ", fieldName,
-                  unionMemberType != null ? unionMemberType : unionMemberTypeName);
-            }
-          } else {
-            codeBlockBuilder.endControlFlow();
-            if (unionMemberSchema.type().equals(AvroType.NULL)) {
-              codeBlockBuilder.beginControlFlow(" else if ($L == null) ", fieldName);
+        if (numberOfUnionMembers > 0) {
+          for (int i = 0; i < numberOfUnionMembers; i++) {
+            AvroSchema unionMemberSchema = ((AvroUnionSchema) fieldSchema).getTypes().get(i).getSchema();
+            Class<?> unionMemberType =
+                SpecificRecordGeneratorUtil.getJavaClassForAvroTypeIfApplicable(unionMemberSchema.type(),
+                    config.getDefaultFieldStringRepresentation(), true);
+            TypeName unionMemberTypeName =
+                SpecificRecordGeneratorUtil.getTypeName(unionMemberSchema, unionMemberSchema.type(), false,
+                    config.getDefaultFieldStringRepresentation());
+
+            if (i == 0) {
+              if (unionMemberSchema.type().equals(AvroType.NULL)) {
+                codeBlockBuilder.beginControlFlow("if ($L == null) ", fieldName);
+              } else {
+                codeBlockBuilder.beginControlFlow("if ($L instanceof $T) ", fieldName,
+                    unionMemberType != null ? unionMemberType : unionMemberTypeName);
+              }
             } else {
-              codeBlockBuilder.beginControlFlow(" else if ($L instanceof $T) ", fieldName,
-                  unionMemberType != null ? unionMemberType : unionMemberTypeName);
+              codeBlockBuilder.endControlFlow();
+              if (unionMemberSchema.type().equals(AvroType.NULL)) {
+                codeBlockBuilder.beginControlFlow(" else if ($L == null) ", fieldName);
+              } else {
+                codeBlockBuilder.beginControlFlow(" else if ($L instanceof $T) ", fieldName,
+                    unionMemberType != null ? unionMemberType : unionMemberTypeName);
+              }
             }
+            codeBlockBuilder.addStatement("out.writeIndex($L)", i)
+                .addStatement(
+                    getSerializedCustomEncodeBlock(config, unionMemberSchema, unionMemberSchema.type(), fieldName));
           }
-          codeBlockBuilder.addStatement("out.writeIndex($L)", i)
-              .addStatement(getSerializedCustomEncodeBlock(config, unionMemberSchema, unionMemberSchema.type(),
-                   fieldName));
-        }
-        codeBlockBuilder.endControlFlow()
-            .beginControlFlow("else")
-            .addStatement("throw new $T($S)", IllegalArgumentException.class, "Value does not match any union member")
-            .endControlFlow();
+          codeBlockBuilder.endControlFlow()
+              .beginControlFlow("else")
+              .addStatement("throw new $T($S)", IllegalArgumentException.class, "Value does not match any union member")
+              .endControlFlow();
 
         serializedCodeBlock = codeBlockBuilder.build().toString();
+        }
         break;
       case RECORD:
         TypeName className = SpecificRecordGeneratorUtil.getTypeName(fieldSchema, fieldType, true,

diff --git a/...odegen/src/test/java/com/linkedin/avroutil1/codegen/SpecificRecordClassGeneratorTest.java b/...odegen/src/test/java/com/linkedin/avroutil1/codegen/SpecificRecordClassGeneratorTest.java
@@ -190,6 +190,5 @@ public void testProblematicRecord() throws Exception {
     JavaFileObject javaFileObject =
         generator.generateSpecificClass(schema, SpecificRecordGenerationConfig.BROAD_COMPATIBILITY);
     CompilerHelper.assertCompiles(javaFileObject);
-
   }
 }
diff --git a/avro-codegen/src/test/resources/schemas/TestProblematicRecord.avsc b/avro-codegen/src/test/resources/schemas/TestProblematicRecord.avsc
@@ -14,6 +14,10 @@
     {
       "name": "field",
       "type": "string"
+    },
+    {
+      "name": "fieldOfEmptyUnion",
+        "type": [ ]
     }
   ]
 }