From d4e545e67d65dcbd59808a79b2b5721d75ae098a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 18 Nov 2022 14:56:31 +0100 Subject: [PATCH 01/13] use setUserAsActive instead of userManager.disableAllUsersWithoutId just refactoring for now. this doesn't solve the bug! Problem that needs to be solved: When adding a new Account (User), it is marked as "current", while for the other logged in users "current" must be unset (-> disabled). The problem is, that for the old active user, "current" is not unset so there were multiple accounts marked as "current". In the ChooseAccountDialogFragment, only one of the current accounts is shown at the top. Below the set status field, all accounts are listed that are not marked with "current". So as a result, there can be accounts hidden that were marked as "current". Signed-off-by: Marcel Hibbe --- .../AccountVerificationController.kt | 39 ++++++++++++------- .../com/nextcloud/talk/data/user/UsersDao.kt | 10 +++-- .../talk/data/user/UsersRepository.kt | 1 - .../talk/data/user/UsersRepositoryImpl.kt | 4 -- .../dialog/ChooseAccountDialogFragment.java | 5 +++ .../com/nextcloud/talk/users/UserManager.kt | 16 -------- 6 files changed, 36 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt b/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt index 36b50f983..6a8670ac3 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt @@ -28,6 +28,7 @@ import android.os.Handler import android.text.TextUtils import android.util.Log import android.view.View +import android.widget.Toast import androidx.work.Data import androidx.work.OneTimeWorkRequest import androidx.work.WorkManager @@ -443,25 +444,33 @@ class AccountVerificationController(args: Bundle? = null) : } private fun proceedWithLogin() { + Log.d(TAG, "proceedWithLogin...") cookieManager.cookieStore.removeAll() - val userDisabledCount = userManager.disableAllUsersWithoutId(internalAccountId).blockingGet() - Log.d(TAG, "Disabled $userDisabledCount users that had no id") - if (activity != null) { - activity!!.runOnUiThread { - if (userManager.users.blockingGet().size == 1) { - router.setRoot( - RouterTransaction.with(ConversationsListController(Bundle())) - .pushChangeHandler(HorizontalChangeHandler()) - .popChangeHandler(HorizontalChangeHandler()) - ) - } else { - if (isAccountImport) { - ApplicationWideMessageHolder.getInstance().messageType = - ApplicationWideMessageHolder.MessageType.ACCOUNT_WAS_IMPORTED + + val userToSetAsActive = userManager.getUserWithId(internalAccountId).blockingGet() + Log.d(TAG, "userToSetAsActive: " + userToSetAsActive.username) + + if (userManager.setUserAsActive(userToSetAsActive).blockingGet()) { + if (activity != null) { + activity!!.runOnUiThread { + if (userManager.users.blockingGet().size == 1) { + router.setRoot( + RouterTransaction.with(ConversationsListController(Bundle())) + .pushChangeHandler(HorizontalChangeHandler()) + .popChangeHandler(HorizontalChangeHandler()) + ) + } else { + if (isAccountImport) { + ApplicationWideMessageHolder.getInstance().messageType = + ApplicationWideMessageHolder.MessageType.ACCOUNT_WAS_IMPORTED + } + router.popToRoot() } - router.popToRoot() } } + } else { + Log.e(TAG, "failed to set active user") + Toast.makeText(context, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() } } diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt index dc0cd86cc..4092c8267 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt @@ -74,9 +74,6 @@ abstract class UsersDao { @Query("SELECT * FROM User where userId = :userId") abstract fun getUserWithUserId(userId: String): Maybe - @Query("SELECT * FROM User where id != :id") - abstract fun getUsersWithoutId(id: Long): Single> - @Query("SELECT * FROM User where scheduledForDeletion = 1") abstract fun getUsersScheduledForDeletion(): Single> @@ -92,6 +89,13 @@ abstract class UsersDao { return try { getUsers().blockingGet().forEach { user -> user.current = user.id == id + + Log.d(TAG, "xxxxxxxxxxxx") + Log.d(TAG, "setUserAsActiveWithId. user.username: " + user.username) + Log.d(TAG, "setUserAsActiveWithId. user.id: " + user.id) + Log.d(TAG, "setUserAsActiveWithId. user.current: " + user.current) + Log.d(TAG, "xxxxxxxxxxxx") + updateUser(user) } true diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt index 4d15a97b8..e5bdcc3f2 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt @@ -35,7 +35,6 @@ interface UsersRepository { fun getUserWithId(id: Long): Maybe fun getUserWithIdNotScheduledForDeletion(id: Long): Maybe fun getUserWithUserId(userId: String): Maybe - fun getUsersWithoutUserId(id: Long): Single> fun getUsersScheduledForDeletion(): Single> fun getUsersNotScheduledForDeletion(): Single> fun getUserWithUsernameAndServer(username: String, server: String): Maybe 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 d88d2147a..28b4de389 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 @@ -54,10 +54,6 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { return usersDao.getUserWithUserId(userId).map { UserMapper.toModel(it) } } - override fun getUsersWithoutUserId(id: Long): Single> { - return usersDao.getUsersWithoutId(id).map { UserMapper.toModel(it) } - } - override fun getUsersScheduledForDeletion(): Single> { return usersDao.getUsersScheduledForDeletion().map { UserMapper.toModel(it) } } diff --git a/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java b/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java index d75c99403..578d84dc0 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java +++ b/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java @@ -148,6 +148,11 @@ public class ChooseAccountDialogFragment extends DialogFragment { for (Object userItem : userManager.getUsers().blockingGet()) { userEntity = (User) userItem; + Log.d(TAG, "---------------------"); + Log.d(TAG, "userEntity.getUserId() " + userEntity.getUserId()); + Log.d(TAG, "userEntity.getCurrent() " + userEntity.getCurrent()); + Log.d(TAG, "---------------------"); + if (!userEntity.getCurrent()) { String userId; if (userEntity.getUserId() != 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 a61894bfa..c20a231ac 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -58,22 +58,6 @@ class UserManager internal constructor(private val userRepository: UsersReposito return userRepository.getUserWithId(id) } - fun disableAllUsersWithoutId(id: Long): Single { - val results = userRepository.getUsersWithoutUserId(id) - - return results.map { users -> - var count = 0 - if (users.isNotEmpty()) { - for (entity in users) { - entity.current = false - userRepository.updateUser(entity) - count++ - } - } - count - } - } - fun checkIfUserIsScheduledForDeletion(username: String, server: String): Single { return userRepository .getUserWithUsernameAndServer(username, server) From e9dbcc554a3918cc7963647a6b3b5e343ba74e2b Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 18 Nov 2022 15:01:43 +0100 Subject: [PATCH 02/13] set user.current by value from userAttributes this should have no effect but it should ensure "current" is not falsely set to true if the method storeProfile will be used for more scenarios in the future. Signed-off-by: Marcel Hibbe --- app/src/main/java/com/nextcloud/talk/users/UserManager.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 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 c20a231ac..814eeb895 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -149,7 +149,7 @@ class UserManager internal constructor(private val userRepository: UsersReposito else -> { user.token = userAttributes.token user.baseUrl = userAttributes.serverUrl - user.current = true + user.current = userAttributes.currentUser user.userId = userAttributes.userId user.token = userAttributes.token user.displayName = userAttributes.displayName @@ -237,7 +237,7 @@ class UserManager internal constructor(private val userRepository: UsersReposito data class UserAttributes( val id: Long?, val serverUrl: String?, - val currentUser: Boolean?, + val currentUser: Boolean, val userId: String?, val token: String?, val displayName: String?, From f73d1c14fc7773eb35cdf1d8c0806ffe5d0dbe50 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 13 Dec 2022 15:24:56 +0100 Subject: [PATCH 03/13] add logging and toasts if current user was null Signed-off-by: Marcel Hibbe --- .../AccountVerificationController.kt | 6 +-- .../ConversationsListController.kt | 43 +++++++++++-------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt b/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt index 6a8670ac3..984d6131f 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt @@ -324,9 +324,9 @@ class AccountVerificationController(args: Bundle? = null) : } if (!TextUtils.isEmpty(displayName)) { storeProfile( - displayName, userProfileOverall.ocs!!.data!!.userId!!, - capabilities.ocs!!.data!! - .capabilities!! + displayName, + userProfileOverall.ocs!!.data!!.userId!!, + capabilities.ocs!!.data!!.capabilities!! ) } else { if (activity != null) { diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt index 12e3e492d..350d21921 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt @@ -209,26 +209,30 @@ class ConversationsListController(bundle: Bundle) : private fun loadUserAvatar( target: Target ) { - if (activity != null) { - val url = ApiUtils.getUrlForAvatar( - currentUser!!.baseUrl, - currentUser!!.userId, - true - ) + if (currentUser != null) { + val url = ApiUtils.getUrlForAvatar( + currentUser!!.baseUrl, + currentUser!!.userId, + true + ) - val credentials = ApiUtils.getCredentials(currentUser!!.username, currentUser!!.token) + val credentials = ApiUtils.getCredentials(currentUser!!.username, currentUser!!.token) - context.imageLoader.enqueue( - ImageRequest.Builder(context) - .data(url) - .addHeader("Authorization", credentials) - .placeholder(R.drawable.ic_user) - .transformations(CircleCropTransformation()) - .crossfade(true) - .target(target) - .build() - ) + context.imageLoader.enqueue( + ImageRequest.Builder(context) + .data(url) + .addHeader("Authorization", credentials) + .placeholder(R.drawable.ic_user) + .transformations(CircleCropTransformation()) + .crossfade(true) + .target(target) + .build() + ) + } else { + Log.e(TAG, "currentUser was null in loadUserAvatar") + Toast.makeText(context, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() + } } } @@ -238,6 +242,7 @@ class ConversationsListController(bundle: Bundle) : override fun onStart(placeholder: Drawable?) { button.icon = placeholder } + override fun onSuccess(result: Drawable) { button.icon = result } @@ -251,6 +256,7 @@ class ConversationsListController(bundle: Bundle) : override fun onStart(placeholder: Drawable?) { menuItem.icon = placeholder } + override fun onSuccess(result: Drawable) { menuItem.icon = result } @@ -288,6 +294,9 @@ class ConversationsListController(bundle: Bundle) : .colorMaterialTextButton((activity as MainActivity?)!!.binding.switchAccountButton) } fetchRooms() + } else { + Log.e(TAG, "currentUser was null in ConversationsListController.onAttach") + Toast.makeText(context, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() } } From 962b1d4e3a67c6ffb126e5e0036358dbab1c509c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 13 Dec 2022 15:32:23 +0100 Subject: [PATCH 04/13] WIP. replace loop to set current state of users by single UPDATE query i was able to see that the loop was somehow interrupted during debugging which caused two users to have current =true this should avoid the problem with the loop. anyway, this doesn't seem to solve the issue completely as i was able to reproduce it again with the new solution. so maybe there are still more methods/scenarios which can cause this. additionally, i managed to have all users to have current =false with this new query (while switching accounts very fast and often in ChooseAccountDialogFragment..) Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/data/user/UsersDao.kt | 31 +++++-------------- .../talk/data/user/UsersRepositoryImpl.kt | 7 ++++- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt index 4092c8267..eee72ca34 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt @@ -22,13 +22,11 @@ package com.nextcloud.talk.data.user -import android.util.Log import androidx.room.Dao import androidx.room.Delete import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query -import androidx.room.Transaction import androidx.room.Update import com.nextcloud.talk.data.user.model.UserEntity import io.reactivex.Maybe @@ -83,27 +81,14 @@ abstract class UsersDao { @Query("SELECT * FROM User WHERE username = :username AND baseUrl = :server") abstract fun getUserWithUsernameAndServer(username: String, server: String): Maybe - @Transaction - @Suppress("Detekt.TooGenericExceptionCaught") // blockingGet() only throws RuntimeExceptions per rx docs - open fun setUserAsActiveWithId(id: Long): Boolean { - return try { - getUsers().blockingGet().forEach { user -> - user.current = user.id == id - - Log.d(TAG, "xxxxxxxxxxxx") - Log.d(TAG, "setUserAsActiveWithId. user.username: " + user.username) - Log.d(TAG, "setUserAsActiveWithId. user.id: " + user.id) - Log.d(TAG, "setUserAsActiveWithId. user.current: " + user.current) - Log.d(TAG, "xxxxxxxxxxxx") - - updateUser(user) - } - true - } catch (e: RuntimeException) { - Log.e(TAG, "Error setting user active", e) - false - } - } + @Query( + "UPDATE User " + + "SET current = CASE " + + "WHEN id == :id THEN 1 " + + "WHEN userId != :id THEN 0 " + + "END" + ) + abstract fun setUserAsActiveWithId(id: Long): Int companion object { const val TAG = "UsersDao" 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 28b4de389..35faca8a0 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 @@ -75,7 +75,12 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { } override fun setUserAsActiveWithId(id: Long): Single { - return Single.just(usersDao.setUserAsActiveWithId(id)) + val amountUpdated = usersDao.setUserAsActiveWithId(id) + return if (amountUpdated > 0) { + Single.just(true) + } else { + Single.just(false) + } } override fun deleteUser(user: User): Int { From 25dd46f95a15dec9f724d3a987fe5da83540bda4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey?= Date: Wed, 14 Dec 2022 12:10:36 +0100 Subject: [PATCH 05/13] UsersDao: fix wrong attribute used in setUserAsActive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marcel Hibbe Signed-off-by: Álvaro Brey --- app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt index eee72ca34..d4e333163 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt @@ -82,10 +82,9 @@ abstract class UsersDao { abstract fun getUserWithUsernameAndServer(username: String, server: String): Maybe @Query( - "UPDATE User " + - "SET current = CASE " + + "UPDATE User SET current = CASE " + "WHEN id == :id THEN 1 " + - "WHEN userId != :id THEN 0 " + + "WHEN id != :id THEN 0 " + "END" ) abstract fun setUserAsActiveWithId(id: Long): Int From 2e2d4a9ca3f802a3eddbda15fa604b9b3ba1618c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey?= Date: Wed, 14 Dec 2022 12:13:57 +0100 Subject: [PATCH 06/13] CurrentUserProviderImpl: ensure only one observer is set up for all threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Álvaro Brey --- .../utils/database/user/CurrentUserProviderImpl.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt b/app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt index 57848ff47..21aaa8875 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/database/user/CurrentUserProviderImpl.kt @@ -31,7 +31,12 @@ import javax.inject.Inject * Listens to changes in the database and provides the current user without needing to query the database everytime. */ class CurrentUserProviderImpl @Inject constructor(private val userManager: UserManager) : CurrentUserProviderNew { + private var _currentUser: User? = null + + // synchronized to avoid multiple observers initialized from different threads + @get:Synchronized + @set:Synchronized private var currentUserObserver: Disposable? = null override val currentUser: Maybe @@ -40,8 +45,10 @@ class CurrentUserProviderImpl @Inject constructor(private val userManager: UserM // immediately get a result synchronously _currentUser = userManager.currentUser.blockingGet() if (currentUserObserver == null) { - // start observable for auto-updates - currentUserObserver = userManager.currentUserObservable.subscribe { _currentUser = it } + currentUserObserver = userManager.currentUserObservable + .subscribe { + _currentUser = it + } } } return _currentUser?.let { Maybe.just(it) } ?: Maybe.empty() From a37f947eb9222590840822aeb77a283e3e7649a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey?= Date: Wed, 14 Dec 2022 12:15:21 +0100 Subject: [PATCH 07/13] PushUtils: update only pushState when updating users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to work around a race condition where this class would asynchronously overwrite other user attributes with old values after a user switch. Co-authored-by: Marcel Hibbe Signed-off-by: Álvaro Brey --- app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt | 4 ++++ .../java/com/nextcloud/talk/data/user/UsersRepository.kt | 2 ++ .../java/com/nextcloud/talk/data/user/UsersRepositoryImpl.kt | 5 +++++ app/src/main/java/com/nextcloud/talk/users/UserManager.kt | 4 ++++ app/src/main/java/com/nextcloud/talk/utils/PushUtils.java | 3 +-- 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt index d4e333163..565d980f1 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersDao.kt @@ -29,6 +29,7 @@ import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Update import com.nextcloud.talk.data.user.model.UserEntity +import com.nextcloud.talk.models.json.push.PushConfigurationState import io.reactivex.Maybe import io.reactivex.Observable import io.reactivex.Single @@ -89,6 +90,9 @@ abstract class UsersDao { ) abstract fun setUserAsActiveWithId(id: Long): Int + @Query("Update User SET pushConfigurationState = :state WHERE id == :id") + abstract fun updatePushState(id: Long, state: PushConfigurationState): Single + companion object { const val TAG = "UsersDao" } diff --git a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt index e5bdcc3f2..e8470b59f 100644 --- a/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/data/user/UsersRepository.kt @@ -23,6 +23,7 @@ package com.nextcloud.talk.data.user import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.json.push.PushConfigurationState import io.reactivex.Maybe import io.reactivex.Observable import io.reactivex.Single @@ -42,4 +43,5 @@ interface UsersRepository { fun insertUser(user: User): Long fun setUserAsActiveWithId(id: Long): Single fun deleteUser(user: User): Int + fun updatePushState(id: Long, state: PushConfigurationState): Single } 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 35faca8a0..d6723d7ee 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 @@ -23,6 +23,7 @@ package com.nextcloud.talk.data.user import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.models.json.push.PushConfigurationState import io.reactivex.Maybe import io.reactivex.Observable import io.reactivex.Single @@ -86,4 +87,8 @@ class UsersRepositoryImpl(private val usersDao: UsersDao) : UsersRepository { override fun deleteUser(user: User): Int { return usersDao.deleteUser(UserMapper.toEntity(user)) } + + override fun updatePushState(id: Long, state: PushConfigurationState): Single { + return usersDao.updatePushState(id, state) + } } 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 814eeb895..2fb2acd12 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -230,6 +230,10 @@ class UserManager internal constructor(private val userRepository: UsersReposito return user } + fun updatePushState(id: Long, state: PushConfigurationState): Single { + return userRepository.updatePushState(id, state) + } + companion object { const val TAG = "UserManager" } diff --git a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java index 70b2179a1..1032bda91 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java +++ b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java @@ -338,8 +338,7 @@ public class PushUtils { pushConfigurationState.setUserPublicKey(proxyMap.get("userPublicKey")); pushConfigurationState.setUsesRegularPass(Boolean.FALSE); - user.setPushConfigurationState(pushConfigurationState); - userManager.saveUser(user).subscribe(new SingleObserver() { + userManager.updatePushState(user.getId(), pushConfigurationState).subscribe(new SingleObserver() { @Override public void onSubscribe(Disposable d) { // unused atm From dd155db6abc3f443e7677a6d62463354ce034618 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 14 Dec 2022 14:47:55 +0100 Subject: [PATCH 08/13] rename method to update PushState for a user Signed-off-by: Marcel Hibbe --- app/src/main/java/com/nextcloud/talk/utils/PushUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java index 1032bda91..34980305b 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java +++ b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java @@ -311,7 +311,7 @@ public class PushUtils { public void onNext(@NonNull Void aVoid) { try { Log.d(TAG, "pushToken successfully registered at pushproxy."); - createOrUpdateUser(proxyMap, user); + updatePushStateForUser(proxyMap, user); } catch (IOException e) { Log.e(TAG, "IOException while updating user", e); } @@ -330,7 +330,7 @@ public class PushUtils { }); } - private void createOrUpdateUser(Map proxyMap, User user) throws IOException { + private void updatePushStateForUser(Map proxyMap, User user) throws IOException { PushConfigurationState pushConfigurationState = new PushConfigurationState(); pushConfigurationState.setPushToken(proxyMap.get("pushToken")); pushConfigurationState.setDeviceIdentifier(proxyMap.get("deviceIdentifier")); From e238c4c9e5ea86d0ec46ba0c049991de2d5cd5a6 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 14 Dec 2022 15:50:07 +0100 Subject: [PATCH 09/13] ensure that there is always a current user this should also fix the app to start after it always crashed on startup because of https://github.com/nextcloud/talk-android/issues/2531 Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/controllers/ConversationsListController.kt | 2 +- app/src/main/java/com/nextcloud/talk/users/UserManager.kt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt index 350d21921..3674200d9 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ConversationsListController.kt @@ -295,7 +295,7 @@ class ConversationsListController(bundle: Bundle) : } fetchRooms() } else { - Log.e(TAG, "currentUser was null in ConversationsListController.onAttach") + Log.e(TAG, "userManager.currentUser.blockingGet() returned null") Toast.makeText(context, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() } } 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 2fb2acd12..378cb8112 100644 --- a/app/src/main/java/com/nextcloud/talk/users/UserManager.kt +++ b/app/src/main/java/com/nextcloud/talk/users/UserManager.kt @@ -43,6 +43,7 @@ class UserManager internal constructor(private val userRepository: UsersReposito val currentUser: Maybe get() { return userRepository.getActiveUser() + .switchIfEmpty(getAnyUserAndSetAsActive()) } val currentUserObservable: Observable From da8148a134f2a9feb87b149a8777c02fe3353588 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 14 Dec 2022 17:55:44 +0100 Subject: [PATCH 10/13] fix spotbug warning "PRMC: Possibly Redundant Method Calls" PRMC: In class com.nextcloud.talk.utils.ssl.MagicKeyManager In class com.nextcloud.talk.utils.ssl.MagicKeyManager In method com.nextcloud.talk.utils.ssl.MagicKeyManager.chooseClientAlias(String[], Principal[], Socket) At MagicKeyManager.java:[line 68] Value getCurrentUser()Lio/reactivex/Maybe; Method com.nextcloud.talk.utils.ssl.MagicKeyManager.chooseClientAlias(String[], Principal[], Socket) appears to call the same method on the same object redundantly Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/utils/ssl/MagicKeyManager.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicKeyManager.java b/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicKeyManager.java index 14a13903d..6e1748c0b 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicKeyManager.java +++ b/app/src/main/java/com/nextcloud/talk/utils/ssl/MagicKeyManager.java @@ -64,8 +64,9 @@ public class MagicKeyManager implements X509KeyManager { @Override public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) { String alias; - if ((userManager.getCurrentUser().blockingGet() != null && - !TextUtils.isEmpty(alias = userManager.getCurrentUser().blockingGet().getClientCertificate())) || + User currentUser = userManager.getCurrentUser().blockingGet(); + if ((currentUser != null && + !TextUtils.isEmpty(alias = currentUser.getClientCertificate())) || !TextUtils.isEmpty(alias = appPreferences.getTemporaryClientCertAlias()) && new ArrayList<>(Arrays.asList(getClientAliases())).contains(alias)) { return alias; From eff4b912bdae0cf2820fc1dcc24154240bceb770 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 14 Dec 2022 17:57:14 +0100 Subject: [PATCH 11/13] fix Spot Bugs warning Possible null pointer dereference in com.nextcloud.talk.utils.PushUtils.updatePushStateForUser(Map, User) due to return value of called method Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/utils/PushUtils.java | 65 ++++++++++--------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java index 34980305b..2890acfea 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java +++ b/app/src/main/java/com/nextcloud/talk/utils/PushUtils.java @@ -235,17 +235,17 @@ public class PushUtils { List users = userManager.getUsers().blockingGet(); - for (User user : users) { - if (!user.getScheduledForDeletion()) { - Map nextcloudRegisterPushMap = new HashMap<>(); - nextcloudRegisterPushMap.put("format", "json"); - nextcloudRegisterPushMap.put("pushTokenHash", pushTokenHash); - nextcloudRegisterPushMap.put("devicePublicKey", devicePublicKeyBase64); - nextcloudRegisterPushMap.put("proxyServer", proxyServer); + for (User user : users) { + if (!user.getScheduledForDeletion()) { + Map nextcloudRegisterPushMap = new HashMap<>(); + nextcloudRegisterPushMap.put("format", "json"); + nextcloudRegisterPushMap.put("pushTokenHash", pushTokenHash); + nextcloudRegisterPushMap.put("devicePublicKey", devicePublicKeyBase64); + nextcloudRegisterPushMap.put("proxyServer", proxyServer); - registerDeviceWithNextcloud(ncApi, nextcloudRegisterPushMap, token, user); - } + registerDeviceWithNextcloud(ncApi, nextcloudRegisterPushMap, token, user); } + } } } else { @@ -260,9 +260,9 @@ public class PushUtils { String credentials = ApiUtils.getCredentials(user.getUsername(), user.getToken()); ncApi.registerDeviceForNotificationsWithNextcloud( - credentials, - ApiUtils.getUrlNextcloudPush(user.getBaseUrl()), - nextcloudRegisterPushMap) + credentials, + ApiUtils.getUrlNextcloudPush(user.getBaseUrl()), + nextcloudRegisterPushMap) .subscribe(new Observer() { @Override public void onSubscribe(@NonNull Disposable d) { @@ -338,26 +338,31 @@ public class PushUtils { pushConfigurationState.setUserPublicKey(proxyMap.get("userPublicKey")); pushConfigurationState.setUsesRegularPass(Boolean.FALSE); - userManager.updatePushState(user.getId(), pushConfigurationState).subscribe(new SingleObserver() { - @Override - public void onSubscribe(Disposable d) { - // unused atm - } + if (user.getId() != null) { + userManager.updatePushState(user.getId(), pushConfigurationState).subscribe(new SingleObserver() { + @Override + public void onSubscribe(Disposable d) { + // unused atm + } - @Override - public void onSuccess(Integer integer) { - eventBus.post(new EventStatus(UserIdUtils.INSTANCE.getIdForUser(user), - EventStatus.EventType.PUSH_REGISTRATION, - true)); - } + @Override + public void onSuccess(Integer integer) { + eventBus.post(new EventStatus(UserIdUtils.INSTANCE.getIdForUser(user), + EventStatus.EventType.PUSH_REGISTRATION, + true)); + } + + @Override + public void onError(Throwable e) { + eventBus.post(new EventStatus(UserIdUtils.INSTANCE.getIdForUser(user), + EventStatus.EventType.PUSH_REGISTRATION, + false)); + } + }); + } else { + Log.e(TAG, "failed to update updatePushStateForUser. user.getId() was null"); + } - @Override - public void onError(Throwable e) { - eventBus.post(new EventStatus(UserIdUtils.INSTANCE.getIdForUser(user), - EventStatus.EventType.PUSH_REGISTRATION, - false)); - } - }); } private Key readKeyFromString(boolean readPublicKey, String keyString) { From d858169668ee04883a399a4290ea6be8af7b9022 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 15 Dec 2022 13:22:47 +0100 Subject: [PATCH 12/13] prevent NPE Signed-off-by: Andy Scherzinger --- .../main/java/com/nextcloud/talk/activities/CallActivity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java index 1093934dc..497899e31 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java @@ -1661,7 +1661,7 @@ public class CallActivity extends CallBaseActivity { } private void processMessage(NCSignalingMessage ncSignalingMessage) { - if (ncSignalingMessage.getRoomType().equals("video") || ncSignalingMessage.getRoomType().equals("screen")) { + if ("video".equals(ncSignalingMessage.getRoomType()) || "screen".equals(ncSignalingMessage.getRoomType())) { String type = null; if (ncSignalingMessage.getPayload() != null && ncSignalingMessage.getPayload().getType() != null) { type = ncSignalingMessage.getPayload().getType(); From 20ad8892067972829e708f055f48b13ef93d7dcf Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 15 Dec 2022 13:23:06 +0100 Subject: [PATCH 13/13] remove unneeded class cast Signed-off-by: Andy Scherzinger --- .../nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java b/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java index 578d84dc0..6007db129 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java +++ b/app/src/main/java/com/nextcloud/talk/ui/dialog/ChooseAccountDialogFragment.java @@ -146,8 +146,8 @@ public class ChooseAccountDialogFragment extends DialogFragment { User userEntity; Participant participant; - for (Object userItem : userManager.getUsers().blockingGet()) { - userEntity = (User) userItem; + for (User userItem : userManager.getUsers().blockingGet()) { + userEntity = userItem; Log.d(TAG, "---------------------"); Log.d(TAG, "userEntity.getUserId() " + userEntity.getUserId()); Log.d(TAG, "userEntity.getCurrent() " + userEntity.getCurrent());