[Bug 2003236] refactor NimbusClient params

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt

@@ -43,25 +43,15 @@ import org.mozilla.experiments.nimbus.internal.MetricsHandler
 import org.mozilla.experiments.nimbus.internal.NimbusClient
 import org.mozilla.experiments.nimbus.internal.NimbusClientInterface
 import org.mozilla.experiments.nimbus.internal.NimbusException
+import org.mozilla.experiments.nimbus.internal.NimbusServerSettings
 import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
 import org.mozilla.experiments.nimbus.internal.RecordedContext
 import java.io.File
 import java.io.IOException
 import kotlin.system.measureTimeMillis
 
-private const val EXPERIMENT_COLLECTION_NAME = "nimbus-mobile-experiments"
 const val NIMBUS_DATA_DIR: String = "nimbus_data"
 
-/**
- * This class allows client apps to configure Nimbus to point to your own server.
- * Client app developers should set up their own Nimbus infrastructure, to avoid different
- * organizations running conflicting experiments or hitting servers with extra network traffic.
- */
-class NimbusServerSettings(
-    val remoteSettingsService: RemoteSettingsService?,
-    val collection: String = EXPERIMENT_COLLECTION_NAME,
-)
-
 /**
  * A implementation of the [NimbusInterface] interface backed by the Nimbus SDK.
  */
@@ -173,18 +163,14 @@ open class Nimbus(
         // Build Nimbus AppContext object to pass into initialize
         val experimentContext = buildExperimentContext(context, appInfo, deviceInfo)
 
-        val remoteSettingsService = server?.remoteSettingsService
-        val collectionName = server?.collection
-
         nimbusClient = NimbusClient(
             experimentContext,
             recordedContext,
             coenrollingFeatureIds,
             dataDir.path,
             metricsHandler,
             geckoPrefHandler,
-            remoteSettingsService,
-            collectionName,
+            server,
         )
     }
 

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt

@@ -12,6 +12,7 @@ import kotlinx.coroutines.runBlocking
 import mozilla.appservices.remotesettings.RemoteSettingsService
 import org.mozilla.experiments.nimbus.internal.FeatureManifestInterface
 import org.mozilla.experiments.nimbus.internal.GeckoPrefHandler
+import org.mozilla.experiments.nimbus.internal.NimbusServerSettings
 import org.mozilla.experiments.nimbus.internal.RecordedContext
 
 private const val TIME_OUT_LOADING_EXPERIMENT_FROM_DISK_MS = 200L

components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusBuilderTest.kt

@@ -15,6 +15,7 @@ import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mozilla.experiments.nimbus.internal.NimbusServerSettings
 import org.robolectric.RobolectricTestRunner
 import kotlin.random.Random
 
@@ -34,8 +35,8 @@ class NimbusBuilderTest {
         }.build(
             appInfo,
             NimbusServerSettings(
-                remoteSettingsService = RemoteSettingsService(storageDir = "dummy", config = RemoteSettingsConfig2()),
-                collection = "nimbus-preview",
+                rsService = RemoteSettingsService(storageDir = "dummy", config = RemoteSettingsConfig2()),
+                collectionName = "nimbus-preview",
             ),
         ) as DummyNimbus
         assertTrue(n1.usePreviewCollection)
@@ -143,7 +144,7 @@ class DummyNimbus(
     var initialExperiments: Int? = null
 
     val usePreviewCollection: Boolean
-        get() = serverSettings?.collection == "nimbus-preview"
+        get() = serverSettings?.collectionName == "nimbus-preview"
 
     override fun applyLocalExperiments(file: Int): Job {
         initialExperiments = file

components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/util/TestNimbusBuilder.kt

@@ -11,7 +11,7 @@ import org.mozilla.experiments.nimbus.NimbusAppInfo
 import org.mozilla.experiments.nimbus.NimbusDelegate
 import org.mozilla.experiments.nimbus.NimbusDeviceInfo
 import org.mozilla.experiments.nimbus.NimbusInterface
-import org.mozilla.experiments.nimbus.NimbusServerSettings
+import org.mozilla.experiments.nimbus.internal.NimbusServerSettings
 import org.mozilla.experiments.nimbus.uninitialized
 
 class TestNimbusBuilder(context: Context) : AbstractNimbusBuilder<NimbusInterface>(context) {

components/nimbus/src/nimbus.udl

@@ -150,6 +150,12 @@ dictionary PrefEnrollmentData {
     string variable;
 };
 
+
+dictionary NimbusServerSettings {
+    RemoteSettingsService rs_service;
+    string collection_name;
+};
+
 [Error]
 enum NimbusError {
     "InvalidPersistedData", "RkvError", "IOError",
@@ -188,8 +194,7 @@ interface NimbusClient {
         string dbpath,
         MetricsHandler metrics_handler,
         GeckoPrefHandler? gecko_pref_handler,
-        RemoteSettingsService? remote_settings_service,
-        string? collection_name
+        NimbusServerSettings? remote_settings_info
     );
 
     /// Initializes the database and caches enough information so that the

components/nimbus/src/stateful/client/mod.rs

@@ -14,13 +14,20 @@ use null_client::NullClient;
 use remote_settings::RemoteSettingsService;
 use url::Url;
 
+pub struct NimbusServerSettings {
+    pub rs_service: Arc<RemoteSettingsService>,
+    pub collection_name: String,
+}
+
 pub(crate) fn create_client(
-    rs_service: Option<Arc<RemoteSettingsService>>,
-    collection_name: Option<String>,
+    rs_info: Option<NimbusServerSettings>,
 ) -> Result<Box<dyn SettingsClient + Send>> {
-    Ok(match (rs_service, collection_name) {
-        (Some(rs_service), Some(collection_name)) => {
-            let url = Url::parse(&rs_service.client_url())?; // let call this the server url
+    Ok(match rs_info {
+        Some(NimbusServerSettings {
+            rs_service,
+            collection_name,
+        }) => {
+            let url = Url::parse(&rs_service.client_url())?; // server url
             match url.scheme() {
                 "file" => {
                     // Everything in `config` other than the url/path is ignored for the
@@ -35,8 +42,7 @@ pub(crate) fn create_client(
                 _ => Box::new(rs_service.make_client(collection_name)),
             }
         }
-        (Some(_), None) => return Err(NimbusError::InternalError("collection name required")),
-        (None, _) => Box::new(NullClient::new()),
+        None => Box::new(NullClient::new()),
     })
 }
 

components/nimbus/src/stateful/nimbus_client.rs

@@ -23,7 +23,7 @@ use crate::{
     schema::parse_experiments,
     stateful::{
         behavior::EventStore,
-        client::{create_client, SettingsClient},
+        client::{create_client, NimbusServerSettings, SettingsClient},
         dbcache::DatabaseCache,
         enrollment::{
             get_experiment_participation, get_rollout_participation, opt_in_with_branch, opt_out,
@@ -109,10 +109,9 @@ impl NimbusClient {
         db_path: P,
         metrics_handler: Box<dyn MetricsHandler>,
         gecko_pref_handler: Option<Box<dyn GeckoPrefHandler>>,
-        remote_settings_service: Option<Arc<RemoteSettingsService>>,
-        collection_name: Option<String>,
+        remote_settings_info: Option<NimbusServerSettings>,
     ) -> Result<Self> {
-        let settings_client = Mutex::new(create_client(remote_settings_service, collection_name)?);
+        let settings_client = Mutex::new(create_client(remote_settings_info)?);
 
         let targeting_attributes: TargetingAttributes = app_context.clone().into();
         let mutable_state = Mutex::new(InternalMutableState {

components/nimbus/src/tests/stateful/client/test_null_client.rs

@@ -26,7 +26,6 @@ fn test_null_client() -> Result<()> {
         Box::new(metrics),
         None,
         None,
-        None,
     )?;
     client.fetch_experiments()?;
     client.apply_pending_experiments()?;