From 9ac96ce64525a4287d8403a9b6ece71c8ed179c0 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 12 Mar 2020 09:57:41 +0100 Subject: [PATCH 1/3] Move lazy_load in railtie.rb Create railtie with lazy load ActionController and ActiveRecord Rearrange file requiring for Rails and non Rails to make sure ActiveRecord is lazy loaded properly. Add CHANGELOG.md entry --- CHANGELOG.md | 3 +- lib/paper_trail.rb | 39 ++++--------------- lib/paper_trail/frameworks/active_record.rb | 5 --- .../frameworks/rails/controller.rb | 6 --- lib/paper_trail/railtie.rb | 24 ++++++++++++ 5 files changed, 33 insertions(+), 44 deletions(-) delete mode 100644 lib/paper_trail/frameworks/active_record.rb create mode 100644 lib/paper_trail/railtie.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 472c93c50..c86358790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Fixed -- None +- [#1237](https://github.com/paper-trail-gem/paper_trail/pull/1237) + Fixed Rails lazy load ActiveRecord ### Dependencies diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb index a64d6a4a9..d3b1864d9 100644 --- a/lib/paper_trail.rb +++ b/lib/paper_trail.rb @@ -8,24 +8,13 @@ # can revisit this decision. require "active_support/all" -# AR is required for, eg. has_paper_trail.rb, so we could put this `require` in -# all of those files, but it seems easier to troubleshoot if we just make sure -# AR is loaded here before loading *any* of PT. See discussion of -# performance/simplicity tradeoff for activesupport above. -require "active_record" - -require "request_store" require "paper_trail/cleaner" require "paper_trail/compatibility" require "paper_trail/config" -require "paper_trail/has_paper_trail" require "paper_trail/record_history" -require "paper_trail/reifier" require "paper_trail/request" -require "paper_trail/version_concern" require "paper_trail/version_number" require "paper_trail/serializers/json" -require "paper_trail/serializers/yaml" # An ActiveRecord extension that tracks changes to your models, for auditing or # versioning. @@ -126,27 +115,13 @@ def version end end -# We use the `on_load` "hook" instead of `ActiveRecord::Base.include` because we -# don't want to cause all of AR to be autloaded yet. See -# https://guides.rubyonrails.org/engines.html#what-are-on-load-hooks-questionmark -# to learn more about `on_load`. -ActiveSupport.on_load(:active_record) do - include PaperTrail::Model -end - -# Require frameworks -if defined?(::Rails) - # Rails module is sometimes defined by gems like rails-html-sanitizer - # so we check for presence of Rails.application. - if defined?(::Rails.application) - require "paper_trail/frameworks/rails" - else - ::Kernel.warn(::PaperTrail::E_RAILS_NOT_LOADED) - end +if defined?(Rails::Railtie) + require "paper_trail/railtie" else - require "paper_trail/frameworks/active_record" -end - -if defined?(::ActiveRecord) + require "active_record" ::PaperTrail::Compatibility.check_activerecord(::ActiveRecord.gem_version) + require "paper_trail/has_paper_trail" + require "paper_trail/reifier" + require "paper_trail/frameworks/active_record/models/paper_trail/version" + ActiveRecord.include PaperTrail::Model end diff --git a/lib/paper_trail/frameworks/active_record.rb b/lib/paper_trail/frameworks/active_record.rb deleted file mode 100644 index 655657107..000000000 --- a/lib/paper_trail/frameworks/active_record.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -# This file only needs to be loaded if the gem is being used outside of Rails, -# since otherwise the model(s) will get loaded in via the `Rails::Engine`. -require "paper_trail/frameworks/active_record/models/paper_trail/version" diff --git a/lib/paper_trail/frameworks/rails/controller.rb b/lib/paper_trail/frameworks/rails/controller.rb index f388e75f3..a9d89eca3 100644 --- a/lib/paper_trail/frameworks/rails/controller.rb +++ b/lib/paper_trail/frameworks/rails/controller.rb @@ -103,9 +103,3 @@ def set_paper_trail_controller_info end end end - -if defined?(::ActionController) - ::ActiveSupport.on_load(:action_controller) do - include ::PaperTrail::Rails::Controller - end -end diff --git a/lib/paper_trail/railtie.rb b/lib/paper_trail/railtie.rb new file mode 100644 index 000000000..a3c778615 --- /dev/null +++ b/lib/paper_trail/railtie.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require "paper_trail/frameworks/rails/engine" + +module PaperTrail + # Represents code to load within Rails framework + # @api private + class Railtie < ::Rails::Railtie + initializer "paper_trail" do + ActiveSupport.on_load(:action_controller) do + require "paper_trail/frameworks/rails/controller" + include PaperTrail::Rails::Controller + end + + ActiveSupport.on_load(:active_record) do + ::PaperTrail::Compatibility.check_activerecord(::ActiveRecord.gem_version) + require "paper_trail/has_paper_trail" + require "paper_trail/reifier" + require "paper_trail/frameworks/active_record/models/paper_trail/version" + include PaperTrail::Model + end + end + end +end From f47bd71e3d56b315e232bcb9675ab7afd5afc5a5 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Sun, 19 Apr 2020 19:18:47 -0400 Subject: [PATCH 2/3] Update conditional to match railtie docs See railties-6.0.2.2/lib/rails/railtie.rb:45 --- lib/paper_trail.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb index d3b1864d9..00ed31f5d 100644 --- a/lib/paper_trail.rb +++ b/lib/paper_trail.rb @@ -115,7 +115,7 @@ def version end end -if defined?(Rails::Railtie) +if defined?(Rails) require "paper_trail/railtie" else require "active_record" From 8ef39a29e658070e1ef065c446f5f8cc0408eb47 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Sun, 19 Apr 2020 19:21:18 -0400 Subject: [PATCH 3/3] Comments on railties, generally --- lib/paper_trail/railtie.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/paper_trail/railtie.rb b/lib/paper_trail/railtie.rb index a3c778615..c65f86cf5 100644 --- a/lib/paper_trail/railtie.rb +++ b/lib/paper_trail/railtie.rb @@ -3,10 +3,17 @@ require "paper_trail/frameworks/rails/engine" module PaperTrail - # Represents code to load within Rails framework + # Represents code to load within Rails framework. See documentation in + # `rails/railtie.rb`. # @api private class Railtie < ::Rails::Railtie + # PaperTrail only has one initializer. The `initializer` method can take a + # `before:` or `after:` argument, but that's only relevant for railties with + # more than one initializer. initializer "paper_trail" do + # `on_load` is a "lazy load hook". It "declares a block that will be + # executed when a Rails component is fully loaded". (See + # `active_support/lazy_load_hooks.rb`) ActiveSupport.on_load(:action_controller) do require "paper_trail/frameworks/rails/controller" include PaperTrail::Rails::Controller