codegen: Getters for collection fields no longer return unmodifiable collection

avro-builder/tests/codegen-110/src/main/avro/vs110/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-111/src/main/avro/vs111/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-14/src/main/avro/vs14/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-15/src/main/avro/vs15/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-16/src/main/avro/vs16/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-17/src/main/avro/vs17/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-18/src/main/avro/vs18/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-19/src/main/avro/vs19/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-charseq-method/src/main/avro/charseqmethod/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/codegen-no-utf8-in-putbyindex/src/main/avro/charseqmethod/TestCollections.avsc

@@ -6,6 +6,13 @@
       "name": "str",
       "type": "string"
     },
+    {
+      "name": "intAr",
+      "type": {
+        "type": "array",
+        "items": "int"
+      }
+    },
     {
       "name": "strAr",
       "type": {

avro-builder/tests/tests-allavro/src/test/java/com/linkedin/avroutil1/builder/SpecificRecordTest.java

@@ -1834,7 +1834,7 @@ public void testNewBuilder() throws Exception {
   public static Object[][] testPrivateModifierOnChunkMethodProvider() {
     return new Object[][]{{vs14.ThousandField.class}, {vs19.ThousandField.class}};
   }
-  
+
  @Test(dataProvider = "testPrivateModifierOnChunkMethodProvider")
   public <T extends IndexedRecord> void testPrivateModifierOnChunkMethod(Class<T> clazz) {
 
@@ -1854,6 +1854,33 @@ public <T extends IndexedRecord> void testPrivateModifierOnChunkMethod(Class<T>
     chunkMethodNames.forEach(method -> Assert.assertTrue(Modifier.isPrivate(method.getModifiers())));
   }
 
+  @Test
+  public void modifiablePrimitiveCollectionTest() {
+    String tba = "NewElement";
+    RandomRecordGenerator generator = new RandomRecordGenerator();
+    TestCollections instance = generator.randomSpecific(TestCollections.class, RecordGenerationConfig.newConfig().withAvoidNulls(true));
+
+    // array of string
+    instance.getStrAr().add(tba);
+    Assert.assertTrue(instance.getStrAr().contains(tba));
+
+    // union[null, List<String>]
+    instance.getUnionOfArray().add(tba);
+    Assert.assertTrue(instance.getUnionOfArray().contains(tba));
+
+    // array (union[null, string])
+    instance.getArOfUnionOfStr().add(tba);
+    Assert.assertTrue(instance.getArOfUnionOfStr().contains(tba));
+
+
+    // Union (null, Map<String, String>)
+    instance.getUnionOfMap().put("key1", tba);
+    Assert.assertEquals(tba, instance.getUnionOfMap().get("key1"));
+
+    instance.getIntAr().add(Integer.MAX_VALUE);
+    Assert.assertEquals((int) instance.getIntAr().get(instance.getIntAr().size() - 1), Integer.MAX_VALUE);
+  }
+
   @BeforeClass
   public void setup() {
     System.setProperty("org.apache.avro.specific.use_custom_coders", "true");

avro-codegen/src/main/java/com/linkedin/avroutil1/codegen/SpecificRecordClassGenerator.java

@@ -1701,12 +1701,14 @@ private void addGetByIndexMethod(TypeSpec.Builder classBuilder, AvroRecordSchema
         switchBuilder.addStatement("case $L: return com.linkedin.avroutil1.compatibility.StringConverterUtil.get$L(this.$L)", fieldIndex++, fieldClass.getSimpleName(), escapedFieldName);
       } else if (SpecificRecordGeneratorUtil.isListTransformerApplicableForSchema(field.getSchema())) {
         switchBuilder.addStatement(
-            "case $L: return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$LList(this.$L)",
-            fieldIndex++, config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName);
+            "case $L: return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$LList(this.$L, $L)",
+            fieldIndex++, config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName,
+            SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
       } else if (SpecificRecordGeneratorUtil.isMapTransformerApplicable(field.getSchema())) {
         switchBuilder.addStatement(
-            "case $L: return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$LMap(this.$L)",
-            fieldIndex++, config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName);
+            "case $L: return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$LMap(this.$L, $L)",
+            fieldIndex++, config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName,
+            SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
       } else if (field.getSchema() != null && AvroType.UNION.equals(field.getSchema().type())) {
 
         switchBuilder.addStatement("case $L:", fieldIndex++);
@@ -1726,16 +1728,16 @@ private void addGetByIndexMethod(TypeSpec.Builder classBuilder, AvroRecordSchema
               unionMemberSchema.getSchema())) {
             switchBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, List.class)
                 .addStatement(
-                    "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$1LList($2L)",
-                    config.getDefaultMethodStringRepresentation().getJsonValue(),
-                    escapedFieldName)
+                    "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$1LList($2L, $3L)",
+                    config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName,
+                    SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()))
                 .endControlFlow();
           } else if (SpecificRecordGeneratorUtil.isMapTransformerApplicable(unionMemberSchema.getSchema())) {
             switchBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, Map.class)
                 .addStatement(
-                    "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$1LMap($2L)",
-                    config.getDefaultMethodStringRepresentation().getJsonValue(),
-                    escapedFieldName)
+                    "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$1LMap($2L, $3L)",
+                    config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName,
+                    SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()))
                 .endControlFlow();
           }
         }
@@ -1920,25 +1922,25 @@ private MethodSpec getSetterMethodSpec(AvroSchemaField field, SpecificRecordGene
           methodSpecBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, List.class);
           if (config.getDefaultFieldStringRepresentation().equals(AvroJavaStringRepresentation.STRING)) {
             methodSpecBuilder.addStatement(
-                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.getStringList($1L)",
-                escapedFieldName);
+                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.getStringList($1L, $2L)",
+                escapedFieldName, SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
           } else {
             methodSpecBuilder.addStatement(
-                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.getUtf8List($1L)",
-                escapedFieldName);
+                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.getUtf8List($1L, $2L)",
+                escapedFieldName, SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
           }
           methodSpecBuilder.endControlFlow();
 
         } else if (SpecificRecordGeneratorUtil.isMapTransformerApplicable(unionMemberSchema.getSchema())) {
           methodSpecBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, Map.class);
           if (config.getDefaultFieldStringRepresentation().equals(AvroJavaStringRepresentation.STRING)) {
             methodSpecBuilder.addStatement(
-                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.getStringMap($1L)",
-                escapedFieldName);
+                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.getStringMap($1L, $2L)",
+                escapedFieldName, SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
           } else {
             methodSpecBuilder.addStatement(
-                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.getUtf8Map($1L)",
-                escapedFieldName);
+                "this.$1L = com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.getUtf8Map($1L, $2L)",
+                escapedFieldName, SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
           }
           methodSpecBuilder.endControlFlow();
         }
@@ -1983,14 +1985,14 @@ private MethodSpec getGetterMethodSpec(AvroSchemaField field, SpecificRecordGene
           escapedFieldName, config.getDefaultMethodStringRepresentation().getJsonValue());
     } else if (SpecificRecordGeneratorUtil.isListTransformerApplicableForSchema(field.getSchema())) {
       methodSpecBuilder.addStatement(
-          "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$LList(this.$L)",
-          config.getDefaultMethodStringRepresentation().getJsonValue(),
-          escapedFieldName);
+          "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$LList(this.$L, $L)",
+          config.getDefaultMethodStringRepresentation().getJsonValue(), escapedFieldName,
+          SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
     } else if (SpecificRecordGeneratorUtil.isMapTransformerApplicable(field.getSchema())) {
       methodSpecBuilder.addStatement(
-          "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$LMap(this.$L)",
+          "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$LMap(this.$L, $L)",
           config.getDefaultMethodStringRepresentation().getJsonValue(),
-          escapedFieldName);
+          escapedFieldName, SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()));
     } else if (field.getSchema() != null && AvroType.UNION.equals(field.getSchema().type())) {
 
       methodSpecBuilder.beginControlFlow("if (this.$1L == null)", escapedFieldName)
@@ -2007,14 +2009,16 @@ private MethodSpec getGetterMethodSpec(AvroSchemaField field, SpecificRecordGene
         } else if (SpecificRecordGeneratorUtil.isListTransformerApplicableForSchema(unionMemberSchema.getSchema())) {
           methodSpecBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, List.class)
               .addStatement(
-                  "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$2LList($1L)",
-                  escapedFieldName, config.getDefaultMethodStringRepresentation().getJsonValue())
+                  "return com.linkedin.avroutil1.compatibility.collectiontransformer.ListTransformer.get$2LList($1L, $3L)",
+                  escapedFieldName, config.getDefaultMethodStringRepresentation().getJsonValue(),
+                  SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()))
               .endControlFlow();
         } else if (SpecificRecordGeneratorUtil.isMapTransformerApplicable(unionMemberSchema.getSchema())) {
           methodSpecBuilder.beginControlFlow("else if($1L instanceof $2T)", escapedFieldName, Map.class)
               .addStatement(
-                  "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$2LMap($1L)",
-                  escapedFieldName, config.getDefaultMethodStringRepresentation().getJsonValue())
+                  "return com.linkedin.avroutil1.compatibility.collectiontransformer.MapTransformer.get$2LMap($1L, $3L)",
+                  escapedFieldName, config.getDefaultMethodStringRepresentation().getJsonValue(),
+                  SpecificRecordGeneratorUtil.isCollectionSchemaValuePrimitive(field.getSchema()))
               .endControlFlow();
         }
       }

avro-codegen/src/main/java/com/linkedin/avroutil1/codegen/SpecificRecordGeneratorUtil.java

@@ -288,6 +288,54 @@ public static boolean isMapTransformerApplicable(AvroSchema schema) {
     return isNullUnionOf(AvroType.MAP, schema);
   }
 
+  /**
+   * If a schema is primitive type or a null union of primitive type, it can be handled as a primitive type
+   * @param schema schema to be validated
+   * @return true if schema can be handled as primitive, else false
+   */
+  private static boolean canBeHandledAsPrimitiveType(AvroSchema schema) {
+    if (schema instanceof AvroUnionSchema) {
+      if (((AvroUnionSchema) schema).getTypes().size() != 2) {
+        return false;
+      }
+
+      return ((AvroUnionSchema) schema).getTypes().get(0).getSchema().type().equals(AvroType.NULL)
+          && ((AvroUnionSchema) schema).getTypes().get(1).getSchema().type().isPrimitive()
+          || ((AvroUnionSchema) schema).getTypes().get(1).getSchema().type().equals(AvroType.NULL)
+          && ((AvroUnionSchema) schema).getTypes().get(0).getSchema().type().isPrimitive();
+    }
+
+    return schema.type().isPrimitive();
+  }
+
+  /**
+   * schema type must be either a List of Map (or a null union of List/Map)
+   * @return true if schema value is primitive
+   */
+  public static boolean isCollectionSchemaValuePrimitive(AvroSchema schema) {
+    if(!isListTransformerApplicableForSchema(schema) && !isMapTransformerApplicable(schema)) {
+      return false;
+    }
+    if (schema.type().equals(AvroType.UNION)) {
+      if (((AvroUnionSchema) schema).getTypes().get(0).getSchema().type().equals(AvroType.NULL)) {
+        schema = ((AvroUnionSchema) schema).getTypes().get(1).getSchema();
+      } else if (((AvroUnionSchema) schema).getTypes().get(1).getSchema().type().equals(AvroType.NULL)) {
+        schema = ((AvroUnionSchema) schema).getTypes().get(0).getSchema();
+      } else {
+        // checks with isListTransformerApplicable and isMapTransformerApplicable should prevent this from happening
+        throw new IllegalArgumentException("schema type must be either a List of Map (or a null union of List/Map)");
+      }
+    }
+
+    if(AvroType.ARRAY.equals(schema.type())) {
+      return canBeHandledAsPrimitiveType(((AvroArraySchema) schema).getValueSchema());
+    } else if (AvroType.MAP.equals(schema.type())) {
+      return canBeHandledAsPrimitiveType(((AvroMapSchema) schema).getValueSchema());
+    } else {
+      // checks with isListTransformerApplicable and isMapTransformerApplicable should prevent this from happening
+      throw new IllegalArgumentException("schema type must be either a List of Map (or a null union of List/Map)");
+    }
+  }
 
   public static boolean schemaContainsString(AvroSchema schema) {
     if (schema == null) {