From f2cfccd4981ddc45667609c19e864b096d0350a7 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Thu, 4 Jul 2019 22:33:06 +0300 Subject: [PATCH 1/8] Commit 1 --- .../nrw/commons/category/CategoryClient.java | 32 +++++++++++++++++++ .../commons/category/CategoryInterface.java | 16 ++++++++++ 2 files changed, 48 insertions(+) create mode 100644 app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java create mode 100644 app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java new file mode 100644 index 0000000000..7796621003 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java @@ -0,0 +1,32 @@ +package fr.free.nrw.commons.category; + + +import javax.inject.Inject; +import javax.inject.Singleton; + +import io.reactivex.Single; +import retrofit2.http.Query; + +/** + * Category Client to handle custom calls to Commons CategoryWiki APIs + */ +@Singleton +public class CategoryClient { + + private final CategoryInterface CategoryInterface; + + @Inject + public CategoryClient(CategoryInterface CategoryInterface) { + this.CategoryInterface = CategoryInterface; + } + + /** + * + */ + public Single searchCategories(String filterValue, int searchCatsLimit) { + return CategoryInterface.searchCategories(filterValue, searchCatsLimit) + .map(mwQueryResponse -> mwQueryResponse + .query().firstPage().pageId() > 0) + .singleOrError(); + } +} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java new file mode 100644 index 0000000000..df3168baae --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java @@ -0,0 +1,16 @@ +package fr.free.nrw.commons.category; + +import org.wikipedia.dataclient.mwapi.MwQueryResponse; + +import io.reactivex.Observable; +import retrofit2.http.GET; +import retrofit2.http.Query; + +/** + * Interface for interacting with Commons category related APIs + */ +public interface CategoryInterface { + + @GET("w/api.php?action=query&format=json&formatversion=2") + Observable searchCategories(@Query("srsearch") String filterValue, @Query("srlimit") int searchCatsLimit); +} From e5c0dc9b37c528100ad84d102bbd4868bbed67a1 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 02:40:55 +0300 Subject: [PATCH 2/8] searchCategories migrated to retrofit --- .../nrw/commons/category/CategoriesModel.java | 7 ++-- .../nrw/commons/category/CategoryClient.java | 32 ++++++++++++++----- .../commons/category/CategoryInterface.java | 13 ++++++-- .../free/nrw/commons/di/NetworkingModule.java | 7 ++++ 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java index 9b084da491..50c87de6fd 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java @@ -23,6 +23,7 @@ public class CategoriesModel{ private static final int SEARCH_CATS_LIMIT = 25; private final MediaWikiApi mwApi; + private final CategoryClient categoryClient; private final CategoryDao categoryDao; private final JsonKvStore directKvStore; @@ -32,9 +33,11 @@ public class CategoriesModel{ @Inject GpsCategoryModel gpsCategoryModel; @Inject public CategoriesModel(MediaWikiApi mwApi, + CategoryClient categoryClient, CategoryDao categoryDao, @Named("default_preferences") JsonKvStore directKvStore) { this.mwApi = mwApi; + this.categoryClient = categoryClient; this.categoryDao = categoryDao; this.directKvStore = directKvStore; this.categoriesCache = new HashMap<>(); @@ -121,8 +124,8 @@ public Observable searchAll(String term, List imageTitleLi } //otherwise, search API for matching categories - return mwApi - .allCategories(term, SEARCH_CATS_LIMIT) + return categoryClient + .searchCategories(term, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java index 7796621003..5df1238b8e 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java @@ -1,14 +1,18 @@ package fr.free.nrw.commons.category; +import org.wikipedia.dataclient.mwapi.MwQueryPage; + +import java.util.List; + import javax.inject.Inject; import javax.inject.Singleton; -import io.reactivex.Single; -import retrofit2.http.Query; +import io.reactivex.Observable; +import timber.log.Timber; /** - * Category Client to handle custom calls to Commons CategoryWiki APIs + * Category Client to handle custom calls to Commons MediaWiki APIs */ @Singleton public class CategoryClient { @@ -21,12 +25,24 @@ public CategoryClient(CategoryInterface CategoryInterface) { } /** + * Searches for categories with the specified name. * + * @param filter The string to be searched + * @param itemLimit How many results are returned + * @return */ - public Single searchCategories(String filterValue, int searchCatsLimit) { - return CategoryInterface.searchCategories(filterValue, searchCatsLimit) - .map(mwQueryResponse -> mwQueryResponse - .query().firstPage().pageId() > 0) - .singleOrError(); + public Observable searchCategories(String filter, int itemLimit) { + return CategoryInterface.searchCategories(filter, itemLimit) + .flatMap(mwQueryResponse -> { + List pages = mwQueryResponse.query().pages(); + if(pages != null) + return Observable.fromIterable(pages); + else + Timber.d("No categories returned."); + return Observable.empty(); + }) + .map(MwQueryPage::title) + .doOnEach(s->Timber.d("Category returned: %s", s)) + .map(cat->cat.replace("Category:", "")); } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java index df3168baae..e5e364e3b7 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java @@ -11,6 +11,15 @@ */ public interface CategoryInterface { - @GET("w/api.php?action=query&format=json&formatversion=2") - Observable searchCategories(@Query("srsearch") String filterValue, @Query("srlimit") int searchCatsLimit); + /** + * Searches for categories with the specified name. + * Replaces ApacheHttpClientMediaWikiApi#allCategories + * + * @param filter The string to be searched + * @param itemLimit How many results are returned + * @return + */ + @GET("w/api.php?action=query&format=json&formatversion=2" + + "&generator=search&gsrnamespace=14") + Observable searchCategories(@Query("gsrsearch") String filter, @Query("gsrlimit") int itemLimit); } 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 103f0d9a82..5494ae3053 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 @@ -18,6 +18,7 @@ import dagger.Module; import dagger.Provides; import fr.free.nrw.commons.BuildConfig; +import fr.free.nrw.commons.category.CategoryInterface; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.media.MediaInterface; import fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi; @@ -132,4 +133,10 @@ public ReviewInterface provideReviewInterface(@Named(NAMED_COMMONS_WIKI_SITE) Wi public MediaInterface provideMediaInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, MediaInterface.class); } + + @Provides + @Singleton + public CategoryInterface provideCategoryInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { + return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, CategoryInterface.class); + } } From 42e89a88e947ebfea568572be58ae2055108f4a7 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 12:29:35 +0300 Subject: [PATCH 3/8] SearchCategoriesFragment migrated to new API --- .../explore/categories/SearchCategoryFragment.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java index b5301e39a8..e20b251c98 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java @@ -25,6 +25,7 @@ import butterknife.BindView; import butterknife.ButterKnife; import fr.free.nrw.commons.R; +import fr.free.nrw.commons.category.CategoryClient; import fr.free.nrw.commons.category.CategoryDetailsActivity; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.explore.recentsearches.RecentSearch; @@ -61,6 +62,8 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { @Inject RecentSearchesDao recentSearchesDao; @Inject MediaWikiApi mwApi; + @Inject CategoryClient categoryClient; + @Inject @Named("default_preferences") JsonKvStore basicKvStore; @@ -135,11 +138,12 @@ public void updateCategoryList(String query) { progressBar.setVisibility(GONE); queryList.clear(); categoriesAdapter.clear(); - compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) + compositeDisposable.add(categoryClient.searchCategories(query,queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .doOnSubscribe(disposable -> saveQuery(query)) + .collect(ArrayList::new, ArrayList::add) .subscribe(this::handleSuccess, this::handleError)); } @@ -151,10 +155,11 @@ public void addCategoriesToList(String query) { this.query = query; bottomProgressBar.setVisibility(View.VISIBLE); progressBar.setVisibility(GONE); - compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) + compositeDisposable.add(categoryClient.searchCategories(query,queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) + .collect(ArrayList::new, ArrayList::add) .subscribe(this::handlePaginationSuccess, this::handleError)); } @@ -213,7 +218,7 @@ private void handleError(Throwable throwable) { private void initErrorView() { progressBar.setVisibility(GONE); categoriesNotFoundView.setVisibility(VISIBLE); - categoriesNotFoundView.setText(getString(R.string.categories_not_found, query)); + categoriesNotFoundView.setText(getString(R.string.categories_not_found)); } /** From d3f54e17281ade993b28ba9ef230944087cbf1e5 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 12:30:11 +0300 Subject: [PATCH 4/8] Removed unused code --- .../mwapi/ApacheHttpClientMediaWikiApi.java | 127 +++--------------- .../free/nrw/commons/mwapi/MediaWikiApi.java | 9 -- 2 files changed, 15 insertions(+), 121 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java b/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java index 727a8f2a2c..98270c9866 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java @@ -152,7 +152,7 @@ private String getErrorCodeToReturn(CustomApiResult loginCustomApiResult) { status.equals("UI") && loginCustomApiResult.getString("/api/clientlogin/requests/_v/@id").equals("TOTPAuthenticationRequest") && loginCustomApiResult.getString("/api/clientlogin/requests/_v/@provider").equals("Two-factor authentication (OATH).") - ) { + ) { setAuthCookieOnLogin(false); return "2FA"; } @@ -251,7 +251,6 @@ public String edit(String editToken, String processedPageContent, String filenam } - @Override @Nullable public String appendEdit(String editToken, String processedPageContent, String filename, String summary) throws IOException { @@ -305,71 +304,6 @@ public Single fetchMediaByFilename(String filename) { }); } - @Override - @NonNull - public Observable searchCategories(String filterValue, int searchCatsLimit) { - List categories = new ArrayList<>(); - return Single.fromCallable(() -> { - List categoryNodes = null; - try { - categoryNodes = api.action("query") - .param("format", "xml") - .param("list", "search") - .param("srwhat", "text") - .param("srnamespace", "14") - .param("srlimit", searchCatsLimit) - .param("srsearch", filterValue) - .get() - .getNodes("/api/query/search/p/@title"); - } catch (IOException e) { - Timber.e(e, "Failed to obtain searchCategories"); - } - - if (categoryNodes == null) { - return new ArrayList(); - } - - for (CustomApiResult categoryNode : categoryNodes) { - String cat = categoryNode.getDocument().getTextContent(); - String catString = cat.replace("Category:", ""); - if (!categories.contains(catString)) { - categories.add(catString); - } - } - - return categories; - }).flatMapObservable(Observable::fromIterable); - } - - @Override - @NonNull - public Observable allCategories(String filterValue, int searchCatsLimit) { - return Single.fromCallable(() -> { - ArrayList categoryNodes = null; - try { - categoryNodes = api.action("query") - .param("list", "allcategories") - .param("acprefix", filterValue) - .param("aclimit", searchCatsLimit) - .get() - .getNodes("/api/query/allcategories/c"); - } catch (IOException e) { - Timber.e(e, "Failed to obtain allCategories"); - } - - if (categoryNodes == null) { - return new ArrayList(); - } - - List categories = new ArrayList<>(); - for (CustomApiResult categoryNode : categoryNodes) { - categories.add(categoryNode.getDocument().getTextContent()); - } - - return categories; - }).flatMapObservable(Observable::fromIterable); - } - @Override public String getWikidataCsrfToken() throws IOException { String wikidataCsrfToken = wikidataApi.action("query") @@ -385,10 +319,11 @@ public String getWikidataCsrfToken() throws IOException { /** * Creates a new claim using the wikidata API * https://www.mediawiki.org/wiki/Wikibase/API + * * @param entityId the wikidata entity to be edited * @param property the property to be edited, for eg P18 for images * @param snaktype the type of value stored for that property - * @param value the actual value to be stored for the property, for eg filename in case of P18 + * @param value the actual value to be stored for the property, for eg filename in case of P18 * @return returns revisionId if the claim is successfully created else returns null * @throws IOException */ @@ -422,6 +357,7 @@ public String wikidataCreateClaim(String entityId, String property, String snakt /** * Adds the wikimedia-commons-app tag to the edits made on wikidata + * * @param revisionId * @return * @throws IOException @@ -543,13 +479,13 @@ public List getNotifications(boolean archived) { try { if (archived) { notfilter = "read"; - }else { + } else { notfilter = "!read"; } - String language=Locale.getDefault().getLanguage(); - if(StringUtils.isBlank(language)){ + String language = Locale.getDefault().getLanguage(); + if (StringUtils.isBlank(language)) { //if no language is set we use the default user language defined on wikipedia - language="user"; + language = "user"; } notificationNode = api.action("query") .param("notprop", "list") @@ -595,6 +531,7 @@ public boolean markNotificationAsRead(Notification notification) throws IOExcept * 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 */ @@ -606,7 +543,7 @@ public List getSubCategoryList(String categoryName) { CustomMwApi.RequestBuilder requestBuilder = api.action("query") .param("generator", "categorymembers") .param("format", "xml") - .param("gcmtype","subcat") + .param("gcmtype", "subcat") .param("gcmtitle", categoryName) .param("prop", "info") .param("gcmlimit", "500") @@ -636,6 +573,7 @@ public List getSubCategoryList(String 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 */ @@ -673,48 +611,12 @@ public List getParentCategoryList(String categoryName) { return CategoryImageUtils.getSubCategoryList(childNodes); } - /** - * This method takes search keyword as input and returns a list of categories objects filtered using search query - * It uses the generator query API to get the categories searched using a query, 25 at a time. - * @param query keyword to search categories on commons - * @return - */ - @Override - @NonNull - public List searchCategory(String query, int offset) { - List categoryNodes = null; - try { - categoryNodes = api.action("query") - .param("format", "xml") - .param("list", "search") - .param("srwhat", "text") - .param("srnamespace", "14") - .param("srlimit", "25") - .param("sroffset",offset) - .param("srsearch", query) - .get() - .getNodes("/api/query/search/p/@title"); - } catch (IOException e) { - Timber.e(e, "Failed to obtain searchCategories"); - } - - if (categoryNodes == null) { - return new ArrayList<>(); - } - - List categories = new ArrayList<>(); - for (CustomApiResult categoryNode : categoryNodes) { - String catName = categoryNode.getDocument().getTextContent(); - categories.add(catName); - } - return categories; - } - /** * 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 */ @@ -724,6 +626,7 @@ private void setQueryContinueValues(String keyword, QueryContinue queryContinue) /** * Before making a paginated API call, this method is called to get the latest query continue values to be used + * * @param keyword * @return */ @@ -751,7 +654,7 @@ public Single uploadFile( if (!resultStatus.equals("Success")) { String errorCode = result.getString("/api/error/@code"); Timber.e(errorCode); - + if (errorCode.equals(ERROR_CODE_BAD_TOKEN)) { ViewUtil.showLongToast(context, R.string.bad_token_error_proposed_solution); } @@ -799,8 +702,8 @@ public Single uploadFileFinalize( } /** - * Checks to see if a user is currently blocked from Commons + * * @return whether or not the user is blocked from Commons */ @Override diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java b/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java index 9fafacfffe..08ea53730f 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java @@ -34,9 +34,6 @@ public interface MediaWikiApi { List getParentCategoryList(String categoryName); - @NonNull - List searchCategory(String title, int offset); - @NonNull Single uploadFile(String filename, InputStream file, long dataLength, Uri fileUri, Uri contentProviderUri, @@ -65,12 +62,6 @@ Single uploadFileFinalize(String filename, String filekey, @NonNull Single fetchMediaByFilename(String filename); - @NonNull - Observable searchCategories(String filterValue, int searchCatsLimit); - - @NonNull - Observable allCategories(String filter, int searchCatsLimit); - @NonNull List getNotifications(boolean archived) throws IOException; From 9d026e4fcc77f5cce017c77c9d5cb6f600448741 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 12:30:57 +0300 Subject: [PATCH 5/8] Created tests --- .../commons/category/CategoryClientTest.kt | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt diff --git a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt new file mode 100644 index 0000000000..5a6ca539d1 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt @@ -0,0 +1,54 @@ +package fr.free.nrw.commons.category + +import io.reactivex.Observable +import junit.framework.Assert +import org.junit.Before +import org.junit.Test +import org.mockito.* +import org.wikipedia.dataclient.mwapi.MwQueryPage +import org.wikipedia.dataclient.mwapi.MwQueryResponse +import org.wikipedia.dataclient.mwapi.MwQueryResult + +class CategoryClientTest { + @Mock + internal var categoryInterface: CategoryInterface? = null + + @InjectMocks + var categoryClient: CategoryClient? = null + + @Before + @Throws(Exception::class) + fun setUp() { + MockitoAnnotations.initMocks(this) + } + + @Test + fun searchCategoriesFound() { + 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!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt())) + .thenReturn(Observable.just(mockResponse)) + + val actualCategoryName = categoryClient!!.searchCategories("tes", 10).blockingFirst() + Assert.assertEquals("Test", actualCategoryName) + } + + @Test + fun searchCategoriesNull() { + 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!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt())) + .thenReturn(Observable.just(mockResponse)) + categoryClient!!.searchCategories("tes", 10).subscribe( + { Assert.fail("SearchCategories returned element when it shouldn't have.") }, + { s -> throw s }) + } +} \ No newline at end of file From 7679136731ea5adfb3011f9411da844fe3e0f1a2 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 16:11:38 +0300 Subject: [PATCH 6/8] implemented searching by prefix fixed SearchCategoryFragment behaviour where the same categories would be added to the list instead of new ones. --- .../nrw/commons/category/CategoriesModel.java | 2 +- .../nrw/commons/category/CategoryClient.java | 61 +++++++++++++++++-- .../commons/category/CategoryInterface.java | 8 ++- .../categories/SearchCategoryFragment.java | 13 ++-- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java index 50c87de6fd..3bf896f141 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java @@ -125,7 +125,7 @@ public Observable searchAll(String term, List imageTitleLi //otherwise, search API for matching categories return categoryClient - .searchCategories(term, SEARCH_CATS_LIMIT) + .searchCategoriesForPrefix(term, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java index 5df1238b8e..603be5d554 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java @@ -2,6 +2,7 @@ import org.wikipedia.dataclient.mwapi.MwQueryPage; +import org.wikipedia.dataclient.mwapi.MwQueryResponse; import java.util.List; @@ -25,24 +26,72 @@ public CategoryClient(CategoryInterface CategoryInterface) { } /** - * Searches for categories with the specified name. + * Searches for categories containing the specified string. + * + * @param filter The string to be searched + * @param itemLimit How many results are returned + * @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result + * @return + */ + public Observable searchCategories(String filter, int itemLimit, int offset) { + return responseToCategoryName(CategoryInterface.searchCategories(filter, itemLimit, offset)); + + } + + /** + * Searches for categories containing the specified string. * * @param filter The string to be searched * @param itemLimit How many results are returned * @return */ public Observable searchCategories(String filter, int itemLimit) { - return CategoryInterface.searchCategories(filter, itemLimit) + return searchCategories(filter, itemLimit, 0); + + } + + /** + * Searches for categories starting with the specified string. + * + * @param prefix The prefix to be searched + * @param itemLimit How many results are returned + * @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result + * @return + */ + public Observable searchCategoriesForPrefix(String prefix, int itemLimit, int offset) { + return responseToCategoryName(CategoryInterface.searchCategoriesForPrefix(prefix, itemLimit, offset)); + } + + /** + * Searches for categories starting with the specified string. + * + * @param prefix The prefix to be searched + * @param itemLimit How many results are returned + * @return + */ + public Observable searchCategoriesForPrefix(String prefix, int itemLimit) { + return searchCategoriesForPrefix(prefix, itemLimit, 0); + } + + + /** + * Internal function to reduce code reuse. Extracts the categories returned from MwQueryResponse. + * + * @param responseObservable The query response observable + * @return Observable emitting the categories returned. If our search yielded "Category:Test", "Test" is emitted. + */ + private Observable responseToCategoryName(Observable responseObservable) { + return responseObservable .flatMap(mwQueryResponse -> { List pages = mwQueryResponse.query().pages(); - if(pages != null) + if (pages != null) return Observable.fromIterable(pages); else Timber.d("No categories returned."); - return Observable.empty(); + return Observable.empty(); }) .map(MwQueryPage::title) - .doOnEach(s->Timber.d("Category returned: %s", s)) - .map(cat->cat.replace("Category:", "")); + .doOnEach(s -> Timber.d("Category returned: %s", s)) + .map(cat -> cat.replace("Category:", "")); } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java index e5e364e3b7..aad1cb892a 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryInterface.java @@ -21,5 +21,11 @@ public interface CategoryInterface { */ @GET("w/api.php?action=query&format=json&formatversion=2" + "&generator=search&gsrnamespace=14") - Observable searchCategories(@Query("gsrsearch") String filter, @Query("gsrlimit") int itemLimit); + Observable searchCategories(@Query("gsrsearch") String filter, + @Query("gsrlimit") int itemLimit, @Query("gsroffset") int offset); + + @GET("w/api.php?action=query&format=json&formatversion=2" + + "&generator=allcategories") + Observable searchCategoriesForPrefix(@Query("gacprefix") String prefix, + @Query("gaclimit") int itemLimit, @Query("gacoffset") int offset); } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java index e20b251c98..7cfea064a3 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/categories/SearchCategoryFragment.java @@ -59,6 +59,7 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { String query; @BindView(R.id.bottomProgressBar) ProgressBar bottomProgressBar; + boolean isLoadingCategories; @Inject RecentSearchesDao recentSearchesDao; @Inject MediaWikiApi mwApi; @@ -138,7 +139,7 @@ public void updateCategoryList(String query) { progressBar.setVisibility(GONE); queryList.clear(); categoriesAdapter.clear(); - compositeDisposable.add(categoryClient.searchCategories(query,queryList.size()) + compositeDisposable.add(categoryClient.searchCategories(query,25) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) @@ -149,13 +150,15 @@ public void updateCategoryList(String query) { /** - * Adds more results to existing search results + * Adds 25 more results to existing search results */ public void addCategoriesToList(String query) { + if(isLoadingCategories) return; + isLoadingCategories=true; this.query = query; bottomProgressBar.setVisibility(View.VISIBLE); progressBar.setVisibility(GONE); - compositeDisposable.add(categoryClient.searchCategories(query,queryList.size()) + compositeDisposable.add(categoryClient.searchCategories(query,25, queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) @@ -166,7 +169,6 @@ public void addCategoriesToList(String query) { /** * Handles the success scenario * it initializes the recycler view by adding items to the adapter - * @param mediaList */ private void handlePaginationSuccess(List mediaList) { queryList.addAll(mediaList); @@ -174,6 +176,7 @@ private void handlePaginationSuccess(List mediaList) { bottomProgressBar.setVisibility(GONE); categoriesAdapter.addAll(mediaList); categoriesAdapter.notifyDataSetChanged(); + isLoadingCategories=false; } @@ -181,7 +184,6 @@ private void handlePaginationSuccess(List mediaList) { /** * Handles the success scenario * it initializes the recycler view by adding items to the adapter - * @param mediaList */ private void handleSuccess(List mediaList) { queryList = mediaList; @@ -199,7 +201,6 @@ private void handleSuccess(List mediaList) { /** * Logs and handles API error scenario - * @param throwable */ private void handleError(Throwable throwable) { Timber.e(throwable, "Error occurred while loading queried categories"); From d95c85ba3b69f55e0606f89ca427deee1316f571 Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 16:20:26 +0300 Subject: [PATCH 7/8] added tests --- .../commons/category/CategoryClientTest.kt | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt index 5a6ca539d1..e988398d7a 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt @@ -31,11 +31,14 @@ class CategoryClientTest { val mockResponse = Mockito.mock(MwQueryResponse::class.java) Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult) - Mockito.`when`(categoryInterface!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt())) + Mockito.`when`(categoryInterface!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())) .thenReturn(Observable.just(mockResponse)) val actualCategoryName = categoryClient!!.searchCategories("tes", 10).blockingFirst() Assert.assertEquals("Test", actualCategoryName) + + val actualCategoryName2 = categoryClient!!.searchCategories("tes", 10, 10).blockingFirst() + Assert.assertEquals("Test", actualCategoryName2) } @Test @@ -45,10 +48,48 @@ class CategoryClientTest { val mockResponse = Mockito.mock(MwQueryResponse::class.java) Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult) - Mockito.`when`(categoryInterface!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt())) + Mockito.`when`(categoryInterface!!.searchCategories(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())) .thenReturn(Observable.just(mockResponse)) + categoryClient!!.searchCategories("tes", 10).subscribe( { Assert.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.") }, + { s -> throw s }) + } + @Test + fun searchCategoriesForPrefixFound() { + 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!!.searchCategoriesForPrefix(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())) + .thenReturn(Observable.just(mockResponse)) + + val actualCategoryName = categoryClient!!.searchCategoriesForPrefix("tes", 10).blockingFirst() + Assert.assertEquals("Test", actualCategoryName) + val actualCategoryName2 = categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).blockingFirst() + Assert.assertEquals("Test", actualCategoryName2) + } + + @Test + fun searchCategoriesForPrefixNull() { + 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!!.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.") }, + { s -> throw s }) + categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).subscribe( + { Assert.fail("SearchCategories returned element when it shouldn't have.") }, + { s -> throw s }) } } \ No newline at end of file From fabf90390f1fd6fc66cb5a5cc1ed05318ffb361f Mon Sep 17 00:00:00 2001 From: ilgazer Date: Fri, 5 Jul 2019 16:31:33 +0300 Subject: [PATCH 8/8] Migrated searchTitles to searchCategories, function behaviour seems identical --- .../nrw/commons/category/CategoriesModel.java | 2 +- .../mwapi/ApacheHttpClientMediaWikiApi.java | 36 ------------------- .../free/nrw/commons/mwapi/MediaWikiApi.java | 3 -- 3 files changed, 1 insertion(+), 40 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java index 3bf896f141..bd0a6a08df 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java @@ -188,7 +188,7 @@ private Observable titleCategories(List titleList) { * @return */ private Observable getTitleCategories(String title) { - return mwApi.searchTitles(title, SEARCH_CATS_LIMIT) + return categoryClient.searchCategories(title, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java b/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java index 98270c9866..e81798a039 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java @@ -387,42 +387,6 @@ public boolean addWikidataEditTag(String revisionId) throws IOException { return false; } - @Override - @NonNull - public Observable searchTitles(String title, int searchCatsLimit) { - return Single.fromCallable((Callable>) () -> { - ArrayList categoryNodes; - - try { - categoryNodes = api.action("query") - .param("format", "xml") - .param("list", "search") - .param("srwhat", "text") - .param("srnamespace", "14") - .param("srlimit", searchCatsLimit) - .param("srsearch", title) - .get() - .getNodes("/api/query/search/p/@title"); - } catch (IOException e) { - Timber.e(e, "Failed to obtain searchTitles"); - return Collections.emptyList(); - } - - if (categoryNodes == null) { - return Collections.emptyList(); - } - - List titleCategories = new ArrayList<>(); - for (CustomApiResult categoryNode : categoryNodes) { - String cat = categoryNode.getDocument().getTextContent(); - String catString = cat.replace("Category:", ""); - titleCategories.add(catString); - } - - return titleCategories; - }).flatMapObservable(Observable::fromIterable); - } - @Override @NonNull public LogEventResult logEvents(String user, String lastModified, String queryContinue, int limit) throws IOException { diff --git a/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java b/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java index 08ea53730f..63faa0e8ec 100644 --- a/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java +++ b/app/src/main/java/fr/free/nrw/commons/mwapi/MediaWikiApi.java @@ -68,9 +68,6 @@ Single uploadFileFinalize(String filename, String filekey, @NonNull boolean markNotificationAsRead(Notification notification) throws IOException; - @NonNull - Observable searchTitles(String title, int searchCatsLimit); - @Nullable String revisionsByFilename(String filename) throws IOException;