From a8094c2c65f7fee3da894079edd7212948978df0 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:11:28 -0400 Subject: [PATCH 1/8] Add integration test for upload progress reporting Add test to verify that upload progress callbacks are invoked correctly and that progress values are monotonically increasing. While this test uses the /media endpoint which only supports single file uploads, it validates the basic progress reporting behavior and documents expected behavior for future multi-file upload scenarios. --- .../kotlin/MediaEndpointTest.kt | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt index e9acf0062..adf9eb06d 100644 --- a/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt +++ b/native/kotlin/api/kotlin/src/integrationTest/kotlin/MediaEndpointTest.kt @@ -74,6 +74,70 @@ class MediaEndpointTest { restoreTestServer() } + @Test + fun testCreateMediaRequestWithProgressReporting() = runTest { + val progressUpdates = mutableListOf() + var uploadStarted = false + + val uploadListener = object : WpRequestExecutor.UploadListener { + override fun onProgressUpdate(uploadedBytes: Long, totalBytes: Long) { + progressUpdates.add(ProgressUpdate(uploadedBytes, totalBytes)) + } + + override fun onUploadStarted(cancellableUpload: WpRequestExecutor.CancellableUpload) { + uploadStarted = true + } + } + + val authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword( + username = TestCredentials.INSTANCE.adminUsername, + password = TestCredentials.INSTANCE.adminPassword + ) + val requestExecutor = WpRequestExecutor( + fileResolver = FileResolverMock(), + uploadListener = uploadListener + ) + val clientWithProgress = WpApiClient( + wpOrgSiteApiRootUrl = TestCredentials.INSTANCE.apiRootUrl, + authProvider = authProvider, + requestExecutor = requestExecutor + ) + + val title = "Testing media upload with progress from Kotlin" + val response = clientWithProgress.request { requestBuilder -> + requestBuilder.media().create( + params = MediaCreateParams(title = title, filePath = "test_media.jpg") + ) + }.assertSuccessAndRetrieveData().data + + // Verify upload was successful + assertEquals(title, response.title.rendered) + + // Verify progress reporting worked + assert(uploadStarted) { "Upload should have started" } + assert(progressUpdates.isNotEmpty()) { "Should have received progress updates" } + + // Verify final progress shows completion + val finalProgress = progressUpdates.last() + assertEquals( + finalProgress.uploadedBytes, + finalProgress.totalBytes, + "Final progress should show upload complete" + ) + + // Verify progress never decreases. Note: The /media endpoint only supports + // single files, so this validates basic progress but not multi-file scenarios. + var previousBytes = 0L + progressUpdates.forEach { update -> + assert(update.uploadedBytes >= previousBytes) { + "Progress decreased from $previousBytes to ${update.uploadedBytes}" + } + previousBytes = update.uploadedBytes + } + + restoreTestServer() + } + fun mediaApiClient(): WpApiClient { val testCredentials = TestCredentials.INSTANCE val authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword( @@ -89,6 +153,8 @@ class MediaEndpointTest { ) } + data class ProgressUpdate(val uploadedBytes: Long, val totalBytes: Long) + class FileResolverMock: FileResolver { // in order to properly resolve the file from the test assets, we need to do it in the following way override fun getFile(path: String): File? = From 39fd7fbc603474856975c03c9f595cecb39b1647 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:11:37 -0400 Subject: [PATCH 2/8] Fix multi-file upload progress reporting Wrap the entire multipart body with ProgressRequestBody instead of wrapping individual files. This ensures progress is cumulative across all files rather than resetting for each file. Before: With multiple files, progress would reset to 0% for each file After: Progress smoothly increases from 0% to 100% across all files Changes: - Remove getRequestBody() helper method - Build complete multipart body first - Wrap entire body with ProgressRequestBody for cumulative tracking --- .../wordpress/api/kotlin/WpRequestExecutor.kt | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 6b52971e0..622bf2a5b 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -98,17 +98,37 @@ class WpRequestExecutor( throw RequestExecutionException.MediaFileNotFound(filePath = fileInfo.filePath) } val mimeType = fileInfo.mimeType ?: "application/octet-stream" - val requestBody = getRequestBody(file, mimeType, uploadListener) val filename = fileInfo.fileName ?: file.name + // Don't wrap individual files - we'll wrap the entire multipart body instead + val requestBody = file.asRequestBody(mimeType.toMediaType()) multipartBodyBuilder.addFormDataPart( name = name, filename = filename, body = requestBody ) } + + // Build the multipart body + val multipartBody = multipartBodyBuilder.build() + + // Wrap the entire multipart body for progress tracking + // This ensures progress is cumulative across all files, not per-file + val bodyWithProgress = if (uploadListener != null) { + ProgressRequestBody( + delegate = multipartBody, + progressListener = object : ProgressRequestBody.ProgressListener { + override fun onProgress(bytesWritten: Long, contentLength: Long) { + uploadListener.onProgressUpdate(bytesWritten, contentLength) + } + } + ) + } else { + multipartBody + } + requestBuilder.method( method = request.method().toString(), - body = multipartBodyBuilder.build() + body = bodyWithProgress ) request.headerMap().toMap().forEach { (key, values) -> values.forEach { value -> @@ -129,25 +149,6 @@ class WpRequestExecutor( } } - private fun getRequestBody( - file: File, - mimeType: String, - uploadListener: UploadListener? - ): RequestBody { - val fileRequestBody = file.asRequestBody(mimeType.toMediaType()) - return if (uploadListener != null) { - ProgressRequestBody( - delegate = fileRequestBody, - progressListener = object : ProgressRequestBody.ProgressListener { - override fun onProgress(bytesWritten: Long, contentLength: Long) { - uploadListener.onProgressUpdate(bytesWritten, contentLength) - } - } - ) - } else { - fileRequestBody - } - } override suspend fun sleep(millis: ULong) { delay(millis.toLong()) From 67273060c2ef4760f3c90bb4124498612dea1a25 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:12:22 -0400 Subject: [PATCH 3/8] Add exception handling to upload() method Add try-catch blocks to handle network exceptions (SSL errors, unknown host, no route to host) in the upload() method, matching the exception handling pattern used in execute(). Without this, upload errors would throw unhandled exceptions instead of being properly wrapped in RequestExecutionException. --- .../wordpress/api/kotlin/WpRequestExecutor.kt | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 622bf2a5b..508d2e92d 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -136,16 +136,28 @@ class WpRequestExecutor( } } - val call = httpClient.getClient().newCall(requestBuilder.build()) - uploadListener?.onUploadStarted(CancellableCall(call)) - call.execute().use { response -> - return@withContext WpNetworkResponse( - body = response.body?.bytes() ?: ByteArray(0), - statusCode = response.code.toUShort(), - responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), - requestUrl = request.url(), - requestHeaderMap = request.headerMap() + val urlRequest = requestBuilder.build() + + try { + val call = httpClient.getClient().newCall(urlRequest) + uploadListener?.onUploadStarted(CancellableCall(call)) + call.execute().use { response -> + return@withContext WpNetworkResponse( + body = response.body?.bytes() ?: ByteArray(0), + statusCode = response.code.toUShort(), + responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), + requestUrl = request.url(), + requestHeaderMap = request.headerMap() + ) + } + } catch (e: SSLPeerUnverifiedException) { + throw requestExecutionFailedWith( + RequestExecutionErrorReason.invalidSSLError(e, urlRequest.url) ) + } catch (e: UnknownHostException) { + throw requestExecutionFailedWith(RequestExecutionErrorReason.unknownHost(e)) + } catch (e: NoRouteToHostException) { + throw requestExecutionFailedWith(RequestExecutionErrorReason.noRouteToHost(e)) } } From 9d76de6927f98b56070835b7f64684bb57bc1e8d Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:12:42 -0400 Subject: [PATCH 4/8] Add User-Agent header to upload() method Add User-Agent header to upload requests to match the behavior of the execute() method. This ensures uploads are properly identified with the client version information. --- .../main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 508d2e92d..794ffbdca 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -135,6 +135,10 @@ class WpRequestExecutor( requestBuilder.addHeader(key, value) } } + requestBuilder.addHeader( + USER_AGENT_HEADER_NAME, + uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") + ) val urlRequest = requestBuilder.build() From f67b9206def3d3b734344c52f0980a53b3446c05 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:13:40 -0400 Subject: [PATCH 5/8] Fix resource leak in SSL error handler Ensure HttpsURLConnection is properly closed after certificate inspection to prevent resource leaks. Wrap the certificate extraction in try-finally block to guarantee cleanup, and add exception handling to gracefully handle cases where certificate inspection fails. --- .../wordpress/api/kotlin/WpRequestExecutor.kt | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 794ffbdca..deb8e5166 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -242,18 +242,32 @@ private fun RequestExecutionErrorReason.Companion.invalidSSLError( // // We spin up a new connection that'll accept any certificate. The connection will then // contain all the details we need for the error. - val newConnection = requestUrl.toUrl().openConnection() as HttpsURLConnection - newConnection.setHostnameVerifier { _, _ -> return@setHostnameVerifier true } - newConnection.connect() + return try { + val newConnection = requestUrl.toUrl().openConnection() as HttpsURLConnection + newConnection.setHostnameVerifier { _, _ -> true } + newConnection.connect() - // Certificate is parsed by the Rust shared implementation. - val certificates = newConnection.serverCertificates.map { parseCertificate(it.encoded) } - return RequestExecutionErrorReason.InvalidSslError( - reason = InvalidSslErrorReason.CertificateNotValidForName( - hostname = requestUrl.host, - presentedHostnames = listOfNotNull(certificates.first()?.commonName()) + try { + // Certificate is parsed by the Rust shared implementation. + val certificates = newConnection.serverCertificates.map { parseCertificate(it.encoded) } + RequestExecutionErrorReason.InvalidSslError( + reason = InvalidSslErrorReason.CertificateNotValidForName( + hostname = requestUrl.host, + presentedHostnames = listOfNotNull(certificates.first()?.commonName()) + ) + ) + } finally { + newConnection.disconnect() + } + } catch (ex: Exception) { + // Fallback if certificate inspection fails + RequestExecutionErrorReason.InvalidSslError( + reason = InvalidSslErrorReason.CertificateNotValidForName( + hostname = requestUrl.host, + presentedHostnames = emptyList() + ) ) - ) + } } private fun requestExecutionFailedWith(reason: RequestExecutionErrorReason) = From c98be1caf56f25e22e544766b403796ee1001b87 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:14:17 -0400 Subject: [PATCH 6/8] Prevent User-Agent header from being overridden Use header() instead of addHeader() when setting User-Agent to ensure it cannot be overridden by headers from the request. The header() method replaces any existing header with the same name, while addHeader() would append and potentially allow unwanted values. --- .../kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index deb8e5166..25978ad3d 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -56,7 +56,8 @@ class WpRequestExecutor( requestBuilder.addHeader(key, value) } } - requestBuilder.addHeader( + // Use header() instead of addHeader() to ensure User-Agent cannot be overridden + requestBuilder.header( USER_AGENT_HEADER_NAME, uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") ) @@ -135,7 +136,8 @@ class WpRequestExecutor( requestBuilder.addHeader(key, value) } } - requestBuilder.addHeader( + // Use header() instead of addHeader() to ensure User-Agent cannot be overridden + requestBuilder.header( USER_AGENT_HEADER_NAME, uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") ) From 40140ac9bd0c16fccac7fe43b95fa0627dec7b6b Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:21:20 -0400 Subject: [PATCH 7/8] Refactor upload() method and fix detekt issues Refactor upload() method to reduce complexity and length by extracting helper methods: - buildMultipartBody(): Builds the multipart form body - wrapWithProgressTracking(): Wraps body with progress tracking - buildUploadRequest(): Constructs the HTTP request - executeUpload(): Executes the upload and returns response Also fix detekt warnings: - Remove unused RequestBody import - Remove extra blank line - Add suppression annotations for SSL error handler with detailed justification comments This reduces the upload() method from 67 lines to 17 lines and improves maintainability. --- .../wordpress/api/kotlin/WpRequestExecutor.kt | 160 ++++++++++-------- 1 file changed, 90 insertions(+), 70 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index 25978ad3d..b2f7f73bb 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -10,7 +10,6 @@ import okhttp3.MediaType.Companion.toMediaType import okhttp3.MultipartBody import okhttp3.OkHttp import okhttp3.Request -import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody import uniffi.wp_api.InvalidSslErrorReason @@ -87,75 +86,12 @@ class WpRequestExecutor( override suspend fun upload(request: WpMultipartFormRequest): WpNetworkResponse = withContext(dispatcher) { - val requestBuilder = Request.Builder().url(request.url()) - val multipartBodyBuilder = MultipartBody.Builder() - .setType(MultipartBody.FORM) - request.fields().forEach { (k, v) -> - multipartBodyBuilder.addFormDataPart(k, v) - } - request.files().forEach { (name, fileInfo) -> - val file = fileResolver.getFile(fileInfo.filePath) - if (file == null || !file.canBeUploaded()) { - throw RequestExecutionException.MediaFileNotFound(filePath = fileInfo.filePath) - } - val mimeType = fileInfo.mimeType ?: "application/octet-stream" - val filename = fileInfo.fileName ?: file.name - // Don't wrap individual files - we'll wrap the entire multipart body instead - val requestBody = file.asRequestBody(mimeType.toMediaType()) - multipartBodyBuilder.addFormDataPart( - name = name, - filename = filename, - body = requestBody - ) - } - - // Build the multipart body - val multipartBody = multipartBodyBuilder.build() - - // Wrap the entire multipart body for progress tracking - // This ensures progress is cumulative across all files, not per-file - val bodyWithProgress = if (uploadListener != null) { - ProgressRequestBody( - delegate = multipartBody, - progressListener = object : ProgressRequestBody.ProgressListener { - override fun onProgress(bytesWritten: Long, contentLength: Long) { - uploadListener.onProgressUpdate(bytesWritten, contentLength) - } - } - ) - } else { - multipartBody - } - - requestBuilder.method( - method = request.method().toString(), - body = bodyWithProgress - ) - request.headerMap().toMap().forEach { (key, values) -> - values.forEach { value -> - requestBuilder.addHeader(key, value) - } - } - // Use header() instead of addHeader() to ensure User-Agent cannot be overridden - requestBuilder.header( - USER_AGENT_HEADER_NAME, - uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") - ) - - val urlRequest = requestBuilder.build() + val multipartBody = buildMultipartBody(request) + val bodyWithProgress = wrapWithProgressTracking(multipartBody) + val urlRequest = buildUploadRequest(request, bodyWithProgress) try { - val call = httpClient.getClient().newCall(urlRequest) - uploadListener?.onUploadStarted(CancellableCall(call)) - call.execute().use { response -> - return@withContext WpNetworkResponse( - body = response.body?.bytes() ?: ByteArray(0), - statusCode = response.code.toUShort(), - responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), - requestUrl = request.url(), - requestHeaderMap = request.headerMap() - ) - } + executeUpload(urlRequest, request) } catch (e: SSLPeerUnverifiedException) { throw requestExecutionFailedWith( RequestExecutionErrorReason.invalidSSLError(e, urlRequest.url) @@ -167,6 +103,87 @@ class WpRequestExecutor( } } + private fun buildMultipartBody(request: WpMultipartFormRequest): MultipartBody { + val multipartBodyBuilder = MultipartBody.Builder().setType(MultipartBody.FORM) + + request.fields().forEach { (k, v) -> + multipartBodyBuilder.addFormDataPart(k, v) + } + + request.files().forEach { (name, fileInfo) -> + val file = fileResolver.getFile(fileInfo.filePath) + if (file == null || !file.canBeUploaded()) { + throw RequestExecutionException.MediaFileNotFound(filePath = fileInfo.filePath) + } + val mimeType = fileInfo.mimeType ?: "application/octet-stream" + val filename = fileInfo.fileName ?: file.name + val requestBody = file.asRequestBody(mimeType.toMediaType()) + multipartBodyBuilder.addFormDataPart( + name = name, + filename = filename, + body = requestBody + ) + } + + return multipartBodyBuilder.build() + } + + private fun wrapWithProgressTracking(multipartBody: MultipartBody): okhttp3.RequestBody { + // Wrap the entire multipart body for progress tracking + // This ensures progress is cumulative across all files, not per-file + return if (uploadListener != null) { + ProgressRequestBody( + delegate = multipartBody, + progressListener = object : ProgressRequestBody.ProgressListener { + override fun onProgress(bytesWritten: Long, contentLength: Long) { + uploadListener.onProgressUpdate(bytesWritten, contentLength) + } + } + ) + } else { + multipartBody + } + } + + private fun buildUploadRequest( + request: WpMultipartFormRequest, + body: okhttp3.RequestBody + ): Request { + val requestBuilder = Request.Builder().url(request.url()) + + requestBuilder.method(request.method().toString(), body) + + request.headerMap().toMap().forEach { (key, values) -> + values.forEach { value -> + requestBuilder.addHeader(key, value) + } + } + + // Use header() instead of addHeader() to ensure User-Agent cannot be overridden + requestBuilder.header( + USER_AGENT_HEADER_NAME, + uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") + ) + + return requestBuilder.build() + } + + private fun executeUpload( + urlRequest: Request, + request: WpMultipartFormRequest + ): WpNetworkResponse { + val call = httpClient.getClient().newCall(urlRequest) + uploadListener?.onUploadStarted(CancellableCall(call)) + return call.execute().use { response -> + WpNetworkResponse( + body = response.body?.bytes() ?: ByteArray(0), + statusCode = response.code.toUShort(), + responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), + requestUrl = request.url(), + requestHeaderMap = request.headerMap() + ) + } + } override suspend fun sleep(millis: ULong) { delay(millis.toLong()) @@ -233,7 +250,7 @@ private fun RequestExecutionErrorReason.Companion.noRouteToHost(e: NoRouteToHost reason = e.localizedMessage ) -@Suppress("UNUSED_PARAMETER") +@Suppress("UNUSED_PARAMETER", "TooGenericExceptionCaught", "SwallowedException") private fun RequestExecutionErrorReason.Companion.invalidSSLError( e: SSLPeerUnverifiedException, // To avoid `SwallowedException` from Detekt requestUrl: HttpUrl @@ -262,7 +279,10 @@ private fun RequestExecutionErrorReason.Companion.invalidSSLError( newConnection.disconnect() } } catch (ex: Exception) { - // Fallback if certificate inspection fails + // Fallback if certificate inspection fails due to network issues, cast failures, etc. + // We intentionally catch Exception here as we want to return a valid error response + // even if certificate inspection fails. The original SSL error (e parameter) is + // preserved in the calling context. This is a best-effort attempt to get cert details. RequestExecutionErrorReason.InvalidSslError( reason = InvalidSslErrorReason.CertificateNotValidForName( hostname = requestUrl.host, From 24dc0ea17ff3843e8c3bf0577a283f8ba102d789 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 24 Oct 2025 20:30:20 -0400 Subject: [PATCH 8/8] Extract common request execution logic Consolidate duplicate code between execute() and upload() methods by extracting shared functionality into helper methods: - addRequestHeaders(): Handles adding custom headers and User-Agent for both request types, ensuring consistent header handling - executeRequestSafely(): Centralizes exception handling, call execution, and response building with optional upload listener notification This eliminates code duplication and ensures both methods handle errors, headers, and responses identically. The upload listener is now correctly notified with the actual Call object that gets executed. Changes: - Removed buildUploadRequest() and executeUpload() helper methods - Both execute() and upload() now use common helpers - Reduced overall code size and improved maintainability - Added @Suppress("ThrowsCount") for exception handling (3 specific network exceptions need to be caught and converted) --- .../wordpress/api/kotlin/WpRequestExecutor.kt | 103 +++++++----------- 1 file changed, 39 insertions(+), 64 deletions(-) diff --git a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt index b2f7f73bb..4e4d748a0 100644 --- a/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt +++ b/native/kotlin/api/kotlin/src/main/kotlin/rs/wordpress/api/kotlin/WpRequestExecutor.kt @@ -50,57 +50,24 @@ class WpRequestExecutor( wpNetworkRequestBody } ) - request.headerMap().toMap().forEach { (key, values) -> - values.forEach { value -> - requestBuilder.addHeader(key, value) - } - } - // Use header() instead of addHeader() to ensure User-Agent cannot be overridden - requestBuilder.header( - USER_AGENT_HEADER_NAME, - uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") - ) + addRequestHeaders(requestBuilder, request.headerMap()) val urlRequest = requestBuilder.build() - try { - httpClient.getClient().newCall(urlRequest).execute().use { response -> - return@withContext WpNetworkResponse( - body = response.body?.bytes() ?: ByteArray(0), - statusCode = response.code.toUShort(), - responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), - requestUrl = request.url(), - requestHeaderMap = request.headerMap() - ) - } - } catch (e: SSLPeerUnverifiedException) { - throw requestExecutionFailedWith( - RequestExecutionErrorReason.invalidSSLError(e, urlRequest.url) - ) - } catch (e: UnknownHostException) { - throw requestExecutionFailedWith(RequestExecutionErrorReason.unknownHost(e)) - } catch (e: NoRouteToHostException) { - throw requestExecutionFailedWith(RequestExecutionErrorReason.noRouteToHost(e)) - } + executeRequestSafely(urlRequest, request.url(), request.headerMap()) } override suspend fun upload(request: WpMultipartFormRequest): WpNetworkResponse = withContext(dispatcher) { val multipartBody = buildMultipartBody(request) val bodyWithProgress = wrapWithProgressTracking(multipartBody) - val urlRequest = buildUploadRequest(request, bodyWithProgress) + val requestBuilder = Request.Builder().url(request.url()) + requestBuilder.method(request.method().toString(), bodyWithProgress) - try { - executeUpload(urlRequest, request) - } catch (e: SSLPeerUnverifiedException) { - throw requestExecutionFailedWith( - RequestExecutionErrorReason.invalidSSLError(e, urlRequest.url) - ) - } catch (e: UnknownHostException) { - throw requestExecutionFailedWith(RequestExecutionErrorReason.unknownHost(e)) - } catch (e: NoRouteToHostException) { - throw requestExecutionFailedWith(RequestExecutionErrorReason.noRouteToHost(e)) - } + addRequestHeaders(requestBuilder, request.headerMap()) + val urlRequest = requestBuilder.build() + + executeRequestSafely(urlRequest, request.url(), request.headerMap(), notifyUploadListener = true) } private fun buildMultipartBody(request: WpMultipartFormRequest): MultipartBody { @@ -145,43 +112,51 @@ class WpRequestExecutor( } } - private fun buildUploadRequest( - request: WpMultipartFormRequest, - body: okhttp3.RequestBody - ): Request { - val requestBuilder = Request.Builder().url(request.url()) - - requestBuilder.method(request.method().toString(), body) - - request.headerMap().toMap().forEach { (key, values) -> + private fun addRequestHeaders(requestBuilder: Request.Builder, headerMap: WpNetworkHeaderMap) { + headerMap.toMap().forEach { (key, values) -> values.forEach { value -> requestBuilder.addHeader(key, value) } } - // Use header() instead of addHeader() to ensure User-Agent cannot be overridden requestBuilder.header( USER_AGENT_HEADER_NAME, uniffi.wp_api.defaultUserAgent("kotlin-okhttp/${OkHttp.VERSION}") ) - - return requestBuilder.build() } - private fun executeUpload( + @Suppress("ThrowsCount") + private fun executeRequestSafely( urlRequest: Request, - request: WpMultipartFormRequest + requestUrl: String, + requestHeaderMap: WpNetworkHeaderMap, + notifyUploadListener: Boolean = false ): WpNetworkResponse { - val call = httpClient.getClient().newCall(urlRequest) - uploadListener?.onUploadStarted(CancellableCall(call)) - return call.execute().use { response -> - WpNetworkResponse( - body = response.body?.bytes() ?: ByteArray(0), - statusCode = response.code.toUShort(), - responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), - requestUrl = request.url(), - requestHeaderMap = request.headerMap() + try { + val call = httpClient.getClient().newCall(urlRequest) + + // Notify upload listener if this is an upload request + if (notifyUploadListener) { + uploadListener?.onUploadStarted(CancellableCall(call)) + } + + return call.execute().use { response -> + WpNetworkResponse( + body = response.body?.bytes() ?: ByteArray(0), + statusCode = response.code.toUShort(), + responseHeaderMap = WpNetworkHeaderMap.fromMultiMap(response.headers.toMultimap()), + requestUrl = requestUrl, + requestHeaderMap = requestHeaderMap + ) + } + } catch (e: SSLPeerUnverifiedException) { + throw requestExecutionFailedWith( + RequestExecutionErrorReason.invalidSSLError(e, urlRequest.url) ) + } catch (e: UnknownHostException) { + throw requestExecutionFailedWith(RequestExecutionErrorReason.unknownHost(e)) + } catch (e: NoRouteToHostException) { + throw requestExecutionFailedWith(RequestExecutionErrorReason.noRouteToHost(e)) } }