[Not to land] Testing Android HttpEngine with Call Decorator #9013
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| /* | ||
| * Copyright (C) 2025 Block, Inc. | ||
| * | ||
| * 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 okhttp.android.test | ||
|
|
||
| import android.content.Context | ||
| import android.net.http.ConnectionMigrationOptions | ||
| import android.net.http.ConnectionMigrationOptions.MIGRATION_OPTION_ENABLED | ||
| import android.net.http.DnsOptions | ||
| import android.net.http.DnsOptions.DNS_OPTION_ENABLED | ||
| import android.net.http.HttpEngine | ||
| import android.net.http.QuicOptions | ||
| import androidx.test.core.app.ApplicationProvider | ||
| import androidx.test.filters.SdkSuppress | ||
| import kotlinx.coroutines.test.runTest | ||
| import okhttp3.Cache | ||
| import okhttp3.HttpUrl.Companion.toHttpUrl | ||
| import okhttp3.OkHttpClient | ||
| import okhttp3.Request | ||
| import okhttp3.android.httpengine.HttpEngineCallDecorator.Companion.callDecorator | ||
| import okhttp3.coroutines.executeAsync | ||
| import okio.Path.Companion.toPath | ||
| import okio.fakefilesystem.FakeFileSystem | ||
| import org.junit.Test | ||
|
|
||
| @SdkSuppress(minSdkVersion = 34) | ||
| class HttpEngineBridgeTest { | ||
| val context = ApplicationProvider.getApplicationContext<Context>() | ||
|
|
||
| val httpEngine = | ||
| HttpEngine | ||
| .Builder(context) | ||
| .setStoragePath( | ||
| context.filesDir | ||
| .resolve("httpEngine") | ||
| .apply { | ||
| mkdirs() | ||
| }.path, | ||
| ).setConnectionMigrationOptions( | ||
| ConnectionMigrationOptions | ||
| .Builder() | ||
| .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) | ||
| .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) | ||
| .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) | ||
| .build(), | ||
| ).addQuicHint("www.google.com", 443, 443) | ||
| .addQuicHint("google.com", 443, 443) | ||
| .setDnsOptions( | ||
| DnsOptions | ||
| .Builder() | ||
| .setPersistHostCache(DNS_OPTION_ENABLED) | ||
| .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) | ||
| .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) | ||
| .setStaleDnsOptions( | ||
| DnsOptions.StaleDnsOptions | ||
| .Builder() | ||
| .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) | ||
| .build(), | ||
| ).build(), | ||
| ).setEnableQuic(true) | ||
| .setQuicOptions( | ||
| QuicOptions | ||
| .Builder() | ||
| .addAllowedQuicHost("www.google.com") | ||
| .addAllowedQuicHost("google.com") | ||
| .build(), | ||
| ).build() | ||
|
|
||
| var client = | ||
| OkHttpClient | ||
| .Builder() | ||
| .addCallDecorator(httpEngine.callDecorator) | ||
| .build() | ||
|
|
||
| val imageUrls = | ||
| listOf( | ||
| "https://storage.googleapis.com/cronet/sun.jpg", | ||
| "https://storage.googleapis.com/cronet/flower.jpg", | ||
| "https://storage.googleapis.com/cronet/chair.jpg", | ||
| "https://storage.googleapis.com/cronet/white.jpg", | ||
| "https://storage.googleapis.com/cronet/moka.jpg", | ||
| "https://storage.googleapis.com/cronet/walnut.jpg", | ||
| ).map { it.toHttpUrl() } | ||
|
|
||
| @Test | ||
| fun testNewCall() = | ||
| runTest { | ||
| val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) | ||
|
|
||
| val response = call.executeAsync() | ||
|
|
||
| println(response.body.string().take(40)) | ||
|
|
||
| val call2 = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) | ||
|
|
||
| val response2 = call2.executeAsync() | ||
|
|
||
| println(response2.body.string().take(40)) | ||
| println(response2.protocol) | ||
| } | ||
|
|
||
| @Test | ||
| fun testWithCache() = | ||
| runTest { | ||
| client = | ||
| client | ||
| .newBuilder() | ||
| .cache(Cache(FakeFileSystem(), "/cache".toPath(), 100_000_000)) | ||
| .build() | ||
|
|
||
| repeat(10) { | ||
| imageUrls.forEach { | ||
| val call = client.newCall(Request(it)) | ||
|
|
||
| val response = call.executeAsync() | ||
|
|
||
| println( | ||
| "${response.request.url} cached=${response.cacheResponse != null} " + | ||
| response.body | ||
| .byteString() | ||
| .md5() | ||
| .hex(), | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| package okhttp3.android.httpengine | ||
|
|
||
| import android.annotation.SuppressLint | ||
| import android.net.http.HttpEngine | ||
| import android.os.Build | ||
| import android.os.ext.SdkExtensions | ||
| import androidx.annotation.RequiresExtension | ||
| import okhttp3.Call | ||
| import okhttp3.Interceptor | ||
| import okhttp3.OkHttpClient | ||
| import okhttp3.Request | ||
| import okhttp3.internal.SuppressSignatureCheck | ||
| import okhttp3.internal.cache.CacheInterceptor | ||
| import okhttp3.internal.http.BridgeInterceptor | ||
| import okhttp3.internal.http.RetryAndFollowUpInterceptor | ||
|
|
||
| @SuppressSignatureCheck | ||
| class HttpEngineCallDecorator( | ||
| internal val httpEngine: HttpEngine, | ||
|
There was a problem hiding this comment. It is debatable whether we want to let the user choose which HttpEngine to use. Arguably this code should instantiate HttpEngine and should configure it by interpreting OkHttpClient settings. Otherwise we may end up backing ourselves into a corner by providing too much flexibility to the user. There was a problem hiding this comment. Yep, I'm not aware of what we expect an app developer to configure in typical cases? Do we want them to allowlist their known hosts for HTTP/3? But no objection from me. Your call. |
||
| private val useHttpEngine: (Request) -> Boolean = { isHttpEngineSupported() }, | ||
| ) : Call.Decorator { | ||
| // TODO make this work with forked clients | ||
| internal lateinit var client: OkHttpClient | ||
|
|
||
| @SuppressLint("NewApi") | ||
| private val httpEngineInterceptor = HttpEngineInterceptor(this) | ||
|
|
||
| override fun newCall(chain: Call.Chain): Call { | ||
| val call = httpEngineCall(chain) | ||
|
|
||
| return call ?: chain.proceed(chain.request) | ||
| } | ||
|
|
||
| @SuppressLint("NewApi") | ||
| @Synchronized | ||
| private fun httpEngineCall(chain: Call.Chain): Call? { | ||
| if (!useHttpEngine(chain.request)) { | ||
| return null | ||
| } | ||
|
|
||
| if (!::client.isInitialized) { | ||
| val originalClient = chain.client | ||
| client = | ||
| originalClient | ||
| .newBuilder() | ||
| .apply { | ||
| networkInterceptors.clear() | ||
|
|
||
| // TODO refactor RetryAndFollowUpInterceptor to not require the Client directly | ||
| interceptors += RetryAndFollowUpInterceptor(originalClient) | ||
| interceptors += BridgeInterceptor(originalClient.cookieJar) | ||
| interceptors += CacheInterceptor(originalClient.cache) | ||
| interceptors += httpEngineInterceptor | ||
| interceptors += | ||
| Interceptor { | ||
| throw IllegalStateException("Shouldn't attempt to connect with OkHttp") | ||
| } | ||
|
|
||
| // Keep decorators after this one in the new client | ||
| callDecorators.subList(0, callDecorators.indexOf(this@HttpEngineCallDecorator) + 1).clear() | ||
| }.build() | ||
| } | ||
|
|
||
| return HttpEngineCall(client.newCall(chain.request)) | ||
| } | ||
|
|
||
| @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) | ||
| inner class HttpEngineCall( | ||
| val realCall: Call, | ||
| ) : Call by realCall { | ||
| val httpEngine: HttpEngine | ||
| get() = [email protected] | ||
|
|
||
| override fun cancel() { | ||
| realCall.cancel() | ||
| httpEngineInterceptor.cancelCall(realCall) | ||
| } | ||
| } | ||
|
|
||
| companion object { | ||
| val HttpEngine.callDecorator | ||
| get() = HttpEngineCallDecorator(this) | ||
| } | ||
| } | ||
|
|
||
| private fun isHttpEngineSupported(): Boolean = | ||
| Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && SdkExtensions.getExtensionVersion(Build.VERSION_CODES.S) >= 7 | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is debatable whether this should use HttpEngine directly, or if it should use the Cronet API wrapper (which would then call HttpEngine). The Cronet API wrapper is more flexible and would make it possible to use other variants of Cronet (such as embedded of Play Services) if we ever want to, but the downside is it would involve OkHttp taking a new dependency on the (very small) cronet-api Maven package.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that dependency is the blocker here.
But also, a first version of this should probably be a v2.0 of the google/cronet-transport-for-okhttp
So that can do whatever makes sense?