From 307ebd384d7893743397ec3736e11f36693c3bfc 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 9f52614927..ec4de28533 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 22b8b59fb318183b53ffc13e528b0fc052113ef9 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 ec4de28533..8df87671eb 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 b69f21580a70442b474da02252fa0278e0f7cd5b 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 5578607636..66b51ee446 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 + } }