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 deleted file mode 100644 index d8f9eb45e..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ /dev/null @@ -1,242 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.text.TextUtils; -import fr.free.nrw.commons.kvstore.JsonKvStore; -import fr.free.nrw.commons.upload.GpsCategoryModel; -import fr.free.nrw.commons.utils.StringSortingUtils; -import io.reactivex.Observable; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.Comparator; -import java.util.Date; -import java.util.List; -import javax.inject.Inject; -import javax.inject.Named; -import timber.log.Timber; - -/** - * The model class for categories in upload - */ -public class CategoriesModel{ - private static final int SEARCH_CATS_LIMIT = 25; - - private final CategoryClient categoryClient; - private final CategoryDao categoryDao; - private final JsonKvStore directKvStore; - private final GpsCategoryModel gpsCategoryModel; - - private List selectedCategories; - - @Inject - public CategoriesModel(CategoryClient categoryClient, - CategoryDao categoryDao, - @Named("default_preferences") JsonKvStore directKvStore, - final GpsCategoryModel gpsCategoryModel) { - this.categoryClient = categoryClient; - this.categoryDao = categoryDao; - this.directKvStore = directKvStore; - this.gpsCategoryModel = gpsCategoryModel; - this.selectedCategories = new ArrayList<>(); - } - - /** - * Sorts CategoryItem by similarity - * @param filter - * @return - */ - public Comparator sortBySimilarity(final String filter) { - Comparator stringSimilarityComparator = StringSortingUtils.sortBySimilarity(filter); - return (firstItem, secondItem) -> stringSimilarityComparator - .compare(firstItem.getName(), secondItem.getName()); - } - - /** - * Returns if the item contains an year - * @param item - * @return - */ - public boolean containsYear(String item) { - //Check for current and previous year to exclude these categories from removal - Calendar now = Calendar.getInstance(); - int year = now.get(Calendar.YEAR); - String yearInString = String.valueOf(year); - - int prevYear = year - 1; - String prevYearInString = String.valueOf(prevYear); - Timber.d("Previous year: %s", prevYearInString); - - //Check if item contains a 4-digit word anywhere within the string (.* is wildcard) - //And that item does not equal the current year or previous year - //And if it is an irrelevant category such as Media_needing_categories_as_of_16_June_2017(Issue #750) - //Check if the year in the form of XX(X)0s is relevant, i.e. in the 2000s or 2010s as stated in Issue #1029 - return ((item.matches(".*(19|20)\\d{2}.*") && !item.contains(yearInString) && !item.contains(prevYearInString)) - || item.matches("(.*)needing(.*)") || item.matches("(.*)taken on(.*)") - || (item.matches(".*0s.*") && !item.matches(".*(200|201)0s.*"))); - } - - /** - * Updates category count in category dao - * @param item - */ - public void updateCategoryCount(CategoryItem item) { - Category category = categoryDao.find(item.getName()); - - // Newly used category... - if (category == null) { - category = new Category(null, item.getName(), new Date(), 0); - } - - category.incTimesUsed(); - categoryDao.save(category); - } - - - /** - * Regional category search - * @param term - * @param imageTitleList - * @return - */ - public Observable searchAll(String term, List imageTitleList) { - //If query text is empty, show him category based on gps and title and recent searches - if (TextUtils.isEmpty(term)) { - Observable categoryItemObservable = - Observable.concat(gpsCategories(), titleCategories(imageTitleList)); - if (hasDirectCategories()) { - return Observable.concat( - categoryItemObservable, - directCategories(), - recentCategories() - ); - } - return categoryItemObservable; - } - - //otherwise, search API for matching categories - //term passed as lower case to make search case-insensitive(taking only lower case for everything) - return categoryClient - .searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT) - .map(name -> new CategoryItem(name, false)); - } - - - /** - * Returns if we have a category in DirectKV Store - * @return - */ - private boolean hasDirectCategories() { - return !directKvStore.getString("Category", "").equals(""); - } - - /** - * Returns categories in DirectKVStore - * @return - */ - private Observable directCategories() { - String directCategory = directKvStore.getString("Category", ""); - List categoryList = new ArrayList<>(); - Timber.d("Direct category found: " + directCategory); - - if (!directCategory.equals("")) { - categoryList.add(directCategory); - Timber.d("DirectCat does not equal emptyString. Direct Cat list has " + categoryList); - } - return Observable.fromIterable(categoryList).map(name -> new CategoryItem(name, false)); - } - - /** - * Returns GPS categories - * @return - */ - Observable gpsCategories() { - return Observable.fromIterable(gpsCategoryModel.getCategoryList()) - .map(name -> new CategoryItem(name, false)); - } - - /** - * Returns title based categories - * @param titleList - * @return - */ - private Observable titleCategories(List titleList) { - return Observable.fromIterable(titleList) - .concatMap(this::getTitleCategories); - } - - /** - * Return category for single title - * title is converted to lower case to make search case-insensitive - * @param title - * @return - */ - private Observable getTitleCategories(String title) { - return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT) - .map(name -> new CategoryItem(name, false)); - } - - /** - * Returns recent categories - * @return - */ - private Observable recentCategories() { - return Observable.fromIterable(categoryDao.recentCategories(SEARCH_CATS_LIMIT)) - .map(s -> new CategoryItem(s, false)); - } - - /** - * Handles category item selection - * @param item - */ - public void onCategoryItemClicked(CategoryItem item) { - if (item.isSelected()) { - selectCategory(item); - updateCategoryCount(item); - } else { - unselectCategory(item); - } - } - - /** - * Select's category - * @param item - */ - public void selectCategory(CategoryItem item) { - selectedCategories.add(item); - } - - /** - * Unselect Category - * @param item - */ - public void unselectCategory(CategoryItem item) { - selectedCategories.remove(item); - } - - - /** - * Get Selected Categories - * @return - */ - public List getSelectedCategories() { - return selectedCategories; - } - - /** - * Get Categories String List - * @return - */ - public List getCategoryStringList() { - List output = new ArrayList<>(); - for (CategoryItem item : selectedCategories) { - output.add(item.getName()); - } - return output; - } - - /** - * Cleanup the existing in memory cache's - */ - public void cleanUp() { - this.selectedCategories.clear(); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.kt b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.kt new file mode 100644 index 000000000..ae2d0b0fd --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.kt @@ -0,0 +1,152 @@ +package fr.free.nrw.commons.category + +import android.text.TextUtils +import fr.free.nrw.commons.upload.GpsCategoryModel +import fr.free.nrw.commons.utils.StringSortingUtils +import io.reactivex.Observable +import io.reactivex.functions.Function3 +import timber.log.Timber +import java.util.* +import javax.inject.Inject + +/** + * The model class for categories in upload + */ +class CategoriesModel @Inject constructor( + private val categoryClient: CategoryClient, + private val categoryDao: CategoryDao, + private val gpsCategoryModel: GpsCategoryModel +) { + private val selectedCategories: MutableList = mutableListOf() + + /** + * Returns if the item contains an year + * @param item + * @return + */ + fun containsYear(item: String): Boolean { + //Check for current and previous year to exclude these categories from removal + val now = Calendar.getInstance() + val year = now[Calendar.YEAR] + val yearInString = year.toString() + val prevYear = year - 1 + val prevYearInString = prevYear.toString() + Timber.d("Previous year: %s", prevYearInString) + + //Check if item contains a 4-digit word anywhere within the string (.* is wildcard) + //And that item does not equal the current year or previous year + //And if it is an irrelevant category such as Media_needing_categories_as_of_16_June_2017(Issue #750) + //Check if the year in the form of XX(X)0s is relevant, i.e. in the 2000s or 2010s as stated in Issue #1029 + return item.matches(".*(19|20)\\d{2}.*".toRegex()) + && !item.contains(yearInString) + && !item.contains(prevYearInString) + || item.matches("(.*)needing(.*)".toRegex()) + || item.matches("(.*)taken on(.*)".toRegex()) + || item.matches(".*0s.*".toRegex()) + && !item.matches(".*(200|201)0s.*".toRegex()) + } + + /** + * Updates category count in category dao + * @param item + */ + fun updateCategoryCount(item: CategoryItem) { + var category = categoryDao.find(item.name) + + // Newly used category... + if (category == null) { + category = Category(null, item.name, Date(), 0) + } + category.incTimesUsed() + categoryDao.save(category) + } + + /** + * Regional category search + * @param term + * @param imageTitleList + * @return + */ + fun searchAll(term: String, imageTitleList: List): Observable> { + return suggestionsOrSearch(term, imageTitleList) + .map { it.map { CategoryItem(it, false) } } + } + + private fun suggestionsOrSearch(term: String, imageTitleList: List): + Observable> { + return if (TextUtils.isEmpty(term)) + Observable.combineLatest( + gpsCategoryModel.categoriesFromLocation, + titleCategories(imageTitleList), + Observable.just(categoryDao.recentCategories(SEARCH_CATS_LIMIT)), + Function3(::combine) + ) + else + categoryClient.searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT) + .map { it.sortedWith(StringSortingUtils.sortBySimilarity(term)) } + } + + private fun combine( + locationCategories: List, + titles: List, + recents: List + ) = locationCategories + titles + recents + + + /** + * Returns title based categories + * @param titleList + * @return + */ + private fun titleCategories(titleList: List): Observable> { + return if (titleList.isNotEmpty()) + Observable.combineLatest(titleList.map { getTitleCategories(it) }) { searchResults -> + searchResults.map { it as List }.flatten() + } + else + Observable.just(emptyList()) + } + + /** + * Return category for single title + * title is converted to lower case to make search case-insensitive + * @param title + * @return + */ + private fun getTitleCategories(title: String): Observable> { + return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT) + } + + + /** + * Handles category item selection + * @param item + */ + fun onCategoryItemClicked(item: CategoryItem) { + if (item.isSelected) { + selectedCategories.add(item) + updateCategoryCount(item) + } else { + selectedCategories.remove(item) + } + } + + /** + * Get Selected Categories + * @return + */ + fun getSelectedCategories(): List { + return selectedCategories + } + + /** + * Cleanup the existing in memory cache's + */ + fun cleanUp() { + selectedCategories.clear() + } + + companion object { + private const val SEARCH_CATS_LIMIT = 25 + } +} 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 deleted file mode 100644 index 7a4d1718b..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java +++ /dev/null @@ -1,125 +0,0 @@ -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; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import io.reactivex.Observable; -import timber.log.Timber; - -/** - * Category Client to handle custom calls to Commons MediaWiki APIs - */ -@Singleton -public class CategoryClient { - - private final CategoryInterface CategoryInterface; - - @Inject - public CategoryClient(CategoryInterface CategoryInterface) { - this.CategoryInterface = CategoryInterface; - } - - /** - * 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 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); - } - - - /** - * 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 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 getParentCategoryList(String categoryName) { - return responseToCategoryName(CategoryInterface.getParentCategoryList(categoryName)); - } - - /** - * 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 -> { - MwQueryResult query; - List pages; - if ((query = mwQueryResponse.query()) == null || - (pages = query.pages()) == null) { - Timber.d("No categories returned."); - return Observable.empty(); - } else - return Observable.fromIterable(pages); - }) - .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/CategoryClient.kt b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt new file mode 100644 index 000000000..8b07f9d4a --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt @@ -0,0 +1,79 @@ +package fr.free.nrw.commons.category + +import io.reactivex.Observable +import org.wikipedia.dataclient.mwapi.MwQueryResponse +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Category Client to handle custom calls to Commons MediaWiki APIs + */ +@Singleton +class CategoryClient @Inject constructor(private val categoryInterface: CategoryInterface) { + /** + * 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 + */ + @JvmOverloads + fun searchCategories(filter: String?, itemLimit: Int, offset: Int = 0): + Observable> { + return responseToCategoryName(categoryInterface.searchCategories(filter, itemLimit, offset)) + } + + /** + * 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 + */ + @JvmOverloads + fun searchCategoriesForPrefix(prefix: String?, itemLimit: Int, offset: Int = 0): + Observable> { + return responseToCategoryName( + categoryInterface.searchCategoriesForPrefix(prefix, itemLimit, offset) + ) + } + + /** + * 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. + */ + fun getSubCategoryList(categoryName: String?): Observable> { + 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 + */ + fun getParentCategoryList(categoryName: String?): Observable> { + return responseToCategoryName(categoryInterface.getParentCategoryList(categoryName)) + } + + /** + * 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 fun responseToCategoryName(responseObservable: Observable): Observable> { + return responseObservable + .map { it.query()?.pages() ?: emptyList() } + .map { + it.map { page -> page.title().replace("Category:", "") } + } + } + +} diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java deleted file mode 100644 index c8f2a2713..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java +++ /dev/null @@ -1,79 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.os.Parcel; -import android.os.Parcelable; - -public class CategoryItem implements Parcelable { - private final String name; - private boolean selected; - - public static Creator CREATOR = new Creator() { - @Override - public CategoryItem createFromParcel(Parcel parcel) { - return new CategoryItem(parcel); - } - - @Override - public CategoryItem[] newArray(int i) { - return new CategoryItem[0]; - } - }; - - public CategoryItem(String name, boolean selected) { - this.name = name; - this.selected = selected; - } - - private CategoryItem(Parcel in) { - name = in.readString(); - selected = in.readInt() == 1; - } - - public String getName() { - return name; - } - - public boolean isSelected() { - return selected; - } - - public void setSelected(boolean selected) { - this.selected = selected; - } - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel(Parcel parcel, int flags) { - parcel.writeString(name); - parcel.writeInt(selected ? 1 : 0); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - CategoryItem that = (CategoryItem) o; - - return name.equals(that.name); - - } - - @Override - public int hashCode() { - return name.hashCode(); - } - - @Override - public String toString() { - return "CategoryItem: '" + name + '\''; - } -} \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.kt b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.kt new file mode 100644 index 000000000..e6a95d511 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.kt @@ -0,0 +1,27 @@ +package fr.free.nrw.commons.category + +import android.os.Parcelable +import kotlinx.android.parcel.Parcelize + +@Parcelize +data class CategoryItem(val name: String, var isSelected: Boolean) : Parcelable { + + override fun toString(): String { + return "CategoryItem: '$name'" + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + + other as CategoryItem + + if (name != other.name) return false + + return true + } + + override fun hashCode(): Int { + return name.hashCode() + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java b/app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java index 21587bc12..256ebcdea 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/SubCategoryListFragment.java @@ -91,13 +91,11 @@ public class SubCategoryListFragment extends CommonsDaggerSupportFragment { compositeDisposable.add(categoryClient.getParentCategoryList("Category:"+categoryName) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .collect(ArrayList::new, ArrayList::add) .subscribe(this::handleSuccess, this::handleError)); } else { compositeDisposable.add(categoryClient.getSubCategoryList("Category:"+categoryName) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .collect(ArrayList::new, ArrayList::add) .subscribe(this::handleSuccess, this::handleError)); } } 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 6b437c254..ac5797adc 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 @@ -133,7 +133,6 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .doOnSubscribe(disposable -> saveQuery(query)) - .collect(ArrayList::new, ArrayList::add) .subscribe(this::handleSuccess, this::handleError)); } @@ -150,7 +149,6 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { compositeDisposable.add(categoryClient.searchCategories(query,25, queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .collect(ArrayList::new, ArrayList::add) .subscribe(this::handlePaginationSuccess, this::handleError)); } diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java index 5dc9c137a..405ed7ef4 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java @@ -18,7 +18,6 @@ import io.reactivex.Flowable; import io.reactivex.Observable; import io.reactivex.Single; import java.io.IOException; -import java.util.Comparator; import java.util.List; import java.util.Locale; import javax.inject.Inject; @@ -111,20 +110,10 @@ public class UploadRepository { * @param imageTitleList * @return */ - public Observable searchAll(String query, List imageTitleList) { + public Observable> searchAll(String query, List imageTitleList) { return categoriesModel.searchAll(query, imageTitleList); } - /** - * returns the string list of categories - * - * @return - */ - - public List getCategoryStringList() { - return categoriesModel.getCategoryStringList(); - } - /** * sets the list of selected categories for the current upload * @@ -143,16 +132,6 @@ public class UploadRepository { categoriesModel.onCategoryItemClicked(categoryItem); } - /** - * returns category sorted based on similarity with query - * - * @param query - * @return - */ - public Comparator sortBySimilarity(String query) { - return categoriesModel.sortBySimilarity(query); - } - /** * prunes the category list for irrelevant categories see #750 * diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt index adec4e7f5..ff3f63eb8 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt @@ -180,7 +180,7 @@ class FileProcessor @Inject constructor( .subscribeOn(Schedulers.io()) .observeOn(Schedulers.io()) .subscribe( - { gpsCategoryModel.categoryList = it }, + gpsCategoryModel::setCategoriesFromLocation, { Timber.e(it) gpsCategoryModel.clear() 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 deleted file mode 100644 index 78eece3b3..000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.java +++ /dev/null @@ -1,36 +0,0 @@ -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<>()); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.kt b/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.kt new file mode 100644 index 000000000..088b2f39e --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/GpsCategoryModel.kt @@ -0,0 +1,18 @@ +package fr.free.nrw.commons.upload + +import io.reactivex.subjects.BehaviorSubject +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class GpsCategoryModel @Inject constructor() { + val categoriesFromLocation = BehaviorSubject.createDefault(emptyList()) + + fun clear() { + categoriesFromLocation.onNext(emptyList()) + } + + fun setCategoriesFromLocation(categoryList: List) { + categoriesFromLocation.onNext(categoryList) + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java index 535dbb25f..16e437154 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java @@ -30,7 +30,6 @@ import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.LoginActivity; import fr.free.nrw.commons.auth.SessionManager; -import fr.free.nrw.commons.category.CategoriesModel; import fr.free.nrw.commons.contributions.ContributionController; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; @@ -63,8 +62,6 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, @Inject UploadContract.UserActionListener presenter; @Inject - CategoriesModel categoriesModel; - @Inject SessionManager sessionManager; @Inject UserClient userClient; @@ -96,7 +93,7 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, private CompositeDisposable compositeDisposable; private ProgressDialog progressDialog; private UploadImageAdapter uploadImagesAdapter; - private List fragments; + private List fragments; private UploadCategoriesFragment uploadCategoriesFragment; private DepictsFragment depictsFragment; private MediaLicenseFragment mediaLicenseFragment; @@ -416,6 +413,7 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, public void onNextButtonClicked(int index) { if (index < fragments.size() - 1) { vpUpload.setCurrentItem(index + 1, false); + fragments.get(index + 1).onBecameVisible(); } else { presenter.handleSubmit(); } @@ -425,6 +423,7 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, public void onPreviousButtonClicked(int index) { if (index != 0) { vpUpload.setCurrentItem(index - 1, true); + fragments.get(index - 1).onBecameVisible(); } } @@ -433,14 +432,14 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, */ private class UploadImageAdapter extends FragmentStatePagerAdapter { - List fragments; + List fragments; public UploadImageAdapter(FragmentManager fragmentManager) { super(fragmentManager); this.fragments = new ArrayList<>(); } - public void setFragments(List fragments) { + public void setFragments(List fragments) { this.fragments = fragments; notifyDataSetChanged(); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadBaseFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadBaseFragment.java index 967942b4b..c063bc1e9 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadBaseFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadBaseFragment.java @@ -22,6 +22,9 @@ public class UploadBaseFragment extends CommonsDaggerSupportFragment { this.callback = callback; } + protected void onBecameVisible() { + } + public interface Callback { void onNextButtonClicked(int index); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModule.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModule.java index 746bda0d1..0094fda22 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModule.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModule.java @@ -22,8 +22,8 @@ public abstract class UploadModule { presenter); @Binds - public abstract CategoriesContract.UserActionListener bindsCategoriesPresenter(CategoriesPresenter - presenter); + public abstract CategoriesContract.UserActionListener bindsCategoriesPresenter( + CategoriesPresenter presenter); @Binds public abstract MediaLicenseContract.UserActionListener bindsMediaLicensePresenter( diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java deleted file mode 100644 index 28a69110b..000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java +++ /dev/null @@ -1,146 +0,0 @@ -package fr.free.nrw.commons.upload.categories; - -import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; -import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; - -import android.text.TextUtils; -import fr.free.nrw.commons.R; -import fr.free.nrw.commons.category.CategoryItem; -import fr.free.nrw.commons.repository.UploadRepository; -import fr.free.nrw.commons.upload.UploadItem; -import io.reactivex.Observable; -import io.reactivex.Scheduler; -import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.disposables.Disposable; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.List; -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; -import timber.log.Timber; - -/** - * The presenter class for UploadCategoriesFragment - */ -@Singleton -public class CategoriesPresenter implements CategoriesContract.UserActionListener { - - private static final CategoriesContract.View DUMMY = (CategoriesContract.View) Proxy - .newProxyInstance( - CategoriesContract.View.class.getClassLoader(), - new Class[]{CategoriesContract.View.class}, - (proxy, method, methodArgs) -> null); - private final Scheduler ioScheduler; - private final Scheduler mainThreadScheduler; - - CategoriesContract.View view = DUMMY; - private UploadRepository repository; - - private CompositeDisposable compositeDisposable; - - @Inject - public CategoriesPresenter(UploadRepository repository, @Named(IO_THREAD) Scheduler ioScheduler, - @Named(MAIN_THREAD) Scheduler mainThreadScheduler) { - this.repository = repository; - this.ioScheduler = ioScheduler; - this.mainThreadScheduler = mainThreadScheduler; - compositeDisposable = new CompositeDisposable(); - } - - @Override - public void onAttachView(CategoriesContract.View view) { - this.view = view; - } - - @Override - public void onDetachView() { - this.view = DUMMY; - compositeDisposable.clear(); - } - - /** - * asks the repository to fetch categories for the query - * @param query - * - */ - @Override - public void searchForCategories(String query) { - List categoryItems = new ArrayList<>(); - List imageTitleList = getImageTitleList(); - Observable distinctCategoriesObservable = Observable - .fromIterable(repository.getSelectedCategories()) - .subscribeOn(ioScheduler) - .observeOn(mainThreadScheduler) - .doOnSubscribe(disposable -> { - view.showProgress(true); - view.showError(null); - view.setCategories(null); - }) - .observeOn(ioScheduler) - .concatWith( - repository.searchAll(query, imageTitleList) - ) - .filter(categoryItem -> !repository.containsYear(categoryItem.getName())) - .distinct(); - - if(!TextUtils.isEmpty(query)) { - distinctCategoriesObservable=distinctCategoriesObservable.sorted(repository.sortBySimilarity(query)); - } - Disposable searchCategoriesDisposable = distinctCategoriesObservable - .observeOn(mainThreadScheduler) - .subscribe( - s -> categoryItems.add(s), - Timber::e, - () -> { - view.setCategories(categoryItems); - view.showProgress(false); - - if (categoryItems.isEmpty()) { - view.showError(R.string.no_categories_found); - } - } - ); - - compositeDisposable.add(searchCategoriesDisposable); - } - - /** - * Returns image title list from UploadItem - * @return - */ - private List getImageTitleList() { - List titleList = new ArrayList<>(); - for (UploadItem item : repository.getUploads()) { - final String captionText = item.getUploadMediaDetails().get(0).getCaptionText(); - if (!TextUtils.isEmpty(captionText)) { - titleList.add(captionText); - } - } - return titleList; - } - - /** - * Verifies the number of categories selected, prompts the user if none selected - */ - @Override - public void verifyCategories() { - List selectedCategories = repository.getSelectedCategories(); - if (selectedCategories != null && !selectedCategories.isEmpty()) { - repository.setSelectedCategories(repository.getCategoryStringList()); - view.goToNextScreen(); - } else { - view.showNoCategorySelected(); - } - } - - /** - * ask repository to handle category clicked - * - * @param categoryItem - */ - @Override - public void onCategoryItemClicked(CategoryItem categoryItem) { - repository.onCategoryClicked(categoryItem); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt new file mode 100644 index 000000000..a8bbda460 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.kt @@ -0,0 +1,111 @@ +package fr.free.nrw.commons.upload.categories + +import android.text.TextUtils +import fr.free.nrw.commons.R +import fr.free.nrw.commons.category.CategoryItem +import fr.free.nrw.commons.di.CommonsApplicationModule +import fr.free.nrw.commons.repository.UploadRepository +import fr.free.nrw.commons.upload.depicts.proxy +import io.reactivex.Scheduler +import io.reactivex.disposables.CompositeDisposable +import io.reactivex.subjects.PublishSubject +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton + +/** + * The presenter class for UploadCategoriesFragment + */ +@Singleton +class CategoriesPresenter @Inject constructor( + private val repository: UploadRepository, + @param:Named(CommonsApplicationModule.IO_THREAD) private val ioScheduler: Scheduler, + @param:Named(CommonsApplicationModule.MAIN_THREAD) private val mainThreadScheduler: Scheduler +) : CategoriesContract.UserActionListener { + + companion object { + private val DUMMY: CategoriesContract.View = proxy() + } + + var view = DUMMY + private val compositeDisposable = CompositeDisposable() + private val searchTerms = PublishSubject.create() + + override fun onAttachView(view: CategoriesContract.View) { + this.view = view + compositeDisposable.add( + searchTerms + .observeOn(mainThreadScheduler) + .doOnNext { + view.showProgress(true) + view.showError(null) + view.setCategories(null) + } + .switchMap(::searchResults) + .map { repository.selectedCategories + it } + .map { it.distinctBy { categoryItem -> categoryItem.name } } + .observeOn(mainThreadScheduler) + .subscribe( + { + view.setCategories(it) + view.showProgress(false) + if (it.isEmpty()) { + view.showError(R.string.no_categories_found) + } + }, + Timber::e + ) + ) + } + + private fun searchResults(term: String) = + repository.searchAll(term, getImageTitleList()) + .subscribeOn(ioScheduler) + .map { it.filterNot { categoryItem -> repository.containsYear(categoryItem.name) } } + + override fun onDetachView() { + view = DUMMY + compositeDisposable.clear() + } + + /** + * asks the repository to fetch categories for the query + * @param query + */ + override fun searchForCategories(query: String) { + searchTerms.onNext(query) + } + + /** + * Returns image title list from UploadItem + * @return + */ + private fun getImageTitleList(): List { + return repository.uploads + .map { it.uploadMediaDetails[0].captionText } + .filterNot { TextUtils.isEmpty(it) } + } + + /** + * Verifies the number of categories selected, prompts the user if none selected + */ + override fun verifyCategories() { + val selectedCategories = repository.selectedCategories + if (selectedCategories.isNotEmpty()) { + repository.setSelectedCategories(selectedCategories.map { it.name }) + view.goToNextScreen() + } else { + view.showNoCategorySelected() + } + } + + /** + * ask repository to handle category clicked + * + * @param categoryItem + */ + override fun onCategoryItemClicked(categoryItem: CategoryItem) { + repository.onCategoryClicked(categoryItem) + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java index 435e829a8..844dc0267 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/categories/UploadCategoriesFragment.java @@ -1,6 +1,7 @@ package fr.free.nrw.commons.upload.categories; import android.os.Bundle; +import android.text.Editable; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -50,8 +51,6 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate CategoriesContract.UserActionListener presenter; private RVRendererAdapter adapter; private Disposable subscribe; - private List categories; - private boolean isVisible; @Override public void onCreate(@Nullable Bundle savedInstanceState) { @@ -78,15 +77,6 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate presenter.onAttachView(this); initRecyclerView(); addTextChangeListenerToEtSearch(); - //get default categories for empty query - } - - @Override - public void onResume() { - super.onResume(); - if (presenter != null && isVisible && (categories == null || categories.isEmpty())) { - presenter.searchForCategories(null); - } } private void addTextChangeListenerToEtSearch() { @@ -135,7 +125,6 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate public void setCategories(List categories) { adapter.clear(); if (categories != null) { - this.categories = categories; adapter.addAll(categories); adapter.notifyDataSetChanged(); } @@ -174,12 +163,11 @@ public class UploadCategoriesFragment extends UploadBaseFragment implements Cate } @Override - public void setUserVisibleHint(boolean isVisibleToUser) { - super.setUserVisibleHint(isVisibleToUser); - isVisible = isVisibleToUser; - - if (presenter != null && isResumed() && (categories == null || categories.isEmpty())) { - presenter.searchForCategories(null); + protected void onBecameVisible() { + super.onBecameVisible(); + final Editable text = etSearch.getText(); + if (text != null) { + presenter.searchForCategories(text.toString()); } } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt index 67c889e94..33c5371a4 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt @@ -2,14 +2,11 @@ package fr.free.nrw.commons.category import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever -import fr.free.nrw.commons.kvstore.JsonKvStore import fr.free.nrw.commons.upload.GpsCategoryModel import io.reactivex.Observable -import junit.framework.Assert.assertEquals +import io.reactivex.subjects.BehaviorSubject import org.junit.Before import org.junit.Test -import org.mockito.ArgumentMatchers.anyString -import org.mockito.ArgumentMatchers.eq import org.mockito.Mock import org.mockito.MockitoAnnotations @@ -31,48 +28,41 @@ class CategoriesModelTest { // Test Case for verifying that Categories search (MW api calls) are case-insensitive @Test fun searchAllFoundCaseTest() { - val categoriesModel = CategoriesModel(categoryClient, null, null, mock()) + val categoriesModel = CategoriesModel(categoryClient, mock(), mock()) - whenever(categoryClient.searchCategoriesForPrefix(anyString(), eq(25))) - .thenReturn(Observable.just("Test")) + val expectedList = listOf("Test") + whenever(categoryClient.searchCategoriesForPrefix("tes", 25)) + .thenReturn(Observable.just(expectedList)) // Checking if both return "Test" - val actualCategoryName = categoriesModel.searchAll("tes", null).blockingFirst() - assertEquals("Test", actualCategoryName.name) + val expectedItems = expectedList.map { CategoryItem(it, false) } + categoriesModel.searchAll("tes", emptyList()) + .test() + .assertValues(expectedItems) - val actualCategoryNameCaps = categoriesModel.searchAll("Tes", null).blockingFirst() - assertEquals("Test", actualCategoryNameCaps.name) + categoriesModel.searchAll("Tes", emptyList()) + .test() + .assertValues(expectedItems) } - /** - * For testing the substring search algorithm for Categories search - * To be more precise it tests the In Between substring( ex: searching `atte` - * will give search suggestions: `Latte`, `Iced latte` e.t.c) which has been described - * on github repo wiki: - * https://github.com/commons-app/apps-android-commons/wiki/Category-suggestions-(readme)#user-content-3-category-search-when-typing-in-the-search-field-has-been-made-more-flexible - */ @Test - fun searchAllFoundCaseTestForSubstringSearch() { + fun `searchAll with empty search terms creates results from gps, title search & recents`() { val gpsCategoryModel: GpsCategoryModel = mock() - val kvStore: JsonKvStore = mock() - whenever(gpsCategoryModel.categoryList).thenReturn(listOf("gpsCategory")) + whenever(gpsCategoryModel.categoriesFromLocation) + .thenReturn(BehaviorSubject.createDefault(listOf("gpsCategory"))) whenever(categoryClient.searchCategories("tes", 25)) - .thenReturn(Observable.just("tes")) - whenever(kvStore.getString("Category", "")).thenReturn("Random Value") + .thenReturn(Observable.just(listOf("titleSearch"))) whenever(categoryDao.recentCategories(25)).thenReturn(listOf("recentCategories")) - CategoriesModel( - categoryClient, - categoryDao, - kvStore, - gpsCategoryModel - ).searchAll(null, listOf("tes")) + CategoriesModel(categoryClient, categoryDao, gpsCategoryModel) + .searchAll("", listOf("tes")) .test() - .assertValues( - CategoryItem("gpsCategory", false), - CategoryItem("tes", false), - CategoryItem("Random Value", false), - CategoryItem("recentCategories", false) + .assertValue( + listOf( + CategoryItem("gpsCategory", false), + CategoryItem("titleSearch", false), + CategoryItem("recentCategories", false) + ) ) } } 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 a668e5704..8870910af 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 @@ -1,20 +1,26 @@ package fr.free.nrw.commons.category +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever import io.reactivex.Observable -import junit.framework.Assert.* import org.junit.Before import org.junit.Test -import org.mockito.* +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyString +import org.mockito.InjectMocks +import org.mockito.Mock +import org.mockito.Mockito.mock +import org.mockito.MockitoAnnotations 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 + internal lateinit var categoryInterface: CategoryInterface @InjectMocks - var categoryClient: CategoryClient? = null + lateinit var categoryClient: CategoryClient @Before @Throws(Exception::class) @@ -24,132 +30,111 @@ class CategoryClientTest { @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(), ArgumentMatchers.anyInt())) - .thenReturn(Observable.just(mockResponse)) - - val actualCategoryName = categoryClient!!.searchCategories("tes", 10).blockingFirst() - assertEquals("Test", actualCategoryName) - - val actualCategoryName2 = categoryClient!!.searchCategories("tes", 10, 10).blockingFirst() - assertEquals("Test", actualCategoryName2) + val mockResponse = withMockResponse("Category:Test") + whenever(categoryInterface.searchCategories(anyString(), anyInt(), anyInt())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.searchCategories("tes", 10) + .test() + .assertValues(listOf("Test")) + categoryClient.searchCategories("tes", 10, 10) + .test() + .assertValues(listOf("Test")) } @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(), ArgumentMatchers.anyInt())) - .thenReturn(Observable.just(mockResponse)) - - categoryClient!!.searchCategories("tes", 10).subscribe( - { fail("SearchCategories returned element when it shouldn't have.") }, - { s -> throw s }) - categoryClient!!.searchCategories("tes", 10, 10).subscribe( - { fail("SearchCategories returned element when it shouldn't have.") }, - { s -> throw s }) + val mockResponse = withNullPages() + whenever(categoryInterface.searchCategories(anyString(), anyInt(), anyInt())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.searchCategories("tes", 10) + .test() + .assertValues(emptyList()) + categoryClient.searchCategories("tes", 10, 10) + .test() + .assertValues(emptyList()) } @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() - assertEquals("Test", actualCategoryName) - val actualCategoryName2 = categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).blockingFirst() - assertEquals("Test", actualCategoryName2) + val mockResponse = withMockResponse("Category:Test") + whenever(categoryInterface.searchCategoriesForPrefix(anyString(), anyInt(), anyInt())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.searchCategoriesForPrefix("tes", 10) + .test() + .assertValues(listOf("Test")) + categoryClient.searchCategoriesForPrefix("tes", 10, 10) + .test() + .assertValues(listOf("Test")) } @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( - { fail("SearchCategories returned element when it shouldn't have.") }, - { s -> throw s }) - categoryClient!!.searchCategoriesForPrefix("tes", 10, 10).subscribe( - { fail("SearchCategories returned element when it shouldn't have.") }, - { s -> throw s }) + val mockResponse = withNullPages() + whenever(categoryInterface.searchCategoriesForPrefix(anyString(), anyInt(), anyInt())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.searchCategoriesForPrefix("tes", 10) + .test() + .assertValues(emptyList()) + categoryClient.searchCategoriesForPrefix("tes", 10, 10) + .test() + .assertValues(emptyList()) } @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) + val mockResponse = withMockResponse("Category:Test") + whenever(categoryInterface.getParentCategoryList(anyString())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.getParentCategoryList("tes") + .test() + .assertValues(listOf("Test")) } @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 }) + val mockResponse = withNullPages() + whenever(categoryInterface.getParentCategoryList(anyString())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.getParentCategoryList("tes") + .test() + .assertValues(emptyList()) } + @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) + val mockResponse = withMockResponse("Category:Test") + whenever(categoryInterface.getSubCategoryList("tes")) + .thenReturn(Observable.just(mockResponse)) + categoryClient.getSubCategoryList("tes") + .test() + .assertValues(listOf("Test")) } @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 }) + val mockResponse = withNullPages() + whenever(categoryInterface.getSubCategoryList(anyString())) + .thenReturn(Observable.just(mockResponse)) + categoryClient.getSubCategoryList("tes") + .test() + .assertValues(emptyList()) } -} \ No newline at end of file + + private fun withMockResponse(title: String): MwQueryResponse? { + val mwQueryPage: MwQueryPage = mock() + whenever(mwQueryPage.title()).thenReturn(title) + val mwQueryResult: MwQueryResult = mock() + whenever(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage)) + val mockResponse = mock(MwQueryResponse::class.java) + whenever(mockResponse.query()).thenReturn(mwQueryResult) + return mockResponse + } + + private fun withNullPages(): MwQueryResponse? { + val mwQueryResult = mock(MwQueryResult::class.java) + whenever(mwQueryResult.pages()).thenReturn(null) + val mockResponse = mock(MwQueryResponse::class.java) + whenever(mockResponse.query()).thenReturn(mwQueryResult) + return mockResponse + } +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt index b81173466..bf53c4e12 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/CategoriesPresenterTest.kt @@ -1,7 +1,7 @@ package fr.free.nrw.commons.upload -import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.whenever +import com.nhaarman.mockitokotlin2.* +import fr.free.nrw.commons.R import fr.free.nrw.commons.category.CategoryItem import fr.free.nrw.commons.repository.UploadRepository import fr.free.nrw.commons.upload.categories.CategoriesContract @@ -10,7 +10,6 @@ import io.reactivex.Observable import io.reactivex.schedulers.TestScheduler import org.junit.Before import org.junit.Test -import org.mockito.ArgumentMatchers import org.mockito.Mock import org.mockito.MockitoAnnotations @@ -20,6 +19,7 @@ import org.mockito.MockitoAnnotations class CategoriesPresenterTest { @Mock internal lateinit var repository: UploadRepository + @Mock internal lateinit var view: CategoriesContract.View @@ -49,30 +49,76 @@ class CategoriesPresenterTest { * unit test case for method CategoriesPresenter.searchForCategories */ @Test - fun searchForCategoriesTest() { - whenever(repository.sortBySimilarity(ArgumentMatchers.anyString())).thenReturn(Comparator { _, _ -> 1 }) - whenever(repository.selectedCategories).thenReturn(categoryItems) - whenever(repository.searchAll(ArgumentMatchers.anyString(), ArgumentMatchers.anyList())).thenReturn(Observable.empty()) + fun `searchForCategories combines selection and search results without years distinctly`() { + val nonEmptyCaptionUploadItem = mock() + whenever(nonEmptyCaptionUploadItem.uploadMediaDetails) + .thenReturn(listOf(UploadMediaDetail(captionText = "nonEmpty"))) + val emptyCaptionUploadItem = mock() + whenever(emptyCaptionUploadItem.uploadMediaDetails) + .thenReturn(listOf(UploadMediaDetail(captionText = ""))) + whenever(repository.uploads).thenReturn( + listOf( + nonEmptyCaptionUploadItem, + emptyCaptionUploadItem + ) + ) + whenever(repository.searchAll("test", listOf("nonEmpty"))) + .thenReturn( + Observable.just( + listOf( + categoryItem("selected"), + categoryItem("doesContainYear") + ) + ) + ) + whenever(repository.containsYear("selected")).thenReturn(false) + whenever(repository.containsYear("doesContainYear")).thenReturn(true) + whenever(repository.selectedCategories).thenReturn(listOf(categoryItem("selected", true))) categoriesPresenter.searchForCategories("test") + testScheduler.triggerActions() verify(view).showProgress(true) verify(view).showError(null) verify(view).setCategories(null) - testScheduler.triggerActions() - verify(view).setCategories(categoryItems) + verify(view).setCategories(listOf(categoryItem("selected", true))) verify(view).showProgress(false) + verifyNoMoreInteractions(view) + } + + @Test + fun `searchForCategoriesTest sets Error when list is empty`() { + whenever(repository.uploads).thenReturn(listOf()) + whenever(repository.searchAll(any(), any())).thenReturn(Observable.just(listOf())) + whenever(repository.selectedCategories).thenReturn(listOf()) + categoriesPresenter.searchForCategories("test") + testScheduler.triggerActions() + verify(view).showProgress(true) + verify(view).showError(null) + verify(view).setCategories(null) + verify(view).setCategories(listOf()) + verify(view).showProgress(false) + verify(view).showError(R.string.no_categories_found) + verifyNoMoreInteractions(view) } /** * unit test for method CategoriesPresenter.verifyCategories */ @Test - fun verifyCategoriesTest() { - whenever(repository.selectedCategories).thenReturn(categoryItems) + fun `verifyCategories with non empty selection goes to next screen`() { + val item = categoryItem() + whenever(repository.selectedCategories).thenReturn(listOf(item)) categoriesPresenter.verifyCategories() - verify(repository).setSelectedCategories(ArgumentMatchers.anyList()) + verify(repository).setSelectedCategories(listOf(item.name)) verify(view).goToNextScreen() } + @Test + fun `verifyCategories with empty selection show no category selected`() { + whenever(repository.selectedCategories).thenReturn(listOf()) + categoriesPresenter.verifyCategories() + verify(view).showNoCategorySelected() + } + /** * Test onCategory Item clicked */ @@ -81,4 +127,7 @@ class CategoriesPresenterTest { categoriesPresenter.onCategoryItemClicked(categoryItem) verify(repository).onCategoryClicked(categoryItem) } + + private fun categoryItem(name: String = "name", selected: Boolean = false) = + CategoryItem(name, selected) } 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 index 2377698b3..59b6645bd 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/GpsCategoryModelTest.kt @@ -1,76 +1,33 @@ 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 + lateinit var gpsCategoryModel: GpsCategoryModel @Before fun setUp() { - testObject = GpsCategoryModel() + gpsCategoryModel = GpsCategoryModel() } @Test - fun initiallyTheModelIsEmpty() { - assertFalse(testObject.gpsCatExists) - assertTrue(testObject.categoryList.isEmpty()) + fun `intial value is empty`() { + gpsCategoryModel.categoriesFromLocation.test().assertValues(emptyList()) } @Test - fun addingCategoriesToTheModel() { - testObject.categoryList = listOf("one") - assertTrue(testObject.gpsCatExists) - assertFalse(testObject.categoryList.isEmpty()) - assertEquals(listOf("one"), testObject.categoryList) + fun `setCategoriesFromLocation emits the new value`() { + val expectedList = listOf("category") + gpsCategoryModel.categoriesFromLocation.test() + .also { gpsCategoryModel.setCategoriesFromLocation(expectedList) } + .assertValues(emptyList(), expectedList) } @Test - fun duplicatesAreIgnored() { - testObject.categoryList = listOf("one", "one") - assertEquals(listOf("one"), testObject.categoryList) - } - - @Test - fun modelProtectsAgainstExternalModification() { - testObject.categoryList = listOf("one") - - val list = testObject.categoryList - list.add("two") - - assertEquals(listOf("one"), testObject.categoryList) - } - - @Test - fun clearingTheModel() { - testObject.categoryList = listOf("one") - - testObject.clear() - assertFalse(testObject.gpsCatExists) - assertTrue(testObject.categoryList.isEmpty()) - - testObject.categoryList = listOf("two") - assertEquals(listOf("two"), testObject.categoryList) - } - - @Test - fun settingTheListHandlesNull() { - testObject.categoryList = listOf("one") - - testObject.categoryList = null - - assertFalse(testObject.gpsCatExists) - assertTrue(testObject.categoryList.isEmpty()) - } - - @Test - fun settingTheListOverwritesExistingValues() { - testObject.categoryList = listOf("one") - - testObject.categoryList = listOf("two") - - assertEquals(listOf("two"), testObject.categoryList) + fun `clear emits an empty value`() { + gpsCategoryModel.categoriesFromLocation.test() + .also { gpsCategoryModel.clear() } + .assertValues(emptyList(), emptyList()) } }