From 6c6f7a9aa9d098fccf702ff0c2d160a3866e7560 Mon Sep 17 00:00:00 2001
From: Miguel Portilla <miguelportilla@pobros.com>
Date: Tue, 23 Feb 2021 13:10:23 -0500
Subject: [PATCH] [FOLD] Address feedback from Scott Schurr

---
 src/ripple/basics/RangeSet.h              |  2 +-
 src/ripple/nodestore/ShardInfo.h          | 21 +++++++++++++++++----
 src/ripple/nodestore/Types.h              |  2 +-
 src/ripple/nodestore/impl/ShardInfo.cpp   | 14 +++++++++-----
 src/test/nodestore/DatabaseShard_test.cpp |  4 ++--
 5 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/ripple/basics/RangeSet.h b/src/ripple/basics/RangeSet.h
index 8da78f36eac..e8315d61c9d 100644
--- a/src/ripple/basics/RangeSet.h
+++ b/src/ripple/basics/RangeSet.h
@@ -119,7 +119,7 @@ to_string(RangeSet<T> const& rs)
     @return True on successfully converting styled string
 */
 template <class T>
-bool
+[[nodiscard]] bool
 from_string(RangeSet<T>& rs, std::string const& s)
 {
     std::vector<std::string> intervals;
diff --git a/src/ripple/nodestore/ShardInfo.h b/src/ripple/nodestore/ShardInfo.h
index b1339a81304..bedd97da3e8 100644
--- a/src/ripple/nodestore/ShardInfo.h
+++ b/src/ripple/nodestore/ShardInfo.h
@@ -37,13 +37,26 @@ class ShardInfo
     {
     public:
         Incomplete() = delete;
-        Incomplete(ShardState state_, std::uint32_t percentProgress_)
-            : state(state_), percentProgress(percentProgress_)
+        Incomplete(ShardState state, std::uint32_t percentProgress)
+            : state_(state), percentProgress_(percentProgress)
         {
         }
 
-        ShardState const state;
-        std::uint32_t const percentProgress;
+        [[nodiscard]] ShardState
+        state() const noexcept
+        {
+            return state_;
+        }
+
+        [[nodiscard]] std::uint32_t
+        percentProgress() const noexcept
+        {
+            return percentProgress_;
+        }
+
+    private:
+        ShardState state_;
+        std::uint32_t percentProgress_;
     };
 
 public:
diff --git a/src/ripple/nodestore/Types.h b/src/ripple/nodestore/Types.h
index 781b0bf7a59..6d8583ed9d1 100644
--- a/src/ripple/nodestore/Types.h
+++ b/src/ripple/nodestore/Types.h
@@ -57,7 +57,7 @@ using Batch = std::vector<std::shared_ptr<NodeObject>>;
 }  // namespace NodeStore
 
 /** Shard states. */
-enum class ShardState {
+enum class ShardState : std::uint32_t {
     acquire,     // Acquiring ledgers
     complete,    // Backend is ledger complete, database is unverified
     finalizing,  // Verifying database
diff --git a/src/ripple/nodestore/impl/ShardInfo.cpp b/src/ripple/nodestore/impl/ShardInfo.cpp
index 046ed1bba12..5280dde83bb 100644
--- a/src/ripple/nodestore/impl/ShardInfo.cpp
+++ b/src/ripple/nodestore/impl/ShardInfo.cpp
@@ -43,7 +43,7 @@ ShardInfo::incompleteToString() const
         for (auto const& [shardIndex, incomplete] : incomplete_)
         {
             result += std::to_string(shardIndex) + ":" +
-                std::to_string(incomplete.percentProgress) + ",";
+                std::to_string(incomplete.percentProgress()) + ",";
         }
         result.pop_back();
     }
@@ -95,15 +95,19 @@ ShardInfo::makeMessage(Application& app)
             tmIncomplete->set_shardindex(shardIndex);
             s.add32(shardIndex);
 
-            auto const state{static_cast<std::uint32_t>(incomplete.state)};
+            static_assert(std::is_same_v<
+                          std::underlying_type_t<decltype(incomplete.state())>,
+                          std::uint32_t>);
+            auto const state{static_cast<std::uint32_t>(incomplete.state())};
             tmIncomplete->set_state(state);
             s.add32(state);
 
             // Set progress if greater than zero
-            if (incomplete.percentProgress > 0)
+            auto const percentProgress{incomplete.percentProgress()};
+            if (percentProgress > 0)
             {
-                tmIncomplete->set_progress(incomplete.percentProgress);
-                s.add32(incomplete.percentProgress);
+                tmIncomplete->set_progress(percentProgress);
+                s.add32(percentProgress);
             }
         }
     }
diff --git a/src/test/nodestore/DatabaseShard_test.cpp b/src/test/nodestore/DatabaseShard_test.cpp
index dfab90d2d51..b891c2d5c28 100644
--- a/src/test/nodestore/DatabaseShard_test.cpp
+++ b/src/test/nodestore/DatabaseShard_test.cpp
@@ -425,7 +425,7 @@ class DatabaseShard_test : public TestBase
         }
 
         RangeSet<std::uint32_t> rs;
-        from_string(rs, set);
+        BEAST_EXPECT(from_string(rs, set));
         return ripple::to_string(rs);
     }
 
@@ -788,6 +788,7 @@ class DatabaseShard_test : public TestBase
             if (!BEAST_EXPECT(data.makeLedgers(env)))
                 return;
 
+            BEAST_EXPECT(!db->importShard(1, importPath / "not_exist"));
             db->prepareShards({1});
             BEAST_EXPECT(db->getPreShards() == "1");
 
@@ -795,7 +796,6 @@ class DatabaseShard_test : public TestBase
             remove_all(importPath / LgrDBName);
             remove_all(importPath / TxDBName);
 
-            BEAST_EXPECT(!db->importShard(1, importPath / "not_exist"));
             if (!BEAST_EXPECT(db->importShard(1, importPath)))
                 return;