Skip to content

Commit 286a3fc

Browse files
authored
PIR Android: fix confirmation/maintenance scan for requested/removed opt outs (#6960)
Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1211207086563708?focus=true ### Description See attached task description ### Steps to test this PR https://app.asana.com/1/137249556945/task/1211672394316847?focus=true
1 parent 5ef80fb commit 286a3fc

File tree

2 files changed

+103
-11
lines changed

2 files changed

+103
-11
lines changed

pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/scheduling/EligibleScanJobProvider.kt

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,38 @@ class RealEligibleScanJobProvider @Inject constructor(
7171
}.sortedBy {
7272
it.attemptCount
7373
}.mapNotNull {
74-
pirSchedulingRepository.getValidScanJobRecord(it.brokerName, it.userProfileId)
74+
val schedulingConfig =
75+
schedulingConfigs.find { config -> config.brokerName == it.brokerName }!!
76+
77+
val expectedScanDate = when (it.status) {
78+
OptOutJobStatus.REQUESTED -> it.getRequestConfirmationScanDate(schedulingConfig)
79+
OptOutJobStatus.REMOVED -> it.getRemovedMaintenanceScanDate(schedulingConfig)
80+
else -> return@mapNotNull null
81+
}
82+
val scanJobRecord = pirSchedulingRepository.getValidScanJobRecord(it.brokerName, it.userProfileId)
83+
84+
// If the last scan happened more recently that the expected scan date from opt outs, it means we have already performed the
85+
// maintenance scan / confirmation scan needed for this opt-out.
86+
// More info on https://app.asana.com/1/137249556945/project/488551667048375/task/1211207086563708?focus=true
87+
return@mapNotNull if (scanJobRecord != null && scanJobRecord.lastScanDateInMillis <= expectedScanDate) {
88+
scanJobRecord
89+
} else {
90+
null
91+
}
7592
}
7693
}
7794

7895
private fun OptOutJobRecord.isRequestedAndShouldBeConfirmedNow(
7996
schedulingConfig: BrokerSchedulingConfig,
8097
timeInMillis: Long,
8198
): Boolean {
82-
return this.status == OptOutJobStatus.REQUESTED &&
83-
(this.optOutRequestedDateInMillis + schedulingConfig.confirmOptOutScanInMillis) <= timeInMillis
99+
return this.status == OptOutJobStatus.REQUESTED && this.getRequestConfirmationScanDate(schedulingConfig) <= timeInMillis
100+
}
101+
102+
private fun OptOutJobRecord.getRequestConfirmationScanDate(
103+
schedulingConfig: BrokerSchedulingConfig,
104+
): Long {
105+
return this.optOutRequestedDateInMillis + schedulingConfig.confirmOptOutScanInMillis
84106
}
85107

86108
private fun OptOutJobRecord.isRemovedAndShouldBeMaintainedNow(
@@ -90,7 +112,13 @@ class RealEligibleScanJobProvider @Inject constructor(
90112
return this.status == OptOutJobStatus.REMOVED &&
91113
// do not pick-up deprecated opt-out jobs for maintenance scans as they belong to invalid/removed profiles
92114
!this.deprecated &&
93-
(this.optOutRemovedDateInMillis + schedulingConfig.maintenanceScanInMillis) <= timeInMillis
115+
this.getRemovedMaintenanceScanDate(schedulingConfig) <= timeInMillis
116+
}
117+
118+
private fun OptOutJobRecord.getRemovedMaintenanceScanDate(
119+
schedulingConfig: BrokerSchedulingConfig,
120+
): Long {
121+
return this.optOutRemovedDateInMillis + schedulingConfig.maintenanceScanInMillis
94122
}
95123

96124
private suspend fun getValidScanJobsFromScanJobRecords(

pir/pir-impl/src/test/kotlin/com/duckduckgo/pir/impl/scheduling/RealEligibleScanJobProviderTest.kt

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import org.junit.Assert.assertTrue
3030
import org.junit.Before
3131
import org.junit.Rule
3232
import org.junit.Test
33+
import org.mockito.kotlin.any
3334
import org.mockito.kotlin.mock
3435
import org.mockito.kotlin.whenever
3536
import java.util.concurrent.TimeUnit
@@ -208,7 +209,7 @@ class RealEligibleScanJobProviderTest {
208209

209210
@Test
210211
fun whenGetAllEligibleScanJobsWithOptOutRequestedAndDueForConfirmThenReturnScanRecord() = runTest {
211-
val expectedScanRecord = scanJobRecordNotExecuted
212+
val expectedScanRecord = scanJobRecordMatchFound
212213
whenever(mockPirRepository.getAllBrokerSchedulingConfigs()).thenReturn(listOf(brokerSchedulingConfig))
213214
whenever(mockPirSchedulingRepository.getAllValidOptOutJobRecords()).thenReturn(listOf(optOutJobRecordRequested))
214215
whenever(mockPirSchedulingRepository.getAllValidScanJobRecords()).thenReturn(emptyList())
@@ -404,9 +405,19 @@ class RealEligibleScanJobProviderTest {
404405
)
405406

406407
// Create corresponding scan records for opt-out records
407-
val scanFromOptOut1 = scanJobRecordNotExecuted.copy(userProfileId = 1001L)
408-
val scanFromOptOut2 = scanJobRecordNotExecuted.copy(userProfileId = 1002L)
409-
val scanFromOptOut3 = scanJobRecordNotExecuted.copy(userProfileId = 1003L)
408+
val scanFromOptOut1 = scanJobRecordNotExecuted.copy(
409+
userProfileId = 1001L,
410+
lastScanDateInMillis = currentTimeMillis - TimeUnit.HOURS.toMillis(25),
411+
)
412+
val scanFromOptOut2 = scanJobRecordNotExecuted.copy(
413+
userProfileId = 1002L,
414+
lastScanDateInMillis = currentTimeMillis - TimeUnit.HOURS.toMillis(25),
415+
)
416+
val scanFromOptOut3 = scanJobRecordNotExecuted.copy(
417+
userProfileId = 1003L,
418+
419+
lastScanDateInMillis = currentTimeMillis - TimeUnit.HOURS.toMillis(25),
420+
)
410421

411422
// Add a duplicate scan record that should be deduplicated
412423
val duplicateScanRecord = scanFromOptOut3.copy() // Same as scanRecord1
@@ -542,7 +553,7 @@ class RealEligibleScanJobProviderTest {
542553
// Deprecated flag only affects REMOVED opt-out records for maintenance scans,
543554
// REQUESTED opt-out records should still trigger confirmation scans
544555
val deprecatedOptOutRequested = optOutJobRecordRequested.copy(deprecated = true)
545-
val expectedScanRecord = scanJobRecordNotExecuted
556+
val expectedScanRecord = scanJobRecordMatchFound
546557

547558
whenever(mockPirRepository.getAllBrokerSchedulingConfigs()).thenReturn(listOf(brokerSchedulingConfig))
548559
whenever(mockPirSchedulingRepository.getAllValidOptOutJobRecords()).thenReturn(listOf(deprecatedOptOutRequested))
@@ -602,8 +613,11 @@ class RealEligibleScanJobProviderTest {
602613
)
603614

604615
val scanForDeprecatedRemoved = scanJobRecordNotExecuted.copy(userProfileId = 2001L)
605-
val scanForNonDeprecatedRemoved = scanJobRecordNotExecuted.copy(userProfileId = 2002L)
606-
val scanForRequested = scanJobRecordNotExecuted.copy(userProfileId = 3001L)
616+
val scanForNonDeprecatedRemoved = scanJobRecordNotExecuted.copy(
617+
userProfileId = 2002L,
618+
lastScanDateInMillis = currentTimeMillis - DAYS.toMillis(1),
619+
)
620+
val scanForRequested = scanJobRecordNotExecuted.copy(userProfileId = 3001L, lastScanDateInMillis = currentTimeMillis - DAYS.toMillis(1))
607621

608622
whenever(mockPirRepository.getAllBrokerSchedulingConfigs()).thenReturn(listOf(brokerSchedulingConfig))
609623
whenever(mockPirSchedulingRepository.getAllValidOptOutJobRecords()).thenReturn(
@@ -632,4 +646,54 @@ class RealEligibleScanJobProviderTest {
632646
assertTrue(result.none { it.userProfileId == 1001L }) // deprecated scan record filtered
633647
assertTrue(result.none { it.userProfileId == 2001L }) // deprecated removed opt-out filtered
634648
}
649+
650+
@Test
651+
fun whenOptOutJobRecordIsRequestedButAlreadyScannedThenRecordIsNotEligible() = runTest {
652+
// Confirmation scan should have happened 6 hours ago confirmation scan after 24 hours)
653+
val requestedJobRecord = optOutJobRecordRequested.copy(
654+
userProfileId = 125L,
655+
optOutRequestedDateInMillis = currentTimeMillis - HOURS.toMillis(30),
656+
)
657+
658+
// Scan just happened an hour ago
659+
val scanJobRecord = scanJobRecordMatchFound.copy(
660+
lastScanDateInMillis = currentTimeMillis - HOURS.toMillis(1),
661+
)
662+
whenever(mockPirRepository.getAllBrokerSchedulingConfigs()).thenReturn(listOf(brokerSchedulingConfig))
663+
whenever(mockPirSchedulingRepository.getAllValidOptOutJobRecords()).thenReturn(listOf(requestedJobRecord))
664+
whenever(mockPirSchedulingRepository.getValidScanJobRecord("test-broker", 2001L)).thenReturn(scanJobRecord)
665+
whenever(mockPirSchedulingRepository.getAllValidScanJobRecords()).thenReturn(
666+
listOf(scanJobRecord),
667+
)
668+
669+
val result = testee.getAllEligibleScanJobs(currentTimeMillis)
670+
671+
// Opt-out not eligible as a scan already happened for the confirmation
672+
assertTrue(result.isEmpty())
673+
}
674+
675+
@Test
676+
fun whenOptOutJobRecordIsRemovedButAlreadyScannedThenRecordIsNotEligible() = runTest {
677+
// Maintenance scan should have happened 3 days ago. Maintenance scan after 7 days of removal)
678+
val requestedJobRecord = optOutJobRecordRemoved.copy(
679+
userProfileId = 125L,
680+
optOutRemovedDateInMillis = currentTimeMillis - DAYS.toMillis(10),
681+
)
682+
683+
// Scan just happened an hour ago
684+
val scanJobRecord = scanJobRecordMatchFound.copy(
685+
lastScanDateInMillis = currentTimeMillis - HOURS.toMillis(1),
686+
)
687+
whenever(mockPirRepository.getAllBrokerSchedulingConfigs()).thenReturn(listOf(brokerSchedulingConfig))
688+
whenever(mockPirSchedulingRepository.getAllValidOptOutJobRecords()).thenReturn(listOf(requestedJobRecord))
689+
whenever(mockPirSchedulingRepository.getValidScanJobRecord("test-broker", 2001L)).thenReturn(scanJobRecord)
690+
whenever(mockPirSchedulingRepository.getAllValidScanJobRecords()).thenReturn(
691+
listOf(scanJobRecord),
692+
)
693+
694+
val result = testee.getAllEligibleScanJobs(currentTimeMillis)
695+
696+
// Opt-out not eligible as a scan already happened for the confirmation
697+
assertTrue(result.isEmpty())
698+
}
635699
}

0 commit comments

Comments
 (0)