getSubCategoryList and getParentCategoryList migrated to retrofit (#3055)

* Migrated getSubCategoryList to retrofit

* Migrated getParentCategoryList to retrofit

* Removed obsolete functions

* Added tests

* Fixed small bugs

app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java

@@ -1,8 +1,11 @@
 package fr.free.nrw.commons.category;
 
 
+import androidx.annotation.NonNull;
+
 import org.wikipedia.dataclient.mwapi.MwQueryPage;
 import org.wikipedia.dataclient.mwapi.MwQueryResponse;
+import org.wikipedia.dataclient.mwapi.MwQueryResult;
 
 import java.util.List;
 
@@ -74,6 +77,29 @@ public class CategoryClient {
     }
 
 
+    /**
+     * The method takes categoryName as input and returns a List of Subcategories
+     * It uses the generator query API to get the subcategories in a category, 500 at a time.
+     *
+     * @param categoryName Category name as defined on commons
+     * @return Observable emitting the categories returned. If our search yielded "Category:Test", "Test" is emitted.
+     */
+    public Observable<String> getSubCategoryList(String categoryName) {
+        return responseToCategoryName(CategoryInterface.getSubCategoryList(categoryName));
+    }
+
+    /**
+     * The method takes categoryName as input and returns a List of parent categories
+     * It uses the generator query API to get the parent categories of a category, 500 at a time.
+     *
+     * @param categoryName Category name as defined on commons
+     * @return
+     */
+    @NonNull
+    public Observable<String> getParentCategoryList(String categoryName) {
+        return responseToCategoryName(CategoryInterface.getParentCategoryList(categoryName));
+    }
+
     /**
      * Internal function to reduce code reuse. Extracts the categories returned from MwQueryResponse.
      *
@@ -83,12 +109,14 @@ public class CategoryClient {
     private Observable<String> responseToCategoryName(Observable<MwQueryResponse> responseObservable) {
         return responseObservable
                 .flatMap(mwQueryResponse -> {
-                    List<MwQueryPage> pages = mwQueryResponse.query().pages();
+                    MwQueryResult query;
-                    if (pages != null)
+                    List<MwQueryPage> pages;
-                        return Observable.fromIterable(pages);
+                    if ((query = mwQueryResponse.query()) == null ||
-                    else
+                            (pages = query.pages()) == null) {
                         Timber.d("No categories returned.");
-                    return Observable.empty();
+                        return Observable.empty();
+                    } else
+                        return Observable.fromIterable(pages);
                 })
                 .map(MwQueryPage::title)
                 .doOnEach(s -> Timber.d("Category returned: %s", s))

app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java

@@ -13,7 +13,6 @@ public interface CategoryInterface {
 
     /**
      * Searches for categories with the specified name.
-     * Replaces ApacheHttpClientMediaWikiApi#allCategories
      *
      * @param filter    The string to be searched
      * @param itemLimit How many results are returned
@@ -24,8 +23,25 @@ public interface CategoryInterface {
     Observable<MwQueryResponse> searchCategories(@Query("gsrsearch") String filter,
                                                  @Query("gsrlimit") int itemLimit, @Query("gsroffset") int offset);
 
+    /**
+     * Searches for categories starting with the specified prefix.
+     *
+     * @param prefix    The string to be searched
+     * @param itemLimit How many results are returned
+     * @return
+     */
     @GET("w/api.php?action=query&format=json&formatversion=2"
             + "&generator=allcategories")
     Observable<MwQueryResponse> searchCategoriesForPrefix(@Query("gacprefix") String prefix,
                                                           @Query("gaclimit") int itemLimit, @Query("gacoffset") int offset);
+
+    @GET("w/api.php?action=query&format=json&formatversion=2"
+            + "&generator=categorymembers&gcmtype=subcat"
+            + "&prop=info&gcmlimit=500")
+    Observable<MwQueryResponse> getSubCategoryList(@Query("gcmtitle") String categoryName);
+
+    @GET("w/api.php?action=query&format=json&formatversion=2"
+            + "&generator=categories&prop=info&gcllimit=500")
+    Observable<MwQueryResponse> getParentCategoryList(@Query("titles") String categoryName);
+
 }

app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java

@@ -53,7 +53,7 @@ public class SubCategoryListFragment extends CommonsDaggerSupportFragment {
     TextView categoriesNotFoundView;
 
     private String categoryName = null;
-    @Inject MediaWikiApi mwApi;
+    @Inject CategoryClient categoryClient;
 
     private RVRendererAdapter<String> categoriesAdapter;
     private boolean isParentCategory = true;
@@ -86,7 +86,7 @@ public class SubCategoryListFragment extends CommonsDaggerSupportFragment {
     }
 
     /**
-     * Checks for internet connection and then initializes the recycler view with 25 categories of the searched query
+     * Checks for internet connection and then initializes the recycler view with all(max 500) categories of the searched query
      * Clearing categoryAdapter every time new keyword is searched so that user can see only new results
      */
     public void initSubCategoryList() {
@@ -96,17 +96,19 @@ public class SubCategoryListFragment extends CommonsDaggerSupportFragment {
             return;
         }
         progressBar.setVisibility(View.VISIBLE);
-        if (!isParentCategory){
+        if (isParentCategory) {
-            compositeDisposable.add(Observable.fromCallable(() -> mwApi.getSubCategoryList(categoryName))
+            compositeDisposable.add(categoryClient.getParentCategoryList("Category:"+categoryName)
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
+                    .collect(ArrayList<String>::new, ArrayList::add)
                     .subscribe(this::handleSuccess, this::handleError));
-        }else {
+        } else {
-            compositeDisposable.add(Observable.fromCallable(() -> mwApi.getParentCategoryList(categoryName))
+            compositeDisposable.add(categoryClient.getSubCategoryList("Category:"+categoryName)
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
+                    .collect(ArrayList<String>::new, ArrayList::add)
                     .subscribe(this::handleSuccess, this::handleError));
         }
     }

app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java

@@ -71,7 +71,7 @@ public class NetworkingModule {
     public MediaWikiApi provideMediaWikiApi(Context context,
                                             @Named("default_preferences") JsonKvStore defaultKvStore,
                                             Gson gson) {
-        return new ApacheHttpClientMediaWikiApi(context, BuildConfig.WIKIMEDIA_API_HOST, BuildConfig.WIKIDATA_API_HOST, defaultKvStore, gson);
+        return new ApacheHttpClientMediaWikiApi(context, BuildConfig.WIKIMEDIA_API_HOST, BuildConfig.WIKIDATA_API_HOST, defaultKvStore);
     }
 
     @Provides

app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java

@@ -29,23 +29,19 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.text.ParseException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
-import java.util.concurrent.Callable;
 
 import fr.free.nrw.commons.BuildConfig;
 import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.auth.AccountUtil;
-import fr.free.nrw.commons.category.CategoryImageUtils;
 import fr.free.nrw.commons.category.QueryContinue;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.notification.Notification;
 import fr.free.nrw.commons.notification.NotificationUtils;
 import fr.free.nrw.commons.utils.ViewUtil;
-import io.reactivex.Observable;
 import io.reactivex.Single;
 import timber.log.Timber;
 
@@ -58,15 +54,13 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
     private CustomMwApi wikidataApi;
     private Context context;
     private JsonKvStore defaultKvStore;
-    private Gson gson;
 
     private final String ERROR_CODE_BAD_TOKEN = "badtoken";
 
     public ApacheHttpClientMediaWikiApi(Context context,
                                         String apiURL,
                                         String wikidatApiURL,
-                                        JsonKvStore defaultKvStore,
+                                        JsonKvStore defaultKvStore) {
-                                        Gson gson) {
         this.context = context;
         BasicHttpParams params = new BasicHttpParams();
         SchemeRegistry schemeRegistry = new SchemeRegistry();
@@ -82,7 +76,6 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
         api = new CustomMwApi(apiURL, httpClient);
         wikidataApi = new CustomMwApi(wikidatApiURL, httpClient);
         this.defaultKvStore = defaultKvStore;
-        this.gson = gson;
     }
 
     /**
@@ -491,114 +484,6 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
         return result.equals("success");
     }
 
-    /**
-     * The method takes categoryName as input and returns a List of Subcategories
-     * It uses the generator query API to get the subcategories in a category, 500 at a time.
-     * Uses the query continue values for fetching paginated responses
-     *
-     * @param categoryName Category name as defined on commons
-     * @return
-     */
-    @Override
-    @NonNull
-    public List<String> getSubCategoryList(String categoryName) {
-        CustomApiResult apiResult = null;
-        try {
-            CustomMwApi.RequestBuilder requestBuilder = api.action("query")
-                    .param("generator", "categorymembers")
-                    .param("format", "xml")
-                    .param("gcmtype", "subcat")
-                    .param("gcmtitle", categoryName)
-                    .param("prop", "info")
-                    .param("gcmlimit", "500")
-                    .param("iiprop", "url|extmetadata");
-
-            apiResult = requestBuilder.get();
-        } catch (IOException e) {
-            Timber.e(e, "Failed to obtain searchCategories");
-        }
-
-        if (apiResult == null) {
-            return new ArrayList<>();
-        }
-
-        CustomApiResult categoryImagesNode = apiResult.getNode("/api/query/pages");
-        if (categoryImagesNode == null
-                || categoryImagesNode.getDocument() == null
-                || categoryImagesNode.getDocument().getChildNodes() == null
-                || categoryImagesNode.getDocument().getChildNodes().getLength() == 0) {
-            return new ArrayList<>();
-        }
-
-        NodeList childNodes = categoryImagesNode.getDocument().getChildNodes();
-        return CategoryImageUtils.getSubCategoryList(childNodes);
-    }
-
-    /**
-     * The method takes categoryName as input and returns a List of parent categories
-     * It uses the generator query API to get the parent categories of a category, 500 at a time.
-     *
-     * @param categoryName Category name as defined on commons
-     * @return
-     */
-    @Override
-    @NonNull
-    public List<String> getParentCategoryList(String categoryName) {
-        CustomApiResult apiResult = null;
-        try {
-            CustomMwApi.RequestBuilder requestBuilder = api.action("query")
-                    .param("generator", "categories")
-                    .param("format", "xml")
-                    .param("titles", categoryName)
-                    .param("prop", "info")
-                    .param("cllimit", "500")
-                    .param("iiprop", "url|extmetadata");
-
-            apiResult = requestBuilder.get();
-        } catch (IOException e) {
-            Timber.e(e, "Failed to obtain parent Categories");
-        }
-
-        if (apiResult == null) {
-            return new ArrayList<>();
-        }
-
-        CustomApiResult categoryImagesNode = apiResult.getNode("/api/query/pages");
-        if (categoryImagesNode == null
-                || categoryImagesNode.getDocument() == null
-                || categoryImagesNode.getDocument().getChildNodes() == null
-                || categoryImagesNode.getDocument().getChildNodes().getLength() == 0) {
-            return new ArrayList<>();
-        }
-
-        NodeList childNodes = categoryImagesNode.getDocument().getChildNodes();
-        return CategoryImageUtils.getSubCategoryList(childNodes);
-    }
-
-
-    /**
-     * For APIs that return paginated responses, MediaWiki APIs uses the QueryContinue to facilitate fetching of subsequent pages
-     * https://www.mediawiki.org/wiki/API:Raw_query_continue
-     * After fetching images a page of image for a particular category, shared defaultKvStore are updated with the latest QueryContinue Values
-     *
-     * @param keyword
-     * @param queryContinue
-     */
-    private void setQueryContinueValues(String keyword, QueryContinue queryContinue) {
-        defaultKvStore.putString(keyword, gson.toJson(queryContinue));
-    }
-
-    /**
-     * Before making a paginated API call, this method is called to get the latest query continue values to be used
-     *
-     * @param keyword
-     * @return
-     */
-    @Nullable
-    private QueryContinue getQueryContinueValues(String keyword) {
-        String queryContinueString = defaultKvStore.getString(keyword, null);
-        return gson.fromJson(queryContinueString, QueryContinue.class);
-    }
 
     @Override
     @NonNull

app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java

@@ -30,10 +30,6 @@ public interface MediaWikiApi {
 
     String getCentralAuthToken() throws IOException;
 
-    List<String> getSubCategoryList(String categoryName);
-
-    List<String> getParentCategoryList(String categoryName);
-
     @NonNull
     Single<UploadStash> uploadFile(String filename, InputStream file,
                                    long dataLength, Uri fileUri, Uri contentProviderUri,

app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt

@@ -1,7 +1,7 @@
 package fr.free.nrw.commons.category
 
 import io.reactivex.Observable
-import junit.framework.Assert
+import junit.framework.Assert.*
 import org.junit.Before
 import org.junit.Test
 import org.mockito.*
@@ -35,10 +35,10 @@ class CategoryClientTest {
                 .thenReturn(Observable.just(mockResponse))
 
         val actualCategoryName = categoryClient!!.searchCategories("tes", 10).blockingFirst()
-        Assert.assertEquals("Test", actualCategoryName)
+        assertEquals("Test", actualCategoryName)
 
         val actualCategoryName2 = categoryClient!!.searchCategories("tes", 10, 10).blockingFirst()
-        Assert.assertEquals("Test", actualCategoryName2)
+        assertEquals("Test", actualCategoryName2)
     }
 
     @Test
@@ -52,10 +52,10 @@ class CategoryClientTest {
                 .thenReturn(Observable.just(mockResponse))
 
         categoryClient!!.searchCategories("tes", 10).subscribe(
-                { Assert.fail("SearchCategories returned element when it shouldn't have.") },
+                { fail("SearchCategories returned element when it shouldn't have.") },
                 { s -> throw s })
         categoryClient!!.searchCategories("tes", 10, 10).subscribe(
-                { Assert.fail("SearchCategories returned element when it shouldn't have.") },
+                { fail("SearchCategories returned element when it shouldn't have.") },
                 { s -> throw s })
     }
     @Test
@@ -71,9 +71,9 @@ class CategoryClientTest {
                 .thenReturn(Observable.just(mockResponse))
 
         val actualCategoryName = categoryClient!!.searchCategoriesForPrefix("tes", 10).blockingFirst()
-        Assert.assertEquals("Test", actualCategoryName)
+        assertEquals("Test", actualCategoryName)
         val actualCategoryName2 = categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).blockingFirst()
-        Assert.assertEquals("Test", actualCategoryName2)
+        assertEquals("Test", actualCategoryName2)
     }
 
     @Test
@@ -86,10 +86,68 @@ class CategoryClientTest {
         Mockito.`when`(categoryInterface!!.searchCategoriesForPrefix(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()))
                 .thenReturn(Observable.just(mockResponse))
         categoryClient!!.searchCategoriesForPrefix("tes", 10).subscribe(
-                { Assert.fail("SearchCategories returned element when it shouldn't have.") },
+                { fail("SearchCategories returned element when it shouldn't have.") },
                 { s -> throw s })
         categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).subscribe(
-                { Assert.fail("SearchCategories returned element when it shouldn't have.") },
+                { fail("SearchCategories returned element when it shouldn't have.") },
+                { s -> throw s })
+    }
+    @Test
+    fun getParentCategoryListFound() {
+        val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
+        Mockito.`when`(mwQueryPage.title()).thenReturn("Category:Test")
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage))
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+
+        Mockito.`when`(categoryInterface!!.getParentCategoryList(ArgumentMatchers.anyString()))
+                .thenReturn(Observable.just(mockResponse))
+
+        val actualCategoryName = categoryClient!!.getParentCategoryList("tes").blockingFirst()
+        assertEquals("Test", actualCategoryName)
+    }
+
+    @Test
+    fun getParentCategoryListNull() {
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.pages()).thenReturn(null)
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+
+        Mockito.`when`(categoryInterface!!.getParentCategoryList(ArgumentMatchers.anyString()))
+                .thenReturn(Observable.just(mockResponse))
+        categoryClient!!.getParentCategoryList("tes").subscribe(
+                { fail("SearchCategories returned element when it shouldn't have.") },
+                { s -> throw s })
+    }
+    @Test
+    fun getSubCategoryListFound() {
+        val mwQueryPage = Mockito.mock(MwQueryPage::class.java)
+        Mockito.`when`(mwQueryPage.title()).thenReturn("Category:Test")
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage))
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+
+        Mockito.`when`(categoryInterface!!.getSubCategoryList(ArgumentMatchers.anyString()))
+                .thenReturn(Observable.just(mockResponse))
+
+        val actualCategoryName = categoryClient!!.getSubCategoryList("tes").blockingFirst()
+        assertEquals("Test", actualCategoryName)
+    }
+
+    @Test
+    fun getSubCategoryListNull() {
+        val mwQueryResult = Mockito.mock(MwQueryResult::class.java)
+        Mockito.`when`(mwQueryResult.pages()).thenReturn(null)
+        val mockResponse = Mockito.mock(MwQueryResponse::class.java)
+        Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult)
+
+        Mockito.`when`(categoryInterface!!.getSubCategoryList(ArgumentMatchers.anyString()))
+                .thenReturn(Observable.just(mockResponse))
+        categoryClient!!.getSubCategoryList("tes").subscribe(
+                { fail("SearchCategories returned element when it shouldn't have.") },
                 { s -> throw s })
     }
 }

app/src/test/kotlin/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApiTest.kt

@@ -38,7 +38,7 @@ class ApacheHttpClientMediaWikiApiTest {
         wikidataServer = MockWebServer()
         okHttpClient = OkHttpClient()
         sharedPreferences = mock(JsonKvStore::class.java)
-        testObject = ApacheHttpClientMediaWikiApi(ApplicationProvider.getApplicationContext(), "http://" + server.hostName + ":" + server.port + "/", "http://" + wikidataServer.hostName + ":" + wikidataServer.port + "/", sharedPreferences, Gson())
+        testObject = ApacheHttpClientMediaWikiApi(ApplicationProvider.getApplicationContext(), "http://" + server.hostName + ":" + server.port + "/", "http://" + wikidataServer.hostName + ":" + wikidataServer.port + "/", sharedPreferences)
     }
 
     @After