diff --git a/lib/stroma/dsl/generator.rb b/lib/stroma/dsl/generator.rb index 8bfa489..4ce23bb 100644 --- a/lib/stroma/dsl/generator.rb +++ b/lib/stroma/dsl/generator.rb @@ -16,6 +16,19 @@ module DSL # - ServiceClass gets @stroma_matrix (same reference) # - ServiceClass gets @stroma (unique State per class) # + # ## Deferred Entry Inclusion + # + # Entry extensions are NOT included via `Module#include` at the base class level. + # Instead, only the `self.included` callback is fired (via `send(:included, base)`) + # to set up ClassMethods, constants, etc. The actual module insertion into the + # ancestor chain (`append_features`) is deferred until {Hooks::Applier} interleaves + # entries with hooks in child classes. + # + # **Contract:** Entry extensions MUST implement `self.included` as idempotent. + # The callback fires twice per entry per class hierarchy: + # 1. At base class creation (deferred, via `send(:included, base)`) + # 2. At child class creation (real, via `include` in {Hooks::Applier}) + # # ## Usage # # ```ruby @@ -75,7 +88,13 @@ def included(base) base.instance_variable_set(:@stroma_matrix, mtx) base.instance_variable_set(:@stroma, State.new) - mtx.entries.each { |entry| base.include(entry.extension) } + # Deferred inclusion: triggers `included` callback without `append_features`. + # The callback runs ClassMethods/Workspace setup on base. + # `append_features` (actual module insertion into ancestors) is deferred + # until Applier interleaves entries with hooks in child classes. + # NOTE: `included` will fire again when Applier calls `include` on child, + # so entry extensions must design `self.included` as idempotent. + mtx.entries.each { |entry| entry.extension.send(:included, base) } end end diff --git a/lib/stroma/hooks/applier.rb b/lib/stroma/hooks/applier.rb index 33e3418..2c83992 100644 --- a/lib/stroma/hooks/applier.rb +++ b/lib/stroma/hooks/applier.rb @@ -2,13 +2,15 @@ module Stroma module Hooks - # Applies registered hooks to a target class. + # Applies registered hooks to a target class with deferred entry inclusion. # # ## Purpose # - # Includes hook extension modules into target class. - # Maintains order based on matrix registry entries. - # For each entry: before hooks first, then after hooks. + # Manages hook and entry module inclusion into the target class. + # Operates in three modes depending on current state: + # - No hooks: returns immediately (entries stay deferred) + # - Entries already in ancestors: includes only new hooks + # - Entries not in ancestors: interleaves entries with hooks for correct MRO # # ## Usage # @@ -50,16 +52,59 @@ def initialize(target_class, hooks, matrix) # Applies all registered hooks to the target class. # - # For each registry entry, includes before hooks first, - # then after hooks. Does nothing if hooks collection is empty. + # Three modes based on current state: + # - No hooks: return immediately (defer entry inclusion) + # - Entries already in ancestors: include only hooks + # - Entries not in ancestors: interleave entries with hooks # # @return [void] def apply! return if @hooks.empty? + if entries_in_ancestors? + include_hooks_only + else + include_entries_with_hooks + end + end + + private + + # Checks whether all entry extensions are already in the target class ancestors. + # + # Uses all? so that partial inclusion (some entries present, some not) + # falls through to include_entries_with_hooks where Ruby skips + # already-included modules (idempotent) and interleaves the rest. + # + # @return [Boolean] + def entries_in_ancestors? + ancestors = @target_class.ancestors + @matrix.entries.all? { |e| ancestors.include?(e.extension) } + end + + # Includes entries interleaved with their hooks. + # + # For each entry: after hooks first (reversed), then entry, then before hooks (reversed). + # reverse_each ensures first registered = outermost in MRO. + # + # @return [void] + def include_entries_with_hooks + @matrix.entries.each do |entry| + @hooks.after(entry.key).reverse_each { |hook| @target_class.include(hook.extension) } + @target_class.include(entry.extension) + @hooks.before(entry.key).reverse_each { |hook| @target_class.include(hook.extension) } + end + end + + # Includes only hook extensions without entries. + # + # Used when entries are already in ancestors (multi-level inheritance). + # + # @return [void] + def include_hooks_only @matrix.entries.each do |entry| - @hooks.before(entry.key).each { |hook| @target_class.include(hook.extension) } - @hooks.after(entry.key).each { |hook| @target_class.include(hook.extension) } + @hooks.before(entry.key).reverse_each { |hook| @target_class.include(hook.extension) } + @hooks.after(entry.key).reverse_each { |hook| @target_class.include(hook.extension) } end end end diff --git a/sig/lib/stroma/hooks/applier.rbs b/sig/lib/stroma/hooks/applier.rbs index 628e090..e9ca196 100644 --- a/sig/lib/stroma/hooks/applier.rbs +++ b/sig/lib/stroma/hooks/applier.rbs @@ -10,6 +10,14 @@ module Stroma def initialize: (Class target_class, Collection hooks, Matrix matrix) -> void def apply!: () -> void + + private + + def entries_in_ancestors?: () -> bool + + def include_entries_with_hooks: () -> void + + def include_hooks_only: () -> void end end end diff --git a/spec/stroma/dsl/generator_spec.rb b/spec/stroma/dsl/generator_spec.rb index c2f5b2c..4e23467 100644 --- a/spec/stroma/dsl/generator_spec.rb +++ b/spec/stroma/dsl/generator_spec.rb @@ -56,9 +56,9 @@ expect(base_class).to respond_to(:inherited) end - it "includes all registered DSL modules", :aggregate_failures do - expect(base_class.ancestors).to include(inputs_dsl) - expect(base_class.ancestors).to include(outputs_dsl) + it "does not include entry modules in base ancestors", :aggregate_failures do + expect(base_class.ancestors).not_to include(inputs_dsl) + expect(base_class.ancestors).not_to include(outputs_dsl) end it "creates stroma state" do @@ -93,11 +93,21 @@ def self.included(base) let(:child_class) { Class.new(base_class) } + it "includes entry modules in child via interleaving", :aggregate_failures do + expect(child_class.ancestors).to include(inputs_dsl) + expect(child_class.ancestors).to include(outputs_dsl) + end + it "applies hooks to child class" do expect(child_class.ancestors).to include(extension_module) end - it "child has extension method", :aggregate_failures do + it "positions hook adjacent to its target entry in child" do + ancestors = child_class.ancestors + expect(ancestors.index(extension_module)).to be < ancestors.index(inputs_dsl) + end + + it "propagates extension callback to child", :aggregate_failures do expect(child_class).to respond_to(:extension_method) expect(child_class.extension_method).to eq(:extension_result) end @@ -112,10 +122,127 @@ def self.included(base) end end + describe "multi-level inheritance" do + let(:base_class) do + mtx = matrix + Class.new { include mtx.dsl } + end + + context "with hooks at one level" do + let(:auth_module) do + Module.new do + def self.included(base) + base.define_singleton_method(:auth_configured) { true } + end + end + end + + let(:mid_class) do + base = base_class + auth = auth_module + Class.new(base) do + extensions do + before :inputs, auth + end + end + end + + let(:leaf_class) { Class.new(mid_class) } + + it "does not include entries in base", :aggregate_failures do + expect(base_class.ancestors).not_to include(inputs_dsl) + expect(base_class.ancestors).not_to include(outputs_dsl) + end + + it "does not include entries in mid class", :aggregate_failures do + expect(mid_class.ancestors).not_to include(inputs_dsl) + expect(mid_class.ancestors).not_to include(outputs_dsl) + end + + it "interleaves entries with hooks in leaf class", :aggregate_failures do + ancestors = leaf_class.ancestors + + expect(ancestors).to include(inputs_dsl, outputs_dsl, auth_module) + expect(ancestors.index(auth_module)).to be < ancestors.index(inputs_dsl) + end + + it "propagates hook callbacks to leaf" do + expect(leaf_class).to respond_to(:auth_configured) + end + end + + context "with hooks at every level" do + let(:auth_module) { Module.new } + let(:logging_module) { Module.new } + + let(:mid_class) do + base = base_class + auth = auth_module + Class.new(base) do + extensions do + before :inputs, auth + end + end + end + + let(:leaf_class) do + mid = mid_class + logging = logging_module + Class.new(mid) do + extensions do + after :inputs, logging + end + end + end + + it "includes hooks and entries from all levels", :aggregate_failures do + ancestors = Class.new(leaf_class).ancestors + expect(ancestors).to include(auth_module, logging_module, inputs_dsl, outputs_dsl) + end + + it "positions before hook above entry in MRO" do + ancestors = Class.new(leaf_class).ancestors + expect(ancestors.index(auth_module)).to be < ancestors.index(inputs_dsl) + end + + # Ruby's Module#include cannot position a new module below an inherited module. + # After hooks registered at parent level are placed above inherited entries in + # grandchild MRO. This does not affect phase execution order (controlled by + # the orchestrator), only the super call chain. + it "positions cross-level after hook above entry in MRO" do + ancestors = Class.new(leaf_class).ancestors + expect(ancestors.index(logging_module)).to be < ancestors.index(inputs_dsl) + end + end + + context "when grandchild has no own hooks" do + let(:auth_module) { Module.new } + + let(:mid_class) do + base = base_class + auth = auth_module + Class.new(base) do + extensions do + before :inputs, auth + end + end + end + + let(:child_class) { Class.new(mid_class) } + let(:grandchild) { Class.new(child_class) } + + it "propagates entries and hooks to grandchild", :aggregate_failures do + expect(grandchild.ancestors).to include(inputs_dsl) + expect(grandchild.ancestors).to include(outputs_dsl) + expect(grandchild.ancestors).to include(auth_module) + end + end + end + describe "inheritance isolation" do let(:extension_module) { Module.new } - let(:parent_class) do + let(:base_class) do mtx = matrix ext = extension_module Class.new do @@ -127,9 +254,9 @@ def self.included(base) end end - let(:child_class) { Class.new(parent_class) } + let(:child_class) { Class.new(base_class) } - it "child modifications do not affect parent", :aggregate_failures do + it "child modifications do not affect base", :aggregate_failures do child_extension = Module.new child_class.class_eval do @@ -138,20 +265,20 @@ def self.included(base) end end - expect(parent_class.stroma.hooks.after(:outputs)).to be_empty + expect(base_class.stroma.hooks.after(:outputs)).to be_empty expect(child_class.stroma.hooks.after(:outputs)).not_to be_empty end - it "child inherits parent hooks", :aggregate_failures do + it "child inherits base hooks", :aggregate_failures do expect(child_class.stroma.hooks.before(:inputs).size).to eq(1) expect(child_class.ancestors).to include(extension_module) end - it "parent modifications after child creation do not affect child" do + it "base modifications after child creation do not affect child" do child_before_count = child_class.stroma.hooks.before(:outputs).size new_extension = Module.new - parent_class.class_eval do + base_class.class_eval do extensions do before :outputs, new_extension end diff --git a/spec/stroma/hooks/applier_spec.rb b/spec/stroma/hooks/applier_spec.rb index 93b98d6..2911ace 100644 --- a/spec/stroma/hooks/applier_spec.rb +++ b/spec/stroma/hooks/applier_spec.rb @@ -21,62 +21,181 @@ before do hooks.add(:before, :inputs, before_extension) + described_class.apply!(target_class, hooks, matrix) end it "applies hooks via class method" do - described_class.apply!(target_class, hooks, matrix) expect(target_class.ancestors).to include(before_extension) end end describe "#apply!" do - it "does nothing when hooks empty" do - applier.apply! - expect(target_class.ancestors).not_to include(inputs_dsl) + context "when hooks are empty" do + it "does not modify target class ancestors" do + ancestors_before = target_class.ancestors.dup + applier.apply! + expect(target_class.ancestors).to eq(ancestors_before) + end + + it "does not include entry extensions", :aggregate_failures do + applier.apply! + expect(target_class.ancestors).not_to include(inputs_dsl) + expect(target_class.ancestors).not_to include(outputs_dsl) + end end - context "with before hooks" do - let(:before_extension) { Module.new } + context "when entries are not in ancestors" do + context "with a before hook" do + let(:before_extension) { Module.new } - before do - hooks.add(:before, :inputs, before_extension) + before do + hooks.add(:before, :inputs, before_extension) + applier.apply! + end + + it "positions before hook above entry in MRO" do + ancestors = target_class.ancestors + expect(ancestors.index(before_extension)).to be < ancestors.index(inputs_dsl) + end + + it "includes all entries" do + expect(target_class.ancestors).to include(inputs_dsl, outputs_dsl) + end end - it "includes before hook extension" do - applier.apply! - expect(target_class.ancestors).to include(before_extension) + context "with an after hook" do + let(:after_extension) { Module.new } + + before do + hooks.add(:after, :inputs, after_extension) + applier.apply! + end + + it "positions after hook below entry in MRO" do + ancestors = target_class.ancestors + expect(ancestors.index(after_extension)).to be > ancestors.index(inputs_dsl) + end end - end - context "with after hooks" do - let(:after_extension) { Module.new } + context "with both before and after hooks" do + let(:before_extension) { Module.new } + let(:after_extension) { Module.new } - before do - hooks.add(:after, :outputs, after_extension) + before do + hooks.add(:before, :inputs, before_extension) + hooks.add(:after, :inputs, after_extension) + applier.apply! + end + + it "positions before above and after below entry", :aggregate_failures do + ancestors = target_class.ancestors + expect(ancestors.index(before_extension)).to be < ancestors.index(inputs_dsl) + expect(ancestors.index(after_extension)).to be > ancestors.index(inputs_dsl) + end end - it "includes after hook extension" do - applier.apply! - expect(target_class.ancestors).to include(after_extension) + context "with multiple before hooks" do + let(:first_before) { Module.new } + let(:second_before) { Module.new } + + before do + hooks.add(:before, :inputs, first_before) + hooks.add(:before, :inputs, second_before) + applier.apply! + end + + it "first registered is outermost in MRO", :aggregate_failures do + ancestors = target_class.ancestors + expect(ancestors.index(first_before)).to be < ancestors.index(second_before) + expect(ancestors.index(second_before)).to be < ancestors.index(inputs_dsl) + end end - end - context "with multiple hooks" do # rubocop:disable RSpec/MultipleMemoizedHelpers - let(:before_inputs) { Module.new } - let(:after_inputs) { Module.new } - let(:before_outputs) { Module.new } + context "with multiple after hooks" do + let(:first_after) { Module.new } + let(:second_after) { Module.new } + before do + hooks.add(:after, :inputs, first_after) + hooks.add(:after, :inputs, second_after) + applier.apply! + end + + it "first registered is closest to entry", :aggregate_failures do + ancestors = target_class.ancestors + expect(ancestors.index(inputs_dsl)).to be < ancestors.index(first_after) + expect(ancestors.index(first_after)).to be < ancestors.index(second_after) + end + end + + context "with hooks for different entries" do + let(:before_inputs_extension) { Module.new } + let(:after_outputs_extension) { Module.new } + + before do + hooks.add(:before, :inputs, before_inputs_extension) + hooks.add(:after, :outputs, after_outputs_extension) + applier.apply! + end + + it "each hook is adjacent to its target entry", :aggregate_failures do + ancestors = target_class.ancestors + expect(ancestors.index(before_inputs_extension)).to be < ancestors.index(inputs_dsl) + expect(ancestors.index(after_outputs_extension)).to be > ancestors.index(outputs_dsl) + end + end + end + + context "when entries are already in ancestors" do before do - hooks.add(:before, :inputs, before_inputs) - hooks.add(:after, :inputs, after_inputs) - hooks.add(:before, :outputs, before_outputs) + target_class.include(outputs_dsl) + target_class.include(inputs_dsl) end - it "applies all hooks", :aggregate_failures do - applier.apply! - expect(target_class.ancestors).to include(before_inputs) - expect(target_class.ancestors).to include(after_inputs) - expect(target_class.ancestors).to include(before_outputs) + context "with a before hook" do + let(:before_extension) { Module.new } + + before do + hooks.add(:before, :inputs, before_extension) + applier.apply! + end + + it "includes hook extensions" do + expect(target_class.ancestors).to include(before_extension) + end + + it "does not duplicate entries in ancestors" do + expect(target_class.ancestors.count { |a| a == inputs_dsl }).to eq(1) + end + end + + context "with an after hook" do + let(:after_extension) { Module.new } + + before do + hooks.add(:after, :inputs, after_extension) + applier.apply! + end + + it "includes after hook extensions" do + expect(target_class.ancestors).to include(after_extension) + end + end + + context "with both before and after hooks" do + let(:before_extension) { Module.new } + let(:after_extension) { Module.new } + + before do + hooks.add(:before, :inputs, before_extension) + hooks.add(:after, :inputs, after_extension) + applier.apply! + end + + it "includes both hook types", :aggregate_failures do + expect(target_class.ancestors).to include(before_extension) + expect(target_class.ancestors).to include(after_extension) + end end end end