add space,space.organization includes to /v3/routes

tcdowney · cwlbraa · cwlbraa · commit c67d0245715e · 2020-09-21T13:38:47.000-07:00
Client can now include spaces and organizations inline on GET /v3/routes and GET /v3/routes/:guid endpoints [#174502491](https://www.pivotaltracker.com/story/show/174502491) Co-authored-by: Tim Downey <tdowney@vmware.com> Co-authored-by: Connor Braa <cbraa@pivotal.io>
diff --git a/app/controllers/v3/apps_controller.rb b/app/controllers/v3/apps_controller.rb
@@ -10,8 +10,8 @@
 require 'actions/app_start'
 require 'actions/app_stop'
 require 'actions/app_assign_droplet'
-require 'decorators/include_app_space_decorator'
-require 'decorators/include_app_organization_decorator'
+require 'decorators/include_space_decorator'
+require 'decorators/include_organization_decorator'
 require 'decorators/include_space_organization_decorator'
 require 'messages/apps_list_message'
 require 'messages/app_show_message'
@@ -46,8 +46,8 @@ def index
               end
 
     decorators = []
-    decorators << IncludeAppSpaceDecorator if IncludeAppSpaceDecorator.match?(message.include)
-    decorators << IncludeAppOrganizationDecorator if IncludeAppOrganizationDecorator.match?(message.include)
+    decorators << IncludeSpaceDecorator if IncludeSpaceDecorator.match?(message.include)
+    decorators << IncludeOrganizationDecorator if IncludeOrganizationDecorator.match?(message.include)
 
     render status: :ok,
            json: Presenters::V3::PaginatedListPresenter.new(
@@ -69,8 +69,8 @@ def show
     app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
 
     decorators = []
-    decorators << IncludeAppSpaceDecorator if IncludeAppSpaceDecorator.match?(message.include)
-    decorators << IncludeAppOrganizationDecorator if IncludeAppOrganizationDecorator.match?(message.include)
+    decorators << IncludeSpaceDecorator if IncludeSpaceDecorator.match?(message.include)
+    decorators << IncludeOrganizationDecorator if IncludeOrganizationDecorator.match?(message.include)
 
     render status: :ok, json: Presenters::V3::AppPresenter.new(
       app,
diff --git a/app/controllers/v3/routes_controller.rb b/app/controllers/v3/routes_controller.rb
@@ -29,6 +29,8 @@ def index
 
     decorators = []
     decorators << IncludeRouteDomainDecorator if IncludeRouteDomainDecorator.match?(message.include)
+    decorators << IncludeSpaceDecorator if IncludeSpaceDecorator.match?(message.include)
+    decorators << IncludeOrganizationDecorator if IncludeOrganizationDecorator.match?(message.include)
 
     render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
       presenter: Presenters::V3::RoutePresenter,
@@ -48,6 +50,8 @@ def show
 
     decorators = []
     decorators << IncludeRouteDomainDecorator if IncludeRouteDomainDecorator.match?(message.include)
+    decorators << IncludeSpaceDecorator if IncludeSpaceDecorator.match?(message.include)
+    decorators << IncludeOrganizationDecorator if IncludeOrganizationDecorator.match?(message.include)
 
     render status: :ok, json: Presenters::V3::RoutePresenter.new(
       route,
diff --git a/app/decorators/include_organization_decorator.rb b/app/decorators/include_organization_decorator.rb
@@ -1,13 +1,13 @@
 module VCAP::CloudController
-  class IncludeAppOrganizationDecorator
+  class IncludeOrganizationDecorator
     class << self
       def match?(include)
         include&.any? { |i| %w(org space.organization).include?(i) }
       end
 
-      def decorate(hash, apps)
+      def decorate(hash, resources)
         hash[:included] ||= {}
-        organization_guids = apps.map(&:organization_guid).uniq
+        organization_guids = resources.map(&:organization_guid).uniq
         organizations = Organization.where(guid: organization_guids).order(:created_at)
 
         hash[:included][:organizations] = organizations.map { |organization| Presenters::V3::OrganizationPresenter.new(organization).to_hash }
diff --git a/app/decorators/include_role_organization_decorator.rb b/app/decorators/include_role_organization_decorator.rb
@@ -2,6 +2,8 @@ module VCAP::CloudController
   class IncludeRoleOrganizationDecorator
     class << self
       def match?(include_params)
+        # roles may be associated with an org without being associated with a space
+        # this is why this is not `space.organization`
         include_params&.include?('organization')
       end
 
diff --git a/app/decorators/include_space_decorator.rb b/app/decorators/include_space_decorator.rb
@@ -1,13 +1,13 @@
 module VCAP::CloudController
-  class IncludeAppSpaceDecorator
+  class IncludeSpaceDecorator
     class << self
       def match?(include)
         include&.any? { |i| %w(space space.organization).include?(i) }
       end
 
-      def decorate(hash, apps)
+      def decorate(hash, resources)
         hash[:included] ||= {}
-        space_guids = apps.map(&:space_guid).uniq
+        space_guids = resources.map(&:space_guid).uniq
         spaces = Space.where(guid: space_guids)
 
         hash[:included][:spaces] = spaces.map { |space| Presenters::V3::SpacePresenter.new(space).to_hash }
diff --git a/app/messages/route_show_message.rb b/app/messages/route_show_message.rb
@@ -5,7 +5,7 @@ class RouteShowMessage < BaseMessage
     register_allowed_keys [:guid, :include]
 
     validates_with NoAdditionalParamsValidator
-    validates_with IncludeParamValidator, valid_values: ['domain']
+    validates_with IncludeParamValidator, valid_values: ['domain', 'space', 'space.organization']
 
     validates :guid, presence: true, string: true
 
diff --git a/app/messages/routes_list_message.rb b/app/messages/routes_list_message.rb
@@ -15,7 +15,7 @@ class RoutesListMessage < ListMessage
     ]
 
     validates_with NoAdditionalParamsValidator
-    validates_with IncludeParamValidator, valid_values: ['domain']
+    validates_with IncludeParamValidator, valid_values: ['domain', 'space', 'space.organization']
 
     validates :hosts, allow_nil: true, array: true
     validates :paths, allow_nil: true, array: true
diff --git a/docs/v3/source/includes/concepts/_includes.md.erb b/docs/v3/source/includes/concepts/_includes.md.erb
@@ -16,8 +16,8 @@ Resource | Allowed values
 **apps/[:guid]** | `space.organization`, `space`
 **roles** | `user`, `space`, `organization`
 **roles/[:guid]** | `user`, `space`, `organization`
-**routes** | `domain`
-**routes/[:guid]** | `domain`
+**routes** | `domain`, `space.organization`, `space`
+**routes/[:guid]** | `domain`, `space.organization`, `space`
 **service_plans** | `space.organization`, `service_offering`
 **service_plans/[:guid]** | `space.organization`, `service_offering`
 **spaces** | `organization`
diff --git a/docs/v3/source/includes/resources/routes/_get.md.erb b/docs/v3/source/includes/resources/routes/_get.md.erb
@@ -28,7 +28,7 @@ Content-Type: application/json
 
 Name | Type | Description
 ---- | ---- | ------------
-**include** (*experimental*)| _string_ | Optionally include additional related resources in the response <br>Valid value is `domain`
+**include** (*experimental*)| _string_ | Optionally include additional related resources in the response <br>Valid values are `domain`, `space.organization`, `space`
 
 #### Permitted roles
 
diff --git a/docs/v3/source/includes/resources/routes/_list.md.erb b/docs/v3/source/includes/resources/routes/_list.md.erb
@@ -41,7 +41,7 @@ Name | Type | Description
 **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`
 **label_selector** (*experimental*)| _string_ | A query string containing a list of [label selector](#labels-and-selectors) requirements
-**include** (*experimental*)| _string_ | Optionally include a list of unique related resources in the response <br>Valid value is `domain`
+**include** (*experimental*)| _string_ | Optionally include a list of unique related resources in the response <br>Valid values are `domain`, `space.organization`, `space`
 
 #### Permitted roles
  |
diff --git a/spec/request/routes_spec.rb b/spec/request/routes_spec.rb
@@ -1,18 +1,38 @@
 require 'spec_helper'
 require 'request_spec_shared_examples'
+require 'presenters/v3/space_presenter'
+require 'presenters/v3/organization_presenter'
 
 RSpec.describe 'Routes Request' do
   let(:user) { VCAP::CloudController::User.make }
   let(:admin_header) { admin_headers_for(user) }
-  let(:space) { VCAP::CloudController::Space.make }
-  let(:org) { space.organization }
+  let!(:space) { VCAP::CloudController::Space.make(name: 'a-space') }
+  let!(:org) { space.organization }
+
+  let(:space_json_generator) do
+    lambda { |s|
+      presented_space = VCAP::CloudController::Presenters::V3::SpacePresenter.new(s).to_hash
+      presented_space[:created_at] = iso8601
+      presented_space[:updated_at] = iso8601
+      presented_space
+    }
+  end
+
+  let(:org_json_generator) do
+    lambda { |o|
+      presented_space = VCAP::CloudController::Presenters::V3::OrganizationPresenter.new(o).to_hash
+      presented_space[:created_at] = iso8601
+      presented_space[:updated_at] = iso8601
+      presented_space
+    }
+  end
 
   before do
     TestConfig.override(kubernetes: {})
   end
 
   describe 'GET /v3/routes' do
-    let(:other_space) { VCAP::CloudController::Space.make }
+    let(:other_space) { VCAP::CloudController::Space.make(name: 'b-space') }
     let(:app_model) { VCAP::CloudController::AppModel.make(space: space) }
     let(:domain) { VCAP::CloudController::PrivateDomain.make(owning_organization: space.organization) }
     let!(:route_in_org) do
@@ -312,6 +332,29 @@
           })
         end
       end
+
+      context 'when including spaces and orgs' do
+        it 'includes the unique spaces and organizations for the routes' do
+          get '/v3/routes?include=space,space.organization', nil, admin_header
+          expect(last_response).to have_status_code(200)
+          expect({
+            resources: parsed_response['resources'],
+            included: parsed_response['included']
+          }).to match_json_response({
+            resources: [route_in_org_json, route_in_other_org_json],
+            included: {
+              'spaces' => [
+                space_json_generator.call(space),
+                space_json_generator.call(other_space)
+              ],
+              'organizations' => [
+                org_json_generator.call(org),
+                org_json_generator.call(other_space.organization)
+              ]
+            }
+          })
+        end
+      end
     end
 
     describe 'filters' do
@@ -986,6 +1029,20 @@
           expect(parsed_response).to match_json_response(route_json)
         end
       end
+
+      context 'when including spaces and orgs' do
+        it 'includes the unique spaces and organizations for the routes' do
+          get "/v3/routes/#{route.guid}?include=space,space.organization", nil, admin_header
+          expect(last_response).to have_status_code(200)
+          expect(parsed_response['included']).to match_json_response(
+            'spaces' => [
+              space_json_generator.call(space),
+            ],
+            'organizations' => [
+              org_json_generator.call(org),
+            ])
+        end
+      end
     end
   end
 
diff --git a/spec/unit/decorators/include_organization_decorator_spec.rb b/spec/unit/decorators/include_organization_decorator_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
 module VCAP::CloudController
-  RSpec.describe IncludeAppOrganizationDecorator do
-    subject(:decorator) { IncludeAppOrganizationDecorator }
+  RSpec.describe IncludeOrganizationDecorator do
+    subject(:decorator) { IncludeOrganizationDecorator }
     let(:organization1) { Organization.make(name: 'first-organization') }
     let(:organization2) { Organization.make(name: 'second-organization') }
     let(:space1) { Space.make(name: 'first-space', organization: organization1) }
diff --git a/spec/unit/decorators/include_space_decorator_spec.rb b/spec/unit/decorators/include_space_decorator_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
 module VCAP::CloudController
-  RSpec.describe IncludeAppSpaceDecorator do
-    subject(:decorator) { IncludeAppSpaceDecorator }
+  RSpec.describe IncludeSpaceDecorator do
+    subject(:decorator) { IncludeSpaceDecorator }
     let(:space1) { Space.make(name: 'first-space') }
     let(:space2) { Space.make(name: 'second-space') }
     let(:apps) { [AppModel.make(space: space1), AppModel.make(space: space2), AppModel.make(space: space1)] }
diff --git a/spec/unit/messages/route_show_message_spec.rb b/spec/unit/messages/route_show_message_spec.rb
@@ -29,9 +29,13 @@ module VCAP::CloudController
     end
 
     describe 'includes' do
-      it 'only allows domains to be included' do
+      it 'only allows domains, space, space.organization to be included' do
         message = RouteShowMessage.from_params({ 'guid' => 'some-guid', 'include' => 'domain' })
         expect(message).to be_valid
+        message = RouteShowMessage.from_params({ 'guid' => 'some-guid', 'include' => 'space' })
+        expect(message).to be_valid
+        message = RouteShowMessage.from_params({ 'guid' => 'some-guid', 'include' => 'space.organization' })
+        expect(message).to be_valid
         message = RouteShowMessage.from_params({ 'guid' => 'some-guid', 'include' => 'kube' })
         expect(message).not_to be_valid
       end
diff --git a/spec/unit/messages/routes_list_message_spec.rb b/spec/unit/messages/routes_list_message_spec.rb
@@ -89,11 +89,13 @@ module VCAP::CloudController
         expect(message.errors[:base][0]).to include("Unknown query parameter(s): 'app_guid'")
       end
 
-      it 'does not accept include that is not domain' do
+      it 'does accepts domain,space, and space.organization' do
         message = RoutesListMessage.from_params({ 'include' => 'domain' })
         expect(message).to be_valid
+        message = RoutesListMessage.from_params({ 'include' => 'space' })
+        expect(message).to be_valid
         message = RoutesListMessage.from_params({ 'include' => 'space.organization' })
-        expect(message).not_to be_valid
+        expect(message).to be_valid
         message = RoutesListMessage.from_params({ 'include' => 'eli\'s buildpack' })
         expect(message).not_to be_valid
       end

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ class RoutesListMessage < ListMessage`
`15`	`15`	`]`
`16`	`16`
`17`	`17`	`validates_with NoAdditionalParamsValidator`
`18`		`- validates_with IncludeParamValidator, valid_values: ['domain']`
	`18`	`+ validates_with IncludeParamValidator, valid_values: ['domain', 'space', 'space.organization']`
`19`	`19`
`20`	`20`	`validates :hosts, allow_nil: true, array: true`
`21`	`21`	`validates :paths, allow_nil: true, array: true`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ Name \| Type \| Description`
`41`	`41`	`per_page \| _integer_ \| Number of results per page; <br>valid values are 1 through 5000`
`42`	`42`	order_by \| _string_ \| Value to sort by. Defaults to ascending; prepend with `-` to sort descending. <br>Valid values are `created_at`, `updated_at`
`43`	`43`	`label_selector (experimental)\| _string_ \| A query string containing a list of [label selector](#labels-and-selectors) requirements`
`44`		-include (experimental)\| _string_ \| Optionally include a list of unique related resources in the response <br>Valid value is `domain`
	`44`	+include (experimental)\| _string_ \| Optionally include a list of unique related resources in the response <br>Valid values are `domain`, `space.organization`, `space`
`45`	`45`
`46`	`46`	`#### Permitted roles`
`47`	`47`	`\|`