diff --git a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragmentViewModel.kt b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragmentViewModel.kt index 711cb5f5bd7adcba37acf7a786f23f1fe9ad42be..3240d59a1f3370e871145e5fc985c51e50a6b518 100644 --- a/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragmentViewModel.kt +++ b/Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/datadonation/ui/DataDonationTestFragmentViewModel.kt @@ -129,8 +129,9 @@ class DataDonationTestFragmentViewModel @AssistedInject constructor( fun collectAnalyticsData() = launch { try { + val configData = appConfigProvider.getAppConfig() val ppaDataAndroid = PpaData.PPADataAndroid.newBuilder() - analytics.collectContributions(ppaDataBuilder = ppaDataAndroid) + analytics.collectContributions(configData, ppaDataAndroid) currentAnalyticsDataInternal.value = ppaDataAndroid.build() } catch (e: Exception) { Timber.e(e, "collectContributions() failed.") @@ -140,8 +141,8 @@ class DataDonationTestFragmentViewModel @AssistedInject constructor( fun submitAnalytics() = launch { infoEvents.postValue("Starting Analytics Submission") - val analyticsConfig = appConfigProvider.getAppConfig().analytics - analytics.submitAnalyticsData(analyticsConfig) + val configData = appConfigProvider.getAppConfig() + analytics.submitAnalyticsData(configData) infoEvents.postValue("Analytics Submission Done") checkLastAnalytics() } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/Analytics.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/Analytics.kt index dab8333ca7fc18e204efa713891a1fad97f1b70f..97949918f02c3590afca4dd76ab5fb8b3ae11f88 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/Analytics.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/Analytics.kt @@ -3,6 +3,7 @@ package de.rki.coronawarnapp.datadonation.analytics import androidx.annotation.VisibleForTesting import de.rki.coronawarnapp.appconfig.AnalyticsConfig import de.rki.coronawarnapp.appconfig.AppConfigProvider +import de.rki.coronawarnapp.appconfig.ConfigData import de.rki.coronawarnapp.bugreporting.reportProblem import de.rki.coronawarnapp.datadonation.analytics.modules.DonorModule import de.rki.coronawarnapp.datadonation.analytics.server.DataDonationAnalyticsServer @@ -72,8 +73,13 @@ class Analytics @Inject constructor( } } - suspend fun collectContributions(ppaDataBuilder: PpaData.PPADataAndroid.Builder): List<DonorModule.Contribution> { - val request: DonorModule.Request = object : DonorModule.Request {} + suspend fun collectContributions( + configData: ConfigData, + ppaDataBuilder: PpaData.PPADataAndroid.Builder + ): List<DonorModule.Contribution> { + val request: DonorModule.Request = object : DonorModule.Request { + override val currentConfig: ConfigData = configData + } val contributions = donorModules.mapNotNull { val moduleName = it::class.simpleName @@ -100,12 +106,12 @@ class Analytics @Inject constructor( } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - suspend fun submitAnalyticsData(analyticsConfig: AnalyticsConfig): Result { + suspend fun submitAnalyticsData(configData: ConfigData): Result { Timber.tag(TAG).d("Starting analytics submission") val ppaDataBuilder = PpaData.PPADataAndroid.newBuilder() - val contributions = collectContributions(ppaDataBuilder = ppaDataBuilder) + val contributions = collectContributions(configData, ppaDataBuilder) val analyticsProto = ppaDataBuilder.build() @@ -113,7 +119,7 @@ class Analytics @Inject constructor( // 6min, if attestation and/or submission takes longer than that, // then we want to give modules still time to cleanup and get into a consistent state. withTimeout(360_000) { - trySubmission(analyticsConfig, analyticsProto) + trySubmission(configData.analytics, analyticsProto) } } catch (e: TimeoutCancellationException) { Timber.tag(TAG).e(e, "trySubmission() timed out after 360s.") @@ -173,9 +179,9 @@ class Analytics @Inject constructor( suspend fun submitIfWanted(): Result = submissionLockoutMutex.withLock { Timber.tag(TAG).d("Checking analytics conditions") - val analyticsConfig: AnalyticsConfig = appConfigProvider.getAppConfig().analytics + val configData: ConfigData = appConfigProvider.getAppConfig() - if (stopDueToNoAnalyticsConfig(analyticsConfig)) { + if (stopDueToNoAnalyticsConfig(configData.analytics)) { Timber.tag(TAG).w("Aborting Analytics submission due to noAnalyticsConfig") return@withLock Result(successful = false) } @@ -185,7 +191,7 @@ class Analytics @Inject constructor( return@withLock Result(successful = false) } - if (stopDueToProbabilityToSubmit(analyticsConfig)) { + if (stopDueToProbabilityToSubmit(configData.analytics)) { Timber.tag(TAG).w("Aborting Analytics submission due to probabilityToSubmit") return@withLock Result(successful = false) } @@ -200,7 +206,7 @@ class Analytics @Inject constructor( return@withLock Result(successful = false) } - return@withLock submitAnalyticsData(analyticsConfig) + return@withLock submitAnalyticsData(configData) } private suspend fun deleteAllData() = submissionLockoutMutex.withLock { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/DonorModule.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/DonorModule.kt index 78964dcdce3c85b84184a92cdb70855f0699f78b..fbf05e4b1a4669d60419a7005f18283defc12b7f 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/DonorModule.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/modules/DonorModule.kt @@ -1,5 +1,6 @@ package de.rki.coronawarnapp.datadonation.analytics.modules +import de.rki.coronawarnapp.appconfig.ConfigData import de.rki.coronawarnapp.server.protocols.internal.ppdd.PpaData interface DonorModule { @@ -11,7 +12,14 @@ interface DonorModule { /** * Data that the modules may need to fullfil the request */ - interface Request + interface Request { + /** + * The config data pulled at the start of the submission attempt. + * Should be used by modules to prevent unnecessary config refreshes, + * and to prevent the config from changing DURING the collection/submission. + */ + val currentConfig: ConfigData + } /** * An object that adds the data to the protobuf container, such that the Analytics class doesn't need to know the diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/AnalyticsTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/AnalyticsTest.kt index 306ff85627dfbb7a37afd3a54ae02c870bd3f07c..8e0a287eea637195612554a584d13361ebb4e88c 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/AnalyticsTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/AnalyticsTest.kt @@ -29,6 +29,7 @@ import io.mockk.impl.annotations.MockK import io.mockk.just import io.mockk.mockk import io.mockk.mockkObject +import io.mockk.slot import io.mockk.spyk import kotlinx.coroutines.delay import kotlinx.coroutines.test.runBlockingTest @@ -117,7 +118,7 @@ class AnalyticsTest : BaseTest() { } coVerify(exactly = 0) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) analytics.stopDueToNoUserConsent() } } @@ -142,7 +143,7 @@ class AnalyticsTest : BaseTest() { } coVerify(exactly = 0) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) analytics.stopDueToProbabilityToSubmit(analyticsConfig) } } @@ -168,7 +169,7 @@ class AnalyticsTest : BaseTest() { } coVerify(exactly = 0) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) analytics.stopDueToLastSubmittedTimestamp() } } @@ -195,7 +196,7 @@ class AnalyticsTest : BaseTest() { } coVerify(exactly = 0) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) analytics.stopDueToTimeSinceOnboarding() } } @@ -223,7 +224,7 @@ class AnalyticsTest : BaseTest() { } coVerify(exactly = 0) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) } } @@ -245,7 +246,8 @@ class AnalyticsTest : BaseTest() { .setAuthentication(PpacAndroid.PPACAndroid.getDefaultInstance()) .build() - coEvery { exposureRiskMetadataDonor.beginDonation(any()) } returns + val donationRequestSlot = slot<DonorModule.Request>() + coEvery { exposureRiskMetadataDonor.beginDonation(capture(donationRequestSlot)) } returns ExposureRiskMetadataDonor.ExposureRiskMetadataContribution( contributionProto = metadata, onContributionFinished = {} @@ -269,8 +271,10 @@ class AnalyticsTest : BaseTest() { } } + donationRequestSlot.captured.currentConfig shouldBe configData + coVerify(exactly = 1) { - analytics.submitAnalyticsData(analyticsConfig) + analytics.submitAnalyticsData(configData) dataDonationAnalyticsServer.uploadAnalyticsData(analyticsRequest) } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/ExposureRiskMetadataDonorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/ExposureRiskMetadataDonorTest.kt index 2ae64c468af8852f5eda27236680901da2e19b61..e035de7690afb38099a650787a634fd2d71d566b 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/ExposureRiskMetadataDonorTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/ExposureRiskMetadataDonorTest.kt @@ -1,6 +1,7 @@ package de.rki.coronawarnapp.datadonation.analytics.modules import com.google.android.gms.nearby.exposurenotification.ExposureWindow +import de.rki.coronawarnapp.appconfig.ConfigData import de.rki.coronawarnapp.datadonation.analytics.modules.exposureriskmetadata.ExposureRiskMetadataDonor import de.rki.coronawarnapp.datadonation.analytics.storage.AnalyticsSettings import de.rki.coronawarnapp.risk.RiskLevelResult @@ -13,6 +14,7 @@ import io.mockk.MockKAnnotations import io.mockk.clearAllMocks import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockk import kotlinx.coroutines.flow.flowOf import org.joda.time.Instant import org.junit.jupiter.api.AfterEach @@ -91,7 +93,11 @@ class ExposureRiskMetadataDonorTest : BaseTest() { val parentBuilder = PpaData.PPADataAndroid.newBuilder() runBlockingTest2 { - val contribution = createInstance().beginDonation(object : DonorModule.Request {}) + val contribution = createInstance().beginDonation( + object : DonorModule.Request { + override val currentConfig: ConfigData = mockk() + } + ) contribution.injectData(parentBuilder) contribution.finishDonation(true) } @@ -137,7 +143,11 @@ class ExposureRiskMetadataDonorTest : BaseTest() { val parentBuilder = PpaData.PPADataAndroid.newBuilder() runBlockingTest2 { - val contribution = createInstance().beginDonation(object : DonorModule.Request {}) + val contribution = createInstance().beginDonation( + object : DonorModule.Request { + override val currentConfig: ConfigData = mockk() + } + ) contribution.injectData(parentBuilder) contribution.finishDonation(true) } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/clientmetadata/ClientMetadataDonorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/clientmetadata/ClientMetadataDonorTest.kt index 3a04905ccfd3d5dc2e72ddae701529c783d7d621..b3b262d2539f79fb470db185316c559f8c44c34a 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/clientmetadata/ClientMetadataDonorTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/datadonation/analytics/modules/clientmetadata/ClientMetadataDonorTest.kt @@ -13,6 +13,7 @@ import io.mockk.clearAllMocks import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockk import io.mockk.mockkObject import kotlinx.coroutines.flow.flowOf import org.junit.jupiter.api.AfterEach @@ -75,7 +76,9 @@ class ClientMetadataDonorTest : BaseTest() { val parentBuilder = PpaData.PPADataAndroid.newBuilder() runBlockingTest2 { - val contribution = createInstance().beginDonation(object : DonorModule.Request {}) + val contribution = createInstance().beginDonation(object : DonorModule.Request { + override val currentConfig: ConfigData = mockk() + }) contribution.injectData(parentBuilder) contribution.finishDonation(true) }