SchemaTranslator: support non-null defaults for primitive and collect…

…ion optional fields (#994)

CHANGELOG.md

@@ -14,6 +14,9 @@ and what APIs have changed, if applicable.
 
 ## [Unreleased]
 
+## [29.51.14] - 2024-03-27
+- Support translating default values for optional non-record/union fields to Avro (when TRANSLATE_DEFAULT is enabled). 
+
 ## [29.51.13] - 2024-03-26
 Upgrade the io.envoyproxy.controlplane module to 0.1.35
 
@@ -5671,7 +5674,8 @@ patch operations can re-use these classes for generating patch messages.
 
 ## [0.14.1]
 
-[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.51.13...master
+[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.51.14...master
+[29.51.14]: https://github.com/linkedin/rest.li/compare/v29.51.13...v29.51.14
 [29.51.13]: https://github.com/linkedin/rest.li/compare/v29.51.12...v29.51.13
 [29.51.12]: https://github.com/linkedin/rest.li/compare/v29.51.11...v29.51.12
 [29.51.11]: https://github.com/linkedin/rest.li/compare/v29.51.10...v29.51.11

data-avro/src/main/java/com/linkedin/data/avro/DefaultDataToAvroConvertCallback.java

@@ -137,9 +137,19 @@ protected Object translateField(List<Object> path, Object fieldValue, RecordData
           if (_options.getOptionalDefaultMode() != OptionalDefaultMode.TRANSLATE_TO_NULL &&
               field.getDefault() != null)
           {
-            throw new IllegalArgumentException(
-                message(path,
-                    "cannot translate absent optional field (to have null value) because this field is optional and has a default value"));
+            DataSchema.Type fieldDefaultValueType = field.getType().getType();
+            // punt on other default values for now (too complex to handle)
+            // NOTE: union case was handled above already.
+            if (fieldDefaultValueType == DataSchema.Type.RECORD || fieldDefaultValueType == DataSchema.Type.TYPEREF)
+            {
+              throw new IllegalArgumentException(message(path,
+                  "cannot translate absent optional field (to have null value) because this field is optional and has a default value"));
+            }
+            else
+            // use default value provided by user for primitive, map, and array types.
+            {
+              return translateField(path, field.getDefault(), field);
+            }
           }
           fieldValue = Data.NULL;
           fieldDataSchema = DataSchemaConstants.NULL_DATA_SCHEMA;

data-avro/src/test/java/com/linkedin/data/avro/TestSchemaTranslator.java

@@ -16,6 +16,8 @@
 
 package com.linkedin.data.avro;
 
+import com.google.common.base.Charsets;
+import com.google.common.io.CharStreams;
 import com.linkedin.avroutil1.compatibility.AvroCompatibilityHelper;
 import com.linkedin.avroutil1.compatibility.SchemaParseConfiguration;
 import com.linkedin.data.Data;
@@ -31,6 +33,8 @@
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
@@ -855,8 +859,7 @@ public void testToAvroSchemaTestTypeRefAnnotationPropagation(String schemaBefore
 
 
   @DataProvider
-  public Object[][] toAvroSchemaData()
-  {
+  public Object[][] toAvroSchemaData() throws IOException {
     final String emptyFooSchema = "{ \"type\" : \"record\", \"name\" : \"foo\", \"fields\" : [ ] }";
     final String emptyFooValue = "{}";
 
@@ -895,6 +898,14 @@ public Object[][] toAvroSchemaData()
               null,
               null
           },
+          {
+            getTestResourceAsString("avro/com/linkedin/pegasus/test/NonNullDefaultsTest.avsc"),
+              translateDefault,
+              "{ \"type\" : \"record\", \"name\" : \"Outer\", \"namespace\" : \"foo\", \"fields\" : [ { \"name\" : \"f1\", \"type\" : [ { \"type\" : \"record\", \"name\" : \"Inner\", \"namespace\" : \"bar\", \"fields\" : [ { \"name\" : \"innerArray\", \"type\" : [ { \"type\" : \"array\", \"items\" : \"string\" }, \"null\" ], \"default\" : [  ] }, { \"name\" : \"innerMap\", \"type\" : [ { \"type\" : \"map\", \"values\" : \"string\" }, \"null\" ], \"default\" : {  } }, { \"name\" : \"innerInt\", \"type\" : \"int\", \"default\" : 0 }, { \"name\" : \"innerString\", \"type\" : [ \"string\", \"null\" ], \"default\" : \"defaultValue\" } ] }, \"null\" ], \"default\" : { \"innerInt\" : 0, \"innerArray\" : [  ], \"innerMap\" : {  }, \"innerString\" : \"defaultValue\" } } ] }",
+              null,
+              null,
+              null
+          },
           {
               // required, optional not specified
               "{ \"type\" : \"record\", \"name\" : \"foo\", \"fields\" : [ { \"name\" : \"bar\", \"type\" : ##T_START \"int\" ##T_END } ] }",
@@ -2645,36 +2656,6 @@ public Object[][] toAvroSchemaErrorData()
                 IllegalArgumentException.class,
                 "cannot translate union value"
             },
-            {
-                // inconsistent default,
-                // a referenced record has an optional field "frank" with default,
-                // but field of referenced record type has default value which does not provide value for "frank"
-                "{ " +
-                    "  \"type\" : \"record\", " +
-                    "  \"name\" : \"Bar\", " +
-                    "  \"fields\" : [ " +
-                    "    { " +
-                    "      \"name\" : \"barbara\", " +
-                    "      \"type\" : { " +
-                    "        \"type\" : \"record\", " +
-                    "        \"name\" : \"Foo\", " +
-                    "        \"fields\" : [ " +
-                    "          { " +
-                    "            \"name\" : \"frank\", " +
-                    "            \"type\" : \"string\", " +
-                    "            \"default\" : \"abc\", " +
-                    "            \"optional\" : true" +
-                    "          } " +
-                    "        ] " +
-                    "      }, " +
-                    "      \"default\" : { } " +
-                    "    } " +
-                    "  ]" +
-                    "}",
-                translateDefault,
-                IllegalArgumentException.class,
-                "cannot translate absent optional field (to have null value) because this field is optional and has a default value"
-            },
             {
                 // inconsistent default,
                 // a referenced record has an optional field "bar1" without default which translates with union with null as 1st member
@@ -3463,4 +3444,16 @@ private static String readFile(File file) throws IOException
     }
     return sb.toString();
   }
+
+  private InputStream getTestResource(String resourceName) {
+    return getClass().getClassLoader().getResourceAsStream(resourceName);
+  }
+
+  private String getTestResourceAsString(String resourceName) throws IOException {
+    InputStream is = getTestResource(resourceName);
+    if (is == null) {
+      throw new IllegalArgumentException("not found: " + resourceName);
+    }
+    return CharStreams.toString(new InputStreamReader(is, Charsets.UTF_8));
+  }
 }

data-avro/src/test/resources/avro/com/linkedin/pegasus/test/NonNullDefaultsTest.avsc

@@ -0,0 +1,48 @@
+{
+  "type": "record",
+  "name": "Outer",
+  "namespace": "foo",
+  "fields": [
+    {
+      "name": "f1",
+      "type": {
+        "type": "record",
+        "name": "Inner",
+        "namespace": "bar",
+        "fields": [
+          {
+            "name": "innerArray",
+            "type": {
+              "type": "array",
+              "items": "string"
+            },
+            "default": [],
+            "optional": true
+          },
+          {
+            "name": "innerMap",
+            "type": {
+              "type": "map",
+              "values": "string"
+            },
+            "default": {},
+            "optional": true
+          },
+          {
+            "name": "innerInt",
+            "type": "int",
+            "default": 0
+          },
+          {
+            "name": "innerString",
+            "type": "string",
+            "default": "defaultValue",
+            "optional": true
+          }
+        ]
+      },
+      "default": {},
+      "optional": true
+    }
+  ]
+}

gradle.properties

@@ -1,4 +1,4 @@
-version=29.51.13
+version=29.51.14
 group=com.linkedin.pegasus
 org.gradle.configureondemand=true
 org.gradle.parallel=true