From: Kevin Clark Date: Fri, 18 Jul 2008 22:03:48 +0000 (+0000) Subject: rb: Display field name in type-checking error [THRIFT-78] X-Git-Tag: 0.2.0~466 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=b58633945386f0a879d8da58b271fe1bf0de3cd5;p=common%2Fthrift.git rb: Display field name in type-checking error [THRIFT-78] Author: Kevin Ballard git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@678059 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/rb/lib/thrift/struct.rb b/lib/rb/lib/thrift/struct.rb index d8e8f658..10f2ea07 100644 --- 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) if Thrift.type_checking + Thrift.check_type(value, type, 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] ) if Thrift.type_checking + Thrift.check_type(value, klass::FIELDS.values.find { |f| f[:name].to_s == field.to_s }[:type], 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 6666a486..28fdae23 100644 --- a/lib/rb/lib/thrift/types.rb +++ b/lib/rb/lib/thrift/types.rb @@ -25,7 +25,7 @@ module Thrift class TypeError < Exception end - def self.check_type(value, type) + def self.check_type(value, type, name) return if value.nil? klasses = case type when Types::VOID @@ -48,7 +48,7 @@ module Thrift Array end valid = klasses && [*klasses].any? { |klass| klass === value } - raise TypeError, "Expected #{type_name(type)}, received #{value.class}" unless valid + raise TypeError, "Expected #{type_name(type)}, received #{value.class} for field #{name}" unless valid end def self.type_name(type) diff --git a/lib/rb/spec/struct_spec.rb b/lib/rb/spec/struct_spec.rb index eb0542be..22457782 100644 --- a/lib/rb/spec/struct_spec.rb +++ b/lib/rb/spec/struct_spec.rb @@ -177,7 +177,7 @@ class ThriftStructSpec < Spec::ExampleGroup it "should support optional type-checking in Thrift::Struct.new" do Thrift.type_checking = true begin - lambda { Hello.new(:greeting => 3) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum") + lambda { Hello.new(:greeting => 3) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field greeting") ensure Thrift.type_checking = false end @@ -188,7 +188,7 @@ class ThriftStructSpec < Spec::ExampleGroup Thrift.type_checking = true begin hello = Hello.new - lambda { hello.greeting = 3 }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum") + lambda { hello.greeting = 3 }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field greeting") ensure Thrift.type_checking = false end diff --git a/lib/rb/spec/types_spec.rb b/lib/rb/spec/types_spec.rb index 564d9a69..4b3d7eaf 100644 --- a/lib/rb/spec/types_spec.rb +++ b/lib/rb/spec/types_spec.rb @@ -16,31 +16,40 @@ class ThriftTypesSpec < Spec::ExampleGroup Thrift.type_checking = true begin # lambda { Thrift.check_type(nil, Types::STOP) }.should raise_error(TypeError) - lambda { Thrift.check_type(3, Types::STOP) }.should raise_error(TypeError) - lambda { Thrift.check_type(nil, Types::VOID) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(3, Types::VOID) }.should raise_error(TypeError) - lambda { Thrift.check_type(true, Types::BOOL) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(3, Types::BOOL) }.should raise_error(TypeError) - # lambda { Thrift.check_type(nil, Types::BOOL) }.should raise_error(TypeError) - lambda { Thrift.check_type(42, Types::BYTE) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(42, Types::I16) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(42, Types::I32) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(42, Types::I64) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(3.14, Types::I32) }.should raise_error(TypeError) - lambda { Thrift.check_type(3.14, Types::DOUBLE) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(3, Types::DOUBLE) }.should raise_error(TypeError) - lambda { Thrift.check_type("3", Types::STRING) }.should_not raise_error(TypeError) - lambda { Thrift.check_type(3, Types::STRING) }.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) }.should_not raise_error(TypeError) - lambda { Thrift.check_type("foo", Types::STRUCT) }.should raise_error(TypeError) - lambda { Thrift.check_type({:foo => 1}, Types::MAP) }.should_not raise_error(TypeError) - lambda { Thrift.check_type([1], Types::MAP) }.should raise_error(TypeError) - lambda { Thrift.check_type([1], Types::LIST) }.should_not raise_error(TypeError) - lambda { Thrift.check_type({:foo => 1}, Types::LIST) }.should raise_error(TypeError) - lambda { Thrift.check_type(Set.new([1,2]), Types::SET) }.should_not raise_error(TypeError) - lambda { Thrift.check_type([1,2], Types::SET) }.should raise_error(TypeError) - lambda { Thrift.check_type({:foo => true}, Types::SET) }.should raise_error(TypeError) + 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 + 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