From bb1fccb7b7202ec065fdc39646067054580d278f Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Tue, 15 May 2007 21:32:36 +0000
Subject: [PATCH] Fixed: performance issue on RepositoriesController#revisions
 when a changeset has a great number of changes (eg. 100,000). Also added
 pagination for changes on changeset details view.

git-svn-id: http://redmine.rubyforge.org/svn/trunk@535 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/repositories_controller.rb |  9 ++++++++-
 app/models/repository.rb                   |  4 +++-
 app/views/repositories/revision.rhtml      |  7 ++++---
 test/fixtures/changes.yml                  | 16 ++++++++++++++++
 test/unit/repository_test.rb               | 21 ++++++++++++++++-----
 5 files changed, 47 insertions(+), 10 deletions(-)
 create mode 100644 test/fixtures/changes.yml

diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb
index fdfe936f7..4252e58be 100644
--- a/app/controllers/repositories_controller.rb
+++ b/app/controllers/repositories_controller.rb
@@ -49,7 +49,7 @@ class RepositoriesController < ApplicationController
       show_error and return unless @entry
     end
     @repository.changesets_with_path @path do
-      @changeset_count = @repository.changesets.count
+      @changeset_count = @repository.changesets.count(:select => "DISTINCT #{Changeset.table_name}.id")
       @changeset_pages = Paginator.new self, @changeset_count,
   								      25,
   								      params['page']								
@@ -71,6 +71,13 @@ class RepositoriesController < ApplicationController
   def revision
     @changeset = @repository.changesets.find_by_revision(@rev)
     show_error and return unless @changeset
+    @changes_count = @changeset.changes.size
+    @changes_pages = Paginator.new self, @changes_count, 150, params['page']								
+    @changes = @changeset.changes.find(:all,
+  						:limit  =>  @changes_pages.items_per_page,
+  						:offset =>  @changes_pages.current.offset)
+  	
+  	render :action => "revision", :layout => false if request.xhr?	
   end
   
   def diff
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 02dfda6b7..692c446d6 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -40,7 +40,9 @@ class Repository < ActiveRecord::Base
     path = "/#{path}%"
     path = url.gsub(/^#{root_url}/, '') + path if root_url && root_url != url
     path.squeeze!("/")
-    Changeset.with_scope(:find => { :include => :changes, :conditions => ["#{Change.table_name}.path LIKE ?", path] }) do 
+    # Custom select and joins is done to allow conditions on changes table without loading associated Change objects
+    # Required for changesets with a great number of changes (eg. 100,000)
+    Changeset.with_scope(:find => { :select => "DISTINCT #{Changeset.table_name}.*", :joins => "LEFT OUTER JOIN #{Change.table_name} ON #{Change.table_name}.changeset_id = #{Changeset.table_name}.id", :conditions => ["#{Change.table_name}.path LIKE ?", path] }) do 
       yield
     end 
   end
diff --git a/app/views/repositories/revision.rhtml b/app/views/repositories/revision.rhtml
index 8919e45a2..5cf5c2e41 100644
--- a/app/views/repositories/revision.rhtml
+++ b/app/views/repositories/revision.rhtml
@@ -28,7 +28,7 @@
 <p><%= link_to(l(:label_view_diff), :action => 'diff', :id => @project, :path => "", :rev => @changeset.revision) if @changeset.changes.any? %></p>
 <table class="list">
 <tbody>
-<% @changeset.changes.each do |change| %>
+<% @changes.each do |change| %>
 <tr class="<%= cycle 'odd', 'even' %>">
 <td><div class="square action_<%= change.action %>"></div> <%= change.path %></td>
 <td align="right">
@@ -40,8 +40,9 @@
 <% end %>
 </tbody>
 </table>
-<p><%= lwr(:label_modification, @changeset.changes.length) %></p>
+<p><%= pagination_links_full @changes_pages, :rev => @changeset.revision %>
+[ <%= @changes_pages.current.first_item %> - <%= @changes_pages.current.last_item %> / <%= @changes_count %> ]</p>
 
 <% content_for :header_tags do %>
 <%= stylesheet_link_tag "scm" %>
-<% end %>
\ No newline at end of file
+<% end %>
diff --git a/test/fixtures/changes.yml b/test/fixtures/changes.yml
new file mode 100644
index 000000000..30acbd02d
--- /dev/null
+++ b/test/fixtures/changes.yml
@@ -0,0 +1,16 @@
+--- 
+changes_001: 
+  id: 1
+  changeset_id: 100
+  action: A
+  path: /test/some/path/in/the/repo
+  from_path:
+  from_revision:
+changes_002: 
+  id: 2
+  changeset_id: 100
+  action: A
+  path: /test/some/path/elsewhere/in/the/repo
+  from_path:
+  from_revision:
+  
\ No newline at end of file
diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb
index 5e0fb56fd..fd4fb6737 100644
--- a/test/unit/repository_test.rb
+++ b/test/unit/repository_test.rb
@@ -18,7 +18,11 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class RepositoryTest < Test::Unit::TestCase
-  fixtures :projects, :repositories, :issues, :issue_statuses, :changesets
+  fixtures :projects, :repositories, :issues, :issue_statuses, :changesets, :changes
+  
+  def setup
+    @repository = Project.find(1).repository
+  end
   
   def test_create
     repository = Repository.new(:project => Project.find(2))
@@ -33,10 +37,9 @@ class RepositoryTest < Test::Unit::TestCase
   end
 
   def test_cant_change_url
-    repository = Project.find(1).repository
-    url = repository.url
-    repository.url = "svn://anotherhost"
-    assert_equal  url, repository.url
+    url = @repository.url
+    @repository.url = "svn://anotherhost"
+    assert_equal  url, @repository.url
   end
   
   def test_scan_changesets_for_issue_ids
@@ -56,4 +59,12 @@ class RepositoryTest < Test::Unit::TestCase
     # ignoring commits referencing an issue of another project
     assert_equal [], Issue.find(4).changesets
   end
+  
+  def test_changesets_with_path
+    @repository.changesets_with_path '/some/path' do
+      assert_equal 1, @repository.changesets.count(:select => "DISTINCT #{Changeset.table_name}.id")
+      changesets = @repository.changesets.find(:all)
+      assert_equal 1, changesets.size
+    end
+  end
 end