Re: [PR] Add support for enum default values in IDL serialization [avro]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #3822:
URL: https://github.com/apache/avro/pull/3822#discussion_r3410044530


##########
lang/java/idl/src/main/java/org/apache/avro/idl/IdlUtils.java:
##########
@@ -285,7 +285,12 @@ private static void writeSchema(Schema schema, boolean insideProtocol, Writer wr
       } else {
         throw new AvroRuntimeException("Enum schema must have at least a symbol " + schema);
       }
-      writer.append(NEWLINE).append(indent).append("}").append(NEWLINE);
+      writer.append(NEWLINE).append(indent).append("}");
+      var enumDefault = schema.getEnumDefault();
+      if (enumDefault != null) {
+        writer.append(" = ").append(enumDefault).append(";");
+      }
+      writer.append(NEWLINE);

Review Comment:
   `var` requires Java 10+ local-variable type inference. If this module is built with Java 8/9 source/target compatibility (common for Avro), this will not compile. Replace `var` with an explicit type (e.g., `String enumDefault = ...`).



##########
lang/java/idl/src/test/java/org/apache/avro/idl/IdlUtilsTest.java:
##########
@@ -103,6 +106,23 @@ public void cannotWriteProtocolWithUnnamedTypes() {
         () -> IdlUtils.writeIdlProtocol(new StringWriter(), Schema.create(Schema.Type.STRING)));
   }
 
+  @Test
+  public void enumDefaultIsWrittenToIdl() throws IOException {
+    Schema withDefault = Schema.createEnum("Status", null, "naming", asList("ACTIVE", "INACTIVE"), "ACTIVE");
+    Schema withoutDefault = Schema.createEnum("Status", null, "naming", asList("ACTIVE", "INACTIVE"));
+
+    StringWriter withDefaultWriter = new StringWriter();
+    IdlUtils.writeIdlProtocol(withDefaultWriter, withDefault);
+
+    StringWriter withoutDefaultWriter = new StringWriter();
+    IdlUtils.writeIdlProtocol(withoutDefaultWriter, withoutDefault);
+
+    assertTrue(withDefaultWriter.toString().contains("} = ACTIVE;"),
+        "Enum with default should serialize default value");
+    assertFalse(withoutDefaultWriter.toString().contains("="),
+        "Enum without default should not contain '='");

Review Comment:
   The `assertFalse(...contains(\"=\"))` check is overly broad and can become flaky if `writeIdlProtocol` adds any unrelated `=` to the output (e.g., future annotations/metadata). Prefer asserting the absence of the specific enum-default pattern (e.g., `\"} =\"` / `\" = ACTIVE;\"`) or extracting just the enum declaration portion before checking.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org