Fix v3/roles perf regression for regular users

app/controllers/v3/roles_controller.rb

@@ -173,8 +173,8 @@ def readable_roles
     if permission_queryer.can_read_globally?
       Role
     else
-      readable_by_space = { space_id: permission_queryer.readable_spaces_query.select(:id) }
-      readable_by_org = { organization_id: permission_queryer.readable_orgs_query.select(:id) }
+      readable_by_space = { space_id: permission_queryer.readable_space_ids_query }
+      readable_by_org = { organization_id: permission_queryer.readable_org_ids_query }
       Role.where { Sequel.|(readable_by_space, readable_by_org) }
     end
   end

lib/cloud_controller/membership.rb

@@ -29,13 +29,19 @@ def authorized_org_guids_subquery(roles)
     end
 
     def authorized_orgs_subquery(roles)
+      Organization.where(id: authorized_org_ids_subquery(roles))
+    end
+
+    def authorized_org_ids_subquery(roles)
       space_ids_query = member_space_ids(roles)
-      org_ids_from_space_roles = Space.where(id: space_ids_query).select(:organization_id) if space_ids_query
+      org_ids_query   = member_org_ids(roles)
 
-      if org_ids_from_space_roles
-        Organization.where(id: member_org_ids(roles)).or(id: org_ids_from_space_roles)
+      org_ids_via_space_roles = Space.where(id: space_ids_query).select(:organization_id) if space_ids_query
+
+      if org_ids_query && org_ids_via_space_roles
+        org_ids_query.union(org_ids_via_space_roles, from_self: false)
       else
-        Organization.where(id: member_org_ids(roles))
+        org_ids_query || org_ids_via_space_roles
       end
     end
 
@@ -48,11 +54,19 @@ def authorized_space_guids_subquery(roles)
     end
 
     def authorized_spaces_subquery(roles)
-      org_ids = member_org_ids(roles)
-      if org_ids
-        Space.where(id: member_space_ids(roles)).or(organization_id: org_ids).select(:id, :guid)
+      Space.where(id: authorized_space_ids_subquery(roles)).select(:id, :guid)
+    end
+
+    def authorized_space_ids_subquery(roles)
+      space_ids_query = member_space_ids(roles)
+      org_ids_query   = member_org_ids(roles)
+
+      space_ids_via_org_roles = Space.where(organization_id: org_ids_query).select(:id) if org_ids_query
+
+      if space_ids_query && space_ids_via_org_roles
+        space_ids_query.union(space_ids_via_org_roles, from_self: false)
       else
-        Space.where(id: member_space_ids(roles)).select(:id, :guid)
+        space_ids_query || space_ids_via_org_roles
       end
     end
 

lib/cloud_controller/permissions.rb

@@ -123,6 +123,14 @@ def readable_orgs_query
     end
   end
 
+  def readable_org_ids_query
+    if can_read_globally?
+      VCAP::CloudController::Organization.select(:id)
+    else
+      membership.authorized_org_ids_subquery(ROLES_FOR_ORG_READING)
+    end
+  end
+
   def readable_org_guids_for_domains_query
     if can_read_globally?
       VCAP::CloudController::Organization.select(:guid)
@@ -168,6 +176,12 @@ def readable_spaces_query
     membership.authorized_spaces_subquery(ROLES_FOR_SPACE_READING)
   end
 
+  def readable_space_ids_query
+    raise 'must not be called for users that can read globally' if can_read_globally?
+
+    membership.authorized_space_ids_subquery(ROLES_FOR_SPACE_READING)
+  end
+
   def readable_space_guids_query
     raise 'must not be called for users that can read globally' if can_read_globally?
 

spec/unit/lib/cloud_controller/permissions_spec.rb

@@ -163,6 +163,16 @@ module VCAP::CloudController
       end
     end
 
+    describe '#readable_org_ids_query' do
+      it 'returns subquery from membership' do
+        membership = instance_double(Membership)
+        subquery = instance_double(Sequel::Dataset)
+        expect(Membership).to receive(:new).with(user).and_return(membership)
+        expect(membership).to receive(:authorized_org_ids_subquery).with(Permissions::ROLES_FOR_ORG_READING).and_return(subquery)
+        expect(permissions.readable_org_ids_query).to be(subquery)
+      end
+    end
+
     describe '#readable_org_guids_for_domains_query' do
       context 'when user has valid membership' do
         let(:membership) { instance_double(Membership) }
@@ -388,6 +398,16 @@ module VCAP::CloudController
       end
     end
 
+    describe '#readable_space_ids_query' do
+      it 'returns subquery from membership' do
+        membership = instance_double(Membership)
+        subquery = instance_double(Sequel::Dataset)
+        expect(Membership).to receive(:new).with(user).and_return(membership)
+        expect(membership).to receive(:authorized_space_ids_subquery).with(Permissions::ROLES_FOR_SPACE_READING).and_return(subquery)
+        expect(permissions.readable_space_ids_query).to be(subquery)
+      end
+    end
+
     describe '#readables_space_guids_query' do
       it 'returns subquery from membership' do
         membership = instance_double(Membership)