fix cross-shard api lookups of non-ID columns

fixes CNVS-35954

just use a two-stage query where we translate to an ID first, so that
we can keep the final output as a relation object

Change-Id: Ia5529e5382ceb8f1104e09f8d0a0c04fef0d9efa
Reviewed-on: https://gerrit.instructure.com/110403
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

lib/api.rb

@@ -216,6 +216,7 @@ module Api
     raise ArgumentError, "sis_root_account required for lookups" unless sis_root_account.is_a?(Account)
 
     return relation.none if columns.empty?
+    relation = relation.all unless relation.is_a?(ActiveRecord::Relation)
 
     not_scoped_to_account = sis_mapping[:is_not_scoped_to_account] || []
 
@@ -242,9 +243,26 @@ module Api
           else
             ids_hash = { sis_root_account => ids }
           end
-          ids_hash.each do |root_account, ids|
-            query << "(#{sis_mapping[:scope]} = #{root_account.id} AND #{column} IN (?))"
-            args << ids
+          Shard.partition_by_shard(ids_hash.keys) do |root_accounts_on_shard|
+            sub_query = []
+            sub_args = []
+            root_accounts_on_shard.each do |root_account|
+              ids = ids_hash[root_account]
+              sub_query << "(#{sis_mapping[:scope]} = #{root_account.id} AND #{column} IN (?))"
+              sub_args << ids
+            end
+            if Shard.current == relation.primary_shard
+              query.concat(sub_query)
+              args.concat(sub_args)
+            else
+              raise "cross-shard non-ID Api lookups are only supported for users" unless relation.klass == User
+              sub_args.unshift(sub_query.join(" OR "))
+              users = relation.klass.joins(sis_mapping[:joins]).where(*sub_args).select(:id, :updated_at).to_a
+              User.preload_shard_associations(users)
+              users.each { |u| u.associate_with_shard(relation.primary_shard, :shadow) }
+              query << "#{relation.table_name}.id IN (?)"
+              args << users
+            end
           end
         end
       end

spec/lib/api_spec.rb

@@ -268,6 +268,23 @@ describe Api do
 
         expect(@api.api_find_all(User, [@user2.id, @user3.id]).sort_by(&:global_id)).to eq [@user2, @user3].sort_by(&:global_id)
       end
+
+      it 'find users from other shards via SIS ID' do
+        @shard1.activate do
+          @account = Account.create(name: 'new')
+          @user = user_with_pseudonym username: "sis_user_1@example.com", account: @account
+        end
+        expect(Api).to receive(:sis_parse_id).
+          with("root_account:school:sis_login_id:sis_user_1@example.com", anything, anything, anything).
+          twice.
+          and_return(['LOWER(pseudonyms.unique_id)', [QuotedValue.new("LOWER('sis_user_1@example.com')"), @account]])
+        expect(@api.api_find(User, "root_account:school:sis_login_id:sis_user_1@example.com")).to eq @user
+        # works through an association, too
+        account2 = Account.create!
+        course = account2.courses.create!
+        course.enroll_student(@user)
+        expect(@api.api_find(course.students, "root_account:school:sis_login_id:sis_user_1@example.com")).to eq @user
+      end
     end
   end