From 074adc18a9323f7fe886c3a2626be37af3a950db Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:04 +0100 Subject: [PATCH 01/23] Add field limits to the registry: max_length, min_length, min, max and the install gate (D90) A FieldDef may carry limits: max_length and min_length (Unicode code points, not bytes) on text and longtext, min and max (inclusive, exact) on number, money and decimal. A text field with no max_length has the default of its kind (2,048 and 100,000) and a declared one may not exceed the cap (10,000 and 1,000,000); the four numbers are named constants in limits.rs and are proposals for the product owner. The D41 gate refuses limits that cannot hold (0, above the cap, min above max, wrong kind, a bound that does not fit the decimal's precision and scale). Co-Authored-By: Claude Sonnet 5.5 --- crates/kernel/obop-registry/src/lib.rs | 17 + crates/kernel/obop-registry/src/limits.rs | 369 ++++++++++++ .../kernel/obop-registry/src/limits_tests.rs | 543 ++++++++++++++++++ 3 files changed, 929 insertions(+) create mode 100644 crates/kernel/obop-registry/src/limits.rs create mode 100644 crates/kernel/obop-registry/src/limits_tests.rs diff --git a/crates/kernel/obop-registry/src/lib.rs b/crates/kernel/obop-registry/src/lib.rs index 391d51a5..3779d42d 100644 --- a/crates/kernel/obop-registry/src/lib.rs +++ b/crates/kernel/obop-registry/src/lib.rs @@ -9,12 +9,17 @@ use std::sync::{Arc, Mutex, OnceLock}; pub mod expr; pub mod ext; pub mod labels; +pub mod limits; pub mod rules; pub mod template; pub use ext::{ CondOp, Exit, Guard, History, IndexCond, IndexDef, IndexKind, LockDate, ModelExt, StateSet, }; pub use labels::Translations; +pub use limits::{ + Bound, Breach, DEFAULT_LONGTEXT_MAX, DEFAULT_TEXT_MAX, LONGTEXT_MAX_CAP, Limits, TEXT_MAX_CAP, + char_count, limits_note, +}; pub use rules::{ Access, Assertion, Computed, Effect, EffectKind, Lit, Locator, Reset, SeqFormat, SequenceSpec, ShowsOption, ShowsWhen, Src, Target, Unique, @@ -254,6 +259,9 @@ pub struct FieldDef { /// A text field whose value is shown with the label of the option it equals in a select /// field (D52 amendment). Display only. pub shows_option: Option>, + /// Length and range limits (D90); `None`: none declared (a text field still has the default + /// length, see [`FieldDef::effective_max_length`]). + pub limits: Option>, } #[derive(Debug, Clone)] @@ -346,6 +354,7 @@ impl FieldDef { unique: None, sequence: None, shows_option: None, + limits: None, } } @@ -859,6 +868,10 @@ fn check_basics(m: &Module, reserved: bool) -> Vec { { bad(fp.clone(), &problem); } + // Length and range limits (D90), also for a package that is registered again. + for problem in limits::problems(f) { + bad(fp.clone(), &problem); + } match &f.kind { FieldKind::Select(o) if o.is_empty() => { bad(fp.clone(), "select field has no options") @@ -990,6 +1003,9 @@ fn check_history_setting(model: &ModelDef, mp: &str, bad: &mut impl FnMut(String } } +#[cfg(test)] +mod limits_tests; + #[cfg(test)] mod intern_tests { #[test] @@ -1018,6 +1034,7 @@ mod tests { unique: None, sequence: None, shows_option: None, + limits: None, } } diff --git a/crates/kernel/obop-registry/src/limits.rs b/crates/kernel/obop-registry/src/limits.rs new file mode 100644 index 00000000..4c63ee93 --- /dev/null +++ b/crates/kernel/obop-registry/src/limits.rs @@ -0,0 +1,369 @@ +//! Length and range limits of fields (D90): `max_length` and `min_length` on `text` and +//! `longtext`, `min` and `max` on `number`, `money` and `decimal`. +//! +//! The limits are metadata of the field, so every surface (REST, MCP, CLI, the back office, the +//! OpenAPI document, `GET /api/meta`) reads them from here and the command layer checks them on +//! every write that checks field kinds. They are checked on the value that is written; a value +//! that is already stored is never refused for being outside a limit that was set later (D90 +//! Decision 6). +//! +//! # Lengths count Unicode scalar values +//! +//! A length is the number of code points (`str::chars`), not bytes: `å`, `ä`, `ö` and an emoji +//! such as U+1F600 count 1 each. A letter written with a combining mark and an emoji built from +//! several code points (a flag, a family) count as many as they have; the limit is not about what +//! a person sees as one character. Nothing is trimmed or normalised before counting. +//! +//! # The default and the cap +//! +//! A `text` or `longtext` field that declares no `max_length` has the default of its kind +//! ([`DEFAULT_TEXT_MAX`], [`DEFAULT_LONGTEXT_MAX`]); a field may declare its own up to the hard +//! cap of its kind ([`TEXT_MAX_CAP`], [`LONGTEXT_MAX_CAP`]). All four are named here and nowhere +//! else. + +use crate::{FieldDef, FieldKind}; +use obop_values::Decimal; +use std::cmp::Ordering; +use std::fmt; +use std::sync::Arc; + +/// The longest value, in characters, a `text` field accepts when it declares no `max_length`. +/// +/// 2,048 is the length that browsers, sitemaps and most services accept for a URL, the longest +/// ordinary single-line value; names, titles, addresses and codes are far shorter. A field that +/// holds a list in one line (a comma-separated list of people) declares its own `max_length`. +pub const DEFAULT_TEXT_MAX: u32 = 2_048; + +/// The longest value, in characters, a `longtext` field accepts when it declares no +/// `max_length`. +/// +/// 100,000 characters is about fifty printed pages, more than GitHub allows for an issue +/// (65,536) and about the size at which Gmail clips an HTML mail (102,400 bytes). +pub const DEFAULT_LONGTEXT_MAX: u32 = 100_000; + +/// The largest `max_length` a `text` field may declare. +pub const TEXT_MAX_CAP: u32 = 10_000; + +/// The largest `max_length` a `longtext` field may declare. The request body limit of the HTTP +/// server (2 MiB) still applies first, so text of three or four bytes a character reaches the +/// body limit before it reaches this number. +pub const LONGTEXT_MAX_CAP: u32 = 1_000_000; + +/// A bound of `min` or `max`: a whole number for `number` and `money`, an exact decimal (at the +/// scale of the field) for `decimal`. Never a float. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Bound { + Int(i64), + Dec(Decimal), +} + +impl fmt::Display for Bound { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Int(n) => write!(f, "{n}"), + Self::Dec(d) => write!(f, "{d}"), + } + } +} + +/// The limits a field declares (D90). Every part is optional; a field without any has none +/// (apart from the default length of a text field). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct Limits { + /// `text` and `longtext`: the fewest characters a written value has. + pub min_length: Option, + /// `text` and `longtext`: the most characters a written value has. + pub max_length: Option, + /// `number`, `money` and `decimal`: the smallest value, inclusive. + pub min: Option, + /// `number`, `money` and `decimal`: the largest value, inclusive. + pub max: Option, +} + +impl Limits { + /// Whether nothing is declared. + pub fn is_empty(&self) -> bool { + *self == Self::default() + } +} + +/// How a written value breaks a limit. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Breach { + /// Fewer characters than `min_length`; `have` is the number of characters written. + TooShort { min: u32, have: usize }, + /// More characters than the effective `max_length`. + TooLong { max: u32, have: usize }, + /// Below `min`. + Below(Bound), + /// Above `max`. + Above(Bound), +} + +impl Breach { + /// What the value must be and what it was: `at most 2000 characters; this has 2001`. `value` is + /// the written number, for the two bounds. + pub fn detail(&self, value: &str) -> String { + let chars = |n: u32| format!("{n} character{}", if n == 1 { "" } else { "s" }); + match self { + Self::TooShort { min, have } => format!("at least {}; this has {have}", chars(*min)), + Self::TooLong { max, have } => format!("at most {}; this has {have}", chars(*max)), + Self::Below(b) => format!("at least {b}; this is {value}"), + Self::Above(b) => format!("at most {b}; this is {value}"), + } + } + + /// `model.field must be ...`, naming the field and the limit and saying what was written. + pub fn message(&self, model: &str, field: &str, value: &str) -> String { + format!("{model}.{field} must be {}", self.detail(value)) + } +} + +/// The number of characters (Unicode scalar values) of `s`. +pub fn char_count(s: &str) -> usize { + s.chars().count() +} + +impl FieldDef { + /// This field with these limits (none when `limits` is empty). + pub fn with_limits(mut self, limits: Limits) -> Self { + self.limits = (!limits.is_empty()).then(|| Arc::new(limits)); + self + } + + /// The limits as declared, with no default filled in. + pub fn declared_limits(&self) -> Limits { + self.limits.as_deref().copied().unwrap_or_default() + } + + /// The most characters a value of this field has: the declared `max_length`, else the default + /// of the kind. `None` for every kind but `text` and `longtext`, and for a computed or numbered + /// field, which is never written. + pub fn effective_max_length(&self) -> Option { + if self.computed.is_some() || self.sequence.is_some() { + return None; + } + let default = match self.kind { + FieldKind::Text => DEFAULT_TEXT_MAX, + FieldKind::LongText => DEFAULT_LONGTEXT_MAX, + _ => return None, + }; + Some(self.declared_limits().max_length.unwrap_or(default)) + } + + /// The fewest characters a value of this field has, if it declares a `min_length`. + pub fn effective_min_length(&self) -> Option { + if self.computed.is_some() || self.sequence.is_some() { + return None; + } + match self.kind { + FieldKind::Text | FieldKind::LongText => self.declared_limits().min_length, + _ => None, + } + } + + /// The declared `min` of a number, money or decimal field. + pub fn lower_bound(&self) -> Option { + self.declared_limits().min + } + + /// The declared `max` of a number, money or decimal field. + pub fn upper_bound(&self) -> Option { + self.declared_limits().max + } + + /// Whether `s` is a value this field accepts, as far as its length is concerned. + pub fn check_text(&self, s: &str) -> Result<(), Breach> { + let max = self.effective_max_length(); + let min = self.effective_min_length(); + if max.is_none() && min.is_none() { + return Ok(()); + } + // A string of `n` bytes has at most `n` and at least `n / 4` (rounded up) characters, + // so most values are decided without counting them. + let bytes = s.len(); + let sure_max = max.is_none_or(|m| bytes <= m as usize); + let sure_min = min.is_none_or(|m| bytes.div_ceil(4) >= m as usize); + if sure_max && sure_min { + return Ok(()); + } + let have = char_count(s); + if let Some(m) = max + && have > m as usize + { + return Err(Breach::TooLong { max: m, have }); + } + if let Some(m) = min + && have < m as usize + { + return Err(Breach::TooShort { min: m, have }); + } + Ok(()) + } + + /// Whether `n` is a value this field accepts, as far as `min` and `max` are concerned (a + /// `number` or `money` field). + pub fn check_int(&self, n: i64) -> Result<(), Breach> { + let l = self.declared_limits(); + if let Some(Bound::Int(m)) = l.min + && n < m + { + return Err(Breach::Below(Bound::Int(m))); + } + if let Some(Bound::Int(m)) = l.max + && n > m + { + return Err(Breach::Above(Bound::Int(m))); + } + Ok(()) + } + + /// As [`FieldDef::check_int`], for a `decimal` field. The comparison is exact (`Decimal` + /// compares by value whatever the scales), never through a float. + pub fn check_decimal(&self, d: Decimal) -> Result<(), Breach> { + let l = self.declared_limits(); + if let Some(Bound::Dec(m)) = l.min + && d.compare(m) == Ordering::Less + { + return Err(Breach::Below(Bound::Dec(m))); + } + if let Some(Bound::Dec(m)) = l.max + && d.compare(m) == Ordering::Greater + { + return Err(Breach::Above(Bound::Dec(m))); + } + Ok(()) + } + + /// The limits this field declares, as a sentence for descriptions (OpenAPI, MCP): empty when it + /// declares none, so the default of the kind (which the schema keywords and `/api/meta` state) + /// adds no words to the description of every text field. `(at most 100 characters)`, `(from 1 + /// character to 2000 characters)`, `(from 0 to 100)`. + pub fn limits_note(&self) -> String { + let l = self.declared_limits(); + limits_note(l.min_length, l.max_length, l.min, l.max) + } +} + +/// The text behind [`FieldDef::limits_note`], for callers that hold the limits as plain values. +pub fn limits_note( + min_length: Option, + max_length: Option, + min: Option, + max: Option, +) -> String { + let chars = |n: u32| format!("{n} character{}", if n == 1 { "" } else { "s" }); + match (min_length, max_length, min, max) { + (None, None, None, None) => String::new(), + (Some(lo), Some(hi), _, _) => format!(" (from {} to {})", chars(lo), chars(hi)), + (Some(lo), None, _, _) => format!(" (at least {})", chars(lo)), + (None, Some(hi), _, _) => format!(" (at most {})", chars(hi)), + (_, _, Some(lo), Some(hi)) => format!(" (from {lo} to {hi})"), + (_, _, Some(lo), None) => format!(" (at least {lo})"), + (_, _, None, Some(hi)) => format!(" (at most {hi})"), + } +} + +/// What the install gate (D41) says about the limits of one field: one sentence per problem, +/// each naming the key. `kind` and the digits of a decimal come from the field itself. +pub fn problems(f: &FieldDef) -> Vec { + let l = f.declared_limits(); + let mut out = Vec::new(); + if l.is_empty() { + return out; + } + let is_text = matches!(f.kind, FieldKind::Text | FieldKind::LongText); + let is_num = matches!( + f.kind, + FieldKind::Number | FieldKind::Money | FieldKind::Decimal { .. } + ); + if f.computed.is_some() || f.sequence.is_some() { + out.push( + "a computed or numbered field is never written, so it takes no limits (max_length, min_length, min, max)" + .into(), + ); + return out; + } + if !is_text && (l.max_length.is_some() || l.min_length.is_some()) { + out.push(format!( + "`max_length` and `min_length` are only valid for kind \"text\" or \"longtext\", not {:?}", + f.kind.name() + )); + } + if !is_num && (l.min.is_some() || l.max.is_some()) { + out.push(format!( + "`min` and `max` are only valid for kind \"number\", \"money\" or \"decimal\", not {:?}", + f.kind.name() + )); + } + if is_text { + let cap = if f.kind == FieldKind::LongText { + LONGTEXT_MAX_CAP + } else { + TEXT_MAX_CAP + }; + let default = if f.kind == FieldKind::LongText { + DEFAULT_LONGTEXT_MAX + } else { + DEFAULT_TEXT_MAX + }; + if l.max_length == Some(0) { + out.push("`max_length` must be at least 1".into()); + } + if l.min_length == Some(0) { + out.push("`min_length` must be at least 1 (a minimum of 0 limits nothing)".into()); + } + if let Some(m) = l.max_length + && m > cap + { + out.push(format!( + "`max_length` {m} is above the largest a {} field may declare, {cap}", + f.kind.name() + )); + } + match (l.min_length, l.max_length) { + (Some(lo), Some(hi)) if lo > hi => out.push(format!( + "`min_length` {lo} is above `max_length` {hi}, so no value fits" + )), + (Some(lo), None) if lo > default => out.push(format!( + "`min_length` {lo} is above the default maximum of a {} field ({default}); declare a `max_length` of at least {lo}", + f.kind.name() + )), + _ => {} + } + } + if is_num { + let shape = f.kind.decimal_shape(); + let mut bound = |key: &str, b: Option| match (b, shape) { + (None, _) => {} + (Some(Bound::Int(_)), None) => {} + (Some(Bound::Dec(_)), None) => out.push(format!( + "`{key}` must be a whole number for kind {:?}, not a decimal", + f.kind.name() + )), + (Some(Bound::Int(_)), Some(_)) => out.push(format!( + "`{key}` of a decimal field must be a decimal at the scale of the field" + )), + (Some(Bound::Dec(d)), Some((precision, scale, _))) => { + if d.scale() != scale || d.digits() > u32::from(precision) { + out.push(format!( + "`{key}` {d} does not fit the field: a decimal of {precision} digits, {scale} after the point" + )); + } + } + }; + bound("min", l.min); + bound("max", l.max); + if let (Some(lo), Some(hi)) = (l.min, l.max) { + let above = match (lo, hi) { + (Bound::Int(a), Bound::Int(b)) => a > b, + (Bound::Dec(a), Bound::Dec(b)) => a.compare(b) == Ordering::Greater, + _ => false, + }; + if above { + out.push(format!("`min` {lo} is above `max` {hi}, so no value fits")); + } + } + } + out +} diff --git a/crates/kernel/obop-registry/src/limits_tests.rs b/crates/kernel/obop-registry/src/limits_tests.rs new file mode 100644 index 00000000..bbc6742c --- /dev/null +++ b/crates/kernel/obop-registry/src/limits_tests.rs @@ -0,0 +1,543 @@ +//! Tests of the field limits (D90) at the level of the registry: counting, the default, exact +//! bounds, the sentences, and the install gate. + +use crate::limits::*; +use crate::{FieldDef, FieldKind, ModelDef, Module, Reset, check_module}; +use obop_types::DataClass; +use obop_values::Decimal; + +fn f(name: &'static str) -> FieldDef { + FieldDef::new(name, "d", DataClass::None, false, "t.user") +} +fn dec(s: &str, scale: u8) -> Bound { + Bound::Dec(Decimal::parse_for(s, 12, scale, None).unwrap()) +} +fn with(field: FieldDef, l: Limits) -> FieldDef { + field.with_limits(l) +} + +#[test] +fn the_defaults_are_below_the_caps_and_named_in_one_place() { + assert_eq!((DEFAULT_TEXT_MAX, DEFAULT_LONGTEXT_MAX), (2_048, 100_000)); + assert_eq!((TEXT_MAX_CAP, LONGTEXT_MAX_CAP), (10_000, 1_000_000)); + // The order of the four is checked by the compiler, so a change of one number that breaks it + // does not build: each default is below its cap, and a text may not be allowed more than a + // longtext has by default. + const { + assert!(DEFAULT_TEXT_MAX < TEXT_MAX_CAP && DEFAULT_LONGTEXT_MAX < LONGTEXT_MAX_CAP); + assert!(TEXT_MAX_CAP <= DEFAULT_LONGTEXT_MAX); + } +} + +#[test] +fn the_effective_maximum_is_the_declared_one_or_the_default_of_the_kind() { + assert_eq!(f("a").effective_max_length(), Some(DEFAULT_TEXT_MAX)); + assert_eq!( + f("a").kind(FieldKind::LongText).effective_max_length(), + Some(DEFAULT_LONGTEXT_MAX) + ); + let own = with( + f("a"), + Limits { + max_length: Some(30), + ..Default::default() + }, + ); + assert_eq!(own.effective_max_length(), Some(30)); + // A declared value may also be above the default, up to the cap. + let big = with( + f("a"), + Limits { + max_length: Some(TEXT_MAX_CAP), + ..Default::default() + }, + ); + assert_eq!(big.effective_max_length(), Some(TEXT_MAX_CAP)); + // No length for the other kinds, and none for a field the system writes. + for k in [ + FieldKind::Number, + FieldKind::Money, + FieldKind::Date, + FieldKind::Bool, + ] { + assert_eq!(f("a").kind(k.clone()).effective_max_length(), None, "{k:?}"); + } + assert_eq!(f("a").computed("1").effective_max_length(), None); + assert_eq!( + f("a") + .sequence("{seq}", false, Reset::Never) + .effective_max_length(), + None + ); + assert_eq!(f("a").effective_min_length(), None); +} + +/// The count the plain way, to compare the shortcut by bytes against. +fn naive(s: &str, min: Option, max: Option) -> Result<(), Breach> { + let have = s.chars().count(); + if let Some(m) = max + && have > m as usize + { + return Err(Breach::TooLong { max: m, have }); + } + if let Some(m) = min + && have < m as usize + { + return Err(Breach::TooShort { min: m, have }); + } + Ok(()) +} + +#[test] +fn counting_by_bytes_first_never_changes_the_answer() { + // One letter of each width in UTF-8 (1, 2, 3 and 4 bytes) and a letter with a combining mark. + let alphabet = ["a", "å", "€", "\u{1F600}", "e\u{301}"]; + let mut strings = vec![String::new()]; + let mut layer = vec![String::new()]; + for _ in 0..4 { + layer = layer + .iter() + .flat_map(|s| alphabet.iter().map(move |c| format!("{s}{c}"))) + .collect(); + strings.extend(layer.iter().cloned()); + } + let mut checked = 0usize; + for s in &strings { + for max in [None, Some(1), Some(2), Some(3), Some(5), Some(8)] { + for min in [None, Some(1), Some(2), Some(4)] { + if let (Some(lo), Some(hi)) = (min, max) + && lo > hi + { + continue; + } + let field = with( + f("a"), + Limits { + min_length: min, + max_length: max, + ..Default::default() + }, + ); + // With no maximum declared the default (2,048) applies; none of these strings is + // near it, so the plain count with no maximum gives the same answer. + assert_eq!( + field.check_text(s), + naive(s, min, max), + "{s:?} min {min:?} max {max:?}" + ); + checked += 1; + } + } + } + assert!(checked > 5_000, "{checked}"); +} + +#[test] +fn characters_are_code_points() { + assert_eq!(char_count("åäö"), 3); + assert_eq!(char_count("\u{1F600}"), 1); + // A flag is two regional indicators and a family five code points: counted as they are. + assert_eq!(char_count("\u{1F1EB}\u{1F1EE}"), 2); + assert_eq!(char_count("\u{1F468}\u{200D}\u{1F469}\u{200D}\u{1F467}"), 5); + let three = with( + f("a"), + Limits { + max_length: Some(3), + ..Default::default() + }, + ); + assert_eq!(three.check_text("åäö"), Ok(())); + assert_eq!(three.check_text("\u{1F600}\u{1F600}\u{1F600}"), Ok(())); + assert_eq!( + three.check_text("\u{1F600}\u{1F600}\u{1F600}\u{1F600}"), + Err(Breach::TooLong { max: 3, have: 4 }) + ); +} + +#[test] +fn the_default_applies_to_a_text_field_and_a_longtext_has_its_own() { + let s = "x".repeat(DEFAULT_TEXT_MAX as usize); + assert_eq!(f("a").check_text(&s), Ok(())); + assert_eq!( + f("a").check_text(&format!("{s}x")), + Err(Breach::TooLong { + max: DEFAULT_TEXT_MAX, + have: DEFAULT_TEXT_MAX as usize + 1 + }) + ); + assert_eq!( + f("a") + .kind(FieldKind::LongText) + .check_text(&format!("{s}x")), + Ok(()) + ); +} + +#[test] +fn whole_number_and_decimal_bounds_are_inclusive_and_exact() { + let n = with( + f("n").kind(FieldKind::Number), + Limits { + min: Some(Bound::Int(-3)), + max: Some(Bound::Int(3)), + ..Default::default() + }, + ); + assert_eq!(n.check_int(-3), Ok(())); + assert_eq!(n.check_int(3), Ok(())); + assert_eq!(n.check_int(-4), Err(Breach::Below(Bound::Int(-3)))); + assert_eq!(n.check_int(4), Err(Breach::Above(Bound::Int(3)))); + assert_eq!(f("n").kind(FieldKind::Number).check_int(i64::MIN), Ok(())); + + // Seven digits after the point: 0.1000001 is above 0.1000000 and 0.0999999 is below it, + // which a double cannot tell apart from 0.1. + let d = with( + f("d").decimal(12, 7), + Limits { + min: Some(dec("0.1000000", 7)), + max: Some(dec("0.9000000", 7)), + ..Default::default() + }, + ); + let v = |s: &str| Decimal::parse_for(s, 12, 7, None).unwrap(); + assert_eq!(d.check_decimal(v("0.1000000")), Ok(())); + assert_eq!(d.check_decimal(v("0.1000001")), Ok(())); + assert_eq!(d.check_decimal(v("0.9000000")), Ok(())); + assert!(d.check_decimal(v("0.0999999")).is_err()); + assert!(d.check_decimal(v("0.9000001")).is_err()); + assert_eq!( + d.check_decimal(v("-0.0000001")), + Err(Breach::Below(dec("0.1000000", 7))) + ); + // Larger than a double holds exactly: 2^53 + 1 digits apart. + let big = with( + f("d").decimal(20, 0), + Limits { + max: Some(Bound::Dec( + Decimal::parse_for("9007199254740993", 20, 0, None).unwrap(), + )), + ..Default::default() + }, + ); + let at = |s: &str| Decimal::parse_for(s, 20, 0, None).unwrap(); + assert_eq!(big.check_decimal(at("9007199254740993")), Ok(())); + assert!(big.check_decimal(at("9007199254740994")).is_err()); +} + +#[test] +fn a_breach_says_what_was_wanted_and_what_was_written() { + let msg = |b: Breach, v: &str| b.message("m", "f", v); + assert_eq!( + msg( + Breach::TooLong { + max: 2000, + have: 2001 + }, + "" + ), + "m.f must be at most 2000 characters; this has 2001" + ); + assert_eq!( + msg(Breach::TooLong { max: 1, have: 2 }, ""), + "m.f must be at most 1 character; this has 2" + ); + assert_eq!( + msg(Breach::TooShort { min: 3, have: 0 }, ""), + "m.f must be at least 3 characters; this has 0" + ); + assert_eq!( + msg(Breach::Below(Bound::Int(0)), "-5"), + "m.f must be at least 0; this is -5" + ); + assert_eq!( + msg(Breach::Above(dec("0.90", 2)), "1.00"), + "m.f must be at most 0.90; this is 1.00" + ); +} + +#[test] +fn the_note_for_descriptions_only_speaks_of_declared_limits() { + assert_eq!(f("a").limits_note(), ""); + let own = |l: Limits| with(f("a"), l).limits_note(); + assert_eq!( + own(Limits { + max_length: Some(100), + ..Default::default() + }), + " (at most 100 characters)" + ); + assert_eq!( + own(Limits { + min_length: Some(1), + ..Default::default() + }), + " (at least 1 character)" + ); + assert_eq!( + own(Limits { + min_length: Some(1), + max_length: Some(80), + ..Default::default() + }), + " (from 1 character to 80 characters)" + ); + assert_eq!( + limits_note(None, None, Some(Bound::Int(0)), Some(Bound::Int(100))), + " (from 0 to 100)" + ); + assert_eq!( + limits_note(None, None, None, Some(Bound::Int(100))), + " (at most 100)" + ); +} + +// ---- the install gate --------------------------------------------------------------------- + +fn gate(field: FieldDef) -> Vec { + problems(&field) +} + +#[test] +fn the_gate_accepts_what_makes_sense_and_a_field_with_no_limits_is_free() { + assert!(gate(f("a")).is_empty()); + let ok = [ + with( + f("a"), + Limits { + min_length: Some(1), + max_length: Some(1), + ..Default::default() + }, + ), + with( + f("a"), + Limits { + max_length: Some(TEXT_MAX_CAP), + ..Default::default() + }, + ), + with( + f("a").kind(FieldKind::LongText), + Limits { + max_length: Some(LONGTEXT_MAX_CAP), + ..Default::default() + }, + ), + with( + f("a").kind(FieldKind::Number), + Limits { + min: Some(Bound::Int(5)), + max: Some(Bound::Int(5)), + ..Default::default() + }, + ), + with( + f("a").decimal(10, 2), + Limits { + min: Some(dec("0.00", 2)), + max: Some(dec("99999999.99", 2)), + ..Default::default() + }, + ), + ]; + for field in ok { + assert_eq!( + gate(field.clone()), + Vec::::new(), + "{:?}", + field.limits + ); + } +} + +fn has(found: Vec, needle: &str) { + assert!( + found.iter().any(|p| p.contains(needle)), + "wanted {needle:?} in {found:?}" + ); +} + +#[test] +fn the_gate_refuses_a_length_that_cannot_hold() { + let text = |l: Limits| gate(with(f("a"), l)); + let long = |l: Limits| gate(with(f("a").kind(FieldKind::LongText), l)); + has( + text(Limits { + max_length: Some(0), + ..Default::default() + }), + "`max_length` must be at least 1", + ); + has( + text(Limits { + min_length: Some(0), + ..Default::default() + }), + "`min_length` must be at least 1", + ); + has( + text(Limits { + max_length: Some(TEXT_MAX_CAP + 1), + ..Default::default() + }), + "above the largest a text field may declare, 10000", + ); + has( + long(Limits { + max_length: Some(LONGTEXT_MAX_CAP + 1), + ..Default::default() + }), + "above the largest a longtext field may declare, 1000000", + ); + has( + text(Limits { + min_length: Some(10), + max_length: Some(9), + ..Default::default() + }), + "`min_length` 10 is above `max_length` 9", + ); + has( + text(Limits { + min_length: Some(DEFAULT_TEXT_MAX + 1), + ..Default::default() + }), + "above the default maximum", + ); +} + +#[test] +fn the_gate_refuses_keys_on_the_wrong_kind() { + has( + gate(with( + f("a").kind(FieldKind::Number), + Limits { + max_length: Some(5), + ..Default::default() + }, + )), + "only valid for kind \"text\" or \"longtext\"", + ); + has( + gate(with( + f("a").kind(FieldKind::Date), + Limits { + min: Some(Bound::Int(1)), + ..Default::default() + }, + )), + "only valid for kind \"number\", \"money\" or \"decimal\"", + ); + has( + gate(with( + f("a"), + Limits { + max: Some(Bound::Int(1)), + ..Default::default() + }, + )), + "only valid for kind \"number\"", + ); + // A field the system writes takes none. + has( + gate(with( + f("a").computed("1"), + Limits { + max_length: Some(3), + ..Default::default() + }, + )), + "computed or numbered", + ); +} + +#[test] +fn the_gate_refuses_bounds_that_do_not_fit_the_kind_or_each_other() { + has( + gate(with( + f("a").kind(FieldKind::Number), + Limits { + min: Some(dec("1.0", 1)), + ..Default::default() + }, + )), + "must be a whole number", + ); + has( + gate(with( + f("a").decimal(10, 2), + Limits { + max: Some(Bound::Int(5)), + ..Default::default() + }, + )), + "must be a decimal at the scale of the field", + ); + has( + gate(with( + f("a").decimal(10, 2), + Limits { + max: Some(dec("1.000", 3)), + ..Default::default() + }, + )), + "does not fit the field", + ); + has( + gate(with( + f("a").kind(FieldKind::Number), + Limits { + min: Some(Bound::Int(2)), + max: Some(Bound::Int(1)), + ..Default::default() + }, + )), + "`min` 2 is above `max` 1", + ); + has( + gate(with( + f("a").decimal(10, 2), + Limits { + min: Some(dec("0.11", 2)), + max: Some(dec("0.10", 2)), + ..Default::default() + }, + )), + "`min` 0.11 is above `max` 0.10", + ); +} + +#[test] +fn check_module_runs_the_gate_so_install_and_start_both_refuse() { + let bad = with( + f("title"), + Limits { + max_length: Some(0), + ..Default::default() + }, + ); + let module = Module { + id: "t".into(), + name: "T".into(), + models: vec![ModelDef { + name: "t_doc", + description: "d", + read_group: "t.user", + write_group: "t.user", + access: None, + fields: vec![bad], + commands: vec![], + ext: Default::default(), + }], + network_destinations: vec![], + description: String::new(), + translations: Default::default(), + meta: Default::default(), + }; + let v = check_module(&module); + assert!( + v.iter() + .any(|v| v.path.contains("t_doc.title") && v.problem.contains("max_length")), + "{v:?}" + ); +} From d4bdb2b20559dc40ba2c84c84dcde4bd6ee80d69 Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:04 +0100 Subject: [PATCH 02/23] Refuse values outside a field's limits on every write path with limit_violated (D90) create, update, each row of bulk_create (and so imports and the CLI), the effects of actions, the pre-flight of a governed update and the public submit all hold a written value to the limits of its field. The refusal is a 400 with the new code limit_violated and names the model, the field, the limit and what was written. A value equal to the stored one is not re-checked on update, so lowering a limit locks no existing record. /api/meta (FieldInfo) carries max_length, min_length, min and max. Co-Authored-By: Claude Sonnet 5.5 --- .../kernel/obop-commands/src/engine/bulk.rs | 3 +- .../kernel/obop-commands/src/engine/crud.rs | 56 +- .../obop-commands/src/engine/effects.rs | 4 +- crates/kernel/obop-commands/src/errors.rs | 18 + crates/kernel/obop-commands/src/guards.rs | 4 +- crates/kernel/obop-commands/src/kinds.rs | 35 +- crates/kernel/obop-commands/src/lib.rs | 21 + .../kernel/obop-commands/src/limits_tests.rs | 679 ++++++++++++++++++ crates/kernel/obop-commands/src/public.rs | 2 +- .../kernel/obop-commands/src/public/tests.rs | 32 + crates/kernel/obop-commands/src/store.rs | 22 +- 11 files changed, 852 insertions(+), 24 deletions(-) create mode 100644 crates/kernel/obop-commands/src/limits_tests.rs diff --git a/crates/kernel/obop-commands/src/engine/bulk.rs b/crates/kernel/obop-commands/src/engine/bulk.rs index 49166ac8..b3276493 100644 --- a/crates/kernel/obop-commands/src/engine/bulk.rs +++ b/crates/kernel/obop-commands/src/engine/bulk.rs @@ -445,7 +445,8 @@ pub(crate) fn bulk_create(eng: &Engine, def: &Arc, input: &Value) -> R ids.push(None); failed += 1; if errors.len() < MAX_LISTED_ERRORS { - errors.push((r.row, r.line, e)); + // The list names the row; a refusal with a code (D90) is listed by its text. + errors.push((r.row, r.line, crate::errors::plain(&e).to_string())); } } } diff --git a/crates/kernel/obop-commands/src/engine/crud.rs b/crates/kernel/obop-commands/src/engine/crud.rs index a93d0113..2b57211f 100644 --- a/crates/kernel/obop-commands/src/engine/crud.rs +++ b/crates/kernel/obop-commands/src/engine/crud.rs @@ -12,7 +12,7 @@ use crate::guards; use crate::kinds::{self, Path}; use crate::query::Query; use crate::store::{declared_fields, id_of}; -use obop_registry::{FieldKind, ModelDef, is_valid_date}; +use obop_registry::{FieldDef, FieldKind, ModelDef, is_valid_date}; use obop_storage::Fields; use serde_json::Value as J; use std::collections::BTreeMap; @@ -57,19 +57,32 @@ pub(crate) fn own_default(def: &ModelDef, f: &mut Fields, actor: &Actor) { } } +/// `unchanged`: the value is the one already stored (an update that sends the record back as it +/// is). It is checked for its kind like any other, but not against the limits of the field (D90): +/// a value is held to a limit when it is written, not for being there. fn check_json( reads: &dyn Reads, gate: &Gate<'_>, lookup: &dyn Fn(&str) -> Option>, model: &str, - name: &str, - kind: &FieldKind, + f: &FieldDef, v: &J, + unchanged: bool, ) -> R<()> { + let (name, kind) = (f.name, &f.kind); let wrong = || Err(format!("{model}.{name} must be {}", kind.describe())); match (kind, v) { - (FieldKind::Text | FieldKind::LongText, J::String(_)) => Ok(()), - (FieldKind::Number | FieldKind::Money, J::Number(n)) if n.is_i64() => Ok(()), + (FieldKind::Text | FieldKind::LongText, J::String(s)) => { + if unchanged { + Ok(()) + } else { + kinds::check_text_limits(model, f, s) + } + } + (FieldKind::Number | FieldKind::Money, J::Number(n)) if n.is_i64() => match n.as_i64() { + Some(i) if !unchanged => kinds::check_int_limits(model, f, i), + _ => Ok(()), + }, (FieldKind::Bool, J::Bool(_)) => Ok(()), // A stored image or file (D60): a well-formed id of an existing blob of the right sort. // Who may use which blob is the reference guard's business. @@ -95,7 +108,11 @@ fn check_json( // Decimals and instants are stored as canonical text (D72); every write path // canonicalises first, so anything else here is refused. (FieldKind::Decimal { .. } | FieldKind::Datetime, J::String(t)) => { - kinds::check_canonical(model, name, kind, t) + kinds::check_canonical(model, name, kind, t)?; + if matches!(kind, FieldKind::Decimal { .. }) && !unchanged { + kinds::check_decimal_limits(model, f, t)?; + } + Ok(()) } (FieldKind::Select(o), J::String(t)) if o.contains(&t.as_str()) => Ok(()), (FieldKind::Ref(to), J::String(id)) => { @@ -119,8 +136,8 @@ fn check_json( } /// Validate the fields of a create (`creating`: every required field must be present) or of an -/// update against their kinds (D47), required flags and refs. A ref must name a record the caller -/// can see. +/// update against their kinds (D47), required flags, limits (D90) and refs. A ref must name a +/// record the caller can see. pub(crate) fn check_fields( reads: &dyn Reads, lookup: &dyn Fn(&str) -> Option>, @@ -128,6 +145,22 @@ pub(crate) fn check_fields( def: &ModelDef, fields: &Fields, creating: bool, +) -> R<()> { + check_fields_against(reads, lookup, actor, def, fields, creating, None) +} + +/// As [`check_fields`] for an update of a record that is `stored`: a value that equals the stored +/// one is not held to the limits of its field again (D90 Decision 6), so a record above a limit +/// that was lowered later can still be saved with its other fields changed, and a form that sends +/// the whole record back works. Every other check (kind, refs, required) still runs on it. +pub(crate) fn check_fields_against( + reads: &dyn Reads, + lookup: &dyn Fn(&str) -> Option>, + actor: &Actor, + def: &ModelDef, + fields: &Fields, + creating: bool, + stored: Option<&Fields>, ) -> R<()> { let gate = Gate::new(reads, lookup, actor); for f in &def.fields { @@ -146,7 +179,10 @@ pub(crate) fn check_fields( )); } Some(J::Null) => {} - Some(v) => check_json(reads, &gate, lookup, def.name, f.name, &f.kind, v)?, + Some(v) => { + let unchanged = stored.is_some_and(|s| s.get(f.name) == Some(v)); + check_json(reads, &gate, lookup, def.name, f, v, unchanged)? + } } } Ok(()) @@ -333,7 +369,7 @@ pub(crate) fn update(eng: &Engine, def: &Arc, input: &Value) -> R { .load(def, id) .map_err(|_| self.unavailable(def))?; kinds::canonicalise(def, &mut patch, Path::Write)?; - check_fields(&*unit, &look, self.actor, def, &patch, false)?; + check_fields_against(&*unit, &look, self.actor, def, &patch, false, Some(&cur))?; let mut after = cur; after.extend(patch.clone()); if let Some(why) = creator_blocked(&Gate::new(&*unit, &look, self.actor), def, &after) { diff --git a/crates/kernel/obop-commands/src/errors.rs b/crates/kernel/obop-commands/src/errors.rs index be90e808..460e1877 100644 --- a/crates/kernel/obop-commands/src/errors.rs +++ b/crates/kernel/obop-commands/src/errors.rs @@ -97,6 +97,9 @@ pub mod code { pub const USE_SEARCH: &str = "use_search"; /// D56: an assertion of the action did not hold (REST names it; see the module comment). pub const ASSERTION_FAILED: &str = "assertion_failed"; + /// D90: a value is outside a limit of its field (`max_length`, `min_length`, `min`, + /// `max`, or the default length of a text field). + pub const LIMIT_VIOLATED: &str = "limit_violated"; /// Step 2: the caller is known and may not do this. pub const DENIED: &str = "denied"; /// Step 2: the caller must confirm their identity again. @@ -232,6 +235,13 @@ pub const TABLE: &[Row] = &[ origin: "D56", hint: "A condition of the action does not hold. Change the record as the message says, then retry.", }, + Row { + code: code::LIMIT_VIOLATED, + status: 400, + retry_after: None, + origin: "D90", + hint: "A value is outside the limits of its field (max_length, min_length, min and max in GET /api/meta; a text field with no max_length has the default of its kind). Nothing was changed. Shorten the text or change the number so that it fits, then retry.", + }, Row { code: code::DENIED, status: 403, @@ -699,6 +709,13 @@ fn split_code(m: &str) -> Option<(&'static Row, &str)> { .map(|r| (r, rest)) } +/// The message of a handler string without its leading code (`code: message`), for a place that +/// lists refusals one by one and names the row itself (the rows of `bulk_create`). A text that +/// does not begin with a code of the table is returned as it is. +pub fn plain(m: &str) -> &str { + split_code(m).map_or(m, |(_, rest)| rest) +} + /// The `origin` of the three storage rows. const STORAGE_ORIGIN: &str = "D74"; @@ -889,6 +906,7 @@ mod tests { ("limit_required", 400), ("use_search", 400), ("assertion_failed", 400), + ("limit_violated", 400), ("denied", 403), ("reauth_required", 400), ("incomplete", 409), diff --git a/crates/kernel/obop-commands/src/guards.rs b/crates/kernel/obop-commands/src/guards.rs index f043b455..ad413e4c 100644 --- a/crates/kernel/obop-commands/src/guards.rs +++ b/crates/kernel/obop-commands/src/guards.rs @@ -10,7 +10,7 @@ //! - `delete`: refused while another record refers to the one being deleted. use crate::engine::access::Gate; -use crate::engine::crud::check_fields; +use crate::engine::crud::check_fields_against; use crate::engine::effects::{lookup_fn, precheck, reachable, required_missing}; use crate::engine::unit::{Plain, Reads}; use crate::engine::{Actor, Engine}; @@ -206,7 +206,7 @@ fn update_guard(def: &ModelDef, eng: &Engine) -> Option { .map(|(k, v)| (k, v.to_json())) .collect(); crate::kinds::canonicalise(&def, &mut given, crate::kinds::Path::Write)?; - check_fields(&reads, &look, &actor, &def, &given, false)?; + check_fields_against(&reads, &look, &actor, &def, &given, false, Some(¤t))?; } Ok(verdict) })) diff --git a/crates/kernel/obop-commands/src/kinds.rs b/crates/kernel/obop-commands/src/kinds.rs index 744d001d..dfa162ab 100644 --- a/crates/kernel/obop-commands/src/kinds.rs +++ b/crates/kernel/obop-commands/src/kinds.rs @@ -10,7 +10,8 @@ //! numbers and instants they stand for wherever strings are compared. use crate::Value; -use obop_registry::{FieldKind, ModelDef}; +use crate::errors::{self, code}; +use obop_registry::{Breach, FieldDef, FieldKind, ModelDef}; use obop_storage::Fields; use obop_values::{Decimal, Instant}; use serde_json::Value as J; @@ -116,6 +117,38 @@ pub(crate) fn canonicalise_values( Ok(()) } +/// The refusal of a value that breaks a limit of its field (D90): `limit_violated: `. +fn breach(model: &str, f: &FieldDef, b: Breach, value: &str) -> String { + errors::coded(code::LIMIT_VIOLATED, &b.message(model, f.name, value)) +} + +/// A written text against the length limits of its field, the default included (D90). +pub(crate) fn check_text_limits(model: &str, f: &FieldDef, s: &str) -> Result<(), String> { + f.check_text(s).map_err(|b| breach(model, f, b, "")) +} + +/// A written whole number (`number` or `money`) against `min` and `max` of its field (D90). +pub(crate) fn check_int_limits(model: &str, f: &FieldDef, n: i64) -> Result<(), String> { + f.check_int(n) + .map_err(|b| breach(model, f, b, &n.to_string())) +} + +/// A written decimal, in its canonical text, against `min` and `max` of its field (D90). The +/// comparison is exact, by `Decimal`, and never goes through a float. +pub(crate) fn check_decimal_limits( + model: &str, + f: &FieldDef, + canonical: &str, +) -> Result<(), String> { + if f.lower_bound().is_none() && f.upper_bound().is_none() { + return Ok(()); + } + let d = Decimal::parse(canonical) + .map_err(|e| wrong(model, f.name, &f.kind, Some(&e.to_string())))?; + f.check_decimal(d) + .map_err(|b| breach(model, f, b, canonical)) +} + /// A stored or supplied value must already be in canonical form (see the module notes). pub(crate) fn check_canonical( model: &str, diff --git a/crates/kernel/obop-commands/src/lib.rs b/crates/kernel/obop-commands/src/lib.rs index 2ddd2173..8821575e 100644 --- a/crates/kernel/obop-commands/src/lib.rs +++ b/crates/kernel/obop-commands/src/lib.rs @@ -11,6 +11,7 @@ use shared::{Pending, Plans}; use obop_masking::mode_for; pub use obop_masking::{PrincipalKind, RestoreError, Vault}; +pub use obop_registry::Bound; use obop_registry::labels::Texts; use obop_registry::{FieldDef, Module, Transition, Translations}; use obop_types::MaskMode; @@ -365,6 +366,19 @@ pub struct FieldInfo { pub shows_option_of: Option, /// The condition on the same record under which the label shows; the browser evaluates it. pub shows_option_when: Option, + /// `text` and `longtext` (D90): the most characters a written value has, the default of the + /// kind when the field declares none (`None` for a computed or numbered field and every other + /// kind), and the fewest, when the field declares a minimum. + pub max_length: Option, + pub min_length: Option, + /// `number`, `money` and `decimal` (D90): the declared bounds, inclusive. A `decimal` bound is + /// an exact decimal at the scale of the field. + pub min: Option, + pub max: Option, + /// The limits the field declares as a parenthesised sentence for a description (OpenAPI, MCP), + /// `(at most 2000 characters)` or `(from 0 to 100)`; empty when it declares none (the default + /// of its kind is in `max_length`, not in the description). + pub limits_note: String, } impl FieldInfo { @@ -593,6 +607,11 @@ impl Dispatcher { .as_ref() .and_then(|s| s.when.as_ref()) .map(|w| w.source.clone()), + max_length: f.effective_max_length(), + min_length: f.effective_min_length(), + min: f.lower_bound(), + max: f.upper_bound(), + limits_note: f.limits_note(), } }) .collect(), @@ -1733,6 +1752,8 @@ mod keyexpr_parity; #[cfg(test)] mod kinds_tests; #[cfg(test)] +mod limits_tests; +#[cfg(test)] mod shows_option_tests; #[cfg(test)] mod sysfields_tests; diff --git a/crates/kernel/obop-commands/src/limits_tests.rs b/crates/kernel/obop-commands/src/limits_tests.rs new file mode 100644 index 00000000..ecb1fa1a --- /dev/null +++ b/crates/kernel/obop-commands/src/limits_tests.rs @@ -0,0 +1,679 @@ +//! Field limits (D90) through the dispatcher: `max_length` and `min_length` of text and longtext +//! (in characters, not bytes), the default length of a text field, `min` and `max` of number, +//! money and decimal (exact), on create, update, `bulk_create` (the row is named), the effects of +//! an action, and the rule for a record stored above a limit that was lowered later. Every test +//! runs on the memory and SQLite backends, and on PostgreSQL when `OBOP_TEST_POSTGRES_URL` is set. + +use crate::{CallError, Dispatcher, Principal, Response, Store, Surface, Value, Vault, errors}; +use obop_registry::{ + Bound, CommandDef, DEFAULT_LONGTEXT_MAX, DEFAULT_TEXT_MAX, Effect, FieldDef as F, FieldKind, + Limits, ModelDef, Module, intern, +}; +use obop_types::DataClass; +use obop_values::Decimal; +use std::sync::Arc; + +const U: &str = "lim.user"; + +fn m(pairs: Vec<(&str, Value)>) -> Value { + Value::Map(pairs.into_iter().map(|(k, v)| (k.to_string(), v)).collect()) +} +fn t(s: &str) -> Value { + Value::text(s) +} +fn get<'a>(v: &'a Value, k: &str) -> &'a Value { + match v { + Value::Map(m) => m.get(k).unwrap_or(&Value::Null), + other => panic!("not a map: {other:?}"), + } +} +fn text(v: &Value) -> String { + match v { + Value::Text(s) => s.clone(), + other => panic!("not text: {other:?}"), + } +} +fn list(v: &Value) -> &[Value] { + match v { + Value::List(l) => l, + other => panic!("not a list: {other:?}"), + } +} +fn dec(s: &str, scale: u8) -> Bound { + Bound::Dec(Decimal::parse_for(s, 12, scale, None).unwrap()) +} + +struct Rig { + d: Arc, + p: String, + who: Principal, + store: Store, +} + +fn module(p: &str) -> Module { + let n = |s: &str| intern(&format!("{p}_{s}")); + let f = |name: &'static str| F::new(name, "d", DataClass::None, false, U); + let model = |name: &'static str, fields: Vec, commands: Vec| ModelDef { + name, + description: "d", + read_group: U, + write_group: U, + access: None, + fields, + commands, + ext: Default::default(), + }; + Module { + id: p.into(), + name: "L".into(), + models: vec![ + model( + n("doc"), + vec![ + // Declared: 2 to 10 characters. + f("title").with_limits(Limits { + min_length: Some(2), + max_length: Some(10), + ..Default::default() + }), + // Nothing declared: the default of the kind applies. + f("plain"), + f("body").kind(FieldKind::LongText), + // A longtext with a much lower limit than its default. + f("tiny").kind(FieldKind::LongText).with_limits(Limits { + max_length: Some(5), + ..Default::default() + }), + f("qty").kind(FieldKind::Number).with_limits(Limits { + min: Some(Bound::Int(0)), + max: Some(Bound::Int(100)), + ..Default::default() + }), + f("price").kind(FieldKind::Money).with_limits(Limits { + min: Some(Bound::Int(0)), + max: Some(Bound::Int(100_000)), + ..Default::default() + }), + f("rate").decimal(12, 7).with_limits(Limits { + min: Some(dec("0.1000000", 7)), + max: Some(dec("0.9000000", 7)), + ..Default::default() + }), + // Only a lower bound, only an upper bound. + f("floor").kind(FieldKind::Number).with_limits(Limits { + min: Some(Bound::Int(-5)), + ..Default::default() + }), + f("ceiling").kind(FieldKind::Number).with_limits(Limits { + max: Some(Bound::Int(5)), + ..Default::default() + }), + f("state").select(&["new", "done"]), + ], + vec![ + // An effect that writes the title of the record into another model. + CommandDef::new("finish", "Finish", U) + .transition("state", Some("new"), "done") + .effect(Effect::create( + &format!("{p}_log"), + &[("short", "$self.plain"), ("title", "$self.title")], + )), + ], + ), + model( + n("log"), + vec![ + f("title"), + // At most 3 characters. + f("short").with_limits(Limits { + max_length: Some(3), + ..Default::default() + }), + ], + vec![], + ), + model( + n("counter"), + vec![ + f("doc_id").reference(&format!("{p}_doc")), + f("by").kind(FieldKind::Number), + f("state").select(&["open", "applied"]), + ], + vec![ + // Adds `by` to the quantity of the document, which stops at 100. + CommandDef::new("apply", "Apply", U) + .transition("state", Some("open"), "applied") + .effect(Effect::adjust_ref( + &format!("{p}_doc"), + "doc_id", + "qty", + false, + "$self.by", + )), + ], + ), + ], + network_destinations: vec![], + description: String::new(), + translations: Default::default(), + meta: Default::default(), + } +} + +fn each(f: impl Fn(&Rig)) { + let mut urls = vec!["memory:".to_string(), "sqlite::memory:".to_string()]; + if let Ok(u) = std::env::var("OBOP_TEST_POSTGRES_URL") { + urls.push(u); + } + for url in urls { + let p = format!("l{}", &uuid::Uuid::now_v7().simple().to_string()[22..]); + let store = Store::open(&url).unwrap(); + let d = Arc::new(Dispatcher::new()); + let module = module(&p); + let v = obop_registry::check_module(&module); + assert!(v.is_empty(), "{v:?}"); + d.register_crud(&module, &store); + for mo in &module.models { + for c in &mo.commands { + d.register_transition(&module, mo.name, c.name, &store); + } + } + let who = Principal::user(&format!("u-{p}"), &[U]); + f(&Rig { d, p, who, store }); + } +} + +impl Rig { + fn cmd(&self, model: &str, c: &str) -> String { + format!("{}_{model}.{c}", self.p) + } + fn call(&self, model: &str, c: &str, input: Value) -> Result { + self.d.call( + Surface::Rest, + &self.who, + &mut Vault::new(), + &self.cmd(model, c), + &input, + ) + } + fn ok(&self, model: &str, c: &str, input: Value) -> Value { + match self.call(model, c, input) { + Ok(r) => r.value, + Err(e) => panic!("{model}.{c} failed: {e:?}"), + } + } + fn err(&self, model: &str, c: &str, input: Value) -> String { + match self.call(model, c, input) { + Err(CallError::Handler(m)) => m, + other => panic!("{model}.{c}: expected a handler error, got {other:?}"), + } + } + fn create(&self, model: &str, pairs: Vec<(&str, Value)>) -> String { + text(get(&self.ok(model, "create", m(pairs)), "id")) + } + fn refused(&self, model: &str, pairs: Vec<(&str, Value)>) -> String { + self.err(model, "create", m(pairs)) + } + fn update(&self, id: &str, pairs: Vec<(&str, Value)>) -> Result { + let mut all = vec![("id", t(id))]; + all.extend(pairs); + self.call("doc", "update", m(all)) + } + fn read(&self, model: &str, id: &str) -> Value { + self.ok(model, "get", m(vec![("id", t(id))])) + } + fn count(&self, model: &str) -> usize { + self.store.count(&format!("{}_{model}", self.p)) + } + /// Write a field of a stored record behind the commands' back, as it was before a limit was + /// lowered, or by a package version that had none. + fn corrupt(&self, model: &str, id: &str, field: &str, value: serde_json::Value) { + let patch = [(field.to_string(), value)].into_iter().collect(); + assert!( + self.store + .backend() + .update(&format!("{}_{model}", self.p), id, &patch) + .unwrap() + ); + } +} + +/// `limit_violated: . `: the code, the field and the numbers. +fn assert_limit(msg: &str, field: &str, what: &str) { + assert!( + msg.starts_with("limit_violated: "), + "not a limit_violated refusal: {msg}" + ); + assert!( + msg.contains(&format!(".{field} must be {what}")), + "wanted `.{field} must be {what}` in: {msg}" + ); +} + +#[test] +fn the_default_lengths_are_the_documented_constants() { + assert_eq!(DEFAULT_TEXT_MAX, 2_048); + assert_eq!(DEFAULT_LONGTEXT_MAX, 100_000); +} + +#[test] +fn a_text_is_counted_in_characters_not_bytes() { + each(|r| { + // Ten characters of two bytes each, and ten of four bytes: both fit a limit of 10. + r.create("doc", vec![("title", t(&"å".repeat(10)))]); + r.create("doc", vec![("title", t(&"\u{1F600}".repeat(10)))]); + // Eleven do not, however short they are in bytes for ASCII. + for (s, n) in [("å", 11), ("\u{1F600}", 11), ("a", 11)] { + let e = r.refused("doc", vec![("title", t(&s.repeat(n)))]); + assert_limit(&e, "title", &format!("at most 10 characters; this has {n}")); + } + // A combining sequence is several characters: e + U+0301 is two. + let e = r.refused("doc", vec![("title", t(&"e\u{301}".repeat(6)))]); + assert_limit(&e, "title", "at most 10 characters; this has 12"); + assert_eq!(r.count("doc"), 2); + }); +} + +#[test] +fn the_limit_is_inclusive_at_both_ends_and_a_message_names_the_field_and_the_numbers() { + each(|r| { + for ok in ["ab", "abcdefghij"] { + r.create("doc", vec![("title", t(ok))]); + } + let e = r.refused("doc", vec![("title", t("abcdefghijk"))]); + assert_limit(&e, "title", "at most 10 characters; this has 11"); + assert!(e.contains(&format!("{}_doc.title", r.p)), "{e}"); + let e = r.refused("doc", vec![("title", t("a"))]); + assert_limit(&e, "title", "at least 2 characters; this has 1"); + // An empty string is a value, and too short; an absent field is not checked. + let e = r.refused("doc", vec![("title", t(""))]); + assert_limit(&e, "title", "at least 2 characters; this has 0"); + r.create("doc", vec![("plain", t("no title at all"))]); + // The singular reads as a singular. + assert_eq!(r.count("doc"), 3); + }); +} + +#[test] +fn whitespace_counts_as_characters_and_nothing_is_trimmed() { + each(|r| { + // Two spaces satisfy min_length 2: the value is counted as sent (D90 says so plainly). + let id = r.create("doc", vec![("title", t(" "))]); + assert_eq!(text(get(&r.read("doc", &id), "title")), " "); + // Ten characters with a trailing space fit; eleven do not. + r.create("doc", vec![("title", t("abcdefghi "))]); + let e = r.refused("doc", vec![("title", t("abcdefghi "))]); + assert_limit(&e, "title", "at most 10 characters; this has 11"); + }); +} + +#[test] +fn a_text_field_without_a_declared_limit_has_the_default() { + each(|r| { + r.create( + "doc", + vec![("plain", t(&"x".repeat(DEFAULT_TEXT_MAX as usize)))], + ); + let e = r.refused( + "doc", + vec![("plain", t(&"x".repeat(DEFAULT_TEXT_MAX as usize + 1)))], + ); + assert_limit( + &e, + "plain", + &format!( + "at most {DEFAULT_TEXT_MAX} characters; this has {}", + DEFAULT_TEXT_MAX + 1 + ), + ); + // 2,048 characters of four bytes are 8 KiB and still fit: the count is of characters. + r.create( + "doc", + vec![("plain", t(&"\u{1F600}".repeat(DEFAULT_TEXT_MAX as usize)))], + ); + assert_eq!(r.count("doc"), 2); + }); +} + +#[test] +fn a_longtext_field_has_its_own_larger_default_and_a_declared_limit_can_be_lower() { + each(|r| { + r.create( + "doc", + vec![("body", t(&"x".repeat(DEFAULT_LONGTEXT_MAX as usize)))], + ); + let e = r.refused( + "doc", + vec![("body", t(&"x".repeat(DEFAULT_LONGTEXT_MAX as usize + 1)))], + ); + assert_limit( + &e, + "body", + &format!( + "at most {DEFAULT_LONGTEXT_MAX} characters; this has {}", + DEFAULT_LONGTEXT_MAX + 1 + ), + ); + r.create("doc", vec![("tiny", t("12345"))]); + let e = r.refused("doc", vec![("tiny", t("123456"))]); + assert_limit(&e, "tiny", "at most 5 characters; this has 6"); + assert_eq!(r.count("doc"), 2); + }); +} + +#[test] +fn number_money_and_decimal_hold_their_inclusive_bounds() { + each(|r| { + for (field, ok, below, above) in [ + ("qty", [0i64, 100], -1i64, 101i64), + ("price", [0, 100_000], -1, 100_001), + ] { + for v in ok { + r.create("doc", vec![(field, Value::Int(v))]); + } + let (lo, hi) = (ok[0], ok[1]); + let e = r.refused("doc", vec![(field, Value::Int(below))]); + assert_limit(&e, field, &format!("at least {lo}; this is {below}")); + let e = r.refused("doc", vec![(field, Value::Int(above))]); + assert_limit(&e, field, &format!("at most {hi}; this is {above}")); + } + // One bound only. + r.create( + "doc", + vec![("floor", Value::Int(-5)), ("ceiling", Value::Int(5))], + ); + r.create( + "doc", + vec![ + ("floor", Value::Int(i64::MAX)), + ("ceiling", Value::Int(i64::MIN)), + ], + ); + assert_limit( + &r.refused("doc", vec![("floor", Value::Int(-6))]), + "floor", + "at least -5; this is -6", + ); + assert_limit( + &r.refused("doc", vec![("ceiling", Value::Int(6))]), + "ceiling", + "at most 5; this is 6", + ); + assert_eq!(r.count("doc"), 6); + }); +} + +#[test] +fn a_decimal_bound_is_exact_to_the_last_digit() { + each(|r| { + for ok in ["0.1000000", "0.1000001", "0.5", "0.9000000"] { + r.create("doc", vec![("rate", t(ok))]); + } + let e = r.refused("doc", vec![("rate", t("0.0999999"))]); + assert_limit(&e, "rate", "at least 0.1000000; this is 0.0999999"); + let e = r.refused("doc", vec![("rate", t("0.9000001"))]); + assert_limit(&e, "rate", "at most 0.9000000; this is 0.9000001"); + // Written another way: the canonical form is compared, not the text. + let e = r.refused("doc", vec![("rate", t("1"))]); + assert_limit(&e, "rate", "at most 0.9000000; this is 1.0000000"); + let e = r.refused("doc", vec![("rate", t("-0.1"))]); + assert_limit(&e, "rate", "at least 0.1000000; this is -0.1000000"); + assert_eq!(r.count("doc"), 4); + }); +} + +#[test] +fn a_refusal_leaves_the_store_alone() { + each(|r| { + let id = r.create("doc", vec![("title", t("first")), ("qty", Value::Int(1))]); + let before = r.read("doc", &id); + // A create with one bad field among good ones writes nothing. + let e = r.refused("doc", vec![("title", t("fine")), ("qty", Value::Int(1000))]); + assert_limit(&e, "qty", "at most 100; this is 1000"); + assert_eq!(r.count("doc"), 1); + // An update with one bad field among good ones changes nothing. + let Err(CallError::Handler(e)) = r.update( + &id, + vec![("title", t("changed")), ("qty", Value::Int(1000))], + ) else { + panic!("the update should have been refused") + }; + assert_limit(&e, "qty", "at most 100; this is 1000"); + assert_eq!(r.read("doc", &id), before); + }); +} + +#[test] +fn update_holds_a_changed_value_to_the_limit() { + each(|r| { + let id = r.create("doc", vec![("title", t("first"))]); + let Err(CallError::Handler(e)) = r.update(&id, vec![("title", t("abcdefghijk"))]) else { + panic!("refused expected") + }; + assert_limit(&e, "title", "at most 10 characters; this has 11"); + let Err(CallError::Handler(e)) = r.update(&id, vec![("title", t("x"))]) else { + panic!("refused expected") + }; + assert_limit(&e, "title", "at least 2 characters; this has 1"); + r.update(&id, vec![("title", t("second"))]).unwrap(); + assert_eq!(text(get(&r.read("doc", &id), "title")), "second"); + }); +} + +#[test] +fn a_record_stored_above_a_lowered_limit_stays_editable_in_its_other_fields() { + each(|r| { + let id = r.create("doc", vec![("title", t("first")), ("qty", Value::Int(5))]); + // Before the limits existed these were fine. + r.corrupt( + "doc", + &id, + "title", + serde_json::json!("a title far longer than ten"), + ); + r.corrupt("doc", &id, "qty", serde_json::json!(5000)); + r.corrupt( + "doc", + &id, + "tiny", + serde_json::json!("much longer than five"), + ); + + // Another field can be changed ... + r.update(&id, vec![("plain", t("edited"))]).unwrap(); + // ... also by a form that sends the whole record back, unchanged fields included. + r.update( + &id, + vec![ + ("title", t("a title far longer than ten")), + ("qty", Value::Int(5000)), + ("tiny", t("much longer than five")), + ("plain", t("edited again")), + ], + ) + .unwrap(); + let row = r.read("doc", &id); + assert_eq!(text(get(&row, "plain")), "edited again"); + assert_eq!(text(get(&row, "title")), "a title far longer than ten"); + + // A changed value is held to the limit, even when it is still above it. + let Err(CallError::Handler(e)) = + r.update(&id, vec![("title", t("a title far longer than eleven"))]) + else { + panic!("refused expected") + }; + assert_limit(&e, "title", "at most 10 characters; this has 30"); + let Err(CallError::Handler(e)) = r.update(&id, vec![("qty", Value::Int(5001))]) else { + panic!("refused expected") + }; + assert_limit(&e, "qty", "at most 100; this is 5001"); + // Going back inside the limit is always possible. + r.update(&id, vec![("title", t("short")), ("qty", Value::Int(7))]) + .unwrap(); + // The other checks still run on an unchanged value: a stored kind error is still one. + r.corrupt("doc", &id, "qty", serde_json::json!("not a number")); + assert!(r.update(&id, vec![("qty", t("not a number"))]).is_err()); + }); +} + +#[test] +fn bulk_create_names_the_row_and_the_limit_without_the_error_code() { + each(|r| { + let rows = Value::List(vec![ + m(vec![("title", t("fine"))]), + m(vec![("title", t("abcdefghijk"))]), + m(vec![("title", t("fine too")), ("qty", Value::Int(101))]), + m(vec![("rate", t("0.0999999"))]), + ]); + let msg = r.err("doc", "bulk_create", m(vec![("rows", rows.clone())])); + assert!( + msg.contains("3 of 4 row(s) are invalid") && msg.contains("nothing was created"), + "{msg}" + ); + assert!( + msg.contains("row 2") + && msg.contains(".title must be at most 10 characters; this has 11") + && msg.contains("row 3") + && msg.contains(".qty must be at most 100; this is 101") + && msg.contains("row 4") + && msg.contains(".rate must be at least 0.1000000; this is 0.0999999"), + "{msg}" + ); + assert!( + !msg.contains("limit_violated"), + "a row error lists the sentence, not the code: {msg}" + ); + assert_eq!(r.count("doc"), 0); + + // Partial mode keeps the row that is fine and reports the others by number. + let report = r.ok( + "doc", + "bulk_create", + m(vec![("rows", rows.clone()), ("mode", t("partial"))]), + ); + assert_eq!(r.count("doc"), 1); + let errs: Vec<(Value, String)> = list(get(&report, "errors")) + .iter() + .map(|e| (get(e, "row").clone(), text(get(e, "error")))) + .collect(); + assert_eq!( + errs.iter().map(|e| e.0.clone()).collect::>(), + [Value::Int(2), Value::Int(3), Value::Int(4)] + ); + assert!( + errs[0] + .1 + .ends_with("must be at most 10 characters; this has 11"), + "{errs:?}" + ); + assert!( + errs.iter().all(|e| !e.1.contains("limit_violated")), + "{errs:?}" + ); + + // A dry run reports and writes nothing. + let report = r.ok( + "doc", + "bulk_create", + m(vec![ + ("rows", rows), + ("dry_run", Value::Bool(true)), + ("mode", t("partial")), + ]), + ); + assert_eq!(get(&report, "failed"), &Value::Int(3)); + assert_eq!(r.count("doc"), 1); + }); +} + +#[test] +fn csv_rows_go_through_the_same_check() { + each(|r| { + let csv = "title,qty\nfine,5\nabcdefghijk,5\nfine,101\n"; + let msg = r.err("doc", "bulk_create", m(vec![("csv", t(csv))])); + assert!( + msg.contains("row 2") && msg.contains("this has 11"), + "{msg}" + ); + assert!( + msg.contains("row 3") && msg.contains("this is 101"), + "{msg}" + ); + assert_eq!(r.count("doc"), 0); + }); +} + +#[test] +fn what_an_action_writes_is_held_to_the_limits_of_the_target() { + each(|r| { + // `finish` copies `plain` into `log.short`, which holds three characters. + let fits = r.create("doc", vec![("title", t("ok")), ("plain", t("abc"))]); + r.ok("doc", "finish", m(vec![("id", t(&fits))])); + assert_eq!(r.count("log"), 1); + + let id = r.create("doc", vec![("title", t("long")), ("plain", t("abcd"))]); + let e = r.err("doc", "finish", m(vec![("id", t(&id))])); + assert!( + e.contains(".short must be at most 3 characters; this has 4"), + "{e}" + ); + // The action did not run at all: no log, and the state did not move. + assert_eq!(r.count("log"), 1); + assert_eq!(text(get(&r.read("doc", &id), "state")), "new"); + }); +} + +#[test] +fn an_adjustment_cannot_push_a_number_past_its_bound() { + each(|r| { + let doc = r.create("doc", vec![("title", t("stock")), ("qty", Value::Int(90))]); + let c1 = r.create("counter", vec![("doc_id", t(&doc)), ("by", Value::Int(10))]); + r.ok("counter", "apply", m(vec![("id", t(&c1))])); + assert_eq!(get(&r.read("doc", &doc), "qty"), &Value::Int(100)); + + let c2 = r.create("counter", vec![("doc_id", t(&doc)), ("by", Value::Int(1))]); + let e = r.err("counter", "apply", m(vec![("id", t(&c2))])); + assert!(e.contains(".qty must be at most 100; this is 101"), "{e}"); + // Nothing moved: the quantity and the state are as before. + assert_eq!(get(&r.read("doc", &doc), "qty"), &Value::Int(100)); + assert_eq!(text(get(&r.read("counter", &c2), "state")), "open"); + }); +} + +#[test] +fn the_refusal_is_a_400_with_its_own_code_and_a_hint() { + each(|r| { + let msg = r.refused("doc", vec![("title", t("abcdefghijk"))]); + let c = errors::classify(&CallError::Handler(msg.clone())); + assert_eq!((c.status, c.code), (400, "limit_violated")); + // The body carries the sentence without the code, the code and the hint. + assert_eq!( + c.body["error"], + msg.strip_prefix("limit_violated: ").unwrap() + ); + assert_eq!(c.body["code"], "limit_violated"); + assert!( + c.body["hint"].as_str().unwrap().contains("max_length"), + "{}", + c.body + ); + // Every other kind of error still reads as before: no code. + let c = errors::classify(&CallError::Handler("lim_doc.qty must be an integer".into())); + assert_eq!((c.status, c.code), (400, "bad_request")); + assert!(c.body.get("code").is_none(), "{}", c.body); + }); +} + +#[test] +fn the_plain_text_of_a_row_error_drops_only_a_code_of_the_table() { + assert_eq!( + errors::plain("limit_violated: a.b must be at most 1 character; this has 2"), + "a.b must be at most 1 character; this has 2" + ); + assert_eq!(errors::plain("a.b is required"), "a.b is required"); + // A colon in an ordinary message is not a code. + assert_eq!( + errors::plain("note: a.b is required"), + "note: a.b is required" + ); +} diff --git a/crates/kernel/obop-commands/src/public.rs b/crates/kernel/obop-commands/src/public.rs index 5165c467..23e57d44 100644 --- a/crates/kernel/obop-commands/src/public.rs +++ b/crates/kernel/obop-commands/src/public.rs @@ -948,7 +948,7 @@ impl PublicApi { crate::kinds::canonicalise_values(&def, &mut fields, crate::kinds::Path::Write) .map_err(SubmitError::Rejected)?; validate_with(&def, &self.store, &fields, true, &pending) - .map_err(SubmitError::Rejected)?; + .map_err(|e| SubmitError::Rejected(crate::errors::plain(&e).to_string()))?; let id = uuid::Uuid::now_v7().to_string(); pending.push((s.model.clone(), id.clone())); rows.push(( diff --git a/crates/kernel/obop-commands/src/public/tests.rs b/crates/kernel/obop-commands/src/public/tests.rs index cb856541..ed8f50f7 100644 --- a/crates/kernel/obop-commands/src/public/tests.rs +++ b/crates/kernel/obop-commands/src/public/tests.rs @@ -465,3 +465,35 @@ fn site_settings_reads_only_the_site_object() { assert_eq!(site["title"], "Shop"); assert!(!site.contains_key("theme")); } + +#[test] +fn a_visitor_cannot_send_a_text_longer_than_the_field_allows() { + // D90: the anonymous submit path checks what the form of a visitor sends like any other write. + let (api, s) = api(); + let default = obop_registry::DEFAULT_TEXT_MAX as usize; + let ok = msg(&[("email", t("a@b.test")), ("body", t(&"ä".repeat(default)))]); + api.submit_batch("lim1", vec![ok]).unwrap(); + assert_eq!(s.count("msg"), 1); + + let long = msg(&[ + ("email", t("a@b.test")), + ("body", t(&"ä".repeat(default + 1))), + ]); + let e = api.submit_batch("lim2", vec![long]); + let want = format!( + "msg.body must be at most {default} characters; this has {}", + default + 1 + ); + assert!( + matches!(&e, Err(SubmitError::Rejected(m)) if *m == want), + "{e:?}" + ); + // The batch is all or nothing: a good submission next to the long one is not kept either. + let good = msg(&[("email", t("b@b.test"))]); + let long = msg(&[ + ("email", t("a@b.test")), + ("body", t(&"x".repeat(default + 1))), + ]); + assert!(api.submit_batch("lim3", vec![good, long]).is_err()); + assert_eq!(s.count("msg"), 1); +} diff --git a/crates/kernel/obop-commands/src/store.rs b/crates/kernel/obop-commands/src/store.rs index b8273d64..a0b8791f 100644 --- a/crates/kernel/obop-commands/src/store.rs +++ b/crates/kernel/obop-commands/src/store.rs @@ -4,7 +4,7 @@ use crate::engine::{Engine, crud, seq}; use crate::{Dispatcher, Value, guards}; -use obop_registry::{FieldKind, ModelDef, Module, is_valid_date}; +use obop_registry::{FieldDef, FieldKind, ModelDef, Module, is_valid_date}; use obop_storage::blob::{Blobs, Catalog}; use obop_storage::{Backend, Fields}; use std::collections::BTreeMap; @@ -148,20 +148,28 @@ fn check_value( store: &Store, pending: &[(String, String)], model: &str, - name: &str, - kind: &FieldKind, + f: &FieldDef, v: &Value, ) -> Result<(), String> { + let (name, kind) = (f.name, &f.kind); match (kind, v) { - (FieldKind::Text | FieldKind::LongText, Value::Text(_)) => Ok(()), - (FieldKind::Number | FieldKind::Money, Value::Int(_)) => Ok(()), + (FieldKind::Text | FieldKind::LongText, Value::Text(s)) => { + crate::kinds::check_text_limits(model, f, s) + } + (FieldKind::Number | FieldKind::Money, Value::Int(n)) => { + crate::kinds::check_int_limits(model, f, *n) + } (FieldKind::Bool, Value::Bool(_)) => Ok(()), (FieldKind::Image(_) | FieldKind::File(_), Value::Text(id)) => { crate::blobs::check_field_value(store, model, name, kind, id) } (FieldKind::Date, Value::Text(t)) if is_valid_date(t) => Ok(()), (FieldKind::Decimal { .. } | FieldKind::Datetime, Value::Text(t)) => { - crate::kinds::check_canonical(model, name, kind, t) + crate::kinds::check_canonical(model, name, kind, t)?; + if matches!(kind, FieldKind::Decimal { .. }) { + crate::kinds::check_decimal_limits(model, f, t)?; + } + Ok(()) } (FieldKind::Select(o), Value::Text(t)) if o.contains(&t.as_str()) => Ok(()), (FieldKind::Ref(to), Value::Text(id)) @@ -213,7 +221,7 @@ pub fn validate_with( )); } Some(Value::Null) => {} - Some(v) => check_value(store, pending, def.name, f.name, &f.kind, v)?, + Some(v) => check_value(store, pending, def.name, f, v)?, } } Ok(()) From 4f1f043fe341305da54af48caff727bde1ebd04e Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:04 +0100 Subject: [PATCH 03/23] Read the limit keys in the package manifest and hold migration defaults to them (D90) FieldM takes max_length, min_length, min and max (decimal bounds are strings, a TOML float is refused) and reads them into the field, so obop modules check, the install gate and the upgrade diff see them. A changed limit is the listed, never breaking, change kind field_limits, and a set_default value of a migration step must satisfy the limits of its field. Co-Authored-By: Claude Sonnet 5.5 --- crates/kernel/obop-pkg/src/lib.rs | 2 + crates/kernel/obop-pkg/src/limits_tests.rs | 336 +++++++++++++++++++++ crates/kernel/obop-pkg/src/manifest.rs | 188 ++++++++++-- crates/kernel/obop-pkg/src/migrate.rs | 44 ++- 4 files changed, 539 insertions(+), 31 deletions(-) create mode 100644 crates/kernel/obop-pkg/src/limits_tests.rs diff --git a/crates/kernel/obop-pkg/src/lib.rs b/crates/kernel/obop-pkg/src/lib.rs index 3825e277..d0b48dd7 100644 --- a/crates/kernel/obop-pkg/src/lib.rs +++ b/crates/kernel/obop-pkg/src/lib.rs @@ -29,6 +29,8 @@ mod dependency_tests; #[cfg(test)] mod ledger_tests; #[cfg(test)] +mod limits_tests; +#[cfg(test)] mod public_tests; #[cfg(test)] mod update_tests; diff --git a/crates/kernel/obop-pkg/src/limits_tests.rs b/crates/kernel/obop-pkg/src/limits_tests.rs new file mode 100644 index 00000000..b19355b9 --- /dev/null +++ b/crates/kernel/obop-pkg/src/limits_tests.rs @@ -0,0 +1,336 @@ +//! Field limits (D90) in packages: the keys `max_length`, `min_length`, `min` and `max` of +//! `[[model.field]]`, what the manifest parser and the install gate refuse (naming the field and +//! the key), the version a package that uses them declares, and the upgrade diff. + +use crate::manifest::Manifest; +use crate::migrate::diff; +use crate::resolve::{World, plan_install}; +use crate::semver::{Req, Version}; +use crate::{Bundled, Marketplace}; +use obop_registry::{Bound, DEFAULT_TEXT_MAX, check_module}; +use std::collections::BTreeMap; + +/// A package with one model and the given fields; `core` is the `requires_core` it declares. +fn pkg_with_core(core: &str, version: &str, fields: &str) -> String { + format!( + "[module]\nid = \"lim\"\nversion = \"{version}\"\nname = \"Lim\"\ndescription = \"d\"\nlicense = \"MIT\"\nrequires_core = \"{core}\"\n\n[[model]]\nname = \"lim_doc\"\ndescription = \"d\"\nread_group = \"lim.user\"\nwrite_group = \"lim.user\"\n{fields}" + ) +} + +fn pkg(fields: &str) -> String { + pkg_with_core("^0.2.1", "0.1.0", fields) +} + +fn field(name: &str, extra: &str) -> String { + format!( + "\n[[model.field]]\nname = \"{name}\"\ndescription = \"d\"\nclass = \"none\"\n{extra}\n" + ) +} + +/// The error of parsing, building and gating a package, or `None` when it is fine. +fn problem(text: &str) -> Option { + let m = match Manifest::parse(text) { + Ok(m) => m, + Err(e) => return Some(e), + }; + match m.to_module() { + Ok(module) => { + let v = check_module(&module); + (!v.is_empty()).then(|| format!("{v:?}")) + } + Err(e) => Some(e), + } +} + +#[test] +fn every_limit_key_is_read_into_the_field() { + let text = pkg(&[ + field("title", "max_length = 100\nmin_length = 2"), + field("notes", "kind = \"longtext\"\nmax_length = 50000"), + field("qty", "kind = \"number\"\nmin = 0\nmax = 100"), + field("price", "kind = \"money\"\nmin = -500\nmax = 100000"), + field( + "rate", + "kind = \"decimal\"\nprecision = 12\nscale = 7\nmin = \"0.1000000\"\nmax = \"0.9000000\"", + ), + // A whole number is a fine way to write a decimal bound; it takes the scale of the field. + field( + "share", + "kind = \"decimal\"\nprecision = 5\nscale = 2\nmin = 0\nmax = 1", + ), + field("plain", ""), + ] + .concat()); + assert_eq!(problem(&text), None); + let module = Manifest::parse(&text).unwrap().to_module().unwrap(); + let by = |n: &str| { + module.models[0] + .fields + .iter() + .find(|f| f.name == n) + .unwrap() + .clone() + }; + let l = by("title").declared_limits(); + assert_eq!((l.min_length, l.max_length), (Some(2), Some(100))); + assert_eq!(by("title").effective_max_length(), Some(100)); + assert_eq!(by("notes").effective_max_length(), Some(50_000)); + let l = by("qty").declared_limits(); + assert_eq!((l.min, l.max), (Some(Bound::Int(0)), Some(Bound::Int(100)))); + let l = by("price").declared_limits(); + assert_eq!( + (l.min, l.max), + (Some(Bound::Int(-500)), Some(Bound::Int(100_000))) + ); + let l = by("rate").declared_limits(); + assert_eq!( + (l.min.unwrap().to_string(), l.max.unwrap().to_string()), + ("0.1000000".to_string(), "0.9000000".to_string()) + ); + let l = by("share").declared_limits(); + assert_eq!( + (l.min.unwrap().to_string(), l.max.unwrap().to_string()), + ("0.00".to_string(), "1.00".to_string()) + ); + // A field with no key has no declared limit and the default of its kind. + assert!(by("plain").declared_limits().is_empty()); + assert_eq!(by("plain").effective_max_length(), Some(DEFAULT_TEXT_MAX)); +} + +#[test] +fn a_bad_limit_is_refused_naming_the_field_and_the_key() { + // (the field's keys, a piece of the message). Each message names the model and the field. + let cases: &[(&str, &str)] = &[ + ("max_length = 0", "`max_length` must be at least 1"), + ("min_length = 0", "`min_length` must be at least 1"), + ( + "max_length = 10001", + "above the largest a text field may declare, 10000", + ), + ( + "kind = \"longtext\"\nmax_length = 1000001", + "above the largest a longtext field may declare, 1000000", + ), + ( + "min_length = 5\nmax_length = 4", + "`min_length` 5 is above `max_length` 4", + ), + ("min_length = 2049", "above the default maximum"), + ("max_length = -1", "invalid"), + ("max_length = \"100\"", "invalid"), + ("max_length = 1.5", "invalid"), + ("maxlength = 5", "unknown field"), + ( + "kind = \"number\"\nmax_length = 5", + "only valid for kind \"text\" or \"longtext\"", + ), + ( + "kind = \"date\"\nmin = 1", + "only valid for kind \"number\", \"money\" or \"decimal\"", + ), + ( + "min = 1", + "only valid for kind \"number\", \"money\" or \"decimal\"", + ), + ("kind = \"number\"\nmin = 0.5", "must be a whole number"), + ("kind = \"number\"\nmin = \"1\"", "must be a whole number"), + ( + "kind = \"decimal\"\nprecision = 10\nscale = 2\nmax = 1.5", + "a TOML float is not exact", + ), + ( + "kind = \"decimal\"\nprecision = 10\nscale = 2\nmax = \"1.234\"", + "is not a decimal that fits decimal(10, 2)", + ), + ( + "kind = \"decimal\"\nprecision = 4\nscale = 2\nmax = \"1000.00\"", + "is not a decimal that fits decimal(4, 2)", + ), + ( + "kind = \"decimal\"\nprecision = 10\nscale = 2\nmax = \"abc\"", + "is not a decimal", + ), + ( + "kind = \"number\"\nmin = 5\nmax = 4", + "`min` 5 is above `max` 4", + ), + ( + "kind = \"decimal\"\nprecision = 10\nscale = 2\nmin = \"0.11\"\nmax = \"0.10\"", + "`min` 0.11 is above `max` 0.10", + ), + ( + "computed = \"1 + 1\"\nkind = \"number\"\nmax = 5", + "computed or numbered", + ), + ]; + for (keys, want) in cases { + let text = pkg(&field("target", keys)); + let e = problem(&text).unwrap_or_else(|| panic!("{keys:?} should be refused")); + assert!(e.contains(want), "{keys:?}: wanted {want:?}, got {e}"); + // Where it happens: the error does not leave the person looking for the field. + if !e.contains("unknown field") && !e.contains("invalid") { + assert!( + e.contains("target"), + "{keys:?} does not name the field: {e}" + ); + } + } +} + +#[test] +fn the_edges_of_what_is_allowed_are_allowed() { + for keys in [ + "max_length = 1", + "max_length = 10000", + "min_length = 1\nmax_length = 1", + "kind = \"longtext\"\nmax_length = 1000000", + "kind = \"number\"\nmin = 5\nmax = 5", + "kind = \"number\"\nmin = -9223372036854775808\nmax = 9223372036854775807", + "kind = \"decimal\"\nprecision = 4\nscale = 2\nmin = \"-99.99\"\nmax = \"99.99\"", + ] { + assert_eq!(problem(&pkg(&field("target", keys))), None, "{keys}"); + } +} + +#[test] +fn a_default_of_a_migration_step_is_held_to_the_limits_of_its_field() { + let step = |value: &str| { + format!( + "\n[[migration]]\nfrom = \"<0.2\"\nop = \"set_default\"\nmodel = \"lim_doc\"\nfield = \"title\"\nvalue = {value}\n" + ) + }; + let with = |value: &str| { + pkg_with_core( + "^0.2.1", + "0.2.0", + &format!("{}{}", field("title", "max_length = 5"), step(value)), + ) + }; + assert_eq!(problem(&with("\"fits\"")), None); + let e = problem(&with("\"too long\"")).expect("a default of 8 characters into 5"); + assert!( + e.contains("outside the limits of title") && e.contains("at most 5 characters; this has 8"), + "{e}" + ); +} + +// ---- the version a package that uses limits declares -------------------------------------- + +/// D88 Decision 14: a feature that widens what the manifest parser accepts takes the next patch +/// number of the 0.2 line, and a package that uses it declares `^0.2.N`. This is the number of +/// the limits, written out here and not computed from `CORE_VERSION`, so that the core cannot +/// move it by accident. +const LIMITS_CORE_PATCH: u64 = 1; + +#[test] +fn core_version_requirement_kinds_limits() { + let n = LIMITS_CORE_PATCH; + let text = pkg_with_core( + &format!("^0.2.{n}"), + "0.1.0", + &field("title", "max_length = 100"), + ); + let m = Manifest::parse(&text).unwrap(); + let req = m.core_req(); + + // `^0.2.1` is satisfied by the core that has the keys and by the core that is running. + assert!(req.matches(&Version::new(0, 2, n))); + assert!(req.matches(&Version::parse(obop_types::CORE_VERSION).unwrap())); + assert!(Version::parse(obop_types::CORE_VERSION).unwrap() >= Version::new(0, 2, n)); + // It is not satisfied by the core before it, nor by 0.3.0. + assert!(!req.matches(&Version::new(0, 2, n - 1))); + assert!(!req.matches(&Version::new(0, 3, 0))); + + // The resolver turns that into a blocker on the older core, and into a plan on this one. + let market = |_: &str| Ok(None); + let world = |core: Version| World { + core, + builtin: BTreeMap::new(), + installed: BTreeMap::new(), + market: &market, + registered: &|_| false, + }; + let old = plan_install(&world(Version::new(0, 2, n - 1)), &m, "upload"); + assert!(!old.ok()); + assert_eq!( + old.blockers[0].message, + format!("lim 0.1.0 needs core ^0.2.{n}, you run 0.2.{}", n - 1) + ); + let current = plan_install( + &world(Version::parse(obop_types::CORE_VERSION).unwrap()), + &m, + "upload", + ); + assert!(current.ok(), "{:?}", current.blockers); + + // What the parser does not do, said plainly (D88 Decision 14): it does not check that the + // number a package declares is high enough for the keys it uses. A package with `^0.2` and + // a limit parses and gates like any other; the older core fails on the key at parse time. + let lax = pkg_with_core("^0.2", "0.1.0", &field("title", "max_length = 100")); + assert_eq!(problem(&lax), None); +} + +#[test] +fn a_bundled_package_that_uses_a_limit_must_declare_the_core_that_has_them() { + // The 23 bundled packages keep `^0.2` (OR-35 k). Any of them that gains a limit key must + // declare at least the core that has the keys, so that this test fails if one forgets. + let first = Version::new(0, 2, LIMITS_CORE_PATCH); + let before = Version::new(0, 2, LIMITS_CORE_PATCH - 1); + let mut checked = 0; + for listed in Bundled.list().unwrap() { + let m = Manifest::parse(&Bundled.fetch(&listed.id).unwrap()).unwrap(); + let module = m.to_module().unwrap(); + let uses = module + .models + .iter() + .flat_map(|d| &d.fields) + .any(|f| !f.declared_limits().is_empty()); + let req: Req = m.core_req(); + assert!( + req.matches(&Version::parse(obop_types::CORE_VERSION).unwrap()), + "{} does not run on the core", + listed.id + ); + if uses { + assert!( + req.matches(&first) && !req.matches(&before), + "{} uses field limits and declares {}", + listed.id, + m.module.requires_core + ); + } + checked += 1; + } + assert_eq!(checked, 23); +} + +// ---- upgrades ----------------------------------------------------------------------------- + +#[test] +fn a_changed_limit_is_a_listed_change_and_never_breaking() { + let old = Manifest::parse(&pkg(&field("title", "max_length = 100"))).unwrap(); + let kinds = |a: &Manifest, b: &Manifest| -> Vec<(String, bool)> { + diff(a, b) + .into_iter() + .map(|c| (format!("{}:{}", c.kind, c.detail), c.breaking)) + .collect() + }; + // Lowering a limit may leave a stored record above it; that is not breaking: such a record + // stays editable in its other fields (D90 Decision 6). + let lower = Manifest::parse(&pkg(&field("title", "max_length = 10"))).unwrap(); + let c = kinds(&old, &lower); + assert_eq!(c.len(), 1, "{c:?}"); + assert!(c[0].0.starts_with("field_limits:"), "{c:?}"); + assert!(c[0].0.contains("100") && c[0].0.contains("10"), "{c:?}"); + assert!(!c[0].1, "{c:?}"); + // Adding one, and dropping one, are listed too. + let none = Manifest::parse(&pkg(&field("title", ""))).unwrap(); + assert_eq!(kinds(&none, &old).len(), 1); + assert_eq!(kinds(&old, &none).len(), 1); + // The same limits are no change. + assert!(kinds(&old, &old).is_empty()); + // A number bound is listed as well. + let a = Manifest::parse(&pkg(&field("qty", "kind = \"number\"\nmax = 10"))).unwrap(); + let b = Manifest::parse(&pkg(&field("qty", "kind = \"number\"\nmax = 5"))).unwrap(); + assert_eq!(kinds(&a, &b).len(), 1); +} diff --git a/crates/kernel/obop-pkg/src/manifest.rs b/crates/kernel/obop-pkg/src/manifest.rs index 925da8c2..a704e9e8 100644 --- a/crates/kernel/obop-pkg/src/manifest.rs +++ b/crates/kernel/obop-pkg/src/manifest.rs @@ -8,9 +8,9 @@ use obop_commands::{PublicRead, PublicReceipt, PublicSpec, PublicSubmit}; use obop_registry::labels::{LangLabels, ModelShape, Shape, Translations, check_labels}; use obop_registry::rules::{MAX_ASSERT_KEY, MAX_EFFECTS, SeqFormat, assert_key_ok}; use obop_registry::{ - Access, Assertion, CommandDef, Computed, Effect, EffectKind, FieldDef, FieldKind, - GENERATED_COMMANDS, History, Lit, Locator, MATURITIES, MODULE_CATEGORIES, ModelDef, ModelExt, - Module, ModuleMeta, Reset, SequenceSpec, ShowsOption, Src, Target, Unique, + Access, Assertion, Bound, CommandDef, Computed, Effect, EffectKind, FieldDef, FieldKind, + GENERATED_COMMANDS, History, Limits, Lit, Locator, MATURITIES, MODULE_CATEGORIES, ModelDef, + ModelExt, Module, ModuleMeta, Reset, SequenceSpec, ShowsOption, Src, Target, Unique, check_installed_module, check_module, intern, }; use obop_types::DataClass; @@ -284,6 +284,114 @@ pub struct FieldM { /// browser evaluates) on the same record. The label shows only while it holds, because the /// field may also hold free text. pub when: Option, + /// `kind = "text"` or `"longtext"` (D90): the most characters (Unicode scalar values, not + /// bytes) a written value has, 1 to 10,000 for `text` and 1 to 1,000,000 for `longtext`. + /// Without it the field has the default of its kind (2,048 and 100,000). + pub max_length: Option, + /// `kind = "text"` or `"longtext"` (D90): the fewest characters a written value has, at + /// least 1 and not above `max_length`. A value is counted as stored: nothing is trimmed. + pub min_length: Option, + /// `kind = "number"`, `"money"` or `"decimal"` (D90): the smallest value, inclusive. A whole + /// number for `number` and `money`; for `decimal` a string with at most `scale` fractional + /// digits (`"0.10"`), because a TOML float is not exact. + pub min: Option, + /// As `min`: the largest value, inclusive. + pub max: Option, +} + +/// The registry kind of a manifest field (`sequence` is a text field that is numbered). +fn kind_of(f: &FieldM) -> FieldKind { + match f.kind.as_str() { + "longtext" => FieldKind::LongText, + "number" => FieldKind::Number, + "money" => FieldKind::Money, + "date" => FieldKind::Date, + "datetime" => FieldKind::Datetime, + "decimal" => FieldKind::Decimal { + precision: f.precision.unwrap_or(0) as u8, + scale: f.scale.unwrap_or(0) as u8, + round: f.round.as_deref().and_then(obop_values::Round::parse), + }, + "bool" => FieldKind::Bool, + "select" => FieldKind::Select( + f.options + .iter() + .map(|o| intern(o)) + .collect::>() + .into(), + ), + "ref" => FieldKind::Ref(intern(f.to.as_deref().unwrap_or(""))), + "image" => FieldKind::Image(f.max_bytes.unwrap_or(0)), + "file" => FieldKind::File(f.max_bytes.unwrap_or(0)), + _ => FieldKind::Text, + } +} + +/// One bound of `min` or `max` (D90) as the registry holds it. A `number` or `money` bound is a +/// TOML integer; a `decimal` bound is a string (or an integer) that fits the field's precision, +/// with at most `scale` fractional digits, and is kept at the scale of the field. A float is +/// refused: it is not exact. +fn bound_of(key: &str, v: &toml::Value, f: &FieldM) -> Result { + use toml::Value as T; + match (f.kind.as_str(), v) { + ("number" | "money", T::Integer(n)) => Ok(Bound::Int(*n)), + ("number" | "money", T::Float(_)) => Err(format!( + "`{key}` must be a whole number for kind {:?}, not a float", + f.kind + )), + ("number" | "money", _) => Err(format!( + "`{key}` must be a whole number (a TOML integer without quotes) for kind {:?}", + f.kind + )), + ("decimal", T::Float(_)) => Err(format!( + "`{key}` of a decimal field must be written as a string such as \"0.10\": a TOML float is not exact" + )), + ("decimal", T::String(_) | T::Integer(_)) => { + let text = match v { + T::String(s) => s.clone(), + other => other.to_string(), + }; + let (Some(p), Some(sc)) = (f.precision, f.scale) else { + return Err(format!( + "`{key}` needs the precision and scale of the decimal" + )); + }; + obop_values::Decimal::parse_for(&text, p as u8, sc as u8, None) + .map(Bound::Dec) + .map_err(|e| { + format!( + "`{key}` {text:?} is not a decimal that fits decimal({p}, {sc}) with at most {sc} digits after the point: {e}" + ) + }) + } + ("decimal", _) => Err(format!( + "`{key}` of a decimal field must be a string such as \"0.10\"" + )), + _ => Err(format!( + "`min` and `max` are only valid for kind \"number\", \"money\" or \"decimal\", not {:?}", + f.kind + )), + } +} + +/// The limits a field declares (D90), parsed; their meaning (which kind they fit, `min` above +/// `max`, a length above the cap) is the D41 gate's, which `field_def` runs on the result. +fn limits_of(f: &FieldM) -> Result { + Ok(Limits { + min_length: f.min_length, + max_length: f.max_length, + min: f.min.as_ref().map(|v| bound_of("min", v, f)).transpose()?, + max: f.max.as_ref().map(|v| bound_of("max", v, f)).transpose()?, + }) +} + +/// A field of `f`'s kind and limits, to hold a value against (`set_default`). +fn probe(f: &FieldM) -> Result { + Ok( + FieldDef::new(intern(&f.name), "", DataClass::None, false, "") + .kind(kind_of(f)) + .with_limits(limits_of(f)?), + ) } /// Whether `v` is a value the field can hold, for `set_default`. @@ -312,7 +420,7 @@ fn default_matches(f: &FieldM, v: &toml::Value) -> Result<(), String> { _ => false, }; if ok { - Ok(()) + limits_hold(f, v) } else { Err(format!( "the value does not fit the {} field {}{}", @@ -329,6 +437,41 @@ fn default_matches(f: &FieldM, v: &toml::Value) -> Result<(), String> { } } +/// A `set_default` value is held to the limits of its field like any write (D90): a step cannot +/// store what the field would refuse. A field whose limits do not parse is reported where its +/// limits are read, not here. +fn limits_hold(f: &FieldM, v: &toml::Value) -> Result<(), String> { + use toml::Value as T; + let Ok(d) = probe(f) else { return Ok(()) }; + let refused = |b: obop_registry::Breach, shown: &str| { + format!( + "the value is outside the limits of {}: it must be {}", + f.name, + b.detail(shown) + ) + }; + match (f.kind.as_str(), v) { + ("text" | "longtext", T::String(s)) => d.check_text(s).map_err(|b| refused(b, "")), + ("number" | "money", T::Integer(n)) => { + d.check_int(*n).map_err(|b| refused(b, &n.to_string())) + } + ("decimal", T::String(_) | T::Integer(_)) => { + let (Some(p), Some(sc)) = (f.precision, f.scale) else { + return Ok(()); + }; + let text = match v { + T::String(s) => s.clone(), + other => other.to_string(), + }; + match obop_values::Decimal::parse_for(&text, p as u8, sc as u8, None) { + Ok(x) => d.check_decimal(x).map_err(|b| refused(b, &x.to_string())), + Err(_) => Ok(()), + } + } + _ => Ok(()), + } +} + fn default_kind() -> String { "text".into() } @@ -1555,30 +1698,8 @@ impl Manifest { fn field_def(&self, m: &ModelM, f: &FieldM) -> Result { let at = format!("{}.{}", m.name, f.name); - let kind = match f.kind.as_str() { - "longtext" => FieldKind::LongText, - "number" => FieldKind::Number, - "money" => FieldKind::Money, - "date" => FieldKind::Date, - "datetime" => FieldKind::Datetime, - "decimal" => FieldKind::Decimal { - precision: f.precision.unwrap_or(0) as u8, - scale: f.scale.unwrap_or(0) as u8, - round: f.round.as_deref().and_then(obop_values::Round::parse), - }, - "bool" => FieldKind::Bool, - "select" => FieldKind::Select( - f.options - .iter() - .map(|o| intern(o)) - .collect::>() - .into(), - ), - "ref" => FieldKind::Ref(intern(f.to.as_deref().unwrap_or(""))), - "image" => FieldKind::Image(f.max_bytes.unwrap_or(0)), - "file" => FieldKind::File(f.max_bytes.unwrap_or(0)), - _ => FieldKind::Text, - }; + let kind = kind_of(f); + let limits = limits_of(f).map_err(|e| format!("{at}: {e}"))?; let sequence = if f.kind == "sequence" { let format = SeqFormat::parse(f.format.as_deref().unwrap_or_default()) .map_err(|e| format!("{at}: format: {e}"))?; @@ -1619,7 +1740,7 @@ impl Manifest { )), None => None, }; - Ok(FieldDef { + let def = FieldDef { name: intern(&f.name), description: intern(&f.description), class: DataClass::parse(&f.class), @@ -1633,7 +1754,14 @@ impl Manifest { }), sequence, shows_option, - }) + limits: (!limits.is_empty()).then(|| std::sync::Arc::new(limits)), + }; + // The limits are checked here as well as by the gate, so the error of a bad one names the + // file's field before anything else is parsed (D90). + if let Some(problem) = obop_registry::limits::problems(&def).into_iter().next() { + return Err(format!("{at}: {problem}")); + } + Ok(def) } /// The models as registry data, rules included. Syntax errors in expressions, formats and diff --git a/crates/kernel/obop-pkg/src/migrate.rs b/crates/kernel/obop-pkg/src/migrate.rs index 3b6bfaa0..7150d5d1 100644 --- a/crates/kernel/obop-pkg/src/migrate.rs +++ b/crates/kernel/obop-pkg/src/migrate.rs @@ -32,7 +32,7 @@ use std::collections::BTreeMap; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Change { /// `model_added`, `model_removed`, `field_added`, `field_removed`, `field_retyped`, - /// `field_reshaped`, `field_required`, `option_added`, `option_removed`, `action_added`, + /// `field_reshaped`, `field_limits`, `field_required`, `option_added`, `option_removed`, `action_added`, /// `action_removed`. pub kind: &'static str, pub model: String, @@ -44,6 +44,32 @@ pub struct Change { pub resolved_by: Option, } +/// The limits a field declares (D90) as one line, `none` when it declares none. +fn limits_text(f: &FieldM) -> String { + let mut parts = Vec::new(); + let show = |v: &toml::Value| match v { + toml::Value::String(s) => s.clone(), + other => other.to_string(), + }; + if let Some(n) = f.min_length { + parts.push(format!("min_length {n}")); + } + if let Some(n) = f.max_length { + parts.push(format!("max_length {n}")); + } + if let Some(v) = &f.min { + parts.push(format!("min {}", show(v))); + } + if let Some(v) = &f.max { + parts.push(format!("max {}", show(v))); + } + if parts.is_empty() { + "none".into() + } else { + parts.join(", ") + } +} + /// The structural difference between the installed manifest and the new one. pub fn diff(old: &Manifest, new: &Manifest) -> Vec { let mut out = Vec::new(); @@ -121,6 +147,22 @@ pub fn diff(old: &Manifest, new: &Manifest) -> Vec { of.scale != nf.scale || nf.precision < of.precision, )); } + // Limits (D90) only tighten or loosen what is written from now on; stored + // values are not re-checked, so this is shown and never blocks. + if limits_text(of) != limits_text(nf) { + out.push(ch( + "field_limits", + &nm.name, + Some(&nf.name), + format!( + "{}: limits change from {} to {}; values already stored are not re-checked", + nf.name, + limits_text(of), + limits_text(nf) + ), + false, + )); + } if nf.required && !of.required && nf.computed.is_none() { out.push(ch( "field_required", From 0467d87d5fdd9da6ddb80891e1c000f1454fb2ee Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:05 +0100 Subject: [PATCH 04/23] Show field limits in the MCP tool schemas and the OpenAPI document (D90) maxLength, minLength, minimum and maximum on the input schemas of the write commands (decimals as x-obop-minimum and x-obop-maximum, because a decimal is a string), none on record schemas and none on personal-data fields in MCP, where an agent sees a mask token. A sentence is added to the field description only for a limit the author declared. Co-Authored-By: Claude Sonnet 5.5 --- crates/kernel/obop-http/src/openapi.rs | 40 +++++++- crates/kernel/obop-mcp/Cargo.toml | 1 + crates/kernel/obop-mcp/src/lib.rs | 14 +++ crates/kernel/obop-mcp/src/schema.rs | 128 ++++++++++++++++++++++++- 4 files changed, 181 insertions(+), 2 deletions(-) diff --git a/crates/kernel/obop-http/src/openapi.rs b/crates/kernel/obop-http/src/openapi.rs index c59cf069..a5b828e1 100644 --- a/crates/kernel/obop-http/src/openapi.rs +++ b/crates/kernel/obop-http/src/openapi.rs @@ -5,6 +5,32 @@ use obop_commands::ModuleInfo; use serde_json::{Value as J, json}; +/// The length and range limits of a field (D90) as JSON Schema keywords of its input schema: +/// `maxLength` and `minLength`, `minimum` and `maximum`. A decimal is a string, which JSON Schema +/// cannot bound by value, so its bounds are the extensions `x-obop-minimum` and `x-obop-maximum` +/// (the canonical text). The same as the MCP tool schemas. +fn limit_keywords(f: &obop_commands::FieldInfo) -> serde_json::Map { + let mut kw = serde_json::Map::new(); + if let Some(n) = f.max_length { + kw.insert("maxLength".into(), json!(n)); + } + if let Some(n) = f.min_length { + kw.insert("minLength".into(), json!(n)); + } + let (lo, hi) = if f.kind == "decimal" { + ("x-obop-minimum", "x-obop-maximum") + } else { + ("minimum", "maximum") + }; + if let Some(b) = f.min { + kw.insert(lo.into(), obop_mcp::schema::bound_json(b)); + } + if let Some(b) = f.max { + kw.insert(hi.into(), obop_mcp::schema::bound_json(b)); + } + kw +} + fn error_schema() -> J { json!({ "type": "object", @@ -221,6 +247,10 @@ pub fn instance(base: &str, modules: &[ModuleInfo]) -> J { let name = &model.name; let mut props = serde_json::Map::new(); let mut required = Vec::new(); + // The limits of the fields (D90) are keywords of the input schemas only: a record that was + // stored before a limit was lowered still has to validate as a response. + let mut limit_kw: std::collections::HashMap> = + Default::default(); for f in &model.fields { let options: Vec = f.options.iter().map(|o| o.to_string()).collect(); let mut sch = match f.kind { @@ -275,6 +305,8 @@ pub fn instance(base: &str, modules: &[ModuleInfo]) -> J { desc.push_str(" (a number the system assigns; read-only)"); sch["readOnly"] = J::from(true); } + desc.push_str(&f.limits_note); + limit_kw.insert(f.name.to_string(), limit_keywords(f)); if f.unique && !f.sequence { desc.push_str(" (unique)"); } @@ -293,7 +325,13 @@ pub fn instance(base: &str, modules: &[ModuleInfo]) -> J { let input_props: serde_json::Map = props .iter() .filter(|(_, v)| v.get("readOnly") != Some(&J::from(true))) - .map(|(k, v)| (k.clone(), v.clone())) + .map(|(k, v)| { + let mut v = v.clone(); + for (kw, value) in limit_kw.get(k).into_iter().flatten() { + v[kw] = value.clone(); + } + (k.clone(), v) + }) .collect(); schemas.insert(name.clone(), rec.clone()); let op = |id: &str, summary: &str, body: Option, resp: J| { diff --git a/crates/kernel/obop-mcp/Cargo.toml b/crates/kernel/obop-mcp/Cargo.toml index 44014deb..c076f645 100644 --- a/crates/kernel/obop-mcp/Cargo.toml +++ b/crates/kernel/obop-mcp/Cargo.toml @@ -14,6 +14,7 @@ serde_json = "1" [dev-dependencies] obop-registry.workspace = true +obop-values.workspace = true [lints] workspace = true diff --git a/crates/kernel/obop-mcp/src/lib.rs b/crates/kernel/obop-mcp/src/lib.rs index 6872459a..3754cfb8 100644 --- a/crates/kernel/obop-mcp/src/lib.rs +++ b/crates/kernel/obop-mcp/src/lib.rs @@ -406,6 +406,20 @@ pub fn meta_json(d: &Dispatcher, surface: Surface, p: &Principal) -> J { if let Some(n) = f.max_bytes { o["max_bytes"] = json!(n); } + // Length and range limits (D90). `max_length` is always there for + // a text field: the field's own, else the default of the kind. + if let Some(n) = f.max_length { + o["max_length"] = json!(n); + } + if let Some(n) = f.min_length { + o["min_length"] = json!(n); + } + if let Some(b) = f.min { + o["min"] = schema::bound_json(b); + } + if let Some(b) = f.max { + o["max"] = schema::bound_json(b); + } if f.kind == "decimal" { o["precision"] = json!(f.precision); o["scale"] = json!(f.scale); diff --git a/crates/kernel/obop-mcp/src/schema.rs b/crates/kernel/obop-mcp/src/schema.rs index 0060ffdf..c20450f9 100644 --- a/crates/kernel/obop-mcp/src/schema.rs +++ b/crates/kernel/obop-mcp/src/schema.rs @@ -2,7 +2,9 @@ //! command, named `__`, with a JSON Schema input, an output schema and //! annotations. Both transports list tools through this module, so they cannot diverge. -use obop_commands::{CommandInfo, Dispatcher, FieldInfo, ModelInfo, Principal, Surface, names}; +use obop_commands::{ + Bound, CommandInfo, Dispatcher, FieldInfo, ModelInfo, Principal, Surface, names, +}; use serde_json::{Value as J, json}; use std::collections::HashMap; @@ -25,6 +27,7 @@ pub fn field_schema(f: &FieldInfo) -> J { "select" => json!({"type": "string", "enum": f.options}), _ => json!({"type": "string"}), }; + limit_keywords(&mut s, f); let mut desc = f.description.to_string(); match f.kind { "money" => desc.push_str(" (integer, minor currency units such as cents; never a float)"), @@ -47,6 +50,7 @@ pub fn field_schema(f: &FieldInfo) -> J { ), _ => {} } + desc.push_str(&f.limits_note); if let Some(of) = &f.shows_option_of { desc.push_str(&format!( " (may hold an option key of {of}; the back office shows that option's label, the stored text is the key)" @@ -75,6 +79,42 @@ pub fn field_schema(f: &FieldInfo) -> J { s } +/// A bound as JSON: a number for `number` and `money`, the canonical text for `decimal`. +pub fn bound_json(b: Bound) -> J { + match b { + Bound::Int(n) => json!(n), + Bound::Dec(d) => json!(d.to_string()), + } +} + +/// The length and range limits of a field (D90) as JSON Schema keywords: `maxLength` and +/// `minLength` on a string, `minimum` and `maximum` on an integer. A decimal is a string, which +/// JSON Schema cannot bound by value, so its bounds are the extensions `x-obop-minimum` and +/// `x-obop-maximum` (the canonical text). Fields of personal data get none: an agent sees them as +/// mask tokens (`PERSON_1`), which are not the length or the size of the value. +fn limit_keywords(s: &mut J, f: &FieldInfo) { + if f.personal_data { + return; + } + if let Some(n) = f.max_length { + s["maxLength"] = json!(n); + } + if let Some(n) = f.min_length { + s["minLength"] = json!(n); + } + let (lo, hi) = if f.kind == "decimal" { + ("x-obop-minimum", "x-obop-maximum") + } else { + ("minimum", "maximum") + }; + if let Some(b) = f.min { + s[lo] = bound_json(b); + } + if let Some(b) = f.max { + s[hi] = bound_json(b); + } +} + fn id_property(model: &str) -> J { json!({"type": "string", "description": format!("The id of the {model} record.")}) } @@ -378,6 +418,11 @@ mod tests { sequence: false, shows_option_of: None, shows_option_when: None, + max_length: None, + min_length: None, + min: None, + max: None, + limits_note: String::new(), } } @@ -466,6 +511,11 @@ mod tests { sequence: false, shows_option_of: None, shows_option_when: None, + max_length: None, + min_length: None, + min: None, + max: None, + limits_note: String::new(), }; ModelInfo { name: "doc".into(), @@ -550,4 +600,80 @@ mod tests { ); assert!(output_schema("search")["properties"]["value"]["properties"]["total"].is_object()); } + + fn limited(kind: &'static str, edit: impl FnOnce(&mut FieldInfo)) -> J { + let mut f = field(kind); + edit(&mut f); + field_schema(&f) + } + + #[test] + fn a_text_field_says_its_length_limits() { + let s = limited("text", |f| { + f.max_length = Some(2000); + f.min_length = Some(1); + f.limits_note = " (from 1 character to 2000 characters)".into(); + }); + assert_eq!( + (s["maxLength"].clone(), s["minLength"].clone()), + (json!(2000), json!(1)) + ); + assert!( + s["description"] + .as_str() + .unwrap() + .starts_with("a field (from 1 character to 2000 characters)") + ); + // The default is a keyword too (the tool schema is where an agent looks), but it adds + // no words to the description. + let s = limited("longtext", |f| f.max_length = Some(100_000)); + assert_eq!(s["maxLength"], 100_000); + assert!(s.get("minLength").is_none()); + assert_eq!(s["description"], "a field"); + } + + #[test] + fn a_number_says_its_range_and_a_decimal_does_it_as_exact_strings() { + for kind in ["number", "money"] { + let s = limited(kind, |f| { + f.min = Some(Bound::Int(-5)); + f.max = Some(Bound::Int(100)); + }); + assert_eq!( + (s["minimum"].clone(), s["maximum"].clone()), + (json!(-5), json!(100)), + "{kind}" + ); + assert!(s.get("x-obop-minimum").is_none()); + } + let d = |s: &str| Bound::Dec(obop_values::Decimal::parse_for(s, 12, 2, None).unwrap()); + let s = limited("decimal", |f| { + f.precision = Some(12); + f.scale = Some(2); + f.min = Some(d("0.10")); + f.max = Some(d("99.90")); + }); + assert_eq!( + (s["x-obop-minimum"].clone(), s["x-obop-maximum"].clone()), + (json!("0.10"), json!("99.90")) + ); + assert!( + s.get("minimum").is_none() && s.get("maximum").is_none(), + "{s}" + ); + } + + #[test] + fn personal_data_gets_no_keyword_because_an_agent_sees_a_token() { + let s = limited("text", |f| { + f.personal_data = true; + f.class = "internal"; + f.max_length = Some(5); + f.min_length = Some(3); + }); + assert!( + s.get("maxLength").is_none() && s.get("minLength").is_none(), + "{s}" + ); + } } From 364ad5c20df7fd635ae2f9891809f72da3ab325a Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:24 +0100 Subject: [PATCH 05/23] Take core version 0.2.1 for the new manifest keys, with the pins that follow (D90) The field-limit keys widen what the manifest parser accepts, so by the version rule of D88 Decision 14 this is the first patch number of the 0.2 line. A package that declares a limit writes requires_core = "^0.2.1"; the 23 bundled packages keep ^0.2. Changes every pin of the literal 0.2.0 that D88 Decision 14 lists: the core-version tests, the committed openapi.json (regenerated, with the maxLength keywords), the website snapshots, the e2e assertions and the examples in DEPLOY.md and spec/cli.md. Co-Authored-By: Claude Sonnet 5.5 --- Cargo.lock | 59 ++++---- Cargo.toml | 2 +- crates/kernel/obop-ops/src/backup.rs | 2 +- crates/obop/src/build_commit.rs | 2 +- crates/obop/src/health_tests.rs | 2 +- crates/obop/src/modules_tests.rs | 12 +- crates/obop/tests/version.rs | 4 +- docs/DEPLOY.md | 2 +- docs/spec/cli.md | 2 +- web/apps/site/data/builtin/base.toml | 2 +- web/apps/site/data/builtin/connectors.toml | 2 +- web/apps/site/data/builtin/crm.toml | 2 +- web/apps/site/data/builtin/invoicing.toml | 2 +- web/apps/site/data/builtin/product.toml | 2 +- web/apps/site/data/builtin/sales.toml | 2 +- web/packages/client/openapi.json | 164 ++++++++++++++++++++- web/packages/e2e/tests/modules.spec.ts | 12 +- 17 files changed, 219 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4240593..66c1d942 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -131,7 +131,7 @@ dependencies = [ [[package]] name = "base" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-masking", @@ -299,7 +299,7 @@ checksum = "6e8ccc4ea9f6acc32d102c0f6d471d11d913ad15f20c04de743374861fa1d414" [[package]] name = "connectors" -version = "0.2.0" +version = "0.2.1" dependencies = [ "base", "crm", @@ -339,7 +339,7 @@ dependencies = [ [[package]] name = "crm" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-registry", @@ -986,7 +986,7 @@ dependencies = [ [[package]] name = "invoicing" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-registry", @@ -1150,7 +1150,7 @@ checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" [[package]] name = "mileage" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-registry", @@ -1252,7 +1252,7 @@ dependencies = [ [[package]] name = "obop" -version = "0.2.0" +version = "0.2.1" dependencies = [ "axum", "base", @@ -1293,7 +1293,7 @@ dependencies = [ [[package]] name = "obop-auth" -version = "0.2.0" +version = "0.2.1" dependencies = [ "argon2", "getrandom 0.4.3", @@ -1309,7 +1309,7 @@ dependencies = [ [[package]] name = "obop-commands" -version = "0.2.0" +version = "0.2.1" dependencies = [ "icu_collator", "icu_locale", @@ -1328,7 +1328,7 @@ dependencies = [ [[package]] name = "obop-community" -version = "0.2.0" +version = "0.2.1" dependencies = [ "ammonia", "form_urlencoded", @@ -1345,7 +1345,7 @@ dependencies = [ [[package]] name = "obop-connectors" -version = "0.2.0" +version = "0.2.1" dependencies = [ "base64 0.22.1", "chacha20poly1305", @@ -1368,7 +1368,7 @@ dependencies = [ [[package]] name = "obop-http" -version = "0.2.0" +version = "0.2.1" dependencies = [ "axum", "getrandom 0.4.3", @@ -1394,7 +1394,7 @@ dependencies = [ [[package]] name = "obop-keys" -version = "0.2.0" +version = "0.2.1" dependencies = [ "chacha20poly1305", "getrandom 0.4.3", @@ -1407,7 +1407,7 @@ dependencies = [ [[package]] name = "obop-masking" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-types", "serde_json", @@ -1415,24 +1415,25 @@ dependencies = [ [[package]] name = "obop-mcp" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-registry", "obop-types", + "obop-values", "serde_json", ] [[package]] name = "obop-media" -version = "0.2.0" +version = "0.2.1" dependencies = [ "image", ] [[package]] name = "obop-migrate" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-keys", "obop-storage", @@ -1444,7 +1445,7 @@ dependencies = [ [[package]] name = "obop-oauth" -version = "0.2.0" +version = "0.2.1" dependencies = [ "getrandom 0.4.3", "obop-auth", @@ -1457,7 +1458,7 @@ dependencies = [ [[package]] name = "obop-ops" -version = "0.2.0" +version = "0.2.1" dependencies = [ "base64 0.22.1", "flate2", @@ -1474,7 +1475,7 @@ dependencies = [ [[package]] name = "obop-payments" -version = "0.2.0" +version = "0.2.1" dependencies = [ "base", "getrandom 0.4.3", @@ -1492,7 +1493,7 @@ dependencies = [ [[package]] name = "obop-pkg" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-migrate", @@ -1508,7 +1509,7 @@ dependencies = [ [[package]] name = "obop-platform" -version = "0.2.0" +version = "0.2.1" dependencies = [ "getrandom 0.4.3", "obop-auth", @@ -1527,7 +1528,7 @@ dependencies = [ [[package]] name = "obop-registry" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-types", "obop-values", @@ -1537,7 +1538,7 @@ dependencies = [ [[package]] name = "obop-site" -version = "0.2.0" +version = "0.2.1" dependencies = [ "ammonia", "base", @@ -1555,7 +1556,7 @@ dependencies = [ [[package]] name = "obop-storage" -version = "0.2.0" +version = "0.2.1" dependencies = [ "postgres", "rusqlite", @@ -1569,11 +1570,11 @@ dependencies = [ [[package]] name = "obop-types" -version = "0.2.0" +version = "0.2.1" [[package]] name = "obop-update" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-keys", @@ -1588,7 +1589,7 @@ dependencies = [ [[package]] name = "obop-values" -version = "0.2.0" +version = "0.2.1" dependencies = [ "jiff", "jiff-tzdb", @@ -1831,7 +1832,7 @@ dependencies = [ [[package]] name = "product" -version = "0.2.0" +version = "0.2.1" dependencies = [ "obop-commands", "obop-registry", @@ -2010,7 +2011,7 @@ checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "sales" -version = "0.2.0" +version = "0.2.1" dependencies = [ "invoicing", "obop-commands", diff --git a/Cargo.toml b/Cargo.toml index da57b454..06199536 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = ["crates/kernel/*", "crates/modules/*", "crates/obop"] edition = "2024" rust-version = "1.99" publish = false -version = "0.2.0" +version = "0.2.1" license = "Apache-2.0" repository = "https://github.com/SpaceCorps/obop" diff --git a/crates/kernel/obop-ops/src/backup.rs b/crates/kernel/obop-ops/src/backup.rs index fe6a7401..38222c17 100644 --- a/crates/kernel/obop-ops/src/backup.rs +++ b/crates/kernel/obop-ops/src/backup.rs @@ -6,7 +6,7 @@ //! at a time, so a large database never sits in memory and a running server is not stopped: //! //! ```text -//! {"type":"header","format":1,"core_version":"0.2.0","schema_version":2,"created_at":"...","source":"postgres"} +//! {"type":"header","format":1,"core_version":"0.2.1","schema_version":2,"created_at":"...","source":"postgres"} //! {"m":"","i":"","d":{...fields...}} one line per record, in insertion order //! {"blob":"","ct":"image/png","w":640,"h":480,"created":1,"x":""} //! {"type":"end","records":N,"blobs":M,"content_sha256":"..."} diff --git a/crates/obop/src/build_commit.rs b/crates/obop/src/build_commit.rs index a0bfeb2a..afc82d36 100644 --- a/crates/obop/src/build_commit.rs +++ b/crates/obop/src/build_commit.rs @@ -27,7 +27,7 @@ pub fn short(raw: &str) -> Option { digits.then(|| raw[..SHORT].to_ascii_lowercase()) } -/// The line `obop --version` prints: `obop 0.2.0 (abcdef012345)`. The prefix `obop 0.2.0` is the +/// The line `obop --version` prints: `obop 0.2.1 (abcdef012345)`. The prefix `obop 0.2.1` is the /// one earlier builds printed alone. pub fn version_line(name: &str, version: &str, commit: &str) -> String { format!("{name} {version} ({commit})") diff --git a/crates/obop/src/health_tests.rs b/crates/obop/src/health_tests.rs index 64744c4a..38a7ab7b 100644 --- a/crates/obop/src/health_tests.rs +++ b/crates/obop/src/health_tests.rs @@ -148,7 +148,7 @@ async fn health_endpoints_are_open_and_reveal_nothing_but_status_and_a_fixed_rea for path in ["/healthz", "/readyz"] { let r = send(&f.router, get(path)).await; for secret in [ - "ada", "sqlite", "memory", "postgres", "obop_", "0.2.0", "token", + "ada", "sqlite", "memory", "postgres", "obop_", "0.2.1", "token", ] { assert!( !r.body.contains(secret), diff --git a/crates/obop/src/modules_tests.rs b/crates/obop/src/modules_tests.rs index 47f87357..d42302c1 100644 --- a/crates/obop/src/modules_tests.rs +++ b/crates/obop/src/modules_tests.rs @@ -58,7 +58,7 @@ fn every_existing_model_has_the_default_extensions() { #[test] fn the_core_version_is_the_workspace_version() { assert_eq!(obop_types::CORE_VERSION, env!("CARGO_PKG_VERSION")); - assert_eq!(obop_types::CORE_VERSION, "0.2.0"); + assert_eq!(obop_types::CORE_VERSION, "0.2.1"); let v = Version::parse(obop_types::CORE_VERSION).unwrap(); // The compatibility policy: while below 1.0 a minor bump is breaking, so ^0.2 is 0.2.x. assert!(Req::parse("^0.2").unwrap().matches(&v)); @@ -243,7 +243,7 @@ fn all_23_packages_install_with_their_dependencies_and_the_module_list_shows_eve by("projects")["dependents"], Value::List(vec![Value::text("timesheets")]) ); - assert_eq!(by("base")["version"], Value::text("0.2.0")); + assert_eq!(by("base")["version"], Value::text("0.2.1")); assert_eq!(by("base")["license"], Value::text("Apache-2.0")); let Value::List(base_dependents) = &by("base")["dependents"] else { panic!() @@ -408,7 +408,7 @@ fn the_tree_list_and_plan_commands_run() { "{t}" ); let t = app.installer.tree(Some("field_service")).unwrap(); - assert!(t.contains("base ^0.2 [built in, core 0.2.0]"), "{t}"); + assert!(t.contains("base ^0.2 [built in, core 0.2.1]"), "{t}"); assert!( t.contains("hr >=0.2, <1 [not installed, marketplace offers 0.3.0]"), "{t}" @@ -466,9 +466,9 @@ async fn the_core_version_is_served_and_an_agent_proposes_a_plan_that_one_approv let human = login.1["token"].as_str().unwrap().to_string(); // The one core version, everywhere it is reported. let meta = get(&r, &human, "/api/meta").await; - assert_eq!(meta["core_version"], "0.2.0"); + assert_eq!(meta["core_version"], "0.2.1"); let api = get(&r, &human, "/api/openapi.json").await; - assert_eq!(api["info"]["version"], "0.2.0"); + assert_eq!(api["info"]["version"], "0.2.1"); // The plan over HTTP, read-only. let (s, plan) = post( &r, @@ -480,7 +480,7 @@ async fn the_core_version_is_served_and_an_agent_proposes_a_plan_that_one_approv assert_eq!(s, 200, "{plan}"); let v = &plan["value"]; assert_eq!(v["ok"], true); - assert_eq!(v["core_version"], "0.2.0"); + assert_eq!(v["core_version"], "0.2.1"); assert_eq!(v["steps"][0]["id"], "projects"); assert_eq!(v["steps"][0]["license"], "Apache-2.0"); assert_eq!(v["steps"][1]["id"], "timesheets"); diff --git a/crates/obop/tests/version.rs b/crates/obop/tests/version.rs index 5644d8bb..edf723c5 100644 --- a/crates/obop/tests/version.rs +++ b/crates/obop/tests/version.rs @@ -26,7 +26,7 @@ fn version(args: &[&str]) -> String { String::from_utf8(out.stdout).unwrap() } -/// `obop 0.2.0 (abcdef012345)`: the commit between the parentheses, or `unknown`. +/// `obop 0.2.1 (abcdef012345)`: the commit between the parentheses, or `unknown`. fn commit_of(line: &str) -> &str { let rest = line .strip_prefix(&format!("obop {} (", env!("CARGO_PKG_VERSION"))) @@ -38,7 +38,7 @@ fn commit_of(line: &str) -> &str { #[test] fn the_version_line_keeps_its_prefix_and_ends_in_the_commit() { let line = version(&["--version"]); - assert!(line.starts_with("obop 0.2.0 ("), "{line:?}"); + assert!(line.starts_with("obop 0.2.1 ("), "{line:?}"); assert_eq!(line.lines().count(), 1, "{line:?}"); let commit = commit_of(&line); assert!( diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 9b17e0de..18baffa2 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -641,7 +641,7 @@ See "The published image": build, then `obop platform upgrade-instances --image The build attaches SLSA provenance (`mode=max`) and an SBOM as attestations, and the workflow runs `obop --version` from the pushed image and fails unless it reports the commit that was built. It authenticates with the repository's `GITHUB_TOKEN` (`packages: write`); no other credential exists. Actions are pinned by commit SHA. arm64 is not built: under emulation the Rust build takes hours; add a matrix job on a native arm64 runner and merge the manifests when it is needed. The workflow has never been run from this repository's history (it needs GitHub); the first run is the test, so watch it. -**Which build is running.** `obop --version` prints `obop 0.2.0 (abcdef012345)`: the core version, then the first 12 digits of the commit the binary was built from, so a check that pins a build can tell two builds of one version apart. The commit is read by `crates/obop/build.rs`: the environment variable `OBOP_BUILD_COMMIT` (12 to 64 hexadecimal digits; any other value is ignored, with a warning in the build output) if it is set, else `git rev-parse HEAD` when git is installed and the sources are the top of their own checkout (a plain clone or a linked worktree, on a loose or a packed branch), else `unknown`, printed as `obop 0.2.0 (unknown)`. In a checkout the build runs again when a commit, a checkout, a reset, a pull or `git pack-refs` changes what `HEAD` names; a tree whose git was missing at the first build keeps `unknown` until `crates/obop/build.rs` is touched. The image has no `.git`, so the Dockerfile takes the build argument of the same name and `image.yml` passes `${{ github.sha }}`; build it yourself with `docker build --build-arg OBOP_BUILD_COMMIT=$(git rev-parse HEAD) -t obop:local .`. The argument is declared after the Rust dependencies are cooked, so a new commit does not rebuild them. There is no `-dirty` mark: Cargo runs a build script again only when a file it names changes, so a mark for uncommitted edits would go stale at the first one. A commit hash is not secret; the line is printed by the program and the log of the pushed image check only. +**Which build is running.** `obop --version` prints `obop 0.2.1 (abcdef012345)`: the core version, then the first 12 digits of the commit the binary was built from, so a check that pins a build can tell two builds of one version apart. The commit is read by `crates/obop/build.rs`: the environment variable `OBOP_BUILD_COMMIT` (12 to 64 hexadecimal digits; any other value is ignored, with a warning in the build output) if it is set, else `git rev-parse HEAD` when git is installed and the sources are the top of their own checkout (a plain clone or a linked worktree, on a loose or a packed branch), else `unknown`, printed as `obop 0.2.0 (unknown)`. In a checkout the build runs again when a commit, a checkout, a reset, a pull or `git pack-refs` changes what `HEAD` names; a tree whose git was missing at the first build keeps `unknown` until `crates/obop/build.rs` is touched. The image has no `.git`, so the Dockerfile takes the build argument of the same name and `image.yml` passes `${{ github.sha }}`; build it yourself with `docker build --build-arg OBOP_BUILD_COMMIT=$(git rev-parse HEAD) -t obop:local .`. The argument is declared after the Rust dependencies are cooked, so a new commit does not rebuild them. There is no `-dirty` mark: Cargo runs a build script again only when a file it names changes, so a mark for uncommitted edits would go stale at the first one. A commit hash is not secret; the line is printed by the program and the log of the pushed image check only. Build the image locally with `docker build -t obop:local .`. `docker build --target web .` builds only the website and back office (it needs `Cargo.toml` in the build context for the core version, which the Dockerfile copies). diff --git a/docs/spec/cli.md b/docs/spec/cli.md index 3d9d7a00..a7ad7311 100644 --- a/docs/spec/cli.md +++ b/docs/spec/cli.md @@ -62,7 +62,7 @@ The `obop` binary exists (`crates/obop`, `obop --help` prints one usage page; th | `migrate status\|verify\|plan\|run\|rehearse\|rollback`; `update check\|status\|start\|rollback`; `platform release ...`, `platform upgrade-instances` | `migrate`, `upgrade` (3.1) | | `backup`, `backup verify`, `backup run\|list\|prune`, `restore`, `drill`, `storage check\|attest\|show`, `secrets status\|rotate` | `backup create\|verify\|list\|restore`, `doctor` (3.1) | -Global options built today: `--db URL` (or `OBOP_DATABASE_URL`), `--token` (or `OBOP_TOKEN`), `--demo`, `--dev-auth`, `--version` (one line: `obop 0.2.0 (abcdef012345)`, the core version and the first 12 digits of the commit the binary was built from, or `(unknown)`; docs/DEPLOY.md "The published image"). None of the global flags of 2.2, no profiles, no `--format`, no `--dry-run` as a global flag (several commands have their own `--dry-run`), no `--plan`, no `--idempotency-key` (only `import --key`), no shell completions, no `config.toml` and no login flow exist yet. Output is the command's own text or JSON; there is no common envelope (2.3). The REST surface is `POST /api/{model}/{command}` with bearer tokens, not the dated `/api/v1` of 2.1. Every command of this table except `serve`, `platform serve`, the hosted `/mcp` endpoint, `openapi` and `modules check FILE` opens the database itself (`--db` or `OBOP_DATABASE_URL`, with a token for the business commands) and works without a running server; the CLI has no HTTP client yet, so section 1.3 (a thin HTTP client of `/api/v1`, no direct-database mode) is a Proposed change from today's design. The commands that act on a database and have no API route or page (`migrate`, `backup`, `restore`, `drill`, `storage`, `secrets`) are why 1.1 proposes them as CLI-only entries. Exit statuses built today are in 2.4. +Global options built today: `--db URL` (or `OBOP_DATABASE_URL`), `--token` (or `OBOP_TOKEN`), `--demo`, `--dev-auth`, `--version` (one line: `obop 0.2.1 (abcdef012345)`, the core version and the first 12 digits of the commit the binary was built from, or `(unknown)`; docs/DEPLOY.md "The published image"). None of the global flags of 2.2, no profiles, no `--format`, no `--dry-run` as a global flag (several commands have their own `--dry-run`), no `--plan`, no `--idempotency-key` (only `import --key`), no shell completions, no `config.toml` and no login flow exist yet. Output is the command's own text or JSON; there is no common envelope (2.3). The REST surface is `POST /api/{model}/{command}` with bearer tokens, not the dated `/api/v1` of 2.1. Every command of this table except `serve`, `platform serve`, the hosted `/mcp` endpoint, `openapi` and `modules check FILE` opens the database itself (`--db` or `OBOP_DATABASE_URL`, with a token for the business commands) and works without a running server; the CLI has no HTTP client yet, so section 1.3 (a thin HTTP client of `/api/v1`, no direct-database mode) is a Proposed change from today's design. The commands that act on a database and have no API route or page (`migrate`, `backup`, `restore`, `drill`, `storage`, `secrets`) are why 1.1 proposes them as CLI-only entries. Exit statuses built today are in 2.4. Everything else in sections 2 to 11 (the plugin registry client, `obop.lock`, `init`, `upgrade`, `business`, `pack`, `plan`, `schema`, `login`, completions, man pages, telemetry choices, the golden paths) is Proposed and not built. diff --git a/web/apps/site/data/builtin/base.toml b/web/apps/site/data/builtin/base.toml index 26343fdd..ff762187 100644 --- a/web/apps/site/data/builtin/base.toml +++ b/web/apps/site/data/builtin/base.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/base/src/lib.rs (built-in, not removable). Keep in sync by hand. [module] id = "base" -version = "0.2.0" +version = "0.2.1" name = "Contacts" description = "People and organisations the business deals with: the shared party record that CRM, Sales and Invoicing refer to." summary = "Contacts and companies: the people and organisations the business deals with." diff --git a/web/apps/site/data/builtin/connectors.toml b/web/apps/site/data/builtin/connectors.toml index 1bb21311..af59d0f7 100644 --- a/web/apps/site/data/builtin/connectors.toml +++ b/web/apps/site/data/builtin/connectors.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/connectors/src/meta.rs (built-in). [module] id = "connectors" -version = "0.2.0" +version = "0.2.1" name = "Connectors" description = "Connect external systems and let people connect their own accounts, with the controls that support GDPR compliance built in." summary = "Connect other systems with a recorded legal basis, field allowlists, signed webhooks and OAuth." diff --git a/web/apps/site/data/builtin/crm.toml b/web/apps/site/data/builtin/crm.toml index 10985017..18c3305d 100644 --- a/web/apps/site/data/builtin/crm.toml +++ b/web/apps/site/data/builtin/crm.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/crm/src/lib.rs (built-in). [module] id = "crm" -version = "0.2.0" +version = "0.2.1" name = "CRM" description = "Leads and the sales pipeline." summary = "Leads and opportunities through a pipeline from first contact to won or lost." diff --git a/web/apps/site/data/builtin/invoicing.toml b/web/apps/site/data/builtin/invoicing.toml index d627c805..27cc8e9d 100644 --- a/web/apps/site/data/builtin/invoicing.toml +++ b/web/apps/site/data/builtin/invoicing.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/invoicing/src/lib.rs (built-in). [module] id = "invoicing" -version = "0.2.0" +version = "0.2.1" name = "Invoicing" description = "Customer invoices: drafts, posting with a gapless legal number, cancellation." summary = "Customer invoices with a draft, posted and paid life cycle." diff --git a/web/apps/site/data/builtin/product.toml b/web/apps/site/data/builtin/product.toml index 1e9dbecb..0f3c9e61 100644 --- a/web/apps/site/data/builtin/product.toml +++ b/web/apps/site/data/builtin/product.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/product/src/lib.rs (built-in). [module] id = "product" -version = "0.2.0" +version = "0.2.1" name = "Products" description = "Goods and services the business sells, with SKU, list price and cost." summary = "Products and services with prices and cost, shared by sales, invoicing and stock." diff --git a/web/apps/site/data/builtin/sales.toml b/web/apps/site/data/builtin/sales.toml index 0e671391..d8d16620 100644 --- a/web/apps/site/data/builtin/sales.toml +++ b/web/apps/site/data/builtin/sales.toml @@ -1,7 +1,7 @@ # Snapshot of crates/modules/sales/src/lib.rs (built-in). [module] id = "sales" -version = "0.2.0" +version = "0.2.1" name = "Sales" description = "Quotations and confirmed orders for customers." summary = "Quotations and sales orders for your customers." diff --git a/web/packages/client/openapi.json b/web/packages/client/openapi.json index 49051565..68437614 100644 --- a/web/packages/client/openapi.json +++ b/web/packages/client/openapi.json @@ -1567,7 +1567,7 @@ "info": { "description": "Every model has the same generated commands: list, search, get, create, update, delete and its own actions, all POST with a JSON body. Personal and financial fields are masked for agent tokens; irreversible steps answer 202 with a plan for a human to approve. Errors are JSON with error, code and hint. Every operation can answer 503 (storage_busy, storage_unavailable or storage_timeout) with a Retry-After header; its applied field says what became of the request's own changes: none (repeat it), unknown (after storage_unavailable: read before you repeat a write) or partial (a step was refused after a change committed: do not repeat it blindly). Sign in (or present a token) to see the commands your token may use; this document lists only those.", "title": "obop instance API", - "version": "0.2.0" + "version": "0.2.1" }, "openapi": "3.1.0", "paths": { @@ -2553,38 +2553,47 @@ "properties": { "config": { "description": "Non-secret settings as a JSON object", + "maxLength": 100000, "type": "string" }, "credentials": { "description": "Credentials as a JSON object; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "credentials_hint": { "description": "Whether credentials are set, with at most the last four characters", + "maxLength": 2048, "type": "string" }, "field_allowlist": { "description": "Fields that may leave or be accepted, as a JSON object of model to field names; empty means nothing", + "maxLength": 100000, "type": "string" }, "last_error": { "description": "Summary of the last failure, without payloads", + "maxLength": 2048, "type": "string" }, "last_sync_at": { "description": "When the connection last ran", + "maxLength": 2048, "type": "string" }, "name": { "description": "A name that tells you what the connection is for", + "maxLength": 2048, "type": "string" }, "oauth_tokens": { "description": "OAuth tokens; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "owner_groups": { "description": "Permission groups the owner held when the connection was last changed", + "maxLength": 2048, "type": "string" }, "owner_scope": { @@ -2597,14 +2606,17 @@ }, "owner_user": { "description": "The person who owns the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "policy": { "description": "Treatment overrides by class or field and impact-assessment acknowledgements, as JSON", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider from the catalogue", + "maxLength": 2048, "type": "string" }, "rate_limit_per_hour": { @@ -2613,6 +2625,7 @@ }, "result": { "description": "Output of the last command (test, sync, authorisation link); never stored", + "maxLength": 100000, "type": "string" }, "retention_days": { @@ -2937,38 +2950,47 @@ "properties": { "config": { "description": "Non-secret settings as a JSON object", + "maxLength": 100000, "type": "string" }, "credentials": { "description": "Credentials as a JSON object; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "credentials_hint": { "description": "Whether credentials are set, with at most the last four characters", + "maxLength": 2048, "type": "string" }, "field_allowlist": { "description": "Fields that may leave or be accepted, as a JSON object of model to field names; empty means nothing", + "maxLength": 100000, "type": "string" }, "last_error": { "description": "Summary of the last failure, without payloads", + "maxLength": 2048, "type": "string" }, "last_sync_at": { "description": "When the connection last ran", + "maxLength": 2048, "type": "string" }, "name": { "description": "A name that tells you what the connection is for", + "maxLength": 2048, "type": "string" }, "oauth_tokens": { "description": "OAuth tokens; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "owner_groups": { "description": "Permission groups the owner held when the connection was last changed", + "maxLength": 2048, "type": "string" }, "owner_scope": { @@ -2981,14 +3003,17 @@ }, "owner_user": { "description": "The person who owns the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "policy": { "description": "Treatment overrides by class or field and impact-assessment acknowledgements, as JSON", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider from the catalogue", + "maxLength": 2048, "type": "string" }, "rate_limit_per_hour": { @@ -2997,6 +3022,7 @@ }, "result": { "description": "Output of the last command (test, sync, authorisation link); never stored", + "maxLength": 100000, "type": "string" }, "retention_days": { @@ -4660,18 +4686,22 @@ "properties": { "config": { "description": "Non-secret settings as a JSON object", + "maxLength": 100000, "type": "string" }, "credentials": { "description": "Credentials as a JSON object; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "credentials_hint": { "description": "Whether credentials are set, with at most the last four characters", + "maxLength": 2048, "type": "string" }, "field_allowlist": { "description": "Fields that may leave or be accepted, as a JSON object of model to field names; empty means nothing", + "maxLength": 100000, "type": "string" }, "id": { @@ -4679,22 +4709,27 @@ }, "last_error": { "description": "Summary of the last failure, without payloads", + "maxLength": 2048, "type": "string" }, "last_sync_at": { "description": "When the connection last ran", + "maxLength": 2048, "type": "string" }, "name": { "description": "A name that tells you what the connection is for", + "maxLength": 2048, "type": "string" }, "oauth_tokens": { "description": "OAuth tokens; write-only, stored encrypted, never returned", + "maxLength": 100000, "type": "string" }, "owner_groups": { "description": "Permission groups the owner held when the connection was last changed", + "maxLength": 2048, "type": "string" }, "owner_scope": { @@ -4707,14 +4742,17 @@ }, "owner_user": { "description": "The person who owns the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "policy": { "description": "Treatment overrides by class or field and impact-assessment acknowledgements, as JSON", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider from the catalogue", + "maxLength": 2048, "type": "string" }, "rate_limit_per_hour": { @@ -4723,6 +4761,7 @@ }, "result": { "description": "Output of the last command (test, sync, authorisation link); never stored", + "maxLength": 100000, "type": "string" }, "retention_days": { @@ -5419,6 +5458,7 @@ "properties": { "classes": { "description": "Data classes that may flow under this record, computed from the connection", + "maxLength": 2048, "type": "string" }, "connection": { @@ -5427,10 +5467,12 @@ }, "fields_summary": { "description": "Models and fields that may flow, computed from the connection", + "maxLength": 100000, "type": "string" }, "given_by": { "description": "Who recorded it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "legal_basis": { @@ -5445,18 +5487,22 @@ }, "note": { "description": "Explanation; for legitimate interest the balancing test", + "maxLength": 100000, "type": "string" }, "owner_user": { "description": "Owner of the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "purpose": { "description": "The purpose the data is processed for", + "maxLength": 100000, "type": "string" }, "recorded_at": { "description": "When it was recorded", + "maxLength": 2048, "type": "string" }, "state": { @@ -5470,6 +5516,7 @@ }, "subject": { "description": "Id of the party when the subject is a party (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "subject_kind": { @@ -5483,10 +5530,12 @@ }, "withdrawn_at": { "description": "When it was withdrawn, if it was", + "maxLength": 2048, "type": "string" }, "withdrawn_by": { "description": "Who withdrew it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -5667,6 +5716,7 @@ "properties": { "classes": { "description": "Data classes that may flow under this record, computed from the connection", + "maxLength": 2048, "type": "string" }, "connection": { @@ -5675,10 +5725,12 @@ }, "fields_summary": { "description": "Models and fields that may flow, computed from the connection", + "maxLength": 100000, "type": "string" }, "given_by": { "description": "Who recorded it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "legal_basis": { @@ -5693,18 +5745,22 @@ }, "note": { "description": "Explanation; for legitimate interest the balancing test", + "maxLength": 100000, "type": "string" }, "owner_user": { "description": "Owner of the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "purpose": { "description": "The purpose the data is processed for", + "maxLength": 100000, "type": "string" }, "recorded_at": { "description": "When it was recorded", + "maxLength": 2048, "type": "string" }, "state": { @@ -5718,6 +5774,7 @@ }, "subject": { "description": "Id of the party when the subject is a party (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "subject_kind": { @@ -5731,10 +5788,12 @@ }, "withdrawn_at": { "description": "When it was withdrawn, if it was", + "maxLength": 2048, "type": "string" }, "withdrawn_by": { "description": "Who withdrew it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -6506,6 +6565,7 @@ "properties": { "classes": { "description": "Data classes that may flow under this record, computed from the connection", + "maxLength": 2048, "type": "string" }, "connection": { @@ -6514,10 +6574,12 @@ }, "fields_summary": { "description": "Models and fields that may flow, computed from the connection", + "maxLength": 100000, "type": "string" }, "given_by": { "description": "Who recorded it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "id": { @@ -6535,18 +6597,22 @@ }, "note": { "description": "Explanation; for legitimate interest the balancing test", + "maxLength": 100000, "type": "string" }, "owner_user": { "description": "Owner of the connection (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "purpose": { "description": "The purpose the data is processed for", + "maxLength": 100000, "type": "string" }, "recorded_at": { "description": "When it was recorded", + "maxLength": 2048, "type": "string" }, "state": { @@ -6560,6 +6626,7 @@ }, "subject": { "description": "Id of the party when the subject is a party (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "subject_kind": { @@ -6573,10 +6640,12 @@ }, "withdrawn_at": { "description": "When it was withdrawn, if it was", + "maxLength": 2048, "type": "string" }, "withdrawn_by": { "description": "Who withdrew it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -8014,10 +8083,12 @@ "properties": { "app_version": { "description": "The version of the app, as it reported it at sign-in and at each refresh", + "maxLength": 2048, "type": "string" }, "client_name": { "description": "The mobile app that signed in, as it called itself", + "maxLength": 2048, "type": "string" }, "expires_at": { @@ -8028,6 +8099,7 @@ }, "grant": { "description": "The refresh-token family the device is bound to (an identifier, not a token)", + "maxLength": 2048, "type": "string" }, "id": { @@ -8041,14 +8113,17 @@ }, "name": { "description": "What the device is called; the person can rename it (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "platform": { "description": "The platform the app reported, such as android or ios", + "maxLength": 2048, "type": "string" }, "push_token": { "description": "The address of the push service for this device. Never shown to anyone and not written in this version: push is not part of the first mobile slice", + "maxLength": 2048, "type": "string" }, "registered_at": { @@ -8073,6 +8148,7 @@ }, "user": { "description": "The account the device belongs to (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -8990,6 +9066,7 @@ }, "host_allowlist": { "description": "Hosts connections may reach, comma separated; empty means no extra restriction", + "maxLength": 2048, "type": "string" }, "id": { @@ -9001,6 +9078,7 @@ }, "result": { "description": "Output of the last command (checklist, purge); never stored", + "maxLength": 100000, "type": "string" } }, @@ -9147,6 +9225,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "party_id": { @@ -9463,6 +9542,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "party_id": { @@ -10371,6 +10451,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "id": { @@ -10537,6 +10618,7 @@ "properties": { "description": { "description": "What is invoiced", + "maxLength": 2048, "type": "string" }, "invoice_id": { @@ -10733,6 +10815,7 @@ "properties": { "description": { "description": "What is invoiced", + "maxLength": 2048, "type": "string" }, "invoice_id": { @@ -11520,6 +11603,7 @@ "properties": { "description": { "description": "What is invoiced", + "maxLength": 2048, "type": "string" }, "id": { @@ -11685,10 +11769,12 @@ "properties": { "contact_email": { "description": "Email of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "contact_name": { "description": "Name of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "expected_revenue_minor": { @@ -11707,6 +11793,7 @@ }, "title": { "description": "Short description of the opportunity", + "maxLength": 2048, "type": "string" } }, @@ -11885,10 +11972,12 @@ "properties": { "contact_email": { "description": "Email of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "contact_name": { "description": "Name of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "expected_revenue_minor": { @@ -11907,6 +11996,7 @@ }, "title": { "description": "Short description of the opportunity", + "maxLength": 2048, "type": "string" } }, @@ -13172,10 +13262,12 @@ "properties": { "contact_email": { "description": "Email of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "contact_name": { "description": "Name of the contact person (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "expected_revenue_minor": { @@ -13197,6 +13289,7 @@ }, "title": { "description": "Short description of the opportunity", + "maxLength": 2048, "type": "string" } }, @@ -14060,6 +14153,7 @@ }, "driver": { "description": "Login of the driver of the trip; set by the server from the trip (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "lat": { @@ -14280,6 +14374,7 @@ }, "driver": { "description": "Login of the driver of the trip; set by the server from the trip (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "lat": { @@ -15091,6 +15186,7 @@ }, "driver": { "description": "Login of the driver of the trip; set by the server from the trip (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "id": { @@ -15272,6 +15368,7 @@ "properties": { "client_trip_id": { "description": "Identifier the app made when the driver pressed Start (a random UUID, version 4: not a version 7, which carries the time); unique per driver, and how the app finds its trip again (unique)", + "maxLength": 2048, "type": "string" }, "confirmed_at": { @@ -15282,6 +15379,7 @@ }, "discard_reason": { "description": "Why the driver discards the trip; written before discard (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "distance_m": { @@ -15292,6 +15390,7 @@ }, "driver": { "description": "Login of the person who recorded the trip; set by the server to the caller (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "end_lat": { @@ -15318,6 +15417,7 @@ }, "integrity": { "description": "Flags the validator set when the trip ended: seq_gap, speed_jump and low_accuracy with their counts; empty when the track was clean", + "maxLength": 2048, "type": "string" }, "last_seq": { @@ -15335,10 +15435,12 @@ }, "reject_reason": { "description": "Why the reviewer rejects the trip; written while the trip is in review, before reject; cleared when the driver submits again (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "reviewed_by": { "description": "Login of the reviewer who confirmed or rejected the trip, or of the person who approved an agent's proposal to do so (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "start_lat": { @@ -15373,6 +15475,7 @@ }, "vehicle": { "description": "The vehicle the trip was made with, as the driver names it (a plate or a name) (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -15678,6 +15781,7 @@ "properties": { "client_trip_id": { "description": "Identifier the app made when the driver pressed Start (a random UUID, version 4: not a version 7, which carries the time); unique per driver, and how the app finds its trip again (unique)", + "maxLength": 2048, "type": "string" }, "confirmed_at": { @@ -15688,6 +15792,7 @@ }, "discard_reason": { "description": "Why the driver discards the trip; written before discard (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "distance_m": { @@ -15698,6 +15803,7 @@ }, "driver": { "description": "Login of the person who recorded the trip; set by the server to the caller (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "end_lat": { @@ -15724,6 +15830,7 @@ }, "integrity": { "description": "Flags the validator set when the trip ended: seq_gap, speed_jump and low_accuracy with their counts; empty when the track was clean", + "maxLength": 2048, "type": "string" }, "last_seq": { @@ -15741,10 +15848,12 @@ }, "reject_reason": { "description": "Why the reviewer rejects the trip; written while the trip is in review, before reject; cleared when the driver submits again (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "reviewed_by": { "description": "Login of the reviewer who confirmed or rejected the trip, or of the person who approved an agent's proposal to do so (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "start_lat": { @@ -15779,6 +15888,7 @@ }, "vehicle": { "description": "The vehicle the trip was made with, as the driver names it (a plate or a name) (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -17297,6 +17407,7 @@ "properties": { "client_trip_id": { "description": "Identifier the app made when the driver pressed Start (a random UUID, version 4: not a version 7, which carries the time); unique per driver, and how the app finds its trip again (unique)", + "maxLength": 2048, "type": "string" }, "confirmed_at": { @@ -17307,6 +17418,7 @@ }, "discard_reason": { "description": "Why the driver discards the trip; written before discard (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "distance_m": { @@ -17317,6 +17429,7 @@ }, "driver": { "description": "Login of the person who recorded the trip; set by the server to the caller (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "end_lat": { @@ -17346,6 +17459,7 @@ }, "integrity": { "description": "Flags the validator set when the trip ended: seq_gap, speed_jump and low_accuracy with their counts; empty when the track was clean", + "maxLength": 2048, "type": "string" }, "last_seq": { @@ -17363,10 +17477,12 @@ }, "reject_reason": { "description": "Why the reviewer rejects the trip; written while the trip is in review, before reject; cleared when the driver submits again (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "reviewed_by": { "description": "Login of the reviewer who confirmed or rejected the trip, or of the person who approved an agent's proposal to do so (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "start_lat": { @@ -17401,6 +17517,7 @@ }, "vehicle": { "description": "The vehicle the trip was made with, as the driver names it (a plate or a name) (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" } }, @@ -18703,18 +18820,22 @@ "properties": { "email": { "description": "Work email address (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "name": { "description": "Display name (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "notes": { "description": "Free-text internal notes (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "tax_id": { "description": "Tax identifier of a natural person (personal data of a sensitive class: not given to agents at all)", + "maxLength": 2048, "type": "string" } }, @@ -18893,14 +19014,17 @@ "properties": { "email": { "description": "Work email address (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "name": { "description": "Display name (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "notes": { "description": "Free-text internal notes (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "photo": { @@ -18910,6 +19034,7 @@ }, "tax_id": { "description": "Tax identifier of a natural person (personal data of a sensitive class: not given to agents at all)", + "maxLength": 2048, "type": "string" } }, @@ -19804,6 +19929,7 @@ "properties": { "email": { "description": "Work email address (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "id": { @@ -19811,10 +19937,12 @@ }, "name": { "description": "Display name (personal data: agents see it masked)", + "maxLength": 2048, "type": "string" }, "notes": { "description": "Free-text internal notes (personal data: agents see it masked)", + "maxLength": 100000, "type": "string" }, "photo": { @@ -19824,6 +19952,7 @@ }, "tax_id": { "description": "Tax identifier of a natural person (personal data of a sensitive class: not given to agents at all)", + "maxLength": 2048, "type": "string" } }, @@ -21288,10 +21417,12 @@ }, "name": { "description": "Product name", + "maxLength": 2048, "type": "string" }, "sku": { "description": "Stock keeping unit; no two products share one (unique)", + "maxLength": 2048, "type": "string" } }, @@ -21483,10 +21614,12 @@ }, "name": { "description": "Product name", + "maxLength": 2048, "type": "string" }, "sku": { "description": "Stock keeping unit; no two products share one (unique)", + "maxLength": 2048, "type": "string" } }, @@ -22272,10 +22405,12 @@ }, "name": { "description": "Product name", + "maxLength": 2048, "type": "string" }, "sku": { "description": "Stock keeping unit; no two products share one (unique)", + "maxLength": 2048, "type": "string" } }, @@ -22422,10 +22557,12 @@ "properties": { "allowed_group": { "description": "Permission group that may use it; empty means connectors.user", + "maxLength": 2048, "type": "string" }, "default_policy": { "description": "Policy JSON that new connections of this provider start with", + "maxLength": 100000, "type": "string" }, "dpa_recorded": { @@ -22434,6 +22571,7 @@ }, "dpa_reference": { "description": "Where the agreement is kept or its reference", + "maxLength": 2048, "type": "string" }, "enabled": { @@ -22452,18 +22590,22 @@ }, "legal_basis_text": { "description": "Text shown to people when they are asked for consent", + "maxLength": 100000, "type": "string" }, "oauth_client_id": { "description": "OAuth client id you registered with the provider", + "maxLength": 2048, "type": "string" }, "oauth_client_secret": { "description": "OAuth client secret; write-only, stored encrypted", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider this policy is for", + "maxLength": 2048, "type": "string" }, "users_may_use": { @@ -22646,10 +22788,12 @@ "properties": { "allowed_group": { "description": "Permission group that may use it; empty means connectors.user", + "maxLength": 2048, "type": "string" }, "default_policy": { "description": "Policy JSON that new connections of this provider start with", + "maxLength": 100000, "type": "string" }, "dpa_recorded": { @@ -22658,6 +22802,7 @@ }, "dpa_reference": { "description": "Where the agreement is kept or its reference", + "maxLength": 2048, "type": "string" }, "enabled": { @@ -22676,18 +22821,22 @@ }, "legal_basis_text": { "description": "Text shown to people when they are asked for consent", + "maxLength": 100000, "type": "string" }, "oauth_client_id": { "description": "OAuth client id you registered with the provider", + "maxLength": 2048, "type": "string" }, "oauth_client_secret": { "description": "OAuth client secret; write-only, stored encrypted", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider this policy is for", + "maxLength": 2048, "type": "string" }, "users_may_use": { @@ -23461,10 +23610,12 @@ "properties": { "allowed_group": { "description": "Permission group that may use it; empty means connectors.user", + "maxLength": 2048, "type": "string" }, "default_policy": { "description": "Policy JSON that new connections of this provider start with", + "maxLength": 100000, "type": "string" }, "dpa_recorded": { @@ -23473,6 +23624,7 @@ }, "dpa_reference": { "description": "Where the agreement is kept or its reference", + "maxLength": 2048, "type": "string" }, "enabled": { @@ -23494,18 +23646,22 @@ }, "legal_basis_text": { "description": "Text shown to people when they are asked for consent", + "maxLength": 100000, "type": "string" }, "oauth_client_id": { "description": "OAuth client id you registered with the provider", + "maxLength": 2048, "type": "string" }, "oauth_client_secret": { "description": "OAuth client secret; write-only, stored encrypted", + "maxLength": 100000, "type": "string" }, "provider": { "description": "Id of the provider this policy is for", + "maxLength": 2048, "type": "string" }, "users_may_use": { @@ -23656,6 +23812,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "party_id": { @@ -24092,6 +24249,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "party_id": { @@ -24995,6 +25153,7 @@ "properties": { "currency": { "description": "ISO 4217 currency code", + "maxLength": 2048, "type": "string" }, "id": { @@ -25157,6 +25316,7 @@ "properties": { "description": { "description": "What is sold, as it appears on the invoice", + "maxLength": 2048, "type": "string" }, "order_id": { @@ -25353,6 +25513,7 @@ "properties": { "description": { "description": "What is sold, as it appears on the invoice", + "maxLength": 2048, "type": "string" }, "order_id": { @@ -26140,6 +26301,7 @@ "properties": { "description": { "description": "What is sold, as it appears on the invoice", + "maxLength": 2048, "type": "string" }, "id": { diff --git a/web/packages/e2e/tests/modules.spec.ts b/web/packages/e2e/tests/modules.spec.ts index 6ec2fb6b..83db83b0 100644 --- a/web/packages/e2e/tests/modules.spec.ts +++ b/web/packages/e2e/tests/modules.spec.ts @@ -58,12 +58,12 @@ test.describe("Customize > Modules", () => { await signIn("root", MODULES); await expect(page.getByRole("heading", { name: "Modules", level: 2 })).toBeVisible(); // The header names the core the instance runs and what that means. - await expect(page.getByText("Core 0.2.0", { exact: true }).first()).toBeVisible(); + await expect(page.getByText("Core 0.2.1", { exact: true }).first()).toBeVisible(); await expect(page.getByText(/every module runs on/)).toBeVisible(); // A built-in module has the same facts as a package. const contacts = card(page, "Contacts"); - await expect(contacts).toContainText("0.2.0 · SpaceCorps"); + await expect(contacts).toContainText("0.2.1 · SpaceCorps"); await expect(contacts).toContainText("Apache-2.0"); await expect(contacts).toContainText("Stable"); await expect(contacts).toContainText("Works with core 0.2"); @@ -222,7 +222,7 @@ test.describe("Customize > Modules", () => { .fill(pkg("futurepack", "1.0.0").replace('requires_core = "^0.2"', 'requires_core = ">=9"')); const check = page.locator(".mods-check"); await expect(check.getByText("This package cannot be installed here")).toBeVisible(); - await expect(check).toContainText("futurepack 1.0.0 needs core >=9, you run 0.2.0"); + await expect(check).toContainText("futurepack 1.0.0 needs core >=9, you run 0.2.1"); await expect(check.getByRole("button", { name: "Install" })).toBeDisabled(); // The server refuses it too, whatever the page says. const r = await api.post( @@ -236,7 +236,7 @@ test.describe("Customize > Modules", () => { await api.token("root"), ); expect(r.status).toBe(400); - expect(r.json.error).toBe("futurepack 1.0.0 needs core >=9, you run 0.2.0"); + expect(r.json.error).toBe("futurepack 1.0.0 needs core >=9, you run 0.2.1"); expect(await installed(api)).not.toContain("futurepack"); }); @@ -324,7 +324,7 @@ test.describe("Customize > Modules", () => { await panel.getByRole("tab", { name: "Dependencies" }).click(); await expect(page).toHaveURL(/section=dependencies/); await expect(panel.getByText("Field Service needs core ^0.2")).toBeVisible(); - await expect(panel.getByText("you run 0.2.0").first()).toBeVisible(); + await expect(panel.getByText("you run 0.2.1").first()).toBeVisible(); const tree = panel.getByRole("list", { name: "Dependencies of Field Service" }).first(); const hr = tree.getByRole("listitem").filter({ hasText: "Human Resources" }).first(); await expect(hr).toContainText("≥0.2, <1"); @@ -332,7 +332,7 @@ test.describe("Customize > Modules", () => { await expect(hr).toContainText("Satisfied"); const base = tree.getByRole("listitem").filter({ hasText: "Contacts" }).first(); await expect(base).toContainText("Built in"); - await expect(base).toContainText("installed 0.2.0"); + await expect(base).toContainText("installed 0.2.1"); // A module in the tree opens its own panel; Required by lists the dependents. await hr.getByRole("link", { name: "Human Resources" }).click(); await expect(page).toHaveURL(/id=hr/); From f61ef5b65413c2f9d6b379e80b9ad590fe683305 Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:31 +0100 Subject: [PATCH 06/23] Test field limits through the real router, the stores and the binary (D90) REST create, update, bulk_create and CSV import over a real router; the default length on every built-in model; /api/meta, the OpenAPI document and the MCP tool schemas; installing a package whose limit cannot hold; and obop modules check through the real binary on good and bad packages. Co-Authored-By: Claude Sonnet 5.5 --- crates/obop/src/limits_tests.rs | 739 ++++++++++++++++++++++++++++++++ crates/obop/src/main.rs | 4 +- crates/obop/tests/limits_cli.rs | 147 +++++++ 3 files changed, 889 insertions(+), 1 deletion(-) create mode 100644 crates/obop/src/limits_tests.rs create mode 100644 crates/obop/tests/limits_cli.rs diff --git a/crates/obop/src/limits_tests.rs b/crates/obop/src/limits_tests.rs new file mode 100644 index 00000000..68654b31 --- /dev/null +++ b/crates/obop/src/limits_tests.rs @@ -0,0 +1,739 @@ +//! Field limits (D90) over HTTP and MCP against a real `App`: the 400 with its code, hint, field +//! and numbers on create, update and `bulk_create`, the rule for a record stored above a lowered +//! limit, the default on the built-in models, and what `GET /api/meta`, the OpenAPI document and +//! the MCP tool schemas say. The package under test is installed through `module.install` like +//! any other. + +use crate::app::App; +use axum::{body::Body, http::Request}; +use http_body_util::BodyExt; +use obop_auth::{Auth, AuthConfig}; +use serde_json::{Value as J, json}; +use std::sync::Arc; +use tower::ServiceExt; + +const PW: &str = "correct horse battery"; + +/// A package with a field of every kind that takes a limit, and one of each without. +const LIM: &str = r#" +[module] +id = "lim" +version = "0.1.0" +name = "Lim" +description = "Limits" +license = "MIT" +requires_core = "^0.2.1" + +[[model]] +name = "lim_note" +description = "A note" +read_group = "base.user" +write_group = "base.user" + + [[model.field]] + name = "title" + description = "Title" + class = "none" + max_length = 2000 + min_length = 1 + + [[model.field]] + name = "body" + description = "Body" + class = "none" + kind = "longtext" + + [[model.field]] + name = "plain" + description = "Plain text with no limit declared" + class = "none" + + [[model.field]] + name = "qty" + description = "Quantity" + class = "none" + kind = "number" + min = 0 + max = 100 + + [[model.field]] + name = "price" + description = "Price in minor units" + class = "none" + kind = "money" + min = 0 + max = 100000 + + [[model.field]] + name = "rate" + description = "Rate" + class = "none" + kind = "decimal" + precision = 12 + scale = 7 + min = "0.1000000" + max = "0.9000000" +"#; + +struct Rig { + r: axum::Router, + app: App, + token: String, +} + +async fn send(r: &axum::Router, method: &str, token: &str, path: &str, body: J) -> (u16, J) { + let req = Request::builder() + .method(method) + .uri(path) + .header("content-type", "application/json") + .header("authorization", format!("Bearer {token}")) + .body(Body::from(body.to_string())) + .unwrap(); + let resp = r.clone().oneshot(req).await.unwrap(); + let status = resp.status().as_u16(); + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + (status, serde_json::from_slice(&bytes).unwrap_or(J::Null)) +} + +impl Rig { + /// A server with a user who may do everything and no package installed. + async fn bare() -> Rig { + let app = App::new(); + app.seed_demo(); + let auth = Arc::new(Auth::with_test_config( + app.store.backend(), + AuthConfig { + default_require_admins: false, + ..AuthConfig::insecure_dev() + }, + )); + auth.create_user("root", PW, &app.all_groups()).unwrap(); + let r = obop_http::router( + obop_http::HttpState::new(app.dispatcher.clone(), auth.clone()), + None, + ); + let (s, b) = send( + &r, + "POST", + "", + "/api/auth/login", + json!({"username": "root", "password": PW}), + ) + .await; + assert_eq!(s, 200, "{b}"); + let token = b["token"].as_str().unwrap().to_string(); + Rig { r, app, token } + } + /// As `bare`, with the package `lim` installed. + async fn new() -> Rig { + let rig = Rig::bare().await; + let (s, b) = rig + .post("/api/module/install", json!({"manifest": LIM})) + .await; + assert_eq!(s, 200, "{b}"); + rig + } + async fn post(&self, path: &str, body: J) -> (u16, J) { + send(&self.r, "POST", &self.token, path, body).await + } + async fn get(&self, path: &str) -> J { + let (s, b) = send(&self.r, "GET", &self.token, path, J::Null).await; + assert_eq!(s, 200, "{path}: {b}"); + b + } + async fn create(&self, model: &str, body: J) -> (u16, J) { + self.post(&format!("/api/{model}/create"), body).await + } + async fn count(&self, model: &str) -> u64 { + let (s, b) = self + .post(&format!("/api/{model}/search"), json!({"limit": 1})) + .await; + assert_eq!(s, 200, "{b}"); + b["value"]["total"].as_u64().unwrap() + } +} + +fn error(b: &J) -> String { + b["error"].as_str().unwrap_or_default().to_string() +} + +#[tokio::test] +async fn rest_refuses_a_value_outside_a_limit_with_a_400_that_names_the_field_and_the_numbers() { + let rig = Rig::new().await; + let long = "x".repeat(2001); + let (s, b) = rig.create("lim_note", json!({"title": long})).await; + assert_eq!(s, 400, "{b}"); + assert_eq!( + error(&b), + "lim_note.title must be at most 2000 characters; this has 2001" + ); + assert_eq!(b["code"], "limit_violated"); + assert!(b["hint"].as_str().unwrap().contains("GET /api/meta"), "{b}"); + // 2,000 characters are accepted (the limit is inclusive), counted as characters. + let (s, b) = rig + .create("lim_note", json!({"title": "å".repeat(2000)})) + .await; + assert_eq!(s, 200, "{b}"); + let (s, b) = rig + .create("lim_note", json!({"title": "\u{1F600}".repeat(2001)})) + .await; + assert_eq!(s, 400, "{b}"); + assert!(error(&b).ends_with("at most 2000 characters; this has 2001")); + + for (field, value, want) in [ + ( + "title", + json!(""), + "must be at least 1 character; this has 0", + ), + ("qty", json!(-1), "must be at least 0; this is -1"), + ("qty", json!(101), "must be at most 100; this is 101"), + ( + "price", + json!(100_001), + "must be at most 100000; this is 100001", + ), + ( + "rate", + json!("0.0999999"), + "must be at least 0.1000000; this is 0.0999999", + ), + ( + "rate", + json!("0.9000001"), + "must be at most 0.9000000; this is 0.9000001", + ), + ] { + let (s, b) = rig + .create("lim_note", json!({"title": "t", field: value})) + .await; + assert_eq!(s, 400, "{field}: {b}"); + assert!( + error(&b).starts_with(&format!("lim_note.{field} ")) && error(&b).contains(want), + "{field}: {b}" + ); + assert_eq!(b["code"], "limit_violated", "{field}"); + } + // Nothing was stored by any of the refused calls. + assert_eq!(rig.count("lim_note").await, 1); + + // The inclusive ends are fine. + for body in [ + json!({"title": "t", "qty": 0, "price": 0, "rate": "0.1000000"}), + json!({"title": "t", "qty": 100, "price": 100_000, "rate": "0.9000000"}), + ] { + let (s, b) = rig.create("lim_note", body).await; + assert_eq!(s, 200, "{b}"); + } +} + +#[tokio::test] +async fn rest_update_holds_a_changed_value_to_the_limit_and_spares_an_unchanged_one() { + let rig = Rig::new().await; + let (_, b) = rig + .create("lim_note", json!({"title": "ok", "qty": 5})) + .await; + let id = b["value"]["id"].as_str().unwrap().to_string(); + let (s, b) = rig + .post("/api/lim_note/update", json!({"id": id, "qty": 101})) + .await; + assert_eq!( + (s, b["code"].as_str()), + (400, Some("limit_violated")), + "{b}" + ); + assert_eq!(error(&b), "lim_note.qty must be at most 100; this is 101"); + + // Behind the commands' back: a record from before the limit was lowered. + let patch = [ + ("title".to_string(), json!("t".repeat(3000))), + ("qty".to_string(), json!(5000)), + ] + .into_iter() + .collect(); + assert!( + rig.app + .store + .backend() + .update("lim_note", &id, &patch) + .unwrap() + ); + // Another field can be changed, with or without the stored values sent back with it. + let (s, b) = rig + .post("/api/lim_note/update", json!({"id": id, "plain": "edited"})) + .await; + assert_eq!(s, 200, "{b}"); + let (s, b) = rig + .post( + "/api/lim_note/update", + json!({"id": id, "title": "t".repeat(3000), "qty": 5000, "plain": "again"}), + ) + .await; + assert_eq!(s, 200, "{b}"); + // A changed value is held to the limit, however far above it the stored one was. + let (s, b) = rig + .post( + "/api/lim_note/update", + json!({"id": id, "title": "u".repeat(3000)}), + ) + .await; + assert_eq!(s, 400, "{b}"); + assert_eq!( + error(&b), + "lim_note.title must be at most 2000 characters; this has 3000" + ); +} + +#[tokio::test] +async fn rest_bulk_create_names_the_row_and_csv_goes_through_the_same_check() { + let rig = Rig::new().await; + let (s, b) = rig + .post( + "/api/lim_note/bulk_create", + json!({"rows": [ + {"title": "fine"}, + {"title": "x".repeat(2001)}, + {"title": "fine", "qty": 101}, + ]}), + ) + .await; + assert_eq!(s, 400, "{b}"); + let e = error(&b); + assert!( + e.contains("2 of 3 row(s) are invalid") + && e.contains("row 2") + && e.contains("lim_note.title must be at most 2000 characters; this has 2001") + && e.contains("row 3") + && e.contains("lim_note.qty must be at most 100; this is 101"), + "{e}" + ); + assert_eq!(rig.count("lim_note").await, 0); + + // Partial mode: the errors come as a list with the row number and no code in the text. + let (s, b) = rig + .post( + "/api/lim_note/bulk_create", + json!({"mode": "partial", "rows": [ + {"title": "fine"}, + {"title": "x".repeat(2001)}, + ]}), + ) + .await; + assert_eq!(s, 200, "{b}"); + assert_eq!(b["value"]["created"], 1, "{b}"); + assert_eq!(b["value"]["errors"][0]["row"], 2); + assert_eq!( + b["value"]["errors"][0]["error"], + "lim_note.title must be at most 2000 characters; this has 2001" + ); + + let csv = format!("title,qty\nok,1\n{},1\nok,500\n", "y".repeat(2001)); + let (s, b) = rig + .post("/api/lim_note/bulk_create", json!({"csv": csv})) + .await; + assert_eq!(s, 400, "{b}"); + assert!( + error(&b).contains("row 2") && error(&b).contains("row 3"), + "{b}" + ); + assert_eq!(rig.count("lim_note").await, 1); +} + +#[tokio::test] +async fn the_built_in_models_have_the_default_length() { + let rig = Rig::new().await; + let (s, b) = rig.create("party", json!({"name": "a".repeat(2048)})).await; + assert_eq!(s, 200, "{b}"); + let (s, b) = rig.create("party", json!({"name": "a".repeat(2049)})).await; + assert_eq!(s, 400, "{b}"); + assert_eq!( + error(&b), + "party.name must be at most 2048 characters; this has 2049" + ); + // A longtext field of a built-in model has the larger default. + let (s, b) = rig + .create("party", json!({"name": "n", "notes": "z".repeat(100_000)})) + .await; + assert_eq!(s, 200, "{b}"); + let (s, b) = rig + .create("party", json!({"name": "n", "notes": "z".repeat(100_001)})) + .await; + assert_eq!(s, 400, "{b}"); + assert_eq!( + error(&b), + "party.notes must be at most 100000 characters; this has 100001" + ); +} + +#[tokio::test] +async fn meta_says_the_limit_of_every_field_that_has_one() { + let rig = Rig::new().await; + let meta = rig.get("/api/meta").await; + let fields = |model: &str| -> Vec { + for m in meta["modules"].as_array().unwrap() { + for d in m["models"].as_array().unwrap() { + if d["name"] == model { + return d["fields"].as_array().unwrap().clone(); + } + } + } + panic!("no model {model}"); + }; + let field = |model: &str, name: &str| -> J { + fields(model) + .into_iter() + .find(|f| f["name"] == name) + .unwrap_or_else(|| panic!("no field {model}.{name}")) + }; + let t = field("lim_note", "title"); + assert_eq!( + (t["max_length"].clone(), t["min_length"].clone()), + (json!(2000), json!(1)) + ); + // The default is stated, not left for the client to guess; no minimum is invented. + let p = field("lim_note", "plain"); + assert_eq!(p["max_length"], 2048); + assert!(p.get("min_length").is_none(), "{p}"); + assert_eq!(field("lim_note", "body")["max_length"], 100_000); + let q = field("lim_note", "qty"); + assert_eq!((q["min"].clone(), q["max"].clone()), (json!(0), json!(100))); + assert!(q.get("max_length").is_none(), "{q}"); + let m = field("lim_note", "price"); + assert_eq!( + (m["min"].clone(), m["max"].clone()), + (json!(0), json!(100_000)) + ); + // A decimal bound is an exact string at the scale of the field, never a number. + let d = field("lim_note", "rate"); + assert_eq!( + (d["min"].clone(), d["max"].clone()), + (json!("0.1000000"), json!("0.9000000")) + ); + // Every writable text field of every model says its maximum; no other kind does. + let mut texts = 0; + for m in meta["modules"].as_array().unwrap() { + for d in m["models"].as_array().unwrap() { + for f in d["fields"].as_array().unwrap() { + let kind = f["kind"].as_str().unwrap(); + let system = f["computed"] == true || f["sequence"] == true; + if matches!(kind, "text" | "longtext") && !system { + texts += 1; + let max = f["max_length"].as_u64(); + assert!( + max.is_some_and(|n| (1..=1_000_000).contains(&n)), + "{}.{} has no maximum: {f}", + d["name"], + f["name"] + ); + } else { + assert!( + f.get("max_length").is_none() && f.get("min_length").is_none(), + "{}.{} should have no length: {f}", + d["name"], + f["name"] + ); + } + if !matches!(kind, "number" | "money" | "decimal") { + assert!(f.get("min").is_none() && f.get("max").is_none(), "{f}"); + } + } + } + } + assert!(texts > 50, "{texts}"); +} + +#[tokio::test] +async fn the_openapi_document_has_the_keywords_on_inputs_and_not_on_records() { + let rig = Rig::new().await; + let doc = rig.get("/api/openapi.json").await; + assert_eq!(doc["info"]["version"], "0.2.1"); + let input = |op: &str| -> J { + doc["paths"][format!("/api/lim_note/{op}")]["post"]["requestBody"]["content"] + ["application/json"]["schema"] + .clone() + }; + let c = input("create"); + let p = &c["properties"]; + assert_eq!( + ( + p["title"]["maxLength"].clone(), + p["title"]["minLength"].clone() + ), + (json!(2000), json!(1)) + ); + assert_eq!(p["body"]["maxLength"], 100_000); + assert_eq!(p["plain"]["maxLength"], 2048); + assert_eq!( + (p["qty"]["minimum"].clone(), p["qty"]["maximum"].clone()), + (json!(0), json!(100)) + ); + assert_eq!( + (p["price"]["minimum"].clone(), p["price"]["maximum"].clone()), + (json!(0), json!(100000)) + ); + // A decimal is a string: JSON Schema's `minimum` would be a number, so the bounds go in + // extension keywords, as exact strings. + assert!(p["rate"].get("minimum").is_none(), "{}", p["rate"]); + assert_eq!(p["rate"]["x-obop-minimum"], "0.1000000"); + assert_eq!(p["rate"]["x-obop-maximum"], "0.9000000"); + // A declared limit is also said in words; the default is not repeated on every field. + assert!( + p["title"]["description"] + .as_str() + .unwrap() + .ends_with("(from 1 character to 2000 characters)"), + "{}", + p["title"] + ); + assert_eq!( + p["plain"]["description"], + "Plain text with no limit declared" + ); + // The update input and the bulk rows carry them too. + assert_eq!(input("update")["properties"]["title"]["maxLength"], 2000); + let rows = &input("bulk_create")["properties"]["rows"]["items"]["properties"]; + assert_eq!(rows["title"]["maxLength"], 2000); + // The record schema is what is stored, which may be above a limit set later: no keywords. + let rec = &doc["components"]["schemas"]["lim_note"]["properties"]; + for f in ["title", "plain", "qty", "rate"] { + for k in [ + "maxLength", + "minLength", + "minimum", + "maximum", + "x-obop-minimum", + ] { + assert!(rec[f].get(k).is_none(), "{f} {k}: {}", rec[f]); + } + } + // Every writable text property of every create input has a maximum: the kinds come from + // /api/meta, because a reference or an image is a string in the schema too. + let meta = rig.get("/api/meta").await; + let mut seen = 0; + for m in meta["modules"].as_array().unwrap() { + for d in m["models"].as_array().unwrap() { + let name = d["name"].as_str().unwrap(); + let Some(item) = doc["paths"].get(format!("/api/{name}/create")) else { + continue; + }; + let props = + &item["post"]["requestBody"]["content"]["application/json"]["schema"]["properties"]; + for f in d["fields"].as_array().unwrap() { + let kind = f["kind"].as_str().unwrap(); + if !matches!(kind, "text" | "longtext") + || f["computed"] == true + || f["sequence"] == true + { + continue; + } + let fname = f["name"].as_str().unwrap(); + assert_eq!( + props[fname]["maxLength"], f["max_length"], + "{name}.{fname}: {}", + props[fname] + ); + seen += 1; + } + } + } + assert!(seen > 50, "{seen}"); +} + +// ---- MCP -------------------------------------------------------------------------------- + +const HOST: &str = "inst.test"; + +async fn mcp(r: &axum::Router, token: &str, session: Option<&str>, body: J) -> (u16, J, String) { + let mut req = Request::post("/mcp") + .header("host", HOST) + .header("content-type", "application/json") + .header("accept", "application/json, text/event-stream") + .header("authorization", format!("Bearer {token}")); + if let Some(s) = session { + req = req.header("mcp-session-id", s); + } + let resp = r + .clone() + .oneshot(req.body(Body::from(body.to_string())).unwrap()) + .await + .unwrap(); + let status = resp.status().as_u16(); + let sid = resp + .headers() + .get("mcp-session-id") + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_string(); + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + ( + status, + serde_json::from_slice(&bytes).unwrap_or(J::Null), + sid, + ) +} + +async fn mcp_session(rig: &Rig) -> String { + let (s, b, sid) = mcp( + &rig.r, + &rig.token, + None, + json!({"jsonrpc":"2.0","id":1,"method":"initialize","params":{ + "protocolVersion": "2025-11-25", "capabilities": {}, + "clientInfo": {"name": "test", "version": "0"}}}), + ) + .await; + assert_eq!(s, 200, "{b}"); + assert_eq!(b["result"]["serverInfo"]["version"], "0.2.1", "{b}"); + let (s, _, _) = mcp( + &rig.r, + &rig.token, + Some(&sid), + json!({"jsonrpc":"2.0","method":"notifications/initialized"}), + ) + .await; + assert_eq!(s, 202); + sid +} + +#[tokio::test] +async fn mcp_tool_schemas_carry_the_limits_and_a_call_over_the_limit_is_an_error_result() { + let rig = Rig::new().await; + let sid = mcp_session(&rig).await; + // The list comes in pages. + let mut tools: Vec = Vec::new(); + let mut cursor: Option = None; + loop { + let params = cursor.as_ref().map_or(json!({}), |c| json!({"cursor": c})); + let (s, list, _) = mcp( + &rig.r, + &rig.token, + Some(&sid), + json!({"jsonrpc":"2.0","id":2,"method":"tools/list","params":params}), + ) + .await; + assert_eq!(s, 200, "{list}"); + tools.extend(list["result"]["tools"].as_array().unwrap().clone()); + match list["result"]["nextCursor"].as_str() { + Some(c) => cursor = Some(c.to_string()), + None => break, + } + } + let tool = |name: &str| -> J { + tools + .iter() + .find(|t| t["name"] == name) + .unwrap_or_else(|| panic!("no tool {name}")) + .clone() + }; + let create = tool("lim_note__create"); + let p = &create["inputSchema"]["properties"]; + assert_eq!( + ( + p["title"]["maxLength"].clone(), + p["title"]["minLength"].clone() + ), + (json!(2000), json!(1)) + ); + assert_eq!(p["plain"]["maxLength"], 2048); + assert_eq!( + (p["qty"]["minimum"].clone(), p["qty"]["maximum"].clone()), + (json!(0), json!(100)) + ); + assert_eq!(p["rate"]["x-obop-minimum"], "0.1000000"); + assert!( + p["title"]["description"] + .as_str() + .unwrap() + .contains("(from 1 character to 2000 characters)"), + "{}", + p["title"] + ); + // An agent sees personal data as tokens such as PERSON_1, so a bound on the length of the + // real value would refuse the token: such a field gets no keyword in the tool schema. + let party = tool("party__create"); + assert!( + party["inputSchema"]["properties"]["name"] + .get("maxLength") + .is_none(), + "{}", + party["inputSchema"]["properties"]["name"] + ); + // A field of the same model that is not personal data does get it. + assert_eq!(p["body"]["maxLength"], 100_000); + + // A call that breaks a limit comes back as an error result with the sentence. + let (s, out, _) = mcp( + &rig.r, + &rig.token, + Some(&sid), + json!({"jsonrpc":"2.0","id":3,"method":"tools/call","params":{ + "name": "lim_note__create", "arguments": {"title": "x".repeat(2001)}}}), + ) + .await; + assert_eq!(s, 200, "{out}"); + assert_eq!(out["result"]["isError"], true, "{out}"); + let text = out["result"]["content"][0]["text"].as_str().unwrap(); + assert!( + text.contains("lim_note.title must be at most 2000 characters; this has 2001"), + "{text}" + ); + assert_eq!(rig.count("lim_note").await, 0); + // And a call inside the limits works. + let (_, ok, _) = mcp( + &rig.r, + &rig.token, + Some(&sid), + json!({"jsonrpc":"2.0","id":4,"method":"tools/call","params":{ + "name": "lim_note__create", "arguments": {"title": "x".repeat(2000)}}}), + ) + .await; + assert_ne!(ok["result"]["isError"], true, "{ok}"); + assert_eq!(rig.count("lim_note").await, 1); +} + +#[tokio::test] +async fn installing_a_package_with_a_limit_that_cannot_hold_is_refused() { + let rig = Rig::bare().await; + for (from, to, want) in [ + ( + "max_length = 2000", + "max_length = 0", + "`max_length` must be at least 1", + ), + ( + "max_length = 2000", + "max_length = 10001", + "above the largest a text field may declare", + ), + ( + "min_length = 1", + "min_length = 3000", + "`min_length` 3000 is above `max_length` 2000", + ), + ( + "min = 0\n max = 100\n", + "min = 5\n max = 4\n", + "`min` 5 is above `max` 4", + ), + ( + "min = \"0.1000000\"", + "min = 0.1", + "a TOML float is not exact", + ), + ] { + let bad = LIM.replace(from, to); + assert_ne!(bad, LIM, "{from}"); + let (s, b) = rig + .post("/api/module/install", json!({"manifest": bad})) + .await; + assert_eq!(s, 400, "{to}: {b}"); + assert!(error(&b).contains(want), "{to}: {b}"); + } + // Nothing was installed by the refused ones; the good package still installs. + assert!(!rig.app.dispatcher.has_model("lim_note")); + let (s, b) = rig + .post("/api/module/install", json!({"manifest": LIM})) + .await; + assert_eq!(s, 200, "{b}"); + assert!(rig.app.dispatcher.has_model("lim_note")); +} diff --git a/crates/obop/src/main.rs b/crates/obop/src/main.rs index b3c5210c..29e03f6d 100644 --- a/crates/obop/src/main.rs +++ b/crates/obop/src/main.rs @@ -28,6 +28,8 @@ mod import_cmd; #[cfg(test)] mod kinds_tests; #[cfg(test)] +mod limits_tests; +#[cfg(test)] mod mcp_tests; #[cfg(test)] mod mfa_tests; @@ -65,7 +67,7 @@ use std::process::ExitCode; use std::sync::Arc; const USAGE: &str = "usage: obop - --version the name, the core version and the commit the build was made from: obop 0.2.0 (abcdef012345) + --version the name, the core version and the commit the build was made from: obop 0.2.1 (abcdef012345) modules labels TARGET LANG a label template (every translatable text, English values) for a module, bundled package or FILE modules check [--complete] [--catalog] [FILE...] the install gate over linked modules, or over package files (no database): diff --git a/crates/obop/tests/limits_cli.rs b/crates/obop/tests/limits_cli.rs new file mode 100644 index 00000000..86f0786a --- /dev/null +++ b/crates/obop/tests/limits_cli.rs @@ -0,0 +1,147 @@ +//! `obop modules check` through the real binary on packages with field limits (D90): a good file +//! passes, and a bad one fails with the file, the model and field, and the key, so an author +//! knows where to look. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +struct Dir(PathBuf); + +impl Dir { + fn new(tag: &str) -> Dir { + let d = std::env::temp_dir().join(format!("obop-limits-cli-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + Dir(d) + } + fn file(&self, name: &str, text: &str) -> PathBuf { + let p = self.0.join(name); + std::fs::write(&p, text).unwrap(); + p + } +} + +impl Drop for Dir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +fn package(core: &str, field: &str) -> String { + format!( + r#"[module] +id = "lim" +version = "0.1.0" +name = "Lim" +description = "Limits" +license = "MIT" +requires_core = "{core}" + +[[model]] +name = "lim_note" +description = "A note" +read_group = "base.user" +write_group = "base.user" + + [[model.field]] + name = "target" + description = "The field under test" + class = "none" +{field} +"# + ) +} + +fn check(files: &[&Path]) -> Output { + let mut c = Command::new(env!("CARGO_BIN_EXE_obop")); + c.env_clear().args(["modules", "check"]).args(files); + for keep in ["SystemRoot", "SYSTEMROOT", "PATH"] { + if let Ok(v) = std::env::var(keep) { + c.env(keep, v); + } + } + c.output().unwrap() +} + +fn said(o: &Output) -> String { + format!( + "{}{}", + String::from_utf8_lossy(&o.stdout), + String::from_utf8_lossy(&o.stderr) + ) +} + +#[test] +fn a_package_with_limits_passes_and_the_bundled_ones_still_do() { + let dir = Dir::new("good"); + let good = dir.file( + "good.toml", + &package( + "^0.2.1", + " max_length = 120\n min_length = 2\n\n [[model.field]]\n name = \"qty\"\n description = \"Quantity\"\n class = \"none\"\n kind = \"number\"\n min = 0\n max = 99", + ), + ); + let o = check(&[&good]); + assert!(o.status.success(), "{}", said(&o)); + assert!(said(&o).contains("ok"), "{}", said(&o)); + // The shipped catalogue is untouched by the default length. + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let fleet = root.join("packages/fleet.toml"); + let o = check(&[&fleet]); + assert!(o.status.success(), "{}", said(&o)); +} + +#[test] +fn a_bad_limit_fails_the_run_and_names_the_file_the_field_and_the_key() { + let dir = Dir::new("bad"); + for (keys, want) in [ + (" max_length = 0", "`max_length` must be at least 1"), + ( + " max_length = 10001", + "above the largest a text field may declare", + ), + ( + " min_length = 5\n max_length = 4", + "`min_length` 5 is above `max_length` 4", + ), + ( + " kind = \"number\"\n max_length = 4", + "only valid for kind \"text\" or \"longtext\"", + ), + (" kind = \"number\"\n min = 0.5", "must be a whole number"), + ( + " kind = \"decimal\"\n precision = 10\n scale = 2\n max = 1.5", + "a TOML float is not exact", + ), + ( + " kind = \"decimal\"\n precision = 10\n scale = 2\n max = \"1.234\"", + "is not a decimal that fits decimal(10, 2)", + ), + ( + " kind = \"number\"\n min = 5\n max = 4", + "`min` 5 is above `max` 4", + ), + (" maxlength = 4", "unknown field"), + ] { + let file = dir.file("bad.toml", &package("^0.2.1", keys)); + let o = check(&[&file]); + assert!(!o.status.success(), "{keys}: {}", said(&o)); + let out = said(&o); + assert!( + out.contains("FAIL") && out.contains("bad.toml"), + "{keys}: {out}" + ); + assert!(out.contains(want), "{keys}: wanted {want:?} in {out}"); + // The field is named (not for a key the parser does not know, which names its position). + if !keys.contains("maxlength") { + assert!(out.contains("lim_note.target"), "{keys}: {out}"); + } + } + // One bad file fails the run even next to a good one. + let good = dir.file("good.toml", &package("^0.2.1", " max_length = 10")); + let bad = dir.file("bad.toml", &package("^0.2.1", " max_length = 0")); + let o = check(&[&good, &bad]); + assert!(!o.status.success()); + let out = said(&o); + assert!(out.contains("ok") && out.contains("FAIL"), "{out}"); +} From 338fde6f8cecf44451492184e6be344a7aac66ce Mon Sep 17 00:00:00 2001 From: Mikael Rinne <40919111+rorychatt@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:45:32 +0100 Subject: [PATCH 07/23] Show limits in the back office form: a character counter, bounds and the server's refusal (D90) A code-point counter ("8 of 10 characters") from four fifths of a limit, red above it; a text or number outside a limit is refused before it is sent, naming the field and the limit; number inputs take min and max; a value the person did not change is not held to a limit, as on the server; the server's own limit_violated sentence is shown on the field when the browser did not know the limit. The native maxlength attribute is not set: it counts UTF-16 units and silently cuts a paste. The six machine-drafted catalogues carry the new strings. Co-Authored-By: Claude Sonnet 5.5 --- web/apps/backoffice/locales/de/messages.json | 5 + web/apps/backoffice/locales/de/status.json | 5 + web/apps/backoffice/locales/en/messages.json | 5 + web/apps/backoffice/locales/es/messages.json | 5 + web/apps/backoffice/locales/es/status.json | 5 + web/apps/backoffice/locales/fi/messages.json | 5 + web/apps/backoffice/locales/fi/status.json | 5 + web/apps/backoffice/locales/fr/messages.json | 5 + web/apps/backoffice/locales/fr/status.json | 5 + web/apps/backoffice/locales/pl/messages.json | 5 + web/apps/backoffice/locales/pl/status.json | 5 + web/apps/backoffice/locales/sv/messages.json | 5 + web/apps/backoffice/locales/sv/status.json | 5 + web/apps/backoffice/src/RecordPanel.tsx | 2 + web/apps/backoffice/src/api.ts | 14 + web/apps/backoffice/src/edit.ts | 54 ++-- web/apps/backoffice/src/fields.tsx | 73 ++++- web/apps/backoffice/src/limits.test.tsx | 298 +++++++++++++++++++ web/apps/backoffice/src/limits.ts | 110 +++++++ web/apps/backoffice/src/modules/types.ts | 1 + web/apps/backoffice/src/record.css | 11 + web/apps/backoffice/src/serverMessage.ts | 19 ++ web/apps/backoffice/src/views/InlineEdit.tsx | 5 +- 23 files changed, 618 insertions(+), 34 deletions(-) create mode 100644 web/apps/backoffice/src/limits.test.tsx create mode 100644 web/apps/backoffice/src/limits.ts diff --git a/web/apps/backoffice/locales/de/messages.json b/web/apps/backoffice/locales/de/messages.json index bf4ac893..2c30fa16 100644 --- a/web/apps/backoffice/locales/de/messages.json +++ b/web/apps/backoffice/locales/de/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "{field} muss eine ganze Zahl sein", "{field} must be an amount": "{field} muss ein Betrag sein", "{field} must be an amount like {example}": "{field} muss ein Betrag wie {example} sein", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} muss mindestens {min, plural, one {# Zeichen} other {# Zeichen}} lang sein; hier sind es {have}", + "{field} must be at least {min}; this is {value}": "{field} muss mindestens {min} sein; der Wert ist {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} darf höchstens {max, plural, one {# Zeichen} other {# Zeichen}} lang sein; hier sind es {have}", + "{field} must be at most {max}; this is {value}": "{field} darf höchstens {max} sein; der Wert ist {value}", "{field} must be between the years 0001 and 9999": "{field} muss zwischen den Jahren 0001 und 9999 liegen", "{field} must be one of: {options}": "{field} muss einer dieser Werte sein: {options}", "{field} must be text": "{field} muss Text sein", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} von {total} können jetzt aktualisiert werden. Bei einem Modul mit blockiertem Update steht der Grund auf seiner Karte.", "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} von {max, plural, one {# Zeichen} other {# Zeichen}}", "{user} can sign in again.": "{user} kann sich wieder anmelden.", "{user} is disabled and signed out.": "{user} ist deaktiviert und abgemeldet.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} wurde ausgeschieden: {sessions} Sitzung(en), {tokens} Agent-Token und {apps} verbundene App(s) endgültig beendet.", diff --git a/web/apps/backoffice/locales/de/status.json b/web/apps/backoffice/locales/de/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/de/status.json +++ b/web/apps/backoffice/locales/de/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/locales/en/messages.json b/web/apps/backoffice/locales/en/messages.json index 4d0c60b7..97d16e4a 100644 --- a/web/apps/backoffice/locales/en/messages.json +++ b/web/apps/backoffice/locales/en/messages.json @@ -66,6 +66,10 @@ "{field} must be a whole number": "{field} must be a whole number", "{field} must be an amount": "{field} must be an amount", "{field} must be an amount like {example}": "{field} must be an amount like {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}", + "{field} must be at least {min}; this is {value}": "{field} must be at least {min}; this is {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}", + "{field} must be at most {max}; this is {value}": "{field} must be at most {max}; this is {value}", "{field} must be between the years 0001 and 9999": "{field} must be between the years 0001 and 9999", "{field} must be one of: {options}": "{field} must be one of: {options}", "{field} must be text": "{field} must be text", @@ -132,6 +136,7 @@ "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}: {value}. Open the list.": "{title}: {value}. Open the list.", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} of {max, plural, one {# character} other {# characters}}", "{user} can sign in again.": "{user} can sign in again.", "{user} is disabled and signed out.": "{user} is disabled and signed out.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.", diff --git a/web/apps/backoffice/locales/es/messages.json b/web/apps/backoffice/locales/es/messages.json index 194b4ef6..e1d8dfda 100644 --- a/web/apps/backoffice/locales/es/messages.json +++ b/web/apps/backoffice/locales/es/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "{field} debe ser un número entero", "{field} must be an amount": "{field} debe ser un importe", "{field} must be an amount like {example}": "{field} debe ser un importe como {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} debe tener al menos {min, plural, one {# carácter} other {# caracteres}}; tiene {have}", + "{field} must be at least {min}; this is {value}": "{field} debe ser como mínimo {min}; el valor es {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} debe tener como máximo {max, plural, one {# carácter} other {# caracteres}}; tiene {have}", + "{field} must be at most {max}; this is {value}": "{field} debe ser como máximo {max}; el valor es {value}", "{field} must be between the years 0001 and 9999": "{field} debe estar entre los años 0001 y 9999", "{field} must be one of: {options}": "{field} debe ser uno de estos valores: {options}", "{field} must be text": "{field} debe ser texto", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} de {total} se pueden actualizar ahora. Si la actualización de un módulo está bloqueada, su tarjeta indica el motivo.", "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} de {max, plural, one {# carácter} other {# caracteres}}", "{user} can sign in again.": "{user} puede volver a iniciar sesión.", "{user} is disabled and signed out.": "{user} está desactivado y con la sesión cerrada.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} se ha dado de baja: {sessions} sesión(es), {tokens} token(s) de agente y {apps} aplicación(es) conectada(s) terminadas definitivamente.", diff --git a/web/apps/backoffice/locales/es/status.json b/web/apps/backoffice/locales/es/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/es/status.json +++ b/web/apps/backoffice/locales/es/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/locales/fi/messages.json b/web/apps/backoffice/locales/fi/messages.json index 88d7c4c9..f28009f8 100644 --- a/web/apps/backoffice/locales/fi/messages.json +++ b/web/apps/backoffice/locales/fi/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "{field} on oltava kokonaisluku", "{field} must be an amount": "{field} on oltava rahamäärä", "{field} must be an amount like {example}": "{field} on oltava rahamäärä, esimerkiksi {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} on oltava vähintään {min, plural, one {# merkki} other {# merkkiä}} pitkä; nyt merkkejä on {have}", + "{field} must be at least {min}; this is {value}": "{field} on oltava vähintään {min}; arvo on {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} saa olla enintään {max, plural, one {# merkki} other {# merkkiä}} pitkä; nyt merkkejä on {have}", + "{field} must be at most {max}; this is {value}": "{field} saa olla enintään {max}; arvo on {value}", "{field} must be between the years 0001 and 9999": "{field} on oltava vuosien 0001 ja 9999 välillä", "{field} must be one of: {options}": "{field} on oltava jokin seuraavista: {options}", "{field} must be text": "{field} on oltava tekstiä", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} / {total} voidaan päivittää nyt. Moduulin kortti kertoo, miksi sen päivitys on estetty.", "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} / {max, plural, one {# merkki} other {# merkkiä}}", "{user} can sign in again.": "{user} voi kirjautua taas sisään.", "{user} is disabled and signed out.": "{user} on poistettu käytöstä ja kirjattu ulos.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} on poistettu käytöstä lopullisesti: {sessions} istuntoa, {tokens} agenttitunnistetta ja {apps} yhdistettyä sovellusta päätetty pysyvästi.", diff --git a/web/apps/backoffice/locales/fi/status.json b/web/apps/backoffice/locales/fi/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/fi/status.json +++ b/web/apps/backoffice/locales/fi/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/locales/fr/messages.json b/web/apps/backoffice/locales/fr/messages.json index 65d1e87e..4721a23e 100644 --- a/web/apps/backoffice/locales/fr/messages.json +++ b/web/apps/backoffice/locales/fr/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "{field} doit être un nombre entier", "{field} must be an amount": "{field} doit être un montant", "{field} must be an amount like {example}": "{field} doit être un montant, par exemple {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} doit comporter au moins {min, plural, one {# caractère} other {# caractères}} ; il y en a {have}", + "{field} must be at least {min}; this is {value}": "{field} doit être supérieur ou égal à {min} ; la valeur est {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} doit comporter au plus {max, plural, one {# caractère} other {# caractères}} ; il y en a {have}", + "{field} must be at most {max}; this is {value}": "{field} doit être inférieur ou égal à {max} ; la valeur est {value}", "{field} must be between the years 0001 and 9999": "{field} doit se situer entre les années 0001 et 9999", "{field} must be one of: {options}": "{field} doit avoir l’une des valeurs suivantes : {options}", "{field} must be text": "{field} doit être du texte", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} sur {total} peuvent être mis à jour maintenant. Lorsque la mise à jour d’un module est bloquée, sa carte en indique la raison.", "{row}, {column}: {value}": "{row}, {column} : {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} sur {max, plural, one {# caractère} other {# caractères}}", "{user} can sign in again.": "{user} peut de nouveau se connecter.", "{user} is disabled and signed out.": "{user} est désactivé et déconnecté.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} est hors service : {sessions} session(s), {tokens} jeton(s) d’agent et {apps} application(s) connectée(s) définitivement terminés.", diff --git a/web/apps/backoffice/locales/fr/status.json b/web/apps/backoffice/locales/fr/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/fr/status.json +++ b/web/apps/backoffice/locales/fr/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/locales/pl/messages.json b/web/apps/backoffice/locales/pl/messages.json index b298bef3..433afc1a 100644 --- a/web/apps/backoffice/locales/pl/messages.json +++ b/web/apps/backoffice/locales/pl/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "Pole „{field}” musi być liczbą całkowitą", "{field} must be an amount": "Pole „{field}” musi być kwotą", "{field} must be an amount like {example}": "Pole „{field}” musi być kwotą, np. {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} musi mieć co najmniej {min, plural, one {# znak} few {# znaki} many {# znaków} other {# znaku}}; wpisano {have}", + "{field} must be at least {min}; this is {value}": "{field} musi wynosić co najmniej {min}; wartość to {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} może mieć co najwyżej {max, plural, one {# znak} few {# znaki} many {# znaków} other {# znaku}}; wpisano {have}", + "{field} must be at most {max}; this is {value}": "{field} może wynosić co najwyżej {max}; wartość to {value}", "{field} must be between the years 0001 and 9999": "{field} musi mieścić się między rokiem 0001 a 9999", "{field} must be one of: {options}": "Pole „{field}” musi mieć jedną z wartości: {options}", "{field} must be text": "Pole „{field}” musi być tekstem", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} z {total} można teraz zaktualizować. Gdy aktualizacja modułu jest zablokowana, jego karta podaje powód.", "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} z {max, plural, one {# znaku} few {# znaków} many {# znaków} other {# znaku}}", "{user} can sign in again.": "{user} może się znowu zalogować.", "{user} is disabled and signed out.": "Konto {user} jest wyłączone, a użytkownik wylogowany.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} został wyłączony z firmy: {sessions} sesji, {tokens} tokenów agentów i {apps} połączonych aplikacji zakończono na stałe.", diff --git a/web/apps/backoffice/locales/pl/status.json b/web/apps/backoffice/locales/pl/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/pl/status.json +++ b/web/apps/backoffice/locales/pl/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/locales/sv/messages.json b/web/apps/backoffice/locales/sv/messages.json index 227c08b5..feef386d 100644 --- a/web/apps/backoffice/locales/sv/messages.json +++ b/web/apps/backoffice/locales/sv/messages.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "{field} måste vara ett heltal", "{field} must be an amount": "{field} måste vara ett belopp", "{field} must be an amount like {example}": "{field} måste vara ett belopp, till exempel {example}", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "{field} måste vara minst {min, plural, one {# tecken} other {# tecken}} långt; nu är det {have}", + "{field} must be at least {min}; this is {value}": "{field} måste vara minst {min}; värdet är {value}", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "{field} får vara högst {max, plural, one {# tecken} other {# tecken}} långt; nu är det {have}", + "{field} must be at most {max}; this is {value}": "{field} får vara högst {max}; värdet är {value}", "{field} must be between the years 0001 and 9999": "{field} måste ligga mellan åren 0001 och 9999", "{field} must be one of: {options}": "{field} måste vara något av: {options}", "{field} must be text": "{field} måste vara text", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "{ready} av {total} kan uppdateras nu. På kortet för en modul vars uppdatering är blockerad står det varför.", "{row}, {column}: {value}": "{row}, {column}: {value}", "{title}. {values}.": "{title}. {values}.", + "{used} of {max, plural, one {# character} other {# characters}}": "{used} av {max, plural, one {# tecken} other {# tecken}}", "{user} can sign in again.": "{user} kan logga in igen.", "{user} is disabled and signed out.": "{user} är inaktiverad och utloggad.", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "{user} är avslutad: {sessions} session(er), {tokens} agent-token och {apps} anslutna appar har avslutats för gott.", diff --git a/web/apps/backoffice/locales/sv/status.json b/web/apps/backoffice/locales/sv/status.json index 602cdab2..f01a61c5 100644 --- a/web/apps/backoffice/locales/sv/status.json +++ b/web/apps/backoffice/locales/sv/status.json @@ -1808,6 +1808,10 @@ "{field} must be a whole number": "machine-drafted", "{field} must be an amount": "machine-drafted", "{field} must be an amount like {example}": "machine-drafted", + "{field} must be at least {min, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at least {min}; this is {value}": "machine-drafted", + "{field} must be at most {max, plural, one {# character} other {# characters}}; this has {have}": "machine-drafted", + "{field} must be at most {max}; this is {value}": "machine-drafted", "{field} must be between the years 0001 and 9999": "machine-drafted", "{field} must be one of: {options}": "machine-drafted", "{field} must be text": "machine-drafted", @@ -1875,6 +1879,7 @@ "{ready} of {total} can be updated now. A module whose update is blocked says why on its card.": "machine-drafted", "{row}, {column}: {value}": "machine-drafted", "{title}. {values}.": "machine-drafted", + "{used} of {max, plural, one {# character} other {# characters}}": "machine-drafted", "{user} can sign in again.": "machine-drafted", "{user} is disabled and signed out.": "machine-drafted", "{user} is offboarded: {sessions} session(s), {tokens} agent token(s) and {apps} connected app(s) ended for good.": "machine-drafted", diff --git a/web/apps/backoffice/src/RecordPanel.tsx b/web/apps/backoffice/src/RecordPanel.tsx index 517b5243..bff75191 100644 --- a/web/apps/backoffice/src/RecordPanel.tsx +++ b/web/apps/backoffice/src/RecordPanel.tsx @@ -178,6 +178,8 @@ export function RecordPanel({ value, creating ? undefined : (f) => changed(f) || f.required, currency, + // A value left as stored is not held to limits lowered since it was written (D90). + (f) => !creating && !changed(f), ); setErrors(errs); if (Object.keys(errs).length) return d.notify(t("Fix the highlighted fields first"), "error"); diff --git a/web/apps/backoffice/src/api.ts b/web/apps/backoffice/src/api.ts index 43dc2c95..ec6a066c 100644 --- a/web/apps/backoffice/src/api.ts +++ b/web/apps/backoffice/src/api.ts @@ -55,6 +55,20 @@ export type FieldMeta = { precision?: number; scale?: number; round?: "half_even" | "half_up" | "down"; + /** + * Only for `text` and `longtext` (D90): the most characters (Unicode code points, so "å" and an + * emoji count one each) a written value has, which is the default of the kind when the package + * declares none, and the fewest, when it declares a minimum. Absent for computed and numbered + * fields. + */ + max_length?: number; + min_length?: number; + /** + * Only for `number`, `money` (minor units) and `decimal` (a canonical string at the scale of the + * field) (D90): the inclusive bounds the package declares. + */ + min?: number | string; + max?: number | string; /** The model the field belongs to (set when the metadata is loaded). */ model?: string; /** Moves only through the model's actions, also for someone who can run none of them (D48). */ diff --git a/web/apps/backoffice/src/edit.ts b/web/apps/backoffice/src/edit.ts index ad204095..349808ff 100644 --- a/web/apps/backoffice/src/edit.ts +++ b/web/apps/backoffice/src/edit.ts @@ -4,6 +4,7 @@ import { canonicalDecimal, decimalInput } from "./decimals.ts"; import { isEmpty, majorString, optionLabel, parseMoney, validDate } from "./format.ts"; import { fromZonedInput, toZonedInput } from "./instants.ts"; import { fieldTitle } from "./labels.ts"; +import { rangeError, textLimitError } from "./limits.ts"; /** What a form control holds: text for everything, a boolean for checkboxes. */ export type Edit = string | boolean; @@ -17,11 +18,16 @@ export function toEdit(f: FieldMeta, v: unknown, currency?: string): Edit { return String(v); } -/** Turn what was typed into the wire value, or say why it cannot be one. `null` means empty. */ +/** + * Turn what was typed into the wire value, or say why it cannot be one. `null` means empty. A value + * the person did not change (`unchanged`) is not held to the length and range limits of the field, + * as on the server (D90): a record stored above a limit that was lowered later can still be saved. + */ export function fromEdit( f: FieldMeta, e: Edit, currency?: string, + unchanged = false, ): { value: unknown } | { error: string } { const field = fieldTitle(f); if (f.kind === "bool") return { value: e === true }; @@ -30,20 +36,23 @@ export function fromEdit( return f.required ? { error: t("{field} is required", { field }) } : { value: null }; } switch (f.kind) { - case "number": - return /^-?\d+$/.test(s) - ? { value: parseInt(s, 10) } - : { error: t("{field} must be a whole number", { field }) }; + case "number": { + if (!/^-?\d+$/.test(s)) return { error: t("{field} must be a whole number", { field }) }; + const n = parseInt(s, 10); + const out = unchanged ? null : rangeError(field, f, n); + return out ? { error: out } : { value: n }; + } case "money": { const m = parseMoney(s, currency); - return m === null - ? { - error: t("{field} must be an amount like {example}", { - field, - example: fmt.moneyInput(1250, currency), - }), - } - : { value: m }; + if (m === null) + return { + error: t("{field} must be an amount like {example}", { + field, + example: fmt.moneyInput(1250, currency), + }), + }; + const out = unchanged ? null : rangeError(field, f, m, currency); + return out ? { error: out } : { value: m }; } case "date": return validDate(s) ? { value: s } : { error: t("{field} must be a valid date", { field }) }; @@ -83,7 +92,9 @@ export function fromEdit( digits, }), }; - return { value: canonicalDecimal(p.negative, p.int, p.frac, scale) }; + const dec = canonicalDecimal(p.negative, p.int, p.frac, scale); + const out = unchanged ? null : rangeError(field, f, dec); + return out ? { error: out } : { value: dec }; } case "datetime": { const { zone } = getTime(); @@ -119,22 +130,29 @@ export function fromEdit( options: (f.options ?? []).map((o) => optionLabel(o, f)).join(", "), }), }; - default: - return { value: e as string }; + default: { + // The server counts what is sent, untrimmed (D90). + const out = unchanged ? null : textLimitError(field, f, String(e)); + return out ? { error: out } : { value: e as string }; + } } } -/** Validate a whole draft; returns errors by field name. */ +/** + * Validate a whole draft; returns errors by field name. `unchanged` says which fields hold the + * stored value, which are not held to the limits of their field (see `fromEdit`). + */ export function validate( fields: FieldMeta[], values: (f: FieldMeta) => Edit, only?: (f: FieldMeta) => boolean, currency?: string, + unchanged?: (f: FieldMeta) => boolean, ): Record { const errors: Record = {}; for (const f of fields) { if (only && !only(f)) continue; - const r = fromEdit(f, values(f), currency); + const r = fromEdit(f, values(f), currency, unchanged?.(f) ?? false); if ("error" in r) errors[f.name] = r.error; } return errors; diff --git a/web/apps/backoffice/src/fields.tsx b/web/apps/backoffice/src/fields.tsx index b0da149b..55046aa9 100644 --- a/web/apps/backoffice/src/fields.tsx +++ b/web/apps/backoffice/src/fields.tsx @@ -15,6 +15,7 @@ import { shownLabel, } from "./format.ts"; import { fieldTitle, modelTitle } from "./labels.ts"; +import { counterState } from "./limits.ts"; import { BlobInput, BlobThumb, FileChip } from "./BlobFields.tsx"; import { Badge, Icon } from "./ui.tsx"; @@ -142,10 +143,44 @@ type InputProps = { shown?: string; }; +/** The ids a control is described by: the error, and the counter while it shows. */ +function describe( + errorId: string | undefined, + counterId: string, + counter: unknown, +): string | undefined { + return [errorId, counter ? counterId : ""].filter(Boolean).join(" ") || undefined; +} + +/** `1 842 of 2 000 characters`: red once the text is longer than the field allows. */ +function CharCounter({ + id, + state, +}: { + id: string; + state: { used: number; max: number; over: boolean } | null; +}) { + if (!state) return null; + return ( + + {t("{used} of {max, plural, one {# character} other {# characters}}", { + used: fmt.number(state.used), + max: state.max, + })} + + ); +} + /** The control for one field, chosen by its kind. */ export function FieldInput(p: InputProps) { const { field: f, value, onChange, readOnly } = p; const readText = readOnly && p.shown !== undefined ? p.shown : String(value); + // How full a text is (D90), shown only near the limit, and only where it can still be edited. + const counterId = `${useId()}-count`; + const counter = + !readOnly && (f.kind === "text" || f.kind === "longtext") + ? counterState(f, String(value)) + : null; const common = { id: p.id, autoFocus: p.autoFocus, @@ -191,13 +226,17 @@ export function FieldInput(p: InputProps) { ); case "longtext": return ( -