From 2ad42c1136a1986f5640c4f41092bcba892da272 Mon Sep 17 00:00:00 2001 From: Christophe Le Saec <51320496+clesaec@users.noreply.github.com> Date: Fri, 18 Aug 2023 14:57:40 +0200 Subject: [PATCH] AVRO-3749: avoid conflict in method name for generated code (#2435) --- .../compiler/specific/SpecificCompiler.java | 68 +++++++++++++++++-- .../specific/TestSpecificCompiler.java | 55 +++++++++++++++ .../specific/TestSpecificCompiler.java | 51 +++++++------- 3 files changed, 141 insertions(+), 33 deletions(-) diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java index e98a50b79a0..4e77fbef24c 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java @@ -1264,10 +1264,7 @@ private static String generateMethodName(Schema schema, Field field, String pref // Check for the special case in which the schema defines two fields whose // names are identical except for the case of the first character: - char firstChar = field.name().charAt(0); - String conflictingFieldName = (Character.isLowerCase(firstChar) ? Character.toUpperCase(firstChar) - : Character.toLowerCase(firstChar)) + (field.name().length() > 1 ? field.name().substring(1) : ""); - boolean fieldNameConflict = schema.getField(conflictingFieldName) != null; + int indexNameConflict = calcNameIndex(field.name(), schema); StringBuilder methodBuilder = new StringBuilder(prefix); String fieldName = mangle(field.name(), schema.isError() ? ERROR_RESERVED_WORDS : ACCESSOR_MUTATOR_RESERVED_WORDS, @@ -1287,16 +1284,75 @@ private static String generateMethodName(Schema schema, Field field, String pref methodBuilder.append(postfix); // If there is a field name conflict append $0 or $1 - if (fieldNameConflict) { + if (indexNameConflict >= 0) { if (methodBuilder.charAt(methodBuilder.length() - 1) != '$') { methodBuilder.append('$'); } - methodBuilder.append(Character.isLowerCase(firstChar) ? '0' : '1'); + methodBuilder.append(indexNameConflict); } return methodBuilder.toString(); } + /** + * Calc name index for getter / setter field in case of conflict as example, + * having a schema with fields __X, _X, _x, X, x should result with indexes __X: + * 3, _X: 2, _x: 1, X: 0 x: None (-1) + * + * @param fieldName : field name. + * @param schema : schema. + * @return index for field. + */ + private static int calcNameIndex(String fieldName, Schema schema) { + // get name without underscore at start + // and calc number of other similar fields with same subname. + int countSimilar = 0; + String pureFieldName = fieldName; + while (!pureFieldName.isEmpty() && pureFieldName.charAt(0) == '_') { + pureFieldName = pureFieldName.substring(1); + if (schema.getField(pureFieldName) != null) { + countSimilar++; + } + String reversed = reverseFirstLetter(pureFieldName); + if (schema.getField(reversed) != null) { + countSimilar++; + } + } + // field name start with upper have +1 + String reversed = reverseFirstLetter(fieldName); + if (!pureFieldName.isEmpty() && Character.isUpperCase(pureFieldName.charAt(0)) + && schema.getField(reversed) != null) { + countSimilar++; + } + + int ret = -1; // if no similar name, no index. + if (countSimilar > 0) { + ret = countSimilar - 1; // index is count similar -1 (start with $0) + } + + return ret; + } + + /** + * Reverse first letter upper <=> lower. __Name <=> __name + * + * @param name : input name. + * @return name with change case of first letter. + */ + private static String reverseFirstLetter(String name) { + StringBuilder builder = new StringBuilder(name); + int index = 0; + while (builder.length() > index && builder.charAt(index) == '_') { + index++; + } + if (builder.length() > index) { + char c = builder.charAt(index); + char inverseC = Character.isLowerCase(c) ? Character.toUpperCase(c) : Character.toLowerCase(c); + builder.setCharAt(index, inverseC); + } + return builder.toString(); + } + /** * Tests whether an unboxed Java type can be set to null */ diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java index 05bc7b2f531..cc3fcd31240 100644 --- a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java +++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java @@ -929,4 +929,59 @@ public LogicalType fromSchema(Schema schema) { } } + @Test + void fieldWithUnderscore_avro3826() { + String jsonSchema = "{\n" + " \"name\": \"Value\",\n" + " \"type\": \"record\",\n" + " \"fields\": [\n" + + " { \"name\": \"__deleted\", \"type\": \"string\"\n" + " }\n" + " ]\n" + "}"; + Collection outputs = new SpecificCompiler(new Schema.Parser().parse(jsonSchema)) + .compile(); + assertEquals(1, outputs.size()); + SpecificCompiler.OutputFile outputFile = outputs.iterator().next(); + assertTrue(outputFile.contents.contains("getDeleted()")); + assertFalse(outputFile.contents.contains("$0")); + assertFalse(outputFile.contents.contains("$1")); + + String jsonSchema2 = "{\n" + " \"name\": \"Value\", \"type\": \"record\",\n" + " \"fields\": [\n" + + " { \"name\": \"__deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"_deleted\", \"type\": \"string\"}\n" + " ]\n" + "}"; + Collection outputs2 = new SpecificCompiler(new Schema.Parser().parse(jsonSchema2)) + .compile(); + assertEquals(1, outputs2.size()); + SpecificCompiler.OutputFile outputFile2 = outputs2.iterator().next(); + + assertTrue(outputFile2.contents.contains("getDeleted()")); + assertTrue(outputFile2.contents.contains("getDeleted$0()")); + assertFalse(outputFile.contents.contains("$1")); + + String jsonSchema3 = "{\n" + " \"name\": \"Value\", \"type\": \"record\",\n" + " \"fields\": [\n" + + " { \"name\": \"__deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"_deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"deleted\", \"type\": \"string\"}\n" + " ]\n" + "}"; + Collection outputs3 = new SpecificCompiler(new Schema.Parser().parse(jsonSchema3)) + .compile(); + assertEquals(1, outputs3.size()); + SpecificCompiler.OutputFile outputFile3 = outputs3.iterator().next(); + + assertTrue(outputFile3.contents.contains("getDeleted()")); + assertTrue(outputFile3.contents.contains("getDeleted$0()")); + assertTrue(outputFile3.contents.contains("getDeleted$1()")); + assertFalse(outputFile3.contents.contains("$2")); + + String jsonSchema4 = "{\n" + " \"name\": \"Value\", \"type\": \"record\",\n" + " \"fields\": [\n" + + " { \"name\": \"__deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"_deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"deleted\", \"type\": \"string\"},\n" + + " { \"name\": \"Deleted\", \"type\": \"string\"}\n" + " ]\n" + "}"; + Collection outputs4 = new SpecificCompiler(new Schema.Parser().parse(jsonSchema4)) + .compile(); + assertEquals(1, outputs4.size()); + SpecificCompiler.OutputFile outputFile4 = outputs4.iterator().next(); + + assertTrue(outputFile4.contents.contains("getDeleted()")); + assertTrue(outputFile4.contents.contains("getDeleted$0()")); + assertTrue(outputFile4.contents.contains("getDeleted$1()")); + assertTrue(outputFile4.contents.contains("getDeleted$2()")); + assertFalse(outputFile4.contents.contains("$3")); + } + } diff --git a/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java b/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java index bb249dd8e1c..0af06b9a2b1 100644 --- a/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java +++ b/lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java @@ -300,12 +300,11 @@ void generateGetMethod() { height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("getHeight$0", - SpecificCompiler.generateGetMethod(createRecord("test", false, height, Height), height)); + assertEquals("getHeight", SpecificCompiler.generateGetMethod(createRecord("test", false, height, Height), height)); height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("getHeight$1", + assertEquals("getHeight$0", SpecificCompiler.generateGetMethod(createRecord("test", false, height, Height), Height)); message = new Field("message", Schema.create(Type.STRING), null, null); @@ -314,12 +313,12 @@ void generateGetMethod() { message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("getMessage$0", + assertEquals("getMessage$", SpecificCompiler.generateGetMethod(createRecord("test", true, message, Message), message)); message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("getMessage$1", + assertEquals("getMessage$0", SpecificCompiler.generateGetMethod(createRecord("test", true, message, Message), Message)); schema = new Field("schema", Schema.create(Type.STRING), null, null); @@ -328,12 +327,12 @@ void generateGetMethod() { schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("getSchema$0", + assertEquals("getSchema$", SpecificCompiler.generateGetMethod(createRecord("test", false, schema, Schema$), schema)); schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("getSchema$1", + assertEquals("getSchema$0", SpecificCompiler.generateGetMethod(createRecord("test", false, schema, Schema$), Schema$)); } @@ -376,12 +375,11 @@ void generateSetMethod() { height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("setHeight$0", - SpecificCompiler.generateSetMethod(createRecord("test", false, height, Height), height)); + assertEquals("setHeight", SpecificCompiler.generateSetMethod(createRecord("test", false, height, Height), height)); height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("setHeight$1", + assertEquals("setHeight$0", SpecificCompiler.generateSetMethod(createRecord("test", false, height, Height), Height)); message = new Field("message", Schema.create(Type.STRING), null, null); @@ -390,12 +388,12 @@ void generateSetMethod() { message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("setMessage$0", + assertEquals("setMessage$", SpecificCompiler.generateSetMethod(createRecord("test", true, message, Message), message)); message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("setMessage$1", + assertEquals("setMessage$0", SpecificCompiler.generateSetMethod(createRecord("test", true, message, Message), Message)); schema = new Field("schema", Schema.create(Type.STRING), null, null); @@ -404,12 +402,12 @@ void generateSetMethod() { schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("setSchema$0", + assertEquals("setSchema$", SpecificCompiler.generateSetMethod(createRecord("test", false, schema, Schema$), schema)); schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("setSchema$1", + assertEquals("setSchema$0", SpecificCompiler.generateSetMethod(createRecord("test", false, schema, Schema$), Schema$)); } @@ -452,12 +450,11 @@ void generateHasMethod() { height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("hasHeight$0", - SpecificCompiler.generateHasMethod(createRecord("test", false, height, Height), height)); + assertEquals("hasHeight", SpecificCompiler.generateHasMethod(createRecord("test", false, height, Height), height)); height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("hasHeight$1", + assertEquals("hasHeight$0", SpecificCompiler.generateHasMethod(createRecord("test", false, height, Height), Height)); message = new Field("message", Schema.create(Type.STRING), null, null); @@ -466,12 +463,12 @@ void generateHasMethod() { message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("hasMessage$0", + assertEquals("hasMessage$", SpecificCompiler.generateHasMethod(createRecord("test", true, message, Message), message)); message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("hasMessage$1", + assertEquals("hasMessage$0", SpecificCompiler.generateHasMethod(createRecord("test", true, message, Message), Message)); schema = new Field("schema", Schema.create(Type.STRING), null, null); @@ -480,12 +477,12 @@ void generateHasMethod() { schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("hasSchema$0", + assertEquals("hasSchema$", SpecificCompiler.generateHasMethod(createRecord("test", false, schema, Schema$), schema)); schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("hasSchema$1", + assertEquals("hasSchema$0", SpecificCompiler.generateHasMethod(createRecord("test", false, schema, Schema$), Schema$)); } @@ -528,12 +525,12 @@ void generateClearMethod() { height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("clearHeight$0", + assertEquals("clearHeight", SpecificCompiler.generateClearMethod(createRecord("test", false, height, Height), height)); height = new Field("height", Schema.create(Type.INT), null, null); Height = new Field("Height", Schema.create(Type.INT), null, null); - assertEquals("clearHeight$1", + assertEquals("clearHeight$0", SpecificCompiler.generateClearMethod(createRecord("test", false, height, Height), Height)); message = new Field("message", Schema.create(Type.STRING), null, null); @@ -542,12 +539,12 @@ void generateClearMethod() { message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("clearMessage$0", + assertEquals("clearMessage$", SpecificCompiler.generateClearMethod(createRecord("test", true, message, Message), message)); message = new Field("message", Schema.create(Type.STRING), null, null); Message = new Field("Message", Schema.create(Type.STRING), null, null); - assertEquals("clearMessage$1", + assertEquals("clearMessage$0", SpecificCompiler.generateClearMethod(createRecord("test", true, message, Message), Message)); schema = new Field("schema", Schema.create(Type.STRING), null, null); @@ -556,12 +553,12 @@ void generateClearMethod() { schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("clearSchema$0", + assertEquals("clearSchema$", SpecificCompiler.generateClearMethod(createRecord("test", false, schema, Schema$), schema)); schema = new Field("schema", Schema.create(Type.STRING), null, null); Schema$ = new Field("Schema", Schema.create(Type.STRING), null, null); - assertEquals("clearSchema$1", + assertEquals("clearSchema$0", SpecificCompiler.generateClearMethod(createRecord("test", false, schema, Schema$), Schema$)); }