THRIFT-4086: Add missing calls to get_true_type when generating validator + metadata code
Client: java
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc
index 3ce1450..7dfd10f 100644
--- a/compiler/cpp/src/thrift/generate/t_java_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc
@@ -2192,7 +2192,7 @@
out << indent() << "// check for sub-struct validity" << endl;
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
- t_type* type = (*f_iter)->get_type();
+ t_type* type = get_true_type((*f_iter)->get_type());
if (type->is_struct() && !((t_struct*)type)->is_union()) {
out << indent() << "if (" << (*f_iter)->get_name() << " != null) {" << endl;
out << indent() << " " << (*f_iter)->get_name() << ".validate();" << endl;
@@ -2965,7 +2965,8 @@
out << endl;
indent_up();
indent_up();
- if (type->is_struct() || type->is_xception()) {
+ 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, "
diff --git a/lib/java/gradle/generateTestThrift.gradle b/lib/java/gradle/generateTestThrift.gradle
index e851547..0506aa6 100644
--- a/lib/java/gradle/generateTestThrift.gradle
+++ b/lib/java/gradle/generateTestThrift.gradle
@@ -26,6 +26,8 @@
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")
// Add the generated code directories to the test source set
sourceSets {
@@ -143,3 +145,13 @@
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'
+ generate.dependsOn it
+
+ ext.outputBuffer = new ByteArrayOutputStream()
+
+ thriftCompile(it, 'JavaStructOrderA.thrift', 'java', genStructOrderTestASrc)
+ thriftCompile(it, 'JavaStructOrderB.thrift', 'java', genStructOrderTestBSrc)
+}
diff --git a/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java b/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java
new file mode 100644
index 0000000..dd67d9b
--- /dev/null
+++ b/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.thrift;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.*;
+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
+// identical code
+public class TestStructOrder {
+
+ @Test
+ public void testStructOrder() throws Exception {
+ List<String> filenames = Arrays.asList("Parent.java", "Child.java");
+
+ for (String fn : filenames) {
+ String fnA = "struct-order-test/a/" + fn;
+ String fnB = "struct-order-test/b/" + fn;
+
+ try (InputStream isA = TestStructOrder.class.getClassLoader().getResourceAsStream(fnA);
+ InputStream isB = TestStructOrder.class.getClassLoader().getResourceAsStream(fnB)) {
+
+ assertNotNull(isA, "Resource not found: " + fnA);
+ assertNotNull(isB, "Resource not found: " + fnB);
+
+ int hashA = Arrays.hashCode(readAllBytes(isA));
+ assertEquals(
+ hashA,
+ Arrays.hashCode(readAllBytes(isB)),
+ String.format("Generated Java files %s and %s differ", fnA, fnB));
+ }
+ }
+ }
+
+ // TODO Use InputStream.readAllBytes post-Java8
+ private byte[] readAllBytes(InputStream is) throws IOException {
+ ByteArrayOutputStream os = new ByteArrayOutputStream();
+ byte[] buff = new byte[1024];
+ int bytesRead;
+ while ((bytesRead = is.read(buff, 0, buff.length)) != -1) {
+ os.write(buff, 0, bytesRead);
+ }
+ return os.toByteArray();
+ }
+}
diff --git a/lib/java/src/test/resources/JavaStructOrderA.thrift b/lib/java/src/test/resources/JavaStructOrderA.thrift
new file mode 100644
index 0000000..bac5209
--- /dev/null
+++ b/lib/java/src/test/resources/JavaStructOrderA.thrift
@@ -0,0 +1,9 @@
+// Define Parent, then Child. No forward declarations.
+struct Parent {
+ 1: required string Name
+}
+
+struct Child {
+ 1: required string Name
+ 2: required Parent Parent
+}
diff --git a/lib/java/src/test/resources/JavaStructOrderB.thrift b/lib/java/src/test/resources/JavaStructOrderB.thrift
new file mode 100644
index 0000000..aa2afea
--- /dev/null
+++ b/lib/java/src/test/resources/JavaStructOrderB.thrift
@@ -0,0 +1,10 @@
+// Define Child, then Parent. Parent is a forward declaration and was problematic for our Java compiler before
+// 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
+}
+
+struct Parent {
+ 1: required string Name
+}