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

Incompatible with rails_admin 1.4.x #22

Closed
PikachuEXE opened this issue Oct 8, 2018 · 4 comments
Closed

Incompatible with rails_admin 1.4.x #22

PikachuEXE opened this issue Oct 8, 2018 · 4 comments

Comments

@PikachuEXE
Copy link

Just updated to rails_admin 1.4.2 and got this error on start up:

Required middlewares for RailsAdmin are not added
To fix tihs, add

  config.middleware.use ActionDispatch::Session::RedisStore, {:redis_server=>{:host=>"127.0.0.1", :port=>6379, :password=>nil, :namespace=>"spacious:development:sessions"}, :key=>"_spacious_development_sessions_v2017_01_04_1426"}

to config/application.rb.

error raised from
https://github.com/sferik/rails_admin/blob/v1.4.2/lib/rails_admin/engine.rb#L46

Not sure if this is this gem's issue or rails_admin's issue

@tubbo
Copy link
Contributor

tubbo commented Oct 8, 2018

This seems due to the fact that ActionDispatch::Session::RedisStore is actually a subclass of Rack::Session::Redis, not ActionDispatch::Session::AbstractStore.

This line right here should return true because the middleware is indeed a session store which follows the entire API, but it's testing whether it's a direct subclass of the AbstractStore, which it is not, thus it fails.

The solution here is to extract the common functionality from Rack::Session::Redis into modules, allowing both Rack::Session::Redis and ActionDispatch::Session::AbstractStore to share the same functionality. I'll make an issue on redis-store/redis-rack that accompanies this one.

tubbo pushed a commit to redis-store/redis-rack that referenced this issue Oct 8, 2018
This allows all functionality of `Rack::Session::Redis` to be mixed into
other classes without needing to inherit from `Rack::Session::Redis`
itself. Pre-requisite for the fix in redis-store/redis-actionpack#22

Resolves #38
@tubbo
Copy link
Contributor

tubbo commented Oct 8, 2018 via email

@tubbo
Copy link
Contributor

tubbo commented Oct 8, 2018

Looking at Dalli's implementation of Rack::Session, I believe the problem has more to do with how RailsAdmin is choosing to figure out which of the middleware is your session store. Instead of testing whether the middleware inherits from some base class, RailsAdmin could be looking for a middleware with the class name "ActionDispatch::Session::#{camelized_session_store_name_from_rails_config}", and then it wouldn't matter what the inheritance chain of that middleware was. Additionally, there's nothing preventing two session stores from being in the Rails middleware chain, technically, so this fix doesn't actually solve for all edge cases.

I don't think there's anything to be done in redis-actionpack, and instead the issue should be raised as to how rails-admin is testing for session store middleware in the chain. Gonna close this unless you feel otherwise.

@mshibuya
Copy link

@tubbo Thanks for the suggestion, I've updated the middleware check logic accordingly.

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

No branches or pull requests

3 participants