Skip to content

Commit e64e416

Browse files
etrclaude
andcommitted
TASK-071 (B): collapse route_entry::handler variant; remove dead lambda_handler arm
route_entry::handler was originally `std::variant<lambda_handler, shared_ptr<http_resource>>`. The lambda_handler arm has been dead code since TASK-053: every writer of route_entry (on_*/route entry points, register_path, register_prefix) funnels lambdas through a lambda_resource shim and stores the shim as shared_ptr<http_resource>. No call site stored a lambda directly, and no v2 task introduced a writer that would. Both readers (resolve_resource_for_request, prepare_or_create_lambda_shim) already treated the lambda arm as unreachable. Collapse handler to a bare shared_ptr<http_resource>: - route_entry.hpp drops std::variant + the lambda_handler typedef; - the lambda_handler typedef relocates to detail/lambda_resource.hpp where its remaining uses live (the per-method slot signature); - the dispatch site (webserver_dispatch.cpp::resolve_resource_for_request) reads result.entry.handler directly with a defensive null check; - prepare_or_create_lambda_shim and update_existing_v2_entry shed the std::get_if indirection; - the routing_regression unit pin updates likewise. Pin the new shape with a static_assert in webserver_on_methods_test: TASK-071 history is captured in the assertion message so a future reintroduction of variant storage trips an early-warning canary at compile time. The webserver::route(...) and on_*(...) public signatures take std::function (not std::variant) and were unaffected — no Doxygen update needed on the public surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ce13c96 commit e64e416

8 files changed

Lines changed: 86 additions & 77 deletions

File tree

src/detail/webserver_dispatch.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,10 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
255255
// The dispatch path now does:
256256
// lookup_v2() walks route_lru_cache -> exact_routes_ ->
257257
// param_and_prefix_routes_ -> regex_routes_, returning a
258-
// route_entry whose handler arm is always shared_ptr<http_resource>
258+
// route_entry whose handler is a shared_ptr<http_resource>
259259
// (the on_*/route entry points wrap user lambdas in lambda_resource
260-
// before storing; the variant's lambda_handler arm is dead code).
260+
// before storing — see prepare_or_create_lambda_shim in
261+
// webserver_routes.cpp).
261262
//
262263
// TASK-067: single_resource mode used to short-circuit to a direct read
263264
// of the v1 registered_resources map. With the v1 maps deleted, single
@@ -267,6 +268,13 @@ webserver_impl::lookup_v2(http_method method, const std::string& path) {
267268
// is no separate fast path because the v2 walk for a single registered
268269
// prefix is one shared_lock + one empty-map probe + one trivial radix
269270
// descent: cheap enough that the extra branch was net-negative.
271+
//
272+
// TASK-071: route_entry::handler was previously a
273+
// `std::variant<lambda_handler, shared_ptr<http_resource>>`. The
274+
// lambda_handler arm was dead code (every writer wrapped lambdas in
275+
// lambda_resource) and has been removed. The dispatch path reads the
276+
// shared_ptr directly with a defensive null check that degrades to a
277+
// 404 miss.
270278
bool webserver_impl::resolve_resource_for_request(detail::modded_request* mr,
271279
std::shared_ptr<http_resource>& hrm) {
272280
// matched_path_template + matched_is_prefix feed the route_resolved
@@ -280,20 +288,15 @@ bool webserver_impl::resolve_resource_for_request(detail::modded_request* mr,
280288
lookup_result result = lookup_v2(mr->method_enum, mr->standardized_url);
281289
if (!result.found) return false;
282290

283-
// Extract the shared_ptr<http_resource> arm of the route_entry's
284-
// variant. The on_* / route entry points wrap user lambdas in a
285-
// lambda_resource shim and store the shim as shared_ptr<http_resource>,
286-
// so this arm is the only one populated in practice; treat the
287-
// lambda_handler arm as unreachable (defensive nullptr check below).
288-
auto* hp = std::get_if<std::shared_ptr<http_resource>>(&result.entry.handler);
289-
if (hp == nullptr || *hp == nullptr) {
290-
// Unreachable today: lookup_v2 only returns entries whose
291-
// handler is a shared_ptr<http_resource>. If a future task
292-
// stores a lambda_handler directly we will need to grow this
293-
// path; until then, treat as a not-found miss.
291+
// Read the resource pointer directly. Every writer of route_entry
292+
// populates a non-null shared_ptr (a class-derived http_resource
293+
// for register_path / register_prefix, or a lambda_resource shim
294+
// for on_* / route); a null pointer here would indicate a future
295+
// bug, so degrade to a not-found miss defensively.
296+
if (result.entry.handler == nullptr) {
294297
return false;
295298
}
296-
hrm = *hp;
299+
hrm = result.entry.handler;
297300

298301
// Replay captured URL parameters into the request. This is the v2
299302
// equivalent of the v1-era apply_extracted_params (removed in

src/detail/webserver_routes.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,16 +188,12 @@ webserver_impl::prepare_or_create_lambda_shim(const detail::http_endpoint& idx,
188188
fresh_out = true;
189189
return std::make_shared<detail::lambda_resource>();
190190
}
191-
// Existing entry. The v2 storage uses a variant whose handler arm is
192-
// always shared_ptr<http_resource> in the current code (the
193-
// lambda_handler arm is unused; see resolve_resource_for_request).
194-
// Pull the shared_ptr and dynamic_pointer_cast to lambda_resource:
195-
// if the cast misses, a class-based register_path/register_prefix
196-
// owns this path and we must throw.
197-
const auto* sp = std::get_if<std::shared_ptr<http_resource>>(&existing->handler);
198-
auto shim = (sp != nullptr)
199-
? std::dynamic_pointer_cast<detail::lambda_resource>(*sp)
200-
: nullptr;
191+
// Existing entry. route_entry::handler is a shared_ptr<http_resource>
192+
// (TASK-071 collapsed the prior variant). Dynamic-cast to
193+
// lambda_resource: if the cast misses, a class-based
194+
// register_path/register_prefix owns this path and we must throw.
195+
auto shim = std::dynamic_pointer_cast<detail::lambda_resource>(
196+
existing->handler);
201197
if (!shim) {
202198
throw std::invalid_argument(
203199
"A non-lambda http_resource is already registered at "
@@ -355,8 +351,7 @@ void webserver_impl::update_existing_v2_entry(const std::string& key,
355351
return;
356352
}
357353
for (auto& rr : regex_routes_) {
358-
auto* sp = std::get_if<std::shared_ptr<http_resource>>(&rr.entry.handler);
359-
if (sp && *sp == shim) {
354+
if (rr.entry.handler == shim) {
360355
merge_into(rr.entry);
361356
return;
362357
}

src/httpserver/detail/lambda_resource.hpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include <cassert>
4949
#include <cstddef>
5050
#include <cstdint>
51+
#include <functional>
5152
#include <utility>
5253

5354
#include "httpserver/http_method.hpp"
@@ -62,12 +63,26 @@ class http_request;
6263
namespace httpserver {
6364
namespace detail {
6465

66+
// The per-method slot signature for lambda_resource. Returns
67+
// http_response by value (DR-004) and takes the request by const
68+
// reference. std::function is the chosen storage so users can pass any
69+
// callable (lambda, function pointer, std::bind result, member-function
70+
// adaptor) without leaking the concrete callable type into the slot
71+
// array.
72+
//
73+
// TASK-071 history: this typedef formerly lived in route_entry.hpp as
74+
// the lambda arm of `std::variant<lambda_handler, shared_ptr<...>>`. The
75+
// variant arm was dead code (no writer stored a lambda directly — every
76+
// on_*/route path funnels through lambda_resource), so it was removed.
77+
// The typedef relocates here, where its remaining uses are.
78+
using lambda_handler = std::function<::httpserver::http_response(const ::httpserver::http_request&)>; // NOLINT(whitespace/line_length)
79+
6580
// Tiny adapter that wraps a single lambda_handler as an http_resource
6681
// virtual override. We keep one slot per method enum and dispatch in
6782
// each render_* override. Each slot holds a
6883
// std::function<http_response(const http_request&)>
69-
// (see route_entry.hpp::lambda_handler). Slots are installed via
70-
// set_slot() and invoked by the render_* overrides below.
84+
// (the `lambda_handler` typedef defined above). Slots are installed
85+
// via set_slot() and invoked by the render_* overrides below.
7186
class lambda_resource final : public ::httpserver::http_resource {
7287
public:
7388
lambda_resource() {

src/httpserver/detail/route_entry.hpp

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,44 +40,38 @@
4040
#ifndef SRC_HTTPSERVER_DETAIL_ROUTE_ENTRY_HPP_
4141
#define SRC_HTTPSERVER_DETAIL_ROUTE_ENTRY_HPP_
4242

43-
#include <functional>
4443
#include <memory>
45-
#include <variant>
4644

4745
#include "httpserver/http_method.hpp"
4846

4947
namespace httpserver {
50-
class http_request;
51-
class http_response;
5248
class http_resource;
5349
} // namespace httpserver
5450

5551
namespace httpserver {
5652
namespace detail {
5753

58-
// The lambda arm of the route_entry payload variant. Returns
59-
// http_response by value (DR-004) and takes the request by const
60-
// reference. std::function is the chosen storage so users can pass any
61-
// callable (lambda, function pointer, std::bind result, member-function
62-
// adaptor) without leaking the concrete callable type into the route
63-
// table.
64-
//
65-
// PRD-RSP-REQ-007 dispatch note: when the lookup_v2 path invokes this
66-
// variant arm, the returned http_response prvalue is moved directly into
67-
// modded_request::response via emplace(), matching the pointer-to-member
68-
// dispatch path used by the v1 finalize_answer (see DR-004 §5.3).
69-
using lambda_handler = std::function<::httpserver::http_response(const ::httpserver::http_request&)>; // NOLINT(whitespace/line_length)
70-
7154
// route_entry: §4.7-shape value type stored per route in the route
7255
// table. The `methods` mask holds every HTTP method this entry serves;
73-
// the variant payload holds either a lambda (for on_* registrations)
74-
// or a shared_ptr<http_resource> (for register_path / register_prefix
75-
// registrations). `is_prefix` distinguishes prefix matching from exact
76-
// matching at lookup time.
56+
// the `handler` payload is a shared_ptr to the http_resource serving
57+
// the route — either a class-derived resource (for register_path /
58+
// register_prefix registrations) or a lambda_resource shim
59+
// (for on_* / route registrations, which wrap the user lambda in a
60+
// lambda_resource owning one slot per HTTP method). `is_prefix`
61+
// distinguishes prefix matching from exact matching at lookup time.
62+
//
63+
// TASK-071 history: this field was originally a
64+
// `std::variant<lambda_handler, shared_ptr<http_resource>>`. The
65+
// lambda_handler arm was never populated — every writer wrapped user
66+
// lambdas in lambda_resource (see webserver_routes.cpp's
67+
// prepare_or_create_lambda_shim and the lambda_resource header doc-
68+
// comment for why the shim composes cleanly with register_path's
69+
// resource storage). The variant arm has been removed; the
70+
// lambda_handler typedef now lives in detail/lambda_resource.hpp
71+
// where its remaining uses are (the per-method slot signature).
7772
struct route_entry {
7873
method_set methods{};
79-
std::variant<lambda_handler,
80-
std::shared_ptr<::httpserver::http_resource>> handler;
74+
std::shared_ptr<::httpserver::http_resource> handler;
8175
bool is_prefix = false;
8276
};
8377

test/Makefile.am

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,8 @@ webserver_register_path_prefix_SOURCES = unit/webserver_register_path_prefix_tes
252252
# webserver_on_methods: TASK-025. Compile-time signature contract for
253253
# the seven on_* lambda registration overloads (on_get / on_post / on_put
254254
# / on_delete / on_patch / on_options / on_head) plus static_asserts on
255-
# the §4.7 detail::route_entry shape (method_set + variant<lambda_handler,
256-
# shared_ptr<http_resource>> + bool is_prefix). Runtime tests over real
255+
# the §4.7 detail::route_entry shape (method_set + shared_ptr<http_resource>
256+
# + bool is_prefix; TASK-071 collapsed the original variant arm). Runtime tests over real
257257
# curl round-trips: PRD §3.4 hello-world; per-method 405 + Allow header;
258258
# composition (on_get + on_post on the same path serve both); all seven
259259
# methods dispatch correctly; duplicate (method, path) registration

test/unit/route_table_test.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ namespace ht = httpserver;
3939
namespace htd = httpserver::detail;
4040

4141
// Sentinel makers to avoid crafting a real http_resource just to populate
42-
// the variant; we leave the lambda_handler default-constructed (empty
43-
// std::function) and ride on the sentinel id field in the test below.
44-
// The variant arm itself is irrelevant — the radix_tree / route_cache
45-
// tests only care about the entry payload identity through pointer
46-
// equality and the methods/is_prefix fields.
42+
// the handler field; we leave it null (a default-constructed shared_ptr)
43+
// and ride on the sentinel id field in the test below. The handler field
44+
// is irrelevant — the radix_tree / route_cache tests only care about the
45+
// entry payload identity through pointer equality and the methods/is_prefix
46+
// fields. (TASK-071: route_entry::handler was originally a
47+
// std::variant<lambda_handler, shared_ptr<http_resource>>; the variant arm
48+
// was collapsed to a bare shared_ptr<http_resource>.)
4749
struct test_entry {
4850
int sentinel = 0;
4951
ht::method_set methods{};
@@ -57,7 +59,7 @@ static htd::route_entry make_entry(
5759
htd::route_entry e;
5860
e.methods = ms;
5961
e.is_prefix = is_prefix;
60-
// Leave variant alternative #0 (lambda_handler) default-constructed.
62+
// Leave handler default-constructed (null shared_ptr).
6163
return e;
6264
}
6365

test/unit/routing_regression_test.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -565,10 +565,10 @@ LT_BEGIN_AUTO_TEST(routing_regression_suite,
565565
// literal "foo" (exact child), so the tree descends there first and
566566
// never tries the wildcard root branch required by the second
567567
// pattern. Structural precedence → first-registered wins here.
568-
auto* hp = std::get_if<std::shared_ptr<ht::http_resource>>(
569-
&r.entry.handler);
570-
LT_CHECK(hp != nullptr);
571-
LT_CHECK(*hp == first);
568+
// TASK-071: route_entry::handler is now a bare shared_ptr<http_resource>
569+
// (the variant arm was collapsed); read it directly.
570+
LT_CHECK(r.entry.handler != nullptr);
571+
LT_CHECK(r.entry.handler == first);
572572
LT_END_AUTO_TEST(overlapping_two_regex_routes_deterministic_first_wins)
573573

574574
LT_BEGIN_AUTO_TEST(routing_regression_suite,
@@ -588,10 +588,8 @@ LT_BEGIN_AUTO_TEST(routing_regression_suite,
588588
std::string("/foo/bar/"));
589589
LT_CHECK(r.found);
590590
LT_CHECK(r.tier == ht::detail::webserver_impl::tier_hit::exact);
591-
auto* hp = std::get_if<std::shared_ptr<ht::http_resource>>(
592-
&r.entry.handler);
593-
LT_CHECK(hp != nullptr);
594-
LT_CHECK(*hp == exact);
591+
LT_CHECK(r.entry.handler != nullptr);
592+
LT_CHECK(r.entry.handler == exact);
595593
LT_END_AUTO_TEST(later_exact_registration_shadows_earlier_regex)
596594

597595
// ---------------------------------------------------------------------
@@ -615,10 +613,8 @@ LT_BEGIN_AUTO_TEST(routing_regression_suite,
615613
auto deep = impl_of(ws).lookup_v2(
616614
ht::http_method::get, std::string("/anything/at/all"));
617615
LT_CHECK(deep.found);
618-
auto* hp = std::get_if<std::shared_ptr<ht::http_resource>>(
619-
&deep.entry.handler);
620-
LT_CHECK(hp != nullptr);
621-
LT_CHECK(*hp == only);
616+
LT_CHECK(deep.entry.handler != nullptr);
617+
LT_CHECK(deep.entry.handler == only);
622618
LT_END_AUTO_TEST(single_resource_mode_serves_any_subpath)
623619

624620
// ---------------------------------------------------------------------

test/unit/webserver_on_methods_test.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,19 +172,23 @@ static_assert(std::is_same_v<
172172
void>,
173173
"on_head must exist and return void");
174174

175-
// (2) The route_entry detail type pins the §4.7 shape: method_set + a
176-
// two-arm variant + a bool is_prefix flag.
175+
// (2) The route_entry detail type pins the §4.7 shape: method_set +
176+
// a single-arm shared_ptr<http_resource> handler + a bool is_prefix
177+
// flag. TASK-071 collapsed the variant: the lambda_handler arm was
178+
// dead code (every writer wrapped user lambdas in a lambda_resource
179+
// shim and stored the shim as shared_ptr<http_resource>; no path
180+
// stored a lambda directly).
177181
static_assert(std::is_same_v<
178182
decltype(httpserver::detail::route_entry::methods),
179183
method_set>,
180184
"route_entry::methods must be method_set");
181185

182186
static_assert(std::is_same_v<
183187
decltype(httpserver::detail::route_entry::handler),
184-
std::variant<httpserver::detail::lambda_handler,
185-
std::shared_ptr<http_resource>>>,
186-
"route_entry::handler must be variant<lambda_handler, "
187-
"shared_ptr<http_resource>>");
188+
std::shared_ptr<http_resource>>,
189+
"route_entry::handler must be shared_ptr<http_resource> "
190+
"(TASK-071: lambda_handler variant arm removed; "
191+
"on_*/route shim through lambda_resource).");
188192

189193
static_assert(std::is_same_v<
190194
decltype(httpserver::detail::route_entry::is_prefix),

0 commit comments

Comments
 (0)