Upload tests (commons-app#2086)

* Add unit tests for upload flows

* Tests for upload model

* Test fixes

* Remove empty test cases

* Changes based on comments

Author: maskaravivek

app/build.gradle

@@ -77,6 +77,8 @@ dependencies {
     releaseImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY"
     testImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$LEAK_CANARY"
 
+    androidTestImplementation "org.mockito:mockito-core:2.10.0"
+
     //For handling runtime permissions
     implementation 'com.karumi:dexter:5.0.0'
 
@@ -100,6 +102,8 @@ android {
     }
 
     testOptions {
+        unitTests.returnDefaultValues = true
+
         unitTests.all {
             jvmArgs '-noverify'
         }

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

@@ -119,7 +119,7 @@ public void onCreate() {
         ContributionUtils.emptyTemporaryDirectory();
 
         initAcra();
-        if (BuildConfig.DEBUG) {
+        if (BuildConfig.DEBUG && !isRoboUnitTest()) {
             Stetho.initializeWithDefaults(this);
         }
 
@@ -162,6 +162,10 @@ private void initAcra() {
         Thread.setDefaultUncaughtExceptionHandler(exceptionHandler);
     }
 
+    public static boolean isRoboUnitTest() {
+        return "robolectric".equals(Build.FINGERPRINT);
+    }
+
     private ThreadPoolService getFileLoggingThreadPool() {
         return new ThreadPoolService.Builder("file-logging-thread")
                 .setPriority(Process.THREAD_PRIORITY_LOWEST)

app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java

@@ -2,37 +2,31 @@
 
 import android.annotation.SuppressLint;
 import android.content.ContentResolver;
-import android.content.Context;
 import android.content.SharedPreferences;
 import android.media.ExifInterface;
 import android.net.Uri;
 import android.os.Build;
-import android.os.Bundle;
 import android.os.ParcelFileDescriptor;
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
-import android.support.v7.app.AppCompatActivity;
 
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.util.Date;
 import java.util.List;
 
 import javax.inject.Inject;
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import fr.free.nrw.commons.caching.CacheController;
-import fr.free.nrw.commons.di.ApplicationlessInjection;
 import fr.free.nrw.commons.mwapi.CategoryApi;
 import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
-import static com.mapbox.mapboxsdk.Mapbox.getApplicationContext;
-
 /**
  * Processing of the image file that is about to be uploaded via ShareActivity is done here
  */
+@Singleton
 public class FileProcessor implements SimilarImageDialogFragment.onResponse {
 
     @Inject
@@ -47,24 +41,23 @@ public class FileProcessor implements SimilarImageDialogFragment.onResponse {
     private String filePath;
     private ContentResolver contentResolver;
     private GPSExtractor imageObj;
-    private Context context;
     private String decimalCoords;
     private ExifInterface exifInterface;
-    private boolean useExtStorage;
     private boolean haveCheckedForOtherImages = false;
     private GPSExtractor tempImageObj;
 
-    FileProcessor(@NonNull String filePath, ContentResolver contentResolver, Context context) {
+    @Inject
+    FileProcessor() {
+    }
+
+    void initFileDetails(@NonNull String filePath, ContentResolver contentResolver) {
         this.filePath = filePath;
         this.contentResolver = contentResolver;
-        this.context = context;
-        ApplicationlessInjection.getInstance(context.getApplicationContext()).getCommonsApplicationComponent().inject(this);
         try {
-            exifInterface=new ExifInterface(filePath);
+            exifInterface = new ExifInterface(filePath);
         } catch (IOException e) {
             Timber.e(e);
         }
-        useExtStorage = prefs.getBoolean("useExternalStorage", true);
     }
 
     /**
@@ -85,10 +78,6 @@ GPSExtractor processFileCoordinates(SimilarImageInterface similarImageInterface)
         return imageObj;
     }
 
-    String getDecimalCoords() {
-        return decimalCoords;
-    }
-
     /**
      * Find other images around the same location that were taken within the last 20 sec
      * @param similarImageInterface
@@ -142,7 +131,7 @@ private void findOtherImages(SimilarImageInterface similarImageInterface) {
      * Then initiates the calls to MediaWiki API through an instance of CategoryApi.
      */
     @SuppressLint("CheckResult")
-    public void useImageCoords() {
+    private void useImageCoords() {
         if (decimalCoords != null) {
             Timber.d("Decimal coords of image: %s", decimalCoords);
             Timber.d("is EXIF data present:" + imageObj.imageCoordsExists + " from findOther image");

app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java

@@ -234,8 +234,8 @@ else if ("file".equalsIgnoreCase(uri.getScheme())) {
      * @return The value of the _data column, which is typically a file path.
      */
     @Nullable
-    public static String getDataColumn(Context context, Uri uri, String selection,
-                                       String[] selectionArgs) {
+    private static String getDataColumn(Context context, Uri uri, String selection,
+                                        String[] selectionArgs) {
 
         Cursor cursor = null;
         final String column = MediaStore.Images.ImageColumns.DATA;
@@ -311,7 +311,7 @@ public static void copy(@NonNull FileInputStream source, @NonNull FileOutputStre
      * @param destination file path copied to
      * @throws IOException thrown when failing to read source or opening destination file
      */
-    public static void copy(@NonNull FileDescriptor source, @NonNull String destination)
+    private static void copy(@NonNull FileDescriptor source, @NonNull String destination)
             throws IOException {
         copy(new FileInputStream(source), new FileOutputStream(destination));
     }
@@ -415,7 +415,7 @@ public static String getFilename(Uri uri, ContentResolver contentResolver) {
         return result;
     }
 
-    public static String getFileExt(String fileName){
+    static String getFileExt(String fileName){
         //Default file extension
         String extension=".jpg";
 
@@ -426,7 +426,11 @@ public static String getFileExt(String fileName){
         return extension;
     }
 
-    public static String getFileExt(Uri uri, ContentResolver contentResolver) {
+    private static String getFileExt(Uri uri, ContentResolver contentResolver) {
         return getFileExt(getFilename(uri, contentResolver));
     }
+
+    public static FileInputStream getFileInputStream(String filePath) throws FileNotFoundException {
+        return new FileInputStream(filePath);
+    }
 }

app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java

@@ -0,0 +1,42 @@
+package fr.free.nrw.commons.upload;
+
+import android.content.ContentResolver;
+import android.content.Context;
+import android.net.Uri;
+
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+@Singleton
+public class FileUtilsWrapper {
+
+    @Inject
+    public FileUtilsWrapper() {
+
+    }
+
+    public String createExternalCopyPathAndCopy(Uri uri, ContentResolver contentResolver) throws IOException {
+        return FileUtils.createExternalCopyPathAndCopy(uri, contentResolver);
+    }
+
+    public String createCopyPathAndCopy(Uri uri, Context context) throws IOException {
+        return FileUtils.createCopyPathAndCopy(uri, context);
+    }
+
+    public String getFileExt(String fileName) {
+        return FileUtils.getFileExt(fileName);
+    }
+
+    public String getSHA1(InputStream is) {
+        return FileUtils.getSHA1(is);
+    }
+
+    public FileInputStream getFileInputStream(String filePath) throws FileNotFoundException {
+        return FileUtils.getFileInputStream(filePath);
+    }
+}

app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java

@@ -14,12 +14,12 @@
  * Extracts geolocation to be passed to API for category suggestions. If a picture with geolocation
  * is uploaded, extract latitude and longitude from EXIF data of image.
  */
-public class GPSExtractor {
+class GPSExtractor {
 
-    public static final GPSExtractor DUMMY= new GPSExtractor();
+    static final GPSExtractor DUMMY= new GPSExtractor();
     private double decLatitude;
     private double decLongitude;
-    public boolean imageCoordsExists;
+    boolean imageCoordsExists;
     private String latitude;
     private String longitude;
     private String latitudeRef;
@@ -37,7 +37,7 @@ private GPSExtractor(){
      * @param fileDescriptor the file descriptor of the image
      */
     @RequiresApi(24)
-    public GPSExtractor(@NonNull FileDescriptor fileDescriptor) {
+    GPSExtractor(@NonNull FileDescriptor fileDescriptor) {
         try {
             ExifInterface exif = new ExifInterface(fileDescriptor);
             processCoords(exif);
@@ -51,7 +51,7 @@ public GPSExtractor(@NonNull FileDescriptor fileDescriptor) {
      * @param path file path of the image
      *
      */
-    public GPSExtractor(@NonNull String path) {
+    GPSExtractor(@NonNull String path) {
         try {
             ExifInterface exif = new ExifInterface(path);
             processCoords(exif);
@@ -65,7 +65,7 @@ public GPSExtractor(@NonNull String path) {
      * @param exif exif interface of the image
      *
      */
-    public GPSExtractor(@NonNull ExifInterface exif){
+    GPSExtractor(@NonNull ExifInterface exif){
         processCoords(exif);
     }
 
@@ -89,7 +89,7 @@ private void processCoords(ExifInterface exif){
      * @return coordinates as string (needs to be passed as a String in API query)
      */
     @Nullable
-    public String getCoords() {
+    String getCoords() {
         if(decimalCoords!=null){
             return decimalCoords;
         }else if (latitude!=null && latitudeRef!=null && longitude!=null && longitudeRef!=null) {
@@ -103,11 +103,11 @@ public String getCoords() {
         }
     }
 
-    public double getDecLatitude() {
+    double getDecLatitude() {
         return decLatitude;
     }
 
-    public double getDecLongitude() {
+    double getDecLongitude() {
         return decLongitude;
     }
 

app/src/main/java/fr/free/nrw/commons/upload/UploadController.java

@@ -51,7 +51,7 @@ public UploadController(SessionManager sessionManager, Context context, SharedPr
     }
 
     private boolean isUploadServiceConnected;
-    private ServiceConnection uploadServiceConnection = new ServiceConnection() {
+    public ServiceConnection uploadServiceConnection = new ServiceConnection() {
         @Override
         public void onServiceConnected(ComponentName componentName, IBinder binder) {
             uploadService = (UploadService) ((HandlerService.HandlerServiceLocalBinder) binder).getService();
@@ -61,14 +61,15 @@ public void onServiceConnected(ComponentName componentName, IBinder binder) {
         @Override
         public void onServiceDisconnected(ComponentName componentName) {
             // this should never happen
+            isUploadServiceConnected = false;
             Timber.e(new RuntimeException("UploadService died but the rest of the process did not!"));
         }
     };
 
     /**
      * Prepares the upload service.
      */
-    public void prepareService() {
+    void prepareService() {
         Intent uploadServiceIntent = new Intent(context, UploadService.class);
         uploadServiceIntent.setAction(UploadService.ACTION_START_SERVICE);
         context.startService(uploadServiceIntent);
@@ -78,7 +79,7 @@ public void prepareService() {
     /**
      * Disconnects the upload service.
      */
-    public void cleanup() {
+    void cleanup() {
         if (isUploadServiceConnected) {
             context.unbindService(uploadServiceConnection);
         }
@@ -89,7 +90,7 @@ public void cleanup() {
      *
      * @param contribution the contribution object
      */
-    public void startUpload(Contribution contribution) {
+    void startUpload(Contribution contribution) {
         startUpload(contribution, c -> {});
     }
 
@@ -100,7 +101,7 @@ public void startUpload(Contribution contribution) {
      * @param onComplete   the progress tracker
      */
     @SuppressLint("StaticFieldLeak")
-    public void startUpload(final Contribution contribution, final ContributionUploadProgress onComplete) {
+    private void startUpload(final Contribution contribution, final ContributionUploadProgress onComplete) {
         //Set creator, desc, and license
         if (TextUtils.isEmpty(contribution.getCreator())) {
             Account currentAccount = sessionManager.getCurrentAccount();