From 860a5a723842fe3d178aec639735b527538a1838 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 14 Nov 2019 15:35:01 +0100 Subject: [PATCH 1/4] Move purgeMediaToPostAssociationIfNotInPostAnymore to UseCase --- .../android/ui/posts/EditPostActivity.java | 44 +---- .../android/ui/posts/PostUtilsWrapper.kt | 3 + .../AztecEditorFragmentStaticWrapper.kt | 19 ++ .../CleanUpMediaToPostAssociationUseCase.kt | 63 +++++++ .../ui/posts/editor/media/EditorMedia.kt | 8 + .../android/ui/prefs/AppPrefsWrapper.kt | 1 + .../org/wordpress/android/CoroutinesUtils.kt | 1 + ...leanUpMediaToPostAssociationUseCaseTest.kt | 178 ++++++++++++++++++ .../ui/posts/editor/media/EditorMediaTest.kt | 4 +- 9 files changed, 277 insertions(+), 44 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/posts/editor/AztecEditorFragmentStaticWrapper.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java index e618dbe40d49..741a9287d77d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java @@ -92,7 +92,6 @@ import org.wordpress.android.fluxc.store.QuickStartStore.QuickStartTask; import org.wordpress.android.fluxc.store.SiteStore; import org.wordpress.android.fluxc.store.UploadStore; -import org.wordpress.android.fluxc.store.UploadStore.ClearMediaPayload; import org.wordpress.android.fluxc.tools.FluxCImageLoader; import org.wordpress.android.ui.ActivityId; import org.wordpress.android.ui.ActivityLauncher; @@ -618,48 +617,7 @@ private void initializePostObject() { EventBus.getDefault().postSticky(new PostEvents.PostOpenedInEditor(mEditPostRepository.getLocalSiteId(), mEditPostRepository.getId())); - // run this purge in the background to not delay Editor initialization - new Thread(this::purgeMediaToPostAssociationsIfNotInPostAnymore).start(); - } - } - - private void purgeMediaToPostAssociationsIfNotInPostAnymore() { - boolean useAztec = AppPrefs.isAztecEditorEnabled(); - boolean useGutenberg = AppPrefs.isGutenbergEditorEnabled(); - - ArrayList allMedia = new ArrayList<>(); - allMedia.addAll(mUploadStore.getFailedMediaForPost(mEditPostRepository.getPost())); - allMedia.addAll(mUploadStore.getCompletedMediaForPost(mEditPostRepository.getPost())); - allMedia.addAll(mUploadStore.getUploadingMediaForPost(mEditPostRepository.getPost())); - - if (!allMedia.isEmpty()) { - HashSet mediaToDeleteAssociationFor = new HashSet<>(); - for (MediaModel media : allMedia) { - if (useAztec) { - if (!AztecEditorFragment.isMediaInPostBody(this, - mEditPostRepository.getContent(), String.valueOf(media.getId()))) { - // don't delete featured image uploads - if (!media.getMarkedLocallyAsFeatured()) { - mediaToDeleteAssociationFor.add(media); - } - } - } else if (useGutenberg) { - if (!PostUtils.isMediaInGutenbergPostBody( - mEditPostRepository.getContent(), String.valueOf(media.getId()))) { - // don't delete featured image uploads - if (!media.getMarkedLocallyAsFeatured()) { - mediaToDeleteAssociationFor.add(media); - } - } - } - } - - if (!mediaToDeleteAssociationFor.isEmpty()) { - // also remove the association of Media-to-Post for this post - ClearMediaPayload clearMediaPayload = - new ClearMediaPayload(mEditPostRepository.getPost(), mediaToDeleteAssociationFor); - mDispatcher.dispatch(UploadActionBuilder.newClearMediaForPostAction(clearMediaPayload)); - } + mEditorMedia.purgeMediaToPostAssociationsIfNotInPostAnymoreAsync(); } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt index 074d41098abd..63dad8a46fcb 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt @@ -25,4 +25,7 @@ class PostUtilsWrapper @Inject constructor() { fun postHasEdits(oldPost: PostImmutableModel?, newPost: PostImmutableModel) = PostUtils.postHasEdits(oldPost, newPost) + + fun isMediaInGutenbergPostBody(postContent: String, localMediaId: String) = + PostUtils.isMediaInGutenbergPostBody(postContent, localMediaId) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/AztecEditorFragmentStaticWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/AztecEditorFragmentStaticWrapper.kt new file mode 100644 index 000000000000..bd265f1fee42 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/AztecEditorFragmentStaticWrapper.kt @@ -0,0 +1,19 @@ +package org.wordpress.android.ui.posts.editor + +import android.content.Context +import dagger.Reusable +import org.wordpress.android.editor.AztecEditorFragment +import javax.inject.Inject + +/** + * Injectable wrapper around AztecEditorFragment. + * + * AppPrefs interface contains some static methods, which make the client code difficult to test/mock. Main purpose of + * this wrapper is to make testing of these static methods easier. + * + */ +@Reusable +class AztecEditorFragmentStaticWrapper @Inject constructor(private val appContext: Context) { + fun isMediaInPostBody(postContent: String, localMediaId: String) = + AztecEditorFragment.isMediaInPostBody(appContext, postContent, localMediaId) +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt new file mode 100644 index 000000000000..881e6b9d7585 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt @@ -0,0 +1,63 @@ +package org.wordpress.android.ui.posts.editor.media + +import dagger.Reusable +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.Dispatcher +import org.wordpress.android.fluxc.generated.UploadActionBuilder +import org.wordpress.android.fluxc.model.PostImmutableModel +import org.wordpress.android.fluxc.store.UploadStore +import org.wordpress.android.fluxc.store.UploadStore.ClearMediaPayload +import org.wordpress.android.modules.BG_THREAD +import org.wordpress.android.ui.posts.PostUtilsWrapper +import org.wordpress.android.ui.posts.editor.AztecEditorFragmentStaticWrapper +import org.wordpress.android.ui.prefs.AppPrefsWrapper +import javax.inject.Inject +import javax.inject.Named + +@Reusable +class CleanUpMediaToPostAssociationUseCase @Inject constructor( + private val dispatcher: Dispatcher, + private val uploadStore: UploadStore, + private val appPrefsWrapper: AppPrefsWrapper, + private val aztecEditorWrapper: AztecEditorFragmentStaticWrapper, + private val postUtilsWrapper: PostUtilsWrapper, + @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher +) { + suspend fun purgeMediaToPostAssociationsIfNotInPostAnymore(post: PostImmutableModel) { + withContext(bgDispatcher) { + val useAztec = appPrefsWrapper.isAztecEditorEnabled + val useGutenberg = appPrefsWrapper.isGutenbergEditorEnabled() + + val mediaAssociatedWithPost = uploadStore.getFailedMediaForPost(post) + + uploadStore.getCompletedMediaForPost(post) + + uploadStore.getUploadingMediaForPost(post) + + mediaAssociatedWithPost + .filter { media -> + // Find media which is not in the post anymore + val isNotInAztec = useAztec && !aztecEditorWrapper.isMediaInPostBody( + post.content, + media.id.toString() + ) + val isNotInGutenberg = useGutenberg && !postUtilsWrapper.isMediaInGutenbergPostBody( + post.content, + media.id.toString() + ) + + isNotInAztec || isNotInGutenberg + } + .filter { media -> + // Featured images are not in post content, don't delete them + !media.markedLocallyAsFeatured + } + .toSet() + .let { mediaToDeleteAssociationFor -> + if(mediaToDeleteAssociationFor.isNotEmpty()) { + val clearMediaPayload = ClearMediaPayload(post, mediaToDeleteAssociationFor) + dispatcher.dispatch(UploadActionBuilder.newClearMediaForPostAction(clearMediaPayload)) + } + } + } + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/EditorMedia.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/EditorMedia.kt index 2adb19103f20..c6f6dff9a8ac 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/EditorMedia.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/EditorMedia.kt @@ -58,6 +58,7 @@ class EditorMedia @Inject constructor( private val addLocalMediaToPostUseCase: AddLocalMediaToPostUseCase, private val addExistingMediaToPostUseCase: AddExistingMediaToPostUseCase, private val retryFailedMediaUploadUseCase: RetryFailedMediaUploadUseCase, + private val cleanUpMediaToPostAssociationUseCase: CleanUpMediaToPostAssociationUseCase, @Named(UI_THREAD) private val mainDispatcher: CoroutineDispatcher ) : CoroutineScope { // region Fields @@ -224,6 +225,13 @@ class EditorMedia @Inject constructor( retryFailedMediaUploadUseCase.retryFailedMediaAsync(editorMediaListener, failedMediaIds) } } + + fun purgeMediaToPostAssociationsIfNotInPostAnymoreAsync() { + launch { + cleanUpMediaToPostAssociationUseCase + .purgeMediaToPostAssociationsIfNotInPostAnymore(editorMediaListener.getImmutablePost()) + } + } // endregion fun cancelAddMediaToEditorActions() { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt index 23f3bda92250..e129d6a0f6b9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt @@ -53,6 +53,7 @@ class AppPrefsWrapper @Inject constructor() { fun getAppWidgetSiteId(appWidgetId: Int) = AppPrefs.getStatsWidgetSelectedSiteId(appWidgetId) fun setAppWidgetSiteId(siteId: Long, appWidgetId: Int) = AppPrefs.setStatsWidgetSelectedSiteId(siteId, appWidgetId) fun removeAppWidgetSiteId(appWidgetId: Int) = AppPrefs.removeStatsWidgetSelectedSiteId(appWidgetId) + fun isGutenbergEditorEnabled() = AppPrefs.isGutenbergEditorEnabled() fun getAppWidgetColor(appWidgetId: Int): Color? { return when (AppPrefs.getStatsWidgetColorModeId(appWidgetId)) { diff --git a/WordPress/src/test/java/org/wordpress/android/CoroutinesUtils.kt b/WordPress/src/test/java/org/wordpress/android/CoroutinesUtils.kt index 716b6315deec..09805610db47 100644 --- a/WordPress/src/test/java/org/wordpress/android/CoroutinesUtils.kt +++ b/WordPress/src/test/java/org/wordpress/android/CoroutinesUtils.kt @@ -19,6 +19,7 @@ fun test(context: CoroutineContext = EmptyCoroutineContext, block: suspend C runBlocking(context, block) } +@Suppress("unused") fun KStubbing.onBlocking(methodCall: suspend T.() -> R): OngoingStubbing { return runBlocking { Mockito.`when`(mock.methodCall()) } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt new file mode 100644 index 000000000000..85853c7423e8 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt @@ -0,0 +1,178 @@ +package org.wordpress.android.ui.posts.editor.media + +import com.nhaarman.mockitokotlin2.anyOrNull +import com.nhaarman.mockitokotlin2.argumentCaptor +import com.nhaarman.mockitokotlin2.doAnswer +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.verify +import kotlinx.coroutines.InternalCoroutinesApi +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.mockito.ArgumentMatchers.any +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.TEST_DISPATCHER +import org.wordpress.android.fluxc.Dispatcher +import org.wordpress.android.fluxc.annotations.action.Action +import org.wordpress.android.fluxc.model.MediaModel +import org.wordpress.android.fluxc.store.UploadStore +import org.wordpress.android.fluxc.store.UploadStore.ClearMediaPayload +import org.wordpress.android.test +import org.wordpress.android.ui.posts.PostUtilsWrapper +import org.wordpress.android.ui.posts.editor.AztecEditorFragmentStaticWrapper +import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createAppPrefsWrapper +import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createAztecEditorWrapper +import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createMediaList +import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createPostUtilsWrapper +import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createUploadStore +import org.wordpress.android.ui.prefs.AppPrefsWrapper + +@InternalCoroutinesApi +@RunWith(Parameterized::class) +class CleanUpMediaToPostAssociationUseCaseTest( + private val aztecEnabled: Boolean, + private val gutenbergEnabled: Boolean +) : BaseUnitTest() { + @Test + fun `media which are NOT in post are cleared`() = test { + // Arrange + val mediaList = createMediaList() + val uploadStore = createUploadStore( + failedMedia = setOf(mediaList[0]), + uploadedMedia = setOf(mediaList[1]), + uploadingMedia = setOf(mediaList[2]) + ) + val mediaInPost = mediaList.slice(3..9).toSet() + val dispatcher = mock() + // Act + createUseCase( + dispatcher, + uploadStore, + mediaInPost + ).purgeMediaToPostAssociationsIfNotInPostAnymore(mock()) + + // Assert + val captor = argumentCaptor>() + verify(dispatcher).dispatch(captor.capture()) + assertThat(captor.firstValue.payload.media) + .isEqualTo(setOf(mediaList[0],mediaList[1],mediaList[2])) + } + + @Test + fun `media which are in post are NOT cleared`() = test { + // Arrange + val mediaList = createMediaList() + + val dispatcher = mock() + val uploadStore = createUploadStore( + failedMedia = setOf(mediaList[0]), + uploadedMedia = setOf(mediaList[1]), + uploadingMedia = setOf(mediaList[2]) + ) + // Act + createUseCase( + dispatcher, + uploadStore, + mediaInPost = mediaList.toSet() + ).purgeMediaToPostAssociationsIfNotInPostAnymore(mock()) + + // Assert + verify(dispatcher, never()).dispatch(any>()) + } + + @Test + fun `featured images are NOT cleared even though they are NOT in post`() = test { + // Arrange + val mediaList = createMediaList() + val dispatcher = mock() + val uploadStore = createUploadStore( + failedMedia = setOf(mediaList[0]), + uploadedMedia = setOf(mediaList[1]), + uploadingMedia = setOf(mediaList[2]) + ) + mediaList[0].markedLocallyAsFeatured = true + val mediaInPost = mediaList.slice(3..9).toSet() + // Act + createUseCase( + dispatcher, + uploadStore, + mediaInPost + ).purgeMediaToPostAssociationsIfNotInPostAnymore(mock()) + + // Assert + val captor = argumentCaptor>() + verify(dispatcher).dispatch(captor.capture()) + assertThat(captor.firstValue.payload.media) + .isEqualTo(setOf(mediaList[1],mediaList[2])) + } + + private fun createUseCase( + dispatcher: Dispatcher = mock(), + uploadStore: UploadStore = mock(), + mediaInPost: Set + ) = CleanUpMediaToPostAssociationUseCase( + dispatcher, + uploadStore, + createAppPrefsWrapper(aztecEnabled, gutenbergEnabled), + createAztecEditorWrapper(mediaInPost), + createPostUtilsWrapper(mediaInPost), + TEST_DISPATCHER + ) + + companion object { + @JvmStatic + @Parameterized.Parameters + fun parameters() = listOf( + arrayOf(true, true), // aztec enabled, gutenberg enabled + arrayOf(true, false), // aztec enabled, gutenberg disabled + arrayOf(false, true) // aztec disabled, gutenberg enabled + // arrayOf(false, false) is an invalid option, one of the editors must be enabled + ) + } + + private object Fixtures { + fun createUploadStore( + failedMedia: Set = setOf(), + uploadedMedia: Set = setOf(), + uploadingMedia: Set = setOf() + ) = mock { + on { getFailedMediaForPost(any()) }.thenReturn(failedMedia) + on { getCompletedMediaForPost(any()) }.thenReturn(uploadedMedia) + on { getUploadingMediaForPost(any()) }.thenReturn(uploadingMedia) + } + + fun createAppPrefsWrapper(aztecEnabled: Boolean, gutenbergEnabled: Boolean) = + mock { + on { isAztecEditorEnabled }.thenReturn(aztecEnabled) + on { isGutenbergEditorEnabled() }.thenReturn(gutenbergEnabled) + } + + fun createPostUtilsWrapper(mediaInPost: Set) = + mock { + on { isMediaInGutenbergPostBody(anyOrNull(), anyOrNull()) } + .doAnswer { invocation -> + mediaInPost.map { it.id } + .contains((invocation.arguments[1] as String).toInt()) + } + } + + fun createAztecEditorWrapper(mediaInPost: Set) = + mock { + on { isMediaInPostBody(anyOrNull(), anyOrNull()) } + .doAnswer { invocation -> + mediaInPost.map { it.id } + .contains((invocation.arguments[1] as String).toInt()) + } + } + + fun createMediaList(): List { + val list = mutableListOf() + for (i in 1..10) { + list.add(MediaModel().apply { id = i }) + } + return list + } + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/EditorMediaTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/EditorMediaTest.kt index 1842eb73450f..ee2ea5602a32 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/EditorMediaTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/EditorMediaTest.kt @@ -361,7 +361,8 @@ class EditorMediaTest : BaseUnitTest() { addExistingMediaToPostUseCase: AddExistingMediaToPostUseCase = mock(), retryFailedMediaUploadUseCase: RetryFailedMediaUploadUseCase = mock(), siteModel: SiteModel = mock(), - editorMediaListener: EditorMediaListener = mock() + editorMediaListener: EditorMediaListener = mock(), + cleanUpMediaToPostAssociationUseCase: CleanUpMediaToPostAssociationUseCase = mock() ): EditorMedia { val editorMedia = EditorMedia( updateMediaModelUseCase, @@ -372,6 +373,7 @@ class EditorMediaTest : BaseUnitTest() { addLocalMediaToPostUseCase, addExistingMediaToPostUseCase, retryFailedMediaUploadUseCase, + cleanUpMediaToPostAssociationUseCase, TEST_DISPATCHER ) editorMedia.start(siteModel, editorMediaListener) From 6dbe3d4aa5f8fac9a41ddc62ef4e50965ff9a5ee Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 14 Nov 2019 15:57:04 +0100 Subject: [PATCH 2/4] Fix lint issues --- .../media/CleanUpMediaToPostAssociationUseCase.kt | 13 ++++++++++--- .../CleanUpMediaToPostAssociationUseCaseTest.kt | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt index 881e6b9d7585..85127387f320 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt @@ -53,9 +53,16 @@ class CleanUpMediaToPostAssociationUseCase @Inject constructor( } .toSet() .let { mediaToDeleteAssociationFor -> - if(mediaToDeleteAssociationFor.isNotEmpty()) { - val clearMediaPayload = ClearMediaPayload(post, mediaToDeleteAssociationFor) - dispatcher.dispatch(UploadActionBuilder.newClearMediaForPostAction(clearMediaPayload)) + if (mediaToDeleteAssociationFor.isNotEmpty()) { + val clearMediaPayload = ClearMediaPayload( + post, + mediaToDeleteAssociationFor + ) + dispatcher.dispatch( + UploadActionBuilder.newClearMediaForPostAction( + clearMediaPayload + ) + ) } } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt index 85853c7423e8..4005fdee4778 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt @@ -57,7 +57,7 @@ class CleanUpMediaToPostAssociationUseCaseTest( val captor = argumentCaptor>() verify(dispatcher).dispatch(captor.capture()) assertThat(captor.firstValue.payload.media) - .isEqualTo(setOf(mediaList[0],mediaList[1],mediaList[2])) + .isEqualTo(setOf(mediaList[0], mediaList[1], mediaList[2])) } @Test @@ -105,7 +105,7 @@ class CleanUpMediaToPostAssociationUseCaseTest( val captor = argumentCaptor>() verify(dispatcher).dispatch(captor.capture()) assertThat(captor.firstValue.payload.media) - .isEqualTo(setOf(mediaList[1],mediaList[2])) + .isEqualTo(setOf(mediaList[1], mediaList[2])) } private fun createUseCase( From a2bd9eaa10696a5e3e61cb11f5716edba8906fbd Mon Sep 17 00:00:00 2001 From: malinajirka Date: Mon, 2 Dec 2019 12:18:51 +0100 Subject: [PATCH 3/4] Use PostUtils.containsGutenbergBlocks in CleanUpMediaToPostAssociation --- .../android/ui/posts/PostUtilsWrapper.kt | 3 ++ .../CleanUpMediaToPostAssociationUseCase.kt | 21 ++++--------- ...leanUpMediaToPostAssociationUseCaseTest.kt | 31 +++++++------------ 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt index 63dad8a46fcb..3efe1611e04e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt @@ -28,4 +28,7 @@ class PostUtilsWrapper @Inject constructor() { fun isMediaInGutenbergPostBody(postContent: String, localMediaId: String) = PostUtils.isMediaInGutenbergPostBody(postContent, localMediaId) + + fun contentContainsGutenbergBlocks(postContent: String): Boolean = + PostUtils.contentContainsGutenbergBlocks(postContent) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt index 85127387f320..a0741da66e18 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt @@ -11,7 +11,6 @@ import org.wordpress.android.fluxc.store.UploadStore.ClearMediaPayload import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.ui.posts.PostUtilsWrapper import org.wordpress.android.ui.posts.editor.AztecEditorFragmentStaticWrapper -import org.wordpress.android.ui.prefs.AppPrefsWrapper import javax.inject.Inject import javax.inject.Named @@ -19,16 +18,12 @@ import javax.inject.Named class CleanUpMediaToPostAssociationUseCase @Inject constructor( private val dispatcher: Dispatcher, private val uploadStore: UploadStore, - private val appPrefsWrapper: AppPrefsWrapper, private val aztecEditorWrapper: AztecEditorFragmentStaticWrapper, private val postUtilsWrapper: PostUtilsWrapper, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher ) { suspend fun purgeMediaToPostAssociationsIfNotInPostAnymore(post: PostImmutableModel) { withContext(bgDispatcher) { - val useAztec = appPrefsWrapper.isAztecEditorEnabled - val useGutenberg = appPrefsWrapper.isGutenbergEditorEnabled() - val mediaAssociatedWithPost = uploadStore.getFailedMediaForPost(post) + uploadStore.getCompletedMediaForPost(post) + uploadStore.getUploadingMediaForPost(post) @@ -36,16 +31,12 @@ class CleanUpMediaToPostAssociationUseCase @Inject constructor( mediaAssociatedWithPost .filter { media -> // Find media which is not in the post anymore - val isNotInAztec = useAztec && !aztecEditorWrapper.isMediaInPostBody( - post.content, - media.id.toString() - ) - val isNotInGutenberg = useGutenberg && !postUtilsWrapper.isMediaInGutenbergPostBody( - post.content, - media.id.toString() - ) - - isNotInAztec || isNotInGutenberg + val containsGutenbergBlocks = postUtilsWrapper.contentContainsGutenbergBlocks(post.content) + if(containsGutenbergBlocks) { + !postUtilsWrapper.isMediaInGutenbergPostBody(post.content, media.id.toString()) + } else { + !aztecEditorWrapper.isMediaInPostBody(post.content, media.id.toString()) + } } .filter { media -> // Featured images are not in post content, don't delete them diff --git a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt index 4005fdee4778..97deef417a17 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCaseTest.kt @@ -3,6 +3,7 @@ package org.wordpress.android.ui.posts.editor.media import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor import com.nhaarman.mockitokotlin2.doAnswer +import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify @@ -22,19 +23,14 @@ import org.wordpress.android.fluxc.store.UploadStore.ClearMediaPayload import org.wordpress.android.test import org.wordpress.android.ui.posts.PostUtilsWrapper import org.wordpress.android.ui.posts.editor.AztecEditorFragmentStaticWrapper -import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createAppPrefsWrapper import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createAztecEditorWrapper import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createMediaList import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createPostUtilsWrapper import org.wordpress.android.ui.posts.editor.media.CleanUpMediaToPostAssociationUseCaseTest.Fixtures.createUploadStore -import org.wordpress.android.ui.prefs.AppPrefsWrapper @InternalCoroutinesApi @RunWith(Parameterized::class) -class CleanUpMediaToPostAssociationUseCaseTest( - private val aztecEnabled: Boolean, - private val gutenbergEnabled: Boolean -) : BaseUnitTest() { +class CleanUpMediaToPostAssociationUseCaseTest(private val containsGutenbergBlocks: Boolean) : BaseUnitTest() { @Test fun `media which are NOT in post are cleared`() = test { // Arrange @@ -115,9 +111,8 @@ class CleanUpMediaToPostAssociationUseCaseTest( ) = CleanUpMediaToPostAssociationUseCase( dispatcher, uploadStore, - createAppPrefsWrapper(aztecEnabled, gutenbergEnabled), createAztecEditorWrapper(mediaInPost), - createPostUtilsWrapper(mediaInPost), + createPostUtilsWrapper(mediaInPost, containsGutenbergBlocks), TEST_DISPATCHER ) @@ -125,10 +120,8 @@ class CleanUpMediaToPostAssociationUseCaseTest( @JvmStatic @Parameterized.Parameters fun parameters() = listOf( - arrayOf(true, true), // aztec enabled, gutenberg enabled - arrayOf(true, false), // aztec enabled, gutenberg disabled - arrayOf(false, true) // aztec disabled, gutenberg enabled - // arrayOf(false, false) is an invalid option, one of the editors must be enabled + arrayOf(true), // Test with posts containing gutenberg blocks + arrayOf(false) // Test with posts not containing gutenberg blocks ) } @@ -143,19 +136,19 @@ class CleanUpMediaToPostAssociationUseCaseTest( on { getUploadingMediaForPost(any()) }.thenReturn(uploadingMedia) } - fun createAppPrefsWrapper(aztecEnabled: Boolean, gutenbergEnabled: Boolean) = - mock { - on { isAztecEditorEnabled }.thenReturn(aztecEnabled) - on { isGutenbergEditorEnabled() }.thenReturn(gutenbergEnabled) - } - - fun createPostUtilsWrapper(mediaInPost: Set) = + fun createPostUtilsWrapper( + mediaInPost: Set, + containsGutenbergBlocks: Boolean + ) = mock { on { isMediaInGutenbergPostBody(anyOrNull(), anyOrNull()) } .doAnswer { invocation -> mediaInPost.map { it.id } .contains((invocation.arguments[1] as String).toInt()) } + on { contentContainsGutenbergBlocks(anyOrNull()) }.doReturn( + containsGutenbergBlocks + ) } fun createAztecEditorWrapper(mediaInPost: Set) = From 1d7b7bc95a3e346a8b801014389f4eadc76fc6c6 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Mon, 2 Dec 2019 12:20:11 +0100 Subject: [PATCH 4/4] Fix lint --- .../editor/media/CleanUpMediaToPostAssociationUseCase.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt index a0741da66e18..d6e3fe671848 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/editor/media/CleanUpMediaToPostAssociationUseCase.kt @@ -31,8 +31,8 @@ class CleanUpMediaToPostAssociationUseCase @Inject constructor( mediaAssociatedWithPost .filter { media -> // Find media which is not in the post anymore - val containsGutenbergBlocks = postUtilsWrapper.contentContainsGutenbergBlocks(post.content) - if(containsGutenbergBlocks) { + val containsGutenbergBlocks = postUtilsWrapper.contentContainsGutenbergBlocks(post.content) + if (containsGutenbergBlocks) { !postUtilsWrapper.isMediaInGutenbergPostBody(post.content, media.id.toString()) } else { !aztecEditorWrapper.isMediaInPostBody(post.content, media.id.toString())