From 38e9429c758fb1da869d0fc8c470dc0798f994d9 Mon Sep 17 00:00:00 2001
From: Tom de Bruijn <tom@tomdebruijn.com>
Date: Fri, 14 Jan 2022 09:05:53 +0100
Subject: [PATCH] Fix deprecation warning for working_dir_path

It would only print the warning if the config option was used in the
file source, but if the `APPSIGNAL_WORKING_DIR_PATH` environment
variable was set, it would not print the same deprecation warning. This
only makes the deprecation more visible. This is necessary before we
fully remove it in the next major version.

I've added a changeset, because it prints the warning in more scenarios
and those are scenarios we want to inform our users about before
removing the config option entirely.

Fixes https://github.com/appsignal/appsignal-ruby/issues/805
---
 ...dir_path-option-from-all-config-sources.md |  6 +++
 lib/appsignal/config.rb                       | 37 ++++++------------
 spec/lib/appsignal/config_spec.rb             | 39 +++++++++++++++++++
 3 files changed, 56 insertions(+), 26 deletions(-)
 create mode 100644 .changesets/warn-about-deprecated-working_dir_path-option-from-all-config-sources.md

diff --git a/.changesets/warn-about-deprecated-working_dir_path-option-from-all-config-sources.md b/.changesets/warn-about-deprecated-working_dir_path-option-from-all-config-sources.md
new file mode 100644
index 000000000..5a1978885
--- /dev/null
+++ b/.changesets/warn-about-deprecated-working_dir_path-option-from-all-config-sources.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "deprecate"
+---
+
+Warn about the deprecated `working_dir_path` option from all config sources. It previously only printed a warning when it was configured in the `config/appsignal.yml` file, but now also prints the warning if it's set via the Config class initialize options and environment variables. Please use the `working_directory_path` option instead.
diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb
index c669f157a..7b6be65d4 100644
--- a/lib/appsignal/config.rb
+++ b/lib/appsignal/config.rb
@@ -377,12 +377,9 @@ def load_from_disk
       configurations = YAML.load(ERB.new(IO.read(config_file)).result, **read_options)
       config_for_this_env = configurations[env]
       if config_for_this_env
-        config_for_this_env =
-          config_for_this_env.each_with_object({}) do |(key, value), hash|
-            hash[key.to_sym] = value # convert keys to symbols
-          end
-
-        maintain_file_config_backwards_compatibility(config_for_this_env)
+        config_for_this_env.each_with_object({}) do |(key, value), hash|
+          hash[key.to_sym] = value # convert keys to symbols
+        end
       else
         logger.error "Not loading from config file: config for '#{env}' not found"
         nil
@@ -397,26 +394,6 @@ def load_from_disk
       nil
     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.
-    #
-    # @todo This is incomplete and only considers the file source as the only
-    #   valid config source. This should be moved to
-    #   {maintain_backwards_compatibility}.
-    def maintain_file_config_backwards_compatibility(configuration)
-      configuration.tap do |config|
-        if config.include?(:working_dir_path)
-          deprecation_message \
-            "'working_dir_path' is deprecated, please use " \
-            "'working_directory_path' instead and specify the " \
-            "full path to the working directory",
-            logger
-        end
-      end
-    end
-
     # Maintain backwards compatibility with deprecated config options.
     #
     # Add deprecated config options here with the behavior of setting its
@@ -442,6 +419,14 @@ def maintain_backwards_compatibility
           "deprecated. Please use `send_session_data` instead.",
           logger
       end
+
+      if config_hash.key?(:working_dir_path) # rubocop:disable Style/GuardClause
+        deprecation_message \
+          "The `working_dir_path` option is deprecated, please use " \
+          "`working_directory_path` instead and specify the " \
+          "full path to the working directory",
+          logger
+      end
     end
 
     def load_from_environment
diff --git a/spec/lib/appsignal/config_spec.rb b/spec/lib/appsignal/config_spec.rb
index 55af450ea..60f8727a4 100644
--- a/spec/lib/appsignal/config_spec.rb
+++ b/spec/lib/appsignal/config_spec.rb
@@ -751,6 +751,45 @@
       described_class.new(Dir.pwd, "production", config_options, logger)
     end
 
+    describe "working_dir_path" do
+      let(:err_stream) { std_stream }
+      let(:stderr) { err_stream.read }
+      let(:deprecation_message) do
+        "The `working_dir_path` option is deprecated, please use " \
+          "`working_directory_path` instead and specify the " \
+          "full path to the working directory"
+      end
+      before do
+        capture_std_streams(std_stream, err_stream) { config }
+      end
+
+      context "when not set" do
+        let(:config_options) { {} }
+
+        it "sets the default working_dir_path value" do
+          expect(config[:working_dir_path]).to be_nil
+        end
+
+        it "does not print a deprecation warning" do
+          expect(stderr).to_not include("appsignal WARNING: #{deprecation_message}")
+          expect(logs).to_not include(deprecation_message)
+        end
+      end
+
+      context "when set" do
+        let(:config_options) { { :working_dir_path => "/tmp/appsignal2" } }
+
+        it "sets the default working_dir_path value" do
+          expect(config[:working_dir_path]).to eq("/tmp/appsignal2")
+        end
+
+        it "does not print a deprecation warning" do
+          expect(stderr).to include("appsignal WARNING: #{deprecation_message}")
+          expect(logs).to include(deprecation_message)
+        end
+      end
+    end
+
     describe "skip_session_data" do
       let(:err_stream) { std_stream }
       let(:stderr) { err_stream.read }