Extract backwards compatibility logic to a method

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,21 +191,32 @@ 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
         @logger.error "Not loading from config file: config for '#{env}' not found"
       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 = {}
 

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

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
 

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