Merge pull request #2213 from hannobraun/layers

Clean up the layers code

Author: hannobraun

Cargo.toml

@@ -51,6 +51,12 @@ categories = ["encoding", "mathematics", "rendering"]
 [workspace.lints.rust]
 missing_docs = "warn"
 
+[workspace.lints.clippy]
+# I really don't see any point in this. It can make a public API ugly, but
+# a) that will be obvious, even without a lint, and
+# b) it provides benefits in private APIs with top-level re-exports.
+module_inception = "allow"
+
 
 [workspace.dependencies.fj]
 version = "0.48.0"

crates/fj-core/src/layers/layers.rs

@@ -0,0 +1,49 @@
+use crate::{
+    objects::Objects,
+    validate::{Validation, ValidationConfig},
+};
+
+use super::Layer;
+
+/// # Loosely coupled layers, that together define shapes
+///
+/// Shapes are not a monolithic thing in Fornjot, but instead are defined by
+/// several, loosely coupled layers. These layers are owned by this struct.
+///
+/// ## Implementation Note
+///
+/// It is totally conceivable that one day, this system of layers is extensible
+/// and more layers can be defined by third-party code. The foundation for that,
+/// the loose coupling and inter-layer communication via events, is already
+/// there, conceptually.
+///
+/// For now, there is no need for this, and all layers are just hardcoded here.
+/// That can be changed, once necessary.
+#[derive(Default)]
+pub struct Layers {
+    /// The objects layers
+    ///
+    /// Manages the stores of topological and geometric objects that make up
+    /// shapes.
+    pub objects: Layer<Objects>,
+
+    /// The validation layer
+    ///
+    /// Monitors objects and validates them, as they are inserted.
+    pub validation: Layer<Validation>,
+}
+
+impl Layers {
+    /// Construct an instance of `Layers`
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    /// Construct an instance of `Layers`, using the provided configuration
+    pub fn with_validation_config(config: ValidationConfig) -> Self {
+        Self {
+            validation: Layer::new(Validation::with_validation_config(config)),
+            ..Default::default()
+        }
+    }
+}

crates/fj-core/src/layers/mod.rs

@@ -2,88 +2,13 @@
 //!
 //! See [`Layers`].
 
-mod layer;
-mod objects;
-mod validation;
+pub mod objects;
+pub mod validation;
 
-use crate::{
-    objects::{AboutToBeStored, AnyObject, Objects},
-    validate::{ValidationConfig, ValidationErrors},
-};
+mod layer;
+mod layers;
 
 pub use self::{
     layer::{Layer, State},
-    objects::{InsertObject, Operation},
-    validation::{Validation, ValidationCommand, ValidationEvent},
+    layers::Layers,
 };
-
-/// # Loosely coupled layers, that together define shapes
-///
-/// Shapes are not a monolithic thing in Fornjot, but instead are defined by
-/// several, loosely coupled layers. These layers are owned by this struct.
-///
-/// ## Implementation Note
-///
-/// It is totally conceivable that one day, this system of layers is extensible
-/// and more layers can be defined by third-party code. The foundation for that,
-/// the loose coupling and inter-layer communication via events, is already
-/// there, conceptually.
-///
-/// For now, there is no need for this, and all layers are just hardcoded here.
-/// That can be changed, once necessary.
-#[derive(Default)]
-pub struct Layers {
-    /// The objects layers
-    ///
-    /// Manages the stores of topological and geometric objects that make up
-    /// shapes.
-    pub objects: Layer<Objects>,
-
-    /// The validation layer
-    ///
-    /// Monitors objects and validates them, as they are inserted.
-    pub validation: Layer<Validation>,
-}
-
-impl Layers {
-    /// Construct an instance of `Layers`
-    pub fn new() -> Self {
-        Self::default()
-    }
-
-    /// Construct an instance of `Layers`, using the provided configuration
-    pub fn with_validation_config(config: ValidationConfig) -> Self {
-        let objects = Layer::default();
-        let validation = Layer::new(Validation::with_validation_config(config));
-
-        Self {
-            objects,
-            validation,
-        }
-    }
-
-    /// Insert an object into the stores
-    pub fn insert_object(&mut self, object: AnyObject<AboutToBeStored>) {
-        let mut object_events = Vec::new();
-        self.objects
-            .process(Operation::InsertObject { object }, &mut object_events);
-
-        for object_event in object_events {
-            let command = ValidationCommand::ValidateObject {
-                object: object_event.object.into(),
-            };
-            self.validation.process(command, &mut Vec::new());
-        }
-    }
-
-    /// Drop `Layers`; return any unhandled validation error
-    pub fn drop_and_validate(self) -> Result<(), ValidationErrors> {
-        let errors = self.validation.into_state().into_errors();
-
-        if errors.0.is_empty() {
-            Ok(())
-        } else {
-            Err(errors)
-        }
-    }
-}

crates/fj-core/src/layers/objects.rs

@@ -1,24 +1,46 @@
-use crate::objects::{AboutToBeStored, AnyObject, Objects};
+//! Layer infrastructure for [`Objects`]
 
-use super::State;
+use crate::{
+    objects::{AboutToBeStored, AnyObject, Objects},
+    validate::Validation,
+};
+
+use super::{Layer, State};
+
+impl Layer<Objects> {
+    /// Insert and object into the stores
+    pub fn insert(
+        &mut self,
+        object: AnyObject<AboutToBeStored>,
+        validation: &mut Layer<Validation>,
+    ) {
+        let mut events = Vec::new();
+        self.process(ObjectsCommand::InsertObject { object }, &mut events);
+
+        for event in events {
+            validation.on_objects_event(event);
+        }
+    }
+}
 
 impl State for Objects {
-    type Command = Operation;
-    type Event = InsertObject;
+    type Command = ObjectsCommand;
+    type Event = ObjectsEvent;
 
     fn decide(&self, command: Self::Command, events: &mut Vec<Self::Event>) {
-        let Operation::InsertObject { object } = command;
-        events.push(InsertObject { object });
+        let ObjectsCommand::InsertObject { object } = command;
+        events.push(ObjectsEvent::InsertObject { object });
     }
 
     fn evolve(&mut self, event: &Self::Event) {
-        event.object.clone().insert(self);
+        let ObjectsEvent::InsertObject { object } = event;
+        object.clone().insert(self);
     }
 }
 
 /// Command for `Layer<Objects>`
 #[derive(Debug)]
-pub enum Operation {
+pub enum ObjectsCommand {
     /// Insert an object into the stores
     ///
     /// This is the one primitive operation that all other operations are built
@@ -31,7 +53,10 @@ pub enum Operation {
 
 /// Event produced by `Layer<Objects>`
 #[derive(Clone, Debug)]
-pub struct InsertObject {
-    /// The object to insert
-    pub object: AnyObject<AboutToBeStored>,
+pub enum ObjectsEvent {
+    /// Insert an object into the stores
+    InsertObject {
+        /// The object to insert
+        object: AnyObject<AboutToBeStored>,
+    },
 }

crates/fj-core/src/layers/validation.rs

@@ -1,62 +1,30 @@
-use std::{collections::HashMap, error::Error, thread};
+//! Layer infrastructure for [`Validation`]
 
 use crate::{
     objects::{AnyObject, Stored},
-    storage::ObjectId,
-    validate::{ValidationConfig, ValidationError, ValidationErrors},
+    validate::{Validation, ValidationError, ValidationErrors},
 };
 
-use super::State;
+use super::{objects::ObjectsEvent, Layer, State};
 
-/// Errors that occurred while validating the objects inserted into the stores
-#[derive(Default)]
-pub struct Validation {
-    /// All unhandled validation errors
-    errors: HashMap<ObjectId, ValidationError>,
-
-    /// Validation configuration for the validation service
-    config: ValidationConfig,
-}
-
-impl Validation {
-    /// Construct an instance of `Validation`, using the provided configuration
-    pub fn with_validation_config(config: ValidationConfig) -> Self {
-        let errors = HashMap::new();
-        Self { errors, config }
+impl Layer<Validation> {
+    /// Handler for [`ObjectsEvent`]
+    pub fn on_objects_event(&mut self, event: ObjectsEvent) {
+        let ObjectsEvent::InsertObject { object } = event;
+        let command = ValidationCommand::ValidateObject {
+            object: object.into(),
+        };
+        self.process(command, &mut Vec::new());
     }
 
-    /// Drop this instance, returning the errors it contained
-    pub fn into_errors(mut self) -> ValidationErrors {
-        ValidationErrors(self.errors.drain().map(|(_, error)| error).collect())
-    }
-}
-
-impl Drop for Validation {
-    fn drop(&mut self) {
-        let num_errors = self.errors.len();
-        if num_errors > 0 {
-            println!(
-                "Dropping `Validation` with {num_errors} unhandled validation \
-                errors:"
-            );
+    /// Consume the validation layer, returning any validation errors
+    pub fn into_result(self) -> Result<(), ValidationErrors> {
+        let errors = self.into_state().into_errors();
 
-            for err in self.errors.values() {
-                println!("{}", err);
-
-                // Once `Report` is stable, we can replace this:
-                // https://doc.rust-lang.org/std/error/struct.Report.html
-                let mut source = err.source();
-                while let Some(err) = source {
-                    println!("\nCaused by:\n\t{err}");
-                    source = err.source();
-                }
-
-                print!("\n\n");
-            }
-
-            if !thread::panicking() {
-                panic!();
-            }
+        if errors.0.is_empty() {
+            Ok(())
+        } else {
+            Err(errors)
         }
     }
 }

crates/fj-core/src/operations/insert/insert_trait.rs

@@ -40,7 +40,10 @@ macro_rules! impl_insert {
                 fn insert(self, core: &mut Instance) -> Self::Inserted {
                     let handle = core.layers.objects.$store.reserve();
                     let object = (handle.clone(), self).into();
-                    core.layers.insert_object(object);
+                    core.layers.objects.insert(
+                        object,
+                        &mut core.layers.validation,
+                    );
                     handle
                 }
             }

crates/fj-core/src/validate/mod.rs

@@ -76,13 +76,17 @@ mod solid;
 mod surface;
 mod vertex;
 
+use crate::storage::ObjectId;
+
 pub use self::{
     cycle::CycleValidationError, edge::EdgeValidationError,
     face::FaceValidationError, shell::ShellValidationError,
     sketch::SketchValidationError, solid::SolidValidationError,
 };
 
-use std::{convert::Infallible, fmt};
+use std::{
+    collections::HashMap, convert::Infallible, error::Error, fmt, thread,
+};
 
 use fj_math::Scalar;
 
@@ -99,6 +103,59 @@ macro_rules! assert_contains_err {
     };
 }
 
+/// Errors that occurred while validating the objects inserted into the stores
+#[derive(Default)]
+pub struct Validation {
+    /// All unhandled validation errors
+    pub errors: HashMap<ObjectId, ValidationError>,
+
+    /// Validation configuration for the validation service
+    pub config: ValidationConfig,
+}
+
+impl Validation {
+    /// Construct an instance of `Validation`, using the provided configuration
+    pub fn with_validation_config(config: ValidationConfig) -> Self {
+        let errors = HashMap::new();
+        Self { errors, config }
+    }
+
+    /// Drop this instance, returning the errors it contained
+    pub fn into_errors(mut self) -> ValidationErrors {
+        ValidationErrors(self.errors.drain().map(|(_, error)| error).collect())
+    }
+}
+
+impl Drop for Validation {
+    fn drop(&mut self) {
+        let num_errors = self.errors.len();
+        if num_errors > 0 {
+            println!(
+                "Dropping `Validation` with {num_errors} unhandled validation \
+                errors:"
+            );
+
+            for err in self.errors.values() {
+                println!("{}", err);
+
+                // Once `Report` is stable, we can replace this:
+                // https://doc.rust-lang.org/std/error/struct.Report.html
+                let mut source = err.source();
+                while let Some(err) = source {
+                    println!("\nCaused by:\n\t{err}");
+                    source = err.source();
+                }
+
+                print!("\n\n");
+            }
+
+            if !thread::panicking() {
+                panic!();
+            }
+        }
+    }
+}
+
 /// Validate an object
 ///
 /// This trait is used automatically when inserting an object into a store.