Skip to content

Commit 5b672e8

Browse files
authored
Remove id guid id roundtrips (#5318)
* Add bare-id read-permission helpers to Permissions Add readable_org_ids_for_domains_query and readable_security_group_ids_query, mirroring the existing *_guids_query helpers but returning the flat id UNION without the enclosing SELECT ... FROM organizations/security_groups WHERE id IN wrapper. Prerequisite for removing id -> guid -> id round-trips from V3 read-permission filters. Follow-up to #5157 and #5172. * Filter GET /v3/organizations by org id instead of guid Switch OrgListFetcher and the organization presenter args to the bare-id readable_org_ids_query, collapsing the id -> guid -> id round-trip on the non-admin GET /v3/organizations path into a single WHERE id IN (UNION). Follow-up to #5157 and #5172. * Filter GET /v3/organization_quotas by org id instead of guid Switch OrganizationQuotaListFetcher and OrganizationQuotaPresenter to the bare-id readable_org_ids_query. The presenter's per-quota visible-org filter previously ran an id -> guid -> id round-trip once per quota returned; it now filters directly on organizations.id. Follow-up to #5157 and #5172. * Filter GET /v3/domains* by org id instead of guid Switch DomainFetcher, DomainPresenter, DomainSharedOrgsPresenter and the domains/default-domain paths on OrganizationsController to the bare-id readable_org_ids(_for_domains)_query, removing the id -> guid -> id round-trip from domain read-permission filtering. Follow-up to #5157 and #5172. * Filter service plan visibility by org id instead of guid Switch ServicePlanVisibilityFetcher to the bare-id readable_org_ids_query, removing the id -> guid -> id round-trip from marketplace visibility checks. Follow-up to #5157 and #5172. * Filter GET /v3/security_groups* by security group id instead of guid Switch SecurityGroupListFetcher and SecurityGroupFetcher to the new bare-id readable_security_group_ids_query, removing the id -> guid -> id round-trip from the security group visibility filter. The single-resource guid lookup and the guid-membership checks on individual actions are left untouched. Follow-up to #5157 and #5172. * Filter isolation segment organizations by org id instead of guid Switch IsolationSegmentOrganizationsFetcher (backing GET /v3/isolation_segments/:guid/relationships/organizations for non-admins) to the bare-id readable_org_ids_query. The IsolationSegmentListFetcher path is left as-is: its organizations association is keyed on guid through the join table, so no org id is in scope there. Follow-up to #5157 and #5172.
1 parent 1be6a47 commit 5b672e8

28 files changed

Lines changed: 143 additions & 102 deletions

app/controllers/v3/domains_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def index
1818
message = DomainsListMessage.from_params(query_params)
1919
invalid_param!(message.errors.full_messages) unless message.valid?
2020

21-
dataset = DomainFetcher.fetch(message, permission_queryer.readable_org_guids_for_domains_query)
21+
dataset = DomainFetcher.fetch(message, permission_queryer.readable_org_ids_for_domains_query)
2222

2323
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
2424
presenter: Presenters::V3::DomainPresenter,
@@ -164,7 +164,7 @@ def to_route_list_params(query_params, domain)
164164
def find_domain(message)
165165
DomainFetcher.fetch(
166166
message,
167-
permission_queryer.readable_org_guids_for_domains_query
167+
permission_queryer.readable_org_ids_for_domains_query
168168
).first
169169
end
170170

@@ -249,7 +249,7 @@ def presenter_args
249249
if permission_queryer.can_read_globally?
250250
{ all_orgs_visible: true }
251251
else
252-
{ visible_org_guids_query: permission_queryer.readable_org_guids_query }
252+
{ visible_org_ids_query: permission_queryer.readable_org_ids_query }
253253
end
254254
end
255255
end

app/controllers/v3/isolation_segments_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def relationships_orgs
9090
organizations = if permission_queryer.can_read_globally?
9191
fetcher.fetch_all
9292
else
93-
fetcher.fetch_for_organizations(org_guids_query: permission_queryer.readable_org_guids_query)
93+
fetcher.fetch_for_organizations(org_ids_query: permission_queryer.readable_org_ids_query)
9494
end
9595

9696
render status: :ok, json: Presenters::V3::ToManyRelationshipPresenter.new(

app/controllers/v3/organization_quotas_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def index
1717
dataset = if permission_queryer.can_read_globally?
1818
OrganizationQuotaListFetcher.fetch_all(message:)
1919
else
20-
OrganizationQuotaListFetcher.fetch(message: message, readable_org_guids_query: permission_queryer.readable_org_guids_query)
20+
OrganizationQuotaListFetcher.fetch(message: message, readable_org_ids_query: permission_queryer.readable_org_ids_query)
2121
end
2222

2323
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
@@ -110,7 +110,7 @@ def presenter_args
110110
if permission_queryer.can_read_globally?
111111
{ all_orgs_visible: true }
112112
else
113-
{ visible_org_guids_query: permission_queryer.readable_org_guids_query }
113+
{ visible_org_ids_query: permission_queryer.readable_org_ids_query }
114114
end
115115
end
116116
end

app/controllers/v3/organizations_controller.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ def index_org_domains
149149
message = DomainsListMessage.from_params(query_params.except(:guid))
150150
invalid_param!(message.errors.full_messages) unless message.valid?
151151

152-
domains = DomainFetcher.fetch(message, permission_queryer.readable_org_guids_for_domains_query.where(guid: org.guid))
152+
readable_org_ids = Organization.where(id: org.id).where(id: permission_queryer.readable_org_ids_for_domains_query).select(:id)
153+
domains = DomainFetcher.fetch(message, readable_org_ids)
153154

154155
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
155156
presenter: Presenters::V3::DomainPresenter,
@@ -166,7 +167,7 @@ def show_default_domain
166167
domain = org.default_domain
167168

168169
domain_not_found! unless domain
169-
domain_not_found! if domain.private? && permission_queryer.readable_org_guids_for_domains_query.where(guid: org.guid).empty?
170+
domain_not_found! if domain.private? && Organization.where(id: org.id).where(id: permission_queryer.readable_org_ids_for_domains_query).empty?
170171

171172
render status: :ok, json: Presenters::V3::DomainPresenter.new(domain, **presenter_args)
172173
end
@@ -245,7 +246,7 @@ def fetch_orgs(message)
245246
else
246247
OrgListFetcher.fetch(
247248
message: message,
248-
guids: permission_queryer.readable_org_guids_query,
249+
ids: permission_queryer.readable_org_ids_query,
249250
eager_loaded_associations: Presenters::V3::OrganizationPresenter.associated_resources
250251
)
251252
end
@@ -260,7 +261,7 @@ def fetch_orgs_for_isolation_segment(message)
260261
else
261262
isolation_segment, dataset = OrgListFetcher.fetch_for_isolation_segment(
262263
message: message,
263-
guids: permission_queryer.readable_org_guids_query,
264+
ids: permission_queryer.readable_org_ids_query,
264265
eager_loaded_associations: Presenters::V3::OrganizationPresenter.associated_resources
265266
)
266267
end
@@ -272,7 +273,7 @@ def presenter_args
272273
if permission_queryer.can_read_globally?
273274
{ all_orgs_visible: true }
274275
else
275-
{ visible_org_guids_query: permission_queryer.readable_org_guids_query }
276+
{ visible_org_ids_query: permission_queryer.readable_org_ids_query }
276277
end
277278
end
278279
end

app/controllers/v3/security_groups_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def index
1818
dataset = if permission_queryer.can_read_globally?
1919
SecurityGroupListFetcher.fetch_all(message)
2020
else
21-
SecurityGroupListFetcher.fetch(message, permission_queryer.readable_security_group_guids_query)
21+
SecurityGroupListFetcher.fetch(message, permission_queryer.readable_security_group_ids_query)
2222
end
2323

2424
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
@@ -31,7 +31,7 @@ def index
3131
end
3232

3333
def show
34-
security_group = SecurityGroupFetcher.fetch(hashed_params[:guid], permission_queryer.readable_security_group_guids_query)
34+
security_group = SecurityGroupFetcher.fetch(hashed_params[:guid], permission_queryer.readable_security_group_ids_query)
3535
resource_not_found!(:security_group) unless security_group
3636

3737
render status: :ok, json: Presenters::V3::SecurityGroupPresenter.new(

app/controllers/v3/spaces_controller.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ def running_security_groups
106106
space = SpaceFetcher.new.fetch(hashed_params[:guid])
107107
space_not_found! unless space && permission_queryer.can_read_from_space?(space.id, space.organization_id)
108108

109-
unfiltered_group_guids = fetch_running_security_group_guids(space)
110-
dataset = SecurityGroupListFetcher.fetch(message, unfiltered_group_guids)
109+
unfiltered_group_ids = fetch_running_security_group_ids(space)
110+
dataset = SecurityGroupListFetcher.fetch(message, unfiltered_group_ids)
111111

112112
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
113113
presenter: Presenters::V3::SecurityGroupPresenter,
@@ -125,8 +125,8 @@ def staging_security_groups
125125
space = SpaceFetcher.new.fetch(hashed_params[:guid])
126126
space_not_found! unless space && permission_queryer.can_read_from_space?(space.id, space.organization_id)
127127

128-
unfiltered_group_guids = fetch_staging_security_group_guids(space)
129-
dataset = SecurityGroupListFetcher.fetch(message, unfiltered_group_guids)
128+
unfiltered_group_ids = fetch_staging_security_group_ids(space)
129+
dataset = SecurityGroupListFetcher.fetch(message, unfiltered_group_ids)
130130

131131
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
132132
presenter: Presenters::V3::SecurityGroupPresenter,
@@ -243,16 +243,16 @@ def fetch_isolation_segment(guid)
243243
IsolationSegmentModel.where(guid:).first
244244
end
245245

246-
def fetch_running_security_group_guids(space)
246+
def fetch_running_security_group_ids(space)
247247
space_level_groups = SecurityGroup.where(spaces: space)
248248
global_groups = SecurityGroup.where(running_default: true)
249-
space_level_groups.union(global_groups).select_map(:guid)
249+
space_level_groups.union(global_groups).select_map(:id)
250250
end
251251

252-
def fetch_staging_security_group_guids(space)
252+
def fetch_staging_security_group_ids(space)
253253
space_level_groups = SecurityGroup.where(staging_spaces: space)
254254
global_groups = SecurityGroup.where(staging_default: true)
255-
space_level_groups.union(global_groups).distinct.select_map(:guid)
255+
space_level_groups.union(global_groups).distinct.select_map(:id)
256256
end
257257

258258
def space_not_found!

app/fetchers/domain_fetcher.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
module VCAP::CloudController
44
class DomainFetcher < BaseListFetcher
55
class << self
6-
def fetch_all_for_orgs(readable_org_guids)
6+
def fetch_all_for_orgs(readable_org_ids)
77
# Q: The "Domain" in Domain.dataset is arbitrary -- just a way to get access to any database table.
88
# If there's a way to use a more generic way to access the database, maybe this can be revised.
99
#
10-
readable_orgs_filter = Domain.dataset.db[:organizations].where(guid: readable_org_guids).select(:id)
10+
readable_orgs_filter = Domain.dataset.db[:organizations].where(id: readable_org_ids).select(:id)
1111

1212
readable_shared_private_domains_filter = Domain.dataset.db[:organizations_private_domains].where(
1313
organization_id: readable_orgs_filter
@@ -22,8 +22,8 @@ def fetch_all_for_orgs(readable_org_guids)
2222
Domain.where(user_visible_domains).qualify
2323
end
2424

25-
def fetch(message, readable_org_guids)
26-
dataset = fetch_all_for_orgs(readable_org_guids)
25+
def fetch(message, readable_org_ids)
26+
dataset = fetch_all_for_orgs(readable_org_ids)
2727
filter(message, dataset)
2828
end
2929

app/fetchers/isolation_segment_organizations_fetcher.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ def fetch_all
88
@isolation_segment.organizations
99
end
1010

11-
def fetch_for_organizations(org_guids_query:)
12-
Organization.where(guid: org_guids_query, isolation_segment_models: @isolation_segment).all
11+
def fetch_for_organizations(org_ids_query:)
12+
Organization.where(id: org_ids_query, isolation_segment_models: @isolation_segment).all
1313
end
1414
end
1515
end

app/fetchers/org_list_fetcher.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
module VCAP::CloudController
66
class OrgListFetcher < BaseListFetcher
77
class << self
8-
def fetch(message:, guids:, eager_loaded_associations: [])
9-
dataset = Organization.where(guid: guids)
8+
def fetch(message:, ids:, eager_loaded_associations: [])
9+
dataset = Organization.where(id: ids)
1010
dataset = eager_load(dataset, eager_loaded_associations)
1111
filter(message, dataset)
1212
end
@@ -17,11 +17,11 @@ def fetch_all(message:, eager_loaded_associations: [])
1717
filter(message, dataset)
1818
end
1919

20-
def fetch_for_isolation_segment(message:, guids:, eager_loaded_associations: [])
20+
def fetch_for_isolation_segment(message:, ids:, eager_loaded_associations: [])
2121
isolation_segment = IsolationSegmentModel.where(guid: message.isolation_segment_guid).first
2222
return nil unless isolation_segment
2323

24-
dataset = isolation_segment.organizations_dataset.where(guid: guids)
24+
dataset = isolation_segment.organizations_dataset.where(id: ids)
2525
dataset = eager_load(dataset, eager_loaded_associations)
2626
[isolation_segment, filter(message, dataset)]
2727
end

app/fetchers/organization_quota_list_fetcher.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
module VCAP::CloudController
66
class OrganizationQuotaListFetcher < BaseListFetcher
77
class << self
8-
def fetch(message:, readable_org_guids_query:)
8+
def fetch(message:, readable_org_ids_query:)
99
dataset = QuotaDefinition.dataset
10-
filter(message, dataset, readable_org_guids_query)
10+
filter(message, dataset, readable_org_ids_query)
1111
end
1212

1313
def fetch_all(message:)
@@ -17,7 +17,7 @@ def fetch_all(message:)
1717

1818
private
1919

20-
def filter(message, dataset, readable_org_guids_query=nil)
20+
def filter(message, dataset, readable_org_ids_query=nil)
2121
dataset = dataset.where(name: message.names) if message.requested? :names
2222

2323
if message.requested? :organization_guids
@@ -26,7 +26,7 @@ def filter(message, dataset, readable_org_guids_query=nil)
2626
where(organizations__guid: message.organization_guids).distinct(:id).
2727
qualify(:quota_definitions)
2828

29-
dataset = dataset.where(organizations__guid: readable_org_guids_query) if readable_org_guids_query
29+
dataset = dataset.where(organizations__id: readable_org_ids_query) if readable_org_ids_query
3030
end
3131

3232
super(message, dataset, QuotaDefinition)

0 commit comments

Comments
 (0)