Use Redis Ruby client instead of shelling out to redis-cli

Closes gitlab-org/gitlab-ce#17329

3 files changed, 51 insertions, 49 deletions

diff --git a/spec/gitlab_config_spec.rb b/spec/gitlab_config_spec.rb
index 0ce641b..6c1af85 100644
--- a/spec/gitlab_config_spec.rb
+++ b/spec/gitlab_config_spec.rb
@@ -38,7 +38,7 @@ eos
 
     context 'remove trailing slashes' do
       before { config.send(:config)['gitlab_url'] = url + '//' }
-      
+
       it { should eq(url) }
     end
   end
@@ -48,40 +48,4 @@ eos
 
     it("returns false by default") { should eq(false) }
   end
-
-  describe :redis_command do
-    subject { config.redis_command }
-
-    context "with empty redis config" do
-      before do
-        config.stub(:redis) { {} }
-      end
-
-      it { should be_an(Array) }
-      it { should include('redis-cli') }
-    end
-
-    context "with host and port" do
-      before do
-        config.stub(:redis) { {'host' => 'localhost', 'port' => 1123, 'bin' => '/usr/bin/redis-cli'} }
-      end
-
-      it { should be_an(Array) }
-      it { should include(config.redis['host']) }
-      it { should include(config.redis['bin']) }
-      it { should include(config.redis['port'].to_s) }
-    end
-
-    context "with redis socket" do
-      let(:socket) { '/tmp/redis.socket' }
-      before do
-        config.stub(:redis) { {'bin' => '', 'socket' => socket } }
-      end
-
-      it { should be_an(Array) }
-      it { should include(socket) }
-      it { should_not include('-p') }
-      it { should_not include('-h') }
-    end
-  end
 end
diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb
index e4dee33..1312554 100644
--- a/spec/gitlab_net_spec.rb
+++ b/spec/gitlab_net_spec.rb
@@ -229,4 +229,44 @@ describe GitlabNet, vcr: true do
       store.should_receive(:add_path).with('test_path')
     end
   end
+
+  describe '#redis_client' do
+    let(:config) { double('config') }
+
+    context "with empty redis config" do
+      it 'returns default parameters' do
+        allow(gitlab_net).to receive(:config).and_return(config)
+        allow(config).to receive(:redis).and_return( {} )
+
+        expect_any_instance_of(Redis).to receive(:initialize).with({ host: '127.0.0.1',
+                                                                     port: 6379,
+                                                                     db: 0 }).and_call_original
+        gitlab_net.redis_client
+      end
+    end
+
+    context "with host and port" do
+      it 'uses the specified host and port' do
+        allow(gitlab_net).to receive(:config).and_return(config)
+        allow(config).to receive(:redis).and_return( { 'host' => 'localhost', 'port' => 1123 } )
+
+        expect_any_instance_of(Redis).to receive(:initialize).with({ host: 'localhost',
+                                                                     port: 1123,
+                                                                     db: 0 }).and_call_original
+        gitlab_net.redis_client
+      end
+    end
+
+    context "with redis socket" do
+      let(:socket) { '/tmp/redis.socket' }
+
+      it 'uses the socket' do
+        allow(gitlab_net).to receive(:config).and_return(config)
+        allow(config).to receive(:redis).and_return( { 'socket' => socket })
+
+        expect_any_instance_of(Redis).to receive(:initialize).with({ path: socket, db: 0 }).and_call_original
+        gitlab_net.redis_client
+      end
+    end
+  end
 end
diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb
index 9d7696e..0392bee 100644
--- a/spec/gitlab_post_receive_spec.rb
+++ b/spec/gitlab_post_receive_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 require 'gitlab_post_receive'
 
@@ -11,6 +12,7 @@ describe GitlabPostReceive do
   let(:repo_path)  { File.join(repository_path, repo_name) + ".git" }
   let(:gitlab_post_receive) { GitlabPostReceive.new(repo_path, actor, wrongly_encoded_changes) }
   let(:message) { "test " * 10 + "message " * 10 }
+  let(:redis_client) { double('redis_client') }
 
   before do
     GitlabConfig.any_instance.stub(repos_path: repository_path)
@@ -20,11 +22,11 @@ describe GitlabPostReceive do
   describe "#exec" do
 
     before do
-      GitlabConfig.any_instance.stub(redis_command: %w(env -i redis-cli))
-      allow(gitlab_post_receive).to receive(:system).and_return(true)
+      allow_any_instance_of(GitlabNet).to receive(:redis_client).and_return(redis_client)
     end
 
     it "prints the broadcast message" do
+      expect(redis_client).to receive(:rpush)
       expect(gitlab_post_receive).to receive(:puts).ordered
       expect(gitlab_post_receive).to receive(:puts).with(
         "========================================================================"
@@ -38,7 +40,7 @@ describe GitlabPostReceive do
         "    message message message message message message message message"
       ).ordered
 
-      expect(gitlab_post_receive).to receive(:puts).ordered      
+      expect(gitlab_post_receive).to receive(:puts).ordered
       expect(gitlab_post_receive).to receive(:puts).with(
         "========================================================================"
       ).ordered
@@ -47,12 +49,9 @@ describe GitlabPostReceive do
     end
 
     it "pushes a Sidekiq job onto the queue" do
-      expect(gitlab_post_receive).to receive(:system).with(
-        *[
-          *%w(env -i redis-cli rpush resque:gitlab:queue:post_receive), 
-          %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}/,
-          { err: "/dev/null", out: "/dev/null" }
-        ]
+      expect(redis_client).to receive(:rpush).with(
+        'resque:gitlab:queue:post_receive',
+         %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}/
       ).and_return(true)
 
       gitlab_post_receive.exec
@@ -61,7 +60,7 @@ describe GitlabPostReceive do
     context "when the redis command succeeds" do
 
       before do
-        allow(gitlab_post_receive).to receive(:system).and_return(true)
+        allow(redis_client).to receive(:rpush).and_return(true)
       end
 
       it "returns true" do
@@ -72,8 +71,7 @@ describe GitlabPostReceive do
     context "when the redis command fails" do
 
       before do
-        allow(gitlab_post_receive).to receive(:system).and_return(false)
-        allow($?).to receive(:exitstatus).and_return(nil)
+        allow(redis_client).to receive(:rpush).and_raise('Fail')
       end
 
       it "returns false" do