From a66a0e8ca07cbe942bf02e562c610ace8aecfca2 Mon Sep 17 00:00:00 2001 From: Paul Hawke Date: Sun, 15 Apr 2018 07:32:14 -0500 Subject: [PATCH] Refactoring to extract GpsCategoryModel and ensure single-responsibility-principle is maintained in CategoryApi. --- app/proguard-rules.txt | 1 - .../nrw/commons/caching/CacheController.java | 19 +- .../category/CategorizationFragment.java | 8 +- .../commons/di/CommonsApplicationModule.java | 7 - .../free/nrw/commons/mwapi/CategoryApi.java | 101 ++++++++++ .../nrw/commons/mwapi/model/ApiResponse.java | 12 ++ .../fr/free/nrw/commons/mwapi/model/Page.java | 17 ++ .../nrw/commons/mwapi/model/PageCategory.java | 12 ++ .../free/nrw/commons/mwapi/model/Query.java | 10 + .../free/nrw/commons/upload/CategoryApi.java | 165 ---------------- .../nrw/commons/upload/GpsCategoryModel.java | 40 ++++ .../nrw/commons/upload/ShareActivity.java | 23 ++- .../nrw/commons/TestCommonsApplication.kt | 4 - .../free/nrw/commons/mwapi/CategoryApiTest.kt | 178 ++++++++++++++++++ .../commons/mwapi/model/ApiResponseTest.kt | 28 +++ .../commons/mwapi/model/PageCategoryTest.kt | 20 ++ .../free/nrw/commons/mwapi/model/PageTest.kt | 12 ++ .../commons/upload/GpsCategoryModelTest.kt | 77 ++++++++ 18 files changed, 542 insertions(+), 192 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/mwapi/CategoryApi.java create mode 100644 app/src/main/java/fr/free/nrw/commons/mwapi/model/ApiResponse.java create mode 100644 app/src/main/java/fr/free/nrw/commons/mwapi/model/Page.java create mode 100644 app/src/main/java/fr/free/nrw/commons/mwapi/model/PageCategory.java create mode 100644 app/src/main/java/fr/free/nrw/commons/mwapi/model/Query.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/CategoryApi.java create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.java create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/mwapi/CategoryApiTest.kt create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/ApiResponseTest.kt create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageCategoryTest.kt create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageTest.kt create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt diff --git a/app/proguard-rules.txt b/app/proguard-rules.txt index f3e00b3f5..39b618718 100644 --- a/app/proguard-rules.txt +++ b/app/proguard-rules.txt @@ -1,5 +1,4 @@ -dontobfuscate -keep class org.apache.http.** { *; } -dontwarn org.apache.http.** --keep class fr.free.nrw.commons.upload.CategoryApi$Page {*;} -keep class android.support.v7.widget.ShareActionProvider { *; } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/caching/CacheController.java b/app/src/main/java/fr/free/nrw/commons/caching/CacheController.java index 1b48c3c50..72de0db70 100644 --- a/app/src/main/java/fr/free/nrw/commons/caching/CacheController.java +++ b/app/src/main/java/fr/free/nrw/commons/caching/CacheController.java @@ -7,18 +7,25 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import fr.free.nrw.commons.upload.CategoryApi; +import javax.inject.Inject; +import javax.inject.Singleton; + +import fr.free.nrw.commons.upload.GpsCategoryModel; import timber.log.Timber; +@Singleton public class CacheController { + private final GpsCategoryModel gpsCategoryModel; + private final QuadTree> quadTree; private double x, y; - private QuadTree> quadTree; private double xMinus, xPlus, yMinus, yPlus; private static final int EARTH_RADIUS = 6378137; - public CacheController() { + @Inject + CacheController(GpsCategoryModel gpsCategoryModel) { + this.gpsCategoryModel = gpsCategoryModel; quadTree = new QuadTree<>(-180, -90, +180, +90); } @@ -31,8 +38,8 @@ public class CacheController { public void cacheCategory() { List pointCatList = new ArrayList<>(); - if (CategoryApi.GpsCatExists.getGpsCatExists()) { - pointCatList.addAll(CategoryApi.getGpsCat()); + if (gpsCategoryModel.getGpsCatExists()) { + pointCatList.addAll(gpsCategoryModel.getCategoryList()); Timber.d("Categories being cached: %s", pointCatList); } else { Timber.d("No categories found, so no categories cached"); @@ -65,7 +72,7 @@ public class CacheController { } //Based on algorithm at http://gis.stackexchange.com/questions/2951/algorithm-for-offsetting-a-latitude-longitude-by-some-amount-of-meters - public void convertCoordRange() { + private void convertCoordRange() { //Position, decimal degrees double lat = y; double lon = x; diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 514b2fb5f..93ddb60d5 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -39,7 +39,7 @@ import butterknife.ButterKnife; import fr.free.nrw.commons.R; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.mwapi.MediaWikiApi; -import fr.free.nrw.commons.upload.CategoryApi; +import fr.free.nrw.commons.upload.GpsCategoryModel; import fr.free.nrw.commons.utils.StringSortingUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; @@ -73,6 +73,7 @@ public class CategorizationFragment extends CommonsDaggerSupportFragment { @Inject @Named("prefs") SharedPreferences prefsPrefs; @Inject @Named("direct_nearby_upload_prefs") SharedPreferences directPrefs; @Inject CategoryDao categoryDao; + @Inject GpsCategoryModel gpsCategoryModel; private RVRendererAdapter categoriesAdapter; private OnCategoriesSaveHandler onCategoriesSaveHandler; @@ -253,7 +254,6 @@ public class CategorizationFragment extends CommonsDaggerSupportFragment { } private Observable defaultCategories() { - Observable directCat = directCategories(); if (hasDirectCategories) { Timber.d("Image has direct Cat"); @@ -287,9 +287,7 @@ public class CategorizationFragment extends CommonsDaggerSupportFragment { } private Observable gpsCategories() { - return Observable.fromIterable( - CategoryApi.GpsCatExists.getGpsCatExists() - ? CategoryApi.getGpsCat() : new ArrayList<>()) + return Observable.fromIterable(gpsCategoryModel.getCategoryList()) .map(name -> new CategoryItem(name, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java index b0aa3e5e6..f4a77c449 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java @@ -13,7 +13,6 @@ import dagger.Module; import dagger.Provides; import fr.free.nrw.commons.auth.AccountUtil; import fr.free.nrw.commons.auth.SessionManager; -import fr.free.nrw.commons.caching.CacheController; import fr.free.nrw.commons.data.DBOpenHelper; import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.mwapi.MediaWikiApi; @@ -117,12 +116,6 @@ public class CommonsApplicationModule { return new LocationServiceManager(context); } - @Provides - @Singleton - public CacheController provideCacheController() { - return new CacheController(); - } - @Provides @Singleton public DBOpenHelper provideDBOpenHelper(Context context) { diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/CategoryApi.java b/app/src/main/java/fr/free/nrw/commons/mwapi/CategoryApi.java new file mode 100644 index 000000000..031796745 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/CategoryApi.java @@ -0,0 +1,101 @@ +package fr.free.nrw.commons.mwapi; + +import com.google.gson.Gson; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import javax.inject.Inject; +import javax.inject.Named; + +import fr.free.nrw.commons.mwapi.model.ApiResponse; +import fr.free.nrw.commons.mwapi.model.Page; +import fr.free.nrw.commons.mwapi.model.PageCategory; +import io.reactivex.Single; +import okhttp3.HttpUrl; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.ResponseBody; +import timber.log.Timber; + +/** + * Uses the OkHttp library to implement calls to the Commons MediaWiki API to match GPS coordinates + * with nearby Commons categories. Parses the results using GSON to obtain a list of relevant + * categories. Note: that caller is responsible for executing the request() method on a background + * thread. + */ +public class CategoryApi { + + private final OkHttpClient okHttpClient; + private final HttpUrl mwUrl; + private final Gson gson; + + @Inject + public CategoryApi(OkHttpClient okHttpClient, Gson gson, + @Named("commons_mediawiki_url") HttpUrl mwUrl) { + this.okHttpClient = okHttpClient; + this.mwUrl = mwUrl; + this.gson = gson; + } + + public Single> request(String coords) { + return Single.fromCallable(() -> { + HttpUrl apiUrl = buildUrl(coords); + Timber.d("URL: %s", apiUrl.toString()); + + Request request = new Request.Builder().get().url(apiUrl).build(); + Response response = okHttpClient.newCall(request).execute(); + ResponseBody body = response.body(); + if (body == null) { + return Collections.emptyList(); + } + + ApiResponse apiResponse = gson.fromJson(body.charStream(), ApiResponse.class); + Set categories = new LinkedHashSet<>(); + if (apiResponse != null && apiResponse.hasPages()) { + for (Page page : apiResponse.query.pages) { + for (PageCategory category : page.getCategories()) { + categories.add(category.withoutPrefix()); + } + } + } + return new ArrayList<>(categories); + }); + } + + /** + * Builds URL with image coords for MediaWiki API calls + * Example URL: https://commons.wikimedia.org/w/api.php?action=query&prop=categories|coordinates|pageprops&format=json&clshow=!hidden&coprop=type|name|dim|country|region|globe&codistancefrompoint=38.11386944444445|13.356263888888888&generator=geosearch&redirects=&ggscoord=38.11386944444445|1.356263888888888&ggsradius=100&ggslimit=10&ggsnamespace=6&ggsprop=type|name|dim|country|region|globe&ggsprimary=all&formatversion=2 + * + * @param coords Coordinates to build query with + * @return URL for API query + */ + private HttpUrl buildUrl(String coords) { + return mwUrl.newBuilder() + .addPathSegment("w") + .addPathSegment("api.php") + .addQueryParameter("action", "query") + .addQueryParameter("prop", "categories|coordinates|pageprops") + .addQueryParameter("format", "json") + .addQueryParameter("clshow", "!hidden") + .addQueryParameter("coprop", "type|name|dim|country|region|globe") + .addQueryParameter("codistancefrompoint", coords) + .addQueryParameter("generator", "geosearch") + .addQueryParameter("ggscoord", coords) + .addQueryParameter("ggsradius", "10000") + .addQueryParameter("ggslimit", "10") + .addQueryParameter("ggsnamespace", "6") + .addQueryParameter("ggsprop", "type|name|dim|country|region|globe") + .addQueryParameter("ggsprimary", "all") + .addQueryParameter("formatversion", "2") + .build(); + } + +} + + + diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/model/ApiResponse.java b/app/src/main/java/fr/free/nrw/commons/mwapi/model/ApiResponse.java new file mode 100644 index 000000000..7feb90251 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/model/ApiResponse.java @@ -0,0 +1,12 @@ +package fr.free.nrw.commons.mwapi.model; + +public class ApiResponse { + public Query query; + + public ApiResponse() { + } + + public boolean hasPages() { + return query != null && query.pages != null; + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/model/Page.java b/app/src/main/java/fr/free/nrw/commons/mwapi/model/Page.java new file mode 100644 index 000000000..d01ba658f --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/model/Page.java @@ -0,0 +1,17 @@ +package fr.free.nrw.commons.mwapi.model; + +import android.support.annotation.NonNull; + +public class Page { + public String title; + public PageCategory[] categories; + public PageCategory category; + + public Page() { + } + + @NonNull + public PageCategory[] getCategories() { + return categories != null ? categories : new PageCategory[0]; + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/model/PageCategory.java b/app/src/main/java/fr/free/nrw/commons/mwapi/model/PageCategory.java new file mode 100644 index 000000000..be4b9fd79 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/model/PageCategory.java @@ -0,0 +1,12 @@ +package fr.free.nrw.commons.mwapi.model; + +public class PageCategory { + public String title; + + public PageCategory() { + } + + public String withoutPrefix() { + return title != null ? title.replace("Category:", "") : ""; + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/model/Query.java b/app/src/main/java/fr/free/nrw/commons/mwapi/model/Query.java new file mode 100644 index 000000000..b87f97cc3 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/model/Query.java @@ -0,0 +1,10 @@ +package fr.free.nrw.commons.mwapi.model; + +public class Query { + public Page[] pages; + + public Query() { + pages = new Page[0]; + } + +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/CategoryApi.java b/app/src/main/java/fr/free/nrw/commons/upload/CategoryApi.java deleted file mode 100644 index 8a7dfcc6e..000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/CategoryApi.java +++ /dev/null @@ -1,165 +0,0 @@ -package fr.free.nrw.commons.upload; - -import android.support.annotation.NonNull; - -import com.google.gson.Gson; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import javax.inject.Inject; -import javax.inject.Named; - -import okhttp3.Call; -import okhttp3.Callback; -import okhttp3.HttpUrl; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.ResponseBody; -import timber.log.Timber; - -/** - * Uses the OkHttp library to implement asynchronous calls to the Commons MediaWiki API to match - * GPS coordinates with nearby Commons categories. Parses the results using GSON to obtain a list - * of relevant categories. - */ -public class CategoryApi { - - private static Set categorySet; - private static List categoryList; - private final OkHttpClient okHttpClient; - private final HttpUrl mwUrl; - private final Gson gson; - - @Inject - public CategoryApi(OkHttpClient okHttpClient, @Named("commons_mediawiki_url") HttpUrl mwUrl, Gson gson) { - this.okHttpClient = okHttpClient; - this.mwUrl = mwUrl; - this.gson = gson; - categorySet = new HashSet<>(); - } - - public static List getGpsCat() { - return categoryList; - } - - public static void setGpsCat(List cachedList) { - categoryList = new ArrayList<>(); - categoryList.addAll(cachedList); - Timber.d("Setting GPS cats from cache: %s", categoryList); - } - - public void request(String coords) { - String apiUrl = buildUrl(coords); - Timber.d("URL: %s", apiUrl); - - Call call = okHttpClient.newCall(new Request.Builder().get().url(apiUrl).build()); - call.enqueue(new Callback() { - @Override - public void onFailure(@NonNull Call call, @NonNull IOException e) { - Timber.e(e); - GpsCatExists.setGpsCatExists(false); - } - - @Override - public void onResponse(@NonNull Call call, @NonNull okhttp3.Response response) { - categoryList = new ArrayList<>(); - categorySet = new HashSet<>(); - ResponseBody body = response.body(); - if (body == null) { - return; - } - QueryResponse queryResponse = gson.fromJson(body.charStream(), QueryResponse.class); - if (queryResponse != null && queryResponse.query != null && queryResponse.query.pages != null) { - for (Page page : queryResponse.query.pages) { - if (page.categories != null) { - for (Category category : page.categories) { - String categoryString = category.title.replace("Category:", ""); - categorySet.add(categoryString); - } - categoryList = new ArrayList<>(categorySet); - } - } - } - GpsCatExists.setGpsCatExists(!categorySet.isEmpty()); - } - }); - } - - /** - * Builds URL with image coords for MediaWiki API calls - * Example URL: https://commons.wikimedia.org/w/api.php?action=query&prop=categories|coordinates|pageprops&format=json&clshow=!hidden&coprop=type|name|dim|country|region|globe&codistancefrompoint=38.11386944444445|13.356263888888888&generator=geosearch&redirects=&ggscoord=38.11386944444445|1.356263888888888&ggsradius=100&ggslimit=10&ggsnamespace=6&ggsprop=type|name|dim|country|region|globe&ggsprimary=all&formatversion=2 - * - * @param coords Coordinates to build query with - * @return URL for API query - */ - private String buildUrl(String coords) { - return mwUrl.newBuilder() - .addPathSegment("w") - .addPathSegment("api.php") - .addQueryParameter("action", "query") - .addQueryParameter("prop", "categories|coordinates|pageprops") - .addQueryParameter("format", "json") - .addQueryParameter("clshow", "!hidden") - .addQueryParameter("coprop", "type|name|dim|country|region|globe") - .addQueryParameter("codistancefrompoint", coords) - .addQueryParameter("generator", "geosearch") - .addQueryParameter("ggscoord", coords) - .addQueryParameter("ggsradius", "10000") - .addQueryParameter("ggslimit", "10") - .addQueryParameter("ggsnamespace", "6") - .addQueryParameter("ggsprop", "type|name|dim|country|region|globe") - .addQueryParameter("ggsprimary", "all") - .addQueryParameter("formatversion", "2") - .build().toString(); - } - - public static class GpsCatExists { - private static boolean gpsCatExists; - - public static void setGpsCatExists(boolean gpsCat) { - gpsCatExists = gpsCat; - } - - public static boolean getGpsCatExists() { - return gpsCatExists; - } - } - - private static class QueryResponse { - public Query query; - - public QueryResponse() { - } - } - - private static class Query { - public Page[] pages; - - public Query() { - pages = new Page[0]; - } - } - - private static class Page { - public String title; - public Category[] categories; - public Category category; - - public Page() { - } - } - - private static class Category { - public String title; - - public Category() { - } - } -} - - - diff --git a/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.java b/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.java new file mode 100644 index 000000000..841210453 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.java @@ -0,0 +1,40 @@ +package fr.free.nrw.commons.upload; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import javax.inject.Inject; +import javax.inject.Singleton; + +@Singleton +public class GpsCategoryModel { + private Set categorySet; + + @Inject + public GpsCategoryModel() { + clear(); + } + + public void clear() { + categorySet = new HashSet<>(); + } + + public boolean getGpsCatExists() { + return !categorySet.isEmpty(); + } + + public List getCategoryList() { + return new ArrayList<>(categorySet); + } + + public void setCategoryList(List categoryList) { + clear(); + categorySet.addAll(categoryList != null ? categoryList : new ArrayList<>()); + } + + public void add(String categoryString) { + categorySet.add(categoryString); + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index e3188534c..98d11c20e 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -1,6 +1,7 @@ package fr.free.nrw.commons.upload; import android.Manifest; +import android.annotation.SuppressLint; import android.app.Activity; import android.animation.Animator; import android.animation.AnimatorListenerAdapter; @@ -73,8 +74,10 @@ import fr.free.nrw.commons.modifications.ModificationsContentProvider; import fr.free.nrw.commons.modifications.ModifierSequence; import fr.free.nrw.commons.modifications.ModifierSequenceDao; import fr.free.nrw.commons.modifications.TemplateRemoveModifier; +import fr.free.nrw.commons.mwapi.CategoryApi; import fr.free.nrw.commons.mwapi.MediaWikiApi; +import io.reactivex.schedulers.Schedulers; import fr.free.nrw.commons.utils.ViewUtil; import timber.log.Timber; @@ -127,6 +130,8 @@ public class ShareActivity @Inject @Named("default_preferences") SharedPreferences prefs; + @Inject + GpsCategoryModel gpsCategoryModel; private String source; private String mimeType; @@ -687,8 +692,9 @@ public class ShareActivity /** * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. - * Then initiates the calls to MediaWiki API through an instance of MwVolleyApi. + * Then initiates the calls to MediaWiki API through an instance of CategpryApi. */ + @SuppressLint("CheckResult") public void useImageCoords() { if (decimalCoords != null) { Timber.d("Decimal coords of image: %s", decimalCoords); @@ -707,12 +713,21 @@ public class ShareActivity // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories if (catListEmpty) { cacheFound = false; - apiCall.request(decimalCoords); + apiCall.request(decimalCoords) + .subscribeOn(Schedulers.io()) + .observeOn(Schedulers.io()) + .subscribe( + gpsCategoryModel::setCategoryList, + throwable -> { + Timber.e(throwable); + gpsCategoryModel.clear(); + } + ); Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList); } else { cacheFound = true; - Timber.d("Cache found, setting categoryList in MwVolleyApi to %s", displayCatList); - CategoryApi.setGpsCat(displayCatList); + Timber.d("Cache found, setting categoryList in model to %s", displayCatList); + gpsCategoryModel.setCategoryList(displayCatList); } }else{ Timber.d("EXIF: no coords"); diff --git a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt index 076b3e745..090cf39b5 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/TestCommonsApplication.kt @@ -8,7 +8,6 @@ import com.nhaarman.mockito_kotlin.mock import com.squareup.leakcanary.RefWatcher import fr.free.nrw.commons.auth.AccountUtil import fr.free.nrw.commons.auth.SessionManager -import fr.free.nrw.commons.caching.CacheController import fr.free.nrw.commons.data.DBOpenHelper import fr.free.nrw.commons.di.CommonsApplicationComponent import fr.free.nrw.commons.di.CommonsApplicationModule @@ -42,7 +41,6 @@ class MockCommonsApplicationModule(appContext: Context) : CommonsApplicationModu val uploadController: UploadController = mock() val mockSessionManager: SessionManager = mock() val locationServiceManager: LocationServiceManager = mock() - val cacheController: CacheController = mock() val mockDbOpenHelper: DBOpenHelper = mock() val nearbyPlaces: NearbyPlaces = mock() val lruCache: LruCache = mock() @@ -73,8 +71,6 @@ class MockCommonsApplicationModule(appContext: Context) : CommonsApplicationModu override fun provideLocationServiceManager(context: Context): LocationServiceManager = locationServiceManager - override fun provideCacheController(): CacheController = cacheController - override fun provideDBOpenHelper(context: Context): DBOpenHelper = mockDbOpenHelper override fun provideNearbyPlaces(): NearbyPlaces = nearbyPlaces diff --git a/app/src/test/kotlin/fr/free/nrw/commons/mwapi/CategoryApiTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/CategoryApiTest.kt new file mode 100644 index 000000000..76f34d55d --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/CategoryApiTest.kt @@ -0,0 +1,178 @@ +package fr.free.nrw.commons.mwapi + +import com.google.gson.Gson +import fr.free.nrw.commons.mwapi.model.Page +import fr.free.nrw.commons.mwapi.model.PageCategory +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +class CategoryApiTest { + private lateinit var server: MockWebServer + private lateinit var url: String + private lateinit var categoryApi: CategoryApi + + @Before + fun setUp() { + server = MockWebServer() + url = "http://${server.hostName}:${server.port}/" + categoryApi = CategoryApi(OkHttpClient.Builder().build(), Gson(), HttpUrl.parse(url)) + } + + @After + fun teardown() { + server.shutdown() + } + + @Test + fun apiReturnsEmptyListWhenError() { + server.enqueue(MockResponse().setResponseCode(400).setBody("")) + + assertTrue(categoryApi.request("foo").blockingGet().isEmpty()) + } + + @Test + fun apiReturnsEmptyWhenTheresNoQuery() { + server.success(emptyMap()) + + assertTrue(categoryApi.request("foo").blockingGet().isEmpty()) + } + + @Test + fun apiReturnsEmptyWhenQueryHasNoPages() { + server.success(mapOf("query" to emptyMap())) + + assertTrue(categoryApi.request("foo").blockingGet().isEmpty()) + } + + @Test + fun apiReturnsEmptyWhenQueryHasPagesButTheyreEmpty() { + server.success(mapOf("query" to + mapOf("pages" to emptyList()))) + + assertTrue(categoryApi.request("foo").blockingGet().isEmpty()) + } + + @Test + fun singlePageSingleCategory() { + server.success(mapOf("query" to + mapOf("pages" to listOf( + page(listOf("one")) + )))) + + val response = categoryApi.request("foo").blockingGet() + + assertEquals(1, response.size) + assertEquals("one", response[0]) + } + + @Test + fun multiplePagesSingleCategory() { + server.success(mapOf("query" to + mapOf("pages" to listOf( + page(listOf("one")), + page(listOf("two")) + )))) + + val response = categoryApi.request("foo").blockingGet() + + assertEquals(2, response.size) + assertEquals("one", response[0]) + assertEquals("two", response[1]) + } + + @Test + fun singlePageMultipleCategories() { + server.success(mapOf("query" to + mapOf("pages" to listOf( + page(listOf("one", "two")) + )))) + + val response = categoryApi.request("foo").blockingGet() + + assertEquals(2, response.size) + assertEquals("one", response[0]) + assertEquals("two", response[1]) + } + + @Test + fun multiplePagesMultipleCategories() { + server.success(mapOf("query" to + mapOf("pages" to listOf( + page(listOf("one", "two")), + page(listOf("three", "four")) + )))) + + val response = categoryApi.request("foo").blockingGet() + + assertEquals(4, response.size) + assertEquals("one", response[0]) + assertEquals("two", response[1]) + assertEquals("three", response[2]) + assertEquals("four", response[3]) + } + + @Test + fun multiplePagesMultipleCategories_duplicatesRemoved() { + server.success(mapOf("query" to + mapOf("pages" to listOf( + page(listOf("one", "two", "three")), + page(listOf("three", "four", "one")) + )))) + + val response = categoryApi.request("foo").blockingGet() + + assertEquals(4, response.size) + assertEquals("one", response[0]) + assertEquals("two", response[1]) + assertEquals("three", response[2]) + assertEquals("four", response[3]) + } + + @Test + fun requestSendsWhatWeExpect() { + server.success(mapOf("query" to mapOf("pages" to emptyList()))) + + val coords = "foo,bar" + categoryApi.request(coords).blockingGet() + + server.takeRequest().let { request -> + assertEquals("GET", request.method) + assertEquals("/w/api.php", request.requestUrl.encodedPath()) + request.requestUrl.let { url -> + assertEquals("query", url.queryParameter("action")) + assertEquals("categories|coordinates|pageprops", url.queryParameter("prop")) + assertEquals("json", url.queryParameter("format")) + assertEquals("!hidden", url.queryParameter("clshow")) + assertEquals("type|name|dim|country|region|globe", url.queryParameter("coprop")) + assertEquals(coords, url.queryParameter("codistancefrompoint")) + assertEquals("geosearch", url.queryParameter("generator")) + assertEquals(coords, url.queryParameter("ggscoord")) + assertEquals("10000", url.queryParameter("ggsradius")) + assertEquals("10", url.queryParameter("ggslimit")) + assertEquals("6", url.queryParameter("ggsnamespace")) + assertEquals("type|name|dim|country|region|globe", url.queryParameter("ggsprop")) + assertEquals("all", url.queryParameter("ggsprimary")) + assertEquals("2", url.queryParameter("formatversion")) + } + } + } + + private fun page(catList: List) = Page().apply { + categories = catList.map { + PageCategory().apply { + title = "Category:$it" + } + }.toTypedArray() + } +} + +fun MockWebServer.success(response: Map) { + enqueue(MockResponse().setResponseCode(200).setBody(Gson().toJson(response))) +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/ApiResponseTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/ApiResponseTest.kt new file mode 100644 index 000000000..41406a894 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/ApiResponseTest.kt @@ -0,0 +1,28 @@ +package fr.free.nrw.commons.mwapi.model + +import org.junit.Assert.* +import org.junit.Test + +class ApiResponseTest { + @Test + fun hasPages_whenQueryIsNull() { + val response = ApiResponse() + assertFalse(response.hasPages()) + } + + @Test + fun hasPages_whenPagesIsNull() { + val response = ApiResponse() + response.query = Query() + response.query.pages = null + assertFalse(response.hasPages()) + } + + @Test + fun hasPages_defaultsToSafeValue() { + val response = ApiResponse() + response.query = Query() + assertNotNull(response.query.pages) + assertTrue(response.hasPages()) + } +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageCategoryTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageCategoryTest.kt new file mode 100644 index 000000000..fcc3d408f --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageCategoryTest.kt @@ -0,0 +1,20 @@ +package fr.free.nrw.commons.mwapi.model + +import org.junit.Assert.assertEquals +import org.junit.Test + +class PageCategoryTest { + @Test + fun stripPrefix_whenPresent() { + val testObject = PageCategory() + testObject.title = "Category:Foo" + assertEquals("Foo", testObject.withoutPrefix()) + } + + @Test + fun stripPrefix_prefixAbsent() { + val testObject = PageCategory() + testObject.title = "Foo_Bar" + assertEquals("Foo_Bar", testObject.withoutPrefix()) + } +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageTest.kt new file mode 100644 index 000000000..4179b4fb5 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/mwapi/model/PageTest.kt @@ -0,0 +1,12 @@ +package fr.free.nrw.commons.mwapi.model + +import org.junit.Assert.assertNotNull +import org.junit.Test + +class PageTest { + @Test + fun categoriesDefaultToSafeValue() { + val page = Page() + assertNotNull(page.getCategories()) + } +} \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt new file mode 100644 index 000000000..cd2d77ada --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt @@ -0,0 +1,77 @@ +package fr.free.nrw.commons.upload + +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +class GpsCategoryModelTest { + + private lateinit var testObject : GpsCategoryModel + + @Before + fun setUp() { + testObject = GpsCategoryModel() + } + + @Test + fun initiallyTheModelIsEmpty() { + assertFalse(testObject.gpsCatExists) + assertTrue(testObject.categoryList.isEmpty()) + } + + @Test + fun addingCategoriesToTheModel() { + testObject.add("one") + assertTrue(testObject.gpsCatExists) + assertFalse(testObject.categoryList.isEmpty()) + assertEquals(listOf("one"), testObject.categoryList) + } + + @Test + fun duplicatesAreIgnored() { + testObject.add("one") + testObject.add("one") + assertEquals(listOf("one"), testObject.categoryList) + } + + @Test + fun modelProtectsAgainstExternalModification() { + testObject.add("one") + + val list = testObject.categoryList + list.add("two") + + assertEquals(listOf("one"), testObject.categoryList) + } + + @Test + fun clearingTheModel() { + testObject.add("one") + + testObject.clear() + assertFalse(testObject.gpsCatExists) + assertTrue(testObject.categoryList.isEmpty()) + + testObject.add("two") + assertEquals(listOf("two"), testObject.categoryList) + } + + @Test + fun settingTheListHandlesNull() { + testObject.add("one") + + testObject.categoryList = null + + assertFalse(testObject.gpsCatExists) + assertTrue(testObject.categoryList.isEmpty()) + } + + @Test + fun setttingTheListOverwritesExistingValues() { + testObject.add("one") + + testObject.categoryList = listOf("two") + + assertEquals(listOf("two"), testObject.categoryList) + } +} \ No newline at end of file