From d6d3f5c402141b687fcf061be55705718734f318 Mon Sep 17 00:00:00 2001 From: Derik Evangelista Date: Tue, 4 Aug 2020 11:22:15 +0100 Subject: [PATCH] v3(services): SI can be filtered by Org GUIDs [finishes #171815586](https://www.pivotaltracker.com/story/show/171815586) --- .../v3/service_instances_controller.rb | 30 +++++++++---------- app/fetchers/service_instance_list_fetcher.rb | 15 ++++++++-- .../service_instances_list_message.rb | 1 + .../resources/service_instances/_list.md.erb | 1 + spec/request/service_instances_spec.rb | 10 +++++++ .../service_instance_list_fetcher_spec.rb | 9 ++++++ .../service_instances_list_message_spec.rb | 1 + 7 files changed, 50 insertions(+), 17 deletions(-) diff --git a/app/controllers/v3/service_instances_controller.rb b/app/controllers/v3/service_instances_controller.rb index c201245bcf4..13efb441c95 100644 --- a/app/controllers/v3/service_instances_controller.rb +++ b/app/controllers/v3/service_instances_controller.rb @@ -30,21 +30,6 @@ class ServiceInstancesV3Controller < ApplicationController include ServicePermissions - def show - service_instance = ServiceInstance.first(guid: hashed_params[:guid]) - service_instance_not_found! unless service_instance && can_read_service_instance?(service_instance) - - message = ServiceInstanceShowMessage.from_params(query_params) - invalid_param!(message.errors.full_messages) unless message.valid? - - presenter = Presenters::V3::ServiceInstancePresenter.new( - service_instance, - decorators: decorators_for_fields(message.fields) - ) - - render status: :ok, json: presenter.to_json - end - def index message = ServiceInstancesListMessage.from_params(query_params) invalid_param!(message.errors.full_messages) unless message.valid? @@ -64,6 +49,21 @@ def index ) end + def show + service_instance = ServiceInstance.first(guid: hashed_params[:guid]) + service_instance_not_found! unless service_instance && can_read_service_instance?(service_instance) + + message = ServiceInstanceShowMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + + presenter = Presenters::V3::ServiceInstancePresenter.new( + service_instance, + decorators: decorators_for_fields(message.fields) + ) + + render status: :ok, json: presenter.to_json + end + def create FeatureFlag.raise_unless_enabled!(:service_instance_creation) unless admin? diff --git a/app/fetchers/service_instance_list_fetcher.rb b/app/fetchers/service_instance_list_fetcher.rb index b8da9cf8d3a..5f23b7b677b 100644 --- a/app/fetchers/service_instance_list_fetcher.rb +++ b/app/fetchers/service_instance_list_fetcher.rb @@ -5,10 +5,10 @@ def fetch(message, omniscient: false, readable_space_guids: []) join(:spaces, id: Sequel[:service_instances][:space_id]). left_join(:service_instance_shares, service_instance_guid: Sequel[:service_instances][:guid]) - if !omniscient + unless omniscient dataset = dataset.where do (Sequel[:spaces][:guid] =~ readable_space_guids) | - (Sequel[:service_instance_shares][:target_space_guid] =~ readable_space_guids) + (Sequel[:service_instance_shares][:target_space_guid] =~ readable_space_guids) end end @@ -37,6 +37,17 @@ def filter(dataset, message) end end + if message.requested?(:organization_guids) + spaces_in_orgs = Space.dataset.select(:spaces__guid). + join(:organizations, id: Sequel[:spaces][:organization_id]). + where(Sequel[:organizations][:guid] =~ message.organization_guids) + + dataset = dataset.where do + (Sequel[:spaces][:guid] =~ spaces_in_orgs) | + (Sequel[:service_instance_shares][:target_space_guid] =~ spaces_in_orgs) + end + end + if message.requested?(:space_guids) dataset = dataset.where do (Sequel[:spaces][:guid] =~ message.space_guids) | diff --git a/app/messages/service_instances_list_message.rb b/app/messages/service_instances_list_message.rb index f3a83d1a76f..62b54f6f675 100644 --- a/app/messages/service_instances_list_message.rb +++ b/app/messages/service_instances_list_message.rb @@ -5,6 +5,7 @@ class ServiceInstancesListMessage < MetadataListMessage @array_keys = [ :names, :space_guids, + :organization_guids, :service_plan_guids, :service_plan_names, ] diff --git a/docs/v3/source/includes/resources/service_instances/_list.md.erb b/docs/v3/source/includes/resources/service_instances/_list.md.erb index b0698e4b2dd..fd4a664a1ab 100644 --- a/docs/v3/source/includes/resources/service_instances/_list.md.erb +++ b/docs/v3/source/includes/resources/service_instances/_list.md.erb @@ -34,6 +34,7 @@ Name | Type | Description **names** | _list of strings_ | Comma-delimited list of service instance names to filter by **type** | _string_ | Filter by type; valid values are `managed` and `user-provided` **space_guids** | _list of strings_ | Comma-delimited list of space guids to filter by +**organization_guids** | _list of strings_ | Comma-delimited list of organization guids to filter by **service_plan_guids** | _list of strings_ | Comma-delimited list of service plan guids to filter by **service_plan_names** | _list of strings_ | Comma-delimited list of service plan names to filter by **page** | _integer_ | Page to display; valid values are integers >= 1 diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb index 86729b633ae..206c61d3c70 100644 --- a/spec/request/service_instances_spec.rb +++ b/spec/request/service_instances_spec.rb @@ -182,6 +182,7 @@ { names: ['foo', 'bar'], space_guids: ['foo', 'bar'], + organization_guids: ['org-1', 'org-2'], per_page: '10', page: 2, order_by: 'updated_at', @@ -265,6 +266,15 @@ ) end + it 'filters by organization guids' do + get "/v3/service_instances?organization_guids=#{another_space.organization.guid}", nil, admin_headers + check_filtered_instances( + create_managed_json(msi_2), + create_user_provided_json(upsi_2), + create_managed_json(ssi), + ) + end + it 'filters by label' do VCAP::CloudController::ServiceInstanceLabelModel.make(key_name: 'fruit', value: 'strawberry', service_instance: msi_1) VCAP::CloudController::ServiceInstanceLabelModel.make(key_name: 'fruit', value: 'raspberry', service_instance: msi_2) diff --git a/spec/unit/fetchers/service_instance_list_fetcher_spec.rb b/spec/unit/fetchers/service_instance_list_fetcher_spec.rb index 02d003bf9d7..7da4c3ffef3 100644 --- a/spec/unit/fetchers/service_instance_list_fetcher_spec.rb +++ b/spec/unit/fetchers/service_instance_list_fetcher_spec.rb @@ -59,6 +59,15 @@ module VCAP::CloudController end end + context 'by organization_guids' do + let(:filters) { { organization_guids: [space_1.organization.guid, space_2.organization.guid, 'no-such-org-guid'] } } + + it 'returns instances with matching org guids' do + expect(fetcher.fetch(message, omniscient: true).map(&:guid)).to contain_exactly(*[msi_1, upsi, msi_2, ssi].map(&:guid)) + expect(fetcher.fetch(message, readable_space_guids: [space_1.guid, space_3.guid])).to contain_exactly(msi_1, ssi, upsi) + end + end + context 'by label selector' do let(:filters) { { 'label_selector' => 'key=value' } } before do diff --git a/spec/unit/messages/service_instances_list_message_spec.rb b/spec/unit/messages/service_instances_list_message_spec.rb index 22103eae583..18c7650e222 100644 --- a/spec/unit/messages/service_instances_list_message_spec.rb +++ b/spec/unit/messages/service_instances_list_message_spec.rb @@ -11,6 +11,7 @@ module VCAP::CloudController 'order_by' => 'name', 'names' => 'rabbitmq, redis,mysql', 'space_guids' => 'space-1, space-2, space-3', + 'organization_guids' => 'organization-1, organization-2', 'label_selector' => 'key=value', 'type' => 'managed', 'service_plan_names' => 'plan1, plan2',