Skip to content

Commit b0e746b

Browse files
mykhailo-kuchmadiachenko-mischa
authored andcommitted
Add a task context class.
Class take care of users callbacks, and make sure that client callback is called exactly 1 time. VolatileLayerClientImpl::GetData adapted to TaskContext usage VersionedLayerClientImpl::GetData adapted to TaskContext usage Resolves: OLPEDGE-909 Signed-off-by: Mykhailo Kuchma <[email protected]> Signed-off-by: Diachenko Mykahilo <[email protected]>
1 parent 42b25fc commit b0e746b

File tree

15 files changed

+580
-69
lines changed

15 files changed

+580
-69
lines changed

olp-cpp-sdk-core/include/olp/core/client/ApiResponse.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ namespace client {
3636
template <typename Result, typename Error>
3737
class ApiResponse {
3838
public:
39+
using ResultType = Result;
40+
using ErrorType = Error;
41+
3942
/**
4043
* @brief ApiResponse Default constructor.
4144
*/
@@ -44,12 +47,12 @@ class ApiResponse {
4447
/**
4548
* @brief ApiResponse Constructor for a successfully executed request.
4649
*/
47-
ApiResponse(const Result& result) : result_(result), success_(true) {}
50+
ApiResponse(const ResultType& result) : result_(result), success_(true) {}
4851

4952
/**
5053
* @brief ApiResponse Constructor if request unsuccessfully executed
5154
*/
52-
ApiResponse(const Error& error) : error_(error), success_(false) {}
55+
ApiResponse(const ErrorType& error) : error_(error), success_(false) {}
5356

5457
/**
5558
* @brief ApiResponse Copy constructor.
@@ -68,18 +71,18 @@ class ApiResponse {
6871
* @return A valid Result if IsSuccessful() returns true. Undefined,
6972
* otherwise.
7073
*/
71-
inline const Result& GetResult() const { return result_; }
74+
inline const ResultType& GetResult() const { return result_; }
7275

7376
/**
7477
* @brief GetError Gets the error for an unsucccessful request attempt.
7578
* @return A valid Error if IsSuccessful() returns false. Undefined,
7679
* otherwise.
7780
*/
78-
inline const Error& GetError() const { return error_; }
81+
inline const ErrorType& GetError() const { return error_; }
7982

8083
private:
81-
Result result_ {};
82-
Error error_ {};
84+
ResultType result_{};
85+
ErrorType error_{};
8386
bool success_ {false};
8487
};
8588

olp-cpp-sdk-core/include/olp/core/client/Condition.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ namespace client {
3333
*/
3434
class Condition final {
3535
public:
36-
Condition(std::chrono::milliseconds timeout = std::chrono::seconds(60))
37-
: timeout_{timeout} {}
36+
Condition() = default;
3837

3938
/**
4039
* @brief Should be called by task's callback to notify \c Wait to unblock the
@@ -51,18 +50,19 @@ class Condition final {
5150
/**
5251
* @brief Waits a task for a \c Notify or \c CancellationContext to be
5352
* cancelled by the user.
53+
* @param timeout milliseconds to wait on condition
54+
* @return True on notified wake, False on timeout
5455
*/
55-
bool Wait() {
56+
bool Wait(std::chrono::milliseconds timeout = std::chrono::seconds(60)) {
5657
std::unique_lock<std::mutex> lock(mutex_);
57-
bool triggered = condition_.wait_for(
58-
lock, timeout_, [&] { return signaled_; });
58+
bool triggered =
59+
condition_.wait_for(lock, timeout, [&] { return signaled_; });
5960

6061
signaled_ = false;
6162
return triggered;
6263
}
6364

6465
private:
65-
const std::chrono::milliseconds timeout_;
6666
std::condition_variable condition_;
6767
std::mutex mutex_;
6868
bool signaled_{false};

olp-cpp-sdk-core/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ set(OLP_CPP_SDK_CORE_TESTS_SOURCES
2222
./cache/InMemoryCacheTest.cpp
2323

2424
./client/CancellationContextTest.cpp
25+
./client/ConditionTest.cpp
2526

2627
./geo/coordinates/GeoCoordinates3dTest.cpp
2728
./geo/coordinates/GeoCoordinatesTest.cpp
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright (C) 2019 HERE Europe B.V.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* License-Filename: LICENSE
18+
*/
19+
20+
#include <olp/core/client/Condition.h>
21+
22+
#include <gtest/gtest.h>
23+
#include <future>
24+
25+
using olp::client::Condition;
26+
27+
namespace {
28+
TEST(ConditionTest, NotifyBeforeWaitRespected) {
29+
Condition condition{};
30+
condition.Notify();
31+
EXPECT_TRUE(condition.Wait());
32+
}
33+
34+
TEST(ConditionTest, WaitClearsTriggered) {
35+
Condition condition{};
36+
condition.Notify();
37+
EXPECT_TRUE(condition.Wait());
38+
EXPECT_FALSE(condition.Wait(std::chrono::seconds(0)));
39+
}
40+
41+
TEST(ConditionTest, Timeouted) {
42+
Condition condition{};
43+
EXPECT_FALSE(condition.Wait(std::chrono::seconds(0)));
44+
}
45+
46+
TEST(ConditionTest, WakeUp) {
47+
Condition condition{};
48+
auto wait_future =
49+
std::async(std::launch::async, [&]() { return condition.Wait(); });
50+
EXPECT_EQ(wait_future.wait_for(std::chrono::milliseconds(10)),
51+
std::future_status::timeout);
52+
condition.Notify();
53+
EXPECT_EQ(wait_future.wait_for(std::chrono::milliseconds(10)),
54+
std::future_status::ready);
55+
EXPECT_TRUE(wait_future.get());
56+
}
57+
} // namespace

olp-cpp-sdk-dataservice-read/src/ApiClientLookup.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,7 @@ ApiClientLookup::ApiClientResponse ApiClientLookup::LookupApi(
152152
}
153153
}
154154

155-
client::Condition condition(
156-
std::chrono::seconds(settings.retry_settings.timeout));
155+
client::Condition condition{};
157156

158157
auto client = std::make_shared<client::OlpClient>();
159158
client->SetSettings(std::move(settings));
@@ -190,7 +189,7 @@ ApiClientLookup::ApiClientResponse ApiClientLookup::LookupApi(
190189
condition.Notify();
191190
});
192191

193-
if (!condition.Wait()) {
192+
if (!condition.Wait(std::chrono::seconds(settings.retry_settings.timeout))) {
194193
cancellation_context.CancelOperation();
195194
return client::ApiError(client::ErrorCode::RequestTimeout,
196195
"Network request timed out.");

olp-cpp-sdk-dataservice-read/src/PendingRequests.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
#include "PendingRequests.h"
21+
#include "TaskContext.h"
2122

2223
namespace olp {
2324
namespace dataservice {
@@ -30,11 +31,18 @@ PendingRequests::~PendingRequests(){};
3031
bool PendingRequests::CancelPendingRequests() {
3132
requests_lock_.lock();
3233
// Create local copy of the requests to cancel
34+
auto contexts = std::move(task_contexts_);
3335
auto requests_map = requests_map_;
36+
3437
requests_lock_.unlock();
3538
for (auto& pair : requests_map) {
3639
pair.second.cancel();
3740
}
41+
42+
for (auto context : contexts) {
43+
context.BlockingCancel();
44+
}
45+
3846
return true;
3947
}
4048

@@ -55,6 +63,11 @@ bool PendingRequests::Insert(const client::CancellationToken& request,
5563
return true;
5664
}
5765

66+
void PendingRequests::Insert(TaskContext task_context) {
67+
std::lock_guard<std::mutex> lk(requests_lock_);
68+
task_contexts_.insert(task_context);
69+
}
70+
5871
bool PendingRequests::Remove(int64_t key) {
5972
std::lock_guard<std::mutex> lk(requests_lock_);
6073
auto request = requests_map_.find(key);
@@ -65,6 +78,11 @@ bool PendingRequests::Remove(int64_t key) {
6578
return false;
6679
}
6780

81+
void PendingRequests::Remove(TaskContext task_context) {
82+
std::lock_guard<std::mutex> lk(requests_lock_);
83+
task_contexts_.erase(task_context);
84+
}
85+
6886
} // namespace read
6987
} // namespace dataservice
70-
} // namespace olp
88+
} // namespace olp

olp-cpp-sdk-dataservice-read/src/PendingRequests.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919

2020
#pragma once
2121

22-
#include <unordered_map>
22+
#include <memory>
2323
#include <mutex>
24+
#include <unordered_map>
25+
#include <unordered_set>
2426
#include <vector>
2527

2628
#include <olp/core/client/CancellationToken.h>
29+
#include "TaskContext.h"
2730

2831
namespace olp {
2932
namespace dataservice {
@@ -56,16 +59,29 @@ class PendingRequests final {
5659
*/
5760
bool Insert(const client::CancellationToken& token, int64_t key);
5861

62+
/**
63+
* @brief Inserts task context into the requests container.
64+
* @param task_context Task context.
65+
*/
66+
void Insert(TaskContext task_context);
67+
5968
/**
6069
* @brief Removes a pending request and placholder.
6170
* @param key Internal request key to remove
6271
* @return True on success
6372
*/
6473
bool Remove(int64_t key);
6574

75+
/**
76+
* @brief Removes a task context.
77+
* @param task_context Task context.
78+
*/
79+
void Remove(TaskContext task_context);
80+
6681
private:
6782
int64_t key_ = 0;
6883
std::unordered_map<int64_t, client::CancellationToken> requests_map_;
84+
std::unordered_set<TaskContext, TaskContextHash> task_contexts_;
6985
std::mutex requests_lock_;
7086
};
7187

0 commit comments

Comments
 (0)