Convert OkHttpConnectionFactory and remove an exception class nobody was using

This commit is contained in:
Paul Hawke 2025-07-05 12:20:06 -05:00
parent b4c14776ee
commit ca89b656e8
7 changed files with 236 additions and 269 deletions

View file

@ -1,154 +0,0 @@
package fr.free.nrw.commons;
import androidx.annotation.NonNull;
import fr.free.nrw.commons.wikidata.cookies.CommonsCookieJar;
import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import okhttp3.Cache;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;
import okhttp3.logging.HttpLoggingInterceptor;
import okhttp3.logging.HttpLoggingInterceptor.Level;
import timber.log.Timber;
public final class OkHttpConnectionFactory {
private static final String CACHE_DIR_NAME = "okhttp-cache";
private static final long NET_CACHE_SIZE = 64 * 1024 * 1024;
public static OkHttpClient CLIENT;
@NonNull public static OkHttpClient getClient(final CommonsCookieJar cookieJar) {
if (CLIENT == null) {
CLIENT = createClient(cookieJar);
}
return CLIENT;
}
@NonNull
private static OkHttpClient createClient(final CommonsCookieJar cookieJar) {
return new OkHttpClient.Builder()
.cookieJar(cookieJar)
.cache((CommonsApplication.getInstance()!=null) ? new Cache(new File(CommonsApplication.getInstance().getCacheDir(), CACHE_DIR_NAME), NET_CACHE_SIZE) : null)
.connectTimeout(120, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.addInterceptor(getLoggingInterceptor())
.addInterceptor(new UnsuccessfulResponseInterceptor())
.addInterceptor(new CommonHeaderRequestInterceptor())
.build();
}
private static HttpLoggingInterceptor getLoggingInterceptor() {
final HttpLoggingInterceptor httpLoggingInterceptor = new HttpLoggingInterceptor()
.setLevel(Level.BASIC);
httpLoggingInterceptor.redactHeader("Authorization");
httpLoggingInterceptor.redactHeader("Cookie");
return httpLoggingInterceptor;
}
private static class CommonHeaderRequestInterceptor implements Interceptor {
@Override
@NonNull
public Response intercept(@NonNull final Chain chain) throws IOException {
final Request request = chain.request().newBuilder()
.header("User-Agent", CommonsApplication.getInstance().getUserAgent())
.build();
return chain.proceed(request);
}
}
public static class UnsuccessfulResponseInterceptor implements Interceptor {
private static final String SUPPRESS_ERROR_LOG = "x-commons-suppress-error-log";
public static final String SUPPRESS_ERROR_LOG_HEADER = SUPPRESS_ERROR_LOG+": true";
private static final List<String> DO_NOT_INTERCEPT = Collections.singletonList(
"api.php?format=json&formatversion=2&errorformat=plaintext&action=upload&ignorewarnings=1");
private static final String ERRORS_PREFIX = "{\"error";
@Override
@NonNull
public Response intercept(@NonNull final Chain chain) throws IOException {
final Request rq = chain.request();
// If the request contains our special "suppress errors" header, make note of it
// but don't pass that on to the server.
final boolean suppressErrors = rq.headers().names().contains(SUPPRESS_ERROR_LOG);
final Request request = rq.newBuilder()
.removeHeader(SUPPRESS_ERROR_LOG)
.build();
final Response rsp = chain.proceed(request);
// Do not intercept certain requests and let the caller handle the errors
if(isExcludedUrl(chain.request())) {
return rsp;
}
if (rsp.isSuccessful()) {
try (final ResponseBody responseBody = rsp.peekBody(ERRORS_PREFIX.length())) {
if (ERRORS_PREFIX.equals(responseBody.string())) {
try (final ResponseBody body = rsp.body()) {
throw new IOException(body.string());
}
}
} catch (final IOException e) {
// Log the error as debug (and therefore, "expected") or at error level
if (suppressErrors) {
Timber.d(e, "Suppressed (known / expected) error");
} else {
Timber.e(e);
}
}
return rsp;
}
throw new HttpStatusException(rsp);
}
private boolean isExcludedUrl(final Request request) {
final String requestUrl = request.url().toString();
for(final String url: DO_NOT_INTERCEPT) {
if(requestUrl.contains(url)) {
return true;
}
}
return false;
}
}
private OkHttpConnectionFactory() {
}
public static class HttpStatusException extends IOException {
private final int code;
private final String url;
public HttpStatusException(@NonNull Response rsp) {
this.code = rsp.code();
this.url = rsp.request().url().uri().toString();
try {
if (rsp.body() != null && rsp.body().contentType() != null
&& rsp.body().contentType().toString().contains("json")) {
}
} catch (Exception e) {
// Log?
}
}
public int code() {
return code;
}
@Override
public String getMessage() {
String str = "Code: " + code + ", URL: " + url;
return str;
}
}
}

View file

@ -0,0 +1,122 @@
package fr.free.nrw.commons
import androidx.annotation.VisibleForTesting
import fr.free.nrw.commons.wikidata.cookies.CommonsCookieJar
import okhttp3.Cache
import okhttp3.Interceptor
import okhttp3.OkHttpClient
import okhttp3.Request
import okhttp3.Response
import okhttp3.logging.HttpLoggingInterceptor
import timber.log.Timber
import java.io.File
import java.io.IOException
import java.util.concurrent.TimeUnit
object OkHttpConnectionFactory {
private const val CACHE_DIR_NAME = "okhttp-cache"
private const val NET_CACHE_SIZE = (64 * 1024 * 1024).toLong()
@VisibleForTesting
var CLIENT: OkHttpClient? = null
fun getClient(cookieJar: CommonsCookieJar): OkHttpClient {
if (CLIENT == null) {
CLIENT = createClient(cookieJar)
}
return CLIENT!!
}
private fun createClient(cookieJar: CommonsCookieJar): OkHttpClient {
return OkHttpClient.Builder()
.cookieJar(cookieJar)
.cache(
if (CommonsApplication.instance != null) Cache(
File(CommonsApplication.instance.cacheDir, CACHE_DIR_NAME),
NET_CACHE_SIZE
) else null
)
.connectTimeout(120, TimeUnit.SECONDS)
.writeTimeout(120, TimeUnit.SECONDS)
.readTimeout(120, TimeUnit.SECONDS)
.addInterceptor(HttpLoggingInterceptor().apply {
setLevel(HttpLoggingInterceptor.Level.BASIC)
redactHeader("Authorization")
redactHeader("Cookie")
})
.addInterceptor(UnsuccessfulResponseInterceptor())
.addInterceptor(CommonHeaderRequestInterceptor())
.build()
}
}
private class CommonHeaderRequestInterceptor : Interceptor {
@Throws(IOException::class)
override fun intercept(chain: Interceptor.Chain): Response {
val request = chain.request().newBuilder()
.header("User-Agent", CommonsApplication.instance.userAgent)
.build()
return chain.proceed(request)
}
}
private const val SUPPRESS_ERROR_LOG = "x-commons-suppress-error-log"
const val SUPPRESS_ERROR_LOG_HEADER: String = "$SUPPRESS_ERROR_LOG: true"
private class UnsuccessfulResponseInterceptor : Interceptor {
@Throws(IOException::class)
override fun intercept(chain: Interceptor.Chain): Response {
val rq = chain.request()
// If the request contains our special "suppress errors" header, make note of it
// but don't pass that on to the server.
val suppressErrors = rq.headers.names().contains(SUPPRESS_ERROR_LOG)
val request = rq.newBuilder()
.removeHeader(SUPPRESS_ERROR_LOG)
.build()
val rsp = chain.proceed(request)
// Do not intercept certain requests and let the caller handle the errors
if (isExcludedUrl(chain.request())) {
return rsp
}
if (rsp.isSuccessful) {
try {
rsp.peekBody(ERRORS_PREFIX.length.toLong()).use { responseBody ->
if (ERRORS_PREFIX == responseBody.string()) {
rsp.body.use { body ->
throw IOException(body!!.string())
}
}
}
} catch (e: IOException) {
// Log the error as debug (and therefore, "expected") or at error level
if (suppressErrors) {
Timber.d(e, "Suppressed (known / expected) error")
} else {
Timber.e(e)
}
}
return rsp
}
throw IOException("Unsuccessful response")
}
private fun isExcludedUrl(request: Request): Boolean {
val requestUrl = request.url.toString()
for (url in DO_NOT_INTERCEPT) {
if (requestUrl.contains(url)) {
return true
}
}
return false
}
companion object {
val DO_NOT_INTERCEPT = listOf(
"api.php?format=json&formatversion=2&errorformat=plaintext&action=upload&ignorewarnings=1"
)
const val ERRORS_PREFIX = "{\"error"
}
}

View file

@ -1,6 +1,6 @@
package fr.free.nrw.commons.media
import fr.free.nrw.commons.OkHttpConnectionFactory.UnsuccessfulResponseInterceptor.SUPPRESS_ERROR_LOG_HEADER
import fr.free.nrw.commons.SUPPRESS_ERROR_LOG_HEADER
import fr.free.nrw.commons.wikidata.mwapi.MwQueryResponse
import io.reactivex.Single
import retrofit2.http.GET

View file

@ -1,110 +0,0 @@
package fr.free.nrw.commons;
import static fr.free.nrw.commons.TestConnectionFactoryKt.createTestClient;
import androidx.annotation.NonNull;
import java.util.List;
import java.util.concurrent.AbstractExecutorService;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import okhttp3.Dispatcher;
import okhttp3.OkHttpClient;
import okhttp3.mockwebserver.MockResponse;
import org.junit.After;
import org.junit.Before;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import fr.free.nrw.commons.wikidata.GsonUtil;
import retrofit2.Retrofit;
import retrofit2.converter.gson.GsonConverterFactory;
@RunWith(RobolectricTestRunner.class)
public abstract class MockWebServerTest {
private OkHttpClient okHttpClient;
private final TestWebServer server = new TestWebServer();
@Before public void setUp() throws Throwable {
OkHttpConnectionFactory.CLIENT = createTestClient();
okHttpClient = OkHttpConnectionFactory.CLIENT.newBuilder()
.dispatcher(new Dispatcher(new ImmediateExecutorService())).build();
server.setUp();
}
@After public void tearDown() throws Throwable {
server.tearDown();
}
@NonNull protected TestWebServer server() {
return server;
}
protected void enqueueFromFile(@NonNull String filename) throws Throwable {
String json = TestFileUtil.readRawFile(filename);
server.enqueue(json);
}
protected void enqueue404() {
final int code = 404;
server.enqueue(new MockResponse().setResponseCode(code).setBody("Not Found"));
}
protected void enqueueMalformed() {
server.enqueue("(╯°□°)╯︵ ┻━┻");
}
protected void enqueueEmptyJson() {
server.enqueue(new MockResponse().setBody("{}"));
}
@NonNull protected OkHttpClient okHttpClient() {
return okHttpClient;
}
@NonNull protected <T> T service(Class<T> clazz) {
return service(clazz, server().getUrl());
}
@NonNull protected <T> T service(Class<T> clazz, @NonNull String url) {
return new Retrofit.Builder()
.baseUrl(url)
.callbackExecutor(new ImmediateExecutor())
.client(okHttpClient)
.addConverterFactory(GsonConverterFactory.create(GsonUtil.INSTANCE.getDefaultGson()))
.build()
.create(clazz);
}
public final class ImmediateExecutorService extends AbstractExecutorService {
@Override public void shutdown() {
throw new UnsupportedOperationException();
}
@NonNull @Override public List<Runnable> shutdownNow() {
throw new UnsupportedOperationException();
}
@Override public boolean isShutdown() {
throw new UnsupportedOperationException();
}
@Override public boolean isTerminated() {
throw new UnsupportedOperationException();
}
@Override public boolean awaitTermination(long l, @NonNull TimeUnit timeUnit)
throws InterruptedException {
throw new UnsupportedOperationException();
}
@Override public void execute(@NonNull Runnable runnable) {
runnable.run();
}
}
public class ImmediateExecutor implements Executor {
@Override
public void execute(@NonNull Runnable runnable) {
runnable.run();
}
}
}

View file

@ -0,0 +1,110 @@
package fr.free.nrw.commons
import fr.free.nrw.commons.wikidata.GsonUtil.defaultGson
import okhttp3.Dispatcher
import okhttp3.OkHttpClient
import okhttp3.mockwebserver.MockResponse
import org.junit.After
import org.junit.Before
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import retrofit2.Retrofit
import retrofit2.converter.gson.GsonConverterFactory
import java.util.concurrent.AbstractExecutorService
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit
@RunWith(RobolectricTestRunner::class)
abstract class MockWebServerTest {
private var okHttpClient: OkHttpClient? = null
private val server = TestWebServer()
@Before
@Throws(Throwable::class)
open fun setUp() {
OkHttpConnectionFactory.CLIENT = createTestClient()
okHttpClient = OkHttpConnectionFactory.CLIENT!!.newBuilder()
.dispatcher(Dispatcher(ImmediateExecutorService())).build()
server.setUp()
}
@After
@Throws(Throwable::class)
fun tearDown() {
server.tearDown()
}
protected fun server(): TestWebServer {
return server
}
@Throws(Throwable::class)
protected fun enqueueFromFile(filename: String) {
val json = TestFileUtil.readRawFile(filename)
server.enqueue(json)
}
protected fun enqueue404() {
val code = 404
server.enqueue(MockResponse().setResponseCode(code).setBody("Not Found"))
}
protected fun enqueueMalformed() {
server.enqueue("(╯°□°)╯︵ ┻━┻")
}
protected fun enqueueEmptyJson() {
server.enqueue(MockResponse().setBody("{}"))
}
protected fun okHttpClient(): OkHttpClient {
return okHttpClient!!
}
protected fun <T> service(clazz: Class<T>): T {
return service(clazz, server().url)
}
protected fun <T> service(clazz: Class<T>, url: String): T {
return Retrofit.Builder()
.baseUrl(url)
.callbackExecutor(ImmediateExecutor())
.client(okHttpClient)
.addConverterFactory(GsonConverterFactory.create(defaultGson))
.build()
.create(clazz)
}
inner class ImmediateExecutorService : AbstractExecutorService() {
override fun shutdown() {
throw UnsupportedOperationException()
}
override fun shutdownNow(): List<Runnable> {
throw UnsupportedOperationException()
}
override fun isShutdown(): Boolean {
throw UnsupportedOperationException()
}
override fun isTerminated(): Boolean {
throw UnsupportedOperationException()
}
@Throws(InterruptedException::class)
override fun awaitTermination(l: Long, timeUnit: TimeUnit): Boolean {
throw UnsupportedOperationException()
}
override fun execute(runnable: Runnable) {
runnable.run()
}
}
inner class ImmediateExecutor : Executor {
override fun execute(runnable: Runnable) {
runnable.run()
}
}
}

View file

@ -1,6 +1,5 @@
package fr.free.nrw.commons
import fr.free.nrw.commons.OkHttpConnectionFactory.HttpStatusException
import okhttp3.Interceptor
import okhttp3.OkHttpClient
import okhttp3.Response
@ -39,6 +38,6 @@ private class UnsuccessfulResponseInterceptor : Interceptor {
if (rsp.isSuccessful) {
return rsp
}
throw HttpStatusException(rsp)
throw IOException("Unsuccessful response")
}
}

View file

@ -2,7 +2,6 @@ package fr.free.nrw.commons.auth.csrf
import com.google.gson.stream.MalformedJsonException
import fr.free.nrw.commons.MockWebServerTest
import fr.free.nrw.commons.OkHttpConnectionFactory.HttpStatusException
import fr.free.nrw.commons.auth.SessionManager
import fr.free.nrw.commons.auth.login.LoginClient
import fr.free.nrw.commons.wikidata.mwapi.MwException
@ -13,6 +12,7 @@ import org.mockito.ArgumentMatchers.isA
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import java.io.IOException
class CsrfTokenClientTest : MockWebServerTest() {
private val cb = mock(CsrfTokenClient.Callback::class.java)
@ -53,7 +53,7 @@ class CsrfTokenClientTest : MockWebServerTest() {
performRequest()
verify(cb, never()).success(any(String::class.java))
verify(cb).failure(isA(HttpStatusException::class.java))
verify(cb).failure(isA(IOException::class.java))
}
@Test