You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by kc...@apache.org on 2008/06/18 03:06:15 UTC

svn commit: r668946 - in /incubator/thrift/trunk/lib/rb: lib/thrift/protocol.rb spec/protocol_spec.rb

Author: kclark
Date: Tue Jun 17 18:06:15 2008
New Revision: 668946

URL: http://svn.apache.org/viewvc?rev=668946&view=rev
Log:
Start speccing Protocol.

The Protocol specs exposed a bug in the implementation of skip(Types::STRUCT).
Previously it would call read_struct_end once per field instead of per struct.
This only worked because read_struct_end is a noop.

Also remove all empty parens () from method calls.

Added:
    incubator/thrift/trunk/lib/rb/spec/protocol_spec.rb
Modified:
    incubator/thrift/trunk/lib/rb/lib/thrift/protocol.rb

Modified: incubator/thrift/trunk/lib/rb/lib/thrift/protocol.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/lib/thrift/protocol.rb?rev=668946&r1=668945&r2=668946&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/lib/thrift/protocol.rb (original)
+++ incubator/thrift/trunk/lib/rb/lib/thrift/protocol.rb Tue Jun 17 18:06:15 2008
@@ -44,34 +44,34 @@
     def write_struct_begin(name); nil; end
     deprecate! :writeStructBegin => :write_struct_begin
 
-    def write_struct_end(); nil; end
+    def write_struct_end; nil; end
     deprecate! :writeStructEnd => :write_struct_end
 
     def write_field_begin(name, type, id); nil; end
     deprecate! :writeFieldBegin => :write_field_begin
 
-    def write_field_end(); nil; end
+    def write_field_end; nil; end
     deprecate! :writeFieldEnd => :write_field_end
 
-    def write_field_stop(); nil; end
+    def write_field_stop; nil; end
     deprecate! :writeFieldStop => :write_field_stop
 
     def write_map_begin(ktype, vtype, size); nil; end
     deprecate! :writeMapBegin => :write_map_begin
 
-    def write_map_end(); nil; end
+    def write_map_end; nil; end
     deprecate! :writeMapEnd => :write_map_end
 
     def write_list_begin(etype, size); nil; end
     deprecate! :writeListBegin => :write_list_begin
 
-    def write_list_end(); nil; end
+    def write_list_end; nil; end
     deprecate! :writeListEnd => :write_list_end
 
     def write_set_begin(etype, size); nil; end
     deprecate! :writeSetBegin => :write_set_begin
 
-    def write_set_end(); nil; end
+    def write_set_end; nil; end
     deprecate! :writeSetEnd => :write_set_end
 
     def write_bool(bool); nil; end
@@ -95,61 +95,61 @@
     def write_string(str); nil; end
     deprecate! :writeString => :write_string
 
-    def read_message_begin(); nil; end
+    def read_message_begin; nil; end
     deprecate! :readMessageBegin => :read_message_begin
 
-    def read_message_end(); nil; end
+    def read_message_end; nil; end
     deprecate! :readMessageEnd => :read_message_end
 
-    def read_struct_begin(); nil; end
+    def read_struct_begin; nil; end
     deprecate! :readStructBegin => :read_struct_begin
 
-    def read_struct_end(); nil; end
+    def read_struct_end; nil; end
     deprecate! :readStructEnd => :read_struct_end
 
-    def read_field_begin(); nil; end
+    def read_field_begin; nil; end
     deprecate! :readFieldBegin => :read_field_begin
 
-    def read_field_end(); nil; end
+    def read_field_end; nil; end
     deprecate! :readFieldEnd => :read_field_end
 
-    def read_map_begin(); nil; end
+    def read_map_begin; nil; end
     deprecate! :readMapBegin => :read_map_begin
 
-    def read_map_end(); nil; end
+    def read_map_end; nil; end
     deprecate! :readMapEnd => :read_map_end
 
-    def read_list_begin(); nil; end
+    def read_list_begin; nil; end
     deprecate! :readListBegin => :read_list_begin
 
-    def read_list_end(); nil; end
+    def read_list_end; nil; end
     deprecate! :readListEnd => :read_list_end
 
-    def read_set_begin(); nil; end
+    def read_set_begin; nil; end
     deprecate! :readSetBegin => :read_set_begin
 
-    def read_set_end(); nil; end
+    def read_set_end; nil; end
     deprecate! :readSetEnd => :read_set_end
 
-    def read_bool(); nil; end
+    def read_bool; nil; end
     deprecate! :readBool => :read_bool
 
-    def read_byte(); nil; end
+    def read_byte; nil; end
     deprecate! :readByte => :read_byte
 
-    def read_i16(); nil; end
+    def read_i16; nil; end
     deprecate! :readI16 => :read_i16
 
-    def read_i32(); nil; end
+    def read_i32; nil; end
     deprecate! :readI32 => :read_i32
 
-    def read_i64(); nil; end
+    def read_i64; nil; end
     deprecate! :readI64 => :read_i64
 
-    def read_double(); nil; end
+    def read_double; nil; end
     deprecate! :readDouble => :read_double
 
-    def read_string(); nil; end
+    def read_string; nil; end
     deprecate! :readString => :read_string
 
     def write_field(name, type, fid, value)
@@ -206,50 +206,50 @@
       if type === Types::STOP
         nil
       elsif type === Types::BOOL
-        read_bool()
+        read_bool
       elsif type === Types::BYTE
-        read_byte()
+        read_byte
       elsif type === Types::I16
-        read_i16()
+        read_i16
       elsif type === Types::I32
-        read_i32()
+        read_i32
       elsif type === Types::I64
-        read_i64()
+        read_i64
       elsif type === Types::DOUBLE
-        read_double()
+        read_double
       elsif type === Types::STRING
-        read_string()
+        read_string
       elsif type === Types::STRUCT
-        read_struct_begin()
+        read_struct_begin
         while true
-          name, type, id = read_field_begin()
+          name, type, id = read_field_begin
           if type === Types::STOP
             break
           else
             skip(type)
-            read_field_end()
+            read_field_end
           end
-          read_struct_end()
-        end
+        end  
+        read_struct_end
       elsif type === Types::MAP
-        ktype, vtype, size = read_map_begin()
+        ktype, vtype, size = read_map_begin
         for i in 1..size
           skip(ktype)
           skip(vtype)
         end
-        read_map_end()
+        read_map_end
       elsif type === Types::SET
-        etype, size = read_set_begin()
+        etype, size = read_set_begin
         for i in 1..size
           skip(etype)
         end
-        read_set_end()
+        read_set_end
       elsif type === Types::LIST
-        etype, size = read_list_begin()
+        etype, size = read_list_begin
         for i in 1..size
           skip(etype)
         end
-        read_list_end()
+        read_list_end
       end
     end
 

Added: incubator/thrift/trunk/lib/rb/spec/protocol_spec.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/protocol_spec.rb?rev=668946&view=auto
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/protocol_spec.rb (added)
+++ incubator/thrift/trunk/lib/rb/spec/protocol_spec.rb Tue Jun 17 18:06:15 2008
@@ -0,0 +1,98 @@
+require File.dirname(__FILE__) + '/spec_helper'
+
+class ThriftSpec < Spec::ExampleGroup
+  include Thrift
+
+  before(:each) do
+    @prot = Protocol.new(mock("MockTransport"))
+  end
+
+  describe Protocol do
+    # most of the methods are stubs, so we can ignore them
+    it "should write out a field nicely" do
+      @prot.should_receive(:write_field_begin).with('field', 'type', 'fid').ordered
+      @prot.should_receive(:write_type).with('type', 'value').ordered
+      @prot.should_receive(:write_field_end).ordered
+      @prot.write_field('field', 'type', 'fid', 'value')
+    end
+
+    it "should write out the different types" do
+      @prot.should_receive(:write_bool).with('bool').ordered
+      @prot.should_receive(:write_byte).with('byte').ordered
+      @prot.should_receive(:write_double).with('double').ordered
+      @prot.should_receive(:write_i16).with('i16').ordered
+      @prot.should_receive(:write_i32).with('i32').ordered
+      @prot.should_receive(:write_i64).with('i64').ordered
+      @prot.should_receive(:write_string).with('string').ordered
+      struct = mock('Struct')
+      struct.should_receive(:write).with(@prot).ordered
+      @prot.write_type(Types::BOOL, 'bool')
+      @prot.write_type(Types::BYTE, 'byte')
+      @prot.write_type(Types::DOUBLE, 'double')
+      @prot.write_type(Types::I16, 'i16')
+      @prot.write_type(Types::I32, 'i32')
+      @prot.write_type(Types::I64, 'i64')
+      @prot.write_type(Types::STRING, 'string')
+      @prot.write_type(Types::STRUCT, struct)
+      # all other types are not implemented
+      [Types::STOP, Types::VOID, Types::MAP, Types::SET, Types::LIST].each do |type|
+        lambda { @prot.write_type(type, type.to_s) }.should raise_error(NotImplementedError)
+      end
+    end
+
+    it "should read the different types" do
+      @prot.should_receive(:read_bool).ordered
+      @prot.should_receive(:read_byte).ordered
+      @prot.should_receive(:read_i16).ordered
+      @prot.should_receive(:read_i32).ordered
+      @prot.should_receive(:read_i64).ordered
+      @prot.should_receive(:read_double).ordered
+      @prot.should_receive(:read_string).ordered
+      @prot.read_type(Types::BOOL)
+      @prot.read_type(Types::BYTE)
+      @prot.read_type(Types::I16)
+      @prot.read_type(Types::I32)
+      @prot.read_type(Types::I64)
+      @prot.read_type(Types::DOUBLE)
+      @prot.read_type(Types::STRING)
+      # all other types are not implemented
+      [Types::STOP, Types::VOID, Types::MAP, Types::SET, Types::LIST].each do |type|
+        lambda { @prot.read_type(type) }.should raise_error(NotImplementedError)
+      end
+    end
+
+    it "should skip the basic types" do
+      @prot.should_receive(:read_bool).ordered
+      @prot.should_receive(:read_byte).ordered
+      @prot.should_receive(:read_i16).ordered
+      @prot.should_receive(:read_i32).ordered
+      @prot.should_receive(:read_i64).ordered
+      @prot.should_receive(:read_double).ordered
+      @prot.should_receive(:read_string).ordered
+      @prot.read_type(Types::BOOL)
+      @prot.read_type(Types::BYTE)
+      @prot.read_type(Types::I16)
+      @prot.read_type(Types::I32)
+      @prot.read_type(Types::I64)
+      @prot.read_type(Types::DOUBLE)
+      @prot.read_type(Types::STRING)
+    end
+
+    it "should skip structs" do
+      real_skip = @prot.method(:skip)
+      @prot.should_receive(:read_struct_begin).ordered
+      @prot.should_receive(:read_field_begin).exactly(4).times.and_return(
+        ['field 1', Types::STRING, 1],
+        ['field 2', Types::I32, 2],
+        ['field 3', Types::MAP, 3],
+        [nil, Types::STOP, 0]
+      )
+      @prot.should_receive(:skip).with(Types::STRING).ordered
+      @prot.should_receive(:skip).with(Types::I32).ordered
+      @prot.should_receive(:skip).with(Types::MAP).ordered
+      @prot.should_receive(:read_field_end).exactly(3).times
+      @prot.should_receive(:read_struct_end).ordered
+      real_skip.call(Types::STRUCT)
+    end
+  end
+end