diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt index 0dbb370705..1ed90f9d0b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt @@ -200,7 +200,7 @@ class ConversationViewModel( private val startExpiration = BehaviorSubject.create() - private val _jumpToDateValidator: JumpToDateValidator by lazy { JumpToDateValidator(threadId) } + private val _jumpToDateValidator: JumpToDateValidator by lazy { JumpToDateValidator.create(threadId) } val jumpToDateValidator: JumpToDateValidator get() = _jumpToDateValidator diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidator.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidator.kt index 5c2b852839..8b34989d73 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidator.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidator.kt @@ -9,8 +9,6 @@ import com.google.android.material.datepicker.CalendarConstraints.DateValidator import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import org.signal.core.util.concurrent.SignalExecutors -import org.signal.core.util.logTime -import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.database.SignalDatabase import org.thoughtcrime.securesms.util.LRUCache import java.time.Instant @@ -18,90 +16,136 @@ import java.time.LocalDate import java.time.LocalDateTime import java.time.ZoneId import java.time.temporal.TemporalAdjusters -import java.util.concurrent.locks.Condition -import java.util.concurrent.locks.ReentrantLock -import kotlin.concurrent.withLock +import java.util.concurrent.Executor import kotlin.time.Duration.Companion.days +/** + * Interface for looking up whether messages exist on given dates. + * Allows for easy testing without database dependencies. + */ +private typealias MessageDateLookup = (Collection) -> Map + /** * A calendar validator for jumping to a specific date in a conversation. * This is used to prevent the user from jumping to a date where there are no messages. * * [isValid] is called on the main thread, so we try to race it and fetch the data ahead of time, fetching data in bulk and caching it. + * If data is not yet cached, we return true to avoid blocking the main thread. */ @Parcelize -class JumpToDateValidator(val threadId: Long) : DateValidator { +class JumpToDateValidator private constructor( + private val threadId: Long, + @IgnoredOnParcel private val messageExistanceLookup: MessageDateLookup = createDefaultLookup(threadId), + @IgnoredOnParcel private val executor: Executor = SignalExecutors.BOUNDED, + private val zoneId: ZoneId = ZoneId.systemDefault() +) : DateValidator { companion object { - private val TAG = Log.tag(JumpToDateValidator::class.java) + + fun createDefaultLookup(threadId: Long): MessageDateLookup { + return { dayStarts -> SignalDatabase.messages.messageExistsOnDays(threadId, dayStarts) } + } + + @JvmStatic + fun create(threadId: Long): JumpToDateValidator { + return JumpToDateValidator( + threadId = threadId, + messageExistanceLookup = createDefaultLookup(threadId), + executor = SignalExecutors.BOUNDED, + zoneId = ZoneId.systemDefault() + ).also { + it.performInitialPrefetch() + } + } + + @JvmStatic + fun createForTesting(lookup: MessageDateLookup, executor: Executor, zoneId: ZoneId): JumpToDateValidator { + return JumpToDateValidator( + threadId = -1, + messageExistanceLookup = lookup, + executor = executor, + zoneId = zoneId + ).also { + it.performInitialPrefetch() + } + } } @IgnoredOnParcel - private val lock = ReentrantLock() + private val cachedDates: MutableMap = LRUCache(1000) @IgnoredOnParcel - private val condition: Condition = lock.newCondition() - - @IgnoredOnParcel - private val cachedDates: MutableMap = LRUCache(500) - - init { - val startOfDay = LocalDate.now(ZoneId.systemDefault()) - .atStartOfDay(ZoneId.systemDefault()) - .toInstant() - .toEpochMilli() - loadAround(startOfDay, allowPrefetch = true) - } + private val pendingMonths: MutableSet = mutableSetOf() override fun isValid(dateStart: Long): Boolean { - return lock.withLock { - val localMidnightTimestamp = Instant.ofEpochMilli(dateStart) - .atZone(ZoneId.systemDefault()) - .toLocalDate() - .atStartOfDay(ZoneId.systemDefault()) - .toInstant() - .toEpochMilli() + val localMidnightTimestamp = normalizeToLocalMidnight(dateStart) - var value = cachedDates[localMidnightTimestamp] - - while (value == null || value == LookupState.PENDING) { - loadAround(localMidnightTimestamp, allowPrefetch = true) - condition.await() - value = cachedDates[localMidnightTimestamp] + return synchronized(this) { + when (cachedDates[localMidnightTimestamp]) { + LookupState.FOUND -> true + LookupState.NOT_FOUND -> false + LookupState.PENDING, null -> { + loadAround(localMidnightTimestamp, allowPrefetch = true) + true + } } - - cachedDates[localMidnightTimestamp] == LookupState.FOUND } } + private fun performInitialPrefetch() { + val today = LocalDate.now(zoneId) + val monthsToPrefetch = 6 + + for (i in 0 until monthsToPrefetch) { + val targetMonth = today.minusMonths(i.toLong()).atStartOfDay(zoneId).toInstant().toEpochMilli() + loadAround(targetMonth, allowPrefetch = false) + } + } + + private fun normalizeToLocalMidnight(timestamp: Long): Long { + return Instant.ofEpochMilli(timestamp) + .atZone(zoneId) + .toLocalDate() + .atStartOfDay(zoneId) + .toInstant() + .toEpochMilli() + } + /** * Given a date, this will load all of the dates for entire month the date is in. */ private fun loadAround(dateStart: Long, allowPrefetch: Boolean) { - SignalExecutors.BOUNDED.execute { - val startOfDay = LocalDateTime.ofInstant(Instant.ofEpochMilli(dateStart), ZoneId.systemDefault()) + val startOfDay = LocalDateTime.ofInstant(Instant.ofEpochMilli(dateStart), zoneId) - val startOfMonth = startOfDay - .with(TemporalAdjusters.firstDayOfMonth()) - .atZone(ZoneId.systemDefault()) - .toLocalDate() - .atStartOfDay(ZoneId.systemDefault()) - .toInstant() - .toEpochMilli() + val startOfMonth = startOfDay + .with(TemporalAdjusters.firstDayOfMonth()) + .atZone(zoneId) + .toLocalDate() + .atStartOfDay(zoneId) + .toInstant() + .toEpochMilli() - val endOfMonth = startOfDay - .with(TemporalAdjusters.lastDayOfMonth()) - .atZone(ZoneId.systemDefault()) - .toLocalDate() - .atStartOfDay(ZoneId.systemDefault()) - .toInstant() - .toEpochMilli() + val endOfMonth = startOfDay + .with(TemporalAdjusters.lastDayOfMonth()) + .atZone(zoneId) + .toLocalDate() + .atStartOfDay(zoneId) + .toInstant() + .toEpochMilli() + synchronized(this) { + if (pendingMonths.contains(startOfMonth)) { + return + } + pendingMonths.add(startOfMonth) + } + + executor.execute { val daysOfMonth = (startOfMonth..endOfMonth step 1.days.inWholeMilliseconds).toSet() + dateStart - val lookupsNeeded = lock.withLock { + val lookupsNeeded = synchronized(this) { daysOfMonth - .filter { !cachedDates.containsKey(it) } + .filter { cachedDates[it] == null } .onEach { cachedDates[it] = LookupState.PENDING } } @@ -109,26 +153,18 @@ class JumpToDateValidator(val threadId: Long) : DateValidator { return@execute } - val existence = logTime(TAG, "query(${lookupsNeeded.size})", decimalPlaces = 2) { - SignalDatabase.messages.messageExistsOnDays(threadId, lookupsNeeded) - } + val existence = messageExistanceLookup.invoke(lookupsNeeded) - lock.withLock { + synchronized(this) { cachedDates.putAll(existence.mapValues { if (it.value) LookupState.FOUND else LookupState.NOT_FOUND }) if (allowPrefetch) { val dayInPreviousMonth = startOfMonth - 1.days.inWholeMilliseconds - if (!cachedDates.containsKey(dayInPreviousMonth)) { - loadAround(dayInPreviousMonth, allowPrefetch = false) - } + loadAround(dayInPreviousMonth, allowPrefetch = false) val dayInNextMonth = endOfMonth + 1.days.inWholeMilliseconds - if (!cachedDates.containsKey(dayInNextMonth)) { - loadAround(dayInNextMonth, allowPrefetch = false) - } + loadAround(dayInNextMonth, allowPrefetch = false) } - - condition.signalAll() } } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidatorTest.kt b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidatorTest.kt new file mode 100644 index 0000000000..c3e5d57b40 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/JumpToDateValidatorTest.kt @@ -0,0 +1,190 @@ +package org.thoughtcrime.securesms.conversation.v2 + +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.isEqualTo +import assertk.assertions.isFalse +import assertk.assertions.isGreaterThanOrEqualTo +import assertk.assertions.isNotEmpty +import assertk.assertions.isTrue +import org.junit.Test +import java.time.LocalDate +import java.time.ZoneId +import java.util.concurrent.Executor +import java.util.concurrent.atomic.AtomicInteger + +class JumpToDateValidatorTest { + + private val zoneId = ZoneId.of("UTC") + private val immediateExecutor = Executor { it.run() } + + private fun timestampForDate(year: Int, month: Int, day: Int): Long { + return LocalDate.of(year, month, day) + .atStartOfDay(zoneId) + .toInstant() + .toEpochMilli() + } + + private fun createValidator( + lookup: (Collection) -> Map = { emptyMap() }, + executor: Executor = immediateExecutor, + zone: ZoneId = zoneId + ): JumpToDateValidator { + return JumpToDateValidator.createForTesting(lookup, executor, zone) + } + + @Test + fun `when date is not cached, returns true`() { + val neverExecutor = Executor { /* do nothing */ } + val lookup = { _: Collection -> emptyMap() } + val validator = createValidator(lookup, neverExecutor) + + val result = validator.isValid(timestampForDate(2024, 6, 15)) + + assertThat(result).isTrue() + } + + @Test + fun `when date has message after prefetch, returns true`() { + val dateWithMessage = timestampForDate(2024, 6, 15) + val lookup = { dates: Collection -> + dates.associateWith { it == dateWithMessage } + } + val validator = createValidator(lookup) + + validator.isValid(dateWithMessage) + + assertThat(validator.isValid(dateWithMessage)).isTrue() + } + + @Test + fun `when date has no message after prefetch, returns false`() { + val dateWithoutMessage = timestampForDate(2024, 6, 15) + val lookup = { dates: Collection -> + dates.associateWith { false } + } + val validator = createValidator(lookup) + + validator.isValid(dateWithoutMessage) + + assertThat(validator.isValid(dateWithoutMessage)).isFalse() + } + + @Test + fun `loads entire month of dates at once`() { + val queriedDates = mutableListOf>() + val lookup = { dates: Collection -> + queriedDates.add(dates) + dates.associateWith { false } + } + val validator = createValidator(lookup) + + validator.isValid(timestampForDate(2024, 6, 15)) + + assertThat(queriedDates).isNotEmpty() + + val june1 = timestampForDate(2024, 6, 1) + val june30 = timestampForDate(2024, 6, 30) + val allQueriedTimestamps = queriedDates.flatten().toSet() + assertThat(allQueriedTimestamps).contains(june1) + assertThat(allQueriedTimestamps).contains(june30) + + val juneQuery = queriedDates.find { it.contains(june1) }!! + assertThat(juneQuery.size).isGreaterThanOrEqualTo(30) + } + + @Test + fun `prefetches adjacent months`() { + val queriedDates = mutableListOf>() + val lookup = { dates: Collection -> + queriedDates.add(dates) + dates.associateWith { false } + } + val validator = createValidator(lookup) + + validator.isValid(timestampForDate(2024, 6, 15)) + + assertThat(queriedDates.size).isGreaterThanOrEqualTo(2) + + val allQueriedTimestamps = queriedDates.flatten().toSet() + val may15 = timestampForDate(2024, 5, 15) + val july15 = timestampForDate(2024, 7, 15) + assertThat(allQueriedTimestamps).contains(may15) + assertThat(allQueriedTimestamps).contains(july15) + } + + @Test + fun `does not re-query same month`() { + val queryCount = AtomicInteger(0) + val lookup = { dates: Collection -> + queryCount.incrementAndGet() + dates.associateWith { false } + } + val validator = createValidator(lookup) + + validator.isValid(timestampForDate(2024, 6, 15)) + val initialCount = queryCount.get() + + validator.isValid(timestampForDate(2024, 6, 1)) + validator.isValid(timestampForDate(2024, 6, 30)) + + assertThat(queryCount.get()).isEqualTo(initialCount) + } + + @Test + fun `handles timestamp normalization to local midnight`() { + val june15Midnight = timestampForDate(2024, 6, 15) + val june15Noon = june15Midnight + (12 * 60 * 60 * 1000) + val june15Evening = june15Midnight + (20 * 60 * 60 * 1000) + + val lookup = { dates: Collection -> + dates.associateWith { it == june15Midnight } + } + val validator = createValidator(lookup) + + validator.isValid(june15Midnight) + + assertThat(validator.isValid(june15Midnight)).isTrue() + assertThat(validator.isValid(june15Noon)).isTrue() + assertThat(validator.isValid(june15Evening)).isTrue() + } + + @Test + fun `returns true when lookup is pending`() { + var shouldExecute = false + val delayedExecutor = Executor { runnable -> + if (shouldExecute) { + runnable.run() + } + } + + val lookup = { dates: Collection -> + dates.associateWith { false } + } + val validator = createValidator(lookup, delayedExecutor) + + assertThat(validator.isValid(timestampForDate(2024, 6, 15))).isTrue() + assertThat(validator.isValid(timestampForDate(2024, 6, 15))).isTrue() + + shouldExecute = true + validator.isValid(timestampForDate(2024, 6, 15)) + + val dateInMonth = timestampForDate(2024, 6, 20) + assertThat(validator.isValid(dateInMonth)).isTrue() + } + + @Test + fun `different zones calculate midnight correctly`() { + val newYorkZone = ZoneId.of("America/New_York") + val june15Utc = timestampForDate(2024, 6, 15) + + val lookup = { dates: Collection -> + dates.associateWith { true } + } + val validator = createValidator(lookup, zone = newYorkZone) + + validator.isValid(june15Utc) + + assertThat(validator.isValid(june15Utc)).isTrue() + } +}