Skip to content

Commit ac929fa

Browse files
Save latest version with GetLatestVersion request (#1293)
When user is using fixed catalog versions it is possible that request GetLatestVersion won't return actual version from the cache since it hadn't been written to the cache. Use StartVersion as a new latest version in the cache-only requests when previous version is absent or less than the user-set version Relates-To: OAM-1387 Signed-off-by: Andrey Kashcheev <[email protected]>
1 parent d292b1f commit ac929fa

File tree

3 files changed

+81
-12
lines changed

3 files changed

+81
-12
lines changed

olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/CatalogClient.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2020 HERE Europe B.V.
2+
* Copyright (C) 2019-2021 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -119,6 +119,14 @@ class DATASERVICE_READ_API CatalogClient final {
119119
/**
120120
* @brief Gets the catalog version asynchronously.
121121
*
122+
* @note In case you call this API with `FetchOptions::CacheOnly` and a valid
123+
* version in `CatalogVersionRequest::WithStartVersion()`, i.e. >= 0, then
124+
* please make sure that the provided version is a existing catalog version as
125+
* it will be written for later use to the cache as latest version in the
126+
* following cases:
127+
* - There is no latest version yet written to cache.
128+
* - The latest version written to cache is less then the provided version.
129+
*
122130
* @param request The `CatalogVersionRequest` instance that contains
123131
* a complete set of request parameters.
124132
* @param callback The `CatalogVersionCallback` object that is invoked if
@@ -132,6 +140,14 @@ class DATASERVICE_READ_API CatalogClient final {
132140
/**
133141
* @brief Gets the catalog version asynchronously.
134142
*
143+
* @note In case you call this API with `FetchOptions::CacheOnly` and a valid
144+
* version in `CatalogVersionRequest::WithStartVersion()`, i.e. >= 0, then
145+
* please make sure that the provided version is a existing catalog version as
146+
* it will be written for later use to the cache as latest version in the
147+
* following cases:
148+
* - There is no latest version yet written to cache.
149+
* - The latest version written to cache is less then the provided version.
150+
*
135151
* @param request The `CatalogVersionRequest` instance that contains
136152
* a complete set of request parameters.
137153
*

olp-cpp-sdk-dataservice-read/src/repositories/CatalogRepository.cpp

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2020 HERE Europe B.V.
2+
* Copyright (C) 2019-2021 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -110,13 +110,14 @@ CatalogVersionResponse CatalogRepository::GetLatestVersion(
110110
catalog_, settings_.cache, settings_.default_cache_expiration);
111111

112112
const auto fetch_option = request.GetFetchOption();
113-
// in case if get version online was never called and version was not found in
113+
// in case get version online was never called and version was not found in
114114
// cache
115115
CatalogVersionResponse version_response = {
116116
{client::ErrorCode::NotFound, "Failed to find version."}};
117117

118118
if (fetch_option != CacheOnly) {
119-
version_response = GetLatestVersionOnline(request.GetBillingTag(), context);
119+
version_response =
120+
GetLatestVersionOnline(request.GetBillingTag(), std::move(context));
120121

121122
if (fetch_option == OnlineOnly) {
122123
return version_response;
@@ -135,26 +136,48 @@ CatalogVersionResponse CatalogRepository::GetLatestVersion(
135136
}
136137
}
137138

138-
const auto cached_version = repository.GetVersion();
139+
auto cached_version = repository.GetVersion();
140+
141+
// Using `GetStartVersion` to set up new latest version for CacheOnly
142+
// requests in case of absence of previous latest version or it less than the
143+
// new user set version
144+
if (fetch_option == CacheOnly) {
145+
constexpr auto kDefaultStartVersion = 0;
146+
147+
auto user_set_version = request.GetStartVersion();
148+
if (user_set_version != kDefaultStartVersion) {
149+
if (!cached_version || user_set_version > cached_version->GetVersion()) {
150+
model::VersionResponse new_response;
151+
new_response.SetVersion(user_set_version);
152+
version_response = new_response;
153+
}
154+
}
155+
}
139156

140157
if (version_response.IsSuccessful()) {
141-
const auto online_version = version_response.GetResult().GetVersion();
158+
const auto new_version = version_response.GetResult().GetVersion();
142159
// Write or update the version in cache, updating happens only when the new
143160
// version is greater than cached.
144-
if (!cached_version || (*cached_version).GetVersion() < online_version) {
161+
if (!cached_version || (*cached_version).GetVersion() < new_version) {
145162
repository.PutVersion(version_response.GetResult());
146-
OLP_SDK_LOG_DEBUG_F(
147-
kLogTag, "Latest online version, hrn='%s', version=%" PRId64,
148-
catalog_.ToCatalogHRNString().c_str(), online_version);
163+
if (fetch_option == CacheOnly) {
164+
OLP_SDK_LOG_DEBUG_F(
165+
kLogTag, "Latest user set version, hrn='%s', version=%" PRId64,
166+
catalog_.ToCatalogHRNString().c_str(), new_version);
167+
} else {
168+
OLP_SDK_LOG_DEBUG_F(kLogTag,
169+
"Latest online version, hrn='%s', version=%" PRId64,
170+
catalog_.ToCatalogHRNString().c_str(), new_version);
171+
}
149172
}
150173
return version_response;
151174
}
152175

153176
if (cached_version) {
154-
version_response = std::move(*cached_version);
155177
OLP_SDK_LOG_DEBUG_F(
156178
kLogTag, "Latest cached version, hrn='%s', version=%" PRId64,
157179
catalog_.ToCatalogHRNString().c_str(), (*cached_version).GetVersion());
180+
version_response = *std::move(cached_version);
158181
}
159182

160183
return version_response;

olp-cpp-sdk-dataservice-read/tests/CatalogRepositoryTest.cpp

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ const auto kHrn = olp::client::HRN::FromString(kCatalog);
8585
class CatalogRepositoryTest : public ::testing::Test {
8686
protected:
8787
void SetUp() override {
88-
cache_ = std::make_shared<testing::NiceMock<CacheMock>>();
88+
cache_ = std::make_shared<testing::NaggyMock<CacheMock>>();
8989
network_ = std::make_shared<testing::NiceMock<NetworkMock>>();
9090

9191
settings_.network_request_handler = network_;
@@ -160,6 +160,36 @@ TEST_F(CatalogRepositoryTest, GetLatestVersionCacheOnlyNotFound) {
160160
olp::client::ErrorCode::NotFound);
161161
}
162162

163+
TEST_F(CatalogRepositoryTest, GetLatestVersionCacheOnlyRequestWithMinVersion) {
164+
olp::client::CancellationContext context;
165+
166+
auto request = read::CatalogVersionRequest();
167+
request.WithFetchOption(olp::dataservice::read::CacheOnly)
168+
.WithStartVersion(kStartVersion);
169+
170+
EXPECT_CALL(*cache_, Get(_, _))
171+
.Times(1)
172+
.WillOnce(testing::Return(boost::any{}));
173+
174+
EXPECT_CALL(*cache_, Put(_, _, _, _)).Times(1);
175+
176+
ON_CALL(*network_, Send(_, _, _, _, _))
177+
.WillByDefault([](olp::http::NetworkRequest, olp::http::Network::Payload,
178+
olp::http::Network::Callback,
179+
olp::http::Network::HeaderCallback,
180+
olp::http::Network::DataCallback) {
181+
ADD_FAILURE() << "Should not be called with CacheOnly";
182+
return olp::http::SendOutcome(olp::http::ErrorCode::UNKNOWN_ERROR);
183+
});
184+
185+
ApiLookupClient lookup_client(kHrn, settings_);
186+
repository::CatalogRepository repository(kHrn, settings_, lookup_client);
187+
auto response = repository.GetLatestVersion(request, context);
188+
189+
EXPECT_TRUE(response.IsSuccessful());
190+
EXPECT_EQ(response.GetResult().GetVersion(), kStartVersion);
191+
}
192+
163193
TEST_F(CatalogRepositoryTest, GetLatestVersionOnlineOnlyNotFound) {
164194
olp::client::CancellationContext context;
165195

0 commit comments

Comments
 (0)