Clearance initializer overrides Rails ActiveRecord configs

[calmerwaters]

Hey team,

We just noticed that for as long as we've had it set, a lot of the previous settings configured after upgrading Rails apps didn't apply because the Clearance gem calls ActiveRecord in a way that overrides any settings configured in new_framework_defaults.rb

This can be confirmed by having a Rails app, with Clearance in Gemfile. Any of the settings in the new_framework_defaults.rb relating to ActiveRecord will not be set, because Clearance gem extending ActiveRecord before this is run in the initializers sequence.

See more detail here:

https://github.com/rails/rails/issues/23589#issuecomment-229247727

There was a similar issue with Devise that resolved any of these references too:

https://github.com/heartcombo/devise/commit/c2c74b0a39238e7d997486814a1c8f75fdaf276f

This was confirmed by renaming the initializers/clearance.rb initializer to initializers/z_clearance.rb so that it was run after the initializers/new_framework_defaults.rb.

Any idea on if this would be updated in Clearance?

Cheers,

[mjankowski]

Definitely seems worth fixing.

Can you clarify whether there's something in clearance itself causing this issue, or something in the configuration file (presumably some common config option) which when done in a certain way, triggers the issue? Or if you're not sure, can you provide the contents of your config file which can replicate the issue.

[calmerwaters]

Hey @mjankowski thanks for reviewing this one.

So the Initializer looks like this:

Clearance.configure do |config|
  ... other config ...
  config.password_strategy = Clearance::PasswordStrategies::BCrypt
  config.user_model = User
  ... other config ...
end

If there are any suspicions about what in that config is triggering ActiveRecord, I suspect the fact it is referencing User model may have something to do with it (this is also where include Clearance::User is included in the User model).

Specifically, this is when the clearance initializer is placed above the new_framework_defaults.rb when it is named clearance.rb it can be immediately fixed by renaming.

[mjankowski]

If you quote the model ("User" instead of User) does that change anything?

[eebs]

Hi @calmerwaters, have you had a chance to try changing User to "User"? I think this should fix this particular issue.

We'd like to try and make this problem impossible so we're planning to deprecate passing a constant to user_model and only allow Strings.

[calmerwaters]

Hey @eebs @mjankowski - sorry for the delay in getting back to you guys.

I can confirm that changing it from User to 'User' resolves this issue, and the new_framework_defaults.rb load as intended, even with the Clearance configuration file sitting above the hierarchy in the initializers folder.

Presumably, the precedence in naming of a config file shouldn't be able to make such an impact on the configuration of Rails right - assume that a more robust fix would be to ensure that the way that Clearance hooks into ActiveRecord?

[eebs]

No worries, thanks for the update!

I believe you're right that the naming of initializers should not have an effect on the app. I think the ideal is that initializers could run in any order.

In Clearance itself, if we were to reference the ActiveRecord constant it would force ActiveRecord to load during our gem loading. Any place where we need to reference ActiveRecord should use ActiveSupport.on_load(:active_record) to delay that reference until ActiveRecord has loaded.

I looked through the source and didn't find any places where we reference ActiveRecord outside specs, generators, and comments, so I think we're okay on that front.

Referencing any ActiveRecord subclass constant in an initializer will cause that subclass to load, which will cause ActiveRecord to load. By using a String that we constantize later we can avoid any reference to ActiveRecord during initialization.

I'd like to leave this issue open for now to help us track the work of deprecating passing a constant to user_model (we may create a new issue when we start work), but I'm happy to hear your issue is resolved!

Thanks again!