From 39441ae075449912be82492b5ea686681d822d10 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 9 Mar 2023 20:41:50 +0100 Subject: [PATCH 1/7] Move setupWebsocket() to onAttach() in ChatController Add toast warning for debug mode Rename "magic" stuff Signed-off-by: Marcel Hibbe --- .../talk/application/NextcloudTalkApplication.kt | 1 + .../nextcloud/talk/controllers/ChatController.kt | 13 ++++++------- .../talk/webrtc/WebSocketConnectionHelper.java | 16 ++++++++-------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt index 1b59ce407..e99ec721c 100644 --- a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt +++ b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt @@ -182,6 +182,7 @@ class NextcloudTalkApplication : MultiDexApplication(), LifecycleObserver { HALF_DAY, TimeUnit.HOURS ).build() + val capabilitiesUpdateWork = OneTimeWorkRequest.Builder(CapabilitiesWorker::class.java).build() val signalingSettingsWork = OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java).build() diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 4d0a7b9b4..c92340ec6 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -1783,6 +1783,7 @@ class ChatController(args: Bundle) : eventBus.register(this) + setupWebsocket() webSocketInstance?.getSignalingMessageReceiver()?.addListener(localParticipantMessageListener) if (conversationUser?.userId != "?" && @@ -2002,11 +2003,6 @@ class ChatController(args: Bundle) : logConversationInfos("joinRoomWithPassword#onNext") - // FIXME The web socket should be set up in onAttach(). It is currently setup after joining the - // room to "ensure" (rather, increase the chances) that the WebsocketConnectionsWorker job - // was able to finish and, therefore, that the web socket instance can be got. - setupWebsocket() - // Ensure that the listener is added if the web socket instance was not set up yet when // onAttach() was called. webSocketInstance?.getSignalingMessageReceiver()?.addListener(localParticipantMessageListener) @@ -2225,10 +2221,13 @@ class ChatController(args: Bundle) : return } - webSocketInstance = WebSocketConnectionHelper.getMagicWebSocketInstanceForUserId(conversationUser.id!!) + webSocketInstance = WebSocketConnectionHelper.getWebSocketInstanceForUserId(conversationUser.id!!) if (webSocketInstance == null) { - Log.d(TAG, "magicWebSocketInstance became null") + Log.e(TAG, "failed to setup webSocketInstance") + if (BuildConfig.DEBUG) { + Toast.makeText(context, "failed to setup webSocketInstance", Toast.LENGTH_LONG).show() + } } } diff --git a/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java b/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java index 72554f93f..1a49a1ee4 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java @@ -48,7 +48,7 @@ import okhttp3.OkHttpClient; @AutoInjector(NextcloudTalkApplication.class) public class WebSocketConnectionHelper { public static final String TAG = "WebSocketConnectionHelper"; - private static Map magicWebSocketInstanceMap = new HashMap<>(); + private static Map webSocketInstanceMap = new HashMap<>(); @Inject OkHttpClient okHttpClient; @@ -59,11 +59,11 @@ public class WebSocketConnectionHelper { } @SuppressLint("LongLogTag") - public static synchronized WebSocketInstance getMagicWebSocketInstanceForUserId(long userId) { - WebSocketInstance webSocketInstance = magicWebSocketInstanceMap.get(userId); + public static synchronized WebSocketInstance getWebSocketInstanceForUserId(long userId) { + WebSocketInstance webSocketInstance = webSocketInstanceMap.get(userId); if (webSocketInstance == null) { - Log.d(TAG, "No magicWebSocketInstance found for user " + userId); + Log.e(TAG, "No webSocketInstance found for user " + userId); } return webSocketInstance; @@ -83,24 +83,24 @@ public class WebSocketConnectionHelper { long userId = isGuest ? -1 : user.getId(); WebSocketInstance webSocketInstance; - if (userId != -1 && magicWebSocketInstanceMap.containsKey(user.getId()) && (webSocketInstance = magicWebSocketInstanceMap.get(user.getId())) != null) { + if (userId != -1 && webSocketInstanceMap.containsKey(user.getId()) && (webSocketInstance = webSocketInstanceMap.get(user.getId())) != null) { return webSocketInstance; } else { if (userId == -1) { deleteExternalSignalingInstanceForUserEntity(userId); } webSocketInstance = new WebSocketInstance(user, generatedURL, webSocketTicket); - magicWebSocketInstanceMap.put(user.getId(), webSocketInstance); + webSocketInstanceMap.put(user.getId(), webSocketInstance); return webSocketInstance; } } public static synchronized void deleteExternalSignalingInstanceForUserEntity(long id) { WebSocketInstance webSocketInstance; - if ((webSocketInstance = magicWebSocketInstanceMap.get(id)) != null) { + if ((webSocketInstance = webSocketInstanceMap.get(id)) != null) { if (webSocketInstance.isConnected()) { webSocketInstance.sendBye(); - magicWebSocketInstanceMap.remove(id); + webSocketInstanceMap.remove(id); } } } From 9882ddc5369fdbb41d02dacabf39c493471a67be Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 11:05:51 +0100 Subject: [PATCH 2/7] Fix logic in workers when user was not found Signed-off-by: Marcel Hibbe --- .../main/java/com/nextcloud/talk/jobs/CapabilitiesWorker.java | 4 ++-- .../java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/CapabilitiesWorker.java b/app/src/main/java/com/nextcloud/talk/jobs/CapabilitiesWorker.java index ba20aab32..4fe5c4f32 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/CapabilitiesWorker.java +++ b/app/src/main/java/com/nextcloud/talk/jobs/CapabilitiesWorker.java @@ -112,9 +112,9 @@ public class CapabilitiesWorker extends Worker { long internalUserId = data.getLong(BundleKeys.KEY_INTERNAL_USER_ID, -1); List userEntityObjectList = new ArrayList<>(); - boolean userExists = userManager.getUserWithInternalId(internalUserId).isEmpty().blockingGet(); + boolean userNotFound = userManager.getUserWithInternalId(internalUserId).isEmpty().blockingGet(); - if (internalUserId == -1 || !userExists) { + if (internalUserId == -1 || userNotFound) { userEntityObjectList = userManager.getUsers().blockingGet(); } else { userEntityObjectList.add(userManager.getUserWithInternalId(internalUserId).blockingGet()); diff --git a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java index cdaf5a92f..e69b70ce4 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java +++ b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java @@ -77,9 +77,9 @@ public class SignalingSettingsWorker extends Worker { long internalUserId = data.getLong(BundleKeys.KEY_INTERNAL_USER_ID, -1); List userEntityObjectList = new ArrayList<>(); - boolean userExists = userManager.getUserWithInternalId(internalUserId).isEmpty().blockingGet(); + boolean userNotFound = userManager.getUserWithInternalId(internalUserId).isEmpty().blockingGet(); - if (internalUserId == -1 || !userExists) { + if (internalUserId == -1 || userNotFound) { userEntityObjectList = userManager.getUsers().blockingGet(); } else { userEntityObjectList.add(userManager.getUserWithInternalId(internalUserId).blockingGet()); From 8c991c697fcbdef8e075cefa8c8026b6dbee2a8a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 14:13:02 +0100 Subject: [PATCH 3/7] Fix to save externalSignalingServer to user Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java index e69b70ce4..dc8546b89 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java +++ b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java @@ -115,6 +115,8 @@ public class SignalingSettingsWorker extends Worker { .getExternalSignalingTicket()); } + user.setExternalSignalingServer(externalSignalingServer); + userManager.saveUser(user).subscribe(new SingleObserver() { @Override public void onSubscribe(Disposable d) { From 609e5a2c7113cb7847104122d35dbd7eaca26f7e Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 14:19:45 +0100 Subject: [PATCH 4/7] Use WorkManager queue to chain workers. This is necessary as many of the workers store user data. When running in parallel, there are race conditions and user data that was stored by one worker gets directly overwritten by the next worker. E.g. this happened with the "externalSignalingServer" attribute of user: SignalingSettingsWorker saved the user with the externalSignalingServer value, but then CapabilitiesWorker kicked in and saved the user without this value. Because of this, in WebsocketConnectionsWorker getExternalSignalingServer() of the user was null. Because of this, webSocketConnectionHelper.getExternalSignalingInstanceForServer(..) was not called. So the webSocketInstanceMap in WebSocketConnectionHelper was never filled. This is why WebSocketConnectionHelper.getMagicWebSocketInstanceForUserId(..) in ChatController failed to get a webSocketInstance. Signed-off-by: Marcel Hibbe --- .../application/NextcloudTalkApplication.kt | 19 ++++++++++++------- .../AccountVerificationController.kt | 15 ++++++++++----- .../talk/jobs/SignalingSettingsWorker.java | 7 ------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt index e99ec721c..c8efea780 100644 --- a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt +++ b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt @@ -61,6 +61,7 @@ import com.nextcloud.talk.dagger.modules.ViewModelModule import com.nextcloud.talk.jobs.AccountRemovalWorker import com.nextcloud.talk.jobs.CapabilitiesWorker import com.nextcloud.talk.jobs.SignalingSettingsWorker +import com.nextcloud.talk.jobs.WebsocketConnectionsWorker import com.nextcloud.talk.ui.theme.ThemeModule import com.nextcloud.talk.utils.ClosedInterfaceImpl import com.nextcloud.talk.utils.DeviceUtils @@ -177,18 +178,22 @@ class NextcloudTalkApplication : MultiDexApplication(), LifecycleObserver { DeviceUtils.ignoreSpecialBatteryFeatures() val accountRemovalWork = OneTimeWorkRequest.Builder(AccountRemovalWorker::class.java).build() + val capabilitiesUpdateWork = OneTimeWorkRequest.Builder(CapabilitiesWorker::class.java).build() + val signalingSettingsWork = OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java).build() + val websocketConnectionsWorker = OneTimeWorkRequest.Builder(WebsocketConnectionsWorker::class.java).build() + + WorkManager.getInstance(applicationContext) + .beginWith(accountRemovalWork) + .then(capabilitiesUpdateWork) + .then(signalingSettingsWork) + .then(websocketConnectionsWorker) + .enqueue() + val periodicCapabilitiesUpdateWork = PeriodicWorkRequest.Builder( CapabilitiesWorker::class.java, HALF_DAY, TimeUnit.HOURS ).build() - - val capabilitiesUpdateWork = OneTimeWorkRequest.Builder(CapabilitiesWorker::class.java).build() - val signalingSettingsWork = OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java).build() - - WorkManager.getInstance(applicationContext).enqueue(accountRemovalWork) - WorkManager.getInstance(applicationContext).enqueue(capabilitiesUpdateWork) - WorkManager.getInstance(applicationContext).enqueue(signalingSettingsWork) WorkManager.getInstance(applicationContext).enqueueUniquePeriodicWork( "DailyCapabilitiesUpdateWork", ExistingPeriodicWorkPolicy.REPLACE, 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 df017ac8c..f275b6fba 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/AccountVerificationController.kt @@ -48,6 +48,7 @@ import com.nextcloud.talk.events.EventStatus import com.nextcloud.talk.jobs.CapabilitiesWorker import com.nextcloud.talk.jobs.PushRegistrationWorker import com.nextcloud.talk.jobs.SignalingSettingsWorker +import com.nextcloud.talk.jobs.WebsocketConnectionsWorker import com.nextcloud.talk.models.json.capabilities.Capabilities import com.nextcloud.talk.models.json.capabilities.CapabilitiesOverall import com.nextcloud.talk.models.json.generic.Status @@ -434,11 +435,15 @@ class AccountVerificationController(args: Bundle? = null) : Data.Builder() .putLong(KEY_INTERNAL_USER_ID, internalAccountId) .build() - val signalingSettings = - OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java) - .setInputData(userData) - .build() - WorkManager.getInstance().enqueue(signalingSettings) + val signalingSettings = OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java) + .setInputData(userData) + .build() + val websocketConnectionsWorker = OneTimeWorkRequest.Builder(WebsocketConnectionsWorker::class.java).build() + + WorkManager.getInstance(applicationContext!!) + .beginWith(signalingSettings) + .then(websocketConnectionsWorker) + .enqueue() } private fun proceedWithLogin() { diff --git a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java index dc8546b89..a0e4e7d07 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java +++ b/app/src/main/java/com/nextcloud/talk/jobs/SignalingSettingsWorker.java @@ -42,8 +42,6 @@ import javax.inject.Inject; import androidx.annotation.NonNull; import androidx.work.Data; -import androidx.work.OneTimeWorkRequest; -import androidx.work.WorkManager; import androidx.work.Worker; import androidx.work.WorkerParameters; import autodagger.AutoInjector; @@ -159,11 +157,6 @@ public class SignalingSettingsWorker extends Worker { }); } - OneTimeWorkRequest websocketConnectionsWorker = new OneTimeWorkRequest - .Builder(WebsocketConnectionsWorker.class) - .build(); - WorkManager.getInstance().enqueue(websocketConnectionsWorker); - return Result.success(); } } From abd1d4b2471cc243ea43a72bc5b1d17ef3c7d4ae Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 14:29:59 +0100 Subject: [PATCH 5/7] Remove to set localParticipantMessageListener in joinRoom this is already done in onAttach which seems to do the job fine after using workManager queues and fixing to set externalSignalingServer to user. See commits da1714bb and 29a37086 Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/controllers/ChatController.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index c92340ec6..2fecdf7bb 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2003,10 +2003,6 @@ class ChatController(args: Bundle) : logConversationInfos("joinRoomWithPassword#onNext") - // Ensure that the listener is added if the web socket instance was not set up yet when - // onAttach() was called. - webSocketInstance?.getSignalingMessageReceiver()?.addListener(localParticipantMessageListener) - if (isFirstMessagesProcessing) { pullChatMessages(false) } else { From 6a7f54a5bf43a64ca5157a9f86776a7f301b188a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 15:12:16 +0100 Subject: [PATCH 6/7] Remove debug toast messages Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/controllers/ChatController.kt | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 2fecdf7bb..c68a51a8e 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2095,15 +2095,6 @@ class ChatController(args: Bundle) : "", sessionIdAfterRoomJoined ) - } else { - Log.e(TAG, "magicWebSocketInstance or currentConversation were null! Failed to leave the room!") - if (BuildConfig.DEBUG) { - Toast.makeText( - context, - "magicWebSocketInstance or currentConversation were null! Failed to leave the room!", - Toast.LENGTH_LONG - ).show() - } } sessionIdAfterRoomJoined = "0" @@ -2216,14 +2207,10 @@ class ChatController(args: Bundle) : if (conversationUser == null) { return } - webSocketInstance = WebSocketConnectionHelper.getWebSocketInstanceForUserId(conversationUser.id!!) if (webSocketInstance == null) { - Log.e(TAG, "failed to setup webSocketInstance") - if (BuildConfig.DEBUG) { - Toast.makeText(context, "failed to setup webSocketInstance", Toast.LENGTH_LONG).show() - } + Log.d(TAG, "webSocketInstance not set up. This should only happen when not using the HPB") } } From f05b218743c95bf2624be3d3450306c3cfb74e4e Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Fri, 10 Mar 2023 16:24:15 +0100 Subject: [PATCH 7/7] Extract worker initialization Signed-off-by: Marcel Hibbe --- .../application/NextcloudTalkApplication.kt | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt index c8efea780..130077d8c 100644 --- a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt +++ b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt @@ -177,6 +177,18 @@ class NextcloudTalkApplication : MultiDexApplication(), LifecycleObserver { ClosedInterfaceImpl().providerInstallerInstallIfNeededAsync() DeviceUtils.ignoreSpecialBatteryFeatures() + initWorkers() + + val config = BundledEmojiCompatConfig(this) + config.setReplaceAll(true) + val emojiCompat = EmojiCompat.init(config) + + EmojiManager.install(GoogleEmojiProvider()) + + NotificationUtils.registerNotificationChannels(applicationContext, appPreferences) + } + + private fun initWorkers() { val accountRemovalWork = OneTimeWorkRequest.Builder(AccountRemovalWorker::class.java).build() val capabilitiesUpdateWork = OneTimeWorkRequest.Builder(CapabilitiesWorker::class.java).build() val signalingSettingsWork = OneTimeWorkRequest.Builder(SignalingSettingsWorker::class.java).build() @@ -199,14 +211,6 @@ class NextcloudTalkApplication : MultiDexApplication(), LifecycleObserver { ExistingPeriodicWorkPolicy.REPLACE, periodicCapabilitiesUpdateWork ) - - val config = BundledEmojiCompatConfig(this) - config.setReplaceAll(true) - val emojiCompat = EmojiCompat.init(config) - - EmojiManager.install(GoogleEmojiProvider()) - - NotificationUtils.registerNotificationChannels(applicationContext, appPreferences) } override fun onTerminate() {