action dependencies and unique requirement moved to actions/collection

lib/vagrant.rb

@@ -8,8 +8,8 @@ PROJECT_ROOT = File.join(libdir, '..') unless defined?(PROJECT_ROOT)
 end
 
 # The vagrant specific files which must be loaded prior to the rest
-%w{vagrant/util vagrant/actions/base vagrant/downloaders/base vagrant/actions/runner
-  vagrant/config vagrant/provisioners/base vagrant/provisioners/chef}.each do |f|
+%w{vagrant/util vagrant/actions/base vagrant/downloaders/base vagrant/actions/collection 
+  vagrant/actions/runner vagrant/config vagrant/provisioners/base vagrant/provisioners/chef}.each do |f|
   require File.expand_path(f, libdir)
 end
 

lib/vagrant/actions/base.rb

@@ -82,6 +82,20 @@ module Vagrant
       # all your own exceptions, otherwise it'll mask the initially raised
       # exception.
       def rescue(exception); end
+
+      # The following two methods are used for declaring action dependencies.
+      # For example, you require that the reload action be in place before 
+      # a your new FooAction you might do the following
+      #
+      # def follows; [Reload] end
+      
+      # This method is called when the runner is determining the actions that
+      # must precede a given action. You would say "This action follows [Action1, Action2]"
+      def follows; [] end
+      
+      # This method is called when the runner is determining the actions that
+      # must follow a given action. You would say "This action precedes [Action3, Action4]
+      def precedes; [] end
     end
 
     # An exception which occured within an action. This should be used instead of

lib/vagrant/actions/collection.rb

@@ -0,0 +1,36 @@
+module Vagrant
+  module Actions
+    class Collection < Array
+      def dependencies!
+        each_with_index do |action, i|
+          action.follows.each do |klass|
+            unless self[0..i].klasses.include?(klass)
+              raise DependencyNotSatisfiedException.new("#{action.class} action must follow #{klass}")
+            end	
+          end
+          
+          action.precedes.each do |klass|
+            unless self[i..length].klasses.include?(klass)
+              raise DependencyNotSatisfiedException.new("#{action.class} action must precede #{klass}")
+            end
+          end
+        end
+      end
+
+      def duplicates?
+        klasses.uniq.size != size
+      end
+
+      def duplicates!
+        raise DuplicateActionException.new if duplicates?
+      end
+    
+      def klasses
+        map { |o| o.class }
+      end
+    end
+
+    class DuplicateActionException < Exception; end
+    class DependencyNotSatisfiedException < Exception; end
+  end
+end    

lib/vagrant/actions/runner.rb

@@ -47,7 +47,7 @@ module Vagrant
       #
       # @return [Array]
       def actions
-        @actions ||= []
+        @actions ||= Actions::Collection.new
       end
 
       # Returns the first action instance which matches the given class.
@@ -75,9 +75,9 @@ module Vagrant
           add_action(single_action, *args)
         end
 
-        # Raising it here might be too late and hard debug where the actions are comming from (meta actions)
-        raise DuplicateActionException.new if action_klasses.uniq.size < action_klasses.size
-    
+        actions.duplicates!
+        actions.dependencies!
+
         # Call the prepare method on each once its
         # initialized, then call the execute! method
         begin
@@ -127,12 +127,6 @@ module Vagrant
         end
         results
       end
-
-      def action_klasses
-        actions.map { |a| a.class }
-      end
     end
-    
-    class DuplicateActionException < Exception; end
   end
 end

test/test_helper.rb

@@ -76,6 +76,7 @@ class Test::Unit::TestCase
     vm = mock("vboxvm")
     mock_vm = mock("vm")
     action = action_klass.new(mock_vm, *args)
+    stub_default_action_dependecies(action)
 
     mock_vm.stubs(:vm).returns(vm)
     mock_vm.stubs(:vm=)
@@ -86,6 +87,11 @@ class Test::Unit::TestCase
     [mock_vm, vm, action]
   end
 
+  def stub_default_action_dependecies(mock, klass=MockAction)
+    mock.stubs(:precedes).returns([])
+    mock.stubs(:follows).returns([])
+  end
+
   # Sets up the mocks and stubs for a downloader
   def mock_downloader(downloader_klass)
     tempfile = mock("tempfile")
@@ -94,3 +100,7 @@ class Test::Unit::TestCase
     [downloader_klass.new, tempfile]
   end
 end
+
+class MockAction; end
+class MockActionOther; end
+

test/vagrant/actions/collection_test.rb

@@ -0,0 +1,110 @@
+require File.join(File.dirname(__FILE__), '..', '..', 'test_helper')
+
+class CollectionTest < Test::Unit::TestCase
+  context "checking uniqueness" do
+    setup do
+      @actions = Vagrant::Actions::Collection.new([1])
+    end
+
+    should "return true if there are duplicate classes in the collection" do
+      @actions << 1
+      assert @actions.duplicates?
+    end
+    
+    should "return false it all the classes are unique" do
+      @actions << 1.0 << "foo"
+      assert !@actions.duplicates?
+    end
+
+    should "raise an exception when there are duplicates" do
+      @actions << 1 
+      assert_raise Vagrant::Actions::DuplicateActionException do
+        @actions.duplicates!
+      end
+    end
+
+    should "not raise an exception when there are no duplicates" do
+      @actions << 1.0 << "foo"
+      assert_nothing_raised do
+        @actions.duplicates!
+      end
+    end
+  end
+
+  context "verifying dependencies" do
+    setup do
+      @mock_action = mock('action')
+      @mock_action.stubs(:class).returns(MockAction)
+
+      @mock_action2 = mock('action2')
+      @mock_action2.stubs(:class).returns(MockActionOther)
+      # see test_helper
+      stub_default_action_dependecies(@mock_action)
+      stub_default_action_dependecies(@mock_action2)
+    end
+    
+    context "that come before an action" do
+      setup do
+        @mock_action.stubs(:follows).returns([MockActionOther])
+      end
+      should "raise an exception if they are not met" do
+        assert_raise Vagrant::Actions::DependencyNotSatisfiedException do
+          collection.new([@mock_action]).dependencies!
+        end
+      end
+    
+      should "not raise an exception if they are met" do
+        assert_nothing_raised do
+          collection.new([@mock_action2, @mock_action]).dependencies!
+        end
+      end
+    end
+
+    context "that follow an an action" do
+      setup do
+        @mock_action.stubs(:precedes).returns([MockActionOther])
+      end
+
+      should "raise an exception if they are not met" do
+        assert_raise Vagrant::Actions::DependencyNotSatisfiedException do
+          collection.new([@mock_action]).dependencies!
+        end
+      end
+
+      should "not raise an exception if they are met" do
+        assert_nothing_raised do
+          collection.new([@mock_action, @mock_action2]).dependencies!
+        end
+      end
+    end
+
+    context "that are before and after an action" do
+      setup do
+        @mock_action.stubs(:precedes).returns([MockActionOther])
+        @mock_action.stubs(:follows).returns([MockActionOther])
+      end
+
+      should "raise an exception if they are met" do
+        assert_raise Vagrant::Actions::DependencyNotSatisfiedException do
+          collection.new([@mock_action2, @mock_action]).dependencies!
+        end
+      end
+
+      should "not raise and exception if they are met" do
+        assert_nothing_raised do
+          collection.new([@mock_action2, @mock_action, @mock_action2]).dependencies!
+        end
+      end
+    end
+  end
+  
+  context "klasses" do
+    should "return a list of the collection element's classes" do
+      @action = mock('action')
+      assert_equal collection.new([@action]).klasses, [@action.class]
+      assert_equal collection.new([@action, 1.0, "foo"]).klasses, [@action.class, Float, String]
+    end
+  end
+
+  def collection; Vagrant::Actions::Collection end
+end

test/vagrant/actions/runner_test.rb

@@ -6,6 +6,7 @@ class ActionRunnerTest < Test::Unit::TestCase
     action.stubs(:prepare)
     action.stubs(:execute!)
     action.stubs(:cleanup)
+    stub_default_action_dependecies(action)
     action
   end
 
@@ -135,6 +136,7 @@ class ActionRunnerTest < Test::Unit::TestCase
     should "clear the actions and run a single action if given to execute!" do
       action = mock("action")
       run_action = mock("action_run")
+      stub_default_action_dependecies(run_action)
       run_class = mock("run_class")
       run_class.expects(:new).once.returns(run_action)
       @runner.actions << action
@@ -148,7 +150,6 @@ class ActionRunnerTest < Test::Unit::TestCase
     end
 
     should "clear actions after running execute!" do
-      @runner.actions << mock_fake_action
       @runner.actions << mock_fake_action
       assert !@runner.actions.empty? # sanity
       @runner.execute!
@@ -158,9 +159,10 @@ class ActionRunnerTest < Test::Unit::TestCase
     should "run #prepare on all actions, then #execute!" do
       action_seq = sequence("action_seq")
       actions = []
-      5.times do |i|
+      [MockAction, MockActionOther].each_with_index do |klass, i|
         action = mock("action#{i}")
-
+        action.expects(:class).returns(klass)
+        stub_default_action_dependecies(action, klass)
         @runner.actions << action
         actions << action
       end
@@ -176,10 +178,12 @@ class ActionRunnerTest < Test::Unit::TestCase
 
     context "exceptions" do
       setup do
-        @actions = [mock_fake_action, mock_fake_action]
-        @actions.each do |a|
-          a.stubs(:rescue)
-          @runner.actions << a
+        @actions = [MockAction, MockActionOther].map do |klass|
+          action = mock_fake_action
+          action.expects(:class).returns(klass)
+          action.stubs(:rescue)
+          @runner.actions << action
+          action
         end
 
         @exception = Exception.new