From 7290e906314da1dbcc66a4813a3f0cf61f2c0335 Mon Sep 17 00:00:00 2001 From: Jacques Distler Date: Mon, 25 Apr 2011 10:54:44 -0500 Subject: [PATCH] Efficiency Introduce page.rev_ids, and use that, wherever possible, instead of page.revisions. This avoids fetching the text of all the revisions. D'oh! --- app/controllers/cache_sweeping_helper.rb | 2 +- app/controllers/wiki_controller.rb | 16 ++++++++-------- app/helpers/application_helper.rb | 4 ++-- app/helpers/wiki_helper.rb | 14 +++++++------- app/models/page.rb | 8 +++++--- app/views/wiki/history.html.erb | 2 +- app/views/wiki/page.rhtml | 2 +- app/views/wiki/recently_revised.rhtml | 4 ++-- 8 files changed, 27 insertions(+), 25 deletions(-) diff --git a/app/controllers/cache_sweeping_helper.rb b/app/controllers/cache_sweeping_helper.rb index cb56c08b..74a0c065 100644 --- a/app/controllers/cache_sweeping_helper.rb +++ b/app/controllers/cache_sweeping_helper.rb @@ -26,7 +26,7 @@ module CacheSweepingHelper end def expire_cached_revisions(page) - page.revisions.count.times do |i| + page.rev_ids.count.times do |i| revno = i+1 expire_action :controller => 'wiki', :web => page.web.address, :action => 'revision', :id => page.name, :rev => revno diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 21c21410..94fdf8d3 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -69,7 +69,7 @@ class WikiController < ApplicationController def export_html export_pages_as_zip(html_ext) do |page| - renderer = PageRenderer.new(page.revisions.last) + renderer = PageRenderer.new(page.current_revision) rendered_page = <<-EOL @@ -262,7 +262,7 @@ EOL redirect_home end @link_mode ||= :show - @renderer = PageRenderer.new(@page.revisions.last) + @renderer = PageRenderer.new(@page.current_revision) # to template end @@ -276,7 +276,7 @@ EOL @page ||= wiki.read_page(@web_name, @page_name) @link_mode ||= :publish if @page - @renderer = PageRenderer.new(@page.revisions.last) + @renderer = PageRenderer.new(@page.current_revision) else real_page = WikiReference.page_that_redirects_for(@web, @page_name) if real_page @@ -348,7 +348,7 @@ EOL def show if @page begin - @renderer = PageRenderer.new(@page.revisions.last) + @renderer = PageRenderer.new(@page.current_revision) @show_diff = (params[:mode] == 'diff') render :action => 'page' # TODO this rescue should differentiate between errors due to rendering and errors in @@ -383,8 +383,8 @@ EOL if @page @revisions_by_day = Hash.new { |h, day| h[day] = [] } @revision_numbers = Hash.new { |h, id| h[id] = [] } - revision_number = @page.revisions.size - @page.revisions.reverse.each do |rev| + revision_number = @page.rev_ids.size + @page.rev_ids.reverse.each do |rev| day = Date.new(rev.revised_at.year, rev.revised_at.month, rev.revised_at.day) @revisions_by_day[day] << rev @revision_numbers[rev.id] = revision_number @@ -415,7 +415,7 @@ EOL def s5 if [:markdownMML, :markdownPNG, :markdown].include?(@web.markup) - my_rendered = PageRenderer.new(@page.revisions.last) + my_rendered = PageRenderer.new(@page.current_revision) @s5_content = my_rendered.display_s5 @s5_theme = my_rendered.s5_theme else @@ -506,7 +506,7 @@ EOL if params['rev'] @revision_number = params['rev'].to_i else - @revision_number = @page.revisions.size + @revision_number = @page.rev_ids.size end @revision = @page.revisions[@revision_number - 1] end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5eb146bd..1571913b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -55,7 +55,7 @@ require 'instiki_stringsupport' # Create a hyperlink to a particular revision of a Wiki page def link_to_revision(page, revision_number, text = nil, mode = nil, html_options = {}) - revision_number == page.revisions.size ? + revision_number == page.rev_ids.size ? link_to( text || page.plain_name, {:web => @web.address, :action => 'show', :id => page.name, @@ -109,7 +109,7 @@ require 'instiki_stringsupport' end def rendered_content(page) - PageRenderer.new(page.revisions.last).display_content + PageRenderer.new(page.current_revision).display_content end def truncate(text, *args) diff --git a/app/helpers/wiki_helper.rb b/app/helpers/wiki_helper.rb index 2f2f9ea4..dbbf03b6 100644 --- a/app/helpers/wiki_helper.rb +++ b/app/helpers/wiki_helper.rb @@ -6,7 +6,7 @@ module WikiHelper menu << back_for_revision if @revision_number > 1 menu << current_revision menu << see_or_hide_changes_for_revision if @revision_number > 1 - menu << history if @page.revisions.size > 1 + menu << history if @page.rev_ids.size > 1 menu << rollback menu end @@ -15,11 +15,11 @@ module WikiHelper menu = [] menu << edit_page menu << edit_web if @page.name == "HomePage" - if @page.revisions.size > 1 + if @page.rev_ids.size > 1 menu << back_for_page menu << see_or_hide_changes_for_page end - menu << history if @page.revisions.size > 1 + menu << history if @page.rev_ids.size > 1 menu end @@ -40,11 +40,11 @@ module WikiHelper end def forward - if @revision_number < @page.revisions.size - 1 + if @revision_number < @page.rev_ids.size - 1 link_to('Forward in time', {:web => @web.address, :action => 'revision', :id => @page.name, :rev => @revision_number + 1}, {:class => 'navlink', :accesskey => 'F', :id => 'to_next_revision', :rel => 'nofollow'}) + - " (#{@revision.page.revisions.size - @revision_number} more) ".html_safe + " (#{@revision.page.rev_ids.size - @revision_number} more) ".html_safe else link_to('Forward in time', {:web => @web.address, :action => 'show', :id => @page.name}, {:class => 'navlink', :accesskey => 'F', :id => 'to_next_revision', :rel => 'nofollow'}) + @@ -62,9 +62,9 @@ module WikiHelper def back_for_page link_to('Back in time', {:web => @web.address, :action => 'revision', :id => @page.name, - :rev => @page.revisions.size - 1}, + :rev => @page.rev_ids.size - 1}, {:class => 'navlink', :accesskey => 'B', :id => 'to_previous_revision', :rel => 'nofollow'}) + - " (#{@page.revisions.size - 1} #{@page.revisions.size - 1 == 1 ? 'revision' : 'revisions'})".html_safe + " (#{@page.rev_ids.size - 1} #{@page.rev_ids.size - 1 == 1 ? 'revision' : 'revisions'})".html_safe end def current_revision diff --git a/app/models/page.rb b/app/models/page.rb index 9b291dfb..4b920679 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -1,6 +1,8 @@ class Page < ActiveRecord::Base belongs_to :web has_many :revisions, :order => 'id', :dependent => :destroy + #In many cases, we don't need to instantiate the full revisions (with all that textual data) + has_many :rev_ids, :order => 'id', :class_name => 'Revision', :select => 'id, revised_at, page_id, author, ip' has_many :wiki_references, :order => 'referenced_name' has_one :current_revision, :class_name => 'Revision', :order => 'id DESC' @@ -9,7 +11,7 @@ class Page < ActiveRecord::Base end def revise(content, name, time, author, renderer) - revisions_size = new_record? ? 0 : revisions.size + revisions_size = new_record? ? 0 : rev_ids.size if (revisions_size > 0) and content == current_revision.content and name == self.name raise Instiki::ValidationError.new( "You have tried to save page '#{name}' without changing its content") @@ -46,11 +48,11 @@ class Page < ActiveRecord::Base end def revisions? - revisions.size > 1 + rev_ids.size > 1 end def previous_revision(revision) - revision_index = revisions.each_with_index do |rev, index| + revision_index = rev_ids.each_with_index do |rev, index| if rev.id == revision.id break index else diff --git a/app/views/wiki/history.html.erb b/app/views/wiki/history.html.erb index 92831f88..1e841987 100644 --- a/app/views/wiki/history.html.erb +++ b/app/views/wiki/history.html.erb @@ -7,7 +7,7 @@ <%- for rev in @revisions_by_day[day] -%>
  • <%= link_to_revision(rev.page, @revision_numbers[rev.id], - text= (rev.page.revisions.size == @revision_numbers[rev.id] ? + text= (rev.page.rev_ids.size == @revision_numbers[rev.id] ? "Current" : "Revision #{@revision_numbers[rev.id]}" ) ) %> diff --git a/app/views/wiki/page.rhtml b/app/views/wiki/page.rhtml index aae7dc64..b144deac 100644 --- a/app/views/wiki/page.rhtml +++ b/app/views/wiki/page.rhtml @@ -7,7 +7,7 @@
    <%- if @show_diff -%>

    - Showing changes from revision #<%= @page.revisions.size - 1 %> to #<%= @page.revisions.size %>: + Showing changes from revision #<%= @page.rev_ids.size - 1 %> to #<%= @page.rev_ids.size %>: Added | Removed | Changed

    <%= @renderer.display_diff %> diff --git a/app/views/wiki/recently_revised.rhtml b/app/views/wiki/recently_revised.rhtml index 9c604000..a11e077a 100644 --- a/app/views/wiki/recently_revised.rhtml +++ b/app/views/wiki/recently_revised.rhtml @@ -8,9 +8,9 @@ <%- for page in @pages_by_day[day] -%>
  • <%= link_to_existing_page page %> - <%- if page.revisions.size > 1 %> + <%- if page.rev_ids.size > 1 %> - ( <%= link_to_revision(page, page.revisions.size, text='diff', + ( <%= link_to_revision(page, page.rev_ids.size, text='diff', mode='diff') %> | <%= link_to_history(page, text='history') %> ) <%- end -%>