THRIFT-5521: [java gen] use jdk8 option type in java generator code
Client: Java
Patch: Jiayu Liu
This closes #2525
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc
index efcd7d3..6f42675 100644
--- a/compiler/cpp/src/thrift/generate/t_java_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc
@@ -46,6 +46,9 @@
static const string endl = "\n"; // avoid ostream << std::endl flushes
+static const string thrift_option_class = "org.apache.thrift.Option";
+static const string jdk_option_class = "java.util.Optional";
+
/**
* Java code generator.
*
@@ -70,6 +73,7 @@
reuse_objects_ = false;
use_option_type_ = false;
generate_future_iface_ = false;
+ use_jdk8_option_type_ = false;
undated_generated_annotations_ = false;
suppress_generated_annotations_ = false;
rethrow_unhandled_exceptions_ = false;
@@ -101,6 +105,13 @@
reuse_objects_ = true;
} else if (iter->first.compare("option_type") == 0) {
use_option_type_ = true;
+ if (iter->second.compare("jdk8") == 0) {
+ use_jdk8_option_type_ = true;
+ } else if (iter->second.compare("thrift") == 0 || iter->second.compare("") == 0) {
+ use_jdk8_option_type_ = false;
+ } else {
+ throw "option_type must be 'jdk8' or 'thrift'";
+ }
} else if (iter->first.compare("rethrow_unhandled_exceptions") == 0) {
rethrow_unhandled_exceptions_ = true;
} else if (iter->first.compare("generated_annotations") == 0) {
@@ -423,6 +434,7 @@
bool reuse_objects_;
bool generate_future_iface_;
bool use_option_type_;
+ bool use_jdk8_option_type_;
bool undated_generated_annotations_;
bool suppress_generated_annotations_;
bool rethrow_unhandled_exceptions_;
@@ -1287,7 +1299,7 @@
indent_up();
indent(out) << "switch (setField) {" << endl;
indent_up();
-
+
const vector<t_field*>& members = tstruct->get_members();
vector<t_field*>::const_iterator m_iter;
@@ -2367,18 +2379,36 @@
if (is_deprecated) {
indent(out) << "@Deprecated" << endl;
}
- indent(out) << "public org.apache.thrift.Option<Integer> get" << cap_name;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "public " << jdk_option_class << "<Integer> get" << cap_name;
+ } else {
+ indent(out) << "public " << thrift_option_class << "<Integer> get" << cap_name;
+ }
+
out << get_cap_name("size() {") << endl;
indent_up();
indent(out) << "if (this." << field_name << " == null) {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.none();" << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".empty();" << endl;
+ } else {
+ indent(out) << "return " << thrift_option_class << ".none();" << endl;
+ }
+
indent_down();
indent(out) << "} else {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.some(this." << field_name << ".size());"
- << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".of(this.";
+ } else {
+ indent(out) << "return " << thrift_option_class << ".some(this.";
+ }
+ out << field_name << ".size());" << endl;
+
indent_down();
indent(out) << "}" << endl;
indent_down();
@@ -2411,19 +2441,38 @@
if (is_deprecated) {
indent(out) << "@Deprecated" << endl;
}
- indent(out) << "public org.apache.thrift.Option<java.util.Iterator<"
- << type_name(element_type, true, false) << ">> get" << cap_name;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "public " << jdk_option_class << "<";
+ } else {
+ indent(out) << "public " << thrift_option_class << "<";
+ }
+ out << "java.util.Iterator<" << type_name(element_type, true, false) << ">> get"
+ << cap_name;
+
out << get_cap_name("iterator() {") << endl;
indent_up();
indent(out) << "if (this." << field_name << " == null) {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.none();" << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".empty();" << endl;
+ } else {
+ indent(out) << "return " << thrift_option_class << ".none();" << endl;
+ }
+
indent_down();
indent(out) << "} else {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.some(this." << field_name << ".iterator());"
- << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".of(this.";
+ } else {
+ indent(out) << "return " << thrift_option_class << ".some(this.";
+ }
+ out << field_name << ".iterator());" << endl;
+
indent_down();
indent(out) << "}" << endl;
indent_down();
@@ -2521,7 +2570,13 @@
if (is_deprecated) {
indent(out) << "@Deprecated" << endl;
}
- indent(out) << "public org.apache.thrift.Option<" << type_name(type, true) << ">";
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "public " << jdk_option_class << "<" << type_name(type, true) << ">";
+ } else {
+ indent(out) << "public " << thrift_option_class << "<" << type_name(type, true) << ">";
+ }
+
if (type->is_base_type() && ((t_base_type*)type)->get_base() == t_base_type::TYPE_BOOL) {
out << " is";
} else {
@@ -2532,11 +2587,24 @@
indent(out) << "if (this.isSet" << cap_name << "()) {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.some(this." << field_name << ");" << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".of(this.";
+ } else {
+ indent(out) << "return " << thrift_option_class << ".some(this.";
+ }
+ out << field_name << ");" << endl;
+
indent_down();
indent(out) << "} else {" << endl;
indent_up();
- indent(out) << "return org.apache.thrift.Option.none();" << endl;
+
+ if (use_jdk8_option_type_) {
+ indent(out) << "return " << jdk_option_class << ".empty();" << endl;
+ } else {
+ indent(out) << "return " << thrift_option_class << ".none();" << endl;
+ }
+
indent_down();
indent(out) << "}" << endl;
indent_down();
@@ -5651,7 +5719,10 @@
" android: Generated structures are Parcelable.\n"
" android_legacy: Do not use java.io.IOException(throwable) (available for Android 2.3 and "
"above).\n"
- " option_type: Wrap optional fields in an Option type.\n"
+ " option_type=[thrift|jdk8]:\n"
+ " thrift: wrap optional fields in thrift Option type.\n"
+ " jdk8: Wrap optional fields in JDK8+ Option type.\n"
+ " If the Option type is not specified, 'thrift' is used.\n"
" rethrow_unhandled_exceptions:\n"
" Enable rethrow of unhandled exceptions and let them propagate further."
" (Default behavior is to catch and log it.)\n"
diff --git a/lib/java/gradle/functionalTests.gradle b/lib/java/gradle/functionalTests.gradle
index f00f74b..f197520 100644
--- a/lib/java/gradle/functionalTests.gradle
+++ b/lib/java/gradle/functionalTests.gradle
@@ -91,8 +91,8 @@
def javaExe = file("${System.getProperty('java.home')}/bin/java${execExt}").canonicalPath
// The common Uber jar path
def jarPath = shadowJar.archivePath.canonicalPath
-def trustStore = file('test/.truststore').canonicalPath
-def keyStore = file('test/.keystore').canonicalPath
+def trustStore = file('test/resources/.truststore').canonicalPath
+def keyStore = file('test/resources/.keystore').canonicalPath
task generateRunnerScriptForClient(group: 'Build') {
description = 'Generate a runner script for cross-check tests with TestClient'
diff --git a/lib/java/gradle/generateTestThrift.gradle b/lib/java/gradle/generateTestThrift.gradle
index d5bc3af..c48845e 100644
--- a/lib/java/gradle/generateTestThrift.gradle
+++ b/lib/java/gradle/generateTestThrift.gradle
@@ -24,11 +24,12 @@
ext.genBeanSrc = file("$buildDir/gen-javabean")
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")
// Add the generated code directories to the test source set
sourceSets {
- test.java.srcDirs genSrc, genBeanSrc, genReuseSrc, genFullCamelSrc, genUnsafeSrc
+ test.java.srcDirs genSrc, genBeanSrc, genReuseSrc, genFullCamelSrc, genUnsafeSrc, genOptionTypeJdk8Src
}
// ----------------------------------------------------------------------------
@@ -37,7 +38,10 @@
// A callable closure to make this easier
ext.thriftCompile = { Task task, String thriftFileName, String generator = 'java', File outputDir = genSrc ->
def thriftFile = file("$thriftRoot/test/$thriftFileName")
- assert thriftFile.exists()
+ if (!thriftFile.exists()) {
+ thriftFile = file("$projectDir/test/resources/$thriftFileName")
+ assert thriftFile.exists(), "can't find $thriftFile"
+ }
task.inputs.file thriftFile
task.outputs.dir outputDir
@@ -85,6 +89,15 @@
thriftCompile(it, 'partial/thrift_test_schema.thrift')
}
+task generateOptionalTypeJava(group: 'Build') {
+ description = 'Generate the thrift gen-option-type-jdk8 source'
+ generate.dependsOn it
+
+ ext.outputBuffer = new ByteArrayOutputStream()
+
+ thriftCompile(it, 'JavaOptionTypeJdk8Test.thrift', 'java:option_type=jdk8', genOptionTypeJdk8Src)
+}
+
task generateBeanJava(group: 'Build') {
description = 'Generate the thrift gen-javabean source'
generate.dependsOn it
diff --git a/lib/java/gradle/sourceConfiguration.gradle b/lib/java/gradle/sourceConfiguration.gradle
index ce0db75..c510a40 100644
--- a/lib/java/gradle/sourceConfiguration.gradle
+++ b/lib/java/gradle/sourceConfiguration.gradle
@@ -37,7 +37,7 @@
exclude '**/test/TestTServletServer.java'
}
resources {
- srcDir 'test'
+ srcDir 'test/resources'
include 'log4j.properties'
}
}
diff --git a/lib/java/gradle/unitTests.gradle b/lib/java/gradle/unitTests.gradle
index 61f2fbd..2bf1c03 100644
--- a/lib/java/gradle/unitTests.gradle
+++ b/lib/java/gradle/unitTests.gradle
@@ -74,9 +74,9 @@
systemProperties = [
'build.test': "${compileTestJava.destinationDir}",
'test.port': "${testPort}",
- 'javax.net.ssl.trustStore': "${projectDir}/test/.truststore",
+ 'javax.net.ssl.trustStore': "${projectDir}/test/resources/.truststore",
'javax.net.ssl.trustStorePassword': 'thrift',
- 'javax.net.ssl.keyStore': "${projectDir}/test/.keystore",
+ 'javax.net.ssl.keyStore': "${projectDir}/test/resources/.keystore",
'javax.net.ssl.keyStorePassword': 'thrift'
]
}
diff --git a/lib/java/test/org/apache/thrift/TestOptionType.java b/lib/java/test/org/apache/thrift/TestOptionType.java
index f70285f..ddde9d3 100644
--- a/lib/java/test/org/apache/thrift/TestOptionType.java
+++ b/lib/java/test/org/apache/thrift/TestOptionType.java
@@ -20,11 +20,10 @@
package org.apache.thrift;
import junit.framework.TestCase;
-import org.apache.thrift.Option;
// Tests and documents behavior for the "Option<T>" type
public class TestOptionType extends TestCase {
- public void testSome() throws Exception {
+ public void testSome() {
String name = "Chuck Norris";
Option<String> option = Option.fromNullable(name);
diff --git a/lib/java/test/org/apache/thrift/TestOptionals.java b/lib/java/test/org/apache/thrift/TestOptionals.java
index f3d4cfc..b34e06d 100644
--- a/lib/java/test/org/apache/thrift/TestOptionals.java
+++ b/lib/java/test/org/apache/thrift/TestOptionals.java
@@ -26,7 +26,7 @@
import thrift.test.Opt64;
import thrift.test.Opt80;
-// Exercises the isSet methods using structs from from ManyOptionals.thrift
+// Exercises the isSet methods using structs from ManyOptionals.thrift
public class TestOptionals extends TestCase {
public void testEncodingUtils() throws Exception {
assertEquals((short)0x8, EncodingUtils.setBit((short)0, 3, true));
diff --git a/lib/java/test/org/apache/thrift/TestOptionalsWithJdk8.java b/lib/java/test/org/apache/thrift/TestOptionalsWithJdk8.java
new file mode 100644
index 0000000..93daced
--- /dev/null
+++ b/lib/java/test/org/apache/thrift/TestOptionalsWithJdk8.java
@@ -0,0 +1,63 @@
+/*
+ * 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 junit.framework.TestCase;
+import thrift.test.optiontypejdk8.Person;
+
+// Tests and documents behavior for the JDK8 "Option<T>" type
+public class TestOptionalsWithJdk8 extends TestCase {
+
+ public void testConstruction() {
+ Person person = new Person(1L, "name");
+ assertFalse(person.getAge().isPresent());
+ assertFalse(person.isSetAge());
+ assertFalse(person.getPhone().isPresent());
+ assertFalse(person.isSetPhone());
+ assertEquals(1L, person.getId());
+ assertTrue(person.isSetId());
+ assertEquals("name", person.getName());
+ assertTrue(person.isSetName());
+
+ assertFalse(person.getAddresses().isPresent());
+ assertEquals(Integer.valueOf(0), person.getAddressesSize().orElse(0));
+ assertFalse(person.getPets().isPresent());
+ assertEquals(Integer.valueOf(0), person.getPetsSize().orElse(0));
+ }
+
+ public void testEmpty() {
+ Person person = new Person();
+ person.setPhone("phone");
+ assertFalse(person.getAge().isPresent());
+ assertFalse(person.isSetAge());
+ assertTrue(person.getPhone().isPresent());
+ assertEquals("phone", person.getPhone().get());
+ assertTrue(person.isSetPhone());
+ assertEquals(0L, person.getId());
+ assertFalse(person.isSetId());
+ assertNull(person.getName());
+ assertFalse(person.isSetName());
+
+ assertFalse(person.getAddresses().isPresent());
+ assertEquals(Integer.valueOf(0), person.getAddressesSize().orElse(0));
+ assertFalse(person.getPets().isPresent());
+ assertEquals(Integer.valueOf(0), person.getPetsSize().orElse(0));
+ }
+}
diff --git a/lib/java/test/.keystore b/lib/java/test/resources/.keystore
similarity index 100%
rename from lib/java/test/.keystore
rename to lib/java/test/resources/.keystore
Binary files differ
diff --git a/lib/java/test/.truststore b/lib/java/test/resources/.truststore
similarity index 100%
rename from lib/java/test/.truststore
rename to lib/java/test/resources/.truststore
Binary files differ
diff --git a/test/JavaBeansTest.thrift b/lib/java/test/resources/JavaBeansTest.thrift
similarity index 100%
rename from test/JavaBeansTest.thrift
rename to lib/java/test/resources/JavaBeansTest.thrift
diff --git a/test/JavaBinaryDefault.thrift b/lib/java/test/resources/JavaBinaryDefault.thrift
similarity index 100%
rename from test/JavaBinaryDefault.thrift
rename to lib/java/test/resources/JavaBinaryDefault.thrift
diff --git a/test/JavaDeepCopyTest.thrift b/lib/java/test/resources/JavaDeepCopyTest.thrift
similarity index 100%
rename from test/JavaDeepCopyTest.thrift
rename to lib/java/test/resources/JavaDeepCopyTest.thrift
diff --git a/lib/java/test/resources/JavaOptionTypeJdk8Test.thrift b/lib/java/test/resources/JavaOptionTypeJdk8Test.thrift
new file mode 100644
index 0000000..193e079
--- /dev/null
+++ b/lib/java/test/resources/JavaOptionTypeJdk8Test.thrift
@@ -0,0 +1,44 @@
+/*
+ * 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.
+ *
+ * Contains some contributions under the Thrift Software License.
+ * Please see doc/old-thrift-license.txt in the Thrift distribution for
+ * details.
+ */
+
+namespace java thrift.test.optiontypejdk8
+
+struct Person {
+ 1: required i64 id;
+ 2: required string name;
+ 3: optional i64 age;
+ 4: optional string phone;
+ 5: optional list<string> addresses;
+ 6: optional map<string, Pet> pets;
+}
+
+enum PetType {
+ Cat = 1
+ Dog = 2
+ Bunny = 3
+}
+
+struct Pet {
+ 1: required string name;
+ 2: optional PetType type;
+}
diff --git a/test/JavaTypes.thrift b/lib/java/test/resources/JavaTypes.thrift
similarity index 100%
rename from test/JavaTypes.thrift
rename to lib/java/test/resources/JavaTypes.thrift
diff --git a/lib/java/test/log4j.properties b/lib/java/test/resources/log4j.properties
similarity index 100%
rename from lib/java/test/log4j.properties
rename to lib/java/test/resources/log4j.properties
diff --git a/test/Makefile.am b/test/Makefile.am
index 58482f2..d428086 100755
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -142,10 +142,6 @@
Include.thrift \
Identifiers.thrift \
Int64Test.thrift \
- JavaBeansTest.thrift \
- JavaBinaryDefault.thrift \
- JavaDeepCopyTest.thrift \
- JavaTypes.thrift \
JsDeepConstructorTest.thrift \
ManyOptionals.thrift \
ManyTypedefs.thrift \