Skip to content

Commit

Permalink
update Rust for recorded context to handle event store queries
Browse files Browse the repository at this point in the history
  • Loading branch information
jeddai committed Oct 22, 2024
1 parent c8a1a57 commit f277170
Show file tree
Hide file tree
Showing 18 changed files with 574 additions and 84 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@

## ✨ What's New ✨

## ⚠️ Breaking Changes ⚠️

### Nimbus SDK ⛅️🔬🔭
- Added methods to `RecordedContext` for retrieving event queries and setting their values back to the foreign object ([#6322](https://github.com/mozilla/application-services/pull/6322)).

### Suggest
- Added support for Fakespot suggestions.
- Added support for recording metrics
Expand Down
32 changes: 22 additions & 10 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion components/nimbus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ name = "nimbus"
default=["stateful"]
rkv-safe-mode = ["dep:rkv"]
stateful-uniffi-bindings = []
stateful = ["rkv-safe-mode", "stateful-uniffi-bindings", "dep:remote_settings"]
stateful = ["rkv-safe-mode", "stateful-uniffi-bindings", "dep:remote_settings", "dep:regex"]

[dependencies]
anyhow = "1"
Expand All @@ -39,6 +39,7 @@ unicode-segmentation = "1.8.0"
error-support = { path = "../support/error" }
remote_settings = { path = "../remote_settings", optional = true }
cfg-if = "1.0.0"
regex = { version = "1.10.5", optional = true }

[build-dependencies]
uniffi = { workspace = true, features = ["build"] }
Expand All @@ -49,3 +50,4 @@ viaduct-reqwest = { path = "../support/viaduct-reqwest" }
env_logger = "0.10"
clap = "2.34"
tempfile = "3"
ctor = "0.2.2"
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertThrows
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Ignore
Expand All @@ -37,7 +38,9 @@ import org.mozilla.experiments.nimbus.GleanMetrics.NimbusHealth
import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent
import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEventType
import org.mozilla.experiments.nimbus.internal.JsonObject
import org.mozilla.experiments.nimbus.internal.NimbusException
import org.mozilla.experiments.nimbus.internal.RecordedContext
import org.mozilla.experiments.nimbus.internal.validateEventQueries
import org.robolectric.RobolectricTestRunner
import java.util.Calendar
import java.util.concurrent.Executors
Expand Down Expand Up @@ -71,6 +74,7 @@ class NimbusTests {
private fun createNimbus(
coenrollingFeatureIds: List<String> = listOf(),
recordedContext: RecordedContext? = null,
block: Nimbus.() -> Unit = {},
) = Nimbus(
context = context,
appInfo = appInfo,
Expand All @@ -80,7 +84,7 @@ class NimbusTests {
observer = null,
delegate = nimbusDelegate,
recordedContext = recordedContext,
)
).also(block)

@get:Rule
val gleanRule = GleanTestRule(context)
Expand Down Expand Up @@ -734,21 +738,34 @@ class NimbusTests {
assertEquals("Event count must match", isReadyEvents.count(), 3)
}

@Test
fun `Nimbus records context if it's passed in`() {
class TestRecordedContext : RecordedContext {
var recordCount = 0
class TestRecordedContext(
private val eventQueries: Map<String, String>? = null,
private var eventQueryValues: Map<String, Double>? = null,
) : RecordedContext {
var recorded = mutableListOf<JSONObject>()

override fun record() {
recordCount++
}
override fun getEventQueries(): Map<String, String> {
return eventQueries?.toMap() ?: mapOf()
}

override fun toJson(): JsonObject {
val contextToRecord = JSONObject()
contextToRecord.put("enabled", true)
return contextToRecord
}
override fun setEventQueryValues(eventQueryValues: Map<String, Double>) {
this.eventQueryValues = eventQueryValues
}

override fun record() {
recorded.add(this.toJson())
}

override fun toJson(): JsonObject {
val contextToRecord = JSONObject()
contextToRecord.put("enabled", true)
contextToRecord.put("events", JSONObject(eventQueryValues ?: mapOf<String, Double>()))
return contextToRecord
}
}

@Test
fun `Nimbus records context if it's passed in`() {
val context = TestRecordedContext()
val nimbus = createNimbus(recordedContext = context)

Expand All @@ -761,7 +778,42 @@ class NimbusTests {
job.join()
}

assertEquals(context.recordCount, 1)
assertEquals(context.recorded.size, 1)
}

@Test
fun `Nimbus recorded context event queries are run and the value is written back into the object`() {
val context = TestRecordedContext(
mapOf(
"TEST_QUERY" to "'event'|eventSum('Days', 1, 0)",
),
)
val nimbus = createNimbus(recordedContext = context) {
recordEvent("event")
}

suspend fun getString(): String {
return testExperimentsJsonString(appInfo, packageName)
}

val job = nimbus.applyLocalExperiments(::getString)
runBlocking {
job.join()
}

assertEquals(context.recorded.size, 1)
assertEquals(context.recorded[0].getJSONObject("events").getDouble("TEST_QUERY"), 1.0, 0.0)
}

@Test
fun `Nimbus recorded context event queries are validated`() {
val context = TestRecordedContext(
mapOf(
"FAILING_QUERY" to "'event'|eventSumThisWillFail('Days', 1, 0)",
),
)

assertThrows("Expected an error to be thrown", NimbusException::class.java, { validateEventQueries(context) })
}
}

Expand Down
11 changes: 11 additions & 0 deletions components/nimbus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub enum NimbusError {
CirrusError(#[from] CirrusClientError),
#[error("UniFFI callback error: {0}")]
UniFFICallbackError(#[from] uniffi::UnexpectedUniFFICallbackError),
#[cfg(feature = "stateful")]
#[error("Regex error: {0}")]
RegexError(#[from] regex::Error),
}

#[cfg(feature = "stateful")]
Expand All @@ -81,6 +84,14 @@ pub enum BehaviorError {
IntervalParseError(String),
#[error("The event store is not available on the targeting attributes")]
MissingEventStore,
#[error("The recorded context is not available on the nimbus client")]
MissingRecordedContext,
#[error("EventQueryTypeParseError: {0} is not a valid EventQueryType")]
EventQueryTypeParseError(String),
#[error(r#"EventQueryParseError: "{0}" is not a valid EventQuery"#)]
EventQueryParseError(String),
#[error(r#"TypeError: "{0}" is not of type {1}"#)]
TypeError(String, String),
}

#[cfg(not(feature = "stateful"))]
Expand Down
16 changes: 15 additions & 1 deletion components/nimbus/src/nimbus.udl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@ typedef extern RemoteSettingsConfig;
[ExternalExport="remote_settings"]
typedef extern RemoteSettingsServer;

namespace nimbus {};
namespace nimbus {

/// A test utility used to validate event queries against the jexl evaluator.
///
/// This method should only be used in tests.
[Throws=NimbusError]
void validate_event_queries(RecordedContext recorded_context);
};

dictionary AppContext {
string app_name;
string app_id;
Expand Down Expand Up @@ -104,6 +112,7 @@ enum NimbusError {
"InvalidPath", "InternalError", "NoSuchExperiment", "NoSuchBranch",
"DatabaseNotReady", "VersionParsingError", "BehaviorError", "TryFromIntError",
"ParseIntError", "TransformParameterError", "ClientError", "UniFFICallbackError",
"RegexError",
};

[Custom]
Expand All @@ -113,6 +122,10 @@ typedef string JsonObject;
interface RecordedContext {
JsonObject to_json();

record<string, string> get_event_queries();

void set_event_query_values(record<string, f64> event_query_values);

void record();
};

Expand Down Expand Up @@ -283,6 +296,7 @@ interface NimbusClient {

[Throws=NimbusError]
void dump_state_to_log();

};

interface NimbusTargetingHelper {
Expand Down
16 changes: 12 additions & 4 deletions components/nimbus/src/stateful/behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl PartialEq for Interval {
self.to_string() == other.to_string()
}
}

impl Eq for Interval {}

impl Hash for Interval {
Expand All @@ -84,7 +85,7 @@ impl FromStr for Interval {
_ => {
return Err(NimbusError::BehaviorError(
BehaviorError::IntervalParseError(input.to_string()),
))
));
}
})
}
Expand Down Expand Up @@ -365,7 +366,7 @@ impl EventQueryType {
return Err(NimbusError::TransformParameterError(format!(
"event transform {} requires a positive number as the second parameter",
self
)))
)));
}
} as usize;
let zero = &Value::from(0);
Expand All @@ -375,7 +376,7 @@ impl EventQueryType {
return Err(NimbusError::TransformParameterError(format!(
"event transform {} requires a positive number as the third parameter",
self
)))
)));
}
} as usize;

Expand All @@ -402,7 +403,7 @@ impl EventQueryType {
return Err(NimbusError::TransformParameterError(format!(
"event transform {} requires a positive number as the second parameter",
self
)))
)));
}
} as usize;

Expand All @@ -427,6 +428,13 @@ impl EventQueryType {
})
}

pub fn validate_query(maybe_query: &str) -> Result<bool> {
let regex = regex::Regex::new(
r#"^(?:"[^"']+"|'[^"']+')\|event(?:Sum|LastSeen|CountNonZero|Average|AveragePerNonZeroInterval)\(["'](?:Years|Months|Weeks|Days|Hours|Minutes)["'],\s*\d+\s*(?:,\s*\d+\s*)?\)$"#,
)?;
Ok(regex.is_match(maybe_query))
}

fn error_value(&self) -> f64 {
match self {
Self::LastSeen => f64::MAX,
Expand Down
Loading

0 comments on commit f277170

Please sign in to comment.