From 8474c04c64dc3e76fb3e32f9b5e0dbb84548a7e0 Mon Sep 17 00:00:00 2001 From: Dmitry Brant Date: Tue, 19 Mar 2019 17:08:04 -0400 Subject: [PATCH] Fix crash(es) caused by failing to dispose Rx observables (#2669) --- .../free/nrw/commons/CommonsApplication.java | 5 ++++ .../achievements/AchievementsActivity.java | 10 ++++++-- .../commons/auth/AuthenticatedActivity.java | 7 +++--- .../free/nrw/commons/auth/LoginActivity.java | 7 ++++-- .../pictures/BookmarkPicturesFragment.java | 12 ++++++++-- .../category/CategoryImagesListFragment.java | 16 +++++++++---- .../category/SubCategoryListFragment.java | 8 +++---- .../contributions/ContributionsFragment.java | 10 ++------ .../commons/contributions/MainActivity.java | 4 ++-- .../di/CommonsDaggerSupportFragment.java | 9 +++++++ .../nrw/commons/explore/SearchActivity.java | 4 ++-- .../categories/SearchCategoryFragment.java | 9 ++++--- .../explore/images/SearchImageFragment.java | 9 ++++--- .../commons/media/MediaDetailFragment.java | 4 ++-- .../nrw/commons/nearby/NearbyFragment.java | 21 +++++----------- .../free/nrw/commons/theme/BaseActivity.java | 9 ++++++- .../nrw/commons/upload/FileProcessor.java | 10 ++++++-- .../upload/ImageProcessingService.java | 7 ++++-- .../nrw/commons/upload/UploadActivity.java | 10 ++------ .../free/nrw/commons/upload/UploadModel.java | 24 +++++++++---------- .../nrw/commons/upload/UploadPresenter.java | 12 ++++++---- 21 files changed, 121 insertions(+), 86 deletions(-) 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 0bb6a4598..b69738ca1 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -44,6 +44,8 @@ import fr.free.nrw.commons.modifications.ModifierSequenceDao; import fr.free.nrw.commons.upload.FileUtils; import fr.free.nrw.commons.utils.ConfigUtils; import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.internal.functions.Functions; +import io.reactivex.plugins.RxJavaPlugins; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -128,6 +130,9 @@ public class CommonsApplication extends Application { createNotificationChannel(this); + // This handler will catch exceptions thrown from Observables after they are disposed, + // or from Observables that are (deliberately or not) missing an onError handler. + RxJavaPlugins.setErrorHandler(Functions.emptyConsumer()); if (setupLeakCanary() == RefWatcher.DISABLED) { return; diff --git a/app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java b/app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java index 26aa18640..4caefd1c9 100644 --- a/app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java @@ -142,6 +142,12 @@ public class AchievementsActivity extends NavigationBaseActivity { initDrawer(); } + @Override + public void onDestroy() { + super.onDestroy(); + compositeDisposable.clear(); + } + /** * To invoke the AlertDialog on clicking info button */ @@ -238,12 +244,12 @@ public class AchievementsActivity extends NavigationBaseActivity { if (StringUtils.isNullOrWhiteSpace(userName)) { return; } - okHttpJsonApiClient.getWikidataEdits(userName) + compositeDisposable.add(okHttpJsonApiClient.getWikidataEdits(userName) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(edits -> wikidataEditsText.setText(String.valueOf(edits)), e -> { Timber.e("Error:" + e); - }); + })); } private void showSnackBarWithRetry() { diff --git a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java index 1bfcb7727..3bb1ab9a1 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/AuthenticatedActivity.java @@ -58,13 +58,12 @@ public abstract class AuthenticatedActivity extends NavigationBaseActivity { * Makes API call to check if user is blocked from Commons. If the user is blocked, a snackbar * is created to notify the user */ - protected void showBlockStatus() - { - Observable.fromCallable(() -> mediaWikiApi.isUserBlockedFromCommons()) + protected void showBlockStatus() { + compositeDisposable.add(Observable.fromCallable(() -> mediaWikiApi.isUserBlockedFromCommons()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .filter(result -> result) .subscribe(result -> ViewUtil.showShortSnackbar(findViewById(android.R.id.content), R.string.block_notification) - ); + )); } } diff --git a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java index a0f401a06..464f6feb6 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java @@ -52,6 +52,7 @@ import fr.free.nrw.commons.utils.ConfigUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -83,6 +84,7 @@ public class LoginActivity extends AccountAuthenticatorActivity { ProgressDialog progressDialog; private AppCompatDelegate delegate; private LoginTextWatcher textWatcher = new LoginTextWatcher(); + private CompositeDisposable compositeDisposable = new CompositeDisposable(); private Boolean loginCurrentlyInProgress = false; private Boolean errorMessageShown = false; @@ -195,6 +197,7 @@ public class LoginActivity extends AccountAuthenticatorActivity { @Override protected void onDestroy() { + compositeDisposable.clear(); try { // To prevent leaked window when finish() is called, see http://stackoverflow.com/questions/32065854/activity-has-leaked-window-at-alertdialog-show-method if (progressDialog != null && progressDialog.isShowing()) { @@ -219,10 +222,10 @@ public class LoginActivity extends AccountAuthenticatorActivity { String twoFactorCode = twoFactorEdit.getText().toString(); showLoggingProgressBar(); - Observable.fromCallable(() -> login(username, password, twoFactorCode)) + compositeDisposable.add(Observable.fromCallable(() -> login(username, password, twoFactorCode)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .subscribe(result -> handleLogin(username, rawUsername, password, result)); + .subscribe(result -> handleLogin(username, rawUsername, password, result))); } private String login(String username, String password, String twoFactorCode) { diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesFragment.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesFragment.java index 19e9a9460..d2353d887 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/pictures/BookmarkPicturesFragment.java @@ -30,6 +30,7 @@ import fr.free.nrw.commons.utils.NetworkUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -41,6 +42,7 @@ public class BookmarkPicturesFragment extends DaggerFragment { private static final int TIMEOUT_SECONDS = 15; private GridViewAdapter gridAdapter; + private CompositeDisposable compositeDisposable = new CompositeDisposable(); @BindView(R.id.statusMessage) TextView statusTextView; @BindView(R.id.loadingImagesProgressBar) ProgressBar progressBar; @@ -82,6 +84,12 @@ public class BookmarkPicturesFragment extends DaggerFragment { controller.stop(); } + @Override + public void onDestroy() { + super.onDestroy(); + compositeDisposable.clear(); + } + @Override public void onResume() { super.onResume(); @@ -113,11 +121,11 @@ public class BookmarkPicturesFragment extends DaggerFragment { progressBar.setVisibility(VISIBLE); statusTextView.setVisibility(GONE); - Observable.fromCallable(() -> controller.loadBookmarkedPictures()) + compositeDisposable.add(Observable.fromCallable(() -> controller.loadBookmarkedPictures()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java index 86382b195..022cbb45e 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryImagesListFragment.java @@ -31,6 +31,7 @@ import fr.free.nrw.commons.utils.NetworkUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -51,6 +52,7 @@ public class CategoryImagesListFragment extends DaggerFragment { @BindView(R.id.loadingImagesProgressBar) ProgressBar progressBar; @BindView(R.id.categoryImagesList) GridView gridView; @BindView(R.id.parentLayout) RelativeLayout parentLayout; + private CompositeDisposable compositeDisposable = new CompositeDisposable(); private boolean hasMoreImages = true; private boolean isLoading = true; private String categoryName = null; @@ -74,6 +76,12 @@ public class CategoryImagesListFragment extends DaggerFragment { initViews(); } + @Override + public void onDestroy() { + super.onDestroy(); + compositeDisposable.clear(); + } + /** * Initializes the UI elements for the fragment * Setup the grid view to and scroll listener for it @@ -109,11 +117,11 @@ public class CategoryImagesListFragment extends DaggerFragment { isLoading = true; progressBar.setVisibility(VISIBLE); - Observable.fromCallable(() -> controller.getCategoryImages(categoryName)) + compositeDisposable.add(Observable.fromCallable(() -> controller.getCategoryImages(categoryName)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } /** @@ -215,11 +223,11 @@ public class CategoryImagesListFragment extends DaggerFragment { } progressBar.setVisibility(VISIBLE); - Observable.fromCallable(() -> controller.getCategoryImages(categoryName)) + compositeDisposable.add(Observable.fromCallable(() -> controller.getCategoryImages(categoryName)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } /** 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 20c220cf7..40570b5f8 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 @@ -97,17 +97,17 @@ public class SubCategoryListFragment extends CommonsDaggerSupportFragment { } progressBar.setVisibility(View.VISIBLE); if (!isParentCategory){ - Observable.fromCallable(() -> mwApi.getSubCategoryList(categoryName)) + compositeDisposable.add(Observable.fromCallable(() -> mwApi.getSubCategoryList(categoryName)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); }else { - Observable.fromCallable(() -> mwApi.getParentCategoryList(categoryName)) + compositeDisposable.add(Observable.fromCallable(() -> mwApi.getParentCategoryList(categoryName)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } } diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java index 66eafb106..6f95581b5 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java @@ -61,7 +61,6 @@ import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -101,7 +100,6 @@ public class ContributionsFragment @BindView(R.id.card_view_nearby) public NearbyNotificationCardView nearbyNotificationCardView; @BindView(R.id.campaigns_view) CampaignView campaignView; - private Disposable placesDisposable; private LatLng curLatLng; private boolean firstLocationUpdate = true; @@ -592,7 +590,7 @@ public class ContributionsFragment private void updateClosestNearbyCardViewInfo() { curLatLng = locationManager.getLastLocation(); - placesDisposable = Observable.fromCallable(() -> nearbyController + compositeDisposable.add(Observable.fromCallable(() -> nearbyController .loadAttractionsFromLocation(curLatLng, curLatLng, true, false)) // thanks to boolean, it will only return closest result .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -600,7 +598,7 @@ public class ContributionsFragment throwable -> { Timber.d(throwable); updateNearbyNotification(null); - }); + })); } private void updateNearbyNotification(@Nullable NearbyController.NearbyPlacesInfo nearbyPlacesInfo) { @@ -635,10 +633,6 @@ public class ContributionsFragment isUploadServiceConnected = false; } } - - if (placesDisposable != null) { - placesDisposable.dispose(); - } } @Override diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java index 9f9c6e649..833e835d0 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java @@ -307,11 +307,11 @@ public class MainActivity extends AuthenticatedActivity implements FragmentManag @SuppressLint("CheckResult") private void setNotificationCount() { - Observable.fromCallable(() -> notificationController.getNotifications(false)) + compositeDisposable.add(Observable.fromCallable(() -> notificationController.getNotifications(false)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(this::initNotificationViews, - throwable -> Timber.e(throwable, "Error occurred while loading notifications")); + throwable -> Timber.e(throwable, "Error occurred while loading notifications"))); } private void initNotificationViews(List notificationList) { diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsDaggerSupportFragment.java b/app/src/main/java/fr/free/nrw/commons/di/CommonsDaggerSupportFragment.java index 6d13454e7..992ffb5df 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsDaggerSupportFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsDaggerSupportFragment.java @@ -9,18 +9,27 @@ import androidx.fragment.app.Fragment; import dagger.android.AndroidInjector; import dagger.android.DispatchingAndroidInjector; import dagger.android.support.HasSupportFragmentInjector; +import io.reactivex.disposables.CompositeDisposable; public abstract class CommonsDaggerSupportFragment extends Fragment implements HasSupportFragmentInjector { @Inject DispatchingAndroidInjector childFragmentInjector; + protected CompositeDisposable compositeDisposable = new CompositeDisposable(); + @Override public void onAttach(Context context) { inject(); super.onAttach(context); } + @Override + public void onDestroy() { + super.onDestroy(); + compositeDisposable.clear(); + } + @Override public AndroidInjector supportFragmentInjector() { return childFragmentInjector; diff --git a/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java b/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java index 3e758455a..d84213b45 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java @@ -98,7 +98,7 @@ public class SearchActivity extends NavigationBaseActivity implements MediaDetai viewPagerAdapter.setTabData(fragmentList, titleList); viewPagerAdapter.notifyDataSetChanged(); - RxSearchView.queryTextChanges(searchView) + compositeDisposable.add(RxSearchView.queryTextChanges(searchView) .takeUntil(RxView.detaches(searchView)) .debounce(500, TimeUnit.MILLISECONDS) .observeOn(AndroidSchedulers.mainThread()) @@ -120,7 +120,7 @@ public class SearchActivity extends NavigationBaseActivity implements MediaDetai searchHistoryContainer.setVisibility(View.VISIBLE); } } - ); + )); } /** 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 c01ec35b1..b5301e39a 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 @@ -3,7 +3,6 @@ package fr.free.nrw.commons.explore.categories; import android.content.res.Configuration; import android.os.Bundle; -import android.os.Handler; import androidx.recyclerview.widget.GridLayoutManager; import androidx.recyclerview.widget.LinearLayoutManager; import androidx.recyclerview.widget.RecyclerView; @@ -136,12 +135,12 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { progressBar.setVisibility(GONE); queryList.clear(); categoriesAdapter.clear(); - Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) + compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .doOnSubscribe(disposable -> saveQuery(query)) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } @@ -152,11 +151,11 @@ public class SearchCategoryFragment extends CommonsDaggerSupportFragment { this.query = query; bottomProgressBar.setVisibility(View.VISIBLE); progressBar.setVisibility(GONE); - Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) + compositeDisposable.add(Observable.fromCallable(() -> mwApi.searchCategory(query,queryList.size())) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handlePaginationSuccess, this::handleError); + .subscribe(this::handlePaginationSuccess, this::handleError)); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java index 3cdbf20cd..245ff9f1c 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/images/SearchImageFragment.java @@ -4,7 +4,6 @@ package fr.free.nrw.commons.explore.images; import android.annotation.SuppressLint; import android.content.res.Configuration; import android.os.Bundle; -import android.os.Handler; import androidx.recyclerview.widget.GridLayoutManager; import androidx.recyclerview.widget.LinearLayoutManager; import androidx.recyclerview.widget.RecyclerView; @@ -142,12 +141,12 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment { bottomProgressBar.setVisibility(GONE); queryList.clear(); imagesAdapter.clear(); - okHttpJsonApiClient.searchImages(query, queryList.size()) + compositeDisposable.add(okHttpJsonApiClient.searchImages(query, queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .doOnSubscribe(disposable -> saveQuery(query)) - .subscribe(this::handleSuccess, this::handleError); + .subscribe(this::handleSuccess, this::handleError)); } @@ -159,11 +158,11 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment { this.query = query; bottomProgressBar.setVisibility(View.VISIBLE); progressBar.setVisibility(GONE); - okHttpJsonApiClient.searchImages(query, queryList.size()) + compositeDisposable.add(okHttpJsonApiClient.searchImages(query, queryList.size()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) - .subscribe(this::handlePaginationSuccess, this::handleError); + .subscribe(this::handlePaginationSuccess, this::handleError)); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java index ae097b488..0255e1864 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java @@ -420,7 +420,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { private void onDeleteClicked(Spinner spinner) { String reason = spinner.getSelectedItem().toString(); Single deletionReason = reasonBuilder.getReason(media, reason); - deletionReason + compositeDisposable.add(deletionReason .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(s -> { @@ -428,7 +428,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { deleteTask.execute(); isDeleted = true; enableDeleteButton(false); - }); + })); } @OnClick(R.id.seeMore) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java index 3f7769fde..7b63bdf8b 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java @@ -42,7 +42,6 @@ import fr.free.nrw.commons.utils.ViewUtil; import fr.free.nrw.commons.wikidata.WikidataEditListener; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -85,8 +84,6 @@ public class NearbyFragment extends CommonsDaggerSupportFragment private BottomSheetBehavior bottomSheetBehaviorForDetails; // Behavior for details bottom sheet private LatLng curLatLng; - private Disposable placesDisposable; - private Disposable placesDisposableCustom; private boolean lockNearbyView; //Determines if the nearby places needs to be refreshed public View view; private Snackbar snackbar; @@ -298,7 +295,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment bundle.clear(); bundle.putString("CurLatLng", gsonCurLatLng); - placesDisposable = Observable.fromCallable(() -> nearbyController + compositeDisposable.add(Observable.fromCallable(() -> nearbyController .loadAttractionsFromLocation(curLatLng, curLatLng, false, true)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -307,7 +304,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment Timber.d(throwable); showErrorMessage(getString(R.string.error_fetching_nearby_places)); progressBar.setVisibility(View.GONE); - }); + })); } else if (locationChangeType .equals(LOCATION_SLIGHTLY_CHANGED) && nearbyMapFragment != null) { @@ -335,7 +332,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment populateForCurrentLocation = refreshForCurrentLocation; this.customLatLng = customLatLng; - placesDisposableCustom = Observable.fromCallable(() -> nearbyController + compositeDisposable.add(Observable.fromCallable(() -> nearbyController .loadAttractionsFromLocation(curLatLng, customLatLng, false, populateForCurrentLocation)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -343,7 +340,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment throwable -> { Timber.d(throwable); showErrorMessage(getString(R.string.error_fetching_nearby_places)); - }); + })); if (nearbyMapFragment != null) { nearbyMapFragment.searchThisAreaButton.setVisibility(View.GONE); @@ -467,7 +464,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment || curLatLng.getLongitude() < nearbyMapFragment.boundaryCoordinates[2].getLongitude() || curLatLng.getLongitude() > nearbyMapFragment.boundaryCoordinates[3].getLongitude())) { // populate places - placesDisposable = Observable.fromCallable(() -> nearbyController + compositeDisposable.add(Observable.fromCallable(() -> nearbyController .loadAttractionsFromLocation(curLatLng, curLatLng, false, updateViaButton)) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) @@ -476,7 +473,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment Timber.d(throwable); showErrorMessage(getString(R.string.error_fetching_nearby_places)); progressBar.setVisibility(View.GONE); - }); + })); nearbyMapFragment.setBundleForUpdates(bundle); nearbyMapFragment.updateMapSignificantlyForCurrentLocation(); updateListFragment(); @@ -782,13 +779,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment @Override public void onDestroy() { super.onDestroy(); - if (placesDisposable != null) { - placesDisposable.dispose(); - } wikidataEditListener.setAuthenticationStateListener(null); - if (placesDisposableCustom != null) { - placesDisposableCustom.dispose(); - } } @Override diff --git a/app/src/main/java/fr/free/nrw/commons/theme/BaseActivity.java b/app/src/main/java/fr/free/nrw/commons/theme/BaseActivity.java index e169e6109..82fb6e9dd 100644 --- a/app/src/main/java/fr/free/nrw/commons/theme/BaseActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/theme/BaseActivity.java @@ -8,13 +8,14 @@ import javax.inject.Named; import fr.free.nrw.commons.R; import fr.free.nrw.commons.di.CommonsDaggerAppCompatActivity; import fr.free.nrw.commons.kvstore.JsonKvStore; -import fr.free.nrw.commons.kvstore.JsonKvStore; +import io.reactivex.disposables.CompositeDisposable; public abstract class BaseActivity extends CommonsDaggerAppCompatActivity { @Inject @Named("default_preferences") public JsonKvStore defaultKvStore; + protected CompositeDisposable compositeDisposable = new CompositeDisposable(); protected boolean wasPreviouslyDarkTheme; @Override @@ -33,4 +34,10 @@ public abstract class BaseActivity extends CommonsDaggerAppCompatActivity { super.onResume(); } + + @Override + protected void onDestroy() { + super.onDestroy(); + compositeDisposable.clear(); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java index 4c202071b..16fc5712a 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java @@ -17,6 +17,7 @@ import androidx.exifinterface.media.ExifInterface; import fr.free.nrw.commons.caching.CacheController; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.mwapi.CategoryApi; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -42,11 +43,16 @@ public class FileProcessor implements SimilarImageDialogFragment.onResponse { private ExifInterface exifInterface; private boolean haveCheckedForOtherImages = false; private GPSExtractor tempImageObj; + private CompositeDisposable compositeDisposable = new CompositeDisposable(); @Inject FileProcessor() { } + public void cleanup() { + compositeDisposable.clear(); + } + void initFileDetails(@NonNull String filePath, ContentResolver contentResolver) { this.filePath = filePath; this.contentResolver = contentResolver; @@ -138,7 +144,7 @@ public class FileProcessor implements SimilarImageDialogFragment.onResponse { // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories if (catListEmpty) { - apiCall.request(decimalCoords) + compositeDisposable.add(apiCall.request(decimalCoords) .subscribeOn(Schedulers.io()) .observeOn(Schedulers.io()) .subscribe( @@ -147,7 +153,7 @@ public class FileProcessor implements SimilarImageDialogFragment.onResponse { Timber.e(throwable); gpsCategoryModel.clear(); } - ); + )); Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList); } else { Timber.d("Cache found, setting categoryList in model to %s", displayCatList); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java index ede4ae08b..be6ce413a 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java @@ -33,16 +33,19 @@ public class ImageProcessingService { private final MediaWikiApi mwApi; private final ReadFBMD readFBMD; private final EXIFReader EXIFReader; + private final Context context; @Inject public ImageProcessingService(FileUtilsWrapper fileUtilsWrapper, ImageUtilsWrapper imageUtilsWrapper, - MediaWikiApi mwApi, ReadFBMD readFBMD, EXIFReader EXIFReader) { + MediaWikiApi mwApi, ReadFBMD readFBMD, EXIFReader EXIFReader, + Context context) { this.fileUtilsWrapper = fileUtilsWrapper; this.imageUtilsWrapper = imageUtilsWrapper; this.mwApi = mwApi; this.readFBMD = readFBMD; this.EXIFReader = EXIFReader; + this.context = context; } /** @@ -61,13 +64,13 @@ public class ImageProcessingService { Timber.d("Checking the validity of image"); String filePath = uploadItem.getMediaUri().getPath(); Uri contentUri=uploadItem.getContentUri(); - Context context=uploadItem.getContext(); Single duplicateImage = checkDuplicateImage(filePath); Single wrongGeoLocation = checkImageGeoLocation(uploadItem.getPlace(), filePath); Single darkImage = checkDarkImage(filePath); Single itemTitle = checkTitle ? validateItemTitle(uploadItem) : Single.just(ImageUtils.IMAGE_OK); Single checkFBMD = checkFBMD(context,contentUri); Single checkEXIF = checkEXIF(filePath); + Single zipResult = Single.zip(duplicateImage, wrongGeoLocation, darkImage, itemTitle, (duplicate, wrongGeo, dark, title) -> { Timber.d("Result for duplicate: %d, geo: %d, dark: %d, title: %d", duplicate, wrongGeo, dark, title); 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 53361dc01..4f6b4f056 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 @@ -5,7 +5,6 @@ import android.annotation.SuppressLint; import android.app.ProgressDialog; import android.content.Intent; import android.net.Uri; -import android.os.Build; import android.os.Bundle; import com.google.android.material.textfield.TextInputLayout; import androidx.appcompat.app.AlertDialog; @@ -67,7 +66,6 @@ import fr.free.nrw.commons.utils.PermissionUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -134,7 +132,6 @@ public class UploadActivity extends BaseActivity implements UploadView, SimilarI private DescriptionsAdapter descriptionsAdapter; private RVRendererAdapter categoriesAdapter; - private CompositeDisposable compositeDisposable; private ProgressDialog progressDialog; @@ -145,7 +142,6 @@ public class UploadActivity extends BaseActivity implements UploadView, SimilarI setContentView(R.layout.activity_upload); ButterKnife.bind(this); - compositeDisposable = new CompositeDisposable(); configureLayout(); configureTopCard(); @@ -211,8 +207,6 @@ public class UploadActivity extends BaseActivity implements UploadView, SimilarI @Override protected void onPause() { presenter.removeView(); - compositeDisposable.dispose(); - compositeDisposable = new CompositeDisposable(); super.onPause(); } @@ -571,7 +565,7 @@ public class UploadActivity extends BaseActivity implements UploadView, SimilarI @SuppressLint("CheckResult") private void updateCategoryList(String filter) { List imageTitleList = presenter.getImageTitleList(); - Observable.fromIterable(categoriesModel.getSelectedCategories()) + compositeDisposable.add(Observable.fromIterable(categoriesModel.getSelectedCategories()) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .doOnSubscribe(disposable -> { @@ -602,7 +596,7 @@ public class UploadActivity extends BaseActivity implements UploadView, SimilarI categoriesSearchContainer.setError("No categories found"); } } - ); + )); } private void receiveSharedItems() { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java index 3eb0319f1..afd59f4ca 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java @@ -26,7 +26,7 @@ import fr.free.nrw.commons.utils.ImageUtils; import io.reactivex.Observable; import io.reactivex.Single; import io.reactivex.android.schedulers.AndroidSchedulers; -import io.reactivex.disposables.Disposable; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.functions.Consumer; import io.reactivex.schedulers.Schedulers; import io.reactivex.subjects.BehaviorSubject; @@ -45,6 +45,7 @@ public class UploadModel { }; private final JsonKvStore store; private final List licenses; + private final Context context; private String license; private final Map licensesByName; private List items = new ArrayList<>(); @@ -52,8 +53,7 @@ public class UploadModel { private boolean bottomCardState = true; private boolean rightCardState = true; private int currentStepIndex = 0; - public static Context context; - private Disposable badImageSubscription; + private CompositeDisposable compositeDisposable = new CompositeDisposable(); private SessionManager sessionManager; private FileProcessor fileProcessor; @@ -77,6 +77,11 @@ public class UploadModel { this.imageProcessingService = imageProcessingService; } + void cleanup() { + compositeDisposable.clear(); + fileProcessor.cleanup(); + } + @SuppressLint("CheckResult") Observable preProcessImages(List uploadableFiles, Place place, @@ -219,8 +224,7 @@ public class UploadModel { } public void previous() { - if (badImageSubscription != null) - badImageSubscription.dispose(); + cleanup(); markCurrentUploadVisited(); if (currentStepIndex > 0) { currentStepIndex--; @@ -305,16 +309,16 @@ public class UploadModel { } void deletePicture() { - badImageSubscription.dispose(); + cleanup(); updateItemState(); } void subscribeBadPicture(Consumer consumer, boolean checkTitle) { if (isShowingItem()) { - badImageSubscription = getImageQuality(getCurrentItem(), checkTitle) + compositeDisposable.add(getImageQuality(getCurrentItem(), checkTitle) .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) - .subscribe(consumer, Timber::e); + .subscribe(consumer, Timber::e)); } } @@ -432,10 +436,6 @@ public class UploadModel { public Uri getContentUri() { return originalContentUri; } - - public Context getContext(){ - return UploadModel.context; - } } } \ No newline at end of file diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java index 30bfc037f..36348e7e6 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadPresenter.java @@ -21,6 +21,7 @@ import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.utils.StringUtils; import io.reactivex.Observable; import io.reactivex.android.schedulers.AndroidSchedulers; +import io.reactivex.disposables.CompositeDisposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; @@ -52,6 +53,7 @@ public class UploadPresenter { private final UploadController uploadController; private final Context context; private final JsonKvStore directKvStore; + private CompositeDisposable compositeDisposable = new CompositeDisposable(); @Inject UploadPresenter(UploadModel uploadModel, @@ -77,12 +79,12 @@ public class UploadPresenter { Observable uploadItemObservable = uploadModel .preProcessImages(media, place, source, similarImageInterface); - uploadItemObservable + compositeDisposable.add(uploadItemObservable .toList() .subscribeOn(Schedulers.io()) .observeOn(AndroidSchedulers.mainThread()) .subscribe(uploadItems -> onImagesProcessed(uploadItems, place), - throwable -> Timber.e(throwable, "Error occurred in processing images")); + throwable -> Timber.e(throwable, "Error occurred in processing images"))); } private void onImagesProcessed(List uploadItems, Place place) { @@ -211,9 +213,9 @@ public class UploadPresenter { @SuppressLint("CheckResult") void handleSubmit(CategoriesModel categoriesModel) { if (view.checkIfLoggedIn()) - uploadModel.buildContributions(categoriesModel.getCategoryStringList()) + compositeDisposable.add(uploadModel.buildContributions(categoriesModel.getCategoryStringList()) .observeOn(Schedulers.io()) - .subscribe(uploadController::startUpload); + .subscribe(uploadController::startUpload)); } /** @@ -286,6 +288,8 @@ public class UploadPresenter { } void cleanup() { + compositeDisposable.clear(); + uploadModel.cleanup(); uploadController.cleanup(); }