From 956ed004bbfb7f2bb182282b70686b3ab243e156 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 20 Mar 2018 15:44:23 -0700 Subject: [PATCH] Update trigger config merge function --- plugins/kernel_v2/config/trigger.rb | 43 +++++++++++++++---- .../plugins/kernel_v2/config/trigger_test.rb | 39 +++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/plugins/kernel_v2/config/trigger.rb b/plugins/kernel_v2/config/trigger.rb index 0b24b450c..0535e0565 100644 --- a/plugins/kernel_v2/config/trigger.rb +++ b/plugins/kernel_v2/config/trigger.rb @@ -101,7 +101,6 @@ module VagrantPlugins return trigger end - # Solve the mystery of disappearing state?? def merge(other) super.tap do |result| new_before_triggers = [] @@ -109,17 +108,43 @@ module VagrantPlugins other_defined_before_triggers = other.instance_variable_get(:@_before_triggers) other_defined_after_triggers = other.instance_variable_get(:@_after_triggers) - # TODO: Is this the right solution? - # If a guest in a Vagrantfile exists beyond the default, this check - # will properly set up the defined triggers and validate them. - # overrides??? check for duplicate ids? - if other_defined_before_triggers.empty? && !@_before_triggers.empty? - result.instance_variable_set(:@_before_triggers, @_before_triggers) + @_before_triggers.each do |bt| + other_bft = other_defined_before_triggers.find { |o| bt.id == o.id } + if other_bft + # Override, take it + other_bft = bt.merge(other_bft) + + # Preserve order, always + bt = other_bft + other_defined_before_triggers.delete(other_bft) + end + + new_before_triggers << bt.dup end - if other_defined_before_triggers.empty? && !@_after_triggers.empty? - result.instance_variable_set(:@_after_triggers, @_after_triggers) + other_defined_before_triggers.each do |obt| + new_before_triggers << obt.dup end + result.instance_variable_set(:@_before_triggers, new_before_triggers) + + @_after_triggers.each do |at| + other_aft = other_defined_after_triggers.find { |o| at.id == o.id } + if other_aft + # Override, take it + other_aft = at.merge(other_aft) + + # Preserve order, always + at = other_aft + other_defined_after_triggers.delete(other_aft) + end + + new_after_triggers << at.dup + end + + other_defined_after_triggers.each do |oat| + new_after_triggers << oat.dup + end + result.instance_variable_set(:@_after_triggers, new_after_triggers) end end diff --git a/test/unit/plugins/kernel_v2/config/trigger_test.rb b/test/unit/plugins/kernel_v2/config/trigger_test.rb index a044eff0b..4b5e1ed1f 100644 --- a/test/unit/plugins/kernel_v2/config/trigger_test.rb +++ b/test/unit/plugins/kernel_v2/config/trigger_test.rb @@ -152,4 +152,43 @@ describe VagrantPlugins::Kernel_V2::TriggerConfig do expect(trigger).to be_a(VagrantPlugins::Kernel_V2::VagrantConfigTrigger) end end + + describe "#merge" do + it "merges defined triggers" do + a = described_class.new() + b = described_class.new() + + a.before(splat, hash_block) + a.after(arr, hash_block) + b.before(splat, hash_block) + b.after(arr, hash_block) + + result = a.merge(b) + bf_trigger = result.instance_variable_get(:@_before_triggers) + af_trigger = result.instance_variable_get(:@_after_triggers) + + expect(bf_trigger).to be_a(Array) + expect(af_trigger).to be_a(Array) + expect(bf_trigger.size).to eq(6) + expect(af_trigger.size).to eq(6) + end + + it "merges the other triggers if a class is empty" do + a = described_class.new() + b = described_class.new() + + a.before(splat, hash_block) + a.after(arr, hash_block) + + b_bf_trigger = b.instance_variable_get(:@_before_triggers) + b_af_trigger = b.instance_variable_get(:@_after_triggers) + + result = a.merge(b) + bf_trigger = result.instance_variable_get(:@_before_triggers) + af_trigger = result.instance_variable_get(:@_after_triggers) + + expect(bf_trigger.size).to eq(3) + expect(af_trigger.size).to eq(3) + end + end end