Skip to content
This repository

SPEC does not require `to_hash` on `rack.session` interface #461

Open
pmahoney opened this Issue November 26, 2012 · 5 comments

4 participants

pmahoney Oscar Del Ben Konstantin Haase James Tucker
pmahoney

I encountered this in Rack 1.3.6, but it seems to be also present on master.

The SPEC claims rack.session must implement:

  • store(key, value) (aliased as []=)
  • fetch(key, default = nil) (aliased as [])
  • delete(key)
  • clear

But session/abstract/id.rb:238 expects to be able to merge! a rack.session into a SessionHash. I was able to work around this by having my rack.session also implement to_hash.

Either I am misunderstanding something, or SPEC should be updated to also require to_hash.

Oscar Del Ben

I think you're right, merge! calls stringify_keys which in turn calls each on the store, which is not in the spec.

Oscar Del Ben

I think this issue can now be closed. cc @rkh

Konstantin Haase
Collaborator

thanks

James Tucker
Owner

This issue isn't closed, I reverted the SPEC change for multiple reasons:

  • SPEC changes are significant changes that need to alter major version - we shouldn't do this lightly
  • The SPEC file is auto-generated from Lint, it is not a static file
  • We may want to consider this a bug in session/abstract/id, rather than altering the spec.

The final comment is for many reasons, some of which are more important than others:

  • The session store can be replaced by servers (e.g. memcached, redis, etc)
  • Servers may have multiple interfaces that are not well isolated or defined, #each may not be a valid use case for some
  • each is not strictly required for most session operations
  • we might want to consider adding a to_hash instead, which could be implemented elsehow, in harder cases

We'll need to look into the use case more to decide whether it is really worth modifying the spec. It's also not totally illegal for implementations to extend the spec locally for their own cases. Those need to be documented. The SPEC is a minimum guide that all components should conform to, but optional spec extensions are entirely allowed, just not recommended where they can be avoided.

pmahoney

@raggi Good points.

This is only slightly related, but in implementing my own session store I also discovered that Rails expects the session to implement #key? (as Hash does): lib/action_dispatch/middleware/flash.rb:248

This is not rack's responsibility in any way; just another example of the difficulty in documenting (I have not found documentation of what exactly Rails expects in a session object) and coding to an interface in a dynamic language like Ruby. I applaud Rack's efforts to document this sort of thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.