Skip to content

Commit

Permalink
Do not require resource_type for metric_rollups subcollection
Browse files Browse the repository at this point in the history
resource_type is set by the controller when the request is made based off of the collection that has been specified. This returns a more appropriate error message

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1540254
  • Loading branch information
Jillian Tullo committed Jan 31, 2018
1 parent f5f0c38 commit 28046ba
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 7 deletions.
8 changes: 4 additions & 4 deletions lib/services/api/metric_rollups_service.rb
@@ -1,11 +1,12 @@
module Api
class MetricRollupsService
REQUIRED_PARAMS = %w(resource_type capture_interval start_date).freeze
QUERY_PARAMS = %w(resource_ids).freeze

attr_reader :params

def initialize(params)
@params = params
@params = params.slice(*(REQUIRED_PARAMS + QUERY_PARAMS))
validate_required_params
validate_capture_interval
end
Expand All @@ -20,9 +21,8 @@ def query_metric_rollups
private

def validate_required_params
REQUIRED_PARAMS.each do |key|
raise BadRequestError, "Must specify #{REQUIRED_PARAMS.join(', ')}" unless params[key.to_sym]
end
not_specified = REQUIRED_PARAMS - params.keys
raise BadRequestError, "Must specify #{not_specified.join(', ')}" unless not_specified.empty?
end

def validate_capture_interval
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/services/api/metric_rollups_service_spec.rb
Expand Up @@ -8,9 +8,9 @@

it 'validates the capture interval' do
expect do
described_class.new(:resource_type => 'Service',
:start_date => Time.zone.today.to_s,
:capture_interval => 'bad_interval')
described_class.new('resource_type' => 'Service',
'start_date' => Time.zone.today.to_s,
'capture_interval' => 'bad_interval')
end.to raise_error(Api::BadRequestError, a_string_including('Capture interval must be one of '))
end
end
Expand Down
14 changes: 14 additions & 0 deletions spec/requests/services_spec.rb
Expand Up @@ -1055,6 +1055,20 @@ def expect_svc_with_vms

expect(response).to have_http_status(:forbidden)
end

it 'does not require resource_type for a subcollection' do
api_basic_authorize(subcollection_action_identifier(:services, :metric_rollups, :read, :get))

get(url)

expected = {
'error' => a_hash_including(
'message' => 'Must specify capture_interval, start_date'
)
}
expect(response).to have_http_status(:bad_request)
expect(response.parsed_body).to include(expected)
end
end

describe 'add_provider_vms_resource' do
Expand Down

0 comments on commit 28046ba

Please sign in to comment.