Skip to content

Make handler field support only snake_case in configuration #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 17, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 17, 2025

This PR implements a breaking change to make the handler field in endpoint configuration files support only snake_case naming, matching the same enforcements of the auth.type field.

Changes Made

Before this change:

# endpoint.yml
path: /webhook
handler: GitHubHandler  # PascalCase required

After this change:

# endpoint.yml  
path: /webhook
handler: github_handler  # snake_case required

Implementation Details

  • Updated ConfigValidator: Changed validation pattern from /\A[A-Z][a-zA-Z0-9_]*\z/ to /\A[a-z][a-z0-9_]*\z/ to only accept snake_case handler names
  • Updated PluginLoader: Modified get_handler_plugin method to convert snake_case configuration values to PascalCase for internal class lookup
  • Maintained Internal Structure: File naming (github_handler.rb) and class names (GithubHandler) remain unchanged - only configuration references use snake_case
  • Security Validation: Updated dangerous class validation to convert snake_case to PascalCase before checking against the security blacklist

Example Usage

If you have a handler file handlers/github_handler.rb with contents:

class GithubHandler < Hooks::Plugins::Handlers::Base
  def call(payload:, headers:, env:, config:)
    # Process GitHub webhook
    { status: "success" }
  end
end

It should now be referenced as handler: github_handler in endpoint configurations (previously handler: GithubHandler).

Breaking Change Notice

This is an intentional breaking change with no backwards compatibility. All existing endpoint configurations using PascalCase handler names must be updated to use snake_case.

Testing

  • All existing tests updated to use snake_case handler names
  • Added comprehensive validation for snake_case patterns
  • Maintained 100% test coverage (562 examples, 0 failures)
  • All linting checks pass

Fixes #53.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] task: make the handler field support only snake_case Make handler field support only snake_case in configuration Jun 17, 2025
@Copilot Copilot AI requested a review from GrantBirki June 17, 2025 06:21
Copilot finished work on behalf of GrantBirki June 17, 2025 06:21
@GrantBirki GrantBirki marked this pull request as ready for review June 17, 2025 16:19
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 16:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enforces snake_case naming for the handler field in endpoint configurations, updating validation, loading, and tests to match the new convention.

  • Updated ConfigValidator to only accept snake_case handler names and convert them to PascalCase for security checks
  • Modified PluginLoader to transform snake_case config values into PascalCase for internal class lookup
  • Revised tests and helper methods to use snake_case everywhere and maintain 100% coverage

Reviewed Changes

Copilot reviewed 48 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/hooks/core/config_validator.rb Changed handler-name regex to snake_case and adjusted dangerous-class check
lib/hooks/core/plugin_loader.rb Added snake_case → PascalCase conversion for lookup
lib/hooks/app/helpers.rb Updated docstring to reflect snake_case parameter
spec/unit/lib/hooks/handlers/base_spec.rb Updated test expectations from PascalCase to snake_case
spec/unit/lib/hooks/core/config_validator_spec.rb Switched handler fixtures to snake_case
spec/unit/lib/hooks/core/config_validator_security_spec.rb Updated security tests for snake_case and PascalCase failures
spec/unit/lib/hooks/core/plugin_loader_spec.rb Adjusted plugin lookup tests to use snake_case
spec/unit/lib/hooks/app/helpers_spec.rb Updated helper load expectations
spec/unit/lib/hooks/app/helpers_security_spec.rb Updated helper security tests to snake_case
spec/unit/app/rack_env_builder_spec.rb Switched handler in rack env to snake_case
spec/integration/hooks_integration_spec.rb Updated integration YAML fixture to snake_case
spec/integration/global_lifecycle_hooks_spec.rb Updated global-lifecycle YAML fixture to snake_case
.bundle/config Bundler path updates (unrelated to handler change)
Comments suppressed due to low confidence (2)

spec/unit/lib/hooks/core/plugin_loader_spec.rb:166

  • We only test default_handler → DefaultHandler; consider adding a test case for a handler name containing a number (e.g. team_1_handler) to verify the snake_case→PascalCase conversion handles digits correctly.
      expect(described_class.get_handler_plugin("default_handler")).to eq(DefaultHandler)

.bundle/config:3

  • [nitpick] This CI/deployment configuration change is unrelated to the handler naming change and pollutes the PR; consider removing or splitting it into its own commit.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

@GrantBirki GrantBirki merged commit 81836b9 into main Jun 17, 2025
22 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-53 branch June 17, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: make the handler field support only snake_case
2 participants