From d4905e544b49f00db7568a0f775b13fa60533356 Mon Sep 17 00:00:00 2001 From: Javi R <4920956+rameerez@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:15:26 +0100 Subject: [PATCH] actor_class followed the constant it cached, not the one the app has MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from a Rails app in development: every signup after the first code change of the session failed with Clickwrap was asked to record User as the actor, but `config.actor_class_name` says the records that can act are User. Both halves name the same class, which reads as a contradiction and sends whoever hits it to an initializer that is correct. They were two different Ruby objects printing the same name. `actor_class` cached the constantized Class and re-resolved only when the class NAME changed — which never happens, because the name is set once in an initializer. Reloading replaces the class behind the name on every edit: Zeitwerk unloads the old User and defines a new one. From the second request of a dev session onwards the cache held a class no living record was an instance of, so `actor.is_a?(config.actor_class)` was false for an ordinary User and capture refused to record it. The cache is gone. `constantize` after the first load is a const_get, and this is called once per capture rather than in a loop, so it was never buying anything; `parent_controller_class` two lines below has always resolved this way. The test reloads a constant the way Rails does — remove_const, const_set — and fails against the old implementation. Found the hard way in a host app, where the tell was that a restart fixed it and the next edit broke it again. --- lib/clickwrap/configuration.rb | 31 ++++++++++++++++++++++--------- test/configuration_test.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/lib/clickwrap/configuration.rb b/lib/clickwrap/configuration.rb index d0a7222..3f91aea 100644 --- a/lib/clickwrap/configuration.rb +++ b/lib/clickwrap/configuration.rb @@ -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 diff --git a/test/configuration_test.rb b/test/configuration_test.rb index f04d748..e6ca203 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -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