Implement visit over MultivarJson

XRPLF · Nov 20, 2023 · 0736a11 · 0736a11
1 parent 79c7367
commit 0736a11
Show file tree

Hide file tree

Showing 3 changed files with 137 additions and 26 deletions.
diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp
@@ -3174,24 +3174,9 @@ NetworkOPsImp::transJson(
 
     std::string const hash = to_string(transaction->getTransactionID());
     MultiApiJson multiObj({jvObj, jvObj, jvObj});
-    // Minimum supported API version must match index 0 in MultiApiJson
-    static_assert(apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0);
-    // Last valid (possibly beta) API ver. must match last index in MultiApiJson
-    static_assert(
-        apiVersionSelector(RPC::apiMaximumValidVersion)() + 1  //
-        == MultiApiJson::size);
-    for (unsigned apiVersion = RPC::apiMinimumSupportedVersion,
-                  lastIndex = MultiApiJson::size;
-         apiVersion <= RPC::apiMaximumValidVersion;
-         ++apiVersion)
-    {
-        unsigned const index = apiVersionSelector(apiVersion)();
-        assert(index < MultiApiJson::size);
-        if (index != lastIndex)
-        {
-            lastIndex = index;
-
-            Json::Value& jvTx = multiObj.val[index];
+    visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
+        multiObj,  //
+        [&](Json::Value& jvTx, unsigned int apiVersion) {
             RPC::insertDeliverMax(
                 jvTx[jss::transaction], transaction->getTxnType(), apiVersion);
 
@@ -3204,8 +3189,7 @@ NetworkOPsImp::transJson(
             {
                 jvTx[jss::transaction][jss::hash] = hash;
             }
-        }
-    }
+        });
 
     return multiObj;
 }

diff --git a/src/ripple/json/MultivarJson.h b/src/ripple/json/MultivarJson.h
@@ -26,6 +26,8 @@
 #include <cassert>
 #include <concepts>
 #include <cstdlib>
+#include <type_traits>
+#include <utility>
 
 namespace ripple {
 template <std::size_t Size>
@@ -110,6 +112,35 @@ apiVersionSelector(unsigned int apiVersion) noexcept
     };
 }
 
+// Helper to execute a callback for every version. Want both min and max version
+// provided explicitly, so user will know to do update `size` when they change
+template <
+    unsigned int minVer,
+    unsigned int maxVer,
+    std::size_t size,
+    typename Fn>
+    requires                                       //
+    (maxVer >= minVer) &&                          //
+    (size == maxVer + 1 - minVer) &&               //
+    (apiVersionSelector(minVer)() == 0) &&         //
+    (apiVersionSelector(maxVer)() + 1 == size) &&  //
+    requires(Json::Value& json, Fn fn)
+{
+    fn(json, static_cast<unsigned int>(1));
+}
+void
+visit(MultivarJson<size>& json, Fn fn)
+{
+    [&]<std::size_t... offset>(std::index_sequence<offset...>)
+    {
+        static_assert(((apiVersionSelector(minVer + offset)() >= 0) && ...));
+        static_assert(((apiVersionSelector(minVer + offset)() < size) && ...));
+        (fn(json.val[apiVersionSelector(minVer + offset)()], minVer + offset),
+         ...);
+    }
+    (std::make_index_sequence<size>{});
+}
+
 }  // namespace ripple
 
 #endif
diff --git a/src/test/json/MultivarJson_test.cpp b/src/test/json/MultivarJson_test.cpp
@@ -33,6 +33,14 @@ namespace test {
 
 struct MultivarJson_test : beast::unit_test::suite
 {
+    static auto
+    makeJson(const char* key, int val)
+    {
+        Json::Value obj1(Json::objectValue);
+        obj1[key] = val;
+        return obj1;
+    };
+
     void
     run() override
     {
@@ -213,12 +221,6 @@ struct MultivarJson_test : beast::unit_test::suite
             // Well defined behaviour even if we have different types of members
             BEAST_EXPECT(subject.isMember("foo") == decltype(subject)::none);
 
-            auto const makeJson = [](const char* key, int val) {
-                Json::Value obj1(Json::objectValue);
-                obj1[key] = val;
-                return obj1;
-            };
-
             {
                 // All variants have element "One", none have element "Two"
                 MultivarJson<2> const s1{
@@ -285,6 +287,100 @@ struct MultivarJson_test : beast::unit_test::suite
 
             BEAST_EXPECT(MultiApiJson::size >= 1);
         }
+
+        {
+            testcase("visit");
+
+            MultivarJson<3> s1{
+                {makeJson("value", 2),
+                 makeJson("value", 3),
+                 makeJson("value", 5)}};
+
+            int result = 1;
+            ripple::visit<1, 3>(
+                s1, [&](Json::Value& json, unsigned int i) -> void {
+                    if (BEAST_EXPECT(json.isObject() && json.isMember("value")))
+                    {
+                        auto const value = json["value"].asInt();
+                        BEAST_EXPECT(
+                            (value == 2 && i == 1) ||  //
+                            (value == 3 && i == 2) ||  //
+                            (value == 5 && i == 3));
+                        result *= value;
+                    }
+                });
+            BEAST_EXPECT(result == 30);
+
+            // Can use fn with constexpr functor
+            static_assert([](auto&& v) {
+                return requires
+                {
+                    ripple::visit<1, 3>(
+                        v, [](Json::Value&, unsigned int) constexpr {});
+                };
+            }(s1));
+
+            // Can use fn with deduction over all parameters
+            static_assert([](auto&& v) {
+                return requires
+                {
+                    ripple::visit<1, 3>(v, [](auto&, auto) constexpr {});
+                };
+            }(s1));
+
+            // Can use fn with conversion of version parameter
+            static_assert([](auto&& v) {
+                return requires
+                {
+                    ripple::visit<1, 3>(v, [](auto&, std::size_t) constexpr {});
+                };
+            }(s1));
+
+            // Cannot use fn with const parameter
+            static_assert([](auto&& v) {
+                return !requires
+                {
+                    ripple::visit<1, 3>(
+                        v, [](Json::Value const&, auto) constexpr {});
+                };
+            }(const_cast<MultivarJson<3> const&>(s1)));
+
+            // Cannot call visit with size mismatch
+            static_assert([](auto&& v) {
+                return !requires
+                {
+                    ripple::visit<1, 2>(
+                        v, [](Json::Value&, unsigned int) constexpr {});
+                };
+            }(s1));
+
+            // Cannot call visit with version offset
+            static_assert([](auto&& v) {
+                return !requires
+                {
+                    ripple::visit<0, 2>(
+                        v, [](Json::Value&, unsigned int) constexpr {});
+                };
+            }(s1));
+
+            // Cannot call visit with size mismatch
+            static_assert([](auto&& v) {
+                return !requires
+                {
+                    ripple::visit<1, 4>(
+                        v, [](Json::Value&, unsigned int) constexpr {});
+                };
+            }(s1));
+
+            // Cannot call visit with wrong order of versions
+            static_assert([](auto&& v) {
+                return !requires
+                {
+                    ripple::visit<3, 1>(
+                        v, [](Json::Value&, unsigned int) constexpr {});
+                };
+            }(s1));
+        }
     }
 };