mozilla
/
application-services
зеркало из
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

adjustments for more PR feedback

This commit is contained in:
Charlie 2023-05-23 14:03:46 -05:00
Родитель 3186574632
Коммит a42631a0bf
6 изменённых файлов: 60 добавлений и 81 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

61
components/support/nimbus-fml/src/client.rs
Просмотреть файл

@ -13,15 +13,7 @@ use crate::{
parser::Parser,
util::loaders::FileLoader,
};
use serde::Deserialize;
use serde_json::Value;
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FeatureConfig {
pub feature_id: String,
pub value: serde_json::Value,
}
use std::collections::HashMap;
pub struct MergedJsonWithErrors {
pub json: String,
@ -71,26 +63,25 @@ impl FmlClient {
/// Validates a supplied feature configuration. Returns true or an FMLError.
pub fn is_feature_valid(&self, feature_id: String, value: JsonObject) -> Result<bool> {
self.manifest
.validate_feature_config(&feature_id, Value::Object(value))
.validate_feature_config(&feature_id, serde_json::Value::Object(value))
.map(|_| true)
}
/// Validates a supplied list of feature configurations. The valid configurations will be merged into the manifest's
/// default feature JSON, and invalid configurations will be returned as a list of their respective errors.
pub fn merge(&self, feature_configs: Vec<JsonObject>) -> Result<MergedJsonWithErrors> {
pub fn merge(
&self,
feature_configs: HashMap<String, JsonObject>,
) -> Result<MergedJsonWithErrors> {
let mut json = self.default_json.clone();
let mut errors: Vec<FMLError> = Default::default();
let configs: Vec<FeatureConfig> = feature_configs
.iter()
.map(|fc| serde_json::from_value(Value::Object(fc.to_owned())).unwrap())
.collect();
for feature_config in configs {
for (feature_id, value) in feature_configs {
match self
.manifest
.validate_feature_config(&feature_config.feature_id, feature_config.value)
.validate_feature_config(&feature_id, serde_json::Value::Object(value))
{
Ok(fd) => {
json.insert(feature_config.feature_id, fd.default_json());
json.insert(feature_id, fd.default_json());
}
Err(e) => errors.push(e),
};
@ -117,7 +108,7 @@ impl UniffiCustomTypeConverter for JsonObject {
let json: serde_json::Value = serde_json::from_str(&val)?;
match json.as_object() {
Some(obj) => Ok(obj.clone()),
Some(obj) => Ok(obj.to_owned()),
_ => Err(uniffi::deps::anyhow::anyhow!(
"Unexpected JSON-non-object in the bagging area"
)),
@ -220,28 +211,16 @@ mod unit_tests {
fn test_validate_and_merge_feature_configs() -> Result<()> {
let client: FmlClient = create_manifest().into();
let result = client.merge(vec![
Map::from_iter([
("featureId".to_string(), Value::String("feature".into())),
(
"value".to_string(),
Value::Object(Map::from_iter([(
"prop_1".to_string(),
Value::String("new value".to_string()),
)])),
),
]),
Map::from_iter([
("featureId".to_string(), Value::String("feature_i".into())),
(
"value".to_string(),
Value::Object(Map::from_iter([(
"prop_i_1".to_string(),
Value::Number(Number::from(1)),
)])),
),
]),
])?;
let result = client.merge(HashMap::from_iter([
(
"feature".to_string(),
Map::from_iter([("prop_1".to_string(), Value::String("new value".to_string()))]),
),
(
"feature_i".to_string(),
Map::from_iter([("prop_i_1".to_string(), Value::Number(Number::from(1)))]),
),
]))?;
assert_eq!(
serde_json::from_str::<Value>(&result.json)?,

2
components/support/nimbus-fml/src/fml.udl
Просмотреть файл

@ -31,7 +31,7 @@ interface FmlClient {
// Validates a supplied list of feature configurations. The valid configurations will be merged into the manifest's
// default feature JSON, and invalid configurations will be returned as a list of their respective errors.
[Throws=FMLError]
MergedJsonWithErrors merge(sequence<JsonObject> feature_configs);
MergedJsonWithErrors merge(record<DOMString, JsonObject> feature_configs);
// Returns the default feature JSON for the loaded FML's selected channel.
[Throws=FMLError]

28
components/support/nimbus-fml/src/intermediate_representation.rs
Просмотреть файл

@ -571,12 +571,12 @@ impl FeatureManifest {
self.obj_defs.iter()
}
pub fn iter_object_defs_deep(&self) -> impl Iterator<Item = (&FeatureManifest, &ObjectDef)> {
pub fn iter_all_object_defs(&self) -> impl Iterator<Item = (&FeatureManifest, &ObjectDef)> {
let objects = self.obj_defs.iter().map(move |o| (self, o));
let imported_objects: Vec<(&FeatureManifest, &ObjectDef)> = self
.all_imports
.iter()
.flat_map(|(_, fm)| fm.iter_object_defs_deep())
.flat_map(|(_, fm)| fm.iter_all_object_defs())
.collect();
objects.chain(imported_objects)
}
@ -585,12 +585,12 @@ impl FeatureManifest {
self.feature_defs.iter()
}
pub fn iter_feature_defs_deep(&self) -> impl Iterator<Item = (&FeatureManifest, &FeatureDef)> {
pub fn iter_all_feature_defs(&self) -> impl Iterator<Item = (&FeatureManifest, &FeatureDef)> {
let features = self.feature_defs.iter().map(move |f| (self, f));
let imported_features: Vec<(&FeatureManifest, &FeatureDef)> = self
.all_imports
.iter()
.flat_map(|(_, fm)| fm.iter_feature_defs_deep())
.flat_map(|(_, fm)| fm.iter_all_feature_defs())
.collect();
features.chain(imported_features)
}
@ -616,12 +616,12 @@ impl FeatureManifest {
self.iter_enum_defs().find(|e| e.name() == nm)
}
pub fn find_feature(&self, nm: &str) -> Option<&FeatureDef> {
pub fn get_feature(&self, nm: &str) -> Option<&FeatureDef> {
self.iter_feature_defs().find(|f| f.name() == nm)
}
pub fn find_feature_deep(&self, nm: &str) -> Option<(&FeatureManifest, &FeatureDef)> {
self.iter_feature_defs_deep().find(|(_, f)| f.name() == nm)
pub fn find_feature(&self, nm: &str) -> Option<(&FeatureManifest, &FeatureDef)> {
self.iter_all_feature_defs().find(|(_, f)| f.name() == nm)
}
pub fn find_import(&self, id: &ModuleId) -> Option<&FeatureManifest> {
@ -630,7 +630,7 @@ impl FeatureManifest {
pub fn default_json(&self) -> Value {
Value::Object(
self.iter_feature_defs_deep()
self.iter_all_feature_defs()
.map(|(_, f)| (f.name(), f.default_json()))
.collect(),
)
@ -651,7 +651,7 @@ impl FeatureManifest {
) -> Result<FeatureDef> {
let dummy_channel = "dummy".to_string();
let objects: HashMap<String, &ObjectDef> = self
.iter_object_defs_deep()
.iter_all_object_defs()
.map(|(_, o)| (o.name(), o))
.collect();
let merger = DefaultsMerger::new(
@ -660,7 +660,7 @@ impl FeatureManifest {
dummy_channel,
);
if let Some((manifest, feature_def)) = self.find_feature_deep(feature_name) {
if let Some((manifest, feature_def)) = self.find_feature(feature_name) {
let mut feature_def = feature_def.clone();
merger.merge_feature_defaults(&mut feature_def, &Some(vec![feature_value.into()]))?;
manifest.validate_feature_def(&feature_def)?;
@ -867,7 +867,7 @@ impl<'a> ImportedModule<'a> {
let fm = self.fm;
self.features
.iter()
.filter_map(|f| fm.find_feature(f))
.filter_map(|f| fm.get_feature(f))
.collect()
}
}
@ -1732,7 +1732,7 @@ pub mod unit_tests {
HashMap::from([(ModuleId::Local("test".into()), fm_i)]),
);
let names: Vec<String> = fm.iter_object_defs_deep().map(|(_, o)| o.name()).collect();
let names: Vec<String> = fm.iter_all_object_defs().map(|(_, o)| o.name()).collect();
assert_eq!(names[0], "SampleObj".to_string());
assert_eq!(names[1], "SampleObjImported".to_string());
@ -1764,7 +1764,7 @@ pub mod unit_tests {
);
let names: Vec<String> = fm
.iter_feature_defs_deep()
.iter_all_feature_defs()
.map(|(_, f)| f.props[0].name())
.collect();
@ -1796,7 +1796,7 @@ pub mod unit_tests {
HashMap::from([(ModuleId::Local("test".into()), fm_i)]),
);
let feature = fm.find_feature_deep("feature_i");
let feature = fm.find_feature("feature_i");
assert!(feature.is_some());

1
components/support/nimbus-fml/src/parser.rs
Просмотреть файл

@ -402,7 +402,6 @@ impl<'object> DefaultsMerger<'object> {
supported_channels: Vec<String>,
channel: String,
) -> Self {
#[allow(deprecated)]
Self {
objects,
supported_channels,

30
megazords/cirrus/tests/python-tests/test_cirrus_fml_integration.py
Просмотреть файл

@ -4,13 +4,15 @@
import json
# 1. create request
# 2. enroll, convert response to dict from JSON
# 3. map enrolledFeatureConfigMap values to the feature values
# 4. create FmlClient
# 5. merge feature configs into default JSON and validate their values against the manifest
# 6. load JSON response as dict
def test_enroll_and_get_enrolled_feature_json_control(fml_client, cirrus_client):
"""
1. create request
2. enroll, convert response to dict from JSON
3. map enrolledFeatureConfigMap values to the feature values
4. create FmlClient
5. merge feature configs into default JSON and validate their values against the manifest
6. load JSON response as dict
"""
req = json.dumps(
{
"clientId": "jeddai",
@ -18,9 +20,10 @@ def test_enroll_and_get_enrolled_feature_json_control(fml_client, cirrus_client)
}
)
res = json.loads(cirrus_client.handle_enrollment(req))
feature_configs = [
value["feature"] for value in res["enrolledFeatureConfigMap"].values()
]
feature_configs = {
key: value["feature"]["value"]
for key, value in res["enrolledFeatureConfigMap"].items()
}
assert (
res["enrolledFeatureConfigMap"]["imported-module-1-included-feature-1"]["slug"]
@ -41,8 +44,8 @@ def test_enroll_and_get_enrolled_feature_json_control(fml_client, cirrus_client)
assert len(merged_res.errors) == 0
# repeat the above but with a different client/username on the request
def test_enroll_and_get_enrolled_feature_json_treatment(fml_client, cirrus_client):
# repeat the above test but with a different client/username on the request
req = json.dumps(
{
"clientId": "test",
@ -50,9 +53,10 @@ def test_enroll_and_get_enrolled_feature_json_treatment(fml_client, cirrus_clien
}
)
res = json.loads(cirrus_client.handle_enrollment(req))
feature_configs = [
value["feature"] for value in res["enrolledFeatureConfigMap"].values()
]
feature_configs = {
key: value["feature"]["value"]
for key, value in res["enrolledFeatureConfigMap"].items()
}
assert (
res["enrolledFeatureConfigMap"]["imported-module-1-included-feature-1"]["slug"]

19
megazords/cirrus/tests/python-tests/test_fml.py
Просмотреть файл

@ -53,7 +53,7 @@ def test_is_feature_valid_is_invalid(fml_client):
def test_merge(fml_client):
client = fml_client("test.fml.yml", "developer")
configs = [{"featureId": "example-feature", "value": {"enabled": True}}]
configs = {"example-feature": {"enabled": True}}
result = client.merge(configs)
assert len(result.errors) == 0
@ -65,7 +65,7 @@ def test_merge(fml_client):
@pytest.mark.skip(reason="This functionality is hindered by EXP-3503")
def test_merge_error_on_invalid_key(fml_client):
client = fml_client("test.fml.yml", "developer")
configs = [{"featureId": "example-feature", "value": {"enabled1": False}}]
configs = {"example-feature": {"enabled1": False}}
result = client.merge(configs)
assert len(result.errors) == 1
@ -74,7 +74,7 @@ def test_merge_error_on_invalid_key(fml_client):
def test_merge_error_on_invalid_value(fml_client):
client = fml_client("test.fml.yml", "developer")
configs = [{"featureId": "example-feature", "value": {"enabled": "false"}}]
configs = {"example-feature": {"enabled": "false"}}
result = client.merge(configs)
assert len(result.errors) == 1
@ -86,14 +86,11 @@ def test_merge_included_and_imported_features(fml_client):
"test-include-import.fml.yml",
"developer",
)
configs = [
{"featureId": "example-feature", "value": {"enabled": True}},
{"featureId": "included-feature-1", "value": {"enabled": True}},
{
"featureId": "imported-module-1-included-feature-1",
"value": {"enabled": True},
},
]
configs = {
"example-feature": {"enabled": True},
"included-feature-1": {"enabled": True},
"imported-module-1-included-feature-1": {"enabled": True},
}
result = client.merge(configs)
assert len(result.errors) == 0