Fix #477: "Device" contacts get deleted by account changes from across the system

app/src/main/java/com/bnyro/contacts/App.kt

@@ -57,6 +57,10 @@ class App : Application() {
             ShortcutHelper.createShortcuts(this@App)
         }
 
+        CoroutineScope(Dispatchers.IO).launch {
+            deviceContactsRepository.migrateContactsFromLegacyDeviceAccount()
+        }
+
         initSmsRepo()
     }
 }

app/src/main/java/com/bnyro/contacts/domain/model/AccountType.kt

@@ -1,18 +1,95 @@
 package com.bnyro.contacts.domain.model
 
-data class AccountType(
-    val name: String,
-    val type: String,
-) {
-    val identifier = "$type|$name"
+import android.util.Log
+
+sealed interface AccountType {
+    fun displayName(): String
+    fun displayType(): String
+
+    data class AccountColumns(
+        val accountName: String?,
+        val accountType: String?,
+    )
+
+    fun toAccountColumns(): AccountColumns {
+        return when (this) {
+            is DeviceAccountType -> AccountColumns(null, null)
+            is RealAccountType -> AccountColumns(accountName = this.name, accountType = this.type)
+        }
+    }
+
+    fun toPreferencesString(): String {
+        val (name, type) = when (this) {
+            is DeviceAccountType -> Pair(
+                DeviceAccountType.LEGACY_NAME,
+                DeviceAccountType.LEGACY_TYPE
+            )
+            is RealAccountType -> Pair(this.name, this.type)
+        }
+        return "${type}|${name}"
+    }
 
     companion object {
-        private const val ANDROID_ACCOUNT_TYPE = "com.android.contacts"
-        private const val ANDROID_ACCOUNT_NAME = "DEVICE"
+        fun fromAccountColumns(accountName: String?, accountType: String?): AccountType? {
+            return if (accountName == null && accountType == null) {
+                DeviceAccountType
+            } else if (accountName != null && accountType != null) {
+                fromNameAndType(accountName, accountType)
+            } else {
+                Log.e("AccountType", "Raw contact has partial account: name=($accountName),type=($accountType)")
+                null
+            }
+        }
+
+        fun fromPreferencesString(identifier: String): AccountType? {
+            val sections = identifier.split('|')
+            if (sections.size != 2) return null
+            val type = sections[0]
+            val name = sections[1]
+            return fromNameAndType(name, type)
+        }
+
+        private fun fromNameAndType(name: String, type: String): AccountType {
+            return if (type == DeviceAccountType.LEGACY_TYPE
+                    && name == DeviceAccountType.LEGACY_NAME
+            ) {
+                // This is for contacts saved before the fix for #477
+                DeviceAccountType
+            } else {
+                RealAccountType(name, type)
+            }
+        }
+    }
+}
+
+data object DeviceAccountType: AccountType {
+    /* The legacy signifiers here are very important.  We must always
+      be able to deserialize objects from previous versions of the app -
+      to do that we must know these exact values. */
+    internal const val LEGACY_TYPE: String = "com.android.contacts"
+    internal const val LEGACY_NAME: String = "DEVICE"
+
+    override fun displayName(): String {
+        // TODO: retrieve a localised value instead
+        // e.g. Through context.getString(R.string.device)
+        return "Device"
+    }
+
+    override fun displayType(): String {
+        // TODO choose new value
+        return "com.android.contacts"
+    }
+}
+
+data class RealAccountType(
+    val name: String,
+    val type: String,
+): AccountType {
+    override fun displayName(): String {
+        return name
+    }
 
-        val androidDefault = AccountType(
-            ANDROID_ACCOUNT_NAME,
-            ANDROID_ACCOUNT_TYPE
-        )
+    override fun displayType(): String {
+        return type
     }
 }

app/src/main/java/com/bnyro/contacts/domain/model/ContactData.kt

@@ -8,8 +8,7 @@ data class ContactData(
     var dataId: Int = 0,
     var rawContactId: Int = 0,
     var contactId: Long = 0,
-    var accountType: String? = null,
-    var accountName: String? = null,
+    var account: AccountType? = null,
     var displayName: String? = null,
     var alternativeName: String? = null,
     var firstName: String? = null,
@@ -29,7 +28,6 @@ data class ContactData(
     var ringTone: Uri? = null,
     var favorite: Boolean = false
 ) {
-    val accountIdentifier get() = "$accountType|$accountName"
     fun getNameBySortOrder(sortOrder: SortOrder): String? {
         return when (sortOrder) {
             SortOrder.FIRSTNAME -> displayName

app/src/main/java/com/bnyro/contacts/domain/model/FilterOptions.kt

@@ -5,7 +5,7 @@ import com.bnyro.contacts.util.Preferences
 
 data class FilterOptions(
     var sortOrder: SortOrder,
-    var hiddenAccountIdentifiers: List<String>,
+    var hiddenAccounts: Set<AccountType>,
     var visibleGroups: List<ContactsGroup>,
     var favoritesOnly: Boolean
 ) {
@@ -15,7 +15,9 @@ data class FilterOptions(
             val hiddenAccounts = Preferences.getStringSet(
                 Preferences.hiddenAccountsKey,
                 emptySet()
-            )!!.toList()
+            )!!.mapNotNull {
+                AccountType.fromPreferencesString(it)
+            }.toSet()
             val favoritesOnly = Preferences.getBoolean(Preferences.favoritesOnlyKey, false)
             return FilterOptions(sortOrder, hiddenAccounts, listOf(), favoritesOnly)
         }

app/src/main/java/com/bnyro/contacts/domain/repositories/DeviceContactsRepository.kt

@@ -6,10 +6,12 @@ import android.annotation.SuppressLint
 import android.content.ContentProviderOperation
 import android.content.ContentResolver
 import android.content.ContentUris
+import android.content.ContentValues
 import android.content.Context
 import android.graphics.Bitmap
 import android.graphics.BitmapFactory
 import android.net.Uri
+import android.os.Build
 import android.provider.ContactsContract
 import android.provider.ContactsContract.AUTHORITY
 import android.provider.ContactsContract.CALLER_IS_SYNCADAPTER
@@ -20,6 +22,7 @@ import android.provider.ContactsContract.CommonDataKinds.StructuredName
 import android.provider.ContactsContract.Contacts
 import android.provider.ContactsContract.Data
 import android.provider.ContactsContract.RawContacts
+import android.util.Log
 import androidx.annotation.RequiresPermission
 import com.bnyro.contacts.R
 import com.bnyro.contacts.domain.enums.BackupType
@@ -28,9 +31,12 @@ import com.bnyro.contacts.domain.enums.StringAttribute
 import com.bnyro.contacts.domain.model.AccountType
 import com.bnyro.contacts.domain.model.ContactData
 import com.bnyro.contacts.domain.model.ContactsGroup
+import com.bnyro.contacts.domain.model.DeviceAccountType
+import com.bnyro.contacts.domain.model.RealAccountType
 import com.bnyro.contacts.domain.model.ValueWithType
 import com.bnyro.contacts.util.ContactsHelper
 import com.bnyro.contacts.util.ImageHelper
+import com.bnyro.contacts.util.PermissionHelper
 import com.bnyro.contacts.util.Preferences
 import com.bnyro.contacts.util.extension.boolValue
 import com.bnyro.contacts.util.extension.intValue
@@ -98,8 +104,10 @@ class DeviceContactsRepository(private val context: Context) : ContactsRepositor
                     val contact = ContactData(
                         rawContactId = it.intValue(Data.RAW_CONTACT_ID) ?: 0,
                         contactId = contactId,
-                        accountType = it.stringValue(RawContacts.ACCOUNT_TYPE),
-                        accountName = it.stringValue(RawContacts.ACCOUNT_NAME),
+                        account = AccountType.fromAccountColumns(
+                            accountName = it.stringValue(RawContacts.ACCOUNT_NAME),
+                            accountType = it.stringValue(RawContacts.ACCOUNT_TYPE),
+                        ),
                         displayName = displayName,
                         alternativeName = alternativeName,
                         firstName = firstName,
@@ -167,11 +175,12 @@ class DeviceContactsRepository(private val context: Context) : ContactsRepositor
     override suspend fun createGroup(groupName: String): ContactsGroup? {
         return withContext(Dispatchers.IO) {
             val operations = ArrayList<ContentProviderOperation>()
+            val (accountName, accountType) = DeviceAccountType.toAccountColumns()
             ContentProviderOperation.newInsert(ContactsContract.Groups.CONTENT_URI).apply {
                 withValue(ContactsContract.Groups.TITLE, groupName)
                 withValue(ContactsContract.Groups.GROUP_VISIBLE, 1)
-                withValue(ContactsContract.Groups.ACCOUNT_NAME, AccountType.androidDefault.name)
-                withValue(ContactsContract.Groups.ACCOUNT_TYPE, AccountType.androidDefault.type)
+                withValue(ContactsContract.Groups.ACCOUNT_NAME, accountName)
+                withValue(ContactsContract.Groups.ACCOUNT_TYPE, accountType)
                 operations.add(build())
             }
 
@@ -311,8 +320,7 @@ class DeviceContactsRepository(private val context: Context) : ContactsRepositor
             val lastChosenAccount = Preferences.getLastChosenAccount()
             val ops = listOfNotNull(
                 getCreateAction(
-                    contact.accountType ?: lastChosenAccount.type,
-                    contact.accountName ?: lastChosenAccount.name
+                    contact.account ?: lastChosenAccount
                 ),
                 getInsertAction(
                     StructuredName.CONTENT_ITEM_TYPE,
@@ -437,13 +445,15 @@ class DeviceContactsRepository(private val context: Context) : ContactsRepositor
                     && ContentResolver.getSyncAutomatically(it, AUTHORITY)
         }
 
-        return listOf(AccountType.androidDefault) + accounts.map { AccountType(it.name, it.type) }
+        return listOf<AccountType>(DeviceAccountType) + accounts.map {
+            RealAccountType(it.name, it.type)
+        }
     }
 
     private fun getCreateAction(
-        accountType: String,
-        accountName: String
+        account: AccountType
     ): ContentProviderOperation {
+        val (accountName, accountType) = account.toAccountColumns()
         return ContentProviderOperation.newInsert(RawContacts.CONTENT_URI)
             .withValue(RawContacts.ACCOUNT_TYPE, accountType)
             .withValue(RawContacts.ACCOUNT_NAME, accountName)
@@ -588,6 +598,83 @@ class DeviceContactsRepository(private val context: Context) : ContactsRepositor
             contentResolver.applyBatch(AUTHORITY, arrayListOf(op))
         }
 
+    fun migrateContactsFromLegacyDeviceAccount() {
+        val alreadyMigrated = Preferences.getBoolean(
+            Preferences.legacyDeviceAccountsMigratedKey,
+            false
+        )
+
+        if (!alreadyMigrated
+            && hasPermissionReadWriteContacts()
+            && isLegacyAccountAFakeAccount()
+        ) {
+            runMigration()
+
+            Preferences.edit {
+                putBoolean(
+                    Preferences.legacyDeviceAccountsMigratedKey,
+                    true
+                )
+            }
+        }
+    }
+
+    private fun runMigration() {
+        try {
+            clearLegacyAccountColumns()
+        } catch (error: IllegalArgumentException) {
+            /* At least one version of Samsung Android throws
+             * IllegalArgumentException("Must specify both or neither of ACCOUNT_NAME and ACCOUNT_TYPE")
+             * if you pass in null for both values (despite this being the mentioned "neither" case).
+             */
+            Log.e(
+                "LegacyContactsMigration",
+                "In-place update of contacts with the legacy account values failed. Manufacturer: ${Build.MANUFACTURER}, Model: ${Build.MODEL}",
+                error
+            )
+        }
+    }
+
+    private fun hasPermissionReadWriteContacts(): Boolean {
+        return PermissionHelper.hasPermission(
+            context,
+            Manifest.permission.READ_CONTACTS,
+            Manifest.permission.WRITE_CONTACTS,
+        )
+    }
+
+    private fun isLegacyAccountAFakeAccount(): Boolean {
+        return AccountManager.get(context).accounts.none {
+            it.name == DeviceAccountType.LEGACY_NAME
+                    && it.type == DeviceAccountType.LEGACY_TYPE
+        }
+    }
+
+    private fun clearLegacyAccountColumns() {
+        val values = ContentValues().apply {
+            putNull(RawContacts.ACCOUNT_NAME)
+            putNull(RawContacts.ACCOUNT_TYPE)
+        }
+
+        val selection =
+            "${RawContacts.ACCOUNT_NAME} = ? AND ${RawContacts.ACCOUNT_TYPE} = ?"
+        val selectionArgs =
+            arrayOf(DeviceAccountType.LEGACY_NAME, DeviceAccountType.LEGACY_TYPE)
+
+        contentResolver.update(
+            RawContacts.CONTENT_URI,
+            values,
+            selection,
+            selectionArgs
+        )
+        contentResolver.update(
+            ContactsContract.Groups.CONTENT_URI,
+            values,
+            selection,
+            selectionArgs
+        )
+    }
+
     companion object {
         const val MAX_PHOTO_SIZE = 700f
     }

app/src/main/java/com/bnyro/contacts/presentation/features/FilterDialog.kt

@@ -37,8 +37,8 @@ fun FilterDialog(
         mutableStateOf(initialFilters.sortOrder)
     }
 
-    var hiddenAccountNames by remember {
-        mutableStateOf(initialFilters.hiddenAccountIdentifiers)
+    var hiddenAccounts by remember {
+        mutableStateOf(initialFilters.hiddenAccounts)
     }
 
     var visibleGroups by remember {
@@ -53,7 +53,7 @@ fun FilterDialog(
         onDismissRequest = onDismissRequest,
         confirmButton = {
             DialogButton(text = stringResource(R.string.okay)) {
-                val options = FilterOptions(sortOrder, hiddenAccountNames, visibleGroups, favoritesOnly)
+                val options = FilterOptions(sortOrder, hiddenAccounts, visibleGroups, favoritesOnly)
                 onFilterChanged.invoke(options)
                 onDismissRequest.invoke()
             }
@@ -84,16 +84,16 @@ fun FilterDialog(
                     Spacer(modifier = Modifier.height(10.dp))
                     ChipSelector(
                         title = stringResource(R.string.account_type),
-                        entries = availableAccountTypes.map { it.type },
+                        entries = availableAccountTypes.map { it.displayType() },
                         selections = availableAccountTypes.filter {
-                            !hiddenAccountNames.contains(it.identifier)
-                        }.map { it.type },
+                            !hiddenAccounts.contains(it)
+                        }.map { it.displayType() },
                         onSelectionChanged = { index, newValue ->
                             val selection = availableAccountTypes[index]
-                            hiddenAccountNames = if (newValue) {
-                                hiddenAccountNames - selection.identifier
+                            hiddenAccounts = if (newValue) {
+                                hiddenAccounts - selection
                             } else {
-                                hiddenAccountNames + selection.identifier
+                                hiddenAccounts + selection
                             }
                         }
                     )

app/src/main/java/com/bnyro/contacts/presentation/screens/contact/SingleContactScreen.kt

@@ -273,7 +273,7 @@ fun SingleContactScreen(contact: ContactData, viewModel: ContactsModel, onClose:
             },
             isDeviceContact = (viewModel.contactsSource == ContactsSource.DEVICE),
             onSave = {
-                if (contact.accountIdentifier == it.accountIdentifier) {
+                if (contact.account == it.account) {
                     viewModel.updateContact(context, it)
                 } else {
                     viewModel.deleteContacts(listOf(it))

app/src/main/java/com/bnyro/contacts/presentation/screens/contacts/ContactsScreen.kt

@@ -370,7 +370,10 @@ fun ContactsPage(
             onFilterChanged = {
                 Preferences.edit {
                     putInt(Preferences.sortOrderKey, it.sortOrder.ordinal)
-                    putStringSet(Preferences.hiddenAccountsKey, it.hiddenAccountIdentifiers.toSet())
+                    putStringSet(
+                        Preferences.hiddenAccountsKey,
+                        it.hiddenAccounts.map { account -> account.toPreferencesString() }.toSet(),
+                    )
                     putBoolean(Preferences.favoritesOnlyKey, it.favoritesOnly)
                 }
                 filterOptions = it