Rename MaskRemapper to HasMaskRemapper.

Move HasMaskRemapper to separate file.
Add separate unit test for HasMaskRemapper.

6 files changed, 116 insertions, 60 deletions

diff --git a/storage/src/tests/persistence/CMakeLists.txt b/storage/src/tests/persistence/CMakeLists.txt
index 5e03a2a3f2e..f922689b941 100644
--- a/storage/src/tests/persistence/CMakeLists.txt
+++ b/storage/src/tests/persistence/CMakeLists.txt
@@ -3,6 +3,7 @@
 vespa_add_executable(storage_persistence_gtest_runner_app TEST
     SOURCES
     bucketownershipnotifiertest.cpp
+    has_mask_remapper_test.cpp
     mergehandlertest.cpp
     persistencequeuetest.cpp
     persistencetestutils.cpp
diff --git a/storage/src/tests/persistence/has_mask_remapper_test.cpp b/storage/src/tests/persistence/has_mask_remapper_test.cpp
new file mode 100644
index 00000000000..58f6f5b3fe7
--- /dev/null
+++ b/storage/src/tests/persistence/has_mask_remapper_test.cpp
@@ -0,0 +1,42 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include <vespa/storage/persistence/filestorage/has_mask_remapper.h>
+#include <gtest/gtest.h>
+
+namespace storage {
+
+using NodeList = std::vector<api::MergeBucketCommand::Node>;
+
+const NodeList merge_operation_nodes{{0, true}, {1, true}, {2, false}, {3, false}, {4, false}};
+
+TEST(HasMaskRemapperTest, test_remap_none)
+{
+    HasMaskRemapper remap_mask(merge_operation_nodes, merge_operation_nodes);
+    for (uint32_t i = 0; i < (1u << merge_operation_nodes.size()); ++i) {
+        EXPECT_EQ(i, remap_mask(i));
+    }
+}
+
+TEST(HasMaskRemapperTest, test_remap_subset)
+{
+    NodeList reply_nodes{{0, true}, {1, true}, {3, false}};
+    HasMaskRemapper remap_mask(merge_operation_nodes, reply_nodes);
+    std::vector<uint16_t> remapped;
+    for (uint32_t i = 0; i < (1u << reply_nodes.size()); ++i) {
+        remapped.push_back(remap_mask(i));
+    }
+    EXPECT_EQ((std::vector<uint16_t>{0u, 1u, 2u, 3u, 8u, 9u, 10u, 11u}), remapped);
+}
+
+TEST(HasMaskRemapperTest, test_remap_swapped_subset)
+{
+    NodeList reply_nodes{{1, true}, {0, true}};
+    HasMaskRemapper remap_mask(merge_operation_nodes, reply_nodes);
+    std::vector<uint16_t> remapped;
+    for (uint32_t i = 0; i < (1u << reply_nodes.size()); ++i) {
+        remapped.push_back(remap_mask(i));
+    }
+    EXPECT_EQ((std::vector<uint16_t>{0u, 2u, 1u, 3u}), remapped);
+}
+
+}
diff --git a/storage/src/vespa/storage/persistence/filestorage/CMakeLists.txt b/storage/src/vespa/storage/persistence/filestorage/CMakeLists.txt
index 61cfcbcbfb8..2fd54930f77 100644
--- a/storage/src/vespa/storage/persistence/filestorage/CMakeLists.txt
+++ b/storage/src/vespa/storage/persistence/filestorage/CMakeLists.txt
@@ -4,6 +4,7 @@ vespa_add_library(storage_filestorpersistence OBJECT
     filestorhandlerimpl.cpp
     filestormanager.cpp
     filestormetrics.cpp
+    has_mask_remapper.cpp
     merge_handler_metrics.cpp
     mergestatus.cpp
     modifiedbucketchecker.cpp
diff --git a/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.cpp b/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.cpp
new file mode 100644
index 00000000000..5aea3f94a57
--- /dev/null
+++ b/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.cpp
@@ -0,0 +1,46 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "has_mask_remapper.h"
+#include <vespa/vespalib/stllike/hash_map.h>
+#include <cassert>
+
+namespace storage {
+
+HasMaskRemapper::HasMaskRemapper(const std::vector<api::MergeBucketCommand::Node> &all_nodes,
+                                 const std::vector<api::MergeBucketCommand::Node> &nodes)
+    : _mask_remap()
+{
+    if (nodes != all_nodes) {
+        vespalib::hash_map<uint32_t, uint32_t> node_index_to_mask(all_nodes.size());
+        uint16_t mask = 1u;
+        for (const auto& node : all_nodes) {
+            node_index_to_mask[node.index] = mask;
+            mask <<= 1;
+        }
+        _mask_remap.reserve(nodes.size());
+        for (const auto& node : nodes) {
+            mask = node_index_to_mask[node.index];
+            assert(mask != 0u);
+            _mask_remap.push_back(mask);
+        }
+    }
+}
+
+HasMaskRemapper::~HasMaskRemapper() = default;
+
+uint16_t
+HasMaskRemapper::operator()(uint16_t mask) const
+{
+    if (!_mask_remap.empty()) {
+        uint16_t new_mask = 0u;
+        for (uint32_t i = 0u; i < _mask_remap.size(); ++i) {
+            if ((mask & (1u << i)) != 0u) {
+                new_mask |= _mask_remap[i];
+            }
+        }
+        mask = new_mask;
+    }
+    return mask;
+}
+
+}
diff --git a/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.h b/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.h
new file mode 100644
index 00000000000..94a97ed943a
--- /dev/null
+++ b/storage/src/vespa/storage/persistence/filestorage/has_mask_remapper.h
@@ -0,0 +1,24 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <vespa/storageapi/message/bucket.h>
+
+namespace storage {
+
+/*
+ * Class for remapping bit masks from a partial set of nodes to a full
+ * set of nodes.
+ */
+class HasMaskRemapper
+{
+    std::vector<uint16_t> _mask_remap;
+
+public:
+    HasMaskRemapper(const std::vector<api::MergeBucketCommand::Node> &all_nodes,
+                 const std::vector<api::MergeBucketCommand::Node> &nodes);
+    ~HasMaskRemapper();
+
+    uint16_t operator()(uint16_t mask) const;
+};
+
+}
diff --git a/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp b/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
index 97d3da28abe..b5f493cc921 100644
--- a/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
@@ -1,72 +1,14 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "mergestatus.h"
+#include "has_mask_remapper.h"
 #include <ostream>
 #include <vespa/log/log.h>
-#include <vespa/vespalib/stllike/hash_map.h>
-#include <cassert>
 
 LOG_SETUP(".mergestatus");
 
 namespace storage {
 
-namespace {
-
-/*
- * Class for remapping bit masks from a partial set of nodes to a full
- * set of nodes.
- */
-class MaskRemapper
-{
-    std::vector<uint16_t> _mask_remap;
-
-public:
-    MaskRemapper(const std::vector<api::MergeBucketCommand::Node> &all_nodes,
-                 const std::vector<api::MergeBucketCommand::Node> &nodes);
-    ~MaskRemapper();
-
-    uint16_t operator()(uint16_t mask) const;
-};
-
-MaskRemapper::MaskRemapper(const std::vector<api::MergeBucketCommand::Node> &all_nodes,
-                           const std::vector<api::MergeBucketCommand::Node> &nodes)
-    : _mask_remap()
-{
-    if (nodes != all_nodes) {
-        vespalib::hash_map<uint32_t, uint32_t> node_index_to_mask(all_nodes.size());
-        uint16_t mask = 1u;
-        for (const auto& node : all_nodes) {
-            node_index_to_mask[node.index] = mask;
-            mask <<= 1;
-        }
-        _mask_remap.reserve(nodes.size());
-        for (const auto& node : nodes) {
-            mask = node_index_to_mask[node.index];
-            assert(mask != 0u);
-            _mask_remap.push_back(mask);
-        }
-    }
-}
-
-MaskRemapper::~MaskRemapper() = default;
-
-uint16_t
-MaskRemapper::operator()(uint16_t mask) const
-{
-    if (!_mask_remap.empty()) {
-        uint16_t new_mask = 0u;
-        for (uint32_t i = 0u; i < _mask_remap.size(); ++i) {
-            if ((mask & (1u << i)) != 0u) {
-                new_mask |= _mask_remap[i];
-            }
-        }
-        mask = new_mask;
-    }
-    return mask;
-}
-
-}
-
 MergeStatus::MergeStatus(const framework::Clock& clock, const metrics::LoadType& lt,
                          api::StorageMessage::Priority priority,
                          uint32_t traceLevel)
@@ -91,7 +33,7 @@ MergeStatus::removeFromDiff(
     std::vector<api::ApplyBucketDiffCommand::Entry>::const_iterator it2(
             part.begin());
     bool altered = false;
-    MaskRemapper remap_mask(nodeList, nodes);
+    HasMaskRemapper remap_mask(nodeList, nodes);
     // We expect part array to be sorted in the same order as in the diff,
     // and that all entries in the part should exist in the source list.
     while (it != diff.end() && it2 != part.end()) {