SERVER-28638 Provide richer error messages from InitializerDependency…

…Graph::topSort
mongodb · Apr 5, 2017 · 97f3e2c · 97f3e2c
1 parent c767b02
commit 97f3e2c
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 16 deletions.
diff --git a/src/mongo/base/initializer_dependency_graph.cpp b/src/mongo/base/initializer_dependency_graph.cpp
@@ -85,12 +85,19 @@ Status InitializerDependencyGraph::topSort(std::vector<std::string>* sortedNames
     unordered_set<std::string> visitedNodeNames;
 
     sortedNames->clear();
-    for (NodeMap::const_iterator iter = _nodes.begin(), end = _nodes.end(); iter != end; ++iter) {
+    for (const auto& node : _nodes) {
         Status status =
-            recursiveTopSort(_nodes, *iter, &inProgressNodeNames, &visitedNodeNames, sortedNames);
+            recursiveTopSort(_nodes, node, &inProgressNodeNames, &visitedNodeNames, sortedNames);
         if (Status::OK() != status)
             return status;
     }
+    for (const auto& node : _nodes) {
+        if (!node.second.fn) {
+            std::ostringstream os;
+            os << "No implementation provided for initializer " << node.first;
+            return {ErrorCodes::BadValue, os.str()};
+        }
+    }
     return Status::OK();
 }
 
@@ -99,6 +106,7 @@ Status InitializerDependencyGraph::recursiveTopSort(const NodeMap& nodeMap,
                                                     std::vector<std::string>* inProgressNodeNames,
                                                     unordered_set<std::string>* visitedNodeNames,
                                                     std::vector<std::string>* sortedNames) {
+
     /*
      * The top sort is performed by depth-first traversal starting at each node in the
      * dependency graph, short-circuited any time a node is seen that has already been visited
@@ -113,14 +121,11 @@ Status InitializerDependencyGraph::recursiveTopSort(const NodeMap& nodeMap,
     if ((*visitedNodeNames).count(currentNode.first))
         return Status::OK();
 
-    if (!currentNode.second.fn)
-        return Status(ErrorCodes::BadValue, currentNode.first);
-
     inProgressNodeNames->push_back(currentNode.first);
 
-    std::vector<std::string>::iterator firstOccurence =
+    auto firstOccurence =
         std::find(inProgressNodeNames->begin(), inProgressNodeNames->end(), currentNode.first);
-    if (firstOccurence + 1 != inProgressNodeNames->end()) {
+    if (std::next(firstOccurence) != inProgressNodeNames->end()) {
         sortedNames->clear();
         std::copy(firstOccurence, inProgressNodeNames->end(), std::back_inserter(*sortedNames));
         std::ostringstream os;
@@ -130,13 +135,14 @@ Status InitializerDependencyGraph::recursiveTopSort(const NodeMap& nodeMap,
         return Status(ErrorCodes::GraphContainsCycle, os.str());
     }
 
-    for (unordered_set<std::string>::const_iterator iter = currentNode.second.prerequisites.begin(),
-                                                    end = currentNode.second.prerequisites.end();
-         iter != end;
-         ++iter) {
-        NodeMap::const_iterator nextNode = nodeMap.find(*iter);
-        if (nextNode == nodeMap.end())
-            return Status(ErrorCodes::BadValue, *iter);
+    for (const auto& prereq : currentNode.second.prerequisites) {
+        auto nextNode = nodeMap.find(prereq);
+        if (nextNode == nodeMap.end()) {
+            std::ostringstream os;
+            os << "Initializer " << currentNode.first << " depends on missing initializer "
+               << prereq;
+            return {ErrorCodes::BadValue, os.str()};
+        }
 
         Status status = recursiveTopSort(
             nodeMap, *nextNode, inProgressNodeNames, visitedNodeNames, sortedNames);

diff --git a/src/mongo/base/initializer_dependency_graph_test.cpp b/src/mongo/base/initializer_dependency_graph_test.cpp
@@ -272,7 +272,9 @@ TEST(InitializerDependencyGraphTest, TopSortFailsWhenMissingPrerequisite) {
     InitializerDependencyGraph graph;
     std::vector<std::string> nodeNames;
     ASSERT_ADD_INITIALIZER(graph, "B", doNothing, ("A"), MONGO_NO_DEPENDENTS);
-    ASSERT_EQUALS(ErrorCodes::BadValue, graph.topSort(&nodeNames));
+    auto status = graph.topSort(&nodeNames);
+    ASSERT_EQUALS(ErrorCodes::BadValue, status);
+    ASSERT_STRING_CONTAINS(status.reason(), "depends on missing initializer A");
 }
 
 TEST(InitializerDependencyGraphTest, TopSortFailsWhenMissingDependent) {
@@ -282,7 +284,9 @@ TEST(InitializerDependencyGraphTest, TopSortFailsWhenMissingDependent) {
     InitializerDependencyGraph graph;
     std::vector<std::string> nodeNames;
     ASSERT_ADD_INITIALIZER(graph, "A", doNothing, MONGO_NO_PREREQUISITES, ("B"));
-    ASSERT_EQUALS(ErrorCodes::BadValue, graph.topSort(&nodeNames));
+    auto status = graph.topSort(&nodeNames);
+    ASSERT_EQUALS(ErrorCodes::BadValue, status);
+    ASSERT_STRING_CONTAINS(status.reason(), "No implementation provided for initializer B");
 }
 
 }  // namespace