Revert "Merge branch 'revert-12499' into 'master'"
This reverts merge request !12557
This commit is contained in:
parent
d453bb8635
commit
2c78c7f4cd
5 changed files with 52 additions and 4 deletions
|
@ -227,7 +227,7 @@ class Projects::IssuesController < Projects::ApplicationController
|
||||||
def issue
|
def issue
|
||||||
return @issue if defined?(@issue)
|
return @issue if defined?(@issue)
|
||||||
# The Sortable default scope causes performance issues when used with find_by
|
# The Sortable default scope causes performance issues when used with find_by
|
||||||
@noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
|
@noteable = @issue ||= @project.issues.find_by!(iid: params[:id])
|
||||||
|
|
||||||
return render_404 unless can?(current_user, :read_issue, @issue)
|
return render_404 unless can?(current_user, :read_issue, @issue)
|
||||||
|
|
||||||
|
|
|
@ -5,6 +5,25 @@
|
||||||
module Sortable
|
module Sortable
|
||||||
extend ActiveSupport::Concern
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
|
module DropDefaultScopeOnFinders
|
||||||
|
# Override these methods to drop the `ORDER BY id DESC` default scope.
|
||||||
|
# See http://dba.stackexchange.com/a/110919 for why we do this.
|
||||||
|
%i[find find_by find_by!].each do |meth|
|
||||||
|
define_method meth do |*args, &block|
|
||||||
|
return super(*args, &block) if block
|
||||||
|
|
||||||
|
unordered_relation = unscope(:order)
|
||||||
|
|
||||||
|
# We cannot simply call `meth` on `unscope(:order)`, since that is also
|
||||||
|
# an instance of the same relation class this module is included into,
|
||||||
|
# which means we'd get infinite recursion.
|
||||||
|
# We explicitly use the original implementation to prevent this.
|
||||||
|
original_impl = method(__method__).super_method.unbind
|
||||||
|
original_impl.bind(unordered_relation).call(*args)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
included do
|
included do
|
||||||
# By default all models should be ordered
|
# By default all models should be ordered
|
||||||
# by created_at field starting from newest
|
# by created_at field starting from newest
|
||||||
|
@ -18,6 +37,10 @@ module Sortable
|
||||||
scope :order_updated_asc, -> { reorder(updated_at: :asc) }
|
scope :order_updated_asc, -> { reorder(updated_at: :asc) }
|
||||||
scope :order_name_asc, -> { reorder(name: :asc) }
|
scope :order_name_asc, -> { reorder(name: :asc) }
|
||||||
scope :order_name_desc, -> { reorder(name: :desc) }
|
scope :order_name_desc, -> { reorder(name: :desc) }
|
||||||
|
|
||||||
|
# All queries (relations) on this model are instances of this `relation_klass`.
|
||||||
|
relation_klass = relation_delegate_class(ActiveRecord::Relation)
|
||||||
|
relation_klass.prepend DropDefaultScopeOnFinders
|
||||||
end
|
end
|
||||||
|
|
||||||
module ClassMethods
|
module ClassMethods
|
||||||
|
|
|
@ -815,7 +815,7 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def ci_service
|
def ci_service
|
||||||
@ci_service ||= ci_services.reorder(nil).find_by(active: true)
|
@ci_service ||= ci_services.find_by(active: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
def deployment_services
|
def deployment_services
|
||||||
|
@ -823,7 +823,7 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def deployment_service
|
def deployment_service
|
||||||
@deployment_service ||= deployment_services.reorder(nil).find_by(active: true)
|
@deployment_service ||= deployment_services.find_by(active: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
def monitoring_services
|
def monitoring_services
|
||||||
|
@ -831,7 +831,7 @@ class Project < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def monitoring_service
|
def monitoring_service
|
||||||
@monitoring_service ||= monitoring_services.reorder(nil).find_by(active: true)
|
@monitoring_service ||= monitoring_services.find_by(active: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
def jira_tracker?
|
def jira_tracker?
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Improve performance of lookups of issues, merge requests etc by dropping unnecessary ORDER BY clause
|
||||||
|
merge_request:
|
||||||
|
author:
|
21
spec/models/concerns/sortable_spec.rb
Normal file
21
spec/models/concerns/sortable_spec.rb
Normal file
|
@ -0,0 +1,21 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Sortable do
|
||||||
|
let(:relation) { Issue.all }
|
||||||
|
|
||||||
|
describe '#where' do
|
||||||
|
it 'orders by id, descending' do
|
||||||
|
order_node = relation.where(iid: 1).order_values.first
|
||||||
|
expect(order_node).to be_a(Arel::Nodes::Descending)
|
||||||
|
expect(order_node.expr.name).to eq(:id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#find_by' do
|
||||||
|
it 'does not order' do
|
||||||
|
expect(relation).to receive(:unscope).with(:order).and_call_original
|
||||||
|
|
||||||
|
relation.find_by(iid: 1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue