From 12b8ab451661ab1dfda6dc32d0592ff29cc8a1d1 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Fri, 21 May 2010 23:38:44 -0700
Subject: [PATCH] Refactor the "all or single" behavior of many commands into
 the base.

---
 lib/vagrant/commands/base.rb          | 17 +++++++++++++
 lib/vagrant/commands/destroy.rb       | 15 +-----------
 lib/vagrant/commands/halt.rb          | 14 +----------
 lib/vagrant/commands/reload.rb        | 14 +----------
 lib/vagrant/commands/resume.rb        | 14 +----------
 lib/vagrant/commands/suspend.rb       | 14 +----------
 lib/vagrant/commands/up.rb            | 18 +-------------
 test/vagrant/commands/base_test.rb    | 18 ++++++++++++++
 test/vagrant/commands/destroy_test.rb | 22 ++---------------
 test/vagrant/commands/halt_test.rb    | 22 ++---------------
 test/vagrant/commands/reload_test.rb  | 22 ++---------------
 test/vagrant/commands/resume_test.rb  | 22 ++---------------
 test/vagrant/commands/suspend_test.rb | 22 ++---------------
 test/vagrant/commands/up_test.rb      | 35 ++-------------------------
 14 files changed, 53 insertions(+), 216 deletions(-)

diff --git a/lib/vagrant/commands/base.rb b/lib/vagrant/commands/base.rb
index f58e8e8fa..d1b1db2a0 100644
--- a/lib/vagrant/commands/base.rb
+++ b/lib/vagrant/commands/base.rb
@@ -94,6 +94,23 @@ module Vagrant
       # Methods below are not meant to be overriden/implemented by subclasses
       #-------------------------------------------------------------------
 
+      # Parses the options for a given command and if a name was
+      # given, it calls the single method, otherwise it calls the all
+      # method. This helper is an abstraction which allows commands to
+      # easily be used in both regular and multi-VM environments.
+      def all_or_single(args, method_prefix)
+        args = parse_options(args)
+
+        single_method = "#{method_prefix}_single".to_sym
+        if args[0]
+          send(single_method, args[0])
+        else
+          env.vms.keys.each do |name|
+            send(single_method, name)
+          end
+        end
+      end
+
       # Shows the version
       def puts_version
         File.open(File.join(PROJECT_ROOT, "VERSION"), "r") do |f|
diff --git a/lib/vagrant/commands/destroy.rb b/lib/vagrant/commands/destroy.rb
index 485b85193..ffad6232a 100644
--- a/lib/vagrant/commands/destroy.rb
+++ b/lib/vagrant/commands/destroy.rb
@@ -11,13 +11,7 @@ module Vagrant
       description "Destroys the vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          destroy_single(args[0])
-        else
-          destroy_all
-        end
+        all_or_single(args, :destroy)
       end
 
       # Destroys a single VM by name.
@@ -35,13 +29,6 @@ module Vagrant
         end
       end
 
-      # Destroys all VMs represented by the current environment.
-      def destroy_all
-        env.vms.each do |name, vm|
-          destroy_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant destroy"
       end
diff --git a/lib/vagrant/commands/halt.rb b/lib/vagrant/commands/halt.rb
index 82faab086..a5f6c2d25 100644
--- a/lib/vagrant/commands/halt.rb
+++ b/lib/vagrant/commands/halt.rb
@@ -11,13 +11,7 @@ module Vagrant
       description "Halts the currently running vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          halt_single(args[0])
-        else
-          halt_all
-        end
+        all_or_single(args, :halt)
       end
 
       def halt_single(name)
@@ -34,12 +28,6 @@ module Vagrant
         end
       end
 
-      def halt_all
-        env.vms.keys.each do |name|
-          halt_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant halt"
 
diff --git a/lib/vagrant/commands/reload.rb b/lib/vagrant/commands/reload.rb
index fc32a931c..e4a4217d1 100644
--- a/lib/vagrant/commands/reload.rb
+++ b/lib/vagrant/commands/reload.rb
@@ -10,13 +10,7 @@ module Vagrant
       description "Reload the vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          reload_single(args[0])
-        else
-          reload_all
-        end
+        all_or_single(args, :reload)
       end
 
       def reload_single(name)
@@ -33,12 +27,6 @@ module Vagrant
         end
       end
 
-      def reload_all
-        env.vms.keys.each do |name|
-          reload_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant reload"
       end
diff --git a/lib/vagrant/commands/resume.rb b/lib/vagrant/commands/resume.rb
index 79bef1dfa..f970de3e4 100644
--- a/lib/vagrant/commands/resume.rb
+++ b/lib/vagrant/commands/resume.rb
@@ -10,13 +10,7 @@ module Vagrant
       description "Resumes a suspend vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          resume_single(args[0])
-        else
-          resume_all
-        end
+        all_or_single(args, :resume)
       end
 
       def resume_single(name)
@@ -33,12 +27,6 @@ module Vagrant
         end
       end
 
-      def resume_all
-        env.vms.keys.each do |name|
-          resume_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant resume"
       end
diff --git a/lib/vagrant/commands/suspend.rb b/lib/vagrant/commands/suspend.rb
index fd29483a4..6c0d7fad3 100644
--- a/lib/vagrant/commands/suspend.rb
+++ b/lib/vagrant/commands/suspend.rb
@@ -11,13 +11,7 @@ module Vagrant
       description "Suspends the currently running vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          suspend_single(args[0])
-        else
-          suspend_all
-        end
+        all_or_single(args, :suspend)
       end
 
       def suspend_single(name)
@@ -34,12 +28,6 @@ module Vagrant
         end
       end
 
-      def suspend_all
-        env.vms.keys.each do |name|
-          suspend_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant suspend"
       end
diff --git a/lib/vagrant/commands/up.rb b/lib/vagrant/commands/up.rb
index b71cf95fd..3d7057e1e 100644
--- a/lib/vagrant/commands/up.rb
+++ b/lib/vagrant/commands/up.rb
@@ -9,13 +9,7 @@ module Vagrant
       description "Creates the vagrant environment"
 
       def execute(args=[])
-        args = parse_options(args)
-
-        if args[0]
-          up_single(args[0])
-        else
-          up_all
-        end
+        all_or_single(args, :up)
       end
 
       def up_single(name)
@@ -36,16 +30,6 @@ module Vagrant
         end
       end
 
-      def up_all
-        # First verify that all VMs have valid boxes
-        env.vms.each { |name, vm| vm.env.require_box unless vm.created? }
-
-        # Next, handle each VM
-        env.vms.keys.each do |name|
-          up_single(name)
-        end
-      end
-
       def options_spec(opts)
         opts.banner = "Usage: vagrant up"
       end
diff --git a/test/vagrant/commands/base_test.rb b/test/vagrant/commands/base_test.rb
index 2eca395e3..0aac0e55e 100644
--- a/test/vagrant/commands/base_test.rb
+++ b/test/vagrant/commands/base_test.rb
@@ -78,6 +78,24 @@ class CommandsBaseTest < Test::Unit::TestCase
       end
     end
 
+    context "all or single methods" do
+      should "call the single method if a name is given" do
+        name = "bar"
+        @instance.expects(:foo_single).with(name).once
+        @instance.all_or_single(["bar"], :foo)
+      end
+
+      should "call the single method for each VM if no name is given" do
+        vms = { :foo => nil, :bar => nil }
+        vms.keys.each do |name|
+          @instance.expects(:foo_single).with(name).once
+        end
+
+        @env.stubs(:vms).returns(vms)
+        @instance.all_or_single([], :foo)
+      end
+    end
+
     context "getting the option parser" do
       should "create it with the options spec if it hasn't been created yet" do
         opts = mock("opts")
diff --git a/test/vagrant/commands/destroy_test.rb b/test/vagrant/commands/destroy_test.rb
index f75795c5b..94eb895e3 100644
--- a/test/vagrant/commands/destroy_test.rb
+++ b/test/vagrant/commands/destroy_test.rb
@@ -10,28 +10,10 @@ class CommandsDestroyTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call destroy_all if no name is given" do
-      @instance.expects(:destroy_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :destroy)
       @instance.execute
     end
-
-    should "call destroy_single if a name is given" do
-      @instance.expects(:destroy_single).with("foo").once
-      @instance.execute(["foo"])
-    end
-  end
-
-  context "destroying all" do
-    should "destroy each VM" do
-      vms = { :foo => nil, :bar => nil, :baz => nil }
-      @env.stubs(:vms).returns(vms)
-
-      vms.each do |name, value|
-        @instance.expects(:destroy_single).with(name).once
-      end
-
-      @instance.destroy_all
-    end
   end
 
   context "destroying a single VM" do
diff --git a/test/vagrant/commands/halt_test.rb b/test/vagrant/commands/halt_test.rb
index 569aa5767..2e1ccf44b 100644
--- a/test/vagrant/commands/halt_test.rb
+++ b/test/vagrant/commands/halt_test.rb
@@ -9,28 +9,10 @@ class CommandsHaltTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call halt_all if no name is given" do
-      @instance.expects(:halt_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :halt)
       @instance.execute
     end
-
-    should "call halt_single if a name is given" do
-      @instance.expects(:halt_single).with("foo").once
-      @instance.execute(["foo"])
-    end
-  end
-
-  context "halting all" do
-    should "halt each VM" do
-      vms = { :foo => nil, :bar => nil, :baz => nil }
-      @env.stubs(:vms).returns(vms)
-
-      vms.each do |name, value|
-        @instance.expects(:halt_single).with(name).once
-      end
-
-      @instance.halt_all
-    end
   end
 
   context "halting a single VM" do
diff --git a/test/vagrant/commands/reload_test.rb b/test/vagrant/commands/reload_test.rb
index 9de1411bf..981859068 100644
--- a/test/vagrant/commands/reload_test.rb
+++ b/test/vagrant/commands/reload_test.rb
@@ -9,28 +9,10 @@ class CommandsReloadTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call on all if no name is given" do
-      @instance.expects(:reload_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :reload)
       @instance.execute
     end
-
-    should "call on single if a name is given" do
-      @instance.expects(:reload_single).with("foo").once
-      @instance.execute(["foo"])
-    end
-  end
-
-  context "reloading all" do
-    should "reload each VM" do
-      vms = { :foo => nil, :bar => nil, :baz => nil }
-      @env.stubs(:vms).returns(vms)
-
-      vms.each do |name, value|
-        @instance.expects(:reload_single).with(name).once
-      end
-
-      @instance.reload_all
-    end
   end
 
   context "reloading a single VM" do
diff --git a/test/vagrant/commands/resume_test.rb b/test/vagrant/commands/resume_test.rb
index 6d6b5b864..7da402f05 100644
--- a/test/vagrant/commands/resume_test.rb
+++ b/test/vagrant/commands/resume_test.rb
@@ -9,28 +9,10 @@ class CommandsResumeTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call on all if no name is given" do
-      @instance.expects(:resume_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :resume)
       @instance.execute
     end
-
-    should "call on single if a name is given" do
-      @instance.expects(:resume_single).with("foo").once
-      @instance.execute(["foo"])
-    end
-  end
-
-  context "resume all" do
-    should "resume each VM" do
-      vms = { :foo => nil, :bar => nil, :baz => nil }
-      @env.stubs(:vms).returns(vms)
-
-      vms.each do |name, value|
-        @instance.expects(:resume_single).with(name).once
-      end
-
-      @instance.resume_all
-    end
   end
 
   context "resuming a single VM" do
diff --git a/test/vagrant/commands/suspend_test.rb b/test/vagrant/commands/suspend_test.rb
index 2a00d75f7..bc9b437d1 100644
--- a/test/vagrant/commands/suspend_test.rb
+++ b/test/vagrant/commands/suspend_test.rb
@@ -9,28 +9,10 @@ class CommandsSuspendTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call on all if no name is given" do
-      @instance.expects(:suspend_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :suspend)
       @instance.execute
     end
-
-    should "call on single if a name is given" do
-      @instance.expects(:suspend_single).with("foo").once
-      @instance.execute(["foo"])
-    end
-  end
-
-  context "suspending all" do
-    should "suspend each VM" do
-      vms = { :foo => nil, :bar => nil, :baz => nil }
-      @env.stubs(:vms).returns(vms)
-
-      vms.each do |name, value|
-        @instance.expects(:suspend_single).with(name).once
-      end
-
-      @instance.suspend_all
-    end
   end
 
   context "suspending a single VM" do
diff --git a/test/vagrant/commands/up_test.rb b/test/vagrant/commands/up_test.rb
index 9a0d0c5f0..477c61d5c 100644
--- a/test/vagrant/commands/up_test.rb
+++ b/test/vagrant/commands/up_test.rb
@@ -9,15 +9,10 @@ class CommandsUpTest < Test::Unit::TestCase
   end
 
   context "executing" do
-    should "call up_all if no name is given" do
-      @instance.expects(:up_all).once
+    should "call all or single for the method" do
+      @instance.expects(:all_or_single).with([], :up)
       @instance.execute
     end
-
-    should "call up_single if a name is given" do
-      @instance.expects(:up_single).with("foo").once
-      @instance.execute(["foo"])
-    end
   end
 
   context "upping a single VM" do
@@ -49,30 +44,4 @@ class CommandsUpTest < Test::Unit::TestCase
       @instance.up_single(:foo)
     end
   end
-
-  context "upping all VMs" do
-    setup do
-      @vms = {}
-      @env.stubs(:vms).returns(@vms)
-    end
-
-    def create_vm
-      vm = mock("vm")
-      vm.stubs(:env).returns(mock_environment)
-      vm.stubs(:created?).returns(false)
-      vm
-    end
-
-    should "require a box for all VMs" do
-      @vms[:foo] = create_vm
-      @vms[:bar] = create_vm
-
-      @vms.each do |name, vm|
-        vm.env.expects(:require_box).once
-        @instance.expects(:up_single).with(name).once
-      end
-
-      @instance.up_all
-    end
-  end
 end