Skip to content

Commit 496b93e

Browse files
committed
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 64ef3be commit 496b93e

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
@@ -542,7 +542,7 @@ def invalidate_caches_if_changed
542542
if @invalidations.present?
543543
shard.activate do
544544
@invalidations.each do |key|
545-
Rails.cache.delete([key, id].cache_key)
545+
Rails.cache.delete([key, self.global_id].cache_key)
546546
end
547547
Account.send_later_if_production(:invalidate_inherited_caches, self, @invalidations)
548548
end
@@ -553,8 +553,9 @@ def self.invalidate_inherited_caches(parent_account, keys)
553553
parent_account.shard.activate do
554554
account_ids = Account.sub_account_ids_recursive(parent_account.id)
555555
account_ids.each do |id|
556+
global_id = Shard.global_id_for(id)
556557
keys.each do |key|
557-
Rails.cache.delete([key, id].cache_key)
558+
Rails.cache.delete([key, global_id].cache_key)
558559
end
559560
end
560561
end
@@ -569,7 +570,7 @@ def quota
569570
return self.class.default_storage_quota if root_account?
570571

571572
shard.activate do
572-
Rails.cache.fetch(['current_quota', id].cache_key) do
573+
Rails.cache.fetch(['current_quota', self.global_id].cache_key) do
573574
self.parent_account.default_storage_quota
574575
end
575576
end
@@ -580,7 +581,7 @@ def default_storage_quota
580581
return self.class.default_storage_quota if root_account?
581582

582583
shard.activate do
583-
@default_storage_quota ||= Rails.cache.fetch(['default_storage_quota', id].cache_key) do
584+
@default_storage_quota ||= Rails.cache.fetch(['default_storage_quota', self.global_id].cache_key) do
584585
parent_account.default_storage_quota
585586
end
586587
end
@@ -629,7 +630,7 @@ def default_group_storage_quota
629630
return Group.default_storage_quota if root_account?
630631

631632
shard.activate do
632-
Rails.cache.fetch(['default_group_storage_quota', self.id].cache_key) do
633+
Rails.cache.fetch(['default_group_storage_quota', self.global_id].cache_key) do
633634
self.parent_account.default_group_storage_quota
634635
end
635636
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)