Skip to content

WIP: Remove routing_tree dependency and remake how we are handling routes - #350

Open
burbas wants to merge 16 commits into
masterfrom
new-router
Open

WIP: Remove routing_tree dependency and remake how we are handling routes#350
burbas wants to merge 16 commits into
masterfrom
new-router

Conversation

@burbas

@burbas burbas commented Jan 16, 2026

Copy link
Copy Markdown
Contributor

This pull request introduces significant improvements to the Nova application's ability to manage multiple sub-applications dynamically, refactors routing logic to use a new trie-based backend, and adds new API functions for programmatic control. The changes enhance modularity, allow for pragmatic application lifecycle management, and transition routing from the old routing_tree to nova_routing_trie for better performance and maintainability.

@burbas burbas self-assigned this Jan 16, 2026
@burbas burbas added the enhancement New feature or request label Jan 16, 2026
@burbas
burbas requested a review from Taure as a code owner January 16, 2026 20:26
Taure
Taure previously approved these changes Feb 24, 2026
@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity. It will be closed in 7 days if no further activity occurs.

@Taure

Taure commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

I've been going through this to see what's left before it can land, and I'm picking the work up. One question I can't answer from the diff:

Was the #nova_handler_value.extra_state -> extra rename deliberate?

It's load-bearing in a way that isn't obvious. nova_router now writes extra (into both the record and the Req map), but nova_file_controller still reads extra_state in six places. Both get_file/1 and get_dir/1 have {status, 404} catch-alls, so the mismatch doesn't crash: every static file and priv_dir route just quietly 404s. It also breaks the Req key that nova_json_schemas, egql_nova and nova_request_app read, plus master's own router tests.

Nothing else in the branch seems to depend on the new name, so unless there was a reason I'll revert to extra_state for now and we can rename properly later with a deprecation path if you want it. Say the word if you'd rather keep extra and I'll do the downstream repos in lockstep instead.

For context, the other things I'm fixing on the way through, so you know what to expect in the follow-up PR:

  • nova_routing_trie:find/4 doesn't exist yet, so execute/2 and render_status_page/5 have nothing to call.
  • Parity gaps vs routing_tree: integer status-code paths (this one stops Nova booting at all, because nova_router:routes/1 registers 404/500), the [...] catch-all and its pathinfo, the '_' any-method comparator, and {error, comparator_not_found, AllowedMethods} for 405s.
  • insert/5 returns {ok, Trie} but nova_router:insert/5 treats it as the tree.
  • compile/1 double-wraps the options map, so use_strict_routing never reaches the trie.
  • rebar.lock still pins routing_tree 1.0.11, and it's still in nova.app.src and plt_extra_apps, so the dep isn't actually gone yet.
  • The nova_sup ETS table has no keypos, so every #nova_server{} keys on the record tag and the registry can only ever hold one row.
  • plugin_strategy's lists:ukeysort(1, ...) keys on the phase rather than the module, so at most one pre_request and one post_request plugin survives per route.
  • rebar3_nova's routes, audit, openapi and doctor tasks all include routing_tree.hrl and destructure its records, so they need porting at the same time.

Two other calls I'd like your view on:

  1. Duplicate routes. routing_tree kept the first insert; nova_routing_trie currently last-wins. That's a silent precedence reversal for existing apps. First-wins for compatibility, or last-wins as a documented change?
  2. Per-listener dispatch. Right now every listener reads one global nova_dispatch keyed on host/path/method with no port dimension, so two listeners serve each other's routes. Is "several sockets, one route table" the intended semantics, or does multi-port need real route isolation? That decides whether the nova_sup half is a bug-fix pass or a redesign.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants