Commit 3de4c8d0 authored by James Lopez's avatar James Lopez

Merge branch...

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
parents 161851de 843f4b94
...@@ -3,13 +3,13 @@ ...@@ -3,13 +3,13 @@
module FinderMethods module FinderMethods
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_by!(*args) 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 end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_by(*args) def find_by(*args)
if_authorized execute.find_by(*args) if_authorized execute.reorder(nil).find_by(*args)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -12,7 +12,7 @@ describe FinderMethods do ...@@ -12,7 +12,7 @@ describe FinderMethods do
end end
def execute def execute
Project.all Project.all.order(id: :desc)
end end
end end
end end
...@@ -38,6 +38,16 @@ describe FinderMethods do ...@@ -38,6 +38,16 @@ describe FinderMethods do
it 'raises not found the user does not have access' 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) expect { finder.find_by!(id: unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound)
end 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
describe '#find' do describe '#find' do
...@@ -66,5 +76,15 @@ describe FinderMethods do ...@@ -66,5 +76,15 @@ describe FinderMethods do
it 'returns nil when the user does not have access' do it 'returns nil when the user does not have access' do
expect(finder.find_by(id: unauthorized_project.id)).to be_nil expect(finder.find_by(id: unauthorized_project.id)).to be_nil
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment