From 85ca089fa12581ace98621ed04a9e264ca32cf49 Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Wed, 21 Apr 2021 18:52:46 +0200 Subject: [PATCH] Improve CoronaTestStorage, only save non-generated value (EXPOSUREAPP-6007) (#2899) * Don't store properties that are generated out of other properties. * Complete unit tests. * Fix merge regression. Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../coronatest/type/pcr/PCRCoronaTest.kt | 6 + .../type/rapidantigen/RACoronaTest.kt | 5 + .../storage/CoronaTestStorageTest.kt | 193 ++++++++++++++++++ .../testhelpers/extensions/JsonExtensions.kt | 11 +- 4 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt index 710f2aa34..0f260941b 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt @@ -42,13 +42,19 @@ data class PCRCoronaTest( @Transient override val lastError: Throwable? = null, ) : CoronaTest { + @Transient override val type: CoronaTest.Type = CoronaTest.Type.PCR + @Transient override val isPositive: Boolean = testResult == CoronaTestResult.PCR_POSITIVE + + @Transient override val isPending: Boolean = testResult == CoronaTestResult.PCR_OR_RAT_PENDING + @Transient override val isSubmissionAllowed: Boolean = isPositive && !isSubmitted + @Transient val state: State = when (testResult) { CoronaTestResult.PCR_OR_RAT_PENDING -> State.PENDING CoronaTestResult.PCR_NEGATIVE -> State.NEGATIVE diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt index 77fdd42ba..ca4c97568 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt @@ -55,6 +55,7 @@ data class RACoronaTest( @Transient override val lastError: Throwable? = null, ) : CoronaTest { + @Transient override val type: CoronaTest.Type = CoronaTest.Type.RAPID_ANTIGEN fun getState(nowUTC: Instant) = when (testResult) { @@ -66,10 +67,14 @@ data class RACoronaTest( else -> throw IllegalArgumentException("Invalid RAT test state $testResult") } + @Transient override val isPositive: Boolean = testResult == CoronaTestResult.RAT_POSITIVE + + @Transient override val isPending: Boolean = testResult == CoronaTestResult.PCR_OR_RAT_PENDING || testResult == CoronaTestResult.RAT_PENDING + @Transient override val isSubmissionAllowed: Boolean = isPositive && !isSubmitted enum class State { diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt new file mode 100644 index 000000000..fd88abd78 --- /dev/null +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt @@ -0,0 +1,193 @@ +package de.rki.coronawarnapp.coronatest.storage + +import android.content.Context +import androidx.core.content.edit +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult +import de.rki.coronawarnapp.coronatest.type.pcr.PCRCoronaTest +import de.rki.coronawarnapp.coronatest.type.rapidantigen.RACoronaTest +import de.rki.coronawarnapp.util.serialization.SerializationModule +import io.kotest.matchers.shouldBe +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import org.joda.time.Instant +import org.joda.time.LocalDate +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import testhelpers.BaseTest +import testhelpers.extensions.toComparableJsonPretty +import testhelpers.preferences.MockSharedPreferences +import java.io.IOException + +class CoronaTestStorageTest : BaseTest() { + @MockK lateinit var context: Context + private lateinit var mockPreferences: MockSharedPreferences + + @BeforeEach + fun setup() { + MockKAnnotations.init(this) + + mockPreferences = MockSharedPreferences() + + every { + context.getSharedPreferences("coronatest_localdata", Context.MODE_PRIVATE) + } returns mockPreferences + } + + private fun createInstance() = CoronaTestStorage( + context = context, + baseGson = SerializationModule().baseGson() + ) + + private val pcrTest = PCRCoronaTest( + identifier = "identifier-pcr", + registeredAt = Instant.ofEpochMilli(1000), + registrationToken = "regtoken-pcr", + isSubmitted = true, + isViewed = true, + isAdvancedConsentGiven = true, + isJournalEntryCreated = false, + isResultAvailableNotificationSent = false, + testResult = CoronaTestResult.PCR_POSITIVE, + testResultReceivedAt = Instant.ofEpochMilli(2000) + ) + private val raTest = RACoronaTest( + identifier = "identifier-ra", + registeredAt = Instant.ofEpochMilli(1000), + registrationToken = "regtoken-ra", + isSubmitted = true, + isViewed = true, + isAdvancedConsentGiven = true, + isJournalEntryCreated = false, + isResultAvailableNotificationSent = false, + testResult = CoronaTestResult.RAT_POSITIVE, + testResultReceivedAt = Instant.ofEpochMilli(2000), + firstName = "firstname", + lastName = "lastname", + dateOfBirth = LocalDate.parse("2021-12-24"), + testedAt = Instant.ofEpochMilli(3000) + ) + + @Test + fun `init is sideeffect free`() { + createInstance() + } + + @Test + fun `storing empty set deletes data`() { + mockPreferences.edit { + putString("dontdeleteme", "test") + putString("coronatest.data.ra", "test") + putString("coronatest.data.pcr", "test") + } + createInstance().coronaTests = emptySet() + + mockPreferences.dataMapPeek.keys.single() shouldBe "dontdeleteme" + } + + @Test + fun `store only PCRT`() { + val instance = createInstance() + instance.coronaTests = setOf( + pcrTest.copy( + isProcessing = true, + lastError = IOException() + ) + ) + + val json = (mockPreferences.dataMapPeek["coronatest.data.pcr"] as String) + + json.toComparableJsonPretty() shouldBe """ + [ + { + "identifier": "identifier-pcr", + "registeredAt": 1000, + "registrationToken": "regtoken-pcr", + "isSubmitted": true, + "isViewed": true, + "isAdvancedConsentGiven": true, + "isJournalEntryCreated": false, + "isResultAvailableNotificationSent": false, + "testResultReceivedAt": 2000, + "testResult": 2 + } + ] + """.toComparableJsonPretty() + + instance.coronaTests.single() shouldBe pcrTest.copy( + lastError = null, + isProcessing = false + ) + } + + @Test + fun `store only RAT`() { + val instance = createInstance() + instance.coronaTests = setOf( + raTest.copy( + isProcessing = true, + lastError = IOException() + ) + ) + + val json = (mockPreferences.dataMapPeek["coronatest.data.ra"] as String) + + json.toComparableJsonPretty() shouldBe """ + [ + { + "identifier": "identifier-ra", + "registeredAt": 1000, + "registrationToken": "regtoken-ra", + "isSubmitted": true, + "isViewed": true, + "isAdvancedConsentGiven": true, + "isJournalEntryCreated": false, + "isResultAvailableNotificationSent": false, + "testResultReceivedAt": 2000, + "testResult": 7, + "testedAt": 3000, + "firstName": "firstname", + "lastName": "lastname", + "dateOfBirth": "2021-12-24" + } + ] + """.toComparableJsonPretty() + + instance.coronaTests.single() shouldBe raTest.copy( + lastError = null, + isProcessing = false + ) + } + + @Test + fun `store one of each`() { + val instance = createInstance() + instance.coronaTests = setOf(raTest, pcrTest) + + mockPreferences.contains("coronatest.data.ra") shouldBe true + mockPreferences.contains("coronatest.data.pcr") shouldBe true + + instance.coronaTests shouldBe setOf(raTest, pcrTest) + } + + @Test + fun `storing one and deleting the other`() { + val instance = createInstance() + + instance.coronaTests = setOf(raTest) + mockPreferences.contains("coronatest.data.ra") shouldBe true + mockPreferences.contains("coronatest.data.pcr") shouldBe false + + instance.coronaTests = setOf(pcrTest) + + mockPreferences.contains("coronatest.data.ra") shouldBe false + mockPreferences.contains("coronatest.data.pcr") shouldBe true + instance.coronaTests shouldBe setOf(pcrTest) + + instance.coronaTests = setOf(raTest) + + mockPreferences.contains("coronatest.data.ra") shouldBe true + mockPreferences.contains("coronatest.data.pcr") shouldBe false + instance.coronaTests shouldBe setOf(raTest) + } +} diff --git a/Corona-Warn-App/src/test/java/testhelpers/extensions/JsonExtensions.kt b/Corona-Warn-App/src/test/java/testhelpers/extensions/JsonExtensions.kt index f2cd8033a..679b6650e 100644 --- a/Corona-Warn-App/src/test/java/testhelpers/extensions/JsonExtensions.kt +++ b/Corona-Warn-App/src/test/java/testhelpers/extensions/JsonExtensions.kt @@ -2,6 +2,8 @@ package testhelpers.extensions import com.google.gson.Gson import com.google.gson.GsonBuilder +import com.google.gson.JsonArray +import com.google.gson.JsonElement import com.google.gson.JsonObject import io.kotest.assertions.assertionCounter import io.kotest.assertions.collectOrThrow @@ -9,15 +11,20 @@ import io.kotest.assertions.eq.eq import io.kotest.assertions.errorCollector import okhttp3.mockwebserver.MockResponse +private fun String.determineJsonType(): Class<out JsonElement> = when { + this.trimStart().startsWith("[") -> JsonArray::class.java + else -> JsonObject::class.java +} + fun String.toComparableJson() = try { - Gson().fromJson(this, JsonObject::class.java).toString() + Gson().fromJson(this, this.determineJsonType()).toString() } catch (e: Exception) { throw IllegalArgumentException("'$this' wasn't valid JSON") } fun String.toComparableJsonPretty(): String = try { val gson = GsonBuilder().setPrettyPrinting().create() - val obj = gson.fromJson(this, JsonObject::class.java) + val obj = gson.fromJson(this, this.determineJsonType()) gson.toJson(obj) } catch (e: Exception) { throw IllegalArgumentException("'$this' wasn't valid JSON") -- GitLab