Loading geocoded model in an initializer can cause error with certain dependencies

[wozza35]

Expected behavior

If I have a model:

class TestModel < ApplicationRecord
    geocoded_by :address

    def self.configure(arg)
        .....
    end
end

I should be able to load the model in an initializer? e.g. for configuration

# initializers/test_model.rb
TestModel.configure('foo')

Actual behavior

Depending on what other gems/initializers you have included in your app, It is possible that at the TestModel initializer is run before the Geocoder initializer executes (and inserts the Railtie into ActiveRecord)

Geocoder initializer:

geocoder/lib/geocoder/railtie.rb

Line 7 in 12dded6

initializer 'geocoder.insert_into_active_record' do

This can cause an undefined method 'geocoded_by' error when the application is booting

It's possible to fix this issue by explicitly including Geocoder in the model:

class TestModel < ApplicationRecord
  extend Geocoder::Model::ActiveRecord

end

But I believe the better way to fix this would be to put a constraint on the Geocoder Railtie initializer such that it is forced to run before the application initializers. I can't see any reason why this would be a problem? I've implemented this change in a PR here

Steps to reproduce

Rails loads application initializers together in one group (in alphabetical order), but other initializers and groups of initializers are run after being sorted in topological order. There are some gems that include initializers that will alter the execution order and can therefore delay the Geocoder initializer. (e.g. the heroku_rails_deflate gem)

• Create fresh rails 5.2 app
• Add geocoder gem
• Add test_model.rb in app/models:

class TestModel < ApplicationRecord
   geocoded_by :foo
end

• Add test_model.rb initializer in config/initializers

TestModel

• Add 'heroku_rails_deflate' to gemfile
• Try to boot rails

Environment info

• Geocoder version: 1.4.9
• Rails version: 5.2

[alexreisner]

Hmm...I see what you're saying, but doesn't this problem lie more with the heroku_rails_deflate gem? I'm hesitant to change without understanding exactly what's going on, but it seems (with my partial understanding) like any gem which causes application initializers to run before all gems are initialized is going to cause a lot of problems (wouldn't it break the ability to configure a lot of gems?).

[wozza35]

I think if heroku_rails_deflate, or any other gem, needs to specify that it runs after :load_config_intializers then it should be able to do so. The fact that this also can push other Railtie initializers to execute after the application initializers is perhaps an issue with how rails manages the execution order.

However, it doesn't seem harmful to add a check to ensure geocoder loads before the application initializers, as this is standard behaviour anyway. (I'm not 100% confident I understand it all yet though.)

I've just stumbled upon a similar issue to this one for the 'newrelic_rpm' gem newrelic/newrelic-ruby-agent#279

[wozza35]

FYI, I suspect this issue is related: #1178

The react-rails gem also specifies several initializers with after :load_config_initializers as an argument

https://github.com/reactjs/react-rails/blob/master/lib/react/rails/railtie.rb#L26

It appears that it is down to the order in which the gems are specified in the gemfile as to how the initializers are eventually executed.

[alexreisner]

OK, after some more investigation, I think I agree with this. Going to merge. Thanks for your patience! (And the good research.)

[alexreisner]

Merged #1317