chore: simplify okhttp and jdkhttp transport adapters#183
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
OkHttpTransport.kt, each constructing its ownClientLogger(once for the transport, once for theBuilder). Hoisted a singleprivate val loginto the companion; both call sites now read it.ClientLoggerwraps a thread-safe SLF4JLoggerwith no caller-specific state, so one instance is safe to share and removes the redundant allocation.EMPTY_BODYpassedtoRequestBody(null, 0, 0), which exactly restates the extension's own defaults on a zero-length array; simplified totoRequestBody()for a byte-identical result.NonProxyHostSelectoris a private nested class, not a "top-level private inner class" — corrected the wording.jdkhttp
create()factory. The twocreateoverloads differed only in whether they passedresponseTimeout; the one-arg form was the two-arg form withnull. Folded into one@JvmStatic @JvmOverloadsmethod with a default parameter. Both call arities still resolve for Java and Kotlin.KillSwitchInputStreamextendsFilterInputStream. The hand-rolledread/read(b, off, len)/availableforwarders only delegated to the wrappedPipedInputStream— exactly whatFilterInputStreamprovides. Kept the customclose()(which deliberately does not callsuper.close(), since teardown must also cancel the writer and close the write end).bufferToByteArray. Single-use helper folded into its only caller,eagerPublisher.NonProxyHostSelectorKDoc correction.API / compatibility
The
@JvmOverloadsdefault onJdkHttpTransport.createis source- and binary-compatible for existing callers — both publiccreatearities remain. The default parameter additionally emits a syntheticcreate$default(...), which this repo's binary-compatibility validator includes in the snapshot; the regeneratedsdk-transport-jdkhttp.apiis committed alongside. All other changes touch private members only, soapiCheckis otherwise unaffected.Verification
./gradlew :sdk-transport-okhttp:build :sdk-transport-jdkhttp:buildpasses — tests, ktlint, detekt,apiCheck, and the coverage gate all green.Closes #182.