Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion js/optify-config/src/preferences.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ impl JsGetOptionsPreferences {

#[napi]
pub fn set_overrides_json(&mut self, overrides_json: Option<String>) {
self.inner.overrides_json = overrides_json;
self.inner.overrides =
overrides_json.map(|s| serde_json::from_str(&s).expect("overrides should be valid JSON"));
}

#[napi]
Expand Down
11 changes: 8 additions & 3 deletions ruby/optify/ext/optify_ruby/src/preferences.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,20 @@ impl MutGetOptionsPreferences {

// Overrides Section
pub fn has_overrides(&self) -> bool {
self.0.borrow().overrides_json.is_some()
self.0.borrow().overrides.is_some()
}

pub fn set_overrides_json(&self, overrides: Option<String>) {
self.0.borrow_mut().overrides_json = overrides;
self.0.borrow_mut().overrides =
overrides.map(|s| serde_json::from_str(&s).expect("overrides should be valid JSON"));
}

pub fn get_overrides_json(&self) -> Option<String> {
self.0.borrow().overrides_json.clone()
self.0
.borrow()
.overrides
.as_ref()
.map(|o| serde_json::to_string(&o).unwrap())
}

// Skip Feature Name Conversion Section
Expand Down
3 changes: 3 additions & 0 deletions rust/optify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ Run:
```shell
cargo build --release
cargo bench

# Open one of the reports
open target/criterion/*/report/index.html
```

## Publishing
Expand Down
23 changes: 10 additions & 13 deletions rust/optify/benches/get_options_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ fn benchmark_with_overrides(c: &mut Criterion) {
// With small override
group.bench_function("small_override", |b| {
let mut preferences = GetOptionsPreferences::new();
preferences.overrides_json = Some(r#"{"config1": {"extra": "value"}}"#.to_string());
preferences.overrides = Some(serde_json::json!({"config1": {"extra": "value"}}));

b.iter(|| {
provider
Expand All @@ -232,20 +232,17 @@ fn benchmark_with_overrides(c: &mut Criterion) {
// With larger override
group.bench_function("larger_override", |b| {
let mut preferences = GetOptionsPreferences::new();
preferences.overrides_json = Some(
r#"{
"config1": {
"level1": {
"level2": {
"key1": "overridden value 1",
"key2": "overridden value 2",
"newKey": "brand new value"
}
preferences.overrides = Some(serde_json::json!({
"config1": {
"level1": {
"level2": {
"key1": "overridden value 1",
"key2": "overridden value 2",
"newKey": "brand new value"
}
}
}"#
.to_string(),
);
}
}));

b.iter(|| {
provider
Expand Down
21 changes: 4 additions & 17 deletions rust/optify/src/provider/get_options_preferences.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,19 @@
use crate::provider::constraints::Constraints;
use crate::provider::{constraints::Constraints, SourceValue};

#[derive(Hash, PartialEq, Eq)]
#[derive(Clone, Hash, PartialEq, Eq)]
pub struct GetOptionsPreferences {
/// Allows resolving configurable strings.
/// Defaults to false: no configurable strings will be resolved.
/// Configurable strings must have been enabled when the options were built to have them resolved at runtime.
pub are_configurable_strings_enabled: bool,
pub constraints: Option<Constraints>,
/// Overrides to apply after the built configuration.
/// A string is used because it makes it easier to pass to the `config` library, but this may change in the future.
/// It also makes it simpler and maybe faster to get from other programming languages.
pub overrides_json: Option<String>,
pub overrides: Option<SourceValue>,
/// Determines if the feature names should be converted to canonical feature names.
/// Defaults to false: given features names will be converted to canonical feature names before looking for features or options.
pub skip_feature_name_conversion: bool,
}

impl Clone for GetOptionsPreferences {
fn clone(&self) -> Self {
Self {
are_configurable_strings_enabled: self.are_configurable_strings_enabled,
constraints: self.constraints.clone(),
overrides_json: self.overrides_json.clone(),
skip_feature_name_conversion: self.skip_feature_name_conversion,
}
}
}

impl Default for GetOptionsPreferences {
fn default() -> Self {
Self::new()
Expand All @@ -38,7 +25,7 @@ impl GetOptionsPreferences {
Self {
are_configurable_strings_enabled: false,
constraints: None,
overrides_json: None,
overrides: None,
skip_feature_name_conversion: false,
}
}
Expand Down
123 changes: 90 additions & 33 deletions rust/optify/src/provider/provider_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,41 @@ impl OptionsProvider {
}
};

let mut result = serde_json::Value::Object(serde_json::Map::new());

for canonical_feature_name in feature_names {
let source = match self.sources.get(canonical_feature_name) {
Some(src) => src,
let mut result = if feature_names.len() == 1 {
// Avoid merging to an empty object.
let feature_name = &feature_names[0];
match self.sources.get(feature_name) {
Some(src) => src.clone(),
None => {
// Should not happen.
// All canonical feature names are included as keys in the sources map.
// It could happen in the future if we allow aliases to be added directly, but we should try to validate them when the provider is built.
return Err(format!(
"Feature name {canonical_feature_name:?} is not a known feature."
"Feature name {feature_name:?} is not a known feature."
));
}
};
merge_json_value(&mut result, source);
}
}
} else {
let mut result = serde_json::Value::Object(serde_json::Map::new());

for canonical_feature_name in feature_names {
let source = match self.sources.get(canonical_feature_name) {
Some(src) => src,
None => {
// Should not happen.
// All canonical feature names are included as keys in the sources map.
// It could happen in the future if we allow aliases to be added directly, but we should try to validate them when the provider is built.
return Err(format!(
"Feature name {canonical_feature_name:?} is not a known feature."
));
}
};
merge_json_value(&mut result, source);
}

result
};

if let Some(preferences) = preferences {
if let Some(overrides) = &preferences.overrides_json {
let overrides_value: serde_json::Value = serde_json::from_str(overrides)
.map_err(|e| format!("Failed to parse overrides JSON: {e}"))?;
merge_json_value(&mut result, &overrides_value);
if let Some(overrides) = &preferences.overrides {
merge_json_value(&mut result, overrides);
}
}

Expand All @@ -112,13 +125,71 @@ impl OptionsProvider {
Ok(result)
}

/// Get options for a specific key by extracting and merging only that key from each source.
/// This avoids building the entire config when only one key is needed.
fn get_options_for_key(
&self,
key: &str,
filtered_feature_names: &[String],
original_feature_names: &[impl AsRef<str>],
preferences: Option<&GetOptionsPreferences>,
) -> Result<serde_json::Value, String> {
let mut result = if filtered_feature_names.len() == 1 {
// Avoid merging to an empty object.
let feature_name = &filtered_feature_names[0];
let source = self
.sources
.get(feature_name)
.ok_or_else(|| format!("Feature name {feature_name:?} is not a known feature."))?;
source.get(key).cloned()
} else {
let mut result = None;
for canonical_feature_name in filtered_feature_names {
let source = self.sources.get(canonical_feature_name).ok_or_else(|| {
format!("Feature name {canonical_feature_name:?} is not a known feature.")
})?;

if let Some(source_value) = source.get(key) {
match &mut result {
Some(existing) => merge_json_value(existing, source_value),
None => result = Some(source_value.clone()),
}
}
}
result
};

// Apply overrides for this specific key.
if let Some(preferences) = preferences {
if let Some(overrides) = &preferences.overrides {
if let Some(override_for_key) = overrides.get(key) {
match &mut result {
Some(existing) => merge_json_value(existing, override_for_key),
None => result = Some(override_for_key.clone()),
}
}
}
}

result.ok_or_else(|| {
format!(
"Error getting options with features {:?}: configuration property \"{}\" not found",
original_feature_names
.iter()
.map(|f| f.as_ref())
.collect::<Vec<&str>>(),
key
)
})
}

fn get_entire_config_from_cache(
&self,
feature_names: &[String],
preferences: Option<&GetOptionsPreferences>,
) -> Result<Option<serde_json::Value>, String> {
if let Some(preferences) = preferences {
if preferences.overrides_json.is_some() {
if preferences.overrides.is_some() {
return Err("Caching when overrides are given is not supported.".to_owned());
}
}
Expand Down Expand Up @@ -386,22 +457,8 @@ impl OptionsRegistry for OptionsProvider {
}

let filtered_feature_names = self.get_filtered_feature_names(feature_names, preferences)?;
// TODO Optimization: Don't build entire config, just get what is needed from each one.
// It might be tricky to use the overrides, but it should be much faster overall.
// It was done this way to just get something working adequately
// because it was assumed that each language can optimize
// by caching results including conversion to immutable types.
let config = self.get_entire_config(&filtered_feature_names, cache_options, preferences)?;
let mut value = match config.get(key) {
Some(v) => v.clone(),
None => {
return Err(format!(
"Error getting options with features {:?}: configuration property \"{}\" not found",
feature_names.iter().map(|f| f.as_ref()).collect::<Vec<_>>(),
key
))
}
};
let mut value =
self.get_options_for_key(key, &filtered_feature_names, feature_names, preferences)?;

self.process_configurable_strings(&mut value, Some(key), preferences)?;
if cache_options.is_some() {
Expand Down
2 changes: 1 addition & 1 deletion rust/optify/tests/test_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn test_cache_with_overrides() -> Result<(), Box<dyn std::error::Error>> {
let cache_options = CacheOptions {};

let mut preferences = GetOptionsPreferences::new();
preferences.overrides_json = Some(r#"{"test": "override"}"#.to_string());
preferences.overrides = Some(serde_json::json!({"test": "override"}));

let result = provider.get_all_options(&["a"], Some(&cache_options), Some(&preferences));

Expand Down
86 changes: 61 additions & 25 deletions rust/optify/tests/test_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,62 @@ fn test_provider_get_features_and_aliases() -> Result<(), Box<dyn std::error::Er
Ok(())
}

#[test]
fn test_provider_get_options_missing_key() -> Result<(), Box<dyn std::error::Error>> {
let key = "does not exist";
let feature_names = vec!["a"];
let provider = get_provider();
let opts = provider.get_options(key, &feature_names);
assert!(opts.is_err());
assert_eq!(opts.unwrap_err(), "Error getting options with features [\"a\"]: configuration property \"does not exist\" not found");

let mut preferences = GetOptionsPreferences::new();
preferences.overrides = Some(serde_json::json!({
"does not exist": 42
}));
let opts = provider.get_options_with_preferences(key, &feature_names, None, Some(&preferences));
let value = opts.expect("should be able to get options");
assert_eq!(value, serde_json::json!(42));

Ok(())
}

#[test]
fn test_provider_get_options_no_features() -> Result<(), Box<dyn std::error::Error>> {
let key = "wtv";
let provider = get_provider();
let feature_names: Vec<&str> = vec![];
let opts = provider.get_options(key, &feature_names);
assert!(opts.is_err());
assert_eq!(
opts.unwrap_err(),
"Error getting options with features []: configuration property \"wtv\" not found"
);

let mut preferences = GetOptionsPreferences::new();
preferences.overrides = Some(serde_json::json!({
key: 42
}));
let opts = provider.get_options_with_preferences(key, &feature_names, None, Some(&preferences));
let value = opts.expect("should be able to get options");
assert_eq!(value, serde_json::json!(42));
Ok(())
}

#[test]
fn test_provider_get_options_with_overrides() -> Result<(), Box<dyn std::error::Error>> {
let provider = get_provider();
let mut preferences = GetOptionsPreferences::new();
preferences.overrides_json = Some(
serde_json::json!({
"myConfig": {
"new key": 33,
"rootString": "new string",
"myObject": {
"one": 1321,
"something new for test_provider_get_options_with_overrides": "hello"
}
preferences.overrides = Some(serde_json::json!({
"myConfig": {
"new key": 33,
"rootString": "new string",
"myObject": {
"one": 1321,
"something new for test_provider_get_options_with_overrides": "hello"
}
})
.to_string(),
);
}
}));
let opts = provider.get_options_with_preferences("myConfig", &["a"], None, Some(&preferences));

let expected = serde_json::json!({
Expand Down Expand Up @@ -263,20 +302,17 @@ fn test_configurable_values_get_all_options_with_overrides(
let provider = get_configurable_values_provider();
let mut preferences = GetOptionsPreferences::new();
preferences.are_configurable_strings_enabled = true;
preferences.overrides_json = Some(
serde_json::json!({
"message": {
"$type": "Optify.ConfigurableString",
"base": {
"liquid": "Hello {{ name }}!"
},
"arguments": {
"name": "from the test"
}
preferences.overrides = Some(serde_json::json!({
"message": {
"$type": "Optify.ConfigurableString",
"base": {
"liquid": "Hello {{ name }}!"
},
"arguments": {
"name": "from the test"
}
})
.to_string(),
);
}
}));

let features: Vec<&str> = vec![];
let opts = provider.get_all_options(&features, None, Some(&preferences))?;
Expand Down
Loading