diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb index 6406d61e9..bb9e15643 100644 --- a/lib/appsignal/config.rb +++ b/lib/appsignal/config.rb @@ -63,6 +63,12 @@ class Config "APPSIGNAL_FILES_WORLD_ACCESSIBLE" => :files_world_accessible }.freeze + # Mapping of old and deprecated AppSignal configuration keys + DEPRECATED_CONFIG_KEY_MAPPING = { + :api_key => :push_api_key, + :ignore_exceptions => :ignore_errors + }.freeze + attr_reader :root_path, :env, :initial_config, :config_hash attr_accessor :logger @@ -185,14 +191,7 @@ def load_from_disk [key.to_sym, value] end] # convert keys to symbols - # Backwards compatibility with config files generated by earlier - # versions of the gem - if !config_for_this_env[:push_api_key] && config_for_this_env[:api_key] - config_for_this_env[:push_api_key] = config_for_this_env[:api_key] - end - if !config_for_this_env[:ignore_errors] && config_for_this_env[:ignore_exceptions] - config_for_this_env[:ignore_errors] = config_for_this_env[:ignore_exceptions] - end + config_for_this_env = maintain_backwards_compatibility(config_for_this_env) merge(@config_hash, config_for_this_env) else @@ -200,6 +199,24 @@ def load_from_disk end end + # Maintain backwards compatibility with config files generated by earlier + # versions of the gem + # + # Used by {#load_from_disk}. No compatibility for env variables or initial config currently. + def maintain_backwards_compatibility(configuration) + configuration.tap do |config| + DEPRECATED_CONFIG_KEY_MAPPING.each do |old_key, new_key| + old_config_value = config.delete(old_key) + next unless old_config_value + logger.warn "Old configuration key found. Please update the "\ + "'#{old_key}' to '#{new_key}'." + + next if config[new_key] # Skip if new key is already in use + config[new_key] = old_config_value + end + end + end + def load_from_environment config = {} diff --git a/spec/lib/appsignal/config_spec.rb b/spec/lib/appsignal/config_spec.rb index 0d973b3c8..bc293e296 100644 --- a/spec/lib/appsignal/config_spec.rb +++ b/spec/lib/appsignal/config_spec.rb @@ -222,43 +222,54 @@ end end - describe "old-style config keys" do - describe ":api_key" do - subject { config[:push_api_key] } + describe "support for old config keys" do + let(:config) { project_fixture_config(env, {}, test_logger(log)) } + let(:log) { StringIO.new } + describe ":api_key" do context "without :push_api_key" do - let(:config) { project_fixture_config("old_config") } + let(:env) { "old_config" } it "sets the :push_api_key with the old :api_key value" do - expect(subject).to eq "def" + expect(config[:push_api_key]).to eq "def" + expect(config.config_hash).to_not have_key :api_key + expect(log_contents(log)).to contains_log :warn, + "Old configuration key found. Please update the 'api_key' to 'push_api_key'" end end context "with :push_api_key" do - let(:config) { project_fixture_config("old_config_mixed_with_new_config") } + let(:env) { "old_config_mixed_with_new_config" } - it "ignores the :api_key config" do - expect(subject).to eq "ghi" + it "ignores the :api_key config and deletes it" do + expect(config[:push_api_key]).to eq "ghi" + expect(config.config_hash).to_not have_key :api_key + expect(log_contents(log)).to contains_log :warn, + "Old configuration key found. Please update the 'api_key' to 'push_api_key'" end end end describe ":ignore_exceptions" do - subject { config[:ignore_errors] } - context "without :ignore_errors" do - let(:config) { project_fixture_config("old_config") } + let(:env) { "old_config" } it "sets :ignore_errors with the old :ignore_exceptions value" do - expect(subject).to eq ["StandardError"] + expect(config[:ignore_errors]).to eq ["StandardError"] + expect(config.config_hash).to_not have_key :ignore_exceptions + expect(log_contents(log)).to contains_log :warn, + "Old configuration key found. Please update the 'ignore_exceptions' to 'ignore_errors'" end end context "with :ignore_errors" do - let(:config) { project_fixture_config("old_config_mixed_with_new_config") } + let(:env) { "old_config_mixed_with_new_config" } it "ignores the :ignore_exceptions config" do - expect(subject).to eq ["NoMethodError"] + expect(config[:ignore_errors]).to eq ["NoMethodError"] + expect(config.config_hash).to_not have_key :ignore_exceptions + expect(log_contents(log)).to contains_log :warn, + "Old configuration key found. Please update the 'ignore_exceptions' to 'ignore_errors'" end end end diff --git a/spec/support/helpers/config_helpers.rb b/spec/support/helpers/config_helpers.rb index 21af02b58..95183ac65 100644 --- a/spec/support/helpers/config_helpers.rb +++ b/spec/support/helpers/config_helpers.rb @@ -5,11 +5,12 @@ def project_fixture_path ) end - def project_fixture_config(env = "production", initial_config = {}) + def project_fixture_config(env = "production", initial_config = {}, logger = Appsignal.logger) Appsignal::Config.new( project_fixture_path, env, - initial_config + initial_config, + logger ) end diff --git a/spec/support/helpers/log_helpers.rb b/spec/support/helpers/log_helpers.rb index d92aa1931..3d438d42a 100644 --- a/spec/support/helpers/log_helpers.rb +++ b/spec/support/helpers/log_helpers.rb @@ -1,15 +1,20 @@ module LogHelpers def use_logger_with(log) - Appsignal.logger = Logger.new(log) - Appsignal.logger.formatter = - proc do |severity, _datetime, _progname, msg| - # This format is used in the `contains_log` matcher. - "[#{severity}] #{msg}\n" - end + Appsignal.logger = test_logger(log) yield Appsignal.logger = nil end + def test_logger(log) + Logger.new(log).tap do |logger| + logger.formatter = + proc do |severity, _datetime, _progname, msg| + # This format is used in the `contains_log` matcher. + "[#{severity}] #{msg}\n" + end + end + end + def log_contents(log) log.rewind log.read