Refactoring name validity

libs/attribute/src/ecflow/attribute/GenericAttr.cpp

@@ -26,15 +26,15 @@ GenericAttr::GenericAttr(const std::string& name, const std::vector<std::string>
     : name_(name),
       values_(values) {
     std::string msg;
-    if (!Str::valid_name(name, msg)) {
+    if (!ecf::algorithm::is_valid_name(name, msg)) {
         throw std::runtime_error("GenericAttr::GenericAttr : Invalid generic name : " + msg);
     }
 }
 
 GenericAttr::GenericAttr(const std::string& name)
     : name_(name) {
     std::string msg;
-    if (!Str::valid_name(name, msg)) {
+    if (!ecf::algorithm::is_valid_name(name, msg)) {
         throw std::runtime_error("GenericAttr::GenericAttr : Invalid generic name : " + msg);
     }
 }

libs/attribute/src/ecflow/attribute/NodeAttr.cpp

@@ -51,7 +51,7 @@ Event::Event(int number, const std::string& eventName, bool iv, bool check_name)
       iv_(iv) {
     if (!eventName.empty() && check_name) {
         std::string msg;
-        if (!Str::valid_name(eventName, msg)) {
+        if (!ecf::algorithm::is_valid_name(eventName, msg)) {
             throw std::runtime_error("Event::Event: Invalid event name : " + msg);
         }
     }
@@ -87,7 +87,7 @@ Event::Event(const std::string& eventName, bool iv)
     }
 
     std::string msg;
-    if (!Str::valid_name(eventName, msg)) {
+    if (!ecf::algorithm::is_valid_name(eventName, msg)) {
         throw std::runtime_error("Event::Event: Invalid event name : " + msg);
     }
 }
@@ -214,7 +214,7 @@ Meter::Meter(const std::string& name, int min, int max, int colorChange, int val
       cc_(colorChange),
       n_(name) {
     if (check) {
-        if (!Str::valid_name(name)) {
+        if (!ecf::algorithm::is_valid_name(name)) {
             throw std::runtime_error("Meter::Meter: Invalid Meter name: " + name);
         }
     }
@@ -323,7 +323,7 @@ Label::Label(const std::string& name, const std::string& value, const std::strin
     : n_(name),
       v_(value),
       new_v_(new_value) {
-    if (check_name && !Str::valid_name(n_)) {
+    if (check_name && !ecf::algorithm::is_valid_name(n_)) {
         throw std::runtime_error(MESSAGE("Label::Label: Invalid Label name :" << n_));
     }
 }

libs/attribute/src/ecflow/attribute/QueueAttr.cpp

@@ -35,7 +35,7 @@ QueueAttr::QueueAttr(const std::string& name, const std::vector<std::string>& th
     : theQueue_(theQueue),
       name_(name) {
     std::string msg;
-    if (!Str::valid_name(name, msg)) {
+    if (!ecf::algorithm::is_valid_name(name, msg)) {
         throw std::runtime_error("QueueAttr::QueueAttr: Invalid queue name : " + msg);
     }
     if (theQueue.empty()) {
@@ -273,7 +273,7 @@ void QueueAttr::set_state_vec(const std::vector<NState::State>& state_vec) {
 
 void QueueAttr::set_name(const std::string& name) {
     std::string msg;
-    if (!Str::valid_name(name, msg)) {
+    if (!ecf::algorithm::is_valid_name(name, msg)) {
         throw std::runtime_error("QueueAttr::set_name: Invalid queue name : " + msg);
     }
     name_ = name;

libs/attribute/src/ecflow/attribute/RepeatAttr.cpp

@@ -156,7 +156,7 @@ RepeatDate::RepeatDate(const std::string& variable, int start, int end, int delt
       end_(end),
       delta_(delta),
       value_(start) {
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatDate::RepeatDate: Invalid name: " + variable);
     }
 
@@ -500,7 +500,7 @@ RepeatDateTime::RepeatDateTime(const std::string& variable, Instant start, Insta
       end_(end),
       delta_(delta),
       value_(start) {
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatDateTime::RepeatDateTime: Invalid name: " + variable);
     }
 
@@ -806,7 +806,7 @@ void RepeatDateTime::set_value(long the_new_date) {
 RepeatDateList::RepeatDateList(const std::string& variable, const std::vector<int>& l)
     : RepeatBase(variable),
       list_(l) {
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatDateList: Invalid name: " + variable);
     }
     if (list_.empty()) {
@@ -1131,7 +1131,7 @@ RepeatInteger::RepeatInteger(const std::string& variable, int start, int end, in
       delta_(delta),
       value_(start) {
     // cout << toString() << "\n";
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatInteger: Invalid name: " + variable);
     }
 }
@@ -1325,7 +1325,7 @@ std::string RepeatInteger::prev_value_as_string() const {
 RepeatEnumerated::RepeatEnumerated(const std::string& variable, const std::vector<std::string>& theEnums)
     : RepeatBase(variable),
       theEnums_(theEnums) {
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatEnumerated: Invalid name: " + variable);
     }
     if (theEnums.empty()) {
@@ -1543,7 +1543,7 @@ bool RepeatEnumerated::operator==(const RepeatEnumerated& rhs) const {
 RepeatString::RepeatString(const std::string& variable, const std::vector<std::string>& theEnums)
     : RepeatBase(variable),
       theStrings_(theEnums) {
-    if (!Str::valid_name(variable)) {
+    if (!ecf::algorithm::is_valid_name(variable)) {
         throw std::runtime_error("RepeatString:: Invalid name: " + variable);
     }
     if (theEnums.empty()) {

libs/attribute/src/ecflow/attribute/Variable.cpp

@@ -31,14 +31,14 @@ Variable::Variable(const std::string& name, const std::string& value)
     : n_(name),
       v_(value) {
     std::string msg;
-    if (!Str::valid_name(name, msg)) {
+    if (!ecf::algorithm::is_valid_name(name, msg)) {
         throw std::runtime_error("Variable::Variable: Invalid Variable name: " + msg);
     }
 }
 
 void Variable::set_name(const std::string& v) {
     std::string msg;
-    if (!Str::valid_name(v, msg)) {
+    if (!ecf::algorithm::is_valid_name(v, msg)) {
         throw std::runtime_error("Variable::set_name: Invalid Variable name: " + msg);
     }
     n_ = v;

libs/base/src/ecflow/base/cts/task/QueueCmd.cpp

@@ -277,7 +277,7 @@ void QueueCmd::create(Cmd_ptr& cmd, boost::program_options::variables_map& vm, A
     }
 
     std::string msg;
-    if (!Str::valid_name(queue_name, msg)) {
+    if (!ecf::algorithm::is_valid_name(queue_name, msg)) {
         throw std::runtime_error("QueueCmd: Invalid queue name : " + msg);
     }
 

libs/core/CMakeLists.txt

@@ -47,6 +47,7 @@ set(test_srcs
   test/TestLog.cpp
   test/TestMessage.cpp
   test/TestMigration.cpp
+  test/TestNodeNameValidity.cpp
   test/TestNodePath.cpp
   test/TestPasswdFile.cpp
   test/TestPasswordEncryption.cpp

libs/core/src/ecflow/core/Str.cpp

@@ -14,7 +14,46 @@
 
 namespace ecf {
 
-const char* VALID_NODE_CHARS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.";
+namespace algorithm {
+
+inline bool is_valid_node_name_first_character(char c) {
+    return std::isalnum(c) || c == '_';
+}
+
+inline bool is_valid_node_name_following_character(char c) {
+    return std::isalnum(c) || c == '_' || c == '.';
+}
+
+bool is_valid_name(const std::string& name, std::string& error) {
+
+    if (name.empty()) {
+        error = "Invalid name '': empty string";
+        return false;
+    }
+
+    if (!is_valid_node_name_first_character(name.front())) {
+        error = "Invalid name '";
+        error += name;
+        error += "': only alphanumeric characters or underscore are accepted as first character";
+        return false;
+    }
+
+    if (!std::all_of(name.begin() + 1, name.end(), is_valid_node_name_following_character)) {
+        error = "Invalid name '";
+        error += name;
+        error += "': only alphanumeric characters, underscore or dot are accepted";
+        return false;
+    }
+
+    return true;
+}
+
+bool is_valid_name(const std::string& name) {
+    return !name.empty() && is_valid_node_name_first_character(name.front()) &&
+           std::all_of(name.begin() + 1, name.end(), is_valid_node_name_following_character);
+}
+
+} // namespace algorithm
 
 void Str::removeQuotes(std::string& s) {
     if (!s.empty()) {
@@ -323,67 +362,6 @@ bool Str::caseInsGreater(const std::string& a, const std::string& b) {
     });
 }
 
-bool Str::valid_name(const std::string& name, std::string& msg) {
-    // valid names are alphabetic (alphanumeric | underscore | .)
-    // however we can't have a leading '.' as that can interfere with trigger expressions
-
-    // verify that the string is not empty
-    if (name.empty()) {
-        msg = "Invalid name. Empty string.";
-        return false;
-    }
-
-    // verify that the first character is alphanumeric or is an underscore
-    bool result = ecf::string_constants::alphanumeric_underscore_chars.find(name[0], 0) != std::string::npos;
-    if (!result) {
-        msg = "Valid names can only consist of alphanumeric characters, "
-              "underscores and dots (The first character cannot be a dot). "
-              "The first character is not valid (only alphanumeric or an underscore is allowed): ";
-        msg += name;
-        return false;
-    }
-
-    // verify that any other characters are alphanumeric or underscore
-    if (name.size() > 1) {
-        result = name.find_first_not_of(VALID_NODE_CHARS, 1) == std::string::npos;
-        if (!result) {
-            msg = "Valid names can only consist of alphanumeric characters, "
-                  "underscores and dots (The first character cannot be a dot). ";
-            if (name.find('\r') != std::string::npos) {
-                msg += "Windows line ending ? ";
-            }
-            msg += "'";
-            msg += name;
-            msg += "'"; // use '<name>' to show if PC format, i.e. carriage return
-        }
-    }
-
-    return result;
-}
-
-bool Str::valid_name(const std::string& name) {
-    // valid names are alphabetic (alphanumeric | underscore | .)
-    // however we can't have a leading '.' as that can interfere with trigger expressions
-
-    // verify that the string is not empty
-    if (name.empty()) {
-        return false;
-    }
-
-    // verify that the first character is alphabetic or has underscore
-    bool result = ecf::string_constants::alphanumeric_underscore_chars.find(name[0], 0) != std::string::npos;
-    if (!result) {
-        return false;
-    }
-
-    // verify that any other characters are alphanumeric or underscore
-    if (name.size() > 1) {
-        result = name.find_first_not_of(VALID_NODE_CHARS, 1) == std::string::npos;
-    }
-
-    return result;
-}
-
 int Str::to_int(const std::string& the_str, int error_return) {
     if (the_str.find_first_of(ecf::string_constants::numeric_chars, 0) != std::string::npos) {
         try {

libs/core/src/ecflow/core/Str.hpp

@@ -101,6 +101,30 @@ inline std::string tolower(std::string s) {
     return s;
 }
 
+///
+/// @brief Check if the \param name is valid according to the rules for node/attributes names in ecFlow.
+///
+/// This is used to verify the validity of nodes and attributes (Variable, Label, Event, ...) names in ecFlow.
+///
+/// If the name is valid, the error buffer will not be updated (e.g. cleared), otherwise the error buffer will contain a
+/// description of the validation failure.
+///
+/// @param name The name to be validated.
+/// @param error A buffer to hold the error description, if validation fails (i.e. when function returns false).
+/// @return true if the name is valid, false otherwise.
+///
+bool is_valid_name(const std::string& name, std::string& error);
+
+///
+/// @brief Check if the \param name is valid according to the rules for node names in ecFlow.
+///
+/// This is used to verify the validity of nodes and attributes (Variable, Label, Event, ...) names in ecFlow.
+///
+/// @param name The name to be validated.
+/// @return true if the name is valid, false otherwise.
+///
+bool is_valid_name(const std::string& name);
+
 } // namespace algorithm
 
 class Str {
@@ -188,10 +212,6 @@ class Str {
     /// case-insensitive Greater
     static bool caseInsGreater(const std::string&, const std::string&);
 
-    /// Used for checking node names
-    static bool valid_name(const std::string& name, std::string& msg);
-    static bool valid_name(const std::string& name);
-
     /**
      * Convert a given string to an integer.
      *