From 0e6fdf064671fe10958721029cfad2bba0e1a02b Mon Sep 17 00:00:00 2001
From: Alex Lossent <alexandre.lossent@cern.ch>
Date: Thu, 24 Jan 2019 17:37:42 +0100
Subject: [PATCH] Support polymorphism of Bamboo REST API results

It may return single result or an array of results
---
 app/models/project_services/bamboo_service.rb | 32 +++++++++++++------
 .../24680-support-bamboo-api-polymorphism.yml |  5 +++
 .../project_services/bamboo_service_spec.rb   | 26 ++++++++++++---
 3 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 changelogs/unreleased/24680-support-bamboo-api-polymorphism.yml

diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index a252052200a..71f5607dbdb 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -80,19 +80,27 @@ class BambooService < CiService
 
   private
 
-  def get_build_result_index
-    # When Bamboo returns multiple results for a given changeset, arbitrarily assume the most relevant result to be the last one.
-    -1
+  def get_build_result(response)
+    return if response.code != 200
+
+    # May be nil if no result, a single result hash, or an array if multiple results for a given changeset.
+    result = response.dig('results', 'results', 'result')
+
+    # In case of multiple results, arbitrarily assume the last one is the most relevant.
+    return result.last if result.is_a?(Array)
+
+    result
   end
 
   def read_build_page(response)
+    result = get_build_result(response)
     key =
-      if response.code != 200 || response.dig('results', 'results', 'size') == '0'
+      if result.blank?
         # If actual build link can't be determined, send user to build summary page.
         build_key
       else
         # If actual build link is available, go to build result page.
-        response.dig('results', 'results', 'result', get_build_result_index, 'planResultKey', 'key')
+        result.dig('planResultKey', 'key')
       end
 
     build_url("browse/#{key}")
@@ -101,11 +109,15 @@ class BambooService < CiService
   def read_commit_status(response)
     return :error unless response.code == 200 || response.code == 404
 
-    status = if response.code == 404 || response.dig('results', 'results', 'size') == '0'
-               'Pending'
-             else
-               response.dig('results', 'results', 'result', get_build_result_index, 'buildState')
-             end
+    result = get_build_result(response)
+    status =
+      if result.blank?
+        'Pending'
+      else
+        result.dig('buildState')
+      end
+
+    return :error unless status.present?
 
     if status.include?('Success')
       'success'
diff --git a/changelogs/unreleased/24680-support-bamboo-api-polymorphism.yml b/changelogs/unreleased/24680-support-bamboo-api-polymorphism.yml
new file mode 100644
index 00000000000..5117195cd0c
--- /dev/null
+++ b/changelogs/unreleased/24680-support-bamboo-api-polymorphism.yml
@@ -0,0 +1,5 @@
+---
+title: "Support bamboo api polymorphism"
+merge_request: 24680
+author: Alex Lossent
+type: fixed
\ No newline at end of file
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index ee84fa95f0e..b880d90d28f 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -144,7 +144,7 @@ describe BambooService, :use_clean_rails_memory_store_caching do
     end
   end
 
-  describe '#calculate_reactive_cache' do
+  shared_examples 'reactive cache calculation' do
     context '#build_page' do
       subject { service.calculate_reactive_cache('123', 'unused')[:build_page] }
 
@@ -155,7 +155,7 @@ describe BambooService, :use_clean_rails_memory_store_caching do
       end
 
       it 'returns a specific URL when response has no results' do
-        stub_request(body: bamboo_response(size: 0))
+        stub_request(body: %q({"results":{"results":{"size":"0"}}}))
 
         is_expected.to eq('http://gitlab.com/bamboo/browse/foo')
       end
@@ -224,6 +224,24 @@ describe BambooService, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#calculate_reactive_cache' do
+    context 'when Bamboo API returns single result' do
+      let(:bamboo_response_template) do
+        %q({"results":{"results":{"size":"1","result":{"buildState":"%{build_state}","planResultKey":{"key":"42"}}}}})
+      end
+
+      it_behaves_like 'reactive cache calculation'
+    end
+
+    context 'when Bamboo API returns an array of results and we only consider the last one' do
+      let(:bamboo_response_template) do
+        %q({"results":{"results":{"size":"2","result":[{"buildState":"%{build_state}","planResultKey":{"key":"41"}},{"buildState":"%{build_state}","planResultKey":{"key":"42"}}]}}})
+      end
+
+      it_behaves_like 'reactive cache calculation'
+    end
+  end
+
   def stub_update_and_build_request(status: 200, body: nil)
     bamboo_full_url = 'http://gitlab.com/bamboo/updateAndBuild.action?buildKey=foo&os_authType=basic'
 
@@ -244,8 +262,8 @@ describe BambooService, :use_clean_rails_memory_store_caching do
     ).with(basic_auth: %w(mic password))
   end
 
-  def bamboo_response(result_key: 42, build_state: 'success', size: 1)
+  def bamboo_response(build_state: 'success')
     # reference: https://docs.atlassian.com/atlassian-bamboo/REST/6.2.5/#d2e786
-    %Q({"results":{"results":{"size":"#{size}","result":[{"buildState":"#{build_state}","planResultKey":{"key":"#{result_key}"}}]}}})
+    bamboo_response_template % { build_state: build_state }
   end
 end
-- 
GitLab