Skip to content

Commit

Permalink
AVRO-3749: avoid conflict in method name for generated code (#2435)
Browse files Browse the repository at this point in the history
  • Loading branch information
@clesaec
clesaec authored Aug 18, 2023
1 parent 1711661 commit 2ad42c1
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 33 deletions.
68 changes: 62 additions & 6 deletions lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
*/
Expand Down
55 changes: 55 additions & 0 deletions lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SpecificCompiler.OutputFile> 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<SpecificCompiler.OutputFile> 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<SpecificCompiler.OutputFile> 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<SpecificCompiler.OutputFile> 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"));
}

}
51 changes: 24 additions & 27 deletions lang/java/ipc/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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$));
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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$));
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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$));
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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$));
}

Expand Down

0 comments on commit 2ad42c1

Please sign in to comment.