From 151526279454904b33cf4263f12daf38c7e8708c Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Fri, 7 Apr 2017 15:39:24 -0400 Subject: [PATCH 1/2] select only specified attributes fix parameters --- .../api/base_controller/parameters.rb | 6 ++++-- .../api/base_controller/renderer.rb | 19 ++++++++++++++++++- spec/requests/api/categories_spec.rb | 10 ++++++++++ spec/requests/api/service_catalogs_spec.rb | 12 ++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/base_controller/parameters.rb b/app/controllers/api/base_controller/parameters.rb index 1e4e2de4cab..525acb1ef90 100644 --- a/app/controllers/api/base_controller/parameters.rb +++ b/app/controllers/api/base_controller/parameters.rb @@ -44,8 +44,10 @@ def attribute_format(attr) end def attribute_selection - if !@req.attributes.empty? || @additional_attributes - @req.attributes | Array(@additional_attributes) | ID_ATTRS + if @req.attributes.empty? && @additional_attributes + Array(@additional_attributes) | ID_ATTRS + elsif !@req.attributes.empty? + @req.attributes else "all" end diff --git a/app/controllers/api/base_controller/renderer.rb b/app/controllers/api/base_controller/renderer.rb index 3943d51f4a4..7fb860b8c15 100644 --- a/app/controllers/api/base_controller/renderer.rb +++ b/app/controllers/api/base_controller/renderer.rb @@ -192,7 +192,24 @@ def expand_subcollections(json, type, resource) end def expand_subcollection?(sc, target) - respond_to?(target) && (@req.expand?(sc) || collection_config.show?(sc)) + return false unless respond_to?(target) # If there's no query method, no need to go any further + expand_resources?(sc) || expand_action_resource?(sc) || resource_requested?(sc) + end + + # Expand resource if: + # expand='resources' && no attributes selected && collection is configured for subcollection + def expand_resources?(sc) + @req.expand?('resources') && @req.attributes.empty? && collection_config.show?(sc) + end + + # Expand resource if it is not a get but resource should be expanded + def expand_action_resource?(sc) + @req.method != :get && collection_config.show?(sc) + end + + # Expand if explicitly requested + def resource_requested?(sc) + @req.expand?(sc) end # diff --git a/spec/requests/api/categories_spec.rb b/spec/requests/api/categories_spec.rb index e795adea374..c59cbeba7c4 100644 --- a/spec/requests/api/categories_spec.rb +++ b/spec/requests/api/categories_spec.rb @@ -46,6 +46,16 @@ expect(response).to have_http_status(:ok) end + it "will only return the requested attributes" do + FactoryGirl.create(:category) + api_basic_authorize collection_action_identifier(:categories, :read, :get) + + run_get categories_url, :expand => 'resources', :attributes => 'id' + + expect(response).to have_http_status(:ok) + expect_hash_to_have_only_keys(response.parsed_body['resources'].first, %w(href id)) + end + it "can list all the tags under a category" do classification = FactoryGirl.create(:classification_tag) category = FactoryGirl.create(:category, :children => [classification]) diff --git a/spec/requests/api/service_catalogs_spec.rb b/spec/requests/api/service_catalogs_spec.rb index c24bf78dd48..a2696a0e5bc 100644 --- a/spec/requests/api/service_catalogs_spec.rb +++ b/spec/requests/api/service_catalogs_spec.rb @@ -24,6 +24,18 @@ def sc_templates_url(id, st_id = nil) st_id ? "#{st_base}/#{st_id}" : st_base end + describe "Service Catalog Index" do + it "will return only the requested attributes" do + FactoryGirl.create(:service_template_catalog) + api_basic_authorize collection_action_identifier(:service_catalogs, :read, :get) + + run_get service_catalogs_url, :expand => 'resources', :attributes => 'id' + + expect(response).to have_http_status(:ok) + expect_hash_to_have_only_keys(response.parsed_body['resources'].first, %w(href id)) + end + end + describe "Service Catalogs create" do it "rejects resource creation without appropriate role" do api_basic_authorize From 0e9623d75cf84ef9c6cfb84089cb7fef6c90c56e Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Tue, 11 Apr 2017 15:03:51 -0400 Subject: [PATCH 2/2] specs for all collections in question fixes https://bugzilla.redhat.com/show_bug.cgi?id=1437201 --- app/controllers/api/base_controller/parameters.rb | 2 +- app/controllers/api/base_controller/renderer.rb | 8 ++++---- spec/requests/api/automate_spec.rb | 9 +++++++++ spec/requests/api/categories_spec.rb | 6 +++--- spec/requests/api/reports_spec.rb | 10 ++++++++++ spec/requests/api/roles_spec.rb | 9 +++++++++ spec/requests/api/service_catalogs_spec.rb | 4 ++-- 7 files changed, 38 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/base_controller/parameters.rb b/app/controllers/api/base_controller/parameters.rb index 525acb1ef90..278d9f97fb2 100644 --- a/app/controllers/api/base_controller/parameters.rb +++ b/app/controllers/api/base_controller/parameters.rb @@ -47,7 +47,7 @@ def attribute_selection if @req.attributes.empty? && @additional_attributes Array(@additional_attributes) | ID_ATTRS elsif !@req.attributes.empty? - @req.attributes + @req.attributes | ID_ATTRS else "all" end diff --git a/app/controllers/api/base_controller/renderer.rb b/app/controllers/api/base_controller/renderer.rb index 7fb860b8c15..037821e2907 100644 --- a/app/controllers/api/base_controller/renderer.rb +++ b/app/controllers/api/base_controller/renderer.rb @@ -196,18 +196,18 @@ def expand_subcollection?(sc, target) expand_resources?(sc) || expand_action_resource?(sc) || resource_requested?(sc) end - # Expand resource if: - # expand='resources' && no attributes selected && collection is configured for subcollection + # Expand if: expand='resources' && no attributes specified && subcollection is configured def expand_resources?(sc) @req.expand?('resources') && @req.attributes.empty? && collection_config.show?(sc) end - # Expand resource if it is not a get but resource should be expanded + # Expand if: resource is being returned and subcollection is configured + # IE an update to /service_catalogs expects service_templates as part of its resource def expand_action_resource?(sc) @req.method != :get && collection_config.show?(sc) end - # Expand if explicitly requested + # Expand if: explicitly requested def resource_requested?(sc) @req.expand?(sc) end diff --git a/spec/requests/api/automate_spec.rb b/spec/requests/api/automate_spec.rb index 3d980ae2789..12cd9021593 100644 --- a/spec/requests/api/automate_spec.rb +++ b/spec/requests/api/automate_spec.rb @@ -28,6 +28,15 @@ ) end + it 'returns only the requested attributes' do + api_basic_authorize action_identifier(:automate, :read, :collection_actions, :get) + + run_get automate_url, :expand => 'resources', :attributes => 'name' + + expect(response).to have_http_status(:ok) + response.parsed_body['resources'].each { |res| expect_hash_to_have_only_keys(res, %w(fqname name)) } + end + it "default to depth 0 for non-root queries" do api_basic_authorize action_identifier(:automate, :read, :collection_actions, :get) diff --git a/spec/requests/api/categories_spec.rb b/spec/requests/api/categories_spec.rb index c59cbeba7c4..ed01838f7be 100644 --- a/spec/requests/api/categories_spec.rb +++ b/spec/requests/api/categories_spec.rb @@ -47,13 +47,13 @@ end it "will only return the requested attributes" do - FactoryGirl.create(:category) + FactoryGirl.create(:category, :example_text => 'foo') api_basic_authorize collection_action_identifier(:categories, :read, :get) - run_get categories_url, :expand => 'resources', :attributes => 'id' + run_get categories_url, :expand => 'resources', :attributes => 'example_text' expect(response).to have_http_status(:ok) - expect_hash_to_have_only_keys(response.parsed_body['resources'].first, %w(href id)) + response.parsed_body['resources'].each { |res| expect_hash_to_have_only_keys(res, %w(href id example_text)) } end it "can list all the tags under a category" do diff --git a/spec/requests/api/reports_spec.rb b/spec/requests/api/reports_spec.rb index 51ce7533174..f27d6a7271c 100644 --- a/spec/requests/api/reports_spec.rb +++ b/spec/requests/api/reports_spec.rb @@ -17,6 +17,16 @@ expect(response).to have_http_status(:ok) end + it 'returns only the requested attributes' do + FactoryGirl.create(:miq_report_with_results) + api_basic_authorize collection_action_identifier(:reports, :read, :get) + + run_get reports_url, :expand => 'resources', :attributes => 'template_type' + + expect(response).to have_http_status(:ok) + response.parsed_body['resources'].each { |res| expect_hash_to_have_only_keys(res, %w(href id template_type)) } + end + it "can fetch a report" do report = FactoryGirl.create(:miq_report_with_results) diff --git a/spec/requests/api/roles_spec.rb b/spec/requests/api/roles_spec.rb index 309e04be5d4..5b477b63362 100644 --- a/spec/requests/api/roles_spec.rb +++ b/spec/requests/api/roles_spec.rb @@ -80,6 +80,15 @@ def test_features_query(role, role_url, klass, attr = :id) :miq_product_features => @product_features) test_features_query(role, roles_url(role.id), MiqProductFeature, :identifier) end + + it 'returns only the requested attributes' do + api_basic_authorize action_identifier(:roles, :read, :collection_actions, :get) + + run_get roles_url, :expand => 'resources', :attributes => 'name' + + expect(response).to have_http_status(:ok) + response.parsed_body['resources'].each { |res| expect_hash_to_have_only_keys(res, %w(href id name)) } + end end describe "Roles create" do diff --git a/spec/requests/api/service_catalogs_spec.rb b/spec/requests/api/service_catalogs_spec.rb index a2696a0e5bc..15cb5e25e4a 100644 --- a/spec/requests/api/service_catalogs_spec.rb +++ b/spec/requests/api/service_catalogs_spec.rb @@ -29,10 +29,10 @@ def sc_templates_url(id, st_id = nil) FactoryGirl.create(:service_template_catalog) api_basic_authorize collection_action_identifier(:service_catalogs, :read, :get) - run_get service_catalogs_url, :expand => 'resources', :attributes => 'id' + run_get service_catalogs_url, :expand => 'resources', :attributes => 'name' expect(response).to have_http_status(:ok) - expect_hash_to_have_only_keys(response.parsed_body['resources'].first, %w(href id)) + response.parsed_body['resources'].each { |res| expect_hash_to_have_only_keys(res, %w(href id name)) } end end