THRIFT-10. java: Descriptors used during serialization should be immutable objects

-Descriptor classes all have final members, making them immutable.
-Generated structs now have static constant versions of their TStruct and TField descriptors, and will be used during writing.
-Protocols that can benefit use static constants for various common returned descriptors.
-A duplicate FieldMetaData.java that should have been removed previously was also removed.

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@741984 13f79535-47bb-0310-9956-ffa450edef68

12 files changed, 134 insertions, 122 deletions

diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc
index cee43adcf..d258cd08c 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -9,6 +9,7 @@
 #include <fstream>
 #include <iostream>
 #include <vector>
+#include <cctype>
 
 #include <sys/stat.h>
 #include <stdexcept>
@@ -184,6 +185,7 @@ class t_java_generator : public t_oop_generator {
       ttype->is_string();
   }
 
+  std::string constant_name(std::string name);
 
  private:
 
@@ -609,9 +611,23 @@ void t_java_generator::generate_java_struct_definition(ofstream &out,
 
   scope_up(out);
 
+  indent(out) <<
+    "private static final TStruct STRUCT_DESC = new TStruct(\"" << tstruct->get_name() << "\");" << endl;
+
   // Members are public for -java, private for -javabean
   const vector<t_field*>& members = tstruct->get_members();
   vector<t_field*>::const_iterator m_iter;
+
+  for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+    indent(out) <<
+      "private static final TField " << constant_name((*m_iter)->get_name()) <<
+      "_FIELD_DESC = new TField(\"" << (*m_iter)->get_name() << "\", " <<
+      type_to_enum((*m_iter)->get_type()) << ", " <<
+      "(short)" << (*m_iter)->get_key() << ");" << endl;
+  }
+
+  out << endl;
+
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
     if (bean_style_) {
       indent(out) << "private ";
@@ -1063,12 +1079,8 @@ void t_java_generator::generate_java_struct_writer(ofstream& out,
   // performs various checks (e.g. check that all required fields are set)
   indent(out) << "validate();" << endl << endl;
 
-  indent(out) << "TStruct struct = new TStruct(\"" << name << "\");" << endl;
-  indent(out) << "oprot.writeStructBegin(struct);" << endl;
+  indent(out) << "oprot.writeStructBegin(STRUCT_DESC);" << endl;
 
-  if (!fields.empty()) {
-    indent(out) << "TField field = new TField();" << endl;
-  }
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     bool null_allowed = type_can_be_null((*f_iter)->get_type());
     if (null_allowed) {
@@ -1083,11 +1095,7 @@ void t_java_generator::generate_java_struct_writer(ofstream& out,
       indent_up();
     }
 
-    out <<
-      indent() << "field.name = \"" << (*f_iter)->get_name() << "\";" << endl <<
-      indent() << "field.type = " << type_to_enum((*f_iter)->get_type()) << ";" << endl <<
-      indent() << "field.id = " << upcase_string((*f_iter)->get_name()) << ";" << endl <<
-      indent() << "oprot.writeFieldBegin(field);" << endl;
+    indent(out) << "oprot.writeFieldBegin(" << constant_name((*f_iter)->get_name()) << "_FIELD_DESC);" << endl;
 
     // Write field contents
     generate_serialize_field(out, *f_iter, "this.");
@@ -1134,12 +1142,8 @@ void t_java_generator::generate_java_struct_result_writer(ofstream& out,
   const vector<t_field*>& fields = tstruct->get_members();
   vector<t_field*>::const_iterator f_iter;
 
-  indent(out) << "TStruct struct = new TStruct(\"" << name << "\");" << endl;
-  indent(out) << "oprot.writeStructBegin(struct);" << endl;
+  indent(out) << "oprot.writeStructBegin(STRUCT_DESC);" << endl;
 
-  if (!fields.empty()) {
-    indent(out) << "TField field = new TField();" << endl;
-  }
   bool first = true;
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if (first) {
@@ -1163,11 +1167,7 @@ void t_java_generator::generate_java_struct_result_writer(ofstream& out,
       indent_up();
     }
 
-    out <<
-      indent() << "field.name = \"" << (*f_iter)->get_name() << "\";" << endl <<
-      indent() << "field.type = " << type_to_enum((*f_iter)->get_type()) << ";" << endl <<
-      indent() << "field.id = " << upcase_string((*f_iter)->get_name()) << ";" << endl <<
-      indent() << "oprot.writeFieldBegin(field);" << endl;
+    indent(out) << "oprot.writeFieldBegin(" << constant_name((*f_iter)->get_name()) << "_FIELD_DESC);" << endl;
 
     // Write field contents
     generate_serialize_field(out, *f_iter, "this.");
@@ -2343,7 +2343,7 @@ void t_java_generator::generate_deserialize_list_element(ofstream& out,
   t_field felem(tlist->get_elem_type(), elem);
 
   indent(out) <<
-    declare_field(&felem, true) << endl;
+    declare_field(&felem) << endl;
 
   generate_deserialize_field(out, &felem);
 
@@ -2785,6 +2785,28 @@ std::string t_java_generator::get_cap_name(std::string name){
   }
 }
 
+string t_java_generator::constant_name(string name) {
+  string constant_name;
+
+  bool is_first = true;
+  bool was_previous_char_upper = false;
+  for (string::iterator iter = name.begin(); iter != name.end(); ++iter) {
+    string::value_type character = (*iter);
+
+    bool is_upper = isupper(character);
+
+    if (is_upper && !is_first && !was_previous_char_upper) {
+      constant_name += '_';
+    }
+    constant_name += toupper(character);
+
+    is_first = false;
+    was_previous_char_upper = is_upper;
+  }
+
+  return constant_name;
+}
+
 /**
  * Emits a JavaDoc comment if the provided object has a doc in Thrift
  */
diff --git a/lib/java/src/org/apache/thrift/FieldMetaData.java b/lib/java/src/org/apache/thrift/FieldMetaData.java
deleted file mode 100644
index e69de29bb..000000000
--- a/lib/java/src/org/apache/thrift/FieldMetaData.java
+++ /dev/null
diff --git a/lib/java/src/org/apache/thrift/TApplicationException.java b/lib/java/src/org/apache/thrift/TApplicationException.java
index e2b34b96e..559f8c3c6 100644
--- a/lib/java/src/org/apache/thrift/TApplicationException.java
+++ b/lib/java/src/org/apache/thrift/TApplicationException.java
@@ -19,6 +19,10 @@ import org.apache.thrift.protocol.TType;
  */
 public class TApplicationException extends TException {
 
+  private static final TStruct TAPPLICATION_EXCEPTION_STRUCT = new TStruct("TApplicationException");
+  private static final TField MESSAGE_FIELD = new TField("message", TType.STRING, (short)1);
+  private static final TField TYPE_FIELD = new TField("type", TType.I32, (short)2);
+
   private static final long serialVersionUID = 1L;
 
   public static final int UNKNOWN = 0;
@@ -91,25 +95,16 @@ public class TApplicationException extends TException {
   }
 
   public void write(TProtocol oprot) throws TException {
-    TStruct struct = new TStruct("TApplicationException");
-    TField field = new TField();
-    oprot.writeStructBegin(struct);
+    oprot.writeStructBegin(TAPPLICATION_EXCEPTION_STRUCT);
     if (getMessage() != null) {
-      field.name = "message";
-      field.type = TType.STRING;
-      field.id = 1;
-      oprot.writeFieldBegin(field);
+      oprot.writeFieldBegin(MESSAGE_FIELD);
       oprot.writeString(getMessage());
       oprot.writeFieldEnd();
     }
-    field.name = "type";
-    field.type = TType.I32;
-    field.id = 2;
-    oprot.writeFieldBegin(field);
+    oprot.writeFieldBegin(TYPE_FIELD);
     oprot.writeI32(type_);
     oprot.writeFieldEnd();
     oprot.writeFieldStop();
     oprot.writeStructEnd();
-
   }
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TBinaryProtocol.java b/lib/java/src/org/apache/thrift/protocol/TBinaryProtocol.java
index 36fb566dc..7e199b4fd 100644
--- a/lib/java/src/org/apache/thrift/protocol/TBinaryProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TBinaryProtocol.java
@@ -17,6 +17,7 @@ import org.apache.thrift.transport.TTransport;
  * @author Mark Slee <mcslee@facebook.com>
  */
 public class TBinaryProtocol extends TProtocol {
+  private static final TStruct ANONYMOUS_STRUCT = new TStruct();
 
   protected static final int VERSION_MASK = 0xffff0000;
   protected static final int VERSION_1 = 0x80010000;
@@ -176,71 +177,51 @@ public class TBinaryProtocol extends TProtocol {
    */
 
   public TMessage readMessageBegin() throws TException {
-    TMessage message = new TMessage();
-
     int size = readI32();
     if (size < 0) {
       int version = size & VERSION_MASK;
       if (version != VERSION_1) {
         throw new TProtocolException(TProtocolException.BAD_VERSION, "Bad version in readMessageBegin");
       }
-      message.type = (byte)(size & 0x000000ff);
-      message.name = readString();
-      message.seqid = readI32();
+      return new TMessage(readString(), (byte)(size & 0x000000ff), readI32());
     } else {
       if (strictRead_) {
         throw new TProtocolException(TProtocolException.BAD_VERSION, "Missing version in readMessageBegin, old client?");
       }
-      message.name = readStringBody(size);
-      message.type = readByte();
-      message.seqid = readI32();
+      return new TMessage(readStringBody(size), readByte(), readI32());
     }
-    return message;
   }
 
   public void readMessageEnd() {}
 
   public TStruct readStructBegin() {
-    return new TStruct();
+    return ANONYMOUS_STRUCT;
   }
 
   public void readStructEnd() {}
 
   public TField readFieldBegin() throws TException {
-    TField field = new TField();
-    field.type = readByte();
-    if (field.type != TType.STOP) {
-      field.id = readI16();
-    }
-    return field;
+    byte type = readByte();
+    short id = type == TType.STOP ? 0 : readI16();
+    return new TField("", type, id);
   }
 
   public void readFieldEnd() {}
 
   public TMap readMapBegin() throws TException {
-    TMap map = new TMap();
-    map.keyType = readByte();
-    map.valueType = readByte();
-    map.size = readI32();
-    return map;
+    return new TMap(readByte(), readByte(), readI32());
   }
 
   public void readMapEnd() {}
 
   public TList readListBegin() throws TException {
-    TList list = new TList();
-    list.elemType = readByte();
-    list.size = readI32();
-    return list;
+    return new TList(readByte(), readI32());
   }
 
   public void readListEnd() {}
 
   public TSet readSetBegin() throws TException {
-    TSet set = new TSet();
-    set.elemType = readByte();
-    set.size = readI32();
-    return set;
+    return new TSet(readByte(), readI32());
   }
 
   public void readSetEnd() {}
diff --git a/lib/java/src/org/apache/thrift/protocol/TField.java b/lib/java/src/org/apache/thrift/protocol/TField.java
index d74b0cee7..f7c8e92db 100644
--- a/lib/java/src/org/apache/thrift/protocol/TField.java
+++ b/lib/java/src/org/apache/thrift/protocol/TField.java
@@ -12,7 +12,9 @@ package org.apache.thrift.protocol;
  * @author Mark Slee <mcslee@facebook.com>
  */
 public class TField {
-  public TField() {}
+  public TField() {
+    this("", TType.STOP, (short)0);
+  }
 
   public TField(String n, byte t, short i) {
     name = n;
@@ -20,7 +22,7 @@ public class TField {
     id = i;
   }
 
-  public String name = "";
-  public byte   type = TType.STOP;
-  public short  id   = 0;
+  public final String name;
+  public final byte   type;
+  public final short  id;
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
index b66b8f6c1..911bcd476 100644
--- a/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TJSONProtocol.java
@@ -76,6 +76,8 @@ public class TJSONProtocol extends TProtocol {
   private static final byte[] NAME_LIST = new byte[] {'l','s','t'};
   private static final byte[] NAME_SET = new byte[] {'s','e','t'};
 
+  private static final TStruct ANONYMOUS_STRUCT = new TStruct();
+
   private static final byte[] getTypeNameForTypeID(byte typeID)
     throws TException {
     switch (typeID) {
@@ -769,21 +771,21 @@ public class TJSONProtocol extends TProtocol {
 
   @Override
   public TMessage readMessageBegin() throws TException {
-    TMessage message = new TMessage();
     readJSONArrayStart();
     if (readJSONInteger() != VERSION) {
       throw new TProtocolException(TProtocolException.BAD_VERSION,
                                    "Message contained bad version.");
     }
+    String name;
     try {
-      message.name = readJSONString(false).toString("UTF-8");
+      name = readJSONString(false).toString("UTF-8");
     }
     catch (UnsupportedEncodingException ex) {
       throw new TException("JVM DOES NOT SUPPORT UTF-8");
     }
-    message.type = (byte) readJSONInteger();
-    message.seqid = (int) readJSONInteger();
-    return message;
+    byte type = (byte) readJSONInteger();
+    int seqid = (int) readJSONInteger();
+    return new TMessage(name, type, seqid);
   }
 
   @Override
@@ -794,7 +796,7 @@ public class TJSONProtocol extends TProtocol {
   @Override
   public TStruct readStructBegin() throws TException {
     readJSONObjectStart();
-    return new TStruct();
+    return ANONYMOUS_STRUCT;
   }
 
   @Override
@@ -804,17 +806,18 @@ public class TJSONProtocol extends TProtocol {
 
   @Override
   public TField readFieldBegin() throws TException {
-    TField field = new TField();
     byte ch = reader_.peek();
+    byte type;
+    short id = 0;
     if (ch == RBRACE[0]) {
-      field.type = TType.STOP;
+      type = TType.STOP;
     }
     else {
-      field.id = (short) readJSONInteger();
+      id = (short) readJSONInteger();
       readJSONObjectStart();
-      field.type = getTypeIDForTypeName(readJSONString(false).get());
+      type = getTypeIDForTypeName(readJSONString(false).get());
     }
-    return field;
+    return new TField("", type, id);
   }
 
   @Override
@@ -824,13 +827,12 @@ public class TJSONProtocol extends TProtocol {
 
   @Override
   public TMap readMapBegin() throws TException {
-    TMap map = new TMap();
     readJSONArrayStart();
-    map.keyType = getTypeIDForTypeName(readJSONString(false).get());
-    map.valueType = getTypeIDForTypeName(readJSONString(false).get());
-    map.size = (int)readJSONInteger();
+    byte keyType = getTypeIDForTypeName(readJSONString(false).get());
+    byte valueType = getTypeIDForTypeName(readJSONString(false).get());
+    int size = (int)readJSONInteger();
     readJSONObjectStart();
-    return map;
+    return new TMap(keyType, valueType, size);
   }
 
   @Override
@@ -841,11 +843,10 @@ public class TJSONProtocol extends TProtocol {
 
   @Override
   public TList readListBegin() throws TException {
-    TList list = new TList();
     readJSONArrayStart();
-    list.elemType = getTypeIDForTypeName(readJSONString(false).get());
-    list.size = (int)readJSONInteger();
-    return list;
+    byte elemType = getTypeIDForTypeName(readJSONString(false).get());
+    int size = (int)readJSONInteger();
+    return new TList(elemType, size);
   }
 
   @Override
@@ -855,11 +856,10 @@ public class TJSONProtocol extends TProtocol {
 
   @Override
   public TSet readSetBegin() throws TException {
-    TSet set = new TSet();
     readJSONArrayStart();
-    set.elemType = getTypeIDForTypeName(readJSONString(false).get());
-    set.size = (int)readJSONInteger();
-    return set;
+    byte elemType = getTypeIDForTypeName(readJSONString(false).get());
+    int size = (int)readJSONInteger();
+    return new TSet(elemType, size);
   }
 
   @Override
diff --git a/lib/java/src/org/apache/thrift/protocol/TList.java b/lib/java/src/org/apache/thrift/protocol/TList.java
index 4d356e96c..b099df0c8 100644
--- a/lib/java/src/org/apache/thrift/protocol/TList.java
+++ b/lib/java/src/org/apache/thrift/protocol/TList.java
@@ -11,14 +11,16 @@ package org.apache.thrift.protocol;
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-public class TList {
-  public TList() {}
+public final class TList {
+  public TList() {
+    this(TType.STOP, 0);
+  }
 
   public TList(byte t, int s) {
     elemType = t;
     size = s;
   }
 
-  public byte elemType = TType.STOP;
-  public int  size     = 0;
+  public final byte elemType;
+  public final int  size;
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TMap.java b/lib/java/src/org/apache/thrift/protocol/TMap.java
index 964eb7971..6d34f872f 100644
--- a/lib/java/src/org/apache/thrift/protocol/TMap.java
+++ b/lib/java/src/org/apache/thrift/protocol/TMap.java
@@ -11,8 +11,10 @@ package org.apache.thrift.protocol;
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-public class TMap {
-  public TMap() {}
+public final class TMap {
+  public TMap() {
+    this(TType.STOP, TType.STOP, 0);
+  }
 
   public TMap(byte k, byte v, int s) {
     keyType = k;
@@ -20,7 +22,7 @@ public class TMap {
     size = s;
   }
 
-  public byte  keyType   = TType.STOP;
-  public byte  valueType = TType.STOP;
-  public int   size      = 0;
+  public final byte  keyType;
+  public final byte  valueType;
+  public final int   size;
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TMessage.java b/lib/java/src/org/apache/thrift/protocol/TMessage.java
index fa933946f..d7c03cfdc 100644
--- a/lib/java/src/org/apache/thrift/protocol/TMessage.java
+++ b/lib/java/src/org/apache/thrift/protocol/TMessage.java
@@ -11,8 +11,10 @@ package org.apache.thrift.protocol;
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-public class TMessage {
-  public TMessage() {}
+public final class TMessage {
+  public TMessage() {
+    this("", TType.STOP, 0);
+  }
 
   public TMessage(String n, byte t, int s) {
     name = n;
@@ -20,7 +22,7 @@ public class TMessage {
     seqid = s;
   }
 
-  public String name = "";
-  public byte type;
-  public int seqid;
+  public final String name;
+  public final byte type;
+  public final int seqid;
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TSet.java b/lib/java/src/org/apache/thrift/protocol/TSet.java
index 6f14154b3..515d55869 100644
--- a/lib/java/src/org/apache/thrift/protocol/TSet.java
+++ b/lib/java/src/org/apache/thrift/protocol/TSet.java
@@ -11,14 +11,16 @@ package org.apache.thrift.protocol;
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-public class TSet {
-  public TSet() {}
+public final class TSet {
+  public TSet() {
+    this(TType.STOP, 0);
+  }
 
   public TSet(byte t, int s) {
     elemType = t;
     size = s;
   }
 
-  public byte elemType = TType.STOP;
-  public int  size     = 0;
+  public final byte elemType;
+  public final int  size;
 }
diff --git a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
index 1a5167ce6..5846ea243 100644
--- a/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
+++ b/lib/java/src/org/apache/thrift/protocol/TSimpleJSONProtocol.java
@@ -41,6 +41,13 @@ public class TSimpleJSONProtocol extends TProtocol {
   public static final byte[] RBRACKET = new byte[] {']'};
   public static final char QUOTE = '"';
 
+  private static final TStruct ANONYMOUS_STRUCT = new TStruct();
+  private static final TField ANONYMOUS_FIELD = new TField();
+  private static final TMessage EMPTY_MESSAGE = new TMessage();
+  private static final TSet EMPTY_SET = new TSet();
+  private static final TList EMPTY_LIST = new TList();
+  private static final TMap EMPTY_MAP = new TMap();
+
   protected class Context {
     protected void write() throws TException {}
   }
@@ -278,48 +285,43 @@ public class TSimpleJSONProtocol extends TProtocol {
    */
 
   public TMessage readMessageBegin() throws TException {
-    TMessage message = new TMessage();
     // TODO(mcslee): implement
-    return message;
+    return EMPTY_MESSAGE;
   }
 
   public void readMessageEnd() {}
 
   public TStruct readStructBegin() {
     // TODO(mcslee): implement
-    return new TStruct();
+    return ANONYMOUS_STRUCT;
   }
 
   public void readStructEnd() {}
 
   public TField readFieldBegin() throws TException {
-    TField field = new TField();
     // TODO(mcslee): implement
-    return field;
+    return ANONYMOUS_FIELD;
   }
 
   public void readFieldEnd() {}
 
   public TMap readMapBegin() throws TException {
-    TMap map = new TMap();
     // TODO(mcslee): implement
-    return map;
+    return EMPTY_MAP;
   }
 
   public void readMapEnd() {}
 
   public TList readListBegin() throws TException {
-    TList list = new TList();
     // TODO(mcslee): implement
-    return list;
+    return EMPTY_LIST;
   }
 
   public void readListEnd() {}
 
   public TSet readSetBegin() throws TException {
-    TSet set = new TSet();
     // TODO(mcslee): implement
-    return set;
+    return EMPTY_SET;
   }
 
   public void readSetEnd() {}
diff --git a/lib/java/src/org/apache/thrift/protocol/TStruct.java b/lib/java/src/org/apache/thrift/protocol/TStruct.java
index 2a8f869ef..804ffc16b 100644
--- a/lib/java/src/org/apache/thrift/protocol/TStruct.java
+++ b/lib/java/src/org/apache/thrift/protocol/TStruct.java
@@ -11,12 +11,14 @@ package org.apache.thrift.protocol;
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
-public class TStruct {
-  public TStruct() {}
+public final class TStruct {
+  public TStruct() {
+    this("");
+  }
 
   public TStruct(String n) {
     name = n;
   }
 
-  public String name = "";
+  public final String name;
 }