Skip to content

chore: simplify okhttp and jdkhttp transport adapters#183

Merged
OmarAlJarrah merged 1 commit into
mainfrom
chore/simplify-transport-adapters
Jun 25, 2026
Merged

chore: simplify okhttp and jdkhttp transport adapters#183
OmarAlJarrah merged 1 commit into
mainfrom
chore/simplify-transport-adapters

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

A handful of small, behaviour-preserving cleanups in the two transport adapters (sdk-transport-okhttp, sdk-transport-jdkhttp). None change anything on the wire; they remove duplication, redundant arguments, single-use indirection, and stale doc wording.

Changes

okhttp

  • Shared logger instance. The fully-qualified logger category was written out twice in OkHttpTransport.kt, each constructing its own ClientLogger (once for the transport, once for the Builder). Hoisted a single private val log into the companion; both call sites now read it. ClientLogger wraps a thread-safe SLF4J Logger with no caller-specific state, so one instance is safe to share and removes the redundant allocation.
  • Redundant empty-body defaults. EMPTY_BODY passed toRequestBody(null, 0, 0), which exactly restates the extension's own defaults on a zero-length array; simplified to toRequestBody() for a byte-identical result.
  • Stale KDoc. NonProxyHostSelector is a private nested class, not a "top-level private inner class" — corrected the wording.

jdkhttp

  • Single create() factory. The two create overloads differed only in whether they passed responseTimeout; the one-arg form was the two-arg form with null. Folded into one @JvmStatic @JvmOverloads method with a default parameter. Both call arities still resolve for Java and Kotlin.
  • KillSwitchInputStream extends FilterInputStream. The hand-rolled read / read(b, off, len) / available forwarders only delegated to the wrapped PipedInputStream — exactly what FilterInputStream provides. Kept the custom close() (which deliberately does not call super.close(), since teardown must also cancel the writer and close the write end).
  • Inlined bufferToByteArray. Single-use helper folded into its only caller, eagerPublisher.
  • Applied the same NonProxyHostSelector KDoc correction.

API / compatibility

The @JvmOverloads default on JdkHttpTransport.create is source- and binary-compatible for existing callers — both public create arities remain. The default parameter additionally emits a synthetic create$default(...), which this repo's binary-compatibility validator includes in the snapshot; the regenerated sdk-transport-jdkhttp.api is committed alongside. All other changes touch private members only, so apiCheck is otherwise unaffected.

Verification

./gradlew :sdk-transport-okhttp:build :sdk-transport-jdkhttp:build passes — tests, ktlint, detekt, apiCheck, and the coverage gate all green.

Closes #182.

Mechanical, behaviour-preserving cleanups across the two transport
adapters. None of these change anything on the wire.

okhttp:
- Hoist a single shared ClientLogger into the OkHttpTransport companion
  and have both the transport instance and the Builder read it, instead
  of each constructing its own logger for the identical category.
- Drop the redundant default arguments on the shared empty RequestBody
  (toRequestBody(null, 0, 0) -> toRequestBody()); on a zero-length array
  these restate the extension's own defaults.
- Correct the NonProxyHostSelector KDoc: it is a private nested class,
  not a "top-level private inner class".

jdkhttp:
- Fold the two create() factories into one @jvmoverloads method with a
  default responseTimeout; the one-arg form was the two-arg form with
  null. Both call arities stay for Java and Kotlin; the synthetic
  create$default is reflected in the committed .api snapshot.
- Have KillSwitchInputStream extend FilterInputStream so the read /
  available delegation comes for free, keeping only the custom close()
  that cancels the writer and tears down both pipe ends.
- Inline the single-use bufferToByteArray into eagerPublisher.
- Apply the same NonProxyHostSelector KDoc correction.
@OmarAlJarrah OmarAlJarrah merged commit c291ac0 into main Jun 25, 2026
1 check passed
@OmarAlJarrah OmarAlJarrah deleted the chore/simplify-transport-adapters branch June 25, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transport (okhttp + jdkhttp): simplify the transport adapters

1 participant