diff --git a/app/fetchers/droplet_list_fetcher.rb b/app/fetchers/droplet_list_fetcher.rb index 9ff62bbbff6..47265b69450 100644 --- a/app/fetchers/droplet_list_fetcher.rb +++ b/app/fetchers/droplet_list_fetcher.rb @@ -32,14 +32,19 @@ def droplet_dataset(eager_loaded_associations, dataset=DropletModel.dataset) end def filter(message, app, space_guids, dataset) - if message.requested?(:current) && app - dataset = dataset.extension(:null_dataset) - return dataset.nullify unless app.droplet - - dataset = dataset.where(guid: app.droplet_guid) + if message.requested?(:current) + if app + dataset = dataset.extension(:null_dataset) + return dataset.nullify unless app.droplet + + dataset = dataset.where(guid: app.droplet_guid) + else + dataset = dataset.select_all(DropletModel.table_name). + join_table(:inner, AppModel.table_name, { droplet_guid: Sequel[DropletModel.table_name][:guid] }, { table_alias: :apps_current }) + end end - dataset = dataset.where(app_guid: message.app_guids) if message.requested?(:app_guids) + dataset = dataset.where(Sequel[DropletModel.table_name][:app_guid] => message.app_guids) if message.requested?(:app_guids) dataset = dataset.where(state: message.states) if message.requested?(:states) diff --git a/app/messages/droplets_list_message.rb b/app/messages/droplets_list_message.rb index a28da12c316..ad5b2aab9ed 100644 --- a/app/messages/droplets_list_message.rb +++ b/app/messages/droplets_list_message.rb @@ -18,9 +18,9 @@ class DropletsListMessage < MetadataListMessage validates :states, array: true, allow_nil: true validates :space_guids, array: true, allow_nil: true validates :organization_guids, array: true, allow_nil: true - validates :current, inclusion: { in: ['true'], message: 'only accepts the value \'true\'' }, allow_nil: true, if: -> { app_guid.present? } + validates :current, inclusion: { in: ['true'], message: 'only accepts the value \'true\'' }, allow_nil: true validate :app_nested_request, if: -> { app_guid.present? } - validate :not_app_nested_request, unless: -> { app_guid.present? } + validate :package_nested_request, if: -> { package_guid.present? } def to_param_hash super(exclude: %i[app_guid package_guid]) @@ -32,12 +32,6 @@ def self.from_params(params) private - def not_app_nested_request - invalid_attributes = [] - invalid_attributes << :current if current - errors.add(:base, "Unknown query parameter(s): '#{invalid_attributes.join("', '")}'") if invalid_attributes.present? - end - def app_nested_request invalid_attributes = [] invalid_attributes << :app_guids if app_guids @@ -45,5 +39,11 @@ def app_nested_request invalid_attributes << :space_guids if space_guids errors.add(:base, "Unknown query parameter(s): '#{invalid_attributes.join("', '")}'") if invalid_attributes.present? end + + def package_nested_request + invalid_attributes = [] + invalid_attributes << :current if current + errors.add(:base, "Unknown query parameter(s): '#{invalid_attributes.join("', '")}'") if invalid_attributes.present? + end end end diff --git a/docs/openapi/apis/cf/latest/paths/Droplets.yaml b/docs/openapi/apis/cf/latest/paths/Droplets.yaml index 6a9870671af..301f900457c 100644 --- a/docs/openapi/apis/cf/latest/paths/Droplets.yaml +++ b/docs/openapi/apis/cf/latest/paths/Droplets.yaml @@ -43,6 +43,12 @@ type: string description: | Comma-delimited list of app GUIDs to filter by + - name: current + in: query + schema: + type: boolean + description: | + If true, only include the current droplet for each app - name: space_guids in: query schema: diff --git a/docs/v3/source/includes/resources/droplets/_list.md.erb b/docs/v3/source/includes/resources/droplets/_list.md.erb index adccc40ef62..04bb72b0a82 100644 --- a/docs/v3/source/includes/resources/droplets/_list.md.erb +++ b/docs/v3/source/includes/resources/droplets/_list.md.erb @@ -33,6 +33,7 @@ Name | Type | Description **guids** | _list of strings_ | Comma-delimited list of droplet guids to filter by **states** | _list of strings_ | Comma-delimited list of droplet states to filter by **app_guids** | _list of strings_ | Comma-delimited list of app guids to filter by +**current** | _boolean_ | If true, only include the current droplet for each app **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 **page** | _integer_ | Page to display; valid values are integers >= 1 diff --git a/spec/request/droplets_spec.rb b/spec/request/droplets_spec.rb index c143e6f1fdb..c7a7de55de8 100644 --- a/spec/request/droplets_spec.rb +++ b/spec/request/droplets_spec.rb @@ -659,6 +659,7 @@ space_guids app_guids organization_guids + current ] end let(:params) do @@ -668,7 +669,6 @@ order_by: 'updated_at', guids: 'foo,bar', app_guid: app_model.guid, - current: true, package_guid: package_model.guid, states: %w[test foo], label_selector: 'foo,bar', @@ -941,6 +941,50 @@ returned_guids = parsed_response['resources'].pluck('guid') expect(returned_guids).to contain_exactly(droplet1.guid, droplet2.guid, droplet3.guid) end + + it 'filters by current=true as admin' do + current_droplet_app1 = create(:droplet_model, app: app_model, state: VCAP::CloudController::DropletModel::STAGED_STATE) + current_droplet_app2 = create(:droplet_model, app: app_model2, state: VCAP::CloudController::DropletModel::STAGED_STATE) + current_droplet_app3 = create(:droplet_model, app: app_model3, state: VCAP::CloudController::DropletModel::STAGED_STATE) + app_model.update(droplet: current_droplet_app1) + app_model2.update(droplet: current_droplet_app2) + app_model3.update(droplet: current_droplet_app3) + + get '/v3/droplets?current=true', nil, admin_headers + + expect(last_response.status).to eq(200) + returned_guids = parsed_response['resources'].pluck('guid') + expect(returned_guids).to contain_exactly(current_droplet_app1.guid, current_droplet_app2.guid, current_droplet_app3.guid) + end + + it 'filters by current=true as a non-admin developer, returning only current droplets in readable spaces' do + current_droplet_app1 = create(:droplet_model, app: app_model, state: VCAP::CloudController::DropletModel::STAGED_STATE) + current_droplet_app2 = create(:droplet_model, app: app_model2, state: VCAP::CloudController::DropletModel::STAGED_STATE) + current_droplet_app3 = create(:droplet_model, app: app_model3, state: VCAP::CloudController::DropletModel::STAGED_STATE) + app_model.update(droplet: current_droplet_app1) + app_model2.update(droplet: current_droplet_app2) + app_model3.update(droplet: current_droplet_app3) + + get '/v3/droplets?current=true', nil, developer_headers + + expect(last_response.status).to eq(200) + returned_guids = parsed_response['resources'].pluck('guid') + expect(returned_guids).to contain_exactly(current_droplet_app1.guid, current_droplet_app2.guid) + expect(returned_guids).not_to include(current_droplet_app3.guid) + end + + it 'filters by current=true combined with app_guids' do + current_droplet_app1 = create(:droplet_model, app: app_model, state: VCAP::CloudController::DropletModel::STAGED_STATE) + current_droplet_app2 = create(:droplet_model, app: app_model2, state: VCAP::CloudController::DropletModel::STAGED_STATE) + app_model.update(droplet: current_droplet_app1) + app_model2.update(droplet: current_droplet_app2) + + get "/v3/droplets?current=true&app_guids=#{app_model.guid}", nil, developer_headers + + expect(last_response.status).to eq(200) + returned_guids = parsed_response['resources'].pluck('guid') + expect(returned_guids).to contain_exactly(current_droplet_app1.guid) + end end context 'label_selector' do diff --git a/spec/unit/fetchers/droplet_list_fetcher_spec.rb b/spec/unit/fetchers/droplet_list_fetcher_spec.rb index 5b67d3ede25..0ac9855cb80 100644 --- a/spec/unit/fetchers/droplet_list_fetcher_spec.rb +++ b/spec/unit/fetchers/droplet_list_fetcher_spec.rb @@ -125,6 +125,76 @@ module VCAP::CloudController expect(results).to contain_exactly(staged_droplet_for_app1) end end + + context 'filtering by current=true' do + let(:filters) { { current: 'true' } } + + context 'when some apps have a current droplet set' do + before do + app1.update(droplet: staged_droplet_for_app1) + app2.update(droplet: staged_droplet_for_app2) + end + + it 'returns only current droplets' do + results = fetcher.fetch_all(message).all + expect(results).to contain_exactly(staged_droplet_for_app1, staged_droplet_for_app2) + end + + it 'does not return non-current droplets' do + results = fetcher.fetch_all(message).all + expect(results).not_to include(failed_droplet_for_app1) + end + end + + context 'when no apps have a current droplet set' do + it 'returns an empty list' do + results = fetcher.fetch_all(message).all + expect(results).to be_empty + end + end + + context 'when combined with app_guids filter' do + before do + app1.update(droplet: staged_droplet_for_app1) + app2.update(droplet: staged_droplet_for_app2) + end + + let(:filters) { { current: 'true', app_guids: [app1.guid] } } + + it 'returns only the current droplet for the specified app' do + results = fetcher.fetch_all(message).all + expect(results).to contain_exactly(staged_droplet_for_app1) + end + end + + context 'when combined with space_guids filter' do + before do + app1.update(droplet: staged_droplet_for_app1) + app2.update(droplet: staged_droplet_for_app2) + end + + let(:filters) { { current: 'true', space_guids: [app1.space.guid] } } + + it 'returns only current droplets in the specified space' do + results = fetcher.fetch_all(message).all + expect(results).to contain_exactly(staged_droplet_for_app1) + end + end + + context 'when combined with organization_guids filter' do + before do + app1.update(droplet: staged_droplet_for_app1) + app2.update(droplet: staged_droplet_for_app2) + end + + let(:filters) { { current: 'true', organization_guids: [app1.organization.guid] } } + + it 'returns only current droplets in the specified organization' do + results = fetcher.fetch_all(message).all + expect(results).to contain_exactly(staged_droplet_for_app1) + end + end + end end describe '#fetch_for_spaces' do diff --git a/spec/unit/messages/droplets_list_message_spec.rb b/spec/unit/messages/droplets_list_message_spec.rb index 7968f110a3a..1da80ba08fa 100644 --- a/spec/unit/messages/droplets_list_message_spec.rb +++ b/spec/unit/messages/droplets_list_message_spec.rb @@ -125,8 +125,23 @@ module VCAP::CloudController context 'when the query is not nested under an app' do context 'when the request contains current field' do - it 'is invalid' do + it 'is valid' do message = DropletsListMessage.from_params({ current: 'true' }) + expect(message).to be_valid + end + + it 'validates current must be true' do + message = DropletsListMessage.from_params({ current: 'false' }) + expect(message).not_to be_valid + expect(message.errors[:current]).to include("only accepts the value 'true'") + end + end + end + + context 'when the query is nested under a package' do + context 'when the request contains current field' do + it 'is invalid' do + message = DropletsListMessage.from_params({ package_guid: 'some-package-guid', current: 'true' }) expect(message).not_to be_valid expect(message.errors[:base][0]).to include("Unknown query parameter(s): 'current'") end