Fix upload failure modes in UploadService and UploadWorker

WordPress/src/main/java/org/wordpress/android/AppInitializer.kt

@@ -356,9 +356,6 @@ class AppInitializer @Inject constructor(
         AppCompatDelegate.setCompatVectorFromResourcesEnabled(true)
         AppThemeUtils.setAppTheme(application)
 
-        // verify media is sanitized
-        sanitizeMediaUploadStateForSite()
-
         // remove expired lists
         dispatcher.dispatch(ListActionBuilder.newRemoveExpiredListsAction(RemoveExpiredListsPayload()))
 

WordPress/src/main/java/org/wordpress/android/ui/uploads/PostUploadHandler.java

@@ -662,6 +662,15 @@ public void handleAutoSavePostIfNotDraftResult(@NonNull AutoSavePostIfNotDraftRe
     public void onPostUploaded(OnPostUploaded event) {
         // check if the event is related to the PostModel that is being uploaded by PostUploadHandler
         if (!isPostUploading(event.post)) {
+            AppLog.w(T.POSTS, String.format(
+                    "PostUploadHandler > onPostUploaded for untracked post"
+                            + " (postId=%s, currentUploadingPostId=%s%s)",
+                    event.post != null ? event.post.getId() : "null",
+                    sCurrentUploadingPost != null
+                            ? sCurrentUploadingPost.getId() : "null",
+                    event.isError()
+                            ? ", error=" + event.error.type + ": " + event.error.message
+                            : ""));
             return;
         }
         SiteModel site = mSiteStore.getSiteByLocalId(event.post.getLocalSiteId());

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java

@@ -83,6 +83,8 @@ public class UploadService extends Service {
     // for media that the user actively cancelled uploads for
     private static HashSet<String> mUserDeletedMediaItemIds = new HashSet<>();
 
+    // tracks media IDs that have already been recovered to prevent recovery loops
+    private static final Set<Integer> RECOVERED_MEDIA_IDS = new HashSet<>();
 
     @Inject Dispatcher mDispatcher;
     @Inject MediaStore mMediaStore;
@@ -99,19 +101,80 @@ public void onCreate() {
         AppLog.i(T.MAIN, "UploadService > Created");
         mDispatcher.register(this);
         sInstance = this;
-        // TODO: Recover any posts/media uploads that were interrupted by the service being stopped
+        mMediaUploadHandler = new MediaUploadHandler();
+        mPostUploadNotifier = new PostUploadNotifier(getApplicationContext(), this, mSystemNotificationsTracker);
+        mPostUploadHandler = new PostUploadHandler(mPostUploadNotifier);
 
-        if (mMediaUploadHandler == null) {
-            mMediaUploadHandler = new MediaUploadHandler();
+        recoverInterruptedMediaUploads();
+    }
+
+    /**
+     * Recovers media uploads that were interrupted when the service was killed.
+     * Re-queues QUEUED and UPLOADING media, and retries FAILED media that is
+     * bound to a post.
+     */
+    private void recoverInterruptedMediaUploads() {
+        List<MediaModel> recoveredMedia = new ArrayList<>();
+        for (SiteModel site : mSiteStore.getSites()) {
+            recoverMediaForSite(site, recoveredMedia);
         }
 
-        if (mPostUploadNotifier == null) {
-            mPostUploadNotifier = new PostUploadNotifier(getApplicationContext(), this, mSystemNotificationsTracker);
+        if (recoveredMedia.isEmpty()) {
+            return;
         }
 
-        if (mPostUploadHandler == null) {
-            mPostUploadHandler = new PostUploadHandler(mPostUploadNotifier);
+        AppLog.i(T.MAIN, "UploadService > Recovering "
+                + recoveredMedia.size() + " interrupted media uploads");
+        registerPostModelsForMedia(recoveredMedia, false);
+        for (MediaModel media : recoveredMedia) {
+            mMediaUploadHandler.upload(media);
         }
+        mPostUploadNotifier
+                .addMediaInfoToForegroundNotification(recoveredMedia);
+    }
+
+    private void recoverMediaForSite(
+            @NonNull SiteModel site,
+            @NonNull List<MediaModel> out
+    ) {
+        collectMedia(
+                mMediaStore.getSiteMediaWithState(site, MediaUploadState.QUEUED),
+                out, false
+        );
+        collectMedia(
+                mMediaStore.getSiteMediaWithState(site, MediaUploadState.UPLOADING),
+                out, true
+        );
+
+        for (MediaModel media : mMediaStore.getSiteMediaWithState(site, MediaUploadState.FAILED)) {
+            if (!RECOVERED_MEDIA_IDS.add(media.getId())) {
+                continue;
+            }
+            if (media.getLocalPostId() > 0) {
+                resetToQueued(media);
+                out.add(media);
+            }
+        }
+    }
+
+    private void collectMedia(
+            @NonNull List<MediaModel> source,
+            @NonNull List<MediaModel> out,
+            boolean resetState
+    ) {
+        for (MediaModel media : source) {
+            if (RECOVERED_MEDIA_IDS.add(media.getId())) {
+                if (resetState) {
+                    resetToQueued(media);
+                }
+                out.add(media);
+            }
+        }
+    }
+
+    private void resetToQueued(@NonNull MediaModel media) {
+        media.setUploadState(MediaUploadState.QUEUED.name());
+        mDispatcher.dispatch(MediaActionBuilder.newUpdateMediaAction(media));
     }
 
     @Override
@@ -179,32 +242,6 @@ public void onTimeout(int startId) {
     }
 
     private void unpackMediaIntent(@NonNull Intent intent) {
-        // TODO right now, in the case we had pending uploads and the app/service was restarted,
-        // we don't really have a way to tell which media was supposed to be added to which post,
-        // unless we open each draft post from the PostStore and try to see if there was any locally added media to try
-        // and match their IDs.
-        // So let's hold on a bit on this functionality, the service won't be recovering any
-        // pending / missing / cancelled / interrupted uploads for now
-
-// // add local queued media from store
-// List<MediaModel> localMedia = mMediaStore.getLocalSiteMedia(site);
-// if (localMedia != null && !localMedia.isEmpty()) {
-// // uploading is updated to queued, queued media added to the queue, failed media added to completed list
-// for (MediaModel mediaItem : localMedia) {
-//
-// if (MediaUploadState.UPLOADING.name().equals(mediaItem.getUploadState())) {
-// mediaItem.setUploadState(MediaUploadState.QUEUED.name());
-// mDispatcher.dispatch(MediaActionBuilder.newUpdateMediaAction(mediaItem));
-// }
-//
-// if (MediaUploadState.QUEUED.name().equals(mediaItem.getUploadState())) {
-// addUniqueMediaToQueue(mediaItem);
-// } else if (MediaUploadState.FAILED.name().equals(mediaItem.getUploadState())) {
-// getCompletedItems().add(mediaItem);
-// }
-// }
-// }
-
         // add new media
         @SuppressWarnings("unchecked")
         List<MediaModel> mediaList = (List<MediaModel>) intent.getSerializableExtra(KEY_MEDIA_LIST);

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java

@@ -459,6 +459,8 @@ public static void publishPost(Activity activity, final PostModel post, SiteMode
             if (onPublishingCallback != null) {
                 onPublishingCallback.onPublishing(isFirstTimePublish);
             }
+        } else {
+            ToastUtils.showToast(activity, R.string.no_network_message, ToastUtils.Duration.SHORT);
         }
         PostUtils.trackSavePostAnalytics(post, site);
     }

WordPress/src/main/java/org/wordpress/android/util/UploadWorker.kt

@@ -1,6 +1,7 @@
 package org.wordpress.android.util
 
 import android.content.Context
+import androidx.work.BackoffPolicy
 import androidx.work.Constraints
 import androidx.work.ExistingPeriodicWorkPolicy
 import androidx.work.ExistingWorkPolicy
@@ -21,6 +22,7 @@ import org.wordpress.android.fluxc.model.SiteModel
 import org.wordpress.android.fluxc.store.SiteStore
 import org.wordpress.android.ui.uploads.UploadStarter
 import java.util.concurrent.TimeUnit.HOURS
+import java.util.concurrent.TimeUnit.MINUTES
 
 class UploadWorker(
     appContext: Context,
@@ -32,17 +34,30 @@ class UploadWorker(
         private const val UPLOAD_FROM_ALL_SITES = -1
     }
 
+    @Suppress("TooGenericExceptionCaught")
     override fun doWork(): Result {
         AppLog.i(AppLog.T.MAIN, "UploadWorker started")
-        runBlocking {
-            val job = when (val localSiteId = inputData.getInt(WordPress.LOCAL_SITE_ID, UPLOAD_FROM_ALL_SITES)) {
-                UPLOAD_FROM_ALL_SITES -> uploadStarter.queueUploadFromAllSites()
-                else -> siteStore.getSiteByLocalId(localSiteId)?.let { uploadStarter.queueUploadFromSite(it) }
+        return try {
+            runBlocking {
+                val job = when (
+                    val localSiteId = inputData.getInt(
+                        WordPress.LOCAL_SITE_ID,
+                        UPLOAD_FROM_ALL_SITES
+                    )
+                ) {
+                    UPLOAD_FROM_ALL_SITES -> uploadStarter.queueUploadFromAllSites()
+                    else -> siteStore.getSiteByLocalId(localSiteId)?.let {
+                        uploadStarter.queueUploadFromSite(it)
+                    }
+                }
+                job?.join()
             }
-            job?.join()
+            AppLog.i(AppLog.T.MAIN, "UploadWorker finished")
+            Result.success()
+        } catch (e: Exception) {
+            AppLog.e(AppLog.T.MAIN, "UploadWorker failed", e)
+            Result.retry()
         }
-        AppLog.i(AppLog.T.MAIN, "UploadWorker finished")
-        return Result.success()
     }
 
     class Factory(
@@ -63,19 +78,20 @@ class UploadWorker(
     }
 }
 
-private fun getUploadConstraints(): Constraints {
-    return Constraints.Builder()
-        .setRequiredNetworkType(NetworkType.NOT_ROAMING)
-        .build()
-}
+private const val BACKOFF_DELAY_MINUTES = 10L
+
+private fun getUploadConstraints() = Constraints.Builder()
+    .setRequiredNetworkType(NetworkType.CONNECTED)
+    .build()
 
 fun enqueueUploadWorkRequestForSite(site: SiteModel): Pair<WorkRequest, Operation> {
     val request = OneTimeWorkRequestBuilder<UploadWorker>()
         .setConstraints(getUploadConstraints())
+        .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, BACKOFF_DELAY_MINUTES, MINUTES)
         .setInputData(workDataOf(WordPress.LOCAL_SITE_ID to site.id))
         .build()
     val operation = WorkManager.getInstance(WordPress.getContext()).enqueueUniqueWork(
-        "auto-upload-" + site.id,
+        "auto-upload-${site.id}",
         ExistingWorkPolicy.KEEP, request
     )
     return Pair(request, operation)
@@ -84,6 +100,7 @@ fun enqueueUploadWorkRequestForSite(site: SiteModel): Pair<WorkRequest, Operatio
 fun enqueuePeriodicUploadWorkRequestForAllSites(): Pair<WorkRequest, Operation> {
     val request = PeriodicWorkRequestBuilder<UploadWorker>(8, HOURS, 6, HOURS)
         .setConstraints(getUploadConstraints())
+        .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, BACKOFF_DELAY_MINUTES, MINUTES)
         .build()
     val operation = WorkManager.getInstance(WordPress.getContext()).enqueueUniquePeriodicWork(
         "periodic auto-upload",