From 886cf685e4de0a303c7a1d2b4ac9ac15219d8b80 Mon Sep 17 00:00:00 2001 From: Thomas Nardone Date: Tue, 11 Jun 2024 07:25:20 -0700 Subject: [PATCH 1/4] Add mockito-kotlin Differential Revision: D56311031 --- packages/react-native/ReactAndroid/build.gradle.kts | 1 + packages/react-native/gradle/libs.versions.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/react-native/ReactAndroid/build.gradle.kts b/packages/react-native/ReactAndroid/build.gradle.kts index c1598daa1347a0..2bd09788b5b0a6 100644 --- a/packages/react-native/ReactAndroid/build.gradle.kts +++ b/packages/react-native/ReactAndroid/build.gradle.kts @@ -778,6 +778,7 @@ dependencies { testImplementation(libs.junit) testImplementation(libs.assertj) testImplementation(libs.mockito) + testImplementation(libs.mockito.kotlin) testImplementation(libs.robolectric) testImplementation(libs.thoughtworks) } diff --git a/packages/react-native/gradle/libs.versions.toml b/packages/react-native/gradle/libs.versions.toml index a830563136fb44..85a7927862435f 100644 --- a/packages/react-native/gradle/libs.versions.toml +++ b/packages/react-native/gradle/libs.versions.toml @@ -25,6 +25,7 @@ jsr305 = "3.0.2" junit = "4.13.2" kotlin = "1.9.24" mockito = "3.12.4" +mockito-kotlin = "3.1.0" nexus-publish = "1.3.0" okhttp = "4.9.2" okio = "2.9.0" @@ -69,6 +70,7 @@ javax-annotation-api = { module = "javax.annotation:javax.annotation-api", versi junit = {module = "junit:junit", version.ref = "junit" } assertj = {module = "org.assertj:assertj-core", version.ref = "assertj" } mockito = {module = "org.mockito:mockito-inline", version.ref = "mockito" } +mockito-kotlin = {module = "org.mockito.kotlin:mockito-kotlin", version.ref = "mockito-kotlin" } robolectric = {module = "org.robolectric:robolectric", version.ref = "robolectric" } thoughtworks = {module = "com.thoughtworks.xstream:xstream", version.ref = "xstream" } From dd0027bdd63f3acf6bd0ac9ab6ef01808a962e2c Mon Sep 17 00:00:00 2001 From: Thomas Nardone Date: Tue, 11 Jun 2024 07:25:20 -0700 Subject: [PATCH 2/4] Fix NetworkingModuleTest Differential Revision: D56440749 --- .../modules/network/NetworkingModuleTest.kt | 293 +++++++++--------- 1 file changed, 139 insertions(+), 154 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.kt index 683f52ad3f62d0..704e98b9cb1158 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.kt @@ -12,7 +12,6 @@ import com.facebook.react.bridge.CatalystInstance import com.facebook.react.bridge.JavaOnlyArray import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.bridge.ReactApplicationContext -import com.facebook.react.bridge.ReactContext import com.facebook.react.bridge.WritableArray import com.facebook.react.bridge.WritableMap import com.facebook.react.common.network.OkHttpCallUtil @@ -31,93 +30,72 @@ import okio.Buffer import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Before -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith -import org.mockito.ArgumentCaptor -import org.mockito.ArgumentMatchers.any -import org.mockito.ArgumentMatchers.eq -import org.mockito.Captor import org.mockito.MockedStatic -import org.mockito.Mockito.mock +import org.mockito.Mockito.RETURNS_MOCKS +import org.mockito.Mockito.mockConstruction import org.mockito.Mockito.mockStatic -import org.mockito.Mockito.times -import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` as whenever +import org.mockito.kotlin.KArgumentCaptor +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.mockito.kotlin.withSettings import org.robolectric.RobolectricTestRunner -/** - * Returns Mockito.any() as nullable type to avoid java.lang.IllegalStateException when null is - * returned. - */ -private fun anyOrNull(type: Class): T = any(type) - -/** - * Returns ArgumentCaptor.capture() as nullable type to avoid java.lang.IllegalStateException when - * null is returned. - */ -fun capture(argumentCaptor: ArgumentCaptor): T = argumentCaptor.capture() - +/** Tests [NetworkingModule] */ @RunWith(RobolectricTestRunner::class) class NetworkingModuleTest { + private lateinit var networkingModule: NetworkingModule private lateinit var httpClient: OkHttpClient private lateinit var context: ReactApplicationContext private lateinit var arguments: MockedStatic + private lateinit var okHttpCallUtil: MockedStatic private lateinit var requestBodyUtil: MockedStatic - - @Captor private lateinit var requestArgumentCaptor: ArgumentCaptor + private lateinit var requestArgumentCaptor: KArgumentCaptor @Before - fun prepareModules() { - httpClient = mock(OkHttpClient::class.java) - whenever(httpClient.cookieJar).thenReturn(mock(CookieJarContainer::class.java)) - whenever(httpClient.newCall(anyOrNull(Request::class.java))).thenAnswer { - val callMock = mock(Call::class.java) - callMock - } + fun setUp() { + httpClient = mock() + whenever(httpClient.cookieJar).thenReturn(mock()) + whenever(httpClient.newCall(any())).thenReturn(mock()) - val clientBuilder = mock(OkHttpClient.Builder::class.java) + val clientBuilder = mock() whenever(clientBuilder.build()).thenReturn(httpClient) whenever(httpClient.newBuilder()).thenReturn(clientBuilder) - val reactInstance = mock(CatalystInstance::class.java) - - context = mock(ReactApplicationContext::class.java) + val reactInstance = mock() + context = mock() whenever(context.catalystInstance).thenReturn(reactInstance) whenever(context.hasActiveReactInstance()).thenReturn(true) networkingModule = NetworkingModule(context, "", httpClient) arguments = mockStatic(Arguments::class.java) - arguments.`when`(Arguments::createArray).thenAnswer { JavaOnlyArray() } - arguments.`when`(Arguments::createMap).thenAnswer { JavaOnlyMap() } + arguments.`when` { Arguments.createArray() }.thenAnswer { JavaOnlyArray() } + arguments.`when` { Arguments.createMap() }.thenAnswer { JavaOnlyMap() } - requestBodyUtil = mockStatic(RequestBodyUtil::class.java) - requestBodyUtil - .`when` { - RequestBodyUtil.getFileInputStream(any(ReactContext::class.java), any(String::class.java)) - } - .thenReturn(mock(InputStream::class.java)) - requestBodyUtil - .`when` { - RequestBodyUtil.create(any(MediaType::class.java), any(InputStream::class.java)) - } - .thenReturn(mock(RequestBody::class.java)) + okHttpCallUtil = mockStatic(OkHttpCallUtil::class.java) + requestArgumentCaptor = argumentCaptor() + } + + private fun setupRequestBodyUtil() { + requestBodyUtil = + mockStatic(RequestBodyUtil::class.java, withSettings().defaultAnswer(RETURNS_MOCKS)) requestBodyUtil - .`when` { - RequestBodyUtil.createProgressRequest( - any(RequestBody::class.java), any(ProgressListener::class.java)) - } + .`when` { RequestBodyUtil.createProgressRequest(any(), any()) } .thenCallRealMethod() - - requestArgumentCaptor = ArgumentCaptor.forClass(Request::class.java) } @After fun tearDown() { arguments.close() - requestBodyUtil.close() + okHttpCallUtil.close() } @Test @@ -133,11 +111,14 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) - assertThat(requestArgumentCaptor.value.url.toString()).isEqualTo("http://somedomain/foo") - // We set the User-Agent header by default - assertThat(requestArgumentCaptor.value.headers.size).isEqualTo(1) - assertThat(requestArgumentCaptor.value.method).isEqualTo("GET") + with(requestArgumentCaptor) { + verify(httpClient).newCall(capture()) + + assertThat(firstValue.url.toString()).isEqualTo("http://somedomain/foo") + // We set the User-Agent header by default + assertThat(firstValue.headers.size).isEqualTo(1) + assertThat(firstValue.method).isEqualTo("GET") + } } @Test @@ -194,12 +175,12 @@ class NetworkingModuleTest { } private fun verifyErrorEmit(context: ReactApplicationContext, requestId: Int) { - val captor = ArgumentCaptor.forClass(WritableArray::class.java) + val captor = argumentCaptor() verify(context).emitDeviceEvent(eq("didCompleteNetworkResponse"), captor.capture()) - val array = captor.value + val array = captor.firstValue assertThat(array.getInt(0)).isEqualTo(requestId) - assertThat(array.getString(1)).isNotNull + assertThat(array.getString(1)).isNotBlank } @Test @@ -218,15 +199,17 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) - assertThat(requestArgumentCaptor.value.url.toString()).isEqualTo("http://somedomain/bar") - assertThat(requestArgumentCaptor.value.headers.size).isEqualTo(2) - assertThat(requestArgumentCaptor.value.method).isEqualTo("POST") - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.type).isEqualTo("text") - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.subtype).isEqualTo("plain") - val contentBuffer = Buffer() - requestArgumentCaptor.value.body!!.writeTo(contentBuffer) - assertThat(contentBuffer.readUtf8()).isEqualTo("This is request body") + with(requestArgumentCaptor) { + verify(httpClient).newCall(capture()) + assertThat(firstValue.url.toString()).isEqualTo("http://somedomain/bar") + assertThat(firstValue.headers.size).isEqualTo(2) + assertThat(firstValue.method).isEqualTo("POST") + assertThat(firstValue.body?.contentType()?.type).isEqualTo("text") + assertThat(firstValue.body?.contentType()?.subtype).isEqualTo("plain") + val contentBuffer = Buffer() + firstValue.body?.writeTo(contentBuffer) + assertThat(contentBuffer.readUtf8()).isEqualTo("This is request body") + } } @Test @@ -247,11 +230,13 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) - val requestHeaders = requestArgumentCaptor.value.headers - assertThat(requestHeaders.size).isEqualTo(2) - assertThat(requestHeaders["Accept"]).isEqualTo("text/plain") - assertThat(requestHeaders["User-Agent"]).isEqualTo("React test agent/1.0") + with(requestArgumentCaptor) { + verify(httpClient).newCall(capture()) + val requestHeaders = firstValue.headers + assertThat(requestHeaders.size).isEqualTo(2) + assertThat(requestHeaders["Accept"]).isEqualTo("text/plain") + assertThat(requestHeaders["User-Agent"]).isEqualTo("React test agent/1.0") + } } @Test @@ -270,10 +255,10 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) + verify(httpClient).newCall(requestArgumentCaptor.capture()) // Verify okhttp does not append "charset=utf-8" - assertThat(requestArgumentCaptor.value.body!!.contentType().toString()) + assertThat(requestArgumentCaptor.firstValue.body?.contentType().toString()) .isEqualTo("application/json") } @@ -295,10 +280,10 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) + verify(httpClient).newCall(requestArgumentCaptor.capture()) val contentBuffer = Buffer() - requestArgumentCaptor.value.body!!.writeTo(contentBuffer) + requestArgumentCaptor.firstValue.body?.writeTo(contentBuffer) assertThat(contentBuffer.readString(StandardCharsets.UTF_16)).isEqualTo(testString) } @@ -318,17 +303,18 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) - verify(httpClient).newCall(capture(requestArgumentCaptor)) + verify(httpClient).newCall(requestArgumentCaptor.capture()) val contentBuffer = Buffer() - requestArgumentCaptor.value.body!!.writeTo(contentBuffer) + requestArgumentCaptor.firstValue.body?.writeTo(contentBuffer) assertThat(contentBuffer.readString(StandardCharsets.UTF_8)).isEqualTo("test") - assertThat(requestArgumentCaptor.value.header("Content-Type")).isEqualTo("invalid") + assertThat(requestArgumentCaptor.firstValue.header("Content-Type")).isEqualTo("invalid") } @Test fun testMultipartPostRequestSimple() { + setupRequestBodyUtil() val body = JavaOnlyMap() val formData = JavaOnlyArray() val bodyPart = JavaOnlyMap() @@ -350,17 +336,22 @@ class NetworkingModuleTest { false /* withCredentials */) // verify url, method, headers - verify(httpClient).newCall(capture(requestArgumentCaptor)) - assertThat(requestArgumentCaptor.value.url.toString()).isEqualTo("http://someurl/uploadFoo") - assertThat(requestArgumentCaptor.value.method).isEqualTo("POST") - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.type).isEqualTo(FORM.type) - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.subtype).isEqualTo(FORM.subtype) - val requestHeaders = requestArgumentCaptor.value.headers - assertThat(requestHeaders.size).isEqualTo(1) + with(requestArgumentCaptor) { + verify(httpClient).newCall(capture()) + assertThat(firstValue.url.toString()).isEqualTo("http://someurl/uploadFoo") + assertThat(firstValue.method).isEqualTo("POST") + assertThat(firstValue.body?.contentType()?.type).isEqualTo(FORM.type) + assertThat(firstValue.body?.contentType()?.subtype).isEqualTo(FORM.subtype) + val requestHeaders = firstValue.headers + assertThat(requestHeaders.size).isEqualTo(1) + } + + requestBodyUtil.close() } @Test fun testMultipartPostRequestHeaders() { + setupRequestBodyUtil() val headers = listOf( JavaOnlyArray.of("Accept", "text/plain"), @@ -388,36 +379,37 @@ class NetworkingModuleTest { false /* withCredentials */) // verify url, method, headers - verify(httpClient).newCall(capture(requestArgumentCaptor)) - assertThat(requestArgumentCaptor.value.url.toString()).isEqualTo("http://someurl/uploadFoo") - assertThat(requestArgumentCaptor.value.method).isEqualTo("POST") - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.type).isEqualTo(FORM.type) - assertThat(requestArgumentCaptor.value.body!!.contentType()!!.subtype).isEqualTo(FORM.subtype) - val requestHeaders = requestArgumentCaptor.value.headers - assertThat(requestHeaders.size).isEqualTo(3) - assertThat(requestHeaders["Accept"]).isEqualTo("text/plain") - assertThat(requestHeaders["User-Agent"]).isEqualTo("React test agent/1.0") - assertThat(requestHeaders["content-type"]).isEqualTo("multipart/form-data") + with(requestArgumentCaptor) { + verify(httpClient).newCall(capture()) + assertThat(firstValue.url.toString()).isEqualTo("http://someurl/uploadFoo") + assertThat(firstValue.method).isEqualTo("POST") + assertThat(firstValue.body?.contentType()?.type).isEqualTo(FORM.type) + assertThat(firstValue.body?.contentType()?.subtype).isEqualTo(FORM.subtype) + val requestHeaders = firstValue.headers + assertThat(requestHeaders.size).isEqualTo(3) + assertThat(requestHeaders["Accept"]).isEqualTo("text/plain") + assertThat(requestHeaders["User-Agent"]).isEqualTo("React test agent/1.0") + assertThat(requestHeaders["content-type"]).isEqualTo("multipart/form-data") + } + requestBodyUtil.close() } @Test - @Ignore("TODO: Fix me (T171890419)") fun testMultipartPostRequestBody() { - val inputStream = mock(InputStream::class.java) + val inputStream = mock() whenever(inputStream.available()).thenReturn("imageUri".length) - - val multipartBuilder = mock(MultipartBody.Builder::class.java) - // TODO This PowerMock statement should be migrated to an equivalent for Mockito - // once this test is unsuppressed. - // whenNew(MultipartBody.Builder.class) - // .withNoArguments() - // .thenReturn(multipartBuilder); - whenever(multipartBuilder.setType(anyOrNull(MediaType::class.java))).thenAnswer { - multipartBuilder + setupRequestBodyUtil() + with(requestBodyUtil) { + `when` { RequestBodyUtil.getFileInputStream(any(), any()) } + .thenReturn(inputStream) + `when` { RequestBodyUtil.create(any(), any()) }.thenCallRealMethod() } - whenever(multipartBuilder.addPart(any(Headers::class.java), anyOrNull(RequestBody::class.java))) - .thenAnswer { multipartBuilder } - whenever(multipartBuilder.build()).thenAnswer { mock(MultipartBody::class.java) } + val multipartBodyBuilderMock = + mockConstruction(MultipartBody.Builder::class.java) { mock, _ -> + whenever(mock.setType(any())).thenReturn(mock) + whenever(mock.addPart(any(), any())).thenReturn(mock) + whenever(mock.build()).thenReturn(mock()) + } val headers = listOf(JavaOnlyArray.of("content-type", "multipart/form-data")) @@ -456,23 +448,17 @@ class NetworkingModuleTest { false /* withCredentials */) // verify RequestBodyPart for image - - // TODO This should be migrated to requestBodyUtil.verify(); - // PowerMockito.verifyStatic(RequestBodyUtil.class, times(1)); - RequestBodyUtil.getFileInputStream(any(ReactContext::class.java), eq("imageUri")) - // TODO This should be migrated to requestBodyUtil.verify(); - // PowerMockito.verifyStatic(RequestBodyUtil.class, times(1)); - RequestBodyUtil.create("image/jpg".toMediaTypeOrNull(), inputStream) + requestBodyUtil.verify { RequestBodyUtil.getFileInputStream(any(), eq("imageUri")) } + requestBodyUtil.verify { RequestBodyUtil.create(eq("image/jpg".toMediaTypeOrNull()), any()) } // verify body - // TODO fix it (now mock is not called) + val multipartBuilder = multipartBodyBuilderMock.constructed()[0] verify(multipartBuilder).build() verify(multipartBuilder).setType(FORM) - // TODO fix it (Captors are nulls) - val headersArgumentCaptor = ArgumentCaptor.forClass(Headers::class.java) - val bodyArgumentCaptor = ArgumentCaptor.forClass(RequestBody::class.java) + val headersArgumentCaptor = argumentCaptor() + val bodyArgumentCaptor = argumentCaptor() verify(multipartBuilder, times(2)) - .addPart(capture(headersArgumentCaptor), capture(bodyArgumentCaptor)) + .addPart(headersArgumentCaptor.capture(), bodyArgumentCaptor.capture()) val bodyHeaders = headersArgumentCaptor.allValues assertThat(bodyHeaders.size).isEqualTo(2) @@ -487,20 +473,22 @@ class NetworkingModuleTest { assertThat(bodyRequestBody[1].contentType()) .isEqualTo("image/jpg".toMediaTypeOrNull()) assertThat(bodyRequestBody[1].contentLength()).isEqualTo("imageUri".toByteArray().size.toLong()) + + multipartBodyBuilderMock.close() + requestBodyUtil.close() } @Test - @Ignore("TODO: Fix me (T171890419)") fun testCancelAllCallsInvalidate() { val requests = 3 val calls = arrayOfNulls(requests) for (idx in 0 until requests) { - calls[idx] = mock(Call::class.java) + calls[idx] = mock() } - whenever(httpClient.newCall(anyOrNull(Request::class.java))).thenAnswer { invocation -> + whenever(httpClient.newCall(any())).thenAnswer { invocation -> val request = invocation.arguments[0] as Request - calls[(request.tag() as Int?)!! - 1] + calls[(request.tag() as Int) - 1] } networkingModule.initialize() @@ -517,15 +505,14 @@ class NetworkingModuleTest { false /* withCredentials */) } - verify(httpClient, times(3)).newCall(anyOrNull(Request::class.java)) + verify(httpClient, times(3)).newCall(any()) networkingModule.invalidate() - // TODO This should be migrated to okHttpCallUtil.verify(); - // PowerMockito.verifyStatic(OkHttpCallUtil.class, times(3)); - val clientArguments = ArgumentCaptor.forClass(OkHttpClient::class.java) - val requestIdArguments = ArgumentCaptor.forClass(Int::class.java) - - OkHttpCallUtil.cancelTag(capture(clientArguments), capture(requestIdArguments)) + val clientArguments = argumentCaptor() + val requestIdArguments = argumentCaptor() + okHttpCallUtil.verify( + { OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) }, + times(3)) assertThat(requestIdArguments.allValues.size).isEqualTo(requests) for (idx in 0 until requests) { @@ -534,18 +521,16 @@ class NetworkingModuleTest { } @Test - @Ignore("TODO: Fix me (T171890419)") fun testCancelSomeCallsInvalidate() { val requests = 3 val calls = arrayOfNulls(requests) for (idx in 0 until requests) { - calls[idx] = mock(Call::class.java) + calls[idx] = mock() } - whenever(httpClient.newCall(anyOrNull(Request::class.java))).thenAnswer { invocation -> + whenever(httpClient.newCall(any())).thenAnswer { invocation -> val request = invocation.arguments[0] as Request - calls[(request.tag() as Int?)!! - 1] + calls[(request.tag() as Int) - 1] } - for (idx in 0 until requests) { networkingModule.sendRequest( "GET", @@ -558,14 +543,14 @@ class NetworkingModuleTest { 0.0, /* timeout */ false /* withCredentials */) } - verify(httpClient, times(3)).newCall(anyOrNull(Request::class.java)) + verify(httpClient, times(3)).newCall(any()) networkingModule.abortRequest(requests.toDouble()) - // TODO This should be migrated to okHttpCallUtil.verify(); - // PowerMockito.verifyStatic(OkHttpCallUtil.class, times(1)); - var clientArguments = ArgumentCaptor.forClass(OkHttpClient::class.java) - var requestIdArguments = ArgumentCaptor.forClass(Int::class.java) - OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) + var clientArguments = argumentCaptor() + var requestIdArguments = argumentCaptor() + okHttpCallUtil.verify { + OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) + } println(requestIdArguments.allValues) assertThat(requestIdArguments.allValues.size).isEqualTo(1) assertThat(requestIdArguments.allValues[0]).isEqualTo(requests) @@ -575,11 +560,11 @@ class NetworkingModuleTest { // If `cancelTag` would've been called again for the aborted call, we would have had // `requests + 1` calls. networkingModule.invalidate() - // TODO This should be migrated to okHttpCallUtil.verify(); - // PowerMockito.verifyStatic(OkHttpCallUtil.class, times(requests)); - clientArguments = ArgumentCaptor.forClass(OkHttpClient::class.java) - requestIdArguments = ArgumentCaptor.forClass(Int::class.java) - OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) + clientArguments = argumentCaptor() + requestIdArguments = argumentCaptor() + okHttpCallUtil.verify( + { OkHttpCallUtil.cancelTag(clientArguments.capture(), requestIdArguments.capture()) }, + times(requests)) assertThat(requestIdArguments.allValues.size).isEqualTo(requests) for (idx in 0 until requests) { assertThat(requestIdArguments.allValues.contains(idx + 1)).isTrue From 6191929c31b0d948c193f40350cae558c49bb17e Mon Sep 17 00:00:00 2001 From: Thomas Nardone Date: Tue, 11 Jun 2024 07:25:20 -0700 Subject: [PATCH 3/4] Convert Network Module [1] Differential Revision: D55802173 --- .../modules/network/CookieJarContainer.java | 17 ---- .../modules/network/CookieJarContainer.kt | 17 ++++ ...putStream.java => CountingOutputStream.kt} | 56 ++++++------- .../modules/network/CustomClientBuilder.java | 14 ---- .../modules/network/CustomClientBuilder.kt | 15 ++++ .../modules/network/ProgressRequestBody.java | 79 ------------------- .../modules/network/ProgressRequestBody.kt | 72 +++++++++++++++++ 7 files changed, 128 insertions(+), 142 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.java create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.kt rename packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/{CountingOutputStream.java => CountingOutputStream.kt} (55%) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.java create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.kt delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.java create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.java deleted file mode 100644 index 339bee4e6592ca..00000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.java +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.network; - -import okhttp3.CookieJar; - -public interface CookieJarContainer extends CookieJar { - - void setCookieJar(CookieJar cookieJar); - - void removeCookieJar(); -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.kt new file mode 100644 index 00000000000000..1804f5e9ce57c5 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CookieJarContainer.kt @@ -0,0 +1,17 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.modules.network + +import okhttp3.CookieJar + +public interface CookieJarContainer : CookieJar { + + public fun setCookieJar(cookieJar: CookieJar?) + + public fun removeCookieJar() +} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt similarity index 55% rename from packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.java rename to packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt index 833feca9753d3a..6a1c790bd82c7f 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt @@ -13,12 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +package com.facebook.react.modules.network -package com.facebook.react.modules.network; - -import java.io.FilterOutputStream; -import java.io.IOException; -import java.io.OutputStream; +import java.io.FilterOutputStream +import java.io.IOException +import java.io.OutputStream /** * An OutputStream that counts the number of bytes written. @@ -26,42 +25,35 @@ * @author Chris Nokleberg * @since 1.0 */ -class CountingOutputStream extends FilterOutputStream { - - private long mCount; - - /** - * Constructs a new {@code FilterOutputStream} with {@code out} as its target stream. - * - * @param out the target stream that this stream writes to. - */ - public CountingOutputStream(OutputStream out) { - super(out); - mCount = 0; - } +internal open class CountingOutputStream +/** + * Constructs a new `FilterOutputStream` with `out` as its target stream. + * + * @param out the target stream that this stream writes to. + */ +(out: OutputStream?) : FilterOutputStream(out) { /** Returns the number of bytes written. */ - public long getCount() { - return mCount; - } + var count: Long = 0 + private set - @Override - public void write(byte[] b, int off, int len) throws IOException { - out.write(b, off, len); - mCount += len; + @Throws(IOException::class) + override fun write(b: ByteArray, off: Int, len: Int) { + out.write(b, off, len) + count += len.toLong() } - @Override - public void write(int b) throws IOException { - out.write(b); - mCount++; + @Throws(IOException::class) + override fun write(b: Int) { + out.write(b) + count++ } // Overriding close() because FilterOutputStream's close() method pre-JDK8 has bad behavior: // it silently ignores any exception thrown by flush(). Instead, just close the delegate stream. // It should flush itself if necessary. - @Override - public void close() throws IOException { - out.close(); + @Throws(IOException::class) + override fun close() { + out.close() } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.java deleted file mode 100644 index db81d653baeada..00000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.java +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.network; - -import okhttp3.OkHttpClient; - -public interface CustomClientBuilder { - public void apply(OkHttpClient.Builder builder); -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.kt new file mode 100644 index 00000000000000..3659bb03f43661 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CustomClientBuilder.kt @@ -0,0 +1,15 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.modules.network + +import okhttp3.OkHttpClient +import okhttp3.OkHttpClient.Builder + +public fun interface CustomClientBuilder { + public fun apply(builder: OkHttpClient.Builder) +} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.java deleted file mode 100644 index af43bdde6f7bd5..00000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.modules.network; - -import java.io.IOException; -import okhttp3.MediaType; -import okhttp3.RequestBody; -import okio.BufferedSink; -import okio.Okio; -import okio.Sink; - -class ProgressRequestBody extends RequestBody { - - private final RequestBody mRequestBody; - private final ProgressListener mProgressListener; - private long mContentLength = 0L; - - public ProgressRequestBody(RequestBody requestBody, ProgressListener progressListener) { - mRequestBody = requestBody; - mProgressListener = progressListener; - } - - @Override - public MediaType contentType() { - return mRequestBody.contentType(); - } - - @Override - public long contentLength() throws IOException { - if (mContentLength == 0) { - mContentLength = mRequestBody.contentLength(); - } - return mContentLength; - } - - @Override - public void writeTo(BufferedSink sink) throws IOException { - // In 99% of cases, this method is called strictly once. - // The only case when it is called more than once is internal okhttp upload re-try. - // We need to re-create CountingOutputStream in this case as progress should be re-evaluated. - BufferedSink sinkWrapper = Okio.buffer(outputStreamSink(sink)); - - // contentLength changes for input streams, since we're using inputStream.available(), - // so get the length before writing to the sink - contentLength(); - - mRequestBody.writeTo(sinkWrapper); - sinkWrapper.flush(); - } - - private Sink outputStreamSink(BufferedSink sink) { - return Okio.sink( - new CountingOutputStream(sink.outputStream()) { - @Override - public void write(byte[] data, int offset, int byteCount) throws IOException { - super.write(data, offset, byteCount); - sendProgressUpdate(); - } - - @Override - public void write(int data) throws IOException { - super.write(data); - sendProgressUpdate(); - } - - private void sendProgressUpdate() throws IOException { - long bytesWritten = getCount(); - long contentLength = contentLength(); - mProgressListener.onProgress( - bytesWritten, contentLength, bytesWritten == contentLength); - } - }); - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt new file mode 100644 index 00000000000000..def714d1dfec33 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt @@ -0,0 +1,72 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.modules.network + +import java.io.IOException +import okhttp3.MediaType +import okhttp3.RequestBody +import okio.BufferedSink +import okio.Okio +import okio.Okio.sink +import okio.Sink + +internal class ProgressRequestBody( + private val requestBody: RequestBody, + private val progressListener: ProgressListener +) : RequestBody() { + + private var _contentLength = 0L + + override fun contentType(): MediaType? = requestBody.contentType() + + @Throws(IOException::class) + override fun contentLength(): Long { + if (_contentLength == 0L) { + _contentLength = requestBody.contentLength() + } + return _contentLength + } + + @Throws(IOException::class) + override fun writeTo(sink: BufferedSink) { + // In 99% of cases, this method is called strictly once. + // The only case when it is called more than once is internal okhttp upload re-try. + // We need to re-create CountingOutputStream in this case as progress should be re-evaluated. + val sinkWrapper = Okio.buffer(outputStreamSink(sink)) + + // contentLength changes for input streams, since we're using inputStream.available(), + // so get the length before writing to the sink + contentLength() + requestBody.writeTo(sinkWrapper) + sinkWrapper.flush() + } + + private fun outputStreamSink(sink: BufferedSink): Sink = + Okio.sink( + object : CountingOutputStream(sink.outputStream()) { + @Throws(IOException::class) + override fun write(b: ByteArray, off: Int, len: Int) { + super.write(b, off, len) + sendProgressUpdate() + } + + @Throws(IOException::class) + override fun write(b: Int) { + super.write(b) + sendProgressUpdate() + } + + @Throws(IOException::class) + private fun sendProgressUpdate() { + val bytesWritten = count + val contentLength = contentLength() + progressListener.onProgress( + bytesWritten, contentLength, bytesWritten == contentLength) + } + }) +} From 7917093ef35569c09fa0e24da7c07aa853559f84 Mon Sep 17 00:00:00 2001 From: Thomas Nardone Date: Tue, 11 Jun 2024 07:30:10 -0700 Subject: [PATCH 4/4] Clean up Guava usage (CountingOutputStream) (#43942) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43942 Move the counting inline to remove its only usage Changelog: [Internal] Reviewed By: zeyap Differential Revision: D55802174 --- .../modules/network/CountingOutputStream.kt | 59 ------------------- .../modules/network/ProgressRequestBody.kt | 7 ++- 2 files changed, 6 insertions(+), 60 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt deleted file mode 100644 index 6a1c790bd82c7f..00000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/CountingOutputStream.kt +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright (C) 2007 The Guava Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.facebook.react.modules.network - -import java.io.FilterOutputStream -import java.io.IOException -import java.io.OutputStream - -/** - * An OutputStream that counts the number of bytes written. - * - * @author Chris Nokleberg - * @since 1.0 - */ -internal open class CountingOutputStream -/** - * Constructs a new `FilterOutputStream` with `out` as its target stream. - * - * @param out the target stream that this stream writes to. - */ -(out: OutputStream?) : FilterOutputStream(out) { - - /** Returns the number of bytes written. */ - var count: Long = 0 - private set - - @Throws(IOException::class) - override fun write(b: ByteArray, off: Int, len: Int) { - out.write(b, off, len) - count += len.toLong() - } - - @Throws(IOException::class) - override fun write(b: Int) { - out.write(b) - count++ - } - - // Overriding close() because FilterOutputStream's close() method pre-JDK8 has bad behavior: - // it silently ignores any exception thrown by flush(). Instead, just close the delegate stream. - // It should flush itself if necessary. - @Throws(IOException::class) - override fun close() { - out.close() - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt index def714d1dfec33..364484e09d706a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/ProgressRequestBody.kt @@ -7,6 +7,7 @@ package com.facebook.react.modules.network +import java.io.FilterOutputStream import java.io.IOException import okhttp3.MediaType import okhttp3.RequestBody @@ -48,16 +49,20 @@ internal class ProgressRequestBody( private fun outputStreamSink(sink: BufferedSink): Sink = Okio.sink( - object : CountingOutputStream(sink.outputStream()) { + object : FilterOutputStream(sink.outputStream()) { + private var count = 0L + @Throws(IOException::class) override fun write(b: ByteArray, off: Int, len: Int) { super.write(b, off, len) + count += len.toLong() sendProgressUpdate() } @Throws(IOException::class) override fun write(b: Int) { super.write(b) + count++ sendProgressUpdate() }