TIG-1311 Make NoOp configuration parsing more flexible (mongodb#92)

* Added in an expanded case for YAML scalars * Made NoOp a possible key * Made null phases bad
roganamy · Jan 3, 2019 · 1ed3880 · 1ed3880
1 parent c9b7abd
commit 1ed3880
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 3 deletions.
diff --git a/src/gennylib/include/gennylib/context.hpp b/src/gennylib/include/gennylib/context.hpp
@@ -613,15 +613,20 @@ class PhaseContext final {
      * Called in PhaseLoop during the IterationCompletionCheck constructor.
      */
     bool isNop() const {
-        auto opNode = get<YAML::Node, false>("Operation");
-        bool isNop = opNode && (*opNode)["OperationName"].as<std::string>() == "Nop";
-        if (isNop && _node.size() != 1) {
+        auto isNop = _isNop();
+
+        // Check to make sure we haven't broken our rules
+        if (isNop && _node.size() > 1) {
             throw InvalidConfigurationException(
                 "Nop cannot be used with any other keywords. Check YML configuration.");
         }
+
         return isNop;
     }
 
+private:
+    bool _isNop() const;
+
 private:
     YAML::Node _node;
     const ActorContext* _actor;

diff --git a/src/gennylib/src/context.cpp b/src/gennylib/src/context.cpp
@@ -93,6 +93,14 @@ std::unordered_map<PhaseNumber, std::unique_ptr<PhaseContext>> ActorContext::con
 
     int index = 0;
     for (const auto& phase : *phases) {
+        // If we don't have a node or we are a null type, then we are a NoOp
+        if (!phase || phase.IsNull()){
+            std::ostringstream ss;
+            ss << "Encountered a null/empty phase. "
+                  "Every phase should have at least be an empty map.";
+            throw InvalidConfigurationException(ss.str());
+        }
+
         auto configuredIndex = phase["Phase"].as<PhaseNumber>(index);
         auto [it, success] =
             out.try_emplace(configuredIndex, std::make_unique<PhaseContext>(phase, *actorContext));
@@ -115,4 +123,40 @@ mongocxx::pool::entry ActorContext::client() {
     return std::move(*entry);
 }
 
+bool PhaseContext::_isNop() const {
+    auto hasNoOp = get<bool, false>("Nop").value_or(false)  //
+        || get<bool, false>("nop").value_or(false)          //
+        || get<bool, false>("NoOp").value_or(false)         //
+        || get<bool, false>("noop").value_or(false);
+
+    // If we had the simple Nop key, just exit out now
+    if (hasNoOp)
+        return true;
+
+    // If we don't have an operation or our operation isn't a map, then we're not a NoOp
+    auto maybeOperation = get<YAML::Node, false>("Operation");
+    if (!maybeOperation)
+        return false;
+
+    // If we have a simple string, use that
+    // If we have a full object, get "OperationName"
+    // Otherwise, we're null
+    auto yamlOpName = YAML::Node{};
+    if (maybeOperation->IsScalar())
+        yamlOpName = *maybeOperation;
+    else if (maybeOperation->IsMap())
+        yamlOpName = (*maybeOperation)["OperationName"];
+
+    // At this stage, we should have a string scalar
+    if (!yamlOpName.IsScalar())
+        return false;
+
+    // Fall back to an empty string in case we cannot convert to string
+    const auto& opName = yamlOpName.as<std::string>("");
+    return (opName == "Nop")   //
+        || (opName == "nop")   //
+        || (opName == "NoOp")  //
+        || (opName == "noop");
+}
+
 }  // namespace genny