Repository: thrift Updated Branches: refs/heads/master 0cfdf7cb9 -> e6789480d
THRIFT-1909 Java: Add compiler flag to use the "option pattern" for optional fields Patch: Eirik Sletteberg & rebase by Wouter Lammers Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/e6789480 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/e6789480 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/e6789480 Branch: refs/heads/master Commit: e6789480d262357d4de4ab608267165c79631cb3 Parents: 0cfdf7c Author: Roger Meier <[email protected]> Authored: Mon Mar 23 20:41:15 2015 +0100 Committer: Roger Meier <[email protected]> Committed: Mon Mar 23 20:41:15 2015 +0100 ---------------------------------------------------------------------- compiler/cpp/src/generate/t_java_generator.cc | 137 ++++++++++++++----- lib/java/src/org/apache/thrift/Option.java | 121 ++++++++++++++++ .../test/org/apache/thrift/TestOptionType.java | 66 +++++++++ 3 files changed, 289 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/e6789480/compiler/cpp/src/generate/t_java_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc index df3d1cf..e8c70ad 100644 --- a/compiler/cpp/src/generate/t_java_generator.cc +++ b/compiler/cpp/src/generate/t_java_generator.cc @@ -85,6 +85,9 @@ public: iter = parsed_options.find("reuse-objects"); reuse_objects_ = (iter != parsed_options.end()); + iter = parsed_options.find("option_type"); + use_option_type_ = (iter != parsed_options.end()); + out_dir_base_ = (bean_style_ ? "gen-javabean" : "gen-java"); } @@ -336,6 +339,7 @@ private: bool java5_; bool sorted_containers_; bool reuse_objects_; + bool use_option_type_; }; /** @@ -385,17 +389,25 @@ string t_java_generator::java_package() { string t_java_generator::java_type_imports() { string hash_builder; string tree_set_and_map; + string option; if (sorted_containers_) { tree_set_and_map = string() + "import java.util.TreeSet;\n" + "import java.util.TreeMap;\n"; } + if (use_option_type_) { + option = string() + + "import org.apache.thrift.Option;\n"; + } + return string() + hash_builder + "import org.apache.thrift.scheme.IScheme;\n" + "import org.apache.thrift.scheme.SchemeFactory;\n" + "import org.apache.thrift.scheme.StandardScheme;\n\n" + "import org.apache.thrift.scheme.TupleScheme;\n" + "import org.apache.thrift.protocol.TTupleProtocol;\n" + "import org.apache.thrift.protocol.TProtocolException;\n" - + "import org.apache.thrift.EncodingUtils;\n" + "import org.apache.thrift.TException;\n" + + "import org.apache.thrift.EncodingUtils;\n" + + option + + "import org.apache.thrift.TException;\n" + "import org.apache.thrift.async.AsyncMethodCallback;\n" + "import org.apache.thrift.server.AbstractNonblockingServer.*;\n" + "import java.util.List;\n" + "import java.util.ArrayList;\n" + "import java.util.Map;\n" @@ -2017,16 +2029,7 @@ void t_java_generator::generate_reflection_getters(ostringstream& out, string cap_name) { indent(out) << "case " << constant_name(field_name) << ":" << endl; indent_up(); - - if (type->is_base_type() && !type->is_string()) { - t_base_type* base_type = (t_base_type*)type; - - indent(out) << "return " << type_name(type, true, false) << ".valueOf(" - << (base_type->is_bool() ? "is" : "get") << cap_name << "());" << endl << endl; - } else { - indent(out) << "return get" << cap_name << "();" << endl << endl; - } - + indent(out) << "return " << (type->is_bool() ? "is" : "get") << cap_name << "();" << endl << endl; indent_down(); } @@ -2130,17 +2133,36 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, t_struct* t t_type* type = get_true_type(field->get_type()); std::string field_name = field->get_name(); std::string cap_name = get_cap_name(field_name); + bool optional = use_option_type_ && field->get_req() == t_field::e_req::T_OPTIONAL; if (type->is_container()) { // Method to return the size of the collection - indent(out) << "public int get" << cap_name; - out << get_cap_name("size() {") << endl; + if (optional) { + indent(out) << "public Option<Integer> get" << cap_name; + out << get_cap_name("size() {") << endl; - indent_up(); - indent(out) << "return (this." << field_name << " == null) ? 0 : " - << "this." << field_name << ".size();" << endl; - indent_down(); - indent(out) << "}" << endl << endl; + indent_up(); + indent(out) << "if (this." << field_name << " == null) {" << endl; + indent_up(); + indent(out) << "return Option.none();" << endl; + indent_down(); + indent(out) << "} else {" << endl; + indent_up(); + indent(out) << "return Option.some(this." << field_name << ".size());" << endl; + indent_down(); + indent(out) << "}" << endl; + indent_down(); + indent(out) << "}" << endl << endl; + } else { + indent(out) << "public int get" << cap_name; + out << get_cap_name("size() {") << endl; + + indent_up(); + indent(out) << "return (this." << field_name << " == null) ? 0 : " << + "this." << field_name << ".size();" << endl; + indent_down(); + indent(out) << "}" << endl << endl; + } } if (type->is_set() || type->is_list()) { @@ -2152,15 +2174,34 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, t_struct* t } // Iterator getter for sets and lists - indent(out) << "public java.util.Iterator<" << type_name(element_type, true, false) << "> get" - << cap_name; - out << get_cap_name("iterator() {") << endl; + if (optional) { + indent(out) << "public Option<java.util.Iterator<" << + type_name(element_type, true, false) << ">> get" << cap_name; + out << get_cap_name("iterator() {") << endl; - indent_up(); - indent(out) << "return (this." << field_name << " == null) ? null : " - << "this." << field_name << ".iterator();" << endl; - indent_down(); - indent(out) << "}" << endl << endl; + indent_up(); + indent(out) << "if (this." << field_name << " == null) {" << endl; + indent_up(); + indent(out) << "return Option.none();" << endl; + indent_down(); + indent(out) << "} else {" << endl; + indent_up(); + indent(out) << "return Option.some(this." << field_name << ".iterator());" << endl; + indent_down(); + indent(out) << "}" << endl; + indent_down(); + indent(out) << "}" << endl << endl; + } else { + indent(out) << "public java.util.Iterator<" << + type_name(element_type, true, false) << "> get" << cap_name; + out << get_cap_name("iterator() {") << endl; + + indent_up(); + indent(out) << "return (this." << field_name << " == null) ? null : " << + "this." << field_name << ".iterator();" << endl; + indent_down(); + indent(out) << "}" << endl << endl; + } // Add to set or list, create if the set/list is null indent(out); @@ -2215,17 +2256,42 @@ void t_java_generator::generate_java_bean_boilerplate(ofstream& out, t_struct* t << endl; indent(out) << "}" << endl << endl; } else { - indent(out) << "public " << type_name(type); - if (type->is_base_type() && ((t_base_type*)type)->get_base() == t_base_type::TYPE_BOOL) { - out << " is"; + if (optional) { + indent(out) << "public Option<" << type_name(type, true) << ">"; + if (type->is_base_type() && + ((t_base_type*)type)->get_base() == t_base_type::TYPE_BOOL) { + out << " is"; + } else { + out << " get"; + } + out << cap_name << "() {" << endl; + indent_up(); + + indent(out) << "if (this.isSet" << cap_name << "()) {" << endl; + indent_up(); + indent(out) << "return Option.some(this." << field_name << ");" << endl; + indent_down(); + indent(out) << "} else {" << endl; + indent_up(); + indent(out) << "return Option.none();" << endl; + indent_down(); + indent(out) << "}" << endl; + indent_down(); + indent(out) << "}" << endl << endl; } else { - out << " get"; + indent(out) << "public " << type_name(type); + if (type->is_base_type() && + ((t_base_type*)type)->get_base() == t_base_type::TYPE_BOOL) { + out << " is"; + } else { + out << " get"; + } + out << cap_name << "() {" << endl; + indent_up(); + indent(out) << "return this." << field_name << ";" << endl; + indent_down(); + indent(out) << "}" << endl << endl; } - out << cap_name << "() {" << endl; - indent_up(); - indent(out) << "return this." << field_name << ";" << endl; - indent_down(); - indent(out) << "}" << endl << endl; } // Simple setter @@ -5029,6 +5095,7 @@ THRIFT_REGISTER_GENERATOR( " 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" " java5: Generate Java 1.5 compliant code (includes android_legacy flag).\n" " reuse-objects: Data objects will not be allocated, but existing instances will be used " "(read and write).\n" http://git-wip-us.apache.org/repos/asf/thrift/blob/e6789480/lib/java/src/org/apache/thrift/Option.java ---------------------------------------------------------------------- diff --git a/lib/java/src/org/apache/thrift/Option.java b/lib/java/src/org/apache/thrift/Option.java new file mode 100644 index 0000000..db25ec5 --- /dev/null +++ b/lib/java/src/org/apache/thrift/Option.java @@ -0,0 +1,121 @@ +/* + * 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; + +/** + * Implementation of the Option type pattern + */ +public abstract class Option<T> { + + /** + * Whether the Option is defined or not + * @return + * true if the Option is defined (of type Some) + * false if the Option is not defined (of type None) + */ + public abstract boolean isDefined(); + + /** + * Get the value of the Option (if it is defined) + * @return the value + * @throws IllegalStateException if called on a None + */ + public abstract T get(); + + /** + * Get the contained value (if defined) or else return a default value + * @param other what to return if the value is not defined (a None) + * @return either the value, or other if the value is not defined + */ + public T or(T other) { + if (isDefined()) { + return get(); + } else { + return other; + } + } + /** + * The None type, representing an absent value (instead of "null") + */ + public static class None<T> extends Option<T> { + public boolean isDefined() { + return false; + } + + public T get() { + throw new IllegalStateException("Cannot call get() on None"); + } + + public String toString() { + return "None"; + } + } + + /** + * The Some type, representing an existence of some value + * @param <T> The type of value + */ + public static class Some<T> extends Option<T> { + private final T value; + public Some(T value) { + this.value = value; + } + + public boolean isDefined() { + return true; + } + + public T get() { + return value; + } + + public String toString() { + return "Some("+value.toString()+")"; + } + } + + /** + * Wraps value in an Option type, depending on whether or not value is null + * @param value + * @param <T> type of value + * @return Some(value) if value is not null, None if value is null + */ + public static <T> Option<T> fromNullable(T value) { + if (value != null) { + return new Some<T>(value); + } else { + return new None<T>(); + } + } + + /** + * Wrap value in a Some type (NB! value must not be null!) + * @param value + * @param <T> type of value + * @return a new Some(value) + */ + public static <T> Some<T> some(T value) { + return new Some<T>(value); + } + + public static <T> None<T> none() { + return new None<T>(); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/thrift/blob/e6789480/lib/java/test/org/apache/thrift/TestOptionType.java ---------------------------------------------------------------------- diff --git a/lib/java/test/org/apache/thrift/TestOptionType.java b/lib/java/test/org/apache/thrift/TestOptionType.java new file mode 100644 index 0000000..f70285f --- /dev/null +++ b/lib/java/test/org/apache/thrift/TestOptionType.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 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 { + String name = "Chuck Norris"; + Option<String> option = Option.fromNullable(name); + + assertTrue(option instanceof Option.Some); + assertTrue(option.isDefined()); + assertEquals("Some(Chuck Norris)", option.toString()); + assertEquals(option.or("default value"), "Chuck Norris"); + assertEquals(option.get(),"Chuck Norris"); + } + + public void testNone() throws Exception { + String name = null; + Option<String> option = Option.fromNullable(name); + + assertTrue(option instanceof Option.None); + assertFalse(option.isDefined()); + assertEquals("None", option.toString()); + assertEquals(option.or("default value"), "default value"); + // Expect exception + try { + Object value = option.get(); + fail("Expected IllegalStateException, got no exception"); + } catch (IllegalStateException ex) { + + } catch(Exception ex) { + fail("Expected IllegalStateException, got some other exception: "+ex.toString()); + } + } + + public void testMakeSome() throws Exception { + Option<String> some = Option.some("wee"); + assertTrue(some instanceof Option.Some); + } + + public void testMakeNone() throws Exception { + Option<Integer> none = Option.none(); + assertTrue(none instanceof Option.None); + } +}
