(#7059) handle inherited action binding scope

We implemented the naive version of inheritance in the faces, in which an
action declared on a superclass remains bound to that even when accessed
from a subclass.

As a consequence our visibility of options on inherited classes is actually
much more limited than it should be, and the actions we inherit would never
see the options they should correctly have.

To fix this we correct the binding bug and handle lookup correctly over
inheritance in our code.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>

3 files changed, 41 insertions, 3 deletions

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 302e61901..db338e39e 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -11,6 +11,15 @@ class Puppet::Interface::Action
     attrs.each do |k, v| send("#{k}=", v) end
   end
 
+  # This is not nice, but it is the easiest way to make us behave like the
+  # Ruby Method object rather than UnboundMethod.  Duplication is vaguely
+  # annoying, but at least we are a shallow clone. --daniel 2011-04-12
+  def __dup_and_rebind_to(to)
+    bound_version = self.dup
+    bound_version.instance_variable_set(:@face, to)
+    return bound_version
+  end
+
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
index bb0e5bf57..d75697afa 100644
--- a/lib/puppet/interface/action_manager.rb
+++ b/lib/puppet/interface/action_manager.rb
@@ -35,9 +35,16 @@ module Puppet::Interface::ActionManager
     result = @actions[name.to_sym]
     if result.nil?
       if self.is_a?(Class) and superclass.respond_to?(:get_action)
-        result = superclass.get_action(name)
+        found = superclass.get_action(name)
       elsif self.class.respond_to?(:get_action)
-        result = self.class.get_action(name)
+        found = self.class.get_action(name)
+      end
+
+      if found then
+        # This is not the nicest way to make action equivalent to the Ruby
+        # Method object, rather than UnboundMethod, but it will do for now,
+        # and we only have to make this change in *one* place. --daniel 2011-04-12
+        result = @actions[name.to_sym] = found.__dup_and_rebind_to(self)
       end
     end
     return result
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 2365d5cac..e52b45d8a 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -162,11 +162,17 @@ describe Puppet::Interface do
   end
 
   describe "with inherited options" do
-    let :face do
+    let :parent do
       parent = Class.new(subject)
       parent.option("--inherited")
+      parent.action(:parent_action) do end
+      parent
+    end
+
+    let :face do
       face = parent.new(:example, '0.2.1')
       face.option("--local")
+      face.action(:face_action) do end
       face
     end
 
@@ -174,6 +180,22 @@ describe Puppet::Interface do
       it "should list inherited options" do
         face.options.should =~ [:inherited, :local]
       end
+
+      it "should see all options on face actions" do
+        face.get_action(:face_action).options.should =~ [:inherited, :local]
+      end
+
+      it "should see all options on inherited actions accessed on the subclass" do
+        face.get_action(:parent_action).options.should =~ [:inherited, :local]
+      end
+
+      it "should not see subclass actions on the parent class" do
+        parent.options.should =~ [:inherited]
+      end
+
+      it "should not see subclass actions on actions accessed on the parent class" do
+        parent.get_action(:parent_action).options.should =~ [:inherited]
+      end
     end
 
     describe "#get_option" do