From aa3d8d6d9174bad32a18869e1ed9447b50eefaa8 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 3 Jun 2026 15:29:42 +0200 Subject: [PATCH] DRAFT: retire parent_data via replayed-model recording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exploration / alternative to #61. Instead of lazily reading the parent's cached content when a child is saved, remove the reason the child reads it: the parent_data parameter. ActiveRecordCoder#mount now records the models it replays into a per-generate @replayed_models set; ActiveRecordCoder#generate folds that set into its captured models after the block runs. This works because the runner reuses one coder instance and the parent is mounted inside the child's generate block. Cache#evaluate no longer reads parent.cache.content, and the Coder contract drops the parent_data keyword. I lean against merging this — it trades an explicit parameter for hidden, ordering-dependent instance state and changes the public Coder contract, without buying a real simplification (per-model INSERT tagging would require de-batching the one-execute_batch-per-connection call a spec pins). #66 documents why parent_data stays. Opened as a draft so the alternative is concrete. Includes a propagation test matrix (basic cross-process, STI collapse, second connection pool, delete-only nil sql) that drives the real parent.mount -> @replayed_models path, and rewrites the coder's old parent_data merge spec against the new mechanism. --- docs/reference.md | 5 +- lib/fixture_kit/cache.rb | 6 +- lib/fixture_kit/coder.rb | 2 +- lib/fixture_kit/coders/active_record_coder.rb | 19 +++- spec/unit/coders/active_record_coder_spec.rb | 15 ++- spec/unit/fixture_cache_spec.rb | 105 ++++++++++++++++++ 6 files changed, 140 insertions(+), 12 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 4a78518..191b6a2 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -188,14 +188,15 @@ Default values: Subclass `FixtureKit::Coder` and implement: -`#generate(parent_data: nil, &block)` +`#generate(&block)` - Called once when fixture cache is being built. - Set up observation, then call the block to evaluate the user's fixture definition (and any inner coders). - Return data to be cached for this coder. Will be passed to `#encode` before serialization. -- `parent_data` is the cached data from the same coder on the parent fixture when `extends:` is used; `nil` otherwise. +- For inherited fixtures (`extends:`), the parent is mounted via `#mount` inside the block. A coder that needs the parent's contribution should record what it replays during `#mount` and fold it into its result here (see `ActiveRecordCoder`). The previous `parent_data:` keyword has been removed. `#mount(data)` - Called once per test mount with the data this coder produced. Re-create the state on the test database. +- Also runs while a child fixture is being generated (the parent is mounted inside the child's `#generate` block). Coders that need their replayed models reflected in the child cache record them here. `#encode(data)` - Convert the in-memory representation produced by `#generate` to a JSON-serializable form. Default: identity. diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index c31efba..1a9f4cf 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -68,8 +68,10 @@ def evaluate(coders, context, data = {}, &block) else coder, *remaining_coders = coders - parent_data = fixture.parent ? fixture.parent.cache.content.data_for(coder.class) : nil - data[coder.class] = coder.generate(parent_data: parent_data) do + # No parent_data lookup: the parent's models are recaptured during + # parent.mount (called in the leaf branch above) by the coder itself. + # See ActiveRecordCoder#generate / #mount. + data[coder.class] = coder.generate do evaluate(remaining_coders, context, data, &block) end end diff --git a/lib/fixture_kit/coder.rb b/lib/fixture_kit/coder.rb index 94e3212..91b05ba 100644 --- a/lib/fixture_kit/coder.rb +++ b/lib/fixture_kit/coder.rb @@ -2,7 +2,7 @@ module FixtureKit class Coder - def generate(parent_data: nil, &block) + def generate(&block) raise NotImplementedError, "#{self.class} must implement #generate" end diff --git a/lib/fixture_kit/coders/active_record_coder.rb b/lib/fixture_kit/coders/active_record_coder.rb index 31b9196..508a1e5 100644 --- a/lib/fixture_kit/coders/active_record_coder.rb +++ b/lib/fixture_kit/coders/active_record_coder.rb @@ -8,7 +8,14 @@ class ActiveRecordCoder < FixtureKit::Coder EVENT = "sql.active_record" NAME_PATTERN = /\A(?.+?) (?:(?:Bulk )?(?:Insert|Upsert)|Create|Destroy|(?:Update|Delete)(?: All)?)\z/ - def generate(parent_data: nil, &block) + def generate(&block) + # DRAFT: instead of taking parent_data, we recapture the parent's models + # from #mount, which runs (via parent.mount) inside &block. This relies on + # the runner reusing one coder instance across generate and mount, and on + # the ordering "mount happens inside this block". Reset per generate so a + # previous run's models don't leak in. + @replayed_models = Set.new + captured_models = Set.new subscriber = lambda do |_event_name, _start, _finish, _id, payload| name = payload[:name].to_s @@ -24,12 +31,20 @@ def generate(parent_data: nil, &block) ActiveSupport::Notifications.subscribed(subscriber, EVENT, monotonic: true, &block) captured_models.map! { |model| base_table_model(model) } - captured_models.merge(parent_data.keys) if parent_data + captured_models.merge(@replayed_models) generate_statements(captured_models) + ensure + @replayed_models = nil end def mount(data) + # Replayed INSERTs are tagged "FixtureKit Insert" below, so the #generate + # subscriber cannot see them. When a child fixture is being generated, the + # parent is mounted inside the generate block; record the models we replay + # so #generate can fold them into the child's captured set. + @replayed_models&.merge(data.keys) + models_by_pool(data).each do |pool, models| pool.with_connection do |connection| statements = models.flat_map do |model| diff --git a/spec/unit/coders/active_record_coder_spec.rb b/spec/unit/coders/active_record_coder_spec.rb index 1dd7c78..6a3bad5 100644 --- a/spec/unit/coders/active_record_coder_spec.rb +++ b/spec/unit/coders/active_record_coder_spec.rb @@ -103,11 +103,16 @@ def upsert_all_options(model) expect(coder.generate { user.destroy! }[User]).to be_nil end - it "includes models from parent_data that were not directly captured" do - User.create!(name: "Alice", email: "alice-parent@example.com") - - parent_data = { User => "INSERT INTO users ..." } - result = coder.generate(parent_data: parent_data) do + it "folds models replayed via #mount into the captured set" do + alice = User.create!(name: "Alice", email: "alice-parent@example.com") + # A real serialized payload for the users table (one INSERT for Alice). + user_payload = coder.generate { User.where(id: alice.id).update_all(name: "Alice") } + + # parent.mount runs inside the generate block. Its replayed INSERT is + # tagged "FixtureKit Insert", so generate's subscriber never sees User — + # it can only enter the result via @replayed_models, recorded by #mount. + result = coder.generate do + coder.mount(user_payload) Project.create!(name: "Project", owner: User.find_by!(email: "alice-parent@example.com")) end diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index 8d4a1bf..388b197 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -258,6 +258,111 @@ def identifier_for(identifier) end end + describe "parent model propagation via #mount (draft: no parent_data)" do + # End-to-end exercise of the accumulator approach: a real parent cache is + # saved, then "a fresh process" clears it from memory, and the child is + # saved with the parent mounted from disk. The parent's models can only + # reach the child cache via @replayed_models (recorded in + # ActiveRecordCoder#mount), because the replayed INSERTs are tagged + # "FixtureKit Insert" and the child's subscriber never sees them. None of + # the child definitions below recreate the parent's models with a captured + # write, so the assertions are load-bearing. + def propagate(parent_id:, child_id:, parent_definition:, child_definition:) + parent_fixture = instance_double( + FixtureKit::Fixture, identifier: parent_id, definition: parent_definition, parent: nil + ) + parent_cache = described_class.new(parent_fixture) + parent_cache.save + parent_cache.clear_memory # fresh process: @content nil, file on disk + + allow(parent_fixture).to receive(:cache).and_return(parent_cache) + # Real mount: replays the parent's rows through ActiveRecordCoder#mount, + # which records the models in @replayed_models. + allow(parent_fixture).to receive(:mount) { parent_cache.load } + + child_fixture = instance_double( + FixtureKit::Fixture, identifier: child_id, definition: child_definition, parent: parent_fixture + ) + child_cache = described_class.new(child_fixture) + child_cache.save + + JSON.parse(File.read(child_cache.path))["data"]["FixtureKit::ActiveRecordCoder"] + end + + it "carries a basic parent model into the child cache across processes" do + parent_definition = FixtureKit::Definition.new do + User.create!(name: "Owner", email: "owner-c@example.com") + end + child_definition = FixtureKit::Definition.new do + owner = User.find_by!(email: "owner-c@example.com") + Project.create!(name: "Child Project", owner: owner) + end + + data = propagate( + parent_id: "parent_basic", child_id: "child_basic", + parent_definition: parent_definition, child_definition: child_definition + ) + + expect(data.keys).to include("User", "Project") + end + + it "collapses STI parent models to their base table and carries them" do + parent_definition = FixtureKit::Definition.new do + User.create!(name: "STI Owner", email: "sti-owner@example.com") + Car.create!(name: "Beetle", year: 1968) + end + child_definition = FixtureKit::Definition.new do + owner = User.find_by!(email: "sti-owner@example.com") + Project.create!(name: "STI Child Project", owner: owner) + end + + data = propagate( + parent_id: "parent_sti", child_id: "child_sti", + parent_definition: parent_definition, child_definition: child_definition + ) + + expect(data.keys).to include("User", "Vehicle", "Project") + expect(data.keys).not_to include("Car") # collapsed to the base table model + end + + it "carries parent models from a second connection pool" do + parent_definition = FixtureKit::Definition.new do + user = User.create!(name: "MP Owner", email: "mp-owner@example.com") + ActivityLog.create!(external_user_id: user.id, action: "signed_in") + end + child_definition = FixtureKit::Definition.new do + owner = User.find_by!(email: "mp-owner@example.com") + Project.create!(name: "Multipool Child Project", owner: owner) + end + + data = propagate( + parent_id: "parent_multipool", child_id: "child_multipool", + parent_definition: parent_definition, child_definition: child_definition + ) + + expect(data.keys).to include("User", "ActivityLog", "Project") + end + + it "carries a parent model whose cached sql is nil (delete-only)" do + parent_definition = FixtureKit::Definition.new do + User.create!(name: "Del Owner", email: "del-owner@example.com") + Phone.create!(name: "Doomed").destroy! # STI subclass; base table is gadgets + end + child_definition = FixtureKit::Definition.new do + owner = User.find_by!(email: "del-owner@example.com") + Project.create!(name: "Delete Child Project", owner: owner) + end + + data = propagate( + parent_id: "parent_delete", child_id: "child_delete", + parent_definition: parent_definition, child_definition: child_definition + ) + + expect(data.keys).to include("User", "Gadget", "Project") + expect(data["Gadget"]).to be_nil # carried forward even with no rows to insert + end + end + describe "#clear_memory" do it "nils out @content" do cache.instance_variable_set(:@content, FixtureKit::MemoryCache.new(data: {}, exposed: {}))