Bug #66: Reworking DynamicNode's method_missing, adding respond_to_mi…

…ssing?

Consolidated specialized constructor methods and marked intended internal
methods as private.  Throw a sensible NoMethodError when needed! The
tests checking for NoMethodError would pass... but only because of an
inscrutable error when a method was invoked on NilClass.

Tests that demonstrate the necessity of changes.

lib/om/xml/dynamic_node.rb

@@ -3,8 +3,8 @@ module XML
     #
     # Provides a natural syntax for using OM Terminologies to access values from xml Documents
     #
-    # *Note*: All of these examples assume that @article is an instance of OM::Samples::ModsArticle.  Look at that file to see the Terminology. 
-    # 
+    # *Note*: All of these examples assume that @article is an instance of OM::Samples::ModsArticle.  Look at that file to see the Terminology.
+    #
     # @example Return an array of the value(s) "start page" node(s) from the second issue node within the first journal node
     #   # Using DynamicNode syntax:
     #   @article.journal(0).issue(1).pages.start
@@ -38,37 +38,23 @@ def initialize(key, index, document, term, parent=nil)  ##TODO a real term objec
         self.parent = parent
       end
 
-      def method_missing (name, *args, &block)
-        if /=$/.match(name.to_s)
-          new_update_node(name, args)
-        elsif args.length > 1
-          new_update_node_with_index(name, args)
-        else
-          child = term_child_by_name(term.nil? ? parent.term : term, name)
-          if child
-            OM::XML::DynamicNode.new(name, args.first, @document, child, self)
-          else 
-            val.send(name, *args, &block)
-          end
-        end
+      # In practice, method_missing will respond 4 different ways:
+      # (1) ALL assignment operations are accepted/attempted as new nodes,
+      # (2) ANY operation with multiple arguments is accepted/attempted as a new node (w/ index),
+      # (3) With an auto-constructed sub DynamicNode object,
+      # (4) By handing off to val.  This is the only route that will return NoMethodError.
+      #
+      # Here we don't have args, so we cannot handle cases 2 and 3.  But we can at least do 1 and 4.
+      def respond_to_missing?(name, include_private = false)
+        /=$/.match(name.to_s) || val.respond_to?(name, include_private) || super
       end
 
-      def respond_to?(method)
-        super || val.respond_to?(method)
-      end
-
-      def new_update_node(name, args)
-        modified_name = name.to_s.chop.to_sym
-        child = term.retrieve_term(modified_name)
-        node = OM::XML::DynamicNode.new(modified_name, nil, @document, child, self)
-        node.val=args
-      end
-
-      def new_update_node_with_index(name, args)
-        index = args.shift
-        child = term.retrieve_term(name)
-        node = OM::XML::DynamicNode.new(name, index, @document, child, self)
-        node.val=args
+      def method_missing(name, *args, &block)
+        return new_update_node(name.to_s.chop.to_sym, nil, args) if /=$/.match(name.to_s)
+        return new_update_node(name, args.shift, args) if args.length > 1
+        child = term_child_by_name(term.nil? ? parent.term : term, name)
+        return OM::XML::DynamicNode.new(name, args.first, @document, child, self) if child
+        val.send(name, *args, &block)
       end
 
       def val=(args)
@@ -96,18 +82,9 @@ def val=(args)
         end
       end
 
-
-      def term_child_by_name(term, name)
-        if (term.kind_of? NamedTermProxy)
-           @document.class.terminology.retrieve_node(*(term.proxy_pointer.dup << name)) 
-        else
-          term.retrieve_term(name)
-        end
-      end
-
       # This resolves the target of this dynamic node into a reified Array
       # @return [Array]
-      def val 
+      def val
         query = xpath
         trim_text = !query.index("text()").nil?
         val = @document.find_by_xpath(query).collect {|node| (trim_text ? node.text.strip : node.text) }
@@ -123,7 +100,7 @@ def nodeset
       def delete
         nodeset.delete
       end
-      
+
       def inspect
         val.inspect
       end
@@ -146,7 +123,7 @@ def to_pointer
         else ### A pointer
           parent.nil? ? [key] : parent.to_pointer << key
         end
-      end 
+      end
 
       def xpath
         if parent.nil?
@@ -155,10 +132,8 @@ def xpath
           chain = retrieve_addressed_node( )
           '//' + chain.map { |n| n.xpath}.join('/')
         end
-
       end
 
-
       class AddressedNode
         attr_accessor :xpath, :key, :pointer
         def initialize (pointer, xpath, key)
@@ -167,15 +142,12 @@ def initialize (pointer, xpath, key)
           self.pointer = pointer
         end
       end
-     
+
       ##
       # This is very similar to Terminology#retrieve_term, however it expands proxy paths out into their cannonical paths
       def retrieve_addressed_node()
          chain = []
-
-         if parent
-           chain += parent.retrieve_addressed_node()
-         end
+         chain += parent.retrieve_addressed_node() if parent
          if (self.index)
            ### This is an index
            node = AddressedNode.new(key, term.xpath_relative, self)
@@ -191,12 +163,31 @@ def retrieve_addressed_node()
               p = p.retrieve_term(first)
               chain << AddressedNode.new(p, p.xpath_relative, self)
             end
-         else 
+         else
            chain << AddressedNode.new(key, term.xpath_relative, self)
          end
          chain
       end
 
+      private
+
+      # Only to be called by method_missing, hence the NoMethodError.
+      # We know term.sanitize_new_values would fail in .val= if we pass a nil term.
+      def new_update_node(name, index, args)
+        child = term.retrieve_term(name)
+        raise NoMethodError, "undefined method `#{name}' in OM::XML::DynamicNode for #{self}:#{self.class}" if child.nil?
+        node = OM::XML::DynamicNode.new(name, index, @document, child, self)
+        node.val = args
+      end
+
+      # Only to be called by method_missing
+      def term_child_by_name(term, name)
+        if (term.kind_of? NamedTermProxy)
+          @document.class.terminology.retrieve_node(*(term.proxy_pointer.dup << name))
+        else
+          term.retrieve_term(name)
+        end
+      end
 
     end
   end

spec/unit/dynamic_node_spec.rb

@@ -53,14 +53,14 @@ def self.xml_template
       expect(@sample.foo != ['nearby']).to be true
     end
   end
-        
-  
+
+
   describe "with a template" do
     before(:each) do
       @sample = OM::Samples::ModsArticle.from_xml( fixture( File.join("test_dummy_mods.xml") ) )
       @article = OM::Samples::ModsArticle.from_xml( fixture( File.join("mods_articles","hydrangea_article1.xml") ) )
     end
-    
+
     describe "dynamically created nodes" do
 
       it "should return build an array of values from the nodeset corresponding to the given term" do
@@ -95,7 +95,6 @@ def self.xml_template
         expect(arr).to eq ["FAMILY NAME", "Gautama"]
       end
 
-
       describe "setting attributes" do
         it "when they exist" do
           @article.title_info(0).main_title.main_title_lang = "ger"
@@ -106,18 +105,30 @@ def self.xml_template
           title.language = "rus"
           expect(@article.title_info(0).language).to eq ["rus"]
         end
-
       end
 
       it "should find elements two deep" do
-        #TODO reimplement so that method_missing with name is only called once.  Create a new method for name.
-        expect(@article.name.name_content.val).to eq ["Describes a person"]
-        expect(@article.name.name_content).to eq     ["Describes a person"]
-        expect(@article.name.name_content(0)).to eq  ["Describes a person"]
+        # TODO reimplement so that method_missing with name is only called once.  Create a new method for name.
+        expect(@article.name.name_content.val).to eq(["Describes a person"])
+        expect(@article.name.name_content    ).to eq(["Describes a person"])
+        expect(@article.name.name_content(0) ).to eq(["Describes a person"])
       end
 
-      it "should not find elements that don't  exist" do
-        expect(lambda {@article.name.hedgehog}).to raise_exception NoMethodError
+      it 'should offer some awareness for respond_to?' do
+        expect(@article.name.name_content.include?('Describes a person')).to be_truthy
+        expect(@article.name.name_content.respond_to?(:include?)).to be_truthy
+        expect(@article.name.name_content).to include('Describes a person')
+      end
+
+      it "should not find elements that don't exist" do
+        expect{@article.name.hedgehog     }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.hedgehog = 5 }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.hedgehog(5)  }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.hedgehog(5,1)}.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.name_content.hedgehog         }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.name_content.hedgehog = 'foo' }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.name_content.hedgehog(1)      }.to raise_exception NoMethodError, /hedgehog/
+        expect{@article.name.name_content.hedgehog(1,'foo')}.to raise_exception NoMethodError, /hedgehog/
       end
 
       it "should allow you to call methods on the return value" do
@@ -147,7 +158,7 @@ def self.xml_template
         expect(@article.subject.topic(1)).to eq ["TOPIC 2"]
         expect(@article.subject.topic(1).xpath).to eq "//oxns:subject/oxns:topic[2]"
       end
-      
+
       describe ".nodeset" do
         it "should return a Nokogiri NodeSet" do
           @article.update_values( {[{:journal=>0}, {:issue=>3}, :pages, :start]=>"434" })
@@ -164,7 +175,7 @@ def self.xml_template
         expect(@article.term_values(:journal, :title_info)).to eq ["all", "for", "the", "glory"]
         expect(@article).to be_changed
       end
-    
+
       it "should remove extra nodes if fewer are given than currently exist" do
         @article.journal.title_info = %W(one two three four five)
         @article.journal.title_info = %W(six seven)
@@ -183,7 +194,7 @@ def self.xml_template
           @article.name(0).last_name = "Hogginobble"
           expect(@article.name(0).last_name == @article.name(0).first_name).to be false
         end
-      end 
+      end
     end
   end
 end