don't go into a loop when initializing a recursive Message

fixes mozy#21
fixes mozy#20

lib/protocol_buffers/runtime/field.rb

@@ -120,12 +120,43 @@ def initialize(otype, name, tag, opts = {})
       @opts = opts.dup
     end
 
-    def add_methods_to(klass)
+    def add_reader_to(klass)
       klass.class_eval <<-EOF, __FILE__, __LINE__+1
-        attr_reader :#{name}
+      def #{name}
+        if @set_fields[#{tag}] == nil
+          # first access of this field, generate it
+          initialize_field(#{tag})
+        end
+        @#{name}
+      end
       EOF
+    end
+
+    def add_writer_to(klass)
+      klass.class_eval <<-EOF, __FILE__, __LINE__+1
+        def #{name}=(value)
+          field = fields[#{tag}]
+          if value.nil?
+            @set_fields[#{tag}] = false
+            @#{name} = field.default_value
+          else
+            field.check_valid(value)
+            @set_fields[#{tag}] = true
+            @#{name} = value
+            if @parent_for_notify
+              @parent_for_notify.default_changed(@tag_for_notify)
+              @parent_for_notify = @tag_for_notify = nil
+            end
+          end
+        end
+      EOF
+    end
+
+    def add_methods_to(klass)
       if repeated?
         klass.class_eval <<-EOF, __FILE__, __LINE__+1
+          attr_reader :#{name}
+
           def #{name}=(value)
             if value.nil?
               @#{name}.clear
@@ -138,23 +169,10 @@ def #{name}=(value)
           def has_#{name}?; true; end
         EOF
       else
-        klass.class_eval <<-EOF, __FILE__, __LINE__+1
-          def #{name}=(value)
-            field = fields[#{tag}]
-            if value.nil?
-              @set_fields[#{tag}] = false
-              @#{name} = field.default_value
-            else
-              field.check_valid(value)
-              @set_fields[#{tag}] = true
-              @#{name} = value
-              if @parent_for_notify
-                @parent_for_notify.default_changed(@tag_for_notify)
-                @parent_for_notify = @tag_for_notify = nil
-              end
-            end
-          end
+        add_reader_to(klass)
+        add_writer_to(klass)
 
+        klass.class_eval <<-EOF, __FILE__, __LINE__+1
           def has_#{name}?
             value_for_tag?(#{tag})
           end

lib/protocol_buffers/runtime/message.rb

@@ -233,21 +233,15 @@ class Message
     #   message.attributes = attributes
     def initialize(attributes = {})
       @set_fields = []
-
       fields.each do |tag, field|
+        # repeated fields are always "set"
         if field.repeated?
+          @set_fields[tag] = true
+          # TODO: it seems a shame to do this on every object initialization,
+          # even if the repeated fields are never accessed
           self.instance_variable_set("@#{field.name}", RepeatedField.new(field))
-          @set_fields[tag] = true # repeated fields are always "set"
-        else
-          value = field.default_value
-          self.__send__("#{field.name}=", value)
-          @set_fields[tag] = false
-          if field.class == Field::MessageField
-            value.notify_on_change(self, tag)
-          end
         end
       end
-
       self.attributes = attributes
     end
 
@@ -502,5 +496,17 @@ def self.gen_methods! # :NODOC:
       # some point.
     end
 
+    protected
+
+    def initialize_field(tag)
+      field = fields[tag]
+      new_value = field.default_value
+      self.instance_variable_set("@#{field.name}", new_value)
+      if field.class == Field::MessageField
+        new_value.notify_on_change(self, tag)
+      end
+      @set_fields[tag] = false
+    end
+
   end
 end

spec/runtime_spec.rb

@@ -81,6 +81,45 @@
     a1.sub2.has_subsub1?.should == true
   end
 
+  it "allows directly recursive sub-messages" do
+    ProtocolBuffers::Compiler.compile_and_load_string <<-EOS
+     package foo;
+      message Foo {
+        optional int32 payload = 1;
+        optional Foo foo = 2;
+      }
+    EOS
+
+    foo = Foo::Foo.new
+    foo.has_foo?.should == false
+    foo.foo.payload = 17
+    foo.has_foo?.should == true
+    foo.foo.has_foo?.should == false
+  end
+
+  it "allows indirectly recursive sub-messages" do
+    ProtocolBuffers::Compiler.compile_and_load_string <<-EOS
+     package foo;
+      message Foo {
+        optional int32 payload = 1;
+        optional Bar bar = 2;
+      }
+
+      message Bar {
+        optional Foo foo = 1;
+        optional int32 payload = 2;
+      }
+    EOS
+
+    foo = Foo::Foo.new
+    foo.has_bar?.should == false
+    foo.bar.payload = 17
+    foo.has_bar?.should == true
+    foo.bar.has_foo?.should == false
+    foo.bar.foo.payload = 23
+    foo.bar.has_foo?.should == true
+  end
+
   it "pretends that repeated fields are arrays" do
     # make sure our RepeatedField class acts like a normal Array
     ProtocolBuffers::Compiler.compile_and_load_string <<-EOS