Skip to content

Commit

Permalink
v3: Create 'roles' view to back Role model, instead of dataset
Browse files Browse the repository at this point in the history
We had a dataset representing the union of the 7 roles tables that was
backing the Role Sequel model. This change adds a migration to create a
view that represents the same union, and switches the model to be backed
by the view instead.

This solves a problem introduced as part of #164176828, where the
PollableJob model was trying to run a query against a table called
`roles` to check if a role existed. Since that table didn't exist, it
was failing. Now, there is a view with that name, so those queries work.

[Finishes #164176828]

Authored-by: Reid Mitchell <[email protected]>
  • Loading branch information
reidmit committed Nov 11, 2019
1 parent a61176a commit 30bec82
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 75 deletions.
9 changes: 4 additions & 5 deletions app/controllers/v3/roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,10 @@ def readable_users
def readable_roles
visible_user_ids = readable_users.select(:id)

roles_for_visible_users = Role.where(user_id: visible_user_ids)
roles_in_visible_spaces = roles_for_visible_users.filter(space_id: visible_space_ids)
roles_in_visible_orgs = roles_for_visible_users.filter(organization_id: visible_org_ids)

roles_in_visible_spaces.union(roles_in_visible_orgs)
Role.where(Sequel.or([
[:space_id, visible_space_ids],
[:organization_id, visible_org_ids]
]).&(user_id: visible_user_ids))
end

def visible_space_ids
Expand Down
70 changes: 1 addition & 69 deletions app/models/runtime/role.rb
Original file line number Diff line number Diff line change
@@ -1,73 +1,5 @@
module VCAP::CloudController
SPACE_OR_ORGANIZATION_NOT_SPECIFIED = -1

# Sequel allows to create models based on datasets. The following is a dataset that unions all the individual roles
# tables and labels each row with a `type` column based on which table it came from
class Role < Sequel::Model(
OrganizationUser.select(
Sequel.as(VCAP::CloudController::RoleTypes::ORGANIZATION_USER, :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :space_id),
:created_at,
:updated_at
).union(
OrganizationManager.select(
Sequel.as(VCAP::CloudController::RoleTypes::ORGANIZATION_MANAGER, :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :space_id),
:created_at,
:updated_at)
).union(
OrganizationBillingManager.select(
Sequel.as(VCAP::CloudController::RoleTypes::ORGANIZATION_BILLING_MANAGER, :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :space_id),
:created_at,
:updated_at)
).union(
OrganizationAuditor.select(
Sequel.as(VCAP::CloudController::RoleTypes::ORGANIZATION_AUDITOR, :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :space_id),
:created_at,
:updated_at)
).union(
SpaceDeveloper.select(
Sequel.as(VCAP::CloudController::RoleTypes::SPACE_DEVELOPER, :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :organization_id),
:space_id,
:created_at,
:updated_at)
).union(
SpaceAuditor.select(
Sequel.as(VCAP::CloudController::RoleTypes::SPACE_AUDITOR, :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :organization_id),
:space_id,
:created_at,
:updated_at)
).union(
SpaceManager.select(
Sequel.as(VCAP::CloudController::RoleTypes::SPACE_MANAGER, :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(SPACE_OR_ORGANIZATION_NOT_SPECIFIED, :organization_id),
:space_id,
:created_at,
:updated_at))
)

class Role < Sequel::Model
def user_guid
User.first(id: user_id).guid
end
Expand Down
69 changes: 69 additions & 0 deletions db/migrations/20191108124700_create_roles_view.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
Sequel.migration do
change do
not_specified = -1

create_view :roles,
self[:organizations_users].select(
Sequel.as('organization_user', :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(not_specified, :space_id),
:created_at,
:updated_at
).union(
self[:organizations_managers].select(
Sequel.as('organization_manager', :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(not_specified, :space_id),
:created_at,
:updated_at)
).union(
self[:organizations_billing_managers].select(
Sequel.as('organization_billing_manager', :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(not_specified, :space_id),
:created_at,
:updated_at)
).union(
self[:organizations_auditors].select(
Sequel.as('organization_auditor', :type),
Sequel.as(:role_guid, :guid),
:user_id,
:organization_id,
Sequel.as(not_specified, :space_id),
:created_at,
:updated_at)
).union(
self[:spaces_developers].select(
Sequel.as('space_developer', :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(not_specified, :organization_id),
:space_id,
:created_at,
:updated_at)
).union(
self[:spaces_auditors].select(
Sequel.as('space_auditor', :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(not_specified, :organization_id),
:space_id,
:created_at,
:updated_at)
).union(
self[:spaces_managers].select(
Sequel.as('space_manager', :type),
Sequel.as(:role_guid, :guid),
:user_id,
Sequel.as(not_specified, :organization_id),
:space_id,
:created_at,
:updated_at))
end
end
6 changes: 5 additions & 1 deletion spec/request/roles_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1093,9 +1093,13 @@ def make_space_role_for_current_user(type)
lambda do
expect(last_response.headers['Location']).to match(%r(http.+/v3/jobs/[a-fA-F0-9-]+))

job_guid = last_response.headers['Location'].gsub("#{link_prefix}/v3/jobs/", '')
get "/v3/jobs/#{job_guid}", {}, admin_headers
expect(last_response).to have_status_code(200)

execute_all_jobs(expected_successes: 1, expected_failures: 0)
get "/v3/roles/#{role.guid}", {}, admin_headers
expect(last_response.status).to eq(404)
expect(last_response).to have_status_code(404)
end
end

Expand Down

0 comments on commit 30bec82

Please sign in to comment.