Skip to content

Commit 8302260

Browse files
authored
Merge pull request #14545 from woocommerce/issue/woomob-1207-woo-poslocal-catalog-propagate-header-with-total-number-of
[WOOMOB-1207] Propagate totalPages and Date headers from Local Catalog Rest Client
2 parents 2c804fa + c1d0fc5 commit 8302260

File tree

8 files changed

+431
-59
lines changed

8 files changed

+431
-59
lines changed

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosSyncProductsActionTest.kt

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import kotlinx.coroutines.test.runTest
77
import org.assertj.core.api.Assertions.assertThat
88
import org.junit.Before
99
import org.junit.Test
10+
import org.mockito.ArgumentMatchers.anyBoolean
1011
import org.mockito.kotlin.any
1112
import org.mockito.kotlin.anyOrNull
1213
import org.mockito.kotlin.eq
@@ -95,7 +96,8 @@ class WooPosSyncProductsActionTest {
9596
site = eq(site),
9697
modifiedAfterGmt = eq(modifiedAfter),
9798
offset = eq(0),
98-
pageSize = eq(100)
99+
pageSize = eq(100),
100+
storeInDb = any(),
99101
)
100102
assertThat(result).isInstanceOf(WooPosSyncProductsResult.Success::class.java)
101103
}
@@ -190,7 +192,7 @@ class WooPosSyncProductsActionTest {
190192
// THEN
191193
assertThat(result).isInstanceOf(WooPosSyncProductsResult.Success::class.java)
192194
verify(posLocalCatalogStore, times(1))
193-
.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any())
195+
.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any(), any())
194196
}
195197

196198
@Test
@@ -205,44 +207,47 @@ class WooPosSyncProductsActionTest {
205207
// THEN
206208
assertThat(result).isInstanceOf(WooPosSyncProductsResult.Success::class.java)
207209
verify(posLocalCatalogStore, times(1))
208-
.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any())
210+
.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any(), any())
209211
}
210212

211213
private suspend fun givenSinglePageCatalog(productsCount: Int = PAGE_SIZE / 2) {
212-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any()))
214+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any(), anyBoolean()))
213215
.thenReturn(
214216
KotlinResult.success(
215217
WooPosLocalCatalogSyncResult(
216218
syncedCount = productsCount,
217219
hasMore = false,
218220
nextOffset = 0,
219-
totalPages = 1
221+
totalPages = 1,
222+
serverDate = "",
220223
)
221224
)
222225
)
223226
}
224227

225228
private suspend fun givenMultiPageCatalog(page1Count: Int, page2Count: Int, page3Count: Int, totalPages: Int = 3) {
226-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any()))
229+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any(), any()))
227230
.thenReturn(
228231
KotlinResult.success(
229232
WooPosLocalCatalogSyncResult(
230233
syncedCount = page1Count,
231234
hasMore = true,
232235
nextOffset = page1Count,
233-
totalPages = totalPages
236+
totalPages = totalPages,
237+
serverDate = ""
234238
)
235239
)
236240
)
237241

238-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(page1Count), any()))
242+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(page1Count), any(), any()))
239243
.thenReturn(
240244
KotlinResult.success(
241245
WooPosLocalCatalogSyncResult(
242246
syncedCount = page2Count,
243247
hasMore = true,
244248
nextOffset = page1Count + page2Count,
245-
totalPages = totalPages
249+
totalPages = totalPages,
250+
serverDate = "",
246251
)
247252
)
248253
)
@@ -252,6 +257,7 @@ class WooPosSyncProductsActionTest {
252257
any(),
253258
anyOrNull(),
254259
eq(page1Count + page2Count),
260+
any(),
255261
any()
256262
)
257263
)
@@ -261,76 +267,81 @@ class WooPosSyncProductsActionTest {
261267
syncedCount = page3Count,
262268
hasMore = false,
263269
nextOffset = 0,
264-
totalPages = totalPages
270+
totalPages = totalPages,
271+
serverDate = "",
265272
)
266273
)
267274
)
268275
}
269276

270277
private suspend fun givenEmptyCatalog() {
271-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any()))
278+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any(), any()))
272279
.thenReturn(
273280
KotlinResult.success(
274281
WooPosLocalCatalogSyncResult(
275282
syncedCount = 0,
276283
hasMore = false,
277284
nextOffset = 0,
278-
totalPages = 1
285+
totalPages = 1,
286+
serverDate = "",
279287
)
280288
)
281289
)
282290
}
283291

284292
private suspend fun givenFirstPageFails(errorMessage: String) {
285-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any()))
293+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any(), any()))
286294
.thenReturn(KotlinResult.failure(Exception(errorMessage)))
287295
}
288296

289297
private suspend fun givenFirstPageFailsWithNullMessage() {
290-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any()))
298+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), any(), any(), any()))
291299
.thenReturn(KotlinResult.failure(Exception()))
292300
}
293301

294302
private suspend fun givenSecondPageFails(errorMessage: String) {
295-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any()))
303+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any(), any()))
296304
.thenReturn(
297305
KotlinResult.success(
298306
WooPosLocalCatalogSyncResult(
299307
syncedCount = 100,
300308
hasMore = true,
301309
nextOffset = 100,
302-
totalPages = 2
310+
totalPages = 2,
311+
serverDate = ""
303312
)
304313
)
305314
)
306315

307-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(100), any()))
316+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(100), any(), any()))
308317
.thenReturn(KotlinResult.failure(Exception(errorMessage)))
309318
}
310319

311320
private suspend fun givenPageWithZeroProductsButHasMore() {
312321
// Note: Due to current action logic, it won't continue if syncedCount == 0
313322
// So we use 1 product on first page instead of 0 to demonstrate continuing
314-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any()))
323+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(0), any(), any()))
315324
.thenReturn(
316325
KotlinResult.success(
317326
WooPosLocalCatalogSyncResult(
318327
syncedCount = 0,
319328
hasMore = true,
320329
nextOffset = 100,
321-
totalPages = 2
330+
totalPages = 2,
331+
serverDate = "",
322332
)
323333
)
324334
)
325335

326-
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(100), any()))
336+
whenever(posLocalCatalogStore.syncRecentlyModifiedProducts(any(), anyOrNull(), eq(100), any(), any()))
327337
.thenReturn(
328338
KotlinResult.success(
329339
WooPosLocalCatalogSyncResult(
330340
syncedCount = 50,
331341
hasMore = false,
332342
nextOffset = 0,
333-
totalPages = 2
343+
totalPages = 2,
344+
serverDate = ""
334345
)
335346
)
336347
)

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/WooResult.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package org.wordpress.android.fluxc.network.rest.wpcom.wc
22

33
import org.wordpress.android.fluxc.Payload
44
import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType.UNKNOWN
5+
import org.wordpress.android.fluxc.network.rest.Header
56
import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooErrorType.GENERIC_ERROR
67
import org.wordpress.android.fluxc.store.Store
78

@@ -19,7 +20,7 @@ data class WooPayload<T>(val result: T? = null) : Payload<WooError>() {
1920
}
2021
}
2122

22-
data class WooResult<T>(val model: T? = null) : Store.OnChanged<WooError>() {
23+
data class WooResult<T>(val model: T? = null, val headers: List<Header> = emptyList()) : Store.OnChanged<WooError>() {
2324
constructor(error: WooError) : this() {
2425
this.error = error
2526
}

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/product/pos/WooPosProductRestClient.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class WooPosProductRestClient @Inject constructor(
4949

5050
return when (response) {
5151
is WPAPIResponse.Success -> {
52-
WooResult(response.data ?: emptyArray())
52+
WooResult(response.data ?: emptyArray(), headers = response.headers)
5353
}
5454

5555
is WPAPIResponse.Error -> {

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogStore.kt

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import org.wordpress.android.fluxc.persistence.dao.pos.WooPosVariationsDao
1414
import org.wordpress.android.fluxc.persistence.entity.pos.WCPosProductModel
1515
import org.wordpress.android.fluxc.persistence.entity.pos.WCPosVariationModel
1616
import org.wordpress.android.fluxc.tools.CoroutineEngine
17+
import org.wordpress.android.fluxc.utils.HeadersParser
1718
import org.wordpress.android.util.AppLog.T.API
1819
import javax.inject.Inject
1920
import javax.inject.Singleton
@@ -24,6 +25,7 @@ class WooPosLocalCatalogStore @Inject constructor(
2425
private val coroutineEngine: CoroutineEngine,
2526
private val posProductDao: WooPosProductsDao,
2627
private val posVariationsDao: WooPosVariationsDao,
28+
private val headersParser: HeadersParser,
2729
) {
2830
private companion object {
2931
private const val DEFAULT_PAGE_SIZE = 100
@@ -74,6 +76,7 @@ class WooPosLocalCatalogStore @Inject constructor(
7476
modifiedAfterGmt: String?,
7577
offset: Int = 0,
7678
pageSize: Int = DEFAULT_PAGE_SIZE,
79+
storeInDb: Boolean = true
7780
): Result<WooPosLocalCatalogSyncResult> =
7881
coroutineEngine.withDefaultContext(API, this, "syncRecentlyModifiedProducts") {
7982
val validPageSize = pageSize.coerceIn(1, MAX_PAGE_SIZE)
@@ -85,30 +88,32 @@ class WooPosLocalCatalogStore @Inject constructor(
8588
pageSize = validPageSize
8689
)
8790

88-
when {
89-
response.isError -> {
90-
Result.failure(
91-
mapResponseError(response.error)
92-
)
93-
}
91+
val serverDate = headersParser.getServerDate(response)
9492

95-
response.model.isNullOrEmpty() -> {
96-
Result.success(
97-
WooPosLocalCatalogSyncResult(
98-
syncedCount = 0,
99-
hasMore = false,
100-
nextOffset = offset,
101-
totalPages = 0
102-
)
103-
)
104-
}
93+
if (serverDate == null) {
94+
return@withDefaultContext Result.failure(
95+
WooPosLocalCatalogError.InvalidResponse("Missing required header in response: Server Date.")
96+
)
97+
}
10598

106-
else -> {
107-
val products = response.model.map { it.mapToPOSModel() }
99+
when {
100+
response.isError -> Result.failure(mapResponseError(response.error))
101+
102+
response.model.isNullOrEmpty() -> Result.success(
103+
WooPosLocalCatalogSyncResult(
104+
syncedCount = 0,
105+
hasMore = false,
106+
nextOffset = offset,
107+
totalPages = 0,
108+
serverDate = serverDate
109+
)
110+
)
108111

109-
val upsertResult = runCatching {
110-
posProductDao.upsertProducts(products)
111-
}
112+
else -> {
113+
val products = response.model.map { it.mapToPOSModel() }
114+
115+
if (storeInDb) {
116+
val upsertResult = runCatching { posProductDao.upsertProducts(products) }
112117

113118
if (upsertResult.isFailure) {
114119
return@withDefaultContext Result.failure(
@@ -118,18 +123,30 @@ class WooPosLocalCatalogStore @Inject constructor(
118123
)
119124
)
120125
}
126+
}
121127

122-
val hasMore = products.size == validPageSize
128+
val hasMore = products.size == validPageSize
123129

124-
Result.success(
125-
WooPosLocalCatalogSyncResult(
126-
syncedCount = products.size,
127-
hasMore = hasMore,
128-
nextOffset = if (hasMore) offset + products.size else offset,
129-
totalPages = 3 // Tbd Local Catalog: Read from header.
130+
val totalPages = headersParser.getTotalPages(response)
131+
132+
if (totalPages == null) {
133+
return@withDefaultContext Result.failure(
134+
WooPosLocalCatalogError.InvalidResponse(
135+
"Missing required header in response: Total Pages."
130136
)
131137
)
132138
}
139+
140+
Result.success(
141+
WooPosLocalCatalogSyncResult(
142+
syncedCount = products.size,
143+
hasMore = hasMore,
144+
nextOffset = if (hasMore) offset + products.size else offset,
145+
totalPages = totalPages,
146+
serverDate = serverDate,
147+
)
148+
)
149+
}
133150
}
134151
}
135152

@@ -328,18 +345,22 @@ class WooPosLocalCatalogStore @Inject constructor(
328345
errorMessage = "Request timed out",
329346
code = error.type.name
330347
)
348+
331349
WooErrorType.NO_CONNECTION -> WooPosLocalCatalogError.NetworkError(
332350
errorMessage = error.message ?: "No network connection",
333351
code = error.type.name
334352
)
353+
335354
WooErrorType.INVALID_RESPONSE -> WooPosLocalCatalogError.NetworkError(
336355
errorMessage = "Invalid response from server",
337356
code = error.type.name
338357
)
358+
339359
WooErrorType.API_ERROR -> WooPosLocalCatalogError.NetworkError(
340360
errorMessage = error.message ?: "API error occurred",
341361
code = error.type.name
342362
)
363+
343364
WooErrorType.EMPTY_RESPONSE -> WooPosLocalCatalogError.EmptyResponse
344365
else -> WooPosLocalCatalogError.NetworkError(
345366
errorMessage = error?.message ?: "Unknown error occurred",

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogSyncResult.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ data class WooPosLocalCatalogSyncResult(
55
val hasMore: Boolean,
66
val nextOffset: Int,
77
val totalPages: Int,
8+
val serverDate: String,
89
)
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.wordpress.android.fluxc.utils
2+
3+
import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooResult
4+
import org.wordpress.android.util.AppLog
5+
import javax.inject.Inject
6+
7+
class HeadersParser @Inject constructor(
8+
val logger: AppLogWrapper
9+
) {
10+
companion object Companion {
11+
private const val TOTAL_PAGES_HEADER = "x-wp-totalpages"
12+
private const val SERVER_DATE_HEADER = "date"
13+
}
14+
15+
fun <T> getTotalPages(result: WooResult<T>): Int? {
16+
return try {
17+
result.headers
18+
.findLast { TOTAL_PAGES_HEADER.equals(it.key, ignoreCase = true) }
19+
?.value?.toInt()
20+
} catch (e: NumberFormatException) {
21+
logger.e(AppLog.T.API, "Failed to parse total pages from headers", e)
22+
null
23+
}
24+
}
25+
26+
fun <T> getServerDate(response: WooResult<T>): String? = response.headers
27+
.findLast { SERVER_DATE_HEADER.equals(it.key, ignoreCase = true) }
28+
?.value
29+
}

0 commit comments

Comments
 (0)