Allow current=true filter on GET /v3/droplets to return only current droplets across all apps - #5360
Conversation
…droplets across all apps
|
|
||
| private | ||
|
|
||
| def not_app_nested_request |
There was a problem hiding this comment.
Should we convert that method into a package-specific validator rather than removing it?
not_app_nested_request gated all routes where app_guid is absent, that's both the global GET /v3/droplets route and the package-nested GET /v3/packages/:guid/droplets route. Removing it enables current=true on the global route as intended, but also silently enables it on the package route, which flips that endpoint from returning 422 Unknown query parameter to 200 with a join executed. That's outside the PR's stated scope and untested. A package_nested_request validator (firing on package_guid.present?) would still reject current there while allowing it on the global route.
| @@ -125,10 +125,15 @@ module VCAP::CloudController | |||
|
|
|||
| context 'when the query is not nested under an app' do | |||
There was a problem hiding this comment.
Can we add a test asserting current is still rejected on the package-nested route (package_guid present, app_guid absent)?
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
current=true combined with organization_guids or space_guids isn't covered, those add a second join to apps (apps_orgs/apps_spaces alongside the new apps_current). Could we add fetcher tests for those two combinations?
| space_guids | ||
| app_guids | ||
| organization_guids | ||
| current |
There was a problem hiding this comment.
This moves current into the valid-params list, but there's no request-level test that actually calls GET /v3/droplets?current=true and asserts the response returns only current droplets. The filtering is covered at the fetcher unit level, but a request spec for the happy path would cover the full controller→message→fetcher→presenter path. Non-blocking, but worth adding.
A short explanation of the proposed change:
GET /v3/droplets now accepts current=true as a query parameter, returning only droplets that are set as the current droplet of an app. Previously this filter was only available on the app-nested endpoint GET /v3/apps/:guid/droplets?current=true.
An explanation of the use cases your change solves
Clients that need to display stack information for multiple apps (e.g. to warn about deprecated stacks) previously had to make one request per app to fetch the current droplet. With this change, they can fetch current droplets for all apps in a single request by combining with existing filters:
GET /v3/droplets?current=true&app_guids=guid1,guid2,...
This reduces N+1 API requests to 2 requests total.
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests