Support string type integer for oracle_document_id (#1448)

Fix #1420

src/rpc/common/MetaProcessors.hpp

@@ -20,10 +20,8 @@
 #pragma once
 
 #include "rpc/Errors.hpp"
-#include "rpc/common/Concepts.hpp"
 #include "rpc/common/Specs.hpp"
 #include "rpc/common/Types.hpp"
-#include "rpc/common/Validators.hpp"
 
 #include <boost/json/value.hpp>
 #include <fmt/core.h>
@@ -146,10 +144,10 @@ public:
     [[nodiscard]] MaybeError
     verify(boost::json::value& value, std::string_view key) const
     {
-        if (not value.is_object() or not value.as_object().contains(key.data()))
+        if (not value.is_object() or not value.as_object().contains(key))
             return {};  // ignore. field does not exist, let 'required' fail instead
 
-        if (not rpc::validation::checkType<Type>(value.as_object().at(key.data())))
+        if (not rpc::validation::checkType<Type>(value.as_object().at(key)))
             return {};  // ignore if type does not match
 
         return processor_(value, key);
@@ -162,9 +160,10 @@ private:
 /**
  * @brief A meta-processor that wraps a validator and produces a custom error in case the wrapped validator fails.
  */
-template <typename SomeRequirement>
+template <typename RequirementOrModifierType>
+    requires SomeRequirement<RequirementOrModifierType> or SomeModifier<RequirementOrModifierType>
 class WithCustomError final {
-    SomeRequirement requirement;
+    RequirementOrModifierType reqOrModifier;
     Status error;
 
 public:
@@ -172,10 +171,11 @@ public:
      * @brief Constructs a validator that calls the given validator `req` and returns a custom error `err` in case `req`
      * fails.
      *
-     * @param req The requirement to validate against
+     * @param reqOrModifier The requirement to validate against
      * @param err The custom error to return in case `req` fails
      */
-    WithCustomError(SomeRequirement req, Status err) : requirement{std::move(req)}, error{std::move(err)}
+    WithCustomError(RequirementOrModifierType reqOrModifier, Status err)
+        : reqOrModifier{std::move(reqOrModifier)}, error{std::move(err)}
     {
     }
 
@@ -188,8 +188,9 @@ public:
      */
     [[nodiscard]] MaybeError
     verify(boost::json::value const& value, std::string_view key) const
+        requires SomeRequirement<RequirementOrModifierType>
     {
-        if (auto const res = requirement.verify(value, key); not res)
+        if (auto const res = reqOrModifier.verify(value, key); not res)
             return Error{error};
 
         return {};
@@ -205,12 +206,30 @@ public:
      */
     [[nodiscard]] MaybeError
     verify(boost::json::value& value, std::string_view key) const
+        requires SomeRequirement<RequirementOrModifierType>
     {
-        if (auto const res = requirement.verify(value, key); not res)
+        if (auto const res = reqOrModifier.verify(value, key); not res)
             return Error{error};
 
         return {};
     }
+
+    /**
+     * @brief Runs the stored modifier and produces a custom error if the wrapped modifier fails.
+     *
+     * @param value The JSON value representing the outer object. This value can be modified by the modifier.
+     * @param key The key used to retrieve the element from the outer object
+     * @return Possibly an error
+     */
+    MaybeError
+    modify(boost::json::value& value, std::string_view key) const
+        requires SomeModifier<RequirementOrModifierType>
+
+    {
+        if (auto const res = reqOrModifier.modify(value, key); not res)
+            return Error{error};
+        return {};
+    }
 };
 
 }  // namespace rpc::meta

src/rpc/common/Modifiers.hpp

@@ -19,12 +19,16 @@
 
 #pragma once
 
+#include "rpc/Errors.hpp"
 #include "rpc/common/Types.hpp"
 #include "util/JsonUtils.hpp"
 
 #include <boost/json/value.hpp>
 #include <boost/json/value_to.hpp>
+#include <ripple/protocol/ErrorCodes.h>
 
+#include <exception>
+#include <functional>
 #include <string>
 #include <string_view>
 
@@ -100,4 +104,75 @@ struct ToLower final {
     }
 };
 
+/**
+ * @brief Convert input string to integer.
+ *
+ * Note: the conversion is only performed if the input value is a string.
+ */
+struct ToNumber final {
+    /**
+     * @brief Update the input string to integer if it can be converted to integer by stoi.
+     *
+     * @param value The JSON value representing the outer object
+     * @param key The key used to retrieve the modified value from the outer object
+     * @return Possibly an error
+     */
+    [[nodiscard]] static MaybeError
+    modify(boost::json::value& value, std::string_view key)
+    {
+        if (not value.is_object() or not value.as_object().contains(key))
+            return {};  // ignore. field does not exist, let 'required' fail instead
+
+        if (not value.as_object().at(key).is_string())
+            return {};  // ignore for non-string types
+
+        auto const strInt = boost::json::value_to<std::string>(value.as_object().at(key));
+        if (strInt.find('.') != std::string::npos)
+            return Error{Status{RippledError::rpcINVALID_PARAMS}};  // maybe a float
+
+        try {
+            value.as_object()[key.data()] = std::stoi(strInt);
+        } catch (std::exception& e) {
+            return Error{Status{RippledError::rpcINVALID_PARAMS}};
+        }
+        return {};
+    }
+};
+
+/**
+ * @brief Customised modifier allowing user define how to modify input in provided callable.
+ */
+class CustomModifier final {
+    std::function<MaybeError(boost::json::value&, std::string_view)> modifier_;
+
+public:
+    /**
+     * @brief Constructs a custom modifier from any supported callable.
+     *
+     * @tparam Fn The type of callable
+     * @param fn The callable/function object
+     */
+    template <typename Fn>
+        requires std::invocable<Fn, boost::json::value&, std::string_view>
+    explicit CustomModifier(Fn&& fn) : modifier_{std::forward<Fn>(fn)}
+    {
+    }
+
+    /**
+     * @brief Modify the JSON value according to the custom modifier function stored.
+     *
+     * @param value The JSON value representing the outer object
+     * @param key The key used to retrieve the tested value from the outer object
+     * @return Any compatible user-provided error if modify/verify failed; otherwise no error is returned
+     */
+    [[nodiscard]] MaybeError
+    modify(boost::json::value& value, std::string_view key) const
+    {
+        if (not value.is_object() or not value.as_object().contains(key))
+            return {};  // ignore. field does not exist, let 'required' fail instead
+
+        return modifier_(value.as_object().at(key.data()), key);
+    };
+};
+
 }  // namespace rpc::modifiers

src/rpc/common/Validators.cpp

@@ -29,6 +29,7 @@
 #include <fmt/core.h>
 #include <ripple/basics/base_uint.h>
 #include <ripple/protocol/AccountID.h>
+#include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/UintTypes.h>
 #include <ripple/protocol/tokens.h>
 
@@ -45,7 +46,7 @@ namespace rpc::validation {
 [[nodiscard]] MaybeError
 Required::verify(boost::json::value const& value, std::string_view key)
 {
-    if (not value.is_object() or not value.as_object().contains(key.data()))
+    if (not value.is_object() or not value.as_object().contains(key))
         return Error{Status{RippledError::rpcINVALID_PARAMS, "Required field '" + std::string{key} + "' missing"}};
 
     return {};
@@ -54,10 +55,10 @@ Required::verify(boost::json::value const& value, std::string_view key)
 [[nodiscard]] MaybeError
 CustomValidator::verify(boost::json::value const& value, std::string_view key) const
 {
-    if (not value.is_object() or not value.as_object().contains(key.data()))
+    if (not value.is_object() or not value.as_object().contains(key))
         return {};  // ignore. field does not exist, let 'required' fail instead
 
-    return validator_(value.as_object().at(key.data()), key);
+    return validator_(value.as_object().at(key), key);
 }
 
 [[nodiscard]] bool

src/rpc/common/Validators.hpp

@@ -402,6 +402,7 @@ public:
      * @param fn The callable/function object
      */
     template <typename Fn>
+        requires std::invocable<Fn, boost::json::value const&, std::string_view>
     explicit CustomValidator(Fn&& fn) : validator_{std::forward<Fn>(fn)}
     {
     }

src/rpc/handlers/GetAggregatePrice.hpp

@@ -22,6 +22,7 @@
 #include "data/BackendInterface.hpp"
 #include "rpc/Errors.hpp"
 #include "rpc/JS.hpp"
+#include "rpc/common/Modifiers.hpp"
 #include "rpc/common/Specs.hpp"
 #include "rpc/common/Types.hpp"
 #include "rpc/common/Validators.hpp"
@@ -131,23 +132,26 @@ public:
         static auto constexpr ORACLES_MAX = 200;
 
         static auto const oraclesValidator =
-            validation::CustomValidator{[](boost::json::value const& value, std::string_view) -> MaybeError {
+            modifiers::CustomModifier{[](boost::json::value& value, std::string_view) -> MaybeError {
                 if (!value.is_array() or value.as_array().empty() or value.as_array().size() > ORACLES_MAX)
                     return Error{Status{RippledError::rpcORACLE_MALFORMED}};
 
-                for (auto oracle : value.as_array()) {
+                for (auto& oracle : value.as_array()) {
                     if (!oracle.is_object() or !oracle.as_object().contains(JS(oracle_document_id)) or
                         !oracle.as_object().contains(JS(account)))
                         return Error{Status{RippledError::rpcORACLE_MALFORMED}};
 
-                    auto maybeError =
-                        validation::Type<std::uint32_t>{}.verify(oracle.as_object(), JS(oracle_document_id));
+                    auto maybeError = validation::Type<std::uint32_t, std::string>{}.verify(
+                        oracle.as_object(), JS(oracle_document_id)
+                    );
+                    if (!maybeError)
+                        return maybeError;
 
+                    maybeError = modifiers::ToNumber::modify(oracle, JS(oracle_document_id));
                     if (!maybeError)
                         return maybeError;
 
                     maybeError = validation::AccountBase58Validator.verify(oracle.as_object(), JS(account));
-
                     if (!maybeError)
                         return Error{Status{RippledError::rpcINVALID_PARAMS}};
                 };

src/rpc/handlers/LedgerEntry.hpp

@@ -24,6 +24,7 @@
 #include "rpc/JS.hpp"
 #include "rpc/common/Checkers.hpp"
 #include "rpc/common/MetaProcessors.hpp"
+#include "rpc/common/Modifiers.hpp"
 #include "rpc/common/Specs.hpp"
 #include "rpc/common/Types.hpp"
 #include "rpc/common/Validators.hpp"
@@ -296,8 +297,9 @@ public:
                  {JS(oracle_document_id),
                   meta::WithCustomError{validation::Required{}, Status(ClioError::rpcMALFORMED_REQUEST)},
                   meta::WithCustomError{
-                      validation::Type<uint32_t>{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID)
-                  }},
+                      validation::Type<uint32_t, std::string>{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID)
+                  },
+                  meta::WithCustomError{modifiers::ToNumber{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID)}},
              }}},
             {JS(ledger), check::Deprecated{}},
         };

tests/unit/rpc/BaseTests.cpp

@@ -31,6 +31,7 @@
 #include <boost/json/parse.hpp>
 #include <boost/json/value.hpp>
 #include <fmt/core.h>
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <ripple/protocol/AccountID.h>
 #include <ripple/protocol/ErrorCodes.h>
@@ -604,3 +605,49 @@ TEST_F(RPCBaseTest, ToLowerModifier)
     ASSERT_TRUE(spec.process(passingInput4));  // empty str no problem
     ASSERT_EQ(passingInput4.at("str").as_string(), "");
 }
+
+TEST_F(RPCBaseTest, ToNumberModifier)
+{
+    auto const spec = RpcSpec{
+        {"str", ToNumber{}},
+    };
+
+    auto passingInput = json::parse(R"({ "str": [] })");
+    ASSERT_TRUE(spec.process(passingInput));
+
+    passingInput = json::parse(R"({ "str2": "TesT" })");
+    ASSERT_TRUE(spec.process(passingInput));
+
+    passingInput = json::parse(R"([])");
+    ASSERT_TRUE(spec.process(passingInput));
+
+    passingInput = json::parse(R"({ "str": "123" })");
+    ASSERT_TRUE(spec.process(passingInput));
+    ASSERT_EQ(passingInput.at("str").as_int64(), 123);
+
+    auto failingInput = json::parse(R"({ "str": "ok" })");
+    ASSERT_FALSE(spec.process(failingInput));
+
+    failingInput = json::parse(R"({ "str": "123.123" })");
+    ASSERT_FALSE(spec.process(failingInput));
+}
+
+TEST_F(RPCBaseTest, CustomModifier)
+{
+    testing::StrictMock<testing::MockFunction<MaybeError(json::value & value, std::string_view)>> mockModifier;
+    auto const customModifier = CustomModifier{mockModifier.AsStdFunction()};
+    auto const spec = RpcSpec{
+        {"str", customModifier},
+    };
+
+    EXPECT_CALL(mockModifier, Call).WillOnce(testing::Return(MaybeError{}));
+    auto passingInput = json::parse(R"({ "str": "sss" })");
+    ASSERT_TRUE(spec.process(passingInput));
+
+    passingInput = json::parse(R"({ "strNotExist": 123 })");
+    ASSERT_TRUE(spec.process(passingInput));
+
+    // not a json object
+    passingInput = json::parse(R"([])");
+    ASSERT_TRUE(spec.process(passingInput));
+}

tests/unit/rpc/handlers/GetAggregatePriceTests.cpp

@@ -427,6 +427,55 @@ TEST_F(RPCGetAggregatePriceHandlerTest, OracleLedgerEntrySinglePriceData)
     });
 }
 
+TEST_F(RPCGetAggregatePriceHandlerTest, OracleLedgerEntryStrOracleDocumentId)
+{
+    EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _))
+        .WillOnce(Return(CreateLedgerHeader(LEDGERHASH, RANGEMAX)));
+
+    auto constexpr documentId = 1;
+    mockLedgerObject(*backend, ACCOUNT, documentId, TX1, 1e3, 2);  // 10
+
+    auto const handler = AnyHandler{GetAggregatePriceHandler{backend}};
+    auto const req = json::parse(fmt::format(
+        R"({{
+                "base_asset": "USD",
+                "quote_asset": "XRP",
+                "oracles": 
+                [
+                    {{
+                        "account": "{}",
+                        "oracle_document_id": "{}"
+                    }}
+                ]
+            }})",
+        ACCOUNT,
+        documentId
+    ));
+
+    auto const expected = json::parse(fmt::format(
+        R"({{
+                "entire_set": 
+                {{
+                    "mean": "10",
+                    "size": 1,
+                    "standard_deviation": "0"
+                }},
+                "median": "10",
+                "time": 4321,
+                "ledger_index": {},
+                "ledger_hash": "{}",
+                "validated": true
+            }})",
+        RANGEMAX,
+        LEDGERHASH
+    ));
+    runSpawn([&](auto yield) {
+        auto const output = handler.process(req, Context{yield});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(output.result.value(), expected);
+    });
+}
+
 TEST_F(RPCGetAggregatePriceHandlerTest, PreviousTxNotFound)
 {
     EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _))

tests/unit/rpc/handlers/LedgerEntryTests.cpp

@@ -2315,7 +2315,7 @@ generateTestValuesForNormalPathTest()
             CreateChainOwnedClaimIDObject(ACCOUNT, ACCOUNT, ACCOUNT2, "JPY", ACCOUNT3, ACCOUNT)
         },
         NormalPathTestBundle{
-            "OracleEntryFoundViaObject",
+            "OracleEntryFoundViaIntOracleDocumentId",
             fmt::format(
                 R"({{
                     "binary": true,
@@ -2341,6 +2341,33 @@ generateTestValuesForNormalPathTest()
                 )
             )
         },
+        NormalPathTestBundle{
+            "OracleEntryFoundViaStrOracleDocumentId",
+            fmt::format(
+                R"({{
+                    "binary": true,
+                    "oracle": {{
+                        "account": "{}",
+                        "oracle_document_id": "1"
+                    }}
+                }})",
+                ACCOUNT
+            ),
+            ripple::keylet::oracle(GetAccountIDWithString(ACCOUNT), 1).key,
+            CreateOracleObject(
+                ACCOUNT,
+                "70726F7669646572",
+                32u,
+                1234u,
+                ripple::Blob(8, 's'),
+                ripple::Blob(8, 's'),
+                RANGEMAX - 2,
+                ripple::uint256{"E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321"},
+                CreatePriceDataSeries(
+                    {CreateOraclePriceData(2e4, ripple::to_currency("XRP"), ripple::to_currency("USD"), 3)}
+                )
+            )
+        },
         NormalPathTestBundle{
             "OracleEntryFoundViaString",
             fmt::format(