From b9b0e847044ae09018fa2417277faa3d76b99c35 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Mon, 23 May 2022 14:09:18 +0200 Subject: [PATCH 01/10] Adds set method to MutableDataSource --- .../main/java/im/vector/app/core/utils/DataSource.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index f83eda68e9..32c1cf9424 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -25,6 +25,9 @@ interface DataSource { } interface MutableDataSource : DataSource { + + suspend fun set(value: T) + fun post(value: T) } @@ -42,6 +45,10 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD return mutableFlow } + override suspend fun set(value: T) { + mutableFlow.emit(value) + } + override fun post(value: T) { mutableFlow.tryEmit(value) } @@ -58,6 +65,10 @@ open class PublishDataSource : MutableDataSource { return mutableFlow } + override suspend fun set(value: T) { + mutableFlow.emit(value) + } + override fun post(value: T) { mutableFlow.tryEmit(value) } From f831252e354c94fa47fc1480618b5822e1abd468 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Mon, 23 May 2022 14:09:29 +0200 Subject: [PATCH 02/10] Fixes UpgradeRoom command not working --- .../composer/MessageComposerViewModel.kt | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt index eca5c846ca..d0247553aa 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt @@ -472,14 +472,16 @@ class MessageComposerViewModel @AssistedInject constructor( Unit } is ParsedCommand.UpgradeRoom -> { - _viewEvents.post( - MessageComposerViewEvents.ShowRoomUpgradeDialog( - parsedCommand.newVersion, - room.roomSummary()?.isPublic ?: false - ) - ) - _viewEvents.post(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) - popDraft() + viewModelScope.launch { + _viewEvents.set( + MessageComposerViewEvents.ShowRoomUpgradeDialog( + parsedCommand.newVersion, + room.roomSummary()?.isPublic ?: false + ) + ) + _viewEvents.set(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) + popDraft() + } } } } From b0ce32e97ec17947d6f909bff9417427de2595fe Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 25 May 2022 15:37:31 +0200 Subject: [PATCH 03/10] Adds changelog file --- changelog.d/6154.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6154.bugfix diff --git a/changelog.d/6154.bugfix b/changelog.d/6154.bugfix new file mode 100644 index 0000000000..5c64eb2879 --- /dev/null +++ b/changelog.d/6154.bugfix @@ -0,0 +1 @@ +Fixed /upgraderoom command not doing anything From e84f012b735a58d72cddbacafe03145e70da14ae Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 26 May 2022 15:45:18 +0200 Subject: [PATCH 04/10] Changes set method name to emit --- vector/src/main/java/im/vector/app/core/utils/DataSource.kt | 6 +++--- .../home/room/detail/composer/MessageComposerViewModel.kt | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index 32c1cf9424..ff55b05689 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -26,7 +26,7 @@ interface DataSource { interface MutableDataSource : DataSource { - suspend fun set(value: T) + suspend fun emit(value: T) fun post(value: T) } @@ -45,7 +45,7 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD return mutableFlow } - override suspend fun set(value: T) { + override suspend fun emit(value: T) { mutableFlow.emit(value) } @@ -65,7 +65,7 @@ open class PublishDataSource : MutableDataSource { return mutableFlow } - override suspend fun set(value: T) { + override suspend fun emit(value: T) { mutableFlow.emit(value) } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt index d0247553aa..fcc4313558 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt @@ -473,13 +473,13 @@ class MessageComposerViewModel @AssistedInject constructor( } is ParsedCommand.UpgradeRoom -> { viewModelScope.launch { - _viewEvents.set( + _viewEvents.emit( MessageComposerViewEvents.ShowRoomUpgradeDialog( parsedCommand.newVersion, room.roomSummary()?.isPublic ?: false ) ) - _viewEvents.set(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) + _viewEvents.emit(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) popDraft() } } From c48fd7708cf00c9c6cf88b0af062a66153107822 Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 31 May 2022 20:11:54 +0200 Subject: [PATCH 05/10] Increase extraBufferCapacity (and allow configuration) and validate behavior with tests --- .../im/vector/app/core/utils/DataSource.kt | 7 +- .../composer/MessageComposerViewModel.kt | 18 ++-- .../vector/app/core/utils/DataSourceTest.kt | 82 +++++++++++++++++++ 3 files changed, 95 insertions(+), 12 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index ff55b05689..21316a7bc0 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -56,10 +56,13 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD /** * This datasource only emits all subsequent observed values to each subscriber. + * + * @param bufferSize number of buffered items before it starts dropping oldest. Should be at least 1 + * */ -open class PublishDataSource : MutableDataSource { +open class PublishDataSource(bufferSize: Int = 10) : MutableDataSource { - private val mutableFlow = MutableSharedFlow(replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) + private val mutableFlow = MutableSharedFlow(replay = 0, extraBufferCapacity = bufferSize, onBufferOverflow = BufferOverflow.DROP_OLDEST) override fun stream(): Flow { return mutableFlow diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt index fcc4313558..eca5c846ca 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/MessageComposerViewModel.kt @@ -472,16 +472,14 @@ class MessageComposerViewModel @AssistedInject constructor( Unit } is ParsedCommand.UpgradeRoom -> { - viewModelScope.launch { - _viewEvents.emit( - MessageComposerViewEvents.ShowRoomUpgradeDialog( - parsedCommand.newVersion, - room.roomSummary()?.isPublic ?: false - ) - ) - _viewEvents.emit(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) - popDraft() - } + _viewEvents.post( + MessageComposerViewEvents.ShowRoomUpgradeDialog( + parsedCommand.newVersion, + room.roomSummary()?.isPublic ?: false + ) + ) + _viewEvents.post(MessageComposerViewEvents.SlashCommandResultOk(parsedCommand)) + popDraft() } } } diff --git a/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt new file mode 100644 index 0000000000..c9b351d95d --- /dev/null +++ b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2022 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 im.vector.app.core.utils + +import im.vector.app.test.test +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.test.TestCoroutineScheduler +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import org.amshove.kluent.shouldContainSame +import org.junit.Test + +class DataSourceTest { + + @Test + fun `given PublishDataSource, when posting values before observing, then no value is observed`() = runTest { + val publishDataSource = PublishDataSource() + publishDataSource.post(0) + publishDataSource.post(1) + + publishDataSource.stream() + .test(this) + .assertNoValues() + .finish() + } + + @Test + fun `given PublishDataSource with a large enough buffer size, when posting values after observing, then only the latest values are observed`() = runTest { + val valuesToPost = listOf(2, 3, 4, 5, 6, 7, 8, 9) + val publishDataSource = PublishDataSource(bufferSize = valuesToPost.size) + publishDataSource.test(testScheduler, valuesToPost, valuesToPost) + } + + @Test + fun `given PublishDataSource with a too small buffer size, when posting values after observing, then we are missing some values`() = runTest { + val valuesToPost = listOf(2, 3, 4, 5, 6, 7, 8, 9) + val expectedValues = listOf(2, 9) + val publishDataSource = PublishDataSource(bufferSize = 1) + publishDataSource.test(testScheduler, valuesToPost, expectedValues) + + } + + private suspend fun PublishDataSource.test(testScheduler: TestCoroutineScheduler, valuesToPost: List, expectedValues: List) { + val values = ArrayList() + val job = stream() + .onEach { + // Artificial delay to make consumption longer than production + delay(10) + values.add(it) + } + .launchIn(CoroutineScope(UnconfinedTestDispatcher(testScheduler))) + + valuesToPost.forEach { + post(it) + } + withContext(Dispatchers.Default) { + delay(11L * valuesToPost.size) + } + job.cancel() + + values shouldContainSame expectedValues + } +} From f3d7127f17a671ecb8f355dd696a34e7371d2714 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 3 Jun 2022 11:41:47 +0200 Subject: [PATCH 06/10] Fixes lint error --- vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt index c9b351d95d..f7b6d4a2c8 100644 --- a/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt +++ b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt @@ -56,7 +56,6 @@ class DataSourceTest { val expectedValues = listOf(2, 9) val publishDataSource = PublishDataSource(bufferSize = 1) publishDataSource.test(testScheduler, valuesToPost, expectedValues) - } private suspend fun PublishDataSource.test(testScheduler: TestCoroutineScheduler, valuesToPost: List, expectedValues: List) { From 9c1c87ba5d6c6f102bc5183bdbb04648dc41263d Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 3 Jun 2022 12:05:12 +0200 Subject: [PATCH 07/10] Fixes detekt error on java doc --- vector/src/main/java/im/vector/app/core/utils/DataSource.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index 21316a7bc0..6deca9a6ea 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -57,8 +57,7 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD /** * This datasource only emits all subsequent observed values to each subscriber. * - * @param bufferSize number of buffered items before it starts dropping oldest. Should be at least 1 - * + * @property bufferSize number of buffered items before it starts dropping oldest. Should be at least 1 */ open class PublishDataSource(bufferSize: Int = 10) : MutableDataSource { From a5fd11c20446988e186b431e25d959d9e398ff3c Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 3 Jun 2022 12:50:05 +0200 Subject: [PATCH 08/10] Fixes detekt error --- vector/src/main/java/im/vector/app/core/utils/DataSource.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index 6deca9a6ea..f0880bb0f9 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -57,7 +57,7 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD /** * This datasource only emits all subsequent observed values to each subscriber. * - * @property bufferSize number of buffered items before it starts dropping oldest. Should be at least 1 + * bufferSize - number of buffered items before it starts dropping oldest. Should be at least 1 */ open class PublishDataSource(bufferSize: Int = 10) : MutableDataSource { From d586f64338231153db216a7eba9a516c4b7bdb53 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 3 Jun 2022 14:53:26 +0200 Subject: [PATCH 09/10] Removes emit method from DataSource --- .../main/java/im/vector/app/core/utils/DataSource.kt | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt index f0880bb0f9..60ad91272b 100644 --- a/vector/src/main/java/im/vector/app/core/utils/DataSource.kt +++ b/vector/src/main/java/im/vector/app/core/utils/DataSource.kt @@ -26,8 +26,6 @@ interface DataSource { interface MutableDataSource : DataSource { - suspend fun emit(value: T) - fun post(value: T) } @@ -45,10 +43,6 @@ open class BehaviorDataSource(private val defaultValue: T? = null) : MutableD return mutableFlow } - override suspend fun emit(value: T) { - mutableFlow.emit(value) - } - override fun post(value: T) { mutableFlow.tryEmit(value) } @@ -67,10 +61,6 @@ open class PublishDataSource(bufferSize: Int = 10) : MutableDataSource { return mutableFlow } - override suspend fun emit(value: T) { - mutableFlow.emit(value) - } - override fun post(value: T) { mutableFlow.tryEmit(value) } From 31b245b8e36640e4ef6a472bf04a773f3223879c Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 8 Jun 2022 09:38:30 +0200 Subject: [PATCH 10/10] Changes test name --- vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt index f7b6d4a2c8..46c4406c8c 100644 --- a/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt +++ b/vector/src/test/java/im/vector/app/core/utils/DataSourceTest.kt @@ -44,7 +44,7 @@ class DataSourceTest { } @Test - fun `given PublishDataSource with a large enough buffer size, when posting values after observing, then only the latest values are observed`() = runTest { + fun `given PublishDataSource with a large enough buffer size, when posting values after observing, then all values are observed`() = runTest { val valuesToPost = listOf(2, 3, 4, 5, 6, 7, 8, 9) val publishDataSource = PublishDataSource(bufferSize = valuesToPost.size) publishDataSource.test(testScheduler, valuesToPost, valuesToPost)