Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LocalDraftUploadStarter: Upload local draft pages #9902

Merged
merged 11 commits into from May 24, 2019
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
@@ -1,5 +1,6 @@
12.6
-----
* Local draft pages will be automatically uploaded to the server as soon as an internet connection is available.


12.5
Expand Down
Expand Up @@ -9,9 +9,11 @@ import android.content.Context
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.launch
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.PageStore
import org.wordpress.android.fluxc.store.PostStore
import org.wordpress.android.fluxc.store.SiteStore
import org.wordpress.android.modules.BG_THREAD
Expand Down Expand Up @@ -40,6 +42,7 @@ class LocalDraftUploadStarter @Inject constructor(
*/
private val context: Context,
private val postStore: PostStore,
private val pageStore: PageStore,
private val siteStore: SiteStore,
@Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher,
@Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher,
Expand Down Expand Up @@ -122,9 +125,13 @@ class LocalDraftUploadStarter @Inject constructor(
/**
* This is meant to be used by [checkConnectionAndUpload] only.
*/
private fun upload(site: SiteModel) {
postStore.getLocalDraftPosts(site)
.filterNot { uploadServiceFacade.isPostUploadingOrQueued(it) }
private suspend fun upload(site: SiteModel) = coroutineScope {
val posts = async { postStore.getLocalDraftPosts(site) }
val pages = async { pageStore.getLocalDraftPages(site) }

val postsAndPages = posts.await() + pages.await()

postsAndPages.filterNot { uploadServiceFacade.isPostUploadingOrQueued(it) }
.forEach { localDraft ->
uploadServiceFacade.uploadPost(
context = context,
Expand Down
Expand Up @@ -36,6 +36,7 @@ import org.wordpress.android.ui.pages.PageItem.Action.SET_PARENT
import org.wordpress.android.ui.pages.PageItem.Action.VIEW_PAGE
import org.wordpress.android.ui.pages.PageItem.Page
import org.wordpress.android.ui.pages.SnackbarMessageHolder
import org.wordpress.android.ui.uploads.LocalDraftUploadStarter
import org.wordpress.android.util.AppLog
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.util.analytics.AnalyticsUtils
Expand Down Expand Up @@ -74,6 +75,7 @@ class PagesViewModel
private val dispatcher: Dispatcher,
private val actionPerfomer: ActionPerformer,
private val networkUtils: NetworkUtilsWrapper,
private val localDraftUploadStarter: LocalDraftUploadStarter,
@Named(UI_THREAD) private val uiDispatcher: CoroutineDispatcher,
@Named(BG_THREAD) private val defaultDispatcher: CoroutineDispatcher
) : ScopedViewModel(uiDispatcher) {
Expand Down Expand Up @@ -148,6 +150,8 @@ class PagesViewModel
_site = site

loadPagesAsync()

localDraftUploadStarter.queueUploadFromSite(site)
}
}

Expand Down Expand Up @@ -419,6 +423,8 @@ class PagesViewModel
}

fun onPullToRefresh() {
localDraftUploadStarter.queueUploadFromSite(site)

launch {
reloadPages(FETCHING)
}
Expand Down
Expand Up @@ -15,6 +15,7 @@ import org.junit.runner.RunWith
import org.mockito.junit.MockitoJUnitRunner
import org.wordpress.android.fluxc.model.PostModel
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.PageStore
import org.wordpress.android.fluxc.store.PostStore
import org.wordpress.android.util.NetworkUtilsWrapper

Expand All @@ -40,6 +41,9 @@ class LocalDraftUploadStarterConcurrentTest {
private val postStore = mock<PostStore> {
on { getLocalDraftPosts(eq(site)) } doReturn posts
}
private val pageStore = mock<PageStore> {
onBlocking { getLocalDraftPages(any()) } doReturn emptyList()
}

@Test
fun `it uploads local drafts concurrently`() {
Expand All @@ -66,6 +70,7 @@ class LocalDraftUploadStarterConcurrentTest {
private fun createLocalDraftUploadStarter(uploadServiceFacade: UploadServiceFacade) = LocalDraftUploadStarter(
context = mock(),
postStore = postStore,
pageStore = pageStore,
siteStore = mock(),
bgDispatcher = Dispatchers.Default,
ioDispatcher = Dispatchers.IO,
Expand Down
Expand Up @@ -24,6 +24,7 @@ import org.junit.runner.RunWith
import org.mockito.junit.MockitoJUnitRunner
import org.wordpress.android.fluxc.model.PostModel
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.PageStore
import org.wordpress.android.fluxc.store.PostStore
import org.wordpress.android.fluxc.store.SiteStore
import org.wordpress.android.util.NetworkUtilsWrapper
Expand All @@ -50,12 +51,23 @@ class LocalDraftUploadStarterTest {
)
private val posts = sitesAndPosts.values.flatten()

private val sitesAndPages: Map<SiteModel, List<PostModel>> = mapOf(
sites[0] to listOf(createPostModel(), createPostModel()),
sites[1] to listOf(createPostModel(), createPostModel(), createPostModel(), createPostModel())
)
private val pages = sitesAndPages.values.flatten()

private val siteStore = mock<SiteStore> {
on { sites } doReturn sites
}
private val postStore = mock<PostStore> {
sites.forEach {
on { getLocalDraftPosts(eq(it)) } doReturn sitesAndPosts[it]
on { getLocalDraftPosts(eq(it)) } doReturn sitesAndPosts.getValue(it)
}
}
private val pageStore = mock<PageStore> {
sites.forEach {
onBlocking { getLocalDraftPages(eq(it)) } doReturn sitesAndPages.getValue(it)
}
}

Expand All @@ -72,7 +84,7 @@ class LocalDraftUploadStarterTest {
connectionStatus.postValue(AVAILABLE)

// Then
verify(uploadServiceFacade, times(posts.size)).uploadPost(
verify(uploadServiceFacade, times(posts.size + pages.size)).uploadPost(
context = any(),
post = any(),
trackAnalytics = any(),
Expand All @@ -96,7 +108,7 @@ class LocalDraftUploadStarterTest {
lifecycle.handleLifecycleEvent(Event.ON_START)

// Then
verify(uploadServiceFacade, times(posts.size)).uploadPost(
verify(uploadServiceFacade, times(posts.size + pages.size)).uploadPost(
context = any(),
post = any(),
trackAnalytics = any(),
Expand All @@ -119,7 +131,8 @@ class LocalDraftUploadStarterTest {
starter.queueUploadFromSite(site)

// Then
verify(uploadServiceFacade, times(sitesAndPosts.getValue(site).size)).uploadPost(
val expectedUploadPostExecutions = sitesAndPosts.getValue(site).size + sitesAndPages.getValue(site).size
verify(uploadServiceFacade, times(expectedUploadPostExecutions)).uploadPost(
context = any(),
post = any(),
trackAnalytics = any(),
Expand All @@ -139,12 +152,21 @@ class LocalDraftUploadStarterTest {
posts.subList(posts.size / 2, posts.size)
)
}
val (expectedQueuedPages, expectedUploadedPages) = sitesAndPages.getValue(site).let { pages ->
// Split into halves of already queued and what should be uploaded
return@let Pair(
pages.subList(0, pages.size / 2),
pages.subList(pages.size / 2, pages.size)
)
}
val expectedQueuedPostsAndPages = expectedQueuedPosts + expectedQueuedPages
val expectedUploadPostsAndPages = expectedUploadedPosts + expectedUploadedPages

val connectionStatus = createConnectionStatusLiveData(null)
val uploadServiceFacade = mock<UploadServiceFacade> {
on { isPostUploadingOrQueued(any()) } doAnswer {
val post = it.arguments.first() as PostModel
expectedQueuedPosts.contains(post)
expectedQueuedPostsAndPages.contains(post)
}
}

Expand All @@ -154,14 +176,17 @@ class LocalDraftUploadStarterTest {
starter.queueUploadFromSite(site)

// Then
verify(uploadServiceFacade, times(expectedUploadedPosts.size)).uploadPost(
verify(uploadServiceFacade, times(expectedUploadPostsAndPages.size)).uploadPost(
context = any(),
post = argWhere { expectedUploadedPosts.contains(it) },
post = argWhere { expectedUploadPostsAndPages.contains(it) },
trackAnalytics = any(),
publish = any(),
isRetry = eq(true)
)
verify(uploadServiceFacade, times(sitesAndPosts.getValue(site).size)).isPostUploadingOrQueued(any())
verify(
uploadServiceFacade,
times(sitesAndPosts.getValue(site).size + sitesAndPages.getValue(site).size)
).isPostUploadingOrQueued(any())
verifyNoMoreInteractions(uploadServiceFacade)
}

Expand All @@ -172,6 +197,7 @@ class LocalDraftUploadStarterTest {
) = LocalDraftUploadStarter(
context = mock(),
postStore = postStore,
pageStore = pageStore,
siteStore = siteStore,
bgDispatcher = Dispatchers.Unconfined,
ioDispatcher = Dispatchers.Unconfined,
Expand Down
Expand Up @@ -2,8 +2,13 @@ package org.wordpress.android.viewmodel.pages

import android.arch.core.executor.testing.InstantTaskExecutorRule
import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.eq
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions
import com.nhaarman.mockitokotlin2.whenever
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
Expand All @@ -20,6 +25,7 @@ import org.wordpress.android.fluxc.model.page.PageStatus.DRAFT
import org.wordpress.android.fluxc.store.PageStore
import org.wordpress.android.fluxc.store.PostStore.OnPostChanged
import org.wordpress.android.test
import org.wordpress.android.ui.uploads.LocalDraftUploadStarter
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListState
import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListState.DONE
Expand All @@ -40,21 +46,24 @@ class PagesViewModelTest {
@Mock lateinit var dispatcher: Dispatcher
@Mock lateinit var actionPerformer: ActionPerformer
@Mock lateinit var networkUtils: NetworkUtilsWrapper
@Mock lateinit var localDraftUploadStarter: LocalDraftUploadStarter
private lateinit var viewModel: PagesViewModel
private lateinit var listStates: MutableList<PageListState>
private lateinit var pages: MutableList<List<PageModel>>
private lateinit var searchPages: MutableList<SortedMap<PageListType, List<PageModel>>>
private lateinit var pageModel: PageModel

@UseExperimental(ExperimentalCoroutinesApi::class)
@Before
fun setUp() {
viewModel = PagesViewModel(
pageStore,
dispatcher,
actionPerformer,
networkUtils,
Dispatchers.Unconfined,
Dispatchers.Unconfined
pageStore = pageStore,
dispatcher = dispatcher,
actionPerfomer = actionPerformer,
networkUtils = networkUtils,
localDraftUploadStarter = localDraftUploadStarter,
uiDispatcher = Dispatchers.Unconfined,
defaultDispatcher = Dispatchers.Unconfined
)
listStates = mutableListOf()
pages = mutableListOf()
Expand Down Expand Up @@ -143,6 +152,30 @@ class PagesViewModelTest {
assertThat(result).isNull()
}

@Test
fun onStartUploadsAllLocalDrafts() = runBlocking {
// Act
initSearch()

// Assert
verify(localDraftUploadStarter, times(1)).queueUploadFromSite(eq(site))
verifyNoMoreInteractions(localDraftUploadStarter)
}

@Test
fun onPullToRefreshUploadsAllLocalDrafts() = runBlocking {
// Arrange
initSearch()

// Act
viewModel.onPullToRefresh()

// Assert
// We get 2 calls because the `viewModel.start()` also requests an upload
verify(localDraftUploadStarter, times(2)).queueUploadFromSite(eq(site))
verifyNoMoreInteractions(localDraftUploadStarter)
}

private suspend fun initSearch() {
whenever(pageStore.getPagesFromDb(site)).thenReturn(listOf())
whenever(pageStore.requestPagesFromServer(any())).thenReturn(
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Expand Up @@ -105,5 +105,5 @@ buildScan {

ext {
daggerVersion = '2.22.1'
fluxCVersion = 'b3bbec15b0434c404b778764e26b9df4b18f5ab2'
fluxCVersion = '39a08b3419f2fedfec741ea7fa7fc4dd7b3078ae'
}