feat(semantic-logging): allow ruby standard logger to be configured to ensure backwards compatibility

bethesque · bethesque · commit cfbb704c864d · 2018-10-04T15:45:50.000+10:00
diff --git a/lib/db.rb b/lib/db.rb
@@ -6,7 +6,7 @@
 require 'pact_broker/project_root'
 
 module DB
-  include SemanticLogger::Loggable
+  include PactBroker::Logging
   ##
   # Sequel by default does not test connections in its connection pool before
   # handing them to a client. To enable connection testing you need to load the
diff --git a/lib/pact_broker/api/renderers/html_pact_renderer.rb b/lib/pact_broker/api/renderers/html_pact_renderer.rb
@@ -12,7 +12,7 @@ class HtmlPactRenderer
 
         class NotAPactError < StandardError; end
 
-        include SemanticLogger::Loggable
+        include PactBroker::Logging
 
         def self.call pact, options = {}
           new(pact, options).call
diff --git a/lib/pact_broker/api/resources/base_resource.rb b/lib/pact_broker/api/resources/base_resource.rb
@@ -21,7 +21,7 @@ class BaseResource < Webmachine::Resource
         include PactBroker::Services
         include PactBroker::Api::PactBrokerUrls
         include PactBroker::Api::Resources::Authentication
-        include SemanticLogger::Loggable
+        include PactBroker::Logging
 
 
         attr_accessor :user
diff --git a/lib/pact_broker/api/resources/error_handler.rb b/lib/pact_broker/api/resources/error_handler.rb
@@ -6,7 +6,7 @@ module Api
     module Resources
       class ErrorHandler
 
-        include SemanticLogger::Loggable
+        include PactBroker::Logging
         include PactBroker::Logging
 
         def self.call e, request, response
diff --git a/lib/pact_broker/app.rb b/lib/pact_broker/app.rb
@@ -19,7 +19,7 @@
 module PactBroker
 
   class App
-    include SemanticLogger::Loggable
+    include PactBroker::Logging
 
     attr_accessor :configuration
 
diff --git a/lib/pact_broker/badges/service.rb b/lib/pact_broker/badges/service.rb
@@ -10,7 +10,7 @@ module Badges
     module Service
 
       extend self
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       SPACE_DASH_UNDERSCORE = /[\s_\-]/
       CACHE = {}
diff --git a/lib/pact_broker/certificates/service.rb b/lib/pact_broker/certificates/service.rb
@@ -8,7 +8,7 @@ module Service
 
       extend self
       extend PactBroker::Logging
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       def cert_store
         cert_store = OpenSSL::X509::Store.new
diff --git a/lib/pact_broker/config/load.rb b/lib/pact_broker/config/load.rb
@@ -7,7 +7,7 @@ module PactBroker
   module Config
     class Load
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       def self.call configuration
         new(configuration).call
diff --git a/lib/pact_broker/config/save.rb b/lib/pact_broker/config/save.rb
@@ -6,7 +6,7 @@ module PactBroker
   module Config
     class Save
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       def self.call configuration, setting_names
         new(configuration, setting_names).call
diff --git a/lib/pact_broker/configuration.rb b/lib/pact_broker/configuration.rb
@@ -16,7 +16,6 @@ def self.reset_configuration
   end
 
   class Configuration
-    include SemanticLogger::Loggable
 
     SAVABLE_SETTING_NAMES = [
       :order_versions_by_date,
@@ -44,14 +43,15 @@ class Configuration
     attr_accessor :disable_ssl_verification
     attr_accessor :base_equality_only_on_content_that_affects_verification_results
     attr_reader :api_error_reporters
-    attr_writer :logger
+    attr_reader :custom_logger
 
     def initialize
       @before_resource_hook = ->(resource){}
       @after_resource_hook = ->(resource){}
       @authenticate_with_basic_auth = nil
       @authorize = nil
       @api_error_reporters = []
+      @semantic_logger = SemanticLogger[Configuration]
     end
 
     def self.default_configuration
@@ -83,6 +83,14 @@ def self.default_configuration
       config
     end
 
+    def logger
+      custom_logger || @semantic_logger
+    end
+
+    def logger= logger
+      @custom_logger = logger
+    end
+
     def self.default_html_pact_render
       lambda { |pact, options|
         require 'pact_broker/api/renderers/html_pact_renderer'
@@ -199,9 +207,5 @@ def parse_space_delimited_string_list_property property_name, property_value
         raise ConfigurationError.new("Pact Broker configuration property `#{property_name}` must be a space delimited String or an Array")
       end
     end
-
-    def log_path
-      log_dir + "/pact_broker.log"
-    end
   end
 end
diff --git a/lib/pact_broker/domain/order_versions.rb b/lib/pact_broker/domain/order_versions.rb
@@ -4,7 +4,7 @@ module PactBroker
   module Domain
     class OrderVersions
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       def self.call new_version
         new_version.lock!
diff --git a/lib/pact_broker/domain/webhook_request.rb b/lib/pact_broker/domain/webhook_request.rb
@@ -53,7 +53,7 @@ def redact? name
 
     class WebhookRequest
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
       include PactBroker::Messages
       HEADERS_TO_REDACT = [/authorization/i, /token/i]
 
diff --git a/lib/pact_broker/logging.rb b/lib/pact_broker/logging.rb
@@ -2,10 +2,33 @@
 require 'semantic_logger'
 
 module PactBroker
-
   module Logging
+
     def self.included(base)
-      base.extend(self)
+      base.extend self
+      base.extend SemanticLogger::Loggable::ClassMethods
+      base.class_eval do
+        # Returns [SemanticLogger::Logger] class level logger
+        def self.logger
+          require 'pact_broker/configuration'
+          @logger ||= PactBroker.configuration.custom_logger || SemanticLogger[self]
+        end
+
+        # Replace instance class level logger
+        def self.logger=(logger)
+          @logger = logger
+        end
+
+        # Returns [SemanticLogger::Logger] instance level logger
+        def logger
+          @logger ||= self.class.logger
+        end
+
+        # Replace instance level logger
+        def logger=(logger)
+          @logger = logger
+        end
+      end
     end
 
     def log_error e, description = nil
diff --git a/lib/pact_broker/matrix/deployment_status_summary.rb b/lib/pact_broker/matrix/deployment_status_summary.rb
@@ -3,6 +3,8 @@
 module PactBroker
   module Matrix
     class DeploymentStatusSummary
+      include PactBroker::Logging
+
       attr_reader :rows, :resolved_selectors, :integrations
 
       def initialize(rows, resolved_selectors, integrations)
@@ -88,7 +90,7 @@ def resolved_version_for(pacticipant_id)
         if resolved_selector
           resolved_selector[:pacticipant_version_number]
         else
-          PactBroker.logger.warn "Could not find the resolved version for pacticipant_id #{pacticipant_id} from integrations #{integrations.collect(&:to_s).join(", ")} in resolved selectors #{resolved_selectors.inspect}"
+          logger.warn "Could not find the resolved version for pacticipant_id #{pacticipant_id} from integrations #{integrations.collect(&:to_s).join(", ")} in resolved selectors #{resolved_selectors.inspect}"
           "unresolved version"
         end
       end
diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb
@@ -24,7 +24,7 @@ class Row < Sequel::Model(:matrix)
 
       dataset_module do
         include PactBroker::Repositories::Helpers
-        include SemanticLogger::Loggable
+        include PactBroker::Logging
 
         def matching_selectors selectors
           if selectors.size == 1
diff --git a/lib/pact_broker/pacticipants/service.rb b/lib/pact_broker/pacticipants/service.rb
@@ -10,7 +10,7 @@ class Service
 
       extend PactBroker::Repositories
       extend PactBroker::Services
-      extend PactBroker::Logging
+      include PactBroker::Logging
 
       def self.messages_for_potential_duplicate_pacticipants pacticipant_names, base_url
         messages = []
diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb
@@ -17,7 +17,7 @@ module PactBroker
   module Pacts
     class Repository
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
       include PactBroker::Repositories
 
       def create params
diff --git a/lib/pact_broker/pacts/service.rb b/lib/pact_broker/pacts/service.rb
@@ -11,7 +11,7 @@ module Service
 
       extend PactBroker::Repositories
       extend PactBroker::Services
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
 
       def find_latest_pact params
         pact_repository.find_latest_pact(params[:consumer_name], params[:provider_name], params[:tag])
diff --git a/lib/pact_broker/verifications/service.rb b/lib/pact_broker/verifications/service.rb
@@ -1,6 +1,7 @@
 require 'pact_broker/repositories'
 require 'pact_broker/api/decorators/verification_decorator'
 require 'pact_broker/verifications/summary_for_consumer_version'
+require 'pact_broker/logging'
 
 module PactBroker
 
@@ -11,13 +12,14 @@ module Service
 
       extend PactBroker::Repositories
       extend PactBroker::Services
+      include PactBroker::Logging
 
       def next_number
         verification_repository.next_number
       end
 
       def create next_verification_number, params, pact
-        PactBroker.logger.info "Creating verification #{next_verification_number} for pact_id=#{pact.id} from params #{params}"
+        logger.info "Creating verification #{next_verification_number} for pact_id=#{pact.id} from params #{params}"
         verification = PactBroker::Domain::Verification.new
         provider_version_number = params.fetch('providerApplicationVersion')
         PactBroker::Api::Decorators::VerificationDecorator.new(verification).from_hash(params)
diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb
@@ -1,11 +1,13 @@
 require 'sequel'
+require 'pact_broker/logging'
 require 'pact_broker/domain/version'
 require 'pact_broker/tags/repository'
 
 module PactBroker
   module Versions
     class Repository
 
+      include PactBroker::Logging
       include PactBroker::Repositories::Helpers
 
       def find_by_pacticipant_id_and_number pacticipant_id, number
@@ -53,7 +55,7 @@ def find_by_pacticipant_name_and_number pacticipant_name, number
 
       # There may be a race condition if two simultaneous requests come in to create the same version
       def create args
-        PactBroker.logger.info "Upserting version #{args[:number]} for pacticipant_id=#{args[:pacticipant_id]}"
+        logger.info "Upserting version #{args[:number]} for pacticipant_id=#{args[:pacticipant_id]}"
         version_params = {
           number: args[:number],
           pacticipant_id: args[:pacticipant_id],
diff --git a/lib/pact_broker/webhooks/job.rb b/lib/pact_broker/webhooks/job.rb
@@ -7,7 +7,7 @@ module Webhooks
     class Job
 
       include SuckerPunch::Job
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
       include PactBroker::Logging
 
       def perform data
diff --git a/lib/pact_broker/webhooks/webhook_request_template.rb b/lib/pact_broker/webhooks/webhook_request_template.rb
@@ -7,7 +7,7 @@ module PactBroker
   module Webhooks
     class WebhookRequestTemplate
 
-      include SemanticLogger::Loggable
+      include PactBroker::Logging
       include PactBroker::Messages
       HEADERS_TO_REDACT = [/authorization/i, /token/i]
 
diff --git a/spec/lib/pact_broker/domain/webhook_spec.rb b/spec/lib/pact_broker/domain/webhook_spec.rb
@@ -11,6 +11,11 @@ module Domain
       let(:options) { double('options') }
       let(:pact) { double('pact') }
       let(:verification) { double('verification') }
+      let(:logger) { double('logger').as_null_object }
+
+      before do
+        allow(webhook).to receive(:logger).and_return(logger)
+      end
 
       subject(:webhook) { Webhook.new(request: request_template, consumer: consumer, provider: provider) }
 
@@ -59,8 +64,8 @@ module Domain
         end
 
         it "logs before and after" do
-          allow(PactBroker.logger).to receive(:info)
-          expect(PactBroker.logger).to receive(:info).with(/Executing/)
+          allow(logger).to receive(:info)
+          expect(logger).to receive(:info).with(/Executing/)
           execute
         end
       end
diff --git a/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb b/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb
@@ -6,6 +6,13 @@
 module PactBroker
   module Matrix
     describe DeploymentStatusSummary do
+
+      before do
+        allow(subject).to receive(:logger).and_return(logger)
+      end
+
+      let(:logger) { double('logger').as_null_object }
+
       describe ".call" do
         let(:rows) { [row_1, row_2] }
         let(:row_1) do
@@ -106,7 +113,7 @@ module Matrix
           its(:reasons) { is_expected.to eq ["There is no verified pact between Foo (unresolved version) and Baz (4ee06460f10e8207ad904fa9fa6c4842e462ab59)"] }
 
           it "logs a warning" do
-            expect(PactBroker.logger).to receive(:warn).with(/Could not find the resolved version/)
+            expect(logger).to receive(:warn).with(/Could not find the resolved version/)
             subject.reasons
           end
         end
diff --git a/spec/lib/pact_broker/pacticipants/service_spec.rb b/spec/lib/pact_broker/pacticipants/service_spec.rb
@@ -4,11 +4,14 @@
 require 'pact_broker/domain/pact'
 
 module PactBroker
-
   module Pacticipants
     describe Service do
+      before do
+        allow(Service).to receive(:logger).and_return(logger)
+      end
 
       let(:td) { TestDataBuilder.new }
+      let(:logger) { double('logger').as_null_object }
 
       subject{ Service }
 
@@ -106,8 +109,7 @@ module Pacticipants
         end
 
         it "logs the names" do
-          allow(PactBroker.logger).to receive(:info)
-          expect(PactBroker.logger).to receive(:info).with(/pacticipant_name.*Fred, Mary/)
+          expect(logger).to receive(:info).with(/pacticipant_name.*Fred, Mary/)
           subject.find_potential_duplicate_pacticipants pacticipant_name
         end
       end
diff --git a/spec/lib/pact_broker/verifications/service_spec.rb b/spec/lib/pact_broker/verifications/service_spec.rb
@@ -5,6 +5,11 @@ module PactBroker
 
   module Verifications
     describe Service do
+      before do
+        allow(Service).to receive(:logger).and_return(logger)
+      end
+
+      let(:logger) { double('logger').as_null_object }
 
       subject { PactBroker::Verifications::Service }
 
@@ -18,8 +23,7 @@ module Verifications
         let(:create_verification) { subject.create 3, params, pact }
 
         it "logs the creation" do
-          allow(PactBroker.logger).to receive(:info).and_call_original
-          expect(PactBroker.logger).to receive(:info).with(/.*verification.*3.*success/)
+          expect(logger).to receive(:info).with(/.*verification.*3.*success/)
           create_verification
         end
 
diff --git a/spec/lib/pact_broker/webhooks/service_spec.rb b/spec/lib/pact_broker/webhooks/service_spec.rb
