Add Transaction set_params(_if_nil) methods

Add methods to set the parameters on the Transaction. This replaces and
deprecates the Transaction `params=` writer method that was used before
this.

We need the `_if_nil` variant for the AbstractMiddleware to only write
the parameters if they're not already set by the app it's instrumenting.
Currently it tries to set the parameters before the app handles the
request, but this is unreliable and doesn't always report the parameters
of the request.

The `_if_nil` variant isn't perfect, because it doesn't consider
whatever parameters could be fetched from the request object, because it
now skips this. I can improve this later if necessary.

Part of #1108

lib/appsignal/transaction.rb

@@ -76,18 +76,6 @@ def clear_current_transaction!
     attr_reader :ext, :transaction_id, :action, :namespace, :request, :paused, :tags, :options,
       :discarded, :breadcrumbs
 
-    # @!attribute params
-    #   Attribute for parameters of the transaction.
-    #
-    #   When no parameters are set with {#params=} the parameters it will look
-    #   for parameters on the {#request} environment.
-    #
-    #   The parameters set using {#params=} are leading over those extracted
-    #   from a request's environment.
-    #
-    #   @return [Hash]
-    attr_writer :params
-
     def initialize(transaction_id, namespace, request, options = {})
       @transaction_id = transaction_id
       @action = nil
@@ -156,6 +144,40 @@ def params
       request_params
     end
 
+    # Set parameters on the transaction.
+    #
+    # When no parameters are set this way, the transaction will look for
+    # parameters on the {#request} environment.
+    #
+    # The parameters set using {#set_params} are leading over those extracted
+    # from a request's environment.
+    #
+    # @param given_params [Hash] The parameters to set on the transaction.
+    # @return [void]
+    def set_params(given_params)
+      @params = given_params if given_params
+    end
+
+    # @deprecated Use {#set_params} or {#set_params_if_nil} instead.
+    def params=(given_params)
+      Appsignal::Utils::StdoutAndLoggerMessage.warning(
+        "Transaction#params= is deprecated." \
+          "Use Transaction#set_params or #set_params_if_nil instead."
+      )
+      set_params(given_params)
+    end
+
+    # Set parameters on the transaction if not already set
+    #
+    # When no parameters are set this way, the transaction will look for
+    # parameters on the {#request} environment.
+    #
+    # @param given_params [Hash] The parameters to set on the transaction if none are already set.
+    # @return [void]
+    def set_params_if_nil(given_params)
+      set_params(given_params) unless @params
+    end
+
     # Set tags on the transaction.
     #
     # @param given_tags [Hash] Collection of tags.

spec/lib/appsignal/transaction_spec.rb

@@ -348,9 +348,94 @@ def current_transaction
     end
 
     describe "#params=" do
+      around { |example| keep_transactions { example.run } }
+
       it "sets params on the transaction" do
-        transaction.params = { :foo => "bar" }
-        expect(transaction.params).to eq(:foo => "bar")
+        params = { "foo" => "bar" }
+        transaction.params = params
+
+        transaction.complete # Sample the data
+        expect(transaction.params).to eq(params)
+        expect(transaction.to_h.dig("sample_data", "params")).to eq(params)
+      end
+
+      it "logs a deprecation warning" do
+        transaction.params = { "foo" => "bar" }
+
+        expect(log_contents(log)).to contains_log(
+          :warn,
+          "Transaction#params= is deprecated." \
+            "Use Transaction#set_params or #set_params_if_nil instead."
+        )
+      end
+    end
+
+    describe "#set_params" do
+      around { |example| keep_transactions { example.run } }
+
+      context "when the params are set" do
+        it "updates the params on the transaction" do
+          params = { "key" => "value" }
+          transaction.set_params(params)
+
+          transaction.complete # Sample the data
+          expect(transaction.params).to eq(params)
+          expect(transaction.to_h.dig("sample_data", "params")).to eq(params)
+        end
+      end
+
+      context "when the given params is nil" do
+        it "does not update the params on the transaction" do
+          params = { "key" => "value" }
+          transaction.set_params(params)
+          transaction.set_params(nil)
+
+          transaction.complete # Sample the data
+          expect(transaction.params).to eq(params)
+          expect(transaction.to_h.dig("sample_data", "params")).to eq(params)
+        end
+      end
+    end
+
+    describe "#set_params_if_nil" do
+      around { |example| keep_transactions { example.run } }
+
+      context "when the params are not set" do
+        it "sets the params on the transaction" do
+          expect(transaction.params).to be_nil
+
+          params = { "key" => "value" }
+          transaction.set_params_if_nil(params)
+
+          transaction.complete # Sample the data
+          expect(transaction.params).to eq(params)
+          expect(transaction.to_h.dig("sample_data", "params")).to eq(params)
+        end
+
+        context "when the given params is nil" do
+          it "does not update the params on the transaction" do
+            params = { "key" => "value" }
+            transaction.set_params(params)
+            transaction.set_params_if_nil(nil)
+
+            transaction.complete # Sample the data
+            expect(transaction.params).to eq(params)
+            expect(transaction.to_h.dig("sample_data", "params")).to eq(params)
+          end
+        end
+      end
+
+      context "when the params are set" do
+        it "does not update the params on the transaction" do
+          preset_params = { "other" => "params" }
+          params = { "key" => "value" }
+          transaction.set_params(preset_params)
+          transaction.set_params_if_nil(params)
+
+          transaction.complete # Sample the data
+          expect(transaction.params).to eq(preset_params)
+          expect(transaction.to_h.dig("sample_data", "params")).to eq(preset_params)
+        end
       end
     end