diff --git a/crates/configuration/src/shared/errors.rs b/crates/configuration/src/shared/errors.rs index 89c2e220a..ec6fb8b59 100644 --- a/crates/configuration/src/shared/errors.rs +++ b/crates/configuration/src/shared/errors.rs @@ -107,4 +107,7 @@ pub enum ValidationError { #[error("'{0}' is already used across config")] NodeNameAlreadyUsed(String), + + #[error("can't be empty")] + CantBeEmpty(), } diff --git a/crates/configuration/src/shared/helpers.rs b/crates/configuration/src/shared/helpers.rs index c111e66e0..11cf50957 100644 --- a/crates/configuration/src/shared/helpers.rs +++ b/crates/configuration/src/shared/helpers.rs @@ -28,13 +28,14 @@ pub fn merge_errors_vecs( } pub fn ensure_node_name_unique( - node_name: String, + node_name: impl Into, validation_context: Rc>, ) -> Result<(), anyhow::Error> { let mut context = validation_context .try_borrow_mut() .expect(&format!("{}, {}", BORROWABLE, THIS_IS_A_BUG)); + let node_name = node_name.into(); if !context.used_nodes_names.contains(&node_name) { context.used_nodes_names.push(node_name); return Ok(()); @@ -43,6 +44,14 @@ pub fn ensure_node_name_unique( Err(ValidationError::NodeNameAlreadyUsed(node_name).into()) } +pub fn ensure_value_is_not_empty(value: &str) -> Result<(), anyhow::Error> { + if value.is_empty() { + Err(ValidationError::CantBeEmpty().into()) + } else { + Ok(()) + } +} + pub fn ensure_port_unique( port: Port, validation_context: Rc>, diff --git a/crates/configuration/src/shared/node.rs b/crates/configuration/src/shared/node.rs index 810deafd0..e8f6157a2 100644 --- a/crates/configuration/src/shared/node.rs +++ b/crates/configuration/src/shared/node.rs @@ -5,7 +5,10 @@ use serde::{ser::SerializeStruct, Deserialize, Serialize}; use super::{ errors::FieldError, - helpers::{ensure_node_name_unique, ensure_port_unique, merge_errors, merge_errors_vecs}, + helpers::{ + ensure_node_name_unique, ensure_port_unique, ensure_value_is_not_empty, merge_errors, + merge_errors_vecs, + }, macros::states, resources::ResourcesBuilder, types::{AssetLocation, ChainDefaultContext, Command, Image, ValidationContext, U128}, @@ -338,23 +341,37 @@ impl NodeConfigBuilder { /// Set the name of the node. pub fn with_name + Copy>(self, name: T) -> NodeConfigBuilder { - match ensure_node_name_unique(name.into(), self.validation_context.clone()) { - Ok(_) => Self::transition( + let name: String = name.into(); + + match ( + ensure_value_is_not_empty(&name), + ensure_node_name_unique(&name, self.validation_context.clone()), + ) { + (Ok(_), Ok(_)) => Self::transition( NodeConfig { - name: name.into(), + name, ..self.config }, self.validation_context, self.errors, ), - Err(error) => Self::transition( + (Err(e), _) => Self::transition( NodeConfig { // we still set the name in error case to display error path - name: name.into(), + name, ..self.config }, self.validation_context, - merge_errors(self.errors, FieldError::Name(error).into()), + merge_errors(self.errors, FieldError::Name(e).into()), + ), + (_, Err(e)) => Self::transition( + NodeConfig { + // we still set the name in error case to display error path + name, + ..self.config + }, + self.validation_context, + merge_errors(self.errors, FieldError::Name(e).into()), ), } } @@ -1003,4 +1020,20 @@ mod tests { "p2p_port: '45093' is already used across config" ); } + + #[test] + fn node_config_builder_should_fails_if_node_name_is_empty() { + let validation_context = Rc::new(RefCell::new(ValidationContext { + ..Default::default() + })); + + let (_, errors) = + NodeConfigBuilder::new(ChainDefaultContext::default(), validation_context) + .with_name("") + .build() + .unwrap_err(); + + assert_eq!(errors.len(), 1); + assert_eq!(errors.first().unwrap().to_string(), "name: can't be empty"); + } }