summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdam Niedzielski <adamsunday@gmail.com>2017-06-29 16:14:12 +0200
committerAdam Niedzielski <adamsunday@gmail.com>2017-06-29 16:14:12 +0200
commita3d7983b18cc81607c255f64b678da0ab7b9477d (patch)
tree273b1109d9e3b1061c2251ebcd567e7aca0d9405
parentda6a3fb0c24e2af59d6fbd3421446b8f7666cdf6 (diff)
downloadgitlab-ce-a3d7983b18cc81607c255f64b678da0ab7b9477d.tar.gz
Revert "Merge branch 'dm-drop-default-scope-on-sortable-finders' into 'master'"
This reverts commit b07c00032b038f40796a28e34b6dd4c622bad012, reversing changes made to 2b97d76d0b08a778710410df910a7601f6b8e9e7.
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/models/concerns/sortable.rb23
-rw-r--r--app/models/project.rb6
-rw-r--r--changelogs/unreleased/dm-drop-default-scope-on-sortable-finders.yml4
-rw-r--r--spec/models/concerns/sortable_spec.rb21
5 files changed, 4 insertions, 52 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 5b2de93c168..dfc6baa34a4 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -227,7 +227,7 @@ class Projects::IssuesController < Projects::ApplicationController
def issue
return @issue if defined?(@issue)
# The Sortable default scope causes performance issues when used with find_by
- @noteable = @issue ||= @project.issues.find_by!(iid: params[:id])
+ @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take!
return render_404 unless can?(current_user, :read_issue, @issue)
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
index fdacfa5a194..a155a064032 100644
--- a/app/models/concerns/sortable.rb
+++ b/app/models/concerns/sortable.rb
@@ -5,25 +5,6 @@
module Sortable
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
# By default all models should be ordered
# by created_at field starting from newest
@@ -37,10 +18,6 @@ module Sortable
scope :order_updated_asc, -> { reorder(updated_at: :asc) }
scope :order_name_asc, -> { reorder(name: :asc) }
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
module ClassMethods
diff --git a/app/models/project.rb b/app/models/project.rb
index 228f66b95cd..a75c5209955 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -815,7 +815,7 @@ class Project < ActiveRecord::Base
end
def ci_service
- @ci_service ||= ci_services.find_by(active: true)
+ @ci_service ||= ci_services.reorder(nil).find_by(active: true)
end
def deployment_services
@@ -823,7 +823,7 @@ class Project < ActiveRecord::Base
end
def deployment_service
- @deployment_service ||= deployment_services.find_by(active: true)
+ @deployment_service ||= deployment_services.reorder(nil).find_by(active: true)
end
def monitoring_services
@@ -831,7 +831,7 @@ class Project < ActiveRecord::Base
end
def monitoring_service
- @monitoring_service ||= monitoring_services.find_by(active: true)
+ @monitoring_service ||= monitoring_services.reorder(nil).find_by(active: true)
end
def jira_tracker?
diff --git a/changelogs/unreleased/dm-drop-default-scope-on-sortable-finders.yml b/changelogs/unreleased/dm-drop-default-scope-on-sortable-finders.yml
deleted file mode 100644
index b359a25053a..00000000000
--- a/changelogs/unreleased/dm-drop-default-scope-on-sortable-finders.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Improve performance of lookups of issues, merge requests etc by dropping unnecessary ORDER BY clause
-merge_request:
-author:
diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb
deleted file mode 100644
index d1e17c4f684..00000000000
--- a/spec/models/concerns/sortable_spec.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-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