Revert "feat: add config verify flag (#1814)"

This reverts commit f1698c55ff.

src/app/CliArgs.cpp

@@ -47,7 +47,6 @@ CliArgs::parse(int argc, char const* argv[])
         ("conf,c", po::value<std::string>()->default_value(kDEFAULT_CONFIG_PATH), "configuration file")
         ("ng-web-server,w", "Use ng-web-server")
         ("migrate", po::value<std::string>(), "start migration helper")
-        ("verify", "Checks the validity of config values")
     ;
     // clang-format on
     po::positional_options_description positional;
@@ -76,9 +75,6 @@ CliArgs::parse(int argc, char const* argv[])
         return Action{Action::Migrate{.configPath = std::move(configPath), .subCmd = MigrateSubCmd::migration(opt)}};
     }
 
-    if (parsed.count("verify") != 0u)
-        return Action{Action::VerifyConfig{.configPath = std::move(configPath)}};
-
     return Action{Action::Run{.configPath = std::move(configPath), .useNgWebServer = parsed.count("ng-web-server") != 0}
     };
 }

src/app/CliArgs.hpp

@@ -59,11 +59,6 @@ public:
             MigrateSubCmd subCmd;
         };
 
-        /** @brief Verify Config action. */
-        struct VerifyConfig {
-            std::string configPath;
-        };
-
         /**
          * @brief Construct an action from a Run.
          *
@@ -71,7 +66,7 @@ public:
          */
         template <typename ActionType>
             requires std::is_same_v<ActionType, Run> or std::is_same_v<ActionType, Exit> or
-            std::is_same_v<ActionType, Migrate> or std::is_same_v<ActionType, VerifyConfig>
+            std::is_same_v<ActionType, Migrate>
         explicit Action(ActionType&& action) : action_(std::forward<ActionType>(action))
         {
         }
@@ -91,7 +86,7 @@ public:
         }
 
     private:
-        std::variant<Run, Exit, Migrate, VerifyConfig> action_;
+        std::variant<Run, Exit, Migrate> action_;
     };
 
     /**

src/app/VerifyConfig.hpp

@@ -1,55 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of clio: https://github.com/XRPLF/clio
-    Copyright (c) 2025, the clio developers.
-
-    Permission to use, copy, modify, and distribute this software for any
-    purpose with or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL,  DIRECT,  INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#pragma once
-
-#include "util/newconfig/ConfigDefinition.hpp"
-#include "util/newconfig/ConfigFileJson.hpp"
-
-#include <cstdlib>
-#include <iostream>
-#include <string_view>
-
-namespace app {
-
-/**
- * @brief Verifies user's config values are correct
- *
- * @param configPath The path to config
- * @return true if config values are all correct, false otherwise
- */
-inline bool
-verifyConfig(std::string_view configPath)
-{
-    using namespace util::config;
-
-    auto const json = ConfigFileJson::makeConfigFileJson(configPath);
-    if (!json.has_value()) {
-        std::cerr << json.error().error << std::endl;
-        return false;
-    }
-    auto const errors = gClioConfig.parse(json.value());
-    if (errors.has_value()) {
-        for (auto const& err : errors.value())
-            std::cerr << err.error << std::endl;
-        return false;
-    }
-    return true;
-}
-}  // namespace app

src/main/Main.cpp

@@ -19,12 +19,12 @@
 
 #include "app/CliArgs.hpp"
 #include "app/ClioApplication.hpp"
-#include "app/VerifyConfig.hpp"
 #include "migration/MigrationApplication.hpp"
 #include "rpc/common/impl/HandlerProvider.hpp"
 #include "util/TerminationHandler.hpp"
 #include "util/log/Logger.hpp"
 #include "util/newconfig/ConfigDefinition.hpp"
+#include "util/newconfig/ConfigFileJson.hpp"
 
 #include <cstdlib>
 #include <exception>
@@ -40,27 +40,34 @@ try {
     auto const action = app::CliArgs::parse(argc, argv);
     return action.apply(
         [](app::CliArgs::Action::Exit const& exit) { return exit.exitCode; },
-        [](app::CliArgs::Action::VerifyConfig const& verify) {
-            if (app::verifyConfig(verify.configPath)) {
-                std::cout << "Config is correct" << "\n";
-                return EXIT_SUCCESS;
-            }
-            return EXIT_FAILURE;
-        },
         [](app::CliArgs::Action::Run const& run) {
-            auto const res = app::verifyConfig(run.configPath);
-            if (res != EXIT_SUCCESS)
+            auto const json = ConfigFileJson::makeConfigFileJson(run.configPath);
+            if (!json.has_value()) {
+                std::cerr << json.error().error << std::endl;
                 return EXIT_FAILURE;
-
+            }
+            auto const errors = gClioConfig.parse(json.value());
+            if (errors.has_value()) {
+                for (auto const& err : errors.value())
+                    std::cerr << err.error << std::endl;
+                return EXIT_FAILURE;
+            }
             util::LogService::init(gClioConfig);
             app::ClioApplication clio{gClioConfig};
             return clio.run(run.useNgWebServer);
         },
         [](app::CliArgs::Action::Migrate const& migrate) {
-            auto const res = app::verifyConfig(migrate.configPath);
-            if (res != EXIT_SUCCESS)
+            auto const json = ConfigFileJson::makeConfigFileJson(migrate.configPath);
+            if (!json.has_value()) {
+                std::cerr << json.error().error << std::endl;
                 return EXIT_FAILURE;
-
+            }
+            auto const errors = gClioConfig.parse(json.value());
+            if (errors.has_value()) {
+                for (auto const& err : errors.value())
+                    std::cerr << err.error << std::endl;
+                return EXIT_FAILURE;
+            }
             util::LogService::init(gClioConfig);
             app::MigratorApplication migrator{gClioConfig, migrate.subCmd};
             return migrator.run();

src/util/newconfig/ConfigDefinition.hpp

@@ -416,7 +416,6 @@ static ClioConfigDefinition gClioConfig = ClioConfigDefinition{
       ConfigValue{ConfigType::Integer}.defaultValue(rpc::kAPI_VERSION_MIN).withConstraint(gValidateApiVersion)},
      {"api_version.max",
       ConfigValue{ConfigType::Integer}.defaultValue(rpc::kAPI_VERSION_MAX).withConstraint(gValidateApiVersion)},
-
      {"migration.full_scan_threads", ConfigValue{ConfigType::Integer}.defaultValue(2).withConstraint(gValidateUint32)},
      {"migration.full_scan_jobs", ConfigValue{ConfigType::Integer}.defaultValue(4).withConstraint(gValidateUint32)},
      {"migration.cursors_per_job", ConfigValue{ConfigType::Integer}.defaultValue(100).withConstraint(gValidateUint32)}},

tests/common/util/newconfig/FakeConfigData.hpp

@@ -208,11 +208,3 @@ static constexpr auto kINVALID_JSON_DATA = R"JSON({
         "withDefault" : "0.0"
     }
 })JSON";
-
-// used to Verify Config test
-static constexpr auto kVALID_JSON_DATA = R"JSON({
-     "server": {
-        "ip": "0.0.0.0",
-        "port": 51233
-    }
-})JSON";

tests/unit/CMakeLists.txt

@@ -5,7 +5,6 @@ target_sources(
   PRIVATE # Common
           ConfigTests.cpp
           app/CliArgsTests.cpp
-          app/VerifyConfigTests.cpp
           app/WebHandlersTests.cpp
           data/AmendmentCenterTests.cpp
           data/BackendCountersTests.cpp

tests/unit/app/CliArgsTests.cpp

@@ -32,7 +32,6 @@ struct CliArgsTests : testing::Test {
     testing::StrictMock<testing::MockFunction<int(CliArgs::Action::Run)>> onRunMock;
     testing::StrictMock<testing::MockFunction<int(CliArgs::Action::Exit)>> onExitMock;
     testing::StrictMock<testing::MockFunction<int(CliArgs::Action::Migrate)>> onMigrateMock;
-    testing::StrictMock<testing::MockFunction<int(CliArgs::Action::VerifyConfig)>> onVerifyMock;
 };
 
 TEST_F(CliArgsTests, Parse_NoArgs)
@@ -47,13 +46,7 @@ TEST_F(CliArgsTests, Parse_NoArgs)
         return returnCode;
     });
     EXPECT_EQ(
-        action.apply(
-            onRunMock.AsStdFunction(),
-            onExitMock.AsStdFunction(),
-            onMigrateMock.AsStdFunction(),
-            onVerifyMock.AsStdFunction()
-        ),
-        returnCode
+        action.apply(onRunMock.AsStdFunction(), onExitMock.AsStdFunction(), onMigrateMock.AsStdFunction()), returnCode
     );
 }
 
@@ -69,12 +62,7 @@ TEST_F(CliArgsTests, Parse_NgWebServer)
             return returnCode;
         });
         EXPECT_EQ(
-            action.apply(
-                onRunMock.AsStdFunction(),
-                onExitMock.AsStdFunction(),
-                onMigrateMock.AsStdFunction(),
-                onVerifyMock.AsStdFunction()
-            ),
+            action.apply(onRunMock.AsStdFunction(), onExitMock.AsStdFunction(), onMigrateMock.AsStdFunction()),
             returnCode
         );
     }
@@ -91,12 +79,7 @@ TEST_F(CliArgsTests, Parse_VersionHelp)
 
         EXPECT_CALL(onExitMock, Call).WillOnce([](CliArgs::Action::Exit const& exit) { return exit.exitCode; });
         EXPECT_EQ(
-            action.apply(
-                onRunMock.AsStdFunction(),
-                onExitMock.AsStdFunction(),
-                onMigrateMock.AsStdFunction(),
-                onVerifyMock.AsStdFunction()
-            ),
+            action.apply(onRunMock.AsStdFunction(), onExitMock.AsStdFunction(), onMigrateMock.AsStdFunction()),
             EXIT_SUCCESS
         );
     }
@@ -114,34 +97,6 @@ TEST_F(CliArgsTests, Parse_Config)
         return returnCode;
     });
     EXPECT_EQ(
-        action.apply(
-            onRunMock.AsStdFunction(),
-            onExitMock.AsStdFunction(),
-            onMigrateMock.AsStdFunction(),
-            onVerifyMock.AsStdFunction()
-        ),
-        returnCode
-    );
-}
-
-TEST_F(CliArgsTests, Parse_VerifyConfig)
-{
-    std::string_view configPath = "some_config_path";
-    std::array argv{"clio_server", configPath.data(), "--verify"};  // NOLINT(bugprone-suspicious-stringview-data-usage)
-    auto const action = CliArgs::parse(argv.size(), argv.data());
-
-    int const returnCode = 123;
-    EXPECT_CALL(onVerifyMock, Call).WillOnce([&configPath](CliArgs::Action::VerifyConfig const& verify) {
-        EXPECT_EQ(verify.configPath, configPath);
-        return returnCode;
-    });
-    EXPECT_EQ(
-        action.apply(
-            onRunMock.AsStdFunction(),
-            onExitMock.AsStdFunction(),
-            onMigrateMock.AsStdFunction(),
-            onVerifyMock.AsStdFunction()
-        ),
-        returnCode
+        action.apply(onRunMock.AsStdFunction(), onExitMock.AsStdFunction(), onMigrateMock.AsStdFunction()), returnCode
     );
 }

tests/unit/app/VerifyConfigTests.cpp

@@ -1,62 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of clio: https://github.com/XRPLF/clio
-    Copyright (c) 2025, the clio developers.
-
-    Permission to use, copy, modify, and distribute this software for any
-    purpose with or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL,  DIRECT,  INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#include "app/VerifyConfig.hpp"
-#include "util/TmpFile.hpp"
-#include "util/newconfig/FakeConfigData.hpp"
-
-#include <gtest/gtest.h>
-
-using namespace app;
-using namespace util::config;
-
-TEST(VerifyConfigTest, InvalidConfig)
-{
-    auto const tmpConfigFile = TmpFile(kJSON_DATA);
-
-    // false because json data(kJSON_DATA) is not compatible with current configDefintion
-    EXPECT_FALSE(verifyConfig(tmpConfigFile.path));
-}
-
-TEST(VerifyConfigTest, ValidConfig)
-{
-    auto const tmpConfigFile = TmpFile(kVALID_JSON_DATA);
-
-    // current example config should always be compatible with configDefinition
-    EXPECT_TRUE(verifyConfig(tmpConfigFile.path));
-}
-
-TEST(VerifyConfigTest, ConfigFileNotExist)
-{
-    EXPECT_FALSE(verifyConfig("doesn't exist Config File"));
-}
-
-TEST(VerifyConfigTest, InvalidJsonFile)
-{
-    // invalid json because extra "," after 51233
-    static constexpr auto kINVALID_JSON = R"({
-                                             "server": {
-                                                "ip": "0.0.0.0",
-                                                "port": 51233, 
-                                            }
-                                        })";
-    auto const tmpConfigFile = TmpFile(kINVALID_JSON);
-
-    EXPECT_FALSE(verifyConfig(tmpConfigFile.path));
-}