Merge pull request #3146 from element-hq/feature/bma/invokeAllCallbacks

Ensure that all callback plugins are invoked.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesFlowNode.kt

@@ -107,6 +107,7 @@ class MessagesFlowNode @AssistedInject constructor(
     plugins = plugins
 ) {
     data class Inputs(val focusedEventId: EventId?) : NodeInputs
+
     private val inputs = inputs<Inputs>()
 
     sealed interface NavTarget : Parcelable {
@@ -148,7 +149,7 @@ class MessagesFlowNode @AssistedInject constructor(
         data class EditPoll(val eventId: EventId) : NavTarget
     }
 
-    private val callback = plugins<MessagesEntryPoint.Callback>().firstOrNull()
+    private val callbacks = plugins<MessagesEntryPoint.Callback>()
 
     private val mentionSpanProvider = mentionSpanProviderFactory.create(room.sessionId.value)
 
@@ -167,7 +168,7 @@ class MessagesFlowNode @AssistedInject constructor(
             is NavTarget.Messages -> {
                 val callback = object : MessagesNode.Callback {
                     override fun onRoomDetailsClick() {
-                        callback?.onRoomDetailsClick()
+                        callbacks.forEach { it.onRoomDetailsClick() }
                     }
 
                     override fun onEventClick(event: TimelineItem.Event): Boolean {
@@ -179,11 +180,11 @@ class MessagesFlowNode @AssistedInject constructor(
                     }
 
                     override fun onUserDataClick(userId: UserId) {
-                        callback?.onUserDataClick(userId)
+                        callbacks.forEach { it.onUserDataClick(userId) }
                     }
 
                     override fun onPermalinkClick(data: PermalinkData) {
-                        callback?.onPermalinkClick(data)
+                        callbacks.forEach { it.onPermalinkClick(data) }
                     }
 
                     override fun onShowEventDebugInfoClick(eventId: EventId?, debugInfo: TimelineItemDebugInfo) {
@@ -250,7 +251,7 @@ class MessagesFlowNode @AssistedInject constructor(
                 val inputs = ForwardMessagesNode.Inputs(navTarget.eventId)
                 val callback = object : ForwardMessagesNode.Callback {
                     override fun onForwardedToSingleRoom(roomId: RoomId) {
-                        this@MessagesFlowNode.callback?.onForwardedToSingleRoom(roomId)
+                        callbacks.forEach { it.onForwardedToSingleRoom(roomId) }
                     }
                 }
                 createNode<ForwardMessagesNode>(buildContext, listOf(inputs, callback))

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt

@@ -78,7 +78,7 @@ class MessagesNode @AssistedInject constructor(
     private val timelineController: TimelineController,
 ) : Node(buildContext, plugins = plugins), MessagesNavigator {
     private val presenter = presenterFactory.create(this)
-    private val callback = plugins<Callback>().firstOrNull()
+    private val callbacks = plugins<Callback>()
 
     data class Inputs(val focusedEventId: EventId?) : NodeInputs
 
@@ -113,19 +113,25 @@ class MessagesNode @AssistedInject constructor(
     }
 
     private fun onRoomDetailsClick() {
-        callback?.onRoomDetailsClick()
+        callbacks.forEach { it.onRoomDetailsClick() }
     }
 
     private fun onEventClick(event: TimelineItem.Event): Boolean {
-        return callback?.onEventClick(event).orFalse()
+        // Note: cannot use `callbacks.all { it.onEventClick(event) }` because:
+        // - if callbacks is empty, it will return true and we want to return false.
+        // - if a callback returns false, the other callback will not be invoked.
+        return callbacks.takeIf { it.isNotEmpty() }
+            ?.map { it.onEventClick(event) }
+            ?.all { it }
+            .orFalse()
     }
 
     private fun onPreviewAttachments(attachments: ImmutableList<Attachment>) {
-        callback?.onPreviewAttachments(attachments)
+        callbacks.forEach { it.onPreviewAttachments(attachments) }
     }
 
     private fun onUserDataClick(userId: UserId) {
-        callback?.onUserDataClick(userId)
+        callbacks.forEach { it.onUserDataClick(userId) }
     }
 
     private fun onLinkClick(
@@ -137,7 +143,7 @@ class MessagesNode @AssistedInject constructor(
             is PermalinkData.UserLink -> {
                 // Open the room member profile, it will fallback to
                 // the user profile if the user is not in the room
-                callback?.onUserDataClick(permalink.userId)
+                callbacks.forEach { it.onUserDataClick(permalink.userId) }
             }
             is PermalinkData.RoomLink -> {
                 handleRoomLinkClick(permalink, eventSink)
@@ -159,36 +165,36 @@ class MessagesNode @AssistedInject constructor(
                 context.toast("Already viewing this room!")
             }
         } else {
-            callback?.onPermalinkClick(roomLink)
+            callbacks.forEach { it.onPermalinkClick(roomLink) }
         }
     }
 
     override fun onShowEventDebugInfoClick(eventId: EventId?, debugInfo: TimelineItemDebugInfo) {
-        callback?.onShowEventDebugInfoClick(eventId, debugInfo)
+        callbacks.forEach { it.onShowEventDebugInfoClick(eventId, debugInfo) }
     }
 
     override fun onForwardEventClick(eventId: EventId) {
-        callback?.onForwardEventClick(eventId)
+        callbacks.forEach { it.onForwardEventClick(eventId) }
     }
 
     override fun onReportContentClick(eventId: EventId, senderId: UserId) {
-        callback?.onReportMessage(eventId, senderId)
+        callbacks.forEach { it.onReportMessage(eventId, senderId) }
     }
 
     override fun onEditPollClick(eventId: EventId) {
-        callback?.onEditPollClick(eventId)
+        callbacks.forEach { it.onEditPollClick(eventId) }
     }
 
     private fun onSendLocationClick() {
-        callback?.onSendLocationClick()
+        callbacks.forEach { it.onSendLocationClick() }
     }
 
     private fun onCreatePollClick() {
-        callback?.onCreatePollClick()
+        callbacks.forEach { it.onCreatePollClick() }
     }
 
     private fun onJoinCallClick() {
-        callback?.onJoinCallClick(room.roomId)
+        callbacks.forEach { it.onJoinCallClick(room.roomId) }
     }
 
     @Composable

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/SecureBackupFlowNode.kt

@@ -81,7 +81,7 @@ class SecureBackupFlowNode @AssistedInject constructor(
         data object CreateNewRecoveryKey : NavTarget
     }
 
-    private val callback = plugins<SecureBackupEntryPoint.Callback>().firstOrNull()
+    private val callbacks = plugins<SecureBackupEntryPoint.Callback>()
 
     override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
         return when (navTarget) {
@@ -130,8 +130,8 @@ class SecureBackupFlowNode @AssistedInject constructor(
             NavTarget.EnterRecoveryKey -> {
                 val callback = object : SecureBackupEnterRecoveryKeyNode.Callback {
                     override fun onEnterRecoveryKeySuccess() {
-                        if (callback != null) {
-                            callback.onDone()
+                        if (callbacks.isNotEmpty()) {
+                            callbacks.forEach { it.onDone() }
                         } else {
                             backstack.pop()
                         }

tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistCallbackTest.kt

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.tests.konsist
+
+import com.lemonappdev.konsist.api.Konsist
+import com.lemonappdev.konsist.api.verify.assertFalse
+import org.junit.Test
+
+class KonsistCallbackTest {
+    @Test
+    fun `we should not invoke Callback Input directly, we should use forEach`() {
+        Konsist
+            .scopeFromProduction()
+            .files
+            .assertFalse {
+                it.text.contains("callback?.")
+            }
+    }
+}