Merge pull request #102 from fluent/refactor_fluent_gem_command

Refactor fluent gem command

app/models/fluent_gem.rb

@@ -0,0 +1,58 @@
+module FluentGem
+  class GemError < StandardError; end
+
+  class << self
+    def install(*args)
+      run("install", *args)
+    end
+
+    def uninstall(*args)
+      run("uninstall", *args)
+    end
+
+    def list
+      output = `#{gem} list`
+      unless $?.exitstatus.zero?
+        raise GemError, "failed command: `#{gem} list`"
+      end
+      output.lines
+    end
+
+    def run(*args)
+      # NOTE: use `fluent-gem` instead of `gem`
+      Bundler.with_clean_env do
+        # NOTE: this app is under the Bundler, so call `system` in with_clean_env is Bundler jail breaking
+        cmd = [gem, *args].compact
+        unless system(*cmd)
+          raise GemError, "failed command: `#{cmd.join(" ")}`"
+        end
+      end
+      true
+    end
+
+    def gem
+      # Not yet setup any fluentd/td-agent
+      return "fluent-gem" unless Fluentd.instance
+
+      # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance)
+      if Fluentd.instance && Fluentd.instance.fluentd?
+        "fluent-gem" # maybe `fluent-gem` command is in the $PATH
+      else
+        detect_td_agent_gem
+      end
+    end
+
+    def detect_td_agent_gem
+      # NOTE: td-agent has a command under the /usr/lib{,64}, td-agent2 has under /opt/td-agent
+      %W(
+        /usr/sbin/td-agent-gem
+        /opt/td-agent/embedded/bin/fluent-gem
+        /usr/lib/fluent/ruby/bin/fluent-gem
+        /usr/lib64/fluent/ruby/bin/fluent-gem
+        fluent-gem
+      ).find do |path|
+        system("which #{path}", out: File::NULL, err: File::NULL)
+      end
+    end
+  end
+end

app/models/plugin.rb

@@ -89,10 +89,7 @@ class Plugin
   def self.installed
     Rails.cache.fetch("installed_gems", expires_in: 3.seconds) do
       Bundler.with_clean_env do
-        fluent_gem = fluent_gem_path
-        return [] unless fluent_gem
-        gems = `#{fluent_gem} list`.try(:lines)
-        return [] unless gems
+        gems = FluentGem.list
         gems.grep(/fluent-plugin/).map do |gem|
           name, versions_str = gem.strip.split(" ")
           version = versions_str[/[^(), ]+/]
@@ -141,24 +138,6 @@ class Plugin
     "https://rubygems.org/api/v1/versions/#{gem_name}.json"
   end
 
-  def self.fluent_gem_path
-    # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance)
-    if Fluentd.instance && Fluentd.instance.fluentd?
-      return "fluent-gem" # maybe `fluent-gem` command is in the $PATH
-    end
-
-    # NOTE: td-agent has a command under the /usr/lib{,64}, td-agent2 has under /opt/td-agent
-    %W(
-      /usr/sbin/td-agent-gem
-      /opt/td-agent/embedded/bin/fluent-gem
-      /usr/lib/fluent/ruby/bin/fluent-gem
-      /usr/lib64/fluent/ruby/bin/fluent-gem
-      fluent-gem
-    ).find do |path|
-      system("which #{path}", out: File::NULL, err: File::NULL)
-    end
-  end
-
   private
 
   def gem_install
@@ -166,7 +145,7 @@ class Plugin
     return if processing?
     return if installed?
     WORKING.push(data)
-    fluent_gem("install", gem_name, "--no-document", "-v", version)
+    FluentGem.install(gem_name, "--no-document", "-v", version)
   ensure
     WORKING.delete(data)
   end
@@ -176,23 +155,8 @@ class Plugin
     return if processing?
     return unless installed?
     WORKING.push(data)
-    fluent_gem("uninstall", gem_name, "-x", "-a")
+    FluentGem.uninstall(gem_name, "-x", "-a")
   ensure
     WORKING.delete(data)
   end
-
-  def fluent_gem(*commands)
-    # NOTE: use `fluent-gem` instead of `gem`
-    Bundler.with_clean_env do
-      # NOTE: this app is under the Bundler, so call `system` in with_clean_env is Bundler jail breaking
-      unless system(* [fluent_gem_path, *commands])
-        raise GemError, "failed command #{commands.join(" ")}"
-      end
-    end
-    true
-  end
-
-  def fluent_gem_path
-    self.class.fluent_gem_path
-  end
 end

spec/lib/file_reverse_reader_spec.rb

@@ -39,12 +39,12 @@ describe FileReverseReader do
 
     context "contain ascii only" do
       let(:content) { "ABCDE" }
-      it { should be_false }
+      it { should == false }
     end
 
     context "contain non-ascii" do
       let(:content) { "\x89NG" }
-      it { should be_true }
+      it { should == true }
     end
   end
 

spec/models/fluent_gem_spec.rb

@@ -0,0 +1,110 @@
+require 'spec_helper'
+
+describe FluentGem do
+  describe "#install" do
+    let(:gem) { FluentGem.gem }
+
+    context "no argument" do
+      after { FluentGem.install }
+      it { FluentGem.should_receive(:run).with("install") }
+    end
+
+    context "with arguments" do
+      after { FluentGem.install(*args) }
+
+      context "1" do
+        let(:args) { ["plugin-foo"] }
+        it { FluentGem.should_receive(:run).with("install", *args) }
+      end
+
+      context "2" do
+        let(:args) { ["plugin-foo", "--no-document"] }
+        it { FluentGem.should_receive(:run).with("install", *args) }
+      end
+    end
+  end
+
+  describe "#uninstall" do
+    let(:gem) { FluentGem.gem }
+
+    context "no argument" do
+      after { FluentGem.uninstall }
+      it { FluentGem.should_receive(:run).with("uninstall") }
+    end
+
+    context "with arguments" do
+      after { FluentGem.uninstall(*args) }
+
+      context "1" do
+        let(:args) { ["plugin-foo"] }
+        it { FluentGem.should_receive(:run).with("uninstall", *args) }
+      end
+
+      context "2" do
+        let(:args) { ["plugin-foo", "--no-document"] }
+        it { FluentGem.should_receive(:run).with("uninstall", *args) }
+      end
+    end
+  end
+
+  describe "#list" do
+    before { FluentGem.stub(:`).and_return(gem_list) }
+    subject { FluentGem.list }
+
+    context "no list" do
+      let(:gem_list) { "" }
+      it { subject.to_a.should == [] }
+    end
+
+    context "some lines" do
+      let(:gem_list) { <<-GEM.strip_heredoc }
+        dummy (3.3.3)
+        fluent-plugin-foo (0.1.2)
+        more_dummy (0.0.1)
+      GEM
+      it { subject.to_a.should == gem_list.lines.to_a }
+    end
+
+    context "failed" do
+      let(:gem_list) { "" }
+      before { $?.stub(:exitstatus).and_return(128) }
+      it { expect{ subject }.to raise_error(FluentGem::GemError) }
+    end
+  end
+
+  describe "#run" do
+    before { FluentGem.stub(:system).and_return(ret) }
+    let(:args) { ["install", "foobar"] }
+
+    describe "success" do
+      let(:ret) { true }
+      after { FluentGem.run(*args) }
+      it { FluentGem.should_receive(:system) }
+    end
+
+    describe "failed" do
+      let(:ret) { false }
+      it { expect{ FluentGem.run(*args) }.to raise_error(FluentGem::GemError) }
+    end
+  end
+
+  describe "#gem" do
+    before { Fluentd.stub(:instance).and_return(instance) }
+    subject { FluentGem.gem }
+
+    context "any instance not setup yet" do
+      let(:instance) { nil }
+      it { should == "fluent-gem" }
+    end
+
+    context "fluentd setup" do
+      let(:instance) { Fluentd.new(id: nil, variant: "fluentd_gem", log_file: "dummy.log", pid_file: "dummy.pid", config_file: "dummy.conf") }
+      it { should == "fluent-gem" }
+    end
+
+    context "td-agent 2 setup" do
+      let(:instance) { Fluentd.new(id: nil, variant: "td_agent", log_file: "dummy.log", pid_file: "dummy.pid", config_file: "dummy.conf") }
+      it { should == FluentGem.detect_td_agent_gem }
+    end
+  end
+end

spec/models/plugin_spec.rb

@@ -4,7 +4,7 @@ describe Plugin do
   let(:plugin) { build(:plugin) }
 
   describe ".installed" do
-    before { Plugin.stub(:"`").and_return(gem_list) }
+    before { FluentGem.stub(:"`").and_return(gem_list) }
 
     context "fluent-plugin-foo 0.1.2" do
       let(:target) { Plugin.new(gem_name: "fluent-plugin-foo", version: "0.1.2") }
@@ -73,12 +73,12 @@ describe Plugin do
 
         context "installed" do
           let(:installed) { true }
-          it { plugin.should_not_receive(:fluent_gem) }
+          it { FluentGem.should_not_receive(:install) }
         end
 
         context "not installed" do
           let(:installed) { false }
-          it { plugin.should_receive(:fluent_gem) }
+          it { FluentGem.should_receive(:install) }
         end
       end
 
@@ -87,22 +87,22 @@ describe Plugin do
 
         context "installed" do
           let(:installed) { true }
-          it { plugin.should_not_receive(:fluent_gem) }
+          it { FluentGem.should_not_receive(:install) }
         end
 
         context "not installed" do
           let(:installed) { false }
-          it { plugin.should_not_receive(:fluent_gem) }
+          it { FluentGem.should_not_receive(:installed) }
         end
       end
     end
 
     context "system command error" do
-      before { plugin.should_receive(:system).at_least(1).and_return(false) }
+      before { FluentGem.should_receive(:system).at_least(1).and_return(false) }
       subject { expect { plugin.install! } }
 
       it "raise GemError" do
-        subject.to raise_error(Plugin::GemError)
+        subject.to raise_error(FluentGem::GemError)
       end
 
       it "error message contains gem name" do
@@ -139,10 +139,10 @@ describe Plugin do
     before do
       # NOTE: not `plugin.stub` because upgrade! creates new Plugin instance internally
       installed_plugin.stub(:installed?).and_return(true)
-      Plugin.any_instance.stub(:fluent_gem).and_return(true)
+      FluentGem.stub(:run).and_return(true)
 
       installed_plugin.should_receive(:uninstall!)
-      Plugin.any_instance.should_receive(:install!)
+      FluentGem.should_receive(:install)
     end
 
     it { installed_plugin.upgrade!(target_version) }