From 1fea1bbf49b1f66f49cf82faf2d9b82e94e04a9b Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Sun, 26 Apr 2020 09:17:57 +0300 Subject: [PATCH] Get rid of HandlerService #3555 (#3671) * [WIP] Removed HandlerService * [WIP] Added PublishProcessor for managing queue * Resolved conflicts after merge --- .../fr/free/nrw/commons/HandlerService.java | 74 ---------------- .../contributions/ContributionsFragment.java | 5 +- .../nrw/commons/upload/UploadController.java | 5 +- .../nrw/commons/upload/UploadService.java | 85 +++++++++---------- .../commons/upload/UploadControllerTest.kt | 4 +- 5 files changed, 47 insertions(+), 126 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/HandlerService.java diff --git a/app/src/main/java/fr/free/nrw/commons/HandlerService.java b/app/src/main/java/fr/free/nrw/commons/HandlerService.java deleted file mode 100644 index e5e1b3b1b..000000000 --- a/app/src/main/java/fr/free/nrw/commons/HandlerService.java +++ /dev/null @@ -1,74 +0,0 @@ -package fr.free.nrw.commons; - -import android.content.Intent; -import android.os.Binder; -import android.os.Handler; -import android.os.HandlerThread; -import android.os.IBinder; -import android.os.Looper; -import android.os.Message; - -import fr.free.nrw.commons.di.CommonsDaggerService; - -public abstract class HandlerService extends CommonsDaggerService { - private volatile Looper threadLooper; - private volatile ServiceHandler threadHandler; - private String serviceName; - - private final class ServiceHandler extends Handler { - public ServiceHandler(Looper looper) { - super(looper); - } - - @Override - public void handleMessage(Message msg) { - //FIXME: Google Photos bug - handle(msg.what, (T)msg.obj); - stopSelf(msg.arg1); - } - } - - @Override - public void onDestroy() { - threadLooper.quit(); - super.onDestroy(); - } - - public class HandlerServiceLocalBinder extends Binder { - public HandlerService getService() { - return HandlerService.this; - } - } - - private final IBinder localBinder = new HandlerServiceLocalBinder(); - @Override - public IBinder onBind(Intent intent) { - return localBinder; - } - - protected HandlerService(String serviceName) { - this.serviceName = serviceName; - } - - @Override - public void onCreate() { - super.onCreate(); - HandlerThread thread = new HandlerThread(serviceName); - thread.start(); - - threadLooper = thread.getLooper(); - threadHandler = new ServiceHandler(threadLooper); - } - - private void postMessage(int type, T t) { - Message msg = threadHandler.obtainMessage(type); - msg.obj = t; - threadHandler.sendMessage(msg); - } - - public void queue(int what, T t) { - postMessage(what, t); - } - - protected abstract void handle(int what, T t); -} 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 64b836c39..9022bbfb9 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 @@ -24,7 +24,6 @@ import androidx.fragment.app.FragmentManager.OnBackStackChangedListener; import androidx.fragment.app.FragmentTransaction; import butterknife.BindView; import butterknife.ButterKnife; -import fr.free.nrw.commons.HandlerService; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.campaigns.Campaign; @@ -102,7 +101,7 @@ public class ContributionsFragment private ServiceConnection uploadServiceConnection = new ServiceConnection() { @Override public void onServiceConnected(ComponentName componentName, IBinder binder) { - uploadService = (UploadService) ((HandlerService.HandlerServiceLocalBinder) binder) + uploadService = (UploadService) ((UploadService.UploadServiceLocalBinder) binder) .getService(); isUploadServiceConnected = true; } @@ -587,7 +586,7 @@ public class ContributionsFragment private void retryUpload(Contribution contribution) { if (NetworkUtils.isInternetConnectionEstablished(getContext())) { if (contribution.getState() == STATE_FAILED && null != uploadService) { - uploadService.queue(UploadService.ACTION_UPLOAD_FILE, contribution); + uploadService.queue(contribution); Timber.d("Restarting for %s", contribution.toString()); } else { Timber.d("Skipping re-upload for non-failed %s", contribution.toString()); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java index e28978ff8..cf298f83b 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadController.java @@ -13,7 +13,6 @@ import android.net.Uri; import android.os.IBinder; import android.provider.MediaStore; import android.text.TextUtils; -import fr.free.nrw.commons.HandlerService; import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.SessionManager; @@ -54,7 +53,7 @@ public class UploadController { public ServiceConnection uploadServiceConnection = new ServiceConnection() { @Override public void onServiceConnected(final ComponentName componentName, final IBinder binder) { - uploadService = (UploadService) ((HandlerService.HandlerServiceLocalBinder) binder).getService(); + uploadService = ((UploadService.UploadServiceLocalBinder) binder).getService(); isUploadServiceConnected = true; } @@ -208,7 +207,7 @@ public class UploadController { */ private void upload(final Contribution contribution) { //Starts the upload. If commented out, user can proceed to next Fragment but upload doesn't happen - uploadService.queue(UploadService.ACTION_UPLOAD_FILE, contribution); + uploadService.queue(contribution); } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java index 2dd9b6f1d..89f7fce12 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadService.java @@ -6,24 +6,27 @@ import android.content.ContentResolver; import android.content.Intent; import android.graphics.BitmapFactory; import android.net.Uri; +import android.os.Binder; import android.os.Bundle; +import android.os.IBinder; import androidx.core.app.NotificationCompat; import androidx.core.app.NotificationManagerCompat; import fr.free.nrw.commons.BuildConfig; import fr.free.nrw.commons.CommonsApplication; -import fr.free.nrw.commons.HandlerService; import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.contributions.ContributionDao; import fr.free.nrw.commons.contributions.MainActivity; import fr.free.nrw.commons.di.CommonsApplicationModule; +import fr.free.nrw.commons.di.CommonsDaggerService; import fr.free.nrw.commons.media.MediaClient; import fr.free.nrw.commons.utils.CommonsDateUtil; import fr.free.nrw.commons.wikidata.WikidataEditService; import io.reactivex.Observable; import io.reactivex.Scheduler; import io.reactivex.disposables.CompositeDisposable; +import io.reactivex.processors.PublishProcessor; import io.reactivex.schedulers.Schedulers; import java.io.File; import java.io.IOException; @@ -36,12 +39,10 @@ import javax.inject.Inject; import javax.inject.Named; import timber.log.Timber; -public class UploadService extends HandlerService { +public class UploadService extends CommonsDaggerService { private static final String EXTRA_PREFIX = "fr.free.nrw.commons.upload"; - public static final int ACTION_UPLOAD_FILE = 1; - public static final String ACTION_START_SERVICE = EXTRA_PREFIX + ".upload"; public static final String EXTRA_FILES = EXTRA_PREFIX + ".files"; @Inject WikidataEditService wikidataEditService; @@ -72,10 +73,6 @@ public class UploadService extends HandlerService { public static final int NOTIFICATION_UPLOAD_IN_PROGRESS = 1; public static final int NOTIFICATION_UPLOAD_FAILED = 3; - public UploadService() { - super("UploadService"); - } - protected class NotificationUpdateProgressListener{ String notificationTag; @@ -124,6 +121,21 @@ public class UploadService extends HandlerService { Timber.d("UploadService.onDestroy; %s are yet to be uploaded", unfinishedUploads); } + public class UploadServiceLocalBinder extends Binder { + public UploadService getService() { + return UploadService.this; + } + } + + private final IBinder localBinder = new UploadServiceLocalBinder(); + + private PublishProcessor contributionsToUpload; + + @Override + public IBinder onBind(Intent intent) { + return localBinder; + } + @Override public void onCreate() { super.onCreate(); @@ -131,48 +143,35 @@ public class UploadService extends HandlerService { compositeDisposable = new CompositeDisposable(); notificationManager = NotificationManagerCompat.from(this); curNotification = getNotificationBuilder(CommonsApplication.NOTIFICATION_CHANNEL_ID_ALL); + contributionsToUpload = PublishProcessor.create(); + compositeDisposable.add(contributionsToUpload.subscribe(this::handleUpload)); } - @Override - protected void handle(int what, Contribution contribution) { - switch (what) { - case ACTION_UPLOAD_FILE: + public void handleUpload(Contribution contribution) { + contribution.setState(Contribution.STATE_QUEUED); + contribution.setTransferred(0); + toUpload++; + if (curNotification != null && toUpload != 1) { + curNotification.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload)); + Timber.d("%d uploads left", toUpload); + notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build()); + } + compositeDisposable.add(contributionDao + .save(contribution) + .subscribeOn(ioThreadScheduler) + .observeOn(mainThreadScheduler) + .subscribe(aLong->{ + contribution.set_id(aLong); uploadContribution(contribution); - break; - default: - throw new IllegalArgumentException("Unknown value for what"); - } - } - - @Override - public void queue(int what, Contribution contribution) { - switch (what) { - case ACTION_UPLOAD_FILE: - - contribution.setState(Contribution.STATE_QUEUED); - contribution.setTransferred(0); - toUpload++; - if (curNotification != null && toUpload != 1) { - curNotification.setContentText(getResources().getQuantityString(R.plurals.uploads_pending_notification_indicator, toUpload, toUpload)); - Timber.d("%d uploads left", toUpload); - notificationManager.notify(contribution.getLocalUri().toString(), NOTIFICATION_UPLOAD_IN_PROGRESS, curNotification.build()); - } - compositeDisposable.add(contributionDao - .save(contribution) - .subscribeOn(ioThreadScheduler) - .observeOn(mainThreadScheduler) - .subscribe(aLong->{ - contribution.set_id(aLong); - UploadService.super.queue(what, contribution); - }, Throwable::printStackTrace)); - break; - default: - throw new IllegalArgumentException("Unknown value for what"); - } + }, Throwable::printStackTrace)); } private boolean freshStart = true; + public void queue(Contribution contribution) { + contributionsToUpload.offer(contribution); + } + @Override public int onStartCommand(Intent intent, int flags, int startId) { if (ACTION_START_SERVICE.equals(intent.getAction()) && freshStart) { diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt index cc5025f59..71fdaf1b1 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt @@ -2,10 +2,8 @@ package fr.free.nrw.commons.upload import android.content.ComponentName import android.content.Context -import fr.free.nrw.commons.HandlerService import fr.free.nrw.commons.auth.SessionManager import fr.free.nrw.commons.contributions.Contribution -import fr.free.nrw.commons.kvstore.BasicKvStore import fr.free.nrw.commons.kvstore.JsonKvStore import org.junit.Before import org.junit.Test @@ -31,7 +29,7 @@ class UploadControllerTest { fun setup() { MockitoAnnotations.initMocks(this) val uploadService = mock(UploadService::class.java) - val binder = mock(HandlerService.HandlerServiceLocalBinder::class.java) + val binder = mock(UploadService.UploadServiceLocalBinder::class.java) `when`(binder.service).thenReturn(uploadService) uploadController!!.uploadServiceConnection.onServiceConnected(mock(ComponentName::class.java), binder) }