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

Custom encoder class #16

Closed dplummer closed 10 years ago

dplummer commented 10 years ago

This adds a configuration option for the encoder class to use. In Rails 4, this allows the use of MultiJson for the encoder:

My::Application.config.session_store = :redis_session_store, {
  :key          => 'your_session_key',
  :encoder_class => MultiJson
}
dplummer commented 10 years ago

It probably isn't a good idea to use MultiJson for the encoder in rails 3 because I think the FlashHash implementation doesn't serialize as json well.

Oh, maybe I should name the key serializer instead of encoder_class. What do you think?

meatballhat commented 10 years ago

I prefer serializer, yup.

meatballhat commented 10 years ago

@dplummer Did you happen to check if there's a precedent for this, perhaps in the Dalli session store?

dplummer commented 10 years ago

Excellent question. I looked through the source of dalli and rails' memcache store and I don't see how it marshal's it. I'll keep looking.

meatballhat commented 10 years ago

@dplummer don't sweat it! I was mostly just curious :smiley_cat:

dplummer commented 10 years ago

Thanks for asking about precedent. I tracked down rails issue #13692, which adds similar behavior to the built-in cookie store. Hold up on merging this PR, I want to review what @lukesarnacki did and support the same interface.

lukesarnacki commented 10 years ago

Hi @dplummer, this was actually rewritten so you will probably have better luck looking at this one rails/rails#13945

meatballhat commented 10 years ago

@dplummer sorry about the conflicts! :scream_cat:

dplummer commented 10 years ago

Alright, so I rewrote it to allow you to specify the serializer as :marshal, :json, or :hybrid. :marshal is default.

I'm requiring json. I'm not sure if that's the right thing to do, but I don't know what else to do here.

meatballhat commented 10 years ago

@dplummer you get a chance to look at the conflicts yet?

dplummer commented 10 years ago

Thanks for the bump, I've rebased on master.

meatballhat commented 10 years ago

@dplummer yay!