Merge pull request #1907 from hannobraun/validation

Improve handling of validation errors

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 {

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<ObjectId, ValidationError>,
+    /// All unhandled validation errors
+    pub errors: BTreeMap<ObjectId, ValidationError>,
 }
 
 impl Drop for Validation {

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<Infallible> for ValidationError {
         match infallible {}
     }
 }
+
+/// A collection of validation errors
+#[derive(Debug, thiserror::Error)]
+pub struct ValidationErrors(pub Vec<ValidationError>);
+
+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(())
+    }
+}

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<Tolerance>,
+
+    /// Ignore validation errors
+    #[arg(short, long)]
+    pub ignore_validation: bool,
 }
 
 impl Args {

crates/fj/src/handle_model.rs

@@ -1,9 +1,13 @@
-use std::ops::Deref;
-
-use fj_core::algorithms::{
-    approx::{InvalidTolerance, Tolerance},
-    bounding_volume::BoundingVolume,
-    triangulate::Triangulate,
+use std::{mem, ops::Deref};
+
+use fj_core::{
+    algorithms::{
+        approx::{InvalidTolerance, Tolerance},
+        bounding_volume::BoundingVolume,
+        triangulate::Triangulate,
+    },
+    services::Services,
+    validate::ValidationErrors,
 };
 use fj_interop::model::Model;
 use fj_math::{Aabb, Point, Scalar};
@@ -18,13 +22,22 @@ 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<M>(model: impl Deref<Target = M>) -> Result
+pub fn handle_model<M>(
+    model: impl Deref<Target = M>,
+    services: Services,
+) -> Result
 where
     for<'r> (&'r M, Tolerance): Triangulate,
     M: BoundingVolume<3>,
 {
     let args = Args::parse();
 
+    if args.ignore_validation {
+        mem::forget(services);
+    } else {
+        services.drop_and_validate()?;
+    }
+
     let aabb = model.aabb().unwrap_or(Aabb {
         min: Point::origin(),
         max: Point::origin(),
@@ -80,4 +93,8 @@ pub enum Error {
     /// Invalid tolerance
     #[error(transparent)]
     Tolerance(#[from] InvalidTolerance),
+
+    /// Unhandled validation errors
+    #[error(transparent)]
+    Validation(#[from] ValidationErrors),
 }

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(())
 }

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(())
 }

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(())
 }

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(())
 }