From 673dcac3c1269c27c007c392d652ea779ffd3169 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 30 May 2023 12:29:37 +1000 Subject: [PATCH] chore: support merging configuration hashes when overriding (#611) PACT-1033 --- .../api/middleware/configuration.rb | 2 + lib/pact_broker/configuration.rb | 39 +++++++++++++------ spec/lib/pact_broker/configuration_spec.rb | 34 +++++++++------- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/lib/pact_broker/api/middleware/configuration.rb b/lib/pact_broker/api/middleware/configuration.rb index 5d6846605..ecbcdacf3 100644 --- a/lib/pact_broker/api/middleware/configuration.rb +++ b/lib/pact_broker/api/middleware/configuration.rb @@ -1,5 +1,7 @@ require "pact_broker/logging" +# Allows the load-time configuration to be overridden on a per-request basis (for Pactflow) + module PactBroker module Api module Middleware diff --git a/lib/pact_broker/configuration.rb b/lib/pact_broker/configuration.rb index a41ef9209..9bb19ecd6 100644 --- a/lib/pact_broker/configuration.rb +++ b/lib/pact_broker/configuration.rb @@ -82,23 +82,14 @@ def self.default_configuration def override_runtime_configuration!(overrides) new_runtime_configuration = runtime_configuration.dup - valid_overrides = {} - invalid_overrides = {} - - overrides.each do | key, value | - if new_runtime_configuration.respond_to?("#{key}=") - valid_overrides[key] = Anyway::AutoCast.call(value) - else - invalid_overrides[key] = Anyway::AutoCast.call(value) - end - end + valid_overrides, invalid_overrides = identify_valid_and_invalid_configuration_overrides(new_runtime_configuration, overrides) if logger.debug? logger.debug("Overridding runtime configuration", overrides: valid_overrides, ignoring: invalid_overrides) end valid_overrides.each do | key, value | - new_runtime_configuration.public_send("#{key}=", value) + override_or_merge_value(new_runtime_configuration, key, value) end self.runtime_configuration = new_runtime_configuration @@ -220,5 +211,31 @@ def load_from_database! require "pact_broker/config/load" PactBroker::Config::Load.call(runtime_configuration) end + + private + + def identify_valid_and_invalid_configuration_overrides(new_runtime_configuration, overrides) + valid_overrides = {} + invalid_overrides = {} + + overrides.each do | key, value | + if new_runtime_configuration.respond_to?("#{key}=") + valid_overrides[key] = Anyway::AutoCast.call(value) + else + invalid_overrides[key] = Anyway::AutoCast.call(value) + end + end + + return valid_overrides, invalid_overrides + end + + def override_or_merge_value(new_runtime_configuration, key, value) + # Do a merge if original value and new value are both hashes + if value.is_a?(Hash) && new_runtime_configuration.public_send(key).is_a?(Hash) + new_runtime_configuration.public_send("#{key}=", new_runtime_configuration.public_send(key).merge(value)) + else + new_runtime_configuration.public_send("#{key}=", value) + end + end end end diff --git a/spec/lib/pact_broker/configuration_spec.rb b/spec/lib/pact_broker/configuration_spec.rb index d942251b0..14aa8e480 100644 --- a/spec/lib/pact_broker/configuration_spec.rb +++ b/spec/lib/pact_broker/configuration_spec.rb @@ -38,38 +38,44 @@ module PactBroker describe "override_runtime_configuration!" do let(:logger) { double("logger", debug?: true, debug: nil, warn: nil) } - let(:duped_config) { PactBroker.configuration.dup } + let(:config) { PactBroker.configuration.dup } - it "overrides the specified runtime configuration attributes within the block" do - duped_config.override_runtime_configuration!(disable_ssl_verification: "true") - expect(duped_config.disable_ssl_verification).to eq true + it "overrides the specified runtime configuration attributes" do + config.override_runtime_configuration!(disable_ssl_verification: "true") + expect(config.disable_ssl_verification).to eq true expect(PactBroker.configuration.disable_ssl_verification).to eq false end it "logs the overrides at debug level" do - allow(duped_config).to receive(:logger).and_return(logger) + allow(config).to receive(:logger).and_return(logger) expect(logger).to receive(:debug).with("Overridding runtime configuration", hash_including(overrides: { disable_ssl_verification: true })) - duped_config.override_runtime_configuration!(disable_ssl_verification: "true") + config.override_runtime_configuration!(disable_ssl_verification: "true") end - it "does not override the other runtime configuration attributes within the block" do - attribute_in_block = PactBroker.configuration.webhook_scheme_whitelist - duped_config.override_runtime_configuration!(disable_ssl_verification: "true") - expect(duped_config.webhook_scheme_whitelist).to eq attribute_in_block + it "does not override the other runtime configuration attributes" do + expect { config.override_runtime_configuration!(disable_ssl_verification: "true") }.to_not change { config.webhook_scheme_whitelist } end context "when the specified runtime attribute does not exist" do it "logs that it has ignored those attributes" do - allow(duped_config).to receive(:logger).and_return(logger) + allow(config).to receive(:logger).and_return(logger) expect(logger).to receive(:debug).with("Overridding runtime configuration", hash_including(ignoring: { no_existy: true })) - duped_config.override_runtime_configuration!(no_existy: "true") + config.override_runtime_configuration!(no_existy: "true") + end + end + + context "when overriding config items that are hashes" do + it "merges them" do + config.features = { feat_a: true, feat_b: false } + config.override_runtime_configuration!(features: { feat_a: false, feat_c: true }) + expect(config.features).to eq feat_a: false, feat_b: false, feat_c: true end end context "when the configuration is frozen" do it "raises an error" do - duped_config.freeze - expect { duped_config.override_runtime_configuration!(disable_ssl_verification: "true") }.to raise_error FrozenError + config.freeze + expect { config.override_runtime_configuration!(disable_ssl_verification: "true") }.to raise_error FrozenError end end end