From 9913c07ff2bb305aec53e7c72244113e59f34b47 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 28 Feb 2010 21:42:38 -0800 Subject: [PATCH] Box download action now uses Vagrant "Downloaders." These are abstractions which will allow me to implement file downloading, http downloading etc. File downloading implemented here. --- lib/vagrant.rb | 4 +- lib/vagrant/actions/box/download.rb | 27 +++++++----- lib/vagrant/downloaders/base.rb | 13 ++++++ lib/vagrant/downloaders/file.rb | 21 +++++++++ test/test_helper.rb | 8 ++++ test/vagrant/actions/box/download_test.rb | 53 +++++++++++++---------- test/vagrant/downloaders/base_test.rb | 20 +++++++++ test/vagrant/downloaders/file_test.rb | 32 ++++++++++++++ 8 files changed, 143 insertions(+), 35 deletions(-) create mode 100644 lib/vagrant/downloaders/base.rb create mode 100644 lib/vagrant/downloaders/file.rb create mode 100644 test/vagrant/downloaders/base_test.rb create mode 100644 test/vagrant/downloaders/file_test.rb diff --git a/lib/vagrant.rb b/lib/vagrant.rb index d051f87b0..8483819e5 100644 --- a/lib/vagrant.rb +++ b/lib/vagrant.rb @@ -3,8 +3,8 @@ $:.unshift(libdir) PROJECT_ROOT = File.join(libdir, '..') unless defined?(PROJECT_ROOT) # The libs which must be loaded prior to the rest -%w{tempfile open-uri ftools json pathname logger virtualbox net/ssh tarruby - net/scp fileutils vagrant/util vagrant/actions/base}.each do |f| +%w{tempfile open-uri ftools json pathname logger uri virtualbox net/ssh tarruby + net/scp fileutils vagrant/util vagrant/actions/base vagrant/downloaders/base}.each do |f| require f end diff --git a/lib/vagrant/actions/box/download.rb b/lib/vagrant/actions/box/download.rb index 3c5a5e448..e1413a418 100644 --- a/lib/vagrant/actions/box/download.rb +++ b/lib/vagrant/actions/box/download.rb @@ -7,9 +7,23 @@ module Vagrant BASENAME = "box" BUFFERSIZE = 1048576 # 1 MB + attr_reader :downloader + + def prepare + # Parse the URI given and prepare a downloader + uri = URI.parse(@runner.uri) + + if uri.is_a?(URI::Generic) + logger.info "Generic URI type for box download, assuming file..." + @downloader = Downloaders::File.new + end + + raise ActionException.new("Unknown URI type for box download.") unless @downloader + end + def execute! with_tempfile do |tempfile| - copy_uri_to(tempfile) + download_to(tempfile) @runner.temp_path = tempfile.path end end @@ -26,16 +40,9 @@ module Vagrant end end - # TODO: Need a way to report progress. Downloading/copying a 350 MB - # box without any progress is not acceptable. - def copy_uri_to(f) + def download_to(f) logger.info "Copying box to temporary location..." - open(@runner.uri) do |remote_file| - loop do - break if remote_file.eof? - f.write(remote_file.read(BUFFERSIZE)) - end - end + downloader.download!(@runner.uri, f) end end end diff --git a/lib/vagrant/downloaders/base.rb b/lib/vagrant/downloaders/base.rb new file mode 100644 index 000000000..5f19cd75c --- /dev/null +++ b/lib/vagrant/downloaders/base.rb @@ -0,0 +1,13 @@ +module Vagrant + module Downloaders + # Represents a base class for a downloader. A downloader handles + # downloading a box file to a temporary file. + class Base + include Vagrant::Util + + # Downloads the source file to the destination file. It is up to + # implementors of this class to handle the logic. + def download!(source_url, destination_file); end + end + end +end \ No newline at end of file diff --git a/lib/vagrant/downloaders/file.rb b/lib/vagrant/downloaders/file.rb new file mode 100644 index 000000000..ca119d382 --- /dev/null +++ b/lib/vagrant/downloaders/file.rb @@ -0,0 +1,21 @@ +module Vagrant + module Downloaders + # "Downloads" a file to a temporary file. Basically, this downloader + # simply does a file copy. + class File < Base + BUFFERSIZE = 1048576 # 1 MB + + def download!(source_url, destination_file) + # For now we read the contents of one into a buffer + # and copy it into the other. In the future, we should do + # a system-level file copy (FileUtils.cp). + open(source_url) do |f| + loop do + break if f.eof? + destination_file.write(f.read(BUFFERSIZE)) + end + end + end + end + end +end \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index 2815217dd..0286474f7 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -80,4 +80,12 @@ class Test::Unit::TestCase [mock_vm, vm, action] end + + # Sets up the mocks and stubs for a downloader + def mock_downloader(downloader_klass) + tempfile = mock("tempfile") + tempfile.stubs(:write) + + [downloader_klass.new, tempfile] + end end diff --git a/test/vagrant/actions/box/download_test.rb b/test/vagrant/actions/box/download_test.rb index 0697581ff..1e164e7e9 100644 --- a/test/vagrant/actions/box/download_test.rb +++ b/test/vagrant/actions/box/download_test.rb @@ -11,6 +11,27 @@ class DownloadBoxActionTest < Test::Unit::TestCase Vagrant::Env.stubs(:tmp_path).returns("foo") end + context "preparing" do + setup do + @uri = mock("uri") + @uri.stubs(:is_a?).returns(false) + URI.stubs(:parse).returns(@uri) + end + + should "raise an exception if no URI type is matched" do + @uri.stubs(:is_a?).returns(false) + assert_raises(Vagrant::Actions::ActionException) { + @action.prepare + } + end + + should "set the downloader to file if URI is generic" do + @uri.stubs(:is_a?).with(URI::Generic).returns(true) + @action.prepare + assert @action.downloader.is_a?(Vagrant::Downloaders::File) + end + end + context "executing" do setup do @path = "foo" @@ -19,12 +40,12 @@ class DownloadBoxActionTest < Test::Unit::TestCase @tempfile.stubs(:path).returns(@path) @action.stubs(:with_tempfile).yields(@tempfile) - @action.stubs(:copy_uri_to) + @action.stubs(:download_to) end should "make a tempfile and copy the URI contents to it" do @action.expects(:with_tempfile).yields(@tempfile) - @action.expects(:copy_uri_to).with(@tempfile) + @action.expects(:download_to).with(@tempfile) @action.execute! end @@ -69,30 +90,16 @@ class DownloadBoxActionTest < Test::Unit::TestCase end end - context "copying URI file" do + context "downloading" do setup do - @tempfile = mock("tempfile") - @tempfile.stubs(:write) - - @file = mock("file") - @file.stubs(:read) - @file.stubs(:eof?).returns(false) - @action.stubs(:open).yields(@file) + @downloader = mock("downloader") + @action.stubs(:downloader).returns(@downloader) end - should "open with the given uri" do - @action.expects(:open).with(@uri).once - @action.copy_uri_to(@tempfile) - end - - should "read from the file and write to the tempfile" do - data = mock("data") - write_seq = sequence("write_seq") - @file.stubs(:eof?).returns(false).in_sequence(write_seq) - @file.expects(:read).returns(data).in_sequence(write_seq) - @tempfile.expects(:write).with(data).in_sequence(write_seq) - @file.stubs(:eof?).returns(true).in_sequence(write_seq) - @action.copy_uri_to(@tempfile) + should "call download! on the download with the URI and tempfile" do + tempfile = "foo" + @downloader.expects(:download!).with(@runner.uri, tempfile) + @action.download_to(tempfile) end end end diff --git a/test/vagrant/downloaders/base_test.rb b/test/vagrant/downloaders/base_test.rb new file mode 100644 index 000000000..998eb8faf --- /dev/null +++ b/test/vagrant/downloaders/base_test.rb @@ -0,0 +1,20 @@ +require File.join(File.dirname(__FILE__), '..', '..', 'test_helper') + +class BaseDownloaderTest < Test::Unit::TestCase + should "include the util class so subclasses have access to it" do + assert Vagrant::Downloaders::Base.include?(Vagrant::Util) + end + + context "base instance" do + setup do + @base = Vagrant::Downloaders::Base.new + end + + should "implement prepare which does nothing" do + assert_nothing_raised do + assert @base.respond_to?(:download!) + @base.download!("source", "destination") + end + end + end +end diff --git a/test/vagrant/downloaders/file_test.rb b/test/vagrant/downloaders/file_test.rb new file mode 100644 index 000000000..bcd52cc93 --- /dev/null +++ b/test/vagrant/downloaders/file_test.rb @@ -0,0 +1,32 @@ +require File.join(File.dirname(__FILE__), '..', '..', 'test_helper') + +class FileDownloaderTest < Test::Unit::TestCase + setup do + @downloader, @tempfile = mock_downloader(Vagrant::Downloaders::File) + @uri = "foo.box" + end + + context "downloading" do + setup do + @file = mock("file") + @file.stubs(:read) + @file.stubs(:eof?).returns(false) + @downloader.stubs(:open).yields(@file) + end + + should "open with the given uri" do + @downloader.expects(:open).with(@uri).once + @downloader.download!(@uri, @tempfile) + end + + should "buffer the read from the file and write to the tempfile" do + data = mock("data") + write_seq = sequence("write_seq") + @file.stubs(:eof?).returns(false).in_sequence(write_seq) + @file.expects(:read).returns(data).in_sequence(write_seq) + @tempfile.expects(:write).with(data).in_sequence(write_seq) + @file.stubs(:eof?).returns(true).in_sequence(write_seq) + @downloader.download!(@uri, @tempfile) + end + end +end