Wrap ActiveRecord::Base with ActiveSupport.on_load

[nipe0324]

This PR try to fix #198

Cause

Before: Rails initialization order is NOT good.

First. Set ActiveRecord::Base.filter_attributes which is empty array.

# https://github.com/rails/rails/blob/v7.0.5/activerecord/lib/active_record/railtie.rb#L318-L322
initializer "active_record.set_filter_attributes" do
  ActiveSupport.on_load(:active_record) do
    self.filter_attributes += Rails.application.config.filter_parameters
  end
end

Second. Initialize Rails.application.config.filter_parameters

# My Rails Application
# config/initializers/filter_parameter_logging.rb
Rails.application.config.filter_parameters += [
  :passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn
]

After: Rails initialization order is good.

First. Initialize Rails.application.config.filter_parameters

# My Rails Application
# config/initializers/filter_parameter_logging.rb
Rails.application.config.filter_parameters += [
  :passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn
]

Second. Set ActiveRecord::Base.filter_attributes which is NOT empty array.

# https://github.com/rails/rails/blob/v7.0.5/activerecord/lib/active_record/railtie.rb#L318-L322
initializer "active_record.set_filter_attributes" do
  ActiveSupport.on_load(:active_record) do
    self.filter_attributes += Rails.application.config.filter_parameters
  end
end

Test

Use fixed gem

# Gemfile
gem 'activerecord-multi-tenant', github: 'nipe0324/activerecord-multi-tenant', branch: 'wrap_active_support_on_load'

Run bundle install. And, bin/rails c and filter_parameters works!!

Loading development environment (Rails 7.0.5)
irb(main):001:0> Rails.application.config.filter_parameters
=> [:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn]
irb(main):002:0> Customer.inspection_filter
=> 
#<ActiveSupport::ParameterFilter:0x0000000111df2188
 @filters=[:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn],
 @mask="[FILTERED]">
irb(main):003:0> Customer.new(name: 'name', token: 'some-token')
=> #<Customer:0x0000000111e9b878 id: nil, name: "name", token: "[FILTERED]", created_at: nil, updated_at: nil>

[codecov]

Codecov Report

Merging #199 (c89b21d) into master (076db75) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #199   +/-   ##
=======================================
  Coverage   82.61%   82.61%           
=======================================
  Files          14       14           
  Lines         719      719           
=======================================
  Hits          594      594           
  Misses        125      125           

Impacted Files	Coverage Δ
lib/activerecord-multi-tenant/query_rewriter.rb	92.30% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[gurkanindibay]

@nipe0324 thanks for your contribution.Could you add a test that covers this scenario?
Thanks

[nipe0324]

OK. Please give me a few days to add a test that covers this scenario. Thank you.

[nipe0324]

@microsoft-github-policy-service agree

[gurkanindibay]

Hi @nipe0324

I think you need a test for filters since this fix is about filters. By adding tests for filters

1. We will have a live documentation for filter support
2. We will make sure that our code base will support filter in the future
3. We can make sure that current PR fixes the issue

Thanks for your contribution

[gurkanindibay]

Unit tests are required

[gurkanindibay]

@nipe0324 Just giving a starting point, I prepared a prompt in ChatGPT. I didn't test it but I think test should be like below
https://chat.openai.com/share/ad170578-c628-4182-bc14-b4d46a2b1648

[nipe0324]

This code should execute before ActiveRecord::Base called. So, I moved to here.

[gurkanindibay]

Could you add a comment explaining this necessity? Could you refer the test in the comment as well?

[nipe0324]

Ok, I commented.

[nipe0324]

@gurkanindibay Thanks a lot. I try to add filter test. Could you please confirm?

[gurkanindibay]

Could you add a comment explaining this necessity? Could you refer the test in the comment as well?

[nipe0324]

This 2 lines are not used. So I removed.

[gurkanindibay]

Thanks @nipe0324 for your contribution :)