add cache around account feature checks on every page load

test plan:
* toggling site admin and root account features such
 as Direct Share should still work as expected

closes #LA-983

Change-Id: I26912068c78a7d10449a2245c902ac31f105be94
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/236682
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
This commit is contained in:
James Williams 2020-05-08 09:41:14 -06:00
parent e059cdec0d
commit 8d5d7e7465
4 changed files with 36 additions and 21 deletions

View File

@ -159,7 +159,6 @@ class ApplicationController < ActionController::Base
use_responsive_layout: use_responsive_layout?,
use_rce_enhancements: (@context.is_a?(User) ? @domain_root_account : @context).try(:feature_enabled?, :rce_enhancements),
rce_auto_save: @context.try(:feature_enabled?, :rce_auto_save),
DIRECT_SHARE_ENABLED: !@context.is_a?(Group) && @domain_root_account.try(:feature_enabled?, :direct_share),
help_link_name: help_link_name,
help_link_icon: help_link_icon,
use_high_contrast: @current_user.try(:prefers_high_contrast?),
@ -172,22 +171,14 @@ class ApplicationController < ActionController::Base
collapse_global_nav: @current_user.try(:collapse_global_nav?),
show_feedback_link: show_feedback_link?
},
FEATURES: {
assignment_bulk_edit: @domain_root_account&.feature_enabled?(:assignment_bulk_edit),
assignment_bulk_edit_phase_2: Account.site_admin.feature_enabled?(:assignment_bulk_edit_phase_2),
canvas_k6_theme: @context.try(:feature_enabled?, :canvas_k6_theme),
cc_in_rce_video_tray: Account.site_admin.feature_enabled?(:cc_in_rce_video_tray),
featured_help_links: Account.site_admin.feature_enabled?(:featured_help_links),
responsive_admin_settings: !!@domain_root_account&.feature_enabled?(:responsive_admin_settings),
responsive_awareness: !!@domain_root_account&.feature_enabled?(:responsive_awareness),
responsive_misc: !!@domain_root_account&.feature_enabled?(:responsive_misc),
product_tours: !!@domain_root_account&.feature_enabled?(:product_tours),
module_dnd: !!@domain_root_account&.feature_enabled?(:module_dnd),
files_dnd: !!@domain_root_account&.feature_enabled?(:files_dnd),
unpublished_courses: !!@domain_root_account&.feature_enabled?(:unpublished_courses)
},
}
@js_env[:KILL_JOY] = @domain_root_account.kill_joy? if @domain_root_account&.kill_joy?
cached_features = cached_js_env_account_features
@js_env[:DIRECT_SHARE_ENABLED] = cached_features.delete(:direct_share) && !@context.is_a?(Group)
@js_env[:FEATURES] = cached_features.merge(
canvas_k6_theme: @context.try(:feature_enabled?, :canvas_k6_theme)
)
@js_env[:current_user] = @current_user ? Rails.cache.fetch(['user_display_json', @current_user].cache_key, :expires_in => 1.hour) { user_display_json(@current_user, :profile, [:avatar_is_fallback]) } : {}
@js_env[:page_view_update_url] = page_view_path(@page_view.id, page_view_token: @page_view.token) if @page_view
@js_env[:IS_LARGE_ROSTER] = true if !@js_env[:IS_LARGE_ROSTER] && @context.respond_to?(:large_roster?) && @context.large_roster?
@ -214,6 +205,29 @@ class ApplicationController < ActionController::Base
end
helper_method :js_env
# put feature checks on Account.site_admin and @domain_root_account that we're loading for every page in here
# so altogether we can get them faster the vast majority of the time
JS_ENV_SITE_ADMIN_FEATURES = [:assignment_bulk_edit_phase_2, :cc_in_rce_video_tray, :featured_help_links].freeze
JS_ENV_ROOT_ACCOUNT_FEATURES = [
:direct_share, :assignment_bulk_edit, :responsive_admin_settings, :responsive_awareness,
:responsive_misc, :product_tours, :module_dnd, :files_dnd, :unpublished_courses
].freeze
JS_ENV_FEATURES_HASH = Digest::MD5.hexdigest([JS_ENV_SITE_ADMIN_FEATURES + JS_ENV_ROOT_ACCOUNT_FEATURES].sort.join(",")).freeze
def cached_js_env_account_features
# can be invalidated by a flag change on either site admin or the domain root account
Rails.cache.fetch(["js_env_account_features", JS_ENV_FEATURES_HASH,
Account.site_admin.cache_key(:feature_flags), @domain_root_account&.cache_key(:feature_flags)].cache_key) do
results = {}
JS_ENV_SITE_ADMIN_FEATURES.each do |f|
results[f] = Account.site_admin.feature_enabled?(f)
end
JS_ENV_ROOT_ACCOUNT_FEATURES.each do |f|
results[f] = !!@domain_root_account&.feature_enabled?(f)
end
results
end
end
def add_to_js_env(hash, jsenv, overwrite)
hash.each do |k,v|
if jsenv[k] && !overwrite

View File

@ -54,6 +54,7 @@ class FeatureFlag < ActiveRecord::Base
if self.context
self.class.connection.after_transaction_commit { self.context.feature_flag_cache.delete(self.context.feature_flag_cache_key(feature)) }
self.context.touch if Feature.definitions[feature].try(:touch_context)
self.context.clear_cache_key(:feature_flags) if self.context.is_a?(Account)
if ::Rails.env.development? && self.context.is_a?(Account) && Account.all_special_accounts.include?(self.context)
Account.clear_special_account_cache!(true)
end

View File

@ -27,7 +27,7 @@ module Canvas
# (which is far less often than the many times per day users are currently being touched)
ALLOWED_TYPES = {
'Account' => %w{account_chain role_overrides global_navigation},
'Account' => %w{account_chain role_overrides global_navigation feature_flags},
'Course' => %w{account_associations},
'User' => %w{enrollments groups account_users todo_list submissions user_services},
'Assignment' => %w{availability},

View File

@ -128,7 +128,7 @@ RSpec.describe ApplicationController do
describe "DIRECT_SHARE_ENABLED feature flag" do
it "sets the env var to true when FF is enabled" do
root_account = double(global_id: 1, open_registration?: true, settings: {})
root_account = double(global_id: 1, open_registration?: true, settings: {}, cache_key: "key")
allow(root_account).to receive(:kill_joy?)
allow(root_account).to receive(:feature_enabled?).and_return(false)
allow(root_account).to receive(:feature_enabled?).with(:direct_share).and_return(true)
@ -138,7 +138,7 @@ RSpec.describe ApplicationController do
end
it "sets the env var to false when the context is a group" do
root_account = double(global_id: 1, open_registration?: true, settings: {})
root_account = double(global_id: 1, open_registration?: true, settings: {}, cache_key: "key")
allow(root_account).to receive(:kill_joy?)
allow(root_account).to receive(:feature_enabled?).and_return(false)
allow(root_account).to receive(:feature_enabled?).with(:direct_share).and_return(true)
@ -149,7 +149,7 @@ RSpec.describe ApplicationController do
end
it "sets the env var to false when FF is disabled" do
root_account = double(global_id: 1, open_registration?: true, settings: {})
root_account = double(global_id: 1, open_registration?: true, settings: {}, cache_key: "key")
allow(root_account).to receive(:kill_joy?)
allow(root_account).to receive(:feature_enabled?).and_return(false)
allow(HostUrl).to receive_messages(file_host: 'files.example.com')
@ -183,7 +183,7 @@ RSpec.describe ApplicationController do
end
it 'gets appropriate settings from the root account' do
root_account = double(global_id: 1, feature_enabled?: false, open_registration?: true, settings: {})
root_account = double(global_id: 1, feature_enabled?: false, open_registration?: true, settings: {}, cache_key: "key")
allow(root_account).to receive(:kill_joy?).and_return(false)
allow(HostUrl).to receive_messages(file_host: 'files.example.com')
controller.instance_variable_set(:@domain_root_account, root_account)
@ -192,7 +192,7 @@ RSpec.describe ApplicationController do
end
it 'disables fun when set' do
root_account = double(global_id: 1, feature_enabled?: false, open_registration?: true, settings: {})
root_account = double(global_id: 1, feature_enabled?: false, open_registration?: true, settings: {}, cache_key: "key")
allow(root_account).to receive(:kill_joy?).and_return(true)
allow(HostUrl).to receive_messages(file_host: 'files.example.com')
controller.instance_variable_set(:@domain_root_account, root_account)