Skip to content

Add a byte-transfer progress primitive for uploads and downloads #217

Description

@OmarAlJarrah

Summary

The toolkit invests heavily in moving large payloads — FileRequestBody with a zero-copy FileChannel.transferTo fast path (sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/FileRequestBody.kt:103-123), MultipartBody, and the HttpRange request helper — but exposes no consumer-facing way to observe how far a transfer has progressed. There is no ProgressListener, onProgress, bytesTransferred, CountingSink, or CountingSource anywhere in sdk-core or the reference transports. A consumer that uploads a multi-GB file or streams a large download has no supported hook to drive a progress bar, compute throughput, or detect a stalled transfer.

This proposes a small, zero-dependency counting decorator (a ProgressListener plus counting Source/Sink wrappers) in the io layer, together with an explicit statement of the one place it cannot reach: the zero-copy file-upload fast path.

Problem

Progress reporting is platform surface, not application glue. The consumers of this toolkit are generated service clients, downstream SDKs, and application code that builds on top of sdk-core. Any one of them that exposes an upload or download API to its users will need incremental progress, and today each is forced to invent it out-of-band:

  • Wrap the transport-supplied BufferedSource themselves before handing it to ResponseBody.source() consumers — except the transport already owns construction of that source (sdk-transport-okhttp builds it at ResponseAdapter.kt:73), so a consumer has no clean seam to interpose.
  • Poll Response/ResponseBody.contentLength() against their own manual read loop, giving up the toolkit's buffered readers.
  • Give up on upload progress entirely, because RequestBody.writeTo(BufferedSink) is invoked by the pipeline with a transport-owned sink the consumer never sees.

Because this is a core-primitive gap, every consumer re-solves it, each slightly differently, and none can do it without reaching around the abstractions the toolkit exists to provide. A single primitive in core fixes it once for all of them.

This is distinct from the existing instrumentation layer. Meter/LongCounter/DoubleHistogram (sdk-core/src/main/kotlin/org/dexpace/sdk/core/instrumentation/metrics/) are an aggregate, per-operation observability SPI. Transfer progress is a per-call, incremental, byte-granular concern for one in-flight exchange — a different shape with a different consumer.

Proposed approach

Add to the io layer (org.dexpace.sdk.core.io), keeping the core dependency-free per NFR-1:

  1. ProgressListener — a small functional interface, e.g. onBytesTransferred(bytesTransferred: Long, totalBytes: Long), where totalBytes is the declared content length or -1 when unknown. Listeners must tolerate -1 (streamed request bodies and chunked responses have no known length per BODY-1/HTTP-36).

  2. A counting Sink decorator — wraps a BufferedSink, forwards every write untouched, and advances a running total, invoking the listener. This reuses the running-count pattern already proven in TeeSink (sdk-core/src/main/kotlin/org/dexpace/sdk/core/io/TeeSink.kt:62, the mirrored: Long counter advanced per write). Like TeeSink it must not impose its own timeout and must neither swallow nor duplicate the wrapped stream's cancellation handling (IO-40), and it is a single-threaded contract like every other streaming instance (IO-37).

  3. A counting Source decorator — symmetric: wraps a BufferedSource, forwards reads, advances the total on the bytes actually returned (honoring the -1-at-EOF read contract, IO-1), invoking the listener.

  4. Body-level convenience wrappers — helpers to wrap a RequestBody so its writeTo is instrumented (denominator = contentLength()) and a ResponseBody so its source() is counted (denominator = ResponseBody.contentLength()). For a ranged download the denominator is the ranged content length, not the full resource, so pairing with HttpRange (HTTP-49) behaves correctly. For MultipartBody the counted stream is the full framed wire body (boundaries plus parts), and the denominator is exact only when every part length is known, collapsing to -1 otherwise (BODY-2) — document this so consumers understand what the count measures.

The counting decorators are small enough (a forwarding wrapper plus a Long counter) to belong in core without pulling any runtime dependency. A richer capability — rate/throughput smoothing, ETA estimation, scheduled sampling — carries more machinery and, if it ever needed a third-party runtime, belongs in a separate opt-in adapter module per NFR-2, never in core. The base primitive stays in core; anything heavier is an add-on.

Design ceiling (must be stated in the shipped docs)

A Sink/Source-level decorator only observes bytes that flow through the generic BufferedSink/BufferedSource. This is a genuine ceiling, not an implementation gap, and it is asymmetric:

  • Downloads are fully covered. Response bytes always flow through ResponseBody.source() (sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt:61); the okhttp transport wraps the native stream into a BufferedSource (ResponseAdapter.kt:73) with no zero-copy bypass on the read path. A counting Source observes the full download.
  • Uploads are covered except the zero-copy file path. The toolkit's headline optimization is a transport type-checking FileRequestBody and dispatching FileChannel.transferTo straight to a socket channel (BODY-12; documented on FileRequestBody.kt:34-49). The okhttp transport does exactly this — is FileRequestBody -> FileRequestBodyAdapter(...) at RequestAdapter.kt:164 takes okio's zero-copy file path and never touches the generic sink. A counting Sink decorator therefore reports nothing for a FileRequestBody sent over a zero-copy transport. In-memory, string, stream-backed, and multipart request bodies (which do run through writeTo(BufferedSink)) are observed normally.

The ceiling is transport-specific: the jdkhttp transport does not special-case FileRequestBody today, so it runs the default writeTo through the generic sink (BodyPublishers.kt, body.writeTo(sdkSink)), where a decorator would see file-upload progress. Consumers must not rely on that — it is an implementation detail, and deliberately defeating the zero-copy path to gain progress would be a regression, not a feature.

Reporting upload progress across the zero-copy file path requires transport cooperation: the transport reporting transferred counts from inside its transferTo loop (FileRequestBody.writeTo already keeps such a running transferred counter with short-write detection, BODY-13). That means an SPI touchpoint, which is in tension with keeping the fast path a pure syscall. Treat it as a separate, later decision — not bundled into this primitive.

Why this is a judgement call

Given the ceiling, this is not an unambiguous win. The honest position: a pure decorator delivers correct, complete progress for all downloads and for the common upload bodies, and cannot cover the one upload case (zero-copy file) that is arguably the most likely to want a progress bar. The proposal is to ship the zero-dep decorator plus listener now — covering the cases it can, with the limitation documented at the API — and defer transport-cooperative file-upload progress as a distinct follow-up. Reviewers should weigh whether a primitive with a documented hole in its most compelling use case is worth shipping now, or whether it should wait until the transport-cooperation story is designed alongside it.

Scope and non-goals

In scope:

  • ProgressListener interface and counting Source/Sink decorators in org.dexpace.sdk.core.io, zero runtime dependencies.
  • Body-level convenience wrappers for RequestBody and ResponseBody.
  • Clear API documentation of the zero-copy upload ceiling and the -1/unknown-length behavior.

Non-goals:

  • Transport cooperation to report progress across the FileChannel.transferTo fast path (separate follow-up; requires an SPI decision).
  • Throughput/rate smoothing, ETA, or sampling schedulers (candidate for a later opt-in adapter, not core).
  • Any change that would route a FileRequestBody through the generic sink to make it observable — the zero-copy path must not be regressed.
  • Progress for SSE/event streams beyond whatever a counting Source naturally yields.

Acceptance criteria

  • A ProgressListener and counting Source/Sink decorators exist in org.dexpace.sdk.core.io with no new runtime dependency (NFR-1 preserved; apiCheck snapshots regenerated via apiDump).
  • Wrapping a ResponseBody reports monotonically increasing byte totals that reach contentLength() for a known-length body and behaves sensibly (totalBytes == -1) for an unknown-length body; reads still honor the -1-at-EOF contract (IO-1).
  • Wrapping a stream-backed / in-memory / multipart RequestBody reports totals reaching its declared contentLength() (or streams sensibly with -1), matching the bytes actually written (BODY-1).
  • The decorators add no timeout and do not alter cancellation semantics of the wrapped stream (IO-40), and do not alter the wire bytes (pass-through, in the spirit of the tee invariants IO-25/IO-29).
  • The zero-copy limitation is documented on the public API: a FileRequestBody sent over a zero-copy transport reports no upload progress via the decorator; downloads and non-file upload bodies are covered.
  • Tests using mockwebserver3 demonstrate download progress end-to-end and upload progress for a non-file body, plus a test asserting the counting Sink is bypassed for a FileRequestBody on the okhttp transport (documenting, not fixing, the ceiling).

References

Spec requirements (docs/product-spec.md):

  • BODY-12 (SHOULD) — file body streams via the most efficient transfer and is recognizable by type so transports dispatch a zero-copy kernel path. This is the requirement that creates the ceiling.
  • HTTP-40 / BODY-11, BODY-13 — file-backed body is replayable and detects a short write naming transferred-of-total (the transport-side running count that upload progress would tap).
  • HTTP-36 / BODY-1, BODY-2 — request-body single write-to-sink and content length (-1 = unknown); composite/multipart length collapses to unknown if any part is unknown.
  • HTTP-41 / BODY-14 — response body is single-use; a counting Source must wrap without introducing a second consumption.
  • IO-1, IO-17 — source read / write-all byte-count contracts the decorators must honor (the total-transferred semantics they build on).
  • IO-25IO-29 — the TeeSink contract whose running-count and pass-through pattern the counting Sink reuses.
  • IO-37, IO-38, IO-40 — single-threaded streaming contract; no imposed timeout; cancellation neither swallowed nor duplicated.
  • HTTP-49HttpRange helper; a ranged download's progress denominator is the ranged length.
  • NFR-1, NFR-2 — core stays dependency-free; any heavier capability is a separate opt-in adapter.

Related code:

  • sdk-core/src/main/kotlin/org/dexpace/sdk/core/io/TeeSink.kt (running-count pattern to reuse)
  • sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/FileRequestBody.kt (zero-copy transferTo path and its documented transport dispatch)
  • sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt (source() — the download seam)
  • sdk-transport-okhttp/src/main/kotlin/org/dexpace/sdk/transport/okhttp/internal/RequestAdapter.kt (is FileRequestBody dispatch at line 164) and ResponseAdapter.kt (source construction at line 73)

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestsdk-coresdk-core toolkit

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions