diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchData.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchData.kt index c057c0999..d835ad3c8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchData.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchData.kt @@ -1,5 +1,6 @@ package org.thoughtcrime.securesms.contacts.paged +import androidx.annotation.VisibleForTesting import org.thoughtcrime.securesms.contacts.HeaderAction import org.thoughtcrime.securesms.database.model.DistributionListPrivacyMode import org.thoughtcrime.securesms.recipients.Recipient @@ -36,4 +37,10 @@ sealed class ContactSearchData(val contactSearchKey: ContactSearchKey) { * A row which the user can click to view all entries for a given section. */ class Expand(val sectionKey: ContactSearchConfiguration.SectionKey) : ContactSearchData(ContactSearchKey.Expand(sectionKey)) + + /** + * A row which contains an integer, for testing. + */ + @VisibleForTesting + class TestRow(val value: Int) : ContactSearchData(ContactSearchKey.Expand(ContactSearchConfiguration.SectionKey.RECENTS)) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchItems.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchItems.kt index a77391d91..cf60e759a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchItems.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchItems.kt @@ -76,6 +76,7 @@ object ContactSearchItems { is ContactSearchData.KnownRecipient -> RecipientModel(it, selection.contains(it.contactSearchKey)) is ContactSearchData.Expand -> ExpandModel(it) is ContactSearchData.Header -> HeaderModel(it) + is ContactSearchData.TestRow -> error("This row exists for testing only.") } } ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchPagedDataSource.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchPagedDataSource.kt index 8d5892ce9..28c4287a9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchPagedDataSource.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/ContactSearchPagedDataSource.kt @@ -243,7 +243,7 @@ class ContactSearchPagedDataSource( ): ContactSearchCollection { return when (section) { is ContactSearchConfiguration.Section.Stories -> StoriesSearchCollection(section, records, extraData, recordMapper, activeStoryCount, StoryComparator(latestStorySends)) - else -> ContactSearchCollection(section, records, recordsPredicate, extraData, recordMapper, 0) + else -> ContactSearchCollection(section, records, recordsPredicate, recordMapper, 0) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollection.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollection.kt index 4665d8cef..9a5bf346c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollection.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollection.kt @@ -12,23 +12,20 @@ open class ContactSearchCollection( private val section: ContactSearchConfiguration.Section, private val records: ContactSearchIterator, private val recordPredicate: ((ContactRecord) -> Boolean)? = null, - private val extraData: List, private val recordMapper: (ContactRecord) -> ContactSearchData, private val activeContactCount: Int ) { - private val recordsCount: Int = if (recordPredicate != null) { + private val contentSize: Int = if (recordPredicate != null) { records.asSequence().filter(recordPredicate).count() } else { records.getCount() } - private val contentSize: Int private val aggregateData: SparseArrayCompat = SparseArrayCompat() init { records.moveToPosition(-1) - contentSize = recordsCount + extraData.count() } fun getSize(): Int { @@ -57,7 +54,11 @@ open class ContactSearchCollection( null to 0 } - fillDataWindow(start, end - start) + val windowOffset = start + startOffset - if (section.includeHeader) 1 else 0 + val windowLimit = end - windowOffset - if (section.includeHeader) 1 else 0 + + fillDataWindow(windowOffset, windowLimit) + for (i in (start + startOffset) until (end - endOffset)) { val correctedIndex = if (section.includeHeader) i - 1 else i results.add(getItemAtCorrectedIndex(correctedIndex)) @@ -95,20 +96,9 @@ open class ContactSearchCollection( key++ } - if (isAggregateDataFilled(offset, limit)) { - return + if (!isAggregateDataFilled(offset, limit)) { + error("Data integrity failure: ${section.sectionKey} requesting $offset , $limit") } - - extraData.forEach { - aggregateData.put(key, it) - key++ - } - - if (isAggregateDataFilled(offset, limit)) { - return - } - - throw IllegalStateException("Could not fill aggregate data for bounds $offset $limit") } private fun isAggregateDataFilled(startOffset: Int, limit: Int): Boolean { diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/StoriesSearchCollection.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/StoriesSearchCollection.kt index 678dcd4e8..45977ef85 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/StoriesSearchCollection.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/paged/collections/StoriesSearchCollection.kt @@ -13,7 +13,7 @@ class StoriesSearchCollection( recordMapper: (ContactRecord) -> ContactSearchData, activeContactCount: Int, private val storyComparator: Comparator -) : ContactSearchCollection(section, records, null, extraData, recordMapper, activeContactCount) { +) : ContactSearchCollection(section, records, null, recordMapper, activeContactCount) { private val aggregateStoryData: List by lazy { if (section !is ContactSearchConfiguration.Section.Stories) { error("Aggregate data creation is only necessary for stories.") diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java index 7d3ac4cbe..5386953df 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java @@ -38,7 +38,6 @@ import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; -import org.thoughtcrime.securesms.storage.StorageSyncHelper; import org.thoughtcrime.securesms.util.Util; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil; import org.whispersystems.signalservice.api.groupsv2.GroupChangeReconstruct; @@ -47,7 +46,6 @@ import org.whispersystems.signalservice.api.push.ACI; import org.whispersystems.signalservice.api.push.DistributionId; import org.whispersystems.signalservice.api.push.ServiceId; import org.whispersystems.signalservice.api.util.UuidUtil; -import org.whispersystems.signalservice.internal.push.exceptions.GroupNotFoundException; import java.io.Closeable; import java.security.SecureRandom; diff --git a/app/src/test/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollectionTest.kt b/app/src/test/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollectionTest.kt new file mode 100644 index 000000000..2f1a1664b --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/contacts/paged/collections/ContactSearchCollectionTest.kt @@ -0,0 +1,121 @@ +package org.thoughtcrime.securesms.contacts.paged.collections + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.thoughtcrime.securesms.contacts.paged.ContactSearchConfiguration +import org.thoughtcrime.securesms.contacts.paged.ContactSearchData + +class ContactSearchCollectionTest { + + @Test + fun `When I get size, then I expect size of filtered content`() { + // GIVEN + val testSubject = createTestSubject() + + // WHEN + val size = testSubject.getSize() + + // THEN + assertEquals(5, size) + } + + @Test + fun `When I get size with header, then I expect size of filtered content plus header`() { + // GIVEN + val testSubject = createTestSubject(includeHeader = true) + + // WHEN + val size = testSubject.getSize() + + // THEN + assertEquals(6, size) + } + + @Test + fun `When I getSublist without 5, then I expect the corresponding values without 5`() { + // GIVEN + val testSubject = createTestSubject( + recordPredicate = { it != 5 } + ) + + // WHEN + val result = testSubject.getSublist(0, 9) + + // THEN + assertEquals(listOf(0, 1, 2, 3, 4, 6, 7, 8, 9), result.filterIsInstance(ContactSearchData.TestRow::class.java).map { it.value }) + } + + @Test + fun `Given I got first page when I getSublist without 5, then I expect the corresponding values without 5`() { + // GIVEN + val testSubject = createTestSubject( + recordPredicate = { it != 5 } + ) + testSubject.getSublist(0, 5) + + // WHEN + val result = testSubject.getSublist(5, 9) + + // THEN + assertEquals(listOf(6, 7, 8, 9), result.filterIsInstance(ContactSearchData.TestRow::class.java).map { it.value }) + } + + @Test + fun `Given I get second page with header, then I expect the corresponding values without 5`() { + // GIVEN + val testSubject = createTestSubject( + recordPredicate = { it != 5 }, + includeHeader = true + ) + + // WHEN + val result = testSubject.getSublist(2, testSubject.getSize()) + + // THEN + assertEquals(listOf(1, 2, 3, 4, 6, 7, 8, 9), result.filterIsInstance(ContactSearchData.TestRow::class.java).map { it.value }) + } + + @Test + fun `Given I get entire page with header, then I expect the corresponding values without 5`() { + // GIVEN + val testSubject = createTestSubject( + recordPredicate = { it != 5 }, + includeHeader = true + ) + + // WHEN + val result = testSubject.getSublist(0, testSubject.getSize()) + + // THEN + assertEquals(listOf(0, 1, 2, 3, 4, 6, 7, 8, 9), result.filterIsInstance(ContactSearchData.TestRow::class.java).map { it.value }) + } + + private fun createTestSubject( + size: Int = 10, + includeHeader: Boolean = false, + section: ContactSearchConfiguration.Section = ContactSearchConfiguration.Section.Groups(includeHeader = includeHeader), + records: ContactSearchIterator = FakeContactSearchIterator((0 until size).toList()), + recordPredicate: (Int) -> Boolean = { i -> i % 2 == 0 }, + recordMapper: (Int) -> ContactSearchData = { i -> ContactSearchData.TestRow(i) }, + activeContactCount: Int = 0 + ): ContactSearchCollection { + return ContactSearchCollection(section, records, recordPredicate, recordMapper, activeContactCount) + } + + private class FakeContactSearchIterator(private val numbers: List) : ContactSearchIterator { + + private var position = -1 + + override fun hasNext(): Boolean = position < numbers.lastIndex + + override fun next(): Int = numbers[++position] + + override fun moveToPosition(n: Int) { + position = n + } + + override fun getCount(): Int = numbers.size + + override fun close() = Unit + } +}