rack/rack — pull request #342 by brainopia (2012-02-19 14:33:43+00:00)
Title: Rack::Session remove inheritance from Hash

In discussions with @josevalim we've noticed that the current api provided by `Rack::Session::Abstract::SessionHash` is inconsistent. This class inherits a lot of methods directly from `Hash` and without redefinition they work incorrectly in case of unloaded session. For example , `#replace`, `#each`, `#keys` and a lot of others.

So in e5b4d961 I remove inheritance from `Hash` and define missing methods directly on `SessionHash` (since they were not working correctly previously anyway). This commit is backward-compatible. All rack, rails, sinatra, padrino tests are passing as they should.

----

But in this pull-request I've also made another change. There is `Rack::Session::Abstract::OptionHash` which inherits from `Hash` and redefines exactly one method `[]` in case we pass key `:id` there. Clearly it's not the best place to store session_id.

So in spirit of @evanphx I've removed `OptionHash` completely in 83a270d6. Session options are now a simple hash and session_id is stored as instance variable of `SessionHash` (since the class is already tightly coupled to id). It promotes cleaner code and has advantage of being faster.

Since it's an implementation detail all rack, sinatra and padrino tests are still passing. But Rails tests use explicitly original location to test session. I've fixed them and made a few other changes we've discussed with @josevalim in https://github.com/brainopia/rails/commit/8c9c4c09ea3f46bd3eeef5026f9bf92d3680a999.

Still there is a possibility there may be some plugins which use such implementation detail to access session id directly and not through provided api. I think it's a risk worth taking. But I can provide pull-request with only `SessionHash` changes if needed.