Skip to content

Commit eebf6d7

Browse files
maneframeroor0
authored andcommitted
use global account ids when invalidating cached settings
test plan: * with caching and sharding enabled, should be able to change the inheritable account settings "Restrict students from viewing courses before/after date" and have the changes persist / show up in sub-accounts closes #CNVS-23134 Change-Id: I93b39d96e36040cbc522a21f05cf105128354065 Reviewed-on: https://gerrit.instructure.com/63880 Reviewed-by: Dan Minkevitch <dan@instructure.com> Tested-by: Jenkins QA-Review: Charles Kimball <ckimball@instructure.com> Product-Review: James Williams <jamesw@instructure.com>
1 parent 2a4eebb commit eebf6d7

3 files changed

Lines changed: 35 additions & 8 deletions

File tree

app/models/account.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ def invalidate_caches_if_changed
533533
if @invalidations.present?
534534
shard.activate do
535535
@invalidations.each do |key|
536-
Rails.cache.delete([key, id].cache_key)
536+
Rails.cache.delete([key, self.global_id].cache_key)
537537
end
538538
Account.send_later_if_production(:invalidate_inherited_caches, self, @invalidations)
539539
end
@@ -544,8 +544,9 @@ def self.invalidate_inherited_caches(parent_account, keys)
544544
parent_account.shard.activate do
545545
account_ids = Account.sub_account_ids_recursive(parent_account.id)
546546
account_ids.each do |id|
547+
global_id = Shard.global_id_for(id)
547548
keys.each do |key|
548-
Rails.cache.delete([key, id].cache_key)
549+
Rails.cache.delete([key, global_id].cache_key)
549550
end
550551
end
551552
end
@@ -560,7 +561,7 @@ def quota
560561
return self.class.default_storage_quota if root_account?
561562

562563
shard.activate do
563-
Rails.cache.fetch(['current_quota', id].cache_key) do
564+
Rails.cache.fetch(['current_quota', self.global_id].cache_key) do
564565
self.parent_account.default_storage_quota
565566
end
566567
end
@@ -571,7 +572,7 @@ def default_storage_quota
571572
return self.class.default_storage_quota if root_account?
572573

573574
shard.activate do
574-
@default_storage_quota ||= Rails.cache.fetch(['default_storage_quota', id].cache_key) do
575+
@default_storage_quota ||= Rails.cache.fetch(['default_storage_quota', self.global_id].cache_key) do
575576
parent_account.default_storage_quota
576577
end
577578
end
@@ -620,7 +621,7 @@ def default_group_storage_quota
620621
return Group.default_storage_quota if root_account?
621622

622623
shard.activate do
623-
Rails.cache.fetch(['default_group_storage_quota', self.id].cache_key) do
624+
Rails.cache.fetch(['default_group_storage_quota', self.global_id].cache_key) do
624625
self.parent_account.default_group_storage_quota
625626
end
626627
end

app/models/account/settings.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ def self.included(klass)
3232
end
3333

3434
def cached_inherited_setting(setting)
35-
Rails.cache.fetch([setting, self.global_id].cache_key) do
36-
calculate_inherited_setting(setting)
35+
self.shard.activate do
36+
Rails.cache.fetch([setting, self.global_id].cache_key) do
37+
calculate_inherited_setting(setting)
38+
end
3739
end
3840
end
3941

spec/models/account_spec.rb

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1304,10 +1304,13 @@ def account_with_admin_and_restricted_user(account, restricted_role)
13041304
subaccount = account.sub_accounts.create!
13051305
expect(subaccount.default_storage_quota).to eq 10.megabytes
13061306

1307-
# should reload
13081307
account.default_storage_quota = 20.megabytes
13091308
account.save!
13101309

1310+
# should clear caches
1311+
account = Account.find(account.id)
1312+
expect(account.default_storage_quota).to eq 20.megabytes
1313+
13111314
subaccount = Account.find(subaccount)
13121315
expect(subaccount.default_storage_quota).to eq 20.megabytes
13131316
end
@@ -1358,6 +1361,27 @@ def account_with_admin_and_restricted_user(account, restricted_role)
13581361
@account.save!
13591362
expect(@account.restrict_student_future_view).to eq({:locked => false, :value => true})
13601363
end
1364+
1365+
context "caching" do
1366+
specs_require_sharding
1367+
it "should clear cached values correctly" do
1368+
enable_cache do
1369+
[@account, @sub1, @sub2].each(&:restrict_student_future_view) # preload the cached values
1370+
1371+
@sub1.settings = @sub1.settings.merge(:restrict_student_future_view => {:locked => true, :value => true})
1372+
@sub1.save!
1373+
1374+
# hard reload
1375+
@account = Account.find(@account.id)
1376+
@sub1 = Account.find(@sub1.id)
1377+
@sub2 = Account.find(@sub2.id)
1378+
1379+
expect(@account.restrict_student_future_view).to eq({:locked => false, :value => false})
1380+
expect(@sub1.restrict_student_future_view).to eq({:locked => true, :value => true})
1381+
expect(@sub2.restrict_student_future_view).to eq({:locked => true, :value => true, :inherited => true})
1382+
end
1383+
end
1384+
end
13611385
end
13621386

13631387
context "require terms of use" do

0 commit comments

Comments
 (0)