**V3**: GET /v3/routes filters by app GUIDs

Typical usage:

   `GET /v3/routes?app_guids=guid1,guid2`

Previously, in order to show the routes mapped to each app, we had to go
through each app and fetch the routes that were mapped to it with `GET
/v3/apps/:guid/routes`.

This commit allows us to `GET /v3/routes?app_guids=1,2,3,...` to get all
relevant routes in a *single* request. This is a little unusual for the
v3 API, since routes and apps are not directly associated—they are
associated "through" destinations. But we've already added a convenience
endpoint in the form of `GET /v3/apps/:guid/routes`.

We removed `RoutesListMessage::for_app_guid`. It's no longer necessary
now that filtering by an app's GUID is available.

Docs: we sorted the query parameters for a better user experience.

[finishes #172625783]

Co-authored-by: Brian Cunnie <bcunnie@pivotal.io>
Co-authored-by: Teal Stannard <tstannard@pivotal.io>

Author: Brian Cunnie

app/controllers/v3/routes_controller.rb

@@ -181,7 +181,7 @@ def destroy_destination
   end
 
   def index_by_app
-    message = RoutesListMessage.from_params(query_params).for_app_guid(hashed_params['guid'])
+    message = RoutesListMessage.from_params(query_params.merge({ app_guids: hashed_params['guid'] }))
     invalid_param!(message.errors.full_messages) unless message.valid?
 
     app, space, org = AppFetcher.new.fetch(hashed_params['guid'])

app/fetchers/route_fetcher.rb

@@ -9,11 +9,6 @@ def fetch(message, readable_route_guids, eager_loaded_associations: [])
       private
 
       def filter(message, dataset)
-        if message.app_guid
-          destinations_route_guids = RouteMappingModel.where(app_guid: message.app_guid).select(:route_guid)
-          dataset = dataset.where(guid: destinations_route_guids)
-        end
-
         if message.requested?(:hosts)
           dataset = dataset.where(host: message.hosts)
         end
@@ -35,6 +30,11 @@ def filter(message, dataset)
           dataset = dataset.where(domain_id: Domain.where(guid: message.domain_guids).select(:id))
         end
 
+        if message.requested?(:app_guids)
+          destinations_route_guids = RouteMappingModel.where(app_guid: message.app_guids).select(:route_guid)
+          dataset = dataset.where(guid: destinations_route_guids)
+        end
+
         if message.requested?(:label_selector)
           dataset = LabelSelectorQueryGenerator.add_selector_queries(
             label_klass: RouteLabelModel,

app/messages/routes_list_message.rb

@@ -8,6 +8,7 @@ class RoutesListMessage < ListMessage
       :organization_guids,
       :domain_guids,
       :paths,
+      :app_guids,
       :include,
       :label_selector,
     ]
@@ -17,19 +18,13 @@ class RoutesListMessage < ListMessage
 
     validates :hosts, allow_nil: true, array: true
     validates :paths, allow_nil: true, array: true
+    validates :app_guids, allow_nil: true, array: true
     validates :space_guids, allow_nil: true, array: true
     validates :organization_guids, allow_nil: true, array: true
     validates :domain_guids, allow_nil: true, array: true
 
-    attr_reader :app_guid
-
     def self.from_params(params)
-      super(params, %w(hosts space_guids organization_guids domain_guids paths include))
-    end
-
-    def for_app_guid(app_guid)
-      @app_guid = app_guid
-      self
+      super(params, %w(hosts space_guids organization_guids domain_guids app_guids paths include))
     end
   end
 end

docs/v3/source/includes/resources/routes/_list.md.erb

@@ -30,11 +30,12 @@ Retrieve all routes the user has access to.
 
 Name | Type | Description
 ---- | ---- | ------------
+**app_guids** | _list of strings_ | Comma-delimited list of app guids to filter by.
+**domain_guids** | _list of strings_ | Comma-delimited list of domain guids to filter by.
 **hosts** | _list of strings_ | Comma-delimited list of hostnames to filter by.
+**organization_guids** | _list of strings_ | Comma-delimited list of organization guids to filter by.
 **paths** | _list of strings_ | Comma-delimited list of paths to filter by (e.g. `/path1,/path2`).
-**domain_guids** | _list of strings_ | Comma-delimited list of domain guids to filter by.
 **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.
 **per_page** | _integer_ | Number of results per page. <br>Valid values are 1 through 5000.
 **order_by** | _string_ | Value to sort by. Defaults to ascending. Prepend with `-` to sort descending. <br>Valid values are `created_at`, `updated_at`.

spec/request/routes_spec.rb

@@ -121,6 +121,7 @@
             space_guids: ['foo', 'bar'],
             organization_guids: ['foo', 'bar'],
             domain_guids: ['foo', 'bar'],
+            app_guids: ['foo', 'bar'],
             paths: ['foo', 'bar'],
             hosts: 'foo',
             include: 'domain',
@@ -464,6 +465,18 @@
           })
         end
       end
+
+      context 'app_guids filter' do
+        it 'returns routes filtered by app_guid' do
+          get "/v3/routes?app_guids=#{app_model.guid}", nil, admin_header
+          expect(last_response.status).to eq(200)
+          expect(parsed_response['resources'].size).to eq(1)
+          expect(parsed_response['resources'].first['destinations'].size).to eq(2)
+          expect(
+            parsed_response['resources'].first['destinations'].map { |destination| destination['app']['guid'] }.uniq
+          ).to eq([app_model.guid])
+        end
+      end
     end
 
     describe 'labels' do

spec/unit/fetchers/route_fetcher_spec.rb

@@ -177,11 +177,24 @@ module VCAP::CloudController
         end
       end
 
+      context 'when fetching routes for several apps' do
+        let(:app_model) { AppModel.make(space: space1) }
+        let(:app_model2) { AppModel.make(space: space1) }
+        let!(:destination1) { RouteMappingModel.make(app: app_model, route: route1, process_type: 'web') }
+        let!(:destination2) { RouteMappingModel.make(app: app_model2, route: route2, process_type: 'worker') }
+        let(:routes_filter) { { app_guids: [app_model.guid, app_model2.guid] } }
+
+        it 'only returns routes that are mapped to the app' do
+          results = RouteFetcher.fetch(message, Route.all.map(&:guid)).all
+          expect(results).to contain_exactly(route1, route2)
+        end
+      end
+
       context 'when fetching routes for an app' do
         let(:app_model) { AppModel.make(space: space1) }
         let!(:destination1) { RouteMappingModel.make(app: app_model, route: route1, process_type: 'web') }
         let!(:destination2) { RouteMappingModel.make(app: app_model, route: route2, process_type: 'worker') }
-        let(:message) { RoutesListMessage.from_params({}).for_app_guid(app_model.guid) }
+        let(:routes_filter) { { app_guids: [app_model.guid] } }
 
         it 'only returns routes that are mapped to the app' do
           results = RouteFetcher.fetch(message, Route.all.map(&:guid)).all

spec/unit/messages/routes_list_message_spec.rb

@@ -16,13 +16,6 @@ module VCAP::CloudController
       end
     end
 
-    describe '#for_app_guid' do
-      it 'sets app_guid on message' do
-        message = RoutesListMessage.from_params({}).for_app_guid('some-app-guid')
-        expect(message.app_guid).to eq('some-app-guid')
-      end
-    end
-
     describe 'fields' do
       it 'accepts an empty set' do
         message = RoutesListMessage.from_params({})
@@ -105,6 +98,14 @@ module VCAP::CloudController
         expect(message.errors[:base].length).to eq 1
         expect(message.errors[:base][0]).to match(/Duplicate included resource: 'domain'/)
       end
+
+      context 'when app guids param is provided' do
+        it 'accepts it' do
+          message = RoutesListMessage.from_params({ 'app_guids' => 'guid1,guid2' })
+          expect(message).to be_valid
+          expect(message.app_guids).to eq(['guid1', 'guid2'])
+        end
+      end
     end
   end
 end