diff --git a/.github/actions/build-docker-image/action.yml b/.github/actions/build-docker-image/action.yml index 9a566ea289..0001d106ec 100644 --- a/.github/actions/build-docker-image/action.yml +++ b/.github/actions/build-docker-image/action.yml @@ -52,7 +52,7 @@ runs: cache-image: false - uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1 - - uses: docker/metadata-action@318604b99e75e41977312d83839a89be02ca4893 # v5.9.0 + - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 id: meta with: images: ${{ inputs.images }} diff --git a/.github/scripts/conan/generate_matrix.py b/.github/scripts/conan/generate_matrix.py index 8ac9170d19..3c18f04192 100755 --- a/.github/scripts/conan/generate_matrix.py +++ b/.github/scripts/conan/generate_matrix.py @@ -4,7 +4,7 @@ LINUX_OS = ["heavy", "heavy-arm64"] LINUX_CONTAINERS = [ - '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' ] LINUX_COMPILERS = ["gcc", "clang"] diff --git a/.github/scripts/conan/init.sh b/.github/scripts/conan/init.sh index 5875d57b09..dba73a6ecb 100755 --- a/.github/scripts/conan/init.sh +++ b/.github/scripts/conan/init.sh @@ -40,9 +40,9 @@ mkdir -p "$PROFILES_DIR" if [[ "$(uname)" == "Darwin" ]]; then create_profile_with_sanitizers "apple-clang" "$APPLE_CLANG_PROFILE" - echo "include(apple-clang)" > "$PROFILES_DIR/default" + echo "include(apple-clang)" >"$PROFILES_DIR/default" else create_profile_with_sanitizers "clang" "$CLANG_PROFILE" create_profile_with_sanitizers "gcc" "$GCC_PROFILE" - echo "include(gcc)" > "$PROFILES_DIR/default" + echo "include(gcc)" >"$PROFILES_DIR/default" fi diff --git a/.github/scripts/execute-tests-under-sanitizer.sh b/.github/scripts/execute-tests-under-sanitizer.sh index 18e609c781..9646994bbb 100755 --- a/.github/scripts/execute-tests-under-sanitizer.sh +++ b/.github/scripts/execute-tests-under-sanitizer.sh @@ -22,8 +22,8 @@ fi TEST_BINARY=$1 if [[ ! -f "$TEST_BINARY" ]]; then - echo "Test binary not found: $TEST_BINARY" - exit 1 + echo "Test binary not found: $TEST_BINARY" + exit 1 fi TESTS=$($TEST_BINARY --gtest_list_tests | awk '/^ / {print suite $1} !/^ / {suite=$1}') @@ -35,12 +35,12 @@ export TSAN_OPTIONS="die_after_fork=0" export MallocNanoZone='0' # for MacOSX for TEST in $TESTS; do - OUTPUT_FILE="$OUTPUT_DIR/${TEST//\//_}.log" - $TEST_BINARY --gtest_filter="$TEST" > "$OUTPUT_FILE" 2>&1 - - if [ $? -ne 0 ]; then - echo "'$TEST' failed a sanitizer check." - else - rm "$OUTPUT_FILE" - fi + OUTPUT_FILE="$OUTPUT_DIR/${TEST//\//_}.log" + $TEST_BINARY --gtest_filter="$TEST" >"$OUTPUT_FILE" 2>&1 + + if [ $? -ne 0 ]; then + echo "'$TEST' failed a sanitizer check." + else + rm "$OUTPUT_FILE" + fi done diff --git a/.github/scripts/prepare-release-artifacts.sh b/.github/scripts/prepare-release-artifacts.sh index 2267d8f99d..0461170af2 100755 --- a/.github/scripts/prepare-release-artifacts.sh +++ b/.github/scripts/prepare-release-artifacts.sh @@ -20,5 +20,5 @@ for artifact_name in $(ls); do rm "${artifact_name}/${BINARY_NAME}" rm -r "${artifact_name}" - sha256sum "./${artifact_name}.zip" > "./${artifact_name}.zip.sha256sum" + sha256sum "./${artifact_name}.zip" >"./${artifact_name}.zip.sha256sum" done diff --git a/.github/workflows/build-clio-docker-image.yml b/.github/workflows/build-clio-docker-image.yml index b054d0761f..0427b76636 100644 --- a/.github/workflows/build-clio-docker-image.yml +++ b/.github/workflows/build-clio-docker-image.yml @@ -48,7 +48,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Download Clio binary from artifact if: ${{ inputs.artifact_name != null }} diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b08f797638..bd5d086991 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -49,7 +49,7 @@ jobs: build_type: [Release, Debug] container: [ - '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }', + '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }', ] static: [true] @@ -79,7 +79,7 @@ jobs: uses: ./.github/workflows/reusable-build.yml with: runs_on: heavy - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' conan_profile: gcc build_type: Debug download_ccache: true @@ -98,7 +98,7 @@ jobs: uses: ./.github/workflows/reusable-build.yml with: runs_on: heavy - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' conan_profile: gcc build_type: Release download_ccache: true @@ -115,10 +115,10 @@ jobs: needs: build-and-test runs-on: heavy container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 with: diff --git a/.github/workflows/check-libxrpl.yml b/.github/workflows/check-libxrpl.yml index 866aad1dac..51b389e4c2 100644 --- a/.github/workflows/check-libxrpl.yml +++ b/.github/workflows/check-libxrpl.yml @@ -21,10 +21,10 @@ jobs: name: Build Clio / `libXRPL ${{ github.event.client_payload.version }}` runs-on: heavy container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 @@ -69,7 +69,7 @@ jobs: needs: build runs-on: heavy container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f steps: - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 @@ -92,7 +92,7 @@ jobs: issues: write steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Create an issue uses: ./.github/actions/create-issue diff --git a/.github/workflows/check-pr-title.yml b/.github/workflows/check-pr-title.yml index 0c2bede6b4..c278c08bb3 100644 --- a/.github/workflows/check-pr-title.yml +++ b/.github/workflows/check-pr-title.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: ytanikin/pr-conventional-commits@b72758283dcbee706975950e96bc4bf323a8d8c0 # 1.4.2 + - uses: ytanikin/pr-conventional-commits@fda730cb152c05a849d6d84325e50c6182d9d1e9 # 1.5.1 with: task_types: '["build","feat","fix","docs","test","ci","style","refactor","perf","chore"]' add_label: false diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index f1e2bb65a7..565d252215 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -31,7 +31,7 @@ jobs: if: github.event_name != 'push' || contains(github.event.head_commit.message, 'clang-tidy auto fixes') runs-on: heavy container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f permissions: contents: write @@ -39,7 +39,7 @@ jobs: pull-requests: write steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 @@ -62,27 +62,30 @@ jobs: uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a id: nproc - - name: Run clang-tidy + - name: Run clang-tidy (several times) continue-on-error: true - id: run_clang_tidy + id: clang_tidy run: | - run-clang-tidy-${{ env.LLVM_TOOLS_VERSION }} -p build -j "${{ steps.nproc.outputs.nproc }}" -fix -quiet 1>output.txt + # We run clang-tidy several times, because some fixes may enable new fixes in subsequent runs. + CLANG_TIDY_COMMAND="run-clang-tidy-${{ env.LLVM_TOOLS_VERSION }} -p build -j ${{ steps.nproc.outputs.nproc }} -fix -quiet" + ${CLANG_TIDY_COMMAND} || + ${CLANG_TIDY_COMMAND} || + ${CLANG_TIDY_COMMAND} + + - name: Check for changes + id: files_changed + continue-on-error: true + run: | + git diff --exit-code - name: Fix local includes and clang-format style - if: ${{ steps.run_clang_tidy.outcome != 'success' }} + if: ${{ steps.files_changed.outcome != 'success' }} run: | pre-commit run --all-files fix-local-includes || true pre-commit run --all-files clang-format || true - - name: Print issues found - if: ${{ steps.run_clang_tidy.outcome != 'success' }} - run: | - sed -i '/error\||/!d' ./output.txt - cat output.txt - rm output.txt - - name: Create an issue - if: ${{ steps.run_clang_tidy.outcome != 'success' && github.event_name != 'pull_request' }} + if: ${{ (steps.clang_tidy.outcome != 'success' || steps.files_changed.outcome != 'success') && github.event_name != 'pull_request' }} id: create_issue uses: ./.github/actions/create-issue env: @@ -95,7 +98,7 @@ jobs: List of the issues found: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}/ - uses: crazy-max/ghaction-import-gpg@e89d40939c28e39f97cf32126055eeae86ba74ec # v6.3.0 - if: ${{ steps.run_clang_tidy.outcome != 'success' && github.event_name != 'pull_request' }} + if: ${{ steps.files_changed.outcome != 'success' && github.event_name != 'pull_request' }} with: gpg_private_key: ${{ secrets.ACTIONS_GPG_PRIVATE_KEY }} passphrase: ${{ secrets.ACTIONS_GPG_PASSPHRASE }} @@ -103,8 +106,8 @@ jobs: git_commit_gpgsign: true - name: Create PR with fixes - if: ${{ steps.run_clang_tidy.outcome != 'success' && github.event_name != 'pull_request' }} - uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e # v7.0.8 + if: ${{ steps.files_changed.outcome != 'success' && github.event_name != 'pull_request' }} + uses: peter-evans/create-pull-request@22a9089034f40e5a961c8808d113e2c98fb63676 # v7.0.11 env: GH_REPO: ${{ github.repository }} GH_TOKEN: ${{ github.token }} @@ -119,5 +122,5 @@ jobs: reviewers: "godexsoft,kuznetsss,PeterChen13579,mathbunnyru" - name: Fail the job - if: ${{ steps.run_clang_tidy.outcome != 'success' }} + if: ${{ steps.clang_tidy.outcome != 'success' || steps.files_changed.outcome != 'success' }} run: exit 1 diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 6aae320b23..c3347330c9 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -18,11 +18,11 @@ jobs: build: runs-on: ubuntu-latest container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f steps: - name: Checkout - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: lfs: true diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index eb2af86058..49e060c00f 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -43,17 +43,17 @@ jobs: conan_profile: gcc build_type: Release static: true - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' - os: heavy conan_profile: gcc build_type: Debug static: true - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' - os: heavy conan_profile: gcc.ubsan build_type: Release static: false - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' uses: ./.github/workflows/reusable-build-test.yml with: @@ -77,7 +77,7 @@ jobs: include: - os: heavy conan_profile: clang - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' static: true - os: macos15 conan_profile: apple-clang @@ -145,7 +145,7 @@ jobs: issues: write steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Create an issue uses: ./.github/actions/create-issue diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index b7faa2c19a..c17d2e5740 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -11,4 +11,4 @@ jobs: uses: XRPLF/actions/.github/workflows/pre-commit.yml@34790936fae4c6c751f62ec8c06696f9c1a5753a with: runs_on: heavy - container: '{ "image": "ghcr.io/xrplf/clio-pre-commit:c117f470f2ef954520ab5d1c8a5ed2b9e68d6f8a" }' + container: '{ "image": "ghcr.io/xrplf/clio-pre-commit:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 509387f0a8..76bc0172a2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -29,7 +29,7 @@ jobs: conan_profile: gcc build_type: Release static: true - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' uses: ./.github/workflows/reusable-build-test.yml with: diff --git a/.github/workflows/reusable-build.yml b/.github/workflows/reusable-build.yml index 964cee4ece..e4c9f188c1 100644 --- a/.github/workflows/reusable-build.yml +++ b/.github/workflows/reusable-build.yml @@ -90,7 +90,7 @@ jobs: if: ${{ runner.os == 'macOS' }} uses: XRPLF/actions/.github/actions/cleanup-workspace@ea9970b7c211b18f4c8bcdb28c29f5711752029f - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 # We need to fetch tags to have correct version in the release diff --git a/.github/workflows/reusable-release.yml b/.github/workflows/reusable-release.yml index be0d004110..757774c392 100644 --- a/.github/workflows/reusable-release.yml +++ b/.github/workflows/reusable-release.yml @@ -46,7 +46,7 @@ jobs: release: runs-on: heavy container: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f env: GH_REPO: ${{ github.repository }} GH_TOKEN: ${{ github.token }} @@ -55,7 +55,7 @@ jobs: contents: write steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index 3086e84780..757c9e8f81 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -54,7 +54,7 @@ jobs: if: ${{ runner.os == 'macOS' }} uses: XRPLF/actions/.github/actions/cleanup-workspace@ea9970b7c211b18f4c8bcdb28c29f5711752029f - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 diff --git a/.github/workflows/reusable-upload-coverage-report.yml b/.github/workflows/reusable-upload-coverage-report.yml index c44ed06bbd..a223c27443 100644 --- a/.github/workflows/reusable-upload-coverage-report.yml +++ b/.github/workflows/reusable-upload-coverage-report.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: fetch-depth: 0 diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml index 488583854d..fdf58e7bcb 100644 --- a/.github/workflows/sanitizers.yml +++ b/.github/workflows/sanitizers.yml @@ -44,7 +44,7 @@ jobs: uses: ./.github/workflows/reusable-build-test.yml with: runs_on: heavy - container: '{ "image": "ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7" }' + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' download_ccache: false upload_ccache: false conan_profile: ${{ matrix.compiler }}${{ matrix.sanitizer_ext }} diff --git a/.github/workflows/update-docker-ci.yml b/.github/workflows/update-docker-ci.yml index 5b2270967b..eee543f748 100644 --- a/.github/workflows/update-docker-ci.yml +++ b/.github/workflows/update-docker-ci.yml @@ -56,7 +56,7 @@ jobs: needs: repo steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -94,7 +94,7 @@ jobs: needs: repo steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -132,7 +132,7 @@ jobs: needs: [repo, gcc-amd64, gcc-arm64] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -183,7 +183,7 @@ jobs: needs: repo steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -219,7 +219,7 @@ jobs: needs: [repo, gcc-merge] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -250,7 +250,7 @@ jobs: needs: [repo, gcc-merge] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -281,7 +281,7 @@ jobs: needs: [repo, tools-amd64, tools-arm64] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Get changed files id: changed-files @@ -316,7 +316,7 @@ jobs: needs: [repo, tools-merge] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - uses: ./.github/actions/build-docker-image env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -338,7 +338,7 @@ jobs: needs: [repo, gcc-merge, clang, tools-merge] steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - uses: ./.github/actions/build-docker-image env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 2aada69426..178e80d6c9 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -50,7 +50,7 @@ jobs: outputs: matrix: ${{ steps.set-matrix.outputs.matrix }} steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Calculate conan matrix id: set-matrix @@ -73,7 +73,7 @@ jobs: CONAN_PROFILE: ${{ matrix.compiler }}${{ matrix.sanitizer_ext }} steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Prepare runner uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8be36053a3..91fe5bf7ed 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,12 +29,12 @@ repos: # Autoformat: YAML, JSON, Markdown, etc. - repo: https://github.com/rbubley/mirrors-prettier - rev: 5ba47274f9b181bce26a5150a725577f3c336011 # frozen: v3.6.2 + rev: 3c603eae8faac85303ae675fd33325cff699a797 # frozen: v3.7.3 hooks: - id: prettier - repo: https://github.com/igorshubovych/markdownlint-cli - rev: 192ad822316c3a22fb3d3cc8aa6eafa0b8488360 # frozen: v0.45.0 + rev: c8fd5003603dd6f12447314ecd935ba87c09aff5 # frozen: v0.46.0 hooks: - id: markdownlint-fix exclude: LICENSE.md @@ -58,6 +58,17 @@ repos: --ignore-words=pre-commit-hooks/codespell_ignore.txt, ] + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 2892f1f81088477370d4fbc56545c05d33d2493f # frozen: 25.11.0 + hooks: + - id: black + + - repo: https://github.com/scop/pre-commit-shfmt + rev: 2a30809d16bc7a60d9b97353c797f42b510d3368 # frozen: v3.12.0-2 + hooks: + - id: shfmt + args: ["-i", "4", "--write"] + # Running some C++ hooks before clang-format # to ensure that the style is consistent. - repo: local @@ -83,7 +94,7 @@ repos: language: script - repo: https://github.com/pre-commit/mirrors-clang-format - rev: 719856d56a62953b8d2839fb9e851f25c3cfeef8 # frozen: v21.1.2 + rev: 4c26f99731e7c22a047c35224150ee9e43d7c03e # frozen: v21.1.6 hooks: - id: clang-format args: [--style=file] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 24cb3f8858..ede8fd0fd4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -180,6 +180,7 @@ Existing maintainers can resign, or be subject to a vote for removal at the behe - [kuznetsss](https://github.com/kuznetsss) (Ripple) - [legleux](https://github.com/legleux) (Ripple) - [PeterChen13579](https://github.com/PeterChen13579) (Ripple) +- [mathbunnyru](https://github.com/mathbunnyru) (Ripple) ### Honorable ex-Maintainers diff --git a/cmake/install/clio.service.in b/cmake/install/clio.service.in new file mode 100644 index 0000000000..1ecdddb668 --- /dev/null +++ b/cmake/install/clio.service.in @@ -0,0 +1,17 @@ +[Unit] +Description=Clio XRPL API server +Documentation=https://github.com/XRPLF/clio.git + +After=network-online.target +Wants=network-online.target + +[Service] +Type=simple +ExecStart=@CLIO_INSTALL_DIR@/bin/clio_server @CLIO_INSTALL_DIR@/etc/config.json +Restart=on-failure +User=clio +Group=clio +LimitNOFILE=65536 + +[Install] +WantedBy=multi-user.target diff --git a/cmake/install/install.cmake b/cmake/install/install.cmake index 21f1da1e94..8481979694 100644 --- a/cmake/install/install.cmake +++ b/cmake/install/install.cmake @@ -11,3 +11,6 @@ file(READ docs/examples/config/example-config.json config) string(REGEX REPLACE "./clio_log" "/var/log/clio/" config "${config}") file(WRITE ${CMAKE_BINARY_DIR}/install-config.json "${config}") install(FILES ${CMAKE_BINARY_DIR}/install-config.json DESTINATION etc RENAME config.json) + +configure_file("${CMAKE_SOURCE_DIR}/cmake/install/clio.service.in" "${CMAKE_BINARY_DIR}/clio.service") +install(FILES "${CMAKE_BINARY_DIR}/clio.service" DESTINATION /lib/systemd/system) diff --git a/cmake/pkg/postinst b/cmake/pkg/postinst index 19780b21c4..d10353d9b0 100755 --- a/cmake/pkg/postinst +++ b/cmake/pkg/postinst @@ -10,37 +10,36 @@ CLIO_BIN="$CLIO_PREFIX/bin/${CLIO_EXECUTABLE}" CLIO_CONFIG="$CLIO_PREFIX/etc/config.json" case "$1" in - configure) - if ! id -u "$USER_NAME" >/dev/null 2>&1; then - # Users who should not have a home directory should have their home directory set to /nonexistent - # https://www.debian.org/doc/debian-policy/ch-opersys.html#non-existent-home-directories - useradd \ - --system \ - --home-dir /nonexistent \ - --no-create-home \ - --shell /usr/sbin/nologin \ - --comment "system user for ${CLIO_EXECUTABLE}" \ - --user-group \ - ${USER_NAME} - fi - - install -d -o "$USER_NAME" -g "$GROUP_NAME" /var/log/clio - - if [ -f "$CLIO_CONFIG" ]; then - chown "$USER_NAME:$GROUP_NAME" "$CLIO_CONFIG" - fi - - chown -R "$USER_NAME:$GROUP_NAME" "$CLIO_PREFIX" - - ln -sf "$CLIO_BIN" "/usr/bin/${CLIO_EXECUTABLE}" - - ;; - abort-upgrade|abort-remove|abort-deconfigure) - ;; - *) - echo "postinst called with unknown argument \`$1'" >&2 - exit 1 - ;; +configure) + if ! id -u "$USER_NAME" >/dev/null 2>&1; then + # Users who should not have a home directory should have their home directory set to /nonexistent + # https://www.debian.org/doc/debian-policy/ch-opersys.html#non-existent-home-directories + useradd \ + --system \ + --home-dir /nonexistent \ + --no-create-home \ + --shell /usr/sbin/nologin \ + --comment "system user for ${CLIO_EXECUTABLE}" \ + --user-group \ + ${USER_NAME} + fi + + install -d -o "$USER_NAME" -g "$GROUP_NAME" /var/log/clio + + if [ -f "$CLIO_CONFIG" ]; then + chown "$USER_NAME:$GROUP_NAME" "$CLIO_CONFIG" + fi + + chown -R "$USER_NAME:$GROUP_NAME" "$CLIO_PREFIX" + + ln -sf "$CLIO_BIN" "/usr/bin/${CLIO_EXECUTABLE}" + + ;; +abort-upgrade | abort-remove | abort-deconfigure) ;; +*) + echo "postinst called with unknown argument \`$1'" >&2 + exit 1 + ;; esac exit 0 diff --git a/conan.lock b/conan.lock index 1be65f835b..cb5ca49113 100644 --- a/conan.lock +++ b/conan.lock @@ -3,13 +3,13 @@ "requires": [ "zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76%1756234269.497", "xxhash/0.8.3#681d36a0a6111fc56e5e45ea182c19cc%1756234289.683", - "xrpl/3.0.0-rc1#f5c8ecd42bdf511ad36f57bc702dacd2%1762975621.294", + "xrpl/3.0.0#534d3f65a336109eee929b88962bae4e%1765375071.547", "sqlite3/3.49.1#8631739a4c9b93bd3d6b753bac548a63%1756234266.869", - "spdlog/1.15.3#3ca0e9e6b83af4d0151e26541d140c86%1754401846.61", + "spdlog/1.16.0#942c2c39562ae25ba575d9c8e2bdf3b6%1763984117.108", "soci/4.0.3#a9f8d773cd33e356b5879a4b0564f287%1756234262.318", - "re2/20230301#dfd6e2bf050eb90ddd8729cfb4c844a4%1756234257.976", + "re2/20230301#ca3b241baec15bd31ea9187150e0b333%1764175362.029", "rapidjson/cci.20220822#1b9d8c2256876a154172dc5cfbe447c6%1754325007.656", - "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", + "protobuf/3.21.12#44ee56c0a6eea0c19aeeaca680370b88%1764175361.456", "openssl/1.1.1w#a8f0792d7c5121b954578a7149d23e03%1756223730.729", "nudb/2.0.9#fb8dfd1a5557f5e0528114c2da17721e%1763150366.909", "minizip/1.2.13#9e87d57804bd372d6d1e32b1871517a3%1754325004.374", @@ -17,41 +17,45 @@ "libuv/1.46.0#dc28c1f653fa197f00db5b577a6f6011%1754325003.592", "libiconv/1.17#1e65319e945f2d31941a9d28cc13c058%1756223727.64", "libbacktrace/cci.20210118#a7691bfccd8caaf66309df196790a5a1%1756230911.03", - "libarchive/3.8.1#5cf685686322e906cb42706ab7e099a8%1756234256.696", + "libarchive/3.8.1#ffee18995c706e02bf96e7a2f7042e0d%1764175360.142", "http_parser/2.9.4#98d91690d6fd021e9e624218a85d9d97%1754325001.385", "gtest/1.14.0#f8f0757a574a8dd747d16af62d6eb1b7%1754325000.842", "grpc/1.50.1#02291451d1e17200293a409410d1c4e1%1756234248.958", - "fmt/11.2.0#579bb2cdf4a7607621beea4eb4651e0f%1754324999.086", + "fmt/12.1.0#50abab23274d56bb8f42c94b3b9a40c7%1763984116.926", "doctest/2.4.11#a4211dfc329a16ba9f280f9574025659%1756234220.819", - "date/3.0.4#f74bbba5a08fa388256688743136cb6f%1756234217.493", - "cassandra-cpp-driver/2.17.0#e50919efac8418c26be6671fd702540a%1754324997.363", - "c-ares/1.34.5#b78b91e7cfb1f11ce777a285bbf169c6%1756234217.915", - "bzip2/1.0.8#00b4a4658791c1f06914e087f0e792f5%1756234261.716", - "boost/1.83.0#5d975011d65b51abb2d2f6eb8386b368%1754325043.336", + "date/3.0.4#862e11e80030356b53c2c38599ceb32b%1763584497.32", + "cassandra-cpp-driver/2.17.0#bd3934138689482102c265d01288a316%1764175359.611", + "c-ares/1.34.5#5581c2b62a608b40bb85d965ab3ec7c8%1764175359.429", + "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1764175359.429", + "boost/1.83.0#91d8b1572534d2c334d6790e3c34d0c1%1764175359.61", "benchmark/1.9.4#ce4403f7a24d3e1f907cd9da4b678be4%1754578869.672", - "abseil/20230802.1#f0f91485b111dc9837a68972cb19ca7b%1756234220.907" + "abseil/20230802.1#90ba607d4ee8fb5fb157c3db540671fc%1764175359.429" ], "build_requires": [ "zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76%1756234269.497", - "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1%1756234251.614", - "cmake/3.31.8#dde3bde00bb843687e55aea5afa0e220%1756234232.89", + "protobuf/3.21.12#44ee56c0a6eea0c19aeeaca680370b88%1764175361.456", + "cmake/4.2.0#ae0a44f44a1ef9ab68fd4b3e9a1f8671%1764175359.44", + "cmake/3.31.10#313d16a1aa16bbdb2ca0792467214b76%1764175359.429", "b2/5.3.3#107c15377719889654eb9a162a673975%1756234226.28" ], "python_requires": [], "overrides": { "boost/1.83.0": [ null, - "boost/1.83.0#5d975011d65b51abb2d2f6eb8386b368" + "boost/1.83.0#91d8b1572534d2c334d6790e3c34d0c1" ], "protobuf/3.21.12": [ null, - "protobuf/3.21.12#d927114e28de9f4691a6bbcdd9a529d1" + "protobuf/3.21.12#44ee56c0a6eea0c19aeeaca680370b88" ], "lz4/1.9.4": [ "lz4/1.10.0" ], "sqlite3/3.44.2": [ "sqlite3/3.49.1" + ], + "fmt/12.0.0": [ + "fmt/12.1.0" ] }, "config_requires": [] diff --git a/conanfile.py b/conanfile.py index 4cd3534430..de5a450527 100644 --- a/conanfile.py +++ b/conanfile.py @@ -3,62 +3,60 @@ class ClioConan(ConanFile): - name = 'clio' - license = 'ISC' - author = 'Alex Kremer , John Freeman , Ayaz Salikhov ' - url = 'https://github.com/xrplf/clio' - description = 'Clio RPC server' - settings = 'os', 'compiler', 'build_type', 'arch' + name = "clio" + license = "ISC" + author = "Alex Kremer , John Freeman , Ayaz Salikhov " + url = "https://github.com/xrplf/clio" + description = "Clio RPC server" + settings = "os", "compiler", "build_type", "arch" options = {} requires = [ - 'boost/1.83.0', - 'cassandra-cpp-driver/2.17.0', - 'fmt/11.2.0', - 'protobuf/3.21.12', - 'grpc/1.50.1', - 'openssl/1.1.1w', - 'xrpl/3.0.0-rc1', - 'zlib/1.3.1', - 'libbacktrace/cci.20210118', - 'spdlog/1.15.3', + "boost/1.83.0", + "cassandra-cpp-driver/2.17.0", + "protobuf/3.21.12", + "grpc/1.50.1", + "openssl/1.1.1w", + "xrpl/3.0.0", + "zlib/1.3.1", + "libbacktrace/cci.20210118", + "spdlog/1.16.0", ] default_options = { - 'xrpl/*:tests': False, - 'xrpl/*:rocksdb': False, - 'cassandra-cpp-driver/*:shared': False, - 'date/*:header_only': True, - 'grpc/*:shared': False, - 'grpc/*:secure': True, - 'libpq/*:shared': False, - 'lz4/*:shared': False, - 'openssl/*:shared': False, - 'protobuf/*:shared': False, - 'protobuf/*:with_zlib': True, - 'snappy/*:shared': False, - 'gtest/*:no_main': True, + "xrpl/*:tests": False, + "xrpl/*:rocksdb": False, + "cassandra-cpp-driver/*:shared": False, + "date/*:header_only": True, + "grpc/*:shared": False, + "grpc/*:secure": True, + "libpq/*:shared": False, + "lz4/*:shared": False, + "openssl/*:shared": False, + "protobuf/*:shared": False, + "protobuf/*:with_zlib": True, + "snappy/*:shared": False, + "gtest/*:no_main": True, } - exports_sources = ( - 'CMakeLists.txt', 'cmake/*', 'src/*' - ) + exports_sources = ("CMakeLists.txt", "cmake/*", "src/*") def requirements(self): - self.requires('gtest/1.14.0') - self.requires('benchmark/1.9.4') + self.requires("gtest/1.14.0") + self.requires("benchmark/1.9.4") + self.requires("fmt/12.1.0", force=True) def configure(self): - if self.settings.compiler == 'apple-clang': - self.options['boost'].visibility = 'global' + if self.settings.compiler == "apple-clang": + self.options["boost"].visibility = "global" def layout(self): cmake_layout(self) # Fix this setting to follow the default introduced in Conan 1.48 # to align with our build instructions. - self.folders.generators = 'build/generators' + self.folders.generators = "build/generators" - generators = 'CMakeDeps' + generators = "CMakeDeps" def generate(self): tc = CMakeToolchain(self) diff --git a/docker/ci/Dockerfile b/docker/ci/Dockerfile index a261755770..6d5c5caf0f 100644 --- a/docker/ci/Dockerfile +++ b/docker/ci/Dockerfile @@ -36,7 +36,6 @@ RUN apt-get update \ libmpfr-dev \ libncurses-dev \ make \ - ninja-build \ wget \ zip \ && apt-get clean \ @@ -107,6 +106,7 @@ COPY --from=clio-tools \ /usr/local/bin/git-cliff \ /usr/local/bin/gh \ /usr/local/bin/gdb \ + /usr/local/bin/ninja \ /usr/local/bin/ WORKDIR /root diff --git a/docker/ci/README.md b/docker/ci/README.md index 0dabd9ed9a..d899afab08 100644 --- a/docker/ci/README.md +++ b/docker/ci/README.md @@ -15,6 +15,7 @@ The image is based on Ubuntu 20.04 and contains: - gh 2.82.1 - git-cliff 2.10.1 - mold 2.40.4 +- Ninja 1.13.2 - Python 3.8 - and some other useful tools diff --git a/docker/develop/compose.yaml b/docker/develop/compose.yaml index 3c5368b927..35443fece0 100644 --- a/docker/develop/compose.yaml +++ b/docker/develop/compose.yaml @@ -1,6 +1,6 @@ services: clio_develop: - image: ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 + image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f volumes: - clio_develop_conan_data:/root/.conan2/p - clio_develop_ccache:/root/.ccache diff --git a/docker/develop/run b/docker/develop/run index 6874831e24..88518f14e6 100755 --- a/docker/develop/run +++ b/docker/develop/run @@ -2,7 +2,7 @@ script_dir=$(dirname $0) -pushd $script_dir > /dev/null +pushd $script_dir >/dev/null function start_container { if [ -z "$(docker ps -q -f name=clio_develop)" ]; then @@ -41,21 +41,26 @@ EOF } case $1 in - -h|--help) - print_help ;; - - -t|--terminal) - open_terminal ;; - - -s|--stop) - stop_container ;; - - -*) - echo "Unknown option: $1" - print_help ;; - - *) - run "$@" ;; +-h | --help) + print_help + ;; + +-t | --terminal) + open_terminal + ;; + +-s | --stop) + stop_container + ;; + +-*) + echo "Unknown option: $1" + print_help + ;; + +*) + run "$@" + ;; esac -popd > /dev/null +popd >/dev/null diff --git a/docker/tools/Dockerfile b/docker/tools/Dockerfile index 92ef6ce484..71f09000cf 100644 --- a/docker/tools/Dockerfile +++ b/docker/tools/Dockerfile @@ -12,7 +12,6 @@ ARG BUILD_VERSION=0 RUN apt-get update \ && apt-get install -y --no-install-recommends --no-install-suggests \ - ninja-build \ python3 \ python3-pip \ software-properties-common \ @@ -24,6 +23,15 @@ RUN apt-get update \ WORKDIR /tmp +ARG NINJA_VERSION=1.13.2 + +RUN wget --progress=dot:giga "https://github.com/ninja-build/ninja/archive/refs/tags/v${NINJA_VERSION}.tar.gz" \ + && tar xf "v${NINJA_VERSION}.tar.gz" \ + && cd "ninja-${NINJA_VERSION}" \ + && ./configure.py --bootstrap \ + && mv ninja /usr/local/bin/ninja \ + && rm -rf /tmp/* /var/tmp/* + ARG MOLD_VERSION=2.40.4 RUN wget --progress=dot:giga "https://github.com/rui314/mold/archive/refs/tags/v${MOLD_VERSION}.tar.gz" \ && tar xf "v${MOLD_VERSION}.tar.gz" \ diff --git a/docs/build-clio.md b/docs/build-clio.md index 69c0179a9c..51c6822b5c 100644 --- a/docs/build-clio.md +++ b/docs/build-clio.md @@ -191,7 +191,7 @@ Open the `index.html` file in your browser to see the documentation pages. It is also possible to build Clio using [Docker](https://www.docker.com/) if you don't want to install all the dependencies on your machine. ```sh -docker run -it ghcr.io/xrplf/clio-ci:77387d8f9f13aea8f23831d221ac3e7683bb69b7 +docker run -it ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f git clone https://github.com/XRPLF/clio cd clio ``` diff --git a/pre-commit-hooks/check-doxygen-docs.sh b/pre-commit-hooks/check-doxygen-docs.sh index cc54879258..76a27f2cb8 100755 --- a/pre-commit-hooks/check-doxygen-docs.sh +++ b/pre-commit-hooks/check-doxygen-docs.sh @@ -45,7 +45,7 @@ if [[ "1.14.0" > "$version" ]]; then ERROR ----------------------------------------------------------------------------- - A minimum of version 1.14 of `which doxygen` is required. + A minimum of version 1.14 of $(which doxygen) is required. Your version is $version. Please upgrade it. Your changes may fail CI checks. @@ -55,26 +55,26 @@ EOF exit 0 fi -mkdir -p ${DOCDIR} > /dev/null 2>&1 -pushd ${DOCDIR} > /dev/null 2>&1 +mkdir -p ${DOCDIR} >/dev/null 2>&1 +pushd ${DOCDIR} >/dev/null 2>&1 -cat ${ROOT}/docs/Doxyfile | \ -sed \ - -e "s/\${LINT}/YES/" \ - -e "s/\${WARN_AS_ERROR}/NO/" \ - -e "s!\${SOURCE}!${ROOT}!" \ - -e "s/\${USE_DOT}/NO/" \ - -e "s/\${EXCLUDES}/impl/" \ -| ${DOXYGEN} - 2> ${TMPFILE} 1> /dev/null +cat ${ROOT}/docs/Doxyfile | + sed \ + -e "s/\${LINT}/YES/" \ + -e "s/\${WARN_AS_ERROR}/NO/" \ + -e "s!\${SOURCE}!${ROOT}!" \ + -e "s/\${USE_DOT}/NO/" \ + -e "s/\${EXCLUDES}/impl/" | + ${DOXYGEN} - 2>${TMPFILE} 1>/dev/null # We don't want to check for default values and typedefs as well as for member variables -OUT=$(cat ${TMPFILE} \ - | grep -v "=default" \ - | grep -v "\(variable\)" \ - | grep -v "\(typedef\)") +OUT=$(cat ${TMPFILE} | + grep -v "=default" | + grep -v "\(variable\)" | + grep -v "\(typedef\)") -rm -rf ${TMPFILE} > /dev/null 2>&1 -popd > /dev/null 2>&1 +rm -rf ${TMPFILE} >/dev/null 2>&1 +popd >/dev/null 2>&1 if [[ ! -z "$OUT" ]]; then cat < style - sed -E 's|#include "(.*)"|#include <\1>|g' "$file_path" > "$file_path_all_global" + sed -E 's|#include "(.*)"|#include <\1>|g' "$file_path" >"$file_path_all_global" # Make local includes to be "..." style - sed -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g" "$file_path_all_global" > "$file_path_fixed" + sed -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g" "$file_path_all_global" >"$file_path_fixed" rm "$file_path_all_global" # Check if the temporary file is different from the original file diff --git a/pre-commit-hooks/json_in_cpp.py b/pre-commit-hooks/json_in_cpp.py index 3298b4e82f..c94048ee33 100755 --- a/pre-commit-hooks/json_in_cpp.py +++ b/pre-commit-hooks/json_in_cpp.py @@ -4,7 +4,6 @@ import re from pathlib import Path - PATTERN = r'R"JSON\((.*?)\)JSON"' @@ -40,6 +39,7 @@ def replace_json(match): raw_json = match.group(1) raw_json = re.sub(r'":\n\s*(\[|\{)', r'": \1', raw_json) return f'R"JSON({raw_json})JSON"' + return re.sub(PATTERN, replace_json, cpp_content, flags=re.DOTALL) @@ -49,12 +49,12 @@ def fix_indentation(cpp_content: str) -> str: lines = cpp_content.splitlines() - ends_with_newline = cpp_content.endswith('\n') + ends_with_newline = cpp_content.endswith("\n") def find_indentation(line: str) -> int: return len(line) - len(line.lstrip()) - for (line_num, (line, next_line)) in enumerate(zip(lines[:-1], lines[1:])): + for line_num, (line, next_line) in enumerate(zip(lines[:-1], lines[1:])): if "JSON(" in line and ")JSON" not in line: indent = find_indentation(line) next_indent = find_indentation(next_line) @@ -69,7 +69,11 @@ def find_indentation(line: str) -> int: if ")JSON" in lines[i]: lines[i] = " " * indent + lines[i].lstrip() break - lines[i] = lines[i][by_how_much:] if by_how_much > 0 else " " * (-by_how_much) + lines[i] + lines[i] = ( + lines[i][by_how_much:] + if by_how_much > 0 + else " " * (-by_how_much) + lines[i] + ) result = "\n".join(lines) diff --git a/pre-commit-hooks/run-go-fmt.sh b/pre-commit-hooks/run-go-fmt.sh index 6f73c89443..c13312ea12 100755 --- a/pre-commit-hooks/run-go-fmt.sh +++ b/pre-commit-hooks/run-go-fmt.sh @@ -4,7 +4,7 @@ # set -e -o pipefail -if ! command -v gofmt &> /dev/null ; then +if ! command -v gofmt &>/dev/null; then echo "gofmt not installed or available in the PATH" >&2 exit 1 fi diff --git a/pre-commit-hooks/verify-commits.sh b/pre-commit-hooks/verify-commits.sh index 2ebf170f41..70ddb4d245 100755 --- a/pre-commit-hooks/verify-commits.sh +++ b/pre-commit-hooks/verify-commits.sh @@ -1,5 +1,4 @@ -#!/bin/sh - +#!/bin/bash # git for-each-ref refs/tags # see which tags are annotated and which are lightweight. Annotated tags are "tag" objects. # # Set these so your commits and tags are always signed @@ -7,7 +6,7 @@ # git config tag.gpgsign true verify_commit_signed() { - if git verify-commit HEAD &> /dev/null; then + if git verify-commit HEAD &>/dev/null; then : # echo "HEAD commit seems signed..." else @@ -17,7 +16,7 @@ verify_commit_signed() { } verify_tag() { - if git describe --exact-match --tags HEAD &> /dev/null; then + if git describe --exact-match --tags HEAD &>/dev/null; then : # You might be ok to push # echo "Tag is annotated." return 0 @@ -28,7 +27,7 @@ verify_tag() { } verify_tag_signed() { - if git verify-tag "$version" &> /dev/null ; then + if git verify-tag "$version" &>/dev/null; then : # ok, I guess we'll let you push # echo "Tag appears signed" return 0 @@ -40,11 +39,11 @@ verify_tag_signed() { } # Check some things if we're pushing a branch called "release/" -if echo "$PRE_COMMIT_REMOTE_BRANCH" | grep ^refs\/heads\/release\/ &> /dev/null ; then +if echo "$PRE_COMMIT_REMOTE_BRANCH" | grep ^refs\/heads\/release\/ &>/dev/null; then version=$(git tag --points-at HEAD) echo "Looks like you're trying to push a $version release..." echo "Making sure you've signed and tagged it." - if verify_commit_signed && verify_tag && verify_tag_signed ; then + if verify_commit_signed && verify_tag && verify_tag_signed; then : # Ok, I guess you can push else exit 1 diff --git a/src/app/ClioApplication.cpp b/src/app/ClioApplication.cpp index 23cc4dff74..2cd4803eec 100644 --- a/src/app/ClioApplication.cpp +++ b/src/app/ClioApplication.cpp @@ -91,6 +91,7 @@ ClioApplication::ClioApplication(util::config::ClioConfigDefinition const& confi { LOG(util::LogService::info()) << "Clio version: " << util::build::getClioFullVersionString(); signalsHandler_.subscribeToStop([this]() { appStopper_.stop(); }); + appStopper_.setOnComplete([this]() { signalsHandler_.notifyGracefulShutdownComplete(); }); } int @@ -182,7 +183,7 @@ ClioApplication::run(bool const useNgWebServer) return EXIT_FAILURE; } - httpServer->onGet("/metrics", MetricsHandler{adminVerifier}); + httpServer->onGet("/metrics", MetricsHandler{adminVerifier, workQueue}); httpServer->onGet("/health", HealthCheckHandler{}); httpServer->onGet("/cache_state", CacheStateHandler{cache}); auto requestHandler = RequestHandler{adminVerifier, handler}; diff --git a/src/app/Stopper.cpp b/src/app/Stopper.cpp index 1d25571ac7..173bb0aeb9 100644 --- a/src/app/Stopper.cpp +++ b/src/app/Stopper.cpp @@ -38,7 +38,18 @@ Stopper::~Stopper() void Stopper::setOnStop(std::function cb) { - util::spawn(ctx_, std::move(cb)); + util::spawn(ctx_, [this, cb = std::move(cb)](auto yield) { + cb(yield); + + if (onCompleteCallback_) + onCompleteCallback_(); + }); +} + +void +Stopper::setOnComplete(std::function cb) +{ + onCompleteCallback_ = std::move(cb); } void diff --git a/src/app/Stopper.hpp b/src/app/Stopper.hpp index 883b2e0ed6..190dd5df94 100644 --- a/src/app/Stopper.hpp +++ b/src/app/Stopper.hpp @@ -43,6 +43,7 @@ namespace app { class Stopper { boost::asio::io_context ctx_; std::thread worker_; + std::function onCompleteCallback_; public: /** @@ -58,6 +59,14 @@ class Stopper { void setOnStop(std::function cb); + /** + * @brief Set the callback to be called when graceful shutdown completes. + * + * @param cb The callback to be called when shutdown completes. + */ + void + setOnComplete(std::function cb); + /** * @brief Stop the application and run the shutdown tasks. */ diff --git a/src/app/WebHandlers.cpp b/src/app/WebHandlers.cpp index b0d9938548..a84fd97be1 100644 --- a/src/app/WebHandlers.cpp +++ b/src/app/WebHandlers.cpp @@ -19,7 +19,10 @@ #include "app/WebHandlers.hpp" +#include "rpc/Errors.hpp" +#include "rpc/WorkQueue.hpp" #include "util/Assert.hpp" +#include "util/CoroutineGroup.hpp" #include "util/prometheus/Http.hpp" #include "web/AdminVerificationStrategy.hpp" #include "web/SubscriptionContextInterface.hpp" @@ -31,6 +34,7 @@ #include #include +#include #include #include #include @@ -76,8 +80,8 @@ DisconnectHook::operator()(web::ng::Connection const& connection) dosguard_.get().decrement(connection.ip()); } -MetricsHandler::MetricsHandler(std::shared_ptr adminVerifier) - : adminVerifier_{std::move(adminVerifier)} +MetricsHandler::MetricsHandler(std::shared_ptr adminVerifier, rpc::WorkQueue& workQueue) + : adminVerifier_{std::move(adminVerifier)}, workQueue_{std::ref(workQueue)} { } @@ -86,19 +90,45 @@ MetricsHandler::operator()( web::ng::Request const& request, web::ng::ConnectionMetadata& connectionMetadata, web::SubscriptionContextPtr, - boost::asio::yield_context + boost::asio::yield_context yield ) { - auto const maybeHttpRequest = request.asHttpRequest(); - ASSERT(maybeHttpRequest.has_value(), "Got not a http request in Get"); - auto const& httpRequest = maybeHttpRequest->get(); - - // FIXME(#1702): Using veb server thread to handle prometheus request. Better to post on work queue. - auto maybeResponse = util::prometheus::handlePrometheusRequest( - httpRequest, adminVerifier_->isAdmin(httpRequest, connectionMetadata.ip()) + std::optional response; + util::CoroutineGroup coroutineGroup{yield, 1}; + auto const onTaskComplete = coroutineGroup.registerForeign(yield); + ASSERT(onTaskComplete.has_value(), "Coroutine group can't be full"); + + bool const postSuccessful = workQueue_.get().postCoro( + [this, &request, &response, &onTaskComplete = onTaskComplete.value(), &connectionMetadata]( + boost::asio::yield_context + ) mutable { + auto const maybeHttpRequest = request.asHttpRequest(); + ASSERT(maybeHttpRequest.has_value(), "Got not a http request in Get"); + auto const& httpRequest = maybeHttpRequest->get(); + + auto maybeResponse = util::prometheus::handlePrometheusRequest( + httpRequest, adminVerifier_->isAdmin(httpRequest, connectionMetadata.ip()) + ); + ASSERT(maybeResponse.has_value(), "Got unexpected request for Prometheus"); + response = web::ng::Response{std::move(maybeResponse).value(), request}; + // notify the coroutine group that the foreign task is done + onTaskComplete(); + }, + /* isWhiteListed= */ true, + rpc::WorkQueue::Priority::High ); - ASSERT(maybeResponse.has_value(), "Got unexpected request for Prometheus"); - return web::ng::Response{std::move(maybeResponse).value(), request}; + + if (!postSuccessful) { + return web::ng::Response{ + boost::beast::http::status::too_many_requests, rpc::makeError(rpc::RippledError::rpcTOO_BUSY), request + }; + } + + // Put the coroutine to sleep until the foreign task is done + coroutineGroup.asyncWait(yield); + ASSERT(response.has_value(), "Woke up coroutine without setting response"); + + return std::move(response).value(); } web::ng::Response diff --git a/src/app/WebHandlers.hpp b/src/app/WebHandlers.hpp index a70d0fca6e..cd5880ceae 100644 --- a/src/app/WebHandlers.hpp +++ b/src/app/WebHandlers.hpp @@ -21,6 +21,7 @@ #include "data/LedgerCacheInterface.hpp" #include "rpc/Errors.hpp" +#include "rpc/WorkQueue.hpp" #include "util/log/Logger.hpp" #include "web/AdminVerificationStrategy.hpp" #include "web/SubscriptionContextInterface.hpp" @@ -119,20 +120,23 @@ class DisconnectHook { */ class MetricsHandler { std::shared_ptr adminVerifier_; + std::reference_wrapper workQueue_; public: /** * @brief Construct a new MetricsHandler object * * @param adminVerifier The AdminVerificationStrategy to use for verifying the connection for admin access. + * @param workQueue The WorkQueue to use for handling the request. */ - MetricsHandler(std::shared_ptr adminVerifier); + MetricsHandler(std::shared_ptr adminVerifier, rpc::WorkQueue& workQueue); /** * @brief The call of the function object. * * @param request The request to handle. * @param connectionMetadata The connection metadata. + * @param yield The yield context. * @return The response to the request. */ web::ng::Response @@ -140,7 +144,7 @@ class MetricsHandler { web::ng::Request const& request, web::ng::ConnectionMetadata& connectionMetadata, web::SubscriptionContextPtr, - boost::asio::yield_context + boost::asio::yield_context yield ); }; diff --git a/src/data/AmendmentCenter.hpp b/src/data/AmendmentCenter.hpp index 6a65955866..7b990d8773 100644 --- a/src/data/AmendmentCenter.hpp +++ b/src/data/AmendmentCenter.hpp @@ -152,6 +152,7 @@ struct Amendments { REGISTER(fixDirectoryLimit); REGISTER(fixIncludeKeyletFields); REGISTER(fixTokenEscrowV1); + REGISTER(LendingProtocol); // Obsolete but supported by libxrpl REGISTER(CryptoConditionsSuite); diff --git a/src/etl/impl/TaskManager.cpp b/src/etl/impl/TaskManager.cpp index ea17395eeb..1bb8d31aad 100644 --- a/src/etl/impl/TaskManager.cpp +++ b/src/etl/impl/TaskManager.cpp @@ -87,8 +87,8 @@ TaskManager::run(std::size_t numExtractors) util::async::AnyOperation TaskManager::spawnExtractor(TaskQueue& queue) { - // TODO: these values may be extracted to config later and/or need to be fine-tuned on a realistic system - static constexpr auto kDELAY_BETWEEN_ATTEMPTS = std::chrono::milliseconds{100u}; + // TODO https://github.com/XRPLF/clio/issues/2838: the approach should be changed to a reactive one instead + static constexpr auto kDELAY_BETWEEN_ATTEMPTS = std::chrono::milliseconds{10u}; static constexpr auto kDELAY_BETWEEN_ENQUEUE_ATTEMPTS = std::chrono::milliseconds{1u}; return ctx_.execute([this, &queue](auto stopRequested) { diff --git a/src/rpc/handlers/AMMInfo.cpp b/src/rpc/handlers/AMMInfo.cpp index bc5ed56b2c..d2af7498d2 100644 --- a/src/rpc/handlers/AMMInfo.cpp +++ b/src/rpc/handlers/AMMInfo.cpp @@ -316,8 +316,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::value if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(asset))) input.issue1 = parseIssue(jsonObject.at(JS(asset)).as_object()); diff --git a/src/rpc/handlers/AccountChannels.cpp b/src/rpc/handlers/AccountChannels.cpp index 49f9fbe480..1fb38b4b08 100644 --- a/src/rpc/handlers/AccountChannels.cpp +++ b/src/rpc/handlers/AccountChannels.cpp @@ -154,8 +154,11 @@ tag_invoke(boost::json::value_to_tag, boost::json if (jsonObject.contains(JS(destination_account))) input.destinationAccount = boost::json::value_to(jv.at(JS(destination_account))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/AccountCurrencies.cpp b/src/rpc/handlers/AccountCurrencies.cpp index 78a2ce20ed..d23fbe6886 100644 --- a/src/rpc/handlers/AccountCurrencies.cpp +++ b/src/rpc/handlers/AccountCurrencies.cpp @@ -128,8 +128,11 @@ tag_invoke(boost::json::value_to_tag, boost::js if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/AccountInfo.cpp b/src/rpc/handlers/AccountInfo.cpp index d855fdba39..8631b56325 100644 --- a/src/rpc/handlers/AccountInfo.cpp +++ b/src/rpc/handlers/AccountInfo.cpp @@ -204,8 +204,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::va if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(signer_lists))) input.signerLists = boost::json::value_to(jsonObject.at(JS(signer_lists))); diff --git a/src/rpc/handlers/AccountLines.cpp b/src/rpc/handlers/AccountLines.cpp index 8b3772a994..159089b9f3 100644 --- a/src/rpc/handlers/AccountLines.cpp +++ b/src/rpc/handlers/AccountLines.cpp @@ -215,8 +215,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::v if (jsonObject.contains(JS(ignore_default))) input.ignoreDefault = jv.at(JS(ignore_default)).as_bool(); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/AccountMPTokenIssuances.cpp b/src/rpc/handlers/AccountMPTokenIssuances.cpp index 85c20586dc..9424317380 100644 --- a/src/rpc/handlers/AccountMPTokenIssuances.cpp +++ b/src/rpc/handlers/AccountMPTokenIssuances.cpp @@ -56,6 +56,7 @@ AccountMPTokenIssuancesHandler::addMPTokenIssuance( { MPTokenIssuanceResponse issuance; + issuance.MPTokenIssuanceID = ripple::strHex(sle.key()); issuance.issuer = ripple::to_string(account); issuance.sequence = sle.getFieldU32(ripple::sfSequence); auto const flags = sle.getFieldU32(ripple::sfFlags); @@ -73,6 +74,24 @@ AccountMPTokenIssuancesHandler::addMPTokenIssuance( setFlag(issuance.mptCanTransfer, ripple::lsfMPTCanTransfer); setFlag(issuance.mptCanClawback, ripple::lsfMPTCanClawback); + if (sle.isFieldPresent(ripple::sfMutableFlags)) { + auto const mutableFlags = sle.getFieldU32(ripple::sfMutableFlags); + + auto const setMutableFlag = [&](std::optional& field, std::uint32_t mask) { + if ((mutableFlags & mask) != 0u) + field = true; + }; + + setMutableFlag(issuance.mptCanMutateCanLock, ripple::lsmfMPTCanMutateCanLock); + setMutableFlag(issuance.mptCanMutateRequireAuth, ripple::lsmfMPTCanMutateRequireAuth); + setMutableFlag(issuance.mptCanMutateCanEscrow, ripple::lsmfMPTCanMutateCanEscrow); + setMutableFlag(issuance.mptCanMutateCanTrade, ripple::lsmfMPTCanMutateCanTrade); + setMutableFlag(issuance.mptCanMutateCanTransfer, ripple::lsmfMPTCanMutateCanTransfer); + setMutableFlag(issuance.mptCanMutateCanClawback, ripple::lsmfMPTCanMutateCanClawback); + setMutableFlag(issuance.mptCanMutateMetadata, ripple::lsmfMPTCanMutateMetadata); + setMutableFlag(issuance.mptCanMutateTransferFee, ripple::lsmfMPTCanMutateTransferFee); + } + if (sle.isFieldPresent(ripple::sfTransferFee)) issuance.transferFee = sle.getFieldU16(ripple::sfTransferFee); @@ -164,8 +183,11 @@ tag_invoke(boost::json::value_to_tag, boo if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } @@ -198,6 +220,7 @@ tag_invoke( ) { auto obj = boost::json::object{ + {JS(mpt_issuance_id), issuance.MPTokenIssuanceID}, {JS(issuer), issuance.issuer}, {JS(sequence), issuance.sequence}, }; @@ -224,6 +247,15 @@ tag_invoke( setIfPresent("mpt_can_transfer", issuance.mptCanTransfer); setIfPresent("mpt_can_clawback", issuance.mptCanClawback); + setIfPresent("mpt_can_mutate_can_lock", issuance.mptCanMutateCanLock); + setIfPresent("mpt_can_mutate_require_auth", issuance.mptCanMutateRequireAuth); + setIfPresent("mpt_can_mutate_can_escrow", issuance.mptCanMutateCanEscrow); + setIfPresent("mpt_can_mutate_can_trade", issuance.mptCanMutateCanTrade); + setIfPresent("mpt_can_mutate_can_transfer", issuance.mptCanMutateCanTransfer); + setIfPresent("mpt_can_mutate_can_clawback", issuance.mptCanMutateCanClawback); + setIfPresent("mpt_can_mutate_metadata", issuance.mptCanMutateMetadata); + setIfPresent("mpt_can_mutate_transfer_fee", issuance.mptCanMutateTransferFee); + jv = std::move(obj); } diff --git a/src/rpc/handlers/AccountMPTokenIssuances.hpp b/src/rpc/handlers/AccountMPTokenIssuances.hpp index 109eaff7ad..eafe228d1b 100644 --- a/src/rpc/handlers/AccountMPTokenIssuances.hpp +++ b/src/rpc/handlers/AccountMPTokenIssuances.hpp @@ -61,6 +61,7 @@ class AccountMPTokenIssuancesHandler { * @brief A struct to hold data for one MPTokenIssuance response. */ struct MPTokenIssuanceResponse { + std::string MPTokenIssuanceID; std::string issuer; uint32_t sequence{}; @@ -80,6 +81,15 @@ class AccountMPTokenIssuancesHandler { std::optional mptCanTrade; std::optional mptCanTransfer; std::optional mptCanClawback; + + std::optional mptCanMutateCanLock; + std::optional mptCanMutateRequireAuth; + std::optional mptCanMutateCanEscrow; + std::optional mptCanMutateCanTrade; + std::optional mptCanMutateCanTransfer; + std::optional mptCanMutateCanClawback; + std::optional mptCanMutateMetadata; + std::optional mptCanMutateTransferFee; }; /** diff --git a/src/rpc/handlers/AccountMPTokens.cpp b/src/rpc/handlers/AccountMPTokens.cpp index 3663eebc50..085841d0b4 100644 --- a/src/rpc/handlers/AccountMPTokens.cpp +++ b/src/rpc/handlers/AccountMPTokens.cpp @@ -54,6 +54,7 @@ AccountMPTokensHandler::addMPToken(std::vector& mpts, ripple::S MPTokenResponse token{}; auto const flags = sle.getFieldU32(ripple::sfFlags); + token.MPTokenID = ripple::strHex(sle.key()); token.account = ripple::to_string(sle.getAccountID(ripple::sfAccount)); token.MPTokenIssuanceID = ripple::strHex(sle.getFieldH192(ripple::sfMPTokenIssuanceID)); token.MPTAmount = sle.getFieldU64(ripple::sfMPTAmount); @@ -139,8 +140,11 @@ tag_invoke(boost::json::value_to_tag, boost::json if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } @@ -167,6 +171,7 @@ void tag_invoke(boost::json::value_from_tag, boost::json::value& jv, AccountMPTokensHandler::MPTokenResponse const& mptoken) { auto obj = boost::json::object{ + {"mpt_id", mptoken.MPTokenID}, {JS(account), mptoken.account}, {JS(mpt_issuance_id), mptoken.MPTokenIssuanceID}, {JS(mpt_amount), mptoken.MPTAmount}, diff --git a/src/rpc/handlers/AccountMPTokens.hpp b/src/rpc/handlers/AccountMPTokens.hpp index eef74ce249..67bdccfc09 100644 --- a/src/rpc/handlers/AccountMPTokens.hpp +++ b/src/rpc/handlers/AccountMPTokens.hpp @@ -59,6 +59,7 @@ class AccountMPTokensHandler { * @brief A struct to hold data for one MPToken response. */ struct MPTokenResponse { + std::string MPTokenID; std::string account; std::string MPTokenIssuanceID; uint64_t MPTAmount{}; diff --git a/src/rpc/handlers/AccountNFTs.cpp b/src/rpc/handlers/AccountNFTs.cpp index 7d2dde8d55..d16ccebe67 100644 --- a/src/rpc/handlers/AccountNFTs.cpp +++ b/src/rpc/handlers/AccountNFTs.cpp @@ -157,8 +157,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::va if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(limit))) input.limit = util::integralValueAs(jsonObject.at(JS(limit))); diff --git a/src/rpc/handlers/AccountObjects.cpp b/src/rpc/handlers/AccountObjects.cpp index 74d59b44dd..323b3e3cb5 100644 --- a/src/rpc/handlers/AccountObjects.cpp +++ b/src/rpc/handlers/AccountObjects.cpp @@ -153,8 +153,11 @@ tag_invoke(boost::json::value_to_tag, boost::json: if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(type))) { input.type = diff --git a/src/rpc/handlers/AccountOffers.cpp b/src/rpc/handlers/AccountOffers.cpp index 7ded2b9f87..2d987678fc 100644 --- a/src/rpc/handlers/AccountOffers.cpp +++ b/src/rpc/handlers/AccountOffers.cpp @@ -169,8 +169,11 @@ tag_invoke(boost::json::value_to_tag, boost::json:: if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(limit))) input.limit = util::integralValueAs(jsonObject.at(JS(limit))); diff --git a/src/rpc/handlers/AccountTx.cpp b/src/rpc/handlers/AccountTx.cpp index 043ea4fa20..ad82919e6c 100644 --- a/src/rpc/handlers/AccountTx.cpp +++ b/src/rpc/handlers/AccountTx.cpp @@ -258,8 +258,10 @@ tag_invoke(boost::json::value_to_tag, boost::json::valu input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); if (jsonObject.contains(JS(ledger_index))) { - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); - if (not input.ledgerIndex.has_value()) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) { + input.ledgerIndex = *expectedLedgerIndex; + } else { // could not get the latest validated ledger seq here, using this flag to indicate that input.usingValidatedLedger = true; } diff --git a/src/rpc/handlers/BookChanges.cpp b/src/rpc/handlers/BookChanges.cpp index cc2edbeff2..003ce4ef89 100644 --- a/src/rpc/handlers/BookChanges.cpp +++ b/src/rpc/handlers/BookChanges.cpp @@ -90,8 +90,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::va if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/BookOffers.cpp b/src/rpc/handlers/BookOffers.cpp index bca916f9b0..1383c68fd9 100644 --- a/src/rpc/handlers/BookOffers.cpp +++ b/src/rpc/handlers/BookOffers.cpp @@ -122,8 +122,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::val if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(taker))) input.taker = accountFromStringStrict(boost::json::value_to(jv.at(JS(taker)))); diff --git a/src/rpc/handlers/DepositAuthorized.cpp b/src/rpc/handlers/DepositAuthorized.cpp index 71d77e2229..364760f5d0 100644 --- a/src/rpc/handlers/DepositAuthorized.cpp +++ b/src/rpc/handlers/DepositAuthorized.cpp @@ -145,8 +145,11 @@ tag_invoke(boost::json::value_to_tag, boost::js if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(credentials))) input.credentials = boost::json::value_to(jv.at(JS(credentials))); diff --git a/src/rpc/handlers/Feature.cpp b/src/rpc/handlers/Feature.cpp index 4216ba38b9..1a84227397 100644 --- a/src/rpc/handlers/Feature.cpp +++ b/src/rpc/handlers/Feature.cpp @@ -168,8 +168,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::value if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/GatewayBalances.cpp b/src/rpc/handlers/GatewayBalances.cpp index 6bcec93e3f..ca95142fcf 100644 --- a/src/rpc/handlers/GatewayBalances.cpp +++ b/src/rpc/handlers/GatewayBalances.cpp @@ -249,8 +249,11 @@ tag_invoke(boost::json::value_to_tag, boost::json if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(hotwallet))) { if (jsonObject.at(JS(hotwallet)).is_string()) { diff --git a/src/rpc/handlers/GetAggregatePrice.cpp b/src/rpc/handlers/GetAggregatePrice.cpp index ade1b9da82..f09e2f7c29 100644 --- a/src/rpc/handlers/GetAggregatePrice.cpp +++ b/src/rpc/handlers/GetAggregatePrice.cpp @@ -263,8 +263,11 @@ tag_invoke(boost::json::value_to_tag, boost::js if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } for (auto const& oracle : jsonObject.at(JS(oracles)).as_array()) { input.oracles.push_back( diff --git a/src/rpc/handlers/Ledger.cpp b/src/rpc/handlers/Ledger.cpp index fc87b86edb..69923636ca 100644 --- a/src/rpc/handlers/Ledger.cpp +++ b/src/rpc/handlers/Ledger.cpp @@ -208,8 +208,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::value c if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(transactions))) input.transactions = jv.at(JS(transactions)).as_bool(); diff --git a/src/rpc/handlers/LedgerData.cpp b/src/rpc/handlers/LedgerData.cpp index 4f9ad5ddc4..9b35eed9b4 100644 --- a/src/rpc/handlers/LedgerData.cpp +++ b/src/rpc/handlers/LedgerData.cpp @@ -210,8 +210,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::val if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(type))) input.type = util::LedgerTypes::getLedgerEntryTypeFromStr(boost::json::value_to(jv.at(JS(type)))); diff --git a/src/rpc/handlers/LedgerEntry.cpp b/src/rpc/handlers/LedgerEntry.cpp index 1a62bb34a6..fa173fc5c8 100644 --- a/src/rpc/handlers/LedgerEntry.cpp +++ b/src/rpc/handlers/LedgerEntry.cpp @@ -305,8 +305,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::va if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(binary))) input.binary = jv.at(JS(binary)).as_bool(); diff --git a/src/rpc/handlers/MPTHolders.cpp b/src/rpc/handlers/MPTHolders.cpp index 9725402571..86aa3a99af 100644 --- a/src/rpc/handlers/MPTHolders.cpp +++ b/src/rpc/handlers/MPTHolders.cpp @@ -124,8 +124,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::val if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = jsonObject.at(JS(ledger_hash)).as_string().c_str(); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(limit))) input.limit = util::integralValueAs(jsonObject.at(JS(limit))); diff --git a/src/rpc/handlers/NFTHistory.cpp b/src/rpc/handlers/NFTHistory.cpp index 4b941dacab..9d3dc17ac0 100644 --- a/src/rpc/handlers/NFTHistory.cpp +++ b/src/rpc/handlers/NFTHistory.cpp @@ -215,8 +215,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::val if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(binary))) input.binary = jsonObject.at(JS(binary)).as_bool(); diff --git a/src/rpc/handlers/NFTInfo.cpp b/src/rpc/handlers/NFTInfo.cpp index d2fe5e8f1a..32d6e45781 100644 --- a/src/rpc/handlers/NFTInfo.cpp +++ b/src/rpc/handlers/NFTInfo.cpp @@ -115,8 +115,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::value if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/NFTOffersCommon.cpp b/src/rpc/handlers/NFTOffersCommon.cpp index 76cb01e697..3e33578aee 100644 --- a/src/rpc/handlers/NFTOffersCommon.cpp +++ b/src/rpc/handlers/NFTOffersCommon.cpp @@ -194,8 +194,11 @@ tag_invoke(boost::json::value_to_tag, boost::json:: if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(marker))) input.marker = boost::json::value_to(jsonObject.at(JS(marker))); diff --git a/src/rpc/handlers/NFTsByIssuer.cpp b/src/rpc/handlers/NFTsByIssuer.cpp index eb17f63e2c..c5f3f9c757 100644 --- a/src/rpc/handlers/NFTsByIssuer.cpp +++ b/src/rpc/handlers/NFTsByIssuer.cpp @@ -136,8 +136,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::v if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } if (jsonObject.contains(JS(limit))) input.limit = util::integralValueAs(jsonObject.at(JS(limit))); diff --git a/src/rpc/handlers/NoRippleCheck.cpp b/src/rpc/handlers/NoRippleCheck.cpp index 1c07e88d21..4fe0cb046f 100644 --- a/src/rpc/handlers/NoRippleCheck.cpp +++ b/src/rpc/handlers/NoRippleCheck.cpp @@ -196,8 +196,11 @@ tag_invoke(boost::json::value_to_tag, boost::json:: if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jsonObject.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/TransactionEntry.cpp b/src/rpc/handlers/TransactionEntry.cpp index e9fe7073d9..19d7df8ad4 100644 --- a/src/rpc/handlers/TransactionEntry.cpp +++ b/src/rpc/handlers/TransactionEntry.cpp @@ -109,8 +109,11 @@ tag_invoke(boost::json::value_to_tag, boost::jso if (jsonObject.contains(JS(ledger_hash))) input.ledgerHash = boost::json::value_to(jv.at(JS(ledger_hash))); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jv.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/rpc/handlers/VaultInfo.cpp b/src/rpc/handlers/VaultInfo.cpp index 277c178d1a..ad40745940 100644 --- a/src/rpc/handlers/VaultInfo.cpp +++ b/src/rpc/handlers/VaultInfo.cpp @@ -177,8 +177,11 @@ tag_invoke(boost::json::value_to_tag, boost::json::valu if (jsonObject.contains(JS(vault_id))) input.vaultID = jsonObject.at(JS(vault_id)).as_string(); - if (jsonObject.contains(JS(ledger_index))) - input.ledgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (jsonObject.contains(JS(ledger_index))) { + auto const expectedLedgerIndex = util::getLedgerIndex(jsonObject.at(JS(ledger_index))); + if (expectedLedgerIndex.has_value()) + input.ledgerIndex = *expectedLedgerIndex; + } return input; } diff --git a/src/util/JsonUtils.hpp b/src/util/JsonUtils.hpp index 9170b67782..40800e4def 100644 --- a/src/util/JsonUtils.hpp +++ b/src/util/JsonUtils.hpp @@ -23,10 +23,13 @@ #include #include +#include #include #include +#include #include +#include #include #include @@ -96,12 +99,11 @@ removeSecret(boost::json::object const& object) * * @tparam Type The type to cast to * @param value The JSON value to cast - * @return Value casted to the requested type - * @throws logic_error if the underlying number is neither int64 nor uint64 + * @return Value casted to the requested type or an error message */ template -Type -integralValueAs(boost::json::value const& value) +std::expected +tryIntegralValueAs(boost::json::value const& value) { if (value.is_uint64()) return static_cast(value.as_uint64()); @@ -109,29 +111,49 @@ integralValueAs(boost::json::value const& value) if (value.is_int64()) return static_cast(value.as_int64()); - throw std::logic_error("Value neither uint64 nor int64"); + return std::unexpected("Value neither uint64 nor int64"); +} + +/** + * @brief Detects the type of number stored in value and casts it back to the requested Type. + * @note This conversion can possibly cause wrapping around or UB. Use with caution. + * + * @tparam Type The type to cast to + * @param value The JSON value to cast + * @return Value casted to the requested type + * @throws logic_error if the underlying number is neither int64 nor uint64 + */ +template +Type +integralValueAs(boost::json::value const& value) +{ + auto expectedResult = tryIntegralValueAs(value); + if (expectedResult.has_value()) + return *expectedResult; + + throw std::logic_error(std::move(expectedResult).error()); } /** * @brief Extracts ledger index from a JSON value which can be either a number or a string. * * @param value The JSON value to extract ledger index from - * @return An optional containing the ledger index if it is a number; std::nullopt otherwise - * @throws logic_error comes from integralValueAs if the underlying number is neither int64 nor uint64 - * @throws std::invalid_argument or std::out_of_range if the string cannot be converted to a number + * @return The extracted ledger index or an error message */ -[[nodiscard]] inline std::optional +[[nodiscard]] inline std::expected getLedgerIndex(boost::json::value const& value) { - std::optional ledgerIndex; - if (not value.is_string()) { - ledgerIndex = util::integralValueAs(value); - } else if (value.as_string() != "validated") { - ledgerIndex = std::stoi(value.as_string().c_str()); + return tryIntegralValueAs(value); } - - return ledgerIndex; + if (value.as_string() != "validated") { + uint32_t ledgerIndex{}; + if (beast::lexicalCastChecked(ledgerIndex, value.as_string().c_str())) { + return ledgerIndex; + } + return std::unexpected("Invalid ledger index string"); + } + return std::unexpected("'validated' ledger index is requested"); } } // namespace util diff --git a/src/util/ObservableValue.hpp b/src/util/ObservableValue.hpp new file mode 100644 index 0000000000..c44d2b7684 --- /dev/null +++ b/src/util/ObservableValue.hpp @@ -0,0 +1,426 @@ +//------------------------------------------------------------------------------ +/* + 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 +#include + +namespace util { + +template +concept SomeAtomic = std::same_as, std::atomic>>; + +/** + * @brief Concept defining types that can be observed for changes. + * + * A type is Observable if it satisfies all requirements for being stored + * and monitored in an ObservableValue container: + * + * - Must be equality comparable to detect changes + * - Must be copy constructible for capturing old values in guards + * - Must be move constructible for efficient value updates + * + * @note Copy assignment is intentionally not required since we use move semantics + * for value updates and only need copy construction for change detection. + */ +template +concept Observable = std::equality_comparable && std::copy_constructible && std::move_constructible; + +namespace impl { + +/** + * @brief Base class containing common ObservableValue functionality. + * + * This class contains all the observer management and notification logic + * that is shared between regular and atomic ObservableValue specializations. + * + * @tparam T The value type (for atomic specializations, this is the underlying type, not std::atomic) + */ +template +class ObservableValueBase { +protected: + boost::signals2::signal onUpdate_; + +public: + virtual ~ObservableValueBase() = default; + + /** + * @brief Registers an observer callback for value changes. + * @param fn Callback function/lambda that accepts T const& + * @return Connection object for managing the subscription + */ + boost::signals2::connection + observe(std::invocable auto&& fn) + { + return onUpdate_.connect(std::forward(fn)); + } + + /** + * @brief Checks if there are any active observers. + * @return true if there are observers, false otherwise + */ + [[nodiscard]] bool + hasObservers() const + { + return not onUpdate_.empty(); + } + + /** + * @brief Forces notification of all observers with the current value. + * + * This method will notify all observers with the current value regardless + * of whether the value has changed since the last notification. + */ + virtual void + forceNotify() = 0; + +protected: + /** + * @brief Notifies all observers with the given value. + * @param value The value to send to observers + */ + void + notifyObservers(T const& value) + { + onUpdate_(value); + } +}; + +} // namespace impl + +// Forward declaration +template +class ObservableValue; + +/** + * @brief An observable value container that notifies observers when the value changes. + * + * ObservableValue wraps a value of type T and provides a mechanism to observe changes to that value. + * When the value is modified (and actually changes), all registered observers are notified. + * + * @tparam T The type of value to observe. Must satisfy the Observable concept. + * + * @par Thread Safety + * - Observer subscription/unsubscription (observe() and connection.disconnect()) are thread-safe + * - Value modification operations (set(), operator=) are NOT thread-safe and require external synchronization + * - Observer callbacks are invoked synchronously on the same thread that triggered the value change + * - If observers need to perform work on different threads, they must handle dispatch themselves + * (e.g., using an async execution context or message queue) + * + * @par Exception Handling + * - If an observer callback throws an exception, the exception will propagate to the caller + * - The value will still be updated even if observers throw exceptions + * - No guarantee is made about whether other observers will be called if one throws + * - It is the caller's responsibility to handle exceptions from observer callbacks + */ +template + requires(not SomeAtomic) +class ObservableValue : public impl::ObservableValueBase { + T value_; + + /** + * @brief RAII guard for deferred notification of value changes. + * + * ObservableGuard captures the current value when created and compares it + * with the final value when destroyed. If the values differ, observers + * are notified. This allows for multiple modifications to the value with + * only a single notification at the end. + * + * @note This class is returned by operator->() and should not be used directly. + */ + struct ObservableGuard { + T const oldValue; ///< Value captured at construction time + ObservableValue& ref; ///< Reference to the observable value + + /** + * @brief Constructs guard and captures current value. + * @param observable The ObservableValue to guard + */ + ObservableGuard(ObservableValue& observable) : oldValue(observable), ref(observable) + { + } + + /** + * @brief Destructor that triggers notification if value changed. + * + * Compares the captured value with the current value. If they differ, + * notifies all observers with the current value. + */ + ~ObservableGuard() + { + if (oldValue != ref.value_) + ref.notifyObservers(ref.value_); + } + + /** + * @brief Provides mutable access to the underlying value. + * @return Mutable reference to the wrapped value + */ + [[nodiscard]] + operator T&() + { + return ref.value_; + } + }; + +public: + /** + * @brief Constructs ObservableValue with initial value. + * @param value Initial value (must be convertible to T) + */ + ObservableValue(std::convertible_to auto&& value) : value_{std::forward(value)} + { + } + + /** + * @brief Constructs ObservableValue with default initial value. + */ + ObservableValue() + requires std::default_initializable + : value_{} + { + } + + ObservableValue(ObservableValue const&) = delete; + ObservableValue(ObservableValue&&) = default; + ObservableValue& + operator=(ObservableValue const&) = delete; + ObservableValue& + operator=(ObservableValue&&) = default; + + /** + * @brief Assignment operator that updates value and notifies observers. + * + * Updates the stored value and notifies observers if the new value + * differs from the current value (using operator!=). + * + * @param val New value (must be convertible to T) + * @return Reference to this object for chaining + * + * @throws Any exception thrown by observer callbacks will propagate + */ + ObservableValue& + operator=(std::convertible_to auto&& val) + { + set(val); + return *this; + } + + /** + * @brief Provides deferred notification access to the value. + * + * Returns an ObservableGuard that allows modification of the value + * with notification deferred until the guard is destroyed. + * + * @return ObservableGuard for deferred notification + */ + [[nodiscard]] ObservableGuard + operator->() + { + return {*this}; + } + + /** + * @brief Implicit conversion to const reference of the value. + * @return Const reference to the stored value + */ + [[nodiscard]] + operator T const&() const + { + return value_; + } + + /** + * @brief Explicitly gets the current value. + * @return Const reference to the stored value + */ + [[nodiscard]] T const& + get() const + { + return value_; + } + + /** + * @brief Sets a new value and notifies observers if changed. + * + * Updates the stored value and notifies all observers if the new value + * differs from the current value (using operator!=). If the values are + * equal, no notification occurs. + * + * @param val New value (must be convertible to T) + * + * @throws Any exception thrown by observer callbacks will propagate + * + * @par Thread Safety + * - This method is NOT thread-safe and requires external synchronization for concurrent access + * - Observer callbacks are invoked synchronously on the calling thread + */ + void + set(std::convertible_to auto&& val) + { + if (value_ != val) { + value_ = std::forward(val); + this->notifyObservers(value_); + } + } + + /** + * @brief Forces notification of all observers with the current value. + * + * This method will notify all observers with the current value regardless + * of whether the value has changed since the last notification. + */ + void + forceNotify() override + { + this->notifyObservers(value_); + } +}; + +/** + * @brief Partial specialization of ObservableValue for atomic types. + * + * This specialization provides thread-safe observation of atomic values while + * maintaining atomic semantics. It avoids the issues of copying atomic values + * and handles race conditions properly. + * + * @tparam T The underlying type stored in the atomic + * + * @par Thread Safety + * - All operations are thread-safe + * - Observer notifications are atomic with respect to value changes + * - Multiple threads can safely modify and observe the atomic value + * + * @par Performance Considerations + * - Uses atomic compare-and-swap operations for updates + * - Minimizes atomic reads during guard operations + * - Observer notifications happen outside of atomic operations when possible + */ +template +class ObservableValue> : public impl::ObservableValueBase { + std::atomic value_; + +public: + /** + * @brief Constructs ObservableValue with initial atomic value. + * @param value Initial value (will be stored in the atomic) + */ + ObservableValue(std::convertible_to auto&& value) : value_{std::forward(value)} + { + } + + /** + * @brief Constructs ObservableValue with default initial value. + */ + ObservableValue() + requires std::default_initializable + : value_{} + { + } + + ObservableValue(ObservableValue const&) = delete; + ObservableValue(ObservableValue&&) = default; + ObservableValue& + operator=(ObservableValue const&) = delete; + ObservableValue& + operator=(ObservableValue&&) = default; + + /** + * @brief Assignment operator that updates atomic value and notifies observers. + * + * Uses atomic compare-and-swap to update the value and notifies observers + * only if the value actually changed. + * + * @param val New value + * @return Reference to this object for chaining + */ + ObservableValue& + operator=(std::convertible_to auto&& val) + { + set(std::forward(val)); + return *this; + } + + /** + * @brief Gets the current atomic value. + * @return Current value stored in the atomic + */ + [[nodiscard]] T + get() const + { + return value_.load(); + } + + /** + * @brief Implicit conversion to the current atomic value. + * @return Current value stored in the atomic + */ + [[nodiscard]] + operator T() const + { + return get(); + } + + /** + * @brief Sets a new atomic value and notifies observers if changed. + * + * Uses atomic compare-and-swap to update the value. Notifies all observers + * if the value actually changed. + * + * @param val New value + */ + void + set(std::convertible_to auto&& val) + { + T newValue = std::forward(val); + T oldValue = value_.load(); + + // Use compare-and-swap to atomically update + while (!value_.compare_exchange_weak(oldValue, newValue)) { + // compare_exchange_weak updates oldValue with current value on failure + // Continue until we succeed + } + + // Notify observers if we actually changed the value + // Note: oldValue now contains the actual previous value that was replaced + if (oldValue != newValue) { + this->notifyObservers(newValue); + } + } + + /** + * @brief Forces notification of all observers with the current value. + * + * This method will notify all observers with the current atomic value + * regardless of whether the value has changed since the last notification. + */ + void + forceNotify() override + { + this->notifyObservers(value_.load()); + } +}; + +} // namespace util diff --git a/src/util/Repeat.cpp b/src/util/Repeat.cpp index 725f008a0e..c8ee12a81a 100644 --- a/src/util/Repeat.cpp +++ b/src/util/Repeat.cpp @@ -19,6 +19,8 @@ #include "util/Repeat.hpp" +#include + namespace util { void @@ -27,8 +29,11 @@ Repeat::stop() if (control_->stopping) return; - control_->stopping = true; - control_->timer.cancel(); + boost::asio::post(control_->strand, [control = control_] { + control->stopping = true; + control->timer.cancel(); + }); + control_->semaphore.acquire(); } diff --git a/src/util/Repeat.hpp b/src/util/Repeat.hpp index 1746f87d74..de21e02c48 100644 --- a/src/util/Repeat.hpp +++ b/src/util/Repeat.hpp @@ -21,9 +21,11 @@ #include "util/Assert.hpp" +#include #include #include #include +#include #include #include @@ -41,10 +43,11 @@ namespace util { class Repeat { struct Control { boost::asio::steady_timer timer; + boost::asio::strand strand; std::atomic_bool stopping{true}; std::binary_semaphore semaphore{0}; - Control(auto& ctx) : timer(ctx) + Control(auto& ctx) : timer(ctx), strand(boost::asio::make_strand(ctx)) { } }; @@ -98,15 +101,24 @@ class Repeat { static void startImpl(std::shared_ptr control, std::chrono::steady_clock::duration interval, Action&& action) { - control->timer.expires_after(interval); - control->timer.async_wait([control, interval, action = std::forward(action)](auto const& ec) mutable { - if (ec or control->stopping) { + boost::asio::post(control->strand, [control, interval, action = std::forward(action)]() mutable { + if (control->stopping) { control->semaphore.release(); return; } - action(); - startImpl(std::move(control), interval, std::forward(action)); + control->timer.expires_after(interval); + control->timer.async_wait( + [control, interval, action = std::forward(action)](auto const& ec) mutable { + if (ec or control->stopping) { + control->semaphore.release(); + return; + } + action(); + + startImpl(std::move(control), interval, std::forward(action)); + } + ); }); } }; diff --git a/src/util/SignalsHandler.cpp b/src/util/SignalsHandler.cpp index 9747cc57d1..1e1507d912 100644 --- a/src/util/SignalsHandler.cpp +++ b/src/util/SignalsHandler.cpp @@ -23,10 +23,13 @@ #include "util/config/ConfigDefinition.hpp" #include "util/log/Logger.hpp" +#include #include +#include #include #include -#include +#include +#include #include namespace util { @@ -50,17 +53,11 @@ class SignalsHandlerStatic { } static void - handleSignal(int signal) + handleSignal(int /* signal */) { ASSERT(installedHandler != nullptr, "SignalsHandler is not initialized"); - installedHandler->stopHandler_(signal); - } - - static void - handleSecondSignal(int signal) - { - ASSERT(installedHandler != nullptr, "SignalsHandler is not initialized"); - installedHandler->secondSignalHandler_(signal); + installedHandler->signalReceived_ = true; + installedHandler->cv_.notify_one(); } }; @@ -69,56 +66,109 @@ SignalsHandler* SignalsHandlerStatic::installedHandler = nullptr; } // namespace impl SignalsHandler::SignalsHandler(config::ClioConfigDefinition const& config, std::function forceExitHandler) - : gracefulPeriod_(0) - , context_(1) - , stopHandler_([this, forceExitHandler](int) mutable { - LOG(LogService::info()) << "Got stop signal. Stopping Clio. Graceful period is " - << std::chrono::duration_cast(gracefulPeriod_).count() - << " milliseconds."; - setHandler(impl::SignalsHandlerStatic::handleSecondSignal); - timer_.emplace(context_.scheduleAfter( - gracefulPeriod_, [forceExitHandler = std::move(forceExitHandler)](auto&& stopToken, bool canceled) { - // TODO: Update this after https://github.com/XRPLF/clio/issues/1380 - if (not stopToken.isStopRequested() and not canceled) { - LOG(LogService::warn()) << "Force exit at the end of graceful period."; - forceExitHandler(); - } - } - )); - stopSignal_(); - }) - , secondSignalHandler_([this, forceExitHandler = std::move(forceExitHandler)](int) { - LOG(LogService::warn()) << "Force exit on second signal."; - forceExitHandler(); - cancelTimer(); - setHandler(); - }) + : gracefulPeriod_(util::config::ClioConfigDefinition::toMilliseconds(config.get("graceful_period"))) + , forceExitHandler_(std::move(forceExitHandler)) { impl::SignalsHandlerStatic::registerHandler(*this); - - gracefulPeriod_ = util::config::ClioConfigDefinition::toMilliseconds(config.get("graceful_period")); + workerThread_ = std::thread([this]() { runStateMachine(); }); setHandler(impl::SignalsHandlerStatic::handleSignal); } SignalsHandler::~SignalsHandler() { - cancelTimer(); setHandler(); + + state_ = State::NormalExit; + cv_.notify_one(); + + if (workerThread_.joinable()) + workerThread_.join(); + impl::SignalsHandlerStatic::resetHandler(); // This is needed mostly for tests to reset static state } void -SignalsHandler::cancelTimer() +SignalsHandler::notifyGracefulShutdownComplete() { - if (timer_.has_value()) - timer_->abort(); + if (state_ == State::GracefulShutdown) { + LOG(LogService::info()) << "Graceful shutdown completed successfully."; + state_ = State::NormalExit; + cv_.notify_one(); + } } void SignalsHandler::setHandler(void (*handler)(int)) { - for (int const signal : kHANDLED_SIGNALS) { + for (int const signal : kHANDLED_SIGNALS) std::signal(signal, handler == nullptr ? SIG_DFL : handler); +} + +void +SignalsHandler::runStateMachine() +{ + while (state_ != State::NormalExit) { + auto currentState = state_.load(); + + switch (currentState) { + case State::WaitingForSignal: { + { + std::unique_lock lock(mutex_); + cv_.wait(lock, [this]() { return signalReceived_ or state_ == State::NormalExit; }); + } + + if (state_ == State::NormalExit) + return; + + LOG( + LogService::info() + ) << "Got stop signal. Stopping Clio. Graceful period is " + << std::chrono::duration_cast(gracefulPeriod_).count() << " milliseconds."; + + state_ = State::GracefulShutdown; + signalReceived_ = false; + + stopSignal_(); + break; + } + + case State::GracefulShutdown: { + bool waitResult = false; + { + std::unique_lock lock(mutex_); + + // Wait for either: + // 1. Graceful period to elapse (timeout) + // 2. Another signal (signalReceived_) + // 3. Graceful shutdown completion (state changes to NormalExit) + waitResult = cv_.wait_for(lock, gracefulPeriod_, [this]() { + return signalReceived_ or state_ == State::NormalExit; + }); + } + + if (state_ == State::NormalExit) + break; + + if (signalReceived_) { + LOG(LogService::warn()) << "Force exit on second signal."; + state_ = State::ForceExit; + signalReceived_ = false; + } else if (not waitResult) { + LOG(LogService::warn()) << "Force exit at the end of graceful period."; + state_ = State::ForceExit; + } + break; + } + + case State::ForceExit: { + forceExitHandler_(); + state_ = State::NormalExit; + break; + } + + case State::NormalExit: + return; + } } } diff --git a/src/util/SignalsHandler.hpp b/src/util/SignalsHandler.hpp index d914637f1d..1ffe2190c5 100644 --- a/src/util/SignalsHandler.hpp +++ b/src/util/SignalsHandler.hpp @@ -19,22 +19,20 @@ #pragma once -#include "util/async/context/BasicExecutionContext.hpp" #include "util/config/ConfigDefinition.hpp" #include "util/log/Logger.hpp" -#include -#include -#include #include -#include +#include #include #include +#include #include #include #include -#include +#include +#include namespace util { namespace impl { @@ -48,13 +46,22 @@ class SignalsHandlerStatic; * @note There could be only one instance of this class. */ class SignalsHandler { + /** + * @brief States of the signal handler state machine. + */ + enum class State { WaitingForSignal, GracefulShutdown, ForceExit, NormalExit }; + std::chrono::steady_clock::duration gracefulPeriod_; - async::PoolExecutionContext context_; - std::optional> timer_; + std::function forceExitHandler_; boost::signals2::signal stopSignal_; - std::function stopHandler_; - std::function secondSignalHandler_; + + std::atomic signalReceived_{false}; + std::atomic state_{State::WaitingForSignal}; + + std::mutex mutex_; + std::condition_variable cv_; + std::thread workerThread_; friend class impl::SignalsHandlerStatic; @@ -101,15 +108,16 @@ class SignalsHandler { stopSignal_.connect(static_cast(priority), std::forward(callback)); } - static constexpr auto kHANDLED_SIGNALS = {SIGINT, SIGTERM}; - -private: /** - * @brief Cancel scheduled force exit if any. + * @brief Notify the signal handler that graceful shutdown has completed. + * This allows the handler to transition to NormalExit state. */ void - cancelTimer(); + notifyGracefulShutdownComplete(); + static constexpr auto kHANDLED_SIGNALS = {SIGINT, SIGTERM}; + +private: /** * @brief Set signal handler for handled signals. * @@ -118,6 +126,12 @@ class SignalsHandler { static void setHandler(void (*handler)(int) = nullptr); + /** + * @brief Run the state machine loop in a worker thread. + */ + void + runStateMachine(); + static constexpr auto kDEFAULT_FORCE_EXIT_HANDLER = []() { std::exit(EXIT_FAILURE); }; }; diff --git a/src/util/requests/RequestBuilder.cpp b/src/util/requests/RequestBuilder.cpp index df4ba4e9d8..a8b8faf0a9 100644 --- a/src/util/requests/RequestBuilder.cpp +++ b/src/util/requests/RequestBuilder.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include diff --git a/src/util/requests/impl/StreamData.hpp b/src/util/requests/impl/StreamData.hpp index a976871cc9..e3d040c831 100644 --- a/src/util/requests/impl/StreamData.hpp +++ b/src/util/requests/impl/StreamData.hpp @@ -78,8 +78,8 @@ class SslStreamData { } }; -using SslTcpStreamData = SslStreamData>; +using SslTcpStreamData = SslStreamData>; using SslWsStreamData = - SslStreamData>>; + SslStreamData>>; } // namespace util::requests::impl diff --git a/src/util/requests/impl/WsConnectionImpl.hpp b/src/util/requests/impl/WsConnectionImpl.hpp index 341c118d53..14bcee47e6 100644 --- a/src/util/requests/impl/WsConnectionImpl.hpp +++ b/src/util/requests/impl/WsConnectionImpl.hpp @@ -124,6 +124,6 @@ class WsConnectionImpl : public WsConnection { using PlainWsConnection = WsConnectionImpl>; using SslWsConnection = - WsConnectionImpl>>; + WsConnectionImpl>>; } // namespace util::requests::impl diff --git a/src/web/SslHttpSession.hpp b/src/web/SslHttpSession.hpp index 41a6da20f2..d06dcab112 100644 --- a/src/web/SslHttpSession.hpp +++ b/src/web/SslHttpSession.hpp @@ -61,7 +61,7 @@ using tcp = boost::asio::ip::tcp; template class SslHttpSession : public impl::HttpBase, public std::enable_shared_from_this> { - boost::beast::ssl_stream stream_; + boost::asio::ssl::stream stream_; std::reference_wrapper tagFactory_; std::uint32_t maxWsSendingQueueSize_; @@ -113,7 +113,7 @@ class SslHttpSession : public impl::HttpBase, ~SslHttpSession() override = default; /** @return The SSL stream. */ - boost::beast::ssl_stream& + boost::asio::ssl::stream& stream() { return stream_; diff --git a/src/web/SslWsSession.hpp b/src/web/SslWsSession.hpp index d5b317db36..c58e89f8d1 100644 --- a/src/web/SslWsSession.hpp +++ b/src/web/SslWsSession.hpp @@ -51,7 +51,7 @@ namespace web { */ template class SslWsSession : public impl::WsBase { - using StreamType = boost::beast::websocket::stream>; + using StreamType = boost::beast::websocket::stream>; StreamType ws_; public: @@ -68,7 +68,7 @@ class SslWsSession : public impl::WsBase { * @param maxWsSendingQueueSize The maximum size of the sending queue for websocket */ explicit SslWsSession( - boost::beast::ssl_stream&& stream, + boost::asio::ssl::stream&& stream, std::string ip, std::reference_wrapper tagFactory, std::reference_wrapper dosGuard, @@ -107,7 +107,7 @@ template class SslWsUpgrader : public std::enable_shared_from_this> { using std::enable_shared_from_this>::shared_from_this; - boost::beast::ssl_stream https_; + boost::asio::ssl::stream https_; boost::optional> parser_; boost::beast::flat_buffer buffer_; std::string ip_; @@ -133,7 +133,7 @@ class SslWsUpgrader : public std::enable_shared_from_this stream, + boost::asio::ssl::stream stream, std::string ip, std::reference_wrapper tagFactory, std::reference_wrapper dosGuard, diff --git a/tests/common/util/TestHttpClient.cpp b/tests/common/util/TestHttpClient.cpp index c9d6e611cb..850745523c 100644 --- a/tests/common/util/TestHttpClient.cpp +++ b/tests/common/util/TestHttpClient.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -40,7 +41,6 @@ #include #include #include // IWYU pragma: keep -#include #include #include #include @@ -148,7 +148,7 @@ HttpsSyncClient::syncPost(std::string const& host, std::string const& port, std: ctx.set_verify_mode(ssl::verify_none); tcp::resolver resolver(ioc); - boost::beast::ssl_stream stream(ioc, ctx); + boost::asio::ssl::stream stream(ioc, ctx); // We can't fix this so have to ignore #pragma GCC diagnostic push diff --git a/tests/common/util/TestObject.cpp b/tests/common/util/TestObject.cpp index 7f28a97569..2db24dffd8 100644 --- a/tests/common/util/TestObject.cpp +++ b/tests/common/util/TestObject.cpp @@ -1452,7 +1452,8 @@ createMptIssuanceObject( std::optional assetScale, std::optional maxAmount, std::optional lockedAmount, - std::optional domainId + std::optional domainId, + std::optional mutableFlags ) { ripple::STObject mptIssuance(ripple::sfLedgerEntry); @@ -1479,6 +1480,8 @@ createMptIssuanceObject( } if (domainId.has_value()) mptIssuance.setFieldH256(ripple::sfDomainID, ripple::uint256{*domainId}); + if (mutableFlags.has_value()) + mptIssuance.setFieldU32(ripple::sfMutableFlags, *mutableFlags); return mptIssuance; } @@ -1789,7 +1792,7 @@ createVault( vault[ripple::sfShareMPTID] = shareMPTID; vault.setFieldNumber(ripple::sfAssetsTotal, ripple::STNumber{ripple::sfAssetsTotal, 300}); vault.setFieldNumber(ripple::sfAssetsAvailable, ripple::STNumber{ripple::sfAssetsAvailable, 300}); - vault.setFieldNumber(ripple::sfLossUnrealized, ripple::STNumber{ripple::sfLossUnrealized, 0}); + vault.setFieldNumber(ripple::sfLossUnrealized, ripple::STNumber{ripple::sfLossUnrealized, 1}); vault.setFieldU8(ripple::sfWithdrawalPolicy, 200); vault.setFieldU32(ripple::sfFlags, 0); diff --git a/tests/common/util/TestObject.hpp b/tests/common/util/TestObject.hpp index fd811c59ab..e06fa62b2e 100644 --- a/tests/common/util/TestObject.hpp +++ b/tests/common/util/TestObject.hpp @@ -461,7 +461,8 @@ createMptIssuanceObject( std::optional assetScale = std::nullopt, std::optional maxAmount = std::nullopt, std::optional lockedAmount = std::nullopt, - std::optional domainId = std::nullopt + std::optional domainId = std::nullopt, + std::optional mutableFlags = std::nullopt ); [[nodiscard]] ripple::STObject diff --git a/tests/common/util/TestWebSocketClient.hpp b/tests/common/util/TestWebSocketClient.hpp index 726574ded5..81a6e10b9d 100644 --- a/tests/common/util/TestWebSocketClient.hpp +++ b/tests/common/util/TestWebSocketClient.hpp @@ -80,7 +80,7 @@ class WebSocketAsyncClient { class WebServerSslSyncClient { boost::asio::io_context ioc_; - std::optional>> ws_; + std::optional>> ws_; public: void diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index cff464b7ac..fe15f63305 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -169,6 +169,8 @@ target_sources( util/BytesConverterTests.cpp util/CoroutineTest.cpp util/MoveTrackerTests.cpp + util/ObservableValueTest.cpp + util/ObservableValueAtomicTest.cpp util/RandomTests.cpp util/RepeatTests.cpp util/ResponseExpirationCacheTests.cpp diff --git a/tests/unit/JsonUtilTests.cpp b/tests/unit/JsonUtilTests.cpp index 52d68eefc5..1fe5d36010 100644 --- a/tests/unit/JsonUtilTests.cpp +++ b/tests/unit/JsonUtilTests.cpp @@ -18,6 +18,7 @@ //============================================================================== #include "util/JsonUtils.hpp" +#include "util/NameGenerator.hpp" #include #include @@ -26,7 +27,8 @@ #include #include #include -#include +#include +#include TEST(JsonUtils, RemoveSecrets) { @@ -90,28 +92,123 @@ TEST(JsonUtils, integralValueAs) EXPECT_THROW(util::integralValueAs(stringJson), std::logic_error); } -TEST(JsonUtils, getLedgerIndex) +TEST(JsonUtils, tryIntegralValueAs) { - auto const emptyJson = boost::json::value(); - EXPECT_THROW(std::ignore = util::getLedgerIndex(emptyJson), std::logic_error); - - auto const boolJson = boost::json::value(true); - EXPECT_THROW(std::ignore = util::getLedgerIndex(emptyJson), std::logic_error); - - auto const numberJson = boost::json::value(12345); - auto ledgerIndex = util::getLedgerIndex(numberJson); - EXPECT_TRUE(ledgerIndex.has_value()); - EXPECT_EQ(ledgerIndex.value(), 12345u); + auto const expectedResultUint64 = static_cast(std::numeric_limits::max()) + 1u; + auto const uint64Json = boost::json::value(expectedResultUint64); - auto const validStringJson = boost::json::value("12345"); - ledgerIndex = util::getLedgerIndex(validStringJson); - EXPECT_TRUE(ledgerIndex.has_value()); - EXPECT_EQ(ledgerIndex.value(), 12345u); + auto const expectedResultInt64 = static_cast(std::numeric_limits::max()) + 1u; + auto const int64Json = boost::json::value(expectedResultInt64); - auto const invalidStringJson = boost::json::value("invalid123"); - EXPECT_THROW(std::ignore = util::getLedgerIndex(invalidStringJson), std::invalid_argument); + auto checkHasValue = [&](boost::json::value const& jv, auto const& expectedValue) { + using T = std::remove_cvref_t; + auto const res = util::tryIntegralValueAs(jv); + ASSERT_TRUE(res.has_value()); + EXPECT_EQ(res.value(), expectedValue); + }; + + auto checkError = [&](boost::json::value const& jv) { + auto res = util::tryIntegralValueAs(jv); + EXPECT_FALSE(res.has_value()); + EXPECT_EQ(res.error(), "Value neither uint64 nor int64"); + }; + + // checks for uint64Json + checkHasValue(uint64Json, std::numeric_limits::min()); + checkHasValue(uint64Json, static_cast(expectedResultUint64)); + checkHasValue(uint64Json, static_cast(expectedResultUint64)); + checkHasValue(uint64Json, expectedResultUint64); + + // checks for int64Json + checkHasValue(int64Json, std::numeric_limits::min()); + checkHasValue(int64Json, static_cast(expectedResultInt64)); + checkHasValue(int64Json, expectedResultInt64); + checkHasValue(int64Json, static_cast(expectedResultInt64)); + + // non-integral inputs + checkError(boost::json::value()); + checkError(boost::json::value(false)); + checkError(boost::json::value(3.14)); + checkError(boost::json::value("not a number")); +} - auto const validatedJson = boost::json::value("validated"); - ledgerIndex = util::getLedgerIndex(validatedJson); - EXPECT_FALSE(ledgerIndex.has_value()); +struct GetLedgerIndexParameterTestBundle { + std::string testName; + boost::json::value jv; + std::expected expectedResult; +}; + +// parameterized test cases for parameters check +struct GetLedgerIndexParameterTest : ::testing::TestWithParam {}; + +INSTANTIATE_TEST_CASE_P( + JsonUtils, + GetLedgerIndexParameterTest, + testing::Values( + GetLedgerIndexParameterTestBundle{ + .testName = "EmptyValue", + .jv = boost::json::value(), + .expectedResult = std::unexpected{"Value neither uint64 nor int64"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "BoolValue", + .jv = boost::json::value(false), + .expectedResult = std::unexpected{"Value neither uint64 nor int64"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "NumberValue", + .jv = boost::json::value(123), + .expectedResult = 123u + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringNumberValue", + .jv = boost::json::value("123"), + .expectedResult = 123u + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringNumberWithPlusSignValue", + .jv = boost::json::value("+123"), + .expectedResult = 123u + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringEmptyValue", + .jv = boost::json::value(""), + .expectedResult = std::unexpected{"Invalid ledger index string"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringWithLeadingCharsValue", + .jv = boost::json::value("123invalid"), + .expectedResult = std::unexpected{"Invalid ledger index string"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringWithTrailingCharsValue", + .jv = boost::json::value("invalid123"), + .expectedResult = std::unexpected{"Invalid ledger index string"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "StringWithLeadingAndTrailingCharsValue", + .jv = boost::json::value("123invalid123"), + .expectedResult = std::unexpected{"Invalid ledger index string"} + }, + GetLedgerIndexParameterTestBundle{ + .testName = "ValidatedStringValue", + .jv = boost::json::value("validated"), + .expectedResult = std::unexpected{"'validated' ledger index is requested"} + } + ), + tests::util::kNAME_GENERATOR +); + +TEST_P(GetLedgerIndexParameterTest, getLedgerIndexParams) +{ + auto const& testBundle = GetParam(); + auto const ledgerIndex = util::getLedgerIndex(testBundle.jv); + + if (testBundle.expectedResult.has_value()) { + EXPECT_TRUE(ledgerIndex.has_value()); + EXPECT_EQ(ledgerIndex.value(), testBundle.expectedResult.value()); + } else { + EXPECT_FALSE(ledgerIndex.has_value()); + EXPECT_EQ(ledgerIndex.error(), testBundle.expectedResult.error()); + } } diff --git a/tests/unit/app/StopperTests.cpp b/tests/unit/app/StopperTests.cpp index 0dd5845b2f..a9a03eb564 100644 --- a/tests/unit/app/StopperTests.cpp +++ b/tests/unit/app/StopperTests.cpp @@ -40,6 +40,7 @@ struct StopperTest : virtual public ::testing::Test { protected: // Order here is important, stopper_ should die before mockCallback_, otherwise UB testing::StrictMock> mockCallback_; + testing::StrictMock> mockCompleteCallback_; Stopper stopper_; }; @@ -60,6 +61,22 @@ TEST_F(StopperTest, stopCalledMultipleTimes) stopper_.stop(); } +TEST_F(StopperTest, stopCallsCompletionCallback) +{ + stopper_.setOnStop(mockCallback_.AsStdFunction()); + stopper_.setOnComplete(mockCompleteCallback_.AsStdFunction()); + EXPECT_CALL(mockCallback_, Call); + EXPECT_CALL(mockCompleteCallback_, Call); + stopper_.stop(); +} + +TEST_F(StopperTest, stopWithoutCompletionCallback) +{ + stopper_.setOnStop(mockCallback_.AsStdFunction()); + EXPECT_CALL(mockCallback_, Call); + stopper_.stop(); +} + struct StopperMakeCallbackTest : util::prometheus::WithPrometheus, SyncAsioContextTest { struct ServerMock : web::ServerTag { MOCK_METHOD(void, stop, (boost::asio::yield_context), ()); diff --git a/tests/unit/app/WebHandlersTests.cpp b/tests/unit/app/WebHandlersTests.cpp index c2177e9f37..58b9f0623a 100644 --- a/tests/unit/app/WebHandlersTests.cpp +++ b/tests/unit/app/WebHandlersTests.cpp @@ -19,6 +19,7 @@ #include "app/WebHandlers.hpp" #include "rpc/Errors.hpp" +#include "rpc/WorkQueue.hpp" #include "util/AsioContextTestFixture.hpp" #include "util/MockLedgerCache.hpp" #include "util/MockPrometheus.hpp" @@ -122,7 +123,9 @@ struct MetricsHandlerTests : util::prometheus::WithPrometheus, SyncAsioContextTe std::make_shared>() }; - MetricsHandler metricsHandler{adminVerifier}; + rpc::WorkQueue workQueue{1}; + + MetricsHandler metricsHandler{adminVerifier, workQueue}; web::ng::Request request{http::request{http::verb::get, "/metrics", 11}}; }; diff --git a/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp b/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp index 88d0fd182c..09af77b636 100644 --- a/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp +++ b/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp @@ -62,6 +62,7 @@ constexpr auto kISSUANCE_INDEX2 = "B6DBAFC99223B42257915A63DFC6B0C032D4070F9A574 constexpr uint64_t kISSUANCE1_MAX_AMOUNT = 10000; constexpr uint64_t kISSUANCE1_OUTSTANDING_AMOUNT = 5000; constexpr uint8_t kISSUANCE1_ASSET_SCALE = 2; +constexpr uint16_t kISSUANCE1_TRANSFER_FEE = 10; // unique values for issuance2 constexpr uint64_t kISSUANCE2_MAX_AMOUNT = 20000; @@ -75,6 +76,7 @@ constexpr auto kISSUANCE2_DOMAIN_ID_HEX = "E6DBAFC99223B42257915A63DFC6B0C032D40 // define expected JSON for mpt issuances auto const kISSUANCE_OUT1 = fmt::format( R"JSON({{ + "mpt_issuance_id": "{}", "issuer": "{}", "sequence": 1, "maximum_amount": {}, @@ -85,6 +87,7 @@ auto const kISSUANCE_OUT1 = fmt::format( "mpt_require_auth": true, "mpt_can_transfer": true }})JSON", + kISSUANCE_INDEX1, kACCOUNT, kISSUANCE1_MAX_AMOUNT, kISSUANCE1_OUTSTANDING_AMOUNT, @@ -93,6 +96,7 @@ auto const kISSUANCE_OUT1 = fmt::format( auto const kISSUANCE_OUT2 = fmt::format( R"JSON({{ + "mpt_issuance_id": "{}", "issuer": "{}", "sequence": 2, "maximum_amount": {}, @@ -105,6 +109,7 @@ auto const kISSUANCE_OUT2 = fmt::format( "mpt_locked": true, "mpt_can_clawback": true }})JSON", + kISSUANCE_INDEX2, kACCOUNT, kISSUANCE2_MAX_AMOUNT, kISSUANCE2_OUTSTANDING_AMOUNT, @@ -293,7 +298,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, NonExistLedgerViaLedgerIntIndex) // ledger not found via hash (seq > max) TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LedgerSeqOutOfRangeByHash) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 31); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 31); EXPECT_CALL(*backend_, fetchLedgerByHash(ripple::uint256{kLEDGER_HASH}, _)).WillOnce(Return(ledgerHeader)); auto const input = json::parse( fmt::format( @@ -341,7 +346,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LedgerSeqOutOfRangeByIndex) // account not exist TEST_F(RPCAccountMPTokenIssuancesHandlerTest, NonExistAccount) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerByHash(ripple::uint256{kLEDGER_HASH}, _)).WillOnce(Return(ledgerHeader)); // fetch account object return empty EXPECT_CALL(*backend_, doFetchLedgerObject).WillOnce(Return(std::optional{})); @@ -369,13 +374,13 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, NonExistAccount) // fetch mptoken issuances via account successfully TEST_F(RPCAccountMPTokenIssuancesHandlerTest, DefaultParameters) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); // return non-empty account - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); // return two mptoken issuance objects @@ -385,33 +390,36 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, DefaultParameters) ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); // mocking mptoken issuance ledger objects - std::vector bbs; - auto const issuance1 = createMptIssuanceObject( - kACCOUNT, - 1, - std::nullopt, - ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, - kISSUANCE1_OUTSTANDING_AMOUNT, - std::nullopt, - kISSUANCE1_ASSET_SCALE, - kISSUANCE1_MAX_AMOUNT - ); + auto const bbs = std::vector{ + createMptIssuanceObject( + kACCOUNT, + 1, + std::nullopt, + ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, + kISSUANCE1_OUTSTANDING_AMOUNT, + std::nullopt, + kISSUANCE1_ASSET_SCALE, + kISSUANCE1_MAX_AMOUNT + ) + .getSerializer() + .peekData(), - auto const issuance2 = createMptIssuanceObject( - kACCOUNT, - 2, - kISSUANCE2_METADATA, - ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, - kISSUANCE2_OUTSTANDING_AMOUNT, - kISSUANCE2_TRANSFER_FEE, - std::nullopt, - kISSUANCE2_MAX_AMOUNT, - kISSUANCE2_LOCKED_AMOUNT, - kISSUANCE2_DOMAIN_ID_HEX - ); + createMptIssuanceObject( + kACCOUNT, + 2, + kISSUANCE2_METADATA, + ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, + kISSUANCE2_OUTSTANDING_AMOUNT, + kISSUANCE2_TRANSFER_FEE, + std::nullopt, + kISSUANCE2_MAX_AMOUNT, + kISSUANCE2_LOCKED_AMOUNT, + kISSUANCE2_DOMAIN_ID_HEX + ) + .getSerializer() + .peekData() + }; - bbs.push_back(issuance1.getSerializer().peekData()); - bbs.push_back(issuance2.getSerializer().peekData()); EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); runSpawn([this](auto yield) { @@ -434,7 +442,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, DefaultParameters) kISSUANCE_OUT2 ); auto const input = json::parse(fmt::format(R"JSON({{"account": "{}"}})JSON", kACCOUNT)); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); @@ -445,22 +453,23 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, DefaultParameters) TEST_F(RPCAccountMPTokenIssuancesHandlerTest, UseLimit) { constexpr int kLIMIT = 20; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); ON_CALL(*backend_, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); - std::vector indexes; - std::vector bbs; - - for (int i = 0; i < 50; ++i) { - indexes.emplace_back(kISSUANCE_INDEX1); - auto const issuance = createMptIssuanceObject(kACCOUNT, i); - bbs.push_back(issuance.getSerializer().peekData()); - } + auto const indexes = std::vector(50, ripple::uint256{kISSUANCE_INDEX1}); + auto const bbs = [&]() { + std::vector v; + v.reserve(50); + for (int i = 0; i < 50; ++i) { + v.push_back(createMptIssuanceObject(kACCOUNT, i).getSerializer().peekData()); + } + return v; + }(); ripple::STObject ownerDir = createOwnerDirLedgerObject(indexes, kISSUANCE_INDEX1); ownerDir.setFieldU64(ripple::sfIndexNext, 99); @@ -482,7 +491,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, UseLimit) ) ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); @@ -531,27 +540,25 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MarkerOutput) { constexpr auto kNEXT_PAGE = 99; constexpr auto kLIMIT = 15; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto ownerDirKk = ripple::keylet::ownerDir(account).key; - auto ownerDir2Kk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const ownerDirKk = ripple::keylet::ownerDir(account).key; + auto const ownerDir2Kk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); EXPECT_CALL(*backend_, doFetchLedgerObject).Times(3); - std::vector indexes; - indexes.reserve(10); - for (int i = 0; i < 10; ++i) { - indexes.emplace_back(kISSUANCE_INDEX1); - } - - std::vector bbs; - bbs.reserve(kLIMIT); - for (int i = 0; i < kLIMIT; ++i) { - bbs.push_back(createMptIssuanceObject(kACCOUNT, i).getSerializer().peekData()); - } + auto const indexes = std::vector(10, ripple::uint256{kISSUANCE_INDEX1}); + auto const bbs = [&]() { + std::vector v; + v.reserve(kLIMIT); + for (int i = 0; i < kLIMIT; ++i) { + v.push_back(createMptIssuanceObject(kACCOUNT, i).getSerializer().peekData()); + } + return v; + }(); EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); // mock the first directory page @@ -577,7 +584,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MarkerOutput) kLIMIT ) ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); auto const& resultJson = (*output.result).as_object(); @@ -594,21 +601,24 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MarkerInput) constexpr auto kNEXT_PAGE = 99; constexpr auto kLIMIT = 15; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto ownerDirKk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const ownerDirKk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); EXPECT_CALL(*backend_, doFetchLedgerObject).Times(3); - std::vector bbs; - std::vector indexes; - for (int i = 0; i < kLIMIT; ++i) { - indexes.emplace_back(kISSUANCE_INDEX1); - bbs.push_back(createMptIssuanceObject(kACCOUNT, i).getSerializer().peekData()); - } + auto const indexes = std::vector(kLIMIT, ripple::uint256{kISSUANCE_INDEX1}); + auto const bbs = [&]() { + std::vector v; + v.reserve(kLIMIT); + for (int i = 0; i < kLIMIT; ++i) { + v.push_back(createMptIssuanceObject(kACCOUNT, i).getSerializer().peekData()); + } + return v; + }(); ripple::STObject ownerDir = createOwnerDirLedgerObject(indexes, kISSUANCE_INDEX1); ownerDir.setFieldU64(ripple::sfIndexNext, 0); @@ -631,7 +641,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MarkerInput) kNEXT_PAGE ) ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); @@ -643,47 +653,48 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MarkerInput) TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitLessThanMin) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject( {ripple::uint256{kISSUANCE_INDEX1}, ripple::uint256{kISSUANCE_INDEX2}}, kISSUANCE_INDEX1 ); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); - - std::vector bbs; - auto const issuance1 = createMptIssuanceObject( - kACCOUNT, - 1, - std::nullopt, - ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, - kISSUANCE1_OUTSTANDING_AMOUNT, - std::nullopt, - kISSUANCE1_ASSET_SCALE, - kISSUANCE1_MAX_AMOUNT - ); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); - auto const issuance2 = createMptIssuanceObject( - kACCOUNT, - 2, - kISSUANCE2_METADATA, - ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, - kISSUANCE2_OUTSTANDING_AMOUNT, - kISSUANCE2_TRANSFER_FEE, - std::nullopt, - kISSUANCE2_MAX_AMOUNT, - kISSUANCE2_LOCKED_AMOUNT, - kISSUANCE2_DOMAIN_ID_HEX - ); + auto const bbs = std::vector{ + createMptIssuanceObject( + kACCOUNT, + 1, + std::nullopt, + ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, + kISSUANCE1_OUTSTANDING_AMOUNT, + std::nullopt, + kISSUANCE1_ASSET_SCALE, + kISSUANCE1_MAX_AMOUNT + ) + .getSerializer() + .peekData(), - bbs.push_back(issuance1.getSerializer().peekData()); - bbs.push_back(issuance2.getSerializer().peekData()); + createMptIssuanceObject( + kACCOUNT, + 2, + kISSUANCE2_METADATA, + ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, + kISSUANCE2_OUTSTANDING_AMOUNT, + kISSUANCE2_TRANSFER_FEE, + std::nullopt, + kISSUANCE2_MAX_AMOUNT, + kISSUANCE2_LOCKED_AMOUNT, + kISSUANCE2_DOMAIN_ID_HEX + ) + .getSerializer() + .peekData() + }; EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); @@ -718,7 +729,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitLessThanMin) kISSUANCE_OUT2 ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(json::parse(correctOutput), *output.result); @@ -727,47 +738,48 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitLessThanMin) TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitMoreThanMax) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject( {ripple::uint256{kISSUANCE_INDEX1}, ripple::uint256{kISSUANCE_INDEX2}}, kISSUANCE_INDEX1 ); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); - - std::vector bbs; - auto const issuance1 = createMptIssuanceObject( - kACCOUNT, - 1, - std::nullopt, - ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, - kISSUANCE1_OUTSTANDING_AMOUNT, - std::nullopt, - kISSUANCE1_ASSET_SCALE, - kISSUANCE1_MAX_AMOUNT - ); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); - auto const issuance2 = createMptIssuanceObject( - kACCOUNT, - 2, - kISSUANCE2_METADATA, - ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, - kISSUANCE2_OUTSTANDING_AMOUNT, - kISSUANCE2_TRANSFER_FEE, - std::nullopt, - kISSUANCE2_MAX_AMOUNT, - kISSUANCE2_LOCKED_AMOUNT, - kISSUANCE2_DOMAIN_ID_HEX - ); + auto const bbs = std::vector{ + createMptIssuanceObject( + kACCOUNT, + 1, + std::nullopt, + ripple::lsfMPTCanTrade | ripple::lsfMPTRequireAuth | ripple::lsfMPTCanTransfer | ripple::lsfMPTCanEscrow, + kISSUANCE1_OUTSTANDING_AMOUNT, + std::nullopt, + kISSUANCE1_ASSET_SCALE, + kISSUANCE1_MAX_AMOUNT + ) + .getSerializer() + .peekData(), - bbs.push_back(issuance1.getSerializer().peekData()); - bbs.push_back(issuance2.getSerializer().peekData()); + createMptIssuanceObject( + kACCOUNT, + 2, + kISSUANCE2_METADATA, + ripple::lsfMPTLocked | ripple::lsfMPTCanLock | ripple::lsfMPTCanClawback, + kISSUANCE2_OUTSTANDING_AMOUNT, + kISSUANCE2_TRANSFER_FEE, + std::nullopt, + kISSUANCE2_MAX_AMOUNT, + kISSUANCE2_LOCKED_AMOUNT, + kISSUANCE2_DOMAIN_ID_HEX + ) + .getSerializer() + .peekData() + }; EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); @@ -802,7 +814,7 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitMoreThanMax) kISSUANCE_OUT2 ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(json::parse(correctOutput), *output.result); @@ -811,17 +823,16 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, LimitMoreThanMax) TEST_F(RPCAccountMPTokenIssuancesHandlerTest, EmptyResult) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject({}, kISSUANCE_INDEX1); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); runSpawn([this](auto yield) { auto const input = json::parse( @@ -832,9 +843,292 @@ TEST_F(RPCAccountMPTokenIssuancesHandlerTest, EmptyResult) kACCOUNT ) ); - auto handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ((*output.result).as_object().at("mpt_issuances").as_array().size(), 0); }); } + +TEST_F(RPCAccountMPTokenIssuancesHandlerTest, MutableFlags) +{ + uint32_t const mutableFlags1 = ripple::lsmfMPTCanMutateCanLock | ripple::lsmfMPTCanMutateRequireAuth | + ripple::lsmfMPTCanMutateCanEscrow | ripple::lsmfMPTCanMutateCanTrade; + + uint32_t const mutableFlags2 = ripple::lsmfMPTCanMutateCanTransfer | ripple::lsmfMPTCanMutateCanClawback | + ripple::lsmfMPTCanMutateMetadata | ripple::lsmfMPTCanMutateTransferFee; + + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); + + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); + + ripple::STObject const ownerDir = createOwnerDirLedgerObject( + {ripple::uint256{kISSUANCE_INDEX1}, ripple::uint256{kISSUANCE_INDEX2}}, kISSUANCE_INDEX1 + ); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); + + auto const bbs = std::vector{ + createMptIssuanceObject( + kACCOUNT, + 3, + std::nullopt, + ripple::lsfMPTCanTransfer, + kISSUANCE1_OUTSTANDING_AMOUNT, + kISSUANCE1_TRANSFER_FEE, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + mutableFlags1 + ) + .getSerializer() + .peekData(), + + createMptIssuanceObject( + kACCOUNT, + 5, + kISSUANCE2_METADATA, + ripple::lsfMPTCanTransfer, + kISSUANCE2_OUTSTANDING_AMOUNT, + kISSUANCE2_TRANSFER_FEE, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + mutableFlags2 + ) + .getSerializer() + .peekData() + }; + + EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); + + runSpawn([this](auto yield) { + auto const input = json::parse( + fmt::format( + R"JSON({{ + "account": "{}" + }})JSON", + kACCOUNT + ) + ); + + auto const correctOutput = fmt::format( + R"JSON({{ + "account": "{}", + "ledger_hash": "{}", + "ledger_index": 30, + "validated": true, + "limit": 200, + "mpt_issuances": [ + {{ + "mpt_issuance_id": "{}", + "issuer": "{}", + "sequence": 3, + "outstanding_amount": {}, + "transfer_fee": {}, + "mpt_can_transfer": true, + "mpt_can_mutate_can_lock": true, + "mpt_can_mutate_require_auth": true, + "mpt_can_mutate_can_escrow": true, + "mpt_can_mutate_can_trade": true + }}, + {{ + "mpt_issuance_id": "{}", + "issuer": "{}", + "sequence": 5, + "outstanding_amount": {}, + "transfer_fee": {}, + "mptoken_metadata": "{}", + "mpt_can_transfer": true, + "mpt_can_mutate_can_transfer": true, + "mpt_can_mutate_can_clawback": true, + "mpt_can_mutate_metadata": true, + "mpt_can_mutate_transfer_fee": true + }} + ] + }})JSON", + kACCOUNT, + kLEDGER_HASH, + kISSUANCE_INDEX1, + kACCOUNT, + kISSUANCE1_OUTSTANDING_AMOUNT, + kISSUANCE1_TRANSFER_FEE, + kISSUANCE_INDEX2, + kACCOUNT, + kISSUANCE2_OUTSTANDING_AMOUNT, + kISSUANCE2_TRANSFER_FEE, + kISSUANCE2_METADATA_HEX + ); + + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const output = handler.process(input, Context{yield}); + ASSERT_TRUE(output); + EXPECT_EQ(json::parse(correctOutput), *output.result); + }); +} + +struct SingleFlagTest { + std::string testName; + uint32_t flag; + std::string expectedJsonKey; +}; + +struct AccountMPTokenIssuancesImmutableFlagsTest : RPCAccountMPTokenIssuancesHandlerTest, + WithParamInterface {}; + +static auto +generateSingleFlagTests() +{ + return std::vector{ + {"Locked", ripple::lsfMPTLocked, "mpt_locked"}, + {"CanLock", ripple::lsfMPTCanLock, "mpt_can_lock"}, + {"RequireAuth", ripple::lsfMPTRequireAuth, "mpt_require_auth"}, + {"CanEscrow", ripple::lsfMPTCanEscrow, "mpt_can_escrow"}, + {"CanTrade", ripple::lsfMPTCanTrade, "mpt_can_trade"}, + {"CanTransfer", ripple::lsfMPTCanTransfer, "mpt_can_transfer"}, + {"CanClawback", ripple::lsfMPTCanClawback, "mpt_can_clawback"}, + }; +} + +INSTANTIATE_TEST_SUITE_P( + RPCAccountMPTokenIssuancesImmutableFlagsGroup, + AccountMPTokenIssuancesImmutableFlagsTest, + ValuesIn(generateSingleFlagTests()), + tests::util::kNAME_GENERATOR +); + +TEST_P(AccountMPTokenIssuancesImmutableFlagsTest, SingleFlag) +{ + auto const testParams = GetParam(); + + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); + + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); + + ripple::STObject const ownerDir = createOwnerDirLedgerObject({ripple::uint256{kISSUANCE_INDEX1}}, kISSUANCE_INDEX1); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); + + auto const bbs = std::vector{ + createMptIssuanceObject(kACCOUNT, 1, std::nullopt, testParams.flag, 0).getSerializer().peekData() + }; + + EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); + + runSpawn([this, &testParams](auto yield) { + auto const input = json::parse( + fmt::format( + R"JSON({{ + "account": "{}" + }})JSON", + kACCOUNT + ) + ); + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const output = handler.process(input, Context{yield}); + + ASSERT_TRUE(output); + auto const& resultJson = (*output.result).as_object(); + auto const& issuances = resultJson.at("mpt_issuances").as_array(); + ASSERT_EQ(issuances.size(), 1); + + auto const& issuanceJson = issuances[0].as_object(); + EXPECT_TRUE(issuanceJson.contains(testParams.expectedJsonKey)); + EXPECT_EQ(issuanceJson.at(testParams.expectedJsonKey), true); + }); +} + +struct SingleMutableFlagTest { + std::string testName; + uint32_t mutableFlag; + std::string expectedJsonKey; +}; + +struct AccountMPTokenIssuancesMutableFlagsTest : RPCAccountMPTokenIssuancesHandlerTest, + WithParamInterface {}; + +static auto +generateSingleMutableFlagTests() +{ + return std::vector{ + {"CanMutateCanLock", ripple::lsmfMPTCanMutateCanLock, "mpt_can_mutate_can_lock"}, + {"CanMutateRequireAuth", ripple::lsmfMPTCanMutateRequireAuth, "mpt_can_mutate_require_auth"}, + {"CanMutateCanEscrow", ripple::lsmfMPTCanMutateCanEscrow, "mpt_can_mutate_can_escrow"}, + {"CanMutateCanTrade", ripple::lsmfMPTCanMutateCanTrade, "mpt_can_mutate_can_trade"}, + {"CanMutateCanTransfer", ripple::lsmfMPTCanMutateCanTransfer, "mpt_can_mutate_can_transfer"}, + {"CanMutateCanClawback", ripple::lsmfMPTCanMutateCanClawback, "mpt_can_mutate_can_clawback"}, + {"CanMutateMetadata", ripple::lsmfMPTCanMutateMetadata, "mpt_can_mutate_metadata"}, + {"CanMutateTransferFee", ripple::lsmfMPTCanMutateTransferFee, "mpt_can_mutate_transfer_fee"}, + }; +} + +INSTANTIATE_TEST_SUITE_P( + RPCAccountMPTokenIssuancesMutableFlagsGroup, + AccountMPTokenIssuancesMutableFlagsTest, + ValuesIn(generateSingleMutableFlagTests()), + tests::util::kNAME_GENERATOR +); + +TEST_P(AccountMPTokenIssuancesMutableFlagsTest, SingleMutableFlag) +{ + auto const testParams = GetParam(); + + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); + + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); + + ripple::STObject const ownerDir = createOwnerDirLedgerObject({ripple::uint256{kISSUANCE_INDEX1}}, kISSUANCE_INDEX1); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); + + auto const bbs = std::vector{createMptIssuanceObject( + kACCOUNT, + 1, + std::nullopt, + 0, + 0, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + testParams.mutableFlag + ) + .getSerializer() + .peekData()}; + + EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); + + runSpawn([this, &testParams](auto yield) { + auto const input = json::parse( + fmt::format( + R"JSON({{ + "account": "{}" + }})JSON", + kACCOUNT + ) + ); + auto const handler = AnyHandler{AccountMPTokenIssuancesHandler{this->backend_}}; + auto const output = handler.process(input, Context{yield}); + + ASSERT_TRUE(output); + auto const& resultJson = (*output.result).as_object(); + auto const& issuances = resultJson.at("mpt_issuances").as_array(); + ASSERT_EQ(issuances.size(), 1); + + auto const& issuanceJson = issuances[0].as_object(); + EXPECT_TRUE(issuanceJson.contains(testParams.expectedJsonKey)); + EXPECT_EQ(issuanceJson.at(testParams.expectedJsonKey), true); + }); +} diff --git a/tests/unit/rpc/handlers/AccountMPTokensTests.cpp b/tests/unit/rpc/handlers/AccountMPTokensTests.cpp index 6fe5cf7bc9..8a254391ad 100644 --- a/tests/unit/rpc/handlers/AccountMPTokensTests.cpp +++ b/tests/unit/rpc/handlers/AccountMPTokensTests.cpp @@ -64,12 +64,14 @@ constexpr uint64_t kTOKEN2_AMOUNT = 250; // define expected JSON for mptokens auto const kTOKEN_OUT1 = fmt::format( R"JSON({{ + "mpt_id": "{}", "account": "{}", "mpt_issuance_id": "{}", "mpt_amount": {}, "locked_amount": {}, "mpt_locked": true }})JSON", + kTOKEN_INDEX1, kACCOUNT, kISSUANCE_ID_HEX, kTOKEN1_AMOUNT, @@ -78,11 +80,13 @@ auto const kTOKEN_OUT1 = fmt::format( auto const kTOKEN_OUT2 = fmt::format( R"JSON({{ + "mpt_id": "{}", "account": "{}", "mpt_issuance_id": "{}", "mpt_amount": {}, "mpt_authorized": true }})JSON", + kTOKEN_INDEX2, kACCOUNT, kISSUANCE_ID_HEX, kTOKEN2_AMOUNT @@ -262,7 +266,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, NonExistLedgerViaLedgerIntIndex) TEST_F(RPCAccountMPTokensHandlerTest, LedgerSeqOutOfRangeByHash) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 31); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 31); EXPECT_CALL(*backend_, fetchLedgerByHash(ripple::uint256{kLEDGER_HASH}, _)).WillOnce(Return(ledgerHeader)); auto const input = json::parse( @@ -311,7 +315,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, LedgerSeqOutOfRangeByIndex) TEST_F(RPCAccountMPTokensHandlerTest, NonExistAccount) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerByHash(ripple::uint256{kLEDGER_HASH}, _)).WillOnce(Return(ledgerHeader)); // fetch account object return empty EXPECT_CALL(*backend_, doFetchLedgerObject).WillOnce(Return(std::optional{})); @@ -339,29 +343,31 @@ TEST_F(RPCAccountMPTokensHandlerTest, NonExistAccount) TEST_F(RPCAccountMPTokensHandlerTest, DefaultParameters) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); ON_CALL(*backend_, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject({ripple::uint256{kTOKEN_INDEX1}, ripple::uint256{kTOKEN_INDEX2}}, kTOKEN_INDEX1); ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - std::vector bbs; - auto const token1 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT - ); - - auto const token2 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt - ); + auto const bbs = std::vector{ + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT + ) + .getSerializer() + .peekData(), - bbs.push_back(token1.getSerializer().peekData()); - bbs.push_back(token2.getSerializer().peekData()); + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt + ) + .getSerializer() + .peekData() + }; EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); @@ -385,7 +391,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, DefaultParameters) kTOKEN_OUT2 ); auto const input = json::parse(fmt::format(R"JSON({{"account": "{}"}})JSON", kACCOUNT)); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(json::parse(expected), *output.result); @@ -395,22 +401,25 @@ TEST_F(RPCAccountMPTokensHandlerTest, DefaultParameters) TEST_F(RPCAccountMPTokensHandlerTest, UseLimit) { constexpr int kLIMIT = 20; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); ON_CALL(*backend_, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); - std::vector indexes; - std::vector bbs; - - for (int i = 0; i < 50; ++i) { - indexes.emplace_back(kTOKEN_INDEX1); - auto const token = createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt); - bbs.push_back(token.getSerializer().peekData()); - } + auto const indexes = std::vector(50, ripple::uint256{kTOKEN_INDEX1}); + auto const bbs = [&]() { + std::vector v; + v.reserve(50); + for (int i = 0; i < 50; ++i) { + v.push_back(createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt) + .getSerializer() + .peekData()); + } + return v; + }(); ripple::STObject ownerDir = createOwnerDirLedgerObject(indexes, kTOKEN_INDEX1); ownerDir.setFieldU64(ripple::sfIndexNext, 99); @@ -432,7 +441,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, UseLimit) ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); @@ -454,7 +463,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, UseLimit) ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ((*output.result).as_object().at("limit").as_uint64(), AccountMPTokensHandler::kLIMIT_MIN); @@ -472,7 +481,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, UseLimit) ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ((*output.result).as_object().at("limit").as_uint64(), AccountMPTokensHandler::kLIMIT_MAX); @@ -483,22 +492,25 @@ TEST_F(RPCAccountMPTokensHandlerTest, MarkerOutput) { constexpr auto kNEXT_PAGE = 99; constexpr auto kLIMIT = 15; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto ownerDirKk = ripple::keylet::ownerDir(account).key; - auto ownerDir2Kk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const ownerDirKk = ripple::keylet::ownerDir(account).key; + auto const ownerDir2Kk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); - std::vector bbs; - bbs.reserve(kLIMIT); - for (int i = 0; i < kLIMIT; ++i) { - bbs.push_back(createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt) - .getSerializer() - .peekData()); - } + auto const bbs = [&]() { + std::vector v; + v.reserve(kLIMIT); + for (int i = 0; i < kLIMIT; ++i) { + v.push_back(createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt) + .getSerializer() + .peekData()); + } + return v; + }(); EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); std::vector indexes1; @@ -528,7 +540,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, MarkerOutput) kLIMIT ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); auto const& resultJson = (*output.result).as_object(); @@ -544,22 +556,25 @@ TEST_F(RPCAccountMPTokensHandlerTest, MarkerInput) constexpr auto kNEXT_PAGE = 99; constexpr auto kLIMIT = 15; - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); - auto ownerDirKk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; - - std::vector bbs; - std::vector indexes; - for (int i = 0; i < kLIMIT; ++i) { - indexes.emplace_back(kTOKEN_INDEX1); - bbs.push_back(createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt) - .getSerializer() - .peekData()); - } + auto const ownerDirKk = ripple::keylet::page(ripple::keylet::ownerDir(account), kNEXT_PAGE).key; + + auto const indexes = std::vector(kLIMIT, ripple::uint256{kTOKEN_INDEX1}); + auto const bbs = [&]() { + std::vector v; + v.reserve(kLIMIT); + for (int i = 0; i < kLIMIT; ++i) { + v.push_back(createMpTokenObject(kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), i, 0, std::nullopt) + .getSerializer() + .peekData()); + } + return v; + }(); ripple::STObject ownerDir = createOwnerDirLedgerObject(indexes, kTOKEN_INDEX1); ownerDir.setFieldU64(ripple::sfIndexNext, 0); @@ -583,7 +598,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, MarkerInput) kNEXT_PAGE ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); auto const& resultJson = (*output.result).as_object(); @@ -594,29 +609,31 @@ TEST_F(RPCAccountMPTokensHandlerTest, MarkerInput) TEST_F(RPCAccountMPTokensHandlerTest, LimitLessThanMin) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject({ripple::uint256{kTOKEN_INDEX1}, ripple::uint256{kTOKEN_INDEX2}}, kTOKEN_INDEX1); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); - std::vector bbs; - auto const token1 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT - ); - bbs.push_back(token1.getSerializer().peekData()); + auto const bbs = std::vector{ + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT + ) + .getSerializer() + .peekData(), - auto const token2 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt - ); - bbs.push_back(token2.getSerializer().peekData()); + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt + ) + .getSerializer() + .peekData() + }; EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); @@ -651,7 +668,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, LimitLessThanMin) kTOKEN_OUT2 ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(json::parse(correctOutput), *output.result); @@ -660,29 +677,31 @@ TEST_F(RPCAccountMPTokensHandlerTest, LimitLessThanMin) TEST_F(RPCAccountMPTokensHandlerTest, LimitMoreThanMax) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject({ripple::uint256{kTOKEN_INDEX1}, ripple::uint256{kTOKEN_INDEX2}}, kTOKEN_INDEX1); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); - std::vector bbs; - auto const token1 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT - ); - bbs.push_back(token1.getSerializer().peekData()); + auto const bbs = std::vector{ + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN1_AMOUNT, ripple::lsfMPTLocked, kTOKEN1_LOCKED_AMOUNT + ) + .getSerializer() + .peekData(), - auto const token2 = createMpTokenObject( - kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt - ); - bbs.push_back(token2.getSerializer().peekData()); + createMpTokenObject( + kACCOUNT, ripple::uint192(kISSUANCE_ID_HEX), kTOKEN2_AMOUNT, ripple::lsfMPTAuthorized, std::nullopt + ) + .getSerializer() + .peekData() + }; EXPECT_CALL(*backend_, doFetchLedgerObjects).WillOnce(Return(bbs)); @@ -717,7 +736,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, LimitMoreThanMax) kTOKEN_OUT2 ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ(json::parse(correctOutput), *output.result); @@ -726,17 +745,16 @@ TEST_F(RPCAccountMPTokensHandlerTest, LimitMoreThanMax) TEST_F(RPCAccountMPTokensHandlerTest, EmptyResult) { - auto ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); + auto const ledgerHeader = createLedgerHeader(kLEDGER_HASH, 30); EXPECT_CALL(*backend_, fetchLedgerBySequence).WillOnce(Return(ledgerHeader)); - auto account = getAccountIdWithString(kACCOUNT); - auto accountKk = ripple::keylet::account(account).key; - auto owneDirKk = ripple::keylet::ownerDir(account).key; - ON_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'})); + auto const account = getAccountIdWithString(kACCOUNT); + auto const accountKk = ripple::keylet::account(account).key; + auto const owneDirKk = ripple::keylet::ownerDir(account).key; + EXPECT_CALL(*backend_, doFetchLedgerObject(accountKk, _, _)).WillOnce(Return(Blob{'f', 'a', 'k', 'e'})); ripple::STObject const ownerDir = createOwnerDirLedgerObject({}, kTOKEN_INDEX1); - ON_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillByDefault(Return(ownerDir.getSerializer().peekData())); - EXPECT_CALL(*backend_, doFetchLedgerObject).Times(2); + EXPECT_CALL(*backend_, doFetchLedgerObject(owneDirKk, _, _)).WillOnce(Return(ownerDir.getSerializer().peekData())); runSpawn([this](auto yield) { auto const input = json::parse( @@ -747,7 +765,7 @@ TEST_F(RPCAccountMPTokensHandlerTest, EmptyResult) kACCOUNT ) ); - auto handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; + auto const handler = AnyHandler{AccountMPTokensHandler{this->backend_}}; auto const output = handler.process(input, Context{yield}); ASSERT_TRUE(output); EXPECT_EQ((*output.result).as_object().at("mptokens").as_array().size(), 0); diff --git a/tests/unit/rpc/handlers/VaultInfoTests.cpp b/tests/unit/rpc/handlers/VaultInfoTests.cpp index 17bd794d1d..a7aa80f336 100644 --- a/tests/unit/rpc/handlers/VaultInfoTests.cpp +++ b/tests/unit/rpc/handlers/VaultInfoTests.cpp @@ -295,7 +295,7 @@ TEST_F(RPCVaultInfoHandlerTest, ValidVaultObjectQueryByVaultID) "AssetsTotal": "300", "Flags": 0, "LedgerEntryType": "Vault", - "LossUnrealized": "0", + "LossUnrealized": "1", "Owner": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "OwnerNode": "4", "PreviousTxnID": "0000000000000000000000000000000000000000000000000000000000000002", @@ -378,7 +378,7 @@ TEST_F(RPCVaultInfoHandlerTest, ValidVaultObjectQueryByOwnerAndSeq) "AssetsTotal": "300", "Flags": 0, "LedgerEntryType": "Vault", - "LossUnrealized": "0", + "LossUnrealized": "1", "Owner": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "OwnerNode": "4", "PreviousTxnID": "0000000000000000000000000000000000000000000000000000000000000002", diff --git a/tests/unit/util/ObservableValueAtomicTest.cpp b/tests/unit/util/ObservableValueAtomicTest.cpp new file mode 100644 index 0000000000..aa71518f8a --- /dev/null +++ b/tests/unit/util/ObservableValueAtomicTest.cpp @@ -0,0 +1,445 @@ +//------------------------------------------------------------------------------ +/* + 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/ObservableValue.hpp" + +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace testing; +using namespace util; + +namespace { + +} // namespace + +class ObservableValueAtomicTest : public ::testing::Test {}; + +TEST_F(ObservableValueAtomicTest, BasicConstruction) +{ + ObservableValue> const obs{42}; + + EXPECT_EQ(obs.get(), 42); + EXPECT_EQ(static_cast(obs), 42); + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueAtomicTest, DefaultConstruction) +{ + ObservableValue> const obsInt; + EXPECT_EQ(obsInt.get(), 0); + + ObservableValue> const obsBool; + EXPECT_FALSE(obsBool.get()); + + EXPECT_FALSE(obsInt.hasObservers()); + EXPECT_FALSE(obsBool.hasObservers()); +} + +TEST_F(ObservableValueAtomicTest, BasicObservation) +{ + ObservableValue> obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(20)); + obs = 20; + EXPECT_EQ(obs.get(), 20); +} + +TEST_F(ObservableValueAtomicTest, SetMethod) +{ + ObservableValue> obs{5}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(15)); + obs.set(15); + EXPECT_EQ(obs.get(), 15); + + obs.set(15); // Same value should not notify + EXPECT_EQ(obs.get(), 15); +} + +TEST_F(ObservableValueAtomicTest, AtomicBasicUsage) +{ + ObservableValue> obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(25)); + obs.set(25); + + EXPECT_EQ(obs.get(), 25); +} + +TEST_F(ObservableValueAtomicTest, AtomicMultipleChanges) +{ + ObservableValue> obs{50}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(100)); // First change: 50→100 + EXPECT_CALL(mockObserver, Call(50)); // Second change: 100→50 + obs.set(100); // Should notify: 50→100 + obs.set(50); // Should notify: 100→50 + + EXPECT_EQ(obs.get(), 50); +} + +TEST_F(ObservableValueAtomicTest, AtomicNoChangeNoNotification) +{ + ObservableValue> obs{42}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + // No EXPECT_CALL since no notification should occur + obs.set(42); // Same value, should not notify + obs.set(42); // Same value again, should not notify + + EXPECT_EQ(obs.get(), 42); +} + +TEST_F(ObservableValueAtomicTest, AtomicSequentialChanges) +{ + ObservableValue> obs{1}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(2)); + obs.set(2); + + EXPECT_CALL(mockObserver, Call(3)); + obs.set(3); + + EXPECT_EQ(obs.get(), 3); +} + +TEST_F(ObservableValueAtomicTest, MultipleObservers) +{ + ObservableValue> obs{0}; + + testing::StrictMock> mockObserver1; + testing::StrictMock> mockObserver2; + + auto conn1 = obs.observe(mockObserver1.AsStdFunction()); + auto conn2 = obs.observe(mockObserver2.AsStdFunction()); + + EXPECT_CALL(mockObserver1, Call(42)); + EXPECT_CALL(mockObserver2, Call(42)); + obs = 42; + + conn1.disconnect(); + EXPECT_CALL(mockObserver2, Call(100)); + obs = 100; +} + +TEST_F(ObservableValueAtomicTest, ThreadSafetyBasic) +{ + ObservableValue> obs{0}; + std::atomic notificationCount{0}; + std::vector values; + std::mutex valuesMutex; + + auto connection = obs.observe([&](int const& value) { + notificationCount.fetch_add(1); + std::lock_guard const lock(valuesMutex); + values.push_back(value); + }); + + static constexpr auto kNUM_THREADS = 4; + static constexpr auto kINCREMENTS_PER_THREAD = 100; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs]() { + for (int j = 0; j < kINCREMENTS_PER_THREAD; ++j) { + int const expected = obs.get(); + int const newValue = expected + 1; + obs.set(newValue); + std::this_thread::sleep_for(std::chrono::microseconds(1)); + } + }); + } + + for (auto& thread : threads) + thread.join(); + + // Final value may be less than kNumThreads * kIncrementsPerThread due to race conditions + EXPECT_GT(obs.get(), 0); + EXPECT_GT(notificationCount.load(), 0); + + std::lock_guard const lock(valuesMutex); + for (auto const& value : values) { + EXPECT_GT(value, 0); + } +} + +TEST_F(ObservableValueAtomicTest, ThreadSafetyWithDirectAccess) +{ + ObservableValue> obs{0}; + std::atomic notificationCount{0}; + + auto connection = obs.observe([&](int const&) { notificationCount.fetch_add(1); }); + + static constexpr auto kNUM_THREADS = 4; + static constexpr auto kOPERATIONS_PER_THREAD = 50; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs]() { + for (int j = 0; j < kOPERATIONS_PER_THREAD; ++j) { + int const current = obs.get(); + obs.set(current + 1); + std::this_thread::sleep_for(std::chrono::microseconds(1)); + } + }); + } + + for (auto& thread : threads) + thread.join(); + + EXPECT_GT(obs.get(), 0); + EXPECT_GT(notificationCount.load(), 0); +} + +TEST_F(ObservableValueAtomicTest, AtomicBoolSpecialization) +{ + ObservableValue> obs{false}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(true)); + obs = true; + EXPECT_TRUE(obs.get()); + + obs = true; // Same value should not notify + + EXPECT_CALL(mockObserver, Call(false)); + obs.set(false); + EXPECT_FALSE(obs.get()); +} + +TEST_F(ObservableValueAtomicTest, CompareAndSwapBehavior) +{ + ObservableValue> obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + // Test that compare-and-swap works correctly in set() + obs.set(10); // Same value, should not notify + + EXPECT_CALL(mockObserver, Call(20)); + obs.set(20); // Different value, should notify +} + +TEST_F(ObservableValueAtomicTest, RaceConditionNotificationIntegrity) +{ + ObservableValue> obs{0}; + std::atomic notificationCount{0}; + std::vector values; + std::mutex valuesMutex; + + auto connection = obs.observe([&](int const& value) { + notificationCount.fetch_add(1); + std::lock_guard const lock(valuesMutex); + values.push_back(value); + }); + + static constexpr auto kNUM_THREADS = 10; + static constexpr auto kOPERATIONS_PER_THREAD = 20; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs]() { + for (int j = 0; j < kOPERATIONS_PER_THREAD; ++j) { + obs.set(j % 3); + std::this_thread::sleep_for(std::chrono::microseconds(1)); + } + }); + } + + for (auto& thread : threads) + thread.join(); + + EXPECT_GT(notificationCount.load(), 0); + + std::lock_guard const lock(valuesMutex); + for (auto const& value : values) { + EXPECT_GE(value, 0); + EXPECT_LE(value, 2); + } + + int const finalValue = obs.get(); + EXPECT_GE(finalValue, 0); + EXPECT_LE(finalValue, 2); +} + +TEST_F(ObservableValueAtomicTest, DeterministicNotificationTest) +{ + ObservableValue> obs{0}; + std::atomic notificationCount{0}; + std::vector values; + std::mutex valuesMutex; + + auto connection = obs.observe([&](int const& value) { + notificationCount.fetch_add(1); + std::lock_guard const lock(valuesMutex); + values.push_back(value); + }); + + static constexpr auto kNUM_THREADS = 5; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs, i]() { obs.set(i + 1); }); + } + + for (auto& thread : threads) + thread.join(); + + // Each thread sets a unique value, so expect exactly kNumThreads notifications + EXPECT_EQ(notificationCount.load(), kNUM_THREADS); + + std::lock_guard const lock(valuesMutex); + EXPECT_EQ(values.size(), kNUM_THREADS); + + for (auto const& value : values) { + EXPECT_GE(value, 1); + EXPECT_LE(value, kNUM_THREADS); + } + + int const finalValue = obs.get(); + EXPECT_GE(finalValue, 1); + EXPECT_LE(finalValue, kNUM_THREADS); +} + +TEST_F(ObservableValueAtomicTest, NoNotificationForSameValue) +{ + ObservableValue> obs{42}; + std::atomic notificationCount{0}; + + auto connection = obs.observe([&](int const&) { notificationCount.fetch_add(1); }); + + static constexpr auto kNUM_THREADS = 10; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs]() { obs.set(42); }); + } + + for (auto& thread : threads) + thread.join(); + + EXPECT_EQ(notificationCount.load(), 0); // No notifications since value never changed + EXPECT_EQ(obs.get(), 42); +} + +TEST_F(ObservableValueAtomicTest, AtomicRaceConditionCorrectness) +{ + ObservableValue> obs{0}; + std::atomic notificationCount{0}; + std::vector values; + std::mutex valuesMutex; + + auto connection = obs.observe([&](int const& value) { + notificationCount.fetch_add(1); + std::lock_guard const lock(valuesMutex); + values.push_back(value); + }); + + static constexpr auto kNUM_THREADS = 3; + + std::vector threads; + threads.reserve(kNUM_THREADS); + + // Test that direct access properly notifies for all value changes + // Each thread will make unique changes to avoid race condition conflicts + for (int i = 0; i < kNUM_THREADS; ++i) { + threads.emplace_back([&obs, i]() { + int const baseValue = (i + 1) * 10; // 10, 20, 30 + obs.set(baseValue); // Store unique values + obs.set(baseValue + 1); // Then increment + }); + } + + for (auto& thread : threads) + thread.join(); + + // We should get some notifications (exact count depends on race conditions) + // but at least one per thread since they use unique base values + EXPECT_GE(notificationCount.load(), kNUM_THREADS); + + std::lock_guard const lock(valuesMutex); + EXPECT_GE(values.size(), kNUM_THREADS); + + for (auto const& value : values) + EXPECT_GT(value, 0); +} + +TEST_F(ObservableValueAtomicTest, ForceNotify) +{ + ObservableValue> obs{42}; + testing::StrictMock> mockObserver; + + obs.forceNotify(); + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(42)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(42)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(100)); + obs.set(100); + EXPECT_CALL(mockObserver, Call(100)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(100)).Times(3); + obs.forceNotify(); + obs.forceNotify(); + obs.forceNotify(); +} diff --git a/tests/unit/util/ObservableValueTest.cpp b/tests/unit/util/ObservableValueTest.cpp new file mode 100644 index 0000000000..40a0f16944 --- /dev/null +++ b/tests/unit/util/ObservableValueTest.cpp @@ -0,0 +1,840 @@ +//------------------------------------------------------------------------------ +/* + 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/ObservableValue.hpp" + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace testing; +using namespace util; + +namespace { + +struct TestStruct { + int value = 0; + std::string name; + + bool + operator==(TestStruct const& other) const + { + return value == other.value && name == other.name; + } + + bool + operator!=(TestStruct const& other) const + { + return !(*this == other); + } +}; + +} // namespace + +class ObservableValueTest : public ::testing::Test {}; + +TEST_F(ObservableValueTest, ConceptCompliance) +{ + static_assert(Observable); + static_assert(Observable); + static_assert(Observable); + static_assert(Observable); + static_assert(Observable); + static_assert(Observable); + static_assert(Observable); + + struct NonCopyable { + int value = 0; + NonCopyable() = default; + NonCopyable(NonCopyable const&) = delete; + NonCopyable(NonCopyable&&) = default; + NonCopyable& + operator=(NonCopyable const&) = delete; + NonCopyable& + operator=(NonCopyable&&) = default; + bool + operator==(NonCopyable const& other) const + { + return value == other.value; + } + }; + static_assert(!Observable); + + struct NonMovable { + int value = 0; + NonMovable() = default; + NonMovable(NonMovable const&) = default; + NonMovable(NonMovable&&) = delete; + NonMovable& + operator=(NonMovable const&) = default; + NonMovable& + operator=(NonMovable&&) = delete; + bool + operator==(NonMovable const& other) const + { + return value == other.value; + } + }; + static_assert(!Observable); + + struct NonComparable { + int value = 0; + NonComparable() = default; + NonComparable(NonComparable const&) = default; + NonComparable(NonComparable&&) = default; + NonComparable& + operator=(NonComparable const&) = default; + NonComparable& + operator=(NonComparable&&) = default; + }; + static_assert(!Observable); + + struct NonDefaultInitializable { + int value; + NonDefaultInitializable() = delete; + explicit NonDefaultInitializable(int v) : value(v) + { + } + NonDefaultInitializable(NonDefaultInitializable const&) = default; + NonDefaultInitializable(NonDefaultInitializable&&) = default; + NonDefaultInitializable& + operator=(NonDefaultInitializable const&) = default; + NonDefaultInitializable& + operator=(NonDefaultInitializable&&) = default; + bool + operator==(NonDefaultInitializable const& other) const + { + return value == other.value; + } + }; + + static_assert(Observable); + static_assert(!std::default_initializable); + + static_assert(Observable>); + static_assert(Observable>); + static_assert(Observable>); + static_assert(Observable>); + + static_assert(std::default_initializable); + static_assert(std::default_initializable); + static_assert(std::default_initializable>); + static_assert(std::default_initializable); +} + +TEST_F(ObservableValueTest, Construction) +{ + ObservableValue const obs{42}; + + EXPECT_EQ(static_cast(obs), 42); + EXPECT_EQ(obs.get(), 42); + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ConstructionWithDifferentTypes) +{ + ObservableValue const obsStr{"hello"}; + EXPECT_EQ(obsStr.get(), "hello"); + + ObservableValue const obsDouble{3.14}; + EXPECT_DOUBLE_EQ(obsDouble.get(), 3.14); + + ObservableValue const obsBool{true}; + EXPECT_TRUE(obsBool.get()); +} + +TEST_F(ObservableValueTest, DefaultConstruction) +{ + ObservableValue const obsInt; + EXPECT_EQ(obsInt.get(), 0); + + ObservableValue const obsDouble; + EXPECT_DOUBLE_EQ(obsDouble.get(), 0.0); + + ObservableValue const obsBool; + EXPECT_FALSE(obsBool.get()); + + ObservableValue const obsChar; + EXPECT_EQ(obsChar.get(), '\0'); + + EXPECT_FALSE(obsInt.hasObservers()); + EXPECT_FALSE(obsDouble.hasObservers()); + EXPECT_FALSE(obsBool.hasObservers()); + EXPECT_FALSE(obsChar.hasObservers()); +} + +TEST_F(ObservableValueTest, DefaultConstructionWithContainers) +{ + ObservableValue const obsString; + EXPECT_EQ(obsString.get(), ""); + EXPECT_TRUE(obsString.get().empty()); + + ObservableValue> const obsVector; + EXPECT_TRUE(obsVector.get().empty()); + EXPECT_EQ(obsVector.get().size(), 0); + + ObservableValue> const obsSet; + EXPECT_TRUE(obsSet.get().empty()); + EXPECT_EQ(obsSet.get().size(), 0); + + ObservableValue> const obsMap; + EXPECT_TRUE(obsMap.get().empty()); + EXPECT_EQ(obsMap.get().size(), 0); +} + +TEST_F(ObservableValueTest, DefaultConstructionWithCustomType) +{ + ObservableValue const obsStruct; + EXPECT_EQ(obsStruct.get().value, 0); + EXPECT_EQ(obsStruct.get().name, ""); +} + +TEST_F(ObservableValueTest, DefaultConstructionThenAssignment) +{ + ObservableValue obs; + EXPECT_EQ(obs.get(), 0); + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(42)); + obs = 42; + EXPECT_EQ(obs.get(), 42); + + obs = 42; // Same value, should not notify + + EXPECT_CALL(mockObserver, Call(100)); + obs.set(100); + EXPECT_EQ(obs.get(), 100); +} + +TEST_F(ObservableValueTest, DefaultConstructionWithGuard) +{ + ObservableValue obs; + EXPECT_EQ(obs.get(), ""); + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call("modified through guard")); + { + auto guard = obs.operator->(); + std::string& ref = guard; + ref = "modified through guard"; + } + + EXPECT_EQ(obs.get(), "modified through guard"); +} + +TEST_F(ObservableValueTest, DefaultConstructionNotificationBehavior) +{ + ObservableValue obs; + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(1)); + obs = 1; + + EXPECT_CALL(mockObserver, Call(0)); + obs = 0; + + obs = 0; // Same value, should not notify +} + +TEST_F(ObservableValueTest, NonDefaultInitializableTypeWithParameterizedConstructor) +{ + struct NonDefaultInitializable { + int value; + NonDefaultInitializable() = delete; + explicit NonDefaultInitializable(int v) : value(v) + { + } + NonDefaultInitializable(NonDefaultInitializable const&) = default; + NonDefaultInitializable(NonDefaultInitializable&&) = default; + NonDefaultInitializable& + operator=(NonDefaultInitializable const&) = default; + NonDefaultInitializable& + operator=(NonDefaultInitializable&&) = default; + bool + operator==(NonDefaultInitializable const& other) const + { + return value == other.value; + } + }; + + ObservableValue obs{NonDefaultInitializable{42}}; + EXPECT_EQ(obs.get().value, 42); + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(testing::Field(&NonDefaultInitializable::value, 100))); + obs = NonDefaultInitializable{100}; + EXPECT_EQ(obs.get().value, 100); +} + +TEST_F(ObservableValueTest, MoveSemantics) +{ + ObservableValue const obs1{100}; + + ObservableValue const obs2 = std::move(obs1); + EXPECT_EQ(obs2.get(), 100); + + ObservableValue obs3{200}; + obs3 = std::move(obs2); + EXPECT_EQ(obs3.get(), 100); +} + +TEST_F(ObservableValueTest, CopyOperationsDeleted) +{ + static_assert(!std::is_copy_constructible_v>); + static_assert(!std::is_copy_assignable_v>); +} + +TEST_F(ObservableValueTest, AssignmentOperator) +{ + ObservableValue obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(20)); + obs = 20; + EXPECT_EQ(obs.get(), 20); + + obs = 20; // Same value, should not notify + EXPECT_EQ(obs.get(), 20); +} + +TEST_F(ObservableValueTest, SetMethod) +{ + ObservableValue obs{5}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(15)); + obs.set(15); + EXPECT_EQ(obs.get(), 15); + + obs.set(15); // Same value, should not notify + EXPECT_EQ(obs.get(), 15); +} + +TEST_F(ObservableValueTest, ObserverManagement) +{ + ObservableValue obs{0}; + + EXPECT_FALSE(obs.hasObservers()); + + testing::StrictMock> mockObserver1; + testing::StrictMock> mockObserver2; + + auto conn1 = obs.observe(mockObserver1.AsStdFunction()); + EXPECT_TRUE(obs.hasObservers()); + + auto conn2 = obs.observe(mockObserver2.AsStdFunction()); + EXPECT_TRUE(obs.hasObservers()); + + EXPECT_CALL(mockObserver1, Call(42)); + EXPECT_CALL(mockObserver2, Call(42)); + obs = 42; + + conn1.disconnect(); + EXPECT_CALL(mockObserver2, Call(100)); + obs = 100; + + conn2.disconnect(); + EXPECT_FALSE(obs.hasObservers()); + + obs = 200; // No observers, no calls expected +} + +TEST_F(ObservableValueTest, ObservableGuardBasicUsage) +{ + ObservableValue obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(25)); + { + auto guard = obs.operator->(); + int& ref = guard; + ref = 25; + } + + EXPECT_EQ(obs.get(), 25); +} + +TEST_F(ObservableValueTest, ObservableGuardNoChangeNoNotification) +{ + ObservableValue obs{50}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + // No EXPECT_CALL since no notification should occur + { + auto guard = obs.operator->(); + int& ref = guard; + ref = 100; + ref = 50; // Back to original value + } + + EXPECT_EQ(obs.get(), 50); +} + +TEST_F(ObservableValueTest, ObservableGuardMultipleChanges) +{ + ObservableValue obs{1}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(2)); + { + auto guard = obs.operator->(); + int& ref = guard; + ref = 2; + } + + EXPECT_CALL(mockObserver, Call(3)); + { + auto guard = obs.operator->(); + int& ref = guard; + ref = 3; + } + + EXPECT_EQ(obs.get(), 3); +} + +TEST_F(ObservableValueTest, ComplexTypeObservation) +{ + TestStruct const initial{.value = 42, .name = "test"}; + ObservableValue obs{initial}; + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + TestStruct const newValue{.value = 100, .name = "changed"}; + EXPECT_CALL( + mockObserver, + Call(testing::AllOf(testing::Field(&TestStruct::value, 100), testing::Field(&TestStruct::name, "changed"))) + ); + obs = newValue; +} + +TEST_F(ObservableValueTest, ComplexTypeGuardModification) +{ + TestStruct const initial{.value = 10, .name = "initial"}; + ObservableValue obs{initial}; + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL( + mockObserver, + Call(testing::AllOf(testing::Field(&TestStruct::value, 20), testing::Field(&TestStruct::name, "modified"))) + ); + { + auto guard = obs.operator->(); + TestStruct& ref = guard; + ref.value = 20; + ref.name = "modified"; + } + + EXPECT_EQ(obs.get().value, 20); + EXPECT_EQ(obs.get().name, "modified"); +} + +TEST_F(ObservableValueTest, StringObservation) +{ + ObservableValue obs{"initial"}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call("changed")); + obs = "changed"; + + EXPECT_CALL(mockObserver, Call("set_method")); + obs.set("set_method"); + + obs = "set_method"; // Same value, should not notify +} + +TEST_F(ObservableValueTest, MultipleObserversWithDifferentLifetimes) +{ + ObservableValue obs{0}; + + testing::StrictMock> mockObserver1; + testing::StrictMock> mockObserver2; + testing::StrictMock> mockObserver3; + + auto conn1 = obs.observe(mockObserver1.AsStdFunction()); + + EXPECT_CALL(mockObserver1, Call(1)); + obs = 1; + + auto conn2 = obs.observe(mockObserver2.AsStdFunction()); + EXPECT_CALL(mockObserver1, Call(2)); + EXPECT_CALL(mockObserver2, Call(2)); + obs = 2; + + conn1.disconnect(); + auto conn3 = obs.observe(mockObserver3.AsStdFunction()); + EXPECT_CALL(mockObserver2, Call(3)); + EXPECT_CALL(mockObserver3, Call(3)); + obs = 3; +} + +TEST_F(ObservableValueTest, NoNotificationWhenNoObservers) +{ + ObservableValue obs{0}; + + obs = 1; + obs.set(2); + + { + auto guard = obs.operator->(); + int& ref = guard; + ref = 3; + } + + EXPECT_EQ(obs.get(), 3); + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ManyObservers) +{ + ObservableValue obs{0}; + + std::vector>>> mockObservers; + std::vector connections; + + constexpr int kNUM_OBSERVERS = 100; + for (int i = 0; i < kNUM_OBSERVERS; ++i) { + mockObservers.push_back(std::make_unique>>()); + connections.push_back(obs.observe(mockObservers.back()->AsStdFunction())); + } + + EXPECT_TRUE(obs.hasObservers()); + + for (auto const& mockObserver : mockObservers) { + EXPECT_CALL(*mockObserver, Call(42)); + } + obs = 42; + + for (auto& conn : connections) { + conn.disconnect(); + } + + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, TypeConversions) +{ + ObservableValue obs{1.0}; + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(testing::DoubleEq(2.0))); + obs = 2; + + EXPECT_CALL(mockObserver, Call(testing::DoubleEq(3.14))); + obs = 3.14; + + EXPECT_CALL(mockObserver, Call(testing::DoubleEq(4.0))); + obs = static_cast(4.0f); +} + +TEST_F(ObservableValueTest, EnhancedConceptRequirements) +{ + struct ComplexObservable { + std::string name; + int value{}; + std::vector data; + + ComplexObservable() = default; + ComplexObservable(std::string n, int v, std::vector d) : name(std::move(n)), value(v), data(std::move(d)) + { + } + ComplexObservable(ComplexObservable const& other) = default; + ComplexObservable(ComplexObservable&& other) noexcept = default; + + ComplexObservable& + operator=(ComplexObservable&& other) noexcept + { + if (this != &other) { + name = std::move(other.name); + value = other.value; + data = std::move(other.data); + } + return *this; + } + + bool + operator==(ComplexObservable const& other) const + { + return name == other.name && value == other.value && data == other.data; + } + + ComplexObservable& + operator=(ComplexObservable const& other) + { + if (this != &other) { + name = other.name; + value = other.value; + data = other.data; + } + return *this; + } + }; + + static_assert(Observable); + + ComplexObservable initial{"test", 42, {1, 2, 3}}; + ObservableValue obs{std::move(initial)}; + + testing::StrictMock> mockObserver; + auto connection = obs.observe(mockObserver.AsStdFunction()); + + ComplexObservable const newValue{"changed", 100, {4, 5, 6}}; + EXPECT_CALL( + mockObserver, + Call( + testing::AllOf( + testing::Field(&ComplexObservable::name, "changed"), + testing::Field(&ComplexObservable::value, 100), + testing::Field(&ComplexObservable::data, std::vector({4, 5, 6})) + ) + ) + ); + obs = newValue; + + ComplexObservable const sameValue{"changed", 100, {4, 5, 6}}; + obs = sameValue; // Same value, should not notify +} + +TEST_F(ObservableValueTest, ExceptionInObserver) +{ + ObservableValue obs{0}; + + testing::StrictMock> goodMockObserver; + auto goodConnection = obs.observe(goodMockObserver.AsStdFunction()); + + auto throwingConnection = obs.observe([](int const&) { throw std::runtime_error("Observer exception"); }); + + EXPECT_CALL(goodMockObserver, Call(42)); + EXPECT_THROW(obs = 42, std::runtime_error); + + // Value is still updated even when observers throw + EXPECT_EQ(obs.get(), 42); +} + +TEST_F(ObservableValueTest, GuardExceptionSafety) +{ + ObservableValue obs{10}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(20)); + try { + auto guard = obs.operator->(); + int& ref = guard; + ref = 20; + throw std::runtime_error("Test exception"); + } catch (...) { + [[maybe_unused]] auto nothing = true; + } + + EXPECT_EQ(obs.get(), 20); +} + +TEST_F(ObservableValueTest, ComprehensiveIntegrationTest) +{ + ObservableValue obs{"start"}; + + testing::StrictMock> mockObserver1; + testing::StrictMock> mockObserver2; + auto conn1 = obs.observe(mockObserver1.AsStdFunction()); + auto conn2 = obs.observe(mockObserver2.AsStdFunction()); + + EXPECT_CALL(mockObserver1, Call("first")); + EXPECT_CALL(mockObserver2, Call("first")); + obs = "first"; + + EXPECT_CALL(mockObserver1, Call("second")); + EXPECT_CALL(mockObserver2, Call("second")); + obs.set("second"); + + obs = "second"; // Same value, should not notify + + EXPECT_CALL(mockObserver1, Call("third")); + EXPECT_CALL(mockObserver2, Call("third")); + { + auto guard = obs.operator->(); + std::string& ref = guard; + ref = "third"; + } + + conn1.disconnect(); + EXPECT_CALL(mockObserver2, Call("fourth")); + obs = "fourth"; + + EXPECT_EQ(obs.get(), "fourth"); + EXPECT_TRUE(obs.hasObservers()); + + conn2.disconnect(); + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, RegularConnectionPersistsAfterDestruction) +{ + ObservableValue obs{0}; + testing::StrictMock> mockObserver; + + { + auto connection = obs.observe(mockObserver.AsStdFunction()); + EXPECT_CALL(mockObserver, Call(1)); + obs = 1; + } + + EXPECT_CALL(mockObserver, Call(2)); + obs = 2; + + EXPECT_TRUE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ScopedConnectionDisconnectsOnDestruction) +{ + ObservableValue obs{0}; + testing::StrictMock> mockObserver; + + { + boost::signals2::scoped_connection const scoped = obs.observe(mockObserver.AsStdFunction()); + EXPECT_CALL(mockObserver, Call(1)); + obs = 1; + EXPECT_TRUE(obs.hasObservers()); + } + + obs = 2; // No call expected since connection was destroyed + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ManualDisconnectWithRegularConnection) +{ + ObservableValue obs{0}; + testing::StrictMock> mockObserver; + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(1)); + obs = 1; + EXPECT_TRUE(obs.hasObservers()); + + connection.disconnect(); + + obs = 2; // No call expected since connection was disconnected + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ScopedConnectionCanBeDisconnectedManually) +{ + ObservableValue obs{0}; + testing::StrictMock> mockObserver; + + boost::signals2::scoped_connection const scoped = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(1)); + obs = 1; + EXPECT_TRUE(obs.hasObservers()); + + scoped.disconnect(); + + obs = 2; // No call expected since connection was disconnected + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, MixedConnectionTypes) +{ + ObservableValue obs{0}; + testing::StrictMock> mockObserver1; + testing::StrictMock> mockObserver2; + testing::StrictMock> mockObserver3; + + auto regularConn = obs.observe(mockObserver1.AsStdFunction()); + + { + boost::signals2::scoped_connection const scoped1 = obs.observe(mockObserver2.AsStdFunction()); + boost::signals2::scoped_connection const scoped2 = obs.observe(mockObserver3.AsStdFunction()); + + EXPECT_CALL(mockObserver1, Call(1)); + EXPECT_CALL(mockObserver2, Call(1)); + EXPECT_CALL(mockObserver3, Call(1)); + obs = 1; + EXPECT_TRUE(obs.hasObservers()); + } + + EXPECT_CALL(mockObserver1, Call(2)); + obs = 2; // Only mockObserver1 should be called since scoped connections were destroyed + EXPECT_TRUE(obs.hasObservers()); + + regularConn.disconnect(); + EXPECT_FALSE(obs.hasObservers()); +} + +TEST_F(ObservableValueTest, ForceNotify) +{ + ObservableValue obs{42}; + testing::StrictMock> mockObserver; + + obs.forceNotify(); + + auto connection = obs.observe(mockObserver.AsStdFunction()); + + EXPECT_CALL(mockObserver, Call(42)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(42)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(100)); + obs.set(100); + EXPECT_CALL(mockObserver, Call(100)); + obs.forceNotify(); + + EXPECT_CALL(mockObserver, Call(100)).Times(3); + obs.forceNotify(); + obs.forceNotify(); + obs.forceNotify(); +} diff --git a/tests/unit/util/RepeatTests.cpp b/tests/unit/util/RepeatTests.cpp index 122307ca82..e686fff778 100644 --- a/tests/unit/util/RepeatTests.cpp +++ b/tests/unit/util/RepeatTests.cpp @@ -53,16 +53,16 @@ struct RepeatTests : SyncAsioContextTest { TEST_F(RepeatTests, CallsHandler) { - repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); EXPECT_CALL(handlerMock, Call).Times(testing::AtMost(22)); + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); runContextFor(std::chrono::milliseconds{20}); } TEST_F(RepeatTests, StopsOnStop) { withRunningContext([this]() { - repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); EXPECT_CALL(handlerMock, Call).Times(AtLeast(1)); + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); std::this_thread::sleep_for(std::chrono::milliseconds{10}); repeat.stop(); }); @@ -72,8 +72,8 @@ TEST_F(RepeatTests, RunsAfterStop) { withRunningContext([this]() { for ([[maybe_unused]] auto i : std::ranges::iota_view(0, 2)) { - repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); EXPECT_CALL(handlerMock, Call).Times(AtLeast(1)); + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); std::this_thread::sleep_for(std::chrono::milliseconds{10}); repeat.stop(); } diff --git a/tests/unit/util/SignalsHandlerTests.cpp b/tests/unit/util/SignalsHandlerTests.cpp index 2fa59186c2..3f0a52ef8a 100644 --- a/tests/unit/util/SignalsHandlerTests.cpp +++ b/tests/unit/util/SignalsHandlerTests.cpp @@ -70,7 +70,7 @@ TEST_F(SignalsHandlerAssertTest, CantCreateTwoSignalsHandlers) { auto makeHandler = []() { return SignalsHandler{ - ClioConfigDefinition{{"graceful_period", ConfigValue{ConfigType::Double}.defaultValue(10.f)}}, []() {} + ClioConfigDefinition{{"graceful_period", ConfigValue{ConfigType::Double}.defaultValue(1.f)}}, []() {} }; }; auto const handler = makeHandler(); @@ -96,7 +96,11 @@ TEST_F(SignalsHandlerTests, OneSignal) handler_.subscribeToStop(stopHandler_.AsStdFunction()); handler_.subscribeToStop(anotherStopHandler_.AsStdFunction()); EXPECT_CALL(stopHandler_, Call()); - EXPECT_CALL(anotherStopHandler_, Call()).WillOnce([this]() { allowTestToFinish(); }); + EXPECT_CALL(anotherStopHandler_, Call()).WillOnce([this] { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + handler_.notifyGracefulShutdownComplete(); + allowTestToFinish(); + }); std::raise(SIGINT); wait(); @@ -113,21 +117,44 @@ struct SignalsHandlerTimeoutTests : SignalsHandlerTestsBase { TEST_F(SignalsHandlerTimeoutTests, OneSignalTimeout) { handler_.subscribeToStop(stopHandler_.AsStdFunction()); - EXPECT_CALL(stopHandler_, Call()).WillOnce([] { std::this_thread::sleep_for(std::chrono::milliseconds(2)); }); - EXPECT_CALL(forceExitHandler_, Call()); + EXPECT_CALL(stopHandler_, Call()).WillOnce([] { + // Don't notify completion, let it timeout + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + }); + EXPECT_CALL(forceExitHandler_, Call()).WillOnce([this]() { allowTestToFinish(); }); std::raise(SIGINT); + + wait(); } TEST_F(SignalsHandlerTests, TwoSignals) { handler_.subscribeToStop(stopHandler_.AsStdFunction()); - EXPECT_CALL(stopHandler_, Call()).WillOnce([] { std::raise(SIGINT); }); + EXPECT_CALL(stopHandler_, Call()).WillOnce([] { + // Raise second signal during graceful shutdown + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + std::raise(SIGINT); + }); EXPECT_CALL(forceExitHandler_, Call()).WillOnce([this]() { allowTestToFinish(); }); std::raise(SIGINT); wait(); } +TEST_F(SignalsHandlerTests, GracefulShutdownCompletes) +{ + handler_.subscribeToStop(stopHandler_.AsStdFunction()); + EXPECT_CALL(stopHandler_, Call()).WillOnce([this] { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + handler_.notifyGracefulShutdownComplete(); + allowTestToFinish(); + }); + EXPECT_CALL(forceExitHandler_, Call()).Times(0); + std::raise(SIGINT); + + wait(); +} + struct SignalsHandlerPriorityTestsBundle { std::string name; SignalsHandler::Priority stopHandlerPriority; @@ -164,9 +191,10 @@ TEST_P(SignalsHandlerPriorityTests, Priority) EXPECT_CALL(stopHandler_, Call()).WillOnce([&] { stopHandlerCalled = true; }); EXPECT_CALL(anotherStopHandler_, Call()).WillOnce([&] { EXPECT_TRUE(stopHandlerCalled); + handler_.notifyGracefulShutdownComplete(); allowTestToFinish(); }); - std::raise(SIGINT); + std::raise(SIGINT); wait(); } diff --git a/tests/unit/web/ServerTests.cpp b/tests/unit/web/ServerTests.cpp index 4c588322bb..75a44ec55c 100644 --- a/tests/unit/web/ServerTests.cpp +++ b/tests/unit/web/ServerTests.cpp @@ -54,11 +54,9 @@ #include #include -#include #include #include #include -#include #include #include #include @@ -231,25 +229,7 @@ makeServerSync( std::reference_wrapper cache ) { - auto server = std::shared_ptr>(); - - std::mutex m; - std::condition_variable cv; - bool ready = false; - - boost::asio::dispatch(ioc.get_executor(), [&]() mutable { - server = web::makeHttpServer(config, ioc, dosGuard, handler, cache); - { - std::lock_guard const lk(m); - ready = true; - } - cv.notify_one(); - }); - { - std::unique_lock lk(m); - cv.wait(lk, [&] { return ready; }); - } - return server; + return web::makeHttpServer(config, ioc, dosGuard, handler, cache); } } // namespace diff --git a/tools/rebuild_conan.py b/tools/rebuild_conan.py index 825b012bd9..e255cf5e7e 100755 --- a/tools/rebuild_conan.py +++ b/tools/rebuild_conan.py @@ -1,8 +1,9 @@ #!/usr/bin/env python3 import json -import plumbum from pathlib import Path +import plumbum + THIS_DIR = Path(__file__).parent.resolve() ROOT_DIR = THIS_DIR.parent.resolve() @@ -21,15 +22,23 @@ def rebuild(): for profile in profiles: print(f"Rebuilding {profile} with build type {build_type}") with plumbum.local.cwd(ROOT_DIR): - CONAN[ - "install", ".", - "--build=missing", - f"--output-folder=build_{profile}_{build_type}", - "-s", f"build_type={build_type}", - "-o", "&:tests=True", - "-o", "&:benchmark=True", - "--profile:all", profile - ] & plumbum.FG + ( + CONAN[ + "install", + ".", + "--build=missing", + f"--output-folder=build_{profile}_{build_type}", + "-s", + f"build_type={build_type}", + "-o", + "&:tests=True", + "-o", + "&:benchmark=True", + "--profile:all", + profile, + ] + & plumbum.FG + ) if __name__ == "__main__":