From 427ba477168adc5ae192ad7743cd79ceb7cdafee Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Fri, 7 Feb 2025 13:42:30 +0000 Subject: [PATCH 01/29] chore: Fix error in grafana dashboard example (#1878) --- docs/examples/infrastructure/README.md | 4 ++ .../grafana/clio_dashboard.json | 43 +++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/docs/examples/infrastructure/README.md b/docs/examples/infrastructure/README.md index bf215c99b..7e44d9aab 100644 --- a/docs/examples/infrastructure/README.md +++ b/docs/examples/infrastructure/README.md @@ -1,5 +1,9 @@ # Example of clio monitoring infrastructure +> [!WARNING] +> This is only an example of Grafana dashboard for Clio. It was created for demonstration purposes only and may contain errors. +> Clio team would not recommend to relate on data from this dashboard or use it for monitoring your Clio instances. + This directory contains an example of docker based infrastructure to collect and visualise metrics from clio. The structure of the directory: diff --git a/docs/examples/infrastructure/grafana/clio_dashboard.json b/docs/examples/infrastructure/grafana/clio_dashboard.json index 810653dbe..315b1a2d3 100644 --- a/docs/examples/infrastructure/grafana/clio_dashboard.json +++ b/docs/examples/infrastructure/grafana/clio_dashboard.json @@ -20,7 +20,6 @@ "graphTooltip": 0, "id": 1, "links": [], - "liveNow": false, "panels": [ { "datasource": { @@ -79,6 +78,7 @@ "graphMode": "area", "justifyMode": "auto", "orientation": "auto", + "percentChangeColorMode": "standard", "reduceOptions": { "calcs": [ "lastNotNull" @@ -90,7 +90,7 @@ "textMode": "auto", "wideLayout": true }, - "pluginVersion": "10.4.0", + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -159,6 +159,7 @@ "graphMode": "area", "justifyMode": "auto", "orientation": "auto", + "percentChangeColorMode": "standard", "reduceOptions": { "calcs": [ "lastNotNull" @@ -170,7 +171,7 @@ "textMode": "auto", "wideLayout": true }, - "pluginVersion": "10.4.0", + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -243,6 +244,7 @@ "graphMode": "area", "justifyMode": "auto", "orientation": "auto", + "percentChangeColorMode": "standard", "reduceOptions": { "calcs": [ "lastNotNull" @@ -254,7 +256,7 @@ "textMode": "auto", "wideLayout": true }, - "pluginVersion": "10.4.0", + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -327,6 +329,7 @@ "graphMode": "area", "justifyMode": "auto", "orientation": "auto", + "percentChangeColorMode": "standard", "reduceOptions": { "calcs": [ "lastNotNull" @@ -338,7 +341,7 @@ "textMode": "auto", "wideLayout": true }, - "pluginVersion": "10.4.0", + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -373,6 +376,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -435,6 +439,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -491,6 +496,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -552,6 +558,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -586,6 +593,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -647,6 +655,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -681,6 +690,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -742,6 +752,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -776,6 +787,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -837,6 +849,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -872,6 +885,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -934,6 +948,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -941,7 +956,7 @@ "uid": "PBFA97CFB590B2093" }, "editorMode": "code", - "expr": "rpc_method_duration_us{job=\"clio\"}", + "expr": "sum by (method) (increase(rpc_method_duration_us[$__interval]))\n / \n sum by (method,) (increase(rpc_method_total_number{status=\"finished\"}[$__interval]))", "instant": false, "legendFormat": "{{method}}", "range": true, @@ -968,6 +983,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -1029,6 +1045,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1063,6 +1080,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -1124,6 +1142,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1158,6 +1177,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 10, "gradientMode": "none", @@ -1223,7 +1243,7 @@ "sort": "none" } }, - "pluginVersion": "10.2.0", + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1296,6 +1316,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -1357,6 +1378,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1404,6 +1426,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -1465,6 +1488,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1510,6 +1534,7 @@ "axisLabel": "", "axisPlacement": "auto", "barAlignment": 0, + "barWidthFactor": 0.6, "drawStyle": "line", "fillOpacity": 0, "gradientMode": "none", @@ -1572,6 +1597,7 @@ "sort": "none" } }, + "pluginVersion": "11.4.0", "targets": [ { "datasource": { @@ -1590,8 +1616,9 @@ "type": "timeseries" } ], + "preload": false, "refresh": "5s", - "schemaVersion": 39, + "schemaVersion": 40, "tags": [], "templating": { "list": [] From f5e6c9576ef9bde060a6148397066b7be9e3fbf9 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 10 Feb 2025 16:20:25 +0000 Subject: [PATCH 02/29] feat: Run tests with sanitizers in CI (#1879) Fixes #1075 Fixes #1049 --- .github/actions/build_clio/action.yml | 6 + .github/actions/generate/action.yml | 20 +- .../actions/get_number_of_threads/action.yml | 15 +- .github/scripts/execute-tests-under-sanitizer | 45 ++++ .github/workflows/build.yml | 252 ++++++++---------- .github/workflows/clang-tidy.yml | 4 +- .github/workflows/nightly.yml | 86 ++---- .github/workflows/on_push_checks.yml | 129 +++++++++ .github/workflows/sanitizers.yml | 106 ++++++++ .gitignore | 1 + CMakeLists.txt | 16 +- cmake/Settings.cmake | 28 ++ cmake/deps/libbacktrace.cmake | 13 +- src/util/Assert.hpp | 12 + src/util/CMakeLists.txt | 6 +- src/util/TerminationHandler.cpp | 6 + 16 files changed, 526 insertions(+), 219 deletions(-) create mode 100755 .github/scripts/execute-tests-under-sanitizer create mode 100644 .github/workflows/on_push_checks.yml create mode 100644 .github/workflows/sanitizers.yml diff --git a/.github/actions/build_clio/action.yml b/.github/actions/build_clio/action.yml index 3316d2ed6..6e00dac6e 100644 --- a/.github/actions/build_clio/action.yml +++ b/.github/actions/build_clio/action.yml @@ -4,12 +4,18 @@ inputs: target: description: Build target name default: all + substract_threads: + description: An option for the action get_number_of_threads. See get_number_of_threads + required: true + default: '0' runs: using: composite steps: - name: Get number of threads uses: ./.github/actions/get_number_of_threads id: number_of_threads + with: + substract_threads: inputs.substract_threads - name: Build Clio shell: bash diff --git a/.github/actions/generate/action.yml b/.github/actions/generate/action.yml index 1dcfe0193..4e96a5eb1 100644 --- a/.github/actions/generate/action.yml +++ b/.github/actions/generate/action.yml @@ -12,6 +12,10 @@ inputs: description: Build type for third-party libraries and clio. Could be 'Release', 'Debug' required: true default: 'Release' + build_integration_tests: + description: Whether to build integration tests + required: true + default: 'true' code_coverage: description: Whether conan's coverage option should be on or not required: true @@ -20,6 +24,10 @@ inputs: description: Whether Clio is to be statically linked required: true default: 'false' + sanitizer: + description: Sanitizer to use + required: true + default: 'false' # false, tsan, asan or ubsan runs: using: composite steps: @@ -33,14 +41,20 @@ runs: BUILD_OPTION: "${{ inputs.conan_cache_hit == 'true' && 'missing' || '' }}" CODE_COVERAGE: "${{ inputs.code_coverage == 'true' && 'True' || 'False' }}" STATIC_OPTION: "${{ inputs.static == 'true' && 'True' || 'False' }}" + INTEGRATION_TESTS_OPTION: "${{ inputs.build_integration_tests == 'true' && 'True' || 'False' }}" run: | cd build - conan install .. -of . -b $BUILD_OPTION -s build_type=${{ inputs.build_type }} -o clio:static="${STATIC_OPTION}" -o clio:tests=True -o clio:integration_tests=True -o clio:lint=False -o clio:coverage="${CODE_COVERAGE}" --profile ${{ inputs.conan_profile }} - + conan install .. -of . -b $BUILD_OPTION -s build_type=${{ inputs.build_type }} -o clio:static="${STATIC_OPTION}" -o clio:tests=True -o clio:integration_tests="${INTEGRATION_TESTS_OPTION}" -o clio:lint=False -o clio:coverage="${CODE_COVERAGE}" --profile ${{ inputs.conan_profile }} + - name: Run cmake shell: bash env: BUILD_TYPE: "${{ inputs.build_type }}" + SANITIZER_OPTION: | + ${{ inputs.sanitizer == 'tsan' && '-Dsan=thread' || + inputs.sanitizer == 'ubsan' && '-Dsan=undefined' || + inputs.sanitizer == 'asan' && '-Dsan=address' || + '' }} run: | cd build - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} ${{ inputs.extra_cmake_args }} .. -G Ninja + cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} ${SANITIZER_OPTION} .. -G Ninja diff --git a/.github/actions/get_number_of_threads/action.yml b/.github/actions/get_number_of_threads/action.yml index 3af2e9f2b..b32735838 100644 --- a/.github/actions/get_number_of_threads/action.yml +++ b/.github/actions/get_number_of_threads/action.yml @@ -1,5 +1,10 @@ name: Get number of threads description: Determines number of threads to use on macOS and Linux +inputs: + substract_threads: + description: How many threads to substract from the calculated number + required: true + default: '0' outputs: threads_number: description: Number of threads to use @@ -19,8 +24,12 @@ runs: shell: bash run: echo "num=$(($(nproc) - 2))" >> $GITHUB_OUTPUT - - name: Export output variable - shell: bash + - name: Shift and export number of threads id: number_of_threads_export + shell: bash + env: + NUM_OF_THREADS: steps.mac_threads.outputs.num || steps.linux_threads.outputs.num + SHIFT_BY: inputs.substract_threads run: | - echo "num=${{ steps.mac_threads.outputs.num || steps.linux_threads.outputs.num }}" >> $GITHUB_OUTPUT + shifted=$((NUM_OF_THREADS - SHIFT_BY)) + echo "num=$(( shifted > 1 ? shifted : 1 ))" >> $GITHUB_OUTPUT diff --git a/.github/scripts/execute-tests-under-sanitizer b/.github/scripts/execute-tests-under-sanitizer new file mode 100755 index 000000000..4e1cc087e --- /dev/null +++ b/.github/scripts/execute-tests-under-sanitizer @@ -0,0 +1,45 @@ +#!/bin/bash + +set -o pipefail + +# Note: This script is intended to be run from the root of the repository. +# +# This script runs each unit-test separately and generates reports from the currently active sanitizer. +# Output is saved in ./.sanitizer-report in the root of the repository + +if [[ -z "$1" ]]; then + cat < /dev/null 2>&1 + + if [ $? -ne 0 ]; then + echo "'$TEST' failed a sanitizer check." + fi +done diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e200a3ad3..69b754a38 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,85 +1,82 @@ -name: Build +name: Reusable build on: - push: - branches: [master, release/*, develop] - pull_request: - branches: [master, release/*, develop] - workflow_dispatch: + workflow_call: + inputs: + runs_on: + description: Runner to run the job on + required: true + type: string + default: heavy + + container: + description: "The container object as a JSON string (leave empty to run natively)" + required: true + type: string + default: "" + + conan_profile: + description: Conan profile to use + required: true + type: string + + build_type: + description: Build type + required: true + type: string + + disable_cache: + description: Whether ccache and conan cache should be disabled + required: false + type: boolean + default: false + + code_coverage: + description: Whether to enable code coverage + required: true + type: boolean + default: false + + static: + description: Whether to build static binaries + required: true + type: boolean + default: true + + unit_tests: + description: Whether to run unit tests + required: true + type: boolean + default: false + + integration_tests: + description: Whether to run integration tests + required: true + type: boolean + default: false + + clio_server: + description: Whether to build clio_server + required: true + type: boolean + default: true + + target: + description: Build target name + required: false + type: string + default: all + + sanitizer: + description: Sanitizer to use + required: false + type: string + default: 'false' jobs: - check_format: - name: Check format - runs-on: ubuntu-20.04 - container: - image: rippleci/clio_ci:latest - steps: - - name: Fix git permissions on Linux - shell: bash - run: git config --global --add safe.directory $PWD - - - uses: actions/checkout@v4 - - name: Run formatters - id: run_formatters - run: | - ./.githooks/check-format --diff - shell: bash - - check_docs: - name: Check documentation - runs-on: ubuntu-20.04 - container: - image: rippleci/clio_ci:latest - steps: - - uses: actions/checkout@v4 - - name: Run linter - id: run_linter - run: | - ./.githooks/check-docs - shell: bash - build: - name: Build - needs: - - check_format - - check_docs - strategy: - fail-fast: false - matrix: - include: - - os: heavy - container: - image: rippleci/clio_ci:latest - build_type: Release - conan_profile: gcc - code_coverage: false - static: true - - os: heavy - container: - image: rippleci/clio_ci:latest - build_type: Debug - conan_profile: gcc - code_coverage: true - static: true - - os: heavy - container: - image: rippleci/clio_ci:latest - build_type: Release - conan_profile: clang - code_coverage: false - static: true - - os: heavy - container: - image: rippleci/clio_ci:latest - build_type: Debug - conan_profile: clang - code_coverage: false - static: true - - os: macos15 - build_type: Release - code_coverage: false - static: false - runs-on: [self-hosted, "${{ matrix.os }}"] - container: ${{ matrix.container }} + name: Build ${{ inputs.container != '' && 'in container' || 'natively' }} + runs-on: ${{ inputs.runs_on }} + container: ${{ inputs.container != '' && fromJson(inputs.container) || null }} steps: - name: Clean workdir @@ -93,37 +90,42 @@ jobs: - name: Prepare runner uses: ./.github/actions/prepare_runner with: - disable_ccache: false + disable_ccache: ${{ inputs.disable_cache }} - name: Setup conan uses: ./.github/actions/setup_conan id: conan with: - conan_profile: ${{ matrix.conan_profile }} + conan_profile: ${{ inputs.conan_profile }} - name: Restore cache + if: ${{ !inputs.disable_cache }} uses: ./.github/actions/restore_cache id: restore_cache with: conan_dir: ${{ env.CONAN_USER_HOME }}/.conan conan_profile: ${{ steps.conan.outputs.conan_profile }} ccache_dir: ${{ env.CCACHE_DIR }} - build_type: ${{ matrix.build_type }} - code_coverage: ${{ matrix.code_coverage }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} - name: Run conan and cmake uses: ./.github/actions/generate with: conan_profile: ${{ steps.conan.outputs.conan_profile }} - conan_cache_hit: ${{ steps.restore_cache.outputs.conan_cache_hit }} - build_type: ${{ matrix.build_type }} - code_coverage: ${{ matrix.code_coverage }} - static: ${{ matrix.static }} + conan_cache_hit: ${{ !inputs.disable_cache && steps.restore_cache.outputs.conan_cache_hit }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} + static: ${{ inputs.static }} + sanitizer: ${{ inputs.sanitizer }} - name: Build Clio uses: ./.github/actions/build_clio + with: + target: ${{ inputs.target }} - name: Show ccache's statistics + if: ${{ !inputs.disable_cache }} shell: bash id: ccache_stats run: | @@ -132,24 +134,37 @@ jobs: echo "miss_rate=${miss_rate}" >> $GITHUB_OUTPUT cat /tmp/ccache.stats - - name: Strip tests - if: ${{ !matrix.code_coverage }} - run: strip build/clio_tests && strip build/clio_integration_tests + - name: Strip unit_tests + if: ${{ inputs.unit_tests && !inputs.code_coverage && inputs.sanitizer == 'false' }} + run: strip build/clio_tests + + - name: Strip integration_tests + if: ${{ inputs.integration_tests && !inputs.code_coverage }} + run: strip build/clio_integration_tests - name: Upload clio_server + if: ${{ inputs.clio_server }} uses: actions/upload-artifact@v4 with: - name: clio_server_${{ runner.os }}_${{ matrix.build_type }}_${{ steps.conan.outputs.conan_profile }} + name: clio_server_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} path: build/clio_server - + - name: Upload clio_tests - if: ${{ !matrix.code_coverage }} + if: ${{ inputs.unit_tests && !inputs.code_coverage }} uses: actions/upload-artifact@v4 with: - name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ steps.conan.outputs.conan_profile }} - path: build/clio_*tests + name: clio_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} + path: build/clio_tests + + - name: Upload clio_integration_tests + if: ${{ inputs.integration_tests && !inputs.code_coverage }} + uses: actions/upload-artifact@v4 + with: + name: clio_integration_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} + path: build/clio_integration_tests - name: Save cache + if: ${{ !inputs.disable_cache }} uses: ./.github/actions/save_cache with: conan_dir: ${{ env.CONAN_USER_HOME }}/.conan @@ -158,61 +173,20 @@ jobs: ccache_dir: ${{ env.CCACHE_DIR }} ccache_cache_hit: ${{ steps.restore_cache.outputs.ccache_cache_hit }} ccache_cache_miss_rate: ${{ steps.ccache_stats.outputs.miss_rate }} - build_type: ${{ matrix.build_type }} - code_coverage: ${{ matrix.code_coverage }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} conan_profile: ${{ steps.conan.outputs.conan_profile }} # TODO: This is not a part of build process but it is the easiest way to do it here. # It will be refactored in https://github.com/XRPLF/clio/issues/1075 - name: Run code coverage - if: ${{ matrix.code_coverage }} + if: ${{ inputs.code_coverage }} uses: ./.github/actions/code_coverage upload_coverage_report: + if: ${{ inputs.code_coverage }} name: Codecov needs: build uses: ./.github/workflows/upload_coverage_report.yml secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - - test: - name: Run Tests - needs: build - strategy: - fail-fast: false - matrix: - include: - - os: heavy - container: - image: rippleci/clio_ci:latest - conan_profile: gcc - build_type: Release - - os: heavy - container: - image: rippleci/clio_ci:latest - conan_profile: clang - build_type: Release - - os: heavy - container: - image: rippleci/clio_ci:latest - conan_profile: clang - build_type: Debug - - os: macos15 - conan_profile: apple_clang_16 - build_type: Release - runs-on: [self-hosted, "${{ matrix.os }}"] - container: ${{ matrix.container }} - - steps: - - name: Clean workdir - if: ${{ runner.os == 'macOS' }} - uses: kuznetsss/workspace-cleanup@1.0 - - - uses: actions/download-artifact@v4 - with: - name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} - - - name: Run clio_tests - run: | - chmod +x ./clio_tests - ./clio_tests diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 54fe48dbb..e251aff1d 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -1,7 +1,7 @@ name: Clang-tidy check on: schedule: - - cron: "0 6 * * 1-5" + - cron: "0 9 * * 1-5" workflow_dispatch: pull_request: branches: [develop] @@ -12,7 +12,7 @@ on: jobs: clang_tidy: - runs-on: [self-hosted, Linux] + runs-on: heavy container: image: rippleci/clio_ci:latest permissions: diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 080df1760..a5616d0f1 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -1,7 +1,7 @@ name: Nightly release on: schedule: - - cron: '0 5 * * 1-5' + - cron: '0 8 * * 1-5' workflow_dispatch: pull_request: paths: @@ -21,68 +21,23 @@ jobs: - os: heavy build_type: Release static: true - container: - image: rippleci/clio_ci:latest + container: '{ "image": "rippleci/clio_ci:latest" }' - os: heavy build_type: Debug static: true - container: - image: rippleci/clio_ci:latest - runs-on: [self-hosted, "${{ matrix.os }}"] - container: ${{ matrix.container }} - - steps: - - name: Clean workdir - if: ${{ runner.os == 'macOS' }} - uses: kuznetsss/workspace-cleanup@1.0 - - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Prepare runner - uses: ./.github/actions/prepare_runner - with: - disable_ccache: true - - - name: Setup conan - uses: ./.github/actions/setup_conan - id: conan - with: - conan_profile: gcc - - - name: Run conan and cmake - uses: ./.github/actions/generate - with: - conan_profile: ${{ steps.conan.outputs.conan_profile }} - conan_cache_hit: ${{ steps.restore_cache.outputs.conan_cache_hit }} - build_type: ${{ matrix.build_type }} - static: ${{ matrix.static }} - - - name: Build Clio - uses: ./.github/actions/build_clio - - - name: Strip tests - run: strip build/clio_tests && strip build/clio_integration_tests - - - name: Upload clio_tests - uses: actions/upload-artifact@v4 - with: - name: clio_tests_${{ runner.os }}_${{ matrix.build_type }} - path: build/clio_*tests - - - name: Compress clio_server - shell: bash - run: | - cd build - tar czf ./clio_server_${{ runner.os }}_${{ matrix.build_type }}.tar.gz ./clio_server - - - name: Upload clio_server - uses: actions/upload-artifact@v4 - with: - name: clio_server_${{ runner.os }}_${{ matrix.build_type }} - path: build/clio_server_${{ runner.os }}_${{ matrix.build_type }}.tar.gz - + container: '{ "image": "rippleci/clio_ci:latest" }' + uses: ./.github/workflows/build.yml + with: + runs_on: ${{ matrix.os }} + container: ${{ matrix.container }} + conan_profile: gcc + build_type: ${{ matrix.build_type }} + code_coverage: false + static: ${{ matrix.static }} + unit_tests: true + integration_tests: true + clio_server: true + disable_cache: true run_tests: needs: build @@ -91,14 +46,17 @@ jobs: matrix: include: - os: macos15 + conan_profile: apple_clang_16 build_type: Release integration_tests: false - os: heavy + conan_profile: gcc build_type: Release container: image: rippleci/clio_ci:latest integration_tests: true - os: heavy + conan_profile: gcc build_type: Debug container: image: rippleci/clio_ci:latest @@ -122,13 +80,17 @@ jobs: - uses: actions/download-artifact@v4 with: - name: clio_tests_${{ runner.os }}_${{ matrix.build_type }} + name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} - name: Run clio_tests run: | chmod +x ./clio_tests ./clio_tests + - uses: actions/download-artifact@v4 + with: + name: clio_integration_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} + # To be enabled back once docker in mac runner arrives # https://github.com/XRPLF/clio/issues/1400 - name: Run clio_integration_tests @@ -189,7 +151,7 @@ jobs: tags: | type=raw,value=nightly type=raw,value=${{ github.sha }} - artifact_name: clio_server_Linux_Release + artifact_name: clio_server_Linux_Release_gcc strip_binary: true publish_image: ${{ github.event_name != 'pull_request' }} diff --git a/.github/workflows/on_push_checks.yml b/.github/workflows/on_push_checks.yml new file mode 100644 index 000000000..9e1f4ebb4 --- /dev/null +++ b/.github/workflows/on_push_checks.yml @@ -0,0 +1,129 @@ +name: Check codebase +on: + push: + branches: [master, release/*, develop] + pull_request: + branches: [master, release/*, develop] + workflow_dispatch: + +jobs: + check_format: + name: Check format + runs-on: ubuntu-20.04 + container: + image: rippleci/clio_ci:latest + steps: + - name: Fix git permissions on Linux + shell: bash + run: git config --global --add safe.directory $PWD + + - uses: actions/checkout@v4 + - name: Run formatters + id: run_formatters + run: | + ./.githooks/check-format --diff + shell: bash + + check_docs: + name: Check documentation + runs-on: ubuntu-20.04 + container: + image: rippleci/clio_ci:latest + steps: + - uses: actions/checkout@v4 + - name: Run linter + id: run_linter + run: | + ./.githooks/check-docs + shell: bash + + build: + name: Build + needs: + - check_format + - check_docs + strategy: + fail-fast: false + matrix: + include: + - os: heavy + conan_profile: gcc + build_type: Release + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: heavy + conan_profile: gcc + build_type: Debug + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: true + static: true + - os: heavy + conan_profile: clang + build_type: Release + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: heavy + conan_profile: clang + build_type: Debug + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: macos15 + build_type: Release + code_coverage: false + static: false + uses: ./.github/workflows/build.yml + with: + runs_on: ${{ matrix.os }} + container: ${{ matrix.container }} + conan_profile: ${{ matrix.conan_profile }} + build_type: ${{ matrix.build_type }} + code_coverage: ${{ matrix.code_coverage }} + static: ${{ matrix.static }} + unit_tests: true + integration_tests: true + clio_server: true + + test: + name: Run Tests + needs: build + strategy: + fail-fast: false + matrix: + include: + - os: heavy + conan_profile: gcc + build_type: Release + container: + image: rippleci/clio_ci:latest + - os: heavy + conan_profile: clang + build_type: Release + container: + image: rippleci/clio_ci:latest + - os: heavy + conan_profile: clang + build_type: Debug + container: + image: rippleci/clio_ci:latest + - os: macos15 + conan_profile: apple_clang_16 + build_type: Release + runs-on: ${{ matrix.os }} + container: ${{ matrix.container }} + + steps: + - name: Clean workdir + if: ${{ runner.os == 'macOS' }} + uses: kuznetsss/workspace-cleanup@1.0 + + - uses: actions/download-artifact@v4 + with: + name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} + + - name: Run clio_tests + run: | + chmod +x ./clio_tests + ./clio_tests diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml new file mode 100644 index 000000000..0e573bc34 --- /dev/null +++ b/.github/workflows/sanitizers.yml @@ -0,0 +1,106 @@ +name: Run tests with sanitizers +on: + schedule: + - cron: "0 4 * * 1-5" + workflow_dispatch: + pull_request: + paths: + - '.github/workflows/sanitizers.yml' + +jobs: + build: + name: Build clio tests + strategy: + fail-fast: false + matrix: + include: + - sanitizer: tsan + compiler: gcc + - sanitizer: asan + compiler: gcc + # - sanitizer: ubsan # todo: enable when heavy runners are available + # compiler: gcc + uses: ./.github/workflows/build.yml + with: + runs_on: ubuntu-latest # todo: change to heavy + container: '{ "image": "rippleci/clio_ci:latest" }' + disable_cache: true + conan_profile: ${{ matrix.compiler }}.${{ matrix.sanitizer }} + build_type: Release + code_coverage: false + static: false + unit_tests: true + integration_tests: false + clio_server: false + target: clio_tests + sanitizer: ${{ matrix.sanitizer }} + + # consider combining this with the previous matrix instead + run_tests: + needs: build + strategy: + fail-fast: false + matrix: + include: + - sanitizer: tsan + compiler: gcc + - sanitizer: asan + compiler: gcc + # - sanitizer: ubsan # todo: enable when heavy runners are available + # compiler: gcc + runs-on: ubuntu-latest # todo: change to heavy + container: + image: rippleci/clio_ci:latest + permissions: + contents: write + issues: write + + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/download-artifact@v4 + with: + name: clio_tests_${{ runner.os }}_Release_${{ matrix.compiler }}.${{ matrix.sanitizer }} + + - name: Run clio_tests [${{ matrix.compiler }} / ${{ matrix.sanitizer }}] + shell: bash + run: | + chmod +x ./clio_tests + ./.github/scripts/execute-tests-under-sanitizer ./clio_tests + + - name: Check for sanitizer report + shell: bash + id: check_report + run: | + if ls .sanitizer-report/* 1> /dev/null 2>&1; then + echo "found_report=true" >> $GITHUB_OUTPUT + else + echo "found_report=false" >> $GITHUB_OUTPUT + fi + + - name: Upload report + if: ${{ steps.check_report.outputs.found_report == 'true' }} + uses: actions/upload-artifact@v4 + with: + name: ${{ matrix.compiler }}_${{ matrix.sanitizer }}_report + path: .sanitizer-report/* + include-hidden-files: true + + # + # todo: enable when we have fixed all currently existing issues from sanitizers + # + # - name: Create an issue + # if: ${{ steps.check_report.outputs.found_report == 'true' }} + # uses: ./.github/actions/create_issue + # env: + # GH_TOKEN: ${{ github.token }} + # with: + # labels: 'bug' + # title: '[${{ matrix.sanitizer }}/${{ matrix.compiler }}] reported issues' + # body: > + # Clio tests failed one or more sanitizer checks when built with ${{ matrix.compiler }}`. + + # Workflow: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}/ + # Reports are available as artifacts. diff --git a/.gitignore b/.gitignore index 2f3da47ef..f9f86f4ac 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ .vscode .python-version .DS_Store +.sanitizer-report CMakeUserPresets.json config.json src/util/build/Build.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 14136cda5..c31647f1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -65,15 +65,21 @@ endif () # Enable selected sanitizer if enabled via `san` if (san) + set(SUPPORTED_SANITIZERS "address" "thread" "memory" "undefined") + list(FIND SUPPORTED_SANITIZERS "${san}" INDEX) + if (INDEX EQUAL -1) + message(FATAL_ERROR "Error: Unsupported sanitizer '${san}'. Supported values are: ${SUPPORTED_SANITIZERS}.") + endif () + target_compile_options( - clio PUBLIC # Sanitizers recommend minimum of -O1 for reasonable performance - $<$:-O1> ${SAN_FLAG} -fno-omit-frame-pointer + clio_options INTERFACE # Sanitizers recommend minimum of -O1 for reasonable performance + $<$:-O1> ${SAN_FLAG} -fno-omit-frame-pointer ) target_compile_definitions( - clio PUBLIC $<$:SANITIZER=ASAN> $<$:SANITIZER=TSAN> - $<$:SANITIZER=MSAN> $<$:SANITIZER=UBSAN> + clio_options INTERFACE $<$:SANITIZER=ASAN> $<$:SANITIZER=TSAN> + $<$:SANITIZER=MSAN> $<$:SANITIZER=UBSAN> ) - target_link_libraries(clio INTERFACE ${SAN_FLAG} ${SAN_LIB}) + target_link_libraries(clio_options INTERFACE ${SAN_FLAG} ${SAN_LIB}) endif () # Generate `docs` target for doxygen documentation if enabled Note: use `make docs` to generate the documentation diff --git a/cmake/Settings.cmake b/cmake/Settings.cmake index e469b0920..6ed745272 100644 --- a/cmake/Settings.cmake +++ b/cmake/Settings.cmake @@ -39,6 +39,34 @@ if (is_appleclang) list(APPEND COMPILER_FLAGS -Wreorder-init-list) endif () +if (san) + # When building with sanitizers some compilers will actually produce extra warnings/errors. We don't want this yet, at + # least not until we have fixed all runtime issues reported by the sanitizers. Once that is done we can start removing + # some of these and trying to fix it in our codebase. We can never remove all of below because most of them are + # reported from deep inside libraries like boost or libxrpl. + # + # TODO: Address in https://github.com/XRPLF/clio/issues/1885 + list( + APPEND + COMPILER_FLAGS + -Wno-error=tsan # Disables treating TSAN warnings as errors + -Wno-tsan # Disables TSAN warnings (thread-safety analysis) + -Wno-uninitialized # Disables warnings about uninitialized variables (AddressSanitizer, UndefinedBehaviorSanitizer, + # etc.) + -Wno-stringop-overflow # Disables warnings about potential string operation overflows (AddressSanitizer) + -Wno-unsafe-buffer-usage # Disables warnings about unsafe memory operations (AddressSanitizer) + -Wno-frame-larger-than # Disables warnings about stack frame size being too large (AddressSanitizer) + -Wno-unused-function # Disables warnings about unused functions (LeakSanitizer, memory-related issues) + -Wno-unused-but-set-variable # Disables warnings about unused variables (MemorySanitizer) + -Wno-thread-safety-analysis # Disables warnings related to thread safety usage (ThreadSanitizer) + -Wno-thread-safety # Disables warnings related to thread safety usage (ThreadSanitizer) + -Wno-sign-compare # Disables warnings about signed/unsigned comparison (UndefinedBehaviorSanitizer) + -Wno-nonnull # Disables warnings related to null pointer dereferencing (UndefinedBehaviorSanitizer) + -Wno-address # Disables warnings about address-related issues (UndefinedBehaviorSanitizer) + -Wno-array-bounds # Disables array bounds checks (UndefinedBehaviorSanitizer) + ) +endif () + # See https://github.com/cpp-best-practices/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang for # the flags description diff --git a/cmake/deps/libbacktrace.cmake b/cmake/deps/libbacktrace.cmake index d4babd8a1..ddd6c4c9e 100644 --- a/cmake/deps/libbacktrace.cmake +++ b/cmake/deps/libbacktrace.cmake @@ -1,3 +1,10 @@ -target_compile_definitions(clio_options INTERFACE BOOST_STACKTRACE_LINK) -target_compile_definitions(clio_options INTERFACE BOOST_STACKTRACE_USE_BACKTRACE) -find_package(libbacktrace REQUIRED CONFIG) +if (Boost_STACKTRACE_FOUND) + target_compile_definitions(clio_options INTERFACE BOOST_STACKTRACE_LINK) + target_compile_definitions(clio_options INTERFACE BOOST_STACKTRACE_USE_BACKTRACE) + find_package(libbacktrace REQUIRED CONFIG) +else () + # Some sanitizers (TSAN and ASAN for sure) can't be used with libbacktrace because they have their own backtracing + # capabilities and there are conflicts. In any case, this makes sure Clio code knows that backtrace is not available. + # See relevant conan profiles for sanitizers where we disable stacktrace in Boost explicitly. + target_compile_definitions(clio_options INTERFACE CLIO_WITHOUT_STACKTRACE) +endif () diff --git a/src/util/Assert.hpp b/src/util/Assert.hpp index 47bf2f530..ad19ea6fd 100644 --- a/src/util/Assert.hpp +++ b/src/util/Assert.hpp @@ -23,8 +23,10 @@ #include "util/log/Logger.hpp" #include +#ifndef CLIO_WITHOUT_STACKTRACE #include #include +#endif // CLIO_WITHOUT_STACKTRACE #include #include @@ -55,6 +57,7 @@ assertImpl( ) { if (!condition) { +#ifndef CLIO_WITHOUT_STACKTRACE auto const resultMessage = fmt::format( "Assertion '{}' failed at {}:{}:\n{}\nStacktrace:\n{}", expression, @@ -63,6 +66,15 @@ assertImpl( fmt::format(format, std::forward(args)...), boost::stacktrace::to_string(boost::stacktrace::stacktrace()) ); +#else + auto const resultMessage = fmt::format( + "Assertion '{}' failed at {}:{}:\n{}", + expression, + location.file_name(), + location.line(), + fmt::format(format, std::forward(args)...) + ); +#endif if (boost::log::core::get()->get_logging_enabled()) { LOG(LogService::fatal()) << resultMessage; } else { diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 32e792e80..dba29a36b 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -40,9 +40,7 @@ target_sources( target_link_libraries( clio_util PUBLIC Boost::headers - Boost::stacktrace_backtrace dl - libbacktrace::libbacktrace fmt::fmt openssl::openssl xrpl::libxrpl @@ -50,6 +48,10 @@ target_link_libraries( clio_options ) +if (!san) + target_link_libraries(clio_util PUBLIC Boost::stacktrace_backtrace libbacktrace::libbacktrace) +endif () + # FIXME: needed on gcc-12, clang-16 and AppleClang for now (known boost 1.82 issue for some compilers) # # For some reason cmake doesn't propagate the compile definitions from clio_options so we need to add them here diff --git a/src/util/TerminationHandler.cpp b/src/util/TerminationHandler.cpp index e2f886412..9343841ff 100644 --- a/src/util/TerminationHandler.cpp +++ b/src/util/TerminationHandler.cpp @@ -21,7 +21,9 @@ #include "util/log/Logger.hpp" +#ifndef CLIO_WITHOUT_STACKTRACE #include +#endif // CLIO_WITHOUT_STACKTRACE #include #include @@ -33,11 +35,15 @@ namespace { void terminationHandler() { +#ifndef CLIO_WITHOUT_STACKTRACE try { LOG(LogService::fatal()) << "Exit on terminate. Backtrace:\n" << boost::stacktrace::stacktrace(); } catch (...) { LOG(LogService::fatal()) << "Exit on terminate. Can't get backtrace."; } +#else + LOG(LogService::fatal()) << "Exit on terminate. Stacktrace disabled."; +#endif // CLIO_WITHOUT_STACKTRACE std::abort(); } From cd6289b79ad05483c621e51262083b5223375191 Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:29:00 -0500 Subject: [PATCH 03/29] feat: Generate config descriptions (#1842) Fixes #1460 --- src/app/CliArgs.cpp | 22 ++- src/util/CMakeLists.txt | 1 + src/util/newconfig/Array.hpp | 15 ++ src/util/newconfig/ConfigConstraints.hpp | 78 ++++++++++ src/util/newconfig/ConfigDefinition.cpp | 1 + src/util/newconfig/ConfigDefinition.hpp | 27 ---- src/util/newconfig/ConfigDescription.hpp | 144 ++++++++++++++---- src/util/newconfig/ConfigValue.hpp | 26 +++- src/util/newconfig/Types.cpp | 66 ++++++++ src/util/newconfig/Types.hpp | 23 ++- src/util/newconfig/ValueView.hpp | 27 ++++ tests/common/util/TmpFile.hpp | 6 + tests/unit/app/CliArgsTests.cpp | 75 +++++++++ .../newconfig/ClioConfigDefinitionTests.cpp | 5 +- 14 files changed, 454 insertions(+), 62 deletions(-) create mode 100644 src/util/newconfig/Types.cpp diff --git a/src/app/CliArgs.cpp b/src/app/CliArgs.cpp index 4673c9f90..2cfa04b2f 100644 --- a/src/app/CliArgs.cpp +++ b/src/app/CliArgs.cpp @@ -21,6 +21,7 @@ #include "migration/MigrationApplication.hpp" #include "util/build/Build.hpp" +#include "util/newconfig/ConfigDescription.hpp" #include #include @@ -29,6 +30,7 @@ #include #include +#include #include #include #include @@ -42,12 +44,13 @@ CliArgs::parse(int argc, char const* argv[]) // clang-format off po::options_description description("Options"); description.add_options() - ("help,h", "print help message and exit") - ("version,v", "print version and exit") - ("conf,c", po::value()->default_value(kDEFAULT_CONFIG_PATH), "configuration file") + ("help,h", "Print help message and exit") + ("version,v", "Print version and exit") + ("conf,c", po::value()->default_value(kDEFAULT_CONFIG_PATH), "Configuration file") ("ng-web-server,w", "Use ng-web-server") - ("migrate", po::value(), "start migration helper") + ("migrate", po::value(), "Start migration helper") ("verify", "Checks the validity of config values") + ("config-description,d", po::value(), "Generate config description markdown file") ; // clang-format on po::positional_options_description positional; @@ -67,6 +70,17 @@ CliArgs::parse(int argc, char const* argv[]) return Action{Action::Exit{EXIT_SUCCESS}}; } + if (parsed.count("config-description") != 0u) { + std::filesystem::path filePath = parsed["config-description"].as(); + + auto const res = util::config::ClioConfigDescription::generateConfigDescriptionToFile(filePath); + if (res.has_value()) + return Action{Action::Exit{EXIT_SUCCESS}}; + + std::cerr << res.error().error << std::endl; + return Action{Action::Exit{EXIT_FAILURE}}; + } + auto configPath = parsed["conf"].as(); if (parsed.count("migrate") != 0u) { diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index dba29a36b..af17600cb 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -34,6 +34,7 @@ target_sources( newconfig/ConfigDefinition.cpp newconfig/ConfigFileJson.cpp newconfig/ObjectView.cpp + newconfig/Types.cpp newconfig/ValueView.cpp ) diff --git a/src/util/newconfig/Array.hpp b/src/util/newconfig/Array.hpp index a6ccbe958..672757655 100644 --- a/src/util/newconfig/Array.hpp +++ b/src/util/newconfig/Array.hpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -103,4 +104,18 @@ private: std::vector elements_; }; +/** + * @brief Custom output stream for Array + * + * @param stream The output stream + * @param arr The Array + * @return The same ostream we were given + */ +inline std::ostream& +operator<<(std::ostream& stream, Array arr) +{ + stream << arr.getArrayPattern(); + return stream; +} + } // namespace util::config diff --git a/src/util/newconfig/ConfigConstraints.hpp b/src/util/newconfig/ConfigConstraints.hpp index 315ea05f7..910dff1e5 100644 --- a/src/util/newconfig/ConfigConstraints.hpp +++ b/src/util/newconfig/ConfigConstraints.hpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -149,6 +150,28 @@ protected: */ virtual std::optional checkValueImpl(Value const& val) const = 0; + + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + virtual void + print(std::ostream& stream) const = 0; + + /** + * @brief Custom output stream for constraint + * + * @param stream The output stream + * @param cons The constraint + * @return The same ostream we were given + */ + friend std::ostream& + operator<<(std::ostream& stream, Constraint const& cons) + { + cons.print(stream); + return stream; + } }; /** @@ -177,6 +200,17 @@ private: [[nodiscard]] std::optional checkValueImpl(Value const& port) const override; + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + void + print(std::ostream& stream) const override + { + stream << fmt::format("The minimum value is `{}`. The maximum value is `{}", kPORT_MIN, kPORT_MAX); + } + static constexpr uint32_t kPORT_MIN = 1; static constexpr uint32_t kPORT_MAX = 65535; }; @@ -206,6 +240,17 @@ private: */ [[nodiscard]] std::optional checkValueImpl(Value const& ip) const override; + + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + void + print(std::ostream& stream) const override + { + stream << "The value must be a valid IP address"; + } }; /** @@ -260,6 +305,17 @@ private: return Error{makeErrorMsg(key_, val, arr_)}; } + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + void + print(std::ostream& stream) const override + { + stream << fmt::format("The value must be one of the following: `{}`", fmt::join(arr_, ", ")); + } + std::string_view key_; std::array arr_; }; @@ -312,6 +368,17 @@ private: return Error{fmt::format("Number must be between {} and {}", min_, max_)}; } + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + void + print(std::ostream& stream) const override + { + stream << fmt::format("The minimum value is `{}`. The maximum value is `{}`", min_, max_); + } + NumType min_; NumType max_; }; @@ -341,6 +408,17 @@ private: */ [[nodiscard]] std::optional checkValueImpl(Value const& num) const override; + + /** + * @brief Prints to the output stream for this specific constraint. + * + * @param stream The output stream + */ + void + print(std::ostream& stream) const override + { + stream << fmt::format("The value must be a positive double number"); + } }; static constinit PortConstraint gValidatePort{}; diff --git a/src/util/newconfig/ConfigDefinition.cpp b/src/util/newconfig/ConfigDefinition.cpp index bad33717e..bbfff1d94 100644 --- a/src/util/newconfig/ConfigDefinition.cpp +++ b/src/util/newconfig/ConfigDefinition.cpp @@ -65,6 +65,7 @@ ClioConfigDefinition::getObject(std::string_view prefix, std::optional(mapVal)) { ASSERT(std::get(mapVal).size() > idx.value(), "Index provided is out of scope"); + // we want to support getObject("array") and getObject("array.[]"), so we check if "[]" exists if (!prefix.contains("[]")) return ObjectView{prefixWithDot + "[]", idx.value(), *this}; diff --git a/src/util/newconfig/ConfigDefinition.hpp b/src/util/newconfig/ConfigDefinition.hpp index 94a70b671..b3c5761a4 100644 --- a/src/util/newconfig/ConfigDefinition.hpp +++ b/src/util/newconfig/ConfigDefinition.hpp @@ -23,7 +23,6 @@ #include "util/Assert.hpp" #include "util/newconfig/Array.hpp" #include "util/newconfig/ConfigConstraints.hpp" -#include "util/newconfig/ConfigDescription.hpp" #include "util/newconfig/ConfigFileInterface.hpp" #include "util/newconfig/ConfigValue.hpp" #include "util/newconfig/Error.hpp" @@ -31,16 +30,10 @@ #include "util/newconfig/Types.hpp" #include "util/newconfig/ValueView.hpp" -#include -#include -#include - #include -#include #include #include #include -#include #include #include #include @@ -84,26 +77,6 @@ public: [[nodiscard]] std::optional> parse(ConfigFileInterface const& config); - /** - * @brief Validates the configuration file - * - * Should only check for valid values, without populating - * - * @param config The configuration file interface - * @return An optional vector of Error objects stating all the failures if validation fails - */ - [[nodiscard]] std::optional> - validate(ConfigFileInterface const& config) const; - - /** - * @brief Generate markdown file of all the clio config descriptions - * - * @param configDescription The configuration description object - * @return An optional Error if generating markdown fails - */ - [[nodiscard]] std::expected - getMarkdown(ClioConfigDescription const& configDescription) const; - /** * @brief Returns the ObjectView specified with the prefix * diff --git a/src/util/newconfig/ConfigDescription.hpp b/src/util/newconfig/ConfigDescription.hpp index 71335a66c..52fbc6d06 100644 --- a/src/util/newconfig/ConfigDescription.hpp +++ b/src/util/newconfig/ConfigDescription.hpp @@ -20,9 +20,19 @@ #pragma once #include "util/Assert.hpp" +#include "util/newconfig/ConfigDefinition.hpp" +#include "util/newconfig/Error.hpp" + +#include #include #include +#include +#include +#include +#include +#include +#include #include namespace util::config { @@ -61,26 +71,90 @@ public: return itr->value; } + /** + * @brief Generate markdown file of all the clio config descriptions + * + * @param path The path location to generate the Config-description file + * @return An Error if generating markdown fails, otherwise nothing + */ + [[nodiscard]] static std::expected + generateConfigDescriptionToFile(std::filesystem::path path) + { + namespace fs = std::filesystem; + + // Validate the directory exists + auto const dir = path.parent_path(); + if (!dir.empty() && !fs::exists(dir)) { + return std::unexpected{ + fmt::format("Error: Directory '{}' does not exist or provided path is invalid", dir.string()) + }; + } + + std::ofstream file(path.string()); + if (!file.is_open()) { + return std::unexpected{fmt::format("Failed to create file '{}': {}", path.string(), std::strerror(errno))}; + } + + writeConfigDescriptionToFile(file); + file.close(); + + std::cout << "Markdown file generated successfully: " << path << "\n"; + return {}; + } + + /** + * @brief Writes to Config description to file + * + * @param file The config file to write to + */ + static void + writeConfigDescriptionToFile(std::ostream& file) + { + file << "# Clio Config Description\n"; + file << "This file lists all Clio Configuration definitions in detail.\n\n"; + file << "## Configuration Details\n\n"; + + for (auto const& [key, val] : kCONFIG_DESCRIPTION) { + file << "### Key: " << key << "\n"; + + // Every type of value is directed to operator<< in ConfigValue.hpp + // as ConfigValue is the one that holds all the info regarding the config values + if (key.contains("[]")) { + file << gClioConfig.asArray(key); + } else { + file << gClioConfig.getValueView(key); + } + file << " - **Description**: " << val << "\n"; + } + file << "\n"; + } + private: static constexpr auto kCONFIG_DESCRIPTION = std::array{ - KV{.key = "database.type", .value = "Type of database to use. Default is Scylladb."}, + KV{.key = "database.type", + .value = "Type of database to use. We currently support Cassandra and Scylladb. We default to Scylladb."}, KV{.key = "database.cassandra.contact_points", - .value = - "A list of IP addresses or hostnames of the initial nodes (Cassandra/Scylladb cluster nodes) that the " - "client will connect to when establishing a connection with the database."}, + .value = "A list of IP addresses or hostnames of the initial nodes (Cassandra/Scylladb cluster nodes) that " + "the client will connect to when establishing a connection with the database. If you're running " + "locally, it should be 'localhost' or 127.0.0.1"}, KV{.key = "database.cassandra.secure_connect_bundle", .value = "Configuration file that contains the necessary security credentials and connection details for " "securely " "connecting to a Cassandra database cluster."}, - KV{.key = "database.cassandra.port", .value = "Port number to connect to Cassandra."}, - KV{.key = "database.cassandra.keyspace", .value = "Keyspace to use in Cassandra."}, - KV{.key = "database.cassandra.replication_factor", .value = "Number of replicated nodes for Scylladb."}, - KV{.key = "database.cassandra.table_prefix", .value = "Prefix for Cassandra table names."}, + KV{.key = "database.cassandra.port", .value = "Port number to connect to the database."}, + KV{.key = "database.cassandra.keyspace", .value = "Keyspace to use for the database."}, + KV{.key = "database.cassandra.replication_factor", + .value = "Number of replicated nodes for Scylladb. Visit this link for more details : " + "https://university.scylladb.com/courses/scylla-essentials-overview/lessons/high-availability/" + "topic/fault-tolerance-replication-factor/ "}, + KV{.key = "database.cassandra.table_prefix", .value = "Prefix for Database table names."}, KV{.key = "database.cassandra.max_write_requests_outstanding", - .value = "Maximum number of outstanding write requests."}, + .value = "Maximum number of outstanding write requests. Write requests are api calls that write to database " + }, KV{.key = "database.cassandra.max_read_requests_outstanding", - .value = "Maximum number of outstanding read requests."}, - KV{.key = "database.cassandra.threads", .value = "Number of threads for Cassandra operations."}, + .value = "Maximum number of outstanding read requests, which reads from database"}, + KV{.key = "database.cassandra.threads", .value = "Number of threads that will be used for database operations." + }, KV{.key = "database.cassandra.core_connections_per_host", .value = "Number of core connections per host for Cassandra."}, KV{.key = "database.cassandra.queue_size_io", .value = "Queue size for I/O operations in Cassandra."}, @@ -106,10 +180,10 @@ private: .value = "Timeout duration for the forwarding cache used in Rippled communication."}, KV{.key = "forwarding.request_timeout", .value = "Timeout duration for the forwarding request used in Rippled communication."}, - KV{.key = "rpc.cache_timeout", .value = "Timeout duration for the rpc request."}, + KV{.key = "rpc.cache_timeout", .value = "Timeout duration for RPC requests."}, KV{.key = "num_markers", - .value = "The number of markers is the number of coroutines to load the cache concurrently."}, - KV{.key = "dos_guard.[].whitelist", .value = "List of IP addresses to whitelist for DOS protection."}, + .value = "The number of markers is the number of coroutines to download the initial ledger"}, + KV{.key = "dos_guard.whitelist.[]", .value = "List of IP addresses to whitelist for DOS protection."}, KV{.key = "dos_guard.max_fetches", .value = "Maximum number of fetch operations allowed by DOS guard."}, KV{.key = "dos_guard.max_connections", .value = "Maximum number of concurrent connections allowed by DOS guard." }, @@ -120,39 +194,53 @@ private: KV{.key = "server.port", .value = "Port number of the Clio HTTP server."}, KV{.key = "server.max_queue_size", .value = "Maximum size of the server's request queue. Value of 0 is no limit."}, - KV{.key = "server.local_admin", .value = "Indicates if the server should run with admin privileges."}, - KV{.key = "server.admin_password", .value = "Password for Clio admin-only APIs."}, + KV{.key = "server.local_admin", + .value = "Indicates if the server should run with admin privileges. Only one of local_admin or " + "admin_password can be set."}, + KV{.key = "server.admin_password", + .value = "Password for Clio admin-only APIs. Only one of local_admin or admin_password can be set."}, KV{.key = "server.processing_policy", .value = R"(Could be "sequent" or "parallel". For the sequent policy, requests from a single client connection are processed one by one, with the next request read only after the previous one is processed. For the parallel policy, Clio will accept all requests and process them in parallel, sending a reply for each request as soon as it is ready.)"}, KV{.key = "server.parallel_requests_limit", - .value = R"(Optional parameter, used only if "processing_strategy" is - "parallel". It limits the number of requests for a single client connection that are processed in parallel. If not specified, the limit is infinite.)" + .value = + R"(Optional parameter, used only if processing_strategy `parallel`. It limits the number of requests for a single client connection that are processed in parallel. If not specified, the limit is infinite.)" }, KV{.key = "server.ws_max_sending_queue_size", .value = "Maximum size of the websocket sending queue."}, KV{.key = "prometheus.enabled", .value = "Enable or disable Prometheus metrics."}, KV{.key = "prometheus.compress_reply", .value = "Enable or disable compression of Prometheus responses."}, - KV{.key = "io_threads", .value = "Number of I/O threads. Value must be greater than 1"}, + KV{.key = "io_threads", .value = "Number of I/O threads. Value cannot be less than 1"}, KV{.key = "subscription_workers", .value = "The number of worker threads or processes that are responsible for managing and processing " - "subscription-based tasks."}, + "subscription-based tasks from rippled"}, KV{.key = "graceful_period", .value = "Number of milliseconds server will wait to shutdown gracefully."}, - KV{.key = "cache.num_diffs", .value = "Number of diffs to cache."}, + KV{.key = "cache.num_diffs", .value = "Number of diffs to cache. For more info, consult readme.md in etc"}, KV{.key = "cache.num_markers", .value = "Number of markers to cache."}, KV{.key = "cache.num_cursors_from_diff", .value = "Num of cursors that are different."}, KV{.key = "cache.num_cursors_from_account", .value = "Number of cursors from an account."}, KV{.key = "cache.page_fetch_size", .value = "Page fetch size for cache operations."}, KV{.key = "cache.load", .value = "Cache loading strategy ('sync' or 'async')."}, - KV{.key = "log_channels.[].channel", .value = "Name of the log channel."}, - KV{.key = "log_channels.[].log_level", .value = "Log level for the log channel."}, - KV{.key = "log_level", .value = "General logging level of Clio."}, + KV{.key = "log_channels.[].channel", + .value = "Name of the log channel." + "'RPC', 'ETL', and 'Performance'"}, + KV{.key = "log_channels.[].log_level", + .value = "Log level for the specific log channel." + "`warning`, `error`, `fatal`"}, + KV{.key = "log_level", + .value = "General logging level of Clio. This level will be applied to all log channels that do not have an " + "explicitly defined logging level."}, KV{.key = "log_format", .value = "Format string for log messages."}, KV{.key = "log_to_console", .value = "Enable or disable logging to console."}, KV{.key = "log_directory", .value = "Directory path for log files."}, - KV{.key = "log_rotation_size", .value = "Log rotation size in megabytes."}, + KV{.key = "log_rotation_size", + .value = + "Log rotation size in megabytes. When the log file reaches this particular size, a new log file starts." + }, KV{.key = "log_directory_max_size", .value = "Maximum size of the log directory in megabytes."}, - KV{.key = "log_rotation_hour_interval", .value = "Interval in hours for log rotation."}, + KV{.key = "log_rotation_hour_interval", + .value = "Interval in hours for log rotation. If the current log file reaches this value in logging, a new " + "log file starts."}, KV{.key = "log_tag_style", .value = "Style for log tags."}, KV{.key = "extractor_threads", .value = "Number of extractor threads."}, KV{.key = "read_only", .value = "Indicates if the server should have read-only privileges."}, @@ -164,8 +252,8 @@ private: KV{.key = "api_version.default", .value = "Default API version Clio will run on."}, KV{.key = "api_version.min", .value = "Minimum API version."}, KV{.key = "api_version.max", .value = "Maximum API version."}, - KV{.key = "migration.full_scan_threads", .value = "The number of threads used to scan table."}, - KV{.key = "migration.full_scan_jobs", .value = "The number of coroutines used to scan table."}, + KV{.key = "migration.full_scan_threads", .value = "The number of threads used to scan the table."}, + KV{.key = "migration.full_scan_jobs", .value = "The number of coroutines used to scan the table."}, KV{.key = "migration.cursors_per_job", .value = "The number of cursors each coroutine will scan."} }; }; diff --git a/src/util/newconfig/ConfigValue.hpp b/src/util/newconfig/ConfigValue.hpp index d68daaa60..d83025acf 100644 --- a/src/util/newconfig/ConfigValue.hpp +++ b/src/util/newconfig/ConfigValue.hpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -141,7 +142,7 @@ public: * * @return An optional reference to the associated Constraint. */ - [[nodiscard]] std::optional> + [[nodiscard]] constexpr std::optional> getConstraint() const { return cons_; @@ -201,6 +202,29 @@ public: return value_.value(); } + /** + * @brief Prints all the info of this config value to the output stream. + * + * @param stream The output stream + * @param val The config value to output to osstream + * @return The same ostream we were given + */ + friend std::ostream& + operator<<(std::ostream& stream, ConfigValue val) + { + stream << "- **Required**: " << (val.isOptional() ? "False" : "True") << "\n"; + stream << "- **Type**: " << val.type() << "\n"; + stream << "- **Default value**: " << (val.hasValue() ? *val.value_ : "None") << "\n"; + stream << "- **Constraints**: "; + + if (val.getConstraint().has_value()) { + stream << val.getConstraint()->get() << "\n"; + } else { + stream << "None" << "\n"; + } + return stream; + } + private: /** * @brief Checks if the value type is consistent with the specified ConfigType diff --git a/src/util/newconfig/Types.cpp b/src/util/newconfig/Types.cpp new file mode 100644 index 000000000..b8a6330db --- /dev/null +++ b/src/util/newconfig/Types.cpp @@ -0,0 +1,66 @@ +//------------------------------------------------------------------------------ +/* + 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 "util/newconfig/Types.hpp" + +#include +#include +#include +#include + +namespace util::config { + +std::ostream& +operator<<(std::ostream& stream, ConfigType type) +{ + switch (type) { + case ConfigType::Integer: + stream << "int"; + break; + case ConfigType::String: + stream << "string"; + break; + case ConfigType::Double: + stream << "double"; + break; + case ConfigType::Boolean: + stream << "boolean"; + break; + default: + stream << "unsupported type"; + } + return stream; +} + +std::ostream& +operator<<(std::ostream& stream, Value value) +{ + if (std::holds_alternative(value)) { + stream << std::get(value); + } else if (std::holds_alternative(value)) { + stream << (std::get(value) ? "False" : "True"); + } else if (std::holds_alternative(value)) { + stream << std::get(value); + } else if (std::holds_alternative(value)) { + stream << std::get(value); + } + return stream; +} + +} // namespace util::config diff --git a/src/util/newconfig/Types.hpp b/src/util/newconfig/Types.hpp index 42c9ddffa..bc2f1cfdf 100644 --- a/src/util/newconfig/Types.hpp +++ b/src/util/newconfig/Types.hpp @@ -22,8 +22,9 @@ #include "util/UnsupportedType.hpp" #include +#include +#include #include -#include #include namespace util::config { @@ -31,9 +32,29 @@ namespace util::config { /** @brief Custom clio config types */ enum class ConfigType { Integer, String, Double, Boolean }; +/** + * @brief Prints the specified config type to output stream + * + * @param stream The output stream + * @param type The config type + * @return The same ostream we were given + */ +std::ostream& +operator<<(std::ostream& stream, ConfigType type); + /** @brief Represents the supported Config Values */ using Value = std::variant; +/** + * @brief Prints the specified value to output stream + * + * @param stream The output stream + * @param value The value type + * @return The same ostream we were given + */ +std::ostream& +operator<<(std::ostream& stream, Value value); + /** * @brief Get the corresponding clio config type * diff --git a/src/util/newconfig/ValueView.hpp b/src/util/newconfig/ValueView.hpp index 078296fa6..d23848542 100644 --- a/src/util/newconfig/ValueView.hpp +++ b/src/util/newconfig/ValueView.hpp @@ -20,6 +20,7 @@ #pragma once #include "util/Assert.hpp" +#include "util/newconfig/ConfigConstraints.hpp" #include "util/newconfig/ConfigValue.hpp" #include "util/newconfig/Types.hpp" @@ -30,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -141,6 +143,17 @@ public: return configVal_.get().isOptional(); } + /** + * @brief Retrieves the constraint associated with the ConfigValue in this ValueView, if any. + * + * @return An optional reference to the associated Constraint + */ + [[nodiscard]] constexpr std::optional> + getConstraint() const + { + return configVal_.get().getConstraint(); + } + /** * @brief Retrieves the stored value as the specified type T * @@ -186,6 +199,20 @@ public: return std::make_optional(getValueImpl()); } + /** + * @brief Custom output stream for ValueView + * + * @param stream The output stream + * @param value The ValueView + * @return The same ostream we were given + */ + friend std::ostream& + operator<<(std::ostream& stream, ValueView value) + { + stream << value.configVal_; + return stream; + } + private: std::reference_wrapper configVal_; }; diff --git a/tests/common/util/TmpFile.hpp b/tests/common/util/TmpFile.hpp index f09647035..a74f80ff4 100644 --- a/tests/common/util/TmpFile.hpp +++ b/tests/common/util/TmpFile.hpp @@ -37,6 +37,12 @@ struct TmpFile { ofs << content; } + static TmpFile + empty() + { + return TmpFile{""}; + } + TmpFile(TmpFile const&) = delete; TmpFile(TmpFile&& other) : path{std::move(other.path)} { diff --git a/tests/unit/app/CliArgsTests.cpp b/tests/unit/app/CliArgsTests.cpp index d6e098420..fd5ecb5f2 100644 --- a/tests/unit/app/CliArgsTests.cpp +++ b/tests/unit/app/CliArgsTests.cpp @@ -18,12 +18,19 @@ //============================================================================== #include "app/CliArgs.hpp" +#include "util/TmpFile.hpp" +#include "util/newconfig/ConfigDefinition.hpp" +#include "util/newconfig/ConfigDescription.hpp" #include #include #include #include +#include +#include +#include +#include #include using namespace app; @@ -145,3 +152,71 @@ TEST_F(CliArgsTests, Parse_VerifyConfig) returnCode ); } + +TEST_F(CliArgsTests, Parse_ConfigDescriptionInvalidPath) +{ + using namespace util::config; + std::array argv{"clio_server", "--config-description", ""}; + auto const action = CliArgs::parse(argv.size(), argv.data()); + 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() + ), + EXIT_FAILURE + ); +} + +struct CliArgsTestsWithTmpFile : CliArgsTests { + TmpFile tmpFile = TmpFile::empty(); +}; + +TEST_F(CliArgsTestsWithTmpFile, Parse_ConfigDescription) +{ + std::array argv{"clio_server", "--config-description", tmpFile.path.c_str()}; + auto const action = CliArgs::parse(argv.size(), argv.data()); + EXPECT_CALL(onExitMock, Call).WillOnce([](CliArgs::Action::Exit const& exit) { return exit.exitCode; }); + + // user provide config markdown file name as well + ASSERT_TRUE(std::filesystem::exists(tmpFile.path)); + + EXPECT_EQ( + action.apply( + onRunMock.AsStdFunction(), + onExitMock.AsStdFunction(), + onMigrateMock.AsStdFunction(), + onVerifyMock.AsStdFunction() + ), + EXIT_SUCCESS + ); +} + +TEST_F(CliArgsTestsWithTmpFile, Parse_ConfigDescriptionFileContent) +{ + using namespace util::config; + + std::ofstream file(tmpFile.path); + ASSERT_TRUE(file.is_open()); + ClioConfigDescription::writeConfigDescriptionToFile(file); + file.close(); + + std::ifstream inFile(tmpFile.path); + ASSERT_TRUE(inFile.is_open()); + + std::stringstream buffer; + buffer << inFile.rdbuf(); + inFile.close(); + + auto const fileContent = buffer.str(); + EXPECT_TRUE(fileContent.find("# Clio Config Description") != std::string::npos); + EXPECT_TRUE(fileContent.find("This file lists all Clio Configuration definitions in detail.") != std::string::npos); + EXPECT_TRUE(fileContent.find("## Configuration Details") != std::string::npos); + + // all keys that exist in clio config should be listed in config description file + for (auto const& key : gClioConfig) + EXPECT_TRUE(fileContent.find(key.first)); +} diff --git a/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp b/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp index 6d5846dd2..b2358a2e2 100644 --- a/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp +++ b/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp @@ -165,7 +165,10 @@ TEST(ConfigDescription, GetValues) { ClioConfigDescription const definition{}; - EXPECT_EQ(definition.get("database.type"), "Type of database to use. Default is Scylladb."); + EXPECT_EQ( + definition.get("database.type"), + "Type of database to use. We currently support Cassandra and Scylladb. We default to Scylladb." + ); EXPECT_EQ(definition.get("etl_sources.[].ip"), "IP address of the ETL source."); EXPECT_EQ(definition.get("prometheus.enabled"), "Enable or disable Prometheus metrics."); } From b5fe22da1882530424b562383f51b9ee2b278e9c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:01:31 +0000 Subject: [PATCH 04/29] style: clang-tidy auto fixes (#1889) Fixes #1888. Please review and commit clang-tidy fixes. Co-authored-by: godexsoft <385326+godexsoft@users.noreply.github.com> --- src/app/CliArgs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/CliArgs.cpp b/src/app/CliArgs.cpp index 2cfa04b2f..ce461c83b 100644 --- a/src/app/CliArgs.cpp +++ b/src/app/CliArgs.cpp @@ -71,7 +71,7 @@ CliArgs::parse(int argc, char const* argv[]) } if (parsed.count("config-description") != 0u) { - std::filesystem::path filePath = parsed["config-description"].as(); + std::filesystem::path const filePath = parsed["config-description"].as(); auto const res = util::config::ClioConfigDescription::generateConfigDescriptionToFile(filePath); if (res.has_value()) From cd1aa8fb702e54dd2ee59e7e9254bee0ab73ea6f Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Tue, 11 Feb 2025 18:08:47 +0000 Subject: [PATCH 05/29] chore: Revert workflow names (#1890) --- .github/actions/build_clio/action.yml | 2 +- .github/actions/generate/action.yml | 2 +- .../actions/get_number_of_threads/action.yml | 7 +- .github/workflows/build.yml | 295 +++++++----------- .github/workflows/build_impl.yml | 192 ++++++++++++ .github/workflows/nightly.yml | 2 +- .github/workflows/on_push_checks.yml | 129 -------- .github/workflows/sanitizers.yml | 2 +- 8 files changed, 315 insertions(+), 316 deletions(-) create mode 100644 .github/workflows/build_impl.yml delete mode 100644 .github/workflows/on_push_checks.yml diff --git a/.github/actions/build_clio/action.yml b/.github/actions/build_clio/action.yml index 6e00dac6e..2287ad22e 100644 --- a/.github/actions/build_clio/action.yml +++ b/.github/actions/build_clio/action.yml @@ -15,7 +15,7 @@ runs: uses: ./.github/actions/get_number_of_threads id: number_of_threads with: - substract_threads: inputs.substract_threads + substract_threads: ${{ inputs.substract_threads }} - name: Build Clio shell: bash diff --git a/.github/actions/generate/action.yml b/.github/actions/generate/action.yml index 4e96a5eb1..1926cf135 100644 --- a/.github/actions/generate/action.yml +++ b/.github/actions/generate/action.yml @@ -57,4 +57,4 @@ runs: '' }} run: | cd build - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} ${SANITIZER_OPTION} .. -G Ninja + cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" ${SANITIZER_OPTION} .. -G Ninja diff --git a/.github/actions/get_number_of_threads/action.yml b/.github/actions/get_number_of_threads/action.yml index b32735838..088371b9e 100644 --- a/.github/actions/get_number_of_threads/action.yml +++ b/.github/actions/get_number_of_threads/action.yml @@ -27,9 +27,8 @@ runs: - name: Shift and export number of threads id: number_of_threads_export shell: bash - env: - NUM_OF_THREADS: steps.mac_threads.outputs.num || steps.linux_threads.outputs.num - SHIFT_BY: inputs.substract_threads run: | - shifted=$((NUM_OF_THREADS - SHIFT_BY)) + num_of_threads=${{ steps.mac_threads.outputs.num || steps.linux_threads.outputs.num }} + shift_by=${{ inputs.substract_threads }} + shifted=$((num_of_threads - shift_by)) echo "num=$(( shifted > 1 ? shifted : 1 ))" >> $GITHUB_OUTPUT diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 69b754a38..b4b5a5b89 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,192 +1,129 @@ -name: Reusable build +name: Build on: - workflow_call: - inputs: - runs_on: - description: Runner to run the job on - required: true - type: string - default: heavy - - container: - description: "The container object as a JSON string (leave empty to run natively)" - required: true - type: string - default: "" - - conan_profile: - description: Conan profile to use - required: true - type: string - - build_type: - description: Build type - required: true - type: string - - disable_cache: - description: Whether ccache and conan cache should be disabled - required: false - type: boolean - default: false - - code_coverage: - description: Whether to enable code coverage - required: true - type: boolean - default: false - - static: - description: Whether to build static binaries - required: true - type: boolean - default: true - - unit_tests: - description: Whether to run unit tests - required: true - type: boolean - default: false - - integration_tests: - description: Whether to run integration tests - required: true - type: boolean - default: false - - clio_server: - description: Whether to build clio_server - required: true - type: boolean - default: true - - target: - description: Build target name - required: false - type: string - default: all - - sanitizer: - description: Sanitizer to use - required: false - type: string - default: 'false' + push: + branches: [master, release/*, develop] + pull_request: + branches: [master, release/*, develop] + workflow_dispatch: jobs: + check_format: + name: Check format + runs-on: ubuntu-20.04 + container: + image: rippleci/clio_ci:latest + steps: + - name: Fix git permissions on Linux + shell: bash + run: git config --global --add safe.directory $PWD + + - uses: actions/checkout@v4 + - name: Run formatters + id: run_formatters + run: | + ./.githooks/check-format --diff + shell: bash + + check_docs: + name: Check documentation + runs-on: ubuntu-20.04 + container: + image: rippleci/clio_ci:latest + steps: + - uses: actions/checkout@v4 + - name: Run linter + id: run_linter + run: | + ./.githooks/check-docs + shell: bash + build: - name: Build ${{ inputs.container != '' && 'in container' || 'natively' }} - runs-on: ${{ inputs.runs_on }} - container: ${{ inputs.container != '' && fromJson(inputs.container) || null }} + name: Build + needs: + - check_format + - check_docs + strategy: + fail-fast: false + matrix: + include: + - os: heavy + conan_profile: gcc + build_type: Release + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: heavy + conan_profile: gcc + build_type: Debug + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: true + static: true + - os: heavy + conan_profile: clang + build_type: Release + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: heavy + conan_profile: clang + build_type: Debug + container: '{ "image": "rippleci/clio_ci:latest" }' + code_coverage: false + static: true + - os: macos15 + build_type: Release + code_coverage: false + static: false + uses: ./.github/workflows/build_impl.yml + with: + runs_on: ${{ matrix.os }} + container: ${{ matrix.container }} + conan_profile: ${{ matrix.conan_profile }} + build_type: ${{ matrix.build_type }} + code_coverage: ${{ matrix.code_coverage }} + static: ${{ matrix.static }} + unit_tests: true + integration_tests: true + clio_server: true + + test: + name: Run Tests + needs: build + strategy: + fail-fast: false + matrix: + include: + - os: heavy + conan_profile: gcc + build_type: Release + container: + image: rippleci/clio_ci:latest + - os: heavy + conan_profile: clang + build_type: Release + container: + image: rippleci/clio_ci:latest + - os: heavy + conan_profile: clang + build_type: Debug + container: + image: rippleci/clio_ci:latest + - os: macos15 + conan_profile: apple_clang_16 + build_type: Release + runs-on: ${{ matrix.os }} + container: ${{ matrix.container }} steps: - name: Clean workdir if: ${{ runner.os == 'macOS' }} uses: kuznetsss/workspace-cleanup@1.0 - - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 with: - fetch-depth: 0 + name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} - - name: Prepare runner - uses: ./.github/actions/prepare_runner - with: - disable_ccache: ${{ inputs.disable_cache }} - - - name: Setup conan - uses: ./.github/actions/setup_conan - id: conan - with: - conan_profile: ${{ inputs.conan_profile }} - - - name: Restore cache - if: ${{ !inputs.disable_cache }} - uses: ./.github/actions/restore_cache - id: restore_cache - with: - conan_dir: ${{ env.CONAN_USER_HOME }}/.conan - conan_profile: ${{ steps.conan.outputs.conan_profile }} - ccache_dir: ${{ env.CCACHE_DIR }} - build_type: ${{ inputs.build_type }} - code_coverage: ${{ inputs.code_coverage }} - - - name: Run conan and cmake - uses: ./.github/actions/generate - with: - conan_profile: ${{ steps.conan.outputs.conan_profile }} - conan_cache_hit: ${{ !inputs.disable_cache && steps.restore_cache.outputs.conan_cache_hit }} - build_type: ${{ inputs.build_type }} - code_coverage: ${{ inputs.code_coverage }} - static: ${{ inputs.static }} - sanitizer: ${{ inputs.sanitizer }} - - - name: Build Clio - uses: ./.github/actions/build_clio - with: - target: ${{ inputs.target }} - - - name: Show ccache's statistics - if: ${{ !inputs.disable_cache }} - shell: bash - id: ccache_stats + - name: Run clio_tests run: | - ccache -s > /tmp/ccache.stats - miss_rate=$(cat /tmp/ccache.stats | grep 'Misses' | head -n1 | sed 's/.*(\(.*\)%).*/\1/') - echo "miss_rate=${miss_rate}" >> $GITHUB_OUTPUT - cat /tmp/ccache.stats - - - name: Strip unit_tests - if: ${{ inputs.unit_tests && !inputs.code_coverage && inputs.sanitizer == 'false' }} - run: strip build/clio_tests - - - name: Strip integration_tests - if: ${{ inputs.integration_tests && !inputs.code_coverage }} - run: strip build/clio_integration_tests - - - name: Upload clio_server - if: ${{ inputs.clio_server }} - uses: actions/upload-artifact@v4 - with: - name: clio_server_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} - path: build/clio_server - - - name: Upload clio_tests - if: ${{ inputs.unit_tests && !inputs.code_coverage }} - uses: actions/upload-artifact@v4 - with: - name: clio_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} - path: build/clio_tests - - - name: Upload clio_integration_tests - if: ${{ inputs.integration_tests && !inputs.code_coverage }} - uses: actions/upload-artifact@v4 - with: - name: clio_integration_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} - path: build/clio_integration_tests - - - name: Save cache - if: ${{ !inputs.disable_cache }} - uses: ./.github/actions/save_cache - with: - conan_dir: ${{ env.CONAN_USER_HOME }}/.conan - conan_hash: ${{ steps.restore_cache.outputs.conan_hash }} - conan_cache_hit: ${{ steps.restore_cache.outputs.conan_cache_hit }} - ccache_dir: ${{ env.CCACHE_DIR }} - ccache_cache_hit: ${{ steps.restore_cache.outputs.ccache_cache_hit }} - ccache_cache_miss_rate: ${{ steps.ccache_stats.outputs.miss_rate }} - build_type: ${{ inputs.build_type }} - code_coverage: ${{ inputs.code_coverage }} - conan_profile: ${{ steps.conan.outputs.conan_profile }} - - # TODO: This is not a part of build process but it is the easiest way to do it here. - # It will be refactored in https://github.com/XRPLF/clio/issues/1075 - - name: Run code coverage - if: ${{ inputs.code_coverage }} - uses: ./.github/actions/code_coverage - - upload_coverage_report: - if: ${{ inputs.code_coverage }} - name: Codecov - needs: build - uses: ./.github/workflows/upload_coverage_report.yml - secrets: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + chmod +x ./clio_tests + ./clio_tests diff --git a/.github/workflows/build_impl.yml b/.github/workflows/build_impl.yml new file mode 100644 index 000000000..69b754a38 --- /dev/null +++ b/.github/workflows/build_impl.yml @@ -0,0 +1,192 @@ +name: Reusable build +on: + workflow_call: + inputs: + runs_on: + description: Runner to run the job on + required: true + type: string + default: heavy + + container: + description: "The container object as a JSON string (leave empty to run natively)" + required: true + type: string + default: "" + + conan_profile: + description: Conan profile to use + required: true + type: string + + build_type: + description: Build type + required: true + type: string + + disable_cache: + description: Whether ccache and conan cache should be disabled + required: false + type: boolean + default: false + + code_coverage: + description: Whether to enable code coverage + required: true + type: boolean + default: false + + static: + description: Whether to build static binaries + required: true + type: boolean + default: true + + unit_tests: + description: Whether to run unit tests + required: true + type: boolean + default: false + + integration_tests: + description: Whether to run integration tests + required: true + type: boolean + default: false + + clio_server: + description: Whether to build clio_server + required: true + type: boolean + default: true + + target: + description: Build target name + required: false + type: string + default: all + + sanitizer: + description: Sanitizer to use + required: false + type: string + default: 'false' + +jobs: + build: + name: Build ${{ inputs.container != '' && 'in container' || 'natively' }} + runs-on: ${{ inputs.runs_on }} + container: ${{ inputs.container != '' && fromJson(inputs.container) || null }} + + steps: + - name: Clean workdir + if: ${{ runner.os == 'macOS' }} + uses: kuznetsss/workspace-cleanup@1.0 + + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Prepare runner + uses: ./.github/actions/prepare_runner + with: + disable_ccache: ${{ inputs.disable_cache }} + + - name: Setup conan + uses: ./.github/actions/setup_conan + id: conan + with: + conan_profile: ${{ inputs.conan_profile }} + + - name: Restore cache + if: ${{ !inputs.disable_cache }} + uses: ./.github/actions/restore_cache + id: restore_cache + with: + conan_dir: ${{ env.CONAN_USER_HOME }}/.conan + conan_profile: ${{ steps.conan.outputs.conan_profile }} + ccache_dir: ${{ env.CCACHE_DIR }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} + + - name: Run conan and cmake + uses: ./.github/actions/generate + with: + conan_profile: ${{ steps.conan.outputs.conan_profile }} + conan_cache_hit: ${{ !inputs.disable_cache && steps.restore_cache.outputs.conan_cache_hit }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} + static: ${{ inputs.static }} + sanitizer: ${{ inputs.sanitizer }} + + - name: Build Clio + uses: ./.github/actions/build_clio + with: + target: ${{ inputs.target }} + + - name: Show ccache's statistics + if: ${{ !inputs.disable_cache }} + shell: bash + id: ccache_stats + run: | + ccache -s > /tmp/ccache.stats + miss_rate=$(cat /tmp/ccache.stats | grep 'Misses' | head -n1 | sed 's/.*(\(.*\)%).*/\1/') + echo "miss_rate=${miss_rate}" >> $GITHUB_OUTPUT + cat /tmp/ccache.stats + + - name: Strip unit_tests + if: ${{ inputs.unit_tests && !inputs.code_coverage && inputs.sanitizer == 'false' }} + run: strip build/clio_tests + + - name: Strip integration_tests + if: ${{ inputs.integration_tests && !inputs.code_coverage }} + run: strip build/clio_integration_tests + + - name: Upload clio_server + if: ${{ inputs.clio_server }} + uses: actions/upload-artifact@v4 + with: + name: clio_server_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} + path: build/clio_server + + - name: Upload clio_tests + if: ${{ inputs.unit_tests && !inputs.code_coverage }} + uses: actions/upload-artifact@v4 + with: + name: clio_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} + path: build/clio_tests + + - name: Upload clio_integration_tests + if: ${{ inputs.integration_tests && !inputs.code_coverage }} + uses: actions/upload-artifact@v4 + with: + name: clio_integration_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ steps.conan.outputs.conan_profile }} + path: build/clio_integration_tests + + - name: Save cache + if: ${{ !inputs.disable_cache }} + uses: ./.github/actions/save_cache + with: + conan_dir: ${{ env.CONAN_USER_HOME }}/.conan + conan_hash: ${{ steps.restore_cache.outputs.conan_hash }} + conan_cache_hit: ${{ steps.restore_cache.outputs.conan_cache_hit }} + ccache_dir: ${{ env.CCACHE_DIR }} + ccache_cache_hit: ${{ steps.restore_cache.outputs.ccache_cache_hit }} + ccache_cache_miss_rate: ${{ steps.ccache_stats.outputs.miss_rate }} + build_type: ${{ inputs.build_type }} + code_coverage: ${{ inputs.code_coverage }} + conan_profile: ${{ steps.conan.outputs.conan_profile }} + + # TODO: This is not a part of build process but it is the easiest way to do it here. + # It will be refactored in https://github.com/XRPLF/clio/issues/1075 + - name: Run code coverage + if: ${{ inputs.code_coverage }} + uses: ./.github/actions/code_coverage + + upload_coverage_report: + if: ${{ inputs.code_coverage }} + name: Codecov + needs: build + uses: ./.github/workflows/upload_coverage_report.yml + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index a5616d0f1..11b1e1d56 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -26,7 +26,7 @@ jobs: build_type: Debug static: true container: '{ "image": "rippleci/clio_ci:latest" }' - uses: ./.github/workflows/build.yml + uses: ./.github/workflows/build_impl.yml with: runs_on: ${{ matrix.os }} container: ${{ matrix.container }} diff --git a/.github/workflows/on_push_checks.yml b/.github/workflows/on_push_checks.yml deleted file mode 100644 index 9e1f4ebb4..000000000 --- a/.github/workflows/on_push_checks.yml +++ /dev/null @@ -1,129 +0,0 @@ -name: Check codebase -on: - push: - branches: [master, release/*, develop] - pull_request: - branches: [master, release/*, develop] - workflow_dispatch: - -jobs: - check_format: - name: Check format - runs-on: ubuntu-20.04 - container: - image: rippleci/clio_ci:latest - steps: - - name: Fix git permissions on Linux - shell: bash - run: git config --global --add safe.directory $PWD - - - uses: actions/checkout@v4 - - name: Run formatters - id: run_formatters - run: | - ./.githooks/check-format --diff - shell: bash - - check_docs: - name: Check documentation - runs-on: ubuntu-20.04 - container: - image: rippleci/clio_ci:latest - steps: - - uses: actions/checkout@v4 - - name: Run linter - id: run_linter - run: | - ./.githooks/check-docs - shell: bash - - build: - name: Build - needs: - - check_format - - check_docs - strategy: - fail-fast: false - matrix: - include: - - os: heavy - conan_profile: gcc - build_type: Release - container: '{ "image": "rippleci/clio_ci:latest" }' - code_coverage: false - static: true - - os: heavy - conan_profile: gcc - build_type: Debug - container: '{ "image": "rippleci/clio_ci:latest" }' - code_coverage: true - static: true - - os: heavy - conan_profile: clang - build_type: Release - container: '{ "image": "rippleci/clio_ci:latest" }' - code_coverage: false - static: true - - os: heavy - conan_profile: clang - build_type: Debug - container: '{ "image": "rippleci/clio_ci:latest" }' - code_coverage: false - static: true - - os: macos15 - build_type: Release - code_coverage: false - static: false - uses: ./.github/workflows/build.yml - with: - runs_on: ${{ matrix.os }} - container: ${{ matrix.container }} - conan_profile: ${{ matrix.conan_profile }} - build_type: ${{ matrix.build_type }} - code_coverage: ${{ matrix.code_coverage }} - static: ${{ matrix.static }} - unit_tests: true - integration_tests: true - clio_server: true - - test: - name: Run Tests - needs: build - strategy: - fail-fast: false - matrix: - include: - - os: heavy - conan_profile: gcc - build_type: Release - container: - image: rippleci/clio_ci:latest - - os: heavy - conan_profile: clang - build_type: Release - container: - image: rippleci/clio_ci:latest - - os: heavy - conan_profile: clang - build_type: Debug - container: - image: rippleci/clio_ci:latest - - os: macos15 - conan_profile: apple_clang_16 - build_type: Release - runs-on: ${{ matrix.os }} - container: ${{ matrix.container }} - - steps: - - name: Clean workdir - if: ${{ runner.os == 'macOS' }} - uses: kuznetsss/workspace-cleanup@1.0 - - - uses: actions/download-artifact@v4 - with: - name: clio_tests_${{ runner.os }}_${{ matrix.build_type }}_${{ matrix.conan_profile }} - - - name: Run clio_tests - run: | - chmod +x ./clio_tests - ./clio_tests diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml index 0e573bc34..ce0f2bb33 100644 --- a/.github/workflows/sanitizers.yml +++ b/.github/workflows/sanitizers.yml @@ -20,7 +20,7 @@ jobs: compiler: gcc # - sanitizer: ubsan # todo: enable when heavy runners are available # compiler: gcc - uses: ./.github/workflows/build.yml + uses: ./.github/workflows/build_impl.yml with: runs_on: ubuntu-latest # todo: change to heavy container: '{ "image": "rippleci/clio_ci:latest" }' From e503dffc9ac54e8d1afd5adf850fb948a487b539 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Wed, 12 Feb 2025 13:28:06 +0000 Subject: [PATCH 06/29] fix: Array parsing in new config (#1884) Fixes #1870. --- src/util/newconfig/Array.cpp | 16 +- src/util/newconfig/Array.hpp | 11 + src/util/newconfig/ConfigFileJson.cpp | 122 +++-- src/util/newconfig/ConfigFileJson.hpp | 20 +- src/util/newconfig/ConfigValue.hpp | 26 +- src/util/newconfig/Types.hpp | 37 +- tests/unit/CMakeLists.txt | 3 +- tests/unit/util/newconfig/ArrayTests.cpp | 12 + .../util/newconfig/ConfigFileJsonTests.cpp | 480 ++++++++++++++++++ .../unit/util/newconfig/ConfigValueTests.cpp | 165 +++++- .../util/newconfig/JsonConfigFileTests.cpp | 113 ----- tests/unit/util/newconfig/JsonFileTests.cpp | 4 +- 12 files changed, 818 insertions(+), 191 deletions(-) create mode 100644 tests/unit/util/newconfig/ConfigFileJsonTests.cpp delete mode 100644 tests/unit/util/newconfig/JsonConfigFileTests.cpp diff --git a/src/util/newconfig/Array.cpp b/src/util/newconfig/Array.cpp index 13d9b8361..46cbe1ee3 100644 --- a/src/util/newconfig/Array.cpp +++ b/src/util/newconfig/Array.cpp @@ -36,14 +36,22 @@ Array::Array(ConfigValue arg) : itemPattern_{std::move(arg)} { } +std::string_view +Array::prefix(std::string_view key) +{ + static constexpr std::string_view kARRAY_SUFFIX = ".[]"; + ASSERT(key.contains(kARRAY_SUFFIX), "Provided key is not an array key: {}", key); + + return key.substr(0, key.rfind(kARRAY_SUFFIX) + kARRAY_SUFFIX.size()); +} + std::optional Array::addValue(Value value, std::optional key) { - auto const& configValPattern = itemPattern_; - auto const constraint = configValPattern.getConstraint(); + auto const constraint = itemPattern_.getConstraint(); - auto newElem = constraint.has_value() ? ConfigValue{configValPattern.type()}.withConstraint(constraint->get()) - : ConfigValue{configValPattern.type()}; + auto newElem = constraint.has_value() ? ConfigValue{itemPattern_.type()}.withConstraint(constraint->get()) + : ConfigValue{itemPattern_.type()}; if (auto const maybeError = newElem.setValue(value, key); maybeError.has_value()) return maybeError; elements_.emplace_back(std::move(newElem)); diff --git a/src/util/newconfig/Array.hpp b/src/util/newconfig/Array.hpp index 672757655..ca8b4cf09 100644 --- a/src/util/newconfig/Array.hpp +++ b/src/util/newconfig/Array.hpp @@ -47,6 +47,17 @@ public: */ Array(ConfigValue arg); + /** + * @brief Extract array prefix from a key, For example for a key foo.[].bar the method will return foo.[] + * @note Provided key must contain '.[]' + * @warning Be careful with string_view! Returned value is valid only while the key is valid + * + * @param key The key to extract the array prefix from + * @return Prefix of array extracted from the key + */ + static std::string_view + prefix(std::string_view key); + /** * @brief Add ConfigValues to Array class * diff --git a/src/util/newconfig/ConfigFileJson.cpp b/src/util/newconfig/ConfigFileJson.cpp index bb9730fb8..976523bee 100644 --- a/src/util/newconfig/ConfigFileJson.cpp +++ b/src/util/newconfig/ConfigFileJson.cpp @@ -20,6 +20,7 @@ #include "util/newconfig/ConfigFileJson.hpp" #include "util/Assert.hpp" +#include "util/newconfig/Array.hpp" #include "util/newconfig/Error.hpp" #include "util/newconfig/Types.hpp" @@ -30,15 +31,19 @@ #include #include +#include #include #include #include #include #include #include +#include +#include #include #include #include +#include #include #include @@ -69,14 +74,17 @@ extractJsonValue(boost::json::value const& jsonValue) if (jsonValue.is_double()) { return jsonValue.as_double(); } - ASSERT(false, "Json is not of type int, uint, string, bool or double"); + if (jsonValue.is_null()) { + return NullType{}; + } + ASSERT(false, "Json is not of type null, int, uint, string, bool or double"); std::unreachable(); } } // namespace ConfigFileJson::ConfigFileJson(boost::json::object jsonObj) { - flattenJson(jsonObj, ""); + flattenJson(jsonObj); } std::expected @@ -86,8 +94,7 @@ ConfigFileJson::makeConfigFileJson(std::filesystem::path const& configFilePath) if (auto const in = std::ifstream(configFilePath.string(), std::ios::in | std::ios::binary); in) { std::stringstream contents; contents << in.rdbuf(); - auto opts = boost::json::parse_options{}; - opts.allow_comments = true; + auto const opts = boost::json::parse_options{.allow_comments = true}; auto const tempObj = boost::json::parse(contents.str(), {}, opts).as_object(); return ConfigFileJson{tempObj}; } @@ -105,7 +112,9 @@ ConfigFileJson::makeConfigFileJson(std::filesystem::path const& configFilePath) Value ConfigFileJson::getValue(std::string_view key) const { + ASSERT(containsKey(key), "Key {} not found in ConfigFileJson", key); auto const jsonValue = jsonObject_.at(key); + ASSERT(jsonValue.is_primitive(), "Key {} has value that is not a primitive", key); auto const value = extractJsonValue(jsonValue); return value; } @@ -113,14 +122,15 @@ ConfigFileJson::getValue(std::string_view key) const std::vector ConfigFileJson::getArray(std::string_view key) const { + ASSERT(containsKey(key), "Key {} not found in ConfigFileJson", key); ASSERT(jsonObject_.at(key).is_array(), "Key {} has value that is not an array", key); std::vector configValues; auto const arr = jsonObject_.at(key).as_array(); for (auto const& item : arr) { - auto const value = extractJsonValue(item); - configValues.emplace_back(value); + auto value = extractJsonValue(item); + configValues.emplace_back(std::move(value)); } return configValues; } @@ -131,38 +141,90 @@ ConfigFileJson::containsKey(std::string_view key) const return jsonObject_.contains(key); } -void -ConfigFileJson::flattenJson(boost::json::object const& obj, std::string const& prefix) +boost::json::object const& +ConfigFileJson::inner() const { - for (auto const& [key, value] : obj) { - std::string const fullKey = prefix.empty() ? std::string(key) : fmt::format("{}.{}", prefix, std::string(key)); + return jsonObject_; +} - // In ClioConfigDefinition, value must be a primitive or array - if (value.is_object()) { - flattenJson(value.as_object(), fullKey); - } else if (value.is_array()) { - auto const& arr = value.as_array(); - for (std::size_t i = 0; i < arr.size(); ++i) { - std::string const arrayPrefix = fullKey + ".[]"; - if (arr[i].is_object()) { - flattenJson(arr[i].as_object(), arrayPrefix); +void +ConfigFileJson::flattenJson(boost::json::object const& jsonRootObject) +{ + struct Task { + boost::json::object const& object; + std::string prefix; + std::optional arrayIndex = std::nullopt; + }; + + std::queue tasks; + tasks.push(Task{.object = jsonRootObject, .prefix = ""}); + + std::unordered_map arraysSizes; + + while (not tasks.empty()) { + auto const task = std::move(tasks.front()); + tasks.pop(); + + for (auto const& [key, value] : task.object) { + auto fullKey = + task.prefix.empty() ? std::string(key) : fmt::format("{}.{}", task.prefix, std::string_view{key}); + + if (value.is_object()) { + tasks.push( + Task{.object = value.as_object(), .prefix = std::move(fullKey), .arrayIndex = task.arrayIndex} + ); + } else if (value.is_array()) { + fullKey += ".[]"; + auto const& array = value.as_array(); + + if (std::ranges::all_of(array, [](auto const& v) { return v.is_primitive(); })) { + jsonObject_[fullKey] = array; + } else if (std::ranges::all_of(array, [](auto const& v) { return v.is_object(); })) { + for (size_t i = 0; i < array.size(); ++i) { + tasks.push(Task{.object = array.at(i).as_object(), .prefix = fullKey, .arrayIndex = i}); + } } else { - jsonObject_[arrayPrefix] = arr; + ASSERT( + false, + "Arrays containing both values and objects are not supported. Please check the array {}", + fullKey + ); } - } - } else { - // if "[]" is present in key, then value must be an array instead of primitive - if (fullKey.contains(".[]") && !jsonObject_.contains(fullKey)) { - boost::json::array newArray; - newArray.emplace_back(value); - jsonObject_[fullKey] = newArray; - } else if (fullKey.contains(".[]") && jsonObject_.contains(fullKey)) { - jsonObject_[fullKey].as_array().emplace_back(value); } else { - jsonObject_[fullKey] = value; + if (task.arrayIndex.has_value()) { + if (not jsonObject_.contains(fullKey)) { + jsonObject_[fullKey] = boost::json::array{}; + } + + auto& targetArray = jsonObject_.at(fullKey).as_array(); + while (targetArray.size() < (*task.arrayIndex + 1)) { + targetArray.push_back(boost::json::value()); + } + targetArray.at(*task.arrayIndex) = value; + auto const prefix = std::string{Array::prefix(fullKey)}; + arraysSizes[prefix] = std::max(arraysSizes[prefix], targetArray.size()); + } else { + jsonObject_[fullKey] = value; + } } } } + + // adjust length of each array containing objects + std::ranges::for_each(jsonObject_, [&arraysSizes](auto& item) { + auto const key = item.key(); + if (not key.contains("[]")) + return; + + auto& value = item.value(); + auto const prefix = std::string{Array::prefix(key)}; + if (auto const it = arraysSizes.find(prefix); it != arraysSizes.end()) { + auto const size = it->second; + while (value.as_array().size() < size) { + value.as_array().push_back(boost::json::value{}); + } + } + }); } } // namespace util::config diff --git a/src/util/newconfig/ConfigFileJson.hpp b/src/util/newconfig/ConfigFileJson.hpp index c208a334b..a21b18f2b 100644 --- a/src/util/newconfig/ConfigFileJson.hpp +++ b/src/util/newconfig/ConfigFileJson.hpp @@ -35,6 +35,8 @@ namespace util::config { /** @brief Json representation of config */ class ConfigFileJson final : public ConfigFileInterface { + boost::json::object jsonObject_; + public: /** * @brief Construct a new ConfigJson object and stores the values from @@ -81,20 +83,26 @@ public: [[nodiscard]] static std::expected makeConfigFileJson(std::filesystem::path const& configFilePath); + /** + * @brief Get the inner representation of json file. + * @note This method is mostly used for testing purposes. + * + * @return The inner representation of json file. + */ + [[nodiscard]] boost::json::object const& + inner() const; + private: /** - * @brief Recursive function to flatten a JSON object into the same structure as the Clio Config. + * @brief Method to flatten a JSON object into the same structure as the Clio Config. * - * The keys will end up having the same naming convensions in Clio Config. + * The keys will end up having the same naming conventions in Clio Config. * Other than the keys specified in user Config file, no new keys are created. * * @param obj The JSON object to flatten. - * @param prefix The prefix to use for the keys in the flattened object. */ void - flattenJson(boost::json::object const& obj, std::string const& prefix); - - boost::json::object jsonObject_; + flattenJson(boost::json::object const& jsonRootObject); }; } // namespace util::config diff --git a/src/util/newconfig/ConfigValue.hpp b/src/util/newconfig/ConfigValue.hpp index d83025acf..d1a50193c 100644 --- a/src/util/newconfig/ConfigValue.hpp +++ b/src/util/newconfig/ConfigValue.hpp @@ -34,6 +34,7 @@ #include #include #include +#include #include namespace util::config { @@ -80,18 +81,31 @@ public: [[nodiscard]] std::optional setValue(Value value, std::optional key = std::nullopt) { + if (std::holds_alternative(value)) { + if (hasValue()) { + // Using default value + return std::nullopt; + } + if (not isOptional()) { + return Error{ + key.value_or("Unknown_key"), + "Provided value is null but ConfigValue is not optional and doesn't have a default value." + }; + } + value_ = std::move(value); + return std::nullopt; + } + auto err = checkTypeConsistency(type_, value); if (err.has_value()) { - if (key.has_value()) - err->error = fmt::format("{} {}", key.value(), err->error); + err->error = fmt::format("{} {}", key.value_or("Unknown_key"), err->error); return err; } if (cons_.has_value()) { auto constraintCheck = cons_->get().checkConstraint(value); if (constraintCheck.has_value()) { - if (key.has_value()) - constraintCheck->error = fmt::format("{} {}", key.value(), constraintCheck->error); + constraintCheck->error = fmt::format("{} {}", key.value_or("Unknown_key"), constraintCheck->error); return constraintCheck; } } @@ -127,7 +141,8 @@ public: [&type](bool tmp) { type = fmt::format("bool {}", tmp); }, [&type](std::string const& tmp) { type = fmt::format("string {}", tmp); }, [&type](double tmp) { type = fmt::format("double {}", tmp); }, - [&type](int64_t tmp) { type = fmt::format("int {}", tmp); } + [&type](int64_t tmp) { type = fmt::format("int {}", tmp); }, + [&type](NullType) { type = "null"; }, }, value_.value() ); @@ -199,6 +214,7 @@ public: [[nodiscard]] Value const& getValue() const { + ASSERT(value_.has_value(), "getValue() is called when there is no value set"); return value_.value(); } diff --git a/src/util/newconfig/Types.hpp b/src/util/newconfig/Types.hpp index bc2f1cfdf..4e24514c4 100644 --- a/src/util/newconfig/Types.hpp +++ b/src/util/newconfig/Types.hpp @@ -21,6 +21,8 @@ #include "util/UnsupportedType.hpp" +#include + #include #include #include @@ -30,7 +32,23 @@ namespace util::config { /** @brief Custom clio config types */ -enum class ConfigType { Integer, String, Double, Boolean }; +enum class ConfigType { Integer, String, Double, Boolean, Null }; + +/** + * @brief A type that represents a null value + */ +struct NullType { + /** + * @brief Compare two NullType objects + * + * @return true always. Any two NullType objects are equal + */ + [[nodiscard]] bool + operator==(NullType const&) const + { + return true; + } +}; /** * @brief Prints the specified config type to output stream @@ -43,7 +61,7 @@ std::ostream& operator<<(std::ostream& stream, ConfigType type); /** @brief Represents the supported Config Values */ -using Value = std::variant; +using Value = std::variant; /** * @brief Prints the specified value to output stream @@ -73,9 +91,24 @@ getType() return ConfigType::Double; } else if constexpr (std::is_same_v) { return ConfigType::Boolean; + } else if constexpr (std::is_same_v) { + return ConfigType::Null; } else { static_assert(util::Unsupported, "Wrong config type"); } } } // namespace util::config + +/** @cond */ +// Doxygen could not parse this +template <> +struct fmt::formatter : fmt::formatter { + [[nodiscard]] + auto + format(util::config::NullType const&, fmt::format_context& ctx) + { + return fmt::formatter::format("null", ctx); + } +}; +/** @endcond */ diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 2f0dd7c77..aa37a69da 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -183,8 +183,7 @@ target_sources( util/newconfig/ClioConfigDefinitionTests.cpp util/newconfig/ConfigValueTests.cpp util/newconfig/ObjectViewTests.cpp - util/newconfig/JsonConfigFileTests.cpp - util/newconfig/JsonFileTests.cpp + util/newconfig/ConfigFileJsonTests.cpp util/newconfig/ValueViewTests.cpp ) diff --git a/tests/unit/util/newconfig/ArrayTests.cpp b/tests/unit/util/newconfig/ArrayTests.cpp index 3135fdace..59de95b67 100644 --- a/tests/unit/util/newconfig/ArrayTests.cpp +++ b/tests/unit/util/newconfig/ArrayTests.cpp @@ -31,6 +31,18 @@ using namespace util::config; +TEST(ArrayTest, prefix) +{ + EXPECT_EQ(Array::prefix("foo.[]"), "foo.[]"); + EXPECT_EQ(Array::prefix("foo.[].bar"), "foo.[]"); + EXPECT_EQ(Array::prefix("foo.bar.[].baz"), "foo.bar.[]"); +} + +TEST(ArrayDeathTest, prefix) +{ + EXPECT_DEATH(Array::prefix("foo.bar"), ".*"); +} + TEST(ArrayTest, addSingleValue) { auto arr = Array{ConfigValue{ConfigType::Double}}; diff --git a/tests/unit/util/newconfig/ConfigFileJsonTests.cpp b/tests/unit/util/newconfig/ConfigFileJsonTests.cpp new file mode 100644 index 000000000..5ff6cd7e9 --- /dev/null +++ b/tests/unit/util/newconfig/ConfigFileJsonTests.cpp @@ -0,0 +1,480 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 "util/LoggerFixtures.hpp" +#include "util/NameGenerator.hpp" +#include "util/OverloadSet.hpp" +#include "util/TmpFile.hpp" +#include "util/newconfig/ConfigFileJson.hpp" +#include "util/newconfig/Types.hpp" + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace util::config; + +namespace { +constexpr auto kEPS = 1e-9; +} // namespace + +struct ConfigFileJsonParseTestBundle { + using ValidationMap = std::unordered_map< + std::string, + std::variant>; + + std::string testName; + std::string configStr; + ValidationMap validationMap; +}; + +struct ConfigFileJsonParseTest : NoLoggerFixture, testing::WithParamInterface {}; + +TEST_P(ConfigFileJsonParseTest, parseValues) +{ + ConfigFileJson const configFile{boost::json::parse(GetParam().configStr).as_object()}; + + auto const& flatJson = configFile.inner(); + + ASSERT_EQ(GetParam().validationMap.size(), flatJson.size()); + std::ranges::for_each(GetParam().validationMap, [&flatJson](auto const& kvPair) { + auto const& key = kvPair.first; + auto const& value = kvPair.second; + + EXPECT_TRUE(flatJson.contains(key)); + + std::visit( + util::OverloadSet{ + [&flatJson, &key](int64_t const v) { + EXPECT_TRUE(flatJson.at(key).is_number()) << key << ": " << v; + EXPECT_EQ(flatJson.at(key).as_int64(), v) << key << ": " << v; + }, + [&flatJson, &key](double const v) { + EXPECT_TRUE(flatJson.at(key).is_double()) << key << ": " << v; + EXPECT_NEAR(flatJson.at(key).as_double(), v, kEPS) << key << ": " << v; + }, + [&flatJson, &key](bool const v) { + EXPECT_TRUE(flatJson.at(key).is_bool()) << key << ": " << v; + EXPECT_EQ(flatJson.at(key).as_bool(), v) << key << ": " << v; + }, + [&flatJson, &key](std::string const& v) { + EXPECT_TRUE(flatJson.at(key).is_string()) << key << ": " << v; + EXPECT_EQ(flatJson.at(key).as_string(), v) << key << ": " << v; + }, + [&flatJson, &key](boost::json::object const& v) { + EXPECT_TRUE(flatJson.at(key).is_object()) << key << ": " << v; + EXPECT_EQ(flatJson.at(key).as_object(), v) << key << ": " << v; + }, + [&flatJson, &key](boost::json::array const& v) { + EXPECT_TRUE(flatJson.at(key).is_array()) << key << ": " << v; + EXPECT_EQ(flatJson.at(key).as_array(), v) << key << ": " << v; + }, + }, + value + ); + }); +} + +INSTANTIATE_TEST_CASE_P( + ConfigFileJsonParseTestGroup, + ConfigFileJsonParseTest, + testing::Values( + ConfigFileJsonParseTestBundle{ + .testName = "values", + .configStr = R"json({ + "int": 42, + "double": 123.456, + "bool": true, + "string": "some string" + })json", + .validationMap = {{"int", 42}, {"double", 123.456}, {"bool", true}, {"string", "some string"}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "nested", + .configStr = R"json({ + "level_0": { + "int": 42, + "level_1":{ + "double": 123.456, + "level_2": { + "bool": true, + "level_3": { + "string": "some string" + } + } + } + } + })json", + .validationMap = + {{"level_0.int", 42}, + {"level_0.level_1.double", 123.456}, + {"level_0.level_1.level_2.bool", true}, + {"level_0.level_1.level_2.level_3.string", "some string"}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "array", + .configStr = R"json({ + "array": [1, 2, 3] + })json", + .validationMap = {{"array.[]", boost::json::array{1, 2, 3}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "nested_array", + .configStr = R"json({ + "level_0": { + "array": [1, 2, 3], + "level_1": { + "array": [4, 5, 6], + "level_2": { + "array": [7, 8, 9] + } + } + } + })json", + .validationMap = + { + {"level_0.array.[]", boost::json::array{1, 2, 3}}, + {"level_0.level_1.array.[]", boost::json::array{4, 5, 6}}, + {"level_0.level_1.level_2.array.[]", boost::json::array{7, 8, 9}}, + } + }, + ConfigFileJsonParseTestBundle{ + .testName = "mixed", + .configStr = R"json({ + "int": 42, + "double": 123.456, + "bool": true, + "string": "some string", + "array": [1, 2, 3], + "nested": { + "int": 42, + "double": 123.456, + "bool": true, + "string": "some string", + "array": [1, 2, 3] + } + })json", + .validationMap = + { + {"int", 42}, + {"double", 123.456}, + {"bool", true}, + {"string", "some string"}, + {"array.[]", boost::json::array{1, 2, 3}}, + {"nested.int", 42}, + {"nested.double", 123.456}, + {"nested.bool", true}, + {"nested.string", "some string"}, + {"nested.array.[]", boost::json::array{1, 2, 3}}, + } + }, + ConfigFileJsonParseTestBundle{.testName = "empty", .configStr = R"json({})json", .validationMap = {}}, + ConfigFileJsonParseTestBundle{ + .testName = "empty_nested", + .configStr = R"json({ + "level_0": { + "level_1": { + "level_2": { + "level_3": {} + } + } + } + })json", + .validationMap = {} + }, + ConfigFileJsonParseTestBundle{ + .testName = "empty_array", + .configStr = R"json({ + "array": [] + })json", + .validationMap = {{"array.[]", boost::json::array{}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "empty_nested_array", + .configStr = R"json({ + "level_0": { + "array": [], + "level_1": { + "array": [], + "level_2": { + "array": [] + } + } + } + })json", + .validationMap = + { + {"level_0.array.[]", boost::json::array{}}, + {"level_0.level_1.array.[]", boost::json::array{}}, + {"level_0.level_1.level_2.array.[]", boost::json::array{}}, + } + }, + ConfigFileJsonParseTestBundle{ + .testName = "object_inside_array", + .configStr = R"json({ + "array": [ + { "int": 42 } + ] + })json", + .validationMap = {{"array.[].int", boost::json::array{42}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "object_with_optional_fields_inside_array", + .configStr = R"json({ + "array": [ + {"int": 42}, + {"int": 24, "bool": true} + ] + })json", + .validationMap = + {{"array.[].int", boost::json::array{42, 24}}, + {"array.[].bool", boost::json::array{boost::json::value{}, true}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "full_object_is_at_the_front_of_array", + .configStr = R"json({ + "array": [ + {"int": 42, "bool": true}, + {"int": 2}, + {"int": 4} + ] + })json", + .validationMap = + {{"array.[].int", boost::json::array{42, 2, 4}}, + {"array.[].bool", boost::json::array{true, boost::json::value{}, boost::json::value{}}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "full_object_is_in_the_middle_of_array", + .configStr = R"json({ + "array": [ + {"int": 42}, + {"int": 2, "bool": true}, + {"int": 4} + ] + })json", + .validationMap = + {{"array.[].int", boost::json::array{42, 2, 4}}, + {"array.[].bool", boost::json::array{boost::json::value{}, true, boost::json::value{}}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "no_full_object", + .configStr = R"json({ + "array": [ + {"int": 42}, + {"int": 2}, + {"bool": true} + ] + })json", + .validationMap = + {{"array.[].int", boost::json::array{42, 2, boost::json::value{}}}, + {"array.[].bool", boost::json::array{boost::json::value{}, boost::json::value{}, true}}} + }, + ConfigFileJsonParseTestBundle{ + .testName = "array_with_nexted_objects", + .configStr = R"json({ + "array": [ + { "object": { "int": 42 } }, + { "object": { "string": "some string" } } + ] + })json", + .validationMap = + {{"array.[].object.int", boost::json::array{42, boost::json::value{}}}, + {"array.[].object.string", boost::json::array{boost::json::value{}, "some string"}}} + } + ), + tests::util::kNAME_GENERATOR +); + +struct ConfigFileJsonTest : NoLoggerFixture {}; + +TEST_F(ConfigFileJsonTest, getValue) +{ + auto const jsonStr = R"json({ + "int": 42, + "object": { "string": "some string" }, + "bool": true, + "double": 123.456, + "null": null + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + + auto const intValue = jsonFileObj.getValue("int"); + ASSERT_TRUE(std::holds_alternative(intValue)); + EXPECT_EQ(std::get(intValue), 42); + + auto const stringValue = jsonFileObj.getValue("object.string"); + ASSERT_TRUE(std::holds_alternative(stringValue)); + EXPECT_EQ(std::get(stringValue), "some string"); + + auto const boolValue = jsonFileObj.getValue("bool"); + ASSERT_TRUE(std::holds_alternative(boolValue)); + EXPECT_EQ(std::get(boolValue), true); + + auto const doubleValue = jsonFileObj.getValue("double"); + ASSERT_TRUE(std::holds_alternative(doubleValue)); + EXPECT_NEAR(std::get(doubleValue), 123.456, kEPS); + + auto const nullValue = jsonFileObj.getValue("null"); + EXPECT_TRUE(std::holds_alternative(nullValue)); + + EXPECT_FALSE(jsonFileObj.containsKey("object.int")); +} + +struct ConfigFileJsonDeathTest : ConfigFileJsonTest {}; + +TEST_F(ConfigFileJsonDeathTest, getValueInvalidKey) +{ + auto const jsonFileObj = ConfigFileJson{boost::json::parse("{}").as_object()}; + EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getValue("some_key"), ".*"); +} + +TEST_F(ConfigFileJsonDeathTest, getValueOfArray) +{ + auto const jsonStr = R"json({ + "array": [1, 2, 3] + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getValue("array"), ".*"); +} + +TEST_F(ConfigFileJsonTest, getArray) +{ + auto const jsonStr = R"json({ + "array": [1, "2", 3.14, true], + "object": { "array": [3, 4] } + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + + auto const array = jsonFileObj.getArray("array.[]"); + ASSERT_EQ(array.size(), 4); + ASSERT_TRUE(std::holds_alternative(array.at(0))); + EXPECT_EQ(std::get(array.at(0)), 1); + ASSERT_TRUE(std::holds_alternative(array.at(1))); + EXPECT_EQ(std::get(array.at(1)), "2"); + ASSERT_TRUE(std::holds_alternative(array.at(2))); + EXPECT_NEAR(std::get(array.at(2)), 3.14, kEPS); + ASSERT_TRUE(std::holds_alternative(array.at(3))); + EXPECT_EQ(std::get(array.at(3)), true); + + auto const arrayFromObject = jsonFileObj.getArray("object.array.[]"); + ASSERT_EQ(arrayFromObject.size(), 2); + EXPECT_EQ(std::get(arrayFromObject.at(0)), 3); + EXPECT_EQ(std::get(arrayFromObject.at(1)), 4); +} + +TEST_F(ConfigFileJsonTest, getArrayObjectInArray) +{ + auto const jsonStr = R"json({ + "array": [ + { "int": 42 }, + { "string": "some string" } + ] + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + + auto const ints = jsonFileObj.getArray("array.[].int"); + ASSERT_EQ(ints.size(), 2); + ASSERT_TRUE(std::holds_alternative(ints.at(0))); + EXPECT_EQ(std::get(ints.at(0)), 42); + EXPECT_TRUE(std::holds_alternative(ints.at(1))); + + auto const strings = jsonFileObj.getArray("array.[].string"); + ASSERT_EQ(strings.size(), 2); + EXPECT_TRUE(std::holds_alternative(strings.at(0))); + ASSERT_TRUE(std::holds_alternative(strings.at(1))); + EXPECT_EQ(std::get(strings.at(1)), "some string"); +} + +TEST_F(ConfigFileJsonDeathTest, getArrayInvalidKey) +{ + auto const jsonFileObj = ConfigFileJson{boost::json::parse("{}").as_object()}; + EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getArray("some_key"), ".*"); +} + +TEST_F(ConfigFileJsonDeathTest, getArrayNotArray) +{ + auto const jsonStr = R"json({ + "int": 42 + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getArray("int"), ".*"); +} + +TEST_F(ConfigFileJsonTest, containsKey) +{ + auto const jsonStr = R"json({ + "int": 42, + "object": { "string": "some string", "array": [1, 2, 3] }, + "array2": [1, 2, 3], + "array_of_objects": [ {"int": 42}, {"string": "some string"} ] + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + + EXPECT_TRUE(jsonFileObj.containsKey("int")); + EXPECT_FALSE(jsonFileObj.containsKey("other_key")); + + EXPECT_TRUE(jsonFileObj.containsKey("object.string")); + EXPECT_FALSE(jsonFileObj.containsKey("object.int")); + EXPECT_TRUE(jsonFileObj.containsKey("object.array.[]")); + EXPECT_FALSE(jsonFileObj.containsKey("object.array")); + + EXPECT_TRUE(jsonFileObj.containsKey("array2.[]")); + EXPECT_FALSE(jsonFileObj.containsKey("array2")); + EXPECT_FALSE(jsonFileObj.containsKey("array2.[].int")); + + EXPECT_TRUE(jsonFileObj.containsKey("array_of_objects.[].int")); + EXPECT_TRUE(jsonFileObj.containsKey("array_of_objects.[].string")); + EXPECT_FALSE(jsonFileObj.containsKey("array_of_objects.[]")); + EXPECT_FALSE(jsonFileObj.containsKey("array_of_objects.[].object")); +} + +struct ConfigFileJsonMakeTest : ConfigFileJsonTest {}; + +TEST_F(ConfigFileJsonMakeTest, invalidFile) +{ + auto const jsonFileObj = ConfigFileJson::makeConfigFileJson("does_not_exist"); + EXPECT_FALSE(jsonFileObj.has_value()); +} + +TEST_F(ConfigFileJsonMakeTest, invalidJson) +{ + auto const file = TmpFile("invalid json"); + auto const jsonFileObj = ConfigFileJson::makeConfigFileJson(file.path); + EXPECT_FALSE(jsonFileObj.has_value()); +} + +TEST_F(ConfigFileJsonMakeTest, validFile) +{ + auto const file = TmpFile(R"json({ "int": 42 })json"); + auto const jsonFileObj = ConfigFileJson::makeConfigFileJson(file.path); + ASSERT_TRUE(jsonFileObj.has_value()); + + auto const& flatJson = jsonFileObj->inner(); + ASSERT_EQ(flatJson.size(), 1); + ASSERT_TRUE(flatJson.contains("int")); + ASSERT_TRUE(flatJson.at("int").is_number()); + EXPECT_EQ(flatJson.at("int").as_int64(), 42); +} diff --git a/tests/unit/util/newconfig/ConfigValueTests.cpp b/tests/unit/util/newconfig/ConfigValueTests.cpp index cee07ac75..8faf9dc47 100644 --- a/tests/unit/util/newconfig/ConfigValueTests.cpp +++ b/tests/unit/util/newconfig/ConfigValueTests.cpp @@ -17,62 +17,173 @@ */ //============================================================================== +#include "util/LoggerFixtures.hpp" #include "util/newconfig/ConfigConstraints.hpp" #include "util/newconfig/ConfigValue.hpp" +#include "util/newconfig/Error.hpp" #include "util/newconfig/Types.hpp" #include +#include #include #include +#include +#include #include using namespace util::config; -TEST(ConfigValue, GetSetString) +struct ConfigValueTest : NoLoggerFixture {}; +struct ConfigValueDeathTest : ConfigValueTest {}; + +TEST_F(ConfigValueTest, construct) { - auto const cvStr = ConfigValue{ConfigType::String}.defaultValue("12345"); - EXPECT_EQ(cvStr.type(), ConfigType::String); - EXPECT_TRUE(cvStr.hasValue()); - EXPECT_FALSE(cvStr.isOptional()); + ConfigValue const cv{ConfigType::Integer}; + EXPECT_FALSE(cv.hasValue()); + EXPECT_FALSE(cv.isOptional()); + EXPECT_EQ(cv.type(), ConfigType::Integer); } -TEST(ConfigValue, GetSetInteger) +TEST_F(ConfigValueTest, optional) { - auto const cvInt = ConfigValue{ConfigType::Integer}.defaultValue(543); - EXPECT_EQ(cvInt.type(), ConfigType::Integer); - EXPECT_TRUE(cvInt.hasValue()); - EXPECT_FALSE(cvInt.isOptional()); + auto const cv = ConfigValue{ConfigType::Integer}.optional(); + EXPECT_FALSE(cv.hasValue()); + EXPECT_TRUE(cv.isOptional()); + EXPECT_EQ(cv.type(), ConfigType::Integer); +} - auto const cvOpt = ConfigValue{ConfigType::Integer}.optional(); - EXPECT_TRUE(cvOpt.isOptional()); +TEST_F(ConfigValueTest, defaultValue) +{ + auto const cv = ConfigValue{ConfigType::Integer}.defaultValue(123); + EXPECT_TRUE(cv.hasValue()); + EXPECT_FALSE(cv.isOptional()); + EXPECT_EQ(cv.type(), ConfigType::Integer); +} + +TEST_F(ConfigValueDeathTest, invalidDefaultValue) +{ + EXPECT_DEATH({ [[maybe_unused]] auto const a = ConfigValue{ConfigType::String}.defaultValue(33); }, ".*"); +} + +TEST_F(ConfigValueTest, setValueNull) +{ + auto cv = ConfigValue{ConfigType::Integer}; + auto const err = cv.setValue(NullType{}); + EXPECT_TRUE(err.has_value()); +} + +TEST_F(ConfigValueTest, setValueNullOptional) +{ + auto cv = ConfigValue{ConfigType::Integer}.optional(); + auto const err = cv.setValue(NullType{}); + EXPECT_FALSE(err.has_value()); +} + +TEST_F(ConfigValueTest, setValueNullDefault) +{ + auto cv = ConfigValue{ConfigType::Integer}.defaultValue(123); + auto const err = cv.setValue(NullType{}); + EXPECT_FALSE(err.has_value()); + EXPECT_EQ(cv.getValue(), Value{123}); +} + +TEST_F(ConfigValueTest, setValueWrongType) +{ + auto cv = ConfigValue{ConfigType::Integer}; + auto const err = cv.setValue("123"); + EXPECT_TRUE(err.has_value()); +} + +TEST_F(ConfigValueTest, setValueNormalPath) +{ + auto cv = ConfigValue{ConfigType::Integer}; + auto const err = cv.setValue(123); + EXPECT_FALSE(err.has_value()); + EXPECT_EQ(cv.getValue(), Value{123}); +} + +struct ConfigValueConstraintTest : ConfigValueTest { + struct MockConstraint : Constraint { + MOCK_METHOD(std::optional, checkTypeImpl, (Value const&), (const, override)); + MOCK_METHOD(std::optional, checkValueImpl, (Value const&), (const, override)); + MOCK_METHOD(void, print, (std::ostream&), (const, override)); + }; + + testing::StrictMock constraint; +}; + +TEST_F(ConfigValueConstraintTest, setValueWithConstraint) +{ + auto cv = ConfigValue{ConfigType::Integer}.withConstraint(constraint); + auto const value = Value{123}; + EXPECT_CALL(constraint, checkTypeImpl).WillOnce(testing::Return(std::nullopt)); + EXPECT_CALL(constraint, checkValueImpl).WillOnce(testing::Return(std::nullopt)); + auto const err = cv.setValue(value); + EXPECT_FALSE(err.has_value()); + EXPECT_EQ(cv.getValue(), value); +} + +TEST_F(ConfigValueConstraintTest, setValueWithConstraintTypeCheckError) +{ + auto cv = ConfigValue{ConfigType::Integer}.withConstraint(constraint); + auto const value = 123; + EXPECT_CALL(constraint, checkTypeImpl).WillOnce(testing::Return(Error{"type error"})); + auto const err = cv.setValue(value); + EXPECT_TRUE(err.has_value()); + EXPECT_EQ(err->error, "Unknown_key type error"); +} + +TEST_F(ConfigValueConstraintTest, defaultValueWithConstraint) +{ + EXPECT_CALL(constraint, checkTypeImpl).WillOnce(testing::Return(std::nullopt)); + EXPECT_CALL(constraint, checkValueImpl).WillOnce(testing::Return(std::nullopt)); + auto const cv = ConfigValue{ConfigType::Integer}.defaultValue(123).withConstraint(constraint); + EXPECT_EQ(cv.getValue(), Value{123}); +} + +struct ConfigValueConstraintDeathTest : ConfigValueConstraintTest {}; + +TEST_F(ConfigValueConstraintDeathTest, defaultValueWithConstraintCheckError) +{ + EXPECT_DEATH( + { + EXPECT_CALL(constraint, checkTypeImpl).WillOnce(testing::Return(std::nullopt)); + EXPECT_CALL(constraint, checkValueImpl).WillOnce(testing::Return(Error{"value error"})); + [[maybe_unused]] auto const cv = + ConfigValue{ConfigType::Integer}.defaultValue(123).withConstraint(constraint); + }, + ".*" + ); } // A test for each constraint so it's easy to change in the future -TEST(ConfigValue, PortConstraint) +struct ConstraintTest : NoLoggerFixture {}; + +TEST_F(ConstraintTest, PortConstraint) { auto const portConstraint{PortConstraint{}}; EXPECT_FALSE(portConstraint.checkConstraint(4444).has_value()); EXPECT_TRUE(portConstraint.checkConstraint(99999).has_value()); } -TEST(ConfigValue, SetValuesOnPortConstraint) +TEST_F(ConstraintTest, SetValuesOnPortConstraint) { auto cvPort = ConfigValue{ConfigType::Integer}.defaultValue(4444).withConstraint(gValidatePort); auto const err = cvPort.setValue(99999); EXPECT_TRUE(err.has_value()); - EXPECT_EQ(err->error, "Port does not satisfy the constraint bounds"); + EXPECT_EQ(err->error, "Unknown_key Port does not satisfy the constraint bounds"); EXPECT_TRUE(cvPort.setValue(33.33).has_value()); - EXPECT_TRUE(cvPort.setValue(33.33).value().error == "value does not match type integer"); + EXPECT_EQ(cvPort.setValue(33.33).value().error, "Unknown_key value does not match type integer"); EXPECT_FALSE(cvPort.setValue(1).has_value()); auto cvPort2 = ConfigValue{ConfigType::String}.defaultValue("4444").withConstraint(gValidatePort); auto const strPortError = cvPort2.setValue("100000"); EXPECT_TRUE(strPortError.has_value()); - EXPECT_EQ(strPortError->error, "Port does not satisfy the constraint bounds"); + EXPECT_EQ(strPortError->error, "Unknown_key Port does not satisfy the constraint bounds"); } -TEST(ConfigValue, OneOfConstraintOneValue) +TEST_F(ConstraintTest, OneOfConstraintOneValue) { std::array const arr = {"tracer"}; auto const databaseConstraint{OneOf{"database.type", arr}}; @@ -88,7 +199,7 @@ TEST(ConfigValue, OneOfConstraintOneValue) ); } -TEST(ConfigValue, OneOfConstraint) +TEST_F(ConstraintTest, OneOfConstraint) { std::array const arr = {"123", "trace", "haha"}; auto const oneOfCons{OneOf{"log_level", arr}}; @@ -105,14 +216,14 @@ TEST(ConfigValue, OneOfConstraint) ); } -TEST(ConfigValue, IpConstraint) +TEST_F(ConstraintTest, IpConstraint) { auto ip = ConfigValue{ConfigType::String}.defaultValue("127.0.0.1").withConstraint(gValidateIp); EXPECT_FALSE(ip.setValue("http://127.0.0.1").has_value()); EXPECT_FALSE(ip.setValue("http://127.0.0.1.com").has_value()); auto const err = ip.setValue("123.44"); EXPECT_TRUE(err.has_value()); - EXPECT_EQ(err->error, "Ip is not a valid ip address"); + EXPECT_EQ(err->error, "Unknown_key Ip is not a valid ip address"); EXPECT_FALSE(ip.setValue("126.0.0.2")); EXPECT_TRUE(ip.setValue("644.3.3.0")); @@ -123,7 +234,7 @@ TEST(ConfigValue, IpConstraint) EXPECT_FALSE(ip.setValue("http://example.com:8080/path")); } -TEST(ConfigValue, positiveNumConstraint) +TEST_F(ConstraintTest, positiveNumConstraint) { auto const numCons{NumberValueConstraint{0, 5}}; EXPECT_FALSE(numCons.checkConstraint(0)); @@ -136,7 +247,7 @@ TEST(ConfigValue, positiveNumConstraint) EXPECT_EQ(numCons.checkConstraint(8)->error, fmt::format("Number must be between {} and {}", 0, 5)); } -TEST(ConfigValue, SetValuesOnNumberConstraint) +TEST_F(ConstraintTest, SetValuesOnNumberConstraint) { auto positiveNum = ConfigValue{ConfigType::Integer}.defaultValue(20u).withConstraint(gValidateUint16); auto const err = positiveNum.setValue(-22, "key"); @@ -145,7 +256,7 @@ TEST(ConfigValue, SetValuesOnNumberConstraint) EXPECT_FALSE(positiveNum.setValue(99, "key")); } -TEST(ConfigValue, PositiveDoubleConstraint) +TEST_F(ConstraintTest, PositiveDoubleConstraint) { auto const doubleCons{PositiveDouble{}}; EXPECT_FALSE(doubleCons.checkConstraint(0.2)); @@ -162,7 +273,7 @@ struct ConstraintTestBundle { Constraint const& constraint; }; -struct ConstraintDeathTest : public testing::Test, public testing::WithParamInterface {}; +struct ConstraintDeathTest : testing::TestWithParam {}; INSTANTIATE_TEST_SUITE_P( EachConstraints, @@ -195,7 +306,7 @@ TEST_P(ConstraintDeathTest, TestEachConstraint) ); } -TEST(ConfigValueDeathTest, SetInvalidValueTypeStringAndBool) +TEST(ConstraintDeathTest, SetInvalidValueTypeStringAndBool) { EXPECT_DEATH( { @@ -207,7 +318,7 @@ TEST(ConfigValueDeathTest, SetInvalidValueTypeStringAndBool) EXPECT_DEATH({ [[maybe_unused]] auto a = ConfigValue{ConfigType::Boolean}.defaultValue(-66); }, ".*"); } -TEST(ConfigValueDeathTest, OutOfBounceIntegerConstraint) +TEST(ConstraintDeathTest, OutOfBounceIntegerConstraint) { EXPECT_DEATH( { diff --git a/tests/unit/util/newconfig/JsonConfigFileTests.cpp b/tests/unit/util/newconfig/JsonConfigFileTests.cpp deleted file mode 100644 index c8c11390a..000000000 --- a/tests/unit/util/newconfig/JsonConfigFileTests.cpp +++ /dev/null @@ -1,113 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of clio: https://github.com/XRPLF/clio - Copyright (c) 2024, 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 "util/TmpFile.hpp" -#include "util/newconfig/ConfigFileJson.hpp" -#include "util/newconfig/FakeConfigData.hpp" - -#include -#include - -#include -#include -#include -#include -#include - -TEST(CreateConfigFile, filePath) -{ - auto const jsonFileObj = ConfigFileJson::makeConfigFileJson(TmpFile(kJSON_DATA).path); - EXPECT_TRUE(jsonFileObj.has_value()); - - EXPECT_TRUE(jsonFileObj->containsKey("array.[].sub")); - auto const arrSub = jsonFileObj->getArray("array.[].sub"); - EXPECT_EQ(arrSub.size(), 3); -} - -TEST(CreateConfigFile, incorrectFilePath) -{ - auto const jsonFileObj = util::config::ConfigFileJson::makeConfigFileJson("123/clio"); - EXPECT_FALSE(jsonFileObj.has_value()); -} - -struct ParseJson : testing::Test { - ParseJson() : jsonFileObj{boost::json::parse(kJSON_DATA).as_object()} - { - } - - ConfigFileJson const jsonFileObj; -}; - -TEST_F(ParseJson, validateValues) -{ - EXPECT_TRUE(jsonFileObj.containsKey("header.text1")); - EXPECT_EQ(std::get(jsonFileObj.getValue("header.text1")), "value"); - - EXPECT_TRUE(jsonFileObj.containsKey("header.sub.sub2Value")); - EXPECT_EQ(std::get(jsonFileObj.getValue("header.sub.sub2Value")), "TSM"); - - EXPECT_TRUE(jsonFileObj.containsKey("dosguard.port")); - EXPECT_EQ(std::get(jsonFileObj.getValue("dosguard.port")), 44444); - - EXPECT_FALSE(jsonFileObj.containsKey("idk")); - EXPECT_FALSE(jsonFileObj.containsKey("optional.withNoDefault")); -} - -TEST_F(ParseJson, validateArrayValue) -{ - // validate array.[].sub matches expected values - EXPECT_TRUE(jsonFileObj.containsKey("array.[].sub")); - auto const arrSub = jsonFileObj.getArray("array.[].sub"); - EXPECT_EQ(arrSub.size(), 3); - - std::vector expectedArrSubVal{111.11, 4321.55, 5555.44}; - std::vector actualArrSubVal{}; - - for (auto it = arrSub.begin(); it != arrSub.end(); ++it) { - ASSERT_TRUE(std::holds_alternative(*it)); - actualArrSubVal.emplace_back(std::get(*it)); - } - EXPECT_TRUE(std::ranges::equal(expectedArrSubVal, actualArrSubVal)); - - // validate array.[].sub2 matches expected values - EXPECT_TRUE(jsonFileObj.containsKey("array.[].sub2")); - auto const arrSub2 = jsonFileObj.getArray("array.[].sub2"); - EXPECT_EQ(arrSub2.size(), 3); - std::vector expectedArrSub2Val{"subCategory", "temporary", "london"}; - std::vector actualArrSub2Val{}; - - for (auto it = arrSub2.begin(); it != arrSub2.end(); ++it) { - ASSERT_TRUE(std::holds_alternative(*it)); - actualArrSub2Val.emplace_back(std::get(*it)); - } - EXPECT_TRUE(std::ranges::equal(expectedArrSub2Val, actualArrSub2Val)); - - EXPECT_TRUE(jsonFileObj.containsKey("dosguard.whitelist.[]")); - auto const whitelistArr = jsonFileObj.getArray("dosguard.whitelist.[]"); - EXPECT_EQ(whitelistArr.size(), 2); - EXPECT_EQ("125.5.5.1", std::get(whitelistArr.at(0))); - EXPECT_EQ("204.2.2.1", std::get(whitelistArr.at(1))); -} - -struct JsonValueDeathTest : ParseJson {}; - -TEST_F(JsonValueDeathTest, invalidGetArray) -{ - EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getArray("header.text1"), ".*"); -} diff --git a/tests/unit/util/newconfig/JsonFileTests.cpp b/tests/unit/util/newconfig/JsonFileTests.cpp index 8204004ec..94bde3122 100644 --- a/tests/unit/util/newconfig/JsonFileTests.cpp +++ b/tests/unit/util/newconfig/JsonFileTests.cpp @@ -89,9 +89,9 @@ TEST_F(JsonFromTempFile, validateArrayValue) EXPECT_EQ("204.2.2.1", std::get(whitelistArr.at(1))); } -struct JsonValueDeathTest : JsonFromTempFile {}; +struct ConfigValueJsonGetArrayDeathTest : JsonFromTempFile {}; -TEST_F(JsonValueDeathTest, invalidGetValues) +TEST_F(ConfigValueJsonGetArrayDeathTest, invalidGetValues) { // not possible for json value to call a value that doesn't exist EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getArray("header.text1"), ".*"); From 624f7ff6d566905e4345aa60e11d14f53443640c Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:00:04 -0500 Subject: [PATCH 07/29] feat: Support Simulate (#1891) fixes #1887 --- src/rpc/common/impl/ForwardingProxy.hpp | 1 + tests/unit/rpc/ForwardingProxyTests.cpp | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/rpc/common/impl/ForwardingProxy.hpp b/src/rpc/common/impl/ForwardingProxy.hpp index e4e2b32ea..c60773df1 100644 --- a/src/rpc/common/impl/ForwardingProxy.hpp +++ b/src/rpc/common/impl/ForwardingProxy.hpp @@ -116,6 +116,7 @@ public: "manifest", "channel_authorize", "channel_verify", + "simulate", }; return kPROXIED_COMMANDS.contains(method); diff --git a/tests/unit/rpc/ForwardingProxyTests.cpp b/tests/unit/rpc/ForwardingProxyTests.cpp index aadce3c7a..5626f926f 100644 --- a/tests/unit/rpc/ForwardingProxyTests.cpp +++ b/tests/unit/rpc/ForwardingProxyTests.cpp @@ -105,6 +105,22 @@ generateTestValuesForParametersTest() .called = 1, .isAdmin = !isAdmin, .expected = shouldForward}, + {.testName = "ShouldForwardReturnsTrueSimulate", + .apiVersion = 1u, + .method = "simulate", + .testJson = "{}", + .mockedIsClioOnly = !isClioOnly, + .called = 1, + .isAdmin = isAdmin, + .expected = shouldForward}, + {.testName = "ShouldForwardReturnsTrueRipplePathFind", + .apiVersion = 2u, + .method = "ripple_path_find", + .testJson = R"({"force_forward": true})", + .mockedIsClioOnly = !isClioOnly, + .called = 1, + .isAdmin = isAdmin, + .expected = shouldForward}, {.testName = "ShouldForwardReturnsTrueIfCurrentLedgerSpecified", .apiVersion = 2u, .method = "anymethod", From 3c008b6bb453485ddd6937df6da276bc364b288f Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:56:04 +0000 Subject: [PATCH 08/29] feat: Snapshot exporting tool (#1877) In this PR: 1 We create a golang grpc client to request data from rippled 2 We store the data into the specific place 3 Add unittests 4 Create build script, the build can be initiated by set conan option `snapshot` being true. Please ignore the grpc server part. It will be implemented in importing tool. --- CMakeLists.txt | 6 + conanfile.py | 3 + tools/snapshot/CMakeLists.txt | 66 +++++ tools/snapshot/go.mod | 23 ++ tools/snapshot/go.sum | 69 +++++ tools/snapshot/internal/export/export.go | 245 +++++++++++++++++ tools/snapshot/internal/export/export_test.go | 153 +++++++++++ .../internal/parse_args/parse_args.go | 63 +++++ .../internal/parse_args/parse_args_test.go | 94 +++++++ tools/snapshot/mocks/mock_grpc_server.go | 247 ++++++++++++++++++ tools/snapshot/snapshot.go | 23 ++ 11 files changed, 992 insertions(+) create mode 100644 tools/snapshot/CMakeLists.txt create mode 100644 tools/snapshot/go.mod create mode 100644 tools/snapshot/go.sum create mode 100644 tools/snapshot/internal/export/export.go create mode 100644 tools/snapshot/internal/export/export_test.go create mode 100644 tools/snapshot/internal/parse_args/parse_args.go create mode 100644 tools/snapshot/internal/parse_args/parse_args_test.go create mode 100644 tools/snapshot/mocks/mock_grpc_server.go create mode 100644 tools/snapshot/snapshot.go diff --git a/CMakeLists.txt b/CMakeLists.txt index c31647f1d..27b1c3d55 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,6 +16,8 @@ option(coverage "Build test coverage report" FALSE) option(packaging "Create distribution packages" FALSE) option(lint "Run clang-tidy checks during compilation" FALSE) option(static "Statically linked Clio" FALSE) +option(snapshot "Build snapshot tool" FALSE) + # ========================================================================== # set(san "" CACHE STRING "Add sanitizer instrumentation") set(CMAKE_EXPORT_COMPILE_COMMANDS TRUE) @@ -91,3 +93,7 @@ include(install/install) if (packaging) include(cmake/packaging.cmake) # This file exists only in build runner endif () + +if (snapshot) + add_subdirectory(tools/snapshot) +endif () diff --git a/conanfile.py b/conanfile.py index ee1d02598..040e3b9fe 100644 --- a/conanfile.py +++ b/conanfile.py @@ -19,6 +19,7 @@ class Clio(ConanFile): 'packaging': [True, False], # create distribution packages 'coverage': [True, False], # build for test coverage report; create custom target `clio_tests-ccov` 'lint': [True, False], # run clang-tidy checks during compilation + 'snapshot': [True, False], # build export/import snapshot tool } requires = [ @@ -44,6 +45,7 @@ class Clio(ConanFile): 'coverage': False, 'lint': False, 'docs': False, + 'snapshot': False, 'xrpl/*:tests': False, 'xrpl/*:rocksdb': False, @@ -92,6 +94,7 @@ class Clio(ConanFile): tc.variables['docs'] = self.options.docs tc.variables['packaging'] = self.options.packaging tc.variables['benchmark'] = self.options.benchmark + tc.variables['snapshot'] = self.options.snapshot tc.generate() def build(self): diff --git a/tools/snapshot/CMakeLists.txt b/tools/snapshot/CMakeLists.txt new file mode 100644 index 000000000..d8bdd8e68 --- /dev/null +++ b/tools/snapshot/CMakeLists.txt @@ -0,0 +1,66 @@ +# Build snapshot tool +# Need to be installed before building: go protoc-gen-go protoc-gen-go-grpc + +set(GO_EXECUTABLE "go") +set(GO_SOURCE_DIR "${CMAKE_SOURCE_DIR}/tools/snapshot") +set(PROTO_INC_DIR "${xrpl_PACKAGE_FOLDER_RELEASE}/include/xrpl/proto") +set(PROTO_SOURCE_DIR "${PROTO_INC_DIR}/org/xrpl/rpc/v1/") +set(GO_OUTPUT "${CMAKE_BINARY_DIR}/clio_snapshot") + +set(PROTO_FILES + ${PROTO_SOURCE_DIR}/xrp_ledger.proto ${PROTO_SOURCE_DIR}/ledger.proto ${PROTO_SOURCE_DIR}/get_ledger.proto + ${PROTO_SOURCE_DIR}/get_ledger_entry.proto ${PROTO_SOURCE_DIR}/get_ledger_data.proto + ${PROTO_SOURCE_DIR}/get_ledger_diff.proto +) + +execute_process(COMMAND go env GOPATH OUTPUT_VARIABLE GOPATH_VALUE OUTPUT_STRIP_TRAILING_WHITESPACE) + +# Target Go package path +set(GO_IMPORT_PATH "org/xrpl/rpc/v1") + +# Initialize the options strings +set(GO_OPTS "") +set(GRPC_OPTS "") + +# Loop through each proto file +foreach(proto ${PROTO_FILES}) + get_filename_component(proto_filename ${proto} NAME) + + # Build the --go_opt and --go-grpc_opt mappings + set(GO_OPTS ${GO_OPTS} --go_opt=M${GO_IMPORT_PATH}/${proto_filename}=${GO_IMPORT_PATH}) + set(GRPC_OPTS ${GRPC_OPTS} --go-grpc_opt=M${GO_IMPORT_PATH}/${proto_filename}=${GO_IMPORT_PATH}) +endforeach() + +foreach (proto ${PROTO_FILES}) + get_filename_component(proto_name ${proto} NAME_WE) + add_custom_command( + OUTPUT ${GO_SOURCE_DIR}/${proto_name}.pb.go + COMMAND + protoc ${GO_OPTS} ${GRPC_OPTS} + --go-grpc_out=${GO_SOURCE_DIR} -I${PROTO_INC_DIR} ${proto} --plugin=${GOPATH_VALUE}/bin/protoc-gen-go + --plugin=${GOPATH_VALUE}/bin/protoc-gen-go-grpc --go_out=${GO_SOURCE_DIR}/ + DEPENDS ${proto} + COMMENT "Generating Go code for ${proto}" + VERBATIM + ) + + list(APPEND GENERATED_GO_FILES ${GO_SOURCE_DIR}/${proto_name}.pb.go) +endforeach () + +add_custom_target(build_clio_snapshot ALL DEPENDS run_go_tests ${GO_OUTPUT}) + +add_custom_target(run_go_tests + COMMAND go test ./... + WORKING_DIRECTORY ${GO_SOURCE_DIR} + COMMENT "Running clio_snapshot unittests" + VERBATIM + DEPENDS ${GENERATED_GO_FILES} +) + +add_custom_command( + OUTPUT ${GO_OUTPUT} + COMMAND ${GO_EXECUTABLE} build -o ${GO_OUTPUT} ${GO_SOURCE_DIR} + WORKING_DIRECTORY ${GO_SOURCE_DIR} + COMMENT "Building clio_snapshot" + VERBATIM +) diff --git a/tools/snapshot/go.mod b/tools/snapshot/go.mod new file mode 100644 index 000000000..a7d92ef3a --- /dev/null +++ b/tools/snapshot/go.mod @@ -0,0 +1,23 @@ +module xrplf/clio/clio_snapshot + +go 1.22 + +toolchain go1.22.11 + +require ( + github.com/golang/mock v1.6.0 + github.com/spf13/pflag v1.0.6 + github.com/stretchr/testify v1.10.0 + google.golang.org/grpc v1.69.4 + google.golang.org/protobuf v1.36.3 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + golang.org/x/net v0.30.0 // indirect + golang.org/x/sys v0.26.0 // indirect + golang.org/x/text v0.19.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20241015192408-796eee8c2d53 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/tools/snapshot/go.sum b/tools/snapshot/go.sum new file mode 100644 index 000000000..6b29896c8 --- /dev/null +++ b/tools/snapshot/go.sum @@ -0,0 +1,69 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= +github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +go.opentelemetry.io/otel v1.31.0 h1:NsJcKPIW0D0H3NgzPDHmo0WW6SptzPdqg/L1zsIm2hY= +go.opentelemetry.io/otel v1.31.0/go.mod h1:O0C14Yl9FgkjqcCZAsE053C13OaddMYr/hz6clDkEJE= +go.opentelemetry.io/otel/metric v1.31.0 h1:FSErL0ATQAmYHUIzSezZibnyVlft1ybhy4ozRPcF2fE= +go.opentelemetry.io/otel/metric v1.31.0/go.mod h1:C3dEloVbLuYoX41KpmAhOqNriGbA+qqH6PQ5E5mUfnY= +go.opentelemetry.io/otel/sdk v1.31.0 h1:xLY3abVHYZ5HSfOg3l2E5LUj2Cwva5Y7yGxnSW9H5Gk= +go.opentelemetry.io/otel/sdk v1.31.0/go.mod h1:TfRbMdhvxIIr/B2N2LQW2S5v9m3gOQ/08KsbbO5BPT0= +go.opentelemetry.io/otel/sdk/metric v1.31.0 h1:i9hxxLJF/9kkvfHppyLL55aW7iIJz4JjxTeYusH7zMc= +go.opentelemetry.io/otel/sdk/metric v1.31.0/go.mod h1:CRInTMVvNhUKgSAMbKyTMxqOBC0zgyxzW55lZzX43Y8= +go.opentelemetry.io/otel/trace v1.31.0 h1:ffjsj1aRouKewfr85U2aGagJ46+MvodynlQ1HYdmJys= +go.opentelemetry.io/otel/trace v1.31.0/go.mod h1:TXZkRk7SM2ZQLtR6eoAWQFIHPvzQ06FJAsO1tJg480A= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= +golang.org/x/net v0.30.0 h1:AcW1SDZMkb8IpzCdQUaIq2sP4sZ4zw+55h6ynffypl4= +golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= +golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= +golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241015192408-796eee8c2d53 h1:X58yt85/IXCx0Y3ZwN6sEIKZzQtDEYaBWrDvErdXrRE= +google.golang.org/genproto/googleapis/rpc v0.0.0-20241015192408-796eee8c2d53/go.mod h1:GX3210XPVPUjJbTUbvwI8f2IpZDMZuPJWDzDuebbviI= +google.golang.org/grpc v1.69.4 h1:MF5TftSMkd8GLw/m0KM6V8CMOCY6NZ1NQDPGFgbTt4A= +google.golang.org/grpc v1.69.4/go.mod h1:vyjdE6jLBI76dgpDojsFGNaHlxdjXN9ghpnd2o7JGZ4= +google.golang.org/protobuf v1.36.3 h1:82DV7MYdb8anAVi3qge1wSnMDrnKK7ebr+I0hHRN1BU= +google.golang.org/protobuf v1.36.3/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/tools/snapshot/internal/export/export.go b/tools/snapshot/internal/export/export.go new file mode 100644 index 000000000..264d9bd2a --- /dev/null +++ b/tools/snapshot/internal/export/export.go @@ -0,0 +1,245 @@ +package export + +import ( + "context" + "fmt" + "log" + "os" + "path/filepath" + "sync" + pb "xrplf/clio/clio_snapshot/org/xrpl/rpc/v1" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/protobuf/proto" +) + +const ( + deltaDataFolderDiv = 10000 + grpcUser = "clio-snapshot" + markerNum = 16 +) + +type gRPCClient struct { + Client pb.XRPLedgerAPIServiceClient + conn *grpc.ClientConn +} + +func (c *gRPCClient) Close() error { + return c.conn.Close() +} + +func createGRPCClient(serverAddr string) (*gRPCClient, error) { + opts := []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + + conn, err := grpc.NewClient(serverAddr, opts...) + if err != nil { + return nil, fmt.Errorf("Failed to dial: %v", err) + } + + client := pb.NewXRPLedgerAPIServiceClient(conn) + return &gRPCClient{ + Client: client, + conn: conn, + }, nil +} + +func getLedgerDeltaData(client pb.XRPLedgerAPIServiceClient, seq uint32, path string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + request := pb.GetLedgerRequest{} + ledger := &pb.LedgerSpecifier{ + Ledger: &pb.LedgerSpecifier_Sequence{ + Sequence: seq, + }, + } + request.Ledger = ledger + request.User = grpcUser + request.GetObjectNeighbors = true + request.Transactions = true + request.Expand = true + request.GetObjects = true + + response, err := client.GetLedger(ctx, &request) + + if err != nil { + log.Fatalf("Error getting ledger data: %v", err) + } + + saveLedgerDeltaData(seq, response, path) + + log.Printf("Processing delta sequence: %d\n", seq) +} + +func roundDown(n uint32, roundTo uint32) uint32 { + if roundTo == 0 { + return n + } + return n - (n % roundTo) +} + +func saveLedgerDeltaData(seq uint32, response *pb.GetLedgerResponse, path string) { + subPath := filepath.Join(path, fmt.Sprintf("ledger_diff_%d", roundDown(seq, deltaDataFolderDiv))) + err := os.MkdirAll(subPath, os.ModePerm) + if err != nil { + log.Fatalf("Error creating directory: %v", err) + } + + protoData, err := proto.Marshal(response) + if err != nil { + log.Fatalf("Error marshalling data: %v", err) + } + + filePath := filepath.Join(subPath, fmt.Sprintf("%d.dat", seq)) + + err = os.WriteFile(filePath, protoData, 0644) + if err != nil { + log.Fatalf("failed to write file: %v", err) + } +} + +func generateMarkers(markerNum uint32) [][32]byte { + var byteArray [32]byte + + incr := 256 / markerNum + + var byteArrayList [][32]byte + + for i := 0; i < int(markerNum); i++ { + byteArray[0] = byte(i * int(incr)) // Increment the highest byte + byteArrayList = append(byteArrayList, byteArray) + } + + return byteArrayList +} + +func saveLedgerData(path string, data *pb.GetLedgerDataResponse) { + protoData, err := proto.Marshal(data) + if err != nil { + log.Fatalf("Error marshalling data: %v", err) + } + + err = os.WriteFile(path, protoData, 0644) + if err != nil { + log.Fatalf("failed to write file: %v", err) + } +} + +func getLedgerData(client pb.XRPLedgerAPIServiceClient, seq uint32, marker []byte, end []byte, path string) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + request := pb.GetLedgerDataRequest{} + ledger := &pb.LedgerSpecifier{ + Ledger: &pb.LedgerSpecifier_Sequence{ + Sequence: seq, + }, + } + request.Ledger = ledger + request.Marker = marker[:] + if end != nil { + request.EndMarker = end[:] + } + request.User = grpcUser + + subPath := filepath.Join(path, fmt.Sprintf("ledger_data_%d", seq), fmt.Sprintf("marker_%x", marker)) + err := os.MkdirAll(subPath, os.ModePerm) + if err != nil { + log.Fatalf("Error creating directory: %v", err) + } + + for request.Marker != nil { + res, err := client.GetLedgerData(ctx, &request) + if err != nil { + log.Fatalf("Error getting ledger data: %v", err) + } + + filePath := filepath.Join(subPath, fmt.Sprintf("%x.dat", request.Marker)) + saveLedgerData(filePath, res) + request.Marker = res.Marker + } +} + +func getLedgerFullData(client pb.XRPLedgerAPIServiceClient, seq uint32, path string) { + log.Printf("Processing full sequence: %d\n", seq) + + markers := generateMarkers(markerNum) + + var wg sync.WaitGroup + + // Launch a goroutine for each marker + for i, marker := range markers { + wg.Add(1) + var end []byte + if i != len(markers)-1 { + end = markers[i+1][:] + } + + fmt.Printf("Got ledger data marker: %x-%x\n", marker, end) + + go func() { + defer wg.Done() + getLedgerData(client, seq, marker[:], end, path) + }() + + } + + wg.Wait() +} + +func checkPath(path string) { + if _, err := os.Stat(path); os.IsNotExist(err) { + // Create the directory if it doesn't exist + err := os.MkdirAll(path, os.ModePerm) + if err != nil { + log.Fatalf("Error creating directory: %v", err) + } + } +} + +func ExportFromFullLedger(grpcServer string, startSeq uint32, endSeq uint32, path string) { + checkPath(path) + + client, err := createGRPCClient(grpcServer) + if err != nil { + log.Fatalf("Error creating gRPC client: %v", err) + } + + defer client.Close() + + exportFromFullLedgerImpl(client.Client, startSeq, endSeq, path) +} + +func exportFromFullLedgerImpl(client pb.XRPLedgerAPIServiceClient, startSeq uint32, endSeq uint32, path string) { + + getLedgerFullData(client, startSeq, path) + + //We need to fetch the ledger header and txs for startSeq as well + for i := startSeq; i <= endSeq; i++ { + getLedgerDeltaData(client, i, path) + } + + log.Printf("Exporting from full ledger: %d to %d at path %s\n", startSeq, endSeq, path) +} + +func ExportFromDeltaLedger(grpcServer string, startSeq uint32, endSeq uint32, path string) { + checkPath(path) + + client, err := createGRPCClient(grpcServer) + if err != nil { + log.Fatalf("Error creating gRPC client: %v", err) + } + + defer client.Close() + + exportFromDeltaLedgerImpl(client.Client, startSeq, endSeq, path) +} + +func exportFromDeltaLedgerImpl(client pb.XRPLedgerAPIServiceClient, startSeq uint32, endSeq uint32, path string) { + for i := startSeq; i <= endSeq; i++ { + getLedgerDeltaData(client, i, path) + } + + log.Printf("Exporting from ledger: %d to %d at path %s\n", startSeq, endSeq, path) +} diff --git a/tools/snapshot/internal/export/export_test.go b/tools/snapshot/internal/export/export_test.go new file mode 100644 index 000000000..08d4c79aa --- /dev/null +++ b/tools/snapshot/internal/export/export_test.go @@ -0,0 +1,153 @@ +package export + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + + "xrplf/clio/clio_snapshot/mocks" + pb "xrplf/clio/clio_snapshot/org/xrpl/rpc/v1" + + "github.com/golang/mock/gomock" +) + +func TestExportDeltaLedgerData(t *testing.T) { + tests := []struct { + name string + startSeq uint32 + endSeq uint32 + }{ + {"OneSeq", 1, 1}, + {"MultipleSeq", 1, 20}, + {"EndSeqLessThanStartSeq", 20, 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockXRPLedgerAPIServiceClient(ctrl) + + mockResponse := &pb.GetLedgerResponse{} + + times := tt.endSeq - tt.startSeq + 1 + if tt.endSeq < tt.startSeq { + times = 0 + } + + mockClient.EXPECT().GetLedger(gomock.Any(), gomock.Any()).Return(mockResponse, nil).Times(int(times)) + + defer os.RemoveAll("test") + + exportFromDeltaLedgerImpl(mockClient, tt.startSeq, tt.endSeq, "test") + + _, err := os.Stat("test") + + assert.Equal(t, os.IsNotExist(err), tt.endSeq < tt.startSeq) + }) + } +} + +func TestExportFullLedgerData(t *testing.T) { + tests := []struct { + name string + startSeq uint32 + endSeq uint32 + }{ + {"OneSeq", 1, 1}, + {"MultipleSeq", 1, 20}, + {"EndSeqLessThanStartSeq", 20, 1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockXRPLedgerAPIServiceClient(ctrl) + + mockDataResponse := &pb.GetLedgerDataResponse{} + mockLedgerResponse := &pb.GetLedgerResponse{} + + timesLedgerDataCalled := 16 + timesLedgerCalled := tt.endSeq - tt.startSeq + 1 + + if tt.endSeq < tt.startSeq { + timesLedgerCalled = 0 + } + + mockClient.EXPECT().GetLedgerData(gomock.Any(), gomock.Any()).Return(mockDataResponse, nil).Times(timesLedgerDataCalled) + + mockClient.EXPECT().GetLedger(gomock.Any(), gomock.Any()).Return(mockLedgerResponse, nil).Times(int(timesLedgerCalled)) + + defer os.RemoveAll("test") + exportFromFullLedgerImpl(mockClient, tt.startSeq, tt.endSeq, "test") + + _, err := os.Stat("test") + + assert.False(t, os.IsNotExist(err)) + }) + } +} + +func TestRoundDown(t *testing.T) { + tests := []struct { + name string + in1 uint32 + in2 uint32 + out uint32 + }{ + {"RoundDownToZero", 10, 0, 10}, + {"RoundDown12To10", 12, 10, 10}, + {"RoundDownToOne", 13, 1, 13}, + {"RoundDown100", 103, 100, 100}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, roundDown(tt.in1, tt.in2), tt.out) + }) + } +} + +func TestGenerateMarkers(t *testing.T) { + tests := []struct { + name string + in uint32 + out [][32]byte + }{ + {"GenerateMarkers1", 1, [][32]byte{{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}}}, + {"GenerateMarkers2", 2, [][32]byte{{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + {0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}}}, + {"GenerateMarkers4", 4, [][32]byte{{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + {0x40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + {0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + {0xc0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, generateMarkers(tt.in), tt.out) + }) + } +} + +func TestCheckPath(t *testing.T) { + tests := []struct { + name string + path string + }{ + {"Path", "test"}, + {"NestedPath", "test/test"}} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + checkPath(tt.path) + defer os.RemoveAll(tt.path) + _, err := os.Stat(tt.path) + assert.False(t, os.IsNotExist(err)) + }) + } + +} diff --git a/tools/snapshot/internal/parse_args/parse_args.go b/tools/snapshot/internal/parse_args/parse_args.go new file mode 100644 index 000000000..c41cbf014 --- /dev/null +++ b/tools/snapshot/internal/parse_args/parse_args.go @@ -0,0 +1,63 @@ +package parse_args + +import ( + "fmt" + "os" + + flag "github.com/spf13/pflag" +) + +type Args struct { + ExportMode string + StartSeq uint32 + EndSeq uint32 + Path string + GrpcServer string + ServerMode bool + GrpcPort uint32 + WsPort uint32 +} + +func Parse() (*Args, error) { + + fs := flag.NewFlagSet(os.Args[0], flag.ContinueOnError) + fs.Usage = PrintUsage + + exportMode := fs.StringP("export", "e", "", "Set export mode: 'full' (export full ledger data of start_seq) or 'delta' (only export ledger diff data)") + seq := fs.Uint32("start_seq", 0, "Starting sequence number") + endSeq := fs.Uint32("end_seq", 0, "Ending sequence number") + path := fs.StringP("path", "p", "", "Path to the data") + grpcServer := fs.StringP("grpc_server", "g", "localhost:50051", "rippled's gRPC server address") + serverMode := fs.BoolP("server", "s", false, "Start server mode") + grpcPort := fs.Uint32("grpc_port", 0, "Port for gRPC server to listen on") + wsPort := fs.Uint32("ws_port", 0, "Port for WebSocket server to listen on") + fs.Parse(os.Args[1:]) + + if *serverMode && *exportMode != "" { + return nil, fmt.Errorf("Invalid usage: --server and --export cannot be used at the same time.") + } + + if *serverMode { + if *grpcPort == 0 || *wsPort == 0 || *path == "" { + return nil, fmt.Errorf("Invalid usage: --grpc_port and --ws_port and --path are required for server mode.") + } + } else if *exportMode != "" { + if *exportMode == "full" || *exportMode == "delta" { + if *seq == 0 || *endSeq == 0 || *path == "" || *grpcServer == "" { + return nil, fmt.Errorf("Invalid usage: --start_seq, --end_seq, --grpc_server and --path are required for export") + } + } else { + return nil, fmt.Errorf("Invalid usage: Invalid export mode. Use 'full' or 'delta'.") + } + } else { + return nil, fmt.Errorf("Invalid usage: --export or --server flag is required.") + } + + return &Args{*exportMode, *seq, *endSeq, *path, *grpcServer, *serverMode, *grpcPort, *wsPort}, nil +} + +func PrintUsage() { + fmt.Println("Usage: clio_snapshot [options]") + fmt.Println("Options:") + flag.PrintDefaults() +} diff --git a/tools/snapshot/internal/parse_args/parse_args_test.go b/tools/snapshot/internal/parse_args/parse_args_test.go new file mode 100644 index 000000000..83db3451b --- /dev/null +++ b/tools/snapshot/internal/parse_args/parse_args_test.go @@ -0,0 +1,94 @@ +package parse_args + +import ( + "flag" + "fmt" + "os" + "reflect" + "testing" +) + +func TestParse(t *testing.T) { + tests := []struct { + name string + args []string + want *Args + expectErr bool + errMessage string + }{ + { + name: "Valid export full mode", + args: []string{"cmd", "--export=full", "--start_seq=1", "--end_seq=10", "--path=/data", "--grpc_server=localhost:50051"}, + want: &Args{ + ExportMode: "full", + StartSeq: 1, + EndSeq: 10, + Path: "/data", + GrpcServer: "localhost:50051", + ServerMode: false, + }, + expectErr: false, + }, + { + name: "Missing required flags in export mode", + args: []string{"cmd", "--export=delta", "--start_seq=1"}, + want: nil, + expectErr: true, + errMessage: "Invalid usage: --start_seq, --end_seq, --grpc_server and --path are required for export", + }, + { + name: "Invalid export mode", + args: []string{"cmd", "--export=invalid"}, + want: nil, + expectErr: true, + errMessage: "Invalid usage: Invalid export mode. Use 'full' or 'delta'.", + }, + { + name: "Server mode with default grpc server flags", + args: []string{"cmd", "--server", "--ws_port=1234", "--grpc_port=22", "--path=/server_data"}, + want: &Args{ + ServerMode: true, + GrpcPort: 22, + WsPort: 1234, + StartSeq: 0, + EndSeq: 0, + Path: "/server_data", + GrpcServer: "localhost:50051", + }, + expectErr: false, + }, + { + name: "Server and export mode together (error)", + args: []string{"cmd", "--server", "--export=full"}, + want: nil, + expectErr: true, + errMessage: "Invalid usage: --server and --export cannot be used at the same time.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + flag.CommandLine = flag.NewFlagSet(os.Args[1], flag.ExitOnError) + + os.Args = tt.args + + fmt.Println("Running test with args:", tt.args) + got, err := Parse() + + if tt.expectErr { + if err == nil { + t.Errorf("Expected error but got none") + } else if err.Error() != tt.errMessage { + t.Errorf("Expected error message '%s', got '%s'", tt.errMessage, err.Error()) + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Expected %+v, got %+v", tt.want, got) + } + } + }) + } +} diff --git a/tools/snapshot/mocks/mock_grpc_server.go b/tools/snapshot/mocks/mock_grpc_server.go new file mode 100644 index 000000000..3ef0d2121 --- /dev/null +++ b/tools/snapshot/mocks/mock_grpc_server.go @@ -0,0 +1,247 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: org/xrpl/rpc/v1/xrp_ledger_grpc.pb.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + v1 "xrplf/clio/clio_snapshot/org/xrpl/rpc/v1" + + gomock "github.com/golang/mock/gomock" + grpc "google.golang.org/grpc" +) + +// MockXRPLedgerAPIServiceClient is a mock of XRPLedgerAPIServiceClient interface. +type MockXRPLedgerAPIServiceClient struct { + ctrl *gomock.Controller + recorder *MockXRPLedgerAPIServiceClientMockRecorder +} + +// MockXRPLedgerAPIServiceClientMockRecorder is the mock recorder for MockXRPLedgerAPIServiceClient. +type MockXRPLedgerAPIServiceClientMockRecorder struct { + mock *MockXRPLedgerAPIServiceClient +} + +// NewMockXRPLedgerAPIServiceClient creates a new mock instance. +func NewMockXRPLedgerAPIServiceClient(ctrl *gomock.Controller) *MockXRPLedgerAPIServiceClient { + mock := &MockXRPLedgerAPIServiceClient{ctrl: ctrl} + mock.recorder = &MockXRPLedgerAPIServiceClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockXRPLedgerAPIServiceClient) EXPECT() *MockXRPLedgerAPIServiceClientMockRecorder { + return m.recorder +} + +// GetLedger mocks base method. +func (m *MockXRPLedgerAPIServiceClient) GetLedger(ctx context.Context, in *v1.GetLedgerRequest, opts ...grpc.CallOption) (*v1.GetLedgerResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetLedger", varargs...) + ret0, _ := ret[0].(*v1.GetLedgerResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedger indicates an expected call of GetLedger. +func (mr *MockXRPLedgerAPIServiceClientMockRecorder) GetLedger(ctx, in interface{}, opts ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedger", reflect.TypeOf((*MockXRPLedgerAPIServiceClient)(nil).GetLedger), varargs...) +} + +// GetLedgerData mocks base method. +func (m *MockXRPLedgerAPIServiceClient) GetLedgerData(ctx context.Context, in *v1.GetLedgerDataRequest, opts ...grpc.CallOption) (*v1.GetLedgerDataResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetLedgerData", varargs...) + ret0, _ := ret[0].(*v1.GetLedgerDataResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerData indicates an expected call of GetLedgerData. +func (mr *MockXRPLedgerAPIServiceClientMockRecorder) GetLedgerData(ctx, in interface{}, opts ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerData", reflect.TypeOf((*MockXRPLedgerAPIServiceClient)(nil).GetLedgerData), varargs...) +} + +// GetLedgerDiff mocks base method. +func (m *MockXRPLedgerAPIServiceClient) GetLedgerDiff(ctx context.Context, in *v1.GetLedgerDiffRequest, opts ...grpc.CallOption) (*v1.GetLedgerDiffResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetLedgerDiff", varargs...) + ret0, _ := ret[0].(*v1.GetLedgerDiffResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerDiff indicates an expected call of GetLedgerDiff. +func (mr *MockXRPLedgerAPIServiceClientMockRecorder) GetLedgerDiff(ctx, in interface{}, opts ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerDiff", reflect.TypeOf((*MockXRPLedgerAPIServiceClient)(nil).GetLedgerDiff), varargs...) +} + +// GetLedgerEntry mocks base method. +func (m *MockXRPLedgerAPIServiceClient) GetLedgerEntry(ctx context.Context, in *v1.GetLedgerEntryRequest, opts ...grpc.CallOption) (*v1.GetLedgerEntryResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetLedgerEntry", varargs...) + ret0, _ := ret[0].(*v1.GetLedgerEntryResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerEntry indicates an expected call of GetLedgerEntry. +func (mr *MockXRPLedgerAPIServiceClientMockRecorder) GetLedgerEntry(ctx, in interface{}, opts ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerEntry", reflect.TypeOf((*MockXRPLedgerAPIServiceClient)(nil).GetLedgerEntry), varargs...) +} + +// MockXRPLedgerAPIServiceServer is a mock of XRPLedgerAPIServiceServer interface. +type MockXRPLedgerAPIServiceServer struct { + ctrl *gomock.Controller + recorder *MockXRPLedgerAPIServiceServerMockRecorder +} + +// MockXRPLedgerAPIServiceServerMockRecorder is the mock recorder for MockXRPLedgerAPIServiceServer. +type MockXRPLedgerAPIServiceServerMockRecorder struct { + mock *MockXRPLedgerAPIServiceServer +} + +// NewMockXRPLedgerAPIServiceServer creates a new mock instance. +func NewMockXRPLedgerAPIServiceServer(ctrl *gomock.Controller) *MockXRPLedgerAPIServiceServer { + mock := &MockXRPLedgerAPIServiceServer{ctrl: ctrl} + mock.recorder = &MockXRPLedgerAPIServiceServerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockXRPLedgerAPIServiceServer) EXPECT() *MockXRPLedgerAPIServiceServerMockRecorder { + return m.recorder +} + +// GetLedger mocks base method. +func (m *MockXRPLedgerAPIServiceServer) GetLedger(arg0 context.Context, arg1 *v1.GetLedgerRequest) (*v1.GetLedgerResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLedger", arg0, arg1) + ret0, _ := ret[0].(*v1.GetLedgerResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedger indicates an expected call of GetLedger. +func (mr *MockXRPLedgerAPIServiceServerMockRecorder) GetLedger(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedger", reflect.TypeOf((*MockXRPLedgerAPIServiceServer)(nil).GetLedger), arg0, arg1) +} + +// GetLedgerData mocks base method. +func (m *MockXRPLedgerAPIServiceServer) GetLedgerData(arg0 context.Context, arg1 *v1.GetLedgerDataRequest) (*v1.GetLedgerDataResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLedgerData", arg0, arg1) + ret0, _ := ret[0].(*v1.GetLedgerDataResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerData indicates an expected call of GetLedgerData. +func (mr *MockXRPLedgerAPIServiceServerMockRecorder) GetLedgerData(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerData", reflect.TypeOf((*MockXRPLedgerAPIServiceServer)(nil).GetLedgerData), arg0, arg1) +} + +// GetLedgerDiff mocks base method. +func (m *MockXRPLedgerAPIServiceServer) GetLedgerDiff(arg0 context.Context, arg1 *v1.GetLedgerDiffRequest) (*v1.GetLedgerDiffResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLedgerDiff", arg0, arg1) + ret0, _ := ret[0].(*v1.GetLedgerDiffResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerDiff indicates an expected call of GetLedgerDiff. +func (mr *MockXRPLedgerAPIServiceServerMockRecorder) GetLedgerDiff(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerDiff", reflect.TypeOf((*MockXRPLedgerAPIServiceServer)(nil).GetLedgerDiff), arg0, arg1) +} + +// GetLedgerEntry mocks base method. +func (m *MockXRPLedgerAPIServiceServer) GetLedgerEntry(arg0 context.Context, arg1 *v1.GetLedgerEntryRequest) (*v1.GetLedgerEntryResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLedgerEntry", arg0, arg1) + ret0, _ := ret[0].(*v1.GetLedgerEntryResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetLedgerEntry indicates an expected call of GetLedgerEntry. +func (mr *MockXRPLedgerAPIServiceServerMockRecorder) GetLedgerEntry(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLedgerEntry", reflect.TypeOf((*MockXRPLedgerAPIServiceServer)(nil).GetLedgerEntry), arg0, arg1) +} + +// mustEmbedUnimplementedXRPLedgerAPIServiceServer mocks base method. +func (m *MockXRPLedgerAPIServiceServer) mustEmbedUnimplementedXRPLedgerAPIServiceServer() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "mustEmbedUnimplementedXRPLedgerAPIServiceServer") +} + +// mustEmbedUnimplementedXRPLedgerAPIServiceServer indicates an expected call of mustEmbedUnimplementedXRPLedgerAPIServiceServer. +func (mr *MockXRPLedgerAPIServiceServerMockRecorder) mustEmbedUnimplementedXRPLedgerAPIServiceServer() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "mustEmbedUnimplementedXRPLedgerAPIServiceServer", reflect.TypeOf((*MockXRPLedgerAPIServiceServer)(nil).mustEmbedUnimplementedXRPLedgerAPIServiceServer)) +} + +// MockUnsafeXRPLedgerAPIServiceServer is a mock of UnsafeXRPLedgerAPIServiceServer interface. +type MockUnsafeXRPLedgerAPIServiceServer struct { + ctrl *gomock.Controller + recorder *MockUnsafeXRPLedgerAPIServiceServerMockRecorder +} + +// MockUnsafeXRPLedgerAPIServiceServerMockRecorder is the mock recorder for MockUnsafeXRPLedgerAPIServiceServer. +type MockUnsafeXRPLedgerAPIServiceServerMockRecorder struct { + mock *MockUnsafeXRPLedgerAPIServiceServer +} + +// NewMockUnsafeXRPLedgerAPIServiceServer creates a new mock instance. +func NewMockUnsafeXRPLedgerAPIServiceServer(ctrl *gomock.Controller) *MockUnsafeXRPLedgerAPIServiceServer { + mock := &MockUnsafeXRPLedgerAPIServiceServer{ctrl: ctrl} + mock.recorder = &MockUnsafeXRPLedgerAPIServiceServerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUnsafeXRPLedgerAPIServiceServer) EXPECT() *MockUnsafeXRPLedgerAPIServiceServerMockRecorder { + return m.recorder +} + +// mustEmbedUnimplementedXRPLedgerAPIServiceServer mocks base method. +func (m *MockUnsafeXRPLedgerAPIServiceServer) mustEmbedUnimplementedXRPLedgerAPIServiceServer() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "mustEmbedUnimplementedXRPLedgerAPIServiceServer") +} + +// mustEmbedUnimplementedXRPLedgerAPIServiceServer indicates an expected call of mustEmbedUnimplementedXRPLedgerAPIServiceServer. +func (mr *MockUnsafeXRPLedgerAPIServiceServerMockRecorder) mustEmbedUnimplementedXRPLedgerAPIServiceServer() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "mustEmbedUnimplementedXRPLedgerAPIServiceServer", reflect.TypeOf((*MockUnsafeXRPLedgerAPIServiceServer)(nil).mustEmbedUnimplementedXRPLedgerAPIServiceServer)) +} diff --git a/tools/snapshot/snapshot.go b/tools/snapshot/snapshot.go new file mode 100644 index 000000000..2ee464f69 --- /dev/null +++ b/tools/snapshot/snapshot.go @@ -0,0 +1,23 @@ +package main + +import ( + "log" + + "xrplf/clio/clio_snapshot/internal/export" + "xrplf/clio/clio_snapshot/internal/parse_args" +) + +func main() { + args, err := parse_args.Parse() + if err != nil { + log.Fatal(err) + } + + if args.ExportMode == "full" { + export.ExportFromFullLedger(args.GrpcServer, args.StartSeq, args.EndSeq, args.Path) + + } else if args.ExportMode == "delta" { + export.ExportFromDeltaLedger(args.GrpcServer, args.StartSeq, args.EndSeq, args.Path) + } + //TODO: Implement server mode +} From 531e1dad6d0abca30641e6f12db6bc89712e33f4 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Fri, 14 Feb 2025 16:54:08 +0000 Subject: [PATCH 09/29] ci: Upload cache only for develop branch (#1897) --- .github/workflows/build_impl.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_impl.yml b/.github/workflows/build_impl.yml index 69b754a38..db4a7edd0 100644 --- a/.github/workflows/build_impl.yml +++ b/.github/workflows/build_impl.yml @@ -164,7 +164,7 @@ jobs: path: build/clio_integration_tests - name: Save cache - if: ${{ !inputs.disable_cache }} + if: ${{ !inputs.disable_cache && github.ref == 'refs/heads/develop' }} uses: ./.github/actions/save_cache with: conan_dir: ${{ env.CONAN_USER_HOME }}/.conan From fcebd715ba7344348f7086605254949b3a65e485 Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Fri, 14 Feb 2025 13:00:40 -0500 Subject: [PATCH 10/29] test: add non-admin test for simulate (#1893) --- tests/unit/rpc/ForwardingProxyTests.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/unit/rpc/ForwardingProxyTests.cpp b/tests/unit/rpc/ForwardingProxyTests.cpp index 5626f926f..c63263712 100644 --- a/tests/unit/rpc/ForwardingProxyTests.cpp +++ b/tests/unit/rpc/ForwardingProxyTests.cpp @@ -105,7 +105,15 @@ generateTestValuesForParametersTest() .called = 1, .isAdmin = !isAdmin, .expected = shouldForward}, - {.testName = "ShouldForwardReturnsTrueSimulate", + {.testName = "ShouldForwardReturnsTrueIsNotAdminSimulate", + .apiVersion = 1u, + .method = "simulate", + .testJson = "{}", + .mockedIsClioOnly = !isClioOnly, + .called = 1, + .isAdmin = !isAdmin, + .expected = shouldForward}, + {.testName = "ShouldForwardReturnsTrueIsAdminSimulate", .apiVersion = 1u, .method = "simulate", .testJson = "{}", @@ -113,7 +121,15 @@ generateTestValuesForParametersTest() .called = 1, .isAdmin = isAdmin, .expected = shouldForward}, - {.testName = "ShouldForwardReturnsTrueRipplePathFind", + {.testName = "ShouldForwardReturnsTrueIsNotAdminRipplePathFind", + .apiVersion = 2u, + .method = "ripple_path_find", + .testJson = R"({"force_forward": true})", + .mockedIsClioOnly = !isClioOnly, + .called = 1, + .isAdmin = !isAdmin, + .expected = shouldForward}, + {.testName = "ShouldForwardReturnsTrueIsAdminRipplePathFind", .apiVersion = 2u, .method = "ripple_path_find", .testJson = R"({"force_forward": true})", From 4b178805dec208f6a0596b5c107f3d1c2f978c91 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 18 Feb 2025 13:29:43 +0000 Subject: [PATCH 11/29] fix: Array parsing in new config (#1896) Improving array parsing in config: - Allow null values in arrays for optional fields - Allow empty array even for required field - Allow to not put an empty array in config even if array contains required fields --- src/util/newconfig/Array.cpp | 19 ++- src/util/newconfig/Array.hpp | 12 +- src/util/newconfig/ConfigDefinition.cpp | 56 ++++++- src/util/newconfig/ConfigFileInterface.hpp | 5 +- src/util/newconfig/ConfigFileJson.cpp | 15 +- src/util/newconfig/ConfigFileJson.hpp | 4 +- src/util/newconfig/ConfigValue.hpp | 17 --- src/util/newconfig/Types.hpp | 34 +---- tests/unit/LoggerTests.cpp | 14 +- tests/unit/util/newconfig/ArrayTests.cpp | 43 +++++- .../newconfig/ClioConfigDefinitionTests.cpp | 140 +++++++++++++++++- .../util/newconfig/ConfigFileJsonTests.cpp | 79 +++++++--- .../unit/util/newconfig/ConfigValueTests.cpp | 22 --- 13 files changed, 328 insertions(+), 132 deletions(-) diff --git a/src/util/newconfig/Array.cpp b/src/util/newconfig/Array.cpp index 46cbe1ee3..46c2b31eb 100644 --- a/src/util/newconfig/Array.cpp +++ b/src/util/newconfig/Array.cpp @@ -48,13 +48,22 @@ Array::prefix(std::string_view key) std::optional Array::addValue(Value value, std::optional key) { - auto const constraint = itemPattern_.getConstraint(); + auto newItem = itemPattern_; - auto newElem = constraint.has_value() ? ConfigValue{itemPattern_.type()}.withConstraint(constraint->get()) - : ConfigValue{itemPattern_.type()}; - if (auto const maybeError = newElem.setValue(value, key); maybeError.has_value()) + if (auto const maybeError = newItem.setValue(value, key); maybeError.has_value()) return maybeError; - elements_.emplace_back(std::move(newElem)); + elements_.emplace_back(std::move(newItem)); + return std::nullopt; +} + +std::optional +Array::addNull(std::optional key) +{ + if (not itemPattern_.isOptional() and not itemPattern_.hasValue()) { + return Error{key.value_or("Unknown_key"), "value for the array (or object field inside array) is required"}; + } + + elements_.push_back(itemPattern_); return std::nullopt; } diff --git a/src/util/newconfig/Array.hpp b/src/util/newconfig/Array.hpp index ca8b4cf09..71e946aab 100644 --- a/src/util/newconfig/Array.hpp +++ b/src/util/newconfig/Array.hpp @@ -65,9 +65,19 @@ public: * @param key optional string key to include that will show in error message * @return optional error if adding config value to array fails. nullopt otherwise */ - std::optional + [[nodiscard]] std::optional addValue(Value value, std::optional key = std::nullopt); + /** + * @brief Add null value to the array + * @note The error will be returned if item pattern of the array is neither optional nor has a default value + * + * @param key An optional key which will be used for error message only (if any) + * @return An error if any or nullopt if the operation succeeded + */ + [[nodiscard]] std::optional + addNull(std::optional key = std::nullopt); + /** * @brief Returns the number of values stored in the Array * diff --git a/src/util/newconfig/ConfigDefinition.cpp b/src/util/newconfig/ConfigDefinition.cpp index bbfff1d94..a041d9412 100644 --- a/src/util/newconfig/ConfigDefinition.cpp +++ b/src/util/newconfig/ConfigDefinition.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -157,16 +158,19 @@ std::optional> ClioConfigDefinition::parse(ConfigFileInterface const& config) { std::vector listOfErrors; + std::unordered_map> arrayPrefixesToKeysMap; for (auto& [key, value] : map_) { + if (key.contains(".[]")) { + auto const prefix = Array::prefix(key); + arrayPrefixesToKeysMap[prefix].push_back(key); + } + // if key doesn't exist in user config, makes sure it is marked as ".optional()" or has ".defaultValue()"" in - // ClioConfigDefitinion above + // ClioConfigDefinition above if (!config.containsKey(key)) { if (std::holds_alternative(value)) { if (!(std::get(value).isOptional() || std::get(value).hasValue())) listOfErrors.emplace_back(key, "key is required in user Config"); - } else if (std::holds_alternative(value)) { - if (!(std::get(value).getArrayPattern().isOptional())) - listOfErrors.emplace_back(key, "key is required in user Config"); } continue; } @@ -179,21 +183,59 @@ ClioConfigDefinition::parse(ConfigFileInterface const& config) // attempt to set the value from the configuration for the specified key. [&key, &config, &listOfErrors](ConfigValue& val) { if (auto const maybeError = val.setValue(config.getValue(key), key); - maybeError.has_value()) + maybeError.has_value()) { listOfErrors.emplace_back(maybeError.value()); + } }, // handle the case where the config value is an array. // iterate over each provided value in the array and attempt to set it for the key. [&key, &config, &listOfErrors](Array& arr) { for (auto const& val : config.getArray(key)) { - if (auto const maybeError = arr.addValue(val, key); maybeError.has_value()) - listOfErrors.emplace_back(maybeError.value()); + if (val.has_value()) { + if (auto const maybeError = arr.addValue(*val, key); maybeError.has_value()) { + listOfErrors.emplace_back(*maybeError); + } + } else { + if (auto const maybeError = arr.addNull(key); maybeError.has_value()) { + listOfErrors.emplace_back(*maybeError); + } + } } } }, value ); } + + if (!listOfErrors.empty()) + return listOfErrors; + + // The code above couldn't detect whether some fields in an array are missing. + // So to fix it for each array we determine it's size and add empty values if the field is optional + // or generate an error. + for (auto const& [_, keys] : arrayPrefixesToKeysMap) { + size_t maxSize = 0; + std::ranges::for_each(keys, [&](std::string_view key) { + ASSERT(std::holds_alternative(map_.at(key)), "{} is not array", key); + maxSize = std::max(maxSize, arraySize(key)); + }); + if (maxSize == 0) { + // empty arrays are allowed + continue; + } + + std::ranges::for_each(keys, [&](std::string_view key) { + auto& array = std::get(map_.at(key)); + while (array.size() < maxSize) { + auto const err = array.addNull(key); + if (err.has_value()) { + listOfErrors.emplace_back(*err); + break; + } + } + }); + } + if (!listOfErrors.empty()) return listOfErrors; diff --git a/src/util/newconfig/ConfigFileInterface.hpp b/src/util/newconfig/ConfigFileInterface.hpp index a26dda043..4749fe4d9 100644 --- a/src/util/newconfig/ConfigFileInterface.hpp +++ b/src/util/newconfig/ConfigFileInterface.hpp @@ -21,6 +21,7 @@ #include "util/newconfig/Types.hpp" +#include #include #include @@ -49,9 +50,9 @@ public: * @brief Retrieves an array of configuration values. * * @param key The key of the configuration array. - * @return A vector of configuration values if found, otherwise std::nullopt. + * @return A vector of configuration values some of which could be nullopt */ - virtual std::vector + virtual std::vector> getArray(std::string_view key) const = 0; /** diff --git a/src/util/newconfig/ConfigFileJson.cpp b/src/util/newconfig/ConfigFileJson.cpp index 976523bee..a9d50fe52 100644 --- a/src/util/newconfig/ConfigFileJson.cpp +++ b/src/util/newconfig/ConfigFileJson.cpp @@ -74,9 +74,6 @@ extractJsonValue(boost::json::value const& jsonValue) if (jsonValue.is_double()) { return jsonValue.as_double(); } - if (jsonValue.is_null()) { - return NullType{}; - } ASSERT(false, "Json is not of type null, int, uint, string, bool or double"); std::unreachable(); } @@ -119,18 +116,22 @@ ConfigFileJson::getValue(std::string_view key) const return value; } -std::vector +std::vector> ConfigFileJson::getArray(std::string_view key) const { ASSERT(containsKey(key), "Key {} not found in ConfigFileJson", key); ASSERT(jsonObject_.at(key).is_array(), "Key {} has value that is not an array", key); - std::vector configValues; + std::vector> configValues; auto const arr = jsonObject_.at(key).as_array(); for (auto const& item : arr) { - auto value = extractJsonValue(item); - configValues.emplace_back(std::move(value)); + if (item.is_null()) { + configValues.emplace_back(std::nullopt); + } else { + auto value = extractJsonValue(item); + configValues.emplace_back(std::move(value)); + } } return configValues; } diff --git a/src/util/newconfig/ConfigFileJson.hpp b/src/util/newconfig/ConfigFileJson.hpp index a21b18f2b..11eb92e10 100644 --- a/src/util/newconfig/ConfigFileJson.hpp +++ b/src/util/newconfig/ConfigFileJson.hpp @@ -27,7 +27,7 @@ #include #include -#include +#include #include #include @@ -61,7 +61,7 @@ public: * @param key The key of the configuration array to retrieve. * @return A vector of variants holding the config values specified by user. */ - [[nodiscard]] std::vector + [[nodiscard]] std::vector> getArray(std::string_view key) const override; /** diff --git a/src/util/newconfig/ConfigValue.hpp b/src/util/newconfig/ConfigValue.hpp index d1a50193c..b8422d298 100644 --- a/src/util/newconfig/ConfigValue.hpp +++ b/src/util/newconfig/ConfigValue.hpp @@ -34,7 +34,6 @@ #include #include #include -#include #include namespace util::config { @@ -81,21 +80,6 @@ public: [[nodiscard]] std::optional setValue(Value value, std::optional key = std::nullopt) { - if (std::holds_alternative(value)) { - if (hasValue()) { - // Using default value - return std::nullopt; - } - if (not isOptional()) { - return Error{ - key.value_or("Unknown_key"), - "Provided value is null but ConfigValue is not optional and doesn't have a default value." - }; - } - value_ = std::move(value); - return std::nullopt; - } - auto err = checkTypeConsistency(type_, value); if (err.has_value()) { err->error = fmt::format("{} {}", key.value_or("Unknown_key"), err->error); @@ -142,7 +126,6 @@ public: [&type](std::string const& tmp) { type = fmt::format("string {}", tmp); }, [&type](double tmp) { type = fmt::format("double {}", tmp); }, [&type](int64_t tmp) { type = fmt::format("int {}", tmp); }, - [&type](NullType) { type = "null"; }, }, value_.value() ); diff --git a/src/util/newconfig/Types.hpp b/src/util/newconfig/Types.hpp index 4e24514c4..7cb1bd4c4 100644 --- a/src/util/newconfig/Types.hpp +++ b/src/util/newconfig/Types.hpp @@ -32,23 +32,7 @@ namespace util::config { /** @brief Custom clio config types */ -enum class ConfigType { Integer, String, Double, Boolean, Null }; - -/** - * @brief A type that represents a null value - */ -struct NullType { - /** - * @brief Compare two NullType objects - * - * @return true always. Any two NullType objects are equal - */ - [[nodiscard]] bool - operator==(NullType const&) const - { - return true; - } -}; +enum class ConfigType { Integer, String, Double, Boolean }; /** * @brief Prints the specified config type to output stream @@ -61,7 +45,7 @@ std::ostream& operator<<(std::ostream& stream, ConfigType type); /** @brief Represents the supported Config Values */ -using Value = std::variant; +using Value = std::variant; /** * @brief Prints the specified value to output stream @@ -91,8 +75,6 @@ getType() return ConfigType::Double; } else if constexpr (std::is_same_v) { return ConfigType::Boolean; - } else if constexpr (std::is_same_v) { - return ConfigType::Null; } else { static_assert(util::Unsupported, "Wrong config type"); } @@ -100,15 +82,3 @@ getType() } // namespace util::config -/** @cond */ -// Doxygen could not parse this -template <> -struct fmt::formatter : fmt::formatter { - [[nodiscard]] - auto - format(util::config::NullType const&, fmt::format_context& ctx) - { - return fmt::formatter::format("null", ctx); - } -}; -/** @endcond */ diff --git a/tests/unit/LoggerTests.cpp b/tests/unit/LoggerTests.cpp index a8f986bf5..0a31c592a 100644 --- a/tests/unit/LoggerTests.cpp +++ b/tests/unit/LoggerTests.cpp @@ -80,8 +80,8 @@ using util::config::ConfigValue; struct LoggerInitTest : LoggerTest { protected: util::config::ClioConfigDefinition config_{ - {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}.optional()}}, - {"log_channels.[].log_level", Array{ConfigValue{ConfigType::String}.optional()}}, + {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}}}, + {"log_channels.[].log_level", Array{ConfigValue{ConfigType::String}}}, {"log_level", ConfigValue{ConfigType::String}.defaultValue("info")}, @@ -181,9 +181,15 @@ TEST_F(LoggerInitTest, LogSizeAndHourRotationCannotBeZero) "log_rotation_hour_interval", "log_directory_max_size", "log_rotation_size" }; + auto const jsonStr = fmt::format(R"json({{ + "{}": 0, + "{}": 0, + "{}": 0 + }})json", keys[0], keys[1], keys[2]); + auto const parsingErrors = - config_.parse(ConfigFileJson{boost::json::object{{keys[0], 0}, {keys[1], 0}, {keys[2], 0}}}); - ASSERT_TRUE(parsingErrors->size() == 3); + config_.parse(ConfigFileJson{boost::json::parse(jsonStr).as_object()}); + ASSERT_EQ(parsingErrors->size(), 3); for (std::size_t i = 0; i < parsingErrors->size(); ++i) { EXPECT_EQ( (*parsingErrors)[i].error, diff --git a/tests/unit/util/newconfig/ArrayTests.cpp b/tests/unit/util/newconfig/ArrayTests.cpp index 59de95b67..52a2804f1 100644 --- a/tests/unit/util/newconfig/ArrayTests.cpp +++ b/tests/unit/util/newconfig/ArrayTests.cpp @@ -46,16 +46,16 @@ TEST(ArrayDeathTest, prefix) TEST(ArrayTest, addSingleValue) { auto arr = Array{ConfigValue{ConfigType::Double}}; - arr.addValue(111.11); + ASSERT_FALSE(arr.addValue(111.11)); EXPECT_EQ(arr.size(), 1); } TEST(ArrayTest, addAndCheckMultipleValues) { auto arr = Array{ConfigValue{ConfigType::Double}}; - arr.addValue(111.11); - arr.addValue(222.22); - arr.addValue(333.33); + ASSERT_FALSE(arr.addValue(111.11)); + ASSERT_FALSE(arr.addValue(222.22)); + ASSERT_FALSE(arr.addValue(333.33)); EXPECT_EQ(arr.size(), 3); auto const cv = arr.at(0); @@ -67,7 +67,7 @@ TEST(ArrayTest, addAndCheckMultipleValues) EXPECT_EQ(vv2.asDouble(), 222.22); EXPECT_EQ(arr.size(), 3); - arr.addValue(444.44); + ASSERT_FALSE(arr.addValue(444.44)); EXPECT_EQ(arr.size(), 4); auto const cv4 = arr.at(3); @@ -88,7 +88,7 @@ TEST(ArrayTest, iterateValueArray) std::vector const expected{543, 123, 909}; for (auto const num : expected) - arr.addValue(num); + ASSERT_FALSE(arr.addValue(num)); std::vector actual; for (auto it = arr.begin(); it != arr.end(); ++it) @@ -96,3 +96,34 @@ TEST(ArrayTest, iterateValueArray) EXPECT_TRUE(std::ranges::equal(expected, actual)); } + +TEST(ArrayTest, addNullOptional) +{ + Array arr{ConfigValue{ConfigType::Integer}.optional()}; + ASSERT_FALSE(arr.addNull()); + ASSERT_FALSE(arr.addValue(1)); + + ASSERT_EQ(arr.size(), 2); + EXPECT_FALSE(arr.at(0).hasValue()); + EXPECT_TRUE(arr.at(1).hasValue()); + EXPECT_EQ(std::get(arr.at(1).getValue()), 1); +} + +TEST(ArrayTest, addNullDefault) +{ + Array arr{ConfigValue{ConfigType::Integer}.defaultValue(42)}; + ASSERT_FALSE(arr.addNull()); + ASSERT_FALSE(arr.addValue(1)); + + ASSERT_EQ(arr.size(), 2); + EXPECT_EQ(std::get(arr.at(0).getValue()), 42); + EXPECT_EQ(std::get(arr.at(1).getValue()), 1); +} + +TEST(ArrayTest, addNullRequired) +{ + Array arr{ConfigValue{ConfigType::Integer}}; + auto const error = arr.addNull(); + EXPECT_TRUE(error.has_value()); +} + diff --git a/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp b/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp index b2358a2e2..40733969d 100644 --- a/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp +++ b/tests/unit/util/newconfig/ClioConfigDefinitionTests.cpp @@ -17,10 +17,13 @@ */ //============================================================================== +#include "util/LoggerFixtures.hpp" +#include "util/newconfig/Array.hpp" #include "util/newconfig/ArrayView.hpp" #include "util/newconfig/ConfigDefinition.hpp" #include "util/newconfig/ConfigDescription.hpp" #include "util/newconfig/ConfigFileJson.hpp" +#include "util/newconfig/ConfigValue.hpp" #include "util/newconfig/FakeConfigData.hpp" #include "util/newconfig/Types.hpp" #include "util/newconfig/ValueView.hpp" @@ -29,10 +32,12 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -304,19 +309,144 @@ TEST_F(IncorrectOverrideValues, InvalidJsonErrors) EXPECT_TRUE(errors.has_value()); // Expected error messages - std::unordered_set const expectedErrors{ + std::set const expectedErrors{ "dosguard.whitelist.[] value does not match type string", - "higher.[].low.section key is required in user Config", - "higher.[].low.admin key is required in user Config", - "array.[].sub key is required in user Config", "header.port value does not match type integer", "header.admin value does not match type boolean", "optional.withDefault value does not match type double" }; - std::unordered_set actualErrors; + std::set actualErrors; for (auto const& error : errors.value()) { actualErrors.insert(error.error); } EXPECT_EQ(expectedErrors, actualErrors); } + +struct ClioConfigDefinitionParseArrayTest : NoLoggerFixture { + ClioConfigDefinition config{ + {"array.[].int", Array{ConfigValue{ConfigType::Integer}}}, + {"array.[].string", Array{ConfigValue{ConfigType::String}.optional()}} + }; +}; + +TEST_F(ClioConfigDefinitionParseArrayTest, emptyArray) +{ + auto const configJson = boost::json::parse(R"json({ + "array": [] + })json").as_object(); + + auto const result = config.parse(ConfigFileJson{configJson}); + EXPECT_FALSE(result.has_value()); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, emptyJson) +{ + auto const configJson = boost::json::object{}; + + auto const result = config.parse(ConfigFileJson{configJson}); + EXPECT_FALSE(result.has_value()); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, fullArray) +{ + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"int": 1, "string": "one"}, + {"int": 2, "string": "two"} + ] + })json").as_object(); + + auto const result = config.parse(ConfigFileJson{configJson}); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(config.arraySize("array.[]"), 2); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, onlyRequiredFields) { + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"int": 1}, + {"int": 2} + ] + })json").as_object(); + + auto const configFile = ConfigFileJson{configJson}; + auto const result = config.parse(configFile); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(config.arraySize("array.[]"), 2); + + EXPECT_EQ(config.getArray("array.[].int").valueAt(0).asIntType(), 1); + EXPECT_EQ(config.getArray("array.[].int").valueAt(1).asIntType(), 2); + EXPECT_FALSE(config.getArray("array.[].string").valueAt(0).hasValue()); + EXPECT_FALSE(config.getArray("array.[].string").valueAt(1).hasValue()); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, someOptionalFieldsMissing) +{ + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"int": 1, "string": "one"}, + {"int": 2} + ] + })json").as_object(); + + auto const configFile = ConfigFileJson{configJson}; + auto const result = config.parse(configFile); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(config.arraySize("array.[]"), 2); + + EXPECT_EQ(config.getArray("array.[].int").valueAt(0).asIntType(), 1); + EXPECT_EQ(config.getArray("array.[].int").valueAt(1).asIntType(), 2); + EXPECT_EQ(config.getArray("array.[].string").valueAt(0).asString(), "one"); + EXPECT_FALSE(config.getArray("array.[].string").valueAt(1).hasValue()); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, optionalFieldMissingAtFirstPosition) { + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"int": 1}, + {"int": 2, "string": "two"} + ] + })json").as_object(); + + auto const configFile = ConfigFileJson{configJson}; + auto const result = config.parse(configFile); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(config.arraySize("array.[]"), 2); + + EXPECT_EQ(config.getArray("array.[].int").valueAt(0).asIntType(), 1); + EXPECT_EQ(config.getArray("array.[].int").valueAt(1).asIntType(), 2); + + EXPECT_FALSE(config.getArray("array.[].string").valueAt(0).hasValue()); + EXPECT_EQ(config.getArray("array.[].string").valueAt(1).asString(), "two"); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, missingRequiredFields) { + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"int": 1}, + {"string": "two"} + ] + })json").as_object(); + + auto const configFile = ConfigFileJson{configJson}; + auto const result = config.parse(configFile); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->size(), 1); + EXPECT_THAT(result->at(0).error, testing::StartsWith("array.[].int")); +} + +TEST_F(ClioConfigDefinitionParseArrayTest, missingAllRequiredFields) { + auto const configJson = boost::json::parse(R"json({ + "array": [ + {"string": "one"}, + {"string": "two"} + ] + })json").as_object(); + + auto const configFile = ConfigFileJson{configJson}; + auto const result = config.parse(configFile); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->size(), 1); + EXPECT_THAT(result->at(0).error, testing::StartsWith("array.[].int")); +} diff --git a/tests/unit/util/newconfig/ConfigFileJsonTests.cpp b/tests/unit/util/newconfig/ConfigFileJsonTests.cpp index 5ff6cd7e9..b7f2693de 100644 --- a/tests/unit/util/newconfig/ConfigFileJsonTests.cpp +++ b/tests/unit/util/newconfig/ConfigFileJsonTests.cpp @@ -22,7 +22,6 @@ #include "util/OverloadSet.hpp" #include "util/TmpFile.hpp" #include "util/newconfig/ConfigFileJson.hpp" -#include "util/newconfig/Types.hpp" #include #include @@ -316,8 +315,7 @@ TEST_F(ConfigFileJsonTest, getValue) "int": 42, "object": { "string": "some string" }, "bool": true, - "double": 123.456, - "null": null + "double": 123.456 })json"; auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; @@ -337,9 +335,6 @@ TEST_F(ConfigFileJsonTest, getValue) ASSERT_TRUE(std::holds_alternative(doubleValue)); EXPECT_NEAR(std::get(doubleValue), 123.456, kEPS); - auto const nullValue = jsonFileObj.getValue("null"); - EXPECT_TRUE(std::holds_alternative(nullValue)); - EXPECT_FALSE(jsonFileObj.containsKey("object.int")); } @@ -360,6 +355,15 @@ TEST_F(ConfigFileJsonDeathTest, getValueOfArray) EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getValue("array"), ".*"); } +TEST_F(ConfigFileJsonDeathTest, nullIsNotSupported) +{ + auto const jsonStr = R"json({ + "null": null + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + EXPECT_DEATH([[maybe_unused]] auto a = jsonFileObj.getValue("null"), ".*"); +} + TEST_F(ConfigFileJsonTest, getArray) { auto const jsonStr = R"json({ @@ -370,19 +374,27 @@ TEST_F(ConfigFileJsonTest, getArray) auto const array = jsonFileObj.getArray("array.[]"); ASSERT_EQ(array.size(), 4); - ASSERT_TRUE(std::holds_alternative(array.at(0))); - EXPECT_EQ(std::get(array.at(0)), 1); - ASSERT_TRUE(std::holds_alternative(array.at(1))); - EXPECT_EQ(std::get(array.at(1)), "2"); - ASSERT_TRUE(std::holds_alternative(array.at(2))); - EXPECT_NEAR(std::get(array.at(2)), 3.14, kEPS); - ASSERT_TRUE(std::holds_alternative(array.at(3))); - EXPECT_EQ(std::get(array.at(3)), true); + + auto const value0 = array.at(0).value(); + ASSERT_TRUE(std::holds_alternative(value0)); + EXPECT_EQ(std::get(value0), 1); + + auto const value1 = array.at(1).value(); + ASSERT_TRUE(std::holds_alternative(value1)); + EXPECT_EQ(std::get(value1), "2"); + + auto const value2 = array.at(2).value(); + ASSERT_TRUE(std::holds_alternative(value2)); + EXPECT_NEAR(std::get(value2), 3.14, kEPS); + + auto const value3 = array.at(3).value(); + ASSERT_TRUE(std::holds_alternative(value3)); + EXPECT_EQ(std::get(value3), true); auto const arrayFromObject = jsonFileObj.getArray("object.array.[]"); ASSERT_EQ(arrayFromObject.size(), 2); - EXPECT_EQ(std::get(arrayFromObject.at(0)), 3); - EXPECT_EQ(std::get(arrayFromObject.at(1)), 4); + EXPECT_EQ(std::get(arrayFromObject.at(0).value()), 3); + EXPECT_EQ(std::get(arrayFromObject.at(1).value()), 4); } TEST_F(ConfigFileJsonTest, getArrayObjectInArray) @@ -397,15 +409,38 @@ TEST_F(ConfigFileJsonTest, getArrayObjectInArray) auto const ints = jsonFileObj.getArray("array.[].int"); ASSERT_EQ(ints.size(), 2); - ASSERT_TRUE(std::holds_alternative(ints.at(0))); - EXPECT_EQ(std::get(ints.at(0)), 42); - EXPECT_TRUE(std::holds_alternative(ints.at(1))); + ASSERT_TRUE(std::holds_alternative(ints.at(0).value())); + EXPECT_EQ(std::get(ints.at(0).value()), 42); + EXPECT_FALSE(ints.at(1).has_value()); auto const strings = jsonFileObj.getArray("array.[].string"); ASSERT_EQ(strings.size(), 2); - EXPECT_TRUE(std::holds_alternative(strings.at(0))); - ASSERT_TRUE(std::holds_alternative(strings.at(1))); - EXPECT_EQ(std::get(strings.at(1)), "some string"); + EXPECT_FALSE(strings.at(0).has_value()); + ASSERT_TRUE(std::holds_alternative(strings.at(1).value())); + EXPECT_EQ(std::get(strings.at(1).value()), "some string"); +} + +TEST_F(ConfigFileJsonTest, getArrayOptionalInArray) { + auto const jsonStr = R"json({ + "array": [ + { "int": 42 }, + { "int": 24, "bool": true } + ] + })json"; + auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()}; + + auto const ints = jsonFileObj.getArray("array.[].int"); + ASSERT_EQ(ints.size(), 2); + ASSERT_TRUE(std::holds_alternative(ints.at(0).value())); + EXPECT_EQ(std::get(ints.at(0).value()), 42); + ASSERT_TRUE(std::holds_alternative(ints.at(1).value())); + EXPECT_EQ(std::get(ints.at(1).value()), 24); + + auto const bools = jsonFileObj.getArray("array.[].bool"); + ASSERT_EQ(bools.size(), 2); + EXPECT_FALSE(bools.at(0).has_value()); + ASSERT_TRUE(std::holds_alternative(bools.at(1).value())); + EXPECT_EQ(std::get(bools.at(1).value()), true); } TEST_F(ConfigFileJsonDeathTest, getArrayInvalidKey) diff --git a/tests/unit/util/newconfig/ConfigValueTests.cpp b/tests/unit/util/newconfig/ConfigValueTests.cpp index 8faf9dc47..04c9ad705 100644 --- a/tests/unit/util/newconfig/ConfigValueTests.cpp +++ b/tests/unit/util/newconfig/ConfigValueTests.cpp @@ -66,28 +66,6 @@ TEST_F(ConfigValueDeathTest, invalidDefaultValue) EXPECT_DEATH({ [[maybe_unused]] auto const a = ConfigValue{ConfigType::String}.defaultValue(33); }, ".*"); } -TEST_F(ConfigValueTest, setValueNull) -{ - auto cv = ConfigValue{ConfigType::Integer}; - auto const err = cv.setValue(NullType{}); - EXPECT_TRUE(err.has_value()); -} - -TEST_F(ConfigValueTest, setValueNullOptional) -{ - auto cv = ConfigValue{ConfigType::Integer}.optional(); - auto const err = cv.setValue(NullType{}); - EXPECT_FALSE(err.has_value()); -} - -TEST_F(ConfigValueTest, setValueNullDefault) -{ - auto cv = ConfigValue{ConfigType::Integer}.defaultValue(123); - auto const err = cv.setValue(NullType{}); - EXPECT_FALSE(err.has_value()); - EXPECT_EQ(cv.getValue(), Value{123}); -} - TEST_F(ConfigValueTest, setValueWrongType) { auto cv = ConfigValue{ConfigType::Integer}; From 25296f8ffa865ad965cbd08bba075d8e95641986 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Tue, 18 Feb 2025 15:43:13 +0000 Subject: [PATCH 12/29] fix: Better errors on logger init failure (#1857) Fixes #1326. --- src/main/Main.cpp | 10 +++++-- src/util/log/Logger.cpp | 26 +++++++++++------ src/util/log/Logger.hpp | 4 ++- src/util/newconfig/ConfigDefinition.hpp | 4 +-- tests/unit/LoggerTests.cpp | 38 +++++++++++++++++++++++-- 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/main/Main.cpp b/src/main/Main.cpp index 4dd39e712..6d1ff8e11 100644 --- a/src/main/Main.cpp +++ b/src/main/Main.cpp @@ -52,7 +52,10 @@ try { if (not app::parseConfig(run.configPath)) return EXIT_FAILURE; - util::LogService::init(gClioConfig); + if (auto const initSuccess = util::LogService::init(gClioConfig); not initSuccess) { + std::cerr << initSuccess.error() << std::endl; + return EXIT_FAILURE; + } app::ClioApplication clio{gClioConfig}; return clio.run(run.useNgWebServer); }, @@ -60,7 +63,10 @@ try { if (not app::parseConfig(migrate.configPath)) return EXIT_FAILURE; - util::LogService::init(gClioConfig); + if (auto const initSuccess = util::LogService::init(gClioConfig); not initSuccess) { + std::cerr << initSuccess.error() << std::endl; + return EXIT_FAILURE; + } app::MigratorApplication migrator{gClioConfig, migrate.subCmd}; return migrator.run(); } diff --git a/src/util/log/Logger.cpp b/src/util/log/Logger.cpp index 3f9a35dda..13d57174a 100644 --- a/src/util/log/Logger.cpp +++ b/src/util/log/Logger.cpp @@ -28,8 +28,6 @@ #include #include -#include -#include #include #include #include @@ -48,18 +46,20 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include #include -#include #include #include +#include #include #include @@ -111,7 +111,7 @@ getSeverityLevel(std::string_view logLevel) std::unreachable(); } -void +std::expected LogService::init(config::ClioConfigDefinition const& config) { namespace keywords = boost::log::keywords; @@ -132,9 +132,15 @@ LogService::init(config::ClioConfigDefinition const& config) auto const logDir = config.maybeValue("log_directory"); if (logDir) { - boost::filesystem::path dirPath{logDir.value()}; - if (!boost::filesystem::exists(dirPath)) - boost::filesystem::create_directories(dirPath); + std::filesystem::path dirPath{logDir.value()}; + if (not std::filesystem::exists(dirPath)) { + if (std::error_code error; not std::filesystem::create_directories(dirPath, error)) { + return std::unexpected{ + fmt::format("Couldn't create logs directory '{}': {}", dirPath.string(), error.message()) + }; + } + } + auto const rotationPeriod = config.get("log_rotation_hour_interval"); // the below are taken from user in MB, but boost::log::add_file_log needs it to be in bytes @@ -169,8 +175,9 @@ LogService::init(config::ClioConfigDefinition const& config) for (auto it = overrides.begin(); it != overrides.end(); ++it) { auto const& channelConfig = *it; auto const name = channelConfig.get("channel"); - if (std::count(std::begin(Logger::kCHANNELS), std::end(Logger::kCHANNELS), name) == 0) - throw std::runtime_error("Can't override settings for log channel " + name + ": invalid channel"); + if (std::ranges::count(Logger::kCHANNELS, name) == 0) { // TODO: use std::ranges::contains when available + return std::unexpected{fmt::format("Can't override settings for log channel {}: invalid channel", name)}; + } minSeverity[name] = getSeverityLevel(channelConfig.get("log_level")); } @@ -189,6 +196,7 @@ LogService::init(config::ClioConfigDefinition const& config) filter = boost::log::filter{std::move(logFilter)}; boost::log::core::get()->set_filter(filter); LOG(LogService::info()) << "Default log level = " << defaultSeverity; + return {}; } Logger::Pump diff --git a/src/util/log/Logger.hpp b/src/util/log/Logger.hpp index a3df8ffa9..eb6ddcaee 100644 --- a/src/util/log/Logger.hpp +++ b/src/util/log/Logger.hpp @@ -46,6 +46,7 @@ #include #include +#include #include #include #include @@ -278,8 +279,9 @@ public: * @brief Global log core initialization from a @ref Config * * @param config The configuration to use + * @return Void on success, error message on failure */ - static void + [[nodiscard]] static std::expected init(config::ClioConfigDefinition const& config); /** diff --git a/src/util/newconfig/ConfigDefinition.hpp b/src/util/newconfig/ConfigDefinition.hpp index b3c5761a4..cdfcb3ed2 100644 --- a/src/util/newconfig/ConfigDefinition.hpp +++ b/src/util/newconfig/ConfigDefinition.hpp @@ -345,10 +345,10 @@ static ClioConfigDefinition gClioConfig = ClioConfigDefinition{ {"cache.page_fetch_size", ConfigValue{ConfigType::Integer}.defaultValue(512).withConstraint(gValidateUint16)}, {"cache.load", ConfigValue{ConfigType::String}.defaultValue("async").withConstraint(gValidateLoadMode)}, - {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}.optional().withConstraint(gValidateChannelName)} + {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}.withConstraint(gValidateChannelName)} }, {"log_channels.[].log_level", - Array{ConfigValue{ConfigType::String}.optional().withConstraint(gValidateLogLevelName)}}, + Array{ConfigValue{ConfigType::String}.withConstraint(gValidateLogLevelName)}}, {"log_level", ConfigValue{ConfigType::String}.defaultValue("info").withConstraint(gValidateLogLevelName)}, diff --git a/tests/unit/LoggerTests.cpp b/tests/unit/LoggerTests.cpp index 0a31c592a..c9ee3ae64 100644 --- a/tests/unit/LoggerTests.cpp +++ b/tests/unit/LoggerTests.cpp @@ -26,9 +26,11 @@ #include "util/newconfig/ConfigValue.hpp" #include "util/newconfig/Types.hpp" +#include #include #include #include +#include #include #include @@ -113,7 +115,7 @@ TEST_F(LoggerInitTest, DefaultLogLevel) ASSERT_FALSE(parsingErrors.has_value()); std::string const logString = "some log"; - LogService::init(config_); + EXPECT_TRUE(LogService::init(config_)); for (auto const& channel : Logger::kCHANNELS) { Logger const log{channel}; log.trace() << logString; @@ -151,7 +153,7 @@ TEST_F(LoggerInitTest, ChannelLogLevel) ASSERT_FALSE(parsingErrors.has_value()); std::string const logString = "some log"; - LogService::init(config_); + EXPECT_TRUE(LogService::init(config_)); for (auto const& channel : Logger::kCHANNELS) { Logger const log{channel}; log.trace() << logString; @@ -175,6 +177,38 @@ TEST_F(LoggerInitTest, ChannelLogLevel) } } +TEST_F(LoggerInitTest, InitReturnsErrorIfCouldNotCreateLogDirectory) +{ + // "/proc" directory is read only on any unix OS + auto const parsingErrors = config_.parse(ConfigFileJson{boost::json::object{{"log_directory", "/proc/logs"}}}); + ASSERT_FALSE(parsingErrors.has_value()); + + auto const result = LogService::init(config_); + EXPECT_FALSE(result); + EXPECT_THAT(result.error(), testing::HasSubstr("Couldn't create logs directory")); +} + +TEST_F(LoggerInitTest, InitReturnsErrorIfProvidedInvalidChannel) +{ + auto const jsonStr = R"json( + { + "log_channels": [ + { + "channel": "SomeChannel", + "log_level": "warn" + } + ] + })json"; + + auto const json = boost::json::parse(jsonStr).as_object(); + auto const parsingErrors = config_.parse(ConfigFileJson{json}); + ASSERT_FALSE(parsingErrors.has_value()); + + auto const result = LogService::init(config_); + EXPECT_FALSE(result); + EXPECT_EQ(result.error(), "Can't override settings for log channel SomeChannel: invalid channel"); +} + TEST_F(LoggerInitTest, LogSizeAndHourRotationCannotBeZero) { std::vector const keys{ From 491cd58f93d430ae6ad47abdb34d7eb07fdfd6c1 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Fri, 21 Feb 2025 16:10:25 +0000 Subject: [PATCH 13/29] feat: ETLng monitor (#1898) For #1594 --- src/etl/NetworkValidatedLedgers.cpp | 26 ++-- src/etl/NetworkValidatedLedgers.hpp | 14 ++- src/etl/NetworkValidatedLedgersInterface.hpp | 17 ++- src/etlng/CMakeLists.txt | 10 +- src/etlng/MonitorInterface.hpp | 67 +++++++++++ src/etlng/impl/Monitor.cpp | 99 ++++++++++++++++ src/etlng/impl/Monitor.hpp | 80 +++++++++++++ src/util/async/AnyOperation.hpp | 13 +- src/util/async/AnyStrand.hpp | 42 ++++++- src/util/async/Concepts.hpp | 8 ++ src/util/async/Operation.hpp | 21 +++- .../async/context/BasicExecutionContext.hpp | 5 +- src/util/async/context/impl/Strand.hpp | 22 +++- src/util/async/impl/ErasedOperation.hpp | 18 +++ src/util/log/Logger.cpp | 2 +- src/util/newconfig/ConfigDefinition.hpp | 6 +- src/util/newconfig/Types.hpp | 1 - .../util/MockNetworkValidatedLedgers.hpp | 9 +- tests/common/util/MockOperation.hpp | 1 + tests/common/util/MockStrand.hpp | 9 ++ tests/unit/CMakeLists.txt | 2 + tests/unit/LoggerTests.cpp | 12 +- tests/unit/etlng/MonitorTests.cpp | 112 ++++++++++++++++++ .../etlng/NetworkValidatedLedgersTests.cpp | 91 ++++++++++++++ tests/unit/util/async/AnyOperationTests.cpp | 11 ++ tests/unit/util/async/AnyStrandTests.cpp | 15 +++ .../util/async/AsyncExecutionContextTests.cpp | 57 +++++++++ tests/unit/util/newconfig/ArrayTests.cpp | 1 - .../newconfig/ClioConfigDefinitionTests.cpp | 35 ++++-- .../util/newconfig/ConfigFileJsonTests.cpp | 3 +- 30 files changed, 756 insertions(+), 53 deletions(-) create mode 100644 src/etlng/MonitorInterface.hpp create mode 100644 src/etlng/impl/Monitor.cpp create mode 100644 src/etlng/impl/Monitor.hpp create mode 100644 tests/unit/etlng/MonitorTests.cpp create mode 100644 tests/unit/etlng/NetworkValidatedLedgersTests.cpp diff --git a/src/etl/NetworkValidatedLedgers.cpp b/src/etl/NetworkValidatedLedgers.cpp index 9a317c062..cd9fa93b8 100644 --- a/src/etl/NetworkValidatedLedgers.cpp +++ b/src/etl/NetworkValidatedLedgers.cpp @@ -19,6 +19,8 @@ #include "etl/NetworkValidatedLedgers.hpp" +#include + #include #include #include @@ -35,25 +37,27 @@ NetworkValidatedLedgers::makeValidatedLedgers() void NetworkValidatedLedgers::push(uint32_t idx) { - std::lock_guard const lck(m_); - if (!max_ || idx > *max_) - max_ = idx; + std::lock_guard const lck(mtx_); + if (!latest_ || idx > *latest_) + latest_ = idx; + + notificationChannel_(idx); cv_.notify_all(); } std::optional NetworkValidatedLedgers::getMostRecent() { - std::unique_lock lck(m_); - cv_.wait(lck, [this]() { return max_; }); - return max_; + std::unique_lock lck(mtx_); + cv_.wait(lck, [this]() { return latest_; }); + return latest_; } bool NetworkValidatedLedgers::waitUntilValidatedByNetwork(uint32_t sequence, std::optional maxWaitMs) { - std::unique_lock lck(m_); - auto pred = [sequence, this]() -> bool { return (max_ && sequence <= *max_); }; + std::unique_lock lck(mtx_); + auto pred = [sequence, this]() -> bool { return (latest_ && sequence <= *latest_); }; if (maxWaitMs) { cv_.wait_for(lck, std::chrono::milliseconds(*maxWaitMs)); } else { @@ -62,4 +66,10 @@ NetworkValidatedLedgers::waitUntilValidatedByNetwork(uint32_t sequence, std::opt return pred(); } +boost::signals2::scoped_connection +NetworkValidatedLedgers::subscribe(SignalType::slot_type const& subscriber) +{ + return notificationChannel_.connect(subscriber); +} + } // namespace etl diff --git a/src/etl/NetworkValidatedLedgers.hpp b/src/etl/NetworkValidatedLedgers.hpp index c694c3b85..28acc0abf 100644 --- a/src/etl/NetworkValidatedLedgers.hpp +++ b/src/etl/NetworkValidatedLedgers.hpp @@ -21,6 +21,10 @@ #include "etl/NetworkValidatedLedgersInterface.hpp" +#include +#include +#include + #include #include #include @@ -38,12 +42,13 @@ namespace etl { * remains stopped for the rest of its lifetime. */ class NetworkValidatedLedgers : public NetworkValidatedLedgersInterface { - // max sequence validated by network - std::optional max_; + std::optional latest_; // currently known latest sequence validated by network - mutable std::mutex m_; + mutable std::mutex mtx_; std::condition_variable cv_; + SignalType notificationChannel_; + public: /** * @brief A factory function for NetworkValidatedLedgers @@ -81,6 +86,9 @@ public: */ bool waitUntilValidatedByNetwork(uint32_t sequence, std::optional maxWaitMs = {}) final; + + boost::signals2::scoped_connection + subscribe(SignalType::slot_type const& subscriber) override; }; } // namespace etl diff --git a/src/etl/NetworkValidatedLedgersInterface.hpp b/src/etl/NetworkValidatedLedgersInterface.hpp index aaa456295..d2ab9d12b 100644 --- a/src/etl/NetworkValidatedLedgersInterface.hpp +++ b/src/etl/NetworkValidatedLedgersInterface.hpp @@ -20,6 +20,10 @@ /** @file */ #pragma once +#include +#include +#include + #include #include namespace etl { @@ -29,6 +33,8 @@ namespace etl { */ class NetworkValidatedLedgersInterface { public: + using SignalType = boost::signals2::signal; + virtual ~NetworkValidatedLedgersInterface() = default; /** @@ -46,7 +52,7 @@ public: * * @return Sequence of most recently validated ledger. empty optional if the datastructure has been stopped */ - virtual std::optional + [[nodiscard]] virtual std::optional getMostRecent() = 0; /** @@ -59,6 +65,15 @@ public: */ virtual bool waitUntilValidatedByNetwork(uint32_t sequence, std::optional maxWaitMs = {}) = 0; + + /** + * @brief Allows clients to get notified when a new validated ledger becomes known to Clio + * + * @param subscriber The slot to connect + * @return A connection object that automatically disconnects the subscription once destroyed + */ + [[nodiscard]] virtual boost::signals2::scoped_connection + subscribe(SignalType::slot_type const& subscriber) = 0; }; } // namespace etl diff --git a/src/etlng/CMakeLists.txt b/src/etlng/CMakeLists.txt index 57fafcdfc..4b97c43ab 100644 --- a/src/etlng/CMakeLists.txt +++ b/src/etlng/CMakeLists.txt @@ -1,8 +1,14 @@ add_library(clio_etlng) target_sources( - clio_etlng PRIVATE impl/AmendmentBlockHandler.cpp impl/AsyncGrpcCall.cpp impl/Extraction.cpp impl/GrpcSource.cpp - impl/Loading.cpp impl/TaskManager.cpp + clio_etlng + PRIVATE impl/AmendmentBlockHandler.cpp + impl/AsyncGrpcCall.cpp + impl/Extraction.cpp + impl/GrpcSource.cpp + impl/Loading.cpp + impl/Monitor.cpp + impl/TaskManager.cpp ) target_link_libraries(clio_etlng PUBLIC clio_data) diff --git a/src/etlng/MonitorInterface.hpp b/src/etlng/MonitorInterface.hpp new file mode 100644 index 000000000..8e4b745e9 --- /dev/null +++ b/src/etlng/MonitorInterface.hpp @@ -0,0 +1,67 @@ +//------------------------------------------------------------------------------ +/* + 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 +#include +#include + +#include +#include + +namespace etlng { + +/** + * @brief An interface for the monitor service + * An implementation of this service is responsible for periodically checking various datasources to detect newly + * ingested ledgers. + */ +class MonitorInterface { +public: + static constexpr auto kDEFAULT_REPEAT_INTERVAL = std::chrono::seconds{1}; + using SignalType = boost::signals2::signal; + + virtual ~MonitorInterface() = default; + + /** + * @brief Allows clients to get notified when a new ledger becomes available in Clio's database + * + * @param subscriber The slot to connect + * @return A connection object that automatically disconnects the subscription once destroyed + */ + [[nodiscard]] virtual boost::signals2::scoped_connection + subscribe(SignalType::slot_type const& subscriber) = 0; + + /** + * @brief Run the monitor service + * + * @param repeatInterval The interval between attempts to check the database for new ledgers + */ + virtual void + run(std::chrono::steady_clock::duration repeatInterval = kDEFAULT_REPEAT_INTERVAL) = 0; + + /** + * @brief Stops the monitor service + */ + virtual void + stop() = 0; +}; + +} // namespace etlng diff --git a/src/etlng/impl/Monitor.cpp b/src/etlng/impl/Monitor.cpp new file mode 100644 index 000000000..8b8bedad2 --- /dev/null +++ b/src/etlng/impl/Monitor.cpp @@ -0,0 +1,99 @@ +//------------------------------------------------------------------------------ +/* + 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 "etlng/impl/Monitor.hpp" + +#include "data/BackendInterface.hpp" +#include "etl/NetworkValidatedLedgersInterface.hpp" +#include "util/Assert.hpp" +#include "util/async/AnyExecutionContext.hpp" +#include "util/async/AnyOperation.hpp" +#include "util/log/Logger.hpp" + +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace etlng::impl { +Monitor::Monitor( + util::async::AnyExecutionContext ctx, + std::shared_ptr backend, + std::shared_ptr validatedLedgers, + uint32_t startSequence +) + : strand_(ctx.makeStrand()) + , backend_(std::move(backend)) + , validatedLedgers_(std::move(validatedLedgers)) + , nextSequence_(startSequence) +{ +} + +Monitor::~Monitor() +{ + stop(); +} + +void +Monitor::run(std::chrono::steady_clock::duration repeatInterval) +{ + ASSERT(not repeatedTask_.has_value(), "Monitor attempted to run more than once"); + LOG(log_.debug()) << "Starting monitor"; + + repeatedTask_ = strand_.executeRepeatedly(repeatInterval, std::bind_front(&Monitor::doWork, this)); + subscription_ = validatedLedgers_->subscribe(std::bind_front(&Monitor::onNextSequence, this)); +} + +void +Monitor::stop() +{ + if (repeatedTask_.has_value()) + repeatedTask_->abort(); + + repeatedTask_ = std::nullopt; +} + +boost::signals2::scoped_connection +Monitor::subscribe(SignalType::slot_type const& subscriber) +{ + return notificationChannel_.connect(subscriber); +} + +void +Monitor::onNextSequence(uint32_t seq) +{ + LOG(log_.debug()) << "rippled published sequence " << seq; + repeatedTask_->invoke(); // force-invoke immediately +} + +void +Monitor::doWork() +{ + if (auto rng = backend_->hardFetchLedgerRangeNoThrow(); rng) { + while (rng->maxSequence >= nextSequence_) + notificationChannel_(nextSequence_++); + } +} + +} // namespace etlng::impl diff --git a/src/etlng/impl/Monitor.hpp b/src/etlng/impl/Monitor.hpp new file mode 100644 index 000000000..9b5112cc3 --- /dev/null +++ b/src/etlng/impl/Monitor.hpp @@ -0,0 +1,80 @@ +//------------------------------------------------------------------------------ +/* + 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 "data/BackendInterface.hpp" +#include "etl/NetworkValidatedLedgersInterface.hpp" +#include "etlng/MonitorInterface.hpp" +#include "util/async/AnyExecutionContext.hpp" +#include "util/async/AnyOperation.hpp" +#include "util/async/AnyStrand.hpp" +#include "util/log/Logger.hpp" + +#include +#include + +#include +#include +#include +#include +#include + +namespace etlng::impl { + +class Monitor : public MonitorInterface { + util::async::AnyStrand strand_; + std::shared_ptr backend_; + std::shared_ptr validatedLedgers_; + + uint32_t nextSequence_; + std::optional> repeatedTask_; + std::optional subscription_; // network validated ledgers subscription + + SignalType notificationChannel_; + + util::Logger log_{"ETL"}; + +public: + Monitor( + util::async::AnyExecutionContext ctx, + std::shared_ptr backend, + std::shared_ptr validatedLedgers, + uint32_t startSequence + ); + ~Monitor() override; + + void + run(std::chrono::steady_clock::duration repeatInterval) override; + + void + stop() override; + + boost::signals2::scoped_connection + subscribe(SignalType::slot_type const& subscriber) override; + +private: + void + onNextSequence(uint32_t seq); + + void + doWork(); +}; + +} // namespace etlng::impl diff --git a/src/util/async/AnyOperation.hpp b/src/util/async/AnyOperation.hpp index 439b91b3d..2835d8b56 100644 --- a/src/util/async/AnyOperation.hpp +++ b/src/util/async/AnyOperation.hpp @@ -19,7 +19,6 @@ #pragma once -#include "util/async/Concepts.hpp" #include "util/async/Error.hpp" #include "util/async/impl/ErasedOperation.hpp" @@ -107,6 +106,18 @@ public: } } + /** + * @brief Force-invoke the operation + * @note The action is scheduled on the underlying context/strand + * @warning The code of the user-provided action is expected to take care of thread-safety unless this operation is + * scheduled through a strand + */ + void + invoke() + { + operation_.invoke(); + } + private: impl::ErasedOperation operation_; }; diff --git a/src/util/async/AnyStrand.hpp b/src/util/async/AnyStrand.hpp index 487d65b2a..f2d7b8ed5 100644 --- a/src/util/async/AnyStrand.hpp +++ b/src/util/async/AnyStrand.hpp @@ -131,14 +131,43 @@ public: ); } + /** + * @brief Schedule a repeating operation on the execution context + * + * @param interval The interval at which the operation should be repeated + * @param fn The block of code to execute; no args allowed and return type must be void + * @return A repeating stoppable operation that can be used to wait for its cancellation + */ + [[nodiscard]] auto + executeRepeatedly(SomeStdDuration auto interval, SomeHandlerWithoutStopToken auto&& fn) + { + using RetType = std::decay_t; + static_assert(not std::is_same_v); + + auto const millis = std::chrono::duration_cast(interval); + return AnyOperation( // + pimpl_->executeRepeatedly( + millis, + [fn = std::forward(fn)] -> std::any { + fn(); + return {}; + } + ) + ); + } + private: struct Concept { virtual ~Concept() = default; [[nodiscard]] virtual impl::ErasedOperation - execute(std::function, std::optional timeout = std::nullopt) - const = 0; + execute( + std::function, + std::optional timeout = std::nullopt + ) = 0; [[nodiscard]] virtual impl::ErasedOperation execute(std::function) = 0; + [[nodiscard]] virtual impl::ErasedOperation + executeRepeatedly(std::chrono::milliseconds, std::function) = 0; }; template @@ -152,8 +181,7 @@ private: } [[nodiscard]] impl::ErasedOperation - execute(std::function fn, std::optional timeout) - const override + execute(std::function fn, std::optional timeout) override { return strand.execute(std::move(fn), timeout); } @@ -163,6 +191,12 @@ private: { return strand.execute(std::move(fn)); } + + impl::ErasedOperation + executeRepeatedly(std::chrono::milliseconds interval, std::function fn) override + { + return strand.executeRepeatedly(interval, std::move(fn)); + } }; private: diff --git a/src/util/async/Concepts.hpp b/src/util/async/Concepts.hpp index b6b86038f..eb863699b 100644 --- a/src/util/async/Concepts.hpp +++ b/src/util/async/Concepts.hpp @@ -75,6 +75,14 @@ concept SomeOperationWithData = SomeOperation and requires(T v) { { v.get() }; }; +/** + * @brief Specifies the interface for an operation that can force-invoked + */ +template +concept SomeForceInvocableOperation = SomeOperation and requires(T v) { + { v.invoke() }; +}; + /** * @brief Specifies the interface for an operation that can be stopped */ diff --git a/src/util/async/Operation.hpp b/src/util/async/Operation.hpp index 8bb4a531d..3b1a7420c 100644 --- a/src/util/async/Operation.hpp +++ b/src/util/async/Operation.hpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -227,6 +228,7 @@ using ScheduledOperation = impl::BasicScheduledOperation; template class RepeatingOperation : public util::MoveTracker { util::Repeat repeat_; + std::function action_; public: /** @@ -237,10 +239,11 @@ public: * @param interval Time to wait before repeating the user-provided block of code * @param fn The function to execute repeatedly */ - RepeatingOperation(auto& executor, std::chrono::steady_clock::duration interval, std::invocable auto&& fn) - : repeat_(executor) + template + RepeatingOperation(auto& executor, std::chrono::steady_clock::duration interval, FnType&& fn) + : repeat_(executor), action_([fn = std::forward(fn), &executor] { boost::asio::post(executor, fn); }) { - repeat_.start(interval, std::forward(fn)); + repeat_.start(interval, action_); } ~RepeatingOperation() override @@ -266,6 +269,18 @@ public: { repeat_.stop(); } + + /** + * @brief Force-invoke the operation + * @note The action is scheduled on the underlying context/strand + * @warning The code of the user-provided action is expected to take care of thread-safety unless this operation is + * scheduled through a strand + */ + void + invoke() + { + action_(); + } }; } // namespace util::async diff --git a/src/util/async/context/BasicExecutionContext.hpp b/src/util/async/context/BasicExecutionContext.hpp index b2b921828..288e93141 100644 --- a/src/util/async/context/BasicExecutionContext.hpp +++ b/src/util/async/context/BasicExecutionContext.hpp @@ -61,8 +61,8 @@ struct AsioPoolStrandContext { using Executor = boost::asio::strand; using Timer = SteadyTimer; - Executor const& - getExecutor() const + Executor& + getExecutor() { return executor; } @@ -272,6 +272,7 @@ public: /** * @brief Schedule a repeating operation on the execution context + * @warning The code of the user-provided action is expected to be thread-safe * * @param interval The interval at which the operation should be repeated * @param fn The block of code to execute; no args allowed and return type must be void diff --git a/src/util/async/context/impl/Strand.hpp b/src/util/async/context/impl/Strand.hpp index 6a6cfd116..cb0a38d89 100644 --- a/src/util/async/context/impl/Strand.hpp +++ b/src/util/async/context/impl/Strand.hpp @@ -20,6 +20,7 @@ #pragma once #include "util/async/Concepts.hpp" +#include "util/async/Operation.hpp" #include "util/async/context/impl/Cancellation.hpp" #include "util/async/context/impl/Execution.hpp" #include "util/async/context/impl/Timer.hpp" @@ -52,6 +53,7 @@ public: using StopToken = typename StopSourceType::Token; using Timer = typename ParentContextType::ContextHolderType::Timer; // timers are associated with the parent context + using RepeatedOperation = RepeatingOperation; BasicStrand(ParentContextType& parent, auto&& strand) : parentContext_{std::ref(parent)}, context_{std::forward(strand)} @@ -64,8 +66,10 @@ public: BasicStrand(BasicStrand const&) = delete; [[nodiscard]] auto - execute(SomeHandlerWith auto&& fn, std::optional timeout = std::nullopt) const - noexcept(kIS_NOEXCEPT) + execute( + SomeHandlerWith auto&& fn, + std::optional timeout = std::nullopt + ) noexcept(kIS_NOEXCEPT) { return DispatcherType::dispatch( context_, @@ -89,7 +93,7 @@ public: } [[nodiscard]] auto - execute(SomeHandlerWith auto&& fn, SomeStdDuration auto timeout) const noexcept(kIS_NOEXCEPT) + execute(SomeHandlerWith auto&& fn, SomeStdDuration auto timeout) noexcept(kIS_NOEXCEPT) { return execute( std::forward(fn), @@ -98,7 +102,7 @@ public: } [[nodiscard]] auto - execute(SomeHandlerWithoutStopToken auto&& fn) const noexcept(kIS_NOEXCEPT) + execute(SomeHandlerWithoutStopToken auto&& fn) noexcept(kIS_NOEXCEPT) { return DispatcherType::dispatch( context_, @@ -114,6 +118,16 @@ public: }) ); } + + [[nodiscard]] auto + executeRepeatedly(SomeStdDuration auto interval, SomeHandlerWithoutStopToken auto&& fn) noexcept(kIS_NOEXCEPT) + { + if constexpr (not std::is_same_v) { + return TimerContextProvider::getContext(*this).executeRepeatedly(interval, std::forward(fn)); + } else { + return RepeatedOperation(impl::extractAssociatedExecutor(*this), interval, std::forward(fn)); + } + } }; } // namespace util::async::impl diff --git a/src/util/async/impl/ErasedOperation.hpp b/src/util/async/impl/ErasedOperation.hpp index 480295ddb..d42de490c 100644 --- a/src/util/async/impl/ErasedOperation.hpp +++ b/src/util/async/impl/ErasedOperation.hpp @@ -71,6 +71,12 @@ public: pimpl_->abort(); } + void + invoke() + { + pimpl_->invoke(); + } + private: struct Concept { virtual ~Concept() = default; @@ -81,6 +87,8 @@ private: get() = 0; virtual void abort() = 0; + virtual void + invoke() = 0; }; template @@ -133,6 +141,16 @@ private: } } } + + void + invoke() override + { + if constexpr (not SomeForceInvocableOperation) { + ASSERT(false, "Called invoke() on an operation that can't be force-invoked"); + } else { + operation.invoke(); + } + } }; private: diff --git a/src/util/log/Logger.cpp b/src/util/log/Logger.cpp index 13d57174a..2e4815a8d 100644 --- a/src/util/log/Logger.cpp +++ b/src/util/log/Logger.cpp @@ -175,7 +175,7 @@ LogService::init(config::ClioConfigDefinition const& config) for (auto it = overrides.begin(); it != overrides.end(); ++it) { auto const& channelConfig = *it; auto const name = channelConfig.get("channel"); - if (std::ranges::count(Logger::kCHANNELS, name) == 0) { // TODO: use std::ranges::contains when available + if (std::ranges::count(Logger::kCHANNELS, name) == 0) { // TODO: use std::ranges::contains when available return std::unexpected{fmt::format("Can't override settings for log channel {}: invalid channel", name)}; } diff --git a/src/util/newconfig/ConfigDefinition.hpp b/src/util/newconfig/ConfigDefinition.hpp index cdfcb3ed2..0e90ba911 100644 --- a/src/util/newconfig/ConfigDefinition.hpp +++ b/src/util/newconfig/ConfigDefinition.hpp @@ -345,10 +345,8 @@ static ClioConfigDefinition gClioConfig = ClioConfigDefinition{ {"cache.page_fetch_size", ConfigValue{ConfigType::Integer}.defaultValue(512).withConstraint(gValidateUint16)}, {"cache.load", ConfigValue{ConfigType::String}.defaultValue("async").withConstraint(gValidateLoadMode)}, - {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}.withConstraint(gValidateChannelName)} - }, - {"log_channels.[].log_level", - Array{ConfigValue{ConfigType::String}.withConstraint(gValidateLogLevelName)}}, + {"log_channels.[].channel", Array{ConfigValue{ConfigType::String}.withConstraint(gValidateChannelName)}}, + {"log_channels.[].log_level", Array{ConfigValue{ConfigType::String}.withConstraint(gValidateLogLevelName)}}, {"log_level", ConfigValue{ConfigType::String}.defaultValue("info").withConstraint(gValidateLogLevelName)}, diff --git a/src/util/newconfig/Types.hpp b/src/util/newconfig/Types.hpp index 7cb1bd4c4..5ff744efd 100644 --- a/src/util/newconfig/Types.hpp +++ b/src/util/newconfig/Types.hpp @@ -81,4 +81,3 @@ getType() } } // namespace util::config - diff --git a/tests/common/util/MockNetworkValidatedLedgers.hpp b/tests/common/util/MockNetworkValidatedLedgers.hpp index 3371c67ca..5af4d850c 100644 --- a/tests/common/util/MockNetworkValidatedLedgers.hpp +++ b/tests/common/util/MockNetworkValidatedLedgers.hpp @@ -19,8 +19,9 @@ #pragma once -#include "etl/ETLHelpers.hpp" +#include "etl/NetworkValidatedLedgersInterface.hpp" +#include #include #include @@ -31,6 +32,12 @@ struct MockNetworkValidatedLedgers : public etl::NetworkValidatedLedgersInterfac MOCK_METHOD(void, push, (uint32_t), (override)); MOCK_METHOD(std::optional, getMostRecent, (), (override)); MOCK_METHOD(bool, waitUntilValidatedByNetwork, (uint32_t, std::optional), (override)); + MOCK_METHOD( + boost::signals2::scoped_connection, + subscribe, + (etl::NetworkValidatedLedgersInterface::SignalType::slot_type const& subscriber), + (override) + ); }; template