diff --git a/CHANGES.md b/CHANGES.md index 08f375f8c1..3fc21c7d07 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,11 @@ +Changes in Element v1.5.25 (2023-02-15) +======================================= + +Bugfixes 🐛 +---------- + - CountUpTimer - Fix StackOverFlow exception when stop action is called within the tick event ([#8127](https://github.com/vector-im/element-android/issues/8127)) + + Changes in Element v1.5.24 (2023-02-08) ======================================= diff --git a/fastlane/metadata/android/en-US/changelogs/40105250.txt b/fastlane/metadata/android/en-US/changelogs/40105250.txt new file mode 100644 index 0000000000..aaceef9ce6 --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/40105250.txt @@ -0,0 +1,2 @@ +Main changes in this version: Mainly bugfixing, in particular fix message not appearing on the timeline. +Full changelog: https://github.com/vector-im/element-android/releases diff --git a/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt index 3ed63a407b..17af1c3190 100644 --- a/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt +++ b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/CountUpTimer.kt @@ -33,12 +33,15 @@ class CountUpTimer( private val lastTime: AtomicLong = AtomicLong(clock.epochMillis()) private val elapsedTime: AtomicLong = AtomicLong(0) + // To ensure that the regular tick value is an exact multiple of `intervalInMs` + private val specialRound = SpecialRound(intervalInMs) private fun startCounter() { + counterJob?.cancel() counterJob = coroutineScope.launch { while (true) { delay(intervalInMs - elapsedTime() % intervalInMs) - tickListener?.onTick(elapsedTime()) + tickListener?.onTick(specialRound.round(elapsedTime())) } } } @@ -54,29 +57,54 @@ class CountUpTimer( } } + /** + * Start a new timer with the initial given time, if any. + * If the timer is already started, it will be restarted. + */ fun start(initialTime: Long = 0L) { elapsedTime.set(initialTime) - resume() - } - - fun pause() { - tickListener?.onTick(elapsedTime()) - counterJob?.cancel() - counterJob = null - } - - fun resume() { lastTime.set(clock.epochMillis()) startCounter() } + /** + * Pause the timer at the current time. + */ + fun pause() { + pauseAndTick() + } + + /** + * Resume the timer from the current time. + * Does nothing if the timer is already running. + */ + fun resume() { + if (counterJob?.isActive != true) { + lastTime.set(clock.epochMillis()) + startCounter() + } + } + + /** + * Stop and reset the timer. + */ fun stop() { - tickListener?.onTick(elapsedTime()) - counterJob?.cancel() - counterJob = null + pauseAndTick() elapsedTime.set(0L) } + private fun pauseAndTick() { + if (counterJob?.isActive == true) { + // get the elapsed time before cancelling the timer + val elapsedTime = elapsedTime() + // cancel the timer before ticking + counterJob?.cancel() + counterJob = null + // tick with the computed elapsed time + tickListener?.onTick(elapsedTime) + } + } + fun interface TickListener { fun onTick(milliseconds: Long) } diff --git a/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/SpecialRound.kt b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/SpecialRound.kt new file mode 100644 index 0000000000..82fead13e0 --- /dev/null +++ b/library/core-utils/src/main/java/im/vector/lib/core/utils/timer/SpecialRound.kt @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2023 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.lib.core.utils.timer + +import kotlin.math.round + +class SpecialRound(private val step: Long) { + /** + * Round the provided value to the nearest multiple of `step`. + */ + fun round(value: Long): Long { + return round(value.toDouble() / step).toLong() * step + } +} diff --git a/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/CountUpTimerTest.kt b/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/CountUpTimerTest.kt index 83f11900b1..27d07bba66 100644 --- a/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/CountUpTimerTest.kt +++ b/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/CountUpTimerTest.kt @@ -19,6 +19,8 @@ package im.vector.lib.core.utils.timer import im.vector.lib.core.utils.test.fakes.FakeClock import io.mockk.every import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify import io.mockk.verifySequence import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceTimeBy @@ -36,6 +38,7 @@ internal class CountUpTimerTest { @Test fun `when pausing and resuming the timer, the timer ticks the right values at the right moments`() = runTest { + // Given every { fakeClock.epochMillis() } answers { currentTime } val tickListener = mockk(relaxed = true) val timer = CountUpTimer( @@ -44,6 +47,7 @@ internal class CountUpTimerTest { intervalInMs = AN_INTERVAL, ).also { it.tickListener = tickListener } + // When timer.start() advanceTimeBy(AN_INTERVAL / 2) // no tick timer.pause() // tick @@ -52,6 +56,7 @@ internal class CountUpTimerTest { advanceTimeBy(AN_INTERVAL * 4) // tick * 4 timer.stop() // tick + // Then verifySequence { tickListener.onTick(AN_INTERVAL / 2) tickListener.onTick(AN_INTERVAL) @@ -64,6 +69,7 @@ internal class CountUpTimerTest { @Test fun `given an initial time, the timer ticks the right values at the right moments`() = runTest { + // Given every { fakeClock.epochMillis() } answers { currentTime } val tickListener = mockk(relaxed = true) val timer = CountUpTimer( @@ -72,6 +78,7 @@ internal class CountUpTimerTest { intervalInMs = AN_INTERVAL, ).also { it.tickListener = tickListener } + // When timer.start(AN_INITIAL_TIME) advanceTimeBy(AN_INTERVAL) // tick timer.pause() // tick @@ -80,6 +87,7 @@ internal class CountUpTimerTest { advanceTimeBy(AN_INTERVAL * 4) // tick * 4 timer.stop() // tick + // Then val offset = AN_INITIAL_TIME % AN_INTERVAL verifySequence { tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL - offset) @@ -91,4 +99,54 @@ internal class CountUpTimerTest { tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 5) } } + + @Test + fun `when stopping the timer on tick, the stop action is called twice and the timer ticks twice`() = runTest { + // Given + every { fakeClock.epochMillis() } answers { currentTime } + val timer = spyk( + CountUpTimer( + coroutineScope = this, + clock = fakeClock, + intervalInMs = AN_INTERVAL, + ) + ) + val tickListener = mockk { + every { onTick(any()) } answers { timer.stop() } + } + timer.tickListener = tickListener + + // When + timer.start() + advanceTimeBy(AN_INTERVAL * 10) + + // Then + verify(exactly = 2) { timer.stop() } // one call at the first tick, a second time because of the tick of the first stop + verify(exactly = 2) { tickListener.onTick(any()) } // one after reaching the first interval, a second after the stop action + } + + @Test + fun `when pausing the timer on tick, the pause action is called twice and the timer ticks twice`() = runTest { + // Given + every { fakeClock.epochMillis() } answers { currentTime } + val timer = spyk( + CountUpTimer( + coroutineScope = this, + clock = fakeClock, + intervalInMs = AN_INTERVAL, + ) + ) + val tickListener = mockk { + every { onTick(any()) } answers { timer.pause() } + } + timer.tickListener = tickListener + + // When + timer.start() + advanceTimeBy(AN_INTERVAL * 10) + + // Then + verify(exactly = 2) { timer.pause() } // one call at the first tick, a second time because of the tick of the first pause + verify(exactly = 2) { tickListener.onTick(any()) } // one after reaching the first interval, a second after the pause action + } } diff --git a/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/SpecialRoundTest.kt b/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/SpecialRoundTest.kt new file mode 100644 index 0000000000..c41557ee2f --- /dev/null +++ b/library/core-utils/src/test/java/im/vector/lib/core/utils/timer/SpecialRoundTest.kt @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2023 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.lib.core.utils.timer + +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +class SpecialRoundTest { + @Test + fun `test special round 500`() { + val sut = SpecialRound(500) + sut.round(1) shouldBeEqualTo 0 + sut.round(499) shouldBeEqualTo 500 + sut.round(500) shouldBeEqualTo 500 + sut.round(501) shouldBeEqualTo 500 + sut.round(999) shouldBeEqualTo 1_000 + sut.round(1000) shouldBeEqualTo 1_000 + sut.round(1001) shouldBeEqualTo 1_000 + sut.round(1499) shouldBeEqualTo 1_500 + sut.round(1500) shouldBeEqualTo 1_500 + sut.round(1501) shouldBeEqualTo 1_500 + } + + @Test + fun `test special round 1_000`() { + val sut = SpecialRound(1_000) + sut.round(1) shouldBeEqualTo 0 + sut.round(499) shouldBeEqualTo 0 + sut.round(500) shouldBeEqualTo 0 + sut.round(501) shouldBeEqualTo 1_000 + sut.round(999) shouldBeEqualTo 1_000 + sut.round(1000) shouldBeEqualTo 1_000 + sut.round(1001) shouldBeEqualTo 1_000 + sut.round(1499) shouldBeEqualTo 1_000 + sut.round(1500) shouldBeEqualTo 2_000 + sut.round(1501) shouldBeEqualTo 2_000 + } +} diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index 33afbfe02c..ed15d0c8f9 100644 --- a/matrix-sdk-android/build.gradle +++ b/matrix-sdk-android/build.gradle @@ -62,7 +62,7 @@ android { // that the app's state is completely cleared between tests. testInstrumentationRunnerArguments clearPackageData: 'true' - buildConfigField "String", "SDK_VERSION", "\"1.5.24\"" + buildConfigField "String", "SDK_VERSION", "\"1.5.25\"" buildConfigField "String", "GIT_SDK_REVISION", "\"${gitRevision()}\"" buildConfigField "String", "GIT_SDK_REVISION_UNIX_DATE", "\"${gitRevisionUnixDate()}\"" diff --git a/vector-app/build.gradle b/vector-app/build.gradle index 5fc3158fe5..267da95c67 100644 --- a/vector-app/build.gradle +++ b/vector-app/build.gradle @@ -37,7 +37,7 @@ ext.versionMinor = 5 // Note: even values are reserved for regular release, odd values for hotfix release. // When creating a hotfix, you should decrease the value, since the current value // is the value for the next regular release. -ext.versionPatch = 24 +ext.versionPatch = 25 static def getGitTimestamp() { def cmd = 'git show -s --format=%ct'