Merge pull request #63 from github/config-updates

Config Updates

Author: GrantBirki

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hooks-ruby (0.4.0)
+    hooks-ruby (0.5.0)
       dry-schema (~> 1.14, >= 1.14.1)
       grape (~> 2.3)
       puma (~> 6.6)

docs/design.md

@@ -109,7 +109,7 @@ export HOOKS_ROOT_PATH="/webhooks"
 export HOOKS_LOG_LEVEL=info
 
 # Paths
-export HOOKS_HANDLER_DIR=./handlers
+export HOOKS_HANDLER_PLUGIN_DIR=./handlers
 export HOOKS_HEALTH_PATH=/health
 export HOOKS_VERSION_PATH=/version
 
@@ -163,7 +163,7 @@ lib/hooks/
 ```yaml
 # config/endpoints/team1.yaml
 path: /team1                  # Mounted at <root_path>/team1
-handler: Team1Handler         # Class in handler_dir
+handler: Team1Handler         # Class in handler_plugin_dir
 
 # Signature validation
 auth:
@@ -181,7 +181,7 @@ opts:                         # Freeform user-defined options
 
 ```yaml
 # config/config.yaml
-handler_dir:     ./handlers         # handler class directory
+handler_plugin_dir:     ./handlers         # handler class directory
 log_level:       info               # debug | info | warn | error
 
 # Request handling
@@ -345,7 +345,7 @@ app = Hooks.build(
 
 **Handler & Plugin Discovery:**
 
-* Handler classes are auto-discovered from `handler_dir` using file naming convention
+* Handler classes are auto-discovered from `handler_plugin_dir` using file naming convention
 * File `team1_handler.rb` → class `Team1Handler`
 * Plugin classes are loaded from `plugin_dir` and registered based on class inheritance
 * All classes must inherit from appropriate base classes to be recognized

lib/hooks/core/config_loader.rb

@@ -32,16 +32,24 @@ class ConfigLoader
       #
       # @param config_path [String, Hash] Path to config file or config hash
       # @return [Hash] Merged configuration
+      # @raise [ArgumentError] if config file path is provided but file doesn't exist
+      # @raise [RuntimeError] if config file exists but fails to load
       def self.load(config_path: nil)
         config = DEFAULT_CONFIG.dup
         overrides = []
 
         # Load from file if path provided
-        if config_path.is_a?(String) && File.exist?(config_path)
+        if config_path.is_a?(String)
+          unless File.exist?(config_path)
+            raise ArgumentError, "Configuration file not found: #{config_path}"
+          end
+
           file_config = load_config_file(config_path)
           if file_config
             overrides << "file config"
             config.merge!(file_config)
+          else
+            raise RuntimeError, "Failed to load configuration from file: #{config_path}"
           end
         end
 
@@ -127,7 +135,6 @@ def self.load_env_config
         env_config = {}
 
         env_mappings = {
-          "HOOKS_HANDLER_DIR" => :handler_dir,
           "HOOKS_HANDLER_PLUGIN_DIR" => :handler_plugin_dir,
           "HOOKS_AUTH_PLUGIN_DIR" => :auth_plugin_dir,
           "HOOKS_LIFECYCLE_PLUGIN_DIR" => :lifecycle_plugin_dir,

lib/hooks/core/config_validator.rb

@@ -12,18 +12,17 @@ class ValidationError < StandardError; end
 
       # Global configuration schema
       GLOBAL_CONFIG_SCHEMA = Dry::Schema.Params do
-        optional(:handler_dir).filled(:string)  # For backward compatibility
-        optional(:handler_plugin_dir).filled(:string)
+        required(:handler_plugin_dir).filled(:string)
         optional(:auth_plugin_dir).maybe(:string)
         optional(:lifecycle_plugin_dir).maybe(:string)
         optional(:instruments_plugin_dir).maybe(:string)
-        optional(:log_level).filled(:string, included_in?: %w[debug info warn error])
+        required(:log_level).filled(:string, included_in?: %w[debug info warn error])
         optional(:request_limit).filled(:integer, gt?: 0)
         optional(:request_timeout).filled(:integer, gt?: 0)
-        optional(:root_path).filled(:string)
+        required(:root_path).filled(:string)
         optional(:health_path).filled(:string)
         optional(:version_path).filled(:string)
-        optional(:environment).filled(:string, included_in?: %w[development production])
+        required(:environment).filled(:string, included_in?: %w[development production])
         optional(:endpoints_dir).filled(:string)
         optional(:use_catchall_route).filled(:bool)
         optional(:normalize_headers).filled(:bool)

lib/hooks/version.rb

@@ -4,5 +4,5 @@
 module Hooks
   # Current version of the Hooks webhook framework
   # @return [String] The version string following semantic versioning
-  VERSION = "0.4.0".freeze
+  VERSION = "0.5.0".freeze
 end

spec/unit/lib/hooks/core/config_loader_spec.rb

@@ -101,12 +101,10 @@
       context "when file does not exist" do
         let(:config_file) { File.join(temp_dir, "nonexistent.yml") }
 
-        it "returns default configuration" do
-          config = described_class.load(config_path: config_file)
-
-          expect(config[:log_level]).to eq("info")
-          expect(config[:environment]).to eq("production")
-          expect(config[:production]).to be true
+        it "raises ArgumentError" do
+          expect {
+            described_class.load(config_path: config_file)
+          }.to raise_error(ArgumentError, "Configuration file not found: #{config_file}")
         end
       end
 
@@ -117,11 +115,10 @@
           File.write(config_file, "invalid: yaml: content: [")
         end
 
-        it "returns default configuration" do
-          config = described_class.load(config_path: config_file)
-
-          expect(config[:log_level]).to eq("info")
-          expect(config[:environment]).to eq("production")
+        it "raises RuntimeError" do
+          expect {
+            described_class.load(config_path: config_file)
+          }.to raise_error(RuntimeError, "Failed to load configuration from file: #{config_file}")
         end
       end
 
@@ -132,11 +129,10 @@
           File.write(config_file, "log_level: debug")
         end
 
-        it "returns default configuration" do
-          config = described_class.load(config_path: config_file)
-
-          expect(config[:log_level]).to eq("info")
-          expect(config[:environment]).to eq("production")
+        it "raises RuntimeError" do
+          expect {
+            described_class.load(config_path: config_file)
+          }.to raise_error(RuntimeError, "Failed to load configuration from file: #{config_file}")
         end
       end
     end

spec/unit/lib/hooks/core/config_validator_spec.rb

@@ -5,7 +5,7 @@
     context "with valid configuration" do
       it "returns validated configuration with all optional fields" do
         config = {
-          handler_dir: "./custom_handlers",
+          handler_plugin_dir: "./custom_handlers",
           log_level: "debug",
           request_limit: 2_048_000,
           request_timeout: 45,
@@ -24,15 +24,25 @@
       end
 
       it "returns validated configuration with minimal fields" do
-        config = {}
+        config = {
+          handler_plugin_dir: "/path/to/handlers",
+          log_level: "info",
+          root_path: "/app",
+          environment: "development"
+        }
 
         result = described_class.validate_global_config(config)
 
-        expect(result).to eq({})
+        expect(result).to eq(config)
       end
 
       it "accepts production environment" do
-        config = { environment: "production" }
+        config = {
+          environment: "production",
+          handler_plugin_dir: "/path/to/handlers",
+          log_level: "info",
+          root_path: "/app"
+        }
 
         result = described_class.validate_global_config(config)
 
@@ -41,7 +51,12 @@
 
       it "accepts valid log levels" do
         %w[debug info warn error].each do |log_level|
-          config = { log_level: log_level }
+          config = {
+            log_level: log_level,
+            handler_plugin_dir: "/path/to/handlers",
+            root_path: "/app",
+            environment: "development"
+          }
 
           result = described_class.validate_global_config(config)
 
@@ -101,7 +116,7 @@
 
       it "raises ValidationError for empty string values" do
         config = {
-          handler_dir: "",
+          handler_plugin_dir: "",
           root_path: "",
           health_path: ""
         }
@@ -114,7 +129,11 @@
       it "coerces boolean-like string values" do
         config = {
           use_catchall_route: "true",
-          normalize_headers: "1"
+          normalize_headers: "1",
+          handler_plugin_dir: "/path/to/handlers",
+          log_level: "info",
+          root_path: "/app",
+          environment: "development"
         }
 
         result = described_class.validate_global_config(config)
@@ -125,7 +144,7 @@
 
       it "raises ValidationError for non-string paths" do
         config = {
-          handler_dir: 123,
+          handler_plugin_dir: 123,
           root_path: [],
           endpoints_dir: {}
         }
@@ -138,7 +157,11 @@
       it "coerces string numeric values" do
         config = {
           request_limit: "1024",
-          request_timeout: "30"
+          request_timeout: "30",
+          handler_plugin_dir: "/path/to/handlers",
+          log_level: "info",
+          root_path: "/app",
+          environment: "development"
         }
 
         result = described_class.validate_global_config(config)
@@ -164,7 +187,13 @@
       end
 
       it "coerces float values to integers by truncating" do
-        config = { request_timeout: 30.5 }
+        config = {
+          request_timeout: 30.5,
+          handler_plugin_dir: "/path/to/handlers",
+          log_level: "info",
+          root_path: "/app",
+          environment: "development"
+        }
 
         result = described_class.validate_global_config(config)