Skip to content

[18.0][IMP] endpoint_route_handler: detect route conflicts - #178

Open
simahawk wants to merge 2 commits into
OCA:18.0from
camptocamp:18-imp-endpoint_route_handler-route-conflicts
Open

simahawk wants to merge 2 commits into
OCA:18.0from
camptocamp:18-imp-endpoint_route_handler-route-conflicts

Conversation

@simahawk

Copy link
Copy Markdown
Contributor

Follow-up of #176 (comment)

Detect route conflicts

When the routing map is built, each custom route (stored or generated) is checked against the routes already loaded: standard controllers and other custom routes.
A route w/ the same URL and overlapping methods (no methods = any) was silently shadowed (werkzeug matches the first rule only).
Now it's skipped and an error is logged w/ both owners. The same URL w/ different methods is still fine.

  • matching is done by utils.RouteConflictTracker (one dict lookup per route, ~3 ms for ~800 rules)
  • ir.http._endpoint_routing_rules now yields (endpoint rule, url, endpoint)

Stable endpoint hash

endpoint_hash was computed w/ the builtin hash(), randomized per process for strings: unique(endpoint_hash) only caught duplicates computed in the same process.

  • use a md5 of the routing values instead (utils.stable_hash)
  • consumer models now include indirect ones (eg: endpoint.endpoint, edi.endpoint): routes were not checked for uniqueness across them
  • migration: routes in sync are registered again to store the new hash

@OCA-git-bot OCA-git-bot added mod:endpoint Module endpoint series:18.0 mod:endpoint_route_handler Module endpoint_route_handler labels Oct 15, 2026
@simahawk
simahawk force-pushed the 18-imp-endpoint_route_handler-route-conflicts branch 2 times, most recently from 6f368bd to e39a9bd Compare October 16, 2026 13:44
@simahawk

Copy link
Copy Markdown
Contributor Author

@TDu ping :)

@simahawk

Copy link
Copy Markdown
Contributor Author

@lmignon may ask you to have a look at this one?

@simahawk
simahawk force-pushed the 18-imp-endpoint_route_handler-route-conflicts branch from e39a9bd to f45dfea Compare October 20, 2026 13:54
Comment thread endpoint_route_handler/utils.py Outdated

@lmignon lmignon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A little change to improve readability ...

@simahawk
simahawk force-pushed the 18-imp-endpoint_route_handler-route-conflicts branch from f45dfea to 7bbf4ba Compare October 20, 2026 17:09
Comment thread endpoint_route_handler/utils.py Outdated
When the routing map is built, check each custom route (stored or
generated) against the routes already loaded: standard controllers and
other custom routes.

A route w/ the same URL and overlapping methods (no methods = any)
was silently shadowed (werkzeug matches the first rule only).
Now it's skipped and an error is logged w/ both owners.
The same URL w/ different methods is still fine.
`endpoint_hash` was computed w/ the builtin `hash()`, which is randomized
per process for strings: the same route got a different hash after
a restart, so `unique(endpoint_hash)` only caught duplicates computed
in the same process.

Use a md5 of the routing impacting values instead (`utils.stable_hash`).

The migration (end of the update, once all modules are loaded)
forcedly registers again the routes in sync of all the consumer models:
rows are dropped and created w/ the new hash. Records not in sync
w/ the registry get their new hash when they are synced.

Consumer models now include indirect ones (eg: `endpoint.endpoint`
via `endpoint.mixin`, `edi.endpoint`): they were missing, hence routes
were not checked for uniqueness across them either.
@simahawk
simahawk force-pushed the 18-imp-endpoint_route_handler-route-conflicts branch from 7bbf4ba to 648b2fc Compare October 21, 2026 07:14
@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants