Skip to content

Commit

Permalink
simplify logic around if we should check the user's home shard for a …
Browse files Browse the repository at this point in the history
…pseudonym first

fixes FOO-987

Change-Id: I46fdfabcc639d77b4cbad2fae50a0fd52667d30d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249478
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
  • Loading branch information
ccutrer committed Oct 7, 2020
1 parent 282abfe commit 3ddae01
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
8 changes: 5 additions & 3 deletions app/models/sis_pseudonym.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ def find_in_other_accounts

trusted_account_ids = root_account.trusted_account_ids.group_by { |id| Shard.shard_for(id) } if type == :trusted

# try the user's home shard first
unless @in_region && (!user.shard.in_current_region? || !user.shard.default?)
# try the user's home shard first if it's fast
# the default shard has a replica in every region, so is always fast
user_shard_is_in_region = user.shard.in_current_region? || user.shard.default?
if user_shard_is_in_region
if type != :trusted || (account_ids = trusted_account_ids[user.shard])
user.shard.activate do
result = find_in_trusted_accounts(account_ids)
Expand All @@ -92,7 +94,7 @@ def find_in_other_accounts
return nil if shards.empty?

Shard.with_each_shard(shards.sort) do
next if Shard.current == user.shard
next if Shard.current == user.shard && user_shard_is_in_region
account_ids = trusted_account_ids[Shard.current] if type == :trusted
result = find_in_trusted_accounts(account_ids)
return result if result
Expand Down
13 changes: 13 additions & 0 deletions spec/models/sis_pseudonym_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,19 @@ def pseud_params(unique_id, account = Account.default)
expect(SisPseudonym.for(@user, Account.default)).to eq @pseudonym
end
end

it "looks in other accounts" do
@shard1.activate do
@s1root = account_model
@user = User.create!
@pseudonym = @s1root.pseudonyms.create!(user: @user, unique_id: 'user') do |p|
p.sis_user_id = 'abc'
end
allow_any_instantiation_of(@pseudonym).to receive(:works_for_account?).with(Account.default, true).and_return(true)
end
expect(SisPseudonym.for(@user, Account.default, type: :implicit)).to eq @pseudonym
expect(SisPseudonym.for(@user, Account.default, type: :implicit, in_region: true)).to eq @pseudonym
end
end

end

0 comments on commit 3ddae01

Please sign in to comment.