Get rid of HandlerService #3555 (#3671)

* [WIP] Removed HandlerService

* [WIP] Added PublishProcessor for managing queue

* Resolved conflicts after merge

app/src/main/java/fr/free/nrw/commons/HandlerService.java

@@ -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<T> 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);
-}

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());

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);
     }
 
 

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<Contribution> {
+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<Contribution> {
     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<Contribution> {
         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<Contribution> contributionsToUpload;
+
+    @Override
+    public IBinder onBind(Intent intent) {
+        return localBinder;
+    }
+
     @Override
     public void onCreate() {
         super.onCreate();
@@ -131,48 +143,35 @@ public class UploadService extends HandlerService<Contribution> {
         compositeDisposable = new CompositeDisposable();
         notificationManager = NotificationManagerCompat.from(this);
         curNotification = getNotificationBuilder(CommonsApplication.NOTIFICATION_CHANNEL_ID_ALL);
+        contributionsToUpload = PublishProcessor.create();
+        compositeDisposable.add(contributionsToUpload.subscribe(this::handleUpload));
     }
 
-    @Override
+    public void handleUpload(Contribution contribution) {
-    protected void handle(int what, Contribution contribution) {
+        contribution.setState(Contribution.STATE_QUEUED);
-        switch (what) {
+        contribution.setTransferred(0);
-            case ACTION_UPLOAD_FILE:
+        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;
+            }, Throwable::printStackTrace));
-            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");
-        }
     }
 
     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) {

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)
     }