Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8ff12c6
Add LOCAL_CATALOG_FILE to SyncStrategy enum
malinajirka Feb 4, 2026
0cf6419
Add file-based sync properties to PosLocalCatalogSyncResult
malinajirka Feb 4, 2026
7ed8ba8
Add poll attempts persistence to WooPosPreferencesRepository
malinajirka Feb 4, 2026
dd5c0a1
Add generation duration calculation to WooPosSyncTimestampManager
malinajirka Feb 4, 2026
d6c1bac
Track poll attempts and generation duration in WooPosFileBasedSyncAction
malinajirka Feb 4, 2026
156eb30
Add file-based sync properties to analytics events
malinajirka Feb 4, 2026
a8b90d7
Pass file-based sync analytics data to tracking events
malinajirka Feb 4, 2026
d564a77
Fix SyncStrategy checks to include LOCAL_CATALOG_FILE
malinajirka Feb 4, 2026
585d1ff
Fix detekt
malinajirka Feb 4, 2026
286cae3
Merge branch 'trunk' into woomob/add-file-based-catalog-analytics
malinajirka Feb 4, 2026
fb649df
Remove analytics plan for file-based local catalog sync documentation
malinajirka Feb 4, 2026
43101d9
Replace `getFile...PollAttempts` with `getAndClear...PollAttempts`
malinajirka Feb 4, 2026
1521985
Remove unused `clearFileBasedSyncPollAttempts` function from WooPosPr…
malinajirka Feb 4, 2026
c617c1a
Add unit tests for poll attempts, generation duration, and timeout ha…
malinajirka Feb 4, 2026
8c261aa
Refactor poll attempt tracking in `WooPosFileBasedSyncAction`
malinajirka Feb 4, 2026
1e42304
Merge branch 'trunk' into woomob/add-file-based-catalog-analytics
malinajirka Mar 5, 2026
3da68ee
Refactor WooPosFileBasedSyncAction failure handling and improve track…
malinajirka Mar 5, 2026
cb6851f
Refactor error tracking in WooPosLocalCatalogSyncRepository to use re…
malinajirka Mar 5, 2026
bd140cd
Add tests to validate poll attempt persistence and failure details in…
malinajirka Mar 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class WooPosSearchByIdentifier @Inject constructor(

val localResult = localSearcher(identifier, syncStrategy)
// When product not found in local catalog, immediately return "not found" to avoid unnecessary remote call
if (localResult.isSuccess || syncStrategy == WooPosProductsDataSource.SyncStrategy.LOCAL_CATALOG) {
if (localResult.isSuccess || syncStrategy != WooPosProductsDataSource.SyncStrategy.REMOTE) {
return filterUnsupportedProductResult(localResult)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class WooPosSearchByIdentifierLocal @Inject constructor(
val siteId = selectedSite.get().localId()

return when (syncStrategy) {
SyncStrategy.LOCAL_CATALOG -> searchInLocalCatalog(identifier, siteId)
SyncStrategy.LOCAL_CATALOG,
SyncStrategy.LOCAL_CATALOG_FILE -> searchInLocalCatalog(identifier, siteId)
SyncStrategy.REMOTE -> searchInMemoryCache(identifier)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModel
import com.woocommerce.android.ui.woopos.common.data.models.WooPosProductModelMapper
import com.woocommerce.android.ui.woopos.common.data.models.WooPosWCProductToWooPosProductModelMapper
import com.woocommerce.android.ui.woopos.common.data.toWooPosVariation
import com.woocommerce.android.ui.woopos.featureflags.WooPosLocalCatalogFileApproachEnabled
import com.woocommerce.android.ui.woopos.home.items.search.WooPosProductsSearchInDbDataSource
import com.woocommerce.android.ui.woopos.home.items.search.WooPosSearchProductsDataSource
import com.woocommerce.android.ui.woopos.home.items.variations.WooPosVariationsLRUCache
Expand Down Expand Up @@ -56,17 +57,23 @@ class WooPosProductsDataSource @Inject constructor(
private val remoteDataSource: WooPosProductsRemoteDataSource,
private val localDbDataSource: WooPosProductsInDbDataSource,
private val syncStatusChecker: WooPosFullSyncStatusChecker,
private val fileApproachEnabled: WooPosLocalCatalogFileApproachEnabled,
) {
enum class SyncStrategy {
REMOTE,
LOCAL_CATALOG,
LOCAL_CATALOG_FILE,
}

private var activeSource: WooPosProductsDataSourceInterface? = null

fun getCurrentSyncStrategy(): SyncStrategy {
return when (activeSource) {
localDbDataSource -> SyncStrategy.LOCAL_CATALOG
localDbDataSource -> if (fileApproachEnabled()) {
SyncStrategy.LOCAL_CATALOG_FILE
} else {
SyncStrategy.LOCAL_CATALOG
}
remoteDataSource -> SyncStrategy.REMOTE
else -> error("Unknown sync strategy")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ class WooPosItemsSearchViewModel @Inject constructor(

private fun determinePullToRefreshState(): WooPosPullToRefreshState {
return when (dataSource.getCurrentSyncStrategy()) {
SyncStrategy.LOCAL_CATALOG -> WooPosPullToRefreshState.Enabled
SyncStrategy.LOCAL_CATALOG,
SyncStrategy.LOCAL_CATALOG_FILE -> WooPosPullToRefreshState.Enabled
SyncStrategy.REMOTE -> WooPosPullToRefreshState.Disabled
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.woocommerce.android.ui.woopos.localcatalog

import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper
import com.woocommerce.android.ui.woopos.util.datastore.WooPosPreferencesRepository
import com.woocommerce.android.ui.woopos.util.datastore.WooPosSyncTimestampManager
import kotlinx.coroutines.delay
import org.wordpress.android.fluxc.model.LocalOrRemoteId
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosGenerateCatalogResult
import org.wordpress.android.fluxc.store.pos.localcatalog.WooPosGenerateCatalogState
Expand All @@ -14,6 +17,8 @@ class WooPosFileBasedSyncAction @Inject constructor(
private val catalogFileDownloader: WooPosCatalogFileDownloader,
private val catalogFileParser: WooPosCatalogFileParser,
private val syncWithFts: WooPosLocalCatalogSyncWithFts,
private val preferencesRepository: WooPosPreferencesRepository,
private val syncTimestampManager: WooPosSyncTimestampManager,
private val logger: WooPosLogWrapper,
) {
companion object {
Expand All @@ -39,68 +44,121 @@ class WooPosFileBasedSyncAction @Inject constructor(
val startTime = System.currentTimeMillis()
logger.d("WooPosFileBasedSyncAction: Starting file-based catalog generation for site ${site.id}")

val siteId = site.localId()
val accumulatedPollAttempts = preferencesRepository.getAndClearFileBasedSyncPollAttempts(siteId)
var lastGenerationState: WooPosGenerateCatalogState? = null
Comment on lines +47 to +49
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAndClearFileBasedSyncPollAttempts() clears the persisted counter at the start of a run, but most non-timeout failure paths (e.g., consecutive network failures, download/parse/store failures, missing URL) return without re-persisting the updated count. This can drop previously accumulated attempts and defeats the “accumulated across app restarts” behavior. Consider reading without clearing, incrementing as you poll, and only clearing on successful completion (or re-saving the latest total on any early exit/failure).

Copilot uses AI. Check for mistakes.
var failedConsecutiveAttempts = 0

repeat(MAX_POLL_ATTEMPTS) { attemptCount ->
if (attemptCount > 0) {
val delayMs = computeBackoffDelay(attemptCount)
logger.d("WooPosFileBasedSyncAction: Waiting ${delayMs}ms before poll attempt $attemptCount")
repeat(MAX_POLL_ATTEMPTS) { attemptIndex ->
if (attemptIndex > 0) {
val delayMs = computeBackoffDelay(attemptIndex)
logger.d("WooPosFileBasedSyncAction: Waiting ${delayMs}ms before poll attempt $attemptIndex")
delay(delayMs)
}

val response = posLocalCatalogStore.generateCatalogOrGetStatus(site)

if (response.isFailure) {
if (++failedConsecutiveAttempts >= MAX_CONSECUTIVE_FAILED_ATTEMPTS) {
val error = response.exceptionOrNull()
logger.e(
"WooPosFileBasedSyncAction: File-based sync failed " +
"after $MAX_CONSECUTIVE_FAILED_ATTEMPTS consecutive failures"
)
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.NetworkError(
error?.message ?: "API error during catalog sync"
)
val totalPollAttempts = accumulatedPollAttempts + attemptIndex + 1
return handleConsecutiveFailures(
siteId,
totalPollAttempts,
lastGenerationState,
response.exceptionOrNull()
)
} else {
logger.w("Poll attempt $attemptCount failed: ${response.exceptionOrNull()?.message}")
logger.w("Poll attempt $attemptIndex failed: ${response.exceptionOrNull()?.message}")
return@repeat
}
}
failedConsecutiveAttempts = 0

val result = response.getOrThrow()
logger.d("WooPosFileBasedSyncAction: Poll attempt $attemptCount")
lastGenerationState = result.state
logger.d("WooPosFileBasedSyncAction: Poll attempt $attemptIndex, state: ${result.state}")

val processedResult = processPollingResult(result, site, startTime = startTime)
val totalPollAttempts = accumulatedPollAttempts + attemptIndex + 1
val processedResult = processPollingResult(
result,
site,
startTime,
totalPollAttempts
)
if (processedResult != null) {
if (processedResult is WooPosFileBasedSyncResult.Failure) {
preferencesRepository.setFileBasedSyncPollAttempts(siteId, totalPollAttempts)
return WooPosFileBasedSyncResult.Failure(
processedResult.result.withTrackingData(totalPollAttempts, lastGenerationState.rawValue)
)
}
return processedResult
}
}

logger.e("WooPosFileBasedSyncAction: Catalog generation timed out after $MAX_POLL_ATTEMPTS attempts")
return handleTimeout(
siteId = siteId,
totalPollAttempts = accumulatedPollAttempts + MAX_POLL_ATTEMPTS,
lastGenerationState = lastGenerationState
)
}

private suspend fun handleConsecutiveFailures(
siteId: LocalOrRemoteId.LocalId,
totalPollAttempts: Int,
lastGenerationState: WooPosGenerateCatalogState?,
error: Throwable?
): WooPosFileBasedSyncResult.Failure {
preferencesRepository.setFileBasedSyncPollAttempts(siteId, totalPollAttempts)
logger.e(
"WooPosFileBasedSyncAction: File-based sync failed " +
"after $MAX_CONSECUTIVE_FAILED_ATTEMPTS consecutive failures"
)
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.NetworkError(
error = error?.message ?: "API error during catalog sync",
pollAttempts = totalPollAttempts,
lastGenerationState = lastGenerationState?.rawValue
)
)
}

private suspend fun handleTimeout(
siteId: LocalOrRemoteId.LocalId,
totalPollAttempts: Int,
lastGenerationState: WooPosGenerateCatalogState?
): WooPosFileBasedSyncResult.Failure {
preferencesRepository.setFileBasedSyncPollAttempts(siteId, totalPollAttempts)

logger.e(
"WooPosFileBasedSyncAction: Catalog generation timed out after $totalPollAttempts total attempts. " +
"Last state: $lastGenerationState"
)
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.CatalogGenerationTimeout(
"Catalog generation is taking longer than expected."
error = "Catalog generation is taking longer than expected.",
lastGenerationState = lastGenerationState?.rawValue,
pollAttempts = totalPollAttempts
)
)
}

private suspend fun processPollingResult(
result: WooPosGenerateCatalogResult,
site: SiteModel,
startTime: Long
startTime: Long,
pollAttempts: Int
): WooPosFileBasedSyncResult? {
return when (result.state) {
WooPosGenerateCatalogState.COMPLETED -> {
if (result.url != null) {
logger.d("WooPosFileBasedSyncAction: Catalog available, starting download.")
processDownloadAndStore(result, site, startTime)
processDownloadAndStore(result, site, startTime, pollAttempts)
} else {
logger.e("WooPosFileBasedSyncAction: Catalog generation completed but URL is missing")
WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.InvalidResponse(
"Catalog generation completed but download URL is missing."
error = "Catalog generation completed but download URL is missing."
)
)
}
Expand All @@ -118,14 +176,15 @@ class WooPosFileBasedSyncAction @Inject constructor(
private suspend fun processDownloadAndStore(
result: WooPosGenerateCatalogResult,
site: SiteModel,
startTime: Long
startTime: Long,
pollAttempts: Int
): WooPosFileBasedSyncResult {
val downloadedFile = catalogFileDownloader.downloadCatalogFile(result.url!!, site.localId())
.onFailureLog("Failed to download catalog file")
.getOrElse {
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.NetworkError(
it.message ?: "Failed to download catalog file"
error = it.message ?: "Failed to download catalog file"
)
)
}
Expand All @@ -135,7 +194,7 @@ class WooPosFileBasedSyncAction @Inject constructor(
.getOrElse {
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.InvalidResponse(
it.message ?: "Failed to parse catalog file"
error = it.message ?: "Failed to parse catalog file"
)
)
}
Expand All @@ -148,7 +207,7 @@ class WooPosFileBasedSyncAction @Inject constructor(
.getOrElse {
return WooPosFileBasedSyncResult.Failure(
PosLocalCatalogSyncResult.Failure.DatabaseError(
it.message ?: "Failed to store catalog data"
error = it.message ?: "Failed to store catalog data"
)
)
}
Expand All @@ -161,20 +220,35 @@ class WooPosFileBasedSyncAction @Inject constructor(

catalogFileDownloader.cleanupOldCatalogFiles(keepLatest = downloadedFile)

return buildSuccessResult(result, parsedData, startTime, pollAttempts)
}

private fun buildSuccessResult(
result: WooPosGenerateCatalogResult,
parsedData: WooPosCatalogFileParser.ParsedCatalogData,
startTime: Long,
pollAttempts: Int
): WooPosFileBasedSyncResult.Success {
val syncDuration = System.currentTimeMillis() - startTime
val generationDuration = syncTimestampManager.calculateGenerationDuration(
scheduledAt = result.scheduledAt,
completedAt = result.completedAt
)

logger.d(
"WooPosFileBasedSyncAction: File-based sync completed successfully. " +
"Products: ${parsedData.products.size}, Variations: ${parsedData.variations.size}. " +
"Duration: ${syncDuration}ms."
"Duration: ${syncDuration}ms. Generation: ${generationDuration}ms. Poll attempts: $pollAttempts."
)

return WooPosFileBasedSyncResult.Success(
PosLocalCatalogSyncResult.Success(
productsSynced = parsedData.products.size,
variationsSynced = parsedData.variations.size,
syncDurationMs = syncDuration
syncDurationMs = syncDuration,
generationDurationMs = generationDuration,
pollAttempts = pollAttempts
),
// Using scheduledAt (not completedAt) to not miss changes made during generation
lastModifiedDate = result.scheduledAt
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ class WooPosLocalCatalogSyncRepository @Inject constructor(
variationsSynced = result.variationsSynced,
totalProducts = totalProducts,
totalVariations = totalVariations,
syncDurationMs = result.syncDurationMs
syncDurationMs = result.syncDurationMs,
generationDurationMs = result.generationDurationMs,
pollAttempts = result.pollAttempts
)
)
}
Expand All @@ -157,7 +159,9 @@ class WooPosLocalCatalogSyncRepository @Inject constructor(
syncType = syncType,
errorContext = "WooPosLocalCatalogSyncRepository",
errorType = errorType,
errorDescription = result.error
errorDescription = result.error,
lastGenerationState = result.lastGenerationState,
pollAttempts = result.pollAttempts
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,58 @@ sealed class PosLocalCatalogSyncResult {
data class Success(
val productsSynced: Int,
val variationsSynced: Int,
val syncDurationMs: Long
val syncDurationMs: Long,
val generationDurationMs: Long? = null,
val pollAttempts: Int? = null
) : PosLocalCatalogSyncResult()

sealed class Failure(val error: String) : PosLocalCatalogSyncResult() {
sealed class Failure(
val error: String,
val pollAttempts: Int? = null,
val lastGenerationState: String? = null
) : PosLocalCatalogSyncResult() {
class CatalogTooLarge(error: String) : Failure(error)
class NetworkError(error: String) : Failure(error)
class DatabaseError(error: String) : Failure(error)
class InvalidResponse(error: String) : Failure(error)
class UnexpectedError(error: String) : Failure(error)
class CatalogGenerationTimeout(error: String) : Failure(error)
class NetworkError(
error: String,
pollAttempts: Int? = null,
lastGenerationState: String? = null
) : Failure(error, pollAttempts, lastGenerationState)

class DatabaseError(
error: String,
pollAttempts: Int? = null,
lastGenerationState: String? = null
) : Failure(error, pollAttempts, lastGenerationState)

class InvalidResponse(
error: String,
pollAttempts: Int? = null,
lastGenerationState: String? = null
) : Failure(error, pollAttempts, lastGenerationState)

class UnexpectedError(
error: String,
pollAttempts: Int? = null,
lastGenerationState: String? = null
) : Failure(error, pollAttempts, lastGenerationState)

class CatalogGenerationTimeout(
error: String,
pollAttempts: Int? = null,
lastGenerationState: String? = null
) : Failure(error, pollAttempts, lastGenerationState)

fun withTrackingData(
pollAttempts: Int,
lastGenerationState: String?
): Failure = when (this) {
is NetworkError -> NetworkError(error, pollAttempts, lastGenerationState)
is DatabaseError -> DatabaseError(error, pollAttempts, lastGenerationState)
is InvalidResponse -> InvalidResponse(error, pollAttempts, lastGenerationState)
is CatalogGenerationTimeout -> CatalogGenerationTimeout(error, pollAttempts, lastGenerationState)
is CatalogTooLarge -> this
is UnexpectedError -> UnexpectedError(error, pollAttempts, lastGenerationState)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class WooPosSettingsCategoriesViewModel @Inject constructor(
init {
val categories = WooPosSettingsCategory.entries.filter {
if (it == WooPosSettingsCategory.LOCAL_CATALOG) {
productsDataSource.getCurrentSyncStrategy() == WooPosProductsDataSource.SyncStrategy.LOCAL_CATALOG
productsDataSource.getCurrentSyncStrategy() != WooPosProductsDataSource.SyncStrategy.REMOTE
} else {
true
}
Expand Down
Loading