[Bookings] Refactor caching strategy to handle stale data cleanup for all filters

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

@@ -15,7 +15,6 @@ import org.wordpress.android.fluxc.store.WCOrderStore
 import org.wordpress.android.fluxc.tools.CoroutineEngine
 import org.wordpress.android.fluxc.utils.HeadersParser
 import org.wordpress.android.util.AppLog
-import java.time.Clock
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -27,7 +26,6 @@ class BookingsStore @Inject internal constructor(
     private val bookingDtoMapper: BookingDtoMapper,
     private val headersParser: HeadersParser,
     private val coroutineEngine: CoroutineEngine,
-    private val clock: Clock,
 ) {
     suspend fun fetchBookings(
         site: SiteModel,
@@ -56,31 +54,23 @@ class BookingsStore @Inject internal constructor(
                             )
                         }
                     }
-                    // Clear existing bookings when fetching the first page.
-                    // If filters are applied, only clear entries that match the applied filters,
-                    // otherwise (no filters) clear all entries for the site.
-                    if (page == 1 && query.isNullOrEmpty()) {
-                        when {
-                            filters == BookingFilters.EMPTY -> {
-                                bookingsDao.replaceAllForSite(site.localId(), entities)
-                            }
 
-                            filters.dateRange != null && filters.isTodayOrUpcoming -> {
-                                bookingsDao.cleanAndUpsertBookings(
-                                    site.localId(),
-                                    filters.dateRange,
-                                    entities
-                                )
-                            }
+                    when {
+                        page == 1 && query.isNullOrEmpty() && filters == BookingFilters.EMPTY -> {
+                            // Refreshing with no filters applied: perform a bulk delete.
+                            bookingsDao.replaceAllForSite(site.localId(), entities)
+                        }
 
-                            else -> {
-                                // For any other filters, avoid deletions to prevent removing unrelated cached items
-                                bookingsDao.insertOrReplace(entities)
-                            }
+                        page == 1 && query.isNullOrEmpty() -> {
+                            // Refreshing with filters applied: selectively remove only the stale entries.
+                            bookingsDao.cleanAndUpsertBookings(site.localId(), filters, entities)
+                        }
+
+                        else -> {
+                            bookingsDao.upsert(entities)
                         }
-                    } else {
-                        bookingsDao.insertOrReplace(entities)
                     }
+
                     val totalPages = headersParser.getTotalPages(response.headers)
                     // Determine if we can load more from the total pages header if available, otherwise
                     // infer it from the number of items returned
@@ -98,13 +88,6 @@ class BookingsStore @Inject internal constructor(
         }
     }
 
-    private val BookingFilters.isTodayOrUpcoming: Boolean
-        get() {
-            val today = BookingsDateRangePresets.today(clock)
-            val upcoming = BookingsDateRangePresets.upcoming(clock)
-            return (dateRange == today || dateRange == upcoming) && enabledFiltersCount == 1
-        }
-
     fun observeBookings(
         site: SiteModel,
         limit: Int? = null,
@@ -150,7 +133,7 @@ class BookingsStore @Inject internal constructor(
                             orderEntity = orderResult?.model,
                         )
                     }
-                    bookingsDao.insertOrReplace(listOf(entity))
+                    bookingsDao.upsert(listOf(entity))
                     WooResult(entity)
                 }
 
@@ -186,7 +169,7 @@ class BookingsStore @Inject internal constructor(
                 val entity = with(bookingDtoMapper) {
                     response.result.toEntity(site.localId())
                 }
-                bookingsDao.insertOrReplace(entity)
+                bookingsDao.upsert(entity)
                 WooResult(entity)
             }
 
@@ -230,7 +213,7 @@ class BookingsStore @Inject internal constructor(
                     if (updatedBookingEntity == null) {
                         return@withDefaultContext WooResult(WooError(GENERIC_ERROR, UNKNOWN))
                     } else {
-                        bookingsDao.insertOrReplace(updatedBookingEntity)
+                        bookingsDao.upsert(updatedBookingEntity)
                         return@withDefaultContext WooResult(updatedBookingEntity)
                     }
                 }

libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/persistence/dao/BookingsDao.kt

@@ -8,7 +8,6 @@ import androidx.room.Transaction
 import kotlinx.coroutines.flow.Flow
 import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingFilters
-import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsOrderOption
 import org.wordpress.android.fluxc.persistence.entity.BookingEntity
 import org.wordpress.android.fluxc.persistence.entity.BookingResourceEntity
@@ -57,18 +56,18 @@ interface BookingsDao {
     fun observeBookingsCount(localSiteId: LocalId): Flow<Long>
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
-    suspend fun insertOrReplace(entity: BookingEntity): Long
+    suspend fun upsert(entity: BookingEntity): Long
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
-    suspend fun insertOrReplace(entities: List<BookingEntity>)
+    suspend fun upsert(entities: List<BookingEntity>)
 
     @Query("DELETE FROM Bookings WHERE localSiteId = :localSiteId")
     suspend fun deleteAllForSite(localSiteId: LocalId)
 
     @Transaction
     suspend fun replaceAllForSite(siteId: LocalId, entities: List<BookingEntity>) {
         deleteAllForSite(siteId)
-        insertOrReplace(entities)
+        upsert(entities)
     }
 
     @Suppress("LongParameterList")
@@ -78,46 +77,69 @@ interface BookingsDao {
             WHERE localSiteId = :localSiteId
             AND (:startDateBefore IS NULL OR start <= :startDateBefore)
             AND (:startDateAfter IS NULL OR start >= :startDateAfter)
-            AND ((:idsSize = 0) OR id NOT IN (:ids))
+            AND (:customerId IS NULL OR customerId = :customerId)
+            AND ((:attendanceStatusesSize = 0) OR attendanceStatus IN (:attendanceStatuses))
+            AND ((:resourceIdsSize = 0) OR resourceId IN (:resourceIds))
+            AND ((:productIdsSize = 0) OR productId IN (:productIds))
+            AND ((:keepIdsSize = 0) OR id NOT IN (:keepIds))
         """
     )
-    suspend fun deleteForSiteWithDateRangeFilter(
+    suspend fun deleteStaleBookings(
         localSiteId: LocalId,
         startDateBefore: Long?,
         startDateAfter: Long?,
-        ids: List<Long>,
-        idsSize: Int,
+        customerId: Long?,
+        resourceIds: List<Long>,
+        resourceIdsSize: Int,
+        attendanceStatuses: List<String>,
+        attendanceStatusesSize: Int,
+        productIds: List<Long>,
+        productIdsSize: Int,
+        keepIds: List<Long>,
+        keepIdsSize: Int,
     )
 
-    private suspend fun deleteForSiteWithDateRangeFilter(
+    private suspend fun deleteStaleBookings(
         localSiteId: LocalId,
-        dateRange: BookingsFilterOption.DateRange,
-        ids: List<Long>
+        filters: BookingFilters,
+        keepIds: List<Long>
     ) {
-        deleteForSiteWithDateRangeFilter(
+        val resourceIdsKeySet = filters.teamMembers.values.map { it.value }
+        val attendanceStatusKeySet = filters.attendanceStatuses.values.map { it.key }
+        val productIds = filters.serviceEvents.values.map { it.productId }
+
+        deleteStaleBookings(
             localSiteId = localSiteId,
-            startDateBefore = dateRange.before?.epochSecond,
-            startDateAfter = dateRange.after?.epochSecond,
-            ids = ids,
-            idsSize = ids.size,
+            startDateBefore = filters.dateRange.before?.epochSecond,
+            startDateAfter = filters.dateRange.after?.epochSecond,
+            customerId = filters.customer?.customerId,
+            resourceIds = resourceIdsKeySet.toList(),
+            resourceIdsSize = resourceIdsKeySet.size,
+            attendanceStatuses = attendanceStatusKeySet.toList(),
+            attendanceStatusesSize = attendanceStatusKeySet.size,
+            productIds = productIds,
+            productIdsSize = productIds.size,
+            keepIds = keepIds,
+            keepIdsSize = keepIds.size
         )
     }
 
     /**
-     * Delete Booking entities that are not present in the new list and then insert the new entities
+     * Delete Booking entities that match the filters but are not present in the new list,
+     * and then insert the new entities.
      */
     @Transaction
     suspend fun cleanAndUpsertBookings(
         localSiteId: LocalId,
-        dateRange: BookingsFilterOption.DateRange,
+        filters: BookingFilters,
         entities: List<BookingEntity>,
     ) {
-        deleteForSiteWithDateRangeFilter(
+        deleteStaleBookings(
             localSiteId = localSiteId,
-            dateRange = dateRange,
-            ids = entities.map { it.id.value },
+            filters = filters,
+            keepIds = entities.map { it.id.value },
         )
-        insertOrReplace(entities)
+        upsert(entities)
     }
 
     fun observeBookings(
@@ -161,17 +183,17 @@ interface BookingsDao {
     suspend fun getResource(localSiteId: LocalId, resourceId: Long): BookingResourceEntity?
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
-    suspend fun insertOrReplace(resource: BookingResourceEntity): Long
+    suspend fun upsert(resource: BookingResourceEntity): Long
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
-    suspend fun insertOrReplaceResources(resources: List<BookingResourceEntity>)
+    suspend fun upsertResources(resources: List<BookingResourceEntity>)
 
     @Query("DELETE FROM BookingResources WHERE localSiteId = :localSiteId")
     suspend fun deleteAllResourcesForSite(localSiteId: LocalId)
 
     @Transaction
     suspend fun replaceAllResourcesForSite(siteId: LocalId, resources: List<BookingResourceEntity>) {
         deleteAllResourcesForSite(siteId)
-        insertOrReplaceResources(resources)
+        upsertResources(resources)
     }
 }

libs/fluxc-plugin/src/test/java/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsStoreTest.kt

@@ -50,7 +50,6 @@ class BookingsStoreTest {
             bookingDtoMapper = bookingDtoMapper,
             headersParser = headersParser,
             coroutineEngine = CoroutineEngine(EmptyCoroutineContext, mock()),
-            clock = clock,
         )
     }
 
@@ -64,7 +63,7 @@ class BookingsStoreTest {
             whenever(bookingsDao.getBooking(TEST_LOCAL_SITE_ID, dto.id)).thenReturn(storedBooking)
             whenever(bookingsRestClient.updateBooking(site, dto.id, BookingUpdatePayload(note = "n")))
                 .thenReturn(WooPayload(dto))
-            whenever(bookingsDao.insertOrReplace(any<BookingEntity>())).thenReturn(1L)
+            whenever(bookingsDao.upsert(any<BookingEntity>())).thenReturn(1L)
 
             // when
             val result = sut.updateBooking(
@@ -78,7 +77,7 @@ class BookingsStoreTest {
             assertThat(result.isError).isFalse()
             assertThat(result.model).isNotNull
             // The store preserves the stored order on the mapped entity
-            verify(bookingsDao).insertOrReplace(argThat<BookingEntity> { this.order == storedBooking.order })
+            verify(bookingsDao).upsert(argThat<BookingEntity> { this.order == storedBooking.order })
         }
 
     @Test
@@ -92,7 +91,7 @@ class BookingsStoreTest {
             whenever(orderStore.fetchSingleOrderSync(site, dto.orderId)).thenReturn(WooResult(fetchedOrder))
             whenever(bookingsRestClient.updateBooking(site, dto.id, BookingUpdatePayload(status = Status.Confirmed)))
                 .thenReturn(WooPayload(dto))
-            whenever(bookingsDao.insertOrReplace(any<BookingEntity>())).thenReturn(1L)
+            whenever(bookingsDao.upsert(any<BookingEntity>())).thenReturn(1L)
 
             // when
             val result = sut.updateBooking(
@@ -106,7 +105,7 @@ class BookingsStoreTest {
             assertThat(result.isError).isFalse()
             val expected = with(bookingDtoMapper) { dto.toEntity(TEST_LOCAL_SITE_ID, fetchedOrder) }
             assertThat(result.model).isEqualTo(expected)
-            verify(bookingsDao).insertOrReplace(expected)
+            verify(bookingsDao).upsert(expected)
             verify(bookingsDao, never()).getBooking(TEST_LOCAL_SITE_ID, dto.id)
         }
 
@@ -128,7 +127,7 @@ class BookingsStoreTest {
 
         // then
         assertThat(result.isError).isTrue()
-        verify(bookingsDao, never()).insertOrReplace(any<BookingEntity>())
+        verify(bookingsDao, never()).upsert(any<BookingEntity>())
     }
 
     @Test
@@ -171,7 +170,7 @@ class BookingsStoreTest {
             assertThat(result.isError).isFalse
             verify(bookingsDao).getBooking(TEST_LOCAL_SITE_ID, dto.id)
             // The store preserves the stored order on the mapped entity
-            verify(bookingsDao).insertOrReplace(argThat<BookingEntity> { this.order == storedBooking.order })
+            verify(bookingsDao).upsert(argThat<BookingEntity> { this.order == storedBooking.order })
         }
 
     @Test
@@ -210,14 +209,14 @@ class BookingsStoreTest {
             // We delete only matching filtered rows then insert the fetched page
             verify(bookingsDao).cleanAndUpsertBookings(
                 any(),
-                any<BookingsFilterOption.DateRange>(),
+                any<BookingFilters>(),
                 any<List<BookingEntity>>()
             )
             verify(bookingsDao, never()).replaceAllForSite(any(), any())
         }
 
     @Test
-    fun `given page 1, today date filter, customer filter and no query, when fetchBookings, then only insertOrReplace is called`(): Unit =
+    fun `given page 1, today date filter, customer filter and no query, when fetchBookings, then cleanAndUpsertBookings is called`(): Unit =
         runBlocking {
             // given
             val site = SiteModel().apply { id = TEST_LOCAL_SITE_ID.value }
@@ -251,12 +250,12 @@ class BookingsStoreTest {
             // then
             assertThat(result.isError).isFalse()
             // We delete only matching filtered rows then insert the fetched page
-            verify(bookingsDao).insertOrReplace(any<List<BookingEntity>>())
-            verify(bookingsDao, never()).cleanAndUpsertBookings(
+            verify(bookingsDao).cleanAndUpsertBookings(
                 any(),
-                any<BookingsFilterOption.DateRange>(),
+                any<BookingFilters>(),
                 any<List<BookingEntity>>()
             )
+            verify(bookingsDao, never()).upsert(any<List<BookingEntity>>())
             verify(bookingsDao, never()).replaceAllForSite(any(), any())
         }
 
@@ -296,7 +295,7 @@ class BookingsStoreTest {
             // We delete only matching filtered rows then insert the fetched page
             verify(bookingsDao).cleanAndUpsertBookings(
                 any(),
-                any<BookingsFilterOption.DateRange>(),
+                any<BookingFilters>(),
                 any<List<BookingEntity>>()
             )
             verify(bookingsDao, never()).replaceAllForSite(any(), any())
@@ -337,10 +336,10 @@ class BookingsStoreTest {
 
             // then
             assertThat(result.isError).isFalse()
-            verify(bookingsDao).insertOrReplace(any<List<BookingEntity>>())
+            verify(bookingsDao).upsert(any<List<BookingEntity>>())
             verify(bookingsDao, never()).replaceAllForSite(any(), any())
             verify(bookingsDao, never())
-                .cleanAndUpsertBookings(any(), any<BookingsFilterOption.DateRange>(), any<List<BookingEntity>>())
+                .cleanAndUpsertBookings(any(), any<BookingFilters>(), any<List<BookingEntity>>())
         }
 
     @Test
@@ -377,13 +376,13 @@ class BookingsStoreTest {
             verify(bookingsDao).replaceAllForSite(any(), any())
             verify(bookingsDao, never()).cleanAndUpsertBookings(
                 any(),
-                any<BookingsFilterOption.DateRange>(),
+                any<BookingFilters>(),
                 any<List<BookingEntity>>()
             )
         }
 
     @Test
-    fun `given page 1 with custom date range filter and no query, when fetchBookings, then only insertOrReplace is called`(): Unit =
+    fun `given page 1 with custom date range filter and no query, when fetchBookings, then cleanAndUpsertBookings is called`(): Unit =
         runBlocking {
             // given
             val site = SiteModel().apply { id = TEST_LOCAL_SITE_ID.value }
@@ -416,11 +415,9 @@ class BookingsStoreTest {
 
             // then
             assertThat(result.isError).isFalse()
-            verify(bookingsDao).insertOrReplace(any<List<BookingEntity>>())
-            verify(bookingsDao, never()).replaceAllForSite(any(), any())
-            verify(bookingsDao, never()).cleanAndUpsertBookings(
+            verify(bookingsDao).cleanAndUpsertBookings(
                 any(),
-                any<BookingsFilterOption.DateRange>(),
+                any<BookingFilters>(),
                 any<List<BookingEntity>>()
             )
         }