cache all feature flags on root accounts

since they are the most common target of feature flag predicates

Change-Id: I2c6be68f442b2eb66f8cd5ad57cafbc83f114ba1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271502
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2021-08-16 10:29:14 -06:00
parent cd7f6d91d7
commit 3e2762061f
6 changed files with 32 additions and 19 deletions

View File

@ -1574,6 +1574,7 @@ class Account < ActiveRecord::Base
# an opportunity for plugins to load some other stuff up before caching the account
def precache
feature_flags.load
end
class ::Canvas::AccountCacheError < StandardError; end

View File

@ -64,19 +64,22 @@ class FeatureFlag < ActiveRecord::Base
end
def clear_cache
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)
if self.context.is_a?(Account)
if self.context.site_admin?
Switchman::DatabaseServer.send_in_each_region(self.context, :clear_cache_key, {}, :feature_flags)
else
self.context.clear_cache_key(:feature_flags)
end
end
if context
self.class.connection.after_transaction_commit do
context.feature_flag_cache.delete(context.feature_flag_cache_key(feature))
context.touch if Feature.definitions[feature].try(:touch_context) || context.try(:root_account?)
if ::Rails.env.development? && self.context.is_a?(Account) && Account.all_special_accounts.include?(self.context)
Account.clear_special_account_cache!(true)
if context.is_a?(Account)
if context.site_admin?
Switchman::DatabaseServer.send_in_each_region(context, :clear_cache_key, {}, :feature_flags)
else
context.clear_cache_key(:feature_flags)
end
end
if !::Rails.env.production? && context.is_a?(Account) && Account.all_special_accounts.include?(context)
Account.clear_special_account_cache!(true)
end
end
end
end

View File

@ -45,12 +45,12 @@ module FeatureFlags
def set_feature_flag!(feature, state)
feature = feature.to_s
flag = self.feature_flags.where(feature: feature).first
flag ||= self.feature_flags.build(feature: feature)
flag = feature_flags.find_or_initialize_by(feature: feature)
flag.state = state
@feature_flag_cache ||= {}
@feature_flag_cache[feature] = flag
flag.save!
association(:feature_flags).reset
end
def allow_feature!(feature)
@ -141,6 +141,10 @@ module FeatureFlags
# find the highest flag that doesn't allow override,
# or the most specific flag otherwise
accounts = feature_flag_account_ids.map do |id|
# optimizations for accounts we likely already have loaded (including their feature flags!)
next Account.site_admin if id == Account.site_admin.global_id
next Account.current_domain_root_account if id == Account.current_domain_root_account&.global_id
account = Account.new
account.id = id
account.shard = Shard.shard_for(id, self.shard)
@ -150,9 +154,10 @@ module FeatureFlags
all_contexts = (accounts + [self]).uniq
all_contexts -= [self] if inherited_only
all_contexts.each_with_index do |context, idx|
all_contexts.each do |context|
flag = context.feature_flag(feature, skip_cache: context == self && skip_cache)
next unless flag
retval = flag
break unless flag.can_override?
end

View File

@ -2977,6 +2977,7 @@ describe Enrollment do
before :each do
course_with_student
@course.root_account.enable_feature!(:granular_permissions_manage_users)
@enrollment.reload
end
it 'is true for a user who has been granted :manage_students' do
@ -3041,6 +3042,7 @@ describe Enrollment do
before :each do
course_with_observer
@course.root_account.enable_feature!(:granular_permissions_manage_users)
@enrollment.reload
end
it 'is true with :manage_students' do
@ -3074,6 +3076,7 @@ describe Enrollment do
before :each do
course_with_teacher
@course.root_account.enable_feature!(:granular_permissions_manage_users)
@enrollment.reload
end
it 'is false with :manage_students' do

View File

@ -1147,7 +1147,7 @@ describe Quizzes::Quiz do
end
it "returns false when feature flag is off" do
@course.root_account.set_feature_flag! 'timer_without_autosubmission', 'off'
@quiz.context.root_account.set_feature_flag! 'timer_without_autosubmission', 'off'
expect(@quiz.timer_autosubmit_disabled?).to be(false)
@quiz.update({
disable_timer_autosubmission: true
@ -1156,12 +1156,12 @@ describe Quizzes::Quiz do
end
it "returns false when feature flag is on and disable_timer_autosubmission is false" do
@course.root_account.set_feature_flag! 'timer_without_autosubmission', 'on'
@quiz.context.root_account.set_feature_flag! 'timer_without_autosubmission', 'on'
expect(@quiz.timer_autosubmit_disabled?).to be(false)
end
it "returns true when feature flag is on and disable_timer_autosubmission is true" do
@course.root_account.set_feature_flag! 'timer_without_autosubmission', 'on'
@quiz.context.root_account.set_feature_flag! 'timer_without_autosubmission', 'on'
@quiz.update({
disable_timer_autosubmission: true
})

View File

@ -145,6 +145,7 @@ describe "courses/settings.html.erb" do
end
before :each do
@course.root_account.reload
view_context(@course, @teacher)
end
@ -247,7 +248,7 @@ describe "courses/settings.html.erb" do
end
context "with the feature enabled" do
before { Account.default.enable_feature!(:course_templates) }
before { @course.root_account.enable_feature!(:course_templates) }
it "is visible" do
render