Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start cleanup of validation infrastructure #2229

Merged
merged 7 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fj-core/src/algorithms/approx/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use fj_interop::Color;
use crate::{
objects::{Face, Handedness, ObjectSet},
operations::presentation::GetColor,
validate::ValidationConfig,
validation::ValidationConfig,
Core,
};

Expand Down
2 changes: 1 addition & 1 deletion crates/fj-core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! See [`Core`].

use crate::{layers::Layers, validate::ValidationConfig};
use crate::{layers::Layers, validation::ValidationConfig};

/// An instance of the Fornjot core
///
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-core/src/layers/layers.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
objects::Objects,
presentation::Presentation,
validate::{Validation, ValidationConfig},
validation::{Validation, ValidationConfig},
};

use super::Layer;
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-core/src/layers/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::{
objects::{AboutToBeStored, AnyObject, Objects},
validate::Validation,
validation::Validation,
};

use super::{Command, Event, Layer};
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-core/src/layers/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::{
objects::{AnyObject, Stored},
validate::{Validation, ValidationError, ValidationErrors},
validation::{Validation, ValidationError, ValidationErrors},
};

use super::{objects::InsertObject, Command, Event, Layer};
Expand Down
1 change: 1 addition & 0 deletions crates/fj-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub mod presentation;
pub mod queries;
pub mod storage;
pub mod validate;
pub mod validation;

mod core;

Expand Down
3 changes: 2 additions & 1 deletion crates/fj-core/src/objects/any_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::{
Surface, Vertex,
},
storage::{Handle, HandleWrapper, ObjectId},
validate::{Validate, ValidationConfig, ValidationError},
validate::Validate,
validation::{ValidationConfig, ValidationError},
};

macro_rules! any_object {
Expand Down
7 changes: 5 additions & 2 deletions crates/fj-core/src/validate/curve.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::objects::Curve;
use crate::{
objects::Curve,
validation::{ValidationConfig, ValidationError},
};

use super::{Validate, ValidationConfig, ValidationError};
use super::Validate;

impl Validate for Curve {
fn validate_with_config(
Expand Down
6 changes: 4 additions & 2 deletions crates/fj-core/src/validate/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ use fj_math::{Point, Scalar};
use crate::{
objects::{Cycle, HalfEdge},
storage::Handle,
validation::{ValidationConfig, ValidationError},
};

use super::{Validate, ValidationConfig, ValidationError};
use super::Validate;

impl Validate for Cycle {
fn validate_with_config(
Expand Down Expand Up @@ -83,7 +84,8 @@ mod tests {
build::{BuildCycle, BuildHalfEdge},
update::UpdateCycle,
},
validate::{cycle::CycleValidationError, Validate, ValidationError},
validate::{cycle::CycleValidationError, Validate},
validation::ValidationError,
Core,
};

Expand Down
10 changes: 7 additions & 3 deletions crates/fj-core/src/validate/edge.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use fj_math::{Point, Scalar};

use crate::objects::HalfEdge;
use crate::{
objects::HalfEdge,
validation::{ValidationConfig, ValidationError},
};

use super::{Validate, ValidationConfig, ValidationError};
use super::Validate;

impl Validate for HalfEdge {
fn validate_with_config(
Expand Down Expand Up @@ -70,7 +73,8 @@ mod tests {
assert_contains_err,
objects::HalfEdge,
operations::build::BuildHalfEdge,
validate::{EdgeValidationError, Validate, ValidationError},
validate::{EdgeValidationError, Validate},
validation::ValidationError,
Core,
};

Expand Down
10 changes: 7 additions & 3 deletions crates/fj-core/src/validate/face.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use fj_math::Winding;

use crate::objects::Face;
use crate::{
objects::Face,
validation::{ValidationConfig, ValidationError},
};

use super::{Validate, ValidationConfig, ValidationError};
use super::Validate;

impl Validate for Face {
fn validate_with_config(
Expand Down Expand Up @@ -94,7 +97,8 @@ mod tests {
reverse::Reverse,
update::{UpdateCycle, UpdateFace, UpdateRegion},
},
validate::{FaceValidationError, Validate, ValidationError},
validate::{FaceValidationError, Validate},
validation::ValidationError,
Core,
};

Expand Down
155 changes: 7 additions & 148 deletions crates/fj-core/src/validate/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
//! # Infrastructure for validating objects
//!
//! ## Structure and Nomenclature
//!
//! **Validation** is the process of checking that objects meet specific
//! requirements. Each kind of object has its own set of requirements.
//!
//! An object that meets all the requirement for its kind is considered
//! **valid**. An object that does not meet all of them is considered
//! **invalid**. This results in a **validation error**, which is represented by
//! [`ValidationError`].
//! ## Structure
//!
//! Every single requirement is checked by a dedicated function. These functions
//! are called **validation checks**. Validation checks are currently not
Expand Down Expand Up @@ -60,6 +52,11 @@
//! [`Validate::validate_with_config`] and [`ValidationConfig`].
//!
//!
//! ## Implementation Note
//!
//! This module is in the process of being replaced. See [`crate::validation`].
//!
//!
//! [`fj-export`]: https://crates.io/crates/fj-export
//! [issue tracker]: https://github.com/hannobraun/fornjot/issues
//! [`Layers`]: crate::layers::Layers
Expand All @@ -76,20 +73,14 @@ mod solid;
mod surface;
mod vertex;

use crate::storage::ObjectId;
use crate::validation::{ValidationConfig, ValidationError};

pub use self::{
cycle::CycleValidationError, edge::EdgeValidationError,
face::FaceValidationError, shell::ShellValidationError,
sketch::SketchValidationError, solid::SolidValidationError,
};

use std::{
collections::HashMap, convert::Infallible, error::Error, fmt, thread,
};

use fj_math::Scalar;

/// Assert that some object has a validation error which matches a specific
/// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order.
#[macro_export]
Expand All @@ -103,54 +94,6 @@ 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 }
}
}

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.
Expand Down Expand Up @@ -180,87 +123,3 @@ pub trait Validate: Sized {
errors: &mut Vec<ValidationError>,
);
}

/// Configuration required for the validation process
#[derive(Debug, Clone, Copy)]
pub struct ValidationConfig {
/// The minimum distance between distinct objects
///
/// Objects whose distance is less than the value defined in this field, are
/// considered identical.
pub distinct_min_distance: Scalar,

/// The maximum distance between identical objects
///
/// Objects that are considered identical might still have a distance
/// between them, due to inaccuracies of the numerical representation. If
/// that distance is less than the one defined in this field, can not be
/// considered identical.
pub identical_max_distance: Scalar,
}

impl Default for ValidationConfig {
fn default() -> Self {
Self {
distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm,

// This value was chosen pretty arbitrarily. Seems small enough to
// catch errors. If it turns out it's too small (because it produces
// false positives due to floating-point accuracy issues), we can
// adjust it.
identical_max_distance: Scalar::from_f64(5e-14),
}
}
}

/// An error that can occur during a validation
#[derive(Clone, Debug, thiserror::Error)]
pub enum ValidationError {
/// `Cycle` validation error
#[error("`Cycle` validation error")]
Cycle(#[from] CycleValidationError),

/// `Edge` validation error
#[error("`Edge` validation error")]
Edge(#[from] EdgeValidationError),

/// `Face` validation error
#[error("`Face` validation error")]
Face(#[from] FaceValidationError),

/// `Shell` validation error
#[error("`Shell` validation error")]
Shell(#[from] ShellValidationError),

/// `Solid` validation error
#[error("`Solid` validation error")]
Solid(#[from] SolidValidationError),

/// `Sketch` validation error
#[error("`Sketch` validation error")]
Sketch(#[from] SketchValidationError),
}

impl From<Infallible> for ValidationError {
fn from(infallible: Infallible) -> Self {
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(())
}
}
33 changes: 33 additions & 0 deletions crates/fj-core/src/validation/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use fj_math::Scalar;

/// Configuration required for the validation process
#[derive(Debug, Clone, Copy)]
pub struct ValidationConfig {
/// The minimum distance between distinct objects
///
/// Objects whose distance is less than the value defined in this field, are
/// considered identical.
pub distinct_min_distance: Scalar,

/// The maximum distance between identical objects
///
/// Objects that are considered identical might still have a distance
/// between them, due to inaccuracies of the numerical representation. If
/// that distance is less than the one defined in this field, can not be
/// considered identical.
pub identical_max_distance: Scalar,
}

impl Default for ValidationConfig {
fn default() -> Self {
Self {
distinct_min_distance: Scalar::from_f64(5e-7), // 0.5 µm,

// This value was chosen pretty arbitrarily. Seems small enough to
// catch errors. If it turns out it's too small (because it produces
// false positives due to floating-point accuracy issues), we can
// adjust it.
identical_max_distance: Scalar::from_f64(5e-14),
}
}
}
Loading