From 737118bad3c6ad6d9b68cc0b3205ee7b024d1470 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Mon, 14 Apr 2025 14:16:50 +0200 Subject: [PATCH 1/3] Add test to check whether pendingDecisions are empty after cancellation --- .../cert4android/UserDecisionRegistryTest.kt | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt b/lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt index 28f1ddf..f799e54 100644 --- a/lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt +++ b/lib/src/androidTest/java/at/bitfire/cert4android/UserDecisionRegistryTest.kt @@ -1,15 +1,22 @@ package at.bitfire.cert4android +import androidx.core.app.NotificationManagerCompat import androidx.test.platform.app.InstrumentationRegistry import io.mockk.every +import io.mockk.just +import io.mockk.mockk import io.mockk.mockkObject +import io.mockk.runs import io.mockk.unmockkAll import io.mockk.verify import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import org.junit.After -import org.junit.Assert.* +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import java.util.Collections @@ -111,6 +118,37 @@ class UserDecisionRegistryTest { verify(exactly = 1) { registry.requestDecision(any(), any(), any()) } } + @Test + fun testCheck_MultipleDecisionsForSameCert_cancel() { + val canSendFeedback = Semaphore(0) + val nm = mockk() + every { nm.cancel(any(), any()) } just runs + every { NotificationUtils.createChannels(any()) } returns nm + every { registry.requestDecision(testCert, any(), any()) } answers { + thread { + canSendFeedback.acquire() + registry.onUserDecision(testCert, false) + } + } + val results = Collections.synchronizedList(mutableListOf()) + runBlocking { + repeat(5) { + val job = launch(Dispatchers.Default) { + results += registry.check(testCert, true) + } + delay(1000) + job.cancel() // Cancel the job + delay(1000) + } + canSendFeedback.release() + } + synchronized(registry.pendingDecisions) { + assertFalse(registry.pendingDecisions.containsKey(testCert)) + } + assertEquals(0, results.size) + verify(exactly = 5) { registry.requestDecision(any(), any(), any()) } + } + @Test fun testCheck_UserDecisionImpossible() { every { NotificationUtils.notificationsPermitted(any()) } returns false From c8dffd9d27a09457467a7cb80037847fbde0424a Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Mon, 14 Apr 2025 14:17:41 +0200 Subject: [PATCH 2/3] Remove last continuation from pendingDecisions when cancelled --- .../java/at/bitfire/cert4android/UserDecisionRegistry.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt b/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt index 4028db9..6c909c8 100644 --- a/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt +++ b/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt @@ -53,11 +53,17 @@ class UserDecisionRegistry private constructor( // User decision possible → remember request in pendingDecisions so that a later decision will be applied to this request cont.invokeOnCancellation { + val decisionsList = pendingDecisions[cert] + // remove from pending decisions on cancellation synchronized(pendingDecisions) { - pendingDecisions[cert]?.remove(cont) + decisionsList?.remove(cont) } + // Remove decisions list if empty + if (decisionsList?.isEmpty() == true) + pendingDecisions.remove(cert) + val nm = NotificationUtils.createChannels(context) nm.cancel(CertUtils.getTag(cert), NotificationUtils.ID_CERT_DECISION) } From 8d67f802985cb470e6fb6ec27d1d6ffa4507f372 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Tue, 15 Apr 2025 09:50:04 +0200 Subject: [PATCH 3/3] Synchronize both check and removal of empty decisions list --- .../bitfire/cert4android/UserDecisionRegistry.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt b/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt index 6c909c8..d43a578 100644 --- a/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt +++ b/lib/src/main/java/at/bitfire/cert4android/UserDecisionRegistry.kt @@ -53,16 +53,16 @@ class UserDecisionRegistry private constructor( // User decision possible → remember request in pendingDecisions so that a later decision will be applied to this request cont.invokeOnCancellation { - val decisionsList = pendingDecisions[cert] - - // remove from pending decisions on cancellation synchronized(pendingDecisions) { + val decisionsList = pendingDecisions[cert] + + // remove from pending decisions on cancellation decisionsList?.remove(cont) - } - // Remove decisions list if empty - if (decisionsList?.isEmpty() == true) - pendingDecisions.remove(cert) + // Remove decisions list if empty + if (decisionsList?.isEmpty() == true) + pendingDecisions -= cert + } val nm = NotificationUtils.createChannels(context) nm.cancel(CertUtils.getTag(cert), NotificationUtils.ID_CERT_DECISION)