fix(routes): canonicalize path parameters so client calls match handlers (#505)#506
Open
imateusdev wants to merge 1 commit into
Open
fix(routes): canonicalize path parameters so client calls match handlers (#505)#506imateusdev wants to merge 1 commit into
imateusdev wants to merge 1 commit into
Conversation
Route QNs (__route__METHOD__/path) are the rendezvous key for joining a client
HTTP_CALLS to the server handler that serves it, but they were built from the raw
path. Frameworks write path parameters with different placeholder syntaxes
(":id" Express/clients, "{id}" Axum/Spring/OpenAPI, "<id>" Flask/Rocket), so a
client call and its handler produced different QNs and never joined. Static
routes joined; parameterized routes (most of a REST surface) did not.
Add cbm_route_canon_path() which collapses each parameter token (":name",
"{name}", "<name>", "${...}") to a single "{}" token, and apply it to the path
at every HTTP-route QN construction site (pass_route_nodes, pass_parallel,
pass_calls, pass_cross_repo). Broker/async/infra QNs are left untouched. Only the
QN (identity/dedup key) is canonicalized; the Route node display name keeps the
original path. Parameter names are intentionally discarded so the same logical
endpoint matches across services that name the variable differently.
Adds tests/test_route_canon.c (11 cases). Fixes DeusData#505.
Signed-off-by: imateusdev <imateusdev@gmail.com>
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 does this PR do?
Canonicalizes route-path parameter placeholders when building Route QNs so that
a client
HTTP_CALLSand the server handler that serves it rendezvous on the sameRoutenode even when each side uses a different framework's placeholder syntax(or a different parameter name).
Fixes #505.
Problem
Route QNs (
__route__METHOD__/path) are the rendezvous key, but they were builtfrom the raw path. A Rust/Axum handler
GET /players/{id}and a TypeScript clientcall
/players/:idtherefore produced different QNs and never joined. Staticroutes joined fine; parameterized routes (the bulk of a REST surface) did not:
Fix
New shared helper
cbm_route_canon_path()(declared inpipeline_internal.h,defined in
pass_route_nodes.c) collapses each parameter token to a single{}:/players/:id/players/{}/players/{id}/players/{}/users/<int:id>/users/{}/players/${id}/players/{}/orders/{id}/items/{itemIndex}/orders/{}/items/{}Applied to the path at every HTTP-route QN construction site
(
pass_route_nodes.c,pass_parallel.c,pass_calls.c,pass_cross_repo.c);broker/async/infra QNs are left untouched. The Route node's display
namekeepsthe original path — only the QN (identity/dedup key) is canonicalized — so the
existing ANY-vs-method reconciliation does the rest, exactly as it already does
for static routes.
Design note / tradeoff
Parameter names are intentionally discarded:
/users/{id}and/users/{slug}canonicalize to the same node. For REST surfaces these are virtually always the
same logical endpoint; flagging it here for reviewer visibility.
Tests
tests/test_route_canon.c— 11 unit cases covering each placeholder syntax, the:id↔{id}convergence invariant, parameter-name agnosticism, literal:midsegment, multiple params, NULL/empty, and tight-buffer truncation safety.
Verification
make -f Makefile.cbm cbm— clean build (-Wall -Wextra -Werror).make -f Makefile.cbm test— full ASan/UBSan suite; the newroute_canonsuite passes 11/11 and no previously-passing test regresses.
clang-format-18 --dry-run --Werrorclean on all changed files.Checklist
git commit -s)