Related: #113 tracks the JDK transport buffering non-replayable bodies through the same toReplayable path — a separate behavior fix, not part of these cleanups.
While reading through http/request I noticed a small cluster of declarations that
restate behavior they already inherit. The bodies that report isReplayable() == true
each override toReplayable to return this, which is exactly what the base class already
does for any replayable body; and Method threads a wire-token string through its
constructor that is always identical to the enum constant's own name. None of this is
load-bearing — it's duplication that makes the types look like they're doing more than they
are. The three cleanups below are behavior-preserving.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/RequestBody.kt:228, 247, 277 — Drop the no-op toReplayable overrides on the replayable bodies
The base RequestBody.toReplayable short-circuits to this whenever isReplayable() is
true. The three private replayable bodies all return true from isReplayable() and then
re-declare an override that returns this — the exact behavior they'd inherit.
Old code:
// Base RequestBody.toReplayable already returns `this` for any replayable body (UNCHANGED — shown for context):
@JvmOverloads
public open fun toReplayable(provider: IoProvider = Io.provider): RequestBody {
if (isReplayable()) return this
val buffer = provider.buffer()
writeTo(buffer)
return BufferRequestBody(buffer, mediaType(), buffer.size)
}
// ...
/** Replayable body backed by an in-memory [Buffer]. Reads via non-consuming `peek()`. */
private class BufferRequestBody(
private val buffer: Buffer,
private val mediaType: MediaType?,
private val length: Long,
) : RequestBody() {
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = length
override fun isReplayable(): Boolean = true
override fun toReplayable(provider: IoProvider): RequestBody = this
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
sink.writeAll(buffer.peek())
}
}
/** Replayable body backed by a byte array. */
private class ByteArrayRequestBody(
private val bytes: ByteArray,
private val mediaType: MediaType?,
) : RequestBody() {
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = bytes.size.toLong()
override fun isReplayable(): Boolean = true
override fun toReplayable(provider: IoProvider): RequestBody = this
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
sink.write(bytes)
}
}
/**
* Replayable body backed by an [InputStream] that supports `mark/reset`. The stream is
* marked at construction time; each [writeTo] after the first calls `reset()` to rewind
* before reading.
*
* The `firstWrite` toggle is an [AtomicBoolean] so concurrent `writeTo` calls cannot both
* skip (or both perform) the `reset()` — at most one transitions the flag, ensuring exactly
* one reset between any two writes.
*/
private class ResettableInputStreamRequestBody(
private val input: InputStream,
private val length: Long,
private val mediaType: MediaType?,
) : RequestBody() {
private val hasWritten = AtomicBoolean(false)
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = length
override fun isReplayable(): Boolean = true
override fun toReplayable(provider: IoProvider): RequestBody = this
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
if (hasWritten.getAndSet(true)) {
input.reset()
}
copyExactly(input, sink, length)
}
}
New code:
/** Replayable body backed by an in-memory [Buffer]. Reads via non-consuming `peek()`. */
private class BufferRequestBody(
private val buffer: Buffer,
private val mediaType: MediaType?,
private val length: Long,
) : RequestBody() {
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = length
override fun isReplayable(): Boolean = true
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
sink.writeAll(buffer.peek())
}
}
/** Replayable body backed by a byte array. */
private class ByteArrayRequestBody(
private val bytes: ByteArray,
private val mediaType: MediaType?,
) : RequestBody() {
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = bytes.size.toLong()
override fun isReplayable(): Boolean = true
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
sink.write(bytes)
}
}
/**
* Replayable body backed by an [InputStream] that supports `mark/reset`. The stream is
* marked at construction time; each [writeTo] after the first calls `reset()` to rewind
* before reading.
*
* The `firstWrite` toggle is an [AtomicBoolean] so concurrent `writeTo` calls cannot both
* skip (or both perform) the `reset()` — at most one transitions the flag, ensuring exactly
* one reset between any two writes.
*/
private class ResettableInputStreamRequestBody(
private val input: InputStream,
private val length: Long,
private val mediaType: MediaType?,
) : RequestBody() {
private val hasWritten = AtomicBoolean(false)
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = length
override fun isReplayable(): Boolean = true
@Throws(IOException::class)
override fun writeTo(sink: BufferedSink) {
if (hasWritten.getAndSet(true)) {
input.reset()
}
copyExactly(input, sink, length)
}
}
Usage — before → after:
// `body` is one of the three replayable bodies; isReplayable() == true
val replayable = body.toReplayable() // base short-circuit returns `this`
val replayable = body.toReplayable() // call sites unchanged — behavior-preserving
Why: Each class already returns true from isReplayable(), so the inherited
toReplayable hits its if (isReplayable()) return this short-circuit and hands back the
same instance. The overrides only restate that, so removing them changes nothing.
API / Build: All three classes are private and never appear in the .api snapshot, so
no apiDump is needed. The Io / IoProvider imports stay referenced by the base
toReplayable, so nothing becomes an unused import.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/FileRequestBody.kt:103 — Drop the no-op toReplayable override and its now-unused import
Same story as above, but this body is public. The base returns this for a replayable body,
and this is still the FileRequestBody, so the transport-side is FileRequestBody
zero-copy / sendfile(2) dispatch keeps matching. Removing the override leaves
import org.dexpace.sdk.core.io.IoProvider referenced nowhere else in the file.
Old code:
import org.dexpace.sdk.core.http.common.MediaType
import org.dexpace.sdk.core.io.BufferedSink
import org.dexpace.sdk.core.io.IoProvider
import java.io.IOException
import java.nio.ByteBuffer
import java.nio.channels.Channels
import java.nio.channels.FileChannel
import java.nio.file.Files
import java.nio.file.NoSuchFileException
import java.nio.file.Path
import java.nio.file.StandardOpenOption
// ...
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = count
override fun isReplayable(): Boolean = true
override fun toReplayable(provider: IoProvider): RequestBody = this
New code:
import org.dexpace.sdk.core.http.common.MediaType
import org.dexpace.sdk.core.io.BufferedSink
import java.io.IOException
import java.nio.ByteBuffer
import java.nio.channels.Channels
import java.nio.channels.FileChannel
import java.nio.file.Files
import java.nio.file.NoSuchFileException
import java.nio.file.Path
import java.nio.file.StandardOpenOption
// ...
override fun mediaType(): MediaType? = mediaType
override fun contentLength(): Long = count
override fun isReplayable(): Boolean = true
Usage — before → after:
val replayable = body.toReplayable()
check(replayable is FileRequestBody) // true — base toReplayable() returns `this`
val replayable = body.toReplayable()
check(replayable is FileRequestBody) // call sites unchanged — behavior-preserving
Why: FileRequestBody.isReplayable() is true, so the inherited toReplayable returns
this — the same FileRequestBody — which is exactly what the override does. Transports that
type-check is FileRequestBody for the sendfile(2) fast path keep matching, so the override
is dead weight.
API: FileRequestBody is public, so the override is in the snapshot
(sdk-core/api/sdk-core.api:
public fun toReplayable (Lorg/dexpace/sdk/core/io/IoProvider;)Lorg/dexpace/sdk/core/http/request/RequestBody;).
Removing it drops that line — run ./gradlew apiDump and commit the regenerated
sdk-core/api/sdk-core.api. The inherited RequestBody.toReplayable stays public and callable.
Build: with the override gone, import org.dexpace.sdk.core.io.IoProvider is referenced
nowhere in the file; ktlint's no-unused-imports (and allWarningsAsErrors) fails the build
unless it is deleted in the same change.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Method.kt:29-44 — Derive the wire token from the enum name instead of a duplicated constructor arg
Every constant passes a string literal that is identical to its own enum name
(GET("GET", …), POST("POST", …), …), and toString() just returns that string — which is
also what the default enum toString() already does. The token can be a computed property over
name, dropping the constructor parameter, the nine duplicated literals, and the toString()
override.
Old code:
/**
* HTTP request methods recognized by the SDK. Each constant carries the canonical token used
* on the wire; [toString] returns that same token so the enum can be written directly into a
* request line without translation.
*
* @property method Canonical uppercase method token sent in the request line.
* @property permitsRequestBody Whether this SDK permits the method to carry a request body.
* `false` for `GET`, `HEAD`, `TRACE`, and `CONNECT`. Of these only `TRACE` is forbidden a body
* outright by HTTP — a TRACE request "MUST NOT send content" (RFC 9110 §9.3.8); for `GET`/`HEAD`
* (§9.3.1/§9.3.2) and `CONNECT` (§9.3.6) a request payload has no generally defined semantics
* and is discouraged rather than illegal. The SDK rejects a body on all four as one consistent
* policy, because the reference transports otherwise diverge on the case — OkHttp throws
* `IllegalArgumentException`, the JDK builder silently ignores the body — so it is rejected once
* at request construction (see `Request.RequestBuilder.build`) rather than left to behave
* differently per transport.
*/
@Suppress("unused")
public enum class Method(
public val method: String,
public val permitsRequestBody: Boolean,
) {
GET("GET", permitsRequestBody = false),
POST("POST", permitsRequestBody = true),
PUT("PUT", permitsRequestBody = true),
DELETE("DELETE", permitsRequestBody = true),
PATCH("PATCH", permitsRequestBody = true),
HEAD("HEAD", permitsRequestBody = false),
OPTIONS("OPTIONS", permitsRequestBody = true),
TRACE("TRACE", permitsRequestBody = false),
CONNECT("CONNECT", permitsRequestBody = false),
;
override fun toString(): String = method
}
New code:
/**
* HTTP request methods recognized by the SDK. Each constant's name is the canonical token used
* on the wire, so the enum's default `toString()` returns that token and the constant can be
* written directly into a request line without translation.
*
* @property permitsRequestBody Whether this SDK permits the method to carry a request body.
* `false` for `GET`, `HEAD`, `TRACE`, and `CONNECT`. Of these only `TRACE` is forbidden a body
* outright by HTTP — a TRACE request "MUST NOT send content" (RFC 9110 §9.3.8); for `GET`/`HEAD`
* (§9.3.1/§9.3.2) and `CONNECT` (§9.3.6) a request payload has no generally defined semantics
* and is discouraged rather than illegal. The SDK rejects a body on all four as one consistent
* policy, because the reference transports otherwise diverge on the case — OkHttp throws
* `IllegalArgumentException`, the JDK builder silently ignores the body — so it is rejected once
* at request construction (see `Request.RequestBuilder.build`) rather than left to behave
* differently per transport.
*/
@Suppress("unused")
public enum class Method(public val permitsRequestBody: Boolean) {
GET(permitsRequestBody = false),
POST(permitsRequestBody = true),
PUT(permitsRequestBody = true),
DELETE(permitsRequestBody = true),
PATCH(permitsRequestBody = true),
HEAD(permitsRequestBody = false),
OPTIONS(permitsRequestBody = true),
TRACE(permitsRequestBody = false),
CONNECT(permitsRequestBody = false),
;
/** Canonical uppercase method token sent in the request line; identical to the enum [name]. */
public val method: String get() = name
}
Usage — before → after:
val token = request.method.method // "POST"
val rendered = Method.POST.toString() // "POST"
val token = request.method.method // "POST" — `method` now returns `name`
val rendered = Method.POST.toString() // "POST" — default enum toString() returns `name`
Why: The token literal in every constant is just the constant's own name, and the
overridden toString() returns the same value the default enum toString() already produces.
Computing method from name removes nine duplicated literals plus the override while keeping
Method.POST.method and Method.POST.toString() equal to "POST".
API: getMethod() is unchanged (the property is still public, now computed), but the
toString() override leaves the snapshot — sdk-core/api/sdk-core.api currently lists
public fun toString ()Ljava/lang/String; under Method. Run ./gradlew apiDump and commit
the regenerated sdk-core/api/sdk-core.api. (The enum constructor is implicitly private and
not part of the snapshot, so dropping the parameter is not itself an API change.)
Related: #113 tracks the JDK transport buffering non-replayable bodies through the same
toReplayablepath — a separate behavior fix, not part of these cleanups.While reading through
http/requestI noticed a small cluster of declarations thatrestate behavior they already inherit. The bodies that report
isReplayable() == trueeach override
toReplayabletoreturn this, which is exactly what the base class alreadydoes for any replayable body; and
Methodthreads a wire-token string through itsconstructor that is always identical to the enum constant's own
name. None of this isload-bearing — it's duplication that makes the types look like they're doing more than they
are. The three cleanups below are behavior-preserving.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/RequestBody.kt:228, 247, 277 — Drop the no-op
toReplayableoverrides on the replayable bodiesThe base
RequestBody.toReplayableshort-circuits tothiswheneverisReplayable()istrue. The three private replayable bodies all return
truefromisReplayable()and thenre-declare an override that returns
this— the exact behavior they'd inherit.Old code:
New code:
Usage — before → after:
Why: Each class already returns
truefromisReplayable(), so the inheritedtoReplayablehits itsif (isReplayable()) return thisshort-circuit and hands back thesame instance. The overrides only restate that, so removing them changes nothing.
API / Build: All three classes are
privateand never appear in the.apisnapshot, sono
apiDumpis needed. TheIo/IoProviderimports stay referenced by the basetoReplayable, so nothing becomes an unused import.sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/FileRequestBody.kt:103 — Drop the no-op
toReplayableoverride and its now-unused importSame story as above, but this body is public. The base returns
thisfor a replayable body,and
thisis still theFileRequestBody, so the transport-sideis FileRequestBodyzero-copy /
sendfile(2)dispatch keeps matching. Removing the override leavesimport org.dexpace.sdk.core.io.IoProviderreferenced nowhere else in the file.Old code:
New code:
Usage — before → after:
Why:
FileRequestBody.isReplayable()istrue, so the inheritedtoReplayablereturnsthis— the sameFileRequestBody— which is exactly what the override does. Transports thattype-check
is FileRequestBodyfor thesendfile(2)fast path keep matching, so the overrideis dead weight.
API:
FileRequestBodyis public, so the override is in the snapshot(
sdk-core/api/sdk-core.api:public fun toReplayable (Lorg/dexpace/sdk/core/io/IoProvider;)Lorg/dexpace/sdk/core/http/request/RequestBody;).Removing it drops that line — run
./gradlew apiDumpand commit the regeneratedsdk-core/api/sdk-core.api. The inheritedRequestBody.toReplayablestays public and callable.Build: with the override gone,
import org.dexpace.sdk.core.io.IoProvideris referencednowhere in the file; ktlint's
no-unused-imports(andallWarningsAsErrors) fails the buildunless it is deleted in the same change.
sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/request/Method.kt:29-44 — Derive the wire token from the enum name instead of a duplicated constructor arg
Every constant passes a string literal that is identical to its own enum
name(
GET("GET", …),POST("POST", …), …), andtoString()just returns that string — which isalso what the default enum
toString()already does. The token can be a computed property overname, dropping the constructor parameter, the nine duplicated literals, and thetoString()override.
Old code:
New code:
Usage — before → after:
Why: The token literal in every constant is just the constant's own
name, and theoverridden
toString()returns the same value the default enumtoString()already produces.Computing
methodfromnameremoves nine duplicated literals plus the override while keepingMethod.POST.methodandMethod.POST.toString()equal to"POST".API:
getMethod()is unchanged (the property is still public, now computed), but thetoString()override leaves the snapshot —sdk-core/api/sdk-core.apicurrently listspublic fun toString ()Ljava/lang/String;underMethod. Run./gradlew apiDumpand committhe regenerated
sdk-core/api/sdk-core.api. (The enum constructor is implicitly private andnot part of the snapshot, so dropping the parameter is not itself an API change.)