From 92331cfc9a0f6553a0e16885a6030655bcb30681 Mon Sep 17 00:00:00 2001 From: Renan Ferrari Date: Thu, 4 Mar 2021 15:56:38 -0300 Subject: [PATCH 1/4] Pass SiteModel parameter down to the Story block searching method --- .../ui/media/services/MediaUploadReadyListener.java | 6 +++++- .../android/ui/stories/SaveStoryGutenbergBlockUseCase.kt | 8 +++++++- .../android/ui/uploads/MediaUploadReadyProcessor.java | 6 ++++-- .../org/wordpress/android/ui/uploads/UploadService.java | 2 +- .../ui/stories/SaveStoryGutenbergBlockUseCaseTest.kt | 6 +++++- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/media/services/MediaUploadReadyListener.java b/WordPress/src/main/java/org/wordpress/android/ui/media/services/MediaUploadReadyListener.java index a17ab55ba944..b8ebe1d28fc1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/media/services/MediaUploadReadyListener.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/media/services/MediaUploadReadyListener.java @@ -3,6 +3,7 @@ import androidx.annotation.Nullable; import org.wordpress.android.fluxc.model.PostModel; +import org.wordpress.android.fluxc.model.SiteModel; import org.wordpress.android.util.helpers.MediaFile; /** @@ -10,7 +11,10 @@ * and mark media failed if could not be uploaded */ public interface MediaUploadReadyListener { + // TODO: We're passing a SiteModel parameter here in order to debug a crash on SaveStoryGutenbergBlockUseCase. + // Once that's done, the parameter should be replaced with a site url String, like it was before. + // See: https://git.io/JqfhK PostModel replaceMediaFileWithUrlInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile, - String siteUrl); + @Nullable SiteModel site); PostModel markMediaUploadFailedInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile); } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt index 21d264213e3a..e14886d3add9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt @@ -93,6 +93,7 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( fun findAllStoryBlocksInPostAndPerformOnEachMediaFilesJson( postModel: PostModel, + siteModel: SiteModel?, listener: DoWithMediaFilesListener ) { var content = postModel.content @@ -124,7 +125,11 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( postModel.setContent(content) } - fun replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel: PostModel, mediaFile: MediaFile) { + fun replaceLocalMediaIdsWithRemoteMediaIdsInPost( + postModel: PostModel, + siteModel: SiteModel?, + mediaFile: MediaFile + ) { if (TextUtils.isEmpty(mediaFile.mediaId)) { // if for any reason we couldn't obtain a remote mediaId, it's not worth spending time // looking to replace anything in the Post. Skip processing for later in error handling. @@ -133,6 +138,7 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( val gson = Gson() findAllStoryBlocksInPostAndPerformOnEachMediaFilesJson( postModel, + siteModel, object : DoWithMediaFilesListener { override fun doWithMediaFilesJson(content: String, mediaFilesJsonString: String): String { var processedContent = content diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/MediaUploadReadyProcessor.java b/WordPress/src/main/java/org/wordpress/android/ui/uploads/MediaUploadReadyProcessor.java index 7eb3180df08d..86a4da36d608 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/MediaUploadReadyProcessor.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/MediaUploadReadyProcessor.java @@ -5,6 +5,7 @@ import org.wordpress.android.WordPress; import org.wordpress.android.editor.AztecEditorFragment; import org.wordpress.android.fluxc.model.PostModel; +import org.wordpress.android.fluxc.model.SiteModel; import org.wordpress.android.ui.media.services.MediaUploadReadyListener; import org.wordpress.android.ui.posts.PostUtils; import org.wordpress.android.ui.prefs.AppPrefs; @@ -23,15 +24,16 @@ public class MediaUploadReadyProcessor implements MediaUploadReadyListener { @Override public PostModel replaceMediaFileWithUrlInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile, - String siteUrl) { + @Nullable SiteModel site) { if (post != null) { boolean showAztecEditor = AppPrefs.isAztecEditorEnabled(); boolean showGutenbergEditor = AppPrefs.isGutenbergEditorEnabled(); if (PostUtils.contentContainsWPStoryGutenbergBlocks(post.getContent())) { mSaveStoryGutenbergBlockUseCase - .replaceLocalMediaIdsWithRemoteMediaIdsInPost(post, mediaFile); + .replaceLocalMediaIdsWithRemoteMediaIdsInPost(post, site, mediaFile); } else if (showGutenbergEditor && PostUtils.contentContainsGutenbergBlocks(post.getContent())) { + String siteUrl = site != null ? site.getUrl() : ""; post.setContent( PostUtils.replaceMediaFileWithUrlInGutenbergPost(post.getContent(), localMediaId, mediaFile, siteUrl)); diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java index 41e2efaa3ad8..65a05ca0470e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadService.java @@ -624,7 +624,7 @@ private static synchronized PostModel updatePostWithMediaUrl(PostModel post, Med // actually replace the media ID with the media uri processor.replaceMediaFileWithUrlInPost(post, String.valueOf(media.getId()), - FluxCUtils.mediaFileFromMediaModel(media), site.getUrl()); + FluxCUtils.mediaFileFromMediaModel(media), site); // we changed the post, so let’s mark this down if (!post.isLocalDraft()) { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCaseTest.kt index db1eebfd36fe..ab40d3c42071 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCaseTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCaseTest.kt @@ -17,6 +17,7 @@ import org.mockito.junit.MockitoJUnitRunner import org.wordpress.android.BaseUnitTest import org.wordpress.android.TEST_DISPATCHER import org.wordpress.android.fluxc.model.PostModel +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.ui.posts.EditPostRepository import org.wordpress.android.ui.posts.mediauploadcompletionprocessors.TestContent @@ -140,10 +141,12 @@ class SaveStoryGutenbergBlockUseCaseTest : BaseUnitTest() { val mediaFile = getMediaFile(1) val postModel = PostModel() postModel.setContent(BLOCK_WITH_NON_EMPTY_MEDIA_FILES) + val siteModel = SiteModel() // When saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost( postModel, + siteModel, mediaFile ) @@ -178,9 +181,10 @@ class SaveStoryGutenbergBlockUseCaseTest : BaseUnitTest() { whenever(mediaFile.fileURL).thenReturn(TestContent.remoteImageUrl) val postModel = PostModel() postModel.setContent(TestContent.storyBlockWithLocalIdsAndUrls) + val siteModel = SiteModel() // act - saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel, mediaFile) + saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel, siteModel, mediaFile) // assert Assertions.assertThat(postModel.content).isEqualTo(TestContent.storyBlockWithFirstRemoteIdsAndUrlsReplaced) From 35cc0371941372de51b7db9d917641ab7b099059 Mon Sep 17 00:00:00 2001 From: Renan Ferrari Date: Thu, 4 Mar 2021 15:57:30 -0300 Subject: [PATCH 2/4] Add content logging to the Story block searching method --- .../ui/stories/SaveStoryGutenbergBlockUseCase.kt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt index e14886d3add9..dbb55fafafb7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt @@ -7,6 +7,7 @@ import com.wordpress.stories.compose.story.StoryFrameItem import com.wordpress.stories.compose.story.StoryIndex import org.wordpress.android.fluxc.model.PostModel import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.post.PostStatus.PRIVATE import org.wordpress.android.ui.posts.EditPostRepository import org.wordpress.android.ui.stories.prefs.StoriesPrefs import org.wordpress.android.ui.stories.prefs.StoriesPrefs.TempId @@ -116,8 +117,11 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( content = listener.doWithMediaFilesJson(content, jsonString) storyBlockStartIndex += HEADING_START.length } catch (exception: StringIndexOutOfBoundsException) { + AppLog.e(EDITOR, "Error while parsing Story blocks: ${exception.message}") + if (shouldLogContent(siteModel, postModel)) { + AppLog.e(EDITOR, "HTML content of the post before the crash: ${postModel.content}") + } crashLogging.reportException(exception, EDITOR.toString()) - AppLog.e(EDITOR, exception.message) } } } @@ -125,6 +129,13 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( postModel.setContent(content) } + // See: https://git.io/JqfhK + private fun shouldLogContent(siteModel: SiteModel?, postModel: PostModel) = siteModel != null + && siteModel.isWPCom + && !siteModel.isPrivate + && postModel.password.isEmpty() + && postModel.status != PRIVATE.toString() + fun replaceLocalMediaIdsWithRemoteMediaIdsInPost( postModel: PostModel, siteModel: SiteModel?, From 29a131cb32d198844c93a4ab55d8c362350b9930 Mon Sep 17 00:00:00 2001 From: Renan Ferrari Date: Thu, 4 Mar 2021 17:27:33 -0300 Subject: [PATCH 3/4] Fix lint --- .../ui/stories/SaveStoryGutenbergBlockUseCase.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt index dbb55fafafb7..d8f9b0e4de8c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt @@ -130,11 +130,11 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( } // See: https://git.io/JqfhK - private fun shouldLogContent(siteModel: SiteModel?, postModel: PostModel) = siteModel != null - && siteModel.isWPCom - && !siteModel.isPrivate - && postModel.password.isEmpty() - && postModel.status != PRIVATE.toString() + private fun shouldLogContent(siteModel: SiteModel?, postModel: PostModel) = siteModel != null && + siteModel.isWPCom && + !siteModel.isPrivate && + postModel.password.isEmpty() && + postModel.status != PRIVATE.toString() fun replaceLocalMediaIdsWithRemoteMediaIdsInPost( postModel: PostModel, From d6d9f96348bc210654e66c22a2ab33310862ab8a Mon Sep 17 00:00:00 2001 From: Renan Ferrari Date: Thu, 4 Mar 2021 17:58:50 -0300 Subject: [PATCH 4/4] Extract exception logging logic --- .../ui/stories/SaveStoryGutenbergBlockUseCase.kt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt index d8f9b0e4de8c..dfa8c34ac584 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stories/SaveStoryGutenbergBlockUseCase.kt @@ -117,11 +117,7 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( content = listener.doWithMediaFilesJson(content, jsonString) storyBlockStartIndex += HEADING_START.length } catch (exception: StringIndexOutOfBoundsException) { - AppLog.e(EDITOR, "Error while parsing Story blocks: ${exception.message}") - if (shouldLogContent(siteModel, postModel)) { - AppLog.e(EDITOR, "HTML content of the post before the crash: ${postModel.content}") - } - crashLogging.reportException(exception, EDITOR.toString()) + logException(exception, postModel, siteModel) } } } @@ -129,6 +125,14 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor( postModel.setContent(content) } + private fun logException(exception: Throwable, postModel: PostModel, siteModel: SiteModel?) { + AppLog.e(EDITOR, "Error while parsing Story blocks: ${exception.message}") + if (shouldLogContent(siteModel, postModel)) { + AppLog.e(EDITOR, "HTML content of the post before the crash: ${postModel.content}") + } + crashLogging.reportException(exception, EDITOR.toString()) + } + // See: https://git.io/JqfhK private fun shouldLogContent(siteModel: SiteModel?, postModel: PostModel) = siteModel != null && siteModel.isWPCom &&