Added unit specs for source, dsl, index, source_list

9 files changed, 213 insertions, 20 deletions

diff --git a/lib/bundler/plugin/api.rb b/lib/bundler/plugin/api.rb
index 68f812d7..14ed8d21 100644
--- a/lib/bundler/plugin/api.rb
+++ b/lib/bundler/plugin/api.rb
@@ -40,7 +40,7 @@ module Bundler
       # @param [Class] (optional) class that handles the source. If not
       #                 provided, the `self` class will be used.
       def self.source(source, cls = self)
-        include Bundler::Plugin::API::Source
+        cls.include Bundler::Plugin::API::Source
         Plugin.add_source source, cls
       end
 
diff --git a/lib/bundler/plugin/api/source.rb b/lib/bundler/plugin/api/source.rb
index 8cb8424c..701264ef 100644
--- a/lib/bundler/plugin/api/source.rb
+++ b/lib/bundler/plugin/api/source.rb
@@ -18,6 +18,12 @@ module Bundler
       # be (preferably should be) overridden as per the plugins' needs safely
       # (as long as they behave as expected).
       # On overriding `initialize` you should call super first.
+      #
+      # If required plugin should override `hash`, `==` and `eql?` methods to be
+      # able to match objects representing same sources, but may be created in
+      # different situation (like form gemfile and lockfile). The default ones
+      # checks only for class and uri, but elaborate source plugins may need
+      # more comparisons (like, git checking on branch or tag).
       module Source
         attr_reader :uri, :options
         attr_accessor :dependency_names
@@ -123,6 +129,12 @@ module Bundler
           other.is_a?(self.class) && uri == other.uri
         end
 
+        alias_method :eql?, :==
+
+        def hash
+          [self.class, uri].hash
+        end
+
         def installed?
           File.directory?(install_path)
         end
diff --git a/lib/bundler/plugin/dsl.rb b/lib/bundler/plugin/dsl.rb
index b734df3a..296bfdbc 100644
--- a/lib/bundler/plugin/dsl.rb
+++ b/lib/bundler/plugin/dsl.rb
@@ -34,8 +34,10 @@ module Bundler
         return super unless options.key?("type")
 
         plugin_name = "bundler-source-#{options["type"]}"
-        plugin(plugin_name)
-        @auto_plugins << plugin_name
+        unless @auto_plugins.include? plugin_name
+          plugin(plugin_name)
+          @auto_plugins << plugin_name
+        end
       end
     end
   end
diff --git a/lib/bundler/plugin/index.rb b/lib/bundler/plugin/index.rb
index 096390ba..e36baf6a 100644
--- a/lib/bundler/plugin/index.rb
+++ b/lib/bundler/plugin/index.rb
@@ -35,6 +35,8 @@ module Bundler
       # @param [String] path where the plugin is installed
       # @param [Array<String>] commands that are handled by the plugin
       def register_plugin(name, path, commands, sources)
+        old_commands = @commands.dup
+
         common = commands & @commands.keys
         raise CommandConflict.new(name, common) unless common.empty?
         commands.each {|c| @commands[c] = name }
@@ -45,6 +47,9 @@ module Bundler
 
         @plugin_paths[name] = path
         save_index
+      rescue
+        @commands = old_commands
+        raise
       end
 
       # Path where the index file is stored
diff --git a/spec/bundler/plugin/api_spec.rb b/spec/bundler/plugin/api_spec.rb
index 23134dd8..d1c12b94 100644
--- a/spec/bundler/plugin/api_spec.rb
+++ b/spec/bundler/plugin/api_spec.rb
@@ -7,18 +7,36 @@ describe Bundler::Plugin::API do
       stub_const "UserPluginClass", Class.new(Bundler::Plugin::API)
     end
 
-    it "declares a command plugin with same class as handler" do
-      allow(Bundler::Plugin).
-        to receive(:add_command).with("meh", UserPluginClass).once
+    describe "#command" do
+      it "declares a command plugin with same class as handler" do
+        expect(Bundler::Plugin).
+          to receive(:add_command).with("meh", UserPluginClass).once
 
-      UserPluginClass.command "meh"
+        UserPluginClass.command "meh"
+      end
+
+      it "accepts another class as argument that handles the command" do
+        stub_const "NewClass", Class.new
+        expect(Bundler::Plugin).to receive(:add_command).with("meh", NewClass).once
+
+        UserPluginClass.command "meh", NewClass
+      end
     end
 
-    it "accepts another class as argument that handles the command" do
-      stub_const "NewClass", Class.new
-      allow(Bundler::Plugin).to receive(:add_command).with("meh", NewClass).once
+    describe "#source" do
+      it "declares a source plugin with same class as handler" do
+        expect(Bundler::Plugin).
+          to receive(:add_source).with("a_source", UserPluginClass).once
 
-      UserPluginClass.command "meh", NewClass
+        UserPluginClass.source "a_source"
+      end
+
+      it "accepts another class as argument that handles the command" do
+        stub_const "NewClass", Class.new
+        expect(Bundler::Plugin).to receive(:add_source).with("a_source", NewClass).once
+
+        UserPluginClass.source "a_source", NewClass
+      end
     end
   end
 
diff --git a/spec/bundler/plugin/dsl_spec.rb b/spec/bundler/plugin/dsl_spec.rb
index 876564f2..5b6d20d7 100644
--- a/spec/bundler/plugin/dsl_spec.rb
+++ b/spec/bundler/plugin/dsl_spec.rb
@@ -19,4 +19,21 @@ describe Bundler::Plugin::DSL do
       expect { dsl.no_method }.to raise_error(DSL::PluginGemfileError)
     end
   end
+
+  describe "source block" do
+    it "adds #source with :type to list and also auto_plugins list" do
+      expect(dsl).to receive(:plugin).with("bundler-source-news").once
+
+      dsl.source("some_random_url", :type => "news") {}
+
+      expect(dsl.auto_plugins).to eq(["bundler-source-news"])
+    end
+
+    it "registers a source type plugin only once for multiple declataions" do
+      expect(dsl).to receive(:plugin).with("bundler-source-news").once
+
+      dsl.source("some_random_url", :type => "news") {}
+      dsl.source("another_random_url", :type => "news") {}
+    end
+  end
 end
diff --git a/spec/bundler/plugin/index_spec.rb b/spec/bundler/plugin/index_spec.rb
index c2d05777..59b1a0c6 100644
--- a/spec/bundler/plugin/index_spec.rb
+++ b/spec/bundler/plugin/index_spec.rb
@@ -6,12 +6,6 @@ describe Bundler::Plugin::Index do
 
   subject(:index) { Index.new }
 
-  before do
-    build_lib "new-plugin", :path => lib_path("new-plugin") do |s|
-      s.write "plugins.rb"
-    end
-  end
-
   describe "#register plugin" do
     before do
       index.register_plugin("new-plugin", lib_path("new-plugin").to_s, [], [])
@@ -41,5 +35,81 @@ describe Bundler::Plugin::Index do
         index.register_plugin("aplugin", lib_path("aplugin").to_s, ["newco"], [])
       end.to raise_error(Index::CommandConflict)
     end
+
+    it "is persistent" do
+      new_index = Index.new
+      expect(new_index.command_plugin("newco")).to eq("cplugin")
+    end
+  end
+
+  describe "source" do
+    before do
+      index.register_plugin("splugin", lib_path("splugin").to_s, [], ["new_source"])
+    end
+
+    it "returns the plugins name on query" do
+      expect(index.source_plugin("new_source")).to eq("splugin")
+    end
+
+    it "raises error on conflict" do
+      expect do
+        index.register_plugin("aplugin", lib_path("aplugin").to_s, [], ["new_source"])
+      end.to raise_error(Index::SourceConflict)
+    end
+
+    it "is persistent" do
+      new_index = Index.new
+      expect(new_index.source_plugin("new_source")).to eq("splugin")
+    end
+  end
+
+  describe "after conflict" do
+    before do
+      index.register_plugin("aplugin", lib_path("aplugin").to_s, ["foo"], ["bar"])
+    end
+
+    shared_examples "it cleans up" do
+      it "the path" do
+        expect(index.installed?("cplugin")).to be_falsy
+      end
+
+      it "the command" do
+        expect(index.command_plugin("xfoo")).to be_falsy
+      end
+
+      it "the source" do
+        expect(index.source_plugin("xbar")).to be_falsy
+      end
+    end
+
+    context "on command conflict it cleans up" do
+      before do
+        expect do
+          index.register_plugin("cplugin", lib_path("cplugin").to_s, ["foo"], ["xbar"])
+        end.to raise_error(Index::CommandConflict)
+      end
+
+      include_examples "it cleans up"
+    end
+
+    context "on source conflict it cleans up" do
+      before do
+        expect do
+          index.register_plugin("cplugin", lib_path("cplugin").to_s, ["xfoo"], ["bar"])
+        end.to raise_error(Index::SourceConflict)
+      end
+
+      include_examples "it cleans up"
+    end
+
+    context "on command and source conflict it cleans up" do
+      before do
+        expect do
+          index.register_plugin("cplugin", lib_path("cplugin").to_s, ["foo"], ["bar"])
+        end.to raise_error(Index::CommandConflict)
+      end
+
+      include_examples "it cleans up"
+    end
   end
 end
diff --git a/spec/bundler/plugin_spec.rb b/spec/bundler/plugin_spec.rb
index 8e85be67..20505be0 100644
--- a/spec/bundler/plugin_spec.rb
+++ b/spec/bundler/plugin_spec.rb
@@ -167,7 +167,7 @@ describe Bundler::Plugin do
     end
 
     it "loads the plugin, if not loaded" do
-      allow(index).to receive(:source_plugin).with("foo-bar") {"plugin_name"}
+      allow(index).to receive(:source_plugin).with("foo-bar") { "plugin_name" }
 
       expect(subject).to receive(:load_plugin).with("plugin_name")
       subject.source("foo-bar")
@@ -184,7 +184,7 @@ describe Bundler::Plugin do
 
   describe "#source_from_lock" do
     it "returns instance of registered class initialized with locked opts" do
-      opts = {"type" => "l_source", "remote" => "xyz", "other" => "random"}
+      opts = { "type" => "l_source", "remote" => "xyz", "other" => "random" }
       allow(index).to receive(:source_plugin).with("l_source") { "plugin_name" }
 
       stub_const "SClass", Class.new
@@ -192,7 +192,7 @@ describe Bundler::Plugin do
       subject.add_source("l_source", SClass)
 
       expect(SClass).to receive(:new).with(hash_including(
-        "type" => "l_source", "uri" => "xyz", "other" => "random")) { s_instance }
+                                             "type" => "l_source", "uri" => "xyz", "other" => "random")) { s_instance }
       expect(subject.source_from_lock(opts)).to be(s_instance)
     end
   end
diff --git a/spec/bundler/source_list_spec.rb b/spec/bundler/source_list_spec.rb
index d7567d6e..f518a5ac 100644
--- a/spec/bundler/source_list_spec.rb
+++ b/spec/bundler/source_list_spec.rb
@@ -4,6 +4,10 @@ require "spec_helper"
 describe Bundler::SourceList do
   before do
     allow(Bundler).to receive(:root) { Pathname.new "/" }
+
+    stub_const "ASourcePlugin", Class.new(Bundler::Plugin::API)
+    ASourcePlugin.source "new_source"
+    allow(Bundler::Plugin).to receive(:source?).with("new_source").and_return(true)
   end
 
   subject(:source_list) { Bundler::SourceList.new }
@@ -15,6 +19,7 @@ describe Bundler::SourceList do
       source_list.add_path_source("path" => "/existing/path/to/gem")
       source_list.add_git_source("uri" => "git://existing-git.org/path.git")
       source_list.add_rubygems_source("remotes" => ["https://existing-rubygems.org"])
+      source_list.add_plugin_source("new_source", "uri" => "https://some.url/a")
     end
 
     describe "#add_path_source" do
@@ -100,6 +105,29 @@ describe Bundler::SourceList do
         expect(@returned_source.remotes.first).to eq(URI("https://othersource.org/"))
       end
     end
+
+    describe "#add_plugin_source" do
+      before do
+        @duplicate = source_list.add_plugin_source("new_source", "uri" => "http://host/path.")
+        @new_source = source_list.add_plugin_source("new_source", "uri" => "http://host/path.")
+      end
+
+      it "returns the new plugin source" do
+        expect(@new_source).to be_a(Bundler::Plugin::API::Source)
+      end
+
+      it "passes the provided options to the new source" do
+        expect(@new_source.options).to eq("uri" => "http://host/path.")
+      end
+
+      it "adds the source to the beginning of git_sources" do
+        expect(source_list.plugin_sources.first).to equal(@new_source)
+      end
+
+      it "removes existing duplicates" do
+        expect(source_list.plugin_sources).not_to include equal(@duplicate)
+      end
+    end
   end
 
   describe "#all_sources" do
@@ -107,6 +135,7 @@ describe Bundler::SourceList do
       source_list.add_git_source("uri" => "git://host/path.git")
       source_list.add_rubygems_source("remotes" => ["https://rubygems.org"])
       source_list.add_path_source("path" => "/path/to/gem")
+      source_list.add_plugin_source("new_source", "uri" => "https://some.url/a")
 
       expect(source_list.all_sources).to include rubygems_aggregate
     end
@@ -114,6 +143,7 @@ describe Bundler::SourceList do
     it "includes the aggregate rubygems source when no rubygems sources have been added" do
       source_list.add_git_source("uri" => "git://host/path.git")
       source_list.add_path_source("path" => "/path/to/gem")
+      source_list.add_plugin_source("new_source", "uri" => "https://some.url/a")
 
       expect(source_list.all_sources).to include rubygems_aggregate
     end
@@ -122,12 +152,15 @@ describe Bundler::SourceList do
       source_list.add_git_source("uri" => "git://third-git.org/path.git")
       source_list.add_rubygems_source("remotes" => ["https://fifth-rubygems.org"])
       source_list.add_path_source("path" => "/third/path/to/gem")
+      source_list.add_plugin_source("new_source", "uri" => "https://some.url/b")
       source_list.add_rubygems_source("remotes" => ["https://fourth-rubygems.org"])
       source_list.add_path_source("path" => "/second/path/to/gem")
       source_list.add_rubygems_source("remotes" => ["https://third-rubygems.org"])
+      source_list.add_plugin_source("new_source", "uri" => "https://some.o.url/")
       source_list.add_git_source("uri" => "git://second-git.org/path.git")
       source_list.add_rubygems_source("remotes" => ["https://second-rubygems.org"])
       source_list.add_path_source("path" => "/first/path/to/gem")
+      source_list.add_plugin_source("new_source", "uri" => "https://some.url/c")
       source_list.add_rubygems_source("remotes" => ["https://first-rubygems.org"])
       source_list.add_git_source("uri" => "git://first-git.org/path.git")
 
@@ -138,6 +171,9 @@ describe Bundler::SourceList do
         Bundler::Source::Git.new("uri" => "git://first-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://second-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://third-git.org/path.git"),
+        ASourcePlugin.new("uri" => "https://some.url/c"),
+        ASourcePlugin.new("uri" => "https://some.o.url/"),
+        ASourcePlugin.new("uri" => "https://some.url/b"),
         Bundler::Source::Rubygems.new("remotes" => ["https://first-rubygems.org"]),
         Bundler::Source::Rubygems.new("remotes" => ["https://second-rubygems.org"]),
         Bundler::Source::Rubygems.new("remotes" => ["https://third-rubygems.org"]),
@@ -205,6 +241,35 @@ describe Bundler::SourceList do
     end
   end
 
+  describe "#plugin_sources" do
+    it "returns an empty array when no plugin sources have been added" do
+      source_list.add_rubygems_remote("https://rubygems.org")
+      source_list.add_path_source("path" => "/path/to/gem")
+
+      expect(source_list.plugin_sources).to be_empty
+    end
+
+    it "returns plugin sources in the reverse order that they were added" do
+      source_list.add_plugin_source("new_source", "uri" => "https://third-git.org/path.git")
+      source_list.add_git_source("https://new-git.org")
+      source_list.add_path_source("path" => "/third/path/to/gem")
+      source_list.add_rubygems_remote("https://fourth-rubygems.org")
+      source_list.add_path_source("path" => "/second/path/to/gem")
+      source_list.add_rubygems_remote("https://third-rubygems.org")
+      source_list.add_plugin_source("new_source", "uri" => "git://second-git.org/path.git")
+      source_list.add_rubygems_remote("https://second-rubygems.org")
+      source_list.add_path_source("path" => "/first/path/to/gem")
+      source_list.add_rubygems_remote("https://first-rubygems.org")
+      source_list.add_plugin_source("new_source", "uri" => "git://first-git.org/path.git")
+
+      expect(source_list.plugin_sources).to eq [
+        ASourcePlugin.new("uri" => "git://first-git.org/path.git"),
+        ASourcePlugin.new("uri" => "git://second-git.org/path.git"),
+        ASourcePlugin.new("uri" => "https://third-git.org/path.git"),
+      ]
+    end
+  end
+
   describe "#rubygems_sources" do
     it "includes the aggregate rubygems source when rubygems sources have been added" do
       source_list.add_git_source("uri" => "git://host/path.git")
@@ -268,17 +333,21 @@ describe Bundler::SourceList do
     it "combines the rubygems sources into a single instance, removing duplicate remotes from the end" do
       source_list.add_git_source("uri" => "git://third-git.org/path.git")
       source_list.add_rubygems_source("remotes" => ["https://duplicate-rubygems.org"])
+      source_list.add_plugin_source("new_source", "uri" => "https://third-bar.org/foo")
       source_list.add_path_source("path" => "/third/path/to/gem")
       source_list.add_rubygems_source("remotes" => ["https://third-rubygems.org"])
       source_list.add_path_source("path" => "/second/path/to/gem")
       source_list.add_rubygems_source("remotes" => ["https://second-rubygems.org"])
       source_list.add_git_source("uri" => "git://second-git.org/path.git")
       source_list.add_rubygems_source("remotes" => ["https://first-rubygems.org"])
+      source_list.add_plugin_source("new_source", "uri" => "https://second-plugin.org/random")
       source_list.add_path_source("path" => "/first/path/to/gem")
       source_list.add_rubygems_source("remotes" => ["https://duplicate-rubygems.org"])
       source_list.add_git_source("uri" => "git://first-git.org/path.git")
 
       expect(source_list.lock_sources).to eq [
+        ASourcePlugin.new("uri" => "https://second-plugin.org/random"),
+        ASourcePlugin.new("uri" => "https://third-bar.org/foo"),
         Bundler::Source::Git.new("uri" => "git://first-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://second-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://third-git.org/path.git"),