From 20c08903a2dae68ed2ef097a55d2a5e37627077c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 28 Jan 2025 15:11:07 +0100 Subject: [PATCH 1/3] ensure that getAnyUserAndSetAsActive will set other users to current=false Before, getAnyUserAndSetAsActive did only set a random first user to current=true without to check if there is any other current user. So it was up to the calling function of getAnyUserAndSetAsActive to check this. I did not identify a scenario where the getAnyUserAndSetAsActive could cause to set multiple users to current, but anyway the new implementation might fix some scenario that i could not reproduce. Signed-off-by: Marcel Hibbe --- .../main/java/com/nextcloud/talk/users/UserManager.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 9f5261492..ec4de2853 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -8,6 +8,7 @@ package com.nextcloud.talk.users import android.text.TextUtils +import android.util.Log import com.bluelinelabs.logansquare.LoganSquare import com.nextcloud.talk.data.user.UsersRepository import com.nextcloud.talk.data.user.model.User @@ -88,12 +89,11 @@ class UserManager internal constructor(private val userRepository: UsersReposito .flatMapMaybe { if (it.isNotEmpty()) { val user = it.first() - user.apply { - current = true - }.also { currentUser -> - userRepository.updateUser(currentUser) + if (setUserAsActive(user).blockingGet()) { + userRepository.getActiveUser() + } else { + Maybe.empty() } - Maybe.just(user) } else { Maybe.empty() } @@ -123,6 +123,7 @@ class UserManager internal constructor(private val userRepository: UsersReposito } fun setUserAsActive(user: User): Single { + Log.d(TAG, "setUserAsActive:" + user.id!!) return userRepository.setUserAsActiveWithId(user.id!!) } From 5fdfa49e6da4d1a44fc57b16aa1ccdbdb44dbe2c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 28 Jan 2025 15:15:06 +0100 Subject: [PATCH 2/3] fix to not execute getAnyUserAndSetAsActive() until userRepository.getActiveUser() is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make sure getAnyUserAndSetAsActive() is invoked lazily and avoid any side effects, it's explicitly wrapped it in a lambda. Maybe.defer ensures that getAnyUserAndSetAsActive() is not invoked until switchIfEmpty decides it’s needed. Signed-off-by: Marcel Hibbe --- app/src/main/java/com/nextcloud/talk/users/UserManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ec4de2853..8df87671e 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -31,7 +31,7 @@ class UserManager internal constructor(private val userRepository: UsersReposito val currentUser: Maybe get() { return userRepository.getActiveUser() - .switchIfEmpty(getAnyUserAndSetAsActive()) + .switchIfEmpty(Maybe.defer { getAnyUserAndSetAsActive() }) } val currentUserObservable: Observable From caeeb16ea4b80e8df067477dcc6ebc78b88f9b0b Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 28 Jan 2025 15:57:41 +0100 Subject: [PATCH 3/3] self heal if multiple current users exist usersDao.getActiveUser() will return only one current users even if multiple exist for whatever reason. By executing setUserAsActiveWithId(it.id) with the same user, it is made sure all other users current status is set to 0. This change won't fix the root cause if multiple users are set to current, but it will make sure the state is fixed as soon as there is a query to get the active user. In fact, this change might also make it harder to find the root cause because debugging may be more difficult! When searching for the root cause, always keep this in mind and maybe revert the change. Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/data/user/UsersRepositoryImpl.kt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt index 557860763..66b51ee44 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt @@ -7,6 +7,7 @@ */ package com.nextcloud.talk.data.user +import android.util.Log import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.json.push.PushConfigurationState import io.reactivex.Maybe @@ -17,7 +18,12 @@ import io.reactivex.Single class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { override fun getActiveUser(): Maybe { - return usersDao.getActiveUser().map { UserMapper.toModel(it) } + val user = usersDao.getActiveUser() + .map { + setUserAsActiveWithId(it.id) + UserMapper.toModel(it)!! + } + return user } override fun getActiveUserObservable(): Observable { @@ -62,6 +68,7 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { override fun setUserAsActiveWithId(id: Long): Single { val amountUpdated = usersDao.setUserAsActiveWithId(id) + Log.d(TAG, "setUserAsActiveWithId. amountUpdated: $amountUpdated") return if (amountUpdated > 0) { Single.just(true) } else { @@ -76,4 +83,8 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { override fun updatePushState(id: Long, state: PushConfigurationState): Single { return usersDao.updatePushState(id, state) } + + companion object { + private val TAG = UsersRepositoryImpl::class.simpleName + } }