Skip to content

Commit 5466698

Browse files
mahibimahibi
authored
Merge pull request #6559 from nextcloud/bugfix/noid/fixDuplicateAccounts
fix(account): avoid duplicated accounts + remove existing duplicates
2 parents 37f0fff + ba4eba5 commit 5466698

4 files changed

Lines changed: 187 additions & 14 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/account/AccountVerificationActivity.kt‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@ class AccountVerificationActivity : BaseActivity() {
9898
initSystemBars()
9999

100100
handleIntent()
101+
102+
if (
103+
isAccountImport &&
104+
!UriUtils.hasHttpProtocolPrefixed(baseUrl!!) ||
105+
isNotSameProtocol(baseUrl!!, originalProtocol)
106+
) {
107+
determineBaseUrlProtocol(true)
108+
} else {
109+
findServerTalkApp()
110+
}
101111
}
102112

103113
private fun handleIntent() {
@@ -113,20 +123,6 @@ class AccountVerificationActivity : BaseActivity() {
113123
}
114124
}
115125

116-
override fun onResume() {
117-
super.onResume()
118-
119-
if (
120-
isAccountImport &&
121-
!UriUtils.hasHttpProtocolPrefixed(baseUrl!!) ||
122-
isNotSameProtocol(baseUrl!!, originalProtocol)
123-
) {
124-
determineBaseUrlProtocol(true)
125-
} else {
126-
findServerTalkApp()
127-
}
128-
}
129-
130126
private fun isNotSameProtocol(baseUrl: String, originalProtocol: String?): Boolean {
131127
if (originalProtocol == null) {
132128
return true

‎app/src/main/java/com/nextcloud/talk/jobs/AccountRemovalWorker.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.nextcloud.talk.data.database.dao.ChatMessagesDao;
2020
import com.nextcloud.talk.data.database.dao.ConversationsDao;
2121
import com.nextcloud.talk.data.user.model.User;
22+
import com.nextcloud.talk.logger.Logger;
2223
import com.nextcloud.talk.models.json.generic.GenericMeta;
2324
import com.nextcloud.talk.models.json.generic.GenericOverall;
2425
import com.nextcloud.talk.models.json.push.PushConfigurationState;
@@ -66,6 +67,8 @@ public class AccountRemovalWorker extends Worker {
6667

6768
@Inject ChatBlocksDao chatBlocksDao;
6869

70+
@Inject Logger logger;
71+
6972
NcApi ncApi;
7073

7174
public AccountRemovalWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) {
@@ -77,6 +80,12 @@ public AccountRemovalWorker(@NonNull Context context, @NonNull WorkerParameters
7780
public Result doWork() {
7881
Objects.requireNonNull(NextcloudTalkApplication.Companion.getSharedApplication()).getComponentApplication().inject(this);
7982

83+
int duplicateAccountsScheduled = userManager.scheduleDuplicateAccountsForDeletion().blockingGet();
84+
if (duplicateAccountsScheduled > 0) {
85+
logger.w(TAG, "Found and scheduled " + duplicateAccountsScheduled +
86+
" duplicate account(s) for deletion");
87+
}
88+
8089
List<User> users = userManager.getUsersScheduledForDeletion().blockingGet();
8190
for (User user : users) {
8291
if (user.getPushConfigurationState() != null) {

‎app/src/main/java/com/nextcloud/talk/users/UserManager.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,36 @@ class UserManager internal constructor(private val userRepository: UsersReposito
7474
.map { true }
7575
.switchIfEmpty(Single.just(false))
7676

77+
/**
78+
* If there is more than one local User row for the same username+baseUrl (e.g. reusing the
79+
* same token): Keep the current user if it's one of the duplicates, otherwise the oldest (lowest id) row,
80+
* and schedules the rest for deletion so AccountRemovalWorker cleans them up like any other removed account.
81+
*
82+
* @return the number of duplicate rows scheduled for deletion
83+
*/
84+
fun scheduleDuplicateAccountsForDeletion(): Single<Int> =
85+
users.map { allUsers ->
86+
allUsers
87+
.filter { !it.username.isNullOrEmpty() && !it.baseUrl.isNullOrEmpty() }
88+
.groupBy { it.username to it.baseUrl }
89+
.values
90+
.filter { it.size > 1 }
91+
}.map { duplicateGroups ->
92+
var scheduledCount = 0
93+
duplicateGroups.forEach { duplicates ->
94+
val userToKeep = duplicates.firstOrNull { it.current }
95+
?: duplicates.minByOrNull { it.id ?: Long.MAX_VALUE }
96+
duplicates
97+
.filter { it.id != userToKeep?.id }
98+
.forEach { duplicate ->
99+
duplicate.scheduledForDeletion = true
100+
userRepository.updateUser(duplicate)
101+
scheduledCount++
102+
}
103+
}
104+
scheduledCount
105+
}
106+
77107
private fun getAnyUserAndSetAsActive(): Maybe<User> {
78108
val results = userRepository.getUsersNotScheduledForDeletion()
79109

‎app/src/test/java/com/nextcloud/talk/users/UserManagerTest.kt‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* Nextcloud Talk - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: GPL-3.0-or-later
6+
*/
7+
package com.nextcloud.talk.users
8+
9+
import com.nextcloud.talk.data.user.UsersRepository
10+
import com.nextcloud.talk.data.user.model.User
11+
import io.reactivex.Single
12+
import org.junit.Assert.assertEquals
13+
import org.junit.Assert.assertFalse
14+
import org.junit.Assert.assertTrue
15+
import org.junit.Test
16+
import org.mockito.kotlin.mock
17+
import org.mockito.kotlin.verify
18+
import org.mockito.kotlin.whenever
19+
20+
class UserManagerTest {
21+
22+
private val usersRepository: UsersRepository = mock()
23+
private val userManager = UserManager(usersRepository)
24+
25+
private fun user(id: Long, username: String, baseUrl: String, current: Boolean = false) =
26+
User(id = id, username = username, baseUrl = baseUrl, current = current)
27+
28+
@Test
29+
fun `keeps the current user among duplicates and schedules the rest for deletion`() {
30+
val current = user(id = 2, username = "userA", baseUrl = "https://example.com", current = true)
31+
val duplicate = user(id = 1, username = "userA", baseUrl = "https://example.com", current = false)
32+
whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(current, duplicate)))
33+
34+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
35+
36+
assertEquals(1, scheduledCount)
37+
assertTrue(duplicate.scheduledForDeletion)
38+
assertFalse(current.scheduledForDeletion)
39+
verify(usersRepository).updateUser(duplicate)
40+
}
41+
42+
@Test
43+
fun `keeps the oldest row when none of the duplicates is current`() {
44+
val oldest = user(id = 1, username = "userA", baseUrl = "https://example.com")
45+
val newer = user(id = 2, username = "userA", baseUrl = "https://example.com")
46+
whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(newer, oldest)))
47+
48+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
49+
50+
assertEquals(1, scheduledCount)
51+
assertTrue(newer.scheduledForDeletion)
52+
assertFalse(oldest.scheduledForDeletion)
53+
}
54+
55+
@Test
56+
fun `does nothing when there are no duplicates`() {
57+
val userA = user(id = 1, username = "userA", baseUrl = "https://example.com", current = true)
58+
val userB = user(id = 2, username = "userB", baseUrl = "https://example.com")
59+
whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(userA, userB)))
60+
61+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
62+
63+
assertEquals(0, scheduledCount)
64+
assertFalse(userA.scheduledForDeletion)
65+
assertFalse(userB.scheduledForDeletion)
66+
}
67+
68+
@Test
69+
fun `different servers with the same username are not treated as duplicates`() {
70+
val userA = user(id = 1, username = "userA", baseUrl = "https://example.com")
71+
val userB = user(id = 2, username = "userA", baseUrl = "https://other.example.com")
72+
whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(userA, userB)))
73+
74+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
75+
76+
assertEquals(0, scheduledCount)
77+
}
78+
79+
@Test
80+
fun `rows with a null or blank username or baseUrl are never grouped as duplicates`() {
81+
val nullUsername = user(id = 1, username = "userA", baseUrl = "https://example.com")
82+
.apply { username = null }
83+
val anotherNullUsername = user(id = 2, username = "userA", baseUrl = "https://example.com")
84+
.apply { username = null }
85+
val blankBaseUrl = user(id = 3, username = "userA", baseUrl = "")
86+
val anotherBlankBaseUrl = user(id = 4, username = "userA", baseUrl = "")
87+
whenever(usersRepository.getUsers()).thenReturn(
88+
Single.just(listOf(nullUsername, anotherNullUsername, blankBaseUrl, anotherBlankBaseUrl))
89+
)
90+
91+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
92+
93+
assertEquals(0, scheduledCount)
94+
}
95+
96+
@Test
97+
fun `keeps only one row out of three or more duplicates`() {
98+
val current = user(id = 3, username = "userA", baseUrl = "https://example.com", current = true)
99+
val duplicate1 = user(id = 1, username = "userA", baseUrl = "https://example.com")
100+
val duplicate2 = user(id = 2, username = "userA", baseUrl = "https://example.com")
101+
whenever(usersRepository.getUsers()).thenReturn(Single.just(listOf(duplicate1, duplicate2, current)))
102+
103+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
104+
105+
assertEquals(2, scheduledCount)
106+
assertTrue(duplicate1.scheduledForDeletion)
107+
assertTrue(duplicate2.scheduledForDeletion)
108+
assertFalse(current.scheduledForDeletion)
109+
}
110+
111+
@Test
112+
fun `handles multiple independent duplicate groups in one pass`() {
113+
val userACurrent = user(id = 1, username = "userA", baseUrl = "https://example.com", current = true)
114+
val userADuplicate = user(id = 2, username = "userA", baseUrl = "https://example.com")
115+
val userBOldest = user(id = 3, username = "userB", baseUrl = "https://example.com")
116+
val userBNewer = user(id = 4, username = "userB", baseUrl = "https://example.com")
117+
whenever(usersRepository.getUsers()).thenReturn(
118+
Single.just(listOf(userACurrent, userADuplicate, userBNewer, userBOldest))
119+
)
120+
121+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
122+
123+
assertEquals(2, scheduledCount)
124+
assertTrue(userADuplicate.scheduledForDeletion)
125+
assertTrue(userBNewer.scheduledForDeletion)
126+
assertFalse(userACurrent.scheduledForDeletion)
127+
assertFalse(userBOldest.scheduledForDeletion)
128+
}
129+
130+
@Test
131+
fun `does nothing when there are no users at all`() {
132+
whenever(usersRepository.getUsers()).thenReturn(Single.just(emptyList()))
133+
134+
val scheduledCount = userManager.scheduleDuplicateAccountsForDeletion().blockingGet()
135+
136+
assertEquals(0, scheduledCount)
137+
}
138+
}

0 commit comments

Comments
 (0)