From 9af23d9327277a791c8b22c4323b8e17e2bfc378 Mon Sep 17 00:00:00 2001 From: Bryan Duxbury Date: Thu, 19 Nov 2009 17:26:38 +0000 Subject: [PATCH] THRIFT-551. java: Enumeration doesn't generate real enum in Java This patch makes the compiler generate actual Enum classes. git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@882211 13f79535-47bb-0310-9956-ffa450edef68 --- compiler/cpp/src/generate/t_java_generator.cc | 161 +++++++++--------- lib/java/src/org/apache/thrift/TEnum.java | 24 +++ .../apache/thrift/meta_data/EnumMetaData.java | 31 ++++ .../thrift/meta_data/FieldValueMetaData.java | 6 +- .../src/org/apache/thrift/protocol/TType.java | 2 +- .../org/apache/thrift/test/TestClient.java | 14 +- .../org/apache/thrift/test/TestServer.java | 16 +- .../org/apache/thrift/test/UnionTest.java | 5 + test/DebugProtoTest.thrift | 6 + test/ThriftTest.thrift | 3 + 10 files changed, 166 insertions(+), 102 deletions(-) create mode 100644 lib/java/src/org/apache/thrift/TEnum.java create mode 100644 lib/java/src/org/apache/thrift/meta_data/EnumMetaData.java diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc index 8381f8ec..24c7ef54 100644 --- a/compiler/cpp/src/generate/t_java_generator.cc +++ b/compiler/cpp/src/generate/t_java_generator.cc @@ -229,7 +229,8 @@ class t_java_generator : public t_oop_generator { ttype->is_container() || ttype->is_struct() || ttype->is_xception() || - ttype->is_string(); + ttype->is_string() || + ttype->is_enum(); } std::string constant_name(std::string name); @@ -361,22 +362,21 @@ void t_java_generator::generate_enum(t_enum* tenum) { f_enum << autogen_comment() << java_package() << endl; - + // Add java imports f_enum << string() + - "import java.util.Set;\n" + - "import java.util.HashSet;\n" + - "import java.util.Collections;\n" + - "import org.apache.thrift.IntRangeSet;\n" + "import java.util.Map;\n" + - "import java.util.HashMap;\n" << endl; + "import java.util.HashMap;\n" + + "import org.apache.thrift.TEnum;" << endl; - f_enum << - "public class " << tenum->get_name() << " "; + generate_java_doc(f_enum, tenum); + indent(f_enum) << + "public enum " << tenum->get_name() << " implements TEnum"; scope_up(f_enum); vector constants = tenum->get_constants(); vector::iterator c_iter; + bool first = true; int value = -1; for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) { if ((*c_iter)->has_value()) { @@ -385,39 +385,50 @@ void t_java_generator::generate_enum(t_enum* tenum) { ++value; } + if (first) { + first = false; + } else { + f_enum << "," << endl; + } + generate_java_doc(f_enum, *c_iter); - indent(f_enum) << - "public static final int " << (*c_iter)->get_name() << - " = " << value << ";" << endl; - } - - // Create a static Set with all valid values for this enum - f_enum << endl; - indent(f_enum) << "public static final IntRangeSet VALID_VALUES = new IntRangeSet("; - indent_up(); - bool first = true; - for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) { - // populate set - f_enum << (first ? "" : ", ") << endl; - first = false; - indent(f_enum) << (*c_iter)->get_name(); + indent(f_enum) << " " << (*c_iter)->get_name() << "(" << value << ")"; } - f_enum << " );" << endl << endl; - indent_down(); + f_enum << ";" << endl << endl; - indent(f_enum) << "public static final Map VALUES_TO_NAMES = new HashMap() {{" << endl; + indent(f_enum) << "private static final Mapget_name() + + "> BY_VALUE = new HashMapget_name() +">() {{" << endl; + indent(f_enum) << " for("+ tenum->get_name() +" val : "+ tenum->get_name() +".values()) {" << endl; + indent(f_enum) << " put(val.getValue(), val);" << endl; + indent(f_enum) << " }" << endl; + indent(f_enum) << "}};" << endl; - indent_up(); - for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) { - indent(f_enum) << "put(" << (*c_iter)->get_name() << ", \"" << (*c_iter)->get_name() <<"\");" << endl; - } - indent_down(); + f_enum << endl; - - indent(f_enum) << "}};" << endl; + // Field for thriftCode + indent(f_enum) << "private final int value;" << endl << endl; + + indent(f_enum) << "private " << tenum->get_name() << "(int value) {" << endl; + indent(f_enum) << " this.value = value;" <get_name() + " findByValue(int value) { " << endl; + indent(f_enum) << " return BY_VALUE.get(value);" << endl; + indent(f_enum) << "}" << endl; scope_down(f_enum); - + f_enum.close(); } @@ -985,7 +996,7 @@ void t_java_generator::generate_union_hashcode(ofstream& out, t_struct* tstruct) if (gen_hash_code_) { indent(out) << "@Override" << endl; indent(out) << "public int hashCode() {" << endl; - indent(out) << " return new HashCodeBuilder().append(getSetField().getThriftFieldId()).append(getFieldValue()).toHashCode();" << endl; + indent(out) << " return new HashCodeBuilder().append(getSetField().getThriftFieldId()).append((getFieldValue() instanceof TEnum) ? ((TEnum)getFieldValue()).getValue() : getFieldValue()).toHashCode();" << endl; indent(out) << "}"; } else { indent(out) << "/**" << endl; @@ -1305,12 +1316,14 @@ void t_java_generator::generate_java_struct_equality(ofstream& out, present += " && (" + generate_isset_check(*m_iter) + ")"; } - out << - indent() << "boolean present_" << name << " = " - << present << ";" << endl << - indent() << "builder.append(present_" << name << ");" << endl << - indent() << "if (present_" << name << ")" << endl << - indent() << " builder.append(" << name << ");" << endl; + indent(out) << "boolean present_" << name << " = " << present << ";" << endl; + indent(out) << "builder.append(present_" << name << ");" << endl; + indent(out) << "if (present_" << name << ")" << endl; + if (t->is_enum()) { + indent(out) << " builder.append(" << name << ".getValue());" << endl; + } else { + indent(out) << " builder.append(" << name << ");" << endl; + } } out << endl; @@ -1401,9 +1414,9 @@ void t_java_generator::generate_java_struct_reader(ofstream& out, indent(out) << "} else {" << endl; indent_up(); - indent(out) << "switch (fieldId)" << endl; + indent(out) << "switch (fieldId) {" << endl; - scope_up(out); + indent_up(); // Generate deserialization code for known cases for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { @@ -1425,7 +1438,8 @@ void t_java_generator::generate_java_struct_reader(ofstream& out, indent_down(); } - scope_down(out); + indent_down(); + indent(out) << "}" << endl; // Read field end marker indent(out) << @@ -1467,11 +1481,11 @@ void t_java_generator::generate_java_struct_reader(ofstream& out, void t_java_generator::generate_java_validator(ofstream& out, t_struct* tstruct){ indent(out) << "public void validate() throws TException {" << endl; - indent_up(); - + indent_up(); + const vector& fields = tstruct->get_members(); vector::const_iterator f_iter; - + out << indent() << "// check for required fields" << endl; for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { if ((*f_iter)->get_req() == t_field::T_REQUIRED) { @@ -1488,25 +1502,10 @@ void t_java_generator::generate_java_validator(ofstream& out, } else { indent(out) << "// alas, we cannot check '" << (*f_iter)->get_name() << "' because it's a primitive and you chose the non-beans generator." << endl; } - } + } } } - - // check that fields of type enum have valid values - out << indent() << "// check that fields of type enum have valid values" << endl; - for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { - t_field* field = (*f_iter); - t_type* type = field->get_type(); - // if field is an enum, check that its value is valid - if (type->is_enum()){ - indent(out) << "if (" << generate_isset_check(field) << " && !" << get_enum_class_name(type) << ".VALID_VALUES.contains(" << field->get_name() << ")){" << endl; - indent_up(); - indent(out) << "throw new TProtocolException(\"The field '" << field->get_name() << "' has been assigned the invalid value \" + " << field->get_name() << ");" << endl; - indent_down(); - indent(out) << "}" << endl; - } - } - + indent_down(); indent(out) << "}" << endl << endl; } @@ -1935,7 +1934,7 @@ void t_java_generator::generate_java_struct_tostring(ofstream& out, indent(out) << " }" << endl; indent(out) << " if (this." << field->get_name() << ".length > 128) sb.append(\" ...\");" << endl; } else if(field->get_type()->is_enum()) { - indent(out) << "String " << field->get_name() << "_name = " << get_enum_class_name(field->get_type()) << ".VALUES_TO_NAMES.get(this." << (*f_iter)->get_name() << ");"<< endl; + indent(out) << "String " << field->get_name() << "_name = " << field->get_name() << ".name();"<< endl; indent(out) << "if (" << field->get_name() << "_name != null) {" << endl; indent(out) << " sb.append(" << field->get_name() << "_name);" << endl; indent(out) << " sb.append(\" (\");" << endl; @@ -2028,7 +2027,7 @@ std::string t_java_generator::get_java_type_string(t_type* type) { } else if (type->is_struct() || type->is_xception()) { return "TType.STRUCT"; } else if (type->is_enum()) { - return "TType.I32"; + return "TType.ENUM"; } else if (type->is_typedef()) { return get_java_type_string(((t_typedef*)type)->get_type()); } else if (type->is_base_type()) { @@ -2071,6 +2070,8 @@ void t_java_generator::generate_field_value_meta_data(std::ofstream& out, t_type out << ", "; generate_field_value_meta_data(out, val_type); } + } else if (type->is_enum()) { + indent(out) << "new EnumMetaData(TType.ENUM, " << type_name(type) << ".class"; } else { indent(out) << "new FieldValueMetaData(" << get_java_type_string(type); } @@ -2642,14 +2643,11 @@ void t_java_generator::generate_deserialize_field(ofstream& out, name); } else if (type->is_container()) { generate_deserialize_container(out, type, name); - } else if (type->is_base_type() || type->is_enum()) { - - indent(out) << - name << " = iprot."; + } else if (type->is_base_type()) { + indent(out) << name << " = iprot."; - if (type->is_base_type()) { - t_base_type::t_base tbase = ((t_base_type*)type)->get_base(); - switch (tbase) { + t_base_type::t_base tbase = ((t_base_type*)type)->get_base(); + switch (tbase) { case t_base_type::TYPE_VOID: throw "compiler error: cannot serialize void field in a struct: " + name; @@ -2681,12 +2679,10 @@ void t_java_generator::generate_deserialize_field(ofstream& out, break; default: throw "compiler error: no Java name for base type " + t_base_type::t_base_name(tbase); - } - } else if (type->is_enum()) { - out << "readI32();"; } - out << - endl; + out << endl; + } else if (type->is_enum()) { + indent(out) << name << " = " << type_name(tfield->get_type(), true, false) + ".findByValue(iprot.readI32());" << endl; } else { printf("DO NOT KNOW HOW TO DESERIALIZE FIELD '%s' TYPE '%s'\n", tfield->get_name().c_str(), type_name(type).c_str()); @@ -2856,8 +2852,9 @@ void t_java_generator::generate_serialize_field(ofstream& out, generate_serialize_container(out, type, prefix + tfield->get_name()); - } else if (type->is_base_type() || type->is_enum()) { - + } else if (type->is_enum()){ + indent(out) << "oprot.writeI32(" << prefix + tfield->get_name() << ".getValue());" << endl; + } else if (type->is_base_type()) { string name = prefix + tfield->get_name(); indent(out) << "oprot."; @@ -3045,8 +3042,6 @@ string t_java_generator::type_name(t_type* ttype, bool in_container, bool in_ini if (ttype->is_base_type()) { return base_type_name((t_base_type*)ttype, in_container); - } else if (ttype->is_enum()) { - return (in_container ? "Integer" : "int"); } else if (ttype->is_map()) { t_map* tmap = (t_map*) ttype; if (in_init) { diff --git a/lib/java/src/org/apache/thrift/TEnum.java b/lib/java/src/org/apache/thrift/TEnum.java new file mode 100644 index 00000000..325fdece --- /dev/null +++ b/lib/java/src/org/apache/thrift/TEnum.java @@ -0,0 +1,24 @@ +/* + * 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; + +public interface TEnum { + public int getValue(); +} diff --git a/lib/java/src/org/apache/thrift/meta_data/EnumMetaData.java b/lib/java/src/org/apache/thrift/meta_data/EnumMetaData.java new file mode 100644 index 00000000..be49cb94 --- /dev/null +++ b/lib/java/src/org/apache/thrift/meta_data/EnumMetaData.java @@ -0,0 +1,31 @@ +/* + * 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.meta_data; + +import org.apache.thrift.TEnum; + +public class EnumMetaData extends FieldValueMetaData { + public final Class enumClass; + + public EnumMetaData(byte type, Class sClass){ + super(type); + this.enumClass = sClass; + } +} diff --git a/lib/java/src/org/apache/thrift/meta_data/FieldValueMetaData.java b/lib/java/src/org/apache/thrift/meta_data/FieldValueMetaData.java index f72da0cd..076a768d 100644 --- a/lib/java/src/org/apache/thrift/meta_data/FieldValueMetaData.java +++ b/lib/java/src/org/apache/thrift/meta_data/FieldValueMetaData.java @@ -27,15 +27,15 @@ import org.apache.thrift.protocol.TType; */ public class FieldValueMetaData implements java.io.Serializable { public final byte type; - + public FieldValueMetaData(byte type){ this.type = type; } - + public boolean isStruct() { return type == TType.STRUCT; } - + public boolean isContainer() { return type == TType.LIST || type == TType.MAP || type == TType.SET; } diff --git a/lib/java/src/org/apache/thrift/protocol/TType.java b/lib/java/src/org/apache/thrift/protocol/TType.java index dbdc3caa..c3c1a0ab 100644 --- a/lib/java/src/org/apache/thrift/protocol/TType.java +++ b/lib/java/src/org/apache/thrift/protocol/TType.java @@ -21,7 +21,6 @@ package org.apache.thrift.protocol; /** * Type constants in the Thrift protocol. - * */ public final class TType { public static final byte STOP = 0; @@ -37,4 +36,5 @@ public final class TType { public static final byte MAP = 13; public static final byte SET = 14; public static final byte LIST = 15; + public static final byte ENUM = 16; } diff --git a/lib/java/test/org/apache/thrift/test/TestClient.java b/lib/java/test/org/apache/thrift/test/TestClient.java index 4a95f7a6..aee3bbfe 100644 --- a/lib/java/test/org/apache/thrift/test/TestClient.java +++ b/lib/java/test/org/apache/thrift/test/TestClient.java @@ -281,7 +281,7 @@ public class TestClient { * ENUM TEST */ System.out.print("testEnum(ONE)"); - int ret = testClient.testEnum(Numberz.ONE); + Numberz ret = testClient.testEnum(Numberz.ONE); System.out.print(" = " + ret + "\n"); System.out.print("testEnum(TWO)"); @@ -328,7 +328,7 @@ public class TestClient { * INSANITY TEST */ insane = new Insanity(); - insane.userMap = new HashMap(); + insane.userMap = new HashMap(); insane.userMap.put(Numberz.FIVE, (long)5000); Xtruct truck = new Xtruct(); truck.string_thing = "Truck"; @@ -338,20 +338,20 @@ public class TestClient { insane.xtructs = new ArrayList(); insane.xtructs.add(truck); System.out.print("testInsanity()"); - Map> whoa = + Map> whoa = testClient.testInsanity(insane); System.out.print(" = {"); for (long key : whoa.keySet()) { - Map val = whoa.get(key); + Map val = whoa.get(key); System.out.print(key + " => {"); - for (int k2 : val.keySet()) { + for (Numberz k2 : val.keySet()) { Insanity v2 = val.get(k2); System.out.print(k2 + " => {"); - Map userMap = v2.userMap; + Map userMap = v2.userMap; System.out.print("{"); if (userMap != null) { - for (int k3 : userMap.keySet()) { + for (Numberz k3 : userMap.keySet()) { System.out.print(k3 + " => " + userMap.get(k3) + ", "); } } diff --git a/lib/java/test/org/apache/thrift/test/TestServer.java b/lib/java/test/org/apache/thrift/test/TestServer.java index 986f8890..304d8a3d 100644 --- a/lib/java/test/org/apache/thrift/test/TestServer.java +++ b/lib/java/test/org/apache/thrift/test/TestServer.java @@ -140,7 +140,7 @@ public class TestServer { return thing; } - public int testEnum(int thing) { + public Numberz testEnum(Numberz thing) { System.out.print("testEnum(" + thing + ")\n"); return thing; } @@ -168,7 +168,7 @@ public class TestServer { return mapmap; } - public Map> testInsanity(Insanity argument) { + public Map> testInsanity(Insanity argument) { System.out.print("testInsanity()\n"); Xtruct hello = new Xtruct(); @@ -184,7 +184,7 @@ public class TestServer { goodbye.i64_thing = (long)4; Insanity crazy = new Insanity(); - crazy.userMap = new HashMap(); + crazy.userMap = new HashMap(); crazy.xtructs = new ArrayList(); crazy.userMap.put(Numberz.EIGHT, (long)8); @@ -194,23 +194,23 @@ public class TestServer { crazy.userMap.put(Numberz.FIVE, (long)5); crazy.xtructs.add(hello); - HashMap first_map = new HashMap(); - HashMap second_map = new HashMap();; + HashMap first_map = new HashMap(); + HashMap second_map = new HashMap();; first_map.put(Numberz.TWO, crazy); first_map.put(Numberz.THREE, crazy); second_map.put(Numberz.SIX, looney); - Map> insane = - new HashMap>(); + Map> insane = + new HashMap>(); insane.put((long)1, first_map); insane.put((long)2, second_map); return insane; } - public Xtruct testMulti(byte arg0, int arg1, long arg2, Map arg3, int arg4, long arg5) { + public Xtruct testMulti(byte arg0, int arg1, long arg2, Map arg3, Numberz arg4, long arg5) { System.out.print("testMulti()\n"); Xtruct hello = new Xtruct();; diff --git a/lib/java/test/org/apache/thrift/test/UnionTest.java b/lib/java/test/org/apache/thrift/test/UnionTest.java index cb69063a..670a33fa 100644 --- a/lib/java/test/org/apache/thrift/test/UnionTest.java +++ b/lib/java/test/org/apache/thrift/test/UnionTest.java @@ -7,6 +7,7 @@ import org.apache.thrift.transport.TMemoryBuffer; import thrift.test.Empty; import thrift.test.StructWithAUnion; import thrift.test.TestUnion; +import thrift.test.SomeEnum; import thrift.test.ComparableUnion; public class UnionTest { @@ -56,6 +57,7 @@ public class UnionTest { if (union.getI32_field() != 1) { throw new RuntimeException("didn't get the right value for i32 field!"); } + union.hashCode(); try { union.getString_field(); @@ -69,6 +71,9 @@ public class UnionTest { if (union.equals((TestUnion)null)) { throw new RuntimeException("uh oh, union.equals(null)!"); } + + union = TestUnion.enum_field(SomeEnum.ONE); + union.hashCode(); } diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift index 5b2f672e..549462eb 100644 --- a/test/DebugProtoTest.thrift +++ b/test/DebugProtoTest.thrift @@ -252,6 +252,11 @@ service ReverseOrderService { void myMethod(4: string first, 3: i16 second, 2: i32 third, 1: i64 fourth); } +enum SomeEnum { + ONE + TWO +} + union TestUnion { /** * A doc string @@ -261,6 +266,7 @@ union TestUnion { 3: OneOfEach struct_field; 4: list struct_list; 5: i32 other_i32_field; + 6: SomeEnum enum_field; } union ComparableUnion { diff --git a/test/ThriftTest.thrift b/test/ThriftTest.thrift index 3517640a..f16c02e3 100644 --- a/test/ThriftTest.thrift +++ b/test/ThriftTest.thrift @@ -23,6 +23,9 @@ namespace rb Thrift.Test namespace perl ThriftTest namespace csharp Thrift.Test +/** + * Docstring! + */ enum Numberz { ONE = 1, -- 2.17.1