ActionItem1185: fixing blog cache

* now is created one cache to user who can edit and other to other people * added touch plugin to update articles * added migrate to add column updated_at to profiles * added migrate to remove column lock_version from articles

ActionItem1185: fixing blog cache
* now is created one cache to user who can edit and other to other people * added touch plugin to update articles * added migrate to add column updated_at to profiles * added migrate to remove column lock_version from articles
Daniela Feitosa · Antonio Terceiro
1 parent 9628a907
Showing 8 changed files with 70 additions and 21 deletions Show diff stats
app/models/article.rb
app/sweepers/article_sweeper.rb
app/views/content_viewer/view_page.rhtml
db/migrate/070_add_updated_at_to_profiles.rb
db/migrate/071_remove_lock_version.rb
test/unit/article_test.rb
test/unit/blog_test.rb
vendor/plugins/touch/init.rb
@@ -214,6 +214,13 @@ class Article &lt; ActiveRecord::Base
     end
   end
+  def allow_post_content?(logged_person = nil)
+    if logged_person && logged_person.has_permission?('post_content', profile)
+      return true
+    end
+    false
+  end
+
   def comments_updated
     ferret_update
   end
@@ -266,8 +273,10 @@ class Article &lt; ActiveRecord::Base
       profile
   end
-  def cache_key(params = {})
-    "article-id-#{id}" +
+  alias :active_record_cache_key :cache_key
+  def cache_key(params = {}, the_profile = nil)
+    active_record_cache_key +
+      (allow_post_content?(the_profile) ? "-owner" : '') +
       (params[:npage] ? "-npage-#{params[:npage]}" : '') +
       (params[:year] ? "-year-#{params[:year]}" : '') +
       (params[:month] ? "-month-#{params[:month]}" : '')
@@ -13,7 +13,11 @@ class ArticleSweeper &lt; ActiveRecord::Observer
 protected
   def expire_caches(article)
-    article.hierarchy.each {|a| expire_fragment(a.cache_key) }
+    article.hierarchy.each do |a|
+      if a != article
+        a.touch
+      end
+    end
     blocks = (article.profile.blocks + article.profile.environment.blocks).select{|b|[RecentDocumentsBlock, BlogArchivesBlock].any?{|c| b.kind_of?(c)}}
     blocks.map(&:cache_keys).each{|ck|expire_timeout_fragment(ck)}
     env = article.profile.environment
@@ -23,7 +23,7 @@
 <div>
   <%= article_title(@page, :no_link => true) %>
-  <% if logged_in? && current_user.person.has_permission?('post_content', profile) %>
+  <% if @page.allow_post_content?(user) %>
     <div id="article-actions">
       <% unless @page.blog? %>
       <%= link_to content_tag( 'span', label_for_edit_article(@page) ),
@@ -80,7 +80,7 @@
   </div>
 <% end %>
-<% cache(@page.cache_key(params)) do %>
+<% cache(@page.cache_key(params, user)) do %>
   <div class="<%="article-body article-body-" + @page.css_class_name %>">
     <%= article_to_html(@page) %>
     <br style="clear:both" />
@@ -0,0 +1,10 @@
+class AddUpdatedAtToProfiles < ActiveRecord::Migration
+  def self.up
+    add_column :profiles, :updated_at, :datetime
+    execute 'update profiles set updated_at = created_at'
+  end
+
+  def self.down
+    remove_column :profiles, :updated_at
+  end
+end
@@ -0,0 +1,9 @@
+class RemoveLockVersion < ActiveRecord::Migration
+  def self.up
+    remove_column :articles, :lock_version
+  end
+
+  def self.down
+    add_column  :articles, :lock_version, :integer, :default => 0
+  end
+end
@@ -705,15 +705,13 @@ class ArticleTest &lt; Test::Unit::TestCase
   end
   should 'use npage to compose cache key' do
-    a = Article.new
-    a.expects(:id).returns(34)
-    assert_equal 'article-id-34-npage-2', a.cache_key(:npage => 2)
+    a = Article.create!(:name => 'Published at', :profile => profile)
+    assert_match(/-npage-2/,a.cache_key(:npage => 2))
   end
   should 'use year and month to compose cache key' do
-    a = Article.new
-    a.expects(:id).returns(34)
-    assert_equal 'article-id-34-year-2009-month-04', a.cache_key(:year => '2009', :month => '04')
+    a = Article.create!(:name => 'Published at', :profile => profile)
+    assert_match(/-year-2009-month-04/, a.cache_key(:year => '2009', :month => '04'))
   end
   should 'not be highlighted by default' do
@@ -749,4 +747,20 @@ class ArticleTest &lt; Test::Unit::TestCase
     assert_equal [c], a.categories
   end
+  should 'add owner on cache_key when has profile' do
+    a = profile.articles.create!(:name => 'a test article')
+    assert_match(/-owner/, a.cache_key({}, profile))
+  end
+
+  should 'not add owner on cache_key when has no profile' do
+    a = profile.articles.create!(:name => 'a test article')
+    assert_no_match(/-owner/, a.cache_key({}))
+  end
+
+  should 'add owner on cache_key when profile is community' do
+    c = Community.create!(:name => 'new_comm')
+    a = c.articles.create!(:name => 'a test article')
+    assert_match(/-owner/, a.cache_key({}, c))
+  end
+
 end
@@ -130,16 +130,6 @@ class BlogTest &lt; ActiveSupport::TestCase
     assert ! blog.valid?
   end
-  should 'expires cache when update post' do
-    p = create_user('testuser').person
-    blog = Blog.create!(:name => 'Blog test', :profile => p)
-    post = Article.create!(:name => 'First post', :profile => p, :parent => blog)
-    ArticleSweeper.any_instance.expects(:expire_fragment).with(blog.cache_key)
-    ArticleSweeper.any_instance.expects(:expire_fragment).with(post.cache_key)
-    post.name = 'Edited First post'
-    assert post.save!
-  end
-
   should 'remove external feed when removing blog' do
     p = create_user('testuser').person
     blog = Blog.create!(:name => 'Blog test', :profile => p, :external_feed_builder => {:enabled => true, :address => "http://bli.org/feed"})
@@ -0,0 +1,13 @@
+if ActiveRecord::Base.instance_methods.include?("touch") && Class.const_defined?('TOUCH_LOADED')
+  puts "W: ActiveRecord already provides a touch method, which means you must be using rails 2.3.3 or later."
+  puts "W: In this case the touch plugin could probably be removed"
+end
+TOUCH_LOADED = true
+
+module Touch
+  def touch
+    update_attribute(:updated_at, Time.now)
+  end
+end
+
+ActiveRecord::Base.send(:include, Touch)
	@@ -23,7 +23,7 @@		@@ -23,7 +23,7 @@
23		23
24	<div>	24	<div>
25	<%= article_title(@page, :no_link => true) %>	25	<%= article_title(@page, :no_link => true) %>
26	- <% if logged_in? && current_user.person.has_permission?('post_content', profile) %>	26	+ <% if @page.allow_post_content?(user) %>
27	<div id="article-actions">	27	<div id="article-actions">
28	<% unless @page.blog? %>	28	<% unless @page.blog? %>
29	<%= link_to content_tag( 'span', label_for_edit_article(@page) ),	29	<%= link_to content_tag( 'span', label_for_edit_article(@page) ),
	@@ -80,7 +80,7 @@		@@ -80,7 +80,7 @@
80	</div>	80	</div>
81	<% end %>	81	<% end %>
82		82
83	-<% cache(@page.cache_key(params)) do %>	83	+<% cache(@page.cache_key(params, user)) do %>
84	<div class="<%="article-body article-body-" + @page.css_class_name %>">	84	<div class="<%="article-body article-body-" + @page.css_class_name %>">
85	<%= article_to_html(@page) %>	85	<%= article_to_html(@page) %>
86	<br style="clear:both" />	86	<br style="clear:both" />
@@ -0,0 +1,10 @@		@@ -0,0 +1,10 @@
	1	+class AddUpdatedAtToProfiles < ActiveRecord::Migration
	2	+ def self.up
	3	+ add_column :profiles, :updated_at, :datetime
	4	+ execute 'update profiles set updated_at = created_at'
	5	+ end
	6	+
	7	+ def self.down
	8	+ remove_column :profiles, :updated_at
	9	+ end
	10	+end
@@ -0,0 +1,9 @@		@@ -0,0 +1,9 @@
	1	+class RemoveLockVersion < ActiveRecord::Migration
	2	+ def self.up
	3	+ remove_column :articles, :lock_version
	4	+ end
	5	+
	6	+ def self.down
	7	+ add_column :articles, :lock_version, :integer, :default => 0
	8	+ end
	9	+end
	@@ -705,15 +705,13 @@ class ArticleTest < Test::Unit::TestCase		@@ -705,15 +705,13 @@ class ArticleTest < Test::Unit::TestCase
705	end	705	end
706		706
707	should 'use npage to compose cache key' do	707	should 'use npage to compose cache key' do
708	- a = Article.new
709	- a.expects(:id).returns(34)
710	- assert_equal 'article-id-34-npage-2', a.cache_key(:npage => 2)	708	+ a = Article.create!(:name => 'Published at', :profile => profile)
		709	+ assert_match(/-npage-2/,a.cache_key(:npage => 2))
711	end	710	end
712		711
713	should 'use year and month to compose cache key' do	712	should 'use year and month to compose cache key' do
714	- a = Article.new
715	- a.expects(:id).returns(34)
716	- assert_equal 'article-id-34-year-2009-month-04', a.cache_key(:year => '2009', :month => '04')	713	+ a = Article.create!(:name => 'Published at', :profile => profile)
		714	+ assert_match(/-year-2009-month-04/, a.cache_key(:year => '2009', :month => '04'))
717	end	715	end
718		716
719	should 'not be highlighted by default' do	717	should 'not be highlighted by default' do
	@@ -749,4 +747,20 @@ class ArticleTest < Test::Unit::TestCase		@@ -749,4 +747,20 @@ class ArticleTest < Test::Unit::TestCase
749	assert_equal [c], a.categories	747	assert_equal [c], a.categories
750	end	748	end
751		749
		750	+ should 'add owner on cache_key when has profile' do
		751	+ a = profile.articles.create!(:name => 'a test article')
		752	+ assert_match(/-owner/, a.cache_key({}, profile))
		753	+ end
		754	+
		755	+ should 'not add owner on cache_key when has no profile' do
		756	+ a = profile.articles.create!(:name => 'a test article')
		757	+ assert_no_match(/-owner/, a.cache_key({}))
		758	+ end
		759	+
		760	+ should 'add owner on cache_key when profile is community' do
		761	+ c = Community.create!(:name => 'new_comm')
		762	+ a = c.articles.create!(:name => 'a test article')
		763	+ assert_match(/-owner/, a.cache_key({}, c))
		764	+ end
		765	+
752	end	766	end
	@@ -130,16 +130,6 @@ class BlogTest < ActiveSupport::TestCase		@@ -130,16 +130,6 @@ class BlogTest < ActiveSupport::TestCase
130	assert ! blog.valid?	130	assert ! blog.valid?
131	end	131	end
132		132
133	- should 'expires cache when update post' do
134	- p = create_user('testuser').person
135	- blog = Blog.create!(:name => 'Blog test', :profile => p)
136	- post = Article.create!(:name => 'First post', :profile => p, :parent => blog)
137	- ArticleSweeper.any_instance.expects(:expire_fragment).with(blog.cache_key)
138	- ArticleSweeper.any_instance.expects(:expire_fragment).with(post.cache_key)
139	- post.name = 'Edited First post'
140	- assert post.save!
141	- end
142	-
143	should 'remove external feed when removing blog' do	133	should 'remove external feed when removing blog' do
144	p = create_user('testuser').person	134	p = create_user('testuser').person
145	blog = Blog.create!(:name => 'Blog test', :profile => p, :external_feed_builder => {:enabled => true, :address => "http://bli.org/feed"})	135	blog = Blog.create!(:name => 'Blog test', :profile => p, :external_feed_builder => {:enabled => true, :address => "http://bli.org/feed"})
@@ -0,0 +1,13 @@		@@ -0,0 +1,13 @@
	1	+if ActiveRecord::Base.instance_methods.include?("touch") && Class.const_defined?('TOUCH_LOADED')
	2	+ puts "W: ActiveRecord already provides a touch method, which means you must be using rails 2.3.3 or later."
	3	+ puts "W: In this case the touch plugin could probably be removed"
	4	+end
	5	+TOUCH_LOADED = true
	6	+
	7	+module Touch
	8	+ def touch
	9	+ update_attribute(:updated_at, Time.now)
	10	+ end
	11	+end
	12	+
	13	+ActiveRecord::Base.send(:include, Touch)