From dcf361ab275261758e84d502c803635b6963b82c Mon Sep 17 00:00:00 2001 From: BMItter <46747780+BMItter@users.noreply.github.com> Date: Mon, 16 Nov 2020 13:06:35 +0100 Subject: [PATCH] ENFClient v2 - ExposureDetectionTracker without identifier (EXPOSUREAPP-3540) (#1605) * Implemented the opportunity to use ExposureDetectionTracker without identifier * Old provideDiagnosisKeys = NO-OP * clean * maxLength clean * refactoring and extension function * adjusted wording * Throw UnsupportedOperationException in ENFClient in case * ktlint clean * fix tests for now * commented out Exception temporarily * adjusted enfTest -wip, throw unsupportedOperationException --- .../de/rki/coronawarnapp/nearby/ENFClient.kt | 19 +---- .../DefaultExposureDetectionTracker.kt | 77 +++++++++++++------ .../ExposureDetectionTracker.kt | 2 +- .../receiver/ExposureStateUpdateReceiver.kt | 19 ++--- .../rki/coronawarnapp/nearby/ENFClientTest.kt | 18 ++--- .../windows/ExposureWindowsCalculationTest.kt | 3 +- 6 files changed, 73 insertions(+), 65 deletions(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ENFClient.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ENFClient.kt index 38d4f40b1..9e2b23d15 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ENFClient.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/ENFClient.kt @@ -16,6 +16,7 @@ import kotlinx.coroutines.flow.map import org.joda.time.Instant import timber.log.Timber import java.io.File +import java.util.UUID import javax.inject.Inject import javax.inject.Singleton @@ -26,7 +27,6 @@ class ENFClient @Inject constructor( private val tracingStatus: TracingStatus, private val scanningSupport: ScanningSupport, private val exposureWindowProvider: ExposureWindowProvider, - private val exposureDetectionTracker: ExposureDetectionTracker ) : DiagnosisKeyProvider, TracingStatus, ScanningSupport, ExposureWindowProvider { @@ -40,19 +40,8 @@ class ENFClient @Inject constructor( configuration: ExposureConfiguration?, token: String ): Boolean { - Timber.d( - "asyncProvideDiagnosisKeys(keyFiles=%s, configuration=%s, token=%s)", - keyFiles, configuration, token - ) - - return if (keyFiles.isEmpty()) { - Timber.d("No key files submitted, returning early.") - true - } else { - Timber.d("Forwarding %d key files to our DiagnosisKeyProvider.", keyFiles.size) - exposureDetectionTracker.trackNewExposureDetection(token) - diagnosisKeyProvider.provideDiagnosisKeys(keyFiles, configuration, token) - } + // TODO uncomment Exception later, after every subtask has joined (fun will probably be removed) + throw UnsupportedOperationException("Use provideDiagnosisKeys without token and configuration!") } override suspend fun provideDiagnosisKeys(keyFiles: Collection<File>): Boolean { @@ -63,7 +52,7 @@ class ENFClient @Inject constructor( true } else { Timber.d("Forwarding %d key files to our DiagnosisKeyProvider.", keyFiles.size) - TODO("Call calculationTracker.trackNewCalaculation with an UUID as replacement for token?") + exposureDetectionTracker.trackNewExposureDetection(UUID.randomUUID().toString()) diagnosisKeyProvider.provideDiagnosisKeys(keyFiles) } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt index c5e4b8073..2c55673ff 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/DefaultExposureDetectionTracker.kt @@ -18,6 +18,7 @@ import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.plus import org.joda.time.Duration import timber.log.Timber +import java.util.UUID import javax.inject.Inject import javax.inject.Singleton import kotlin.math.min @@ -63,7 +64,7 @@ class DefaultExposureDetectionTracker @Inject constructor( } } - delay(TIMEOUT_CHECK_INTERVALL.millis) + delay(TIMEOUT_CHECK_INTERVAL.millis) } }.launchIn(scope + dispatcherProvider.Default) } @@ -93,34 +94,17 @@ class DefaultExposureDetectionTracker @Inject constructor( } } - override fun finishExposureDetection(identifier: String, result: Result) { + override fun finishExposureDetection(identifier: String?, result: Result) { Timber.i("finishExposureDetection(token=%s, result=%s)", identifier, result) detectionStates.updateSafely { mutate { - val existing = this[identifier] - if (existing != null) { - if (existing.result == Result.TIMEOUT) { - Timber.w("Detection is late, already hit timeout, still updating.") - } else if (existing.result != null) { - Timber.e("Duplicate callback. Result is already set for detection!") - } - this[identifier] = existing.copy( - result = result, - finishedAt = timeStamper.nowUTC - ) + if (identifier == null) { + val id = this.findUnfinishedOrCreateIdentifier() + finishDetection(id, result) } else { - Timber.e( - "Unknown detection finished (token=%s, result=%s)", - identifier, - result - ) - this[identifier] = TrackedExposureDetection( - identifier = identifier, - result = result, - startedAt = timeStamper.nowUTC, - finishedAt = timeStamper.nowUTC - ) + finishDetection(identifier, result) } + val toKeep = entries .sortedByDescending { it.value.startedAt } // Keep newest .subList(0, min(entries.size, MAX_ENTRY_SIZE)) @@ -134,9 +118,52 @@ class DefaultExposureDetectionTracker @Inject constructor( } } + private fun Map<String, TrackedExposureDetection>.findUnfinishedOrCreateIdentifier(): String { + val newestUnfinishedDetection = this + .map { it.value } + .filter { it.finishedAt == null } + .maxByOrNull { it.startedAt.millis } + + return if (newestUnfinishedDetection != null) { + Timber.d("findUnfinishedOrCreateIdentifier(): Found unfinished detection, return identifier") + newestUnfinishedDetection.identifier + } else { + Timber.d("findUnfinishedOrCreateIdentifier(): No unfinished detection found, create identifier") + UUID.randomUUID().toString() + } + } + + private fun MutableMap<String, TrackedExposureDetection>.finishDetection(identifier: String, result: Result) { + Timber.i("finishDetection(token=%s, result=%s)", identifier, result) + val existing = this[identifier] + if (existing != null) { + if (existing.result == Result.TIMEOUT) { + Timber.w("Detection is late, already hit timeout, still updating.") + } else if (existing.result != null) { + Timber.e("Duplicate callback. Result is already set for detection!") + } + this[identifier] = existing.copy( + result = result, + finishedAt = timeStamper.nowUTC + ) + } else { + Timber.e( + "Unknown detection finished (token=%s, result=%s)", + identifier, + result + ) + this[identifier] = TrackedExposureDetection( + identifier = identifier, + result = result, + startedAt = timeStamper.nowUTC, + finishedAt = timeStamper.nowUTC + ) + } + } + companion object { private const val TAG = "DefaultExposureDetectionTracker" private const val MAX_ENTRY_SIZE = 5 - private val TIMEOUT_CHECK_INTERVALL = Duration.standardMinutes(3) + private val TIMEOUT_CHECK_INTERVAL = Duration.standardMinutes(3) } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt index 4d9bcf0c6..1f0740acd 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTracker.kt @@ -7,5 +7,5 @@ interface ExposureDetectionTracker { fun trackNewExposureDetection(identifier: String) - fun finishExposureDetection(identifier: String, result: TrackedExposureDetection.Result) + fun finishExposureDetection(identifier: String? = null, result: TrackedExposureDetection.Result) } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiver.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiver.kt index dab0da7f0..d58137f5e 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiver.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/receiver/ExposureStateUpdateReceiver.kt @@ -11,7 +11,6 @@ import com.google.android.gms.nearby.exposurenotification.ExposureNotificationCl import com.google.android.gms.nearby.exposurenotification.ExposureNotificationClient.EXTRA_TOKEN import dagger.android.AndroidInjection import de.rki.coronawarnapp.exception.ExceptionCategory.INTERNAL -import de.rki.coronawarnapp.exception.NoTokenException import de.rki.coronawarnapp.exception.UnknownBroadcastException import de.rki.coronawarnapp.exception.reporting.report import de.rki.coronawarnapp.nearby.ExposureStateUpdateWorker @@ -55,9 +54,10 @@ class ExposureStateUpdateReceiver : BroadcastReceiver() { val async = goAsync() scope.launch(context = dispatcherProvider.Default) { try { + val token = intent.getStringExtra(EXTRA_TOKEN) when (action) { - ACTION_EXPOSURE_STATE_UPDATED -> processStateUpdates(intent) - ACTION_EXPOSURE_NOT_FOUND -> processNotFound(intent) + ACTION_EXPOSURE_STATE_UPDATED -> processStateUpdates(token) + ACTION_EXPOSURE_NOT_FOUND -> processNotFound(token) else -> throw UnknownBroadcastException(action) } } catch (e: Exception) { @@ -69,13 +69,12 @@ class ExposureStateUpdateReceiver : BroadcastReceiver() { } } - private fun processStateUpdates(intent: Intent) { + private fun processStateUpdates(token: String?) { Timber.tag(TAG).i("Processing ACTION_EXPOSURE_STATE_UPDATED") val workManager = WorkManager.getInstance(context) - val token = intent.requireToken() - + // TODO("Remove token from ExposureStateUpdateWorker") val data = Data .Builder() .putString(EXTRA_TOKEN, token) @@ -93,21 +92,15 @@ class ExposureStateUpdateReceiver : BroadcastReceiver() { ) } - private fun processNotFound(intent: Intent) { + private fun processNotFound(token: String?) { Timber.tag(TAG).i("Processing ACTION_EXPOSURE_NOT_FOUND") - val token = intent.requireToken() - exposureDetectionTracker.finishExposureDetection( token, TrackedExposureDetection.Result.NO_MATCHES ) } - private fun Intent.requireToken(): String = getStringExtra(EXTRA_TOKEN).also { - Timber.tag(TAG).v("Extracted token: %s", it) - } ?: throw NoTokenException(IllegalArgumentException("no token was found in the intent")) - companion object { private val TAG: String? = ExposureStateUpdateReceiver::class.simpleName } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/ENFClientTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/ENFClientTest.kt index 1f242c4c2..35a4efbfe 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/ENFClientTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/ENFClientTest.kt @@ -77,21 +77,19 @@ class ENFClientTest : BaseTest() { val configuration = mockk<ExposureConfiguration>() val token = "123" - coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any(), any(), any()) } returns true + coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any()) } returns true runBlocking { - client.provideDiagnosisKeys(keyFiles, configuration, token) shouldBe true + client.provideDiagnosisKeys(keyFiles) shouldBe true } - coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any(), any(), any()) } returns false + coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any()) } returns false runBlocking { - client.provideDiagnosisKeys(keyFiles, configuration, token) shouldBe false + client.provideDiagnosisKeys(keyFiles) shouldBe false } coVerify(exactly = 2) { diagnosisKeyProvider.provideDiagnosisKeys( - keyFiles, - configuration, - token + keyFiles ) } } @@ -103,13 +101,13 @@ class ENFClientTest : BaseTest() { val configuration = mockk<ExposureConfiguration>() val token = "123" - coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any(), any(), any()) } returns true + coEvery { diagnosisKeyProvider.provideDiagnosisKeys(any()) } returns true runBlocking { - client.provideDiagnosisKeys(keyFiles, configuration, token) shouldBe true + client.provideDiagnosisKeys(keyFiles) shouldBe true } coVerify(exactly = 0) { - diagnosisKeyProvider.provideDiagnosisKeys(any(), any(), any()) + diagnosisKeyProvider.provideDiagnosisKeys(any()) } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/windows/ExposureWindowsCalculationTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/windows/ExposureWindowsCalculationTest.kt index 3dab010d2..41c26ebc2 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/windows/ExposureWindowsCalculationTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/windows/ExposureWindowsCalculationTest.kt @@ -297,7 +297,8 @@ class ExposureWindowsCalculationTest : BaseTest() { serverTime = Instant.now(), localOffset = Duration.ZERO, mappedConfig = configData, - isFallback = false + identifier = "soup", + configType = ConfigData.Type.FROM_SERVER ) val attenuationFilters = mutableListOf<RiskCalculationParametersOuterClass.MinutesAtAttenuationFilter>() -- GitLab