From 833206abf833b0c43e35d97b3f2fea98d91d5572 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 18:24:52 +0530
Subject: Use hash methods to filter params

- This is so that we don't have to check that params is of type
ActionController::Parameters in ProxyService.
---
 app/services/prometheus/proxy_service.rb       | 14 +++++---------
 spec/services/prometheus/proxy_service_spec.rb | 14 +++++++-------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index c08adfda851..cadfe59b900 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -15,11 +15,11 @@ module Prometheus
     PROXY_SUPPORT = {
       'query' => {
         method: ['GET'],
-        params: [:query, :time, :timeout]
+        params: %w(query time timeout)
       },
       'query_range' => {
         method: ['GET'],
-        params: [:query, :start, :end, :step, :timeout]
+        params: %w(query start end step timeout)
       }
     }.freeze
 
@@ -44,7 +44,7 @@ module Prometheus
 
       # Convert ActionController::Parameters to hash because reactive_cache_worker
       # does not play nice with ActionController::Parameters.
-      @params = permit_params(params, path).to_hash
+      @params = filter_params(params, path).to_hash
 
       @method = method
     end
@@ -102,12 +102,8 @@ module Prometheus
       prometheus_adapter&.can_query?
     end
 
-    def permit_params(params, path)
-      if params.is_a?(ActionController::Parameters)
-        params.permit(PROXY_SUPPORT.dig(path, :params))
-      else
-        params
-      end
+    def filter_params(params, path)
+      params.slice(*PROXY_SUPPORT.dig(path, :params))
     end
 
     def can_proxy?
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 8de5f491427..3e2eea12d95 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -9,15 +9,15 @@ describe Prometheus::ProxyService do
   set(:environment) { create(:environment, project: project) }
 
   describe '#initialize' do
-    let(:params) { ActionController::Parameters.new(query: '1') }
+    let(:params) { ActionController::Parameters.new(query: '1').permit! }
 
     it 'initializes attributes' do
-      result = described_class.new(environment, 'GET', 'query', { query: '1' })
+      result = described_class.new(environment, 'GET', 'query', params)
 
       expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq(query: '1')
+      expect(result.params).to eq('query' => '1')
     end
 
     it 'converts ActionController::Parameters into hash' do
@@ -27,7 +27,7 @@ describe Prometheus::ProxyService do
     end
 
     context 'with unknown params' do
-      let(:params) { ActionController::Parameters.new(query: '1', other_param: 'val') }
+      let(:params) { ActionController::Parameters.new(query: '1', other_param: 'val').permit! }
 
       it 'filters unknown params' do
         result = described_class.new(environment, 'GET', 'query', params)
@@ -39,7 +39,7 @@ describe Prometheus::ProxyService do
 
   describe '#execute' do
     let(:prometheus_adapter) { instance_double(PrometheusService) }
-    let(:params) { ActionController::Parameters.new(query: '1') }
+    let(:params) { ActionController::Parameters.new(query: '1').permit! }
 
     subject { described_class.new(environment, 'GET', 'query', params) }
 
@@ -182,14 +182,14 @@ describe Prometheus::ProxyService do
   describe '.from_cache' do
     it 'initializes an instance of ProxyService class' do
       result = described_class.from_cache(
-        environment.class.name, environment.id, 'GET', 'query', { query: '1' }
+        environment.class.name, environment.id, 'GET', 'query', { 'query' => '1' }
       )
 
       expect(result).to be_an_instance_of(described_class)
       expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq(query: '1')
+      expect(result.params).to eq('query' => '1')
     end
   end
 end
-- 
cgit v1.2.1