fix: Unit 1 — make publish() work end-to-end (P-01, P-02, P-05, P-08, P-09, P-12 #1)
Fixes the Pub operation's client→responder path so a publish() actually works on any transport. Previously the wire was broken in three compounding ways and the unit tests couldn't see it. P-01 [critical] — publish chunks were written to a *fresh* open_bi stream (stream B) while the responder read chunks from the stream that carried call.requested (stream A). Stream B's frames were silently discarded by the dispatch loop's "ignoring non-requested event" branch. On single-stream transports (TCP+TLS, SSH) the second open_bi returns StreamClosed outright, so every publish failed. Fix: pump call.requested + call.published + call.completed on the *same* write half via the new pump_publish_to_wire free function; the read half is pumped concurrently for the single call.responded. P-02 [major] — publish() / publish_with_payload() now take Pin<Box<dyn Stream<Item = Value> + Send>> per ADR-046 §8, not Vec<Value>. The Vec shape buffered the entire publish in memory and made the ADR's flagship use cases (telemetry ingest, live upload, drag-drop file streaming) impossible. The wire format is unchanged; this corrects API drift (no deployments exist). P-05 [major] — publish now registers with timeout: None (like subscribe), not DEFAULT_CALL_TIMEOUT (30s). A publish whose stream took >30s wall-clock got a spurious client-side TIMEOUT while the responder kept consuming. PendingEntry::Call.timeout is now Option<Instant> so the sweeper never evicts unbounded calls; all register_call callers updated (Some(...) for call(), None for publish()). P-08 [major] — the dispatch Pub branch re-implemented invoke_sink's not-found / visibility / ACL / handler-kind checks inline; invoke_sink was only ever called from its own tests. Any future fix in invoke_sink (e.g. the missing publish_schema validation, P-03) wouldn't reach the wire path. Fix: extract OperationRegistry::resolve_sink_handler (pub(crate)) as the single source of truth for the sink dispatch checks; both invoke_sink and Dispatcher::dispatch (Pub branch) call it. The two paths can no longer diverge. P-09 [major, side-effect] — make_sink_forwarding_handler in from_call.rs was store-and-forward (collect into Vec) and silently discarded Err items (filter_map(|item| item.ok())), contradicting the SinkHandler contract that an Err terminates the stream. Now that publish_with_payload takes a Stream, the forwarding handler passes the stream through directly (streamed, not buffered) and uses take_while(Ok) + filter_map to terminate on Err. P-09 was listed in Unit 6 but depends on P-01/P-02, so it falls out naturally here. P-12 #1 [acceptance gate] — adds the end-to-end publish test that the review identifies as the gate for this unit: publish_end_to_end_delivers_chunks_and_returns_response wires CallConnection::publish ↔ Dispatcher::run_loop over a real tokio::io::duplex pair (new duplex_connection_pair test helper + SingleStreamSource BidiStreamSource impl), publishes 3 chunks, and asserts the responder saw all 3 and returned the right result. A second test covers the unknown-op → NOT_FOUND path. These tests would have caught P-01 immediately. Verification: - cargo test --lib → 434 passed, 0 failed (was 432; +2 e2e tests) - cargo clippy --all-targets -- -D warnings → clean - cargo fmt --check → clean - cargo doc --no-deps → 4 warnings (pre-existing C-09, unchanged)
This commit is contained in:
1 parent
f1d6796436
commit
502488cc72
9 files changed
+400
-188
No files matched your search
+12
-64
@@ -35,10 +35,8 @@ use super::wire::{
|
||||
use crate::protocol::adapter::SessionOverlaySource;
|
||||
use crate::registry::context::{AbortPolicy, OperationContext, ScopedPeerEnv};
|
||||
use crate::registry::env::{LocalOperationEnv, OperationEnv, PeerCompositeEnv};
|
||||
use crate::registry::registration::{
|
||||
extract_json_pointer, HandlerKind, OperationRegistry, PublishStream, ResponseStream,
|
||||
};
|
||||
use crate::registry::spec::{AccessResult, OperationType, Visibility};
|
||||
use crate::registry::registration::{OperationRegistry, PublishStream, ResponseStream};
|
||||
use crate::registry::spec::OperationType;
|
||||
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const SWEEPER_INTERVAL: Duration = Duration::from_secs(10);
|
||||
@@ -308,64 +306,14 @@ impl Dispatcher {
|
||||
}
|
||||
OperationType::Pub => {
|
||||
context.deadline = None;
|
||||
let registration = match self.registry.registration(&operation_name) {
|
||||
Some(r) => r,
|
||||
None => {
|
||||
return DispatchResult::Once(ResponseEnvelope::not_found(
|
||||
request_id.clone(),
|
||||
&operation_name,
|
||||
));
|
||||
}
|
||||
};
|
||||
if registration.spec.visibility == Visibility::Internal && !context.internal {
|
||||
return DispatchResult::Once(ResponseEnvelope::not_found(
|
||||
request_id.clone(),
|
||||
&operation_name,
|
||||
));
|
||||
}
|
||||
let acl = ®istration.spec.access_control;
|
||||
let identity = if context.internal {
|
||||
context
|
||||
.handler_identity
|
||||
.as_ref()
|
||||
.and_then(|ca| ca.as_identity())
|
||||
} else {
|
||||
context.identity.clone()
|
||||
};
|
||||
let resource_id = registration
|
||||
.spec
|
||||
.resource_id_path
|
||||
.as_ref()
|
||||
.and_then(|path| extract_json_pointer(&input, path));
|
||||
if let AccessResult::Forbidden(message) = acl.check(
|
||||
identity.as_ref(),
|
||||
resource_id.as_deref(),
|
||||
context.ownership.as_deref(),
|
||||
) {
|
||||
return DispatchResult::Once(ResponseEnvelope::forbidden(
|
||||
request_id.clone(),
|
||||
message,
|
||||
));
|
||||
}
|
||||
let sink_handler = match ®istration.handler {
|
||||
HandlerKind::Sink(h) => Arc::clone(h),
|
||||
HandlerKind::Once(_) => {
|
||||
return DispatchResult::Once(ResponseEnvelope::error(
|
||||
request_id.clone(),
|
||||
CallError::invalid_operation_type(
|
||||
"invoke_sink() called on a Query/Mutation op; use invoke()",
|
||||
),
|
||||
));
|
||||
}
|
||||
HandlerKind::Stream(_) => {
|
||||
return DispatchResult::Once(ResponseEnvelope::error(
|
||||
request_id.clone(),
|
||||
CallError::invalid_operation_type(
|
||||
"invoke_sink() called on a Sub op; use invoke_streaming()",
|
||||
),
|
||||
));
|
||||
}
|
||||
};
|
||||
let sink_handler =
|
||||
match self
|
||||
.registry
|
||||
.resolve_sink_handler(&operation_name, &input, &context)
|
||||
{
|
||||
Ok(h) => h,
|
||||
Err(envelope) => return DispatchResult::Once(envelope),
|
||||
};
|
||||
let (chunk_tx, chunk_rx) =
|
||||
mpsc::channel::<Result<Value, CallError>>(PUBLISH_CHANNEL_BUFFER);
|
||||
let publish_stream: PublishStream = Box::pin(chunk_rx);
|
||||
@@ -1111,12 +1059,12 @@ mod tests {
|
||||
let mut pending = conn.pending().lock();
|
||||
pending.register_call(
|
||||
parent_id.clone(),
|
||||
Instant::now() + Duration::from_secs(30),
|
||||
Some(Instant::now() + Duration::from_secs(30)),
|
||||
None,
|
||||
);
|
||||
pending.register_call(
|
||||
child_id.clone(),
|
||||
Instant::now() + Duration::from_secs(30),
|
||||
Some(Instant::now() + Duration::from_secs(30)),
|
||||
Some(parent_id.clone()),
|
||||
);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user