From be70a5330a2c852ae8b1b88bd81ab7ed8ea3b47a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:11:49 +0200 Subject: [PATCH 1/2] Fix validation errors overwriting each other Validation errors were stored by object ID, which made no sense. It wasn't necessary, but provided the possibility of a validation error overwriting a previous one that applied to the same object. On top of that, this change is a simplification, so a win all around. --- crates/fj-core/src/layers/validation.rs | 4 ++-- crates/fj-core/src/validation/validation.rs | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/layers/validation.rs b/crates/fj-core/src/layers/validation.rs index 1583a1ca2..76681e55b 100644 --- a/crates/fj-core/src/layers/validation.rs +++ b/crates/fj-core/src/layers/validation.rs @@ -60,7 +60,7 @@ impl Command for TakeErrors { state: &Validation, events: &mut Vec, ) -> Self::Result { - let errors = ValidationErrors(state.errors.values().cloned().collect()); + let errors = ValidationErrors(state.errors.to_vec()); events.push(self); @@ -92,6 +92,6 @@ pub struct ValidationFailed { impl Event for ValidationFailed { fn evolve(&self, state: &mut Validation) { - state.errors.insert(self.object.id(), self.err.clone()); + state.errors.push(self.err.clone()); } } diff --git a/crates/fj-core/src/validation/validation.rs b/crates/fj-core/src/validation/validation.rs index 430ae6527..c94229f65 100644 --- a/crates/fj-core/src/validation/validation.rs +++ b/crates/fj-core/src/validation/validation.rs @@ -1,6 +1,4 @@ -use std::{collections::HashMap, error::Error, thread}; - -use crate::storage::ObjectId; +use std::{error::Error, thread}; use super::{ValidationConfig, ValidationError}; @@ -8,7 +6,7 @@ use super::{ValidationConfig, ValidationError}; #[derive(Default)] pub struct Validation { /// All unhandled validation errors - pub errors: HashMap, + pub errors: Vec, /// Validation configuration for the validation service pub config: ValidationConfig, @@ -17,7 +15,7 @@ pub struct Validation { impl Validation { /// Construct an instance of `Validation`, using the provided configuration pub fn with_validation_config(config: ValidationConfig) -> Self { - let errors = HashMap::new(); + let errors = Vec::new(); Self { errors, config } } } @@ -31,7 +29,7 @@ impl Drop for Validation { errors:" ); - for err in self.errors.values() { + for err in self.errors.iter() { println!("{}", err); // Once `Report` is stable, we can replace this: From e977da834fb3e001493d3adec69c083d90ba21e9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 12 Aug 2024 21:16:07 +0200 Subject: [PATCH 2/2] Remove unused struct field --- crates/fj-core/src/layers/validation.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/fj-core/src/layers/validation.rs b/crates/fj-core/src/layers/validation.rs index 76681e55b..29ee541a1 100644 --- a/crates/fj-core/src/layers/validation.rs +++ b/crates/fj-core/src/layers/validation.rs @@ -38,10 +38,7 @@ impl Command for ValidateObject<'_> { panic!("{:#?}", err); } - events.push(ValidationFailed { - object: self.object.clone(), - err, - }); + events.push(ValidationFailed { err }); } } } @@ -83,9 +80,6 @@ impl Event for TakeErrors { /// Event produced by `Layer`. #[derive(Clone)] pub struct ValidationFailed { - /// The object for which validation failed - pub object: AnyObject, - /// The validation error pub err: ValidationError, }