From ca2d49bc4e82db28ebc5505e6df15bad34438a65 Mon Sep 17 00:00:00 2001
From: yaphet <4414314+darionyaphet@users.noreply.github.com>
Date: Wed, 12 Jan 2022 10:27:55 +0800
Subject: [PATCH] fix divide zone should be failed if two zones use the same
 host (#3699)

---
 .../processors/zone/DivideZoneProcessor.cpp   |  9 ++++++
 src/meta/test/MetaClientTest.cpp              | 20 ++++++++++++
 src/meta/test/ProcessorTest.cpp               | 32 +++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/src/meta/processors/zone/DivideZoneProcessor.cpp b/src/meta/processors/zone/DivideZoneProcessor.cpp
index 3659a81e3de..6b43915a5ad 100644
--- a/src/meta/processors/zone/DivideZoneProcessor.cpp
+++ b/src/meta/processors/zone/DivideZoneProcessor.cpp
@@ -33,6 +33,7 @@ void DivideZoneProcessor::process(const cpp2::DivideZoneReq& req) {
 
   std::vector<std::string> zoneNames;
   std::unordered_set<HostAddr> totalHosts;
+  size_t totalHostsSize = 0;
   auto batchHolder = std::make_unique<kvstore::BatchHolder>();
   nebula::cpp2::ErrorCode code = nebula::cpp2::ErrorCode::SUCCEEDED;
   for (auto iter = zoneItems.begin(); iter != zoneItems.end(); iter++) {
@@ -65,6 +66,7 @@ void DivideZoneProcessor::process(const cpp2::DivideZoneReq& req) {
       break;
     }
 
+    totalHostsSize += hosts.size();
     std::copy(hosts.begin(), hosts.end(), std::inserter(totalHosts, totalHosts.end()));
 
     auto key = MetaKeyUtils::zoneKey(std::move(zone));
@@ -85,6 +87,13 @@ void DivideZoneProcessor::process(const cpp2::DivideZoneReq& req) {
     return;
   }
 
+  if (totalHostsSize != totalHosts.size()) {
+    LOG(ERROR) << "The host in zone list have duplicate element";
+    handleErrorCode(nebula::cpp2::ErrorCode::E_INVALID_PARM);
+    onFinished();
+    return;
+  }
+
   for (auto& host : totalHosts) {
     auto iter = std::find(zoneHosts.begin(), zoneHosts.end(), host);
     if (iter == zoneHosts.end()) {
diff --git a/src/meta/test/MetaClientTest.cpp b/src/meta/test/MetaClientTest.cpp
index b5675e2d110..d8007b18434 100644
--- a/src/meta/test/MetaClientTest.cpp
+++ b/src/meta/test/MetaClientTest.cpp
@@ -2479,6 +2479,26 @@ TEST(MetaClientTest, DivideZoneTest) {
     auto result = client->divideZone("default_zone", std::move(zoneItems)).get();
     EXPECT_FALSE(result.ok());
   }
+  {
+    std::unordered_map<std::string, std::vector<HostAddr>> zoneItems;
+    std::vector<HostAddr> oneHosts = {
+        {"127.0.0.1", 8986}, {"127.0.0.1", 8987}, {"127.0.0.1", 8988}};
+    zoneItems.emplace("one_zone", std::move(oneHosts));
+    std::vector<HostAddr> anotherHosts = {{"127.0.0.1", 8988}, {"127.0.0.1", 8989}};
+    zoneItems.emplace("another_zone", std::move(anotherHosts));
+    auto result = client->divideZone("default_zone", std::move(zoneItems)).get();
+    EXPECT_FALSE(result.ok());
+  }
+  {
+    std::unordered_map<std::string, std::vector<HostAddr>> zoneItems;
+    std::vector<HostAddr> oneHosts = {
+        {"127.0.0.1", 8986}, {"127.0.0.1", 8987}, {"127.0.0.1", 8987}};
+    zoneItems.emplace("one_zone", std::move(oneHosts));
+    std::vector<HostAddr> anotherHosts = {{"127.0.0.1", 8988}, {"127.0.0.1", 8989}};
+    zoneItems.emplace("another_zone", std::move(anotherHosts));
+    auto result = client->divideZone("default_zone", std::move(zoneItems)).get();
+    EXPECT_FALSE(result.ok());
+  }
   {
     std::unordered_map<std::string, std::vector<HostAddr>> zoneItems;
     std::vector<HostAddr> hosts0 = {};
diff --git a/src/meta/test/ProcessorTest.cpp b/src/meta/test/ProcessorTest.cpp
index 559872ae3b4..6eba572f768 100644
--- a/src/meta/test/ProcessorTest.cpp
+++ b/src/meta/test/ProcessorTest.cpp
@@ -4102,6 +4102,38 @@ TEST(ProcessorTest, DivideZoneTest) {
     auto resp = std::move(f).get();
     ASSERT_EQ(nebula::cpp2::ErrorCode::E_INVALID_PARM, resp.get_code());
   }
+  {
+    cpp2::DivideZoneReq req;
+    req.zone_name_ref() = "default_zone";
+    std::unordered_map<std::string, std::vector<HostAddr>> zoneItems;
+    std::vector<HostAddr> oneHosts = {
+        {"127.0.0.1", 8986}, {"127.0.0.1", 8987}, {"127.0.0.1", 8988}};
+    zoneItems.emplace("one_zone", std::move(oneHosts));
+    std::vector<HostAddr> anotherHosts = {{"127.0.0.1", 8988}, {"127.0.0.1", 8989}};
+    zoneItems.emplace("another_zone", std::move(anotherHosts));
+    req.zone_items_ref() = std::move(zoneItems);
+    auto* processor = DivideZoneProcessor::instance(kv.get());
+    auto f = processor->getFuture();
+    processor->process(req);
+    auto resp = std::move(f).get();
+    ASSERT_EQ(nebula::cpp2::ErrorCode::E_INVALID_PARM, resp.get_code());
+  }
+  {
+    cpp2::DivideZoneReq req;
+    req.zone_name_ref() = "default_zone";
+    std::unordered_map<std::string, std::vector<HostAddr>> zoneItems;
+    std::vector<HostAddr> oneHosts = {
+        {"127.0.0.1", 8986}, {"127.0.0.1", 8987}, {"127.0.0.1", 8987}};
+    zoneItems.emplace("one_zone", std::move(oneHosts));
+    std::vector<HostAddr> anotherHosts = {{"127.0.0.1", 8988}, {"127.0.0.1", 8989}};
+    zoneItems.emplace("another_zone", std::move(anotherHosts));
+    req.zone_items_ref() = std::move(zoneItems);
+    auto* processor = DivideZoneProcessor::instance(kv.get());
+    auto f = processor->getFuture();
+    processor->process(req);
+    auto resp = std::move(f).get();
+    ASSERT_EQ(nebula::cpp2::ErrorCode::E_INVALID_PARM, resp.get_code());
+  }
   {
     cpp2::DivideZoneReq req;
     req.zone_name_ref() = "default_zone";