element-hq · Florian14 · Feb 6, 2023 · Jan 31, 2023 · Jan 31, 2023 · Jan 31, 2023
diff --git a/changelog.d/8058.misc b/changelog.d/8058.misc
@@ -0,0 +1 @@
+Improve the `CountUpTimer` implementation
diff --git a/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt b/library/attachment-viewer/src/main/java/im/vector/lib/attachmentviewer/VideoViewHolder.kt
@@ -110,7 +110,7 @@ class VideoViewHolder constructor(itemView: View) :
                     //                        Log.v("FOO", "isPlaying $isPlaying $progress/$duration")
                     eventListener?.get()?.onEvent(AttachmentEvents.VideoEvent(isPlaying, progress, duration))
                 }
-                it.resume()
+                it.start()
             }
         }
         try {

diff --git a/library/core-utils/build.gradle b/library/core-utils/build.gradle
@@ -53,4 +53,12 @@ android {
 
 dependencies {
     implementation libs.jetbrains.coroutinesAndroid
+
+    // TESTS
+    testImplementation libs.tests.junit
+    testImplementation libs.tests.kluent
+    testImplementation libs.mockk.mockk
+    testImplementation(libs.jetbrains.coroutinesTest) {
+        exclude group: "org.jetbrains.kotlinx", module: "kotlinx-coroutines-debug"
+    }
 }
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
@@ -16,63 +16,65 @@
 
 package im.vector.lib.core.utils.timer
 
-import im.vector.lib.core.utils.flow.tickerFlow
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.cancel
-import kotlinx.coroutines.flow.filter
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.onEach
-import java.util.concurrent.atomic.AtomicBoolean
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import java.util.concurrent.atomic.AtomicLong
 
-@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class)
-class CountUpTimer(initialTime: Long = 0L, private val intervalInMs: Long = 1_000) {
+class CountUpTimer(
+        private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.Main),
+        private val clock: Clock = DefaultClock(),
+        private val intervalInMs: Long = 1_000,
+) {
 
-    private val coroutineScope = CoroutineScope(Dispatchers.Main)
-    private val resumed: AtomicBoolean = AtomicBoolean(false)
+    private var counterJob: Job? = null
 
-    private val clock: Clock = DefaultClock()
-    private val lastTime: AtomicLong = AtomicLong()
-    private val elapsedTime: AtomicLong = AtomicLong(initialTime)
-
-    init {
-        startCounter()
-    }
+    private val lastTime: AtomicLong = AtomicLong(clock.epochMillis())
+    private val elapsedTime: AtomicLong = AtomicLong(0)
 
     private fun startCounter() {
-        tickerFlow(coroutineScope, intervalInMs)
-                .filter { resumed.get() }
-                .map { elapsedTime() }
-                .onEach { tickListener?.onTick(it) }
-                .launchIn(coroutineScope)
+        counterJob = coroutineScope.launch {
+            while (true) {
+                delay(intervalInMs - elapsedTime() % intervalInMs)
+                tickListener?.onTick(elapsedTime())
+            }
+        }
     }
 
     var tickListener: TickListener? = null
 
     fun elapsedTime(): Long {
-        return if (resumed.get()) {
+        return if (counterJob?.isActive == true) {
             val now = clock.epochMillis()
             elapsedTime.addAndGet(now - lastTime.getAndSet(now))
         } else {
             elapsedTime.get()
         }
     }
 
+    fun start(initialTime: Long = 0L) {
+        elapsedTime.set(initialTime)
+        resume()
+    }
+
     fun pause() {
         tickListener?.onTick(elapsedTime())
-        resumed.set(false)
+        counterJob?.cancel()
+        counterJob = null
     }
 
     fun resume() {
         lastTime.set(clock.epochMillis())
-        resumed.set(true)
+        startCounter()
     }
 
     fun stop() {
         tickListener?.onTick(elapsedTime())
-        coroutineScope.cancel()
+        counterJob?.cancel()
+        counterJob = null
+        elapsedTime.set(0L)
     }
 
     fun interface TickListener {

diff --git a/...ain/java/im/vector/app/core/time/Clock.kt → ...or/lib/core/utils/test/fakes/FakeClock.kt b/...ain/java/im/vector/app/core/time/Clock.kt → ...or/lib/core/utils/test/fakes/FakeClock.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 New Vector Ltd
+ * 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.
@@ -14,23 +14,14 @@
  * limitations under the License.
  */
 
-package im.vector.app.core.time
+package im.vector.lib.core.utils.test.fakes
 
-import javax.inject.Inject
+import im.vector.lib.core.utils.timer.Clock
+import io.mockk.every
+import io.mockk.mockk
 
-interface Clock {
-    fun epochMillis(): Long
-}
-
-class DefaultClock @Inject constructor() : Clock {
-
-    /**
-     * Provides a UTC epoch in milliseconds
-     *
-     * This value is not guaranteed to be correct with reality
-     * as a User can override the system time and date to any values.
-     */
-    override fun epochMillis(): Long {
-        return System.currentTimeMillis()
+class FakeClock : Clock by mockk() {
+    fun givenEpoch(epoch: Long) {
+        every { epochMillis() } returns epoch
     }
 }
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
@@ -0,0 +1,94 @@
+/*
+ * 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 im.vector.lib.core.utils.test.fakes.FakeClock
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verifySequence
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.currentTime
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+
+private const val AN_INTERVAL = 500L
+private const val AN_INITIAL_TIME = 2_333L
+
+@OptIn(ExperimentalCoroutinesApi::class)
+internal class CountUpTimerTest {
+
+    private val fakeClock = FakeClock()
+
+    @Test
+    fun `when pausing and resuming the timer, the timer ticks the right values at the right moments`() = runTest {
+        every { fakeClock.epochMillis() } answers { currentTime }
+        val tickListener = mockk<CountUpTimer.TickListener>(relaxed = true)
+        val timer = CountUpTimer(
+                coroutineScope = this,
+                clock = fakeClock,
+                intervalInMs = AN_INTERVAL,
+        ).also { it.tickListener = tickListener }
+
+        timer.start()
+        advanceTimeBy(AN_INTERVAL / 2) // no tick
+        timer.pause() // tick
+        advanceTimeBy(AN_INTERVAL * 10) // no tick
+        timer.resume() // no tick
+        advanceTimeBy(AN_INTERVAL * 4) // tick * 4
+        timer.stop() // tick
+
+        verifySequence {
+            tickListener.onTick(AN_INTERVAL / 2)
+            tickListener.onTick(AN_INTERVAL)
+            tickListener.onTick(AN_INTERVAL * 2)
+            tickListener.onTick(AN_INTERVAL * 3)
+            tickListener.onTick(AN_INTERVAL * 4)
+            tickListener.onTick(AN_INTERVAL * 4 + AN_INTERVAL / 2)
+        }
+    }
+
+    @Test
+    fun `given an initial time, the timer ticks the right values at the right moments`() = runTest {
+        every { fakeClock.epochMillis() } answers { currentTime }
+        val tickListener = mockk<CountUpTimer.TickListener>(relaxed = true)
+        val timer = CountUpTimer(
+                coroutineScope = this,
+                clock = fakeClock,
+                intervalInMs = AN_INTERVAL,
+        ).also { it.tickListener = tickListener }
+
+        timer.start(AN_INITIAL_TIME)
+        advanceTimeBy(AN_INTERVAL) // tick
+        timer.pause() // tick
+        advanceTimeBy(AN_INTERVAL * 10) // no tick
+        timer.resume() // no tick
+        advanceTimeBy(AN_INTERVAL * 4) // tick * 4
+        timer.stop() // tick
+
+        val offset = AN_INITIAL_TIME % AN_INTERVAL
+        verifySequence {
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL - offset)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 2 - offset)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 3 - offset)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 4 - offset)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 5 - offset)
+            tickListener.onTick(AN_INITIAL_TIME + AN_INTERVAL * 5)
+        }
+    }
+}
diff --git a/tools/check/forbidden_strings_in_code.txt b/tools/check/forbidden_strings_in_code.txt
@@ -176,7 +176,7 @@ PreferenceManager\.getDefaultSharedPreferences==2
 R\.string\.template_
 
 ### Use the Clock interface, or use `measureTimeMillis`
-System\.currentTimeMillis\(\)===3
+System\.currentTimeMillis\(\)===2
 
 ### Remove extra space between the name and the description
 \* @\w+ \w+  +

diff --git a/vector-app/build.gradle b/vector-app/build.gradle
@@ -347,6 +347,7 @@ android {
 dependencies {
     implementation project(':vector')
     implementation project(':vector-config')
+    implementation project(':library:core-utils')
     debugImplementation project(':library:ui-styles')
     implementation libs.dagger.hilt
     implementation 'androidx.multidex:multidex:2.0.1'

diff --git a/vector-app/src/androidTest/java/im/vector/app/EspressoExt.kt b/vector-app/src/androidTest/java/im/vector/app/EspressoExt.kt
@@ -43,8 +43,8 @@ import com.google.android.material.bottomsheet.BottomSheetBehavior
 import com.google.android.material.bottomsheet.BottomSheetDialog
 import com.google.android.material.bottomsheet.BottomSheetDialogFragment
 import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment
-import im.vector.app.core.time.DefaultClock
 import im.vector.app.espresso.tools.waitUntilViewVisible
+import im.vector.lib.core.utils.timer.DefaultClock
 import org.hamcrest.Matcher
 import org.hamcrest.Matchers
 import org.hamcrest.StringDescription

diff --git a/vector-app/src/androidTest/java/im/vector/app/espresso/tools/ScreenshotFailureRule.kt b/vector-app/src/androidTest/java/im/vector/app/espresso/tools/ScreenshotFailureRule.kt
@@ -24,7 +24,7 @@ import android.os.Build
 import android.os.Environment
 import android.provider.MediaStore
 import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation
-import im.vector.app.core.time.DefaultClock
+import im.vector.lib.core.utils.timer.DefaultClock
 import org.junit.rules.TestWatcher
 import org.junit.runner.Description
 import timber.log.Timber

diff --git a/vector-app/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt b/vector-app/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt
@@ -28,7 +28,6 @@ import dagger.hilt.android.AndroidEntryPoint
 import im.vector.app.R
 import im.vector.app.core.extensions.registerStartForActivityResult
 import im.vector.app.core.platform.VectorBaseActivity
-import im.vector.app.core.time.Clock
 import im.vector.app.core.utils.PERMISSIONS_FOR_TAKING_PHOTO
 import im.vector.app.core.utils.checkPermissions
 import im.vector.app.core.utils.registerForPermissionsResult
@@ -41,6 +40,7 @@ import im.vector.app.features.debug.sas.DebugSasEmojiActivity
 import im.vector.app.features.debug.settings.DebugPrivateSettingsActivity
 import im.vector.app.features.qrcode.QrCodeScannerActivity
 import im.vector.application.databinding.ActivityDebugMenuBinding
+import im.vector.lib.core.utils.timer.Clock
 import im.vector.lib.ui.styles.debug.DebugMaterialThemeDarkDefaultActivity
 import im.vector.lib.ui.styles.debug.DebugMaterialThemeDarkTestActivity
 import im.vector.lib.ui.styles.debug.DebugMaterialThemeDarkVectorActivity

diff --git a/vector-app/src/fdroid/java/im/vector/app/fdroid/BackgroundSyncStarter.kt b/vector-app/src/fdroid/java/im/vector/app/fdroid/BackgroundSyncStarter.kt
@@ -18,10 +18,10 @@ package im.vector.app.fdroid
 
 import android.content.Context
 import im.vector.app.core.di.ActiveSessionHolder
-import im.vector.app.core.time.Clock
 import im.vector.app.fdroid.receiver.AlarmSyncBroadcastReceiver
 import im.vector.app.features.settings.BackgroundSyncMode
 import im.vector.app.features.settings.VectorPreferences
+import im.vector.lib.core.utils.timer.Clock
 import timber.log.Timber
 import javax.inject.Inject
 

diff --git a/vector-app/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt b/vector-app/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt
@@ -28,7 +28,7 @@ import androidx.core.content.getSystemService
 import im.vector.app.core.extensions.singletonEntryPoint
 import im.vector.app.core.platform.PendingIntentCompat
 import im.vector.app.core.services.VectorSyncAndroidService
-import im.vector.app.core.time.Clock
+import im.vector.lib.core.utils.timer.Clock
 import org.matrix.android.sdk.api.session.sync.job.SyncAndroidService
 import timber.log.Timber
 

diff --git a/vector-app/src/gplay/java/im/vector/app/nightly/FirebaseNightlyProxy.kt b/vector-app/src/gplay/java/im/vector/app/nightly/FirebaseNightlyProxy.kt
@@ -22,8 +22,8 @@ import com.google.firebase.appdistribution.FirebaseAppDistribution
 import com.google.firebase.appdistribution.FirebaseAppDistributionException
 import im.vector.app.core.di.DefaultPreferences
 import im.vector.app.core.resources.BuildMeta
-import im.vector.app.core.time.Clock
 import im.vector.app.features.home.NightlyProxy
+import im.vector.lib.core.utils.timer.Clock
 import timber.log.Timber
 import javax.inject.Inject
 

diff --git a/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt b/vector-app/src/main/java/im/vector/app/core/di/SingletonModule.kt
@@ -40,8 +40,6 @@ import im.vector.app.core.dispatchers.CoroutineDispatchers
 import im.vector.app.core.error.DefaultErrorFormatter
 import im.vector.app.core.error.ErrorFormatter
 import im.vector.app.core.resources.BuildMeta
-import im.vector.app.core.time.Clock
-import im.vector.app.core.time.DefaultClock
 import im.vector.app.core.utils.AndroidSystemSettingsProvider
 import im.vector.app.core.utils.SystemSettingsProvider
 import im.vector.app.features.analytics.AnalyticsTracker
@@ -63,6 +61,8 @@ import im.vector.app.features.settings.VectorPreferences
 import im.vector.app.features.ui.SharedPreferencesUiStateRepository
 import im.vector.app.features.ui.UiStateRepository
 import im.vector.application.BuildConfig
+import im.vector.lib.core.utils.timer.Clock
+import im.vector.lib.core.utils.timer.DefaultClock
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.Dispatchers
@@ -106,9 +106,6 @@ import javax.inject.Singleton
     @Binds
     abstract fun bindAutoAcceptInvites(autoAcceptInvites: CompileTimeAutoAcceptInvites): AutoAcceptInvites
 
-    @Binds
-    abstract fun bindDefaultClock(clock: DefaultClock): Clock
-
     @Binds
     abstract fun bindEmojiSpanify(emojiCompatWrapper: EmojiCompatWrapper): EmojiSpanify
 
@@ -243,4 +240,8 @@ import javax.inject.Singleton
     fun providesDefaultSharedPreferences(context: Context): SharedPreferences {
         return PreferenceManager.getDefaultSharedPreferences(context.applicationContext)
     }
+
+    @Singleton
+    @Provides
+    fun providesDefaultClock(): Clock = DefaultClock()
 }
diff --git a/vector/src/main/java/im/vector/app/core/date/VectorDateFormatter.kt b/vector/src/main/java/im/vector/app/core/date/VectorDateFormatter.kt
@@ -22,7 +22,7 @@ import android.text.format.DateUtils
 import im.vector.app.core.resources.DateProvider
 import im.vector.app.core.resources.LocaleProvider
 import im.vector.app.core.resources.toTimestamp
-import im.vector.app.core.time.Clock
+import im.vector.lib.core.utils.timer.Clock
 import org.threeten.bp.LocalDateTime
 import org.threeten.bp.Period
 import org.threeten.bp.format.DateTimeFormatter

diff --git a/vector/src/main/java/im/vector/app/core/di/SingletonEntryPoint.kt b/vector/src/main/java/im/vector/app/core/di/SingletonEntryPoint.kt
@@ -21,7 +21,6 @@ import dagger.hilt.InstallIn
 import dagger.hilt.components.SingletonComponent
 import im.vector.app.core.dialogs.UnrecognizedCertificateDialog
 import im.vector.app.core.error.ErrorFormatter
-import im.vector.app.core.time.Clock
 import im.vector.app.features.analytics.AnalyticsTracker
 import im.vector.app.features.call.webrtc.WebRtcCallManager
 import im.vector.app.features.home.AvatarRenderer
@@ -31,6 +30,7 @@ import im.vector.app.features.rageshake.BugReporter
 import im.vector.app.features.session.SessionListener
 import im.vector.app.features.settings.VectorPreferences
 import im.vector.app.features.ui.UiStateRepository
+import im.vector.lib.core.utils.timer.Clock
 import kotlinx.coroutines.CoroutineScope
 
 @InstallIn(SingletonComponent::class)