roidrage / redis-session-store

A simple session store for Rails based on Redis.
http://github.com/roidrage/redis-session-store
MIT License
366 stars 147 forks source link

Fix undefined constant error #37

Closed MichaelXavier closed 10 years ago

MichaelXavier commented 10 years ago

Evidently this case is never hit as middleware because options is always specified, but if it is not, that constant is undefined.

dplummer commented 10 years ago

Michael, can you make this change to spec/support.rb:

--- a/spec/support.rb
+++ b/spec/support.rb
@@ -1,10 +1,19 @@
 # vim:fileencoding=utf-8

+unless defined?(Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY)
+  module Rack # rubocop:disable Documentation
+    module Session
+      module Abstract
+        ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze
+      end
+    end
+  end
+end
+
 unless defined?(ActionDispatch::Session::AbstractStore)
   module ActionDispatch
     module Session
       class AbstractStore # rubocop:disable Documentation
-        ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze
         DEFAULT_OPTIONS = {
           key: '_session_id',
           path: '/',
MichaelXavier commented 10 years ago

:thumbsup:

dplummer commented 10 years ago

Hm, so this breaks if you're using Rack 1.2. Which Rails 3.0 was using.

dplummer commented 10 years ago

Ya, so in Rails 3.0 that constant was defined in AbstractStore: https://github.com/rails/rails/blob/3-0-stable/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb#L13

But in 3.1+ it was moved to Rack 1.3+ in Rack::Session::Abstract: https://github.com/rack/rack/blob/rack-1.3/lib/rack/session/abstract/id.rb#L19

So I think if we want to continue with rails 3.0 compatibility, we need to do something like:

def session_options_key
  if defined?(ENV_SESSION_OPTIONS_KEY) # For Rails 3.0
    ENV_SESSION_OPTIONS_KEY
  else # For Rails >= 3.1
    Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY
  end
end
MichaelXavier commented 10 years ago

Just moved the constant into the class unless it was previously defined. Eh?