feat: thread safe configuration overrides (#500)

* chore: allow runtime configuration to be overridden for the scope of a request

* chore: pass in retry_schedule to webhook job

* chore: pass in http success codes to webhook job

* style: rubocop

* chore: pass in user agent to webhook job

* chore: remove unused code

* chore: remove configuration middleware from PB app as it will be added to PF

lib/pact_broker/api/middleware/configuration.rb

@@ -0,0 +1,33 @@
+require "pact_broker/logging"
+
+module PactBroker
+  module Api
+    module Middleware
+      class Configuration
+        include PactBroker::Logging
+
+        def initialize(app, configuration)
+          @app = app
+          @configuration = configuration
+        end
+
+        def call(env)
+          if (overrides = env["pactbroker.configuration_overrides"])&.any?
+            dupped_configuration = configuration.dup
+            dupped_configuration.override_runtime_configuration!(overrides)
+            dupped_configuration.freeze
+            PactBroker.set_configuration(dupped_configuration)
+            app.call(env)
+          else
+            PactBroker.set_configuration(configuration)
+            app.call(env)
+          end
+        end
+
+        private
+
+        attr_reader :app, :configuration
+      end
+    end
+  end
+end

lib/pact_broker/app.rb

@@ -25,6 +25,7 @@
 require "rack/pact_broker/add_vary_header"
 require "rack/pact_broker/use_when"
 require "sucker_punch"
+require "pact_broker/api/middleware/configuration"
 require "pact_broker/api/middleware/basic_auth"
 require "pact_broker/config/basic_auth_configuration"
 require "pact_broker/api/authorization/resource_access_policy"
@@ -51,6 +52,7 @@ def initialize
       load_configuration_from_database
       seed_example_data
       print_startup_message
+      @configuration.freeze
     end
 
     # Allows middleware to be inserted at the bottom of the shared middlware stack

lib/pact_broker/configuration.rb

@@ -4,10 +4,15 @@
 require "forwardable"
 require "pact_broker/config/runtime_configuration"
 require "anyway/auto_cast"
+require "request_store"
 
 module PactBroker
   def self.configuration
-    @@configuration ||= Configuration.default_configuration
+    RequestStore.store[:pact_broker_configuration] ||= Configuration.default_configuration
+  end
+
+  def self.set_configuration(configuration)
+    RequestStore.store[:pact_broker_configuration] = configuration
   end
 
   def self.with_runtime_configuration_overrides(overrides, &block)
@@ -16,7 +21,7 @@ def self.with_runtime_configuration_overrides(overrides, &block)
 
   # @private, for testing only
   def self.reset_configuration
-    @@configuration = Configuration.default_configuration
+    RequestStore.store[:pact_broker_configuration] = Configuration.default_configuration
   end
 
   class Configuration
@@ -83,12 +88,36 @@ def self.default_configuration
 
     def with_runtime_configuration_overrides(overrides)
       original_runtime_configuration = runtime_configuration
-      self.runtime_configuration = override_runtime_configuration(overrides, original_runtime_configuration.dup)
+      self.runtime_configuration = override_runtime_configuration!(overrides)
       yield
     ensure
       self.runtime_configuration = original_runtime_configuration
     end
 
+    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
+
+      if logger.debug?
+        logger.debug("Overridding runtime configuration", payload: { overrides: valid_overrides, ignoring: invalid_overrides })
+      end
+
+      valid_overrides.each do | key, value |
+        new_runtime_configuration.public_send("#{key}=", value)
+      end
+
+      self.runtime_configuration = new_runtime_configuration
+    end
+
     def logger_from_runtime_configuration
       @logger_from_runtime_configuration ||= begin
         runtime_configuration.validate_logging_attributes!
@@ -206,30 +235,5 @@ def load_from_database!
       require "pact_broker/config/load"
       PactBroker::Config::Load.call(runtime_configuration)
     end
-
-    private
-
-    def override_runtime_configuration(overrides, new_runtime_configuration)
-      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
-
-      if logger.debug?
-        logger.debug("Overridding runtime configuration", payload: { overrides: valid_overrides, ignoring: invalid_overrides })
-      end
-
-      valid_overrides.each do | key, value |
-        new_runtime_configuration.public_send("#{key}=", value)
-      end
-
-      new_runtime_configuration
-    end
   end
 end

lib/pact_broker/domain/webhook.rb

@@ -50,17 +50,12 @@ def request_description
       def execute pact, verification, event_context, options
         logger.info "Executing #{self} event_context=#{event_context}"
         template_params = template_parameters(pact, verification, event_context, options)
-        webhook_request = request.build(template_params)
+        webhook_request = request.build(template_params, options.fetch(:user_agent))
         http_response, error = execute_request(webhook_request)
-
-        logs = generate_logs(webhook_request, http_response, error, event_context, options.fetch(:logging_options))
+        success = success?(http_response, options)
         http_request = webhook_request.http_request
-        PactBroker::Webhooks::WebhookExecutionResult.new(
-          http_request,
-          http_response,
-          logs,
-          error
-        )
+        logs = generate_logs(webhook_request, http_response, success, error, event_context, options.fetch(:logging_options))
+        result(http_request, http_response, success, logs, error)
       end
 
       def to_s
@@ -116,16 +111,33 @@ def template_parameters(pact, verification, event_context, _options)
         PactBroker::Webhooks::PactAndVerificationParameters.new(pact, verification, event_context).to_hash
       end
 
-      def generate_logs(webhook_request, http_response, error, event_context, logging_options)
+      def success?(http_response, options)
+        !http_response.nil? && options.fetch(:http_success_codes).include?(http_response.code.to_i)
+      end
+
+      # rubocop: disable Metrics/ParameterLists
+      def generate_logs(webhook_request, http_response, success, error, event_context, logging_options)
         webhook_request_logger = PactBroker::Webhooks::WebhookRequestLogger.new(logging_options)
         webhook_request_logger.log(
           uuid,
           webhook_request,
           http_response,
+          success,
           error,
           event_context
         )
       end
+      # robocop: enable Metrics/ParameterLists
+
+      def result(http_request, http_response, success, logs, error)
+        PactBroker::Webhooks::WebhookExecutionResult.new(
+          http_request,
+          http_response,
+          success,
+          logs,
+          error
+        )
+      end
     end
   end
 end

lib/pact_broker/domain/webhook_request.rb

@@ -18,7 +18,7 @@ class WebhookRequest
 
       HEADERS_TO_REDACT = [/authorization/i, /token/i]
 
-      attr_accessor :method, :url, :headers, :body, :username, :password, :uuid
+      attr_accessor :method, :url, :headers, :body, :username, :password, :uuid, :user_agent
 
       # Reform gets confused by the :method method, as :method is a standard
       # Ruby method.
@@ -58,7 +58,7 @@ def http_request
         @http_request ||= begin
           req = Net::HTTP.const_get(method.capitalize).new(uri.request_uri)
           req.delete("accept-encoding")
-          req["user-agent"] = PactBroker.configuration.user_agent
+          req["user-agent"] = user_agent
           headers.each_pair { | name, value | req[name] = value }
           req.basic_auth(username, password) if username && username.size > 0
           req.body = body unless body.nil?

lib/pact_broker/test/test_data_builder.rb

@@ -340,7 +340,7 @@ def create_triggered_webhook params = {}
       def create_webhook_execution params = {}
         params.delete(:comment)
         logs = params[:logs] || "logs"
-        webhook_execution_result = PactBroker::Webhooks::WebhookExecutionResult.new(nil, OpenStruct.new(code: "200"), logs, nil)
+        webhook_execution_result = PactBroker::Webhooks::WebhookExecutionResult.new(nil, OpenStruct.new(code: "200"), true, logs, nil)
         @webhook_execution = PactBroker::Webhooks::Repository.new.create_execution @triggered_webhook, webhook_execution_result
         created_at = params[:created_at] || @pact.created_at + Rational(1, 86400)
         set_created_at_if_set created_at, :webhook_executions, {id: @webhook_execution.id}

lib/pact_broker/webhooks/execution_configuration.rb

@@ -25,14 +25,30 @@ def with_failure_log_message(value)
         with_updated_attribute(logging_options: { failure_log_message: value })
       end
 
+      def with_retry_schedule(value)
+        with_updated_attribute(retry_schedule: value)
+      end
+
+      def with_http_success_codes(value)
+        with_updated_attribute(http_success_codes: value)
+      end
+
       def with_webhook_context(value)
         with_updated_attribute(webhook_context: value)
       end
 
+      def with_user_agent(value)
+        with_updated_attribute(user_agent: value)
+      end
+
       def webhook_context
         self[:webhook_context]
       end
 
+      def retry_schedule
+        self[:retry_schedule]
+      end
+
       def [](key)
         params[key]
       end

lib/pact_broker/webhooks/execution_configuration_creator.rb

@@ -7,6 +7,9 @@ class ExecutionConfigurationCreator
       def self.call(resource)
         PactBroker::Webhooks::ExecutionConfiguration.new
           .with_show_response(PactBroker.configuration.show_webhook_response?)
+          .with_retry_schedule(PactBroker.configuration.webhook_retry_schedule)
+          .with_http_success_codes(PactBroker.configuration.webhook_http_code_success)
+          .with_user_agent(PactBroker.configuration.user_agent)
           .with_webhook_context(base_url: resource.base_url)
       end
     end

lib/pact_broker/webhooks/job.rb

@@ -104,7 +104,7 @@ def backoff_time
       end
 
       def retry_schedule
-        PactBroker.configuration.webhook_retry_schedule
+        data[:webhook_execution_configuration].retry_schedule
       end
     end
   end

lib/pact_broker/webhooks/trigger_service.rb

@@ -31,8 +31,8 @@ def test_execution webhook, event_context, execution_configuration
       end
 
       def execute_triggered_webhook_now triggered_webhook, webhook_execution_configuration_hash
-        webhook_execution_result = triggered_webhook.execute webhook_execution_configuration_hash
-        webhook_repository.create_execution triggered_webhook, webhook_execution_result
+        webhook_execution_result = triggered_webhook.execute(webhook_execution_configuration_hash)
+        webhook_repository.create_execution(triggered_webhook, webhook_execution_result)
         webhook_execution_result
       end
 

lib/pact_broker/webhooks/webhook_execution_result.rb

@@ -6,20 +6,16 @@ module Webhooks
     class WebhookExecutionResult
       attr_reader :request, :response, :logs, :error
 
-      def initialize(request, response, logs, error = nil)
+      def initialize(request, response, success, logs, error = nil)
         @request = PactBroker::Webhooks::HttpRequestWithRedactedHeaders.new(request)
         @response = response ? PactBroker::Webhooks::HttpResponseWithUtf8SafeBody.new(response) : nil
+        @success = success
         @logs = logs
         @error = error
       end
 
       def success?
-        if response
-          # Response HTTP Code must be in success list otherwise it is false
-          PactBroker.configuration.webhook_http_code_success.include? response.code.to_i
-        else
-          false
-        end
+        @success
       end
     end
   end

lib/pact_broker/webhooks/webhook_request_logger.rb

@@ -29,15 +29,17 @@ def initialize(options)
         @options = options
       end
 
-      def log(uuid, webhook_request, http_response, error, webhook_context)
+      # rubocop: disable Metrics/ParameterLists
+      def log(uuid, webhook_request, http_response, success, error, webhook_context)
         safe_response = http_response ? HttpResponseWithUtf8SafeBody.new(http_response) : nil
         log_webhook_context(webhook_context)
         log_request(webhook_request)
         log_response(uuid, safe_response, webhook_context[:base_url]) if safe_response
         log_error(uuid, error, webhook_context[:base_url]) if error
-        log_completion_message(success?(safe_response))
+        log_completion_message(success)
         log_stream.string
       end
+      # rubocop: enable Metrics/ParameterLists
 
       private
 
@@ -115,16 +117,6 @@ def log_error uuid, e, base_url
           execution_logger.error "Error executing webhook #{uuid}. #{response_body_hidden_message(base_url)}"
         end
       end
-
-      def success?(response)
-        if response
-          # Response HTTP Code must be in success list otherwise it is false
-          PactBroker.configuration.webhook_http_code_success.include? response.code.to_i
-        else
-          false
-        end
-      end
-
     end
   end
 end

lib/pact_broker/webhooks/webhook_request_template.rb

@@ -21,24 +21,22 @@ class WebhookRequestTemplate
       alias_method :http_method, :method
 
       def initialize attributes = {}
-        @method = attributes[:method]
-        @url = attributes[:url]
-        @username = attributes[:username]
-        @password = attributes[:password]
+        attributes.each do | (name, value) |
+          instance_variable_set("@#{name}", value) if respond_to?(name)
+        end
         @headers = Rack::Utils::HeaderHash.new(attributes[:headers] || {})
-        @body = attributes[:body]
-        @uuid = attributes[:uuid]
       end
 
-      def build(template_params)
+      def build(template_params, user_agent)
         attributes = {
           method: http_method,
           url: build_url(template_params),
           headers: build_headers(template_params),
           username: build_string(username, template_params),
           password: build_string(password, template_params),
           uuid: uuid,
-          body: build_body(template_params)
+          body: build_body(template_params),
+          user_agent: user_agent
         }
         PactBroker::Domain::WebhookRequest.new(attributes)
       end

pact_broker.gemspec

@@ -66,4 +66,5 @@ Gem::Specification.new do |gem|
   gem.add_runtime_dependency "sanitize", ">= 5.2.1", "~> 5.2"
   gem.add_runtime_dependency "wisper", "~> 2.0"
   gem.add_runtime_dependency "anyway_config", "~> 2.1"
+  gem.add_runtime_dependency "request_store", "~> 1.5"
 end