From 8170384d808eb0712aedb32a1833e2cecc39ff1d Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@mcgivern.me.uk>
Date: Fri, 14 Jul 2017 11:38:06 +0000
Subject: Revert "Merge branch 'revert-2c879643' into 'master'"

This reverts merge request !12633
---
 app/controllers/projects/issues_controller.rb      |  2 +-
 app/models/concerns/sortable.rb                    | 23 ----------------------
 app/models/project.rb                              |  6 +++---
 .../dm-drop-default-scope-on-sortable-finders.yml  |  4 ----
 spec/models/concerns/sortable_spec.rb              | 21 --------------------
 5 files changed, 4 insertions(+), 52 deletions(-)
 delete mode 100644 changelogs/unreleased/dm-drop-default-scope-on-sortable-finders.yml
 delete mode 100644 spec/models/concerns/sortable_spec.rb

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index c9e636fb65e..13f03e7e63e 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 e50818e3dff..c0a1aadc76d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -845,7 +845,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
@@ -853,7 +853,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
@@ -861,7 +861,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
-- 
cgit v1.2.1