Added ETag-caching optimisations

app/controllers/browse_controller.rb

@@ -16,6 +16,7 @@ class BrowseController < ProjectAreaController
   verify :params => [:compare_with], :only => :diff
 
   before_filter :fetch_node
+  before_filter :check_freshness_of_node, :only => ['index']
   before_filter :verify_file_node, :only => ['download', 'diff']
 
   def index
@@ -48,6 +49,10 @@ def diff
   end
 
   protected
+
+    def check_freshness_of_node
+      fresh_when :etag => @node, :last_modified => @node.date
+    end
 
     def render_node
       case @node.content_type

app/controllers/changesets_controller.rb

@@ -12,8 +12,12 @@ class ChangesetsController < ProjectAreaController
   require_permissions :code, 
     :browse => ['diff']  
 
-  keep_params! :only => [:index], :exclude => [:project_id]
+  keep_params! :only => [:index], :exclude => [:project_id]  
+  before_filter :check_freshness_of_index, :only => [:index]  
 
+  before_filter :find_changeset, :only => [:show, :diff]
+  before_filter :check_freshness_of_changeset, :only => [:show, :diff]  
+
   def index
     @changesets = Project.current.changesets.paginate(
       :include => [:user],
@@ -30,9 +34,6 @@ def index
   end
 
   def show
-    @changeset = Project.current.changesets.find_by_revision! params[:id],
-      :include => [:changes, :user]
-
     @next_changeset = @changeset.next_by_project(Project.current)
     @previous_changeset = @changeset.previous_by_project(Project.current)
 
@@ -43,7 +44,6 @@ def show
   end
 
   def diff
-    @changeset = Project.current.changesets.find_by_revision! params[:id]
     @change = @changeset.changes.find(params[:change_id])
     unless @change.diffable?
       raise ActiveRecord::RecordNotFound, "Change #{@change.id} is not diffable." 
@@ -54,5 +54,20 @@ def diff
       format.text { render :text => @change.unified_diff }
     end    
   end
+
+  protected
+
+    def find_changeset
+      @changeset = Project.current.changesets.find_by_revision! params[:id],
+        :include => [:changes, :user]      
+    end
+
+    def check_freshness_of_index
+      fresh_when :etag => Project.current.changesets.count, :last_modified => Project.current.changesets.maximum(:created_at)     
+    end
+
+    def check_freshness_of_changeset
+      fresh_when :etag => @changeset, :last_modified => @changeset.created_at     
+    end
 
 end

app/controllers/milestones_controller.rb

@@ -9,6 +9,7 @@ class MilestonesController < ProjectAreaController
     :update => ['edit', 'update'], 
     :delete => ['destroy']
 
+  before_filter :check_freshness_of_index, :only => [:index]
   before_filter :find_milestone, :only => [:edit, :update, :destroy]
 
   def index
@@ -76,6 +77,10 @@ def destroy
   end
 
   private
+
+    def check_freshness_of_index
+      fresh_when :etag => Project.current.milestones.count, :last_modified => Project.current.milestones.maximum(:updated_at)
+    end
 
     def find_milestone
       @milestone = Project.current.milestones.find(params[:id])      

app/controllers/project_area_controller.rb

@@ -35,6 +35,11 @@ def module_accessible?(project, item)
   end
 
   protected
+
+    def fresh_when(options = {})
+      options[:etag] = [User.current, Project.current, flash] + Array(options[:etag])
+      super      
+    end    
 
     def find_project
       project = Project.find_by_short_name! params[:project_id]      

app/controllers/tickets_controller.rb

@@ -28,21 +28,24 @@ class TicketsController < ProjectAreaController
   require_user 'modify_summary', 'modify_content', 'modify_change_content'
 
   verify        :xhr => true, :only => [:modify_summary, :modify_content, :modify_change_content]
-
+
+  before_filter :check_freshness_of_index, :only => [:index]  
   before_filter :find_report, :only => [:index, :search]
   before_filter :setup_filters, :only => [:index, :search]
-
   before_filter :find_reports, :only => [:index]
+
   before_filter :find_ticket, :only => [:show, :update, :destroy, :toggle_subscription]
+  before_filter :check_freshness_of_ticket, :only => [:show]  
+
   before_filter :new_change, :only => [:show, :update]
   before_filter :new_ticket, :only => [:new, :create]
 
   before_filter :find_ticket_and_verify_permissions, :only => [:modify_summary, :modify_content]
   before_filter :find_change_and_verify_permissions, :only => [:modify_change_content]
 
   before_filter :find_and_verify_attachment, :only => :download
-    
-  def index
+
+  def index    
     @tickets = paginate_tickets(request.format.rss? ? 10 : params[:per_page], request.format.rss? ? 10 : nil)    
 
     respond_to do |format|
@@ -185,6 +188,14 @@ def users
   end
 
   protected
+
+    def check_freshness_of_index
+      fresh_when :etag => Project.current.tickets.count, :last_modified => Project.current.tickets.maximum(:updated_at)
+    end
+
+    def check_freshness_of_ticket
+      fresh_when :etag => @ticket, :last_modified => @ticket.updated_at
+    end
 
     def find_report
       @report = Project.current.ticket_reports.find_by_id params[:report]

app/models/repository/abstract/node.rb

@@ -18,6 +18,11 @@ def initialize(repos, path, selected_revision = nil)
     @selected_revision = selected_revision.to_s
   end
 
+  # Returns the cache key, which is used to generate the ETag
+  def cache_key
+    "#{User.current.id}:#{repos.id}:#{path}:#{revision}"
+  end
+
   # Returns the last actual revision for the current node
   # (the one where the node was modified for tha last time)
   def revision

app/views/layouts/_flash_messages.html.erb

@@ -1,13 +1,13 @@
 <% if flash[:error] -%>
   <div class="content error"><%= [flash[:error]].flatten.join('<br/>') %></div>
-  <% flash[:error] = nil -%>
+  <% flash.delete(:error) -%>
 <% end -%>
 <% if flash[:notice] -%>
   <div class="content notice"><%= [flash[:notice]].flatten.join('<br/>') %></div>
-  <% flash[:notice] = nil -%>
+  <% flash.delete(:notice) -%>
 <% end -%>
 <% if flash[:warning] -%>
   <div class="content warning"><%= [flash[:warning]].flatten.join('<br/>') %></div>
-  <% flash[:warning] = nil -%>
+  <% flash.delete(:warning) -%>
 <% end -%>
 

extensions/retro_blog/lib/blog_controller.rb

@@ -19,7 +19,9 @@ class BlogController < ProjectAreaController
     :update  => ['edit', 'update'],
     :delete  => ['destroy']
 
+  before_filter :check_freshness_of_index, :only => [:index]
   before_filter :find_blog_post, :only => [:show, :comment, :edit, :update, :destroy]
+  before_filter :check_freshness_of_post, :only => [:show]
   before_filter :load_categories, :only => [:index] 
 
   def index
@@ -101,6 +103,14 @@ def load_categories
       @categories = Project.current.blog_posts.categories
     end
 
+    def check_freshness_of_index
+      fresh_when :last_modified => Project.current.blog_posts.maximum(:updated_at)
+    end
+
+    def check_freshness_of_post
+      fresh_when :etag => @blog_post, :last_modified => @blog_post.updated_at 
+    end
+
   private
 
     def options_for_paginate

extensions/retro_blog/models/blog_comment.rb

@@ -14,4 +14,12 @@ def serialize_only
     [:id, :author, :content, :created_at]    
   end
 
+  def before_save
+    blog_post.touch
+  end
+
+  def before_destroy
+    blog_post.touch
+  end
+
 end

extensions/retro_blog/spec/controllers/blog_comments_controller_spec.rb

@@ -140,8 +140,8 @@ def do_put
   describe 'DELETE /destroy' do
 
     before do
-      @blog_comment = mock_model(BlogComment)
-      @comments_proxy.stub!(:destroy).and_return(true)
+      @blog_comment = mock_model(BlogComment, :destroy => true)
+      @comments_proxy.stub!(:find).and_return(@blog_comment)
     end
 
     def do_delete
@@ -155,7 +155,7 @@ def do_delete
     end
 
     it 'should delete the comment' do
-      @comments_proxy.should_receive(:destroy).with('1').and_return(true)
+      @blog_comment.should_receive(:destroy).with().and_return(true)
       do_delete
     end
 

extensions/retro_blog/spec/controllers/blog_controller_spec.rb

@@ -9,6 +9,7 @@
     @posts_proxy = @project.stub_association!(:blog_posts)
     @posts_proxy.stub!(:posted_by).and_return(@posts_proxy)
     @posts_proxy.stub!(:categorized_as).and_return(@posts_proxy)
+    @posts_proxy.stub!(:maximum)
 
     controller.stub!(:cached_user_attribute).with(:name, 'Anonymous').and_return('User Name')
     controller.stub!(:cached_user_attribute).with(:email).and_return('user@host.com')
@@ -18,7 +19,7 @@
 
     before do
       @posts = [mock_model(BlogPost), mock_model(BlogPost)]
-      @posts_proxy.stub!(:paginate).and_return(@posts) 
+      @posts_proxy.stub!(:paginate).and_return(@posts)
 
       @categories = ['News', 'Releases']
       @posts_proxy.stub!(:categories).and_return(@categories)
@@ -28,14 +29,20 @@ def do_get(options = {})
       get :index, options.merge(:project_id => @project.to_param)
     end
 
+    it 'should check the freshness' do
+      @posts_proxy.should_receive(:maximum).with(:updated_at)
+      do_get
+    end
+
     it 'should load the posts' do
       @posts_proxy.should_receive(:posted_by).with('1').and_return(@posts_proxy) 
       @posts_proxy.should_receive(:categorized_as).with('News').and_return(@posts_proxy) 
       @posts_proxy.should_receive(:paginate).with(
         :page => params[:page],
         :include => [:categories, :user, :comments],
         :per_page=>nil,
-        :order=>'blog_posts.created_at DESC'
+        :order=>'blog_posts.created_at DESC',
+        :total_entries=>nil
       ).and_return(@posts) 
       do_get(:u => '1', :c => 'News')
       assigns[:blog_posts].should == @posts 
@@ -66,7 +73,7 @@ def do_get(options = {})
   describe 'GET /show' do
 
     before do
-      @blog_post = mock_model(BlogPost)
+      @blog_post = mock_model(BlogPost, :updated_at => 10.minutes.ago)
       @comment = mock_model(BlogComment)
       @comments_proxy = @blog_post.stub_association!(:comments, :new => @comment)      
       @posts_proxy.stub!(:find).and_return(@blog_post)

extensions/retro_blog/spec/models/blog_comment_spec.rb

@@ -39,6 +39,16 @@
       @comment.should have(1).error_on(:content)
     end
 
+    it 'should touch the blog post on save' do
+      blog_comments(:release_negative).save.should be_true
+      blog_posts(:release).updated_at.should > 1.minute.ago
+    end
+
+    it 'should touch the blog post on destroy' do
+      blog_comments(:release_negative).destroy
+      blog_posts(:release).updated_at.should > 1.minute.ago
+    end
+
   end
 
 end

extensions/retro_wiki/lib/wiki_controller.rb

@@ -20,13 +20,17 @@ class WikiController < ProjectAreaController
     :delete => ['destroy'],
     :upload => ['upload']
 
+  before_filter :check_freshness_of_index, :only => :index
+  before_filter :paginate_pages, :only => [:index]
+
   before_filter :find_page_or_redirect, :only => :show
+  before_filter :find_version, :only => :show
+  before_filter :check_freshness_of_page, :only => :show
+
   before_filter :find_page!, :only => [:rename, :update_title, :destroy]
   before_filter :find_or_build_page, :only => [:edit, :update]
 
   def index
-    @pages = Project.current.wiki_pages.paginate options_for_paginate 
-
     respond_to do |format|
       format.html
       format.rss  { render_rss(WikiPage, @pages) }
@@ -35,17 +39,14 @@ def index
   end
 
   def show
-    version = find_version(params[:version])
-    @wiki_page = version if version
-
     respond_to do |format|
       format.html
       format.xml  { render :xml => @wiki_page.to_xml(:root => 'wiki_page') }
     end  
   end
 
   def edit
-    version = find_version(params[:version])
+    version = @wiki_page.find_version(params[:version])
     @wiki_page.content = version.content if version
     @wiki_page.author = cached_user_attribute(:name, 'Anonymous')
   end
@@ -95,6 +96,14 @@ def destroy
   end
 
   protected
+
+    def check_freshness_of_index
+      fresh_when :etag => Project.current.wiki_pages.count, :last_modified => Project.current.wiki_pages.maximum(:updated_at)
+    end
+
+    def paginate_pages
+      @pages = Project.current.wiki_pages.paginate options_for_paginate      
+    end
 
     def find_page_or_redirect
       @wiki_page = Project.current.wiki_pages.find_by_title params[:id], :include => [:versions]
@@ -106,6 +115,15 @@ def find_page_or_redirect
       end
     end
 
+    def find_version
+      version = @wiki_page.find_version(params[:version])
+      @wiki_page = version if version
+    end
+
+    def check_freshness_of_page
+      fresh_when :etag => @wiki_page, :last_modified => @wiki_page.updated_at
+    end
+
     def find_page!
       @wiki_page = Project.current.wiki_pages.find_by_title! params[:id]
     end
@@ -125,9 +143,4 @@ def pagination_order
       request.format.rss? || params[:order] == 'recent' ? 'wiki_pages.updated_at DESC' : 'wiki_pages.title'
     end
 
-  private
-
-    def find_version(number)
-      number.to_i > 0 ? @wiki_page.versions[number.to_i - 1] : nil
-    end
 end