Improvements after review

- Add a method for returning a service_unavailable response.
- Keep only one spec to test that the prometheus response is returned
as-is.

2 files changed, 21 insertions, 37 deletions

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index d6fe20d377e..bd2f72df64d 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -46,7 +46,7 @@ module Prometheus
     end
 
     def execute
-      return cannot_proxy_response unless can_proxy?(@method, @path)
+      return cannot_proxy_response unless can_proxy?
       return no_prometheus_response unless can_query?
 
       with_reactive_cache(*cache_key) do |result|
@@ -55,15 +55,15 @@ module Prometheus
     end
 
     def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
-      return cannot_proxy_response unless can_proxy?(method, path)
+      return cannot_proxy_response unless can_proxy?
       return no_prometheus_response unless can_query?
 
       response = prometheus_client_wrapper.proxy(path, params)
 
-      success({ http_status: response.code, body: response.body })
+      success(http_status: response.code, body: response.body)
 
     rescue Gitlab::PrometheusClient::Error => err
-      error(err.message, :service_unavailable)
+      service_unavailable_response(err)
     end
 
     def cache_key
@@ -72,6 +72,10 @@ module Prometheus
 
     private
 
+    def service_unavailable_response(exception)
+      error(exception.message, :service_unavailable)
+    end
+
     def no_prometheus_response
       error('No prometheus server found', :service_unavailable)
     end
@@ -92,8 +96,8 @@ module Prometheus
       prometheus_adapter&.can_query?
     end
 
-    def can_proxy?(method, path)
-      PROXY_SUPPORT[path] == method
+    def can_proxy?
+      PROXY_SUPPORT[@path] == @method
     end
   end
 end
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 53e852aa79c..391a108c862 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -128,6 +128,11 @@ describe Prometheus::ProxyService do
 
       context 'Connection to prometheus server succeeds' do
         let(:rest_client_response) { instance_double(RestClient::Response) }
+        let(:prometheus_http_status_code) { 400 }
+
+        let(:response_body) do
+          '{"status":"error","errorType":"bad_data","error":"parse error at char 1: no expression found in input"}'
+        end
 
         before do
           allow(prometheus_client).to receive(:proxy).and_return(rest_client_response)
@@ -137,37 +142,12 @@ describe Prometheus::ProxyService do
           allow(rest_client_response).to receive(:body).and_return(response_body)
         end
 
-        shared_examples 'return prometheus http status code and body' do
-          it do
-            expect(subject.execute).to eq({
-              http_status: prometheus_http_status_code,
-              body: response_body,
-              status: :success
-            })
-          end
-        end
-
-        context 'prometheus returns success' do
-          let(:prometheus_http_status_code) { 200 }
-
-          let(:response_body) do
-            '{"status":"success","data":{"resultType":"scalar","result":[1553864609.117,"1"]}}'
-          end
-
-          before do
-          end
-
-          it_behaves_like 'return prometheus http status code and body'
-        end
-
-        context 'prometheus returns error' do
-          let(:prometheus_http_status_code) { 400 }
-
-          let(:response_body) do
-            '{"status":"error","errorType":"bad_data","error":"parse error at char 1: no expression found in input"}'
-          end
-
-          it_behaves_like 'return prometheus http status code and body'
+        it 'returns the http status code and body from prometheus' do
+          expect(subject.execute).to eq(
+            http_status: prometheus_http_status_code,
+            body: response_body,
+            status: :success
+          )
         end
       end