Merge branch 'hotfix/1.5.25' into main

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)
 =======================================
 

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

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,27 +57,52 @@ 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())
+        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
-        elapsedTime.set(0L)
+            // tick with the computed elapsed time
+            tickListener?.onTick(elapsedTime)
+        }
     }
 
     fun interface TickListener {

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
+    }
+}

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<CountUpTimer.TickListener>(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<CountUpTimer.TickListener>(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<CountUpTimer.TickListener> {
+            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<CountUpTimer.TickListener> {
+            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
+    }
 }

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
+    }
+}

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()}\""

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'