From e0282ef819b634279a70979ae8064b85a6a8781e Mon Sep 17 00:00:00 2001 From: NotAProfDev <84450364+NotAProfDev@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:36:35 +0000 Subject: [PATCH] refactor(net): harden compose (tests, must_use, docs, neutral param) Follow-up polish to oath-adapter-net-api after the #121 vocabulary rename. All in the kernel crate; no behaviour change. - Add a lock-in test module for compose.rs asserting the ordering invariant (first .layer() is outermost), Identity pass-through, Default == new, and the zero-cost (ZST) property. The ordering invariant was previously only compile-checked by assert-nothing doctests. - Add #[must_use] to LayerBuilder::wrap(): assembling a stack and discarding the composed value is a bug (matches .layer() / new()). - Make the crate's public rustdoc self-contained by dropping the internal ADR-0029 citations from compose.rs, lib.rs, and timer.rs. - Rename the Layer value type parameter S -> T. S was tower's "Service" vestige; the kernel is transport-neutral, so the parameter carries no Service connotation. HTTP layer impls keep S (there it is genuinely a Service). Closes #122 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 5 ++ crates/adapter/net/api/src/compose.rs | 103 +++++++++++++++++++++----- crates/adapter/net/api/src/lib.rs | 9 +-- crates/adapter/net/api/src/timer.rs | 2 +- 4 files changed, 93 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24f266d..4e98461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **net-api compose polish — no behaviour change.** Added `#[must_use]` to + `LayerBuilder::wrap()` (assembling a stack and then discarding it is a bug); added + unit tests locking the compose ordering invariant, `Identity` pass-through, and the + zero-cost (ZST) property; and made the `oath-adapter-net-api` rustdoc self-contained + (removed internal-ADR citations from `compose`/`lib`/`timer`). - **Breaking (pre-release) — net compose vocabulary.** Renamed the transport-neutral composition machinery in `oath-adapter-net-api` to match its `Service`-agnostic role (ADR-0029 §3): `ServiceBuilder` → `LayerBuilder`, the `Layer::Service` associated type diff --git a/crates/adapter/net/api/src/compose.rs b/crates/adapter/net/api/src/compose.rs index bd9f5bb..886952b 100644 --- a/crates/adapter/net/api/src/compose.rs +++ b/crates/adapter/net/api/src/compose.rs @@ -1,11 +1,11 @@ //! Composition machinery: `Layer`, `LayerBuilder`, `Identity`, `Stack`. //! -//! These compose **anything** — `Layer` carries no `Service` bound (ADR-0029 -//! §3), so the same machinery composes an HTTP `Service` stack today and a WS -//! subscription stack tomorrow. The composition *unit* (`Layer` / `LayerBuilder` -//! / `Stack`) is shared; the assembled *product* is transport-specific (an HTTP -//! `Service`, a WS reconnect connector, …), which is why the output type is -//! named [`Layer::Wrapped`], not `Service`. +//! These compose **anything** — `Layer` places no bound on `T`, so the same +//! machinery composes an HTTP `Service` stack today and a WebSocket subscription +//! stack tomorrow. The composition *unit* (`Layer` / `LayerBuilder` / `Stack`) is +//! shared; the assembled *product* is transport-specific (an HTTP `Service`, a WS +//! reconnect connector, …), which is why the output type is named +//! [`Layer::Wrapped`], not `Service`. //! //! # Ordering invariant //! @@ -16,8 +16,8 @@ //! # use oath_adapter_net_api::compose::{Layer, LayerBuilder}; //! # struct TracingLayer; //! # struct MetricsLayer; -//! # impl Layer for TracingLayer { type Wrapped = S; fn layer(&self, s: S) -> S { s } } -//! # impl Layer for MetricsLayer { type Wrapped = S; fn layer(&self, s: S) -> S { s } } +//! # impl Layer for TracingLayer { type Wrapped = T; fn layer(&self, inner: T) -> T { inner } } +//! # impl Layer for MetricsLayer { type Wrapped = T; fn layer(&self, inner: T) -> T { inner } } //! // TracingLayer is added first → outermost → wraps everything else. //! let _svc = LayerBuilder::new() //! .layer(TracingLayer) // outermost @@ -25,12 +25,12 @@ //! .wrap(()); // leaf: any value (a `Service` leaf lives in net-http-api) //! ``` -/// Wrap a value of type `S`, producing a new value that adds behaviour. +/// Wrap a value of type `T`, producing a new value that adds behaviour. /// /// Typically a struct that holds configuration and owns an inner value. The /// outer layer's [`Layer::layer`] method wraps the inner value, producing a /// new value that adds the layer's behaviour. -pub trait Layer { +pub trait Layer { /// The wrapped type produced by this layer. /// /// Transport-neutral: it is an HTTP `Service` for an HTTP stack, a WS @@ -39,7 +39,7 @@ pub trait Layer { type Wrapped; /// Wrap `inner` with this layer's behaviour. - fn layer(&self, inner: S) -> Self::Wrapped; + fn layer(&self, inner: T) -> Self::Wrapped; } /// Type-safe layer compositor. @@ -91,9 +91,10 @@ impl LayerBuilder { /// Consumes the builder and returns the fully composed value. /// The concrete type is fully resolved at compile time — no boxing, no /// `dyn`. - pub fn wrap(self, inner: S) -> L::Wrapped + #[must_use] + pub fn wrap(self, inner: T) -> L::Wrapped where - L: Layer, + L: Layer, { self.layer.layer(inner) } @@ -105,10 +106,10 @@ impl LayerBuilder { #[derive(Debug, Clone)] pub struct Identity; -impl Layer for Identity { - type Wrapped = S; +impl Layer for Identity { + type Wrapped = T; - fn layer(&self, inner: S) -> S { + fn layer(&self, inner: T) -> T { inner } } @@ -129,16 +130,80 @@ pub struct Stack { outer: Outer, } -impl Layer for Stack +impl Layer for Stack where - Inner: Layer, + Inner: Layer, Outer: Layer, { type Wrapped = Outer::Wrapped; - fn layer(&self, value: S) -> Outer::Wrapped { + fn layer(&self, value: T) -> Outer::Wrapped { // Apply Inner first (closer to the leaf), then wrap with Outer. let wrapped = self.inner.layer(value); self.outer.layer(wrapped) } } + +#[cfg(test)] +mod tests { + use super::{Identity, Layer, LayerBuilder, Stack}; + use core::mem::size_of; + + // A layer that parenthesises a `String` under its tag, making the composition + // nesting — and therefore the ordering invariant — directly observable: + // `Tag("A").layer("x".into()) == "A(x)"`. + struct Tag(&'static str); + + impl Layer for Tag { + type Wrapped = String; + + fn layer(&self, inner: String) -> String { + format!("{}({})", self.0, inner) + } + } + + #[test] + fn first_layer_is_outermost() { + // `A` is added first, so it must end up outermost — wrapping `B`, which in + // turn wraps the leaf. This is the module's load-bearing ordering invariant. + let composed = LayerBuilder::new() + .layer(Tag("A")) + .layer(Tag("B")) + .wrap(String::from("leaf")); + assert_eq!(composed, "A(B(leaf))"); + } + + #[test] + fn empty_builder_returns_the_leaf_unchanged() { + // A fresh builder is just `Identity`, so it hands the leaf back as-is. + let composed = LayerBuilder::new().wrap(String::from("leaf")); + assert_eq!(composed, "leaf"); + } + + #[test] + fn identity_in_the_middle_is_a_noop() { + // An explicit `Identity` layer must not affect the composed result. + let composed = LayerBuilder::new() + .layer(Tag("A")) + .layer(Identity) + .layer(Tag("B")) + .wrap(String::from("leaf")); + assert_eq!(composed, "A(B(leaf))"); + } + + #[test] + fn default_matches_new() { + let via_default = LayerBuilder::::default().wrap(String::from("x")); + let via_new = LayerBuilder::new().wrap(String::from("x")); + assert_eq!(via_default, via_new); + } + + #[test] + fn composition_machinery_is_zero_sized() { + // The no-op layer, a fresh builder, and a stack of ZST layers all cost + // zero bytes — the "no boxing, no dyn" promise made testable. + assert_eq!(size_of::(), 0); + assert_eq!(size_of::>(), 0); + assert_eq!(size_of::>(), 0); + } +} diff --git a/crates/adapter/net/api/src/lib.rs b/crates/adapter/net/api/src/lib.rs index 5ebd4b8..2126ea4 100644 --- a/crates/adapter/net/api/src/lib.rs +++ b/crates/adapter/net/api/src/lib.rs @@ -1,15 +1,12 @@ //! `oath-adapter-net-api` — transport-neutral composition primitives + contracts. //! -//! This crate is **std-only** (zero deps — the signal the ADR-0029 cut is -//! clean). It defines the shared abstractions every transport's layers depend -//! on: +//! This crate is **std-only** (zero dependencies — the signal that the +//! transport-neutral cut is clean). It defines the shared abstractions every +//! transport's layers depend on: //! //! - [`compose`] — `Layer`, `LayerBuilder`, `Identity`, `Stack` //! - [`error_kind`] — `ErrorKind`, `HasErrorKind` //! - [`timer`] — `Timer` -//! -//! `Service` is **not** here — it is a per-transport contract in -//! `oath-adapter-net-http-api` (ADR-0029 §2). #![forbid(unsafe_code)] pub mod compose; diff --git a/crates/adapter/net/api/src/timer.rs b/crates/adapter/net/api/src/timer.rs index 21340a8..2c1f1e7 100644 --- a/crates/adapter/net/api/src/timer.rs +++ b/crates/adapter/net/api/src/timer.rs @@ -8,7 +8,7 @@ use std::time::{Duration, Instant}; /// Timing middleware (`Timeout`, `Retry` backoff, `RateLimit` refill, /// `CircuitBreaker` cooldown) is generic over `Timer` so a mock clock can drive /// it deterministically in tests while production passes a runtime-backed impl. -/// A trait — not a runtime — so the kernel stays std-only (ADR-0029 §4). +/// A trait — not a runtime — so the kernel stays std-only. pub trait Timer: Clone + Send + Sync { /// Complete after `dur` has elapsed. fn sleep(&self, dur: Duration) -> impl Future + Send;