From cb684f010957b5c89d9e1ad23c166dde15dd1a9a Mon Sep 17 00:00:00 2001 From: Andrey Kashcheev Date: Fri, 3 Jul 2026 13:07:10 +0200 Subject: [PATCH] Don't cancel merged requests Cancellation is not an error and shouldn't be propagated to parallel requests Relates-To: DATASDK-101 Signed-off-by: Andrey Kashcheev --- .github/workflows/psv_pipelines.yml | 2 +- .../src/repositories/NamedMutex.cpp | 8 +- .../tests/DataRepositoryTest.cpp | 75 ++++++++++++++++++- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/.github/workflows/psv_pipelines.yml b/.github/workflows/psv_pipelines.yml index 3a472acf6..8d81da016 100644 --- a/.github/workflows/psv_pipelines.yml +++ b/.github/workflows/psv_pipelines.yml @@ -274,7 +274,7 @@ jobs: psv-ios-arm64-xcode-26-build: name: PSV.iOS.MacOS15.Xcode26.ARM64 - runs-on: macos-latest + runs-on: macos-15 steps: - name: Check out repository uses: actions/checkout@v6 diff --git a/olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp b/olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp index 409bf71b2..b3e417eb1 100644 --- a/olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp +++ b/olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2023 HERE Europe B.V. + * Copyright (C) 2020-2026 HERE Europe B.V. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -85,6 +85,12 @@ std::mutex& NamedMutexStorage::Impl::GetLockMutex(const std::string& resource) { void NamedMutexStorage::Impl::SetError(const std::string& resource, const client::ApiError& error) { + // Ignore cancellation since this is not an error and parallel requests should + // not be canceled because of it. + if (error.GetErrorCode() == client::ErrorCode::Cancelled) { + return; + } + std::lock_guard lock(mutex_); auto mutex_it = mutexes_.find(resource); if (mutex_it != mutexes_.end()) { diff --git a/olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp b/olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp index e34cb34cf..2f8fa8578 100644 --- a/olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp +++ b/olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019-2024 HERE Europe B.V. + * Copyright (C) 2019-2026 HERE Europe B.V. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -375,6 +375,79 @@ TEST_F(DataRepositoryTest, GetBlobDataSimultaniousFailedCalls) { second_request_thread.join(); } +TEST_F(DataRepositoryTest, GetBlobDataSimultaniousCancelCalls) { + EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _)) + .WillRepeatedly( + ReturnHttpResponse(olp::http::NetworkResponse().WithStatus( + olp::http::HttpStatusCode::OK), + kUrlResponseLookup)); + + std::promise network_request_started_promise; + std::promise finish_network_request_promise; + + auto wait = [&]() { + network_request_started_promise.set_value(); + finish_network_request_promise.get_future().wait(); + }; + + testing::InSequence sequence; + EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _)) + .WillOnce(testing::DoAll( + testing::InvokeWithoutArgs(wait), + ReturnHttpResponse( + olp::http::NetworkResponse().WithStatus( + static_cast(olp::http::ErrorCode::CANCELLED_ERROR)), + "Cancelled"))); + + EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _)) + .WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus( + olp::http::HttpStatusCode::OK), + "someData")); + + olp::client::CancellationContext context; + olp::dataservice::read::repository::NamedMutexStorage storage; + + olp::dataservice::read::model::Partition partition; + partition.SetDataHandle(kUrlBlobDataHandle); + + olp::client::HRN hrn(GetTestCatalog()); + ApiLookupClient lookup_client(hrn, *settings_); + DataRepository repository(hrn, *settings_, lookup_client, storage); + + // Start first request in a separate thread + std::thread first_request_thread([&]() { + auto response = repository.GetBlobData( + kLayerId, kService, partition, + olp::dataservice::read::FetchOptions::OnlineIfNotFound, + olp::porting::none, context, false); + EXPECT_FALSE(response.IsSuccessful()); + ASSERT_EQ(response.GetError().GetErrorCode(), + olp::client::ErrorCode::Cancelled); + }); + + // Wait until network request processing started + network_request_started_promise.get_future().wait(); + + // Get a mutex from the storage. It guarantees that when the second thread + // acquires the mutex, the stored error will not be cleaned up in scope of + // ReleaseLock call from the first thread + olp::dataservice::read::repository::NamedMutex mutex( + storage, hrn.ToString() + kService + kUrlBlobDataHandle, context); + + // Start second request in a separate thread + std::thread second_request_thread([&]() { + auto response = repository.GetBlobData( + kLayerId, kService, partition, + olp::dataservice::read::FetchOptions::OnlineIfNotFound, + olp::porting::none, context, false); + EXPECT_TRUE(response.IsSuccessful()); + }); + + finish_network_request_promise.set_value(); + first_request_thread.join(); + second_request_thread.join(); +} + TEST_F(DataRepositoryTest, GetVersionedDataTile) { EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _)) .WillOnce(ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(