Skip to content

Commit

Permalink
Responding to PR feedback
Browse files Browse the repository at this point in the history
Passed in ValidationConfig directly instead of cloning a reference.
Removed optional ValidationConfig for ValidationService, use default or
config passed in through alternate constructor.
Deleted DerefMut implementation.
  • Loading branch information
brungardtdb committed Jan 13, 2024
1 parent f7d6331 commit f57c77e
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 33 deletions.
7 changes: 3 additions & 4 deletions crates/fj-core/src/services/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ impl Services {
}

/// Construct an instance of `Services` with a pre-defined configuration for the validation service
pub fn with_validation_config(config: &ValidationConfig) -> Self {
pub fn with_validation_config(config: ValidationConfig) -> Self {
let objects = Service::<Objects>::default();
let mut validation = Service::<Validation>::default();
validation
.evolve(&ValidationEvent::ConfigurationDefined { config: *config });
let validation =
Service::new(Validation::with_validation_config(config));
Self {
objects,
validation,
Expand Down
8 changes: 1 addition & 7 deletions crates/fj-core/src/services/service.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::ops::{Deref, DerefMut};
use std::ops::Deref;

/// A service that controls access to some state
///
Expand Down Expand Up @@ -62,12 +62,6 @@ impl<S: State> Deref for Service<S> {
}
}

impl<S: State> DerefMut for Service<S> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.state
}
}

impl<S: State> Default for Service<S>
where
S: Default,
Expand Down
31 changes: 9 additions & 22 deletions crates/fj-core/src/services/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,23 @@ use super::State;
pub struct Validation {
/// All unhandled validation errors
pub errors: BTreeMap<ObjectId, ValidationError>,
/// Optional validation config
config: Option<ValidationConfig>,
/// Validation configuration for the validation service
config: ValidationConfig,
}

impl Validation {
fn with_validation_config(&mut self, config: &ValidationConfig) -> &Self {
self.config = Some(*config);
self
/// A constructor for the validation service that allows a validation configuration to be set for the service
pub fn with_validation_config(config: ValidationConfig) -> Self {
let errors = BTreeMap::new();
Self { errors, config }
}
}

impl Default for Validation {
fn default() -> Self {
let errors = BTreeMap::new();
Self {
errors,
config: None,
}
let config: ValidationConfig = ValidationConfig::default();
Self { errors, config }
}
}

Expand Down Expand Up @@ -72,11 +71,7 @@ impl State for Validation {

match command {
ValidationCommand::ValidateObject { object } => {
match self.config {
Some(c) => object.validate_with_config(&c, &mut errors),
None => object.validate(&mut errors),
}
object.validate(&mut errors);
object.validate_with_config(&self.config, &mut errors);

for err in errors {
events.push(ValidationEvent::ValidationFailed {
Expand All @@ -93,9 +88,6 @@ impl State for Validation {
ValidationEvent::ValidationFailed { object, err } => {
self.errors.insert(object.id(), err.clone());
}
ValidationEvent::ConfigurationDefined { config } => {
self.with_validation_config(config);
}
}
}
}
Expand All @@ -120,9 +112,4 @@ pub enum ValidationEvent {
/// The validation error
err: ValidationError,
},
/// A validation configuration has been defined
ConfigurationDefined {
/// The predefined validation configuration
config: ValidationConfig,
},
}

0 comments on commit f57c77e

Please sign in to comment.