Extract PassageRouteRegistry to deduplicate handler discovery - #228
Merged
Conversation
PassageController::handle(), PassageHealthCommand::passageRoutes(), and
PassageListCommand::handle() each independently found Passage routes,
read the _passage_handler route default, validated it against
PassageControllerInterface, resolved it through the container, and
merged config('passage.options') with the handler's getOptions(). The
three implementations had already drifted apart twice before being
reconciled (PassageListCommand resolving handlers with `new $handler`
instead of the container, and matching routes by a partial action-name
string instead of the fully-qualified class), so a future change to
any one of them could silently diverge from the other two again.
Add Morcen\Passage\Support\PassageRouteRegistry as the single place
that finds Passage routes, reads/validates/resolves their handler, and
merges options, and use it from all three call sites. As part of this,
PassageListCommand's target resolution now merges config('passage.options')
with the handler's own options like the controller and health command
already did, instead of using the handler's raw getOptions() alone.
Fixes #88
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGa5tuhzv66zYWhZ6PWXXr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
PassageController::handle(),PassageHealthCommand::passageRoutes(), andPassageListCommand::handle()each independently implemented the same four steps: find Passage routes, read the_passage_handlerroute default, validate it againstPassageControllerInterface, resolve it through the container, and mergeconfig('passage.options')with the handler'sgetOptions().Because this logic was duplicated three times instead of shared, the implementations had already drifted apart twice in the past:
PassageListCommandused to resolve handlers withnew $handlerinstead of going through the container, which broke for handlers with constructor dependencies (later fixed to match the other two call sites).PassageListCommandused to match routes by a partial action-name substring instead of the fully-qualified controller class (later fixed to matchPassageHealthCommand's approach).Nothing prevented a future change to any one of the three call sites from silently diverging from the other two again in the same way.
What changed
Morcen\Passage\Support\PassageRouteRegistry, the single place that:Passagefacade (routes())handlerClassFor())PassageControllerInterface(isValidHandler())resolveHandler())config('passage.options')with the handler's own options (optionsFor())PassageController::handle(),PassageHealthCommand, andPassageListCommandto use the registry instead of their own copies of this logic.PassageListCommand's target resolution now mergesconfig('passage.options')with the handler's own options the same way the controller and health command already did, instead of using the handler's rawgetOptions()alone.Illuminate\Contracts\Foundation\Application $appdependency fromPassageController's constructor, since handler resolution now goes through the registry.tests/Unit/PassageRouteRegistryTest.phpcovering the registry directly, and updated the existing controller tests that constructPassageControllermanually to pass the newPassageRouteRegistrydependency.Testing
vendor/bin/pint --dirty— passed, no changes neededvendor/bin/pest— 250 passed (631 assertions), full suite greenFixes #88
🤖 Generated with Claude Code
https://claude.ai/code/session_01DGa5tuhzv66zYWhZ6PWXXr
Generated by Claude Code