Merge pull request #252 from skippy/fix-downcasting

don't downcast when assigning to a field_array

lib/protobuf/field/field_array.rb

@@ -66,6 +66,8 @@ def normalize(value)
 
         if field.is_a?(::Protobuf::Field::EnumField)
           field.type_class.fetch(value)
+        elsif field.is_a?(::Protobuf::Field::MessageField) && value.is_a?(field.type_class)
+          value
         elsif field.is_a?(::Protobuf::Field::MessageField) && value.respond_to?(:to_hash)
           field.type_class.new(value.to_hash)
         else

lib/protobuf/field/message_field.rb

@@ -9,7 +9,7 @@ class MessageField < BaseField
       #
 
       def acceptable?(val)
-        unless val.instance_of?(type_class) || val.respond_to?(:to_hash)
+        unless val.is_a?(type_class) || val.is_a?(Hash)
           fail TypeError, "Expected value of type '#{type_class}' for field #{name}, but got '#{val.class}'"
         end
 

spec/lib/protobuf/field/field_array_spec.rb

@@ -0,0 +1,75 @@
+require 'spec_helper'
+
+RSpec.describe Protobuf::Field::FieldArray do
+
+  class SomeBasicMessage < ::Protobuf::Message
+    optional :string, :field, 1
+  end
+
+  class MoreComplexMessage < SomeBasicMessage
+  end
+
+  class OtherBasicMessage < ::Protobuf::Message
+    optional :string, :other_field, 1
+  end
+
+  class SomeRepeatMessage < ::Protobuf::Message
+    optional :string, :some_string, 1
+    repeated :string, :multiple_strings, 2
+    repeated SomeBasicMessage, :multiple_basic_msgs, 3
+  end
+
+  let(:instance) { SomeRepeatMessage.new }
+
+  %w(<< push).each do |method|
+    describe "\##{method}" do
+      context 'when applied to a string field array' do
+        it 'adds a string' do
+          expect(instance.multiple_strings).to be_empty
+          instance.multiple_strings.send(method, 'string 1')
+          expect(instance.multiple_strings).to eq(['string 1'])
+          instance.multiple_strings.send(method, 'string 2')
+          expect(instance.multiple_strings).to eq(['string 1', 'string 2'])
+        end
+
+        it 'fails if not adding a string' do
+          expect { instance.multiple_strings.send(method, 100.0) }.to raise_error(TypeError)
+        end
+      end
+
+      context 'when applied to a MessageField field array' do
+        it 'adds a MessageField object' do
+          expect(instance.multiple_basic_msgs).to be_empty
+          basic_msg1 = SomeBasicMessage.new
+          instance.multiple_basic_msgs.send(method, basic_msg1)
+          expect(instance.multiple_basic_msgs).to eq([basic_msg1])
+          basic_msg2 = SomeBasicMessage.new
+          instance.multiple_basic_msgs.send(method, basic_msg2)
+          expect(instance.multiple_basic_msgs).to eq([basic_msg1, basic_msg2])
+        end
+
+        it 'adds a Hash from a MessageField object' do
+          expect(instance.multiple_basic_msgs).to be_empty
+          basic_msg1 = SomeBasicMessage.new
+          basic_msg1.field = 'my value'
+          instance.multiple_basic_msgs.send(method, basic_msg1.to_hash)
+          expect(instance.multiple_basic_msgs).to eq([basic_msg1])
+        end
+
+        it 'does not downcast a MessageField' do
+          expect(instance.multiple_basic_msgs).to be_empty
+          basic_msg1 = MoreComplexMessage.new
+          instance.multiple_basic_msgs.send(method, basic_msg1)
+          expect(instance.multiple_basic_msgs).to eq([basic_msg1])
+          expect(instance.multiple_basic_msgs.first).to be_a(MoreComplexMessage)
+        end
+
+        it 'fails if not adding the expected MessageField object' do
+          expect { instance.multiple_basic_msgs.send(method, 100.0) }.to raise_error(TypeError)
+          expect { instance.multiple_basic_msgs.send(method, OtherBasicMessage.new) }.to raise_error(TypeError)
+        end
+      end
+    end
+
+  end
+end