apokalipto/devise_saml_authenticatable

Deprecation Warning for autoloading `IdpSettingsAdapter` and `IdpEntityIdReader` from Devise Initializer

Closed this issue · 4 comments

Rails version: 6.0.3.2
Ruby version: ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-linux]

No complaints about the gem, it works reliably. The purpose of raising this Issue is to ask whether there is a way to comply with the requirements for autoloading constants if config.autoloader is set to zeitwerk.

In our project, config.autoloader is set to classic.

I followed the instructions in the README and implemented custom IdpSettingsAdapter and IdpEntityIdReader classes, which are loaded in the Devise initializer.

  # You can support multiple IdPs by setting this value to a class that implements a #settings method which takes
  # an IdP entity id as an argument and returns a hash of idp settings for the corresponding IdP.
  # config.idp_settings_adapter = nil
  config.idp_settings_adapter = IdpSettingsAdapter.new

  # You provide you own method to find the idp_entity_id in a SAML message in the case of multiple IdPs
  # by setting this to a custom reader class, or use the default.
  config.idp_entity_id_reader = IdpEntityIdReader

These classes are reliably loaded when the server starts and thereafter are reliably available.

However, Rails gives the following warning:

W, [2020-07-23T15:24:14.583967 #5998]  WARN -- : DEPRECATION WARNING: Initialization autoloaded the constants IdpSettingsAdapter and IdpEntityIdReader.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload IdpSettingsAdapter, for example,
the expected changes won't be reflected in that stale Class object.

`config.autoloader` is set to `classic`. These autoloaded constants would have been unloaded if `config.autoloader` had been set to `:zeitwerk`.

Please, check the "Autoloading and Reloading Constants" guide for solutions.
 (called from <main> at /home/alexander/Development/inspiregroup/chameleon/creator/services/creator-backend/config/environment.rb:5)

That's an interesting problem. Off the top of my head I'm not sure there's a way around that. We could use procs instead of classes, but that means testing is a lot harder and you're just embedding code into the initializer instead of using a class.

Hi @adamstegman, first of all many thanks for the gem, really happy with how it works.

Any chance you gave this more thoughts ? I recently upgraded my app to rails 6 and ran into this issue, I really don't follow your suggestion, both idp_settings_adapter and idp_entity_id_reader need class names with predefined method names, how'd I go about making the change to replace those classes with procs ?

I'm referring to possibly changing the API to accept a proc instead of a class to avoid this error. Another option would be to set a class name, and constantize it at runtime.

ok sounds interesting, Thanks for the prompt reply. 👍