Skip to content
Merged
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
31 changes: 22 additions & 9 deletions lib/clickwrap/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,16 +307,29 @@ def describe_authentication_with=(value)
@describe_authentication_with = ensure_callable(value, "describe_authentication_with")
end

# The constantized actor class, resolved lazily on first use. Lazy on
# purpose: the initializer that sets `config.actor_class_name = "User"` runs
# before the User model is necessarily loaded.
# The constantized actor class, resolved lazily on EVERY use. Lazy on
# purpose: the initializer that sets `config.actor_class_name = "User"`
# runs before the User model is necessarily loaded.
#
# Not memoized, deliberately. This used to cache the resolved Class object
# and re-resolve only when the class NAME changed — which is never, since
# the name is set once in an initializer. In a Rails app with reloading,
# the class behind that name is replaced on every code change: Zeitwerk
# unloads the old User and defines a new one, same name, different object.
# The cache then held a class no living record was an instance of, and
# `actor.is_a?(config.actor_class)` failed for a perfectly ordinary User.
#
# What that looked like from the outside was worse than the bug: the
# ConfigurationError names the offending class and the configured class,
# and both printed "User". The message read as a contradiction, sent
# people to an initializer that was correct, and came back after every
# edit — the first signup of a dev session worked and the second did not.
#
# `constantize` after the first load is a const_get, and this is called
# once per capture rather than in a loop, so there was nothing to save.
# `parent_controller_class` below has always resolved this way.
def actor_class
name = actor_class_name
if @actor_class.nil? || @actor_class_name_at_resolution != name
@actor_class = name.constantize
@actor_class_name_at_resolution = name
end
@actor_class
actor_class_name.constantize
end

def parent_controller_class = parent_controller_class_name.constantize
Expand Down
29 changes: 29 additions & 0 deletions test/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,35 @@ class ConfigurationTest < ActiveSupport::TestCase
assert_equal User, Clickwrap.config.actor_class
end

# A Rails app with reloading replaces the class behind a name on every code
# change: Zeitwerk unloads the old constant and defines a new one, same
# name, different object. actor_class used to cache the resolved Class and
# re-resolve only when the NAME changed — which never happens, because the
# name is set once in an initializer — so from the second edit of a dev
# session onwards it handed back a class no live record was an instance of.
#
# The symptom was a ConfigurationError that named the same class twice
# ("asked to record User as the actor, but ... the records that can act are
# User"), which reads as a contradiction and points at an initializer that
# is correct.
test "the actor class follows the constant when the app reloads it" do
Clickwrap.config.actor_class_name = "ReloadableActor"
Object.const_set(:ReloadableActor, Class.new)
first = Clickwrap.config.actor_class

# What a reload does, in one line.
Object.send(:remove_const, :ReloadableActor)
Object.const_set(:ReloadableActor, Class.new)

refute_equal first.object_id, Clickwrap.config.actor_class.object_id,
"actor_class handed back the class the app had already thrown away"
assert_equal ReloadableActor, Clickwrap.config.actor_class
assert ReloadableActor.new.is_a?(Clickwrap.config.actor_class),
"a freshly reloaded record is not an instance of the cached class"
ensure
Object.send(:remove_const, :ReloadableActor) if Object.const_defined?(:ReloadableActor)
end

test "an unknown document store is refused by name" do
error = assert_raises(Clickwrap::ConfigurationError) do
Clickwrap.config.store_document_contents_in = :s3
Expand Down
Loading