Merge pull request #38 from github/copilot/fix-37

Comprehensive codebase refinement: DRY principles, YARD documentation, and code quality improvements

Author: GrantBirki

lib/hooks.rb

@@ -7,6 +7,7 @@
 require_relative "hooks/core/logger_factory"
 require_relative "hooks/core/plugin_loader"
 require_relative "hooks/core/global_components"
+require_relative "hooks/core/component_access"
 require_relative "hooks/core/log"
 require_relative "hooks/core/failbot"
 require_relative "hooks/core/stats"

lib/hooks/core/component_access.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module Hooks
+  module Core
+    # Shared module providing access to global components (logger, stats, failbot)
+    #
+    # This module provides a consistent interface for accessing global components
+    # across all plugin types, eliminating code duplication and ensuring consistent
+    # behavior throughout the application.
+    #
+    # @example Usage in a class that needs instance methods
+    #   class MyHandler
+    #     include Hooks::Core::ComponentAccess
+    #
+    #     def process
+    #       log.info("Processing request")
+    #       stats.increment("requests.processed")
+    #       failbot.report("Error occurred") if error?
+    #     end
+    #   end
+    #
+    # @example Usage in a class that needs class methods
+    #   class MyValidator
+    #     extend Hooks::Core::ComponentAccess
+    #
+    #     def self.validate
+    #       log.info("Validating request")
+    #       stats.increment("requests.validated")
+    #     end
+    #   end
+    module ComponentAccess
+      # Short logger accessor
+      # @return [Hooks::Log] Logger instance for logging messages
+      #
+      # Provides a convenient way to log messages without needing
+      # to reference the full Hooks::Log namespace.
+      #
+      # @example Logging an error
+      #   log.error("Something went wrong")
+      def log
+        Hooks::Log.instance
+      end
+
+      # Global stats component accessor
+      # @return [Hooks::Plugins::Instruments::Stats] Stats instance for metrics reporting
+      #
+      # Provides access to the global stats component for reporting metrics
+      # to services like DataDog, New Relic, etc.
+      #
+      # @example Recording a metric
+      #   stats.increment("webhook.processed", { handler: "MyHandler" })
+      def stats
+        Hooks::Core::GlobalComponents.stats
+      end
+
+      # Global failbot component accessor
+      # @return [Hooks::Plugins::Instruments::Failbot] Failbot instance for error reporting
+      #
+      # Provides access to the global failbot component for reporting errors
+      # to services like Sentry, Rollbar, etc.
+      #
+      # @example Reporting an error
+      #   failbot.report("Something went wrong", { context: "additional info" })
+      def failbot
+        Hooks::Core::GlobalComponents.failbot
+      end
+    end
+  end
+end

lib/hooks/core/log.rb

@@ -1,8 +1,23 @@
 # frozen_string_literal: true
 
 module Hooks
+  # Global logger accessor module
+  #
+  # Provides a singleton-like access pattern for the application logger.
+  # The logger instance is set during application initialization and can
+  # be accessed throughout the application lifecycle.
+  #
+  # @example Setting the logger instance
+  #   Hooks::Log.instance = Logger.new(STDOUT)
+  #
+  # @example Accessing the logger
+  #   Hooks::Log.instance.info("Application started")
   module Log
     class << self
+      # Get or set the global logger instance
+      # @return [Logger] The global logger instance
+      # @attr_reader instance [Logger] Current logger instance
+      # @attr_writer instance [Logger] Set the logger instance
       attr_accessor :instance
     end
   end

lib/hooks/plugins/auth/base.rb

@@ -3,6 +3,7 @@
 require "rack/utils"
 require_relative "../../core/log"
 require_relative "../../core/global_components"
+require_relative "../../core/component_access"
 
 module Hooks
   module Plugins
@@ -11,6 +12,8 @@ module Auth
       #
       # All custom Auth plugins must inherit from this class
       class Base
+        extend Hooks::Core::ComponentAccess
+
         # Validate request
         #
         # @param payload [String] Raw request body
@@ -22,42 +25,6 @@ def self.valid?(payload:, headers:, config:)
           raise NotImplementedError, "Validator must implement .valid? class method"
         end
 
-        # Short logger accessor for all subclasses
-        # @return [Hooks::Log] Logger instance for request validation
-        #
-        # Provides a convenient way for validators to log messages without needing
-        # to reference the full Hooks::Log namespace.
-        #
-        # @example Logging an error in an inherited class
-        #   log.error("oh no an error occured")
-        def self.log
-          Hooks::Log.instance
-        end
-
-        # Global stats component accessor
-        # @return [Hooks::Core::Stats] Stats instance for metrics reporting
-        #
-        # Provides access to the global stats component for reporting metrics
-        # to services like DataDog, New Relic, etc.
-        #
-        # @example Recording a metric in an inherited class
-        #   stats.increment("auth.validation", { plugin: "hmac" })
-        def self.stats
-          Hooks::Core::GlobalComponents.stats
-        end
-
-        # Global failbot component accessor
-        # @return [Hooks::Core::Failbot] Failbot instance for error reporting
-        #
-        # Provides access to the global failbot component for reporting errors
-        # to services like Sentry, Rollbar, etc.
-        #
-        # @example Reporting an error in an inherited class
-        #   failbot.report("Auth validation failed", { plugin: "hmac" })
-        def self.failbot
-          Hooks::Core::GlobalComponents.failbot
-        end
-
         # Retrieve the secret from the environment variable based on the key set in the configuration
         #
         # Note: This method is intended to be used by subclasses

lib/hooks/plugins/auth/hmac.rb

@@ -40,6 +40,7 @@ class HMAC < Base
         DEFAULT_CONFIG = {
           algorithm: "sha256",
           format: "algorithm=signature",  # Format: algorithm=hash
+          header: "X-Signature",         # Default header containing the signature
           timestamp_tolerance: 300,       # 5 minutes tolerance for timestamp validation
           version_prefix: "v0"           # Default version prefix for versioned signatures
         }.freeze
@@ -157,7 +158,7 @@ def self.build_config(config)
           tolerance = validator_config[:timestamp_tolerance] || DEFAULT_CONFIG[:timestamp_tolerance]
 
           DEFAULT_CONFIG.merge({
-            header: validator_config[:header] || "X-Signature",
+            header: validator_config[:header] || DEFAULT_CONFIG[:header],
             timestamp_header: validator_config[:timestamp_header],
             timestamp_tolerance: tolerance,
             algorithm: algorithm,

lib/hooks/plugins/handlers/base.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative "../../core/global_components"
+require_relative "../../core/component_access"
 
 module Hooks
   module Plugins
@@ -9,6 +10,8 @@ module Handlers
       #
       # All custom handlers must inherit from this class and implement the #call method
       class Base
+        include Hooks::Core::ComponentAccess
+
         # Process a webhook request
         #
         # @param payload [Hash, String] Parsed request body (JSON Hash) or raw string
@@ -19,42 +22,6 @@ class Base
         def call(payload:, headers:, config:)
           raise NotImplementedError, "Handler must implement #call method"
         end
-
-        # Short logger accessor for all subclasses
-        # @return [Hooks::Log] Logger instance
-        #
-        # Provides a convenient way for handlers to log messages without needing
-        # to reference the full Hooks::Log namespace.
-        #
-        # @example Logging an error in an inherited class
-        #   log.error("oh no an error occured")
-        def log
-          Hooks::Log.instance
-        end
-
-        # Global stats component accessor
-        # @return [Hooks::Core::Stats] Stats instance for metrics reporting
-        #
-        # Provides access to the global stats component for reporting metrics
-        # to services like DataDog, New Relic, etc.
-        #
-        # @example Recording a metric in an inherited class
-        #   stats.increment("webhook.processed", { handler: "MyHandler" })
-        def stats
-          Hooks::Core::GlobalComponents.stats
-        end
-
-        # Global failbot component accessor
-        # @return [Hooks::Core::Failbot] Failbot instance for error reporting
-        #
-        # Provides access to the global failbot component for reporting errors
-        # to services like Sentry, Rollbar, etc.
-        #
-        # @example Reporting an error in an inherited class
-        #   failbot.report("Something went wrong", { handler: "MyHandler" })
-        def failbot
-          Hooks::Core::GlobalComponents.failbot
-        end
       end
     end
   end

lib/hooks/plugins/handlers/default.rb

@@ -1,8 +1,37 @@
 # frozen_string_literal: true
 
-# Default handler when no custom handler is found
-# This handler simply acknowledges receipt of the webhook and shows a few of the built-in features
+# Default webhook handler implementation
+#
+# This handler provides a basic webhook processing implementation that can be used
+# as a fallback when no custom handler is configured for an endpoint. It demonstrates
+# the standard handler interface and provides basic logging functionality.
+#
+# @example Usage in endpoint configuration
+#   handler:
+#     type: DefaultHandler
+#
+# @see Hooks::Plugins::Handlers::Base
 class DefaultHandler < Hooks::Plugins::Handlers::Base
+  # Process a webhook request with basic acknowledgment
+  #
+  # Provides a simple webhook processing implementation that logs the request
+  # and returns a standard acknowledgment response. This is useful for testing
+  # webhook endpoints or as a placeholder during development.
+  #
+  # @param payload [Hash, String] The webhook payload (parsed JSON or raw string)
+  # @param headers [Hash<String, String>] HTTP headers from the webhook request
+  # @param config [Hash] Endpoint configuration containing handler options
+  # @return [Hash] Response indicating successful processing
+  # @option config [Hash] :opts Additional handler-specific configuration options
+  #
+  # @example Basic usage
+  #   handler = DefaultHandler.new
+  #   response = handler.call(
+  #     payload: { "event" => "push" },
+  #     headers: { "Content-Type" => "application/json" },
+  #     config: { opts: {} }
+  #   )
+  #   # => { message: "webhook processed successfully", handler: "DefaultHandler", timestamp: "..." }
   def call(payload:, headers:, config:)
 
     log.info("🔔 Default handler invoked for webhook 🔔")

lib/hooks/plugins/instruments/failbot.rb

@@ -7,11 +7,25 @@ module Plugins
     module Instruments
       # Default failbot instrument implementation
       #
-      # This is a stub implementation that does nothing by default.
-      # Users can replace this with their own implementation for services
-      # like Sentry, Rollbar, etc.
+      # This is a no-op implementation that provides the error reporting interface
+      # without actually sending errors anywhere. It serves as a safe default when
+      # no custom error reporting implementation is configured.
+      #
+      # Users should replace this with their own implementation for services
+      # like Sentry, Rollbar, Honeybadger, etc.
+      #
+      # @example Replacing with a custom implementation
+      #   # In your application initialization:
+      #   custom_failbot = MySentryFailbotImplementation.new
+      #   Hooks::Core::GlobalComponents.failbot = custom_failbot
+      #
+      # @see Hooks::Plugins::Instruments::FailbotBase
+      # @see Hooks::Core::GlobalComponents
       class Failbot < FailbotBase
-        # Inherit from FailbotBase to provide a default implementation of the failbot instrument.
+        # Inherit from FailbotBase to provide a default no-op implementation
+        # of the error reporting instrument interface.
+        #
+        # All methods from FailbotBase are inherited and provide safe no-op behavior.
       end
     end
   end

lib/hooks/plugins/instruments/failbot_base.rb

@@ -1,23 +1,70 @@
 # frozen_string_literal: true
 
+require_relative "../../core/component_access"
+
 module Hooks
   module Plugins
     module Instruments
       # Base class for all failbot instrument plugins
       #
-      # All custom failbot implementations must inherit from this class and implement
-      # the required methods for error reporting.
+      # This class provides the foundation for implementing custom error reporting
+      # instruments. Subclasses should implement specific methods for their target
+      # error reporting service (Sentry, Rollbar, Honeybadger, etc.).
+      #
+      # @abstract Subclass and implement service-specific error reporting methods
+      # @example Implementing a custom failbot instrument
+      #   class MySentryFailbot < Hooks::Plugins::Instruments::FailbotBase
+      #     def report(error_or_message, context = {})
+      #       case error_or_message
+      #       when Exception
+      #         Sentry.capture_exception(error_or_message, extra: context)
+      #       else
+      #         Sentry.capture_message(error_or_message.to_s, extra: context)
+      #       end
+      #       log.debug("Reported error to Sentry")
+      #     end
+      #   end
+      #
+      # @see Hooks::Plugins::Instruments::Failbot
       class FailbotBase
-        # Short logger accessor for all subclasses
-        # @return [Hooks::Log] Logger instance
+        include Hooks::Core::ComponentAccess
+
+        # Report an error or message to the error tracking service
+        #
+        # This is a no-op implementation that subclasses should override
+        # to provide actual error reporting functionality.
+        #
+        # @param error_or_message [Exception, String] The error to report or message string
+        # @param context [Hash] Additional context information about the error
+        # @return [void]
+        # @note Subclasses should implement this method for their specific service
+        # @example Override in subclass
+        #   def report(error_or_message, context = {})
+        #     if error_or_message.is_a?(Exception)
+        #       ErrorService.report_exception(error_or_message, context)
+        #     else
+        #       ErrorService.report_message(error_or_message, context)
+        #     end
+        #   end
+        def report(error_or_message, context = {})
+          # No-op implementation for base class
+        end
+
+        # Report a warning-level message
         #
-        # Provides a convenient way for instruments to log messages without needing
-        # to reference the full Hooks::Log namespace.
+        # This is a no-op implementation that subclasses should override
+        # to provide actual warning reporting functionality.
         #
-        # @example Logging debug info in an inherited class
-        #   log.debug("Sending error to external service")
-        def log
-          Hooks::Log.instance
+        # @param message [String] Warning message to report
+        # @param context [Hash] Additional context information
+        # @return [void]
+        # @note Subclasses should implement this method for their specific service
+        # @example Override in subclass
+        #   def warn(message, context = {})
+        #     ErrorService.report_warning(message, context)
+        #   end
+        def warn(message, context = {})
+          # No-op implementation for base class
         end
       end
     end