summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrpereira2 <rpereira@gitlab.com>2019-04-03 18:24:52 +0530
committerPeter Leitzen <pleitzen@gitlab.com>2019-04-04 22:22:46 +0200
commit833206abf833b0c43e35d97b3f2fea98d91d5572 (patch)
treee5cd1ce331bb00ec3829e0cbb4ce0f44381b695c
parentb3aba1203d7f6a0c96c836593f19165bbf066cd0 (diff)
downloadgitlab-ce-833206abf833b0c43e35d97b3f2fea98d91d5572.tar.gz
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.
-rw-r--r--app/services/prometheus/proxy_service.rb14
-rw-r--r--spec/services/prometheus/proxy_service_spec.rb14
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