Skip to content

Release v4.0.0.alpha2#375

Open
mnovelo wants to merge 22 commits into4-0-alphafrom
main
Open

Release v4.0.0.alpha2#375
mnovelo wants to merge 22 commits into4-0-alphafrom
main

Conversation

@mnovelo
Copy link
Copy Markdown
Collaborator

@mnovelo mnovelo commented Apr 10, 2026 •
edited
Loading

Release: v4.0.0.alpha2

Merges main into 4-0-alpha to trigger the gem publish workflow.

What's new since alpha1

PR Change
#368 Tenant.each for iterating tenants
#369 Remove elevator_insert_before; auto-insert after ActionDispatch::Callbacks
#370 Quote schema names in search_path to support hyphens and special chars
#371 Fix stale docs (quoted search_path examples, elevator insertion pattern)
#372 Apartment.tenant_names and .excluded_models convenience methods
#373 Auto-default default_tenant to 'public' for schema strategy
#374 Shared pinned model connections — pinned models on PG schema and MySQL share the tenant's connection pool via qualified table names. New force_separate_pinned_pool config escape hatch. SOLID refactor of Apartment::Model concern.
#376 Validate persistent_schemas as PostgreSQL identifiers; remove dead enforce_search_path_reset
#377 Delete all completed implementation plans (~11.6K lines); fix overview.md dead links
#378 Deferred pin_tenant processing — TracePoint(:end) deferral so self.table_name can appear anywhere in the class body. AR-only guard, anonymous class warning, nil-adapter warning. Rails main CI canary.

Breaking changes (alpha)

Acknowledgments

Special thank you to @henkesn for #367, which identified the pinned model connection pool problem and proposed the shared_connection_supported? / qualify_pinned_table_name architecture. That design became the foundation for #374 — the largest change in this release.

Test plan

  • CI green on main (unit + integration across Ruby 3.3/3.4/4.0 x Rails 7.2/8.0/8.1/main)
  • gem build ros-apartment.gemspec succeeds
  • Merge triggers gem-publish.yml > RubyGems push

mnovelo and others added 11 commits April 9, 2026 10:41
@mnovelo
Remove v3 Rakefile tasks that block rake release
5909bfd 
The postgres: and mysql: namespaces unconditionally required
spec/support/config (deleted in Phase 2.5), causing rake release
to fail with LoadError. These tasks were unused by v4 CI.

Also simplifies db:test:prepare to a no-op (v4 CI handles DB
provisioning in workflow steps, not rake tasks).
@mnovelo @claude
Add Tenant.each and elevator_insert_before (v4 alpha2) (#368)
e81d6ab 
* Add Tenant.each and elevator_insert_before for v4 alpha2

Tenant.each iterates tenants_provider, switching into each tenant
for the block duration. Accepts an optional tenant list override.
Needed by apps migrating from v3 where Tenant.each was available.

elevator_insert_before lets apps position the elevator middleware
before a named middleware (e.g., Warden) instead of appending to
the end of the stack. Without this, apps with auth middleware need
manual move_before hacks with fragile ordering dependencies.

Bumps version to 4.0.0.alpha2.

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

* Add specs for Tenant.each edge cases and pinned model interaction

- Empty list no-op, return value, pinned model bypass inside each
- Unpinned model routes to tenant pool inside each
- Verifies the contract: pinned models always use default pool,
  even when iterated via Tenant.each

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

* Harden Tenant.each and elevator_insert_before after review

Tenant.each:
- Guard tenants_provider returning nil/non-enumerable with
  ConfigurationError instead of opaque NoMethodError
- Guard unconfigured Apartment with ConfigurationError
- Document fail-fast semantics in method comment
- Add specs: nil provider, unconfigured, mid-iteration exception

elevator_insert_before:
- Add type validation in Config#validate! (String/Class/nil)
- Pass insert_before as explicit kwarg to insert_elevator_middleware
  (matches header_trust_warning? pattern of explicit args)
- Wrap RuntimeError from missing middleware as ConfigurationError
  with actionable message
- Add specs: use-path-with-kwargs, Class target, RuntimeError wrapping
- Add default spec in config_spec
- Document in README (quick-reference + Elevators section)

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

* Scope rescue RuntimeError to insert_before path only

The rescue was outside the if/else, so a RuntimeError from
middleware_stack.use (append path) would produce a misleading
"elevator_insert_before: nil not found" message. Now only the
insert_before path is rescued and re-raised as ConfigurationError.

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

* Add spec: use path does not wrap RuntimeError

Locks in the behavior that only insert_before errors get
re-raised as ConfigurationError; the append path propagates
RuntimeError unchanged.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Remove elevator_insert_before; auto-insert after ActionDispatch::Call…
e96abec 
…backs (#369)

* Remove elevator_insert_before; auto-insert before ActionDispatch::Cookies

No popular gem (Rack::Attack, Devise, Sentry) exposes a middleware
positioning config. The Railtie now hardcodes insert_before
ActionDispatch::Cookies — after error handling/logging, before
sessions/auth/DB queries. Users who need different positioning skip
config.elevator and use the standard Rails config.middleware API.

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

* Use insert_after Callbacks instead of insert_before Cookies

ActionDispatch::Cookies is absent in Rails API mode
(config.api_only = true). ActionDispatch::Callbacks is always
present and sits immediately before Cookies in the full stack,
so insert_after Callbacks gives the same position while working
in both modes.

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

* Update docs to reflect insert_after Callbacks positioning

Sync CLAUDE.md, elevators.md, and elevators/CLAUDE.md with the
new auto-insert position (after ActionDispatch::Callbacks).

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Quote schema names in search_path to support hyphens and special chars (
cdb516b 
#370)

Rails passes schema_search_path directly into SET search_path SQL
with no quoting. Tenant names containing hyphens (e.g. test-tenant)
cause a PG::SyntaxError. Double-quote each schema name so
SET search_path TO "test-tenant","ext" parses correctly.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Update RBAC design doc to reflect search_path quoting
95610e7 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Fix stale docs: quoted search_path examples, elevator insertion patte…
74c9435 
…rn (#371)

- Update search_path examples to show quoted schema names
  (matches PR #370: "acme","ext","public" not acme,ext,public)
- Update elevator CLAUDE.md to show v4 config.elevator pattern
  and insert_before for manual positioning (matches PR #369)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Add Apartment.tenant_names and .excluded_models convenience methods (#…
63d9808 
…372)

Thin delegators preserving the v3 public API so upgrading apps don't
need to find-and-replace call sites for a purely mechanical change.

- Apartment.tenant_names -> config.tenants_provider.call
- Apartment.excluded_models -> config.excluded_models (deprecated)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Auto-default default_tenant to 'public' for schema strategy (#373)
765ddb1 
* Auto-default default_tenant to 'public' for schema strategy (#373)

Schema strategy is PostgreSQL-only; 'public' is the standard default
schema. Without this, every PG schema user had to explicitly set
default_tenant = 'public' or Apartment::Tenant.current returned nil
before any switch, cascading into FK violations and nil errors.

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

* Address review feedback: improve docs and comment clarity

- README: keep default_tenant visible in Quick Start, note it's required
  for :database_name but auto-defaults for :schema
- Upgrading guide: remove commented-out lines, add prose explaining the
  behavioral change and that users can remove explicit default_tenant
- Design spec: update config example to reflect auto-defaulting
- config.rb: improve comment to explain rationale, not restate code

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

* Remove dead fallback in migrator; validate against empty default_tenant

- migrator.rb: remove `|| 'public'` fallback now that validate!
  guarantees a non-nil default_tenant for schema strategy
- config.rb: reject empty/whitespace-only default_tenant strings
- Two new specs for the empty string validation

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

* Document non-public global schema and nil→'public' behavioral change

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

* Remove misleading nil→public migration note

The nil default_tenant was a v4 regression, not v3 behavior. v3 already
returned 'public' by default, so no one migrating would have relied on
nil state.

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

* Add comment explaining strategy choice in CLI tenants spec

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude @henkesn
Share tenant connection for pinned models on PG schema and MySQL (#367)…
8566f76 
… (#374)

* Add design doc for shared pinned model connections

Covers the dual-path approach for pinned model connection strategy:
shared connections (qualified table names) where the engine supports
cross-schema/database queries, separate pools otherwise. Includes
force_separate_pinned_pool config opt-out and FK constraint fix.

Incorporates architectural approach from #367.

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

* Tighten shared pinned connections design doc

Address review feedback: ivar rename touchpoints, adapter matrix
formulas, qualification edge cases (table_name_prefix, raw SQL,
identifier quoting), schema_cache_per_tenant interaction, connects_to
warning, after_commit nuance, FK DDL-vs-DML precision.

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

* Add hybrid table name qualification strategy to design doc

Use Rails table_name_prefix + reset_table_name for convention-named
models; fall back to direct table_name= for models with explicit
self.table_name. Covers STI inheritance, class_attribute propagation,
clear_config teardown, and new test cases for both paths.

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

* Address review gaps in shared pinned connections design

- Robust explicit_table_name? detection via compute_table_name comparison
  instead of bare @table_name ivar check
- sub() prefix stripping instead of split('.').last (single-segment only)
- Teardown tracks qualification path and original table name for restore
- STI wording: only STI subclasses of pinned bases supported
- Note base_config already returns string keys
- Grep note for ivar rename completeness

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

* Final spec polish: edge case note, teardown ivars, explicit_table_name? test

- Document cached==computed edge case (convention path still correct)
- List new ivars in clear_config teardown obligations
- Add explicit_table_name? unit test to testing section

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

* Add implementation plan for shared pinned model connections

10 tasks: config option, template methods on AbstractAdapter,
PG schema + MySQL adapter implementations, dual-path process_pinned_model,
clear_config teardown, ConnectionHandling routing, integration tests,
upgrade guide, and cross-version verification.

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

* Fix plan gaps: mock_adapter default + Tenant.each split

- Add shared_pinned_connection?: false to default mock_adapter double
  so existing tests keep working after ConnectionHandling change
- Split 'pinned model inside Tenant.each' context into shared vs
  separate pool behaviors

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

* Add force_separate_pinned_pool config option

New boolean (default false) on Apartment::Config. Escape hatch for
multi-server MySQL topologies and apps that rely on pinned model
writes surviving tenant transaction rollbacks.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add shared_pinned_connection?, qualify_pinned_table_name, explicit_table_name?

Template methods on AbstractAdapter for the dual-path pinned model
strategy. shared_pinned_connection? returns false by default (safe
fallback). qualify_pinned_table_name raises NotImplementedError as
guard. explicit_table_name? detects models with self.table_name =.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix rubocop offense in qualify_pinned_table_name spec

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

* Implement shared_pinned_connection? and qualify_pinned_table_name for PG schema

PostgresqlSchemaAdapter returns true for shared_pinned_connection?
(schemas share a catalog). qualify_pinned_table_name uses hybrid
strategy: table_name_prefix for convention-named models, direct
table_name= for explicit self.table_name assignments.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Implement shared_pinned_connection? and qualify_pinned_table_name for MySQL

Mysql2Adapter returns true for shared_pinned_connection? (MySQL
supports cross-database queries). qualify_pinned_table_name uses
base_config['database'] as qualifier. TrilogyAdapter inherits both.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Rewrite process_pinned_model with dual-path logic

Shared path: qualify table name, skip establish_connection.
Separate path: establish_connection with pinned_model_config
(includes schema_search_path for PG schema strategy FK fix).
Ivar renamed to @apartment_pinned_processed.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update clear_config teardown for new pinned model ivars

Restores table name qualification on clear_config: convention path
resets table_name_prefix + reset_table_name; explicit path restores
original table_name. Cleans up all three ivars (@apartment_pinned_processed,
@apartment_qualification_path, @apartment_original_table_name).

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Note: do not environmentify pinned model qualifiers

environmentify maps tenant keys to database names; the default
connection's identifiers (default_tenant, base_config['database'])
are already resolved and must not be environmentified.

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

* Conditionally route pinned models through tenant pool

When shared_pinned_connection? is true, pinned models fall through
to the tenant pool lookup instead of short-circuiting to the default
pool. Preserves transactional integrity between pinned and tenant
model writes.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update integration tests for dual-path pinned model behavior

Separate context blocks for shared vs separate pool. Add transactional
integrity tests: rollback rolls back both pinned and tenant writes
when shared connections are supported.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update upgrade guide for shared pinned connections

Add Pinned Model Connections section to upgrading-to-v4.md with
adapter matrix, force_separate_pinned_pool escape hatch, and
after_commit nuance.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address PR review: ivar ordering, teardown robustness, error context, CI canary

Critical fixes:
- Reorder ivars in qualify_pinned_table_name to set after mutations
  succeed (prevents inconsistent state on failure)
- Save original table_name_prefix on convention path for correct
  restoration in clear_config
- Add clear_config teardown tests (convention, explicit, separate-pool)

Important fixes:
- Add else branch with warning in clear_config for unexpected
  qualification_path values
- Make adapter nil check explicit in ConnectionHandling (was relying
  on !nil == true)
- Wrap process_pinned_models with model-identifying error context
- Fix shared_pinned_connection? base class comment (was inaccurate)
- Drop "single-server" from MySQL wording (code doesn't detect topology)

Suggestions:
- Add explicit_table_name? edge case note in comment
- Add Rails main to CI unit test matrix as canary for compute_table_name
  private API (continue-on-error, all Ruby versions)
- Add ivar ordering and original prefix tests for PG schema adapter
- Update design doc status to Implemented

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix rubocop Layout/FirstArgumentIndentation in abstract_adapter_spec

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

* Encapsulate pinned model state in Apartment::Model concern

Move ivar management from external callers into the concern itself:
- apartment_pinned_processed? / apartment_mark_processed! / apartment_restore!
- Eliminates all instance_variable_get/set/defined?/remove_instance_variable
  calls from abstract_adapter.rb, adapters, and apartment.rb
- clear_config delegates to klass.apartment_restore! (one line)
- Guards against models registered without the concern (excluded_models shim)

SOLID principle: the class owns its own state; callers use the public API.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Delegate pinned_model? and move explicit_table_name? onto concern

- pinned_model? now delegates to klass.apartment_pinned? when available,
  falling back to registry walk for excluded_models shim compatibility
- explicit_table_name? moved from AbstractAdapter (private, ivar-poking)
  to Apartment::Model as apartment_explicit_table_name? — the model
  answers its own question about its table name
- Removes all instance_variable_defined?/get calls from adapter code;
  adapters call klass.apartment_explicit_table_name? directly

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: move SOLID/style guidance to CLAUDE.md; document Model APIs

AGENTS.md points at root CLAUDE.md for code style. Root and
lib/apartment/CLAUDE.md document apartment_pinned?,
apartment_explicit_table_name?, and adapter/concern boundaries.

Made-with: Cursor

* Update CLAUDE.md: CI matrix includes Rails main, drop stale spec count

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

* Fix excluded_models shim: include concern + mark pinned in process_pinned_model

Models registered via excluded_models shim or Apartment.register_pinned_model
don't include Apartment::Model. process_pinned_model now includes the concern
and calls apartment_mark_pinned! (avoids pin_tenant recursion) so the model
responds to all concern methods and is recognized as pinned by ConnectionHandling.

Co-Authored-By: henkesn <14108170+henkesn@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add shim regression test + CLAUDE.md note on dynamic include

Unit test: shim-registered model without concern gets Apartment::Model
included and pinned flag set by process_pinned_model.
CLAUDE.md: documents apartment_mark_pinned! and shim compatibility.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: henkesn <14108170+henkesn@users.noreply.github.com>
@mnovelo
Merge branch '4-0-alpha' into main
3303339
@mnovelo @claude
Remove continue-on-error from Rails main CI canary
f1b6fe2 
A canary that can't fail isn't a canary. Rails main failures should
surface as real CI failures so we know when compute_table_name or
other private APIs break.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo mnovelo mentioned this pull request Apr 10, 2026
Merged
mnovelo and others added 11 commits April 10, 2026 12:46
@mnovelo @claude
Validate persistent_schemas; remove dead enforce_search_path_reset (#376
c30242b 
)

persistent_schemas values flow into schema_search_path but were never
validated. PostgresqlConfig#validate! now checks entries using the same
TenantNameValidator rules as tenant names.

Remove enforce_search_path_reset — v4 uses pool-per-tenant with
schema_search_path baked into connection config. There is no runtime
SET search_path to reset. Dead code from v3 mental model.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Delete all completed implementation plans (#377)
fedad6f 
* Delete completed plan: shared-pinned-connections

All 10 tasks implemented and merged in #374. Plan served its purpose
as the agent execution checklist; no remaining items.

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

* Delete all completed phase plans (1-8)

Phases 1-8 merged in PRs #345-366. Only overview.md retained as
roadmap reference. ~340KB of completed checklists removed.

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

* Fix overview.md dead links and stale version strategy

Replace plan file link with PR references. Replace version strategy
section with implementation history showing actual release timeline.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Add design doc for deferred pin_tenant processing
415ab84 
TracePoint(:end) deferral so pin_tenant doesn't process the model
before self.table_name is evaluated. Fixes silent qualification
bug under eager_load=false + Zeitwerk lazy loading.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Add implementation plan for deferred pin_tenant processing
3c677b3 
5 tasks: TracePoint deferral in pin_tenant, unit tests for deferred
behavior + raise cleanup, integration test mirroring CampusESP
failure, documentation updates, full verification.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @henkesn @claude
Defer pin_tenant processing until class body closes (#378)
e274662 
* Defer pin_tenant processing via TracePoint(:end, :raise)

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>

* Update model_spec for deferred pin_tenant processing

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>

* Design doc: note :b_return needed for Class.new blocks

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>

* Add integration test for deferred pin_tenant qualification

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>

* Document deferred pin_tenant processing

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>

* Fix TracePoint: use only :end, drop :b_return and :raise

: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>

* Design doc: note TracePoint coexistence — no interference with user traces

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>

* Align design doc, comments, and PR description with :end-only implementation

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>

* Add defensive guards: AR-only pin_tenant, anonymous class warning, nil 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>

* Sync design doc, document Class.new gotcha, add guard tests

- 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>

* Final doc sync: CLAUDE.md guards section, design doc testing section, 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>

* Polish: fix design doc Class.new wording, integration test title, constant 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>

---------

Co-authored-by: henkesn <14108170+henkesn@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Recommend self.table_name as first line (Rails convention)
887db7e 
The deferral removes the ordering requirement, but the standard Rails
convention of declaring self.table_name at the top of the class body
is still best practice for readability.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Include table_name_prefix/suffix in deferral docs; soften readability…
6395748 
… note

No official Rails convention for self.table_name placement exists
(verified via Rails guides, RuboCop Layout/ClassStructure, and
community style guides). Recommend "early for readability" without
claiming a convention that doesn't exist. Also mention table_name_prefix
and table_name_suffix coverage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Strengthen readability recommendation: table name config first, befor…
87e2259 
…e includes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Fix ArgumentError from TestFixtures setup_shared_connection_pool (#379)
7b8071f 
* Add test_fixture_cleanup config attribute (default: true)

* Add Apartment::TestFixtures to deregister tenant pools before fixture setup

Rails' setup_shared_connection_pool assumes every registered shard has a
:writing pool_config; Apartment's role-keyed tenant shards violate this,
raising ArgumentError. This module strips tenant pools from the
ConnectionHandler and PoolManager before super runs, with a re-entrant
guard to prevent double-cleanup from AR's notification subscriber.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Wire TestFixtures cleanup via Railtie on_load(:active_record_fixtures)

* Add design doc and implementation plan for TestFixtures compatibility

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

* Address PR review: add config validation, extract public reset_tenant_pools!

- Add boolean validation for test_fixture_cleanup in Config#validate!
  (matches pattern of all other boolean config attributes)
- Extract public Apartment.reset_tenant_pools! method encapsulating the
  three-step cleanup (deregister + clear + reset)
- Update TestFixtures to call the public API instead of send(:deregister_all_tenant_pools)

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

* Restore Railtie on_load(:active_record_fixtures) hook

The previous review-fix commit (8fabf00) accidentally reverted the
Railtie wiring added in 35eda8c. The hook was lost because the
working tree had a stale railtie.rb when the review fixes were staged.

Without this hook, test_fixture_cleanup config is validated but never
read, and downstream apps don't get the automatic cleanup.

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

* Address remaining review items

- Restore config_spec default assertion for test_fixture_cleanup
  (lost in external file modification)
- Add comment explaining re-entry test is a unit-level approximation
  of the !connection.active_record subscriber scenario
- Improve YARD doc on reset_tenant_pools! for discoverability

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@mnovelo @claude
Fix TestFixtures subscriber re-entry crash (Trigger B) (#380)
90e63f1 
* Fix TestFixtures subscriber re-entry crash (Trigger B)

PR #379 guarded cleanup but still called super on re-entry.
When the !connection.active_record subscriber fires mid-test
(after the elevator creates a tenant pool), super iterates
shard_names and crashes on the apartment shard that has no
:writing pool_config.

Fix: return early (skip both cleanup AND super) when the guard
is set. Apartment pools should not participate in Rails'
writing/reading pool config swap; the subscriber still pins and
leases the connection independently.

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

* Clarify guard comment: first call vs re-entry behavior

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo @claude
Document cross-tenant fixture limitation; delete completed plans (#381)
38f1415 
* Document cross-tenant transactional fixture limitation; delete completed plans

Add testing guide to v4-test-fixtures-compatibility.md explaining why
pool-per-tenant breaks transactional fixtures for cross-tenant specs,
why pinned models don't fix transaction visibility, and the recommended
per-context use_transactional_tests = false strategy.

Delete completed plan files (elevators, migrations, railtie-test-infra,
deferred-pin-tenant, test-fixtures-compatibility) — design docs are the
permanent reference.

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

* Address review feedback: reconcile Trigger B with #380, soften scope

- Trigger B step 5: describe post-#380 behavior (early return), note
  pre-#380 behavior parenthetically
- Scope section: clarify all strategies have cross-connection visibility
  constraints, not just PG schema
- Add DatabaseCleaner note to cleanup guidance
- Replace brittle line number reference with descriptive text

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mnovelo