Skip to content

Make use of default router arguments#824

Merged
jviotti merged 2 commits intomainfrom
otherwise-router
Apr 9, 2026
Merged

Make use of default router arguments#824
jviotti merged 2 commits intomainfrom
otherwise-router

Conversation

@jviotti
Copy link
Copy Markdown
Member

@jviotti jviotti commented Apr 8, 2026

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti jviotti force-pushed the otherwise-router branch from e9ace0e to 463a2d0 Compare April 8, 2026 23:53
@jviotti jviotti marked this pull request as ready for review April 8, 2026 23:54
@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Apr 8, 2026

🤖 Augment PR Summary

Summary: This PR makes the router provide default (fallback) action arguments and shifts action dispatching to an instance-based dispatcher.

Changes:

  • Bumps the vendored core dependency and extends URITemplateRouter/URITemplateRouterView with otherwise() to define a fallback handler context.
  • Updates router binary format (version 5) to persist otherwise_context and to return the fallback context when no route matches.
  • Registers the fallback route in the routes generator, including default errorSchema arguments.
  • Replaces global action slot initialization/dispatch with a new ActionDispatcher class that owns the per-identifier action instances.
  • Removes hard-coded default error schema construction in ActionDefault_v1 and relies on router-provided arguments instead.

Technical Notes: Action instances are still lazily constructed via std::call_once per route identifier, but configuration (like errorSchema) now comes from router arguments (including the new fallback identifier/context behavior).

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.


router.arguments(identifier, [this](const auto &key, const auto &value) {
if (key == "errorSchema") {
this->error_schema_ = std::get<std::string_view>(value);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

error_schema_ is a default-initialized std::string_view, so if the router has no errorSchema argument for this identifier it stays empty and later json_error/serve calls will emit responses without a schema URI. Other locations where this applies: src/actions/dispatch.cc:53, src/actions/dispatch.cc:73.

Severity: medium

Other Locations
  • src/actions/dispatch.cc:53
  • src/actions/dispatch.cc:73

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

const std::span<std::string_view> matches,
HTTPRequest &request, HTTPResponse &response) -> void;
// NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
const std::filesystem::path &base_;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ActionDispatcher stores base_/router_ by reference, so constructing it with temporaries (or with objects that don’t outlive the server) would leave dangling references and can become a UAF later. Consider documenting/enforcing the required lifetime at the API boundary.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 9 files

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (community)

Details
Benchmark suite Current: f49985b Previous: 414a71a Ratio
Add one schema (0 existing) 18 ms 19 ms 0.95
Add one schema (100 existing) 23 ms 23 ms 1
Add one schema (1000 existing) 78 ms 73 ms 1.07
Add one schema (10000 existing) 922 ms 663 ms 1.39
Update one schema (1 existing) 17 ms 16 ms 1.06
Update one schema (101 existing) 25 ms 23 ms 1.09
Update one schema (1001 existing) 78 ms 74 ms 1.05
Update one schema (10001 existing) 693 ms 929 ms 0.75
Cached rebuild (1 existing) 9 ms 9 ms 1
Cached rebuild (101 existing) 11 ms 11 ms 1
Cached rebuild (1001 existing) 33 ms 30 ms 1.10
Cached rebuild (10001 existing) 279 ms 261 ms 1.07
Index 100 schemas 171 ms 104 ms 1.64
Index 1000 schemas 1083 ms 940 ms 1.15
Index 10000 schemas 13494 ms 13112 ms 1.03

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (enterprise)

Details
Benchmark suite Current: f49985b Previous: 414a71a Ratio
Add one schema (0 existing) 19 ms 20 ms 0.95
Add one schema (100 existing) 24 ms 26 ms 0.92
Add one schema (1000 existing) 72 ms 81 ms 0.89
Add one schema (10000 existing) 898 ms 671 ms 1.34
Update one schema (1 existing) 18 ms 19 ms 0.95
Update one schema (101 existing) 24 ms 27 ms 0.89
Update one schema (1001 existing) 76 ms 81 ms 0.94
Update one schema (10001 existing) 892 ms 677 ms 1.32
Cached rebuild (1 existing) 10 ms 10 ms 1
Cached rebuild (101 existing) 12 ms 12 ms 1
Cached rebuild (1001 existing) 31 ms 44 ms 0.70
Cached rebuild (10001 existing) 275 ms 302 ms 0.91
Index 100 schemas 112 ms 110 ms 1.02
Index 1000 schemas 1167 ms 1029 ms 1.13
Index 10000 schemas 13014 ms 14135 ms 0.92

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti jviotti merged commit a90a634 into main Apr 9, 2026
6 checks passed
@jviotti jviotti deleted the otherwise-router branch April 9, 2026 13:30
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.

1 participant