diff --git a/app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt b/app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt index bd745bacccc..139d5051396 100644 --- a/app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt @@ -98,6 +98,16 @@ class AccountVerificationActivity : BaseActivity() { initSystemBars() handleIntent() + + if ( + isAccountImport && + !UriUtils.hasHttpProtocolPrefixed(baseUrl!!) || + isNotSameProtocol(baseUrl!!, originalProtocol) + ) { + determineBaseUrlProtocol(true) + } else { + findServerTalkApp() + } } private fun handleIntent() { @@ -113,20 +123,6 @@ class AccountVerificationActivity : BaseActivity() { } } - override fun onResume() { - super.onResume() - - if ( - isAccountImport && - !UriUtils.hasHttpProtocolPrefixed(baseUrl!!) || - isNotSameProtocol(baseUrl!!, originalProtocol) - ) { - determineBaseUrlProtocol(true) - } else { - findServerTalkApp() - } - } - private fun isNotSameProtocol(baseUrl: String, originalProtocol: String?): Boolean { if (originalProtocol == null) { return true diff --git a/app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java b/app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java index bbb4f4440ba..6023f20e34e 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java +++ b/app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java @@ -19,6 +19,7 @@ import com.nextcloud.talk.data.database.dao.ChatMessagesDao; import com.nextcloud.talk.data.database.dao.ConversationsDao; import com.nextcloud.talk.data.user.model.User; +import com.nextcloud.talk.logger.Logger; import com.nextcloud.talk.models.json.generic.GenericMeta; import com.nextcloud.talk.models.json.generic.GenericOverall; import com.nextcloud.talk.models.json.push.PushConfigurationState; @@ -66,6 +67,8 @@ public class AccountRemovalWorker extends Worker { @Inject ChatBlocksDao chatBlocksDao; + @Inject Logger logger; + NcApi ncApi; public AccountRemovalWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { @@ -77,6 +80,12 @@ public AccountRemovalWorker(@NonNull Context context, @NonNull WorkerParameters public Result doWork() { Objects.requireNonNull(NextcloudTalkApplication.Companion.getSharedApplication()).getComponentApplication().inject(this); + int duplicateAccountsScheduled = userManager.scheduleDuplicateAccountsForDeletion().blockingGet(); + if (duplicateAccountsScheduled > 0) { + logger.w(TAG, "Found and scheduled " + duplicateAccountsScheduled + + " duplicate account(s) for deletion"); + } + List users = userManager.getUsersScheduledForDeletion().blockingGet(); for (User user : users) { if (user.getPushConfigurationState() != null) { diff --git a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt index 326945a5b5f..8aa5b3f2d66 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -74,6 +74,36 @@ class UserManager internal constructor(private val userRepository: UsersReposito .map { true } .switchIfEmpty(Single.just(false)) + /** + * If there is more than one local User row for the same username+baseUrl (e.g. reusing the + * same token): Keep the current user if it's one of the duplicates, otherwise the oldest (lowest id) row, + * and schedules the rest for deletion so AccountRemovalWorker cleans them up like any other removed account. + * + * @return the number of duplicate rows scheduled for deletion + */ + fun scheduleDuplicateAccountsForDeletion(): Single = + users.map { allUsers -> + allUsers + .filter { !it.username.isNullOrEmpty() && !it.baseUrl.isNullOrEmpty() } + .groupBy { it.username to it.baseUrl } + .values + .filter { it.size > 1 } + }.map { duplicateGroups -> + var scheduledCount = 0 + duplicateGroups.forEach { duplicates -> + val userToKeep = duplicates.firstOrNull { it.current } + ?: duplicates.minByOrNull { it.id ?: Long.MAX_VALUE } + duplicates + .filter { it.id != userToKeep?.id } + .forEach { duplicate -> + duplicate.scheduledForDeletion = true + userRepository.updateUser(duplicate) + scheduledCount++ + } + } + scheduledCount + } + private fun getAnyUserAndSetAsActive(): Maybe { val results = userRepository.getUsersNotScheduledForDeletion() diff --git a/app/src/test/java/com/nextcloud/talk/users/UserManagerTest.kt b/app/src/test/java/com/nextcloud/talk/users/UserManagerTest.kt new file mode 100644 index 00000000000..0c38944db5d --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/users/UserManagerTest.kt @@ -0,0 +1,138 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.users + +import com.nextcloud.talk.data.user.UsersRepository +import com.nextcloud.talk.data.user.model.User +import io.reactivex.Single +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class UserManagerTest { + + private val usersRepository: UsersRepository = mock() + private val userManager = UserManager(usersRepository) + + private fun user(id: Long, username: String, baseUrl: String, current: Boolean = false) = + User(id = id, username = username, baseUrl = baseUrl, current = current) + + @Test + fun `keeps the current user among duplicates and schedules the rest for deletion`() { + val current = user(id = 2, username = "userA", baseUrl = "https://example.com", current = true) + val duplicate = user(id = 1, username = "userA", baseUrl = "https://example.com", current = false) + whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(current, duplicate))) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(1, scheduledCount) + assertTrue(duplicate.scheduledForDeletion) + assertFalse(current.scheduledForDeletion) + verify(usersRepository).updateUser(duplicate) + } + + @Test + fun `keeps the oldest row when none of the duplicates is current`() { + val oldest = user(id = 1, username = "userA", baseUrl = "https://example.com") + val newer = user(id = 2, username = "userA", baseUrl = "https://example.com") + whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(newer, oldest))) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(1, scheduledCount) + assertTrue(newer.scheduledForDeletion) + assertFalse(oldest.scheduledForDeletion) + } + + @Test + fun `does nothing when there are no duplicates`() { + val userA = user(id = 1, username = "userA", baseUrl = "https://example.com", current = true) + val userB = user(id = 2, username = "userB", baseUrl = "https://example.com") + whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(userA, userB))) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(0, scheduledCount) + assertFalse(userA.scheduledForDeletion) + assertFalse(userB.scheduledForDeletion) + } + + @Test + fun `different servers with the same username are not treated as duplicates`() { + val userA = user(id = 1, username = "userA", baseUrl = "https://example.com") + val userB = user(id = 2, username = "userA", baseUrl = "https://other.example.com") + whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(userA, userB))) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(0, scheduledCount) + } + + @Test + fun `rows with a null or blank username or baseUrl are never grouped as duplicates`() { + val nullUsername = user(id = 1, username = "userA", baseUrl = "https://example.com") + .apply { username = null } + val anotherNullUsername = user(id = 2, username = "userA", baseUrl = "https://example.com") + .apply { username = null } + val blankBaseUrl = user(id = 3, username = "userA", baseUrl = "") + val anotherBlankBaseUrl = user(id = 4, username = "userA", baseUrl = "") + whenever(usersRepository.getUsers()).thenReturn( + Single.just(listOf(nullUsername, anotherNullUsername, blankBaseUrl, anotherBlankBaseUrl)) + ) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(0, scheduledCount) + } + + @Test + fun `keeps only one row out of three or more duplicates`() { + val current = user(id = 3, username = "userA", baseUrl = "https://example.com", current = true) + val duplicate1 = user(id = 1, username = "userA", baseUrl = "https://example.com") + val duplicate2 = user(id = 2, username = "userA", baseUrl = "https://example.com") + whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(duplicate1, duplicate2, current))) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(2, scheduledCount) + assertTrue(duplicate1.scheduledForDeletion) + assertTrue(duplicate2.scheduledForDeletion) + assertFalse(current.scheduledForDeletion) + } + + @Test + fun `handles multiple independent duplicate groups in one pass`() { + val userACurrent = user(id = 1, username = "userA", baseUrl = "https://example.com", current = true) + val userADuplicate = user(id = 2, username = "userA", baseUrl = "https://example.com") + val userBOldest = user(id = 3, username = "userB", baseUrl = "https://example.com") + val userBNewer = user(id = 4, username = "userB", baseUrl = "https://example.com") + whenever(usersRepository.getUsers()).thenReturn( + Single.just(listOf(userACurrent, userADuplicate, userBNewer, userBOldest)) + ) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(2, scheduledCount) + assertTrue(userADuplicate.scheduledForDeletion) + assertTrue(userBNewer.scheduledForDeletion) + assertFalse(userACurrent.scheduledForDeletion) + assertFalse(userBOldest.scheduledForDeletion) + } + + @Test + fun `does nothing when there are no users at all`() { + whenever(usersRepository.getUsers()).thenReturn(Single.just(emptyList())) + + val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet() + + assertEquals(0, scheduledCount) + } +}