Skip to content

Always use with_indifferent_access#140

Open
Vasfed wants to merge 1 commit intoroidrage:masterfrom
insales:indifferent_access_always_present
Open

Always use with_indifferent_access#140
Vasfed wants to merge 1 commit intoroidrage:masterfrom
insales:indifferent_access_always_present

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