Merge branch 'new-hook-system'

This implements a new system to hook into middleware sequences
that actually works.

lib/vagrant/action/builder.rb

@@ -17,6 +17,12 @@ module Vagrant
     #     Vagrant::Action.run(app)
     #
     class Builder
+      # This is the stack of middlewares added. This should NOT be used
+      # directly.
+      #
+      # @return [Array]
+      attr_reader :stack
+
       # This is a shortcut for a middleware sequence with only one item
       # in it. For a description of the arguments and the documentation, please
       # see {#use} instead.
@@ -26,6 +32,18 @@ module Vagrant
         new.use(middleware, *args, &block)
       end
 
+      def initialize
+        @stack = []
+      end
+
+      # Implement a custom copy that copies the stack variable over so that
+      # we don't clobber that.
+      def initialize_copy(original)
+        super
+
+        @stack = original.stack.dup
+      end
+
       # Returns a mergeable version of the builder. If `use` is called with
       # the return value of this method, then the stack will merge, instead
       # of being treated as a separate single middleware.
@@ -109,19 +127,27 @@ module Vagrant
       # @param [Vagrant::Action::Environment] env The action environment
       # @return [Object] A callable object
       def to_app(env)
+        app_stack = nil
+
+        # If we have action hooks, then we apply them
+        if env[:action_hooks]
+          builder = self.dup
+
+          # Apply all the hooks to the new builder instance
+          env[:action_hooks].each do |hook|
+            hook.apply(builder)
+          end
+
+          # The stack is now the result of the new builder
+          app_stack = builder.stack.dup
+        end
+
+        # If we don't have a stack then default to using our own
+        app_stack ||= stack.dup
+
         # Wrap the middleware stack with the Warden to provide a consistent
         # and predictable behavior upon exceptions.
-        Warden.new(stack.dup, env)
+        Warden.new(app_stack, env)
-      end
-
-      protected
-
-      # Returns the current stack of middlewares. You probably won't
-      # need to use this directly, and it's recommended that you don't.
-      #
-      # @return [Array]
-      def stack
-        @stack ||= []
       end
     end
   end

lib/vagrant/action/hook.rb

@@ -0,0 +1,103 @@
+module Vagrant
+  module Action
+    # This class manages hooks into existing {Builder} stacks, and lets you
+    # add and remove middleware classes. This is the primary method by which
+    # plugins can hook into built-in middleware stacks.
+    class Hook
+      # This is a hash of the middleware to prepend to a certain
+      # other middleware.
+      #
+      # @return [Hash<Class, Array<Class>>]
+      attr_reader :before_hooks
+
+      # This is a hash of the middleware to append to a certain other
+      # middleware.
+      #
+      # @return [Hash<Class, Array<Class>>]
+      attr_reader :after_hooks
+
+      # This is a list of the hooks to just prepend to the beginning
+      #
+      # @return [Array<Class>]
+      attr_reader :prepend_hooks
+
+      # This is a list of the hooks to just append to the end
+      #
+      # @return [Array<Class>]
+      attr_reader :append_hooks
+
+      def initialize
+        @before_hooks  = Hash.new { |h, k| h[k] = [] }
+        @after_hooks   = Hash.new { |h, k| h[k] = [] }
+        @prepend_hooks = []
+        @append_hooks  = []
+      end
+
+      # Add a middleware before an existing middleware.
+      #
+      # @param [Class] existing The existing middleware.
+      # @param [Class] new The new middleware.
+      def before(existing, new)
+        @before_hooks[existing] << new
+      end
+
+      # Add a middleware after an existing middleware.
+      #
+      # @param [Class] existing The existing middleware.
+      # @param [Class] new The new middleware.
+      def after(existing, new)
+        @after_hooks[existing] << new
+      end
+
+      # Append a middleware to the end of the stack. Note that if the
+      # middleware sequence ends early, then the new middleware won't
+      # be run.
+      #
+      # @param [Class] new The middleware to append.
+      def append(new)
+        @append_hooks << new
+      end
+
+      # Prepend a middleware to the beginning of the stack.
+      #
+      # @param [Class] new The new middleware to prepend.
+      def prepend(new)
+        @prepend_hooks << new
+      end
+
+      # This applies the given hook to a builder. This should not be
+      # called directly.
+      #
+      # @param [Builder] builder
+      def apply(builder)
+        # Prepends first
+        @prepend_hooks.each do |klass|
+          builder.insert(0, klass)
+        end
+
+        # Appends
+        @append_hooks.each do |klass|
+          builder.use(klass)
+        end
+
+        # Before hooks
+        @before_hooks.each do |key, list|
+          next if !builder.index(key)
+
+          list.each do |klass|
+            builder.insert_before(key, klass)
+          end
+        end
+
+        # After hooks
+        @after_hooks.each do |key, list|
+          next if !builder.index(key)
+
+          list.each do |klass|
+            builder.insert_after(key, klass)
+          end
+        end
+      end
+    end
+  end
+end

lib/vagrant/action/runner.rb

@@ -1,5 +1,6 @@
 require 'log4r'
 
+require 'vagrant/action/hook'
 require 'vagrant/util/busy'
 
 # TODO:
@@ -27,6 +28,19 @@ module Vagrant
         environment.merge!(@lazy_globals.call) if @lazy_globals
         environment.merge!(options || {})
 
+        # Setup the action hooks
+        hooks = Vagrant.plugin("2").manager.action_hooks
+        if !hooks.empty?
+          @logger.info("Preparing hooks for middleware sequence...")
+          environment[:action_hooks] = hooks.map do |hook_proc|
+            Hook.new.tap do |h|
+              hook_proc.call(h)
+            end
+          end
+
+          @logger.info("#{environment[:action_hooks].length} hooks defined.")
+        end
+
         # Run the action chain in a busy block, marking the environment as
         # interrupted if a SIGINT occurs, and exiting cleanly once the
         # chain has been run.

lib/vagrant/action/warden.rb

@@ -16,9 +16,9 @@ module Vagrant
       attr_accessor :actions, :stack
 
       def initialize(actions, env)
-        @stack = []
+        @stack      = []
-        @actions = actions.map { |m| finalize_action(m, env) }
+        @actions    = actions.map { |m| finalize_action(m, env) }
-        @logger  = Log4r::Logger.new("vagrant::action::warden")
+        @logger     = Log4r::Logger.new("vagrant::action::warden")
         @last_error = nil
       end
 

lib/vagrant/plugin/v2/components.rb

@@ -6,12 +6,19 @@ module Vagrant
       # components, and the actual container of those components. This
       # removes a bit of state overhead from the plugin class itself.
       class Components
+        # This contains all the action hooks.
+        #
+        # @return [Array<Proc>]
+        attr_reader :action_hooks
+
         # This contains all the configuration plugins by scope.
         #
         # @return [Hash<Symbol, Registry>]
         attr_reader :configs
 
         def initialize
+          @action_hooks = []
+
           # Create the configs hash which defaults to a registry
           @configs = Hash.new { |h, k| h[k] = Registry.new }
         end

lib/vagrant/plugin/v2/manager.rb

@@ -14,6 +14,19 @@ module Vagrant
           @registered = []
         end
 
+        # This returns all the action hooks.
+        #
+        # @return [Array]
+        def action_hooks
+          result = []
+
+          @registered.each do |plugin|
+            result += plugin.components.action_hooks
+          end
+
+          result
+        end
+
         # This returns all the registered commands.
         #
         # @return [Hash]

lib/vagrant/plugin/v2/plugin.rb

@@ -65,18 +65,12 @@ module Vagrant
         # is run. This allows plugin authors to hook into things like VM
         # bootup, VM provisioning, etc.
         #
-        # @param [Symbol] name Name of the action.
+        # @param [String] name Name of the action.
         # @return [Array] List of the hooks for the given action.
         def self.action_hook(name, &block)
-          # Get the list of hooks for the given hook name
+          # The name is currently not used but we want it for the future.
-          data[:action_hooks] ||= {}
-          hooks = data[:action_hooks][name.to_sym] ||= []
 
-          # Return the list if we don't have a block
+          components.action_hooks << block
-          return hooks if !block_given?
-
-          # Otherwise add the block to the list of hooks for this action.
-          hooks << block
         end
 
         # Defines additional command line commands available by key. The key

test/unit/vagrant/action/builder_test.rb

@@ -2,7 +2,6 @@ require File.expand_path("../../../base", __FILE__)
 
 describe Vagrant::Action::Builder do
   let(:data) { { :data => [] } }
-  let(:instance) { described_class.new }
 
   # This returns a proc that can be used with the builder
   # that simply appends data to an array in the env.
@@ -10,13 +9,20 @@ describe Vagrant::Action::Builder do
     Proc.new { |env| env[:data] << data }
   end
 
+  context "copying" do
+    it "should copy the stack" do
+      copy = subject.dup
+      copy.stack.object_id.should_not == subject.stack.object_id
+    end
+  end
+
   context "build" do
     it "should provide build as a shortcut for basic sequences" do
       data = {}
       proc = Proc.new { |env| env[:data] = true }
 
-      instance = described_class.build(proc)
+      subject = described_class.build(proc)
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == true
     end
@@ -27,8 +33,8 @@ describe Vagrant::Action::Builder do
       data = {}
       proc = Proc.new { |env| env[:data] = true }
 
-      instance.use proc
+      subject.use proc
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == true
     end
@@ -38,9 +44,9 @@ describe Vagrant::Action::Builder do
       proc1 = Proc.new { |env| env[:one] = true }
       proc2 = Proc.new { |env| env[:two] = true }
 
-      instance.use proc1
+      subject.use proc1
-      instance.use proc2
+      subject.use proc2
-      instance.call(data)
+      subject.call(data)
 
       data[:one].should == true
       data[:two].should == true
@@ -66,9 +72,9 @@ describe Vagrant::Action::Builder do
 
   context "inserting" do
     it "can insert at an index" do
-      instance.use appender_proc(1)
+      subject.use appender_proc(1)
-      instance.insert(0, appender_proc(2))
+      subject.insert(0, appender_proc(2))
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2, 1]
     end
@@ -78,48 +84,48 @@ describe Vagrant::Action::Builder do
       bar_proc = appender_proc(2)
       def bar_proc.name; :bar; end
 
-      instance.use appender_proc(1)
+      subject.use appender_proc(1)
-      instance.use bar_proc
+      subject.use bar_proc
-      instance.insert_before :bar, appender_proc(3)
+      subject.insert_before :bar, appender_proc(3)
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [1, 3, 2]
     end
 
     it "can insert next to a previous object" do
       proc2 = appender_proc(2)
-      instance.use appender_proc(1)
+      subject.use appender_proc(1)
-      instance.use proc2
+      subject.use proc2
-      instance.insert(proc2, appender_proc(3))
+      subject.insert(proc2, appender_proc(3))
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [1, 3, 2]
     end
 
     it "can insert before" do
-      instance.use appender_proc(1)
+      subject.use appender_proc(1)
-      instance.insert_before 0, appender_proc(2)
+      subject.insert_before 0, appender_proc(2)
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2, 1]
     end
 
     it "can insert after" do
-      instance.use appender_proc(1)
+      subject.use appender_proc(1)
-      instance.use appender_proc(3)
+      subject.use appender_proc(3)
-      instance.insert_after 0, appender_proc(2)
+      subject.insert_after 0, appender_proc(2)
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [1, 2, 3]
     end
 
     it "raises an exception if an invalid object given for insert" do
-      expect { instance.insert "object", appender_proc(1) }.
+      expect { subject.insert "object", appender_proc(1) }.
         to raise_error(RuntimeError)
     end
 
     it "raises an exception if an invalid object given for insert_after" do
-      expect { instance.insert_after "object", appender_proc(1) }.
+      expect { subject.insert_after "object", appender_proc(1) }.
         to raise_error(RuntimeError)
     end
   end
@@ -129,9 +135,9 @@ describe Vagrant::Action::Builder do
       proc1 = appender_proc(1)
       proc2 = appender_proc(2)
 
-      instance.use proc1
+      subject.use proc1
-      instance.replace proc1, proc2
+      subject.replace proc1, proc2
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2]
     end
@@ -140,9 +146,9 @@ describe Vagrant::Action::Builder do
       proc1 = appender_proc(1)
       proc2 = appender_proc(2)
 
-      instance.use proc1
+      subject.use proc1
-      instance.replace 0, proc2
+      subject.replace 0, proc2
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2]
     end
@@ -152,10 +158,10 @@ describe Vagrant::Action::Builder do
     it "can delete by object" do
       proc1 = appender_proc(1)
 
-      instance.use proc1
+      subject.use proc1
-      instance.use appender_proc(2)
+      subject.use appender_proc(2)
-      instance.delete proc1
+      subject.delete proc1
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2]
     end
@@ -163,12 +169,28 @@ describe Vagrant::Action::Builder do
     it "can delete by index" do
       proc1 = appender_proc(1)
 
-      instance.use proc1
+      subject.use proc1
-      instance.use appender_proc(2)
+      subject.use appender_proc(2)
-      instance.delete 0
+      subject.delete 0
-      instance.call(data)
+      subject.call(data)
 
       data[:data].should == [2]
     end
   end
+
+  describe "action hooks" do
+    it "applies them properly" do
+      hook = double("hook")
+      hook.stub(:apply) do |builder|
+        builder.use appender_proc(2)
+      end
+
+      data[:action_hooks] = [hook]
+
+      subject.use appender_proc(1)
+      subject.call(data)
+
+      data[:data].should == [1, 2]
+    end
+  end
 end

test/unit/vagrant/action/hook_test.rb

@@ -0,0 +1,68 @@
+require File.expand_path("../../../base", __FILE__)
+
+require "vagrant/action/builder"
+require "vagrant/action/hook"
+
+describe Vagrant::Action::Hook do
+  describe "defaults" do
+    its("after_hooks")   { should be_empty }
+    its("before_hooks")  { should be_empty }
+    its("append_hooks")  { should be_empty }
+    its("prepend_hooks") { should be_empty }
+  end
+
+  describe "before hooks" do
+    let(:existing) { "foo" }
+
+    it "should append them" do
+      subject.before(existing, 1)
+      subject.before(existing, 2)
+
+      subject.before_hooks[existing].should == [1, 2]
+    end
+  end
+
+  describe "after hooks" do
+    let(:existing) { "foo" }
+
+    it "should append them" do
+      subject.after(existing, 1)
+      subject.after(existing, 2)
+
+      subject.after_hooks[existing].should == [1, 2]
+    end
+  end
+
+  describe "append" do
+    it "should make a list" do
+      subject.append(1)
+      subject.append(2)
+
+      subject.append_hooks.should == [1, 2]
+    end
+  end
+
+  describe "prepend" do
+    it "should make a list" do
+      subject.prepend(1)
+      subject.prepend(2)
+
+      subject.prepend_hooks.should == [1, 2]
+    end
+  end
+
+  describe "applying" do
+    let(:builder) { Vagrant::Action::Builder.new }
+
+    it "should build the proper stack" do
+      subject.prepend("1")
+      subject.append("9")
+      subject.after("1", "2")
+      subject.before("9", "8")
+
+      subject.apply(builder)
+
+      builder.stack.map(&:first).should == %w[1 2 8 9]
+    end
+  end
+end

test/unit/vagrant/plugin/v2/plugin_test.rb

@@ -29,7 +29,7 @@ describe Vagrant::Plugin::V2::Plugin do
         action_hook("foo") { "bar" }
       end
 
-      hooks = plugin.action_hook("foo")
+      hooks = plugin.components.action_hooks
       hooks.length.should == 1
       hooks[0].call.should == "bar"
     end