ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

preload role overrides for roles API 

also only build the overrides cache for the current permission we're
inspecting when in no-cache mode. and other reduce-allocations-optimizations

prevents massive dependence on the query cache and re-instantiating
hundreds-to-thousands of AR objects, and thousands-to-millions of
ruby objects

for a particularly bad case, this should take it from several minutes
to a second or two at most

fixes FOO-1019

Change-Id: I8837a8b5b179491d4f7b3805788b2e3c1db83a6d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268495
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2021-07-06 15:30:13 -06:00
parent 53f6c2dfab
commit 54de40a41a
3 changed files with 58 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/controllers/role_overrides_controller.rb
View File

@ -144,7 +144,7 @@ class RoleOverridesController < ApplicationController
roles = Api.paginate(roles, self, route)
ActiveRecord::Associations::Preloader.new.preload(roles, :account)
render :json => roles.collect{|role| role_json(@context, role, @current_user, session)}
render :json => roles.map { |role| role_json(@context, role, @current_user, session, preloaded_overrides: RoleOverride.preload_overrides(@context, roles)) }
end
end
@ -152,13 +152,15 @@ class RoleOverridesController < ApplicationController
if authorized_action(@context, @current_user, :manage_role_overrides)
account_role_data = []
preloaded_overrides = RoleOverride.preload_overrides(@context, @context.available_account_roles)
@context.available_account_roles.each do |role|
account_role_data << role_json(@context, role, @current_user, session)
account_role_data << role_json(@context, role, @current_user, session, preloaded_overrides: preloaded_overrides)
end
course_role_data = []
preloaded_overrides = RoleOverride.preload_overrides(@context, @context.available_course_roles)
@context.available_course_roles.each do |role|
course_role_data << role_json(@context, role, @current_user, session)
course_role_data << role_json(@context, role, @current_user, session, preloaded_overrides: preloaded_overrides)
end
js_env({

70
app/models/role_override.rb
View File

@ -1592,50 +1592,68 @@ class RoleOverride < ActiveRecord::Base
Setting.get("role_override_local_cache_ttl_seconds", "300").to_i.seconds
end
def self.permission_for(context, permission, role_or_role_id, role_context=:role_account, no_caching=false)
account = context.is_a?(Account) ? context :
Account.new(id: context.account_id) # we can avoid a query since we're just using it for the batched keys on redis
permissionless_base_key = ["role_override_calculation", Shard.global_id_for(role_or_role_id)].compact.join("/")
full_base_key = [permissionless_base_key, permission, Shard.global_id_for(role_context)].join("/")
def self.permission_for(context, permission, role_or_role_id, role_context=:role_account, no_caching=false, preloaded_overrides: nil)
# we can avoid a query since we're just using it for the batched keys on redis
permissionless_base_key = ["role_override_calculation", Shard.global_id_for(role_or_role_id)].join("/") unless no_caching
account = context.is_a?(Account) ? context : Account.new(id: context.account_id)
default_data = self.permissions[permission]
if default_data[:account_allows] || no_caching
# could depend on anything - can't cache (but that's okay because it's not super common)
uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data, no_caching)
uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data, no_caching, preloaded_overrides: preloaded_overrides)
else
full_base_key = [permissionless_base_key, permission, Shard.global_id_for(role_context)].join("/")
LocalCache.fetch([full_base_key, account.global_id].join("/"), expires_in: local_cache_ttl) do
Rails.cache.fetch_with_batched_keys(full_base_key, batch_object: account,
batched_keys: [:account_chain, :role_overrides], skip_cache_if_disabled: true) do
uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data)
uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data, preloaded_overrides: preloaded_overrides)
end
end
end.freeze
end
def self.uncached_overrides_for(context, role, role_context)
context.shard.activate do
accounts = context.account_chain(include_site_admin: true)
overrides = Shard.partition_by_shard(accounts) do |shard_accounts|
def self.preload_overrides(account, roles, role_context = account)
return Hash.new([].freeze) if roles.empty?
account.shard.activate do
result = Hash.new { |h, k| h[k] = Hash.new { |h2, k2| h2[k2] = [] } }
Shard.partition_by_shard(account.account_chain(include_site_admin: true)) do |shard_accounts|
# skip loading from site admin if the role is not from site admin
next if shard_accounts == [Account.site_admin] && role_context != Account.site_admin
RoleOverride.where(:context_id => accounts, :context_type => 'Account', :role_id => role)
end
accounts.reverse!
overrides = overrides.group_by(&:permission)
# every context has to be represented so that we can't miss role_context below
overrides.each_key do |permission|
overrides_by_account = overrides[permission].index_by(&:context_id)
overrides[permission] = accounts.map do |account|
overrides_by_account[account.id] || RoleOverride.new(context_id: account.id, context_type: 'Account')
RoleOverride.where(role: roles, account: shard_accounts).each do |ro|
permission_hash = result[ro.permission]
permission_hash[ro.global_context_id] << ro
end
nil
end
overrides
result
end
end
def self.uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data, no_caching=false)
def self.uncached_overrides_for(context, role, role_context, preloaded_overrides: nil, only_permission: nil)
context.shard.activate do
accounts = context.account_chain(include_site_admin: true)
preloaded_overrides ||= preload_overrides(context, [role], role_context)
accounts.reverse!
overrides = {}
# every context has to be represented so that we can't miss role_context below
preloaded_overrides.each do |(permission, overrides_by_account)|
next if only_permission && permission != only_permission
overrides[permission] = accounts.map do |account|
overrides_by_account[account.global_id].find { |ro| ro.role_id == role.id } || RoleOverride.new(context_id: account.id, context_type: 'Account')
end
end
overrides
end
end
def self.uncached_permission_for(context, permission, role_or_role_id, role_context, account, permissionless_base_key, default_data, no_caching=false, preloaded_overrides: nil)
role = role_or_role_id.is_a?(Role) ? role_or_role_id : Role.get_role_by_id(role_or_role_id)
# be explicit that we're expecting calculation to stop at the role's account rather than, say, passing in a course
@ -1676,13 +1694,13 @@ class RoleOverride < ActiveRecord::Base
return generated_permission if locked
overrides = if no_caching
uncached_overrides_for(context, role, role_context)
uncached_overrides_for(context, role, role_context, preloaded_overrides: preloaded_overrides, only_permission: permission.to_s)
else
RequestCache.cache(permissionless_base_key, account) do
LocalCache.fetch([permissionless_base_key, account.global_id].join("/"), expires_in: local_cache_ttl) do
Rails.cache.fetch_with_batched_keys(permissionless_base_key, batch_object: account,
batched_keys: [:account_chain, :role_overrides], skip_cache_if_disabled: true) do
uncached_overrides_for(context, role, role_context)
uncached_overrides_for(context, role, role_context, preloaded_overrides: preloaded_overrides)
end
end
end
@ -1726,7 +1744,7 @@ class RoleOverride < ActiveRecord::Base
# there was not an override matching this context, so do a half loop
# to set the inherited values
if !last_override
unless last_override
generated_permission[:prior_default] = generated_permission[:enabled]
generated_permission[:readonly] = true if generated_permission[:locked]
end

15
lib/api/v1/role.rb
View File

@ -22,7 +22,7 @@ module Api::V1::Role
include Api::V1::Json
include Api::V1::Account
def role_json(account, role, current_user, session, opts={})
def role_json(account, role, current_user, session, skip_permissions: false, preloaded_overrides: nil)
json = {
:id => role.id,
:role => role.name,
@ -37,17 +37,21 @@ module Api::V1::Role
json[:account] = account_json(role.account, current_user, session, []) if role.account_id
return json if skip_permissions
preloaded_overrides ||= RoleOverride.preload_overrides(account, [role])
RoleOverride.manageable_permissions(account).keys.each do |permission|
perm = RoleOverride.permission_for(account, permission, role, account, true)
perm = RoleOverride.permission_for(account, permission, role, account, true, preloaded_overrides: preloaded_overrides)
next if permission == :manage_developer_keys && !account.root_account?
json[:permissions][permission] = permission_json(perm, current_user, session) if perm[:account_allows]
end unless opts[:skip_permissions]
end
json
end
def permission_json(permission, _current_user, _session)
permission = permission.dup
permission = permission.slice(:enabled, :locked, :readonly, :explicit, :prior_default, :group)
if permission[:enabled]
permission[:applies_to_self] = permission[:enabled].include?(:self)
@ -56,7 +60,6 @@ module Api::V1::Role
permission[:enabled] = !!permission[:enabled]
permission[:prior_default] = !!permission[:prior_default]
permission.delete(:prior_default) unless permission[:explicit]
permission.slice(:enabled, :locked, :readonly, :explicit, :prior_default,
:applies_to_descendants, :applies_to_self, :group)
permission
end
end