Autosave or update draft

WordPress/src/main/java/org/wordpress/android/ui/uploads/AutoSavePostIfNotDraftUseCase.kt

@@ -0,0 +1,155 @@
+package org.wordpress.android.ui.uploads
+
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.GlobalScope
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.suspendCancellableCoroutine
+import org.greenrobot.eventbus.Subscribe
+import org.greenrobot.eventbus.ThreadMode.BACKGROUND
+import org.greenrobot.eventbus.ThreadMode.MAIN
+import org.wordpress.android.fluxc.Dispatcher
+import org.wordpress.android.fluxc.generated.PostActionBuilder
+import org.wordpress.android.fluxc.model.CauseOfOnPostChanged.RemoteAutoSavePost
+import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId
+import org.wordpress.android.fluxc.model.PostModel
+import org.wordpress.android.fluxc.store.PostStore
+import org.wordpress.android.fluxc.store.PostStore.OnPostChanged
+import org.wordpress.android.fluxc.store.PostStore.OnPostStatusFetched
+import org.wordpress.android.fluxc.store.PostStore.PostError
+import org.wordpress.android.fluxc.store.PostStore.RemotePostPayload
+import org.wordpress.android.modules.BG_THREAD
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.FetchPostStatusFailed
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostAutoSaveFailed
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostAutoSaved
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostIsDraftInRemote
+import javax.inject.Inject
+import javax.inject.Named
+import kotlin.coroutines.Continuation
+import kotlin.coroutines.resume
+
+private const val DRAFT_POST_STATUS = "draft"
+
+interface OnAutoSavePostIfNotDraftCallback {
+    fun handleAutoSavePostIfNotDraftResult(result: AutoSavePostIfNotDraftResult)
+}
+
+sealed class AutoSavePostIfNotDraftResult(open val post: PostModel) {
+    // Initial fetch post status request failed
+    data class FetchPostStatusFailed(override val post: PostModel, val error: PostError) :
+            AutoSavePostIfNotDraftResult(post)
+
+    // Post status is `DRAFT` in remote which means we'll want to update the draft directly
+    data class PostIsDraftInRemote(override val post: PostModel) : AutoSavePostIfNotDraftResult(post)
+
+    // Post status is not `DRAFT` in remote and the post was auto-saved successfully
+    data class PostAutoSaved(override val post: PostModel) : AutoSavePostIfNotDraftResult(post)
+
+    // Post status is not `DRAFT` in remote but the post auto-save failed
+    data class PostAutoSaveFailed(override val post: PostModel, val error: PostError) :
+            AutoSavePostIfNotDraftResult(post)
+}
+
+/**
+ * This is a use case that auto-saves a post if it's not a DRAFT in remote and returns various results depending
+ * on the remote post status and whether network requests were successful.
+ *
+ * The reason auto-save is tied to post status is that the `/autosave` REST endpoint will override the changes to
+ * a `DRAFT` directly rather than auto-saving it. While doing so, it'll also disable comments for the post due to
+ * a bug. Both the fact that the endpoint does something we are not expecting and the bug that results from it is
+ * avoided by only calling the `/autosave` endpoint for posts that are not in DRAFT status. We update DRAFTs directly
+ * just as the endpoint would have, but that makes the logic more clear on the client side while avoiding the
+ * comments getting disabled bug.
+ *
+ * See p3hLNG-15Z-p2 for more info.
+ */
+class AutoSavePostIfNotDraftUseCase @Inject constructor(
+    private val dispatcher: Dispatcher,
+    private val postStore: PostStore,
+    @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher
+) {
+    private val postStatusContinuations = HashMap<LocalId, Continuation<OnPostStatusFetched>>()
+    private val autoSaveContinuations = HashMap<LocalId, Continuation<OnPostChanged>>()
+
+    init {
+        dispatcher.register(this)
+    }
+
+    fun autoSavePostOrUpdateDraft(
+        remotePostPayload: RemotePostPayload,
+        callback: OnAutoSavePostIfNotDraftCallback
+    ) {
+        val localPostId = LocalId(remotePostPayload.post.id)
+        if (remotePostPayload.post.isLocalDraft) {
+            throw IllegalArgumentException("Local drafts should not be auto-saved")
+        }
+        if (postStatusContinuations.containsKey(localPostId) ||
+                autoSaveContinuations.containsKey(localPostId)) {
+            throw IllegalArgumentException(
+                    "This post is already being processed. Make sure not to start an autoSave " +
+                            "or update draft action while another one is going on."
+            )
+        }
+        GlobalScope.launch(bgDispatcher) {
+            val onPostStatusFetched = fetchRemotePostStatus(remotePostPayload)
+            val result = when {
+                onPostStatusFetched.isError -> {
+                    FetchPostStatusFailed(
+                            post = remotePostPayload.post,
+                            error = onPostStatusFetched.error
+                    )
+                }
+                onPostStatusFetched.remotePostStatus == DRAFT_POST_STATUS -> {
+                    PostIsDraftInRemote(remotePostPayload.post)
+                }
+                else -> {
+                    autoSavePost(remotePostPayload)
+                }
+            }
+            callback.handleAutoSavePostIfNotDraftResult(result)
+        }
+    }
+
+    private suspend fun fetchRemotePostStatus(remotePostPayload: RemotePostPayload): OnPostStatusFetched {
+        val localPostId = LocalId(remotePostPayload.post.id)
+        return suspendCancellableCoroutine { cont ->
+            postStatusContinuations[localPostId] = cont
+            dispatcher.dispatch(PostActionBuilder.newFetchPostStatusAction(remotePostPayload))
+        }
+    }
+
+    private suspend fun autoSavePost(remotePostPayload: RemotePostPayload): AutoSavePostIfNotDraftResult {
+        val localPostId = LocalId(remotePostPayload.post.id)
+        val onPostChanged: OnPostChanged = suspendCancellableCoroutine { cont ->
+            autoSaveContinuations[localPostId] = cont
+            dispatcher.dispatch(PostActionBuilder.newRemoteAutoSavePostAction(remotePostPayload))
+        }
+        return if (onPostChanged.isError) {
+            PostAutoSaveFailed(remotePostPayload.post, onPostChanged.error)
+        } else {
+            val updatedPost = postStore.getPostByLocalPostId(localPostId.value)
+            PostAutoSaved(updatedPost)
+        }
+    }
+
+    @Subscribe(threadMode = BACKGROUND)
+    @Suppress("unused")
+    fun onPostStatusFetched(event: OnPostStatusFetched) {
+        val localPostId = LocalId(event.post.id)
+        postStatusContinuations[localPostId]?.let { continuation ->
+            continuation.resume(event)
+            postStatusContinuations.remove(localPostId)
+        }
+    }
+
+    @Subscribe(threadMode = MAIN, priority = 9)
+    @Suppress("unused")
+    fun onPostChanged(event: OnPostChanged) {
+        if (event.causeOfChange is RemoteAutoSavePost) {
+            val localPostId = LocalId((event.causeOfChange as RemoteAutoSavePost).localPostId)
+            autoSaveContinuations[localPostId]?.let { continuation ->
+                continuation.resume(event)
+                autoSaveContinuations.remove(localPostId)
+            }
+        }
+    }
+}

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

@@ -14,14 +14,13 @@
 import org.greenrobot.eventbus.EventBus;
 import org.greenrobot.eventbus.Subscribe;
 import org.greenrobot.eventbus.ThreadMode;
+import org.jetbrains.annotations.NotNull;
 import org.wordpress.android.R;
 import org.wordpress.android.WordPress;
 import org.wordpress.android.analytics.AnalyticsTracker.Stat;
 import org.wordpress.android.fluxc.Dispatcher;
 import org.wordpress.android.fluxc.generated.MediaActionBuilder;
 import org.wordpress.android.fluxc.generated.PostActionBuilder;
-import org.wordpress.android.fluxc.model.CauseOfOnPostChanged;
-import org.wordpress.android.fluxc.model.CauseOfOnPostChanged.RemoteAutoSavePost;
 import org.wordpress.android.fluxc.model.MediaModel;
 import org.wordpress.android.fluxc.model.MediaModel.MediaUploadState;
 import org.wordpress.android.fluxc.model.PostImmutableModel;
@@ -31,12 +30,15 @@
 import org.wordpress.android.fluxc.store.MediaStore;
 import org.wordpress.android.fluxc.store.MediaStore.UploadMediaPayload;
 import org.wordpress.android.fluxc.store.PostStore;
-import org.wordpress.android.fluxc.store.PostStore.OnPostChanged;
 import org.wordpress.android.fluxc.store.PostStore.OnPostUploaded;
 import org.wordpress.android.fluxc.store.PostStore.RemotePostPayload;
 import org.wordpress.android.fluxc.store.SiteStore;
 import org.wordpress.android.ui.posts.PostUtils;
 import org.wordpress.android.ui.prefs.AppPrefs;
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.FetchPostStatusFailed;
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostAutoSaveFailed;
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostAutoSaved;
+import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostIsDraftInRemote;
 import org.wordpress.android.ui.uploads.PostEvents.PostUploadStarted;
 import org.wordpress.android.ui.utils.UiHelpers;
 import org.wordpress.android.util.AppLog;
@@ -62,7 +64,7 @@
 
 import javax.inject.Inject;
 
-public class PostUploadHandler implements UploadHandler<PostModel> {
+public class PostUploadHandler implements UploadHandler<PostModel>, OnAutoSavePostIfNotDraftCallback {
     private static ArrayList<PostModel> sQueuedPostsList = new ArrayList<>();
     private static Set<Integer> sFirstPublishPosts = new HashSet<>();
     private static PostModel sCurrentUploadingPost = null;
@@ -79,6 +81,7 @@ public class PostUploadHandler implements UploadHandler<PostModel> {
     @Inject MediaStore mMediaStore;
     @Inject UiHelpers mUiHelpers;
     @Inject UploadActionUseCase mUploadActionUseCase;
+    @Inject AutoSavePostIfNotDraftUseCase mAutoSavePostIfNotDraftUseCase;
 
     PostUploadHandler(PostUploadNotifier postUploadNotifier) {
         ((WordPress) WordPress.getContext().getApplicationContext()).component().inject(this);
@@ -189,7 +192,7 @@ private void finishUpload() {
     }
 
     private enum UploadPostTaskResult {
-        PUSH_POST_DISPATCHED, ERROR, NOTHING_TO_UPLOAD
+        PUSH_POST_DISPATCHED, ERROR, NOTHING_TO_UPLOAD, AUTO_SAVE_OR_UPDATE_DRAFT
     }
 
     private class UploadPostTask extends AsyncTask<PostModel, Boolean, UploadPostTaskResult> {
@@ -286,8 +289,8 @@ protected UploadPostTaskResult doInBackground(PostModel... posts) {
                     break;
                 case REMOTE_AUTO_SAVE:
                     AppLog.d(T.POSTS, "PostUploadHandler - REMOTE_AUTO_SAVE. Post: " + mPost.getTitle());
-                    mDispatcher.dispatch(PostActionBuilder.newRemoteAutoSavePostAction(payload));
-                    break;
+                    mAutoSavePostIfNotDraftUseCase.autoSavePostOrUpdateDraft(payload, PostUploadHandler.this);
+                    return UploadPostTaskResult.AUTO_SAVE_OR_UPDATE_DRAFT;
                 case DO_NOTHING:
                     AppLog.d(T.POSTS, "PostUploadHandler - DO_NOTHING. Post: " + mPost.getTitle());
                     // A single post might be enqueued twice for upload. It might cause some side-effects when the
@@ -604,6 +607,34 @@ private String uploadImageFile(MediaFile mediaFile, SiteModel site) {
         }
     }
 
+    @Override
+    public void handleAutoSavePostIfNotDraftResult(@NotNull AutoSavePostIfNotDraftResult result) {
+        PostModel post = result.getPost();
+        if (result instanceof FetchPostStatusFailed
+            || result instanceof PostAutoSaveFailed
+            || result instanceof PostAutoSaved) {
+            /*
+             * If we fail to check the status of the post or auto-save fails, we deliberately don't show an error
+             * notification since it's not a user initiated action. We'll retry the action later on.
+             */
+            mPostUploadNotifier.incrementUploadedPostCountFromForegroundNotification(post);
+            finishUpload();
+        } else if (result instanceof PostIsDraftInRemote) {
+            /*
+             * If the post is a draft in remote, we'll update it directly instead of auto-saving it. Please see
+             * documentation of `AutoSavePostIfNotDraftUseCase` for details.
+             *
+             * We opted not to restore the current status after the post is uploaded to avoid its complexity and to
+             * replicate `UPLOAD_AS_DRAFT`. We may change this in the future.
+             */
+            post.setStatus(PostStatus.DRAFT.toString());
+            SiteModel site = mSiteStore.getSiteByLocalId(post.getLocalSiteId());
+            mDispatcher.dispatch(PostActionBuilder.newPushPostAction(new RemotePostPayload(post, site)));
+        } else {
+            throw new IllegalStateException("All AutoSavePostIfNotDraftResult types must be handled");
+        }
+    }
+
     /**
      * Has priority 9 on OnPostUploaded events, which ensures that PostUploadHandler is the first to receive
      * and process OnPostUploaded events, before they trickle down to other subscribers.
@@ -660,16 +691,4 @@ public void onPostUploaded(OnPostUploaded event) {
 
         finishUpload();
     }
-
-    @SuppressWarnings("unused")
-    @Subscribe(threadMode = ThreadMode.MAIN, priority = 9)
-    public void onPostChanged(OnPostChanged event) {
-        if (event.causeOfChange instanceof CauseOfOnPostChanged.RemoteAutoSavePost) {
-            int postLocalId = ((RemoteAutoSavePost) event.causeOfChange).getLocalPostId();
-            PostModel post = mPostStore.getPostByLocalPostId(postLocalId);
-            SiteModel site = mSiteStore.getSiteByLocalId(post.getLocalSiteId());
-            mPostUploadNotifier.incrementUploadedPostCountFromForegroundNotification(post);
-            finishUpload();
-        }
-    }
 }