Commit 221acbe37eed5716bc9be773fb71843ca49aa6f2
Committed by
Antonio Terceiro
1 parent
856560cc
Exists in
master
and in
29 other branches
ActionItem955: cache crash blog pagination and year/month filter
Showing
8 changed files
with
33 additions
and
12 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
| @@ -14,10 +14,6 @@ class ContentViewerController < ApplicationController | @@ -14,10 +14,6 @@ class ContentViewerController < ApplicationController | ||
| 14 | return | 14 | return |
| 15 | end | 15 | end |
| 16 | else | 16 | else |
| 17 | - path.gsub!(/\/(\d{4})\/(\d{2})\Z/, '') | ||
| 18 | - year = $1 | ||
| 19 | - month = $2 | ||
| 20 | - | ||
| 21 | @page = profile.articles.find_by_path(path) | 17 | @page = profile.articles.find_by_path(path) |
| 22 | unless @page | 18 | unless @page |
| 23 | page_from_old_path = profile.articles.find_by_old_path(path) | 19 | page_from_old_path = profile.articles.find_by_old_path(path) |
| @@ -79,7 +75,7 @@ class ContentViewerController < ApplicationController | @@ -79,7 +75,7 @@ class ContentViewerController < ApplicationController | ||
| 79 | end | 75 | end |
| 80 | 76 | ||
| 81 | if @page.blog? | 77 | if @page.blog? |
| 82 | - @page.filter = {:year => year, :month => month} | 78 | + @page.filter = {:year => params[:year], :month => params[:month]} |
| 83 | end | 79 | end |
| 84 | 80 | ||
| 85 | if @page.folder? && @page.view_as == 'image_gallery' | 81 | if @page.folder? && @page.view_as == 'image_gallery' |
app/models/article.rb
| @@ -258,8 +258,11 @@ class Article < ActiveRecord::Base | @@ -258,8 +258,11 @@ class Article < ActiveRecord::Base | ||
| 258 | profile | 258 | profile |
| 259 | end | 259 | end |
| 260 | 260 | ||
| 261 | - def cache_key | ||
| 262 | - "article-id-#{id}" | 261 | + def cache_key(params = {}) |
| 262 | + "article-id-#{id}" + | ||
| 263 | + (params[:npage] ? "-npage-#{params[:npage]}" : '') + | ||
| 264 | + (params[:year] ? "-year-#{params[:year]}" : '') + | ||
| 265 | + (params[:month] ? "-month-#{params[:month]}" : '') | ||
| 263 | end | 266 | end |
| 264 | 267 | ||
| 265 | private | 268 | private |
app/models/blog_archives_block.rb
| @@ -22,7 +22,7 @@ class BlogArchivesBlock < Block | @@ -22,7 +22,7 @@ class BlogArchivesBlock < Block | ||
| 22 | results << content_tag('li', content_tag('strong', "#{year} (#{results_by_year.size})")) | 22 | results << content_tag('li', content_tag('strong', "#{year} (#{results_by_year.size})")) |
| 23 | results << "<ul class='#{year}-archive'>" | 23 | results << "<ul class='#{year}-archive'>" |
| 24 | results_by_year.group_by{|i| [ ('%02d' % i.published_at.month()), gettext(MONTHS[i.published_at.month() - 1])]}.sort.each do |month, results_by_month| | 24 | results_by_year.group_by{|i| [ ('%02d' % i.published_at.month()), gettext(MONTHS[i.published_at.month() - 1])]}.sort.each do |month, results_by_month| |
| 25 | - results << content_tag('li', link_to("#{month[1]} (#{results_by_month.size})", owner.generate_url(:controller => 'content_viewer', :action => 'view_page', :page => [owner.blog.path, year, month[0]]))) | 25 | + results << content_tag('li', link_to("#{month[1]} (#{results_by_month.size})", owner.generate_url(:controller => 'content_viewer', :action => 'view_page', :page => [owner.blog.path], :year => year, :month => month[0]))) |
| 26 | end | 26 | end |
| 27 | results << "</ul>" | 27 | results << "</ul>" |
| 28 | end | 28 | end |
app/views/content_viewer/view_page.rhtml
| @@ -83,7 +83,7 @@ | @@ -83,7 +83,7 @@ | ||
| 83 | </div> | 83 | </div> |
| 84 | <% end %> | 84 | <% end %> |
| 85 | 85 | ||
| 86 | -<% cache(@page.cache_key) do %> | 86 | +<% cache(@page.cache_key(params)) do %> |
| 87 | <div class="<%="article-body article-body-" + @page.css_class_name %>"> | 87 | <div class="<%="article-body article-body-" + @page.css_class_name %>"> |
| 88 | <%= article_to_html(@page) %> | 88 | <%= article_to_html(@page) %> |
| 89 | <br style="clear:both" /> | 89 | <br style="clear:both" /> |
test/functional/content_viewer_controller_test.rb
| @@ -638,10 +638,10 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -638,10 +638,10 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
| 638 | assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{blog.path}?npage=2", :rel => 'next' } | 638 | assert_tag :tag => 'a', :attributes => { :href => "/#{profile.identifier}/#{blog.path}?npage=2", :rel => 'next' } |
| 639 | end | 639 | end |
| 640 | 640 | ||
| 641 | - should 'extract year and month from path' do | 641 | + should 'set year and month filter from URL params' do |
| 642 | profile.articles << Blog.new(:name => 'A blog test', :profile => profile) | 642 | profile.articles << Blog.new(:name => 'A blog test', :profile => profile) |
| 643 | year, month = profile.blog.created_at.year.to_s, '%02d' % profile.blog.created_at.month | 643 | year, month = profile.blog.created_at.year.to_s, '%02d' % profile.blog.created_at.month |
| 644 | - get :view_page, :profile => profile.identifier, :page => [profile.blog.path, year, month] | 644 | + get :view_page, :profile => profile.identifier, :page => [profile.blog.path], :year => year, :month => month |
| 645 | assert_equal({ :year => year.to_s, :month => month.to_s }, assigns(:page).filter) | 645 | assert_equal({ :year => year.to_s, :month => month.to_s }, assigns(:page).filter) |
| 646 | end | 646 | end |
| 647 | 647 |
test/unit/article_test.rb
| @@ -695,4 +695,16 @@ class ArticleTest < Test::Unit::TestCase | @@ -695,4 +695,16 @@ class ArticleTest < Test::Unit::TestCase | ||
| 695 | assert_equal a.created_at, a.published_at | 695 | assert_equal a.created_at, a.published_at |
| 696 | end | 696 | end |
| 697 | 697 | ||
| 698 | + should 'use npage to compose cache key' do | ||
| 699 | + a = Article.new | ||
| 700 | + a.expects(:id).returns(34) | ||
| 701 | + assert_equal 'article-id-34-npage-2', a.cache_key(:npage => 2) | ||
| 702 | + end | ||
| 703 | + | ||
| 704 | + should 'use year and month to compose cache key' do | ||
| 705 | + a = Article.new | ||
| 706 | + a.expects(:id).returns(34) | ||
| 707 | + assert_equal 'article-id-34-year-2009-month-04', a.cache_key(:year => '2009', :month => '04') | ||
| 708 | + end | ||
| 709 | + | ||
| 698 | end | 710 | end |
test/unit/blog_archives_block_test.rb
| @@ -38,7 +38,7 @@ class BlogArchivesBlockTest < ActiveSupport::TestCase | @@ -38,7 +38,7 @@ class BlogArchivesBlockTest < ActiveSupport::TestCase | ||
| 38 | end | 38 | end |
| 39 | block = BlogArchivesBlock.new | 39 | block = BlogArchivesBlock.new |
| 40 | block.stubs(:owner).returns(profile) | 40 | block.stubs(:owner).returns(profile) |
| 41 | - assert_tag_in_string block.content, :tag => 'a', :content => 'January (10)', :attributes => {:href => /2008\/01/} | 41 | + assert_tag_in_string block.content, :tag => 'a', :content => 'January (10)', :attributes => {:href => 'http://colivre.net/flatline/blog?month=01&year=2008' } |
| 42 | end | 42 | end |
| 43 | 43 | ||
| 44 | should 'order list of amount posts' do | 44 | should 'order list of amount posts' do |
test/unit/blog_test.rb
| @@ -126,4 +126,14 @@ class BlogTest < ActiveSupport::TestCase | @@ -126,4 +126,14 @@ class BlogTest < ActiveSupport::TestCase | ||
| 126 | assert ! blog.valid? | 126 | assert ! blog.valid? |
| 127 | end | 127 | end |
| 128 | 128 | ||
| 129 | + should 'expires cache when update post' do | ||
| 130 | + p = create_user('testuser').person | ||
| 131 | + blog = Blog.create!(:name => 'Blog test', :profile => p) | ||
| 132 | + post = Article.create!(:name => 'First post', :profile => p, :parent => blog) | ||
| 133 | + ArticleSweeper.any_instance.expects(:expire_fragment).with(/#{blog.cache_key}/) | ||
| 134 | + ArticleSweeper.any_instance.expects(:expire_fragment).with(/#{post.cache_key}/) | ||
| 135 | + post.name = 'Edited First post' | ||
| 136 | + assert post.save! | ||
| 137 | + end | ||
| 138 | + | ||
| 129 | end | 139 | end |