diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt index 0fed3ef9a0..049a2c6b0e 100644 --- a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt +++ b/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,9 +163,6 @@ 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, @@ -183,8 +170,7 @@ open class Nimbus( dataDir.path, metricsHandler, geckoPrefHandler, - remoteSettingsService, - collectionName, + server, ) } diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt index 1c17874545..9f75a0be50 100644 --- a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusBuilder.kt +++ b/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 diff --git a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusBuilderTest.kt b/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusBuilderTest.kt index 58b55224e5..abea8b66a1 100644 --- a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusBuilderTest.kt +++ b/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 diff --git a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/util/TestNimbusBuilder.kt b/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/util/TestNimbusBuilder.kt index 41a83db3c8..03da9bdf34 100644 --- a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/util/TestNimbusBuilder.kt +++ b/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(context) { diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 0b0cc1567e..94184a63d7 100644 --- a/components/nimbus/src/nimbus.udl +++ b/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 diff --git a/components/nimbus/src/stateful/client/mod.rs b/components/nimbus/src/stateful/client/mod.rs index 43025f9d7a..c416b4e077 100644 --- a/components/nimbus/src/stateful/client/mod.rs +++ b/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, + pub collection_name: String, +} + pub(crate) fn create_client( - rs_service: Option>, - collection_name: Option, + rs_info: Option, ) -> Result> { - 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()), }) } diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 85edcfce85..aef821d6b5 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/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, gecko_pref_handler: Option>, - remote_settings_service: Option>, - collection_name: Option, + remote_settings_info: Option, ) -> Result { - 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 { diff --git a/components/nimbus/src/tests/stateful/client/test_null_client.rs b/components/nimbus/src/tests/stateful/client/test_null_client.rs index e235745663..0072df677f 100644 --- a/components/nimbus/src/tests/stateful/client/test_null_client.rs +++ b/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()?; diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index cb9c17d1b3..0e5798da2f 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -47,7 +47,6 @@ fn test_telemetry_reset() -> Result<()> { Box::new(metrics), None, None, - None, )?; let get_targeting_attributes_nimbus_id = || { @@ -141,7 +140,6 @@ fn test_installation_date() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.initialize()?; @@ -177,7 +175,6 @@ fn test_installation_date() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; // Since no installation_date is in context and storage is cleared, // we will default to today's date @@ -200,7 +197,6 @@ fn test_installation_date() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.initialize()?; // Since we already persisted the date from the previous run, @@ -231,7 +227,6 @@ fn test_installation_date() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.initialize()?; client.apply_pending_experiments()?; @@ -261,7 +256,6 @@ fn test_days_since_calculation_happens_at_startup() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; // 0. We haven't initialized anything yet, so dates won't be available. @@ -290,7 +284,6 @@ fn test_days_since_calculation_happens_at_startup() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.apply_pending_experiments()?; let targeting_attributes = client.get_targeting_attributes(); @@ -312,7 +305,6 @@ fn test_days_since_update_changes_with_context() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.initialize()?; @@ -334,7 +326,6 @@ fn test_days_since_update_changes_with_context() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.initialize()?; client.apply_pending_experiments()?; @@ -363,7 +354,6 @@ fn test_days_since_update_changes_with_context() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.initialize()?; client.apply_pending_experiments()?; @@ -398,7 +388,6 @@ fn test_days_since_update_changes_with_context() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.initialize()?; client.apply_pending_experiments()?; @@ -441,7 +430,6 @@ fn test_days_since_install() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_install_time(Utc::now() - Duration::days(10)); client.initialize()?; @@ -513,7 +501,6 @@ fn test_days_since_install_failed_targeting() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_install_time(Utc::now() - Duration::days(10)); client.initialize()?; @@ -584,7 +571,6 @@ fn test_days_since_update() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_update_time(Utc::now() - Duration::days(10)); client.initialize()?; @@ -656,7 +642,6 @@ fn test_days_since_update_failed_targeting() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_update_time(Utc::now() - Duration::days(10)); client.initialize()?; @@ -740,7 +725,6 @@ fn event_store_exists_for_apply_pending_experiments() -> Result<()> { Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { app_context, @@ -863,7 +847,6 @@ fn event_store_on_targeting_attributes_is_updated_after_an_event_is_recorded() - Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { app_context, @@ -946,7 +929,6 @@ fn test_ios_rollout() -> Result<()> { Box::new(metrics), None, None, - None, )?; let exp = get_ios_rollout_experiment(); @@ -983,7 +965,6 @@ fn test_fetch_enabled() -> Result<()> { Box::new(metrics.clone()), None, None, - None, )?; client.set_fetch_enabled(false)?; @@ -998,7 +979,6 @@ fn test_fetch_enabled() -> Result<()> { Box::new(metrics), None, None, - None, )?; assert!(!client.is_fetch_enabled()?); Ok(()) @@ -1024,7 +1004,6 @@ fn test_active_enrollment_in_targeting() -> Result<()> { Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { app_context, @@ -1090,7 +1069,6 @@ fn test_previous_enrollments_in_targeting() -> Result<()> { Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { @@ -1235,7 +1213,6 @@ fn test_opt_out_multiple_experiments_same_feature_does_not_re_enroll() -> Result Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { @@ -1363,7 +1340,6 @@ fn test_enrollment_status_metrics_not_recorded_app_name_mismatch() -> Result<()> Box::new(metrics.clone()), None, None, - None, )?; client.set_nimbus_id(&Uuid::from_str("53baafb3-b800-42ac-878c-c3451e250928")?)?; @@ -1406,7 +1382,6 @@ fn test_enrollment_status_metrics_not_recorded_channel_mismatch() -> Result<()> Box::new(metrics.clone()), None, None, - None, )?; client.set_nimbus_id(&Uuid::from_str("53baafb3-b800-42ac-878c-c3451e250928")?)?; @@ -1446,7 +1421,6 @@ fn with_metrics(metrics: &TestMetrics, coenrolling_feature: &str) -> Result Result<()> { Box::new(metrics), None, None, - None, )?; let targeting_attributes = TargetingAttributes { app_context, @@ -1702,7 +1675,6 @@ fn test_recorded_context_recorded() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; client.initialize()?; @@ -1751,7 +1723,6 @@ fn test_recorded_context_event_queries() -> Result<()> { Box::new(metrics), None, None, - None, )?; client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; client.initialize()?; @@ -1811,7 +1782,6 @@ fn test_gecko_pref_enrollment() -> Result<()> { Box::new(metrics), Some(Box::new(handler)), None, - None, )?; client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; client.initialize()?; @@ -1890,7 +1860,6 @@ fn test_gecko_pref_unenrollment() -> Result<()> { Box::new(metrics), Some(Box::new(handler)), None, - None, )?; client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; client.initialize()?; diff --git a/components/nimbus/tests/common/mod.rs b/components/nimbus/tests/common/mod.rs index c0db8282eb..bbc4c4c930 100644 --- a/components/nimbus/tests/common/mod.rs +++ b/components/nimbus/tests/common/mod.rs @@ -11,6 +11,7 @@ use rkv::StoreOptions; use nimbus::{ error::{debug, Result}, metrics::{EnrollmentStatusExtraDef, MetricsHandler}, + stateful::client::NimbusServerSettings, AppContext, NimbusClient, RemoteSettingsServer, }; @@ -93,8 +94,10 @@ fn new_test_client_internal( tmp_dir.path(), Box::new(NoopMetricsHandler), None, - Some(Arc::new(remote_settings_service)), - Some("collection_name".to_string()), + Some(NimbusServerSettings { + rs_service: Arc::new(remote_settings_service), + collection_name: "collection_name".to_string(), + }), ) } diff --git a/components/nimbus/tests/test_fs_client.rs b/components/nimbus/tests/test_fs_client.rs index b4425ff160..10c2c75c49 100644 --- a/components/nimbus/tests/test_fs_client.rs +++ b/components/nimbus/tests/test_fs_client.rs @@ -7,7 +7,7 @@ use std::sync::Arc; -use nimbus::error::Result; +use nimbus::{error::Result, stateful::client::NimbusServerSettings}; use remote_settings::{RemoteSettingsConfig2, RemoteSettingsContext, RemoteSettingsService}; mod common; @@ -44,8 +44,10 @@ fn test_simple() -> Result<()> { tmp_dir.path(), Box::new(NoopMetricsHandler), None, - Some(Arc::new(remote_settings_service)), - Some("collection_name".to_string()), + Some(NimbusServerSettings { + rs_service: Arc::new(remote_settings_service), + collection_name: "collection_name".to_string(), + }), )?; client.fetch_experiments()?; client.apply_pending_experiments()?; diff --git a/examples/nimbus/src/main.rs b/examples/nimbus/src/main.rs index e1ea037530..33d116870b 100644 --- a/examples/nimbus/src/main.rs +++ b/examples/nimbus/src/main.rs @@ -5,7 +5,10 @@ use clap::{Parser, Subcommand}; use std::path::PathBuf; -use nimbus::error::{info, Result}; +use nimbus::{ + error::{info, Result}, + stateful::client::NimbusServerSettings, +}; #[derive(Parser)] #[command(name = "Nimbus SDK Demo")] @@ -199,8 +202,10 @@ fn main() -> Result<()> { db_path, Box::new(NoopMetricsHandler), None, - Some(Arc::new(remote_settings_services)), - Some(collection_name.to_string()), + Some(NimbusServerSettings { + rs_service: Arc::new(remote_settings_services), + collection_name: collection_name.to_string(), + }), )?; info!("Nimbus ID is {}", nimbus_client.nimbus_id()?);