From 654a2de7a9c6d58f98dc9d4596cbeba2e5caeca2 Mon Sep 17 00:00:00 2001 From: James Coglan Date: Tue, 23 Oct 2012 20:15:53 +0100 Subject: Store FlashHashes in the session as plain hashes rather than custom objects with unstable class names and instance variables. Refactor FlashHash to take values for its ivars in the constructor, to pretty up FlashHash.from_session_value. Remove stale comment on FlashHash: it is no longer Marshaled in the session so we can change its implementation. Remove blank lines I introduced in controller/test_case.rb. Unit tests for FlashHash#to_session_value. Put in a compatibility layer to accept FlashHash serializations from Rails 3.0+. Test that Rails 3.2 session flashes are correctly converted to the new format. Remove code path for processing Rails 3.0 FlashHashes since they can no longer deserialize. Fix session['flash'] deletion condition: it will never be empty?, it will either be nil or a hash with 'discard' and 'flashes' keys. --- actionpack/lib/action_controller/test_case.rb | 3 +- actionpack/lib/action_dispatch/middleware/flash.rb | 34 +++++++++++++++------- actionpack/test/controller/flash_hash_test.rb | 21 +++++++++++++ 3 files changed, 47 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 5aecb59df9..be8055955d 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -509,7 +509,7 @@ module ActionController @request.assign_parameters(@routes, controller_class_name, action.to_s, parameters) @request.session.update(session) if session - @request.session["flash"] = @request.flash.update(flash || {}) + @request.flash.update(flash || {}) @controller.request = @request @controller.response = @response @@ -526,6 +526,7 @@ module ActionController @response.prepare! @assigns = @controller.respond_to?(:view_assigns) ? @controller.view_assigns : {} + @request.session['flash'] = @request.flash.to_session_value @request.session.delete('flash') if @request.session['flash'].blank? @response end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 9928b7cc3a..7b18c57420 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -4,7 +4,7 @@ module ActionDispatch # read a notice you put there or flash["notice"] = "hello" # to put a new one. def flash - @env[Flash::KEY] ||= (session["flash"] || Flash::FlashHash.new).tap(&:sweep) + @env[Flash::KEY] ||= Flash::FlashHash.from_session_value(session["flash"]) end end @@ -70,16 +70,30 @@ module ActionDispatch end end - # Implementation detail: please do not change the signature of the - # FlashHash class. Doing that will likely affect all Rails apps in - # production as the FlashHash currently stored in their sessions will - # become invalid. class FlashHash include Enumerable - def initialize #:nodoc: - @discard = Set.new - @flashes = {} + def self.from_session_value(value) + flash = case value + when FlashHash # Rails 3.1, 3.2 + new(value.instance_variable_get(:@flashes), value.instance_variable_get(:@used)) + when Hash # Rails 4.0 + new(value['flashes'], value['discard']) + else + new + end + + flash.tap(&:sweep) + end + + def to_session_value + return nil if empty? + {'discard' => @discard.to_a, 'flashes' => @flashes} + end + + def initialize(flashes = {}, discard = []) #:nodoc: + @discard = Set.new(discard) + @flashes = flashes @now = nil end @@ -223,7 +237,7 @@ module ActionDispatch if flash_hash if !flash_hash.empty? || session.key?('flash') - session["flash"] = flash_hash + session["flash"] = flash_hash.to_session_value new_hash = flash_hash.dup else new_hash = flash_hash @@ -233,7 +247,7 @@ module ActionDispatch end if (!session.respond_to?(:loaded?) || session.loaded?) && # (reset_session uses {}, which doesn't implement #loaded?) - session.key?('flash') && session['flash'].empty? + session.key?('flash') && session['flash'].nil? session.delete('flash') end end diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index ccca0dac17..5490d9394b 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -46,6 +46,27 @@ module ActionDispatch assert_equal({'foo' => 'bar'}, @hash.to_hash) end + def test_to_session_value + @hash['foo'] = 'bar' + assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => []}, @hash.to_session_value) + + @hash.discard('foo') + assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => %w[foo]}, @hash.to_session_value) + + @hash.now['qux'] = 1 + assert_equal({'flashes' => {'foo' => 'bar', 'qux' => 1}, 'discard' => %w[foo qux]}, @hash.to_session_value) + + @hash.sweep + assert_equal(nil, @hash.to_session_value) + end + + def test_from_session_value + rails_3_2_cookie = 'BAh7B0kiD3Nlc3Npb25faWQGOgZFRkkiJWY4ZTFiODE1MmJhNzYwOWMyOGJiYjE3ZWM5MjYzYmE3BjsAVEkiCmZsYXNoBjsARm86JUFjdGlvbkRpc3BhdGNoOjpGbGFzaDo6Rmxhc2hIYXNoCToKQHVzZWRvOghTZXQGOgpAaGFzaHsAOgxAY2xvc2VkRjoNQGZsYXNoZXN7BkkiDG1lc3NhZ2UGOwBGSSIKSGVsbG8GOwBGOglAbm93MA==' + session = Marshal.load(Base64.decode64(rails_3_2_cookie)) + hash = Flash::FlashHash.from_session_value(session['flash']) + assert_equal({'flashes' => {'message' => 'Hello'}, 'discard' => %w[message]}, hash.to_session_value) + end + def test_empty? assert @hash.empty? @hash['zomg'] = 'bears' -- cgit v1.2.3