diff --git a/crates/forms/src/af/calc.rs b/crates/forms/src/af/calc.rs index 1c3724ef4a..66738246c2 100644 --- a/crates/forms/src/af/calc.rs +++ b/crates/forms/src/af/calc.rs @@ -612,6 +612,9 @@ pub(super) fn eval_notation(src: &str, value_of: &dyn Fn(&str) -> f64) -> Option (i == toks.len() && v.is_finite()).then_some(v) } +/// Most field values one `AFSimple_Calculate` list may expand to (a form has some thousands). +const MAX_EXPANDED_VALUES: usize = 1 << 20; + /// The Calculate event: the new value, given the current values of the other fields /// (`values(name)` returns every field named `name` or below it, for groups). pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option { @@ -623,20 +626,21 @@ pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option< // doesn't count it as 1. (The form's `values` leaves out a field that has no value.) Calculate::Simple { op: CalcOp::Product, fields } if fields.iter().all(|f| values(f).is_empty()) => Some("0".into()), Calculate::Simple { op, fields } => { - let xs: Vec = fields.iter().flat_map(|f| values(f)).map(|v| num(&v)).collect(); + // The values are folded as they are read, never collected: a group name listed many + // times would otherwise be held once per listing. A list that expands past + // MAX_EXPANDED_VALUES is not calculated. + let mut n = 0usize; + let xs = fields.iter().flat_map(|f| values(f)).map(|v| num(&v)).inspect(|_| n += 1).take(MAX_EXPANDED_VALUES + 1); let v = match op { - CalcOp::Sum => xs.iter().sum(), - CalcOp::Product => xs.iter().product(), - CalcOp::Average => { - if xs.is_empty() { - 0.0 - } else { - xs.iter().sum::() / xs.len() as f64 - } - } - CalcOp::Minimum => xs.iter().copied().fold(f64::INFINITY, f64::min), - CalcOp::Maximum => xs.iter().copied().fold(f64::NEG_INFINITY, f64::max), + CalcOp::Sum | CalcOp::Average => xs.sum(), + CalcOp::Product => xs.product(), + CalcOp::Minimum => xs.fold(f64::INFINITY, f64::min), + CalcOp::Maximum => xs.fold(f64::NEG_INFINITY, f64::max), }; + if n > MAX_EXPANDED_VALUES { + return None; + } + let v = if *op == CalcOp::Average { if n == 0 { 0.0 } else { v / n as f64 } } else { v }; Some(if v.is_finite() { js_number(v) } else { "0".into() }) } Calculate::Notation(expr) => { diff --git a/crates/forms/src/af/tests.rs b/crates/forms/src/af/tests.rs index 3c6dab1bef..b4b6452a3c 100644 --- a/crates/forms/src/af/tests.rs +++ b/crates/forms/src/af/tests.rs @@ -486,3 +486,23 @@ fn destructured_for_of_targets_keep_regex_iterables_opaque() { } } } + +#[test] +fn simple_calculations_fold_a_repeated_group_without_collecting_it() { + let group = |_: &str| vec!["2".to_string(); 3]; + let list = |op, n: usize| Calculate::Simple { op, fields: vec!["g".to_string(); n] }; + assert_eq!(calculate(&list(CalcOp::Sum, 4), &group).as_deref(), Some("24")); + assert_eq!(calculate(&list(CalcOp::Average, 4), &group).as_deref(), Some("2")); + assert_eq!(calculate(&list(CalcOp::Product, 2), &group).as_deref(), Some("64")); + assert_eq!(calculate(&list(CalcOp::Maximum, 2), &group).as_deref(), Some("2")); + assert_eq!(calculate(&list(CalcOp::Sum, 0), &group).as_deref(), Some("0")); +} + +#[test] +fn a_simple_calculation_that_expands_past_the_allowance_is_not_calculated() { + let thousand = |_: &str| vec!["1".to_string(); 1000]; + let within = Calculate::Simple { op: CalcOp::Sum, fields: vec!["g".to_string(); 1000] }; + let past = Calculate::Simple { op: CalcOp::Sum, fields: vec!["g".to_string(); 2000] }; + assert_eq!(calculate(&within, &thousand).as_deref(), Some("1000000")); + assert_eq!(calculate(&past, &thousand), None); +}