From cc03e0b7741e556e34788faff8d6a0f16874e164 Mon Sep 17 00:00:00 2001
From: dinedine <didier.lafforgue@gmail.com>
Date: Fri, 9 Jul 2010 12:38:50 +0200
Subject: [PATCH] new cache strategy + remember_me set to true by default + fix
 bugs (one with mongo_tree)

---
 app/helpers/admin/pages_helper.rb      | 13 ++++-----
 app/models/extensions/page/tree.rb     |  8 +-----
 app/models/page.rb                     | 10 +++----
 app/views/admin/pages/_form.html.haml  |  2 +-
 app/views/admin/sessions/new.html.haml |  2 ++
 config/locales/admin_ui_en.yml         | 10 +++----
 config/locales/admin_ui_fr.yml         |  7 ++---
 config/locales/default_fr.yml          |  2 +-
 doc/TODO                               | 11 +++++---
 lib/core_ext.rb                        | 30 +++++++++++++++++++++
 lib/locomotive/httparty/webservice.rb  |  3 ++-
 lib/locomotive/render.rb               | 14 +++++++---
 spec/lib/core_ext_spec.rb              | 37 ++++++++++++++++++++++++++
 spec/lib/locomotive/render_spec.rb     | 24 +++++++++++------
 spec/models/page_spec.rb               |  5 ++++
 spec/support/locomotive.rb             | 16 ++++-------
 16 files changed, 140 insertions(+), 54 deletions(-)
 create mode 100644 spec/lib/core_ext_spec.rb

diff --git a/app/helpers/admin/pages_helper.rb b/app/helpers/admin/pages_helper.rb
index 79b28d84..2335b512 100644
--- a/app/helpers/admin/pages_helper.rb
+++ b/app/helpers/admin/pages_helper.rb
@@ -20,13 +20,14 @@ module Admin::PagesHelper
     list
   end
   
-  def options_for_page_cache_expiration
+  def options_for_page_cache_strategy
     [
-      [t('.expiration.never'), 0],
-      [t('.expiration.hour'), 1.hour],
-      [t('.expiration.day'), 1.day],
-      [t('.expiration.week'), 1.week],
-      [t('.expiration.month'), 1.month]
+      [t('.cache_strategy.none'), 'none'],  
+      [t('.cache_strategy.simple'), 'simple'],
+      [t('.cache_strategy.hour'), 1.hour.to_s],
+      [t('.cache_strategy.day'), 1.day.to_s],
+      [t('.cache_strategy.week'), 1.week.to_s],
+      [t('.cache_strategy.month'), 1.month.to_s]
     ]
   end
   
diff --git a/app/models/extensions/page/tree.rb b/app/models/extensions/page/tree.rb
index 5aade096..60e90a29 100644
--- a/app/models/extensions/page/tree.rb
+++ b/app/models/extensions/page/tree.rb
@@ -23,7 +23,6 @@ module Models
           before_destroy :remove_from_list
 
           # Fixme (Didier L.): Instances methods are defined before the include itself
-          alias :ancestors :hacked_ancestors
           alias :fix_position :hacked_fix_position
         end 
         
@@ -42,12 +41,7 @@ module Models
             self.fix_position(false)
             self.instance_variable_set :@_will_move, true
           end
-          
-          def hacked_ancestors
-            return [] if root?
-            self.class.find(self.path.clone << nil) # bug in mongoid (it does not handle array with one element) 
-          end          
-          
+                    
           protected
           
           def change_parent
diff --git a/app/models/page.rb b/app/models/page.rb
index 3d7f9df4..267dd1f6 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -12,7 +12,7 @@ class Page
   field :slug
   field :fullpath
   field :published, :type => Boolean, :default => false
-  field :cache_expires_in, :type => Integer, :default => 0
+  field :cache_strategy, :default => 'none'
   
   ## associations ##
   belongs_to_related :site
@@ -35,7 +35,6 @@ class Page
   named_scope :not_found, :where => { :slug => '404', :depth => 0, :published => true }
   
   ## behaviours ##
-  # liquid_methods :title, :fullpath
   liquify_template :joined_parts
   
   ## methods ##
@@ -66,6 +65,10 @@ class Page
     "http://#{self.site.domains.first}/#{self.fullpath}.html"
   end
   
+  def with_cache?
+    self.cache_strategy != 'none'
+  end
+  
   def to_liquid(options = {})
     Locomotive::Liquid::Drops::Page.new(self)
   end
@@ -73,9 +76,6 @@ class Page
   protected
   
   def do_not_remove_index_and_404_pages
-    # safe_site = self.site rescue nil
-    
-    # return if safe_site.nil?
     return if (self.site rescue nil).nil?
     
     if self.index? || self.not_found?
diff --git a/app/views/admin/pages/_form.html.haml b/app/views/admin/pages/_form.html.haml
index 283fb752..b1026410 100644
--- a/app/views/admin/pages/_form.html.haml
+++ b/app/views/admin/pages/_form.html.haml
@@ -16,7 +16,7 @@
   = f.custom_input :published, :css => 'toggle' do
     = f.check_box :published
     
-  = f.input :cache_expires_in, :as => :select, :collection => options_for_page_cache_expiration, :include_blank => false
+  = f.input :cache_strategy, :as => :select, :collection => options_for_page_cache_strategy, :include_blank => false
   
 #page-parts
   .nav
diff --git a/app/views/admin/sessions/new.html.haml b/app/views/admin/sessions/new.html.haml
index b3d8df62..eb768406 100644
--- a/app/views/admin/sessions/new.html.haml
+++ b/app/views/admin/sessions/new.html.haml
@@ -1,6 +1,8 @@
 - title t('.title')
 
 = semantic_form_for(resource, :as => resource_name, :url => session_path(resource_name)) do |f|
+  = f.hidden_field :remember_me, :value => "true"
+
   .inner
     = login_flash_message
 
diff --git a/config/locales/admin_ui_en.yml b/config/locales/admin_ui_en.yml
index 48105192..a2b3a73b 100644
--- a/config/locales/admin_ui_en.yml
+++ b/config/locales/admin_ui_en.yml
@@ -25,8 +25,7 @@ en:
         site: Site
         theme_assets: Theme files
       footer:
-        developed_by: Developed by
-        powered_by: and Powered by
+        who_is_behind: "Service developed by {{development}} and designed by <a href=\"http://www.sachagreif.com\">Sacha Greif</a>"
       form_actions:
         back: Back without saving
         create: Create
@@ -97,8 +96,9 @@ en:
         failed_create: "Page was not created."
         failed_update: "Page was not updated."
       form:
-        expiration:
-          never: Never
+        cache_strategy:
+          none: None
+          simple: Simple
           hour: 1 hour
           day: 1 day
           week: 1 week
@@ -331,7 +331,7 @@ en:
     hints:
       page:
         published: "Only authenticated accounts can view unpublished pages."
-        cache_expires_in: "Cache the page for better performance. Pressing shift-reload in the browser will regenerate the page."
+        cache_strategy: "Cache the page for better performance. The 'Simple' choice is a good compromise."
       snippet:
         slug: "You need to know it in order to insert the snippet inside a page or a layout"
       site:
diff --git a/config/locales/admin_ui_fr.yml b/config/locales/admin_ui_fr.yml
index f411312c..76157403 100644
--- a/config/locales/admin_ui_fr.yml
+++ b/config/locales/admin_ui_fr.yml
@@ -118,8 +118,9 @@ fr:
         failed_create: "La page n'a pas été créée."
         failed_update: "La page n'a pas été mise à jour."
       form:
-        expiration:
-          never: Jamais
+        cache_strategy:
+          none: Aucun
+          simple: Simple
           hour: 1 heure
           day: 1 jour
           week: 1 semaine
@@ -352,7 +353,7 @@ fr:
     hints:
       page:
         published: "Seuls les administrateurs authentifiés peuvent voir une page non publiée."
-        cache_expires_in: "Cache la page pour de meilleure performance. Presser la touche SHIFT et le bouton \"Rafraichir\" dans le navigateur rechargera la page."
+        cache_strategy: "Cache la page pour de meilleure performance. L'option 'Simple' est le meilleur compromis."
       snippet:
         slug: "Utilisé pour insérer le snippet dans une page ou un gabarit."
       site:
diff --git a/config/locales/default_fr.yml b/config/locales/default_fr.yml
index 3bc9947f..36486c2e 100644
--- a/config/locales/default_fr.yml
+++ b/config/locales/default_fr.yml
@@ -57,7 +57,7 @@ fr:
         parent: Parent
         slug: Raccourci
         published: Publiée
-        cache_expires_in: Cache expire dans
+        cache_strategy: Cache
       content_type:
         name: Nom
         description: Description
diff --git a/doc/TODO b/doc/TODO
index eba2d891..93c4c8dd 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -3,7 +3,11 @@ BOARD:
 - refactoring admin crud (pages + layouts + snippets)
 - refactor slugify method (use parameterize + create a module)
 
-- change action icons according to the right action [Sacha] 
+- change action icons according to the right action [Sacha]
+- page redirection (option)
+- send email when new content added thru api
+- sitemap
+- save layout / snippet / page / stylesheet / javascript with CMD + S (ajax)
     
 BACKLOG:
 
@@ -12,7 +16,6 @@ BACKLOG:
 - new custom field types:
   - belongs_to => association
 - cucumber features for admin pages
-- sitemap
 - validation for custom fields
 
 BUGS:
@@ -53,4 +56,6 @@ x localize application in French
   x admin
 x change credits in the admin footer
 x license
-x textile filter
\ No newline at end of file
+x textile filter
+x [bug] varnish can not be refreshed in heroku so "max-age" has to be disabled => modify cache strategy
+x "remember me" should always be enabled
\ No newline at end of file
diff --git a/lib/core_ext.rb b/lib/core_ext.rb
index 22dc5ed9..61cfc247 100644
--- a/lib/core_ext.rb
+++ b/lib/core_ext.rb
@@ -29,4 +29,34 @@ class String
     replace(self.slugify(options))
   end
   
+end
+
+## Hash
+
+class Hash
+  
+  def underscore_keys
+    new_hash = {}
+    
+    self.each_pair do |key, value| 
+      if value.respond_to?(:collect!) # Array
+        value.collect do |item|
+          if item.respond_to?(:each_pair) # Hash item within
+            item.underscore_keys
+          else
+            item
+          end
+        end
+      elsif value.respond_to?(:each_pair) # Hash
+        value = value.underscore_keys
+      end
+      
+      new_key = key.is_a?(String) ? key.underscore : key # only String keys
+      
+      new_hash[new_key] = value
+    end
+    
+    self.replace(new_hash)
+  end
+  
 end
\ No newline at end of file
diff --git a/lib/locomotive/httparty/webservice.rb b/lib/locomotive/httparty/webservice.rb
index c05b9760..03901298 100644
--- a/lib/locomotive/httparty/webservice.rb
+++ b/lib/locomotive/httparty/webservice.rb
@@ -17,8 +17,9 @@ module Locomotive
 
         puts "[WebService] consuming #{path}, #{options.inspect}"
 
-        self.get(path, options)
+        self.get(path, options).try(:underscore_keys)
       end
+      
     end
   end
 end
\ No newline at end of file
diff --git a/lib/locomotive/render.rb b/lib/locomotive/render.rb
index 3bb75bc5..d82bbe0f 100644
--- a/lib/locomotive/render.rb
+++ b/lib/locomotive/render.rb
@@ -14,7 +14,7 @@ module Locomotive
         
         output = @page.render(locomotive_context)
         
-        prepare_and_set_response(output, @page.cache_expires_in || 0)
+        prepare_and_set_response(output)
       end
             
       def locomotive_page
@@ -48,9 +48,17 @@ module Locomotive
         ::Liquid::Context.new(assigns, registers)
       end
       
-      def prepare_and_set_response(output, cache_expiration = 0)
-        response.headers['Cache-Control'] = "public, max-age=#{cache_expiration}" if cache_expiration > 0
+      def prepare_and_set_response(output)
         response.headers['Content-Type'] = 'text/html; charset=utf-8'
+
+        if @page.with_cache?
+          fresh_when :etag => @page, :last_modified => @page.updated_at.utc, :public => true
+          
+          if @page.cache_strategy != 'simple' # varnish
+            response.cache_control[:max_age] = @page.cache_strategy
+          end
+        end
+          
         render :text => output, :layout => false, :status => :ok
       end      
       
diff --git a/spec/lib/core_ext_spec.rb b/spec/lib/core_ext_spec.rb
new file mode 100644
index 00000000..cd3eec0f
--- /dev/null
+++ b/spec/lib/core_ext_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+ 
+describe 'Core extensions' do
+  
+  describe 'Adding new methods for Hash items' do
+   
+    describe 'defining underscore_keys' do
+    
+      context 'from simple plain hash' do
+          
+        it 'underscores each key' do
+          { 'foo-bar' => 42, :foo => 42, 'foo' => 42 }.underscore_keys.should == { 'foo_bar' => 42, :foo => 42, 'foo' => 42 }
+        end
+      
+      end
+      
+      context 'from nested hashes' do
+
+        it 'underscores each key' do
+          { 'foo-bar' => { 'bar-foo' => 42, :test => { 'bar-foo' => 42 } } }.underscore_keys.should == { 'foo_bar' => { 'bar_foo' => 42, :test => { 'bar_foo' => 42 } } }
+        end
+        
+      end
+
+      context 'from hash with arrays of hashes' do
+        
+        it 'underscores each key' do
+          { 'foo-bar' => [{ 'bar-foo' => 42 }, 42, { 'bar-foo' => 42 }] }.underscore_keys.should == { 'foo_bar' => [{ 'bar_foo' => 42 }, 42, { 'bar_foo' => 42 }] }
+        end
+        
+      end
+            
+    end
+    
+  end
+  
+end
\ No newline at end of file
diff --git a/spec/lib/locomotive/render_spec.rb b/spec/lib/locomotive/render_spec.rb
index dd824154..e620d65b 100644
--- a/spec/lib/locomotive/render_spec.rb
+++ b/spec/lib/locomotive/render_spec.rb
@@ -8,11 +8,13 @@ describe 'Locomotive rendering system' do
     @site = Factory.build(:site)
     Site.stubs(:find).returns(@site)
     @controller.current_site = @site
+    @page = Factory.build(:page, :site => nil, :published => true)
   end
 
   context 'setting the response' do
     
     before(:each) do
+      @controller.instance_variable_set(:@page, @page)
       @controller.send(:prepare_and_set_response, 'Hello world !')
     end
     
@@ -27,21 +29,27 @@ describe 'Locomotive rendering system' do
     it 'does not set the cache' do
       @controller.response.headers['Cache-Control'].should be_nil
     end
+        
+    it 'sets the cache by simply using etag' do
+      @page.cache_strategy = 'simple'
+      @page.stubs(:updated_at).returns(Time.now)
+      @controller.send(:prepare_and_set_response, 'Hello world !')
+      @controller.response.to_a # force to build headers
+      @controller.response.headers['Cache-Control'].should == 'public'
+    end
     
-    it 'sets the cache' do
-      @controller.send(:prepare_and_set_response, 'Hello world !', 3600)
-      @controller.response.headers['Cache-Control'].should == 'public, max-age=3600'
+    it 'sets the cache for Varnish' do
+      @page.cache_strategy = '3600'
+      @page.stubs(:updated_at).returns(Time.now)
+      @controller.send(:prepare_and_set_response, 'Hello world !')
+      @controller.response.to_a # force to build headers
+      @controller.response.headers['Cache-Control'].should == 'max-age=3600, public'
     end
   
   end
   
   context 'when retrieving page' do
     
-    before(:each) do
-      @page = Factory.build(:page, :site => nil, :published => true)
-      @controller
-    end
-    
     it 'should retrieve the index page /' do
       @controller.request.fullpath = '/'
       @controller.current_site.pages.expects(:where).with({ :fullpath => 'index' }).returns([@page])
diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb
index 6b426a10..49989feb 100644
--- a/spec/models/page_spec.rb
+++ b/spec/models/page_spec.rb
@@ -74,6 +74,11 @@ describe Page do
       page.slug.should == 'Valid_ite'
     end
     
+    it 'has no cache strategy' do
+      page = Factory.build(:page, :site => nil)
+      page.with_cache?.should == false
+    end
+    
   end
   
   describe 'delete' do
diff --git a/spec/support/locomotive.rb b/spec/support/locomotive.rb
index 0ded950a..34fe3c8c 100644
--- a/spec/support/locomotive.rb
+++ b/spec/support/locomotive.rb
@@ -4,7 +4,7 @@ Locomotive.configure do |config|
 end
 
 module Locomotive
-  class TestController
+  class TestController < ApplicationController
    
     include Locomotive::Render
    
@@ -15,26 +15,20 @@ module Locomotive
     end
     
     def response
-      @response ||= TestResponse.new
+      @_response ||= TestResponse.new
     end
     
     def request
-      @request ||= TestRequest.new
+      @_request ||= TestRequest.new
     end
 
   end
   
-  class TestResponse
-    
-    attr_accessor :headers
-    
-    def initialize
-      self.headers = {}
-    end
+  class TestResponse < ActionDispatch::TestResponse
     
   end
   
-  class TestRequest
+  class TestRequest < ActionDispatch::TestRequest
     
     attr_accessor :fullpath