Merge branch '57867-accessing-issue-1-of-gitlab-ce-via-the-api-results-in-500-internal-server-error' into 'master'

Speed up find_by when used on finders

Closes #57867

See merge request gitlab-org/gitlab-ce!25421

2 files changed, 23 insertions, 3 deletions

diff --git a/app/finders/concerns/finder_methods.rb b/app/finders/concerns/finder_methods.rb
index 5290313585f..8de3276184d 100644
--- a/app/finders/concerns/finder_methods.rb
+++ b/app/finders/concerns/finder_methods.rb
@@ -3,13 +3,13 @@
 module FinderMethods
   # rubocop: disable CodeReuse/ActiveRecord
   def find_by!(*args)
-    raise_not_found_unless_authorized execute.find_by!(*args)
+    raise_not_found_unless_authorized execute.reorder(nil).find_by!(*args)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
   # rubocop: disable CodeReuse/ActiveRecord
   def find_by(*args)
-    if_authorized execute.find_by(*args)
+    if_authorized execute.reorder(nil).find_by(*args)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
diff --git a/spec/finders/concerns/finder_methods_spec.rb b/spec/finders/concerns/finder_methods_spec.rb
index a4ad331f613..e074e53c2c5 100644
--- a/spec/finders/concerns/finder_methods_spec.rb
+++ b/spec/finders/concerns/finder_methods_spec.rb
@@ -12,7 +12,7 @@ describe FinderMethods do
       end
 
       def execute
-        Project.all
+        Project.all.order(id: :desc)
       end
     end
   end
@@ -38,6 +38,16 @@ describe FinderMethods do
     it 'raises not found the user does not have access' do
       expect { finder.find_by!(id: unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
+
+    it 'ignores ordering' do
+      # Memoise the finder result so we can add message expectations to it
+      relation = finder.execute
+      allow(finder).to receive(:execute).and_return(relation)
+
+      expect(relation).to receive(:reorder).with(nil).and_call_original
+
+      finder.find_by!(id: authorized_project.id)
+    end
   end
 
   describe '#find' do
@@ -66,5 +76,15 @@ describe FinderMethods do
     it 'returns nil when the user does not have access' do
       expect(finder.find_by(id: unauthorized_project.id)).to be_nil
     end
+
+    it 'ignores ordering' do
+      # Memoise the finder result so we can add message expectations to it
+      relation = finder.execute
+      allow(finder).to receive(:execute).and_return(relation)
+
+      expect(relation).to receive(:reorder).with(nil).and_call_original
+
+      finder.find_by(id: authorized_project.id)
+    end
   end
 end