exclude duplicate tools from find_external_tool

why:
* in a context where there may be hundreds or thousands of duplicate
tools, the find_external_tool operation can take enough time to timeout
a request and throw an error

* add a not_duplicate scope
* search unique tools first, and only search duplicates if no matching
tool was found, or if the match was 1.1 since 1.3 is preferred

closes INTEROP-7067
flag=none

test plan:
* in a rails console with a course `course`
* create a new ContextExternalTool with a name, url, domain, secret,
consumer_key, and some placements defined on it, and with `course` as
its context
* create a duplicate with `t2 = t.dup`
* set the first tool's identity_hash to "duplicate"
* `ContextExternalTool.find_external_tool(t.url, course)` should return
the second tool
* delete the second tool with `t.destroy`
* `ContextExternalTool.find_external_tool(t.url, course)` should return
the first tool with identity_hash "duplicate"

Change-Id: I944f3e0ecc5e9785a79eba6c4e2bde59a50cfd81
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282583
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>

app/models/context_external_tool.rb

@@ -1010,6 +1010,8 @@ class ContextExternalTool < ActiveRecord::Base
       order_clauses = [
         # prefer 1.3 tools
         sort_by_sql_string("developer_key_id IS NOT NULL"),
+        # prefer tools that are not duplicates
+        sort_by_sql_string("identity_hash != 'duplicate'"),
         # prefer tools from closer contexts
         "context_order.ordering",
         # prefer tools with more subdomains
@@ -1047,9 +1049,21 @@ class ContextExternalTool < ActiveRecord::Base
     "CASE WHEN #{condition} THEN 1 ELSE 2 END"
   end
 
-  # Given a collection of tools, finds the first tool that matches the given conditions
+  # Given a collection of tools, finds the first tool that matches the given conditions.
+  #
+  # First only loads non-duplicate tools into memory for matching, then will load
+  # all tools if necessary.
   def self.find_tool_match(tool_collection, matcher, matcher_condition)
-    tool_collection.to_a.find do |tool|
+    possible_match = tool_collection.not_duplicate.find do |tool|
+      matcher_condition.call(tool) && matcher.call(tool)
+    end
+
+    # an LTI 1.1 non-duplicate match means we still need to search
+    # all tools since a 1.3 match with 'duplicate' identity_hash
+    # still takes precedence
+    return possible_match if possible_match&.use_1_3?
+
+    tool_collection.find do |tool|
       matcher_condition.call(tool) && matcher.call(tool)
     end
   end
@@ -1136,6 +1150,10 @@ class ContextExternalTool < ActiveRecord::Base
     where.not(workflow_state: ["deleted", "disabled"])
   }
 
+  scope :not_duplicate, lambda {
+    where.not(identity_hash: "duplicate")
+  }
+
   def self.find_all_for(context, type)
     tools = []
     if !context.is_a?(Account) && context.respond_to?(:context_external_tools)

spec/models/context_external_tool_spec.rb

@@ -1162,6 +1162,69 @@ describe ContextExternalTool do
         expect(external_tool).to eq tool2
       end
     end
+
+    context "with duplicate tools" do
+      let(:url) { "http://example.com/launch" }
+      let(:tool) do
+        t = @course.context_external_tools.create!(name: "test", domain: "example.com", url: url, consumer_key: "12345", shared_secret: "secret")
+        t.global_navigation = {
+          url: "http://www.example.com",
+          text: "Example URL"
+        }
+        t.save!
+        t
+      end
+      let(:duplicate) do
+        t = tool.dup
+        t.save!
+        t
+      end
+
+      context "when original tool exists" do
+        it "finds original tool" do
+          tool
+          expect(ContextExternalTool.find_external_tool(url, @course)).to eq tool
+        end
+      end
+
+      context "when original tool is gone" do
+        before do
+          duplicate
+          tool.destroy
+        end
+
+        it "finds duplicate tool" do
+          expect(ContextExternalTool.find_external_tool(url, @course)).to eq duplicate
+        end
+      end
+
+      context "when non-duplicate tool was created later" do
+        before do
+          duplicate
+          tool.update_column :identity_hash, "duplicate"
+          # re-calculate the identity hash for the later tool
+          duplicate.update!(domain: "fake.com")
+          duplicate.update!(domain: "example.com")
+        end
+
+        it "finds tool with non-duplicate identity_hash" do
+          expect(ContextExternalTool.find_external_tool(url, @course)).to eq duplicate
+        end
+      end
+
+      context "when duplicate is 1.3" do
+        before do
+          duplicate.use_1_3 = true
+          duplicate.developer_key = DeveloperKey.create!
+          duplicate.save!
+          duplicate.update_column :identity_hash, "duplicate"
+        end
+
+        it "finds duplicate tool" do
+          expect(ContextExternalTool.find_external_tool(url, @course)).to eq duplicate
+        end
+      end
+    end
   end
 
   describe "find_and_order_tools" do
@@ -1327,6 +1390,7 @@ describe ContextExternalTool do
 
         lti1tool
         preferred_tool
+        dupe_tool
       end
 
       let(:account_tool) { external_tool_model(context: @course.account, opts: { name: "Account Tool" }) }
@@ -1338,6 +1402,11 @@ describe ContextExternalTool do
         t.save!
         t
       end
+      let(:dupe_tool) do
+        t = tool1.dup
+        t.save!
+        t
+      end
       let(:preferred_tool) do
         t = tool1.dup
         t.name = "preferred"
@@ -1352,8 +1421,9 @@ describe ContextExternalTool do
           tool1,
           tool3,
           account_tool,
+          dupe_tool,
           lti1tool,
-          tool2,
+          tool2
         ].map(&:id)
       end
     end