Skip to content

Commit eb1d233

Browse files
etrclaude
andcommitted
TASK-009: housekeeping (status + checkboxes + arch doc syncs)
- Mark TASK-009 action items complete and flip status to Done (specs/tasks/M2-response/TASK-009.md, specs/tasks/_index.md). - Document the deferral of `final` keyword + std::is_final_v AC from TASK-009 to TASK-013 (v1 response subclasses still inherit at this point, so sealing must wait until TASK-013 removes them). - Sync architecture docs (DR-002, DR-005, 03-system-overview, 04-components/{body-hierarchy,http-response,webserver}, 05-cross-cutting, 06-backend-integration, 09-testing, 12-open-questions) and forward-looking task specs (TASK-014, TASK-015, TASK-020) to use `detail/` (singular) consistently. - Persist remaining minor validation findings to specs/unworked_review_issues/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 146c85d commit eb1d233

17 files changed

Lines changed: 252 additions & 34 deletions

File tree

specs/architecture/03-system-overview.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
│ └── http_request::impl (allocated from connection's arena) │
2424
│ │
2525
│ detail::body (polymorphic; subclasses string/file/iovec/pipe/ │
26-
│ deferred/empty live in details/body.hpp) │
26+
│ deferred/empty live in detail/body.hpp)
2727
└──────────┬───────────────────────────────────────────────────────────┘
2828
2929
┌──────────┴───────────────────────────────────────────────────────────┐
@@ -41,7 +41,7 @@
4141
| `http_response` | Response value: status, headers, footers, cookies, body | Non-PIMPL value type; polymorphic body in 64-byte SBO buffer with heap fallback |
4242
| `http_resource` | Class-form handler (state shared across HTTP methods of one resource) | Public abstract base; allow-mask held as `method_set` (`uint32_t` bitmask) |
4343
| `websocket_handler` | Per-endpoint WebSocket protocol handler | Public abstract base; registered via `unique_ptr` / `shared_ptr` overloads |
44-
| `detail::body` | Polymorphic body kinds (string / file / iovec / pipe / deferred / empty) | Internal hierarchy in `src/httpserver/details/body.hpp` |
44+
| `detail::body` | Polymorphic body kinds (string / file / iovec / pipe / deferred / empty) | Internal hierarchy in `src/httpserver/detail/body.hpp` |
4545
| Route table | Path → (method_set, handler) lookup | `unordered_map` (exact) + radix tree (parameterized + prefix) + regex chain (fallback) |
4646

4747
---

specs/architecture/04-components/body-hierarchy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
**Responsibility:** Polymorphic body representation backing `http_response`'s SBO buffer. Each subclass carries the data needed for one body kind and knows how to stream itself into an MHD response.
44

5-
**Implementation:** Abstract base in `src/httpserver/details/body.hpp` (not installed):
5+
**Implementation:** Abstract base in `src/httpserver/detail/body.hpp` (not installed):
66

77
```cpp
88
namespace httpserver::detail {

specs/architecture/04-components/http-response.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
- `detail::body* body_` — points into `body_storage_` (inline) or to a heap object
1111
- `bool body_inline_` — bookkeeping for destructor / move
1212

13-
The body subclasses (`detail::string_body`, `file_body`, `iovec_body`, `pipe_body`, `deferred_body`, `empty_body`) live in `src/httpserver/details/body.hpp` and are not installed.
13+
The body subclasses (`detail::string_body`, `file_body`, `iovec_body`, `pipe_body`, `deferred_body`, `empty_body`) live in `src/httpserver/detail/body.hpp` and are not installed.
1414

1515
**SBO contract:**
1616
- All current body subclasses are sized to fit in 64 bytes. The largest, `deferred_body` (~56 bytes including vptr + `std::function` on libstdc++), has 8 bytes of headroom.
@@ -20,7 +20,7 @@ The body subclasses (`detail::string_body`, `file_body`, `iovec_body`, `pipe_bod
2020
**Interfaces:**
2121
- Exposes (from PRD §3.5):
2222
- Factories: `http_response::string(...)`, `::file(...)`, `::iovec(std::span<const httpserver::iovec_entry>)`, `::pipe(...)`, `::empty(...)`, `::deferred(...)`, `::unauthorized(scheme, realm, ...)` — all return `http_response` by value.
23-
- **`httpserver::iovec_entry`** is a library-defined POD declared in `<httpserver/http_response.hpp>`: `struct iovec_entry { const void* base; std::size_t len; };`. It mirrors POSIX `struct iovec` exactly in layout but does not require `<sys/uio.h>` in any installed header. The internal dispatch path uses the user-supplied span to build a `struct iovec` array inside `iovec_body`. The implementation file (`details/body.hpp` / `http_response.cpp`) carries `static_assert`s pinning the layout assumption: `static_assert(sizeof(iovec_entry) == sizeof(struct iovec))`, `static_assert(offsetof(iovec_entry, base) == offsetof(struct iovec, iov_base))`, `static_assert(offsetof(iovec_entry, len) == offsetof(struct iovec, iov_len))`. When the asserts hold, conversion is a `reinterpret_cast`; when they fail (a hypothetical platform with divergent layout), the build fails loudly at compile time and we fall back to memcpy. This keeps the public header free of system headers and makes the API uniformly available on platforms where `<sys/uio.h>` is not standard (e.g., MSVC builds).
23+
- **`httpserver::iovec_entry`** is a library-defined POD declared in `<httpserver/http_response.hpp>`: `struct iovec_entry { const void* base; std::size_t len; };`. It mirrors POSIX `struct iovec` exactly in layout but does not require `<sys/uio.h>` in any installed header. The internal dispatch path uses the user-supplied span to build a `struct iovec` array inside `iovec_body`. The implementation file (`detail/body.hpp` / `http_response.cpp`) carries `static_assert`s pinning the layout assumption: `static_assert(sizeof(iovec_entry) == sizeof(struct iovec))`, `static_assert(offsetof(iovec_entry, base) == offsetof(struct iovec, iov_base))`, `static_assert(offsetof(iovec_entry, len) == offsetof(struct iovec, iov_len))`. When the asserts hold, conversion is a `reinterpret_cast`; when they fail (a hypothetical platform with divergent layout), the build fails loudly at compile time and we fall back to memcpy. This keeps the public header free of system headers and makes the API uniformly available on platforms where `<sys/uio.h>` is not standard (e.g., MSVC builds).
2424
- Fluent setters: `with_header`, `with_footer`, `with_cookie`, `with_status` — return `http_response&`.
2525
- `const` accessors: `get_header`, `get_footer`, `get_cookie` returning `string_view` (empty on miss; do not insert).
2626
- `get_headers`, `get_footers`, `get_cookies` returning `const map&`.

specs/architecture/04-components/webserver.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
**Responsibility:** Library entry point. Owns the libmicrohttpd daemon, the route table, the IP block list, the connection arena pool. Provides start/stop, route registration (lambda + class forms), `block_ip`/`unblock_ip`, `features()`.
44

5-
**Implementation:** PIMPL via `std::unique_ptr<webserver_impl>`. Public header `<httpserver/webserver.hpp>` includes only `<httpserver/create_webserver.hpp>` and standard library, never `<microhttpd.h>` or `<pthread.h>`. `webserver_impl` (in `src/httpserver/details/webserver_impl.hpp`) holds the `MHD_Daemon*`, the route-table data structures, per-connection arena state, and synchronization primitives.
5+
**Implementation:** PIMPL via `std::unique_ptr<webserver_impl>`. Public header `<httpserver/webserver.hpp>` includes only `<httpserver/create_webserver.hpp>` and standard library, never `<microhttpd.h>` or `<pthread.h>`. `webserver_impl` (in `src/httpserver/detail/webserver_impl.hpp`) holds the `MHD_Daemon*`, the route-table data structures, per-connection arena state, and synchronization primitives.
66

77
**Interfaces:**
88
- Exposes (from PRD §3.4 and §3.7):

specs/architecture/05-cross-cutting.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ src/
5555
│ ├── create_webserver.hpp
5656
│ ├── create_test_request.hpp
5757
│ ├── file_info.hpp
58-
│ └── details/ # NOT installed (existing convention)
58+
│ └── detail/ # NOT installed (existing convention)
5959
│ ├── webserver_impl.hpp # NEW
6060
│ ├── http_request_impl.hpp # NEW
6161
│ ├── body.hpp # NEW — detail::body + subclasses
@@ -64,6 +64,6 @@ src/
6464
└── *.cpp # implementations
6565
```
6666

67-
Public headers gate on `_HTTPSERVER_HPP_INSIDE_` or `HTTPSERVER_COMPILATION`. `details/` headers gate on `HTTPSERVER_COMPILATION` only (consumers cannot reach in). `Makefile.am` continues to install `httpserver/*.hpp` and exclude `httpserver/details/`.
67+
Public headers gate on `_HTTPSERVER_HPP_INSIDE_` or `HTTPSERVER_COMPILATION`. `detail/` headers gate on `HTTPSERVER_COMPILATION` only (consumers cannot reach in). `Makefile.am` continues to install `httpserver/*.hpp` and exclude `httpserver/detail/`.
6868

6969
---

specs/architecture/06-backend-integration.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
### 6.1 libmicrohttpd
44

5-
The only backend. v2.0 does not abstract over alternative backends and explicitly rules pluggability out (PRD §3.1 out-of-scope). The `MHD_Daemon*`, `MHD_Connection*`, `MHD_Response*` types appear only in `details/` headers and `.cpp` files.
5+
The only backend. v2.0 does not abstract over alternative backends and explicitly rules pluggability out (PRD §3.1 out-of-scope). The `MHD_Daemon*`, `MHD_Connection*`, `MHD_Response*` types appear only in `detail/` headers and `.cpp` files.
66

77
### 6.2 GnuTLS
88

99
Optional (controlled by `HAVE_GNUTLS`). When disabled at build time, the public TLS-related methods on `http_request` (cert DN, fingerprint, etc.) return empty / sentinel values, and `webserver::features().tls == false`. When enabled, the implementation calls `gnutls_*` functions directly; `gnutls_session_t` is never returned through the public API.
1010

1111
### 6.3 pthread
1212

13-
Used by libmicrohttpd's worker pool and by libhttpserver's internal start/stop synchronization (`pthread_mutex_t mutexwait` / `pthread_cond_t mutexcond`). All `pthread.h` inclusions move to `details/` and `.cpp` files. The public API exposes no pthread types.
13+
Used by libmicrohttpd's worker pool and by libhttpserver's internal start/stop synchronization (`pthread_mutex_t mutexwait` / `pthread_cond_t mutexcond`). All `pthread.h` inclusions move to `detail/` and `.cpp` files. The public API exposes no pthread types.
1414

1515
---

specs/architecture/09-testing.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ The architecture itself does not prescribe test frameworks (out of architecture
55
1. **Header hygiene** (PRD-HDR-REQ-001..003): a CI test compiles a TU containing only `#include <httpserver.hpp>` and `int main() {}` with no `-I` to libmicrohttpd / pthread / gnutls headers.
66
2. **Build-flag invariance** (PRD-FLG-REQ-001): the same consumer source compiles against `--disable-tls` and `--enable-tls` builds without changes.
77
3. **Move semantics on `http_response`** (DR-5): sanitizer-clean tests for inline↔inline, inline↔heap, heap↔inline, heap↔heap on both move-construct and move-assign.
8-
4. **SBO size invariant** (DR-5): `static_assert(sizeof(detail::deferred_body) <= http_response::body_buf_size, ...)` at the end of `details/body.hpp`. Compile-time guarantee.
8+
4. **SBO size invariant** (DR-5): `static_assert(sizeof(detail::deferred_body) <= http_response::body_buf_size, ...)` at the end of `detail/body.hpp`. Compile-time guarantee.
99
5. **Routing semantics preservation** (DR-7): the v1 routing-test corpus runs against v2.0 unchanged. Any regression is treated as a release-blocker.
1010
6. **Thread-safety contract** (DR-8): a stress test exercises `register_resource` / `block_ip` from within handlers, verifies no deadlock except for the documented `stop()` case.
1111

specs/architecture/11-decisions/DR-002.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,17 @@
55
**Context:** PIMPL committed; impl headers must live somewhere that's not reachable from `<httpserver.hpp>` and not installed by `make install`.
66

77
**Options considered:**
8-
1. **Everything in `src/`, impls in `src/httpserver/details/`** — small diff; `details/` already exists and is excluded from install.
9-
2. **Two-tier `details/` for shared internals + `src/internal/` for PIMPL impls** — strongest semantic split; more Makefile surface; new directory.
8+
1. **Everything in `src/`, impls in `src/httpserver/detail/`** — small diff; `detail/` already exists and is excluded from install.
9+
2. **Two-tier `detail/` for shared internals + `src/internal/` for PIMPL impls** — strongest semantic split; more Makefile surface; new directory.
1010
3. **Co-locate impls next to public headers (`webserver_impl.hpp` next to `webserver.hpp`) with stricter guard** — best discoverability; one typo and the impl ships to packagers.
1111

1212
**Decision:** Option 1.
1313

14-
**Rationale:** The `details/` convention works, packagers already skip it, and the cost of mixing PIMPL impls with other internal types is low — they're all "things that don't escape the .so." Option 2's clean split adds Makefile complexity for marginal navigability. Option 3 mixes public and private headers under the same `*.hpp` glob, which is install-rule-fragile.
14+
**Rationale:** The `detail/` convention works, packagers already skip it, and the cost of mixing PIMPL impls with other internal types is low — they're all "things that don't escape the .so." Option 2's clean split adds Makefile complexity for marginal navigability. Option 3 mixes public and private headers under the same `*.hpp` glob, which is install-rule-fragile.
1515

1616
**Consequences:**
17-
- File-naming convention: `<class>_impl.hpp` (so `webserver.hpp``details/webserver_impl.hpp`).
18-
- Detail headers in `src/httpserver/details/` use the gate `#if !defined(_HTTPSERVER_HPP_INSIDE_) && !defined(HTTPSERVER_COMPILATION)` (dual-mode). The stricter `#ifndef HTTPSERVER_COMPILATION`-only gate cannot be applied yet because `webserver.hpp` (public) still transitively includes `details/http_endpoint.hpp`, which means the detail header is reached via the umbrella path (`_HTTPSERVER_HPP_INSIDE_` defined). This dual-mode gate will be tightened to `HTTPSERVER_COMPILATION`-only once TASK-014 lands the PIMPL split that removes the transitive include from `webserver.hpp`.
19-
- `src/Makefile.am` lists `details/*.hpp` under `noinst_HEADERS` so they are distributed in the source tarball but never installed under `$prefix/include`.
17+
- File-naming convention: `<class>_impl.hpp` (so `webserver.hpp``detail/webserver_impl.hpp`).
18+
- Detail headers in `src/httpserver/detail/` use the gate `#if !defined(_HTTPSERVER_HPP_INSIDE_) && !defined(HTTPSERVER_COMPILATION)` (dual-mode). The stricter `#ifndef HTTPSERVER_COMPILATION`-only gate cannot be applied yet because `webserver.hpp` (public) still transitively includes `detail/http_endpoint.hpp`, which means the detail header is reached via the umbrella path (`_HTTPSERVER_HPP_INSIDE_` defined). This dual-mode gate will be tightened to `HTTPSERVER_COMPILATION`-only once TASK-014 lands the PIMPL split that removes the transitive include from `webserver.hpp`.
19+
- `src/Makefile.am` lists `detail/*.hpp` under `noinst_HEADERS` so they are distributed in the source tarball but never installed under `$prefix/include`.
2020

2121
---

specs/architecture/11-decisions/DR-005.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
- `http_response` carries `alignas(16) std::byte body_storage_[64]` + `detail::body* body_` + `bool body_inline_`.
2020
- Hand-written move ctor + move assign covering the inline/heap cross-product (4 cases).
2121
- Destructor calls `~body()` always; `delete` only if `!body_inline_`.
22-
- Compile-time `static_assert(sizeof(detail::deferred_body) <= 64)` and per-subclass `static_assert` at end of `details/body.hpp`.
22+
- Compile-time `static_assert(sizeof(detail::deferred_body) <= 64)` and per-subclass `static_assert` at end of `detail/body.hpp`.
2323
- Sanitizer-clean tests required for all 4 move cases.
2424
- Bumping the buffer in v2.x is an ABI break (recompile callers).
2525

specs/architecture/12-open-questions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
| ID | Question / Risk | Impact | Mitigation | Owner |
44
|---|---|---|---|---|
55
| AR-001 | RHEL 9 stock GCC 11 cannot build v2.0 without `gcc-toolset-14`. Distro packagers may push back. | M | Document the toolset requirement in §8 and RELEASE_NOTES. Confirmed Red Hat-supported path. | Maintainer |
6-
| AR-002 | Adding a body kind > 64 B in v2.x causes silent heap fallback (correct but unexpected). | L | `static_assert` guard in `details/body.hpp`; release-process checklist includes "do new body kinds fit in SBO?". | Maintainer |
6+
| AR-002 | Adding a body kind > 64 B in v2.x causes silent heap fallback (correct but unexpected). | L | `static_assert` guard in `detail/body.hpp`; release-process checklist includes "do new body kinds fit in SBO?". | Maintainer |
77
| AR-003 | Routing semantics regression in the hash + radix + regex split (DR-7). | H | Run v1's full routing-test corpus against v2.0 unchanged; treat any failure as release-blocker. | Maintainer |
88
| AR-004 | `http_response` move-semantics (inline↔heap cross-product) is bug-prone. | M | Sanitizer-clean tests for all 4 move cases (covered in §9). | Maintainer |
99
| AR-005 | Per-request arena allocator plumbing leaks abstraction (request constructor needs implicit access to connection state). | L | Plumbing is internal; documented in `webserver_impl` design notes. No public API impact. | Maintainer |

0 commit comments

Comments
 (0)