Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use with_indifferent_access #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vasfed
Copy link

@Vasfed Vasfed commented Dec 6, 2022

In a typical rails app the probability of ActiveSupport being already present is almost certain.
Do we need additional checks?

Formally this is a major change, since someone may have hacks in place to avoid with_indifferent_access (like requiring us before rails) and may require some workaround like

class DifferentAccessRedisSessionStore < RedisSessionStore
  def session_default_values
    [generate_sid, {}]
  end

  def decode(data)
    serializer.load(data)
  end
end

Rails.application.config.session_store(DifferentAccessRedisSessionStore, redis: ...)

@hogelog
Copy link

hogelog commented May 20, 2023

I am not the maintainer but I think this Pull Request is appropriate.

The current redis-session-store uses ActionDispatch::Session::AbstractSecureStore, but this class depends on ActiveSupport. So there is probably no one using the current redis-session-store without ActiveSupport::HashWithIndifferentAccess.
https://github.com/rails/rails/blob/v5.2.4.1/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb#L6
https://github.com/rails/rails/blob/v5.2.4.1/actionpack/lib/action_dispatch/middleware/cookies.rb#L3-L6

I think we should merge this pull request to remove this branch for code simplicity. @Jesterovskiy ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants