Fix a couple more Rx/context inconsistencies. (#2715)

* Stop storing Context!

* Properly contain and dispose of observables.

* Add a few more containments of Rx observables.
This commit is contained in:
Dmitry Brant 2019-03-22 23:22:36 -04:00 committed by Vivek Maskara
parent 7cb1f56165
commit f7302d4301
6 changed files with 76 additions and 92 deletions

View file

@ -236,7 +236,7 @@ public class DeleteTask extends AsyncTask<Void, Integer, Boolean> {
}
}
((ReviewActivity)context).reviewController.swipeToNext();
((ReviewActivity)context).swipeToNext();
((ReviewActivity)context).runRandomizer();
DeleteTask deleteTask = new DeleteTask(context, media, reason);

View file

@ -78,7 +78,7 @@ public class NotificationActivity extends NavigationBaseActivity {
@SuppressLint("CheckResult")
public void removeNotification(Notification notification) {
Observable.fromCallable(() -> controller.markAsRead(notification))
compositeDisposable.add(Observable.fromCallable(() -> controller.markAsRead(notification))
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(result -> {
@ -107,7 +107,7 @@ public class NotificationActivity extends NavigationBaseActivity {
throwable.printStackTrace();
ViewUtil.showShortSnackbar(relativeLayout, R.string.error_notifications);
progressBar.setVisibility(View.GONE);
});
}));
}
@ -140,7 +140,7 @@ public class NotificationActivity extends NavigationBaseActivity {
private void addNotifications(boolean archived) {
Timber.d("Add notifications");
if (mNotificationWorkerFragment == null) {
Observable.fromCallable(() -> {
compositeDisposable.add(Observable.fromCallable(() -> {
progressBar.setVisibility(View.VISIBLE);
return controller.getNotifications(archived);
@ -164,7 +164,7 @@ public class NotificationActivity extends NavigationBaseActivity {
Timber.e(throwable, "Error occurred while loading notifications");
ViewUtil.showShortSnackbar(relativeLayout, R.string.error_notifications);
progressBar.setVisibility(View.GONE);
});
}));
} else {
notificationList = mNotificationWorkerFragment.getNotificationList();
setAdapter(notificationList);

View file

@ -18,6 +18,7 @@ import androidx.appcompat.widget.Toolbar;
import androidx.drawerlayout.widget.DrawerLayout;
import butterknife.BindView;
import butterknife.ButterKnife;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.AuthenticatedActivity;
import fr.free.nrw.commons.mwapi.MediaResult;
@ -75,7 +76,7 @@ public class ReviewActivity extends AuthenticatedActivity {
ButterKnife.bind(this);
initDrawer();
reviewController = new ReviewController(this);
reviewController = new ReviewController();
reviewPagerAdapter = new ReviewPagerAdapter(getSupportFragmentManager());
reviewPager.setAdapter(reviewPagerAdapter);
@ -97,7 +98,7 @@ public class ReviewActivity extends AuthenticatedActivity {
reviewPager.setCurrentItem(0);
compositeDisposable.add(reviewHelper.getRandomMedia()
.map(media -> media.getFilename())
.map(Media::getFilename)
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(this::updateImage));
@ -110,21 +111,21 @@ public class ReviewActivity extends AuthenticatedActivity {
return;
}
reviewController.onImageRefreshed(fileName); //file name is updated
reviewHelper.getFirstRevisionOfFile("File:" + fileName)
compositeDisposable.add(reviewHelper.getFirstRevisionOfFile("File:" + fileName)
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(revision -> {
reviewController.firstRevision = revision;
reviewPagerAdapter.updateFileInformation(fileName, revision);
});
}));
reviewPager.setCurrentItem(0);
Observable.fromCallable(() -> {
compositeDisposable.add(Observable.fromCallable(() -> {
MediaResult media = mwApi.fetchMediaByFilename("File:" + fileName);
return MediaDataExtractorUtil.extractCategories(media.getWikiSource());
})
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(this::updateCategories, this::categoryFetchError);
.subscribe(this::updateCategories, this::categoryFetchError));
}
@ -139,12 +140,13 @@ public class ReviewActivity extends AuthenticatedActivity {
reviewPagerAdapter.updateCategories();
}
/**
* References ReviewPagerAdapter to null before the activity is destroyed
*/
@Override
public void onDestroy() {
super.onDestroy();
public void swipeToNext() {
int nextPos = reviewPager.getCurrentItem() + 1;
if (nextPos <= 3) {
reviewPager.setCurrentItem(nextPos);
} else {
runRandomizer();
}
}
/**

View file

@ -1,6 +1,7 @@
package fr.free.nrw.commons.review;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.NotificationManager;
import android.content.Context;
import android.view.Gravity;
@ -11,9 +12,9 @@ import java.util.ArrayList;
import javax.inject.Inject;
import javax.inject.Singleton;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.core.app.NotificationCompat;
import androidx.viewpager.widget.ViewPager;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.SessionManager;
@ -43,16 +44,6 @@ public class ReviewController {
@Inject
SessionManager sessionManager;
private ReviewPagerAdapter reviewPagerAdapter;
private ViewPager viewPager;
private ReviewActivity reviewActivity;
ReviewController(Context context) {
reviewActivity = (ReviewActivity) context;
reviewPagerAdapter = reviewActivity.reviewPagerAdapter;
viewPager = ((ReviewActivity) context).reviewPager;
}
public void onImageRefreshed(String fileName) {
this.fileName = fileName;
media = new Media("File:" + fileName);
@ -63,49 +54,41 @@ public class ReviewController {
ReviewController.categories = categories;
}
public void swipeToNext() {
int nextPos = viewPager.getCurrentItem() + 1;
if (nextPos <= 3) {
viewPager.setCurrentItem(nextPos);
} else {
reviewActivity.runRandomizer();
}
public void reportSpam(@NonNull Activity activity) {
DeleteTask.askReasonAndExecute(new Media("File:" + fileName),
activity,
activity.getString(R.string.review_spam_report_question),
activity.getString(R.string.review_spam_report_problem));
}
public void reportSpam() {
public void reportPossibleCopyRightViolation(@NonNull Activity activity) {
DeleteTask.askReasonAndExecute(new Media("File:" + fileName),
reviewActivity,
reviewActivity.getResources().getString(R.string.review_spam_report_question),
reviewActivity.getResources().getString(R.string.review_spam_report_problem));
}
public void reportPossibleCopyRightViolation() {
DeleteTask.askReasonAndExecute(new Media("File:" + fileName),
reviewActivity,
reviewActivity.getResources().getString(R.string.review_c_violation_report_question),
reviewActivity.getResources().getString(R.string.review_c_violation_report_problem));
activity,
activity.getResources().getString(R.string.review_c_violation_report_question),
activity.getResources().getString(R.string.review_c_violation_report_problem));
}
@SuppressLint("CheckResult")
public void reportWrongCategory() {
public void reportWrongCategory(@NonNull Activity activity) {
Context context = activity.getApplicationContext();
ApplicationlessInjection
.getInstance(reviewActivity.getApplicationContext())
.getInstance(context)
.getCommonsApplicationComponent()
.inject(this);
notificationManager = (NotificationManager) reviewActivity.getSystemService(Context.NOTIFICATION_SERVICE);
notificationBuilder = new NotificationCompat.Builder(reviewActivity);
Toast toast = new Toast(reviewActivity);
notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
notificationBuilder = new NotificationCompat.Builder(context);
Toast toast = new Toast(context);
toast.setGravity(Gravity.CENTER, 0, 0);
toast = Toast.makeText(reviewActivity, reviewActivity.getString(R.string.check_category_toast, media.getDisplayTitle()), Toast.LENGTH_SHORT);
toast = Toast.makeText(context, context.getString(R.string.check_category_toast, media.getDisplayTitle()), Toast.LENGTH_SHORT);
toast.show();
Observable.fromCallable(() -> {
publishProgress(0);
publishProgress(context, 0);
String editToken;
String authCookie;
String summary = reviewActivity.getString(R.string.check_category_edit_summary);
String summary = context.getString(R.string.check_category_edit_summary);
authCookie = sessionManager.getAuthCookie();
mwApi.setAuthCookie(authCookie);
@ -115,10 +98,10 @@ public class ReviewController {
if (editToken.equals("+\\")) {
return false;
}
publishProgress(1);
publishProgress(context, 1);
mwApi.appendEdit(editToken, "\n{{subst:chc}}\n", media.getFilename(), summary);
publishProgress(2);
publishProgress(context, 2);
} catch (Exception e) {
Timber.d(e);
return false;
@ -132,11 +115,11 @@ public class ReviewController {
String title = "";
if (result) {
title = reviewActivity.getString(R.string.check_category_success_title);
message = reviewActivity.getString(R.string.check_category_success_message, media.getDisplayTitle());
title = context.getString(R.string.check_category_success_title);
message = context.getString(R.string.check_category_success_message, media.getDisplayTitle());
} else {
title = reviewActivity.getString(R.string.check_category_failure_title);
message = reviewActivity.getString(R.string.check_category_failure_message, media.getDisplayTitle());
title = context.getString(R.string.check_category_failure_title);
message = context.getString(R.string.check_category_failure_message, media.getDisplayTitle());
}
notificationBuilder.setDefaults(NotificationCompat.DEFAULT_ALL)
@ -150,17 +133,16 @@ public class ReviewController {
notificationManager.notify(NOTIFICATION_CHECK_CATEGORY, notificationBuilder.build());
}, Timber::e);
swipeToNext();
}
private void publishProgress(int i) {
private void publishProgress(@NonNull Context context, int i) {
int[] messages = new int[]{R.string.getting_edit_token, R.string.check_category_adding_template};
String message = "";
if (0 < i && i < messages.length) {
message = reviewActivity.getString(messages[i]);
message = context.getString(messages[i]);
}
notificationBuilder.setContentTitle(reviewActivity.getString(R.string.check_category_notification_title, media.getDisplayTitle()))
notificationBuilder.setContentTitle(context.getString(R.string.check_category_notification_title, media.getDisplayTitle()))
.setStyle(new NotificationCompat.BigTextStyle()
.bigText(message))
.setSmallIcon(R.drawable.ic_launcher)
@ -170,20 +152,21 @@ public class ReviewController {
}
@SuppressLint("CheckResult")
public void sendThanks() {
public void sendThanks(@NonNull Activity activity) {
Context context = activity.getApplicationContext();
ApplicationlessInjection
.getInstance(reviewActivity.getApplicationContext())
.getInstance(context)
.getCommonsApplicationComponent()
.inject(this);
notificationManager = (NotificationManager) reviewActivity.getSystemService(Context.NOTIFICATION_SERVICE);
notificationBuilder = new NotificationCompat.Builder(reviewActivity);
Toast toast = new Toast(reviewActivity);
notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
notificationBuilder = new NotificationCompat.Builder(context);
Toast toast = new Toast(context);
toast.setGravity(Gravity.CENTER, 0, 0);
toast = Toast.makeText(reviewActivity, reviewActivity.getString(R.string.send_thank_toast, media.getDisplayTitle()), Toast.LENGTH_SHORT);
toast = Toast.makeText(context, context.getString(R.string.send_thank_toast, media.getDisplayTitle()), Toast.LENGTH_SHORT);
toast.show();
Observable.fromCallable(() -> {
publishProgress(0);
publishProgress(context, 0);
String editToken;
String authCookie;
@ -195,10 +178,10 @@ public class ReviewController {
if (editToken.equals("+\\")) {
return false;
}
publishProgress(1);
publishProgress(context, 1);
assert firstRevision != null;
mwApi.thank(editToken, firstRevision.getRevid());
publishProgress(2);
publishProgress(context, 2);
} catch (Exception e) {
Timber.d(e);
return false;
@ -211,11 +194,11 @@ public class ReviewController {
String message = "";
String title = "";
if (result) {
title = reviewActivity.getString(R.string.send_thank_success_title);
message = reviewActivity.getString(R.string.send_thank_success_message, media.getDisplayTitle());
title = context.getString(R.string.send_thank_success_title);
message = context.getString(R.string.send_thank_success_message, media.getDisplayTitle());
} else {
title = reviewActivity.getString(R.string.send_thank_failure_title);
message = reviewActivity.getString(R.string.send_thank_failure_message, media.getDisplayTitle());
title = context.getString(R.string.send_thank_failure_title);
message = context.getString(R.string.send_thank_failure_message, media.getDisplayTitle());
}
notificationBuilder.setDefaults(NotificationCompat.DEFAULT_ALL)
@ -229,6 +212,5 @@ public class ReviewController {
notificationManager.notify(NOTIFICATION_SEND_THANK, notificationBuilder.build());
}, Timber::e);
swipeToNext();
}
}

View file

@ -94,9 +94,7 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
explanation = getString(R.string.review_copyright_explanation);
yesButtonText = getString(R.string.review_copyright_yes_button_text);
noButtonText = getString(R.string.review_copyright_no_button_text);
yesButton.setOnClickListener(view -> {
((ReviewActivity) getActivity()).reviewController.reportPossibleCopyRightViolation();
});
yesButton.setOnClickListener(view -> getReviewActivity().reviewController.reportPossibleCopyRightViolation(requireActivity()));
break;
case CATEGORY:
question = getString(R.string.review_category);
@ -104,7 +102,8 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
yesButtonText = getString(R.string.review_category_yes_button_text);
noButtonText = getString(R.string.review_category_no_button_text);
yesButton.setOnClickListener(view -> {
((ReviewActivity) getActivity()).reviewController.reportWrongCategory();
getReviewActivity().reviewController.reportWrongCategory(requireActivity());
getReviewActivity().swipeToNext();
});
break;
case SPAM:
@ -112,19 +111,18 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
explanation = getString(R.string.review_spam_explanation);
yesButtonText = getString(R.string.review_spam_yes_button_text);
noButtonText = getString(R.string.review_spam_no_button_text);
yesButton.setOnClickListener(view -> {
((ReviewActivity) getActivity()).reviewController.reportSpam();
});
yesButton.setOnClickListener(view -> getReviewActivity().reviewController.reportSpam(requireActivity()));
break;
case THANKS:
question = getString(R.string.review_thanks);
explanation = getString(R.string.review_thanks_explanation, ((ReviewActivity) getActivity()).reviewController.firstRevision.getUser());
explanation = getString(R.string.review_thanks_explanation, getReviewActivity().reviewController.firstRevision.getUser());
yesButtonText = getString(R.string.review_thanks_yes_button_text);
noButtonText = getString(R.string.review_thanks_no_button_text);
yesButton.setTextColor(Color.parseColor("#228b22"));
noButton.setTextColor(Color.parseColor("#116aaa"));
yesButton.setOnClickListener(view -> {
((ReviewActivity) getActivity()).reviewController.sendThanks();
getReviewActivity().reviewController.sendThanks(getReviewActivity());
getReviewActivity().swipeToNext();
});
break;
default :
@ -134,9 +132,7 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
noButtonText = "no";
}
noButton.setOnClickListener(view -> {
((ReviewActivity) getActivity()).reviewController.swipeToNext();
});
noButton.setOnClickListener(view -> getReviewActivity().swipeToNext());
((TextView) textViewQuestion).setText(question);
((TextView) textViewQuestionContext).setText(explanation);
@ -156,6 +152,10 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
return layoutView;
}
private ReviewActivity getReviewActivity() {
return (ReviewActivity) requireActivity();
}
private void fillImageCaption() {
if (imageCaption != null && fileName != null && revision != null) {
((TextView) imageCaption).setText(fileName + " is uploaded by: " + revision.getUser());

View file

@ -115,11 +115,11 @@ public class UploadPresenter {
List<Description> descriptions) {
Timber.e("Inside handleNext");
view.showProgressDialog();
uploadModel.getImageQuality(uploadModel.getCurrentItem(), true)
compositeDisposable.add(uploadModel.getImageQuality(uploadModel.getCurrentItem(), true)
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(imageResult -> handleImage(title, descriptions, imageResult),
throwable -> Timber.e(throwable, "Error occurred while handling image"));
throwable -> Timber.e(throwable, "Error occurred while handling image")));
}
private void handleImage(Title title, List<Description> descriptions, Integer imageResult) {