Rewrites search engine to properly paginate results (#18631).

Instead of counting and retrieving results based on their timestamps, we now load all result ids then load the appropriate results by their ids. This also brings a 2x performance improvement as we search tokens in one of the 2 queries only.

git-svn-id: http://svn.redmine.org/redmine/trunk@13739 e93f8b46-1217-0410-a6f0-8f06a7374b81

8 files changed, 43 insertions, 50 deletions

diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb
index ec8b4812c..0abac94d3 100644
--- a/app/controllers/search_controller.rb
+++ b/app/controllers/search_controller.rb
@@ -36,9 +36,6 @@ class SearchController < ApplicationController
         @project
       end
 
-    offset = nil
-    begin; offset = params[:offset].to_time if params[:offset]; rescue; end
-
     # quick jump to an issue
     if (m = @question.match(/^#?(\d+)$/)) && (issue = Issue.visible.find_by_id(m[1].to_i))
       redirect_to issue_path(issue)
@@ -66,34 +63,39 @@ class SearchController < ApplicationController
       # no more than 5 tokens to search for
       @tokens.slice! 5..-1 if @tokens.size > 5
 
-      @results = []
-      @results_by_type = Hash.new {|h,k| h[k] = 0}
-
       limit = 10
-      @scope.each do |s|
-        r, c = s.singularize.camelcase.constantize.search(@tokens, projects_to_search,
+
+      @result_count = 0
+      @result_count_by_type = Hash.new {|h,k| h[k] = 0}
+      ranks_and_ids = []
+
+      # get all the results ranks and ids
+      @scope.each do |scope|
+        klass = scope.singularize.camelcase.constantize
+        ranks_and_ids_in_scope = klass.search_result_ranks_and_ids(@tokens, User.current, projects_to_search,
           :all_words => @all_words,
-          :titles_only => @titles_only,
-          :limit => (limit+1),
-          :offset => offset,
-          :before => params[:previous].nil?)
-        @results += r
-        @results_by_type[s] += c
+          :titles_only => @titles_only
+        )
+        @result_count_by_type[scope] += ranks_and_ids_in_scope.size
+        @result_count += ranks_and_ids_in_scope.size
+        ranks_and_ids += ranks_and_ids_in_scope.map {|r| [scope, r]}
       end
-      @results = @results.sort {|a,b| b.event_datetime <=> a.event_datetime}
-      if params[:previous].nil?
-        @pagination_previous_date = @results[0].event_datetime if offset && @results[0]
-        if @results.size > limit
-          @pagination_next_date = @results[limit-1].event_datetime
-          @results = @results[0, limit]
-        end
-      else
-        @pagination_next_date = @results[-1].event_datetime if offset && @results[-1]
-        if @results.size > limit
-          @pagination_previous_date = @results[-(limit)].event_datetime
-          @results = @results[-(limit), limit]
-        end
+      @result_pages = Paginator.new @result_count, limit, params['page']
+
+      # sort results, higher rank and id first
+      ranks_and_ids.sort! {|a,b| b.last <=> a.last }
+      ranks_and_ids = ranks_and_ids[@result_pages.offset, limit] || []
+
+      # load the results to display
+      results_by_scope = Hash.new {|h,k| h[k] = []}
+      ranks_and_ids.group_by(&:first).each do |scope, rs|
+        klass = scope.singularize.camelcase.constantize
+        results_by_scope[scope] += klass.search_results_from_ids(rs.map(&:last).map(&:last))
       end
+
+      @results = ranks_and_ids.map do |scope, r|
+        results_by_scope[scope].detect {|record| record.id == r.last}
+      end.compact
     else
       @question = ""
     end
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 4eedae145..78a69422f 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -35,9 +35,9 @@ class Changeset < ActiveRecord::Base
                 :url => Proc.new {|o| {:controller => 'repositories', :action => 'revision', :id => o.repository.project, :repository_id => o.repository.identifier_param, :rev => o.identifier}}
 
   acts_as_searchable :columns => 'comments',
-                     :scope => preload(:repository => :project),
+                     :preload => {:repository => :project},
                      :project_key => "#{Repository.table_name}.project_id",
-                     :date_column => "#{Changeset.table_name}.committed_on"
+                     :date_column => :committed_on
 
   acts_as_activity_provider :timestamp => "#{table_name}.committed_on",
                             :author_key => :user_id,
diff --git a/app/models/document.rb b/app/models/document.rb
index 70e3c9b01..810b15dfa 100644
--- a/app/models/document.rb
+++ b/app/models/document.rb
@@ -22,7 +22,7 @@ class Document < ActiveRecord::Base
   acts_as_attachable :delete_permission => :delete_documents
 
   acts_as_searchable :columns => ['title', "#{table_name}.description"],
-                     :scope => preload(:project)
+                     :preload => :project
   acts_as_event :title => Proc.new {|o| "#{l(:label_document)}: #{o.title}"},
                 :author => Proc.new {|o| o.attachments.reorder("#{Attachment.table_name}.created_on ASC").first.try(:author) },
                 :url => Proc.new {|o| {:controller => 'documents', :action => 'show', :id => o.id}}
diff --git a/app/models/issue.rb b/app/models/issue.rb
index a23d4bd9f..84a5d7b6c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -46,8 +46,7 @@ class Issue < ActiveRecord::Base
   acts_as_customizable
   acts_as_watchable
   acts_as_searchable :columns => ['subject', "#{table_name}.description", "#{Journal.table_name}.notes"],
-                     # sort by id so that limited eager loading doesn't break with postgresql
-                     :order_column => "#{table_name}.id",
+                     :preload => [:project, :status, :tracker],
                      :scope => lambda { joins(:project).
                                         joins("LEFT OUTER JOIN #{Journal.table_name} ON #{Journal.table_name}.journalized_type='Issue'" + 
                                               " AND #{Journal.table_name}.journalized_id = #{Issue.table_name}.id" +
diff --git a/app/models/message.rb b/app/models/message.rb
index dee5a5b62..5f21e88d4 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -25,9 +25,9 @@ class Message < ActiveRecord::Base
   attr_protected :id
 
   acts_as_searchable :columns => ['subject', 'content'],
-                     :scope => preload(:board => :project),
-                     :project_key => "#{Board.table_name}.project_id",
-                     :date_column => "#{table_name}.created_on"
+                     :preload => {:board => :project},
+                     :project_key => "#{Board.table_name}.project_id"
+
   acts_as_event :title => Proc.new {|o| "#{o.board.name}: #{o.subject}"},
                 :description => :content,
                 :group => :parent,
diff --git a/app/models/news.rb b/app/models/news.rb
index 6ade4f4f1..046bb8137 100644
--- a/app/models/news.rb
+++ b/app/models/news.rb
@@ -29,7 +29,7 @@ class News < ActiveRecord::Base
   acts_as_attachable :edit_permission => :manage_news,
                      :delete_permission => :manage_news
   acts_as_searchable :columns => ['title', 'summary', "#{table_name}.description"],
-                     :scope => preload(:project)
+                     :preload => :project
   acts_as_event :url => Proc.new {|o| {:controller => 'news', :action => 'show', :id => o.id}}
   acts_as_activity_provider :scope => preload(:project, :author),
                             :author_key => :author_id
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 61e78873d..8d81bce7e 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -33,7 +33,8 @@ class WikiPage < ActiveRecord::Base
                 :url => Proc.new {|o| {:controller => 'wiki', :action => 'show', :project_id => o.wiki.project, :id => o.title}}
 
   acts_as_searchable :columns => ['title', "#{WikiContent.table_name}.text"],
-                     :scope => preload(:wiki => :project).joins(:content, {:wiki => :project}),
+                     :scope => joins(:content, {:wiki => :project}),
+                     :preload => {:wiki => :project},
                      :permission => :view_wiki_pages,
                      :project_key => "#{Wiki.table_name}.project_id"
 
diff --git a/app/views/search/index.html.erb b/app/views/search/index.html.erb
index 9c884811a..6bbe8d482 100644
--- a/app/views/search/index.html.erb
+++ b/app/views/search/index.html.erb
@@ -23,9 +23,9 @@
 
 <% if @results %>
     <div id="search-results-counts">
-      <%= render_results_by_type(@results_by_type) unless @scope.size == 1 %>
+      <%= render_results_by_type(@result_count_by_type) unless @scope.size == 1 %>
     </div>
-    <h3><%= l(:label_result_plural) %> (<%= @results_by_type.values.sum %>)</h3>
+    <h3><%= l(:label_result_plural) %> (<%= @result_count %>)</h3>
     <dl id="search-results">
       <% @results.each do |e| %>
         <dt class="<%= e.event_type %>">
@@ -38,18 +38,9 @@
     </dl>
 <% end %>
 
-<p class="pagination">
-<% if @pagination_previous_date %>
-<%= link_to_content_update("\xc2\xab " + l(:label_previous),
-      params.merge(:previous => 1,
-                   :offset => @pagination_previous_date.strftime("%Y%m%d%H%M%S"))) %>&nbsp;
+<% if @result_pages %>
+<p class="pagination"><%= pagination_links_full @result_pages, @result_count, :per_page_links => false %></p>
 <% end %>
-<% if @pagination_next_date %>
-<%= link_to_content_update(l(:label_next) + " \xc2\xbb",
-      params.merge(:previous => nil,
-                   :offset => @pagination_next_date.strftime("%Y%m%d%H%M%S"))) %>
-<% end %>
-</p>
 
 <% html_title(l(:label_search)) -%>