From 84cfff6d32c5b58a04dada9e41fdcc572882be73 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:19:57 +0200 Subject: [PATCH 1/6] Make sure validation errors are triggered --- crates/fj/src/handle_model.rs | 21 ++++++++++++++++----- models/all/src/main.rs | 5 +++-- models/cuboid/src/main.rs | 5 +++-- models/spacer/src/main.rs | 5 +++-- models/star/src/main.rs | 5 +++-- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/crates/fj/src/handle_model.rs b/crates/fj/src/handle_model.rs index 407c456e1..53c1ed1ee 100644 --- a/crates/fj/src/handle_model.rs +++ b/crates/fj/src/handle_model.rs @@ -1,9 +1,12 @@ use std::ops::Deref; -use fj_core::algorithms::{ - approx::{InvalidTolerance, Tolerance}, - bounding_volume::BoundingVolume, - triangulate::Triangulate, +use fj_core::{ + algorithms::{ + approx::{InvalidTolerance, Tolerance}, + bounding_volume::BoundingVolume, + triangulate::Triangulate, + }, + services::Services, }; use fj_interop::model::Model; use fj_math::{Aabb, Point, Scalar}; @@ -18,13 +21,21 @@ use crate::Args; /// /// This function is used by Fornjot's own testing infrastructure, but is useful /// beyond that, when using Fornjot directly to define a model. -pub fn handle_model(model: impl Deref) -> Result +pub fn handle_model( + model: impl Deref, + services: Services, +) -> Result where for<'r> (&'r M, Tolerance): Triangulate, M: BoundingVolume<3>, { let args = Args::parse(); + // Dropping `Services` will cause a panic, if there are any unhandled + // validation errors. It would be better to return an error, but this will + // do for now. + drop(services); + let aabb = model.aabb().unwrap_or(Aabb { min: Point::origin(), max: Point::origin(), diff --git a/models/all/src/main.rs b/models/all/src/main.rs index 27e7d3073..d41417248 100644 --- a/models/all/src/main.rs +++ b/models/all/src/main.rs @@ -1,7 +1,8 @@ use fj::{core::services::Services, handle_model}; fn main() -> fj::Result { - let model = all::model(&mut Services::new()); - handle_model(model)?; + let mut services = Services::new(); + let model = all::model(&mut services); + handle_model(model, services)?; Ok(()) } diff --git a/models/cuboid/src/main.rs b/models/cuboid/src/main.rs index 6b9fdc21f..23edaa890 100644 --- a/models/cuboid/src/main.rs +++ b/models/cuboid/src/main.rs @@ -1,7 +1,8 @@ use fj::{core::services::Services, handle_model}; fn main() -> fj::Result { - let model = cuboid::model(3., 2., 1., &mut Services::new()); - handle_model(model)?; + let mut services = Services::new(); + let model = cuboid::model(3., 2., 1., &mut services); + handle_model(model, services)?; Ok(()) } diff --git a/models/spacer/src/main.rs b/models/spacer/src/main.rs index 18bf6af09..64ed2d1d4 100644 --- a/models/spacer/src/main.rs +++ b/models/spacer/src/main.rs @@ -1,7 +1,8 @@ use fj::{core::services::Services, handle_model}; fn main() -> fj::Result { - let model = spacer::model(1., 0.5, 1., &mut Services::new()); - handle_model(model)?; + let mut services = Services::new(); + let model = spacer::model(1., 0.5, 1., &mut services); + handle_model(model, services)?; Ok(()) } diff --git a/models/star/src/main.rs b/models/star/src/main.rs index 32a8f8a8d..6022870fe 100644 --- a/models/star/src/main.rs +++ b/models/star/src/main.rs @@ -1,7 +1,8 @@ use fj::{core::services::Services, handle_model}; fn main() -> fj::Result { - let model = star::model(5, 1., 2., 1., &mut Services::new()); - handle_model(model)?; + let mut services = Services::new(); + let model = star::model(5, 1., 2., 1., &mut services); + handle_model(model, services)?; Ok(()) } From 01b8e3631a8b6b28aae9d143033ad809c6a4941f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:28:19 +0200 Subject: [PATCH 2/6] Add `ValidationErrors` --- crates/fj-core/src/validate/mod.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 9e93a26dc..ea8ea3c45 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -16,7 +16,7 @@ pub use self::{ solid::SolidValidationError, }; -use std::convert::Infallible; +use std::{convert::Infallible, fmt}; use fj_math::Scalar; @@ -124,3 +124,21 @@ impl From for ValidationError { match infallible {} } } + +/// A collection of validation errors +#[derive(Debug, thiserror::Error)] +pub struct ValidationErrors(pub Vec); + +impl fmt::Display for ValidationErrors { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let num_errors = self.0.len(); + + writeln!(f, "{num_errors} unhandled validation errors:")?; + + for err in &self.0 { + writeln!(f, "{err}")?; + } + + Ok(()) + } +} From 1ce1670b5275470a5ec18cb011b6b8dffbbd2570 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:29:13 +0200 Subject: [PATCH 3/6] Make struct field public --- crates/fj-core/src/services/validation.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/services/validation.rs b/crates/fj-core/src/services/validation.rs index afa3b59e9..1b64ec8aa 100644 --- a/crates/fj-core/src/services/validation.rs +++ b/crates/fj-core/src/services/validation.rs @@ -11,7 +11,8 @@ use super::State; /// Errors that occurred while validating the objects inserted into the stores #[derive(Default)] pub struct Validation { - errors: BTreeMap, + /// All unhandled validation errors + pub errors: BTreeMap, } impl Drop for Validation { From 7f39c594f856c47bfffc8de72ee5486b796bcef3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:35:46 +0200 Subject: [PATCH 4/6] Add `Services::drop_and_validate` --- crates/fj-core/src/services/mod.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/services/mod.rs b/crates/fj-core/src/services/mod.rs index 71c3310d2..ee3c48539 100644 --- a/crates/fj-core/src/services/mod.rs +++ b/crates/fj-core/src/services/mod.rs @@ -6,7 +6,10 @@ mod objects; mod service; mod validation; -use crate::objects::{Object, ObjectSet, Objects, WithHandle}; +use crate::{ + objects::{Object, ObjectSet, Objects, WithHandle}, + validate::ValidationErrors, +}; pub use self::{ objects::{InsertObject, Operation}, @@ -61,6 +64,19 @@ impl Services { self.validation .execute(ValidationCommand::OnlyValidate { objects }, &mut events); } + + /// Drop `Services`; return any unhandled validation error + pub fn drop_and_validate(self) -> Result<(), ValidationErrors> { + let errors = ValidationErrors( + self.validation.errors.values().cloned().collect(), + ); + + if errors.0.is_empty() { + Ok(()) + } else { + Err(errors) + } + } } impl Default for Services { From 179af82b253a9c65e7ce4615b0f151f779a35ec3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:38:32 +0200 Subject: [PATCH 5/6] Don't panic on validation errors in `handle_model` --- crates/fj/src/handle_model.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/fj/src/handle_model.rs b/crates/fj/src/handle_model.rs index 53c1ed1ee..26cb8bc4c 100644 --- a/crates/fj/src/handle_model.rs +++ b/crates/fj/src/handle_model.rs @@ -7,6 +7,7 @@ use fj_core::{ triangulate::Triangulate, }, services::Services, + validate::ValidationErrors, }; use fj_interop::model::Model; use fj_math::{Aabb, Point, Scalar}; @@ -31,10 +32,7 @@ where { let args = Args::parse(); - // Dropping `Services` will cause a panic, if there are any unhandled - // validation errors. It would be better to return an error, but this will - // do for now. - drop(services); + services.drop_and_validate()?; let aabb = model.aabb().unwrap_or(Aabb { min: Point::origin(), @@ -91,4 +89,8 @@ pub enum Error { /// Invalid tolerance #[error(transparent)] Tolerance(#[from] InvalidTolerance), + + /// Unhandled validation errors + #[error(transparent)] + Validation(#[from] ValidationErrors), } From 3e10906c722f80725fc709c7a8700c9a37f56ba1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 09:49:09 +0200 Subject: [PATCH 6/6] Add `--ignore-validation` flag --- crates/fj/src/args.rs | 4 ++++ crates/fj/src/handle_model.rs | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/fj/src/args.rs b/crates/fj/src/args.rs index 747bd775a..8753f1e0a 100644 --- a/crates/fj/src/args.rs +++ b/crates/fj/src/args.rs @@ -23,6 +23,10 @@ pub struct Args { /// How much the export can deviate from the original model #[arg(short, long, value_parser = parse_tolerance)] pub tolerance: Option, + + /// Ignore validation errors + #[arg(short, long)] + pub ignore_validation: bool, } impl Args { diff --git a/crates/fj/src/handle_model.rs b/crates/fj/src/handle_model.rs index 26cb8bc4c..5417587a4 100644 --- a/crates/fj/src/handle_model.rs +++ b/crates/fj/src/handle_model.rs @@ -1,4 +1,4 @@ -use std::ops::Deref; +use std::{mem, ops::Deref}; use fj_core::{ algorithms::{ @@ -32,7 +32,11 @@ where { let args = Args::parse(); - services.drop_and_validate()?; + if args.ignore_validation { + mem::forget(services); + } else { + services.drop_and_validate()?; + } let aabb = model.aabb().unwrap_or(Aabb { min: Point::origin(),