docs(arch): add Connection::from_source to ADR-070 — the missing extension point
ADR-070 made Connection hold Box<dyn BidiStreamSource> so downstream crates can add connection shapes without editing core. The trait and three built-in impls landed, but the public constructor that lets a downstream crate construct a Connection from its own BidiStreamSource impl was never added — the source field is private with no from_source constructor. The channels POC update surfaced this when it tried to use the trait directly and found no way to build a Connection from a ChannelBidiStreamSource. This is a gap in the ADR-070 implementation, not a new decision. ADR-070 §Consequences says 'the channels crate implements ChannelBidiStreamSource in its own crate and constructs Connection from it' — the constructor for that path is what was missing. - ADR-070: add from_source to the Constructors table + reword the downstream-crates paragraph to make the two paths explicit (from_source for custom impls; from_quinn/from_iroh/from_stream for built-in impls) - core-types.md: add from_source to the impl Connection block, the built-in implementations table, and the design-decisions table entry - tasks/core/connection-from-source-constructor.md: the implementation task (one constructor + one test, scope narrow, risk low)
This commit is contained in:
glm-5.2 committed
1 parent
3954c788d4
commit
02c5b9e039
4 files changed
+184
-11
No files matched your search
@@ -34,7 +34,7 @@ Core library for ALPN-based protocol dispatch. Every handler crate depends on al
|
||||
| [031](../../decisions/031-credentialstore-repo-trait.md) | CredentialStore Repo Trait | Second repo trait in core; `InMemoryCredentialStore` default adapter |
|
||||
| [033](../../decisions/033-storage-boundary-and-repo-adapter-pattern.md) | Storage Boundary and Repo/Adapter Pattern | Core defines traits + in-memory defaults; persistence adapters are separate crates |
|
||||
| [065](../../decisions/065-connection-from-stream-generic-single-stream.md) | `Connection::from_stream` — Generic Single-Stream Connections | `from_stream`/`from_bidi` accept any `AsyncRead + AsyncWrite`; yield-once `accept_bi` contract; unblocks TCP+TLS, SSH channels, WebTransport, wasm |
|
||||
| [070](../../decisions/070-bidistreamsource-trait.md) | BidiStreamSource Trait — Open Connection for Extension | `Connection` holds `Box<dyn BidiStreamSource>`; QUIC/iroh/stream wrap crate-private impls; downstream crates implement the trait to add connection shapes (channels, future transports) without editing core |
|
||||
| [070](../../decisions/070-bidistreamsource-trait.md) | BidiStreamSource Trait — Open Connection for Extension | `Connection` holds `Box<dyn BidiStreamSource>`; QUIC/iroh/stream wrap crate-private impls; `from_source` is the public constructor for downstream crates that implement the trait (channels, future transports) |
|
||||
|
||||
## Relevant Open Questions
|
||||
|
||||
|
||||
@@ -94,6 +94,11 @@ impl Connection {
|
||||
remote_addr: Option<SocketAddr>,
|
||||
) -> Self;
|
||||
|
||||
/// Construct from a caller-supplied `BidiStreamSource` impl. The
|
||||
/// extension point for downstream crates — implement the trait and
|
||||
/// construct a `Connection` from it without editing core. See ADR-070.
|
||||
pub fn from_source(source: impl BidiStreamSource, alpn: Vec<u8>) -> Self;
|
||||
|
||||
pub async fn accept_bi(&self) -> Result<(SendStream, RecvStream), StreamError>;
|
||||
pub async fn open_bi(&self) -> Result<(SendStream, RecvStream), StreamError>;
|
||||
pub fn remote_alpn(&self) -> &[u8];
|
||||
@@ -186,11 +191,15 @@ indirection and are not the transport's concern.
|
||||
| `QuinnBidiStreamSource` | `Connection::from_quinn` / `from_quinn_with_alpn` (feature `quinn`) | many streams |
|
||||
| `IrohBidiStreamSource` | `Connection::from_iroh` (feature `iroh`) | many streams |
|
||||
| `StreamBidiStreamSource` | `Connection::from_stream` / `from_bidi` (no feature gate) | yield-once, then `ConnectionClosed`; `open_bi` returns `StreamClosed` |
|
||||
| *(caller-supplied)* | `Connection::from_source` (no feature gate) | per the caller's `BidiStreamSource` impl |
|
||||
|
||||
Downstream crates do not wrap `from_stream` to implement
|
||||
`BidiStreamSource` — they implement the trait directly. `from_stream` is
|
||||
the compatibility path for callers that want a `Connection` over a single
|
||||
pre-split stream (the ADR-065 use case).
|
||||
`BidiStreamSource` — they implement the trait directly and construct the
|
||||
`Connection` via `from_source`. `from_stream` is the compatibility path for
|
||||
callers that want a `Connection` over a single pre-split stream (the
|
||||
ADR-065 use case); `from_source` is the extension path for callers that
|
||||
implement `BidiStreamSource` themselves (the channels crate's
|
||||
`ChannelBidiStreamSource`, a future transport, a test double).
|
||||
|
||||
## BiStream
|
||||
|
||||
@@ -366,7 +375,7 @@ registration bundle.
|
||||
| ProtocolHandler receives Connection, not BiStream | [ADR-007](../../decisions/007-bistream-type-definition.md) | Handlers that need multiple streams (SSH, call) have direct access to the Connection |
|
||||
| BiStream is a trait | [ADR-007](../../decisions/007-bistream-type-definition.md) | WASM door preserved, test mocks possible |
|
||||
| `Connection::from_stream` — generic single-stream connections | [ADR-065](../../decisions/065-connection-from-stream-generic-single-stream.md) | `from_stream`/`from_bidi` accept any `AsyncRead + AsyncWrite`; yield-once `accept_bi` contract; unblocks TCP+TLS, SSH channels, WebTransport, wasm; QUIC variants feature-gated, `Stream` variant always available; `MockConnection`/`ConnectionKind::Mock` removed (tests use `from_stream` with `sink`/`empty`) |
|
||||
| `BidiStreamSource` — open `Connection` for extension | [ADR-070](../../decisions/070-bidistreamsource-trait.md) | `Connection` holds `Box<dyn BidiStreamSource>`; QUIC/iroh/stream wrap crate-private impls; downstream crates implement the trait to add connection shapes (channels, future transports) without editing core; public `Connection` API preserved; `close(code, reason)` kept on the trait (non-QUIC impls ignore the args — fixes the ADR-065 leftover clippy warning under `--no-default-features`) |
|
||||
| `BidiStreamSource` — open `Connection` for extension | [ADR-070](../../decisions/070-bidistreamsource-trait.md) | `Connection` holds `Box<dyn BidiStreamSource>`; QUIC/iroh/stream wrap crate-private impls; `from_source` is the public constructor for downstream crates that implement the trait (channels, future transports); `from_quinn`/`from_iroh`/`from_stream`/`from_bidi` preserved; `close(code, reason)` kept on the trait (non-QUIC impls ignore the args — fixes the ADR-065 leftover clippy warning under `--no-default-features`) |
|
||||
| HandlerError is non-fatal | [ADR-010](../../decisions/010-alpn-router-and-endpoint.md) | Handler errors close the connection, not the endpoint |
|
||||
| SendStream/RecvStream wrap quinn + iroh + generic streams | [ADR-010](../../decisions/010-alpn-router-and-endpoint.md), [ADR-065](../../decisions/065-connection-from-stream-generic-single-stream.md) | Internal enum dispatch for QUIC sources and the generic `Stream` variant |
|
||||
| Connection stores handler-resolved identity | OQ-11 (resolved) | `set_identity` via `OnceLock` — write-once-read-many; read by handler-side logging, not by the endpoint (C13 resolved) |
|
||||
|
||||
@@ -140,11 +140,22 @@ delegates to `self.source`. `remote_alpn` reads `self.alpn` (unchanged).
|
||||
| `from_quinn` / `from_quinn_with_alpn` (feature `quinn`) | `QuinnBidiStreamSource` (crate-private) |
|
||||
| `from_iroh` (feature `iroh`) | `IrohBidiStreamSource` (crate-private) |
|
||||
| `from_stream` / `from_bidi` (no feature gate) | `StreamBidiStreamSource` (crate-private, yield-once) |
|
||||
| `from_source` (no feature gate) | caller-supplied `impl BidiStreamSource` — the extension point for downstream crates |
|
||||
|
||||
`from_source(source: impl BidiStreamSource, alpn: Vec<u8>) -> Self` is the
|
||||
constructor that makes the trait the extension point. A downstream crate
|
||||
implements `BidiStreamSource` (e.g. the channels crate's
|
||||
`ChannelBidiStreamSource`) and constructs a `Connection` from it via
|
||||
`from_source` — no core edit. The built-in impls (`QuinnBidiStreamSource`,
|
||||
`IrohBidiStreamSource`, `StreamBidiStreamSource`) are crate-private;
|
||||
`from_source` is the only path a downstream crate uses to wrap its own
|
||||
impl. The `from_quinn` / `from_iroh` / `from_stream` / `from_bidi`
|
||||
constructors are convenience wrappers for the three built-in impls.
|
||||
|
||||
The `Stream`-backend implementations are crate-private; downstream crates
|
||||
do not implement `BidiStreamSource` by wrapping `from_stream`. They
|
||||
implement the trait directly (channels: `ChannelBidiStreamSource`), or they
|
||||
use a public constructor that already wraps an impl.
|
||||
implement the trait directly (channels: `ChannelBidiStreamSource`) and
|
||||
construct the `Connection` via `from_source`.
|
||||
|
||||
### `from_stream`-backed default impl is the compatibility path
|
||||
|
||||
@@ -233,10 +244,10 @@ the args are optional for their transport.
|
||||
|
||||
- `Connection` is open for extension. The channels crate implements
|
||||
`ChannelBidiStreamSource` in its own crate and constructs `Connection`
|
||||
from it — no core edit. A future transport, test double, or relay
|
||||
connection follows the same path. This is the structural payoff: the
|
||||
connection type is no longer a closed enum that every new connection
|
||||
shape must edit.
|
||||
from it via `from_source` — no core edit. A future transport, test
|
||||
double, or relay connection follows the same path. This is the
|
||||
structural payoff: the connection type is no longer a closed enum that
|
||||
every new connection shape must edit.
|
||||
- A channels connection is a first-class peer of QUIC: one
|
||||
`ChannelConnection` that yields N streams, rather than a bag of yield-
|
||||
once `Connection`s. The channels layer's API matches its actual shape.
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
---
|
||||
id: core/connection-from-source-constructor
|
||||
name: "Add Connection::from_source constructor (ADR-070 gap — the public extension point)"
|
||||
status: pending
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: component
|
||||
level: implementation
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
ADR-070 made `Connection` hold `Box<dyn BidiStreamSource>` so downstream
|
||||
crates can add connection shapes without editing core. The trait exists,
|
||||
the three built-in impls exist, but the **public constructor that lets a
|
||||
downstream crate construct a `Connection` from its own `BidiStreamSource`
|
||||
impl is missing.** The `source` field is private; there is no way for
|
||||
the channels crate (or any future crate) to build a `Connection` from a
|
||||
`ChannelBidiStreamSource` — the trait is currently unusable from outside
|
||||
core.
|
||||
|
||||
This is a gap in the ADR-070 implementation, not a new architectural
|
||||
decision. ADR-070 says "the channels crate implements
|
||||
`ChannelBidiStreamSource` in its own crate and constructs `Connection`
|
||||
from it" (§Consequences), but the constructor for that path was never
|
||||
added. The alknet-channels POC update surfaced this when it tried to use
|
||||
the trait directly and found no `from_source`.
|
||||
|
||||
### Current state (`crates/alknet-core/src/types.rs`)
|
||||
|
||||
```rust
|
||||
pub struct Connection {
|
||||
source: Box<dyn BidiStreamSource>, // private
|
||||
alpn: Vec<u8>,
|
||||
identity: OnceLock<Identity>,
|
||||
}
|
||||
|
||||
impl Connection {
|
||||
// from_quinn, from_quinn_with_alpn, from_iroh, from_stream, from_bidi
|
||||
// — all wrap crate-private impls. No constructor takes a
|
||||
// caller-supplied BidiStreamSource.
|
||||
}
|
||||
```
|
||||
|
||||
### Target state
|
||||
|
||||
```rust
|
||||
impl Connection {
|
||||
/// Construct from a caller-supplied `BidiStreamSource` impl. The
|
||||
/// extension point for downstream crates — implement the trait and
|
||||
/// construct a `Connection` from it without editing core. See ADR-070.
|
||||
pub fn from_source(source: impl BidiStreamSource, alpn: Vec<u8>) -> Self {
|
||||
Self {
|
||||
source: Box::new(source),
|
||||
alpn,
|
||||
identity: OnceLock::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The signature takes `impl BidiStreamSource` (not `Box<dyn
|
||||
BidiStreamSource>`) to match the ergonomic style of `from_stream` /
|
||||
`from_bidi` (which take `impl AsyncWrite + Send + Unpin + 'static`). The
|
||||
trait already requires `Send + Sync + 'static`, so no extra bounds are
|
||||
needed. The `Box::new(source)` is an internal implementation detail.
|
||||
|
||||
### Why not just use `from_stream`
|
||||
|
||||
`from_stream` wraps a `StreamBidiStreamSource` (yield-once). A downstream
|
||||
crate that implements `BidiStreamSource` for a multi-stream source (e.g.
|
||||
channels' `ChannelBidiStreamSource` that yields one stream per channel)
|
||||
cannot use `from_stream` — it needs `from_source` to wrap its own impl.
|
||||
Using `from_stream` for a multi-stream source would give yield-once
|
||||
behavior, which is wrong. This is exactly the gap the channels POC hit.
|
||||
|
||||
### Test
|
||||
|
||||
Add a test that constructs a `Connection` from a minimal custom
|
||||
`BidiStreamSource` impl (a mock that yields a fixed `SendStream` /
|
||||
`RecvStream` pair) and verifies `accept_bi` / `open_bi` / `remote_addr` /
|
||||
`close` delegate correctly. The test impl does not need to be realistic —
|
||||
it just needs to prove the public constructor works end-to-end with a
|
||||
non-built-in `BidiStreamSource`.
|
||||
|
||||
```rust
|
||||
#[cfg(test)]
|
||||
mod from_source_tests {
|
||||
use super::*;
|
||||
|
||||
struct MockSource {
|
||||
stream: Option<(SendStream, RecvStream)>,
|
||||
addr: Option<SocketAddr>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl BidiStreamSource for MockSource {
|
||||
async fn accept_bi(&self) -> Result<(SendStream, RecvStream), StreamError> {
|
||||
// yield-once mock
|
||||
}
|
||||
async fn open_bi(&self) -> Result<(SendStream, RecvStream), StreamError> {
|
||||
Err(StreamError::StreamClosed)
|
||||
}
|
||||
fn remote_addr(&self) -> Option<SocketAddr> { self.addr }
|
||||
fn close(&self, _code: u32, _reason: &str) {}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn from_source_delegates_to_custom_impl() {
|
||||
let conn = Connection::from_source(
|
||||
MockSource { stream: None, addr: None },
|
||||
b"alknet/test".to_vec(),
|
||||
);
|
||||
assert_eq!(conn.remote_alpn(), b"alknet/test");
|
||||
assert_eq!(conn.remote_addr(), None);
|
||||
// accept_bi delegates to MockSource::accept_bi
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(The test shape is illustrative — the implementer should make the mock
|
||||
yield a real `SendStream`/`RecvStream` pair via `tokio::io::duplex` so
|
||||
`accept_bi` returns a usable stream, and verify the stream round-trips.)
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `Connection::from_source(source: impl BidiStreamSource, alpn: Vec<u8>) -> Self` added to `types.rs`
|
||||
- [ ] `from_source` is `pub` and has no feature gate (always available, like `from_stream`)
|
||||
- [ ] `from_source` boxes the source internally: `source: Box::new(source)`
|
||||
- [ ] `from_source` initializes `alpn` and `identity: OnceLock::new()` (same as the other constructors)
|
||||
- [ ] Unit test: construct a `Connection` from a custom `BidiStreamSource` impl via `from_source`, verify `remote_alpn` / `remote_addr` / `accept_bi` / `open_bi` / `close` all delegate to the custom impl
|
||||
- [ ] Existing tests pass unchanged (`from_quinn` / `from_iroh` / `from_stream` paths are not affected)
|
||||
- [ ] `cargo test -p alknet-core` succeeds (all feature combos)
|
||||
- [ ] `cargo clippy -p alknet-core` succeeds with no warnings (all feature combos)
|
||||
- [ ] `cargo fmt --check -p alknet-core` passes
|
||||
|
||||
## References
|
||||
|
||||
- docs/architecture/decisions/070-bidistreamsource-trait.md — ADR-070 (§Constructors, §Consequences — the "constructs `Connection` from it" clause that requires this constructor)
|
||||
- docs/architecture/crates/core/core-types.md — `Connection` impl block (updated to include `from_source`), BidiStreamSource section, built-in implementations table
|
||||
|
||||
## Notes
|
||||
|
||||
> This is a gap fix, not a new feature. The ADR-070 design always intended
|
||||
> the trait to be the extension point for downstream crates; the
|
||||
> constructor is how a downstream crate *uses* that extension point. Without
|
||||
> it, the trait is unusable from outside core — the channels crate cannot
|
||||
> build a `Connection` from `ChannelBidiStreamSource`, which is the entire
|
||||
> reason ADR-070 exists. The fix is one constructor (~6 lines) + one test.
|
||||
> The risk is low: `from_source` is additive, the existing constructors
|
||||
> are untouched, and the constructor's body is the same field-init pattern
|
||||
> the other four constructors already use.
|
||||
Reference in new issue
Block a user