Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop RequestStore in favour of ActiveSupport::CurrentAttributes #1447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/paper_trail/frameworks/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
92 changes: 39 additions & 53 deletions lib/paper_trail/request.rb
Original file line number Diff line number Diff line change
@@ -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`.
Expand All @@ -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`.
Expand All @@ -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
Expand All @@ -49,42 +45,37 @@ 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
# request, `false` otherwise.
# @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
Expand All @@ -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
28 changes: 28 additions & 0 deletions lib/paper_trail/request/current_attributes.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 0 additions & 3 deletions paper_trail.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions spec/controllers/widgets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current attributes are automatically reset before each request so this is no longer necessary for the new implementation.


describe "#create" do
context "with PT enabled" do
it "stores information like IP address in version" do
Expand Down
29 changes: 29 additions & 0 deletions spec/paper_trail/request/current_attributes_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading