From ca87d8cb29598509ba6a976f0044ae8b79bb3844 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 28 Jun 2026 03:02:32 +0300 Subject: [PATCH] chore: simplify http/response memoization, docs, and exception constructors Three self-contained cleanups in the http/response subsystem, none of which changes runtime behavior: - ParsedResponse: replace the bespoke private Outcome sealed class with a memoized kotlin.Result. runCatching/getOrThrow preserve the exact Throwable-catching and memoize-before-throw semantics, and a nullable Result? keeps the deliberate null-success vs. unparsed distinction. - ResponseBody: rewrite the class KDoc around the real source(): BufferedSource contract (it previously described byteStream/bytes/string methods the type never declared) and drop the now-unused java.io.InputStream import. - HttpException: add one protected constructor that unpacks status/headers/body from a Response, and collapse the 18 subclasses' identical six-argument delegation blocks to a single-line delegation. Subclass public constructor surfaces are unchanged; the new protected constructor is reflected in the committed API snapshot. --- sdk-core/api/sdk-core.api | 1 + .../sdk/core/http/response/ParsedResponse.kt | 41 ++--- .../sdk/core/http/response/ResponseBody.kt | 28 ++- .../http/response/exception/HttpException.kt | 13 ++ .../http/response/exception/HttpExceptions.kt | 162 ++---------------- 5 files changed, 55 insertions(+), 190 deletions(-) diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index f33e2f7e..f551a3f3 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1435,6 +1435,7 @@ public class org/dexpace/sdk/core/http/response/exception/GoneException : org/de public abstract class org/dexpace/sdk/core/http/response/exception/HttpException : java/lang/RuntimeException, org/dexpace/sdk/core/http/response/exception/Retryable { public static final field Companion Lorg/dexpace/sdk/core/http/response/exception/HttpException$Companion; public static final field DEFAULT_SNAPSHOT_BYTES I + protected fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;)V public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;Ljava/lang/Throwable;)V diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ParsedResponse.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ParsedResponse.kt index b2097ec9..d2161d01 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ParsedResponse.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ParsedResponse.kt @@ -52,11 +52,11 @@ public class ParsedResponse internal constructor( ) : Closeable { private val lock = ReentrantLock() - // Holds the memoized outcome once the handler has run. A non-null holder means "parsed" - // (success or failure); the wrapped value distinguishes the two. A holder (rather than a + // Holds the memoized outcome once the handler has run. A non-null Result means "parsed" + // (success or failure); the wrapped value distinguishes the two. A boxed Result (rather than a // bare value) lets a legitimately-null success memoize without being mistaken for "unparsed". @Volatile - private var outcome: Outcome? = null + private var outcome: Result? = null /** The request that produced [raw]. Does not parse. */ public val request: Request get() = raw.request @@ -93,23 +93,14 @@ public class ParsedResponse internal constructor( */ @Throws(IOException::class) public fun value(): T { - outcome?.let { return it.get() } + outcome?.let { return it.getOrThrow() } return lock.withLock { - outcome?.let { return it.get() } - // Memoize the handler's outcome — success or failure — so neither re-runs the handler - // nor re-reads the (now consumed) body on a subsequent call. - val resolved: Outcome = - try { - Outcome.Success(handler.handle(raw)) - } catch (t: Throwable) { - // Catch Throwable, not Exception, on purpose: once the handler has touched the - // single-use body, re-running it would read an already-consumed stream. Even an - // Error (e.g. an OOM mid-parse) is memoized so a later call re-throws it rather - // than re-reading the body and masking the original failure. - Outcome.Failure(t) - } - outcome = resolved - resolved.get() + outcome?.let { return it.getOrThrow() } + // Memoize the outcome (success or failure) so a later call neither re-runs the handler + // nor re-reads the now-consumed body. `runCatching` catches `Throwable`, not just + // `Exception`: re-running a handler that already drained the single-use body would read + // a consumed stream, so even an `Error` (e.g. OOM mid-parse) is memoized and re-thrown. + runCatching { handler.handle(raw) }.also { outcome = it }.getOrThrow() } } @@ -124,18 +115,6 @@ public class ParsedResponse internal constructor( raw.close() } - private sealed class Outcome { - abstract fun get(): T - - class Success(private val value: T) : Outcome() { - override fun get(): T = value - } - - class Failure(private val error: Throwable) : Outcome() { - override fun get(): Nothing = throw error - } - } - public companion object { /** * Creates a [ParsedResponse] that parses [response] with [handler] on first access. diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt index e01f4244..6aa7fb0f 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/ResponseBody.kt @@ -11,34 +11,32 @@ import org.dexpace.sdk.core.http.common.MediaType import org.dexpace.sdk.core.io.BufferedSource import java.io.Closeable import java.io.IOException -import java.io.InputStream /** * Represents the body of an HTTP response. * - * A `ResponseBody` provides access to the raw bytes of an HTTP response through [byteStream], - * with convenience methods [bytes] and [string] for common consumption patterns. The body - * **must be closed** after use to release the underlying connection — prefer Kotlin's `use {}` - * or Java's try-with-resources. + * A `ResponseBody` exposes the raw bytes of an HTTP response through a single [source] accessor + * returning a [BufferedSource]. The body **must be closed** after use to release the underlying + * connection — prefer Kotlin's `use {}` or Java's try-with-resources, and close it explicitly even + * when the body is skipped without reading. * - * This class uses only `java.io` APIs with no external dependencies, making it compatible - * with JDK 8+ and safe to use from platform threads, virtual threads, Kotlin coroutines, - * and reactive schedulers. The underlying [InputStream] performs blocking I/O; callers in + * This class depends only on the SDK's [BufferedSource] I/O seam with no external dependencies, + * making it compatible with JDK 8+ and safe to use from platform threads, virtual threads, Kotlin + * coroutines, and reactive schedulers. Reading from [source] performs blocking I/O; callers in * non-blocking contexts should dispatch to an appropriate scheduler (e.g., `Dispatchers.IO`, * `Schedulers.boundedElastic()`). * * ## Thread safety * - * Instances are **not** thread-safe. The stream returned by [byteStream] should be read - * from a single thread only. For concurrent access, wrap with - * [LoggableResponseBody] which - * buffers the content and provides thread-safe, repeatable reads. + * Instances are **not** thread-safe. The [BufferedSource] returned by [source] should be read + * from a single thread only. For concurrent or repeatable access, wrap with + * [LoggableResponseBody], which buffers the content and provides thread-safe, repeatable reads. * * ## Single-use contract * - * The base `ResponseBody` can only be read once — [byteStream] returns the same stream on - * every call, and once consumed, the bytes are gone. Use [bytes] or [string] for a - * one-shot read, or wrap with `LoggableResponseBody` for repeatable access. + * The base `ResponseBody` can only be read once — [source] returns the same [BufferedSource] on + * every call, and once that source is consumed, the bytes are gone. Wrap with + * [LoggableResponseBody] for repeatable access. * * @see LoggableResponseBody for a buffered wrapper that * supports repeatable reads and non-destructive logging. diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt index 121976a1..2e090438 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt @@ -8,6 +8,7 @@ package org.dexpace.sdk.core.http.response.exception import org.dexpace.sdk.core.http.common.Headers +import org.dexpace.sdk.core.http.response.Response import org.dexpace.sdk.core.http.response.ResponseBody import org.dexpace.sdk.core.http.response.Status import org.dexpace.sdk.core.io.Buffer @@ -75,6 +76,18 @@ public abstract class HttpException cause: Throwable? = null, public val value: Any? = null, ) : RuntimeException(message ?: defaultMessage(status), cause), Retryable { + /** + * Convenience constructor that unpacks [status], [headers], and [body] from a [response]. + * The per-status subclasses expose a `(response, message?, cause?, value?)` surface and + * delegate here. `protected` so it is reachable only from subclasses. + */ + protected constructor( + response: Response, + message: String?, + cause: Throwable?, + value: Any?, + ) : this(response.status, response.headers, response.body, message, cause, value) + /** * Whether this exception represents a retryable condition. Derived from * [RetryUtils.isRetryable] over [status]'s code so it can never disagree with the diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt index 4db3194c..6c9049b3 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt @@ -36,14 +36,7 @@ public open class BadRequestException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `401 Unauthorized`. The request lacks valid authentication credentials. Not retryable @@ -57,14 +50,7 @@ public open class UnauthorizedException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `403 Forbidden`. The server understood the request but refuses to authorize it. Not @@ -78,14 +64,7 @@ public open class ForbiddenException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `404 Not Found`. The server has no current representation of the target resource. Not @@ -98,14 +77,7 @@ public open class NotFoundException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `405 Method Not Allowed`. The request method is not supported by the target resource. Not @@ -118,14 +90,7 @@ public open class MethodNotAllowedException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `408 Request Timeout`. The server timed out waiting for the request. **Retryable** — the @@ -139,14 +104,7 @@ public open class RequestTimeoutException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `409 Conflict`. The request conflicts with the current state of the target resource @@ -160,14 +118,7 @@ public open class ConflictException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `410 Gone`. The target resource is no longer available at the origin server and the @@ -180,14 +131,7 @@ public open class GoneException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `413 Payload Too Large`. The server is refusing to process the request because the @@ -200,14 +144,7 @@ public open class PayloadTooLargeException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `415 Unsupported Media Type`. The server refuses the request because the payload format @@ -220,14 +157,7 @@ public open class UnsupportedMediaTypeException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `422 Unprocessable Entity`. The server understands the content type and the syntax of the @@ -241,14 +171,7 @@ public open class UnprocessableEntityException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `429 Too Many Requests`. The user has sent too many requests in a given amount of time @@ -262,14 +185,7 @@ public open class TooManyRequestsException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `500 Internal Server Error`. Generic server-side failure with no more specific code @@ -283,14 +199,7 @@ public open class InternalServerErrorException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `502 Bad Gateway`. An intermediary received an invalid response from an upstream server. @@ -303,14 +212,7 @@ public open class BadGatewayException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `503 Service Unavailable`. The server is currently unable to handle the request due to @@ -324,14 +226,7 @@ public open class ServiceUnavailableException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * `504 Gateway Timeout`. An intermediary did not receive a timely response from an upstream @@ -344,14 +239,7 @@ public open class GatewayTimeoutException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * Fallback for 4xx statuses without a dedicated subclass (402, 406, 411, 414, 416, 417, 418, @@ -367,14 +255,7 @@ public open class ClientErrorException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value) /** * Fallback for 5xx statuses without a dedicated subclass (501, 505–511). The baked `retryable` @@ -390,11 +271,4 @@ public open class ServerErrorException message: String? = null, cause: Throwable? = null, value: Any? = null, - ) : HttpException( - status = response.status, - headers = response.headers, - body = response.body, - message = message, - cause = cause, - value = value, - ) + ) : HttpException(response, message, cause, value)