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 Aug 5, 2024
1 parent 8fd08c6 commit e000892
Show file tree
Hide file tree
Showing 18 changed files with 509 additions and 77 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# v130.0 (In progress)

## ⚠️ 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.

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 @@ -19,6 +19,7 @@ import mozilla.telemetry.glean.config.Configuration
import mozilla.telemetry.glean.net.HttpStatus
import mozilla.telemetry.glean.net.PingUploader
import mozilla.telemetry.glean.testing.GleanTestRule
import org.json.JSONException
import org.json.JSONObject
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
Expand Down Expand Up @@ -71,6 +72,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 +82,7 @@ class NimbusTests {
observer = null,
delegate = nimbusDelegate,
recordedContext = recordedContext,
)
).also(block)

@get:Rule
val gleanRule = GleanTestRule(context)
Expand Down Expand Up @@ -734,21 +736,49 @@ 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 var eventQueries: MutableMap<String, Any>? = null,
) : RecordedContext {
var recorded = mutableListOf<JSONObject>()

override fun getEventQueries(): JsonObject {
val queriesJson = JSONObject()
for ((key, value) in eventQueries ?: mapOf()) {
queriesJson.put(key, value)
}
return queriesJson
}

override fun record() {
recordCount++
override fun setEventQueryValues(json: JsonObject) {
for (key in json.keys()) {
try {
eventQueries?.put(key, json.getDouble(key))
} catch (exception: JSONException) {
continue
}
}
}

override fun toJson(): JsonObject {
val contextToRecord = JSONObject()
contextToRecord.put("enabled", true)
return contextToRecord
override fun record() {
recorded.add(this.toJson())
}

override fun toJson(): JsonObject {
val contextToRecord = JSONObject()
contextToRecord.put("enabled", true)
val queries = this.getEventQueries()
for (key in queries.keys()) {
if (queries.get(key)::class == String::class) {
queries.remove(key)
}
}
contextToRecord.put("events", queries)
return contextToRecord
}
}

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

Expand All @@ -761,7 +791,31 @@ 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(
mutableMapOf(
"TEST_QUERY" to "'event'|eventSum('Days', 1, 0)",
),
)
val nimbus = createNimbus(recordedContext = context) {
this.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)
}
}

Expand Down
9 changes: 9 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,12 @@ pub enum BehaviorError {
IntervalParseError(String),
#[error("The event store is not available on the targeting attributes")]
MissingEventStore,
#[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
5 changes: 5 additions & 0 deletions components/nimbus/src/nimbus.udl
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ enum NimbusError {
"InvalidPath", "InternalError", "NoSuchExperiment", "NoSuchBranch",
"DatabaseNotReady", "VersionParsingError", "BehaviorError", "TryFromIntError",
"ParseIntError", "TransformParameterError", "ClientError", "UniFFICallbackError",
"RegexError",
};

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

JsonObject get_event_queries();

void set_event_query_values(JsonObject json);

void record();
};

Expand Down
17 changes: 13 additions & 4 deletions components/nimbus/src/stateful/behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::{
stateful::persistence::{Database, StoreId},
};
use chrono::{DateTime, Datelike, Duration, TimeZone, Utc};
use regex::Regex;
use serde::{Deserialize, Serialize};
use serde_json::{json, Value};
use std::collections::vec_deque::Iter;
Expand Down Expand Up @@ -62,6 +63,7 @@ impl PartialEq for Interval {
self.to_string() == other.to_string()
}
}

impl Eq for Interval {}

impl Hash for Interval {
Expand All @@ -84,7 +86,7 @@ impl FromStr for Interval {
_ => {
return Err(NimbusError::BehaviorError(
BehaviorError::IntervalParseError(input.to_string()),
))
));
}
})
}
Expand Down Expand Up @@ -365,7 +367,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 +377,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 +404,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 +429,13 @@ impl EventQueryType {
})
}

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

fn error_value(&self) -> f64 {
match self {
Self::LastSeen => f64::MAX,
Expand Down
5 changes: 2 additions & 3 deletions components/nimbus/src/stateful/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::{
evaluator::split_locale,
json::JsonObject,
stateful::matcher::AppContext,
targeting::RecordedContext,
};
use chrono::{DateTime, Utc};
use serde_derive::*;
Expand Down Expand Up @@ -50,8 +49,8 @@ impl From<AppContext> for TargetingAttributes {
}

impl TargetingAttributes {
pub(crate) fn set_recorded_context(&mut self, recorded_context: &dyn RecordedContext) {
self.recorded_context = Some(recorded_context.to_json());
pub(crate) fn set_recorded_context(&mut self, recorded_context: JsonObject) {
self.recorded_context = Some(recorded_context);
}

pub(crate) fn update_time_to_now(
Expand Down
Loading

0 comments on commit e000892

Please sign in to comment.