diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt index 6ff66aa5c8ca..796d98949ad3 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/BrokerStepsParser.kt @@ -40,12 +40,14 @@ interface BrokerStepsParser { * * @param brokerName - name of the broker to which these steps belong to * @param stepsJson - string in JSONObject format obtained from the broker's json representing a step (scan / opt-out). + * @param profileQueryId - profile query id associated with the step (used for the opt-out step) * @return list of broker steps resulting from the passed params. If the step is of type OptOut, it will return a list of * OptOutSteps where an OptOut step is mapped to each of the profile for the broker. */ suspend fun parseStep( brokerName: String, stepsJson: String, + profileQueryId: Long? = null, ): List sealed class BrokerStep( @@ -101,16 +103,22 @@ class RealBrokerStepsParser @Inject constructor( override suspend fun parseStep( brokerName: String, stepsJson: String, + profileQueryId: Long?, ): List = withContext(dispatcherProvider.io()) { return@withContext runCatching { adapter.fromJson(stepsJson)?.run { if (this is OptOutStep) { - repository.getExtractProfileResultForBroker(brokerName)?.extractResults?.map { - this.copy( - brokerName = brokerName, - profileToOptOut = it, - ) - } + repository.getExtractProfileResultsForBroker(brokerName) + .filter { it.profileQuery != null && it.profileQuery.id == profileQueryId } + .map { + it.extractResults.map { extractedProfile -> + this.copy( + brokerName = brokerName, + profileToOptOut = extractedProfile, + ) + } + } + .flatten() } else { listOf((this as ScanStep).copy(brokerName = brokerName)) } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt index 1aaa6e883fd2..79bea92ba8a4 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirActionsRunner.kt @@ -145,8 +145,12 @@ class RealPirActionsRunner @AssistedInject constructor( } withContext(dispatcherProvider.main()) { - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} Brokers to execute $brokerSteps" } - logcat { "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} Brokers size: ${brokerSteps.size}" } + logcat { + "PIR-RUNNER (${this@RealPirActionsRunner}): ${Thread.currentThread().name} " + + "Brokers size: ${brokerSteps.size} " + + "profile=$profileQuery " + + "Brokers to execute $brokerSteps" + } detachedWebView = pirDetachedWebViewProvider.createInstance( context, pirScriptToLoad, diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt index 40f297daee3d..ba341c59ee07 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/common/PirUtils.kt @@ -17,10 +17,23 @@ package com.duckduckgo.pir.internal.common internal fun List.splitIntoParts(parts: Int): List> { - return if (this.isEmpty()) { - emptyList() - } else { - val chunkSize = (this.size + parts - 1) / parts // Ensure rounding up - this.chunked(chunkSize) + if (this.isEmpty()) { + return emptyList() } + + val partSize = this.size / parts + val remainder = this.size % parts + + val result = mutableListOf>() + var startIndex = 0 + + for (i in 0 until parts) { + val currentPartSize = partSize + if (i < remainder) 1 else 0 + val endIndex = startIndex + currentPartSize + + result.add(this.subList(startIndex, endIndex)) + startIndex = endIndex + } + + return result } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt index ba5956d4994d..db619ca0aded 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/optout/PirOptOut.kt @@ -101,21 +101,58 @@ class RealPirOptOut @Inject constructor( private val dispatcherProvider: DispatcherProvider, callbacks: PluginPoint, ) : PirOptOut, PirJob(callbacks) { - private var profileQuery: ProfileQuery = ProfileQuery( - firstName = "William", - lastName = "Smith", - city = "Chicago", - state = "IL", - addresses = listOf( - Address( - city = "Chicago", - state = "IL", + private var profileQueries: List = listOf( + ProfileQuery( + id = -1, + firstName = "William", + lastName = "Smith", + city = "Chicago", + state = "IL", + addresses = listOf( + Address( + city = "Chicago", + state = "IL", + ), + ), + birthYear = 1993, + fullName = "William Smith", + age = 32, + deprecated = false, + ), + ProfileQuery( + id = -2, + firstName = "Jane", + lastName = "Doe", + city = "New York", + state = "NY", + addresses = listOf( + Address( + city = "New York", + state = "NY", + ), + ), + birthYear = 1990, + fullName = "Jane Doe", + age = 35, + deprecated = false, + ), + ProfileQuery( + id = -3, + firstName = "Alicia", + lastName = "West", + city = "Los Angeles", + state = "CA", + addresses = listOf( + Address( + city = "Los Angeles", + state = "CA", + ), ), + birthYear = 1985, + fullName = "Alicia West", + age = 40, + deprecated = false, ), - birthYear = 1993, - fullName = "William Smith", - age = 32, - deprecated = false, ) private val runners: MutableList = mutableListOf() @@ -131,9 +168,9 @@ class RealPirOptOut @Inject constructor( cleanRunners() runners.clear() } - obtainProfile() + obtainProfiles() - logcat { "PIR-OPT-OUT: Running opt-out on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-OPT-OUT: Running debug opt-out for $brokers on profiles: $profileQueries on ${Thread.currentThread().name}" } runners.add( pirActionsRunnerFactory.create( @@ -143,21 +180,28 @@ class RealPirOptOut @Inject constructor( ), ) - // Start each runner on a subset of the broker steps + // Load opt-out steps jsons for each broker + val brokerOptOutStepsJsons = brokers.mapNotNull { broker -> + repository.getBrokerOptOutSteps(broker)?.let { broker to it } + } - brokers.mapNotNull { broker -> - repository.getBrokerOptOutSteps(broker)?.run { - brokerStepsParser.parseStep(broker, this) - } - }.filter { - it.isNotEmpty() + // Map broker steps with their associated profile queries + val allSteps = profileQueries.map { profileQuery -> + brokerOptOutStepsJsons.map { (broker, stepsJson) -> + brokerStepsParser.parseStep(broker, stepsJson, profileQuery.id) + }.flatten().map { step -> profileQuery to step } }.flatten() - .also { list -> - runners[0].startOn(webView, profileQuery, list) - runners[0].stop() - } - logcat { "PIR-OPT-OUT: Optout completed for all runners" } + // Execute each steps sequentially on the single runner + allSteps.forEach { (profileQuery, step) -> + logcat { "PIR-OPT-OUT: Start thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + runners[0].startOn(webView, profileQuery, listOf(step)) + runners[0].stop() + logcat { "PIR-OPT-OUT: Finish thread=${Thread.currentThread().name}, profile=$profileQuery and step=$step" } + } + + logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } + emitCompletedPixel() onJobCompleted() return@withContext Result.success(Unit) @@ -173,25 +217,28 @@ class RealPirOptOut @Inject constructor( cleanRunners() runners.clear() } - obtainProfile() + obtainProfiles() - logcat { "PIR-OPT-OUT: Running opt-out on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-OPT-OUT: Running opt-out on profiles: $profileQueries on ${Thread.currentThread().name}" } val script = pirCssScriptLoader.getScript() - val brokerSteps = brokers.mapNotNull { broker -> - repository.getBrokerOptOutSteps(broker)?.run { - brokerStepsParser.parseStep(broker, this) - } - }.filter { - it.isNotEmpty() + // Load opt-out steps jsons for each broker + val brokerOptOutStepsJsons = brokers.mapNotNull { broker -> + repository.getBrokerOptOutSteps(broker)?.let { broker to it } + } + + // Map broker steps with their associated profile queries + val allSteps = profileQueries.map { profileQuery -> + brokerOptOutStepsJsons.map { (broker, stepsJson) -> + brokerStepsParser.parseStep(broker, stepsJson, profileQuery.id) + }.flatten().map { step -> profileQuery to step } }.flatten() - maxWebViewCount = if (brokerSteps.size <= MAX_DETACHED_WEBVIEW_COUNT) { - brokerSteps.size - } else { - MAX_DETACHED_WEBVIEW_COUNT - } + maxWebViewCount = minOf(allSteps.size, MAX_DETACHED_WEBVIEW_COUNT) + + // Assign steps to runners based on the maximum number of WebViews we can use + val stepsPerRunner = allSteps.splitIntoParts(maxWebViewCount) logcat { "PIR-OPT-OUT: Attempting to create $maxWebViewCount parallel runners on ${Thread.currentThread().name}" } @@ -208,45 +255,49 @@ class RealPirOptOut @Inject constructor( createCount++ } - // Start each runner on a subset of the broker steps - brokerSteps.splitIntoParts(maxWebViewCount) - .mapIndexed { index, part -> - async { - runners[index].start(profileQuery, part) + // Execute the steps on all runners in parallel + stepsPerRunner.mapIndexed { index, partSteps -> + async { + partSteps.map { (profileQuery, step) -> + logcat { "PIR-OPT-OUT: Start opt-out on runner=$index, profile=$profileQuery and step=$step" } + runners[index].start(profileQuery, listOf(step)) runners[index].stop() + logcat { "PIR-OPT-OUT: Finish opt-out on runner=$index, profile=$profileQuery and step=$step" } } - }.awaitAll() + } + }.awaitAll() - logcat { "PIR-OPT-OUT: Optout completed for all runners" } + logcat { "PIR-OPT-OUT: Opt-out completed for all runners and profiles" } emitCompletedPixel() onJobCompleted() return@withContext Result.success(Unit) } - private suspend fun obtainProfile() { - repository.getUserProfiles().also { - if (it.isNotEmpty()) { - // Temporarily taking the first profile only for the PoC. In the reality, more than 1 should be allowed. - val storedProfile = it[0] - profileQuery = ProfileQuery( - firstName = storedProfile.userName.firstName, - lastName = storedProfile.userName.lastName, - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, - addresses = listOf( - Address( - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, + private suspend fun obtainProfiles() { + repository.getUserProfiles().also { profiles -> + if (profiles.isNotEmpty()) { + profileQueries = profiles.map { storedProfile -> + ProfileQuery( + id = storedProfile.id, + firstName = storedProfile.userName.firstName, + lastName = storedProfile.userName.lastName, + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + addresses = listOf( + Address( + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + ), ), - ), - birthYear = storedProfile.birthYear, - fullName = storedProfile.userName.middleName?.run { - "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" - } - ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", - age = LocalDate.now().year - storedProfile.birthYear, - deprecated = false, - ) + birthYear = storedProfile.birthYear, + fullName = storedProfile.userName.middleName?.run { + "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" + } + ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", + age = LocalDate.now().year - storedProfile.birthYear, + deprecated = false, + ) + } } } } diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt index e4da5464abd1..f850c8a02958 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scan/PirScan.kt @@ -96,21 +96,58 @@ class RealPirScan @Inject constructor( callbacks: PluginPoint, ) : PirScan, PirJob(callbacks) { - private var profileQuery: ProfileQuery = ProfileQuery( - firstName = "William", - lastName = "Smith", - city = "Chicago", - state = "IL", - addresses = listOf( - Address( - city = "Chicago", - state = "IL", + private var profileQueries: List = listOf( + ProfileQuery( + id = -1, + firstName = "William", + lastName = "Smith", + city = "Chicago", + state = "IL", + addresses = listOf( + Address( + city = "Chicago", + state = "IL", + ), + ), + birthYear = 1993, + fullName = "William Smith", + age = 32, + deprecated = false, + ), + ProfileQuery( + id = -2, + firstName = "Jane", + lastName = "Doe", + city = "New York", + state = "NY", + addresses = listOf( + Address( + city = "New York", + state = "NY", + ), ), + birthYear = 1990, + fullName = "Jane Doe", + age = 35, + deprecated = false, + ), + ProfileQuery( + id = -3, + firstName = "Alicia", + lastName = "West", + city = "Los Angeles", + state = "CA", + addresses = listOf( + Address( + city = "Los Angeles", + state = "CA", + ), + ), + birthYear = 1985, + fullName = "Alicia West", + age = 40, + deprecated = false, ), - birthYear = 1993, - fullName = "William Smith", - age = 32, - deprecated = false, ) private val runners: MutableList = mutableListOf() @@ -129,11 +166,8 @@ class RealPirScan @Inject constructor( cleanPreviousRun() val script = pirCssScriptLoader.getScript() - maxWebViewCount = if (brokers.size <= MAX_DETACHED_WEBVIEW_COUNT) { - brokers.size - } else { - MAX_DETACHED_WEBVIEW_COUNT - } + maxWebViewCount = minOf(brokers.size * profileQueries.size, MAX_DETACHED_WEBVIEW_COUNT) + logcat { "PIR-SCAN: Attempting to create $maxWebViewCount parallel runners on ${Thread.currentThread().name}" } // Initiate runners @@ -149,23 +183,37 @@ class RealPirScan @Inject constructor( createCount++ } - // Start each runner on a subset of the broker steps. - brokers.mapNotNull { broker -> + // Prepare a list of all broker steps that need to be run + val brokerScanSteps = brokers.mapNotNull { broker -> repository.getBrokerScanSteps(broker)?.run { brokerStepsParser.parseStep(broker, this) } }.filter { it.isNotEmpty() - }.flatten().splitIntoParts(maxWebViewCount) - .mapIndexed { index, part -> - // We want to run the runners in parallel but wait for everything complete before we proceed - async { - runners[index].start(profileQuery, part) + }.flatten() + + // Combine the broker steps with each profile and split into equal parts + val stepsPerRunner = profileQueries.map { profileQuery -> + brokerScanSteps.map { scanStep -> + profileQuery to scanStep + } + }.flatten() + .splitIntoParts(maxWebViewCount) + + // Execute the steps in parallel + stepsPerRunner.mapIndexed { index, partSteps -> + // We want to run the runners in parallel but wait for everything to complete before we proceed + async { + partSteps.forEach { (profile, step) -> + logcat { "PIR-SCAN: Start scan on runner=$index for profile=$profile with step=$step" } + runners[index].start(profile, listOf(step)) runners[index].stop() + logcat { "PIR-SCAN: Finish scan on runner=$index for profile=$profile with step=$step" } } - }.awaitAll() + } + }.awaitAll() - logcat { "PIR-SCAN: Scan completed for all runners" } + logcat { "PIR-SCAN: Scan completed for all runners on all profiles" } emitScanCompletedPixel( runType, currentTimeProvider.currentTimeMillis() - startTimeMillis, @@ -183,33 +231,34 @@ class RealPirScan @Inject constructor( runners.clear() } repository.deleteAllScanResults() - repository.getUserProfiles().also { - if (it.isNotEmpty()) { - // Temporarily taking the first profile only for the PoC. In the reality, more than 1 should be allowed. - val storedProfile = it[0] - profileQuery = ProfileQuery( - firstName = storedProfile.userName.firstName, - lastName = storedProfile.userName.lastName, - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, - addresses = listOf( - Address( - city = storedProfile.addresses.city, - state = storedProfile.addresses.state, + repository.getUserProfiles().also { profiles -> + if (profiles.isNotEmpty()) { + profileQueries = profiles.map { storedProfile -> + ProfileQuery( + id = storedProfile.id, + firstName = storedProfile.userName.firstName, + lastName = storedProfile.userName.lastName, + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + addresses = listOf( + Address( + city = storedProfile.addresses.city, + state = storedProfile.addresses.state, + ), ), - ), - birthYear = storedProfile.birthYear, - fullName = storedProfile.userName.middleName?.run { - "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" - } - ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", - age = LocalDate.now().year - storedProfile.birthYear, - deprecated = false, - ) + birthYear = storedProfile.birthYear, + fullName = storedProfile.userName.middleName?.run { + "${storedProfile.userName.firstName} $this ${storedProfile.userName.lastName}" + } + ?: "${storedProfile.userName.firstName} ${storedProfile.userName.lastName}", + age = LocalDate.now().year - storedProfile.birthYear, + deprecated = false, + ) + } } } - logcat { "PIR-SCAN: Running scan on profile: $profileQuery on ${Thread.currentThread().name}" } + logcat { "PIR-SCAN: Running scan on profiles: $profileQueries on ${Thread.currentThread().name}" } } override suspend fun executeAllBrokers( diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt index c0a9e8441f41..200fd13fc788 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/scripts/models/PirScriptProfile.kt @@ -20,7 +20,7 @@ package com.duckduckgo.pir.internal.scripts.models * This profile represents the data we can get from the web UI / from the user */ data class ProfileQuery( - val id: Int? = null, + val id: Long? = null, val firstName: String, val lastName: String, val middleName: String? = null, diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt index 7fd7e67b416f..eabea00c69df 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/PirRepository.kt @@ -81,6 +81,11 @@ interface PirRepository { suspend fun getBrokerOptOutSteps(name: String): String? + /** + * Returns a list of broker names for which an extract profile result is available. + * + * @param formOptOutOnly - True to only return the brokers with a form opt-out type. + */ suspend fun getBrokersForOptOut(formOptOutOnly: Boolean): List suspend fun saveNavigateResult( @@ -99,9 +104,14 @@ interface PirRepository { response: ExtractedResponse, ) - suspend fun getExtractProfileResultForBroker( + /** + * Returns a list of all [ExtractedProfileResult] found for this particular broker. + * + * @param brokerName - Name of the broker + */ + suspend fun getExtractProfileResultsForBroker( brokerName: String, - ): ExtractedProfileResult? + ): List fun getAllScanResultsFlow(): Flow> @@ -306,15 +316,16 @@ class RealPirRepository( it.extractResults.isNotEmpty() }.map { it.brokerName - }.run { - if (formOptOutOnly) { - this.filter { - brokerDao.getOptOutJson(it)?.contains("\"optOutType\":\"formOptOut\"") == true + }.distinct() + .run { + if (formOptOutOnly) { + this.filter { + brokerDao.getOptOutJson(it)?.contains("\"optOutType\":\"formOptOut\"") == true + } + } else { + this } - } else { - this } - } } override suspend fun saveNavigateResult( @@ -371,18 +382,19 @@ class RealPirRepository( } } - override suspend fun getExtractProfileResultForBroker(brokerName: String): ExtractedProfileResult? = withContext(dispatcherProvider.io()) { - return@withContext scanResultsDao.getExtractProfileResultForProfile(brokerName).firstOrNull()?.run { - ExtractedProfileResult( - brokerName = this.brokerName, - completionTimeInMillis = this.completionTimeInMillis, - actionType = this.actionType, - extractResults = this.extractResults.mapNotNull { - extractedProfileAdapter.fromJson(it) - }, - profileQuery = null, - ) - } + override suspend fun getExtractProfileResultsForBroker(brokerName: String): List = withContext(dispatcherProvider.io()) { + return@withContext scanResultsDao.getExtractProfileResultForBroker(brokerName) + .map { extractProfileResult -> + ExtractedProfileResult( + brokerName = extractProfileResult.brokerName, + completionTimeInMillis = extractProfileResult.completionTimeInMillis, + actionType = extractProfileResult.actionType, + extractResults = extractProfileResult.extractResults.mapNotNull { + extractedProfileAdapter.fromJson(it) + }, + profileQuery = profileQueryAdapter.fromJson(extractProfileResult.userData), + ) + } } override fun getAllScanResultsFlow(): Flow> { diff --git a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt index 1f65396432ba..b2f730973e25 100644 --- a/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt +++ b/pir/pir-internal/src/main/java/com/duckduckgo/pir/internal/store/db/ScanResultsDao.kt @@ -55,7 +55,7 @@ interface ScanResultsDao { fun getAllExtractProfileResult(): List @Query("SELECT * FROM pir_scan_extracted_profile WHERE brokerName = :brokerName ORDER BY completionTimeInMillis") - fun getExtractProfileResultForProfile(brokerName: String): List + fun getExtractProfileResultForBroker(brokerName: String): List @Insert(onConflict = OnConflictStrategy.REPLACE) fun insertExtractProfileResult(extractProfileResult: ExtractProfileResult)