-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add asynchronous methods for GetLatestVersion functionality #1306
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1306 +/- ##
========================================
+ Coverage 86.1% 86.2% +0.1%
========================================
Files 381 381
Lines 13425 13497 +72
========================================
+ Hits 11565 11636 +71
- Misses 1860 1861 +1
Continue to review full report at Codecov.
|
7fe7b02
to
c1c638e
Compare
afd16fe
to
21d435a
Compare
0c57b8f
to
e93ebbc
Compare
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
27bc9ff
to
aba52e9
Compare
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
9b268e8
to
1ea88ec
Compare
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp
Outdated
Show resolved
Hide resolved
1ea88ec
to
91318f5
Compare
91318f5
to
8db431e
Compare
Creation of asynchronous methods is a part of task continuation changes, later will be used in asynchronous GetVersion method. GetVersion is not added in this commit as it's in the private section and cannot be covered with tests. Add asynchronous version of GetLatestVersion method which handles the logic of getting data from cache/network. Add asynchronous version of GetLatestCatalogVersion. Move common functionality of getting data from the cache and storing data returned from a request to the cache to the RetrieveLatestVersion function. Add unit tests which cover asynchronous methods for getting the latest version. Replaced mock expectation arguments with WithoutArgs. Relates-To: OLPEDGE-2718 Signed-off-by: Yevhenii Dudnyk <[email protected]>
8db431e
to
20d7ec1
Compare
static client::CancellationToken GetLatestCatalogVersion( | ||
const client::OlpClient& client, int64_t start_version, | ||
boost::optional<std::string> billing_tag, | ||
read::CatalogVersionCallback callback); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read::CatalogVersionCallback callback); | |
CatalogVersionCallback callback); |
} | ||
} | ||
|
||
return std::move(version_response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pessimizing-move
More info: https://stackoverflow.com/questions/62061433/how-to-avoid-the-pessimizing-move-warning-of-nrvo
return std::move(version_response); | |
return version_response; |
version_response = std::move(*cached_version); | ||
} | ||
|
||
return std::move(version_response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pessimizing-move
More info: https://stackoverflow.com/questions/62061433/how-to-avoid-the-pessimizing-move-warning-of-nrvo
return std::move(version_response); | |
return version_response; |
@@ -91,8 +98,7 @@ class CatalogRepositoryTest : public ::testing::Test { | |||
settings_.network_request_handler = network_; | |||
settings_.cache = cache_; | |||
|
|||
lookup_client_ = | |||
std::make_shared<olp::client::ApiLookupClient>(kHrn, settings_); | |||
lookup_client_ = std::make_shared<client::ApiLookupClient>(kHrn, settings_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a distinct using for this one.
lookup_client_ = std::make_shared<client::ApiLookupClient>(kHrn, settings_); | |
lookup_client_ = std::make_shared<ApiLookupClient>(kHrn, settings_); |
olp::client::OlpClientSettings settings_; | ||
std::shared_ptr<olp::client::ApiLookupClient> lookup_client_; | ||
client::OlpClientSettings settings_; | ||
std::shared_ptr<client::ApiLookupClient> lookup_client_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::shared_ptr<client::ApiLookupClient> lookup_client_; | |
std::shared_ptr<ApiLookupClient> lookup_client_; |
olp::client::CancellationContext context; | ||
TEST_F(CatalogRepositoryTest, AsyncGetLatestVersionOnlineOnlyForbidden) { | ||
const auto request = | ||
read::CatalogVersionRequest().WithFetchOption(read::OnlineIfNotFound); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have OnlineIfNotFound
request, but the name of the test contains OnlineOnly
. Also, line 316 contains a mention of OnlineOnly
.
} | ||
|
||
TEST_F(CatalogRepositoryTest, AsyncGetLatestVersionOnlineOnlyUserCancelled2) { | ||
const auto request = read::CatalogVersionRequest(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A default fetch option is OnlineIfNotFound. But the test name contains OnlineOnly
Creation of asynchronous methods is a part of task
continuation changes, later will be used in asynchronous
GetVersion method. GetVersion is not added in this commit
as it's in the private section and cannot be covered with tests.
Add asynchronous version of GetLatestVersion method
which handles the logic of getting data from cache/network.
Add asynchronous version of GetLatestCatalogVersion.
Move common functionality of getting data from the cache
and storing data returned from a request to the cache
to the RetrieveLatestVersion function.
Add unit tests which cover asynchronous methods for getting
the latest version.
Replaced mock expectation arguments with WithoutArgs.
Relates-To: OLPEDGE-2718
Signed-off-by: Yevhenii Dudnyk [email protected]