rb: Check container elements when Thrift.type_checking = true [THRIFT-104]

Author: Kevin Ballard <kevin@rapleaf.com>

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

diff --git a/lib/rb/lib/thrift/struct.rb b/lib/rb/lib/thrift/struct.rb
index d3de200..784fda5 100644 (file)
--- a/lib/rb/lib/thrift/struct.rb
+++ b/lib/rb/lib/thrift/struct.rb
@@ -6,7 +6,7 @@ module Thrift
     def initialize(d={})
       each_field do |fid, type, name, default|
         value = d.delete(name.to_s) { d.delete(name.to_sym) { default.dup rescue default } }
-        Thrift.check_type(value, type, name) if Thrift.type_checking
+        Thrift.check_type(value, struct_fields[fid], name) if Thrift.type_checking
         instance_variable_set("@#{name}", value)
       end
       raise Exception, "Unknown keys given to #{self.class}.new: #{d.keys.join(", ")}" unless d.empty?
@@ -72,7 +72,7 @@ module Thrift
       fields.each do |field|
         klass.send :attr_reader, field
         klass.send :define_method, "#{field}=" do |value|
-          Thrift.check_type(value, klass::FIELDS.values.find { |f| f[:name].to_s == field.to_s }[:type], field) if Thrift.type_checking
+          Thrift.check_type(value, klass::FIELDS.values.find { |f| f[:name].to_s == field.to_s }, field) if Thrift.type_checking
           instance_variable_set("@#{field}", value)
         end
       end

diff --git a/lib/rb/lib/thrift/types.rb b/lib/rb/lib/thrift/types.rb
index 28fdae2..3a83a9f 100644 (file)
--- a/lib/rb/lib/thrift/types.rb
+++ b/lib/rb/lib/thrift/types.rb
@@ -25,9 +25,9 @@ module Thrift
   class TypeError < Exception
   end
 
-  def self.check_type(value, type, name)
-    return if value.nil?
-    klasses = case type
+  def self.check_type(value, field, name, skip_nil=true)
+    return if value.nil? and skip_nil
+    klasses = case field[:type]
               when Types::VOID
                 NilClass
               when Types::BOOL
@@ -48,7 +48,19 @@ module Thrift
                 Array
               end
     valid = klasses && [*klasses].any? { |klass| klass === value }
-    raise TypeError, "Expected #{type_name(type)}, received #{value.class} for field #{name}" unless valid
+    raise TypeError, "Expected #{type_name(field[:type])}, received #{value.class} for field #{name}" unless valid
+    # check elements now
+    case field[:type]
+    when Types::MAP
+      value.each_pair do |k,v|
+        check_type(k, field[:key], "#{name}.key", false)
+        check_type(v, field[:value], "#{name}.value", false)
+      end
+    when Types::SET, Types::LIST
+      value.each do |el|
+        check_type(el, field[:element], "#{name}.element", false)
+      end
+    end
   end
 
   def self.type_name(type)

diff --git a/lib/rb/spec/types_spec.rb b/lib/rb/spec/types_spec.rb
index 4b3d7ea..183a354 100644 (file)
--- a/lib/rb/spec/types_spec.rb
+++ b/lib/rb/spec/types_spec.rb
@@ -4,6 +4,14 @@ require File.dirname(__FILE__) + '/gen-rb/ThriftSpec_types'
 class ThriftTypesSpec < Spec::ExampleGroup
   include Thrift
 
+  before(:each) do
+    Thrift.type_checking = true
+  end
+
+  after(:each) do
+    Thrift.type_checking = false
+  end
+
   describe "Type checking" do
     it "should return the proper name for each type" do
       Thrift.type_name(Types::I16).should == "Types::I16"
@@ -13,52 +21,75 @@ class ThriftTypesSpec < Spec::ExampleGroup
     end
 
     it "should check types properly" do
-      Thrift.type_checking = true
-      begin
-        # lambda { Thrift.check_type(nil, Types::STOP) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::STOP, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(nil, Types::VOID, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::VOID, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(true, Types::BOOL, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::BOOL, :foo) }.should raise_error(TypeError)
-        # lambda { Thrift.check_type(nil, Types::BOOL, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::BYTE, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I16, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I32, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I64, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3.14, Types::I32, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(3.14, Types::DOUBLE, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::DOUBLE, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type("3", Types::STRING, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::STRING, :foo) }.should raise_error(TypeError)
-        hello = SpecNamespace::Hello.new
-        lambda { Thrift.check_type(hello, Types::STRUCT, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type("foo", Types::STRUCT, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => 1}, Types::MAP, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type([1], Types::MAP, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type([1], Types::LIST, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => 1}, Types::LIST, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(Set.new([1,2]), Types::SET, :foo) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type([1,2], Types::SET, :foo) }.should raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => true}, Types::SET, :foo) }.should raise_error(TypeError)
-      ensure
-        Thrift.type_checking = false
-      end
+      # lambda { Thrift.check_type(nil, Types::STOP) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(3,              {:type => Types::STOP},   :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil,            {:type => Types::VOID},   :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(3,              {:type => Types::VOID},   :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(true,           {:type => Types::BOOL},   :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(3,              {:type => Types::BOOL},   :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(42,             {:type => Types::BYTE},   :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(42,             {:type => Types::I16},    :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(42,             {:type => Types::I32},    :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(42,             {:type => Types::I64},    :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(3.14,           {:type => Types::I32},    :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(3.14,           {:type => Types::DOUBLE}, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(3,              {:type => Types::DOUBLE}, :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type("3",            {:type => Types::STRING}, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(3,              {:type => Types::STRING}, :foo) }.should raise_error(TypeError)
+      hello = SpecNamespace::Hello.new
+      lambda { Thrift.check_type(hello,          {:type => Types::STRUCT}, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type("foo",          {:type => Types::STRUCT}, :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type({:foo => 1},    {:type => Types::MAP},    :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type([1],            {:type => Types::MAP},    :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type([1],            {:type => Types::LIST},   :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type({:foo => 1},    {:type => Types::LIST},   :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(Set.new([1,2]), {:type => Types::SET},    :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type([1,2],          {:type => Types::SET},    :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type({:foo => true}, {:type => Types::SET},    :foo) }.should raise_error(TypeError)
     end
 
-    it "should give the TypeError a readable message" do
-      Thrift.type_checking = true
-      begin
-        lambda { Thrift.check_type(3, Types::STRING, :foo) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field foo")
-      ensure
-        Thrift.type_checking = false
-      end
+    it "should error out if nil is passed and skip_types is false" do
+      lambda { Thrift.check_type(nil, {:type => Types::BOOL},   :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::BYTE},   :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::I16},    :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::I32},    :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::I64},    :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::DOUBLE}, :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::STRING}, :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::STRUCT}, :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::LIST},   :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::SET},    :foo, false) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(nil, {:type => Types::MAP},    :foo, false) }.should raise_error(TypeError)
+    end
+
+    it "should check element types on containers" do
+      field = {:type => Types::LIST, :element => {:type => Types::I32}}
+      lambda { Thrift.check_type([1, 2], field, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type([1, nil, 2], field, :foo) }.should raise_error(TypeError)
+      field = {:type => Types::MAP, :key => {:type => Types::I32}, :value => {:type => Types::STRING}}
+      lambda { Thrift.check_type({1 => "one", 2 => "two"}, field, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type({1 => "one", nil => "nil"}, field, :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type({1 => nil, 2 => "two"}, field, :foo) }.should raise_error(TypeError)
+      field = {:type => Types::SET, :element => {:type => Types::I32}}
+      lambda { Thrift.check_type(Set.new([1, 2]), field, :foo) }.should_not raise_error(TypeError)
+      lambda { Thrift.check_type(Set.new([1, nil, 2]), field, :foo) }.should raise_error(TypeError)
+      lambda { Thrift.check_type(Set.new([1, 2.3, 2]), field, :foo) }.should raise_error(TypeError)
     end
 
-    it "should be disabled when Thrift.type_checking = false" do
-      pending "disabled, parents should check Thrift.type_checking"
-      Thrift.type_checking = false
-      lambda { Thrift.check_type(3, Types::STRING) }.should_not raise_error(TypeError)
+    it "should give the TypeError a readable message" do
+      msg = "Expected Types::STRING, received Fixnum for field foo"
+      lambda { Thrift.check_type(3, {:type => Types::STRING}, :foo) }.should raise_error(TypeError, msg)
+      msg = "Expected Types::STRING, received Fixnum for field foo.element"
+      field = {:type => Types::LIST, :element => {:type => Types::STRING}}
+      lambda { Thrift.check_type([3], field, :foo) }.should raise_error(TypeError, msg)
+      msg = "Expected Types::I32, received NilClass for field foo.element.key"
+      field = {:type => Types::LIST,
+               :element => {:type => Types::MAP,
+                            :key => {:type => Types::I32},
+                            :value => {:type => Types::I32}}}
+      lambda { Thrift.check_type([{nil => 3}], field, :foo) }.should raise_error(TypeError, msg)
+      msg = "Expected Types::I32, received NilClass for field foo.element.value"
+      lambda { Thrift.check_type([{1 => nil}], field, :foo) }.should raise_error(TypeError, msg)
     end
   end
 end