Skip to content

Commit 5f003fc

Browse files
committed
refactor: apply idiomatic-Kotlin simplifications across the SDK
Readability cleanups across sdk-core and the adapter modules, replacing "Java-in-Kotlin" patterns with their idiomatic equivalents. Every change is body-only or adds a private member: no public signature changes (apiCheck unchanged), and the full gated build — tests, detekt, ktlint, binary compatibility, the Kover floor, and the R8 shrink guard — passes. Collection operators replace hand-rolled loops and accumulators: - HttpPipelineBuilder / AsyncHttpPipelineBuilder: arrayOfNulls + index-fill + unchecked cast -> Array(size) { ordered[it] }, dropping two @Suppress("UNCHECKED_CAST") and an obsolete KDoc workaround note. - DigestChallengeHandler.pickChallenge: a five-continue accumulation loop -> challenges.mapNotNull(::toCandidate), with the per-challenge validation moved into an extracted toCandidate() that keeps one early return per gate. - DefaultRedirectStep (filter/forEach), UrlRedactor (mapTo), ResponsePipeline (fold), Annotations (filterIsInstance), LinkHeaderPaginationStrategy (sequence/mapNotNull/firstOrNull), TristateModule (forEachIndexed). Control flow and casts: - RetryStep: when (val readyState = prepareNextAttempt(...)) subject form; data object Proceed. - JdkHttpTransport: exhaustive when over the two-constant HttpVersion enum (no else, so a future constant fails to compile). - WriteAllInto: subject when (read). - RequestAdapter: capture request.body into a local so it smart-casts, dropping the cross-module !!. - AuthChallengeParser: long || punctuation chains -> private Set<Char> membership. - PageNumberPaginationStrategy: try/catch(NumberFormatException) -> raw?.toIntOrNull() ?: fallback. - LoggableResponseBody.contentLength: nested if -> a single Elvis fold. Boilerplate de-duplicated via private helpers: - MdcAwareExecutor: the capture/wrap pair repeated across seven overrides -> two MdcSnapshot.wrap helpers (capture stays at the call site). - SpanLoggingExtensions: per-key MDC restore -> restoreMdc(). - TeeSink: the tap-budget clamp arithmetic -> tapAllowance(). - Io.installProvider: hand-thrown IllegalStateException -> check(...) with a lazily built message. Idiom polish: - MediaType: manual StringBuilder -> buildString (capacity hint kept). - DispatchContext: string concatenation -> templates. - Configuration.parseDuration: Character.toUpperCase -> Char.uppercaseChar(). - RequestRebuilder: drop @Suppress("UNUSED_VARIABLE") via catch (ignored: ...). - ThrowOnHttpErrorStep: explicit return type on an anonymous mediaType() override. - ForeignSinkAdapter / ForeignSourceAdapter: return Timeout.NONE inline. - RestrictedHeaders (okhttp + jdkhttp): drop the no-op .lowercase() on already-lower-case literals.
1 parent f8585a6 commit 5f003fc

29 files changed

Lines changed: 140 additions & 142 deletions

File tree

sdk-async-virtualthreads/src/main/kotlin/org/dexpace/sdk/async/virtualthreads/MdcAwareExecutor.kt

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,32 +44,36 @@ import java.util.concurrent.TimeUnit
4444
* [VirtualThreadAsyncHttpClient] for the `close()` path so shutdown semantics are unchanged.
4545
*/
4646
internal class MdcAwareExecutor(private val delegate: ExecutorService) : ExecutorService by delegate {
47+
private fun MdcSnapshot.wrap(command: Runnable): Runnable = Runnable { withMdc { command.run() } }
48+
49+
private fun <T> MdcSnapshot.wrap(task: Callable<T>): Callable<T> = Callable { withMdc { task.call() } }
50+
4751
override fun execute(command: Runnable) {
4852
val snapshot = MdcSnapshot.capture()
49-
delegate.execute { snapshot.withMdc { command.run() } }
53+
delegate.execute(snapshot.wrap(command))
5054
}
5155

5256
override fun <T : Any?> submit(task: Callable<T>): Future<T> {
5357
val snapshot = MdcSnapshot.capture()
54-
return delegate.submit(Callable { snapshot.withMdc { task.call() } })
58+
return delegate.submit(snapshot.wrap(task))
5559
}
5660

5761
override fun submit(task: Runnable): Future<*> {
5862
val snapshot = MdcSnapshot.capture()
59-
return delegate.submit { snapshot.withMdc { task.run() } }
63+
return delegate.submit(snapshot.wrap(task))
6064
}
6165

6266
override fun <T : Any?> submit(
6367
task: Runnable,
6468
result: T,
6569
): Future<T> {
6670
val snapshot = MdcSnapshot.capture()
67-
return delegate.submit({ snapshot.withMdc { task.run() } }, result)
71+
return delegate.submit(snapshot.wrap(task), result)
6872
}
6973

7074
override fun <T : Any?> invokeAll(tasks: MutableCollection<out Callable<T>>): MutableList<Future<T>> {
7175
val snapshot = MdcSnapshot.capture()
72-
return delegate.invokeAll(tasks.map { task -> Callable { snapshot.withMdc { task.call() } } })
76+
return delegate.invokeAll(tasks.map { task -> snapshot.wrap(task) })
7377
}
7478

7579
override fun <T : Any?> invokeAll(
@@ -78,12 +82,12 @@ internal class MdcAwareExecutor(private val delegate: ExecutorService) : Executo
7882
unit: TimeUnit,
7983
): MutableList<Future<T>> {
8084
val snapshot = MdcSnapshot.capture()
81-
return delegate.invokeAll(tasks.map { task -> Callable { snapshot.withMdc { task.call() } } }, timeout, unit)
85+
return delegate.invokeAll(tasks.map { task -> snapshot.wrap(task) }, timeout, unit)
8286
}
8387

8488
override fun <T : Any?> invokeAny(tasks: MutableCollection<out Callable<T>>): T {
8589
val snapshot = MdcSnapshot.capture()
86-
return delegate.invokeAny(tasks.map { task -> Callable { snapshot.withMdc { task.call() } } })
90+
return delegate.invokeAny(tasks.map { task -> snapshot.wrap(task) })
8791
}
8892

8993
override fun <T : Any?> invokeAny(
@@ -92,6 +96,6 @@ internal class MdcAwareExecutor(private val delegate: ExecutorService) : Executo
9296
unit: TimeUnit,
9397
): T {
9498
val snapshot = MdcSnapshot.capture()
95-
return delegate.invokeAny(tasks.map { task -> Callable { snapshot.withMdc { task.call() } } }, timeout, unit)
99+
return delegate.invokeAny(tasks.map { task -> snapshot.wrap(task) }, timeout, unit)
96100
}
97101
}

sdk-core/src/main/kotlin/org/dexpace/sdk/core/config/Configuration.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public class Configuration internal constructor(
202202
// ISO-8601 path: `PT5S`, `P1D`, etc. Reject negative durations (e.g. `PT-5S`) for the
203203
// same reason the shorthand path does below — downstream consumers (Clock.sleep,
204204
// Futures.delay) assume a non-negative duration and throw on a negative one.
205-
if (Character.toUpperCase(raw[0]) == 'P') {
205+
if (raw[0].uppercaseChar() == 'P') {
206206
return try {
207207
val d = Duration.parse(raw)
208208
if (d.isNegative) null else d

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/AuthChallengeParser.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,15 +290,15 @@ public object AuthChallengeParser {
290290
}
291291
}
292292

293+
private val TOKEN_PUNCTUATION: Set<Char> = "!#$%&'*+-.^_`|~".toSet()
294+
295+
private val TOKEN68_PUNCTUATION: Set<Char> = "-._~+/".toSet()
296+
293297
/** RFC 7230 token char: ALPHA / DIGIT / one of the punctuation set. */
294298
private fun isTokenChar(c: Char): Boolean =
295-
(c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') ||
296-
c == '!' || c == '#' || c == '$' || c == '%' || c == '&' ||
297-
c == '\'' || c == '*' || c == '+' || c == '-' || c == '.' ||
298-
c == '^' || c == '_' || c == '`' || c == '|' || c == '~'
299+
(c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN_PUNCTUATION
299300

300301
/** RFC 7235 token68 char (excluding the trailing "=" pad, handled separately). */
301302
private fun isToken68Char(c: Char): Boolean =
302-
(c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') ||
303-
c == '-' || c == '.' || c == '_' || c == '~' || c == '+' || c == '/'
303+
(c in 'a'..'z') || (c in 'A'..'Z') || (c in '0'..'9') || c in TOKEN68_PUNCTUATION
304304
}

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/DigestChallengeHandler.kt

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -96,35 +96,18 @@ public class DigestChallengeHandler
9696
* - it carries a `realm` and `nonce`.
9797
*
9898
* Ordering: we scan [preferredAlgorithms] first, then within that scan the
99-
* challenges. This ensures SHA-256 wins over MD5 even when MD5 appears first
99+
* candidates. This ensures SHA-256 wins over MD5 even when MD5 appears first
100100
* in the header.
101101
*
102-
* Each `continue` skips a challenge that fails a specific validation gate
103-
* (scheme, realm, nonce, qop, algorithm). Collapsing to a single composite
104-
* predicate would obscure which validation rejected the candidate when
105-
* debugging Digest interop.
102+
* Per-challenge validation is delegated to [toCandidate]; this scan only applies
103+
* the algorithm-priority ordering, independent of the order challenges arrived in.
106104
*/
107-
@Suppress("LoopWithTooManyJumpStatements")
108105
private fun pickChallenge(
109106
challenges: List<AuthenticateChallenge>,
110107
): Pair<AuthenticateChallenge, DigestAlgorithm>? {
111108
// Find all challenges that match Digest with a satisfiable qop/realm/nonce
112109
// — we'll filter by algorithm preference below.
113-
val candidates = ArrayList<Pair<AuthenticateChallenge, DigestAlgorithm>>(challenges.size)
114-
for (challenge in challenges) {
115-
if (!challenge.scheme.equals("Digest", ignoreCase = true)) continue
116-
if (challenge.parameters["realm"] == null) continue
117-
if (challenge.parameters["nonce"] == null) continue
118-
if (!qopSupportsAuth(challenge.parameters["qop"])) continue
119-
val algorithmName = challenge.parameters["algorithm"]
120-
val algorithm =
121-
if (algorithmName == null) {
122-
DigestAlgorithm.MD5 // RFC 7616 §3.3: MD5 is the default when omitted.
123-
} else {
124-
DigestAlgorithm.fromString(algorithmName) ?: continue
125-
}
126-
candidates.add(challenge to algorithm)
127-
}
110+
val candidates = challenges.mapNotNull(::toCandidate)
128111
// Pick the candidate whose algorithm appears earliest in our preference list.
129112
// Returns null when no candidate matches any preferred algorithm.
130113
for (preferred in preferredAlgorithms) {
@@ -133,6 +116,30 @@ public class DigestChallengeHandler
133116
return null
134117
}
135118

119+
/**
120+
* Maps a single challenge to a satisfiable (challenge, algorithm) candidate, or
121+
* null when it fails a validation gate. Each gate returns separately rather than
122+
* collapsing into one composite predicate — keeping them distinct preserves which
123+
* validation rejected a candidate when debugging Digest interop.
124+
*/
125+
@Suppress("ReturnCount") // one early return per validation gate: scheme, realm, nonce, qop, algorithm
126+
private fun toCandidate(
127+
challenge: AuthenticateChallenge,
128+
): Pair<AuthenticateChallenge, DigestAlgorithm>? {
129+
if (!challenge.scheme.equals("Digest", ignoreCase = true)) return null
130+
if (challenge.parameters["realm"] == null) return null
131+
if (challenge.parameters["nonce"] == null) return null
132+
if (!qopSupportsAuth(challenge.parameters["qop"])) return null
133+
val algorithmName = challenge.parameters["algorithm"]
134+
val algorithm =
135+
if (algorithmName == null) {
136+
DigestAlgorithm.MD5 // RFC 7616 §3.3: MD5 is the default when omitted.
137+
} else {
138+
DigestAlgorithm.fromString(algorithmName) ?: return null
139+
}
140+
return challenge to algorithm
141+
}
142+
136143
/**
137144
* Builds the `Authorization: Digest ...` header value for a single challenge.
138145
* Follows RFC 7616 §3.4.6 — HA1, HA2, response — plus the standard parameter

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,16 @@ public data class MediaType private constructor(
100100
if (value.isNotEmpty() && value.all(::isTokenChar)) {
101101
return value
102102
}
103-
val sb = StringBuilder(value.length + 2)
104-
sb.append('"')
105-
value.forEach { ch ->
106-
if (ch == '\\' || ch == '"') {
107-
sb.append('\\')
103+
return buildString(value.length + 2) {
104+
append('"')
105+
value.forEach { ch ->
106+
if (ch == '\\' || ch == '"') {
107+
append('\\')
108+
}
109+
append(ch)
108110
}
109-
sb.append(ch)
111+
append('"')
110112
}
111-
sb.append('"')
112-
return sb.toString()
113113
}
114114

115115
/**

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/context/DispatchContext.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public data class DispatchContext(
6161
* counter to it for the actual key.
6262
*/
6363
private fun deriveCallKey(instrumentationContext: InstrumentationContext): String =
64-
instrumentationContext.traceId.value + ":" + instrumentationContext.spanId.value
64+
"${instrumentationContext.traceId.value}:${instrumentationContext.spanId.value}"
6565

6666
/**
6767
* A dispatch context with a no-op instrumentation context; used when tracing is
@@ -81,6 +81,6 @@ public data class DispatchContext(
8181
* [DispatchContext]), so every link in the chain is collision-safe by default.
8282
*/
8383
internal fun mintCallKey(instrumentationContext: InstrumentationContext): String =
84-
deriveCallKey(instrumentationContext) + ":" + mintCounter.incrementAndGet()
84+
"${deriveCallKey(instrumentationContext)}:${mintCounter.incrementAndGet()}"
8585
}
8686
}

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/AsyncHttpPipelineBuilder.kt

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ public class AsyncHttpPipelineBuilder(private val httpClient: AsyncHttpClient) {
9999
/** Builds an immutable [AsyncHttpPipeline]. */
100100
public fun build(): AsyncHttpPipeline {
101101
val ordered = steps.flatten()
102-
val array = arrayOfNulls<AsyncHttpStep>(ordered.size)
103-
for ((i, s) in ordered.withIndex()) array[i] = s
104-
@Suppress("UNCHECKED_CAST")
105-
return AsyncHttpPipeline(httpClient, array as Array<AsyncHttpStep>)
102+
return AsyncHttpPipeline(httpClient, Array(ordered.size) { ordered[it] })
106103
}
107104

108105
public companion object {

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/HttpPipelineBuilder.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,10 @@ public class HttpPipelineBuilder(private val httpClient: HttpClient) {
110110
/**
111111
* Builds an immutable [HttpPipeline] in stage order. [Stage.SEND] is reserved for the
112112
* transport and is skipped.
113-
*
114-
* `arrayOfNulls<HttpStep>` then fill — Kotlin's `List.toTypedArray<T>()` is erased to
115-
* `Array<Any?>` at runtime which fails the `Array<HttpStep>` cast.
116113
*/
117114
public fun build(): HttpPipeline {
118115
val ordered = steps.flatten()
119-
val array = arrayOfNulls<HttpStep>(ordered.size)
120-
for ((i, s) in ordered.withIndex()) array[i] = s
121-
@Suppress("UNCHECKED_CAST")
122-
return HttpPipeline(httpClient, array as Array<HttpStep>)
116+
return HttpPipeline(httpClient, Array(ordered.size) { ordered[it] })
123117
}
124118

125119
public companion object {

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/DefaultRedirectStep.kt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,9 @@ public open class DefaultRedirectStep
258258
// underlying store may return names in mixed case (`Content-Type`), so lower-case
259259
// before the prefix test. Iterate a snapshot of the keys to avoid concurrent
260260
// modification while mutating the builder.
261-
val toRemove = ArrayList<String>()
262-
for (name in headers.names()) {
263-
if (name.lowercase(Locale.US).startsWith("content-")) toRemove.add(name)
264-
}
265-
for (name in toRemove) builder.remove(name)
261+
headers.names()
262+
.filter { it.lowercase(Locale.US).startsWith("content-") }
263+
.forEach { builder.remove(it) }
266264
return builder.build()
267265
}
268266

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/LoggableResponseBody.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ public class LoggableResponseBody
122122
* otherwise the delegate's reported length (the true length), since the capture is just
123123
* a bounded prefix.
124124
*/
125-
override fun contentLength(): Long =
126-
if (fullyCaptured) captured?.size ?: delegate.contentLength() else delegate.contentLength()
125+
override fun contentLength(): Long = (if (fullyCaptured) captured?.size else null) ?: delegate.contentLength()
127126

128127
/**
129128
* Returns a view of the captured body. Drains (up to the cap) on first call. If the drain

0 commit comments

Comments
 (0)