Merge branch '24185-legacy-ci-status-reactive-cache' into 'security'

Use ReactiveCaching to update external CI status asynchronously

See https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2055

5 files changed, 308 insertions, 146 deletions

diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index d7e1a4e3b6c..497a626a418 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -1,14 +1,28 @@
 require 'spec_helper'
 
-describe BambooService, models: true do
+describe BambooService, models: true, caching: true do
+  include ReactiveCachingHelpers
+
+  let(:bamboo_url) { 'http://gitlab.com/bamboo' }
+
+  subject(:service) do
+    described_class.create(
+      project: create(:empty_project),
+      properties: {
+        bamboo_url: bamboo_url,
+        username: 'mic',
+        password: 'password',
+        build_key: 'foo'
+      }
+    )
+  end
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
   end
 
   describe 'Validations' do
-    subject { service }
-
     context 'when service is active' do
       before { subject.active = true }
 
@@ -103,90 +117,103 @@ describe BambooService, models: true do
   end
 
   describe '#build_page' do
-    it 'returns a specific URL when status is 500' do
-      stub_request(status: 500)
+    it 'returns the contents of the reactive cache' do
+      stub_reactive_cache(service, { build_page: 'foo' }, 'sha', 'ref')
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
+      expect(service.build_page('sha', 'ref')).to eq('foo')
     end
+  end
 
-    it 'returns a specific URL when response has no results' do
-      stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+  describe '#commit_status' do
+    it 'returns the contents of the reactive cache' do
+      stub_reactive_cache(service, { commit_status: 'foo' }, 'sha', 'ref')
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
+      expect(service.commit_status('sha', 'ref')).to eq('foo')
     end
+  end
 
-    it 'returns a build URL when bamboo_url has no trailing slash' do
-      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+  describe '#calculate_reactive_cache' do
+    context '#build_page' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:build_page] }
 
-      expect(service(bamboo_url: 'http://gitlab.com/bamboo').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
-    end
+      it 'returns a specific URL when status is 500' do
+        stub_request(status: 500)
 
-    it 'returns a build URL when bamboo_url has a trailing slash' do
-      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+        is_expected.to eq('http://gitlab.com/bamboo/browse/foo')
+      end
 
-      expect(service(bamboo_url: 'http://gitlab.com/bamboo/').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
-    end
-  end
+      it 'returns a specific URL when response has no results' do
+        stub_request(body: bamboo_response(size: 0))
 
-  describe '#commit_status' do
-    it 'sets commit status to :error when status is 500' do
-      stub_request(status: 500)
+        is_expected.to eq('http://gitlab.com/bamboo/browse/foo')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
-    end
+      it 'returns a build URL when bamboo_url has no trailing slash' do
+        stub_request(body: bamboo_response)
 
-    it 'sets commit status to "pending" when status is 404' do
-      stub_request(status: 404)
+        is_expected.to eq('http://gitlab.com/bamboo/browse/42')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
-    end
+      context 'bamboo_url has trailing slash' do
+        let(:bamboo_url) { 'http://gitlab.com/bamboo/' }
 
-    it 'sets commit status to "pending" when response has no results' do
-      stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+        it 'returns a build URL' do
+          stub_request(body: bamboo_response)
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
+          is_expected.to eq('http://gitlab.com/bamboo/browse/42')
+        end
+      end
     end
 
-    it 'sets commit status to "success" when build state contains Success' do
-      stub_request(build_state: 'YAY Success!')
+    context '#commit_status' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:commit_status] }
 
-      expect(service.commit_status('123', 'unused')).to eq('success')
-    end
+      it 'sets commit status to :error when status is 500' do
+        stub_request(status: 500)
 
-    it 'sets commit status to "failed" when build state contains Failed' do
-      stub_request(build_state: 'NO Failed!')
+        is_expected.to eq(:error)
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq('failed')
-    end
+      it 'sets commit status to "pending" when status is 404' do
+        stub_request(status: 404)
 
-    it 'sets commit status to "pending" when build state contains Pending' do
-      stub_request(build_state: 'NO Pending!')
+        is_expected.to eq('pending')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
-    end
+      it 'sets commit status to "pending" when response has no results' do
+        stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
 
-    it 'sets commit status to :error when build state is unknown' do
-      stub_request(build_state: 'FOO BAR!')
+        is_expected.to eq('pending')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
-    end
-  end
+      it 'sets commit status to "success" when build state contains Success' do
+        stub_request(body: bamboo_response(build_state: 'YAY Success!'))
 
-  def service(bamboo_url: 'http://gitlab.com/bamboo')
-    described_class.create(
-      project: create(:empty_project),
-      properties: {
-        bamboo_url: bamboo_url,
-        username: 'mic',
-        password: 'password',
-        build_key: 'foo'
-      }
-    )
+        is_expected.to eq('success')
+      end
+
+      it 'sets commit status to "failed" when build state contains Failed' do
+        stub_request(body: bamboo_response(build_state: 'NO Failed!'))
+
+        is_expected.to eq('failed')
+      end
+
+      it 'sets commit status to "pending" when build state contains Pending' do
+        stub_request(body: bamboo_response(build_state: 'NO Pending!'))
+
+        is_expected.to eq('pending')
+      end
+
+      it 'sets commit status to :error when build state is unknown' do
+        stub_request(body: bamboo_response(build_state: 'FOO BAR!'))
+
+        is_expected.to eq(:error)
+      end
+    end
   end
 
-  def stub_request(status: 200, body: nil, build_state: 'success')
+  def stub_request(status: 200, body: nil)
     bamboo_full_url = 'http://mic:password@gitlab.com/bamboo/rest/api/latest/result?label=123&os_authType=basic'
-    body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
 
     WebMock.stub_request(:get, bamboo_full_url).to_return(
       status: status,
@@ -194,4 +221,8 @@ describe BambooService, models: true do
       body: body
     )
   end
+
+  def bamboo_response(result_key: 42, build_state: 'success', size: 1)
+    %Q({"results":{"results":{"size":"#{size}","result":{"buildState":"#{build_state}","planResultKey":{"key":"#{result_key}"}}}}})
+  end
 end
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 6f65beb79d0..dbd23ff5491 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -1,6 +1,21 @@
 require 'spec_helper'
 
-describe BuildkiteService, models: true do
+describe BuildkiteService, models: true, caching: true do
+  include ReactiveCachingHelpers
+
+  let(:project) { create(:empty_project) }
+
+  subject(:service) do
+    described_class.create(
+      project: project,
+      properties: {
+        service_hook: true,
+        project_url: 'https://buildkite.com/account-name/example-project',
+        token: 'secret-sauce-webhook-token:secret-sauce-status-token'
+      }
+    )
+  end
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -25,21 +40,12 @@ describe BuildkiteService, models: true do
 
   describe 'commits methods' do
     before do
-      @project = Project.new
-      allow(@project).to receive(:default_branch).and_return('default-brancho')
-
-      @service = BuildkiteService.new
-      allow(@service).to receive_messages(
-        project: @project,
-        service_hook: true,
-        project_url: 'https://buildkite.com/account-name/example-project',
-        token: 'secret-sauce-webhook-token:secret-sauce-status-token'
-      )
+      allow(project).to receive(:default_branch).and_return('default-brancho')
     end
 
     describe '#webhook_url' do
       it 'returns the webhook url' do
-        expect(@service.webhook_url).to eq(
+        expect(service.webhook_url).to eq(
           'https://webhook.buildkite.com/deliver/secret-sauce-webhook-token'
         )
       end
@@ -47,7 +53,7 @@ describe BuildkiteService, models: true do
 
     describe '#commit_status_path' do
       it 'returns the correct status page' do
-        expect(@service.commit_status_path('2ab7834c')).to eq(
+        expect(service.commit_status_path('2ab7834c')).to eq(
           'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=2ab7834c'
         )
       end
@@ -55,10 +61,53 @@ describe BuildkiteService, models: true do
 
     describe '#build_page' do
       it 'returns the correct build page' do
-        expect(@service.build_page('2ab7834c', nil)).to eq(
+        expect(service.build_page('2ab7834c', nil)).to eq(
           'https://buildkite.com/account-name/example-project/builds?commit=2ab7834c'
         )
       end
     end
+
+    describe '#commit_status' do
+      it 'returns the contents of the reactive cache' do
+        stub_reactive_cache(service, { commit_status: 'foo' }, 'sha', 'ref')
+
+        expect(service.commit_status('sha', 'ref')).to eq('foo')
+      end
+    end
+
+    describe '#calculate_reactive_cache' do
+      context '#commit_status' do
+        subject { service.calculate_reactive_cache('123', 'unused')[:commit_status] }
+
+        it 'sets commit status to :error when status is 500' do
+          stub_request(status: 500)
+
+          is_expected.to eq(:error)
+        end
+
+        it 'sets commit status to :error when status is 404' do
+          stub_request(status: 404)
+
+          is_expected.to eq(:error)
+        end
+
+        it 'passes through build status untouched when status is 200' do
+          stub_request(body: %Q({"status":"Great Success"}))
+
+          is_expected.to eq('Great Success')
+        end
+      end
+    end
+  end
+
+  def stub_request(status: 200, body: nil)
+    body ||= %Q({"status":"success"})
+    buildkite_full_url = 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123'
+
+    WebMock.stub_request(:get, buildkite_full_url).to_return(
+      status: status,
+      headers: { 'Content-Type' => 'application/json' },
+      body: body
+    )
   end
 end
diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb
index f13bb1e8adf..42c2ed668bc 100644
--- a/spec/models/project_services/drone_ci_service_spec.rb
+++ b/spec/models/project_services/drone_ci_service_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
-describe DroneCiService, models: true do
+describe DroneCiService, models: true, caching: true do
+  include ReactiveCachingHelpers
+
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to have_one(:service_hook) }
@@ -33,6 +35,10 @@ describe DroneCiService, models: true do
     let(:token)      { 'secret' }
     let(:iid)        { rand(1..9999) }
 
+    # URL's
+    let(:build_page) { "#{drone_url}/gitlab/#{path}/redirect/commits/#{sha}?branch=#{branch}" }
+    let(:commit_status_path) { "#{drone_url}/gitlab/#{path}/commits/#{sha}?branch=#{branch}&access_token=#{token}" }
+
     before(:each) do
       allow(drone).to receive_messages(
         project_id: project.id,
@@ -42,22 +48,66 @@ describe DroneCiService, models: true do
         token: token
       )
     end
+
+    def stub_request(status: 200, body: nil)
+      body ||= %Q({"status":"success"})
+
+      WebMock.stub_request(:get, commit_status_path).to_return(
+        status: status,
+        headers: { 'Content-Type' => 'application/json' },
+        body: body
+      )
+    end
   end
 
   describe "service page/path methods" do
     include_context :drone_ci_service
 
-    # URL's
-    let(:commit_page) { "#{drone_url}/gitlab/#{path}/redirect/commits/#{sha}?branch=#{branch}" }
-    let(:merge_request_page) { "#{drone_url}/gitlab/#{path}/redirect/pulls/#{iid}" }
-    let(:commit_status_path) { "#{drone_url}/gitlab/#{path}/commits/#{sha}?branch=#{branch}&access_token=#{token}" }
-    let(:merge_request_status_path) { "#{drone_url}/gitlab/#{path}/pulls/#{iid}?access_token=#{token}" }
-
-    it { expect(drone.build_page(sha, branch)).to eq(commit_page) }
-    it { expect(drone.commit_page(sha, branch)).to eq(commit_page) }
-    it { expect(drone.merge_request_page(iid, sha, branch)).to eq(merge_request_page) }
+    it { expect(drone.build_page(sha, branch)).to eq(build_page) }
     it { expect(drone.commit_status_path(sha, branch)).to eq(commit_status_path) }
-    it { expect(drone.merge_request_status_path(iid, sha, branch)).to eq(merge_request_status_path)  }
+  end
+
+  describe '#commit_status' do
+    include_context :drone_ci_service
+
+    it 'returns the contents of the reactive cache' do
+      stub_reactive_cache(drone, { commit_status: 'foo' }, 'sha', 'ref')
+
+      expect(drone.commit_status('sha', 'ref')).to eq('foo')
+    end
+  end
+
+  describe '#calculate_reactive_cache' do
+    include_context :drone_ci_service
+
+    context '#commit_status' do
+      subject { drone.calculate_reactive_cache(sha, branch)[:commit_status] }
+
+      it 'sets commit status to :error when status is 500' do
+        stub_request(status: 500)
+
+        is_expected.to eq(:error)
+      end
+
+      it 'sets commit status to :error when status is 404' do
+        stub_request(status: 404)
+
+        is_expected.to eq(:error)
+      end
+
+      { "killed"  => :canceled,
+        "failure" => :failed,
+        "error"   => :failed,
+        "success" => "success",
+      }.each do |drone_status, our_status|
+
+        it "sets commit status to #{our_status.inspect} when returned status is #{drone_status.inspect}" do
+          stub_request(body: %Q({"status":"#{drone_status}"}))
+
+          is_expected.to eq(our_status)
+        end
+      end
+    end
   end
 
   describe "execute" do
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index f7e878844dc..a1edd083aa1 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -1,14 +1,28 @@
 require 'spec_helper'
 
-describe TeamcityService, models: true do
+describe TeamcityService, models: true, caching: true do
+  include ReactiveCachingHelpers
+
+  let(:teamcity_url) { 'http://gitlab.com/teamcity' }
+
+  subject(:service) do
+    described_class.create(
+      project: create(:empty_project),
+      properties: {
+        teamcity_url: teamcity_url,
+        username: 'mic',
+        password: 'password',
+        build_type: 'foo'
+      }
+    )
+  end
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
   end
 
   describe 'Validations' do
-    subject { service }
-
     context 'when service is active' do
       before { subject.active = true }
 
@@ -103,73 +117,87 @@ describe TeamcityService, models: true do
   end
 
   describe '#build_page' do
-    it 'returns a specific URL when status is 500' do
-      stub_request(status: 500)
+    it 'returns the contents of the reactive cache' do
+      stub_reactive_cache(service, { build_page: 'foo' }, 'sha', 'ref')
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
+      expect(service.build_page('sha', 'ref')).to eq('foo')
     end
+  end
 
-    it 'returns a build URL when teamcity_url has no trailing slash' do
-      stub_request(body: %Q({"build":{"id":"666"}}))
+  describe '#commit_status' do
+    it 'returns the contents of the reactive cache' do
+      stub_reactive_cache(service, { commit_status: 'foo' }, 'sha', 'ref')
 
-      expect(service(teamcity_url: 'http://gitlab.com/teamcity').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+      expect(service.commit_status('sha', 'ref')).to eq('foo')
     end
+  end
 
-    it 'returns a build URL when teamcity_url has a trailing slash' do
-      stub_request(body: %Q({"build":{"id":"666"}}))
+  describe '#calculate_reactive_cache' do
+    context 'build_page' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:build_page] }
 
-      expect(service(teamcity_url: 'http://gitlab.com/teamcity/').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
-    end
-  end
+      it 'returns a specific URL when status is 500' do
+        stub_request(status: 500)
 
-  describe '#commit_status' do
-    it 'sets commit status to :error when status is 500' do
-      stub_request(status: 500)
+        is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
-    end
+      it 'returns a build URL when teamcity_url has no trailing slash' do
+        stub_request(body: %Q({"build":{"id":"666"}}))
 
-    it 'sets commit status to "pending" when status is 404' do
-      stub_request(status: 404)
+        is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
-    end
+      context 'teamcity_url has trailing slash' do
+        let(:teamcity_url) { 'http://gitlab.com/teamcity/' }
 
-    it 'sets commit status to "success" when build status contains SUCCESS' do
-      stub_request(build_status: 'YAY SUCCESS!')
+        it 'returns a build URL' do
+          stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service.commit_status('123', 'unused')).to eq('success')
+          is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+        end
+      end
     end
 
-    it 'sets commit status to "failed" when build status contains FAILURE' do
-      stub_request(build_status: 'NO FAILURE!')
+    context 'commit_status' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:commit_status] }
 
-      expect(service.commit_status('123', 'unused')).to eq('failed')
-    end
+      it 'sets commit status to :error when status is 500' do
+        stub_request(status: 500)
 
-    it 'sets commit status to "pending" when build status contains Pending' do
-      stub_request(build_status: 'NO Pending!')
+        is_expected.to eq(:error)
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
-    end
+      it 'sets commit status to "pending" when status is 404' do
+        stub_request(status: 404)
 
-    it 'sets commit status to :error when build status is unknown' do
-      stub_request(build_status: 'FOO BAR!')
+        is_expected.to eq('pending')
+      end
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
-    end
-  end
+      it 'sets commit status to "success" when build status contains SUCCESS' do
+        stub_request(build_status: 'YAY SUCCESS!')
 
-  def service(teamcity_url: 'http://gitlab.com/teamcity')
-    described_class.create(
-      project: create(:empty_project),
-      properties: {
-        teamcity_url: teamcity_url,
-        username: 'mic',
-        password: 'password',
-        build_type: 'foo'
-      }
-    )
+        is_expected.to eq('success')
+      end
+
+      it 'sets commit status to "failed" when build status contains FAILURE' do
+        stub_request(build_status: 'NO FAILURE!')
+
+        is_expected.to eq('failed')
+      end
+
+      it 'sets commit status to "pending" when build status contains Pending' do
+        stub_request(build_status: 'NO Pending!')
+
+        is_expected.to eq('pending')
+      end
+
+      it 'sets commit status to :error when build status is unknown' do
+        stub_request(build_status: 'FOO BAR!')
+
+        is_expected.to eq(:error)
+      end
+    end
   end
 
   def stub_request(status: 200, body: nil, build_status: 'success')
diff --git a/spec/support/reactive_caching_helpers.rb b/spec/support/reactive_caching_helpers.rb
index 279db3c5748..98eb57f8b54 100644
--- a/spec/support/reactive_caching_helpers.rb
+++ b/spec/support/reactive_caching_helpers.rb
@@ -3,31 +3,35 @@ module ReactiveCachingHelpers
     ([subject.class.reactive_cache_key.call(subject)].flatten + qualifiers).join(':')
   end
 
-  def stub_reactive_cache(subject = nil, data = nil)
+  def alive_reactive_cache_key(subject, *qualifiers)
+    reactive_cache_key(subject, *(qualifiers + ['alive']))
+  end
+
+  def stub_reactive_cache(subject = nil, data = nil, *qualifiers)
     allow(ReactiveCachingWorker).to receive(:perform_async)
     allow(ReactiveCachingWorker).to receive(:perform_in)
-    write_reactive_cache(subject, data) if data
+    write_reactive_cache(subject, data, *qualifiers) if data
   end
 
-  def read_reactive_cache(subject)
-    Rails.cache.read(reactive_cache_key(subject))
+  def read_reactive_cache(subject, *qualifiers)
+    Rails.cache.read(reactive_cache_key(subject, *qualifiers))
   end
 
-  def write_reactive_cache(subject, data)
-    start_reactive_cache_lifetime(subject)
-    Rails.cache.write(reactive_cache_key(subject), data)
+  def write_reactive_cache(subject, data, *qualifiers)
+    start_reactive_cache_lifetime(subject, *qualifiers)
+    Rails.cache.write(reactive_cache_key(subject, *qualifiers), data)
   end
 
-  def reactive_cache_alive?(subject)
-    Rails.cache.read(reactive_cache_key(subject, 'alive'))
+  def reactive_cache_alive?(subject, *qualifiers)
+    Rails.cache.read(alive_reactive_cache_key(subject, *qualifiers))
   end
 
-  def invalidate_reactive_cache(subject)
-    Rails.cache.delete(reactive_cache_key(subject, 'alive'))
+  def invalidate_reactive_cache(subject, *qualifiers)
+    Rails.cache.delete(alive_reactive_cache_key(subject, *qualifiers))
   end
 
-  def start_reactive_cache_lifetime(subject)
-    Rails.cache.write(reactive_cache_key(subject, 'alive'), true)
+  def start_reactive_cache_lifetime(subject, *qualifiers)
+    Rails.cache.write(alive_reactive_cache_key(subject, *qualifiers), true)
   end
 
   def expect_reactive_cache_update_queued(subject)