Skip to content

Defer pin_tenant processing until class body closes#378

Merged
mnovelo merged 12 commits intomainfrom
feat/deferred-pin-tenant
Apr 10, 2026
Merged

Defer pin_tenant processing until class body closes#378
mnovelo merged 12 commits intomainfrom
feat/deferred-pin-tenant

Conversation

@mnovelo
Copy link
Copy Markdown
Collaborator

@mnovelo mnovelo commented Apr 10, 2026

Summary

Fixes silent table name qualification bug when pin_tenant is called before self.table_name in a class body under Zeitwerk lazy loading (eager_load = false).

pin_tenant now defers process_pinned_model via a one-shot TracePoint(:end) constrained to Thread.current. Processing runs after the class body's closing end keyword, so self.table_name and other declarations are visible regardless of ordering.

Problem

class EngagementReport < ApplicationRecord
  include Apartment::Model
  pin_tenant                          # processes immediately, sees no @table_name
  # ... associations, scopes ...
  self.table_name = 'reports'         # too late — qualification already happened
end

Under eager_load = false + Zeitwerk, pin_tenant fired process_pinned_model immediately. The gem saw no explicit table name, took the convention path, qualified as public.engagement_reports. Then self.table_name = 'reports' silently overwrote the qualification. Queries hit the wrong schema.

How it works

  • Only TracePoint(:end) is used — fires for source-parsed class Foo ... end (Zeitwerk files)
  • :b_return rejected: fires for ALL block returns in class context (each, tap, include hooks), not just Class.new closing — would trigger premature processing
  • :raise rejected: rescued raises (begin/rescue in class body) still produce :end; unconditional disable would prevent processing on successful load. MRI verified: :end fires even when the body raises (event order [:raise, :end])
  • target_thread: Thread.current constrains to the loading thread
  • Multiple TracePoints coexist independently; disabling ours does not affect user traces
  • process_pinned_model wrapped with begin/rescue/warn/raise for error context

Defensive guards

  • pin_tenant raises ArgumentError if called on a non-AR class or module
  • Anonymous classes (Class.new): warns that :end won't fire, skips deferral. Use class MyModel < ApplicationRecord or call process_pinned_model explicitly after assigning the constant
  • Apartment.process_pinned_model warns when adapter is nil instead of silently returning nil

Changes (8 files, +186 / -28)

File Change
lib/apartment/concerns/model.rb apartment_defer_processing! with TracePoint(:end), AR guard, anonymous class warning
lib/apartment.rb process_pinned_model nil-adapter warning
spec/unit/concerns/model_spec.rb 6 new tests: deferred, source-parsed, table_name ordering, AR guard, module guard, anonymous warning
spec/unit/tenant_spec.rb Fix test model to inherit from AR::Base (for new guard)
spec/integration/v4/excluded_models_spec.rb Qualification outcome tests with real adapters
docs/upgrading-to-v4.md No ordering requirement; Class.new + constant assignment gotcha
lib/apartment/CLAUDE.md Guards + deferred processing documentation
docs/designs/v4-deferred-pin-tenant-processing.md Aligned with :end-only implementation

Test plan

  • Unit tests: 668 examples, 0 failures
  • Appraisal (all Rails versions): 0 failures across 14 gemfiles
  • Integration (SQLite): 0 failures
  • Rubocop clean

🤖 Generated with Claude Code

mnovelo and others added 12 commits April 10, 2026 14:30
When activated? is true, pin_tenant now registers a one-shot
TracePoint that fires after the class body closes, ensuring
self.table_name and other declarations are visible. :raise
handling disables unconditionally to prevent trace leaks.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test deferred processing via TracePoint (class body closes before
process_pinned_model runs). Test :raise cleanup (trace disabled on
class body failure, no leak). Existing non-activated path unchanged.

Also adds :b_return to TracePoint events in model.rb so Class.new {}
blocks trigger deferred processing (matches real class...end behavior).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TracePoint(:end) only fires for source-parsed class/module keywords.
Class.new { } blocks fire :b_return instead. Implementation listens
for both to handle Zeitwerk-loaded files and anonymous classes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirrors the CampusESP pattern: pin_tenant above self.table_name.
Verifies qualification uses the explicit table name (not convention)
regardless of declaration order. Adapter-aware assertions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Upgrade guide: no ordering requirement between pin_tenant and
self.table_name. CLAUDE.md: TracePoint deferral mechanism with
:end/:b_return/:raise, thread safety, reopen edge case.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
:b_return fires for ALL block returns in class context (each, tap,
include hooks), not just Class.new closing — triggers premature
processing before self.table_name is evaluated. Confirmed by MRI
testing and Ruby docs (":b_return: block ending" vs ":end: finish
a class or module definition").

:raise fires for rescued exceptions too (begin/rescue in class body).
Unconditional disable prevents :end from running on successful load.
:end always fires for source-parsed class/module, even on unrescued
raise (MRI verified: event order is [:raise, :end]).

For Class.new { } (tests), :end does not fire. Tests use eval'd
source-parsed classes or call process_pinned_model explicitly.

Extracted apartment_defer_processing! private method from pin_tenant
to stay within Metrics/MethodLength.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…races

Multiple TracePoints coexist independently in MRI. Disabling ours
does not affect other traces. Both fire for the same :end event.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ntation

Design doc: rewrite code sketch, TracePoint section, and raise
handling to match :end-only. Remove obsolete :b_return and :raise
mitigation sections. Document why each was rejected.

model.rb comment: clarify :end fires even on raise (event order
[:raise, :end]), add Class.new note.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l adapter warning

- pin_tenant raises ArgumentError if called on non-AR class or module
- pin_tenant warns and skips TracePoint for anonymous classes (Class.new)
  where :end does not fire
- Apartment.process_pinned_model warns when adapter is nil instead of
  silently returning nil via safe navigation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Design doc code sample now shows ArgumentError guard, name.nil?
  check, and marks sketch as abbreviated
- Upgrade guide documents Class.new + constant assignment pattern
  (call process_pinned_model explicitly)
- Unit tests for: ArgumentError on non-AR class, ArgumentError on
  module, anonymous class warning when activated

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… PR description

- CLAUDE.md: add Guards paragraph (ArgumentError on non-AR, anonymous warning)
- Design doc: update Testing section to match actual tests (remove stale
  :raise test, add guard tests)
- PR description: final file count, test count, defensive guards section

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…stant spelling

- Design doc: soften Class.new + raise wording; note name.nil? guard
  makes the :b_return leak path unreachable for named classes
- Integration test: rename "pin_tenant anywhere" to "via process_pinned_model"
  (matches what the test actually does)
- Spec: TracepointTestModel → TracePointTestModel (match Ruby class name)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo mnovelo merged commit e274662 into main Apr 10, 2026
13 checks passed
@mnovelo mnovelo deleted the feat/deferred-pin-tenant branch April 10, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant