From a2772b4891955aa3d3ee865d2fa8b5eb3b255fcb Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 17:56:16 +0100 Subject: [PATCH 01/11] Prevent multiline in Search for Categories --- app/src/main/res/layout/fragment_categorization.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/res/layout/fragment_categorization.xml b/app/src/main/res/layout/fragment_categorization.xml index 83a8a746a..4cdb9604c 100644 --- a/app/src/main/res/layout/fragment_categorization.xml +++ b/app/src/main/res/layout/fragment_categorization.xml @@ -24,6 +24,7 @@ android:layout_width="match_parent" android:hint="@string/categories_search_text_hint" android:maxLines="1" + android:inputType="text" android:imeOptions="flagNoExtractUi" /> From 7b879e459cd396a8c50a8d3f575df32e7eeff2fe Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 17:56:33 +0100 Subject: [PATCH 02/11] Update RvAdapter dependency --- app/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index 99cd9c8cf..a4c72b4e4 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -17,7 +17,7 @@ dependencies { compile "com.android.support:design:${project.supportLibVersion}" compile 'com.google.code.gson:gson:2.8.0' compile "com.jakewharton:butterknife:$BUTTERKNIFE_VERSION" - compile 'com.github.pedrovgs:renderers:3.3.0' + compile 'com.github.pedrovgs:renderers:3.3.3' annotationProcessor "com.jakewharton:butterknife-compiler:$BUTTERKNIFE_VERSION" compile 'com.jakewharton.timber:timber:4.5.1' compile 'com.squareup.okhttp3:okhttp:3.8.1' From 4c0ea9567081eef684d9922e03d582157c707239 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 17:56:51 +0100 Subject: [PATCH 03/11] Add dependencies for RxBindings --- app/build.gradle | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/build.gradle b/app/build.gradle index a4c72b4e4..51d6f56b0 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -30,7 +30,10 @@ dependencies { // Because RxAndroid releases are few and far between, it is recommended you also // explicitly depend on RxJava's latest version for bug fixes and new features. compile 'io.reactivex.rxjava2:rxjava:2.1.2' - + compile 'com.jakewharton.rxbinding2:rxbinding:2.0.0' + compile 'com.jakewharton.rxbinding2:rxbinding-support-v4:2.0.0' + compile 'com.jakewharton.rxbinding2:rxbinding-appcompat-v7:2.0.0' + compile 'com.jakewharton.rxbinding2:rxbinding-design:2.0.0' compile 'com.facebook.fresco:fresco:1.3.0' compile 'com.facebook.stetho:stetho:1.5.0' From 494739da5e53f31c20b037222f19035046f13510 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 17:57:29 +0100 Subject: [PATCH 04/11] Implement equals and hashCode for CategoryItem --- .../free/nrw/commons/category/CategoryItem.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 index 1063979ad..94a920427 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java @@ -51,4 +51,20 @@ class CategoryItem implements Parcelable { 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(); + } } From 0c0c81460417ca911ff0e9c2104761c0a3d7713e Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 17:58:31 +0100 Subject: [PATCH 05/11] Refactor CategorizationFragment to use RxJava2 --- .../category/CategorizationFragment.java | 493 ++++++++---------- .../category/CategoryCountUpdater.java | 59 --- .../nrw/commons/category/MethodAUpdater.java | 100 ---- .../category/OnCategoriesSaveHandler.java | 4 +- .../nrw/commons/category/PrefixUpdater.java | 119 ----- .../nrw/commons/category/TitleCategories.java | 48 -- .../commons/upload/MultipleShareActivity.java | 3 +- .../nrw/commons/upload/ShareActivity.java | 2 +- 8 files changed, 231 insertions(+), 597 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/category/CategoryCountUpdater.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/category/MethodAUpdater.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/category/PrefixUpdater.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/category/TitleCategories.java diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index bda7eecc9..19b0cd822 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -3,7 +3,6 @@ package fr.free.nrw.commons.category; import android.content.ContentProviderClient; import android.content.SharedPreferences; import android.database.Cursor; -import android.os.AsyncTask; import android.os.Bundle; import android.os.RemoteException; import android.preference.PreferenceManager; @@ -11,9 +10,7 @@ import android.support.v4.app.Fragment; import android.support.v7.app.AlertDialog; import android.support.v7.widget.LinearLayoutManager; import android.support.v7.widget.RecyclerView; -import android.text.Editable; import android.text.TextUtils; -import android.text.TextWatcher; import android.view.LayoutInflater; import android.view.Menu; import android.view.MenuInflater; @@ -24,24 +21,28 @@ import android.widget.EditText; import android.widget.ProgressBar; import android.widget.TextView; -import com.pedrogomez.renderers.ListAdapteeCollection; +import com.jakewharton.rxbinding2.view.RxView; +import com.jakewharton.rxbinding2.widget.RxTextView; import com.pedrogomez.renderers.RVRendererAdapter; +import java.io.IOException; import java.util.ArrayList; +import java.util.Calendar; +import java.util.Date; import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import butterknife.BindView; import butterknife.ButterKnife; +import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.R; -import fr.free.nrw.commons.category.CategoriesRenderer.CategoryClickedListener; +import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.upload.MwVolleyApi; +import io.reactivex.Observable; +import io.reactivex.Single; +import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.schedulers.Schedulers; import timber.log.Timber; import static android.view.KeyEvent.ACTION_UP; @@ -51,7 +52,8 @@ import static fr.free.nrw.commons.category.CategoryContentProvider.AUTHORITY; /** * Displays the category suggestion and selection screen. Category search is initiated here. */ -public class CategorizationFragment extends Fragment implements CategoryClickedListener { +public class CategorizationFragment extends Fragment { + public static final int SEARCH_CATS_LIMIT = 25; @BindView(R.id.categoriesListBox) @@ -68,19 +70,54 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL private RVRendererAdapter categoriesAdapter; private OnCategoriesSaveHandler onCategoriesSaveHandler; private HashMap> categoriesCache; - private ArrayList selectedCategories = new ArrayList<>(); + private List selectedCategories = new ArrayList<>(); private ContentProviderClient client; - private PrefixUpdater prefixUpdaterSub; - private MethodAUpdater methodAUpdaterSub; - private final CategoryTextWatcher textWatcher = new CategoryTextWatcher(); - private final CategoriesAdapterFactory adapterFactory = new CategoriesAdapterFactory(this); - private final ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(2); - private final ArrayList titleCatItems = new ArrayList<>(); - private final CountDownLatch mergeLatch = new CountDownLatch(1); - // LHS guarantees ordered insertions, allowing for prioritized method A results - private final Set results = new LinkedHashSet<>(); + private final CategoriesAdapterFactory adapterFactory = new CategoriesAdapterFactory(item -> { + if (item.isSelected()) { + selectedCategories.add(item); + updateCategoryCount(item, client); + } else { + selectedCategories.remove(item); + } + }); + + private void updateCategoryCount(CategoryItem item, ContentProviderClient client) { + Category cat = lookupCategory(item.getName()); + cat.incTimesUsed(); + + cat.setContentProviderClient(client); + cat.save(); + } + + private Category lookupCategory(String name) { + Cursor cursor = null; + try { + cursor = client.query( + CategoryContentProvider.BASE_URI, + Category.Table.ALL_FIELDS, + Category.Table.COLUMN_NAME + "=?", + new String[]{name}, + null); + if (cursor != null && cursor.moveToFirst()) { + return Category.fromCursor(cursor); + } + } catch (RemoteException e) { + // This feels lazy, but to hell with checked exceptions. :) + throw new RuntimeException(e); + } finally { + if (cursor != null) { + cursor.close(); + } + } + + // Newly used category... + Category cat = new Category(); + cat.setName(name); + cat.setLastUsed(new Date()); + cat.setTimesUsed(0); + return cat; + } - @SuppressWarnings("unchecked") @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { @@ -89,30 +126,76 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL categoriesList.setLayoutManager(new LinearLayoutManager(getContext())); - categoriesSkip.setOnClickListener(view -> { - getActivity().onBackPressed(); - getActivity().finish(); - }); + RxView.clicks(categoriesSkip) + .takeUntil(RxView.detaches(categoriesSkip)) + .subscribe(o -> { + getActivity().onBackPressed(); + getActivity().finish(); + }); - ArrayList items; - if (savedInstanceState == null) { - items = new ArrayList<>(); - categoriesCache = new HashMap<>(); - } else { - items = savedInstanceState.getParcelableArrayList("currentCategories"); - categoriesCache = (HashMap>) savedInstanceState - .getSerializable("categoriesCache"); + ArrayList items = new ArrayList<>(); + categoriesCache = new HashMap<>(); + if (savedInstanceState != null) { + items.addAll(savedInstanceState.getParcelableArrayList("currentCategories")); + categoriesCache.putAll((HashMap>) savedInstanceState + .getSerializable("categoriesCache")); } categoriesAdapter = adapterFactory.create(items); categoriesList.setAdapter(categoriesAdapter); - categoriesFilter.addTextChangedListener(textWatcher); - - startUpdatingCategoryList(); + RxTextView.textChanges(categoriesFilter) + .takeUntil(RxView.detaches(categoriesFilter)) + .debounce(300, TimeUnit.MILLISECONDS) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(filter -> updateCategoryList(filter.toString())); return rootView; } + private void updateCategoryList(String filter) { + Observable.fromIterable(selectedCategories) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .doOnSubscribe(disposable -> { + categoriesSearchInProgress.setVisibility(View.VISIBLE); + categoriesNotFoundView.setVisibility(View.GONE); + categoriesSkip.setVisibility(View.GONE); + categoriesAdapter.clear(); + }) + .observeOn(Schedulers.io()) + .concatWith( + search(filter) + .mergeWith(search2(filter)) + .filter(categoryItem -> !selectedCategories.contains(categoryItem)) + .switchIfEmpty( + gpsCategories() + .concatWith(titleCategories()) + .concatWith(recentCategories()) + .filter(categoryItem -> !selectedCategories.contains(categoryItem)) + ) + ) + .filter(categoryItem -> !containsYear(categoryItem.getName())) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe( + s -> categoriesAdapter.add(s), + throwable -> Timber.e(throwable), + () -> { + categoriesAdapter.notifyDataSetChanged(); + categoriesSearchInProgress.setVisibility(View.GONE); + + if (categoriesAdapter.getItemCount() == 0) { + if (TextUtils.isEmpty(filter)) { + // If we found no recent cats, show the skip message! + categoriesSkip.setVisibility(View.VISIBLE); + } else { + categoriesNotFoundView.setText(getString(R.string.categories_not_found, filter)); + categoriesNotFoundView.setVisibility(View.VISIBLE); + } + } + } + ); + } + @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { menu.clear(); @@ -137,12 +220,6 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL } } - @Override - public void onDestroyView() { - categoriesFilter.removeTextChangedListener(textWatcher); - super.onDestroyView(); - } - @Override public void onDestroy() { super.onDestroy(); @@ -165,21 +242,8 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL public boolean onOptionsItemSelected(MenuItem menuItem) { switch (menuItem.getItemId()) { case R.id.menu_save_categories: - - int numberSelected = 0; - - selectedCategories = new ArrayList<>(); - int count = categoriesAdapter.getItemCount(); - for (int i = 0; i < count; i++) { - CategoryItem item = categoriesAdapter.getItem(i); - if (item.isSelected()) { - selectedCategories.add(item.getName()); - numberSelected++; - } - } - //If no categories selected, display warning to user - if (numberSelected == 0) { + if (selectedCategories.size() == 0) { new AlertDialog.Builder(getActivity()) .setMessage("Images without categories are rarely usable. " + "Are you sure you want to submit without selecting " @@ -190,17 +254,18 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL }) .setNegativeButton("Yes, submit", (dialog, id) -> { //Proceed to submission - onCategoriesSaveHandler.onCategoriesSave(selectedCategories); + onCategoriesSaveHandler.onCategoriesSave(getStringList(selectedCategories)); }) .create() .show(); } else { //Proceed to submission - onCategoriesSaveHandler.onCategoriesSave(selectedCategories); - return true; + onCategoriesSaveHandler.onCategoriesSave(getStringList(selectedCategories)); } + return true; + default: + return super.onOptionsItemSelected(menuItem); } - return super.onOptionsItemSelected(menuItem); } @Override @@ -212,8 +277,12 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL client = getActivity().getContentResolver().acquireContentProviderClient(AUTHORITY); } - public HashMap> getCategoriesCache() { - return categoriesCache; + private List getStringList(List input) { + List output = new ArrayList<>(); + for (CategoryItem item : input) { + output.add(item.getName()); + } + return output; } /** @@ -221,36 +290,15 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL * * @return a list containing title-related categories */ - private ArrayList titleCatQuery() { - TitleCategories titleCategoriesSub; - - //Retrieve the title that was saved when user tapped submit icon - SharedPreferences titleDesc = PreferenceManager.getDefaultSharedPreferences(getActivity()); - String title = titleDesc.getString("Title", ""); - Timber.d("Title: %s", title); - - //Override onPostExecute to access the results of async API call - titleCategoriesSub = new TitleCategories(title) { - @Override - protected void onPostExecute(List result) { - super.onPostExecute(result); - Timber.d("Results in onPostExecute: %s", result); - titleCatItems.addAll(result); - Timber.d("TitleCatItems in onPostExecute: %s", titleCatItems); - mergeLatch.countDown(); - } - }; - - titleCategoriesSub.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); - Timber.d("TitleCatItems in titleCatQuery: %s", titleCatItems); - - //Only return titleCatItems after API call has finished + private List titleCatQuery(String title) { + MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); + //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= try { - mergeLatch.await(5L, TimeUnit.SECONDS); - } catch (InterruptedException e) { - Timber.e(e, "Interrupted exception: "); + return api.searchTitles(SEARCH_CATS_LIMIT, title); + } catch (IOException e) { + Timber.e(e, "IO Exception: "); + return new ArrayList<>(); } - return titleCatItems; } /** @@ -284,171 +332,105 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL return items; } - /** - * Merges nearby categories, categories suggested based on title, and recent categories... - * without duplicates. - * - * @return a list containing merged categories - */ - ArrayList mergeItems() { - Set mergedItems = new LinkedHashSet<>(); - - Timber.d("Calling APIs for GPS cats, title cats and recent cats..."); - - List gpsItems = new ArrayList<>(); - if (MwVolleyApi.GpsCatExists.getGpsCatExists()) { - gpsItems.addAll(MwVolleyApi.getGpsCat()); - } - List titleItems = new ArrayList<>(titleCatQuery()); - List recentItems = new ArrayList<>(recentCatQuery()); - - //Await results of titleItems, which is likely to come in last - try { - mergeLatch.await(5L, TimeUnit.SECONDS); - Timber.d("Waited for merge"); - } catch (InterruptedException e) { - Timber.e(e, "Interrupted Exception: "); - } - - mergedItems.addAll(gpsItems); - Timber.d("Adding GPS items: %s", gpsItems); - mergedItems.addAll(titleItems); - Timber.d("Adding title items: %s", titleItems); - mergedItems.addAll(recentItems); - Timber.d("Adding recent items: %s", recentItems); - - // Needs to be an ArrayList and not a List unless we want to modify a big portion - // of preexisting code - ArrayList mergedItemsList = new ArrayList<>(mergedItems); - - Timber.d("Merged item list: %s", mergedItemsList); - return mergedItemsList; + private Observable gpsCategories() { + return Observable.fromIterable( + MwVolleyApi.GpsCatExists.getGpsCatExists() ? + MwVolleyApi.getGpsCat() : new ArrayList<>()) + .map(s -> new CategoryItem(s, false)); } - /** - * Displays categories found to the user as they type in the search box - * - * @param categories a list of all categories found for the search string - * @param filter the search string - */ - private void setCatsAfterAsync(ArrayList categories, String filter) { - if (getActivity() != null) { - ArrayList items = new ArrayList<>(); - HashSet existingKeys = new HashSet<>(); - int count = categoriesAdapter.getItemCount(); - for (int i = 0; i < count; i++) { - CategoryItem item = categoriesAdapter.getItem(i); - if (item.isSelected()) { - items.add(item); - existingKeys.add(item.getName()); - } - } - for (String category : categories) { - if (!existingKeys.contains(category)) { - items.add(new CategoryItem(category, false)); - } - } + private Observable titleCategories() { + //Retrieve the title that was saved when user tapped submit icon + SharedPreferences titleDesc = PreferenceManager.getDefaultSharedPreferences(getActivity()); + String title = titleDesc.getString("Title", ""); - categoriesAdapter.setCollection(new ListAdapteeCollection<>(items)); - categoriesAdapter.notifyDataSetChanged(); - categoriesSearchInProgress.setVisibility(View.GONE); - - if (categories.isEmpty()) { - if (TextUtils.isEmpty(filter)) { - // If we found no recent cats, show the skip message! - categoriesSkip.setVisibility(View.VISIBLE); - } else { - categoriesNotFoundView.setText(getString(R.string.categories_not_found, filter)); - categoriesNotFoundView.setVisibility(View.VISIBLE); - } - } else { - categoriesList.smoothScrollToPosition(existingKeys.size()); - } - } else { - Timber.e("Error: Fragment is null"); - } + return Observable.just(title) + .observeOn(Schedulers.io()) + .flatMapIterable(s -> titleCatQuery(s)) + .map(s -> new CategoryItem(s, false)); } - - /** - * Makes asynchronous calls to the Commons MediaWiki API via anonymous subclasses of - * 'MethodAUpdater' and 'PrefixUpdater'. Some of their methods are overridden in order to - * aggregate the results. A CountDownLatch is used to ensure that MethodA results are shown - * above Prefix results. - */ - private void requestSearchResults() { - final CountDownLatch latch = new CountDownLatch(1); - - prefixUpdaterSub = new PrefixUpdater(this) { - @Override - protected List doInBackground(Void... voids) { - List result = new ArrayList<>(); - try { - result = super.doInBackground(); - latch.await(); - } catch (InterruptedException e) { - Timber.w(e); - //Thread.currentThread().interrupt(); - } - return result; - } - - @Override - protected void onPostExecute(List result) { - super.onPostExecute(result); - - results.addAll(result); - Timber.d("Prefix result: %s", result); - - String filter = categoriesFilter.getText().toString(); - ArrayList resultsList = new ArrayList<>(results); - categoriesCache.put(filter, resultsList); - Timber.d("Final results List: %s", resultsList); - - categoriesAdapter.notifyDataSetChanged(); - setCatsAfterAsync(resultsList, filter); - } - }; - - methodAUpdaterSub = new MethodAUpdater(this) { - @Override - protected void onPostExecute(List result) { - results.clear(); - super.onPostExecute(result); - - results.addAll(result); - Timber.d("Method A result: %s", result); - categoriesAdapter.notifyDataSetChanged(); - - latch.countDown(); - } - }; - prefixUpdaterSub.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); - methodAUpdaterSub.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); + private Observable recentCategories() { + return Observable.fromIterable(recentCatQuery()) + .map(s -> new CategoryItem(s, false)); } - private void startUpdatingCategoryList() { - if (prefixUpdaterSub != null) { - prefixUpdaterSub.cancel(true); - } + private Observable search(String term) { + return Single.just(term) + .map(s -> { + //If user hasn't typed anything in yet, get GPS and recent items + if (TextUtils.isEmpty(s)) { + return new ArrayList(); + } - if (methodAUpdaterSub != null) { - methodAUpdaterSub.cancel(true); - } + //if user types in something that is in cache, return cached category + if (categoriesCache.containsKey(s)) { + return categoriesCache.get(s); + } - requestSearchResults(); + //otherwise if user has typed something in that isn't in cache, search API for matching categories + //URL: https://commons.wikimedia.org/w/api.php?action=query&list=allcategories&acprefix=filter&aclimit=25 + MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); + List categories = new ArrayList<>(); + try { + categories = api.allCategories(SEARCH_CATS_LIMIT, s); + Timber.d("Prefix URL filter %s", categories); + } catch (IOException e) { + Timber.e(e, "IO Exception: "); + //Return empty arraylist + return categories; + } + + Timber.d("Found categories from Prefix search, waiting for filter"); + return categories; + }) + .flatMapObservable(Observable::fromIterable) + .map(s -> new CategoryItem(s, false)); + } + + private Observable search2(String term) { + return Single.just(term) + .map(s -> { + //If user hasn't typed anything in yet, get GPS and recent items + if (TextUtils.isEmpty(s)) { + return new ArrayList(); + } + + MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); + + //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= + try { + return api.searchCategories(SEARCH_CATS_LIMIT, term); + } catch (IOException e) { + Timber.e(e, "IO Exception: "); + return new ArrayList(); + } + }) + .flatMapObservable(Observable::fromIterable) + .map(s -> new CategoryItem(s, false)); + } + + private boolean containsYear(String items) { + + //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 s contains a 4-digit word anywhere within the string (.* is wildcard) + //And that s 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) + return ((items.matches(".*(19|20)\\d{2}.*") && !items.contains(yearInString) && !items.contains(prevYearInString)) + || items.matches("(.*)needing(.*)") || items.matches("(.*)taken on(.*)")); } public int getCurrentSelectedCount() { - int count = 0; - int numberOfItems = categoriesAdapter.getItemCount(); - for (int i = 0; i < numberOfItems; i++) { - CategoryItem item = categoriesAdapter.getItem(i); - if (item.isSelected()) { - count++; - } - } - return count; + return selectedCategories.size(); } public void backButtonDialog() { @@ -463,27 +445,4 @@ public class CategorizationFragment extends Fragment implements CategoryClickedL .create() .show(); } - - @Override - public void categoryClicked(CategoryItem item) { - if (item.isSelected()) { - new CategoryCountUpdater(item.getName(), client).executeOnExecutor(executor); - } - } - - private class CategoryTextWatcher implements TextWatcher { - @Override - public void beforeTextChanged(CharSequence charSequence, int i, int i2, int i3) { - } - - @Override - public void onTextChanged(CharSequence charSequence, int i, int i2, int i3) { - startUpdatingCategoryList(); - } - - @Override - public void afterTextChanged(Editable editable) { - - } - } } diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryCountUpdater.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryCountUpdater.java deleted file mode 100644 index bebbc03a8..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryCountUpdater.java +++ /dev/null @@ -1,59 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.content.ContentProviderClient; -import android.database.Cursor; -import android.os.AsyncTask; -import android.os.RemoteException; - -import java.util.Date; - -class CategoryCountUpdater extends AsyncTask { - - private final String name; - private final ContentProviderClient client; - - CategoryCountUpdater(String name, ContentProviderClient client) { - this.name = name; - this.client = client; - } - - @Override - protected Void doInBackground(Void... voids) { - Category cat = lookupCategory(name); - cat.incTimesUsed(); - - cat.setContentProviderClient(client); - cat.save(); - - return null; // Make the compiler happy. - } - - private Category lookupCategory(String name) { - Cursor cursor = null; - try { - cursor = client.query( - CategoryContentProvider.BASE_URI, - Category.Table.ALL_FIELDS, - Category.Table.COLUMN_NAME + "=?", - new String[]{name}, - null); - if (cursor != null && cursor.moveToFirst()) { - return Category.fromCursor(cursor); - } - } catch (RemoteException e) { - // This feels lazy, but to hell with checked exceptions. :) - throw new RuntimeException(e); - } finally { - if (cursor != null) { - cursor.close(); - } - } - - // Newly used category... - Category cat = new Category(); - cat.setName(name); - cat.setLastUsed(new Date()); - cat.setTimesUsed(0); - return cat; - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/category/MethodAUpdater.java b/app/src/main/java/fr/free/nrw/commons/category/MethodAUpdater.java deleted file mode 100644 index 72b1f5732..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/MethodAUpdater.java +++ /dev/null @@ -1,100 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.os.AsyncTask; -import android.view.View; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.Iterator; -import java.util.List; - -import fr.free.nrw.commons.CommonsApplication; -import fr.free.nrw.commons.mwapi.MediaWikiApi; -import timber.log.Timber; - -import static fr.free.nrw.commons.category.CategorizationFragment.SEARCH_CATS_LIMIT; - -/** - * Sends asynchronous queries to the Commons MediaWiki API to retrieve categories that are close to - * the keyword typed in by the user. The 'srsearch' action-specific parameter is used for this - * purpose. This class should be subclassed in CategorizationFragment.java to aggregate the results. - */ -class MethodAUpdater extends AsyncTask> { - - private final CategorizationFragment catFragment; - private String filter; - - MethodAUpdater(CategorizationFragment catFragment) { - this.catFragment = catFragment; - } - - @Override - protected void onPreExecute() { - super.onPreExecute(); - filter = catFragment.categoriesFilter.getText().toString(); - catFragment.categoriesSearchInProgress.setVisibility(View.VISIBLE); - catFragment.categoriesNotFoundView.setVisibility(View.GONE); - - catFragment.categoriesSkip.setVisibility(View.GONE); - } - - /** - * Remove categories that contain a year in them (starting with 19__ or 20__), except for this year - * and previous year - * Rationale: https://github.com/commons-app/apps-android-commons/issues/47 - * - * @param items Unfiltered list of categories - * @return Filtered category list - */ - private List filterYears(List items) { - - Iterator iterator; - - //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); - Timber.d("Year: %s", yearInString); - - int prevYear = year - 1; - String prevYearInString = String.valueOf(prevYear); - Timber.d("Previous year: %s", prevYearInString); - - //Copy to Iterator to prevent ConcurrentModificationException when removing item - for (iterator = items.iterator(); iterator.hasNext(); ) { - String s = iterator.next(); - - //Check if s contains a 4-digit word anywhere within the string (.* is wildcard) - //And that s does not equal the current year or previous year - if (s.matches(".*(19|20)\\d{2}.*") && !s.contains(yearInString) && !s.contains(prevYearInString)) { - Timber.d("Filtering out year %s", s); - iterator.remove(); - } - } - - Timber.d("Items: %s", items); - return items; - } - - @Override - protected List doInBackground(Void... voids) { - - //otherwise if user has typed something in that isn't in cache, search API for matching categories - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - List categories = new ArrayList<>(); - - //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= - try { - categories = api.searchCategories(SEARCH_CATS_LIMIT, filter); - Timber.d("Method A URL filter %s", categories); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - //Return empty arraylist - return categories; - } - - Timber.d("Found categories from Method A search, waiting for filter"); - return new ArrayList<>(filterYears(categories)); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/category/OnCategoriesSaveHandler.java b/app/src/main/java/fr/free/nrw/commons/category/OnCategoriesSaveHandler.java index a7ae0bfed..5899d5905 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/OnCategoriesSaveHandler.java +++ b/app/src/main/java/fr/free/nrw/commons/category/OnCategoriesSaveHandler.java @@ -1,7 +1,7 @@ package fr.free.nrw.commons.category; -import java.util.ArrayList; +import java.util.List; public interface OnCategoriesSaveHandler { - void onCategoriesSave(ArrayList categories); + void onCategoriesSave(List categories); } diff --git a/app/src/main/java/fr/free/nrw/commons/category/PrefixUpdater.java b/app/src/main/java/fr/free/nrw/commons/category/PrefixUpdater.java deleted file mode 100644 index 7df56eff5..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/PrefixUpdater.java +++ /dev/null @@ -1,119 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.os.AsyncTask; -import android.text.TextUtils; -import android.view.View; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; - -import fr.free.nrw.commons.CommonsApplication; -import fr.free.nrw.commons.mwapi.MediaWikiApi; -import timber.log.Timber; - -import static fr.free.nrw.commons.category.CategorizationFragment.SEARCH_CATS_LIMIT; - -/** - * Sends asynchronous queries to the Commons MediaWiki API to retrieve categories that share the - * same prefix as the keyword typed in by the user. The 'acprefix' action-specific parameter is used - * for this purpose. This class should be subclassed in CategorizationFragment.java to aggregate - * the results. - */ -class PrefixUpdater extends AsyncTask> { - - private final CategorizationFragment catFragment; - private String filter; - - PrefixUpdater(CategorizationFragment catFragment) { - this.catFragment = catFragment; - } - - @Override - protected void onPreExecute() { - super.onPreExecute(); - filter = catFragment.categoriesFilter.getText().toString(); - catFragment.categoriesSearchInProgress.setVisibility(View.VISIBLE); - catFragment.categoriesNotFoundView.setVisibility(View.GONE); - - catFragment.categoriesSkip.setVisibility(View.GONE); - } - - /** - * Remove categories that contain a year in them (starting with 19__ or 20__), except for this year - * and previous year - * Rationale: https://github.com/commons-app/apps-android-commons/issues/47 - * - * @param items Unfiltered list of categories - * @return Filtered category list - */ - private List filterIrrelevantResults(List items) { - - Iterator iterator; - - //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); - Timber.d("Year: %s", yearInString); - - int prevYear = year - 1; - String prevYearInString = String.valueOf(prevYear); - Timber.d("Previous year: %s", prevYearInString); - - //Copy to Iterator to prevent ConcurrentModificationException when removing item - for (iterator = items.iterator(); iterator.hasNext();) { - String s = iterator.next(); - - //Check if s contains a 4-digit word anywhere within the string (.* is wildcard) - //And that s 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) - if ((s.matches(".*(19|20)\\d{2}.*") && !s.contains(yearInString) && !s.contains(prevYearInString)) - || s.matches("(.*)needing(.*)")||s.matches("(.*)taken on(.*)")) { - Timber.d("Filtering out irrelevant result: %s", s); - iterator.remove(); - } - - } - - Timber.d("Items: %s", items); - return items; - } - - @Override - protected List doInBackground(Void... voids) { - //If user hasn't typed anything in yet, get GPS and recent items - if (TextUtils.isEmpty(filter)) { - ArrayList mergedItems = new ArrayList<>(catFragment.mergeItems()); - Timber.d("Merged items, waiting for filter"); - return new ArrayList<>(filterIrrelevantResults(mergedItems)); - } - - //if user types in something that is in cache, return cached category - HashMap> categoriesCache = catFragment.getCategoriesCache(); - if (categoriesCache.containsKey(filter)) { - ArrayList cachedItems = new ArrayList<>(categoriesCache.get(filter)); - Timber.d("Found cache items, waiting for filter"); - return new ArrayList<>(filterIrrelevantResults(cachedItems)); - } - - //otherwise if user has typed something in that isn't in cache, search API for matching categories - //URL: https://commons.wikimedia.org/w/api.php?action=query&list=allcategories&acprefix=filter&aclimit=25 - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - List categories = new ArrayList<>(); - try { - categories = api.allCategories(SEARCH_CATS_LIMIT, this.filter); - Timber.d("Prefix URL filter %s", categories); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - //Return empty arraylist - return categories; - } - - Timber.d("Found categories from Prefix search, waiting for filter"); - return new ArrayList<>(filterIrrelevantResults(categories)); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/category/TitleCategories.java b/app/src/main/java/fr/free/nrw/commons/category/TitleCategories.java deleted file mode 100644 index a4a94cf1d..000000000 --- a/app/src/main/java/fr/free/nrw/commons/category/TitleCategories.java +++ /dev/null @@ -1,48 +0,0 @@ -package fr.free.nrw.commons.category; - -import android.os.AsyncTask; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -import fr.free.nrw.commons.CommonsApplication; -import fr.free.nrw.commons.mwapi.MediaWikiApi; -import timber.log.Timber; - -/** - * Sends asynchronous queries to the Commons MediaWiki API to retrieve categories that are related to - * the title entered in previous screen. The 'srsearch' action-specific parameter is used for this - * purpose. This class should be subclassed in CategorizationFragment.java to add the results to recent and GPS cats. - */ -class TitleCategories extends AsyncTask> { - - private final static int SEARCH_CATS_LIMIT = 25; - - private final String title; - - TitleCategories(String title) { - this.title = title; - } - - @Override - protected List doInBackground(Void... voids) { - - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - List titleCategories = new ArrayList<>(); - - //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= - try { - titleCategories = api.searchTitles(SEARCH_CATS_LIMIT, this.title); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - //Return empty arraylist - return titleCategories; - } - - Timber.d("Title cat query results: %s", titleCategories); - - return titleCategories; - } - -} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java index 1ab00c88f..680264ab3 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/MultipleShareActivity.java @@ -22,6 +22,7 @@ import android.widget.AdapterView; import android.widget.Toast; import java.util.ArrayList; +import java.util.List; import butterknife.ButterKnife; import fr.free.nrw.commons.CommonsApplication; @@ -160,7 +161,7 @@ public class MultipleShareActivity } @Override - public void onCategoriesSave(ArrayList categories) { + public void onCategoriesSave(List categories) { if(categories.size() > 0) { ContentProviderClient client = getContentResolver().acquireContentProviderClient(ModificationsContentProvider.AUTHORITY); for(Contribution contribution: photosList) { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index 1ce86a86a..ecaa71aa5 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -154,7 +154,7 @@ public class ShareActivity } @Override - public void onCategoriesSave(ArrayList categories) { + public void onCategoriesSave(List categories) { if(categories.size() > 0) { ModifierSequence categoriesSequence = new ModifierSequence(contribution.getContentUri()); From 84a5f0a221cd8ab81e49c0044caffe6a35350e28 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 19:27:47 +0100 Subject: [PATCH 06/11] Move DB tasks to model/dao object --- .../category/CategorizationFragment.java | 95 +++---------------- .../category/CategoryContentProvider.java | 1 + .../commons/{category => data}/Category.java | 68 +++++++++++-- .../free/nrw/commons/data/DBOpenHelper.java | 1 - 4 files changed, 77 insertions(+), 88 deletions(-) rename app/src/main/java/fr/free/nrw/commons/{category => data}/Category.java (64%) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 19b0cd822..39248c062 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -2,9 +2,7 @@ package fr.free.nrw.commons.category; import android.content.ContentProviderClient; import android.content.SharedPreferences; -import android.database.Cursor; import android.os.Bundle; -import android.os.RemoteException; import android.preference.PreferenceManager; import android.support.v4.app.Fragment; import android.support.v7.app.AlertDialog; @@ -37,6 +35,7 @@ import butterknife.BindView; import butterknife.ButterKnife; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.R; +import fr.free.nrw.commons.data.Category; import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.upload.MwVolleyApi; import io.reactivex.Observable; @@ -71,11 +70,11 @@ public class CategorizationFragment extends Fragment { private OnCategoriesSaveHandler onCategoriesSaveHandler; private HashMap> categoriesCache; private List selectedCategories = new ArrayList<>(); - private ContentProviderClient client; + private ContentProviderClient databaseClient; private final CategoriesAdapterFactory adapterFactory = new CategoriesAdapterFactory(item -> { if (item.isSelected()) { selectedCategories.add(item); - updateCategoryCount(item, client); + updateCategoryCount(item, databaseClient); } else { selectedCategories.remove(item); } @@ -84,37 +83,20 @@ public class CategorizationFragment extends Fragment { private void updateCategoryCount(CategoryItem item, ContentProviderClient client) { Category cat = lookupCategory(item.getName()); cat.incTimesUsed(); - - cat.setContentProviderClient(client); - cat.save(); + cat.save(client); } private Category lookupCategory(String name) { - Cursor cursor = null; - try { - cursor = client.query( - CategoryContentProvider.BASE_URI, - Category.Table.ALL_FIELDS, - Category.Table.COLUMN_NAME + "=?", - new String[]{name}, - null); - if (cursor != null && cursor.moveToFirst()) { - return Category.fromCursor(cursor); - } - } catch (RemoteException e) { - // This feels lazy, but to hell with checked exceptions. :) - throw new RuntimeException(e); - } finally { - if (cursor != null) { - cursor.close(); - } + Category cat = Category.find(databaseClient, name); + + if (cat == null) { + // Newly used category... + cat = new Category(); + cat.setName(name); + cat.setLastUsed(new Date()); + cat.setTimesUsed(0); } - // Newly used category... - Category cat = new Category(); - cat.setName(name); - cat.setLastUsed(new Date()); - cat.setTimesUsed(0); return cat; } @@ -223,7 +205,7 @@ public class CategorizationFragment extends Fragment { @Override public void onDestroy() { super.onDestroy(); - client.release(); + databaseClient.release(); } @Override @@ -274,7 +256,7 @@ public class CategorizationFragment extends Fragment { setHasOptionsMenu(true); onCategoriesSaveHandler = (OnCategoriesSaveHandler) getActivity(); getActivity().setTitle(R.string.categories_activity_title); - client = getActivity().getContentResolver().acquireContentProviderClient(AUTHORITY); + databaseClient = getActivity().getContentResolver().acquireContentProviderClient(AUTHORITY); } private List getStringList(List input) { @@ -285,53 +267,6 @@ public class CategorizationFragment extends Fragment { return output; } - /** - * Retrieves category suggestions from title input - * - * @return a list containing title-related categories - */ - private List titleCatQuery(String title) { - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= - try { - return api.searchTitles(SEARCH_CATS_LIMIT, title); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - return new ArrayList<>(); - } - } - - /** - * Retrieves recently-used categories - * - * @return a list containing recent categories - */ - private ArrayList recentCatQuery() { - ArrayList items = new ArrayList<>(); - Cursor cursor = null; - try { - cursor = client.query( - CategoryContentProvider.BASE_URI, - Category.Table.ALL_FIELDS, - null, - new String[]{}, - Category.Table.COLUMN_LAST_USED + " DESC"); - // fixme add a limit on the original query instead of falling out of the loop? - while (cursor != null && cursor.moveToNext() - && cursor.getPosition() < SEARCH_CATS_LIMIT) { - Category cat = Category.fromCursor(cursor); - items.add(cat.getName()); - } - } catch (RemoteException e) { - throw new RuntimeException(e); - } finally { - if (cursor != null) { - cursor.close(); - } - } - return items; - } - private Observable gpsCategories() { return Observable.fromIterable( MwVolleyApi.GpsCatExists.getGpsCatExists() ? @@ -351,7 +286,7 @@ public class CategorizationFragment extends Fragment { } private Observable recentCategories() { - return Observable.fromIterable(recentCatQuery()) + return Observable.fromIterable(Category.recentCategories(databaseClient, SEARCH_CATS_LIMIT)) .map(s -> new CategoryItem(s, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryContentProvider.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryContentProvider.java index c95e17cf6..e75f1adcf 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryContentProvider.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryContentProvider.java @@ -11,6 +11,7 @@ import android.support.annotation.NonNull; import android.text.TextUtils; import fr.free.nrw.commons.CommonsApplication; +import fr.free.nrw.commons.data.Category; import fr.free.nrw.commons.data.DBOpenHelper; import timber.log.Timber; diff --git a/app/src/main/java/fr/free/nrw/commons/category/Category.java b/app/src/main/java/fr/free/nrw/commons/data/Category.java similarity index 64% rename from app/src/main/java/fr/free/nrw/commons/category/Category.java rename to app/src/main/java/fr/free/nrw/commons/data/Category.java index 68dd9200e..a5c5b6d75 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/Category.java +++ b/app/src/main/java/fr/free/nrw/commons/data/Category.java @@ -1,4 +1,4 @@ -package fr.free.nrw.commons.category; +package fr.free.nrw.commons.data; import android.content.ContentProviderClient; import android.content.ContentValues; @@ -6,11 +6,15 @@ import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.net.Uri; import android.os.RemoteException; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import java.util.ArrayList; import java.util.Date; +import fr.free.nrw.commons.category.CategoryContentProvider; + public class Category { - private ContentProviderClient client; private Uri contentUri; private String name; @@ -54,11 +58,7 @@ public class Category { } // Database/content-provider stuff - public void setContentProviderClient(ContentProviderClient client) { - this.client = client; - } - - public void save() { + public void save(ContentProviderClient client) { try { if (contentUri == null) { contentUri = client.insert(CategoryContentProvider.BASE_URI, this.toContentValues()); @@ -88,6 +88,60 @@ public class Category { return c; } + public static @Nullable Category find(ContentProviderClient client, String name) { + Cursor cursor = null; + try { + cursor = client.query( + CategoryContentProvider.BASE_URI, + Category.Table.ALL_FIELDS, + Category.Table.COLUMN_NAME + "=?", + new String[]{name}, + null); + if (cursor != null && cursor.moveToFirst()) { + return Category.fromCursor(cursor); + } + } catch (RemoteException e) { + // This feels lazy, but to hell with checked exceptions. :) + throw new RuntimeException(e); + } finally { + if (cursor != null) { + cursor.close(); + } + } + return null; + } + + /** + * Retrieves recently-used categories + * + * @return a list containing recent categories + */ + public static @NonNull ArrayList recentCategories(ContentProviderClient client, int limit) { + ArrayList items = new ArrayList<>(); + Cursor cursor = null; + try { + cursor = client.query( + CategoryContentProvider.BASE_URI, + Category.Table.ALL_FIELDS, + null, + new String[]{}, + Category.Table.COLUMN_LAST_USED + " DESC"); + // fixme add a limit on the original query instead of falling out of the loop? + while (cursor != null && cursor.moveToNext() + && cursor.getPosition() < limit) { + Category cat = Category.fromCursor(cursor); + items.add(cat.getName()); + } + } catch (RemoteException e) { + throw new RuntimeException(e); + } finally { + if (cursor != null) { + cursor.close(); + } + } + return items; + } + public static class Table { public static final String TABLE_NAME = "categories"; diff --git a/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.java b/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.java index 22171857a..8ef9f336c 100644 --- a/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.java +++ b/app/src/main/java/fr/free/nrw/commons/data/DBOpenHelper.java @@ -4,7 +4,6 @@ import android.content.Context; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteOpenHelper; -import fr.free.nrw.commons.category.Category; import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.modifications.ModifierSequence; From b7215c580f66f3ff9f56f2dd546004376573dc7f Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 19:58:58 +0100 Subject: [PATCH 07/11] Migrate network tasks to RxJava --- .../category/CategorizationFragment.java | 122 ++++++---------- .../mwapi/ApacheHttpClientMediaWikiApi.java | 137 +++++++++++------- .../free/nrw/commons/mwapi/MediaWikiApi.java | 7 +- 3 files changed, 133 insertions(+), 133 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 39248c062..4ecb27a8b 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -23,7 +23,6 @@ import com.jakewharton.rxbinding2.view.RxView; import com.jakewharton.rxbinding2.widget.RxTextView; import com.pedrogomez.renderers.RVRendererAdapter; -import java.io.IOException; import java.util.ArrayList; import java.util.Calendar; import java.util.Date; @@ -36,10 +35,8 @@ import butterknife.ButterKnife; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.R; import fr.free.nrw.commons.data.Category; -import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.upload.MwVolleyApi; import io.reactivex.Observable; -import io.reactivex.Single; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -194,7 +191,7 @@ public class CategorizationFragment extends Fragment { rootView.requestFocus(); rootView.setOnKeyListener((v, keyCode, event) -> { if (event.getAction() == ACTION_UP && keyCode == KEYCODE_BACK) { - backButtonDialog(); + showBackButtonDialog(); return true; } return false; @@ -224,25 +221,12 @@ public class CategorizationFragment extends Fragment { public boolean onOptionsItemSelected(MenuItem menuItem) { switch (menuItem.getItemId()) { case R.id.menu_save_categories: - //If no categories selected, display warning to user - if (selectedCategories.size() == 0) { - new AlertDialog.Builder(getActivity()) - .setMessage("Images without categories are rarely usable. " - + "Are you sure you want to submit without selecting " - + "categories?") - .setTitle("No Categories Selected") - .setPositiveButton("No, go back", (dialog, id) -> { - //Exit menuItem so user can select their categories - }) - .setNegativeButton("Yes, submit", (dialog, id) -> { - //Proceed to submission - onCategoriesSaveHandler.onCategoriesSave(getStringList(selectedCategories)); - }) - .create() - .show(); - } else { - //Proceed to submission + if (selectedCategories.size() > 0) { + //Some categories selected, proceed to submission onCategoriesSaveHandler.onCategoriesSave(getStringList(selectedCategories)); + } else { + //No categories selected, prompt the user to select some + showConfirmationDialog(); } return true; default: @@ -271,7 +255,7 @@ public class CategorizationFragment extends Fragment { return Observable.fromIterable( MwVolleyApi.GpsCatExists.getGpsCatExists() ? MwVolleyApi.getGpsCat() : new ArrayList<>()) - .map(s -> new CategoryItem(s, false)); + .map(name -> new CategoryItem(name, false)); } private Observable titleCategories() { @@ -279,10 +263,9 @@ public class CategorizationFragment extends Fragment { SharedPreferences titleDesc = PreferenceManager.getDefaultSharedPreferences(getActivity()); String title = titleDesc.getString("Title", ""); - return Observable.just(title) - .observeOn(Schedulers.io()) - .flatMapIterable(s -> titleCatQuery(s)) - .map(s -> new CategoryItem(s, false)); + return CommonsApplication.getInstance().getMWApi() + .searchTitles(title, SEARCH_CATS_LIMIT) + .map(name -> new CategoryItem(name, false)); } private Observable recentCategories() { @@ -291,62 +274,35 @@ public class CategorizationFragment extends Fragment { } private Observable search(String term) { - return Single.just(term) - .map(s -> { - //If user hasn't typed anything in yet, get GPS and recent items - if (TextUtils.isEmpty(s)) { - return new ArrayList(); - } + //If user hasn't typed anything in yet, get GPS and recent items + if (TextUtils.isEmpty(term)) { + return Observable.empty(); + } - //if user types in something that is in cache, return cached category - if (categoriesCache.containsKey(s)) { - return categoriesCache.get(s); - } + //if user types in something that is in cache, return cached category + if (categoriesCache.containsKey(term)) { + return Observable.fromIterable(categoriesCache.get(term)) + .map(name -> new CategoryItem(name, false)); + } - //otherwise if user has typed something in that isn't in cache, search API for matching categories - //URL: https://commons.wikimedia.org/w/api.php?action=query&list=allcategories&acprefix=filter&aclimit=25 - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - List categories = new ArrayList<>(); - try { - categories = api.allCategories(SEARCH_CATS_LIMIT, s); - Timber.d("Prefix URL filter %s", categories); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - //Return empty arraylist - return categories; - } - - Timber.d("Found categories from Prefix search, waiting for filter"); - return categories; - }) - .flatMapObservable(Observable::fromIterable) - .map(s -> new CategoryItem(s, false)); + //otherwise if user has typed something in that isn't in cache, search API for matching categories + return CommonsApplication.getInstance().getMWApi() + .allCategories(term, SEARCH_CATS_LIMIT) + .map(name -> new CategoryItem(name, false)); } private Observable search2(String term) { - return Single.just(term) - .map(s -> { - //If user hasn't typed anything in yet, get GPS and recent items - if (TextUtils.isEmpty(s)) { - return new ArrayList(); - } + //If user hasn't typed anything in yet, get GPS and recent items + if (TextUtils.isEmpty(term)) { + return Observable.empty(); + } - MediaWikiApi api = CommonsApplication.getInstance().getMWApi(); - - //URL https://commons.wikimedia.org/w/api.php?action=query&format=xml&list=search&srwhat=text&srenablerewrites=1&srnamespace=14&srlimit=10&srsearch= - try { - return api.searchCategories(SEARCH_CATS_LIMIT, term); - } catch (IOException e) { - Timber.e(e, "IO Exception: "); - return new ArrayList(); - } - }) - .flatMapObservable(Observable::fromIterable) + return CommonsApplication.getInstance().getMWApi() + .searchCategories(term, SEARCH_CATS_LIMIT) .map(s -> new CategoryItem(s, false)); } private boolean containsYear(String items) { - //Check for current and previous year to exclude these categories from removal Calendar now = Calendar.getInstance(); int year = now.get(Calendar.YEAR); @@ -356,7 +312,6 @@ public class CategorizationFragment extends Fragment { String prevYearInString = String.valueOf(prevYear); Timber.d("Previous year: %s", prevYearInString); - //Check if s contains a 4-digit word anywhere within the string (.* is wildcard) //And that s 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) @@ -368,7 +323,7 @@ public class CategorizationFragment extends Fragment { return selectedCategories.size(); } - public void backButtonDialog() { + public void showBackButtonDialog() { new AlertDialog.Builder(getActivity()) .setMessage("Are you sure you want to go back? The image will not " + "have any categories saved.") @@ -380,4 +335,21 @@ public class CategorizationFragment extends Fragment { .create() .show(); } + + public void showConfirmationDialog() { + new AlertDialog.Builder(getActivity()) + .setMessage("Images without categories are rarely usable. " + + "Are you sure you want to submit without selecting " + + "categories?") + .setTitle("No Categories Selected") + .setPositiveButton("No, go back", (dialog, id) -> { + //Exit menuItem so user can select their categories + }) + .setNegativeButton("Yes, submit", (dialog, id) -> { + //Proceed to submission + onCategoriesSaveHandler.onCategoriesSave(getStringList(selectedCategories)); + }) + .create() + .show(); + } } 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 60fefe036..7af42b0fe 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 @@ -35,6 +35,7 @@ import fr.free.nrw.commons.BuildConfig; import fr.free.nrw.commons.PageTitle; import fr.free.nrw.commons.Utils; import in.yuvi.http.fluent.Http; +import io.reactivex.Observable; import io.reactivex.Single; import timber.log.Timber; @@ -205,78 +206,104 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi { @Override @NonNull - public List searchCategories(int searchCatsLimit, String filterValue) throws IOException { - List 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"); + public Observable searchCategories(String filterValue, int searchCatsLimit) { + 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("Failed to obtain searchCategories", e); + } - if (categoryNodes == null) { - return Collections.emptyList(); - } + if (categoryNodes == null) { + return new ArrayList(); + } - List categories = new ArrayList<>(); - for (ApiResult categoryNode : categoryNodes) { - String cat = categoryNode.getDocument().getTextContent(); - String catString = cat.replace("Category:", ""); - categories.add(catString); - } + List categories = new ArrayList<>(); + for (ApiResult categoryNode : categoryNodes) { + String cat = categoryNode.getDocument().getTextContent(); + String catString = cat.replace("Category:", ""); + categories.add(catString); + } - return categories; + return categories; + }) + .flatMapObservable(list -> Observable.fromIterable(list)); } @Override @NonNull - public List allCategories(int searchCatsLimit, String filterValue) throws IOException { - ArrayList categoryNodes = api.action("query") - .param("list", "allcategories") - .param("acprefix", filterValue) - .param("aclimit", searchCatsLimit) - .get() - .getNodes("/api/query/allcategories/c"); + 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("Failed to obtain allCategories", e); + } - if (categoryNodes == null) { - return Collections.emptyList(); - } + if (categoryNodes == null) { + return new ArrayList(); + } - List categories = new ArrayList<>(); - for (ApiResult categoryNode : categoryNodes) { - categories.add(categoryNode.getDocument().getTextContent()); - } + List categories = new ArrayList<>(); + for (ApiResult categoryNode : categoryNodes) { + categories.add(categoryNode.getDocument().getTextContent()); + } - return categories; + return categories; + }) + .flatMapObservable(list -> Observable.fromIterable(list)); } @Override @NonNull - public List searchTitles(int searchCatsLimit, String title) throws IOException { - ArrayList 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"); + public Observable searchTitles(String title, int searchCatsLimit) { + return Single.fromCallable(() -> { + ArrayList categoryNodes = null; - if (categoryNodes == null) { - return Collections.emptyList(); - } + 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("Failed to obtain searchTitles", e); + return new ArrayList(); + } - List titleCategories = new ArrayList<>(); - for (ApiResult categoryNode : categoryNodes) { - String cat = categoryNode.getDocument().getTextContent(); - String catString = cat.replace("Category:", ""); - titleCategories.add(catString); - } + if (categoryNodes == null) { + return Collections.emptyList(); + } - return titleCategories; + List titleCategories = new ArrayList<>(); + for (ApiResult categoryNode : categoryNodes) { + String cat = categoryNode.getDocument().getTextContent(); + String catString = cat.replace("Category:", ""); + titleCategories.add(catString); + } + + return titleCategories; + }) + .flatMapObservable(list -> Observable.fromIterable(list)); } @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 c50dd56d2..292c0a5de 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 @@ -7,6 +7,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.List; +import io.reactivex.Observable; import io.reactivex.Single; public interface MediaWikiApi { @@ -38,13 +39,13 @@ public interface MediaWikiApi { MediaResult fetchMediaByFilename(String filename) throws IOException; @NonNull - List searchCategories(int searchCatsLimit, String filterValue) throws IOException; + Observable searchCategories(String filterValue, int searchCatsLimit); @NonNull - List allCategories(int searchCatsLimit, String filter) throws IOException; + Observable allCategories(String filter, int searchCatsLimit); @NonNull - List searchTitles(int searchCatsLimit, String title) throws IOException; + Observable searchTitles(String title, int searchCatsLimit); @Nullable String revisionsByFilename(String filename) throws IOException; From 22cd1ac72a86ff437d2468117fe1bb279fc37559 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 20:09:45 +0100 Subject: [PATCH 08/11] Fix import --- app/src/main/java/fr/free/nrw/commons/CommonsApplication.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java index 94d6e3fb4..ccc85bec1 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -26,8 +26,8 @@ import java.io.IOException; import fr.free.nrw.commons.auth.AccountUtil; import fr.free.nrw.commons.caching.CacheController; -import fr.free.nrw.commons.category.Category; import fr.free.nrw.commons.contributions.Contribution; +import fr.free.nrw.commons.data.Category; import fr.free.nrw.commons.data.DBOpenHelper; import fr.free.nrw.commons.modifications.ModifierSequence; import fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi; From 0443f65c4a53ab2ec8399d3b95a04e0c2180f7c0 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 20:11:27 +0100 Subject: [PATCH 09/11] Reorder to clarify --- .../category/CategorizationFragment.java | 135 +++++++++--------- .../nrw/commons/upload/ShareActivity.java | 2 +- 2 files changed, 69 insertions(+), 68 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 4ecb27a8b..5521ee629 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -68,6 +68,7 @@ public class CategorizationFragment extends Fragment { private HashMap> categoriesCache; private List selectedCategories = new ArrayList<>(); private ContentProviderClient databaseClient; + private final CategoriesAdapterFactory adapterFactory = new CategoriesAdapterFactory(item -> { if (item.isSelected()) { selectedCategories.add(item); @@ -77,26 +78,6 @@ public class CategorizationFragment extends Fragment { } }); - private void updateCategoryCount(CategoryItem item, ContentProviderClient client) { - Category cat = lookupCategory(item.getName()); - cat.incTimesUsed(); - cat.save(client); - } - - private Category lookupCategory(String name) { - Category cat = Category.find(databaseClient, name); - - if (cat == null) { - // Newly used category... - cat = new Category(); - cat.setName(name); - cat.setLastUsed(new Date()); - cat.setTimesUsed(0); - } - - return cat; - } - @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { @@ -131,50 +112,6 @@ public class CategorizationFragment extends Fragment { return rootView; } - private void updateCategoryList(String filter) { - Observable.fromIterable(selectedCategories) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .doOnSubscribe(disposable -> { - categoriesSearchInProgress.setVisibility(View.VISIBLE); - categoriesNotFoundView.setVisibility(View.GONE); - categoriesSkip.setVisibility(View.GONE); - categoriesAdapter.clear(); - }) - .observeOn(Schedulers.io()) - .concatWith( - search(filter) - .mergeWith(search2(filter)) - .filter(categoryItem -> !selectedCategories.contains(categoryItem)) - .switchIfEmpty( - gpsCategories() - .concatWith(titleCategories()) - .concatWith(recentCategories()) - .filter(categoryItem -> !selectedCategories.contains(categoryItem)) - ) - ) - .filter(categoryItem -> !containsYear(categoryItem.getName())) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe( - s -> categoriesAdapter.add(s), - throwable -> Timber.e(throwable), - () -> { - categoriesAdapter.notifyDataSetChanged(); - categoriesSearchInProgress.setVisibility(View.GONE); - - if (categoriesAdapter.getItemCount() == 0) { - if (TextUtils.isEmpty(filter)) { - // If we found no recent cats, show the skip message! - categoriesSkip.setVisibility(View.VISIBLE); - } else { - categoriesNotFoundView.setText(getString(R.string.categories_not_found, filter)); - categoriesNotFoundView.setVisibility(View.VISIBLE); - } - } - } - ); - } - @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { menu.clear(); @@ -243,6 +180,50 @@ public class CategorizationFragment extends Fragment { databaseClient = getActivity().getContentResolver().acquireContentProviderClient(AUTHORITY); } + private void updateCategoryList(String filter) { + Observable.fromIterable(selectedCategories) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .doOnSubscribe(disposable -> { + categoriesSearchInProgress.setVisibility(View.VISIBLE); + categoriesNotFoundView.setVisibility(View.GONE); + categoriesSkip.setVisibility(View.GONE); + categoriesAdapter.clear(); + }) + .observeOn(Schedulers.io()) + .concatWith( + searchAll(filter) + .mergeWith(searchCategories(filter)) + .filter(categoryItem -> !selectedCategories.contains(categoryItem)) + .switchIfEmpty( + gpsCategories() + .concatWith(titleCategories()) + .concatWith(recentCategories()) + .filter(categoryItem -> !selectedCategories.contains(categoryItem)) + ) + ) + .filter(categoryItem -> !containsYear(categoryItem.getName())) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe( + s -> categoriesAdapter.add(s), + throwable -> Timber.e(throwable), + () -> { + categoriesAdapter.notifyDataSetChanged(); + categoriesSearchInProgress.setVisibility(View.GONE); + + if (categoriesAdapter.getItemCount() == 0) { + if (TextUtils.isEmpty(filter)) { + // If we found no recent cats, show the skip message! + categoriesSkip.setVisibility(View.VISIBLE); + } else { + categoriesNotFoundView.setText(getString(R.string.categories_not_found, filter)); + categoriesNotFoundView.setVisibility(View.VISIBLE); + } + } + } + ); + } + private List getStringList(List input) { List output = new ArrayList<>(); for (CategoryItem item : input) { @@ -273,7 +254,7 @@ public class CategorizationFragment extends Fragment { .map(s -> new CategoryItem(s, false)); } - private Observable search(String term) { + private Observable searchAll(String term) { //If user hasn't typed anything in yet, get GPS and recent items if (TextUtils.isEmpty(term)) { return Observable.empty(); @@ -291,7 +272,7 @@ public class CategorizationFragment extends Fragment { .map(name -> new CategoryItem(name, false)); } - private Observable search2(String term) { + private Observable searchCategories(String term) { //If user hasn't typed anything in yet, get GPS and recent items if (TextUtils.isEmpty(term)) { return Observable.empty(); @@ -319,6 +300,26 @@ public class CategorizationFragment extends Fragment { || items.matches("(.*)needing(.*)") || items.matches("(.*)taken on(.*)")); } + private void updateCategoryCount(CategoryItem item, ContentProviderClient client) { + Category cat = lookupCategory(item.getName()); + cat.incTimesUsed(); + cat.save(client); + } + + private Category lookupCategory(String name) { + Category cat = Category.find(databaseClient, name); + + if (cat == null) { + // Newly used category... + cat = new Category(); + cat.setName(name); + cat.setLastUsed(new Date()); + cat.setTimesUsed(0); + } + + return cat; + } + public int getCurrentSelectedCount() { return selectedCategories.size(); } @@ -336,7 +337,7 @@ public class CategorizationFragment extends Fragment { .show(); } - public void showConfirmationDialog() { + private void showConfirmationDialog() { new AlertDialog.Builder(getActivity()) .setMessage("Images without categories are rarely usable. " + "Are you sure you want to submit without selecting " diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index ecaa71aa5..42ec3cfe6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -525,7 +525,7 @@ public class ShareActivity switch (item.getItemId()) { case android.R.id.home: if(categorizationFragment!=null && categorizationFragment.isVisible()) { - categorizationFragment.backButtonDialog(); + categorizationFragment.showBackButtonDialog(); } else { onBackPressed(); } From ef1fc0c9d95dfd5ccee59806a853008169d324f5 Mon Sep 17 00:00:00 2001 From: Mikel Date: Sun, 6 Aug 2017 20:33:29 +0100 Subject: [PATCH 10/11] Improve code readability --- .../category/CategorizationFragment.java | 19 +++++++++++-------- .../nrw/commons/category/CategoryItem.java | 8 ++++++-- .../fr/free/nrw/commons/data/Category.java | 19 +++++++++++++++---- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 5521ee629..63532afb8 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -234,8 +234,8 @@ public class CategorizationFragment extends Fragment { private Observable gpsCategories() { return Observable.fromIterable( - MwVolleyApi.GpsCatExists.getGpsCatExists() ? - MwVolleyApi.getGpsCat() : new ArrayList<>()) + MwVolleyApi.GpsCatExists.getGpsCatExists() + ? MwVolleyApi.getGpsCat() : new ArrayList<>()) .map(name -> new CategoryItem(name, false)); } @@ -266,7 +266,7 @@ public class CategorizationFragment extends Fragment { .map(name -> new CategoryItem(name, false)); } - //otherwise if user has typed something in that isn't in cache, search API for matching categories + //otherwise, search API for matching categories return CommonsApplication.getInstance().getMWApi() .allCategories(term, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); @@ -283,7 +283,7 @@ public class CategorizationFragment extends Fragment { .map(s -> new CategoryItem(s, false)); } - private boolean containsYear(String items) { + private 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); @@ -293,11 +293,11 @@ public class CategorizationFragment extends Fragment { String prevYearInString = String.valueOf(prevYear); Timber.d("Previous year: %s", prevYearInString); - //Check if s contains a 4-digit word anywhere within the string (.* is wildcard) - //And that s does not equal the current year or previous year + //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) - return ((items.matches(".*(19|20)\\d{2}.*") && !items.contains(yearInString) && !items.contains(prevYearInString)) - || items.matches("(.*)needing(.*)") || items.matches("(.*)taken on(.*)")); + return ((item.matches(".*(19|20)\\d{2}.*") && !item.contains(yearInString) && !item.contains(prevYearInString)) + || item.matches("(.*)needing(.*)") || item.matches("(.*)taken on(.*)")); } private void updateCategoryCount(CategoryItem item, ContentProviderClient client) { @@ -324,6 +324,9 @@ public class CategorizationFragment extends Fragment { return selectedCategories.size(); } + /** + * Show dialog asking for confirmation to leave without saving categories. + */ public void showBackButtonDialog() { new AlertDialog.Builder(getActivity()) .setMessage("Are you sure you want to go back? The image will not " 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 index 94a920427..f6bacfb51 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java @@ -54,8 +54,12 @@ class CategoryItem implements Parcelable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } CategoryItem that = (CategoryItem) o; diff --git a/app/src/main/java/fr/free/nrw/commons/data/Category.java b/app/src/main/java/fr/free/nrw/commons/data/Category.java index a5c5b6d75..757f6b691 100644 --- a/app/src/main/java/fr/free/nrw/commons/data/Category.java +++ b/app/src/main/java/fr/free/nrw/commons/data/Category.java @@ -57,7 +57,12 @@ public class Category { touch(); } - // Database/content-provider stuff + //region Database/content-provider stuff + + /** + * Persist category. + * @param client ContentProviderClient to handle DB connection + */ public void save(ContentProviderClient client) { try { if (contentUri == null) { @@ -78,7 +83,7 @@ public class Category { return cv; } - public static Category fromCursor(Cursor cursor) { + private static Category fromCursor(Cursor cursor) { // Hardcoding column positions! Category c = new Category(); c.contentUri = CategoryContentProvider.uriForId(cursor.getInt(0)); @@ -88,6 +93,12 @@ public class Category { return c; } + /** + * Find persisted category in database, based on its name. + * @param client ContentProviderClient to handle DB connection + * @param name Category's name + * @return category from database, or null if not found + */ public static @Nullable Category find(ContentProviderClient client, String name) { Cursor cursor = null; try { @@ -112,8 +123,7 @@ public class Category { } /** - * Retrieves recently-used categories - * + * Retrieve recently-used categories, ordered by descending date. * @return a list containing recent categories */ public static @NonNull ArrayList recentCategories(ContentProviderClient client, int limit) { @@ -198,4 +208,5 @@ public class Category { } } } + //endregion } From 3793516d57ae31d9bf6095c60b6735b2e682c313 Mon Sep 17 00:00:00 2001 From: Mikel Date: Mon, 7 Aug 2017 16:37:12 +0100 Subject: [PATCH 11/11] Fix empty-filter search. Fix repeated suggestions issue. Show "no results found" when appropriate --- .../category/CategorizationFragment.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java index 63532afb8..b281a6c1b 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategorizationFragment.java @@ -194,15 +194,11 @@ public class CategorizationFragment extends Fragment { .concatWith( searchAll(filter) .mergeWith(searchCategories(filter)) - .filter(categoryItem -> !selectedCategories.contains(categoryItem)) - .switchIfEmpty( - gpsCategories() - .concatWith(titleCategories()) - .concatWith(recentCategories()) - .filter(categoryItem -> !selectedCategories.contains(categoryItem)) - ) + .concatWith( TextUtils.isEmpty(filter) + ? defaultCategories() : Observable.empty()) ) .filter(categoryItem -> !containsYear(categoryItem.getName())) + .distinct() .observeOn(AndroidSchedulers.mainThread()) .subscribe( s -> categoriesAdapter.add(s), @@ -211,11 +207,13 @@ public class CategorizationFragment extends Fragment { categoriesAdapter.notifyDataSetChanged(); categoriesSearchInProgress.setVisibility(View.GONE); - if (categoriesAdapter.getItemCount() == 0) { + if (categoriesAdapter.getItemCount() == selectedCategories.size()) { + // There are no suggestions if (TextUtils.isEmpty(filter)) { - // If we found no recent cats, show the skip message! + // Allow to send image with no categories categoriesSkip.setVisibility(View.VISIBLE); } else { + // Inform the user that the searched term matches no category categoriesNotFoundView.setText(getString(R.string.categories_not_found, filter)); categoriesNotFoundView.setVisibility(View.VISIBLE); } @@ -232,6 +230,12 @@ public class CategorizationFragment extends Fragment { return output; } + private Observable defaultCategories() { + return gpsCategories() + .concatWith(titleCategories()) + .concatWith(recentCategories()); + } + private Observable gpsCategories() { return Observable.fromIterable( MwVolleyApi.GpsCatExists.getGpsCatExists()