Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -338,7 +338,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,12 +44,15 @@ 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)
}

Expand All @@ -63,39 +71,61 @@ class WooPosFileBasedSyncAction @Inject constructor(
)
)
} 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) {
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 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(
Expand All @@ -118,7 +148,8 @@ 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")
Expand Down Expand Up @@ -162,17 +193,24 @@ class WooPosFileBasedSyncAction @Inject constructor(
catalogFileDownloader.cleanupOldCatalogFiles(keepLatest = downloadedFile)

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 @@ -152,12 +154,16 @@ class WooPosLocalCatalogSyncRepository @Inject constructor(
is PosLocalCatalogSyncResult.Failure.CatalogGenerationTimeout -> SyncErrorType.CATALOG_GENERATION_TIMEOUT
}

val timeoutResult = result as? PosLocalCatalogSyncResult.Failure.CatalogGenerationTimeout

analyticsTracker.track(
LocalCatalogSyncFailed(
syncType = syncType,
errorContext = "WooPosLocalCatalogSyncRepository",
errorType = errorType,
errorDescription = result.error
errorDescription = result.error,
lastGenerationState = timeoutResult?.lastGenerationState,
pollAttempts = timeoutResult?.pollAttempts
Comment on lines +157 to +166
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.

The new failure analytics properties (lastGenerationState, pollAttempts) are only populated when the failure is CatalogGenerationTimeout (via the cast). Other file-based sync failures will still emit local_catalog_sync_failed without these properties, which doesn’t match the PR description (“times out or fails”). Either propagate these properties for other failure types too (e.g., by carrying pollAttempts/last state in a shared failure type) or adjust the PR description to clarify it’s timeout-only.

Copilot uses AI. Check for mistakes.
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ 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() {
Expand All @@ -16,7 +18,11 @@ sealed class PosLocalCatalogSyncResult {
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 CatalogGenerationTimeout(
error: String,
val lastGenerationState: String? = null,
val pollAttempts: Int? = null
) : Failure(error)
}
}

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
Original file line number Diff line number Diff line change
Expand Up @@ -438,41 +438,47 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent {
val variationsSynced: Int,
val totalProducts: Int,
val totalVariations: Int,
val syncDurationMs: Long
val syncDurationMs: Long,
val generationDurationMs: Long? = null,
val pollAttempts: Int? = null
) : Event() {
override val name: String = "local_catalog_sync_completed"

init {
addProperties(
mapOf(
SyncType.SYNC_TYPE to syncType.toString(),
"products_synced" to productsSynced.toString(),
"variations_synced" to variationsSynced.toString(),
"total_products" to totalProducts.toString(),
"total_variations" to totalVariations.toString(),
"sync_duration_ms" to syncDurationMs.toString()
)
val properties = mutableMapOf(
SyncType.SYNC_TYPE to syncType.toString(),
"products_synced" to productsSynced.toString(),
"variations_synced" to variationsSynced.toString(),
"total_products" to totalProducts.toString(),
"total_variations" to totalVariations.toString(),
"sync_duration_ms" to syncDurationMs.toString()
)
generationDurationMs?.let { properties["generation_duration_ms"] = it.toString() }
pollAttempts?.let { properties["poll_attempts"] = it.toString() }
addProperties(properties)
}
}

data class LocalCatalogSyncFailed(
val syncType: SyncType,
val errorContext: String,
val errorType: SyncErrorType,
val errorDescription: String
val errorDescription: String,
val lastGenerationState: String? = null,
val pollAttempts: Int? = null
) : Event() {
override val name: String = "local_catalog_sync_failed"

init {
addProperties(
mapOf(
SyncType.SYNC_TYPE to syncType.toString(),
"error_context" to errorContext,
SyncErrorType.ERROR_TYPE to errorType.toString(),
"error_description" to errorDescription
)
val properties = mutableMapOf(
SyncType.SYNC_TYPE to syncType.toString(),
"error_context" to errorContext,
SyncErrorType.ERROR_TYPE to errorType.toString(),
"error_description" to errorDescription
)
lastGenerationState?.let { properties["last_generation_state"] = it }
pollAttempts?.let { properties["poll_attempts"] = it.toString() }
addProperties(properties)
}
}

Expand Down Expand Up @@ -1138,5 +1144,6 @@ internal fun SyncStrategy.toAnalyticsValue(): String {
return when (this) {
SyncStrategy.REMOTE -> "remote"
SyncStrategy.LOCAL_CATALOG -> "local_catalog"
SyncStrategy.LOCAL_CATALOG_FILE -> "local_catalog_file"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import androidx.datastore.core.DataStore
import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.booleanPreferencesKey
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.core.intPreferencesKey
import androidx.datastore.preferences.core.longPreferencesKey
import androidx.datastore.preferences.core.stringPreferencesKey
import com.woocommerce.android.tools.SelectedSite
Expand Down Expand Up @@ -128,8 +129,34 @@ class WooPosPreferencesRepository @Inject constructor(
}
}

suspend fun getFileBasedSyncPollAttempts(siteId: LocalOrRemoteId.LocalId): Int {
val key = buildFileBasedSyncPollAttemptsKey(siteId)
return dataStore.data.map { it[key] ?: 0 }.first()
}

suspend fun setFileBasedSyncPollAttempts(siteId: LocalOrRemoteId.LocalId, attempts: Int) {
val key = buildFileBasedSyncPollAttemptsKey(siteId)
dataStore.edit { preferences ->
preferences[key] = attempts
}
}

suspend fun getAndClearFileBasedSyncPollAttempts(siteId: LocalOrRemoteId.LocalId): Int {
val key = buildFileBasedSyncPollAttemptsKey(siteId)
var attempts = 0
dataStore.edit { preferences ->
attempts = preferences[key] ?: 0
preferences.remove(key)
}
return attempts
}

private fun buildPeriodicSyncEnabledKey(siteId: LocalOrRemoteId.LocalId): Preferences.Key<Boolean> =
booleanPreferencesKey("pos_periodic_sync_enabled_v2_${siteId.value}")

private fun buildFileBasedSyncPollAttemptsKey(siteId: LocalOrRemoteId.LocalId): Preferences.Key<Int> =
intPreferencesKey("pos_file_based_sync_poll_attempts_${siteId.value}")

private fun buildSiteSpecificKey(key: String): Preferences.Key<String> =
stringPreferencesKey("${selectedSite.getOrNull()?.id}_v2_$key")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ class WooPosSyncTimestampManager @Inject constructor(
return parseGmtTimestamp(dateFromApi)
}

fun calculateGenerationDuration(scheduledAt: String?, completedAt: String?): Long? {
if (scheduledAt == null || completedAt == null) return null
val scheduledTs = parseGmtTimestamp(scheduledAt) ?: return null
val completedTs = parseGmtTimestamp(completedAt) ?: return null
return completedTs - scheduledTs
}

private fun parseGmtTimestamp(dateFromApi: String): Long? {
for (formatter in PARSING_FORMATTERS) {
try {
Expand Down
Loading
Loading