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

app/controllers/projects/merge_requests_controller.rb

@@ -409,10 +409,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     else
       ci_service = @merge_request.source_project.try(:ci_service)
       status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service
-
-      if ci_service.respond_to?(:commit_coverage)
-        coverage = ci_service.commit_coverage(merge_request.diff_head_sha, merge_request.source_branch)
-      end
     end
 
     response = {

app/models/concerns/reactive_caching.rb

@@ -55,30 +55,30 @@ module ReactiveCaching
     self.reactive_cache_refresh_interval = 1.minute
     self.reactive_cache_lifetime = 10.minutes
 
-    def calculate_reactive_cache
+    def calculate_reactive_cache(*args)
       raise NotImplementedError
     end
 
-    def with_reactive_cache(&blk)
-      within_reactive_cache_lifetime do
-        data = Rails.cache.read(full_reactive_cache_key)
+    def with_reactive_cache(*args, &blk)
+      within_reactive_cache_lifetime(*args) do
+        data = Rails.cache.read(full_reactive_cache_key(*args))
         yield data if data.present?
       end
     ensure
-      Rails.cache.write(full_reactive_cache_key('alive'), true, expires_in: self.class.reactive_cache_lifetime)
-      ReactiveCachingWorker.perform_async(self.class, id)
+      Rails.cache.write(alive_reactive_cache_key(*args), true, expires_in: self.class.reactive_cache_lifetime)
+      ReactiveCachingWorker.perform_async(self.class, id, *args)
     end
 
-    def clear_reactive_cache!
-      Rails.cache.delete(full_reactive_cache_key)
+    def clear_reactive_cache!(*args)
+      Rails.cache.delete(full_reactive_cache_key(*args))
     end
 
-    def exclusively_update_reactive_cache!
-      locking_reactive_cache do
-        within_reactive_cache_lifetime do
-          enqueuing_update do
-            value = calculate_reactive_cache
-            Rails.cache.write(full_reactive_cache_key, value)
+    def exclusively_update_reactive_cache!(*args)
+      locking_reactive_cache(*args) do
+        within_reactive_cache_lifetime(*args) do
+          enqueuing_update(*args) do
+            value = calculate_reactive_cache(*args)
+            Rails.cache.write(full_reactive_cache_key(*args), value)
           end
         end
       end
@@ -93,22 +93,26 @@ module ReactiveCaching
       ([prefix].flatten + qualifiers).join(':')
     end
 
-    def locking_reactive_cache
-      lease = Gitlab::ExclusiveLease.new(full_reactive_cache_key, timeout: reactive_cache_lease_timeout)
+    def alive_reactive_cache_key(*qualifiers)
+      full_reactive_cache_key(*(qualifiers + ['alive']))
+    end
+
+    def locking_reactive_cache(*args)
+      lease = Gitlab::ExclusiveLease.new(full_reactive_cache_key(*args), timeout: reactive_cache_lease_timeout)
       uuid = lease.try_obtain
       yield if uuid
     ensure
-      Gitlab::ExclusiveLease.cancel(full_reactive_cache_key, uuid)
+      Gitlab::ExclusiveLease.cancel(full_reactive_cache_key(*args), uuid)
     end
 
-    def within_reactive_cache_lifetime
-      yield if Rails.cache.read(full_reactive_cache_key('alive'))
+    def within_reactive_cache_lifetime(*args)
+      yield if Rails.cache.read(alive_reactive_cache_key(*args))
     end
 
-    def enqueuing_update
+    def enqueuing_update(*args)
       yield
     ensure
-      ReactiveCachingWorker.perform_in(self.class.reactive_cache_refresh_interval, self.class, id)
+      ReactiveCachingWorker.perform_in(self.class.reactive_cache_refresh_interval, self.class, id, *args)
     end
   end
 end

app/models/concerns/reactive_service.rb

+module ReactiveService
+  extend ActiveSupport::Concern
+
+  included do
+    include ReactiveCaching
+
+    # Default cache key: class name + project_id
+    self.reactive_cache_key = ->(service) { [ service.class.model_name.singular, service.project_id ] }
+  end
+end

app/models/project_services/bamboo_service.rb

 class BambooService < CiService
+  include ReactiveService
+
   prop_accessor :bamboo_url, :build_key, :username, :password
 
   validates :bamboo_url, presence: true, url: true, if: :activated?
@@ -58,31 +60,46 @@ class BambooService < CiService
     %w(push)
   end
 
-  def build_info(sha)
-    @response = get_path("rest/api/latest/result?label=#{sha}")
+  def build_page(sha, ref)
+    with_reactive_cache(sha, ref) {|cached| cached[:build_page] }
   end
 
-  def build_page(sha, ref)
-    build_info(sha) if @response.nil? || !@response.code
+  def commit_status(sha, ref)
+    with_reactive_cache(sha, ref) {|cached| cached[:commit_status] }
+  end
 
-    if @response.code != 200 || @response['results']['results']['size'] == '0'
+  def execute(data)
+    return unless supported_events.include?(data[:object_kind])
+
+    get_path("updateAndBuild.action?buildKey=#{build_key}")
+  end
+
+  def calculate_reactive_cache(sha, ref)
+    response = get_path("rest/api/latest/result?label=#{sha}")
+
+    { build_page: read_build_page(response), commit_status: read_commit_status(response) }
+  end
+
+  private
+
+  def read_build_page(response)
+    if response.code != 200 || response['results']['results']['size'] == '0'
       # If actual build link can't be determined, send user to build summary page.
       URI.join("#{bamboo_url}/", "browse/#{build_key}").to_s
     else
       # If actual build link is available, go to build result page.
-      result_key = @response['results']['results']['result']['planResultKey']['key']
+      result_key = response['results']['results']['result']['planResultKey']['key']
       URI.join("#{bamboo_url}/", "browse/#{result_key}").to_s
     end
   end
 
-  def commit_status(sha, ref)
-    build_info(sha) if @response.nil? || !@response.code
-    return :error unless @response.code == 200 || @response.code == 404
+  def read_commit_status(response)
+    return :error unless response.code == 200 || response.code == 404
 
-    status = if @response.code == 404 || @response['results']['results']['size'] == '0'
+    status = if response.code == 404 || response['results']['results']['size'] == '0'
                'Pending'
              else
-               @response['results']['results']['result']['buildState']
+               response['results']['results']['result']['buildState']
              end
 
     if status.include?('Success')
@@ -96,14 +113,6 @@ class BambooService < CiService
     end
   end
 
-  def execute(data)
-    return unless supported_events.include?(data[:object_kind])
-
-    get_path("updateAndBuild.action?buildKey=#{build_key}")
-  end
-
-  private
-
   def build_url(path)
     URI.join("#{bamboo_url}/", path).to_s
   end

app/models/project_services/buildkite_service.rb

 require "addressable/uri"
 
 class BuildkiteService < CiService
+  include ReactiveService
+
   ENDPOINT = "https://buildkite.com"
 
   prop_accessor :project_url, :token
@@ -33,13 +35,7 @@ class BuildkiteService < CiService
   end
 
   def commit_status(sha, ref)
-    response = HTTParty.get(commit_status_path(sha), verify: false)
-
-    if response.code == 200 && response['status']
-      response['status']
-    else
-      :error
-    end
+    with_reactive_cache(sha, ref) {|cached| cached[:commit_status] }
   end
 
   def commit_status_path(sha)
@@ -78,6 +74,19 @@ class BuildkiteService < CiService
     ]
   end
 
+  def calculate_reactive_cache(sha, ref)
+    response = HTTParty.get(commit_status_path(sha), verify: false)
+
+    status =
+      if response.code == 200 && response['status']
+        response['status']
+      else
+        :error
+      end
+
+    { commit_status: status }
+  end
+
   private
 
   def webhook_token

app/models/project_services/ci_service.rb

@@ -12,15 +12,7 @@ class CiService < Service
     %w(push)
   end
 
-  def merge_request_page(iid, sha, ref)
-    commit_page(sha, ref)
-  end
-
-  def commit_page(sha, ref)
-    build_page(sha, ref)
-  end
-
-  # Return complete url to merge_request page
+  # Return complete url to build page
   #
   # Ex.
   #   http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c
@@ -29,23 +21,6 @@ class CiService < Service
     # implement inside child
   end
 
-  # Return string with build status or :error symbol
-  #
-  # Allowed states: 'success', 'failed', 'running', 'pending', 'skipped'
-  #
-  #
-  # Ex.
-  #   @service.merge_request_status(9, '13be4ac', 'dev')
-  #   # => 'success'
-  #
-  #   @service.merge_request_status(10, '2abe4ac', 'dev)
-  #   # => 'running'
-  #
-  #
-  def merge_request_status(iid, sha, ref)
-    commit_status(sha, ref)
-  end
-
   # Return string with build status or :error symbol
   #
   # Allowed states: 'success', 'failed', 'running', 'pending', 'skipped'

app/models/project_services/drone_ci_service.rb

 class DroneCiService < CiService
+  include ReactiveService
+
   prop_accessor :drone_url, :token
   boolean_accessor :enable_ssl_verification
 
@@ -34,14 +36,6 @@ class DroneCiService < CiService
     %w(push merge_request tag_push)
   end
 
-  def merge_request_status_path(iid, sha = nil, ref = nil)
-    url = [drone_url,
-           "gitlab/#{project.namespace.path}/#{project.path}/pulls/#{iid}",
-           "?access_token=#{token}"]
-
-    URI.join(*url).to_s
-  end
-
   def commit_status_path(sha, ref)
     url = [drone_url,
            "gitlab/#{project.namespace.path}/#{project.path}/commits/#{sha}",
@@ -50,29 +44,14 @@ class DroneCiService < CiService
     URI.join(*url).to_s
   end
 
-  def merge_request_status(iid, sha, ref)
-    response = HTTParty.get(merge_request_status_path(iid), verify: enable_ssl_verification)
-
-    if response.code == 200 and response['status']
-      case response['status']
-      when 'killed'
-        :canceled
-      when 'failure', 'error'
-        # Because drone return error if some test env failed
-        :failed
-      else
-        response["status"]
-      end
-    else
-      :error
-    end
-  rescue Errno::ECONNREFUSED
-    :error
+  def commit_status(sha, ref)
+    with_reactive_cache(sha, ref) {|cached| cached[:commit_status] }
   end
 
-  def commit_status(sha, ref)
+  def calculate_reactive_cache(sha, ref)
     response = HTTParty.get(commit_status_path(sha, ref), verify: enable_ssl_verification)
 
+    status =
       if response.code == 200 and response['status']
         case response['status']
         when 'killed'
@@ -86,18 +65,13 @@ class DroneCiService < CiService
       else
         :error
       end
-  rescue Errno::ECONNREFUSED
-    :error
-  end
-
-  def merge_request_page(iid, sha, ref)
-    url = [drone_url,
-           "gitlab/#{project.namespace.path}/#{project.path}/redirect/pulls/#{iid}"]
 
-    URI.join(*url).to_s
+    { commit_status: status }
+  rescue Errno::ECONNREFUSED
+    { commit_status: :error }
   end
 
-  def commit_page(sha, ref)
+  def build_page(sha, ref)
     url = [drone_url,
            "gitlab/#{project.namespace.path}/#{project.path}/redirect/commits/#{sha}",
            "?branch=#{URI::encode(ref.to_s)}"]
@@ -105,14 +79,6 @@ class DroneCiService < CiService
     URI.join(*url).to_s
   end
 
-  def commit_coverage(sha, ref)
-    nil
-  end
-
-  def build_page(sha, ref)
-    commit_page(sha, ref)
-  end
-
   def title
     'Drone CI'
   end

app/models/project_services/teamcity_service.rb

 class TeamcityService < CiService
+  include ReactiveService
+
   prop_accessor :teamcity_url, :build_type, :username, :password
 
   validates :teamcity_url, presence: true, url: true, if: :activated?
@@ -61,43 +63,18 @@ class TeamcityService < CiService
     ]
   end
 
-  def build_info(sha)
-    @response = get_path("httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}")
-  end
-
   def build_page(sha, ref)
-    build_info(sha) if @response.nil? || !@response.code
-
-    if @response.code != 200
-      # If actual build link can't be determined,
-      # send user to build summary page.
-      build_url("viewLog.html?buildTypeId=#{build_type}")
-    else
-      # If actual build link is available, go to build result page.
-      built_id = @response['build']['id']
-      build_url("viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}")
-    end
+    with_reactive_cache(sha, ref) {|cached| cached[:build_page] }
   end
 
   def commit_status(sha, ref)
-    build_info(sha) if @response.nil? || !@response.code
-    return :error unless @response.code == 200 || @response.code == 404
-
-    status = if @response.code == 404
-               'Pending'
-             else
-               @response['build']['status']
+    with_reactive_cache(sha, ref) {|cached| cached[:commit_status] }
   end
 
-    if status.include?('SUCCESS')
-      'success'
-    elsif status.include?('FAILURE')
-      'failed'
-    elsif status.include?('Pending')
-      'pending'
-    else
-      :error
-    end
+  def calculate_reactive_cache(sha, ref)
+    response = get_path("httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}")
+
+    { build_page: read_build_page(response), commit_status: read_commit_status(response) }
   end
 
   def execute(data)
@@ -122,6 +99,40 @@ class TeamcityService < CiService
 
   private
 
+  def read_build_page(response)
+    if response.code != 200
+      # If actual build link can't be determined,
+      # send user to build summary page.
+      build_url("viewLog.html?buildTypeId=#{build_type}")
+    else
+      # If actual build link is available, go to build result page.
+      built_id = response['build']['id']
+      build_url("viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}")
+    end
+  end
+
+  def read_commit_status(response)
+    return :error unless response.code == 200 || response.code == 404
+
+    status = if response.code == 404
+               'Pending'
+             else
+               response['build']['status']
+             end
+
+    return :error unless status.present?
+
+    if status.include?('SUCCESS')
+      'success'
+    elsif status.include?('FAILURE')
+      'failed'
+    elsif status.include?('Pending')
+      'pending'
+    else
+      :error
+    end
+  end
+
   def build_url(path)
     URI.join("#{teamcity_url}/", path).to_s
   end

app/workers/reactive_caching_worker.rb

@@ -2,7 +2,7 @@ class ReactiveCachingWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
-  def perform(class_name, id)
+  def perform(class_name, id, *args)
     klass = begin
       Kernel.const_get(class_name)
     rescue NameError
@@ -10,6 +10,6 @@ class ReactiveCachingWorker
     end
     return unless klass
 
-    klass.find_by(id: id).try(:exclusively_update_reactive_cache!)
+    klass.find_by(id: id).try(:exclusively_update_reactive_cache!, *args)
   end
 end

changelogs/unreleased/24185-legacy-ci-status-reactive-cache.yml

+---
+title: Query external CI statuses in the background
+merge_request:
+author:

spec/models/project_services/bamboo_service_spec.rb

 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 the contents of the reactive cache' do
+      stub_reactive_cache(service, { build_page: 'foo' }, 'sha', 'ref')
+
+      expect(service.build_page('sha', 'ref')).to eq('foo')
+    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 '#build_page' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:build_page] }
+
       it 'returns a specific URL when status is 500' do
         stub_request(status: 500)
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
+        is_expected.to eq('http://gitlab.com/bamboo/browse/foo')
       end
 
       it 'returns a specific URL when response has no results' do
-      stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+        stub_request(body: bamboo_response(size: 0))
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
+        is_expected.to eq('http://gitlab.com/bamboo/browse/foo')
       end
 
       it 'returns a build URL when bamboo_url has no trailing slash' do
-      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+        stub_request(body: bamboo_response)
 
-      expect(service(bamboo_url: 'http://gitlab.com/bamboo').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
+        is_expected.to eq('http://gitlab.com/bamboo/browse/42')
       end
 
-    it 'returns a build URL when bamboo_url has a trailing slash' do
-      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+      context 'bamboo_url has trailing slash' do
+        let(:bamboo_url) { 'http://gitlab.com/bamboo/' }
+
+        it 'returns a build URL' do
+          stub_request(body: bamboo_response)
 
-      expect(service(bamboo_url: 'http://gitlab.com/bamboo/').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
+          is_expected.to eq('http://gitlab.com/bamboo/browse/42')
+        end
       end
     end
 
-  describe '#commit_status' 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)
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
+        is_expected.to eq(:error)
       end
 
       it 'sets commit status to "pending" when status is 404' do
         stub_request(status: 404)
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
+        is_expected.to eq('pending')
       end
 
       it 'sets commit status to "pending" when response has no results' do
         stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
+        is_expected.to eq('pending')
       end
 
       it 'sets commit status to "success" when build state contains Success' do
-      stub_request(build_state: 'YAY Success!')
+        stub_request(body: bamboo_response(build_state: 'YAY Success!'))
 
-      expect(service.commit_status('123', 'unused')).to eq('success')
+        is_expected.to eq('success')
       end
 
       it 'sets commit status to "failed" when build state contains Failed' do
-      stub_request(build_state: 'NO Failed!')
+        stub_request(body: bamboo_response(build_state: 'NO Failed!'))
 
-      expect(service.commit_status('123', 'unused')).to eq('failed')
+        is_expected.to eq('failed')
       end
 
       it 'sets commit status to "pending" when build state contains Pending' do
-      stub_request(build_state: 'NO Pending!')
+        stub_request(body: bamboo_response(build_state: 'NO Pending!'))
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
+        is_expected.to eq('pending')
       end
 
       it 'sets commit status to :error when build state is unknown' do
-      stub_request(build_state: 'FOO BAR!')
+        stub_request(body: bamboo_response(build_state: 'FOO BAR!'))
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
+        is_expected.to eq(:error)
       end
     end
-
-  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'
-      }
-    )
   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

spec/models/project_services/buildkite_service_spec.rb

 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

spec/models/project_services/drone_ci_service_spec.rb

 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

spec/models/project_services/teamcity_service_spec.rb

 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 the contents of the reactive cache' do
+      stub_reactive_cache(service, { build_page: 'foo' }, 'sha', 'ref')
+
+      expect(service.build_page('sha', 'ref')).to eq('foo')
+    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 'build_page' do
+      subject { service.calculate_reactive_cache('123', 'unused')[:build_page] }
+
       it 'returns a specific URL when status is 500' do
         stub_request(status: 500)
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
+        is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
       end
 
       it 'returns a build URL when teamcity_url has no trailing slash' do
         stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com/teamcity').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+        is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
       end
 
-    it 'returns a build URL when teamcity_url has a trailing slash' do
+      context 'teamcity_url has trailing slash' do
+        let(:teamcity_url) { 'http://gitlab.com/teamcity/' }
+
+        it 'returns a build URL' do
           stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com/teamcity/').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+          is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+        end
       end
     end
 
-  describe '#commit_status' 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)
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
+        is_expected.to eq(:error)
       end
 
       it 'sets commit status to "pending" when status is 404' do
         stub_request(status: 404)
 
-      expect(service.commit_status('123', 'unused')).to eq('pending')
+        is_expected.to eq('pending')
       end
 
       it 'sets commit status to "success" when build status contains SUCCESS' do
         stub_request(build_status: 'YAY SUCCESS!')
 
-      expect(service.commit_status('123', 'unused')).to eq('success')
+        is_expected.to eq('success')
       end
 
       it 'sets commit status to "failed" when build status contains FAILURE' do
         stub_request(build_status: 'NO FAILURE!')
 
-      expect(service.commit_status('123', 'unused')).to eq('failed')
+        is_expected.to eq('failed')
       end
 
       it 'sets commit status to "pending" when build status contains Pending' do
         stub_request(build_status: 'NO Pending!')
 
-      expect(service.commit_status('123', 'unused')).to eq('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!')
 
-      expect(service.commit_status('123', 'unused')).to eq(:error)
+        is_expected.to eq(:error)
       end
     end
-
-  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'
-      }
-    )
   end
 
   def stub_request(status: 200, body: nil, build_status: 'success')

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)