CGI::Session::ActiveRecordStore.data_column_name = 'foobar' to use a different session data column than the 'data' default. References #2731. Remove error-prone method_missing passthrough to session model. Cleanup.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2944 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

4 files changed, 61 insertions, 44 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 54b414a30e..0d7d99236e 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* CGI::Session::ActiveRecordStore.data_column_name = 'foobar' to use a different session data column than the 'data' default.  [nbpwie102@sneakemail.com]
+
 * Do not raise an exception when default helper is missing; log a debug message instead.  It's nice to delete empty helpers.  [Jeremy Kemper]
 
 * Controllers with acronyms in their names (e.g. PDFController) require the correct default helper (PDFHelper in file pdf_helper.rb).  #2262 [jeff@opendbms.com]
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index b787af3fde..f37448d347 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -28,11 +28,15 @@ module ActionController #:nodoc:
   end
   class SessionOverflowError < ActionControllerError #:nodoc:
     DEFAULT_MESSAGE = 'Your session data is larger than the data column in which it is to be stored. You must increase the size of your data column if you intend to store large data.'
+
+    def initialize(message = nil)
+      super(message || DEFAULT_MESSAGE)
+    end
   end
   class DoubleRenderError < ActionControllerError #:nodoc:
     DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and only once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\". Finally, note that to cause a before filter to halt execution of the rest of the filter chain, the filter must return false, explicitly, so \"render(...) and return false\"." 
 
-    def initialize(message=nil)
+    def initialize(message = nil)
       super(message || DEFAULT_MESSAGE)
     end
   end
diff --git a/actionpack/lib/action_controller/session/active_record_store.rb b/actionpack/lib/action_controller/session/active_record_store.rb
index c5f49fa8bc..4a6b05c917 100644
--- a/actionpack/lib/action_controller/session/active_record_store.rb
+++ b/actionpack/lib/action_controller/session/active_record_store.rb
@@ -5,33 +5,42 @@ require 'base64'
 
 class CGI
   class Session
-    # Return this session's underlying Session model. Useful for the DB-backed session stores.
+    # Return this session's underlying Session instance. Useful for the DB-backed session stores.
     def model
-      @dbman.model rescue nil
+      @dbman.model if @dbman
     end
 
-    # Proxy missing methods to the underlying Session model.
-    def method_missing(method, *args, &block)
-      if model then model.send(method, *args, &block) else super end
-    end
 
-    # A session store backed by an Active Record class.
+    # A session store backed by an Active Record class.  A default class is
+    # provided, but any object duck-typing to an Active Record +Session+ class
+    # with text +session_id+ and +data+ attributes is sufficient.
     #
-    # A default class is provided, but any object duck-typing to an Active
-    # Record +Session+ class with text +session_id+ and +data+ attributes
-    # may be used as the backing store.
+    # The default assumes a +sessions+ tables with columns:
+    #   +id+ (numeric primary key),
+    #   +session_id+ (text, or longtext if your session data exceeds 65K), and
+    #   +data+ (text or longtext; careful if your session data exceeds 65KB).
+    # The +session_id+ column should always be indexed for speedy lookups.
+    # Session data is marshaled to the +data+ column in Base64 format.
+    # If the data you write is larger than the column's size limit,
+    # ActionController::SessionOverflowError will be raised.
     #
-    # The default assumes a +sessions+ tables with columns +id+ (numeric
-    # primary key), +session_id+ (text, or longtext if your session data exceeds 65K), 
-    # and +data+ (text).  Session data is marshaled to +data+.  +session_id+ should be 
-    # indexed for speedy lookups.
+    # You may configure the table name, primary key, and data column.
+    # For example, at the end of config/environment.rb:
+    #   CGI::Session::ActiveRecordStore::Session.table_name = 'legacy_session_table'
+    #   CGI::Session::ActiveRecordStore::Session.primary_key = 'session_id'
+    #   CGI::Session::ActiveRecordStore::Session.data_column_name = 'legacy_session_data'
+    # Note that setting the primary key to the session_id frees you from
+    # having a separate id column if you don't want it.  However, you must
+    # set session.model.id = session.session_id by hand!  A before_filter
+    # on ApplicationController is a good place.
     #
     # Since the default class is a simple Active Record, you get timestamps
     # for free if you add +created_at+ and +updated_at+ datetime columns to
     # the +sessions+ table, making periodic session expiration a snap.
     #
-    # You may provide your own session class, whether a feature-packed
-    # Active Record or a bare-metal high-performance SQL store, by setting
+    # You may provide your own session class implementation, whether a
+    # feature-packed Active Record or a bare-metal high-performance SQL
+    # store, by setting
     #   +CGI::Session::ActiveRecordStore.session_class = MySessionClass+
     # You must implement these methods:
     #   self.find_by_session_id(session_id)
@@ -41,28 +50,28 @@ class CGI
     #   save
     #   destroy
     #
-    # The fast SqlBypass class is a generic SQL session store.  You may
+    # The example SqlBypass class is a generic SQL session store.  You may
     # use it as a basis for high-performance database-specific stores.
-    #
-    # If the data you are attempting to write to the +data+ column is larger
-    # than the column's size limit, ActionController::SessionOverflowError 
-    # will be raised.
     class ActiveRecordStore
       # The default Active Record class.
       class Session < ActiveRecord::Base
-        before_update :loaded? # Don't try to save if we haven't loaded the session
+        # Customizable data column name.  Defaults to 'data'.
+        cattr_accessor :data_column_name
+        self.data_column_name = 'data'
+
+        # Don't try to save if we haven't loaded the session.
+        before_update :loaded?
         before_save   :marshal_data!
-        before_save   :ensure_data_not_too_big
-        
-        class << self
+        before_save   :raise_on_session_data_overflow!
 
+        class << self
           # Don't try to reload ARStore::Session in dev mode.
           def reloadable? #:nodoc:
             false
           end
-          
+
           def data_column_size_limit
-            columns_hash['data'].limit
+            @data_column_size_limit ||= columns_hash[@@data_column_name].limit
           end
 
           # Hook to set up sessid compatibility.
@@ -79,7 +88,7 @@ class CGI
               CREATE TABLE #{table_name} (
                 id INTEGER PRIMARY KEY,
                 #{connection.quote_column_name('session_id')} TEXT UNIQUE,
-                #{connection.quote_column_name('data')} TEXT(255)
+                #{connection.quote_column_name(@@data_column_name)} TEXT(255)
               )
             end_sql
           end
@@ -111,11 +120,11 @@ class CGI
         # Lazy-unmarshal session state.
         def data
           unless @data
-            case data = read_attribute('data')
+            case d = read_attribute(@@data_column_name)
               when String
-                @data = self.class.unmarshal(data)
+                @data = self.class.unmarshal(d)
               else
-                @data = data || {}
+                @data = d || {}
             end
           end
           @data
@@ -123,9 +132,9 @@ class CGI
 
         private
           def marshal_data!
-            write_attribute('data', self.class.marshal(self.data))
+            write_attribute(@@data_column_name, self.class.marshal(self.data))
           end
-          
+
           # Has the session been loaded yet?
           def loaded?
             !! @data
@@ -134,15 +143,17 @@ class CGI
           # Ensures that the data about to be stored in the database is not
           # larger than the data storage column. Raises
           # ActionController::SessionOverflowError.
-          def ensure_data_not_too_big
-            return unless limit = self.class.data_column_size_limit
-            raise ActionController::SessionOverflowError, ActionController::SessionOverflowError::DEFAULT_MESSAGE if read_attribute('data').size > limit
+          def raise_on_session_data_overflow!
+            limit = self.class.data_column_size_limit
+            if loaded? and limit and read_attribute(@@data_column_name).size > limit
+              raise ActionController::SessionOverflowError
+            end
           end
-
       end
 
       # A barebones session store which duck-types with the default session
-      # store but bypasses Active Record and issues SQL directly.
+      # store but bypasses Active Record and issues SQL directly.  This is
+      # an example session model class meant as a basis for your own classes.
       #
       # The database connection, table name, and session id and data columns
       # are configurable class attributes.  Marshaling and unmarshaling
@@ -257,13 +268,13 @@ class CGI
             end_sql
           end
         end
-
       end
 
+
       # The class used for session storage.  Defaults to
       # CGI::Session::ActiveRecordStore::Session.
       cattr_accessor :session_class
-      @@session_class = Session
+      self.session_class = Session
 
       # Find or instantiate a session given a CGI::Session.
       def initialize(session, option = nil)
diff --git a/actionpack/test/controller/active_record_store_test.rb b/actionpack/test/controller/active_record_store_test.rb
index f10bbf7434..5c612865c3 100644
--- a/actionpack/test/controller/active_record_store_test.rb
+++ b/actionpack/test/controller/active_record_store_test.rb
@@ -74,7 +74,7 @@ class ActiveRecordStoreTest < Test::Unit::TestCase
 
   def test_model_attribute
     assert_kind_of CGI::Session::ActiveRecordStore::Session, @new_session.model
-    assert_equal @new_session.model.data, @new_session.data
+    assert_equal({ 'foo' => 'bar' }, @new_session.model.data)
   end
 
   def teardown
@@ -98,7 +98,7 @@ class ColumnLimitTest < Test::Unit::TestCase
     too_big = ':(' * limit
     s = @session_class.new(:session_id => '666', :data => {'foo' => too_big})
     s.data
-    assert_raises(ActionController::SessionOverflowError) { s.save }
+    assert_raise(ActionController::SessionOverflowError) { s.save }
   end
 end
 
@@ -132,7 +132,7 @@ class SqlBypassActiveRecordStoreTest < ActiveRecordStoreTest
 
   def test_model_attribute
     assert_kind_of CGI::Session::ActiveRecordStore::SqlBypass, @new_session.model
-    assert_equal @new_session.model.data, @new_session.data
+    assert_equal({ 'foo' => 'bar' }, @new_session.model.data)
   end
 end