Add isReturningUser and isPrivacyProEligible targets and modularize t…

…arget management (#5277)

Task/Issue URL: https://app.asana.com/0/1198194956794324/1208710375047087/f

### Description
This PR adds couple new targets for FF, ie. isReturningUser and isPrivacyProEligible, both of type `Boolean`

The PR also modularizes the way we hande targets, turning them into plugins so that it's easier in the future to add targets without (almost) touching the toggles API

### Steps to test this PR
Added unit and integration tests 

_Test new targets_
- [ ] install from this branch
- [ ] create a test feature
- [ ] create a remote config for the test feature that contains `isPrivacyProEligible` as target
- [ ] in a client that is not eligigle for PPro verify the feature is always disabled
- [ ] in a client that is eligigle for PPro verify the feature is enabled when the feature is enabled in remote config
- [ ] create a remote config for the test feature that contains `isReturningUser ` as target
- [ ] in a fresh install verify the test feature is always disabled
- [ ] in a returning install verify the test feature is enabled when the feature is enabled in remote config

anvil/anvil-compiler/src/main/java/com/duckduckgo/anvil/compiler/ContributesRemoteFeatureCodeGenerator.kt

@@ -118,7 +118,6 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
                                 .flavorNameProvider({ appBuildConfig.flavor.name })
                                 .featureName(%S)
                                 .appVariantProvider({ appBuildConfig.variantName })
-                                .localeProvider({ appBuildConfig.deviceLocale })
                                 .callback(callback)
                                 // save empty variants will force the default variant to be set
                                 .forceDefaultVariantProvider({ variantManager.updateVariants(emptyList()) })
@@ -521,6 +520,8 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
                                         variantKey = target.variantKey,
                                         localeCountry = target.localeCountry,
                                         localeLanguage = target.localeLanguage,
+                                        isReturningUser = target.isReturningUser,
+                                        isPrivacyProEligible = target.isPrivacyProEligible,
                                     )
                                 } ?: emptyList()
                                 val cohorts = jsonToggle?.cohorts?.map { cohort ->
@@ -727,11 +728,22 @@ class ContributesRemoteFeatureCodeGenerator : CodeGenerator {
                     .addParameter("variantKey", String::class.asClassName())
                     .addParameter("localeCountry", String::class.asClassName())
                     .addParameter("localeLanguage", String::class.asClassName())
+                    .addParameter("isReturningUser", Boolean::class.asClassName().copy(nullable = true))
+                    .addParameter("isPrivacyProEligible", Boolean::class.asClassName().copy(nullable = true))
                     .build(),
             )
             .addProperty(PropertySpec.builder("variantKey", String::class.asClassName()).initializer("variantKey").build())
             .addProperty(PropertySpec.builder("localeCountry", String::class.asClassName()).initializer("localeCountry").build())
             .addProperty(PropertySpec.builder("localeLanguage", String::class.asClassName()).initializer("localeLanguage").build())
+            .addProperty(
+                PropertySpec.builder("isReturningUser", Boolean::class.asClassName().copy(nullable = true)).initializer("isReturningUser").build(),
+            )
+            .addProperty(
+                PropertySpec
+                    .builder("isPrivacyProEligible", Boolean::class.asClassName().copy(nullable = true))
+                    .initializer("isPrivacyProEligible")
+                    .build(),
+            )
             .build()
     }
 

experiments/experiments-impl/src/main/java/com/duckduckgo/experiments/impl/ReturningUserToggleTargetMatcher.kt

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2024 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.experiments.impl
+
+import com.duckduckgo.di.scopes.AppScope
+import com.duckduckgo.experiments.api.VariantManager
+import com.duckduckgo.experiments.impl.reinstalls.REINSTALL_VARIANT
+import com.duckduckgo.feature.toggles.api.Toggle.State.Target
+import com.duckduckgo.feature.toggles.api.Toggle.TargetMatcherPlugin
+import com.squareup.anvil.annotations.ContributesMultibinding
+import javax.inject.Inject
+
+@ContributesMultibinding(AppScope::class)
+class ReturningUserToggleTargetMatcher @Inject constructor(
+    private val variantManager: VariantManager,
+) : TargetMatcherPlugin {
+    override fun matchesTargetProperty(target: Target): Boolean {
+        return target.isReturningUser?.let { isReturningUserTarget ->
+            val isReturningUser = variantManager.getVariantKey() == REINSTALL_VARIANT
+            isReturningUserTarget == isReturningUser
+        } ?: true
+    }
+}

experiments/experiments-impl/src/test/java/com/duckduckgo/experiments/impl/ReturningUserToggleTargetMatcherTest.kt

@@ -0,0 +1,62 @@
+package com.duckduckgo.experiments.impl
+
+import com.duckduckgo.experiments.api.VariantManager
+import com.duckduckgo.feature.toggles.api.Toggle
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.whenever
+
+class ReturningUserToggleTargetMatcherTest {
+    private val variantManager: VariantManager = mock()
+    private val matcher = ReturningUserToggleTargetMatcher(variantManager)
+
+    @Test
+    fun whenReturningUserAndNullTargetThenMatchesTargetReturnsTrue() {
+        whenever(variantManager.getVariantKey()).thenReturn("ru")
+
+        assertTrue(matcher.matchesTargetProperty(NULL_TARGET))
+    }
+
+    @Test
+    fun whenNotReturningUserAndNullTargetThenMatchesTargetReturnsTrue() {
+        whenever(variantManager.getVariantKey()).thenReturn("foo")
+
+        assertTrue(matcher.matchesTargetProperty(NULL_TARGET))
+    }
+
+    @Test
+    fun whenReturningUserAndTargetMatchesThenReturnTrue() {
+        whenever(variantManager.getVariantKey()).thenReturn("ru")
+
+        assertTrue(matcher.matchesTargetProperty(RU_TARGET))
+    }
+
+    @Test
+    fun whenNoReturningUserAndTargetMatchesThenReturnTrue() {
+        whenever(variantManager.getVariantKey()).thenReturn("foo")
+
+        assertTrue(matcher.matchesTargetProperty(NOT_RU_TARGET))
+    }
+
+    @Test
+    fun whenReturningUserAndTargetNotMatchingThenReturnFalse() {
+        whenever(variantManager.getVariantKey()).thenReturn("ru")
+
+        assertFalse(matcher.matchesTargetProperty(NOT_RU_TARGET))
+    }
+
+    @Test
+    fun whenNoReturningUserAndTargetNotMatchingThenReturnTrue() {
+        whenever(variantManager.getVariantKey()).thenReturn("foo")
+
+        assertFalse(matcher.matchesTargetProperty(RU_TARGET))
+    }
+
+    companion object {
+        private val NULL_TARGET = Toggle.State.Target(null, null, null, null, null)
+        private val RU_TARGET = NULL_TARGET.copy(isReturningUser = true)
+        private val NOT_RU_TARGET = NULL_TARGET.copy(isReturningUser = false)
+    }
+}

feature-toggles/feature-toggles-api/src/main/java/com/duckduckgo/feature/toggles/api/FeatureToggles.kt

@@ -29,7 +29,6 @@ import java.lang.reflect.Proxy
 import java.time.ZoneId
 import java.time.ZonedDateTime
 import java.time.temporal.ChronoUnit
-import java.util.Locale
 import kotlin.random.Random
 import org.apache.commons.math3.distribution.EnumeratedIntegerDistribution
 
@@ -39,7 +38,6 @@ class FeatureToggles private constructor(
     private val flavorNameProvider: () -> String,
     private val featureName: String,
     private val appVariantProvider: () -> String?,
-    private val localeProvider: () -> Locale?,
     private val forceDefaultVariant: () -> Unit,
     private val callback: FeatureTogglesCallback?,
 ) {
@@ -52,7 +50,6 @@ class FeatureToggles private constructor(
         private var flavorNameProvider: () -> String = { "" },
         private var featureName: String? = null,
         private var appVariantProvider: () -> String? = { "" },
-        private var localeProvider: () -> Locale? = { Locale.getDefault() },
         private var forceDefaultVariant: () -> Unit = { /** noop **/ },
         private var callback: FeatureTogglesCallback? = null,
     ) {
@@ -62,7 +59,6 @@ class FeatureToggles private constructor(
         fun flavorNameProvider(flavorNameProvider: () -> String) = apply { this.flavorNameProvider = flavorNameProvider }
         fun featureName(featureName: String) = apply { this.featureName = featureName }
         fun appVariantProvider(variantName: () -> String?) = apply { this.appVariantProvider = variantName }
-        fun localeProvider(locale: () -> Locale?) = apply { this.localeProvider = locale }
         fun forceDefaultVariantProvider(forceDefaultVariant: () -> Unit) = apply { this.forceDefaultVariant = forceDefaultVariant }
         fun callback(callback: FeatureTogglesCallback) = apply { this.callback = callback }
         fun build(): FeatureToggles {
@@ -82,7 +78,6 @@ class FeatureToggles private constructor(
                 flavorNameProvider = flavorNameProvider,
                 featureName = featureName!!,
                 appVariantProvider = appVariantProvider,
-                localeProvider = localeProvider,
                 forceDefaultVariant = forceDefaultVariant,
                 callback = this.callback,
             )
@@ -130,7 +125,6 @@ class FeatureToggles private constructor(
                 appVersionProvider = appVersionProvider,
                 flavorNameProvider = flavorNameProvider,
                 appVariantProvider = appVariantProvider,
-                localeProvider = localeProvider,
                 forceDefaultVariant = forceDefaultVariant,
                 callback = callback,
             ).also { featureToggleCache[method] = it }
@@ -230,6 +224,8 @@ interface Toggle {
             val variantKey: String?,
             val localeCountry: String?,
             val localeLanguage: String?,
+            val isReturningUser: Boolean?,
+            val isPrivacyProEligible: Boolean?,
         )
         data class Cohort(
             val name: String,
@@ -264,6 +260,18 @@ interface Toggle {
         fun get(key: String): State?
     }
 
+    /**
+     * It is possible to add feature [Target]s.
+     * To do that, just add the property inside the [Target] and implement the [TargetMatcherPlugin] to do the matching
+     */
+    interface TargetMatcherPlugin {
+        /**
+         * Implement this method when adding a new target property.
+         * @return `true` if the target matches else false
+         */
+        fun matchesTargetProperty(target: State.Target): Boolean
+    }
+
     /**
      * This annotation is required.
      * It specifies the default value of the feature flag when it's not remotely defined
@@ -301,7 +309,6 @@ internal class ToggleImpl constructor(
     private val appVersionProvider: () -> Int,
     private val flavorNameProvider: () -> String = { "" },
     private val appVariantProvider: () -> String?,
-    private val localeProvider: () -> Locale?,
     private val forceDefaultVariant: () -> Unit,
     private val callback: FeatureTogglesCallback?,
 ) : Toggle {
@@ -317,31 +324,6 @@ internal class ToggleImpl constructor(
         return this.featureName().hashCode()
     }
 
-    private fun Toggle.State.evaluateTargetMatching(isExperiment: Boolean): Boolean {
-        val variant = appVariantProvider.invoke()
-        // no targets then consider always treated
-        if (this.targets.isEmpty()) {
-            return true
-        }
-        // if it's an experiment we only check target variants and ignore all the rest
-        val variantTargets = this.targets.mapNotNull { it.variantKey }
-        if (isExperiment && variantTargets.isNotEmpty()) {
-            return variantTargets.contains(variant)
-        }
-        // finally, check all other targets
-        val countryTarget = this.targets.mapNotNull { it.localeCountry?.lowercase() }
-        val languageTarget = this.targets.mapNotNull { it.localeLanguage?.lowercase() }
-
-        if (countryTarget.isNotEmpty() && !countryTarget.contains(localeProvider.invoke()?.country?.lowercase())) {
-            return false
-        }
-        if (languageTarget.isNotEmpty() && !languageTarget.contains(localeProvider.invoke()?.language?.lowercase())) {
-            return false
-        }
-
-        return true
-    }
-
     override fun featureName(): FeatureName {
         val parts = key.split("_")
         return if (parts.size == 2) {
@@ -381,6 +363,27 @@ internal class ToggleImpl constructor(
     }
 
     private fun isRolloutEnabled(): Boolean {
+        // This fun is in there because it should never be called outside this method
+        fun Toggle.State.evaluateTargetMatching(isExperiment: Boolean): Boolean {
+            val variant = appVariantProvider.invoke()
+            // no targets then consider always treated
+            if (this.targets.isEmpty()) {
+                return true
+            }
+            // if it's an experiment we only check target variants and ignore all the rest
+            // this is because the (retention) experiments define their targets some place else
+            val variantTargets = this.targets.mapNotNull { it.variantKey }
+            if (isExperiment && variantTargets.isNotEmpty()) {
+                return variantTargets.contains(variant)
+            }
+            // finally, check all other targets
+            val nonVariantTargets = this.targets.filter { it.variantKey == null }
+
+            // callback should never be null, but if it is, consider targets a match
+            return callback?.matchesToggleTargets(nonVariantTargets) ?: true
+        }
+
+        // This fun is in there because it should never be called outside this method
         fun evaluateLocalEnable(state: State, isExperiment: Boolean): Boolean {
             // variants are only considered for Experiment feature flags
             val doTargetsMatch = state.evaluateTargetMatching(isExperiment)
@@ -484,26 +487,12 @@ internal class ToggleImpl constructor(
                 cohorts[randomIndex.sample()]
             }.getOrNull()
         }
-        fun containsAndMatchCohortTargets(targets: State.Target?): Boolean {
-            return targets?.let {
-                targets.localeLanguage?.let { targetLanguage ->
-                    val deviceLocale = localeProvider.invoke()
-                    if (deviceLocale?.language != targetLanguage) {
-                        return false
-                    }
-                }
-                targets.localeCountry?.let { targetCountry ->
-                    val deviceLocale = localeProvider.invoke()
-                    if (deviceLocale?.country != targetCountry) {
-                        return false
-                    }
-                }
-                return true
-            } ?: return true // no targets mean any target
+        fun containsAndMatchCohortTargets(targets: List<State.Target>): Boolean {
+            return callback?.matchesToggleTargets(targets) ?: true
         }
 
         // In the remote config, targets is a list, but it should not be. So we pick the first one (?)
-        if (!containsAndMatchCohortTargets(targets.firstOrNull())) {
+        if (!containsAndMatchCohortTargets(targets)) {
             return null
         }