Extract compareServerList and make it reuseable

.gitignore

@@ -41,7 +41,7 @@ Debug32/
 Release64/
 Release32/
 WindowsLibraries/
-cluster/
+cluster/**
 p/
 
 core

arangod/Agency/Job.cpp

@@ -30,28 +30,6 @@ static std::string const DBServer = "DBServer";
 
 using namespace arangodb::consensus;
 
-bool arangodb::consensus::compareServerLists(Slice plan, Slice current) {
-  if (!plan.isArray() || !current.isArray()) {
-    return false;
-  }
-  std::vector<std::string> planv, currv;
-  for (auto const& srv : VPackArrayIterator(plan)) {
-    if (srv.isString()) {
-      planv.push_back(srv.copyString());
-    }
-  }
-  for (auto const& srv : VPackArrayIterator(current)) {
-    if (srv.isString()) {
-      currv.push_back(srv.copyString());
-    }
-  }
-  bool equalLeader = !planv.empty() && !currv.empty() &&
-                     planv.front() == currv.front();
-  std::sort(planv.begin(), planv.end());
-  std::sort(currv.begin(), currv.end());
-  return equalLeader && currv == planv;
-}
-
 Job::Job(JOB_STATUS status, Node const& snapshot, AgentInterface* agent,
          std::string const& jobId, std::string const& creator)
   : _status(status),

arangod/Agency/Job.h

@@ -39,12 +39,6 @@
 namespace arangodb {
 namespace consensus {
 
-// This is intended for lists of servers with the first being the leader
-// and all others followers. Both arguments must be arrays. Returns true, 
-// if the first items in both slice are equal and if both arrays contain
-// the same set of strings.
-bool compareServerLists(Slice plan, Slice current);
-
 enum JOB_STATUS { TODO, PENDING, FINISHED, FAILED, NOTFOUND };
 const std::vector<std::string> pos({"/Target/ToDo/", "/Target/Pending/",
                                     "/Target/Finished/", "/Target/Failed/"});

arangod/Agency/MoveShard.cpp

@@ -25,7 +25,9 @@
 
 #include "Agency/AgentInterface.h"
 #include "Agency/Job.h"
+#include "Cluster/ClusterHelpers.h"
 
+using namespace arangodb;
 using namespace arangodb::consensus;
 
 MoveShard::MoveShard(Node const& snapshot, AgentInterface* agent,
@@ -585,7 +587,7 @@ JOB_STATUS MoveShard::pendingFollower() {
   size_t done = 0;   // count the number of shards done
   doForAllShards(_snapshot, _database, shardsLikeMe,
     [this, &done](Slice plan, Slice current, std::string& planPath) {
-      if (compareServerLists(plan, current)) {
+      if (ClusterHelpers::compareServerLists(plan, current)) {
         ++done;
       }
     });

arangod/CMakeLists.txt

@@ -194,6 +194,7 @@ SET(ARANGOD_SOURCES
   Cluster/ClusterComm.cpp
   Cluster/ClusterEdgeCursor.cpp
   Cluster/ClusterFeature.cpp
+  Cluster/ClusterHelpers.cpp
   Cluster/ClusterInfo.cpp
   Cluster/ClusterMethods.cpp
   Cluster/ClusterTraverser.cpp

arangod/Cluster/ClusterHelpers.cpp

@@ -0,0 +1,55 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2014-2017 ArangoDB GmbH, Cologne, Germany
+/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Andreas Streichardt
+////////////////////////////////////////////////////////////////////////////////
+
+#include "Cluster/ClusterHelpers.h"
+
+#include <velocypack/Iterator.h>
+#include <velocypack/velocypack-aliases.h>
+
+using namespace arangodb;
+
+bool ClusterHelpers::compareServerLists(Slice plan, Slice current) {
+  if (!plan.isArray() || !current.isArray()) {
+    return false;
+  }
+  std::vector<std::string> planv, currv;
+  for (auto const& srv : VPackArrayIterator(plan)) {
+    if (srv.isString()) {
+      planv.push_back(srv.copyString());
+    }
+  }
+  for (auto const& srv : VPackArrayIterator(current)) {
+    if (srv.isString()) {
+      currv.push_back(srv.copyString());
+    }
+  }
+  return compareServerLists(planv, currv);
+}
+
+bool ClusterHelpers::compareServerLists(std::vector<std::string> planned, std::vector<std::string> current) {
+  bool equalLeader =
+      !planned.empty() && !current.empty() && planned.front() == current.front();
+  std::sort(planned.begin(), planned.end());
+  std::sort(current.begin(), current.end());
+  return equalLeader && current == planned;
+}

arangod/Cluster/ClusterHelpers.h

@@ -0,0 +1,39 @@
+////////////////////////////////////////////////////////////////////////////////
+/// DISCLAIMER
+///
+/// Copyright 2014-2017 ArangoDB GmbH, Cologne, Germany
+/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Andreas Streichardt
+////////////////////////////////////////////////////////////////////////////////
+
+#ifndef ARANGOD_CLUSTER_CLUSTER_HELPERS_H
+#define ARANGOD_CLUSTER_CLUSTER_HELPERS_H 1
+
+#include <velocypack/Slice.h>
+
+using namespace arangodb::velocypack;
+
+namespace arangodb {
+class ClusterHelpers {
+ public:
+  static bool compareServerLists(Slice plan, Slice current);
+  static bool compareServerLists(std::vector<std::string>, std::vector<std::string>);
+};
+}
+
+#endif

tests/CMakeLists.txt

@@ -51,6 +51,7 @@ add_executable(
   Cache/TransactionalStore.cpp
   Cache/TransactionManager.cpp
   Cache/TransactionsWithBackingStore.cpp
+  Cluster/ClusterHelpers.cpp
   Geo/georeg.cpp
   Pregel/typedbuffer.cpp
   RocksDBEngine/IndexEstimatorTest.cpp

tests/Cluster/ClusterHelpers.cpp

@@ -0,0 +1,105 @@
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test suite for arangodb::cache::Manager
+///
+/// @file
+///
+/// DISCLAIMER
+///
+/// Copyright 2017 ArangoDB GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is ArangoDB GmbH, Cologne, Germany
+///
+/// @author Andreas Streichardt
+/// @author Copyright 2017, ArangoDB GmbH, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+#include "catch.hpp"
+
+#include "Cluster/ClusterHelpers.h"
+
+#include <velocypack/Builder.h>
+#include <velocypack/velocypack-aliases.h>
+
+using namespace arangodb;
+
+TEST_CASE("comparing server lists", "[cluster][helpers]") {
+
+SECTION("comparing non array slices will return false") {
+  VPackBuilder a;
+  VPackBuilder b;
+
+  REQUIRE(ClusterHelpers::compareServerLists(a.slice(), b.slice()) == false);
+}
+
+SECTION("comparing same server vpack lists returns true") {
+  VPackBuilder a;
+  VPackBuilder b;
+
+  {
+    VPackArrayBuilder aa(&a);
+    a.add(VPackValue("test"));
+  }
+  {
+    VPackArrayBuilder ba(&b);
+    b.add(VPackValue("test"));
+  }
+  INFO(a.toJson());
+  INFO(b.toJson());
+  REQUIRE(ClusterHelpers::compareServerLists(a.slice(), b.slice()) == true);
+}
+
+SECTION("comparing same server lists returns true") {
+  std::vector<std::string> a {"test"};
+  std::vector<std::string> b {"test"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == true);
+}
+
+SECTION("comparing same server lists with multiple entries returns true") {
+  std::vector<std::string> a {"test", "test1", "test2"};
+  std::vector<std::string> b {"test", "test1", "test2"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == true);
+}
+
+SECTION("comparing different server lists with multiple entries returns false") {
+  std::vector<std::string> a {"test", "test1"};
+  std::vector<std::string> b {"test", "test1", "test2"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == false);
+}
+
+SECTION("comparing different server lists with multiple entries returns false 2") {
+  std::vector<std::string> a {"test", "test1", "test2"};
+  std::vector<std::string> b {"test", "test1"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == false);
+}
+
+SECTION("comparing different server lists with multiple entries BUT same contents returns true") {
+  std::vector<std::string> a {"test", "test1", "test2"};
+  std::vector<std::string> b {"test", "test2", "test1"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == true);
+}
+
+SECTION("comparing different server lists with multiple entries but different leader returns false") {
+  std::vector<std::string> a {"test", "test1", "test2"};
+  std::vector<std::string> b {"test2", "test", "test1"};
+
+  REQUIRE(ClusterHelpers::compareServerLists(a, b) == false);
+}
+
+}