search

mgomes / api_auth

HMAC authentication for Rails and HTTP Clients
MIT License
480 stars 147 forks source link

Fix Rails 6 ActionController::Base deprecation warnings #179

Closed taylorthurlow closed 4 years ago

taylorthurlow commented 4 years ago

Referencing ActionController::Base within an initializer or railtie causes Rails to print a deprecation warning. The relevant Rails issue is here: https://github.com/rails/rails/issues/36546

This is fixed by using ActiveSupport.on_load before referencing the module in the railtie. I chose to use the :action_controller hook name because the more appropriate :action_controller_base hook is only available in Rails versions >= 5.1. If it's preferable, we could check the Rails version and set the hook name conditionally.

taylorthurlow commented 4 years ago

I'm going to mark this ready for review. CI is failing because of an update to Rubocop which involved a cop name change. I've opened a separate PR to solve that issue.

EDIT: I've made some changes to the linked Rubocop PR which will cause this PR to have merge conflicts. I'll solve those once the other PR is merged.

pjmartorell commented 4 years ago

@mgomes Could you review and merge this PR and the other one related #180, please? 🙏

taylorthurlow commented 4 years ago

GitHub seems to be having some issues picking up the passed status of the Travis CI job, but I've rebased my branch and solved the merge conflicts. The changes are good to go.

mgomes commented 4 years ago

Thanks @taylorthurlow. I've been noticing that as well.

I really appreciate your work. I'll get this merged and cut a new version shortly.

taylorthurlow commented 4 years ago

@mgomes I'm thinking I unwittingly introduced an ActiveSupport dependency to the gem with this PR. With the gem being able to be used without Rails (and I assume ActiveSupport) this seems like a problem to me. That said, the railtie file itself isn't conditionally included, so lines with if-statements like this one essentially meant that loading the railtie definition in a gem with no Rails dependency in your project didn't matter. That's not the case anymore.

Do you mind sanity checking what I've said? I can open a new PR to fix this if necessary.

EDIT: I'm getting this in a project with no Rails dependency, BUT with an ActiveSupport dependency. Error looks something like:

NameError:
    uninitialized constant ApiAuth::Rails::ControllerMethods::ActiveSupport

EDIT2: Another solution could be conditionally including the railtie here, checking if ::Rails is defined?

EDIT3: I've opened a PR with a solution where we can discuss it: #189