diff options
author | Klemen Košir <klemen.kosir@kream.io> | 2023-04-27 15:13:18 +0900 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-27 14:13:18 +0800 |
commit | 4f63573f5a49fb564e7b65b9573769963511dbea (patch) | |
tree | a6512beee4a22514acacef76bfb9938d61afadf5 | |
parent | a4156083c397af7f0539d9bd1327054dc839985b (diff) | |
download | thrift-4f63573f5a49fb564e7b65b9573769963511dbea.tar.gz |
THRIFT-4086: Use true type when generating field meta data (#2765)
Client: java
-rw-r--r-- | compiler/cpp/src/thrift/generate/t_java_generator.cc | 30 | ||||
-rw-r--r-- | compiler/cpp/src/thrift/generate/t_oop_generator.h | 2 | ||||
-rw-r--r-- | lib/java/gradle/generateTestThrift.gradle | 12 | ||||
-rw-r--r-- | lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java (renamed from lib/java/src/test/java/org/apache/thrift/TestStructOrder.java) | 16 | ||||
-rw-r--r-- | lib/java/src/test/resources/JavaDefinitionOrderA.thrift (renamed from lib/java/src/test/resources/JavaStructOrderA.thrift) | 24 | ||||
-rw-r--r-- | lib/java/src/test/resources/JavaDefinitionOrderB.thrift (renamed from lib/java/src/test/resources/JavaStructOrderB.thrift) | 23 |
6 files changed, 75 insertions, 32 deletions
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc index ee98600f2..4259cf8d7 100644 --- a/compiler/cpp/src/thrift/generate/t_java_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc @@ -3023,46 +3023,46 @@ void t_java_generator::generate_metadata_for_field_annotations(std::ostream& out } void t_java_generator::generate_field_value_meta_data(std::ostream& out, t_type* type) { + t_type* ttype = get_true_type(type); out << endl; indent_up(); indent_up(); - t_type* ttype = get_true_type(type); if (ttype->is_struct() || ttype->is_xception()) { indent(out) << "new " "org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType." "STRUCT, " - << type_name(type) << ".class"; - } else if (type->is_container()) { - if (type->is_list()) { + << type_name(ttype) << ".class"; + } else if (ttype->is_container()) { + if (ttype->is_list()) { indent(out) << "new org.apache.thrift.meta_data.ListMetaData(org.apache.thrift.protocol.TType.LIST, "; - t_type* elem_type = ((t_list*)type)->get_elem_type(); + t_type* elem_type = ((t_list*)ttype)->get_elem_type(); generate_field_value_meta_data(out, elem_type); - } else if (type->is_set()) { + } else if (ttype->is_set()) { indent(out) << "new org.apache.thrift.meta_data.SetMetaData(org.apache.thrift.protocol.TType.SET, "; - t_type* elem_type = ((t_set*)type)->get_elem_type(); + t_type* elem_type = ((t_set*)ttype)->get_elem_type(); generate_field_value_meta_data(out, elem_type); } else { // map indent(out) << "new org.apache.thrift.meta_data.MapMetaData(org.apache.thrift.protocol.TType.MAP, "; - t_type* key_type = ((t_map*)type)->get_key_type(); - t_type* val_type = ((t_map*)type)->get_val_type(); + t_type* key_type = ((t_map*)ttype)->get_key_type(); + t_type* val_type = ((t_map*)ttype)->get_val_type(); generate_field_value_meta_data(out, key_type); out << ", "; generate_field_value_meta_data(out, val_type); } - } else if (type->is_enum()) { + } else if (ttype->is_enum()) { indent(out) << "new org.apache.thrift.meta_data.EnumMetaData(org.apache.thrift.protocol.TType.ENUM, " - << type_name(type) << ".class"; + << type_name(ttype) << ".class"; } else { indent(out) << "new org.apache.thrift.meta_data.FieldValueMetaData(" - << get_java_type_string(type); - if (type->is_typedef()) { - indent(out) << ", \"" << ((t_typedef*)type)->get_symbolic() << "\""; - } else if (type->is_binary()) { + << get_java_type_string(ttype); + if (ttype->is_binary()) { indent(out) << ", true"; + } else if (type->is_typedef()) { + indent(out) << ", \"" << ((t_typedef*)type)->get_symbolic() << "\""; } } out << ")"; diff --git a/compiler/cpp/src/thrift/generate/t_oop_generator.h b/compiler/cpp/src/thrift/generate/t_oop_generator.h index 2df1be413..884196203 100644 --- a/compiler/cpp/src/thrift/generate/t_oop_generator.h +++ b/compiler/cpp/src/thrift/generate/t_oop_generator.h @@ -70,7 +70,7 @@ public: } virtual void generate_java_doc(std::ostream& out, t_field* field) { - if (field->get_type()->is_enum()) { + if (get_true_type(field->get_type())->is_enum()) { std::string combined_message = field->get_doc() + "\n@see " + get_enum_class_name(field->get_type()); generate_java_docstring_comment(out, combined_message); diff --git a/lib/java/gradle/generateTestThrift.gradle b/lib/java/gradle/generateTestThrift.gradle index 301812fcc..10bb16bba 100644 --- a/lib/java/gradle/generateTestThrift.gradle +++ b/lib/java/gradle/generateTestThrift.gradle @@ -26,8 +26,8 @@ ext.genReuseSrc = file("$buildDir/gen-javareuse") ext.genFullCamelSrc = file("$buildDir/gen-fullcamel") ext.genOptionTypeJdk8Src = file("$buildDir/gen-option-type-jdk8") ext.genUnsafeSrc = file("$buildDir/gen-unsafe") -ext.genStructOrderTestASrc = file("$buildDir/resources/test/struct-order-test/a") -ext.genStructOrderTestBSrc = file("$buildDir/resources/test/struct-order-test/b") +ext.genDefinitionOrderTestASrc = file("$buildDir/resources/test/definition-order-test/a") +ext.genDefinitionOrderTestBSrc = file("$buildDir/resources/test/definition-order-test/b") // Add the generated code directories to the test source set sourceSets { @@ -146,12 +146,12 @@ task generateWithAnnotationMetadata(group: 'Build') { thriftCompile(it, 'JavaAnnotationTest.thrift', 'java:annotations_as_metadata', genSrc) } -task generateJavaStructOrderTestJava(group: 'Build') { - description = 'Generate structs defined in different order and add to build dir so we can compare them' +task generateJavaDefinitionOrderTestJava(group: 'Build') { + description = 'Generate fields defined in different order and add to build dir so we can compare them' generate.dependsOn it ext.outputBuffer = new ByteArrayOutputStream() - thriftCompile(it, 'JavaStructOrderA.thrift', 'java', genStructOrderTestASrc) - thriftCompile(it, 'JavaStructOrderB.thrift', 'java', genStructOrderTestBSrc) + thriftCompile(it, 'JavaDefinitionOrderA.thrift', 'java', genDefinitionOrderTestASrc) + thriftCompile(it, 'JavaDefinitionOrderB.thrift', 'java', genDefinitionOrderTestBSrc) } diff --git a/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java b/lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java index dd67d9b5d..51021b791 100644 --- a/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java +++ b/lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java @@ -26,20 +26,20 @@ import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.Test; -// Tests that declaring structs in different order (esp. when they reference each other) generates +// Tests that declaring fields in different order (esp. when they reference each other) generates // identical code -public class TestStructOrder { +public class TestDefinitionOrder { @Test - public void testStructOrder() throws Exception { - List<String> filenames = Arrays.asList("Parent.java", "Child.java"); + public void testDefinitionOrder() throws Exception { + List<String> filenames = Arrays.asList("Parent.java", "Child.java", "MyEnum.java"); for (String fn : filenames) { - String fnA = "struct-order-test/a/" + fn; - String fnB = "struct-order-test/b/" + fn; + String fnA = "definition-order-test/a/" + fn; + String fnB = "definition-order-test/b/" + fn; - try (InputStream isA = TestStructOrder.class.getClassLoader().getResourceAsStream(fnA); - InputStream isB = TestStructOrder.class.getClassLoader().getResourceAsStream(fnB)) { + try (InputStream isA = TestDefinitionOrder.class.getClassLoader().getResourceAsStream(fnA); + InputStream isB = TestDefinitionOrder.class.getClassLoader().getResourceAsStream(fnB)) { assertNotNull(isA, "Resource not found: " + fnA); assertNotNull(isB, "Resource not found: " + fnB); diff --git a/lib/java/src/test/resources/JavaStructOrderA.thrift b/lib/java/src/test/resources/JavaDefinitionOrderA.thrift index f00a7106d..9b4296443 100644 --- a/lib/java/src/test/resources/JavaStructOrderA.thrift +++ b/lib/java/src/test/resources/JavaDefinitionOrderA.thrift @@ -22,7 +22,29 @@ struct Parent { 1: required string Name } +typedef Parent MyParent +typedef list<Parent> Parents + +enum MyEnum { + FOO = 1 + BAR = 2 +} + +typedef i8 Age +typedef MyEnum MyEnumV2 +typedef set<MyEnum> MyEnums +typedef map<MyEnumV2, Parent> MyMapping +typedef binary MyBinary + struct Child { 1: required string Name - 2: required Parent Parent + 2: required Age Age + 3: required Parent Parent1 + 4: required MyParent Parent2 + 5: required Parents GrandParents + 6: required MyEnum MyEnum + 7: required MyEnumV2 MyEnumV2 + 8: required MyEnums MyEnums + 9: required MyMapping MyMapping + 10: required MyBinary MyBinary } diff --git a/lib/java/src/test/resources/JavaStructOrderB.thrift b/lib/java/src/test/resources/JavaDefinitionOrderB.thrift index 4682668b3..0cccd9705 100644 --- a/lib/java/src/test/resources/JavaStructOrderB.thrift +++ b/lib/java/src/test/resources/JavaDefinitionOrderB.thrift @@ -21,9 +21,30 @@ // fixing THRIFT-4086: Java compiler generates different meta data depending on order of structures in file struct Child { 1: required string Name - 2: required Parent Parent + 2: required Age Age + 3: required Parent Parent1 + 4: required MyParent Parent2 + 5: required Parents GrandParents + 6: required MyEnum MyEnum + 7: required MyEnumV2 MyEnumV2 + 8: required MyEnums MyEnums + 9: required MyMapping MyMapping + 10: required MyBinary MyBinary } +typedef i8 Age +typedef Parent MyParent +typedef list<Parent> Parents +typedef MyEnum MyEnumV2 +typedef set<MyEnum> MyEnums +typedef map<MyEnumV2, Parent> MyMapping +typedef binary MyBinary + struct Parent { 1: required string Name } + +enum MyEnum { + FOO = 1 + BAR = 2 +} |