From c43279b7d32ad18dc52732eef96333353364b063 Mon Sep 17 00:00:00 2001 From: Joshua Young Date: Thu, 16 Nov 2023 18:52:09 +0900 Subject: [PATCH] Drop `RequestStore` in favour of `ActiveSupport::CurrentAttributes` --- lib/paper_trail/frameworks/rails/railtie.rb | 7 ++ lib/paper_trail/request.rb | 92 ++++++++----------- lib/paper_trail/request/current_attributes.rb | 28 ++++++ paper_trail.gemspec | 3 - spec/controllers/widgets_controller_spec.rb | 2 - .../request/current_attributes_spec.rb | 29 ++++++ 6 files changed, 103 insertions(+), 58 deletions(-) create mode 100644 lib/paper_trail/request/current_attributes.rb create mode 100644 spec/paper_trail/request/current_attributes_spec.rb diff --git a/lib/paper_trail/frameworks/rails/railtie.rb b/lib/paper_trail/frameworks/rails/railtie.rb index 2d93672b..77b9b852 100644 --- a/lib/paper_trail/frameworks/rails/railtie.rb +++ b/lib/paper_trail/frameworks/rails/railtie.rb @@ -26,5 +26,12 @@ class Railtie < ::Rails::Railtie require "paper_trail/frameworks/active_record" end end + + # ActiveSupport::CurrentAttributes resets all attributes before and after each request. + # Resetting after a request breaks backwards compatibility with the previous RequestStore + # implementation. This initializer ensures this reset is skipped. + initializer "active_support.reset_all_current_attributes_instances" do |app| + app.executor.to_run { PaperTrail::Request::CurrentAttributes.skip_reset = true } + end end end diff --git a/lib/paper_trail/request.rb b/lib/paper_trail/request.rb index dbc4e873..da084c69 100644 --- a/lib/paper_trail/request.rb +++ b/lib/paper_trail/request.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "request_store" +require "paper_trail/request/current_attributes" module PaperTrail # Manages variables that affect the current HTTP request, such as `whodunnit`. @@ -20,9 +20,7 @@ class << self # PaperTrail.request.controller_info # => { ip: '127.0.0.1' } # # @api public - def controller_info=(value) - store[:controller_info] = value - end + delegate :controller_info=, to: :current_attributes # Returns the data from the controller that you want PaperTrail to store. # See also `PaperTrail::Rails::Controller#info_for_paper_trail`. @@ -31,9 +29,7 @@ def controller_info=(value) # PaperTrail.request.controller_info # => { ip: '127.0.0.1' } # # @api public - def controller_info - store[:controller_info] - end + delegate :controller_info, to: :current_attributes # Switches PaperTrail off for the given model. # @api public @@ -49,22 +45,20 @@ def enable_model(model_class) # Sets whether PaperTrail is enabled or disabled for the current request. # @api public - def enabled=(value) - store[:enabled] = value - end + delegate :enabled=, to: :current_attributes # Returns `true` if PaperTrail is enabled for the request, `false` otherwise. # See `PaperTrail::Rails::Controller#paper_trail_enabled_for_controller`. # @api public def enabled? - !!store[:enabled] + !!current_attributes.enabled end # Sets whether PaperTrail is enabled or disabled for this model in the # current request. # @api public def enabled_for_model(model, value) - store[:"enabled_for_#{model}"] = value + current_attributes.enabled_for[model] = value end # Returns `true` if PaperTrail is enabled for this model in the current @@ -72,19 +66,16 @@ def enabled_for_model(model, value) # @api public def enabled_for_model?(model) model.include?(::PaperTrail::Model::InstanceMethods) && - !!store.fetch(:"enabled_for_#{model}", true) + !!(current_attributes.enabled_for[model] || + current_attributes.enabled_for[model].nil?) end # Temporarily set `options` and execute a block. # @api private - def with(options) - return unless block_given? - validate_public_options(options) - before = to_h - merge(options) - yield - ensure - set(before) + def with(options, &block) + validate_public_options!(options) + transform_public_options!(options) + current_attributes.set(options, &block) end # Sets who is responsible for any changes that occur during request. You @@ -97,67 +88,62 @@ def with(options) # inserting a `Version` record. # # @api public - def whodunnit=(value) - store[:whodunnit] = value - end + delegate :whodunnit=, to: :current_attributes - # Returns who is reponsible for any changes that occur during request. + # Returns who is responsible for any changes that occur during request. # # @api public def whodunnit - who = store[:whodunnit] + who = current_attributes.whodunnit who.respond_to?(:call) ? who.call : who end private + # Returns the current request attributes with default values initialized if necessary. # @api private - def merge(options) - options.to_h.each do |k, v| - store[k] = v + def current_attributes + CurrentAttributes.tap do |attrs| + attrs.enabled = true if attrs.enabled.nil? end end - # @api private - def set(options) - store.clear - merge(options) - end - - # Returns a Hash, initializing with default values if necessary. - # @api private - def store - RequestStore.store[:paper_trail] ||= { - enabled: true - } - end - - # Returns a deep copy of the internal hash from our RequestStore. Keys are + # Returns a deep copy of the current attributes. Keys are # all symbols. Values are mostly primitives, but whodunnit can be a Proc. # We cannot use Marshal.dump here because it doesn't support Proc. It is # unclear exactly how `deep_dup` handles a Proc, but it doesn't complain. # @api private def to_h - store.deep_dup + current_attributes.attributes.except(:skip_reset).deep_dup end # Provide a helpful error message if someone has a typo in one of their # option keys. We don't validate option values here. That's traditionally # been handled with casting (`to_s`, `!!`) in the accessor method. # @api private - def validate_public_options(options) - options.each do |k, _v| - case k - when :controller_info, - /enabled_for_/, - :enabled, - :whodunnit + def validate_public_options!(options) + options.keys.each do |key| + case key + when :enabled, + /^enabled_for_/, + :controller_info, + :whodunnit next else - raise InvalidOption, "Invalid option: #{k}" + raise InvalidOption, "Invalid option: #{key}" end end end + + # Transform public options into internal attributes. + # @api private + def transform_public_options!(options) + options[:enabled_for] = current_attributes.enabled_for.deep_dup + options.keys.grep(/^enabled_for_/).each do |key| + model_klass = key.to_s.sub("enabled_for_", "").constantize + options[:enabled_for][model_klass] = options.delete(key) + end + end end end end diff --git a/lib/paper_trail/request/current_attributes.rb b/lib/paper_trail/request/current_attributes.rb new file mode 100644 index 00000000..9492addb --- /dev/null +++ b/lib/paper_trail/request/current_attributes.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module PaperTrail + module Request + # Thread-isolated attributes for managing the current request. + class CurrentAttributes < ActiveSupport::CurrentAttributes + attribute :enabled + attribute :enabled_for + attribute :controller_info + attribute :whodunnit + attribute :skip_reset + + def enabled_for + super || self.enabled_for = {} + end + + def controller_info + super || self.controller_info = {} + end + + # Overrides ActiveSupport::CurrentAttributes#reset to support skipping. + def reset + return if skip_reset + super + end + end + end +end diff --git a/paper_trail.gemspec b/paper_trail.gemspec index 8c757c70..ab119829 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -49,9 +49,6 @@ has been destroyed. # See discussion in paper_trail/compatibility.rb s.add_dependency "activerecord", ::PaperTrail::Compatibility::ACTIVERECORD_GTE - # PT supports request_store versions for 3 years. - s.add_dependency "request_store", "~> 1.4" - s.add_development_dependency "appraisal", "~> 2.4.1" s.add_development_dependency "byebug", "~> 11.1" s.add_development_dependency "ffaker", "~> 2.20" diff --git a/spec/controllers/widgets_controller_spec.rb b/spec/controllers/widgets_controller_spec.rb index ae24ff42..ef97f7ac 100644 --- a/spec/controllers/widgets_controller_spec.rb +++ b/spec/controllers/widgets_controller_spec.rb @@ -5,8 +5,6 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do before { request.env["REMOTE_ADDR"] = "127.0.0.1" } - after { RequestStore.store[:paper_trail] = nil } - describe "#create" do context "with PT enabled" do it "stores information like IP address in version" do diff --git a/spec/paper_trail/request/current_attributes_spec.rb b/spec/paper_trail/request/current_attributes_spec.rb new file mode 100644 index 00000000..6e191f58 --- /dev/null +++ b/spec/paper_trail/request/current_attributes_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "spec_helper" + +module PaperTrail + module Request + ::RSpec.describe CurrentAttributes.new do + describe ".enabled_for" do + context "when enabled_for is nil" do + it "sets enabled_for to an empty hash to and returns it" do + expect(described_class.attributes[:enabled_for]).to be_nil + expect(described_class.enabled_for).to eq({}) + expect(described_class.attributes[:enabled_for]).to eq({}) + end + end + end + + describe ".controller_info" do + context "when controller_info is nil" do + it "sets controller_info to an empty hash to and returns it" do + expect(described_class.attributes[:controller_info]).to be_nil + expect(described_class.controller_info).to eq({}) + expect(described_class.attributes[:controller_info]).to eq({}) + end + end + end + end + end +end