diff --git a/app/core/src/main/java/com/fsck/k9/KoinModule.kt b/app/core/src/main/java/com/fsck/k9/KoinModule.kt index 772813729..3c4389380 100644 --- a/app/core/src/main/java/com/fsck/k9/KoinModule.kt +++ b/app/core/src/main/java/com/fsck/k9/KoinModule.kt @@ -28,7 +28,7 @@ val mainModule = module { single { get().resources } single { get().contentResolver } single { LocalStoreProvider() } - single { Contacts(contactDataSource = get()) } + single { Contacts() } single { LocalKeyStore(directoryProvider = get()) } single { TrustManagerFactory.createInstance(get()) } single { LocalKeyStoreManager(get()) } diff --git a/app/core/src/main/java/com/fsck/k9/helper/ContactNameProvider.kt b/app/core/src/main/java/com/fsck/k9/helper/ContactNameProvider.kt index feea87c29..4d16061b9 100644 --- a/app/core/src/main/java/com/fsck/k9/helper/ContactNameProvider.kt +++ b/app/core/src/main/java/com/fsck/k9/helper/ContactNameProvider.kt @@ -1,13 +1,16 @@ package com.fsck.k9.helper +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress interface ContactNameProvider { fun getNameForAddress(address: String): String? } -class RealContactNameProvider(private val contacts: Contacts) : ContactNameProvider { +class RealContactNameProvider( + private val contactRepository: ContactRepository, +) : ContactNameProvider { override fun getNameForAddress(address: String): String? { - return contacts.getNameFor(EmailAddress(address)) + return contactRepository.getContactFor(EmailAddress(address))?.name } } diff --git a/app/core/src/main/java/com/fsck/k9/helper/Contacts.kt b/app/core/src/main/java/com/fsck/k9/helper/Contacts.kt index f92d10da2..50501ab9c 100644 --- a/app/core/src/main/java/com/fsck/k9/helper/Contacts.kt +++ b/app/core/src/main/java/com/fsck/k9/helper/Contacts.kt @@ -1,63 +1,11 @@ package com.fsck.k9.helper -import android.net.Uri -import app.k9mail.core.android.common.contact.ContactDataSource -import app.k9mail.core.common.mail.EmailAddress import com.fsck.k9.mail.Address -import timber.log.Timber /** * Helper class to access the contacts stored on the device. */ -open class Contacts( - private val contactDataSource: ContactDataSource, -) { - - /** - * Check whether the provided email address belongs to one of the contacts. - * - * @param emailAddress The email address to look for. - * @return true, if the email address belongs to a contact. - * false, otherwise. - */ - fun isInContacts(emailAddress: EmailAddress): Boolean = contactDataSource.hasContactFor(emailAddress) - - /** - * Check whether one of the provided email addresses belongs to one of the contacts. - * - * @param emailAddresses The email addresses to search in contacts - * @return true, if one of the email addresses belongs to a contact. - * false, otherwise. - */ - fun isAnyInContacts(emailAddresses: List): Boolean = - emailAddresses.any { emailAddress -> isInContacts(emailAddress) } - - fun getContactUri(emailAddress: EmailAddress): Uri? { - val contact = contactDataSource.getContactFor(emailAddress) - return contact?.uri - } - - /** - * Get the name of the contact an email address belongs to. - * - * @param emailAddress The email address to search for. - * @return The name of the contact the email address belongs to. Or - * null if there's no matching contact. - */ - open fun getNameFor(emailAddress: EmailAddress): String? { - if (nameCache.containsKey(emailAddress)) { - return nameCache[emailAddress] - } - - val contact = contactDataSource.getContactFor(emailAddress) - return if (contact != null) { - nameCache[emailAddress] = contact.name - contact.name - } else { - null - } - } - +class Contacts { /** * Mark contacts with the provided email addresses as contacted. */ @@ -65,33 +13,4 @@ open class Contacts( // TODO: Keep track of this information in a local database. Then use this information when sorting contacts for // auto-completion. } - - /** - * Get URI to the picture of the contact with the supplied email address. - * - * @param emailAddress An email address, the contact database is searched for. - * - * @return URI to the picture of the contact with the supplied email address. `null` if - * no such contact could be found or the contact doesn't have a picture. - */ - fun getPhotoUri(emailAddress: EmailAddress): Uri? { - return try { - val contact = contactDataSource.getContactFor(emailAddress) - contact?.photoUri - } catch (e: Exception) { - Timber.e(e, "Couldn't fetch photo for contact with email ${emailAddress.address}") - null - } - } - - companion object { - private val nameCache = HashMap() - - /** - * Clears the cache for names and photo uris - */ - fun clearCache() { - nameCache.clear() - } - } } diff --git a/app/core/src/main/java/com/fsck/k9/helper/KoinModule.kt b/app/core/src/main/java/com/fsck/k9/helper/KoinModule.kt index e44767ce3..547d68bd9 100644 --- a/app/core/src/main/java/com/fsck/k9/helper/KoinModule.kt +++ b/app/core/src/main/java/com/fsck/k9/helper/KoinModule.kt @@ -7,9 +7,9 @@ import org.koin.dsl.module val helperModule = module { single { ClipboardManager(get()) } - single { MessageHelper(resourceProvider = get(), contacts = get()) } + single { MessageHelper(resourceProvider = get(), contactRepository = get()) } factory { AndroidKeyStoreDirectoryProvider(context = get()) } factory { get().getSystemService(Context.ALARM_SERVICE) as AlarmManager } single { AlarmManagerCompat(alarmManager = get()) } - factory { RealContactNameProvider(contacts = get()) } + factory { RealContactNameProvider(contactRepository = get()) } } diff --git a/app/core/src/main/java/com/fsck/k9/helper/MessageHelper.kt b/app/core/src/main/java/com/fsck/k9/helper/MessageHelper.kt index b603b5ed3..f5a7a160d 100644 --- a/app/core/src/main/java/com/fsck/k9/helper/MessageHelper.kt +++ b/app/core/src/main/java/com/fsck/k9/helper/MessageHelper.kt @@ -5,6 +5,7 @@ import android.text.SpannableString import android.text.SpannableStringBuilder import android.text.TextUtils import android.text.style.ForegroundColorSpan +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress import com.fsck.k9.CoreResourceProvider import com.fsck.k9.K9.contactNameColor @@ -16,23 +17,23 @@ import java.util.regex.Pattern class MessageHelper( private val resourceProvider: CoreResourceProvider, - private val contacts: Contacts, + private val contactRepository: ContactRepository, ) { fun getSenderDisplayName(address: Address?): CharSequence { if (address == null) { return resourceProvider.contactUnknownSender() } - val contactHelper = if (isShowContactName) contacts else null - return toFriendly(address, contactHelper) + val repository = if (isShowContactName) contactRepository else null + return toFriendly(address, repository) } fun getRecipientDisplayNames(addresses: Array
?): CharSequence { if (addresses == null || addresses.isEmpty()) { return resourceProvider.contactUnknownRecipient() } - val contactHelper = if (isShowContactName) contacts else null - val recipients = toFriendly(addresses, contactHelper) + val repository = if (isShowContactName) contactRepository else null + val recipients = toFriendly(addresses, repository) return SpannableStringBuilder(resourceProvider.contactDisplayNamePrefix()).append(recipients) } @@ -59,28 +60,28 @@ class MessageHelper( * @param contacts A [Contacts] instance or `null`. * @return A "friendly" name for this [Address]. */ - fun toFriendly(address: Address, contacts: Contacts?): CharSequence { + fun toFriendly(address: Address, contactRepository: ContactRepository?): CharSequence { return toFriendly( address, - contacts, + contactRepository, isShowCorrespondentNames, isChangeContactNameColor, contactNameColor, ) } - fun toFriendly(addresses: Array
?, contacts: Contacts?): CharSequence? { - var contacts = contacts + fun toFriendly(addresses: Array
?, contactRepository: ContactRepository?): CharSequence? { + var repository = contactRepository if (addresses == null) { return null } if (addresses.size >= TOO_MANY_ADDRESSES) { // Don't look up contacts if the number of addresses is very high. - contacts = null + repository = null } val stringBuilder = SpannableStringBuilder() for (i in addresses.indices) { - stringBuilder.append(toFriendly(addresses[i], contacts)) + stringBuilder.append(toFriendly(addresses[i], repository)) if (i < addresses.size - 1) { stringBuilder.append(',') } @@ -92,15 +93,15 @@ class MessageHelper( @JvmStatic fun toFriendly( address: Address, - contacts: Contacts?, + contactRepository: ContactRepository?, showCorrespondentNames: Boolean, changeContactNameColor: Boolean, contactNameColor: Int, ): CharSequence { if (!showCorrespondentNames) { return address.address - } else if (contacts != null) { - val name = contacts.getNameFor(EmailAddress(address.address)) + } else if (contactRepository != null) { + val name = contactRepository.getContactFor(EmailAddress(address.address))?.name if (name != null) { return if (changeContactNameColor) { val coloredName = SpannableString(name) diff --git a/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt b/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt index f570abec7..7d6a80de8 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/CoreKoinModule.kt @@ -80,7 +80,7 @@ val coreNotificationModule = module { clock = get(), ) } - factory { NotificationContentCreator(resourceProvider = get(), contacts = get()) } + factory { NotificationContentCreator(resourceProvider = get(), contactRepository = get()) } factory { BaseNotificationDataCreator() } factory { SingleMessageNotificationDataCreator() } factory { SummaryNotificationDataCreator(singleMessageNotificationDataCreator = get()) } diff --git a/app/core/src/main/java/com/fsck/k9/notification/NotificationContentCreator.kt b/app/core/src/main/java/com/fsck/k9/notification/NotificationContentCreator.kt index 3f0fcfd0c..9749a5dc6 100644 --- a/app/core/src/main/java/com/fsck/k9/notification/NotificationContentCreator.kt +++ b/app/core/src/main/java/com/fsck/k9/notification/NotificationContentCreator.kt @@ -1,9 +1,9 @@ package com.fsck.k9.notification import android.text.SpannableStringBuilder +import app.k9mail.core.android.common.contact.ContactRepository import com.fsck.k9.Account import com.fsck.k9.K9 -import com.fsck.k9.helper.Contacts import com.fsck.k9.helper.MessageHelper import com.fsck.k9.mail.Message import com.fsck.k9.mailstore.LocalMessage @@ -11,7 +11,7 @@ import com.fsck.k9.message.extractors.PreviewResult.PreviewType internal class NotificationContentCreator( private val resourceProvider: NotificationResourceProvider, - private val contacts: Contacts, + private val contactRepository: ContactRepository, ) { fun createFromMessage(account: Account, message: LocalMessage): NotificationContent { val sender = getMessageSender(account, message) @@ -69,14 +69,14 @@ internal class NotificationContentCreator( } private fun getMessageSender(account: Account, message: Message): String? { - val localContacts = if (K9.isShowContactName) contacts else null + val localContactRepository = if (K9.isShowContactName) contactRepository else null var isSelf = false val fromAddresses = message.from if (!fromAddresses.isNullOrEmpty()) { isSelf = account.isAnIdentity(fromAddresses) if (!isSelf) { - return MessageHelper.toFriendly(fromAddresses.first(), localContacts).toString() + return MessageHelper.toFriendly(fromAddresses.first(), localContactRepository).toString() } } @@ -84,7 +84,10 @@ internal class NotificationContentCreator( // show To: if the message was sent from me val recipients = message.getRecipients(Message.RecipientType.TO) if (!recipients.isNullOrEmpty()) { - val recipientDisplayName = MessageHelper.toFriendly(recipients.first(), localContacts).toString() + val recipientDisplayName = MessageHelper.toFriendly( + address = recipients.first(), + contactRepository = localContactRepository, + ).toString() return resourceProvider.recipientDisplayName(recipientDisplayName) } } diff --git a/app/core/src/test/java/com/fsck/k9/helper/MessageHelperTest.kt b/app/core/src/test/java/com/fsck/k9/helper/MessageHelperTest.kt index c6bc43b89..2f31e774f 100644 --- a/app/core/src/test/java/com/fsck/k9/helper/MessageHelperTest.kt +++ b/app/core/src/test/java/com/fsck/k9/helper/MessageHelperTest.kt @@ -2,6 +2,8 @@ package com.fsck.k9.helper import android.graphics.Color import android.text.SpannableString +import app.k9mail.core.android.common.contact.Contact +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress import assertk.assertThat import assertk.assertions.isEqualTo @@ -9,49 +11,25 @@ import assertk.assertions.isInstanceOf import com.fsck.k9.RobolectricTest import com.fsck.k9.helper.MessageHelper.Companion.toFriendly import com.fsck.k9.mail.Address -import org.junit.Before import org.junit.Test +import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock +import org.mockito.kotlin.stub class MessageHelperTest : RobolectricTest() { - private lateinit var contacts: Contacts - private lateinit var contactsWithFakeContact: Contacts - private lateinit var contactsWithFakeSpoofContact: Contacts - - @Before - fun setUp() { - contacts = mock() - contactsWithFakeContact = object : Contacts(mock()) { - override fun getNameFor(emailAddress: EmailAddress): String? { - return if ("test@testor.com" == emailAddress.address) { - "Tim Testor" - } else { - null - } - } - } - contactsWithFakeSpoofContact = object : Contacts(mock()) { - override fun getNameFor(emailAddress: EmailAddress): String? { - return if ("test@testor.com" == emailAddress.address) { - "Tim@Testor" - } else { - null - } - } - } - } + private val contactRepository: ContactRepository = mock() @Test fun testToFriendlyShowsPersonalPartIfItExists() { val address = Address("test@testor.com", "Tim Testor") - assertThat(toFriendly(address, contacts)).isEqualTo("Tim Testor") + assertThat(toFriendly(address, contactRepository)).isEqualTo("Tim Testor") } @Test fun testToFriendlyShowsEmailPartIfNoPersonalPartExists() { val address = Address("test@testor.com") - assertThat(toFriendly(address, contacts)).isEqualTo("test@testor.com") + assertThat(toFriendly(address, contactRepository)).isEqualTo("test@testor.com") } @Test @@ -59,21 +37,25 @@ class MessageHelperTest : RobolectricTest() { val address1 = Address("test@testor.com", "Tim Testor") val address2 = Address("foo@bar.com", "Foo Bar") val addresses = arrayOf(address1, address2) - assertThat(toFriendly(addresses, contacts).toString()).isEqualTo("Tim Testor,Foo Bar") + assertThat(toFriendly(addresses, contactRepository).toString()).isEqualTo("Tim Testor,Foo Bar") } @Test fun testToFriendlyWithContactLookup() { - val address = Address("test@testor.com") - assertThat(toFriendly(address, contactsWithFakeContact)).isEqualTo("Tim Testor") + val address = Address(EMAIL_ADDRESS.address) + setupContactRepositoryWithFakeContact(EMAIL_ADDRESS) + + assertThat(toFriendly(address, contactRepository)).isEqualTo("Tim Testor") } @Test fun testToFriendlyWithChangeContactColor() { - val address = Address("test@testor.com") + val address = Address(EMAIL_ADDRESS.address) + setupContactRepositoryWithFakeContact(EMAIL_ADDRESS) + val friendly = toFriendly( address = address, - contacts = contactsWithFakeContact, + contactRepository = contactRepository, showCorrespondentNames = true, changeContactNameColor = true, contactNameColor = Color.RED, @@ -84,10 +66,12 @@ class MessageHelperTest : RobolectricTest() { @Test fun testToFriendlyWithoutCorrespondentNames() { - val address = Address("test@testor.com", "Tim Testor") + val address = Address(EMAIL_ADDRESS.address, "Tim Testor") + setupContactRepositoryWithFakeContact(EMAIL_ADDRESS) + val friendly = toFriendly( address = address, - contacts = contactsWithFakeContact, + contactRepository = contactRepository, showCorrespondentNames = false, changeContactNameColor = false, contactNameColor = 0, @@ -98,34 +82,66 @@ class MessageHelperTest : RobolectricTest() { @Test fun toFriendly_spoofPreventionOverridesPersonal() { val address = Address("test@testor.com", "potus@whitehouse.gov") - val friendly = toFriendly(address, contacts) + val friendly = toFriendly(address, contactRepository) assertThat(friendly).isEqualTo("test@testor.com") } @Test fun toFriendly_atPrecededByOpeningParenthesisShouldNotTriggerSpoofPrevention() { val address = Address("gitlab@gitlab.example", "username (@username)") - val friendly = toFriendly(address, contacts) + val friendly = toFriendly(address, contactRepository) assertThat(friendly).isEqualTo("username (@username)") } @Test fun toFriendly_nameStartingWithAtShouldNotTriggerSpoofPrevention() { val address = Address("address@domain.example", "@username") - val friendly = toFriendly(address, contacts) + val friendly = toFriendly(address, contactRepository) assertThat(friendly).isEqualTo("@username") } @Test fun toFriendly_spoofPreventionDoesntOverrideContact() { - val address = Address("test@testor.com", "Tim Testor") + val address = Address(EMAIL_ADDRESS.address, "Tim Testor") + setupContactRepositoryWithSpoofContact(EMAIL_ADDRESS) + val friendly = toFriendly( address = address, - contacts = contactsWithFakeSpoofContact, + contactRepository = contactRepository, showCorrespondentNames = true, changeContactNameColor = false, contactNameColor = 0, ) assertThat(friendly).isEqualTo("Tim@Testor") } + + private fun setupContactRepositoryWithFakeContact(emailAddress: EmailAddress) { + contactRepository.stub { + on { getContactFor(emailAddress) } doReturn + Contact( + id = 1L, + name = "Tim Testor", + emailAddress = emailAddress, + uri = mock(), + photoUri = null, + ) + } + } + + private fun setupContactRepositoryWithSpoofContact(emailAddress: EmailAddress) { + contactRepository.stub { + on { getContactFor(emailAddress) } doReturn + Contact( + id = 1L, + name = "Tim@Testor", + emailAddress = emailAddress, + uri = mock(), + photoUri = null, + ) + } + } + + private companion object { + val EMAIL_ADDRESS = EmailAddress("test@testor.com") + } } diff --git a/app/core/src/test/java/com/fsck/k9/notification/NotificationContentCreatorTest.kt b/app/core/src/test/java/com/fsck/k9/notification/NotificationContentCreatorTest.kt index 0863c44f9..071978280 100644 --- a/app/core/src/test/java/com/fsck/k9/notification/NotificationContentCreatorTest.kt +++ b/app/core/src/test/java/com/fsck/k9/notification/NotificationContentCreatorTest.kt @@ -1,9 +1,9 @@ package com.fsck.k9.notification +import app.k9mail.core.android.common.contact.ContactRepository import com.fsck.k9.Account import com.fsck.k9.RobolectricTest import com.fsck.k9.controller.MessageReference -import com.fsck.k9.helper.Contacts import com.fsck.k9.mail.Address import com.fsck.k9.mail.Message.RecipientType import com.fsck.k9.mailstore.LocalMessage @@ -26,8 +26,8 @@ private const val RECIPIENT_ADDRESS = "bob@example.com" private const val RECIPIENT_NAME = "Bob" class NotificationContentCreatorTest : RobolectricTest() { + private val contactRepository = createFakeContentRepository() private val resourceProvider = TestNotificationResourceProvider() - private val contacts = mock() private val contentCreator = createNotificationContentCreator() private val messageReference = createMessageReference() private val account = createFakeAccount() @@ -138,11 +138,16 @@ class NotificationContentCreatorTest : RobolectricTest() { } private fun createNotificationContentCreator(): NotificationContentCreator { - return NotificationContentCreator(resourceProvider, contacts) + return NotificationContentCreator( + resourceProvider, + contactRepository, + ) } private fun createFakeAccount(): Account = mock() + private fun createFakeContentRepository(): ContactRepository = mock() + private fun createMessageReference(): MessageReference { return MessageReference(ACCOUNT_UUID, FOLDER_ID, UID) } diff --git a/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt b/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt index d70db06c7..242569abf 100644 --- a/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt +++ b/app/k9mail/src/main/java/com/fsck/k9/notification/K9NotificationStrategy.kt @@ -1,9 +1,9 @@ package com.fsck.k9.notification +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress import com.fsck.k9.Account import com.fsck.k9.K9 -import com.fsck.k9.helper.Contacts import com.fsck.k9.mail.Flag import com.fsck.k9.mail.K9MailLib import com.fsck.k9.mail.Message @@ -12,7 +12,9 @@ import com.fsck.k9.mailstore.LocalFolder.isModeMismatch import com.fsck.k9.mailstore.LocalMessage import timber.log.Timber -class K9NotificationStrategy(private val contacts: Contacts) : NotificationStrategy { +class K9NotificationStrategy( + private val contactRepository: ContactRepository, +) : NotificationStrategy { override fun shouldNotifyForMessage( account: Account, @@ -86,7 +88,7 @@ class K9NotificationStrategy(private val contacts: Contacts) : NotificationStrat } if (account.isNotifyContactsMailOnly && - !contacts.isAnyInContacts(message.from.map { EmailAddress(it.address) }) + !contactRepository.hasAnyContactFor(message.from.asList().mapNotNull { EmailAddress(it.address) }) ) { Timber.v("No notification: Message is not from a known contact") return false diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt b/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt index b9682bf36..a3b7a0fa4 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/activity/MessageList.kt @@ -26,6 +26,8 @@ import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentTransaction import androidx.fragment.app.commit import androidx.fragment.app.commitNow +import app.k9mail.core.android.common.contact.CachingRepository +import app.k9mail.core.android.common.contact.ContactRepository import com.fsck.k9.Account import com.fsck.k9.K9 import com.fsck.k9.K9.SplitViewMode @@ -34,7 +36,6 @@ import com.fsck.k9.account.BackgroundAccountRemover import com.fsck.k9.activity.compose.MessageActions import com.fsck.k9.controller.MessageReference import com.fsck.k9.controller.MessagingController -import com.fsck.k9.helper.Contacts import com.fsck.k9.helper.ParcelableUtil import com.fsck.k9.mailstore.SearchStatusManager import com.fsck.k9.preferences.GeneralSettingsManager @@ -89,6 +90,7 @@ open class MessageList : private val accountRemover: BackgroundAccountRemover by inject() private val generalSettingsManager: GeneralSettingsManager by inject() private val messagingController: MessagingController by inject() + private val contactRepository: ContactRepository by inject() private val permissionUiHelper: PermissionUiHelper = K9PermissionUiHelper(this) @@ -532,7 +534,10 @@ open class MessageList : override fun onStart() { super.onStart() - Contacts.clearCache() + + if (contactRepository is CachingRepository) { + (contactRepository as CachingRepository).clearCache() + } } public override fun onSaveInstanceState(outState: Bundle) { diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/contacts/ContactPhotoLoader.kt b/app/ui/legacy/src/main/java/com/fsck/k9/contacts/ContactPhotoLoader.kt index 825d06ca9..29b88aeed 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/contacts/ContactPhotoLoader.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/contacts/ContactPhotoLoader.kt @@ -3,13 +3,16 @@ package com.fsck.k9.contacts import android.content.ContentResolver import android.graphics.Bitmap import android.graphics.BitmapFactory +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress -import com.fsck.k9.helper.Contacts import timber.log.Timber -internal class ContactPhotoLoader(private val contentResolver: ContentResolver, private val contacts: Contacts) { +internal class ContactPhotoLoader( + private val contentResolver: ContentResolver, + private val contactRepository: ContactRepository, +) { fun loadContactPhoto(emailAddress: String): Bitmap? { - val photoUri = contacts.getPhotoUri(EmailAddress(emailAddress)) ?: return null + val photoUri = contactRepository.getContactFor(EmailAddress(emailAddress))?.photoUri ?: return null return try { contentResolver.openInputStream(photoUri).use { inputStream -> BitmapFactory.decodeStream(inputStream) diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/contacts/KoinModule.kt b/app/ui/legacy/src/main/java/com/fsck/k9/contacts/KoinModule.kt index aecc78f88..2032ea293 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/contacts/KoinModule.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/contacts/KoinModule.kt @@ -6,7 +6,7 @@ val contactsModule = module { single { ContactLetterExtractor() } factory { ContactLetterBitmapConfig(context = get(), themeManager = get()) } factory { ContactLetterBitmapCreator(letterExtractor = get(), config = get()) } - factory { ContactPhotoLoader(contentResolver = get(), contacts = get()) } + factory { ContactPhotoLoader(contentResolver = get(), contactRepository = get()) } factory { ContactPictureLoader(context = get(), contactLetterBitmapCreator = get()) } factory { ContactImageBitmapDecoderFactory(contactPhotoLoader = get()) } } diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt index 1e4d09f3d..d792363c5 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt @@ -10,7 +10,7 @@ val messageDetailsUiModule = module { messageRepository = get(), folderRepository = get(), contactSettingsProvider = get(), - contacts = get(), + contactRepository = get(), clipboardManager = get(), accountManager = get(), participantFormatter = get(), diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt index 7680c0bf1..972b855e3 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt @@ -4,11 +4,11 @@ import android.app.PendingIntent import android.content.res.Resources import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress import com.fsck.k9.Account import com.fsck.k9.controller.MessageReference import com.fsck.k9.helper.ClipboardManager -import com.fsck.k9.helper.Contacts import com.fsck.k9.mail.Address import com.fsck.k9.mailstore.CryptoResultAnnotation import com.fsck.k9.mailstore.Folder @@ -33,7 +33,7 @@ internal class MessageDetailsViewModel( private val messageRepository: MessageRepository, private val folderRepository: FolderRepository, private val contactSettingsProvider: ContactSettingsProvider, - private val contacts: Contacts, + private val contactRepository: ContactRepository, private val clipboardManager: ClipboardManager, private val accountManager: AccountManager, private val participantFormatter: MessageDetailsParticipantFormatter, @@ -104,7 +104,7 @@ internal class MessageDetailsViewModel( Participant( displayName = displayName, emailAddress = emailAddress, - contactLookupUri = contacts.getContactUri(EmailAddress(emailAddress)), + contactLookupUri = contactRepository.getContactFor(EmailAddress(emailAddress))?.uri, ) } } diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageTopView.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageTopView.kt index 18baa18ff..35352bfa2 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageTopView.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messageview/MessageTopView.kt @@ -17,10 +17,10 @@ import android.widget.ImageView import android.widget.LinearLayout import android.widget.ProgressBar import android.widget.TextView +import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress import com.fsck.k9.Account import com.fsck.k9.Account.ShowPictures -import com.fsck.k9.helper.Contacts import com.fsck.k9.mail.Message import com.fsck.k9.mailstore.AttachmentViewInfo import com.fsck.k9.mailstore.MessageViewInfo @@ -37,7 +37,7 @@ class MessageTopView( attrs: AttributeSet?, ) : LinearLayout(context, attrs), KoinComponent { - private val contacts: Contacts by inject() + private val contactRepository: ContactRepository by inject() private lateinit var layoutInflater: LayoutInflater @@ -262,7 +262,7 @@ class MessageTopView( return false } val senderEmailAddress = getSenderEmailAddress(message) ?: return false - return contacts.isInContacts(EmailAddress(senderEmailAddress)) + return contactRepository.hasContactFor(EmailAddress(senderEmailAddress)) } private fun getSenderEmailAddress(message: Message): String? { diff --git a/config/detekt/baseline.xml b/config/detekt/baseline.xml index c4ecee1f8..db9353ab7 100644 --- a/config/detekt/baseline.xml +++ b/config/detekt/baseline.xml @@ -174,7 +174,7 @@ LongParameterList:ImapSync.kt$ImapSync$( syncConfig: SyncConfig, remoteFolder: ImapFolder, unsyncedMessages: List<ImapMessage>, smallMessages: MutableList<ImapMessage>, largeMessages: MutableList<ImapMessage>, progress: AtomicInteger, todo: Int, listener: SyncListener, ) LongParameterList:MessageDatabaseHelpers.kt$( folderId: Long, deleted: Boolean = false, uid: String? = null, subject: String = "", date: Long = 0L, flags: String = "", senderList: String = "", toList: String = "", ccList: String = "", bccList: String = "", replyToList: String = "", attachmentCount: Int = 0, internalDate: Long = 0L, messageIdHeader: String? = null, previewType: DatabasePreviewType = DatabasePreviewType.NONE, preview: String = "", mimeType: String = "text/plain", normalizedSubjectHash: Long = 0L, empty: Boolean = false, read: Boolean = false, flagged: Boolean = false, answered: Boolean = false, forwarded: Boolean = false, messagePartId: Long = 0L, encryptionType: String? = null, newMessage: Boolean = false, ) LongParameterList:MessageDatabaseHelpers.kt$( type: Int = 0, root: Int? = null, parent: Int = -1, seq: Int = 0, mimeType: String = "text/plain", decodedBodySize: Int = 0, displayName: String? = null, header: String? = null, encoding: String = "7bit", charset: String? = null, dataLocation: Int = 0, data: ByteArray? = null, preamble: String? = null, epilogue: String? = null, boundary: String? = null, contentId: String? = null, serverExtra: String? = null, directory: File? = null, ) - LongParameterList:MessageDetailsViewModel.kt$MessageDetailsViewModel$( private val resources: Resources, private val messageRepository: MessageRepository, private val folderRepository: FolderRepository, private val contactSettingsProvider: ContactSettingsProvider, private val contacts: Contacts, private val clipboardManager: ClipboardManager, private val accountManager: AccountManager, private val participantFormatter: MessageDetailsParticipantFormatter, private val folderNameFormatter: FolderNameFormatter, ) + LongParameterList:MessageDetailsViewModel.kt$MessageDetailsViewModel$( private val resources: Resources, private val messageRepository: MessageRepository, private val folderRepository: FolderRepository, private val contactSettingsProvider: ContactSettingsProvider, private val contactRepository: ContactRepository, private val clipboardManager: ClipboardManager, private val accountManager: AccountManager, private val participantFormatter: MessageDetailsParticipantFormatter, private val folderNameFormatter: FolderNameFormatter, ) LongParameterList:MessageListAdapterTest.kt$MessageListAdapterTest$( account: Account = Account(SOME_ACCOUNT_UUID), subject: String? = "irrelevant", threadCount: Int = 0, messageDate: Long = 0L, internalDate: Long = 0L, displayName: CharSequence = "irrelevant", displayAddress: Address? = Address.parse("irrelevant@domain.example").first(), previewText: String = "irrelevant", isMessageEncrypted: Boolean = false, isRead: Boolean = false, isStarred: Boolean = false, isAnswered: Boolean = false, isForwarded: Boolean = false, hasAttachments: Boolean = false, uniqueId: Long = 0L, folderId: Long = 0L, messageUid: String = "irrelevant", databaseId: Long = 0L, threadRoot: Long = 0L, ) LongParameterList:MessageListAdapterTest.kt$MessageListAdapterTest$( fontSizes: FontSizes = createFontSizes(), previewLines: Int = 0, stars: Boolean = true, senderAboveSubject: Boolean = false, showContactPicture: Boolean = true, showingThreadedList: Boolean = true, backGroundAsReadIndicator: Boolean = false, showAccountChip: Boolean = false, density: UiDensity = UiDensity.Default, ) LongParameterList:MessagePartDatabaseHelpers.kt$( type: Int = MessagePartType.UNKNOWN, root: Long? = null, parent: Long = -1, seq: Int = 0, mimeType: String? = null, decodedBodySize: Int? = null, displayName: String? = null, header: String? = null, encoding: String? = null, charset: String? = null, dataLocation: Int = DataLocation.MISSING, data: ByteArray? = null, preamble: String? = null, epilogue: String? = null, boundary: String? = null, contentId: String? = null, serverExtra: String? = null, ) @@ -548,7 +548,7 @@ ReturnCount:ListUnsubscribeHelper.kt$ListUnsubscribeHelper$fun getPreferredListUnsubscribeUri(message: Message): UnsubscribeUri? ReturnCount:ListUnsubscribeHelper.kt$ListUnsubscribeHelper$private fun extractUri(headerValue: String?): Uri? ReturnCount:MailSyncWorker.kt$MailSyncWorker$override fun doWork(): Result - ReturnCount:MessageHelper.kt$MessageHelper.Companion$@JvmStatic fun toFriendly( address: Address, contacts: Contacts?, showCorrespondentNames: Boolean, changeContactNameColor: Boolean, contactNameColor: Int, ): CharSequence + ReturnCount:MessageHelper.kt$MessageHelper.Companion$@JvmStatic fun toFriendly( address: Address, contactRepository: ContactRepository?, showCorrespondentNames: Boolean, changeContactNameColor: Boolean, contactNameColor: Int, ): CharSequence ReturnCount:MessageList.kt$MessageList$private fun decodeExtrasToLaunchData(intent: Intent): LaunchData ReturnCount:MessageList.kt$MessageList$private fun onCustomKeyDown(event: KeyEvent): Boolean ReturnCount:MessageList.kt$MessageList$public override fun onCreate(savedInstanceState: Bundle?) @@ -666,7 +666,6 @@ TooGenericExceptionCaught:CommandSync.kt$CommandSync$e: Exception TooGenericExceptionCaught:ContactPhotoLoader.kt$ContactPhotoLoader$e: Exception TooGenericExceptionCaught:ContactPictureLoader.kt$ContactPictureLoader$e: Exception - TooGenericExceptionCaught:Contacts.kt$Contacts$e: Exception TooGenericExceptionCaught:GeneralSettingsViewModel.kt$GeneralSettingsViewModel$e: Exception TooGenericExceptionCaught:ImapFolderPusher.kt$ImapFolderPusher$e: Exception TooGenericExceptionCaught:ImapSync.kt$ImapSync$e: Exception diff --git a/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactKoinModule.kt b/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactKoinModule.kt index 371f27286..d542a5100 100644 --- a/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactKoinModule.kt +++ b/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactKoinModule.kt @@ -1,9 +1,27 @@ package app.k9mail.core.android.common.contact +import app.k9mail.core.common.cache.Cache +import app.k9mail.core.common.cache.ExpiringCache +import app.k9mail.core.common.cache.SynchronizedCache +import app.k9mail.core.common.mail.EmailAddress +import org.koin.core.qualifier.named import org.koin.dsl.module internal val contactModule = module { + single>(named(CACHE_NAME)) { + SynchronizedCache( + delegateCache = ExpiringCache(clock = get()), + ) + } factory { ContentResolverContactDataSource(context = get()) } + factory { + CachingContactRepository( + cache = get(named(CACHE_NAME)), + dataSource = get(), + ) + } } + +internal const val CACHE_NAME = "ContactCache" diff --git a/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactRepository.kt b/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactRepository.kt new file mode 100644 index 000000000..db7054c60 --- /dev/null +++ b/core/android/common/src/main/kotlin/app/k9mail/core/android/common/contact/ContactRepository.kt @@ -0,0 +1,48 @@ +package app.k9mail.core.android.common.contact + +import app.k9mail.core.common.cache.Cache +import app.k9mail.core.common.mail.EmailAddress + +interface ContactRepository { + + fun getContactFor(emailAddress: EmailAddress): Contact? + + fun hasContactFor(emailAddress: EmailAddress): Boolean + + fun hasAnyContactFor(emailAddresses: List): Boolean +} + +interface CachingRepository { + fun clearCache() +} + +internal class CachingContactRepository( + private val cache: Cache, + private val dataSource: ContactDataSource, +) : ContactRepository, CachingRepository { + + override fun getContactFor(emailAddress: EmailAddress): Contact? { + if (cache.hasKey(emailAddress)) { + return cache[emailAddress] + } + + val contact = dataSource.getContactFor(emailAddress) + + if (contact != null) { + cache[emailAddress] = contact + } + + return contact + } + + override fun hasContactFor(emailAddress: EmailAddress): Boolean { + return dataSource.hasContactFor(emailAddress) + } + + override fun hasAnyContactFor(emailAddresses: List): Boolean = + emailAddresses.any { emailAddress -> hasContactFor(emailAddress) } + + override fun clearCache() { + cache.clear() + } +} diff --git a/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/CachingContactRepositoryTest.kt b/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/CachingContactRepositoryTest.kt new file mode 100644 index 000000000..e1ef55732 --- /dev/null +++ b/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/CachingContactRepositoryTest.kt @@ -0,0 +1,119 @@ +package app.k9mail.core.android.common.contact + +import app.k9mail.core.common.cache.InMemoryCache +import app.k9mail.core.common.mail.EmailAddress +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isFalse +import assertk.assertions.isNull +import assertk.assertions.isTrue +import kotlin.test.Test +import org.junit.Before +import org.junit.runner.RunWith +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.doReturnConsecutively +import org.mockito.kotlin.mock +import org.mockito.kotlin.stub +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class CachingContactRepositoryTest { + + private val dataSource = mock() + private val cache = InMemoryCache() + + private val testSubject = CachingContactRepository(cache = cache, dataSource = dataSource) + + @Before + fun setUp() { + cache.clear() + } + + @Test + fun `getContactFor() returns null if no contact exists`() { + val result = testSubject.getContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result).isNull() + } + + @Test + fun `getContactFor() returns contact if it exists`() { + dataSource.stub { on { getContactFor(CONTACT_EMAIL_ADDRESS) } doReturn CONTACT } + + val result = testSubject.getContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result).isEqualTo(CONTACT) + } + + @Test + fun `getContactFor() caches contact`() { + dataSource.stub { + on { getContactFor(CONTACT_EMAIL_ADDRESS) } doReturnConsecutively listOf( + CONTACT, + CONTACT.copy(id = 567L), + ) + } + + val result1 = testSubject.getContactFor(CONTACT_EMAIL_ADDRESS) + val result2 = testSubject.getContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result1).isEqualTo(result2) + } + + @Test + fun `getContactFor() returns cached contact`() { + cache[CONTACT_EMAIL_ADDRESS] = CONTACT + + val result = testSubject.getContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result).isEqualTo(CONTACT) + } + + @Test + fun `hasContactFor() returns false if no contact exists`() { + val result = testSubject.hasContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result).isFalse() + } + + @Test + fun `hasContactFor() returns true if contact exists`() { + dataSource.stub { on { hasContactFor(CONTACT_EMAIL_ADDRESS) } doReturn true } + + val result = testSubject.hasContactFor(CONTACT_EMAIL_ADDRESS) + + assertThat(result).isTrue() + } + + @Test + fun `hasAnyContactFor() returns false if no contact exists`() { + val result = testSubject.hasAnyContactFor(listOf(CONTACT_EMAIL_ADDRESS)) + + assertThat(result).isFalse() + } + + @Test + fun `hasAnyContactFor() returns false if list is empty`() { + val result = testSubject.hasAnyContactFor(listOf()) + + assertThat(result).isFalse() + } + + @Test + fun `hasAnyContactFor() returns true if contact exists`() { + dataSource.stub { on { hasContactFor(CONTACT_EMAIL_ADDRESS) } doReturn true } + + val result = testSubject.hasAnyContactFor(listOf(CONTACT_EMAIL_ADDRESS)) + + assertThat(result).isTrue() + } + + @Test + fun `clearCache() clears cache`() { + cache[CONTACT_EMAIL_ADDRESS] = CONTACT + + testSubject.clearCache() + + assertThat(cache[CONTACT_EMAIL_ADDRESS]).isNull() + } +} diff --git a/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/ContactKoinModuleTest.kt b/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/ContactKoinModuleTest.kt index 0b31f3670..fca928968 100644 --- a/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/ContactKoinModuleTest.kt +++ b/core/android/common/src/test/kotlin/app/k9mail/core/android/common/contact/ContactKoinModuleTest.kt @@ -1,5 +1,6 @@ package app.k9mail.core.android.common.contact +import app.k9mail.core.android.common.coreCommonAndroidModule import org.junit.Test import org.junit.runner.RunWith import org.koin.android.ext.koin.androidContext @@ -13,7 +14,10 @@ internal class ContactKoinModuleTest { @Test fun `should have a valid di module`() { koinApplication { + modules(coreCommonAndroidModule) + modules(contactModule) + androidContext(RuntimeEnvironment.getApplication()) checkModules() }