From ab9e57f5be7c15513372a523cadb61022e350f00 Mon Sep 17 00:00:00 2001 From: Paul Hawke Date: Mon, 29 Jan 2024 16:46:01 -0600 Subject: [PATCH] Decouple from the data-client service factory, with some code cleanup in the process (#5496) --- .../free/nrw/commons/CommonsAppAdapter.java | 3 +- .../free/nrw/commons/auth/LoginActivity.java | 4 - .../free/nrw/commons/di/NetworkingModule.java | 94 +++++++++---------- .../commons/wikidata/CommonsServiceFactory.kt | 25 +++++ .../free/nrw/commons/MockWebServerTest.java | 5 +- .../fr/free/nrw/commons/TestAppAdapter.java | 3 +- .../main/java/org/wikipedia/AppAdapter.java | 3 +- .../wikipedia/dataclient/ServiceFactory.java | 2 +- .../java/org/wikipedia/TestAppAdapter.java | 3 +- .../org/wikipedia/test/MockWebServerTest.java | 4 +- 10 files changed, 75 insertions(+), 71 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/wikidata/CommonsServiceFactory.kt diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsAppAdapter.java b/app/src/main/java/fr/free/nrw/commons/CommonsAppAdapter.java index 1439147ec..57cbf13b1 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsAppAdapter.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsAppAdapter.java @@ -6,7 +6,6 @@ import fr.free.nrw.commons.kvstore.JsonKvStore; import okhttp3.OkHttpClient; import org.wikipedia.AppAdapter; import org.wikipedia.dataclient.SharedPreferenceCookieManager; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonMarshaller; import org.wikipedia.json.GsonUnmarshaller; @@ -33,7 +32,7 @@ public class CommonsAppAdapter extends AppAdapter { } @Override - public OkHttpClient getOkHttpClient(@NonNull WikiSite wikiSite) { + public OkHttpClient getOkHttpClient() { return OkHttpConnectionFactory.getClient(); } diff --git a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java index bf7183de4..40646bc35 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java @@ -26,13 +26,10 @@ import androidx.core.app.NavUtils; import androidx.core.content.ContextCompat; import fr.free.nrw.commons.auth.login.LoginClient; -import fr.free.nrw.commons.auth.login.LoginInterface; import fr.free.nrw.commons.auth.login.LoginResult; import fr.free.nrw.commons.databinding.ActivityLoginBinding; import fr.free.nrw.commons.utils.ActivityUtils; import java.util.Locale; -import org.wikipedia.dataclient.ServiceFactory; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.dataclient.mwapi.MwQueryResponse; import fr.free.nrw.commons.auth.login.LoginCallback; @@ -57,7 +54,6 @@ import timber.log.Timber; import static android.view.KeyEvent.KEYCODE_ENTER; import static android.view.View.VISIBLE; import static android.view.inputmethod.EditorInfo.IME_ACTION_DONE; -import static fr.free.nrw.commons.di.NetworkingModule.NAMED_COMMONS_WIKI_SITE; public class LoginActivity extends AccountAuthenticatorActivity { diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java index 75113cca6..9d6459d45 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java @@ -7,6 +7,7 @@ import dagger.Module; import dagger.Provides; import fr.free.nrw.commons.BetaConstants; import fr.free.nrw.commons.BuildConfig; +import fr.free.nrw.commons.OkHttpConnectionFactory; import fr.free.nrw.commons.actions.PageEditClient; import fr.free.nrw.commons.actions.PageEditInterface; import fr.free.nrw.commons.actions.ThanksInterface; @@ -27,6 +28,7 @@ import fr.free.nrw.commons.review.ReviewInterface; import fr.free.nrw.commons.upload.UploadInterface; import fr.free.nrw.commons.upload.WikiBaseInterface; import fr.free.nrw.commons.upload.depicts.DepictsInterface; +import fr.free.nrw.commons.wikidata.CommonsServiceFactory; import fr.free.nrw.commons.wikidata.WikidataInterface; import java.io.File; import java.util.Locale; @@ -39,7 +41,7 @@ import okhttp3.OkHttpClient; import okhttp3.logging.HttpLoggingInterceptor; import okhttp3.logging.HttpLoggingInterceptor.Level; import fr.free.nrw.commons.auth.csrf.CsrfTokenClient; -import org.wikipedia.dataclient.ServiceFactory; +import org.wikipedia.AppAdapter; import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonUtil; import fr.free.nrw.commons.auth.login.LoginClient; @@ -53,7 +55,6 @@ public class NetworkingModule { public static final long OK_HTTP_CACHE_SIZE = 10 * 1024 * 1024; - public static final String NAMED_COMMONS_WIKI_SITE = "commons-wikisite"; private static final String NAMED_WIKI_DATA_WIKI_SITE = "wikidata-wikisite"; private static final String NAMED_WIKI_PEDIA_WIKI_SITE = "wikipedia-wikisite"; @@ -75,6 +76,12 @@ public class NetworkingModule { .build(); } + @Provides + @Singleton + public CommonsServiceFactory serviceFactory() { + return new CommonsServiceFactory(AppAdapter.get().getOkHttpClient()); + } + @Provides @Singleton public HttpLoggingInterceptor provideHttpLoggingInterceptor() { @@ -110,14 +117,14 @@ public class NetworkingModule { @Provides @Singleton - public CsrfTokenInterface provideCsrfTokenInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, CsrfTokenInterface.class); + public CsrfTokenInterface provideCsrfTokenInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, CsrfTokenInterface.class); } @Provides @Singleton - public LoginInterface provideLoginInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, LoginInterface.class); + public LoginInterface provideLoginInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, LoginInterface.class); } @Provides @@ -142,13 +149,6 @@ public class NetworkingModule { return HttpUrl.parse(TOOLS_FORGE_URL); } - @Provides - @Singleton - @Named(NAMED_COMMONS_WIKI_SITE) - public WikiSite provideCommonsWikiSite() { - return new WikiSite(BuildConfig.COMMONS_URL); - } - @Provides @Singleton @Named(NAMED_WIKI_DATA_WIKI_SITE) @@ -169,40 +169,40 @@ public class NetworkingModule { @Provides @Singleton - public ReviewInterface provideReviewInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, ReviewInterface.class); + public ReviewInterface provideReviewInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, ReviewInterface.class); } @Provides @Singleton - public DepictsInterface provideDepictsInterface(@Named(NAMED_WIKI_DATA_WIKI_SITE) WikiSite wikidataWikiSite) { - return ServiceFactory.get(wikidataWikiSite, BuildConfig.WIKIDATA_URL, DepictsInterface.class); + public DepictsInterface provideDepictsInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.WIKIDATA_URL, DepictsInterface.class); } @Provides @Singleton - public WikiBaseInterface provideWikiBaseInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, WikiBaseInterface.class); + public WikiBaseInterface provideWikiBaseInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, WikiBaseInterface.class); } @Provides @Singleton - public UploadInterface provideUploadInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, UploadInterface.class); + public UploadInterface provideUploadInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, UploadInterface.class); } @Named("commons-page-edit-service") @Provides @Singleton - public PageEditInterface providePageEditService(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, PageEditInterface.class); + public PageEditInterface providePageEditService(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, PageEditInterface.class); } @Named("wikidata-page-edit-service") @Provides @Singleton - public PageEditInterface provideWikiDataPageEditService(@Named(NAMED_WIKI_DATA_WIKI_SITE) WikiSite wikiDataWikiSite) { - return ServiceFactory.get(wikiDataWikiSite, BuildConfig.WIKIDATA_URL, PageEditInterface.class); + public PageEditInterface provideWikiDataPageEditService(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.WIKIDATA_URL, PageEditInterface.class); } @Named("commons-page-edit") @@ -215,8 +215,8 @@ public class NetworkingModule { @Provides @Singleton - public MediaInterface provideMediaInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, MediaInterface.class); + public MediaInterface provideMediaInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, MediaInterface.class); } /** @@ -227,52 +227,44 @@ public class NetworkingModule { */ @Provides @Singleton - public WikidataMediaInterface provideWikidataMediaInterface( - @Named(NAMED_COMMONS_WIKI_SITE) final WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, - BetaConstants.COMMONS_URL, WikidataMediaInterface.class); + public WikidataMediaInterface provideWikidataMediaInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BetaConstants.COMMONS_URL, WikidataMediaInterface.class); } @Provides @Singleton - public MediaDetailInterface providesMediaDetailInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikisite) { - return ServiceFactory.get(commonsWikisite, BuildConfig.COMMONS_URL, MediaDetailInterface.class); + public MediaDetailInterface providesMediaDetailInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, MediaDetailInterface.class); } @Provides @Singleton - public CategoryInterface provideCategoryInterface( - @Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory - .get(commonsWikiSite, BuildConfig.COMMONS_URL, CategoryInterface.class); + public CategoryInterface provideCategoryInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, CategoryInterface.class); } @Provides @Singleton - public ThanksInterface provideThanksInterface( - @Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory - .get(commonsWikiSite, BuildConfig.COMMONS_URL, ThanksInterface.class); + public ThanksInterface provideThanksInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, ThanksInterface.class); } @Provides @Singleton - public NotificationInterface provideNotificationInterface( - @Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory - .get(commonsWikiSite, BuildConfig.COMMONS_URL, NotificationInterface.class); + public NotificationInterface provideNotificationInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, NotificationInterface.class); } @Provides @Singleton - public UserInterface provideUserInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { - return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, UserInterface.class); + public UserInterface provideUserInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.COMMONS_URL, UserInterface.class); } @Provides @Singleton - public WikidataInterface provideWikidataInterface(@Named(NAMED_WIKI_DATA_WIKI_SITE) WikiSite wikiDataWikiSite) { - return ServiceFactory.get(wikiDataWikiSite, BuildConfig.WIKIDATA_URL, WikidataInterface.class); + public WikidataInterface provideWikidataInterface(CommonsServiceFactory serviceFactory) { + return serviceFactory.create(BuildConfig.WIKIDATA_URL, WikidataInterface.class); } /** @@ -281,8 +273,8 @@ public class NetworkingModule { */ @Provides @Singleton - public PageMediaInterface providePageMediaInterface(@Named(NAMED_LANGUAGE_WIKI_PEDIA_WIKI_SITE) WikiSite wikiSite) { - return ServiceFactory.get(wikiSite, wikiSite.url(), PageMediaInterface.class); + public PageMediaInterface providePageMediaInterface(@Named(NAMED_LANGUAGE_WIKI_PEDIA_WIKI_SITE) WikiSite wikiSite, CommonsServiceFactory serviceFactory) { + return serviceFactory.create(wikiSite.url(), PageMediaInterface.class); } @Provides diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/CommonsServiceFactory.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/CommonsServiceFactory.kt new file mode 100644 index 000000000..8a46c5de3 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/CommonsServiceFactory.kt @@ -0,0 +1,25 @@ +package fr.free.nrw.commons.wikidata + +import okhttp3.OkHttpClient +import org.wikipedia.json.GsonUtil +import retrofit2.Retrofit +import retrofit2.adapter.rxjava2.RxJava2CallAdapterFactory +import retrofit2.converter.gson.GsonConverterFactory + +class CommonsServiceFactory(private val okHttpClient: OkHttpClient) { + + private val builder: Retrofit.Builder by lazy { + // All instances of retrofit share this configuration, but create it lazily + Retrofit.Builder().client(okHttpClient) + .addCallAdapterFactory(RxJava2CallAdapterFactory.create()) + .addConverterFactory(GsonConverterFactory.create(GsonUtil.getDefaultGson())) + } + + private val retrofitCache: MutableMap = mutableMapOf() + + fun create(baseUrl: String, service: Class): T = retrofitCache.getOrPut(baseUrl) { + // Cache instances of retrofit based on API backend + builder.baseUrl(baseUrl).build() + }.create(service) + +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/MockWebServerTest.java b/app/src/test/kotlin/fr/free/nrw/commons/MockWebServerTest.java index 5f884a8bd..667e35e63 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/MockWebServerTest.java +++ b/app/src/test/kotlin/fr/free/nrw/commons/MockWebServerTest.java @@ -1,7 +1,5 @@ package fr.free.nrw.commons; -import static fr.free.nrw.commons.wikidata.WikidataConstants.WIKIPEDIA_URL; - import androidx.annotation.NonNull; import java.util.List; import java.util.concurrent.AbstractExecutorService; @@ -15,7 +13,6 @@ import org.junit.Before; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.wikipedia.AppAdapter; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonUtil; import retrofit2.Retrofit; import retrofit2.converter.gson.GsonConverterFactory; @@ -27,7 +24,7 @@ public abstract class MockWebServerTest { @Before public void setUp() throws Throwable { AppAdapter.set(new TestAppAdapter()); - OkHttpClient.Builder builder = AppAdapter.get().getOkHttpClient(new WikiSite(WIKIPEDIA_URL)).newBuilder(); + OkHttpClient.Builder builder = AppAdapter.get().getOkHttpClient().newBuilder(); okHttpClient = builder.dispatcher(new Dispatcher(new ImmediateExecutorService())).build(); server.setUp(); } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/TestAppAdapter.java b/app/src/test/kotlin/fr/free/nrw/commons/TestAppAdapter.java index a83219fbb..051646449 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/TestAppAdapter.java +++ b/app/src/test/kotlin/fr/free/nrw/commons/TestAppAdapter.java @@ -5,7 +5,6 @@ import fr.free.nrw.commons.wikidata.WikidataConstants; import okhttp3.OkHttpClient; import org.wikipedia.AppAdapter; import org.wikipedia.dataclient.SharedPreferenceCookieManager; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.dataclient.okhttp.TestStubInterceptor; import org.wikipedia.dataclient.okhttp.UnsuccessfulResponseInterceptor; @@ -22,7 +21,7 @@ public class TestAppAdapter extends AppAdapter { } @Override - public OkHttpClient getOkHttpClient(@NonNull WikiSite wikiSite) { + public OkHttpClient getOkHttpClient() { return new OkHttpClient.Builder() .addInterceptor(new UnsuccessfulResponseInterceptor()) .addInterceptor(new TestStubInterceptor()) diff --git a/data-client/src/main/java/org/wikipedia/AppAdapter.java b/data-client/src/main/java/org/wikipedia/AppAdapter.java index aa83cf409..f89bcae51 100644 --- a/data-client/src/main/java/org/wikipedia/AppAdapter.java +++ b/data-client/src/main/java/org/wikipedia/AppAdapter.java @@ -3,7 +3,6 @@ package org.wikipedia; import androidx.annotation.NonNull; import org.wikipedia.dataclient.SharedPreferenceCookieManager; -import org.wikipedia.dataclient.WikiSite; import okhttp3.OkHttpClient; @@ -11,7 +10,7 @@ public abstract class AppAdapter { public abstract String getMediaWikiBaseUrl(); public abstract String getRestbaseUriFormat(); - public abstract OkHttpClient getOkHttpClient(@NonNull WikiSite wikiSite); + public abstract OkHttpClient getOkHttpClient(); public abstract int getDesiredLeadImageDp(); public abstract boolean isLoggedIn(); diff --git a/data-client/src/main/java/org/wikipedia/dataclient/ServiceFactory.java b/data-client/src/main/java/org/wikipedia/dataclient/ServiceFactory.java index 522d57e82..48841ea97 100644 --- a/data-client/src/main/java/org/wikipedia/dataclient/ServiceFactory.java +++ b/data-client/src/main/java/org/wikipedia/dataclient/ServiceFactory.java @@ -57,7 +57,7 @@ public final class ServiceFactory { private static Retrofit createRetrofit(@NonNull WikiSite wiki, @NonNull String baseUrl) { return new Retrofit.Builder() - .client(AppAdapter.get().getOkHttpClient(wiki)) + .client(AppAdapter.get().getOkHttpClient()) .baseUrl(baseUrl) .addCallAdapterFactory(RxJava2CallAdapterFactory.create()) .addConverterFactory(GsonConverterFactory.create(GsonUtil.getDefaultGson())) diff --git a/data-client/src/test/java/org/wikipedia/TestAppAdapter.java b/data-client/src/test/java/org/wikipedia/TestAppAdapter.java index e708b5ca0..c2f58865e 100644 --- a/data-client/src/test/java/org/wikipedia/TestAppAdapter.java +++ b/data-client/src/test/java/org/wikipedia/TestAppAdapter.java @@ -4,7 +4,6 @@ import androidx.annotation.NonNull; import org.wikipedia.dataclient.Service; import org.wikipedia.dataclient.SharedPreferenceCookieManager; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.dataclient.okhttp.TestStubInterceptor; import org.wikipedia.dataclient.okhttp.UnsuccessfulResponseInterceptor; import org.wikipedia.login.LoginResult; @@ -24,7 +23,7 @@ public class TestAppAdapter extends AppAdapter { } @Override - public OkHttpClient getOkHttpClient(@NonNull WikiSite wikiSite) { + public OkHttpClient getOkHttpClient() { return new OkHttpClient.Builder() .addInterceptor(new UnsuccessfulResponseInterceptor()) .addInterceptor(new TestStubInterceptor()) diff --git a/data-client/src/test/java/org/wikipedia/test/MockWebServerTest.java b/data-client/src/test/java/org/wikipedia/test/MockWebServerTest.java index 1c9a3e633..82bc3de98 100644 --- a/data-client/src/test/java/org/wikipedia/test/MockWebServerTest.java +++ b/data-client/src/test/java/org/wikipedia/test/MockWebServerTest.java @@ -8,8 +8,6 @@ import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.wikipedia.AppAdapter; import org.wikipedia.TestAppAdapter; -import org.wikipedia.dataclient.Service; -import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonUtil; import okhttp3.Dispatcher; @@ -25,7 +23,7 @@ public abstract class MockWebServerTest { @Before public void setUp() throws Throwable { AppAdapter.set(new TestAppAdapter()); - OkHttpClient.Builder builder = AppAdapter.get().getOkHttpClient(new WikiSite(Service.WIKIPEDIA_URL)).newBuilder(); + OkHttpClient.Builder builder = AppAdapter.get().getOkHttpClient().newBuilder(); okHttpClient = builder.dispatcher(new Dispatcher(new ImmediateExecutorService())).build(); server.setUp(); }