Add caching for versions infos #900

LiubovDidkivska · 2020-06-08T07:30:01Z

Add implementation to put/get versions
list to catalog client cache.
Implement versions infos cache tests.

Relates-To: OLPEDGE-2080

Signed-off-by: Liubov Didkivska ext-liubov.didkivska@here.com

codecov · 2020-06-08T07:42:29Z

Codecov Report

Merging #900 into master will increase coverage by 0.0%.
The diff coverage is 87.8%.

@@          Coverage Diff           @@
##           master    #900   +/-   ##
======================================
  Coverage    81.2%   81.2%           
======================================
  Files         303     305    +2     
  Lines       10575   10620   +45     
======================================
+ Hits         8591    8628   +37     
- Misses       1984    1992    +8

Impacted Files	Coverage Δ
...ice-read/src/repositories/CatalogCacheRepository.h	`100.0% <ø> (ø)`
...rc/generated/serializer/VersionInfosSerializer.cpp	`64.3% <64.3%> (ø)`
...e-read/src/generated/parser/JsonParserCacheValue.h	`100.0% <100.0%> (ø)`
...e-read/src/repositories/CatalogCacheRepository.cpp	`100.0% <100.0%> (ø)`
...ude/olp/dataservice/read/model/VersionDependency.h	`57.1% <0.0%> (-42.9%)`	⬇️
...d/include/olp/dataservice/read/model/VersionInfo.h	`100.0% <0.0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd2d656...0ef9d58. Read the comment docs.

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

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

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

andescu · 2020-06-09T09:58:16Z

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

+      return repository::CatalogRepository::GetVersionsList(
+          std::move(catalog), context, std::move(request), std::move(settings));


I wonder why do we need then copy parameters on CatalogRepository::GetVersionsList() if it is sync? Can we move towards pass by reference to save copies?

Could we do this in separate pull request and make this changes also in other clients?

Considering our current backlog I dont think that will be any time soon. Ill add a ticket to the backlog for this but i would prefer if we can already do it in your PR as we also did it in #891.
But please note that this is not valid for all methods. For example CatalogClient::GetCatalog() could still use it as it does not have any version relation.

andescu

Tests missing!

andescu · 2020-06-09T14:01:12Z

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

+      return repository::CatalogRepository::GetVersionsList(
+          std::move(catalog), context, std::move(request), std::move(settings));


Considering our current backlog I dont think that will be any time soon. Ill add a ticket to the backlog for this but i would prefer if we can already do it in your PR as we also did it in #891.
But please note that this is not valid for all methods. For example CatalogClient::GetCatalog() could still use it as it does not have any version relation.

olp-cpp-sdk-dataservice-read/src/generated/serializer/VersionInfosSerializer.cpp

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

olp-cpp-sdk-dataservice-read/src/generated/serializer/VersionInfosSerializer.h

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

andescu · 2020-06-10T10:22:06Z

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

@@ -65,4 +68,50 @@ TEST(PartitionsCacheRepositoryTest, DefaultExpiry) {
  }
 }

+TEST(CatalogCacheRepositoryTest, VersionsList) {


Please also add a test case for start_version = -1 as specified by the documentation:

startVersion
The beginning of the range of versions you want to get (exclusive). By convention -1 indicates the initial version before the first publication. After the first publication, the catalog version is 0.

Test will be added to CatalogRepositoryTest

But why in CatalogRepositoryTest as this will have direct implication upon the key generation, which needs to be tested that it works correctly, means that not by mistake the -1 startVersion is converted to 0 as we do not support -1 while converting to string.

olp-cpp-sdk-dataservice-read/src/generated/parser/JsonParserCacheValue.h

Add implementation to put/get versions list to catalog client cache. Implement versions infos cache tests. Relates-To: OLPEDGE-1606 Signed-off-by: Liubov Didkivska <ext-liubov.didkivska@here.com>

andescu suggested changes Jun 9, 2020

View reviewed changes

olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/VersionedLayerClient.h Outdated Show resolved Hide resolved

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch 7 times, most recently from db5e6c8 to ba94dd9 Compare June 9, 2020 08:17

LiubovDidkivska changed the title ~~Add metadata api to get versions list~~ Add versions list implementation Jun 9, 2020

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from 610f818 to 0d299bd Compare June 9, 2020 09:52

LiubovDidkivska requested a review from andescu June 9, 2020 09:52

andescu reviewed Jun 9, 2020

View reviewed changes

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from 019bbda to 13c5256 Compare June 9, 2020 10:48

LiubovDidkivska requested review from andescu, mykhailo-kuchma, meravingen123 and kzvieriev June 9, 2020 12:32

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from c50c5ff to 0f27430 Compare June 9, 2020 16:02

andescu reviewed Jun 10, 2020

View reviewed changes

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from 0f27430 to e327e4e Compare June 10, 2020 09:02

LiubovDidkivska changed the title ~~Add versions list implementation~~ Add caching for versions infos Jun 10, 2020

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from e327e4e to 61e3967 Compare June 10, 2020 09:05

LiubovDidkivska requested a review from andescu June 10, 2020 09:29

andescu reviewed Jun 10, 2020

View reviewed changes

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from 61e3967 to d18a5f0 Compare June 10, 2020 11:15

LiubovDidkivska requested a review from andescu June 10, 2020 11:26

LiubovDidkivska closed this Jun 10, 2020

LiubovDidkivska reopened this Jun 10, 2020

andescu reviewed Jun 10, 2020

View reviewed changes

olp-cpp-sdk-dataservice-read/src/generated/parser/JsonParserCacheValue.h Outdated Show resolved Hide resolved

Liubov Didkivska added 3 commits June 10, 2020 17:24

Add caching versions infos

e5e476d

Add implementation to put/get versions list to catalog client cache. Implement versions infos cache tests. Relates-To: OLPEDGE-1606 Signed-off-by: Liubov Didkivska <ext-liubov.didkivska@here.com>

Add caching versions infos

5a9c5ee

Add implementation to put/get versions list to catalog client cache. Implement versions infos cache tests. Relates-To: OLPEDGE-1606 Signed-off-by: Liubov Didkivska <ext-liubov.didkivska@here.com>

Add caching versions infos

0ef9d58

Add implementation to put/get versions list to catalog client cache. Implement versions infos cache tests. Relates-To: OLPEDGE-1606 Signed-off-by: Liubov Didkivska <ext-liubov.didkivska@here.com>

LiubovDidkivska force-pushed the task/olpedge-1606_versions_api_2__ branch from e3b5dbd to 0ef9d58 Compare June 10, 2020 14:24

LiubovDidkivska requested a review from andescu June 11, 2020 10:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add caching for versions infos #900

Add caching for versions infos #900

LiubovDidkivska commented Jun 8, 2020 •

edited

codecov bot commented Jun 8, 2020 •

edited

andescu Jun 9, 2020

LiubovDidkivska Jun 9, 2020

andescu Jun 9, 2020

andescu left a comment

andescu Jun 9, 2020

andescu Jun 10, 2020

LiubovDidkivska Jun 10, 2020

andescu Jun 10, 2020

		return repository::CatalogRepository::GetVersionsList(
		std::move(catalog), context, std::move(request), std::move(settings));

Add caching for versions infos #900

Are you sure you want to change the base?

Add caching for versions infos #900

Conversation

LiubovDidkivska commented Jun 8, 2020 • edited

codecov bot commented Jun 8, 2020 • edited

Codecov Report

andescu Jun 9, 2020

Choose a reason for hiding this comment

LiubovDidkivska Jun 9, 2020

Choose a reason for hiding this comment

andescu Jun 9, 2020

Choose a reason for hiding this comment

andescu left a comment

Choose a reason for hiding this comment

andescu Jun 9, 2020

Choose a reason for hiding this comment

andescu Jun 10, 2020

Choose a reason for hiding this comment

LiubovDidkivska Jun 10, 2020

Choose a reason for hiding this comment

andescu Jun 10, 2020

Choose a reason for hiding this comment

LiubovDidkivska commented Jun 8, 2020 •

edited

codecov bot commented Jun 8, 2020 •

edited