Ruby default values patch

Summary: Submitted by Dan Sully, reviewed by Kevin Clark

Reviewed By: dreiss

Test Plan: New ruby generated code with default vals, and new test scripts


git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665418 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_rb_generator.cc b/compiler/cpp/src/generate/t_rb_generator.cc
index 54b5005..59f25de 100644
--- a/compiler/cpp/src/generate/t_rb_generator.cc
+++ b/compiler/cpp/src/generate/t_rb_generator.cc
@@ -34,14 +34,14 @@
     rb_imports() << endl <<
     render_includes() << endl;
     begin_namespace(f_types_, ruby_modules(program_));
-  
+
   f_consts_ <<
     rb_autogen_comment() << endl <<
     rb_imports() << endl <<
     "require '" << program_name_ << "_types'" << endl <<
     endl;
     begin_namespace(f_consts_, ruby_modules(program_));
-    
+
 }
 
 /**
@@ -107,7 +107,7 @@
   indent(f_types_) <<
     "module " << capitalize(tenum->get_name()) << endl;
   indent_up();
-  
+
   vector<t_enum_value*> constants = tenum->get_constants();
   vector<t_enum_value*>::iterator c_iter;
   int value = -1;
@@ -117,12 +117,12 @@
     } else {
       ++value;
     }
-    
+
     // Ruby class constants have to be capitalized... omg i am so on the fence
     // about languages strictly enforcing capitalization why can't we just all
     // agree and play nice.
     string name = capitalize((*c_iter)->get_name());
-    
+
     f_types_ <<
       indent() << name << " = " << value << endl;
   }
@@ -139,7 +139,7 @@
   t_type* type = tconst->get_type();
   string name = tconst->get_name();
   t_const_value* value = tconst->get_value();
-  
+
   name[0] = toupper(name[0]);
 
   indent(f_consts_) << name << " = " << render_const_value(type, value);
@@ -181,7 +181,7 @@
   } else if (type->is_enum()) {
     indent(out) << value->get_integer();
   } else if (type->is_struct() || type->is_xception()) {
-    out << type->get_name() << "({" << endl;
+    out << type->get_name() << ".new({" << endl;
     indent_up();
     const vector<t_field*>& fields = ((t_struct*)type)->get_members();
     vector<t_field*>::const_iterator f_iter;
@@ -280,10 +280,10 @@
     out << " < StandardError";
   }
   out  << endl;
-  
+
   indent_up();
   indent(out) << "include ThriftStruct" << endl;
-  
+
   generate_accessors(out, tstruct);
   generate_field_defns(out, tstruct);
 
@@ -294,7 +294,7 @@
 void t_rb_generator::generate_accessors(std::ofstream& out, t_struct* tstruct) {
   const vector<t_field*>& members = tstruct->get_members();
   vector<t_field*>::const_iterator m_iter;
-    
+
   if (members.size() > 0) {
     indent(out) << "attr_accessor ";
     for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
@@ -310,34 +310,39 @@
 void t_rb_generator::generate_field_defns(std::ofstream& out, t_struct* tstruct) {
   const vector<t_field*>& fields = tstruct->get_members();
   vector<t_field*>::const_iterator f_iter;
-  
+
   indent(out) << "FIELDS = {" << endl;
   indent_up();
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if (f_iter != fields.begin()) {
       out << "," << endl;
     }
-    
-    indent(out) << 
+
+    indent(out) <<
       (*f_iter)->get_key() << " => ";
-    
-    generate_field_data(out, (*f_iter)->get_type(), (*f_iter)->get_name());
+
+    generate_field_data(out, (*f_iter)->get_type(), (*f_iter)->get_name(), (*f_iter)->get_value());
   }
   indent_down();
   out << endl;
   indent(out) << "}" << endl;
 }
 
-void t_rb_generator::generate_field_data(std::ofstream& out, t_type* field_type, const std::string& field_name = "") {
+void t_rb_generator::generate_field_data(std::ofstream& out, t_type* field_type,
+    const std::string& field_name = "", t_const_value* field_value = NULL) {
   field_type = get_true_type(field_type);
 
   // Begin this field's defn
   out << "{:type => " << type_to_enum(field_type);
-    
+
   if (!field_name.empty()) {
     out << ", :name => '" << field_name << "'";
   }
 
+  if (field_value != NULL) {
+    out << ", :default => " << render_const_value(field_type, field_value);
+  }
+
   if (!field_type->is_base_type()) {
     if (field_type->is_struct() || field_type->is_xception()) {
       out << ", :class => " << type_name(((t_struct*)field_type));
@@ -354,7 +359,7 @@
       generate_field_data(out, ((t_set*)field_type)->get_elem_type());
     }
   }
-   
+
   // End of this field's defn
   out << "}";
 }
@@ -394,7 +399,7 @@
 
   f_service_ <<
     "require 'thrift/thrift'" << endl <<
-    "require '" << program_name_ << "_types'" << endl << 
+    "require '" << program_name_ << "_types'" << endl <<
     endl;
 
   begin_namespace(f_service_, ruby_modules(tservice->get_program()));
@@ -410,7 +415,7 @@
   indent_down();
   indent(f_service_) << "end" << endl <<
     endl;
-    
+
   end_namespace(f_service_, ruby_modules(tservice->get_program()));
 
   // Close service file
@@ -476,10 +481,10 @@
 
   indent(f_service_) <<
     "include ThriftClient" << endl << endl;
-  
+
   // Generate client method implementations
   vector<t_function*> functions = tservice->get_functions();
-  vector<t_function*>::const_iterator f_iter;    
+  vector<t_function*>::const_iterator f_iter;
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
     t_struct* arg_struct = (*f_iter)->get_arglist();
     const vector<t_field*>& fields = arg_struct->get_members();
@@ -526,8 +531,8 @@
 
       for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
         f_service_ << ", :" << (*fld_iter)->get_name() << " => " << (*fld_iter)->get_name();
-      }        
-      
+      }
+
       f_service_ << ")" << endl;
 
     indent_down();
@@ -536,7 +541,7 @@
     if (!(*f_iter)->is_async()) {
       std::string resultname = capitalize((*f_iter)->get_name() + "_result");
       t_struct noargs(program_);
-      
+
       t_function recv_function((*f_iter)->get_returntype(),
                                string("recv_") + (*f_iter)->get_name(),
                                &noargs);
@@ -550,11 +555,11 @@
 
       f_service_ <<
         indent() << "result = receive_message(" << resultname << ")" << endl;
-      
+
       // Careful, only return _result if not a void function
       if (!(*f_iter)->get_returntype()->is_void()) {
         f_service_ <<
-          indent() << "return result.success unless result.success.nil?" << endl;          
+          indent() << "return result.success unless result.success.nil?" << endl;
       }
 
       t_struct* xs = (*f_iter)->get_xceptions();
@@ -562,7 +567,7 @@
       vector<t_field*>::const_iterator x_iter;
       for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
         indent(f_service_) <<
-          "raise result." << (*x_iter)->get_name() << 
+          "raise result." << (*x_iter)->get_name() <<
             " unless result." << (*x_iter)->get_name() << ".nil?" << endl;
       }
 
@@ -573,12 +578,12 @@
       } else {
         f_service_ <<
           indent() << "raise TApplicationException.new(TApplicationException::MISSING_RESULT, '" << (*f_iter)->get_name() << " failed: unknown result')" << endl;
-      }     
+      }
 
       // Close function
       indent_down();
       indent(f_service_) << "end" << endl << endl;
-    }      
+    }
   }
 
   indent_down();
@@ -593,7 +598,7 @@
 void t_rb_generator::generate_service_server(t_service* tservice) {
   // Generate the dispatch methods
   vector<t_function*> functions = tservice->get_functions();
-  vector<t_function*>::iterator f_iter; 
+  vector<t_function*>::iterator f_iter;
 
   string extends = "";
   string extends_processor = "";
@@ -655,7 +660,7 @@
       indent() << "begin" << endl;
     indent_up();
   }
- 
+
   // Generate the function call
   t_struct* arg_struct = tfunction->get_arglist();
   const std::vector<t_field*>& fields = arg_struct->get_members();
@@ -711,22 +716,6 @@
 }
 
 /**
- * Declares a field, which may include initialization as necessary.
- *
- * @param ttype The type
- */
-string t_rb_generator::declare_field(t_field* tfield) {
-  string result = "@" + tfield->get_name();
-  t_type* type = get_true_type(tfield->get_type());
-  if (tfield->get_value() != NULL) {
-    result += " = " + render_const_value(type, tfield->get_value());
-  } else {
-    result += " = nil";
-  }
-  return result;
-}
-
-/**
  * Renders a function signature of the form 'type name(args)'
  *
  * @param tfunction Function definition
@@ -776,7 +765,7 @@
  */
 string t_rb_generator::type_to_enum(t_type* type) {
   type = get_true_type(type);
-  
+
   if (type->is_base_type()) {
     t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
     switch (tbase) {
diff --git a/compiler/cpp/src/generate/t_rb_generator.h b/compiler/cpp/src/generate/t_rb_generator.h
index d5b8abc..dc8d4a0 100644
--- a/compiler/cpp/src/generate/t_rb_generator.h
+++ b/compiler/cpp/src/generate/t_rb_generator.h
@@ -58,7 +58,7 @@
   void generate_rb_function_helpers(t_function* tfunction);
   void generate_accessors   (std::ofstream& out, t_struct* tstruct);
   void generate_field_defns (std::ofstream& out, t_struct* tstruct);
-  void generate_field_data  (std::ofstream& out, t_type* field_type, const std::string& field_name);
+  void generate_field_data  (std::ofstream& out, t_type* field_type, const std::string& field_name, t_const_value* field_value);
 
   /**
    * Service-level generation functions
diff --git a/lib/rb/lib/thrift/thrift.rb b/lib/rb/lib/thrift/thrift.rb
index 13b7db6..ab21646 100644
--- a/lib/rb/lib/thrift/thrift.rb
+++ b/lib/rb/lib/thrift/thrift.rb
@@ -177,8 +177,8 @@
 
 module ThriftStruct
   def initialize(d={})
-    each_field do |fid, type, name|
-      instance_variable_set("@#{name}", d[name.to_s])
+    each_field do |fid, type, name, default|
+      instance_variable_set("@#{name}", d[name.to_s] || default)
     end
   end
 
@@ -188,7 +188,7 @@
 
   def each_field
     struct_fields.each do |fid, data|
-      yield fid, data[:type], data[:name]
+      yield fid, data[:type], data[:name], data[:default]
     end
   end
 
diff --git a/test/rb/Makefile b/test/rb/Makefile
index 8b3cb98..5ea5305 100644
--- a/test/rb/Makefile
+++ b/test/rb/Makefile
@@ -9,10 +9,14 @@
 # Tools
 THRIFT = ../../compiler/cpp/thrift
 
-all: stubs
+all: stubs tests
 
-stubs: ../ThriftTest.thrift
+stubs: ../ThriftTest.thrift ../SmallTest.thrift
 	$(THRIFT) --rb ../ThriftTest.thrift
+	$(THRIFT) --rb ../SmallTest.thrift
+
+tests: stubs
+	ruby TestSuite.rb
 
 clean:
 	rm -fr gen-rb
diff --git a/test/rb/TestClient.rb b/test/rb/TestClient.rb
deleted file mode 100755
index 1adb5c7..0000000
--- a/test/rb/TestClient.rb
+++ /dev/null
@@ -1,37 +0,0 @@
-#!/usr/bin/env ruby
-
-$:.push('gen-rb')
-$:.push('../../lib/rb/lib')
-
-require 'thrift/transport/tsocket'
-require 'thrift/protocol/tbinaryprotocol'
-require 'ThriftTest'
-
-s = TSocket.new('localhost', 9090)
-p = TBinaryProtocol.new(s)
-c = Thrift::Test::ThriftTest::Client.new(p)
-
-s.open()
-
-puts c.testString('string')
-puts c.testByte(8)
-puts c.testByte(-8)
-puts c.testI32(32)
-puts c.testI32(-32)
-puts c.testI64(64)
-puts c.testI64(-64)
-puts c.testDouble(3.14)
-puts c.testDouble(-3.14)
-puts c.testMap({1 => 1, 2 => 2, 3 => 3})
-puts c.testList([1,2,3,4,5])
-puts c.testSet({1 => true, 2 => true, 3 => true})
-struct = Thrift::Test::Xtruct.new({'string_thing' => 'hi!', 'i32_thing' => 4 })
-puts c.testStruct(struct)
-puts c.testNest(Thrift::Test::Xtruct2.new({'struct_thing' => struct, 'i32_thing' => 10}))
-insane = Thrift::Test::Insanity.new({'userMap' => { Thrift::Test::Numberz::ONE => 44 }, 'xtructs' => [struct, Thrift::Test::Xtruct.new({'string_thing' => 'hi again', 'i32_thing' => 12})]})
-puts c.testInsanity(insane)
-puts c.testMapMap(4).inspect
-
-s.close()
-
-
diff --git a/test/rb/TestHandler.rb b/test/rb/TestHandler.rb
new file mode 100644
index 0000000..2a42541
--- /dev/null
+++ b/test/rb/TestHandler.rb
@@ -0,0 +1,73 @@
+#!/usr/bin/env ruby
+
+$:.push('gen-rb')
+$:.push('../../lib/rb/lib')
+
+require 'ThriftTest'
+
+class TestHandler
+  def testVoid
+  end
+
+  def testString(thing)
+    return thing
+  end
+
+  def testByte(thing)
+    return thing
+  end
+
+  def testI32(thing)
+    return thing
+  end
+
+  def testI64(thing)
+    return thing
+  end
+
+  def testDouble(thing)
+    return thing
+  end
+
+  def testStruct(thing)
+    return thing
+  end
+
+  def testMap(thing)
+    return thing
+  end
+
+  def testSet(thing)
+    return thing
+  end
+
+  def testList(thing)
+    return thing
+  end
+
+  def testNest(thing)
+    return thing
+  end
+
+  def testInsanity(thing)
+    num, uid = thing.userMap.find { true }
+    return {uid => {num => thing}}
+  end
+
+  def testMapMap(thing)
+    return {thing => {thing => thing}}
+  end
+
+  def testEnum(thing)
+    return thing
+  end
+
+  def testTypedef(thing)
+    return thing
+  end
+
+  def testException(thing)
+    raise Thrift::Test::Xception, 'error'
+  end
+
+end
diff --git a/test/rb/TestServer.rb b/test/rb/TestServer.rb
deleted file mode 100755
index 16fe5f6..0000000
--- a/test/rb/TestServer.rb
+++ /dev/null
@@ -1,100 +0,0 @@
-#!/usr/bin/env ruby
-
-$:.push('gen-rb')
-$:.push('../../lib/rb/lib')
-
-require 'thrift/transport/tsocket'
-require 'thrift/protocol/tbinaryprotocol'
-require 'thrift/server/tserver'
-require 'ThriftTest'
-
-class TestHandler
-  def testVoid()
-    print "testVoid()\n"
-  end
-
-  def testString(thing)
-    print "testString(#{thing})\n"
-    return thing
-  end
-
-  def testByte(thing)
-    print "testByte(#{thing})\n"
-    return thing
-  end
-
-  def testI32(thing)
-    print "testI32(#{thing})\n"
-    return thing
-  end
-
-  def testI64(thing)
-    print "testI64(#{thing})\n"
-    return thing
-  end
-
-  def testDouble(thing)
-    print "testDouble(#{thing})\n"
-    return thing
-  end
-
-  def testStruct(thing)
-    print "testStruct(#{thing})\n"
-    print "  with attrs: #{thing.string_thing}, #{thing.byte_thing}, #{thing.i32_thing}"
-    return thing
-  end
-
-  def testMap(thing)
-    print "testMap(#{thing})\n"
-    return thing
-  end
-
-  def testSet(thing)
-    print "testSet(#{thing})\n"
-    return thing
-  end
-
-  def testList(thing)
-    print "testList(#{thing})\n"
-    return thing
-  end
-
-  def testNest(thing)
-    print "testNest(#{thing})\n"
-    puts "  i32_thing: #{thing.i32_thing}"
-    puts "  with struct: "
-    %w{ string_thing byte_thing i32_thing }.each do |t|
-      puts "    #{t}: #{thing.struct_thing.send(t)}"
-    end
-    return thing
-  end
-
-  def testInsanity(thing)
-    puts "insanity:"
-    puts "  #{thing.inspect}"
-    num, uid = thing.userMap.find { true }
-    return {uid => {num => thing}}
-  end
-
-  def testMapMap(thing)
-    puts "got: #{thing}"
-    return {thing => {thing => thing}}
-  end
-
-  def testEnum(thing)
-    puts "testEnum(#{thing})"
-    return thing
-  end
-
-  def testTypedef(thing)
-    puts "testTypedef(#{thing})"
-    return thing
-  end
-
-end
-
-handler = TestHandler.new()
-processor = Thrift::Test::ThriftTest::Processor.new(handler)
-transport = TServerSocket.new(9090)
-server = TSimpleServer.new(processor, transport)
-server.serve()
diff --git a/test/rb/TestSmallService.rb b/test/rb/TestSmallService.rb
new file mode 100644
index 0000000..9ac287a
--- /dev/null
+++ b/test/rb/TestSmallService.rb
@@ -0,0 +1,32 @@
+#!/usr/bin/env ruby
+
+$:.push('gen-rb')
+$:.push('../../lib/rb/lib')
+
+require 'SmallService'
+require 'rubygems'
+require 'test/unit'
+
+class TestSmallService < Test::Unit::TestCase
+
+  def test_default_value
+    hello = Hello.new
+
+    assert_kind_of(Hello, hello)
+    assert_nil(hello.complexer)
+    
+    assert_equal(hello.simple, 53)
+    assert_equal(hello.words, 'words')
+
+    assert_kind_of(Goodbyez, hello.thinz)
+    assert_equal(hello.thinz.val, 36632)
+
+    assert_kind_of(Hash, hello.complex)
+    assert_equal(hello.complex, { 6243 => 632, 2355 => 532, 23 => 532})
+  end
+
+  def test_goodbyez
+    assert_equal(Goodbyez.new.val, 325)
+  end
+
+end
diff --git a/test/rb/TestSuite.rb b/test/rb/TestSuite.rb
new file mode 100644
index 0000000..04a4e2b
--- /dev/null
+++ b/test/rb/TestSuite.rb
@@ -0,0 +1,5 @@
+#!/usr/bin/env ruby
+
+require 'test/unit'
+require 'TestThrift'
+require 'TestSmallService'
diff --git a/test/rb/TestThrift.rb b/test/rb/TestThrift.rb
new file mode 100644
index 0000000..2b59fa9
--- /dev/null
+++ b/test/rb/TestThrift.rb
@@ -0,0 +1,168 @@
+#!/usr/bin/env ruby
+
+$:.push('gen-rb')
+$:.push('../../lib/rb/lib')
+
+require 'thrift/transport/tsocket'
+require 'thrift/protocol/tbinaryprotocol'
+require 'thrift/server/tserver'
+require 'ThriftTest'
+require 'TestHandler'
+
+require 'rubygems'
+require 'test/unit'
+
+class TestThrift < Test::Unit::TestCase
+
+  @@INIT = nil
+
+  def setup
+    if @@INIT.nil?
+      # Initialize the server
+      @handler   = TestHandler.new()
+      @processor = Thrift::Test::ThriftTest::Processor.new(@handler)
+      @transport = TServerSocket.new(9090)
+      @server    = TThreadedServer.new(@processor, @transport)
+
+      @thread    = Thread.new { @server.serve }
+
+      # And the Client
+      @socket   = TSocket.new('localhost', 9090)
+      @protocol = TBinaryProtocol.new(@socket)
+      @client   = Thrift::Test::ThriftTest::Client.new(@protocol)
+      @socket.open
+    end
+  end
+
+  def test_string
+    assert_equal(@client.testString('string'), 'string')
+  end
+
+  def test_byte
+    val = 8
+    assert_equal(@client.testByte(val), val)
+    assert_equal(@client.testByte(-val), -val)
+  end
+
+  def test_i32
+    val = 32
+    assert_equal(@client.testI32(val), val)
+    assert_equal(@client.testI32(-val), -val)
+  end
+
+  def test_i64
+    val = 64
+    assert_equal(@client.testI64(val), val)
+    assert_equal(@client.testI64(-val), -val)
+  end
+
+  def test_double
+    val = 3.14
+    assert_equal(@client.testDouble(val), val)
+    assert_equal(@client.testDouble(-val), -val)
+    assert_kind_of(Float, @client.testDouble(val))
+  end
+
+  def test_map
+    val = {1 => 1, 2 => 2, 3 => 3}
+    assert_equal(@client.testMap(val), val)
+    assert_kind_of(Hash, @client.testMap(val))
+  end
+
+  def test_list
+    val = [1,2,3,4,5]
+    assert_equal(@client.testList(val), val)
+    assert_kind_of(Array, @client.testList(val))
+  end
+
+  def test_enum
+    val = Thrift::Test::Numberz::SIX
+    ret = @client.testEnum(val)
+
+    assert_equal(ret, 6)
+    assert_kind_of(Fixnum, ret)
+  end
+
+  def test_typedef
+    #UserId  testTypedef(1: UserId thing),
+    true
+  end
+
+  def test_set
+    val = {1 => true, 2 => true, 3 => true}
+    assert_equal(@client.testSet(val), val)
+    assert_kind_of(Hash, @client.testSet(val))
+  end
+
+  def get_struct
+    Thrift::Test::Xtruct.new({'string_thing' => 'hi!', 'i32_thing' => 4 })
+  end
+
+  def test_struct
+    ret = @client.testStruct(get_struct)
+
+    assert_nil(ret.byte_thing, nil)
+    assert_nil(ret.i64_thing, nil)
+    assert_equal(ret.string_thing, 'hi!')
+    assert_equal(ret.i32_thing, 4)
+    assert_kind_of(Thrift::Test::Xtruct, ret)
+  end
+
+  def test_nest
+    struct2 = Thrift::Test::Xtruct2.new({'struct_thing' => get_struct, 'i32_thing' => 10})
+
+    ret = @client.testNest(struct2)
+
+    assert_nil(ret.struct_thing.byte_thing, nil)
+    assert_nil(ret.struct_thing.i64_thing, nil)
+    assert_equal(ret.struct_thing.string_thing, 'hi!')
+    assert_equal(ret.struct_thing.i32_thing, 4)
+    assert_equal(ret.i32_thing, 10)
+
+    assert_kind_of(Thrift::Test::Xtruct, ret.struct_thing)
+    assert_kind_of(Thrift::Test::Xtruct2, ret)
+  end
+
+  def test_insane
+    insane = Thrift::Test::Insanity.new({
+      'userMap' => { Thrift::Test::Numberz::ONE => 44 },
+      'xtructs' => [get_struct, 
+        Thrift::Test::Xtruct.new({
+          'string_thing' => 'hi again',
+          'i32_thing' => 12
+        })
+      ]
+    })
+
+    ret = @client.testInsanity(insane)
+
+    assert_not_nil(ret[44])
+    assert_not_nil(ret[44][1])
+
+    struct = ret[44][1]
+
+    assert_equal(struct.userMap[Thrift::Test::Numberz::ONE], 44)
+    assert_equal(struct.xtructs[1].string_thing, 'hi again')
+    assert_equal(struct.xtructs[1].i32_thing, 12)
+
+    assert_kind_of(Hash, struct.userMap)
+    assert_kind_of(Array, struct.xtructs)
+    assert_kind_of(Thrift::Test::Insanity, struct)
+  end
+
+  def test_map_map
+    ret = @client.testMapMap(4)
+    assert_kind_of(Hash, ret)
+    assert_equal(ret, { 4 => { 4 => 4}})
+  end
+
+  def test_exception
+    assert_raise Thrift::Test::Xception do
+      @client.testException('foo')
+    end
+  end
+
+  def teardown
+  end
+
+end