Align VersionedLayerClient with VolatileLayerClient

mykhailo-kuchma · mykhailo-kuchma · commit 8f3194a24eea · 2019-10-25T11:57:57.000+03:00
Remove olp namespace for parameters.
Align GetData implementation.
Align the order of parameters in DataRepository.

Relates-To: OLPEDGE-957

Signed-off-by: Mykhailo Kuchma &lt;ext-mykhailo.kuchma@here.com&gt;
diff --git a/olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/VersionedLayerClient.h b/olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/VersionedLayerClient.h
@@ -68,13 +68,14 @@ class DATASERVICE_READ_API VersionedLayerClient final {
 
   /**
    * @brief VersionedLayerClient constructor
-   * @param catalog this versioned layer client uses during requests.
-   * @param layer_id this versioned layer client uses during requests.
-   * @param settings the settings used to control behaviour of the client
-   * instance.
+   * @param catalog a catalog that the versioned layer client uses during
+   * requests.
+   * @param layer_id a layer id that the versioned layer client uses during
+   * requests.
+   * @param settings settings used to control the client instance behavior.
    */
-  VersionedLayerClient(olp::client::HRN catalog, std::string layer_id,
-                       olp::client::OlpClientSettings client_settings);
+  VersionedLayerClient(client::HRN catalog, std::string layer_id,
+                       client::OlpClientSettings settings);
 
   ~VersionedLayerClient();
 
@@ -91,8 +92,8 @@ class DATASERVICE_READ_API VersionedLayerClient final {
    * error is encountered.
    * @return A token that can be used to cancel this request.
    */
-  olp::client::CancellationToken GetData(DataRequest data_request,
-                                         Callback callback);
+  client::CancellationToken GetData(DataRequest data_request,
+                                    Callback callback);
 
   /**
    * @brief fetches a list partitions for given generic layer asynchronously.
@@ -123,7 +124,7 @@ class DATASERVICE_READ_API VersionedLayerClient final {
    * available, or an error is encountered.
    * @return A token that can be used to cancel this request.
    */
-  olp::client::CancellationToken PrefetchTiles(
+  client::CancellationToken PrefetchTiles(
       PrefetchTilesRequest request, PrefetchTilesResponseCallback callback);
 
  private:
diff --git a/olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/VolatileLayerClient.h b/olp-cpp-sdk-dataservice-read/include/olp/dataservice/read/VolatileLayerClient.h
@@ -87,8 +87,7 @@ class DATASERVICE_READ_API VolatileLayerClient final {
    * requests.
    * @param layer_id a layer id that the volatile layer client uses during
    * requests.
-   * @param client_settings settings used to control the client instance
-   * behavior.
+   * @param settings settings used to control the client instance behavior.
    */
   VolatileLayerClient(client::HRN catalog, std::string layer_id,
                       client::OlpClientSettings settings);
diff --git a/olp-cpp-sdk-dataservice-read/src/VersionedLayerClient.cpp b/olp-cpp-sdk-dataservice-read/src/VersionedLayerClient.cpp
@@ -26,16 +26,15 @@ namespace olp {
 namespace dataservice {
 namespace read {
 
-VersionedLayerClient::VersionedLayerClient(
-    olp::client::HRN catalog, std::string layer_id,
-    olp::client::OlpClientSettings client_settings)
+VersionedLayerClient::VersionedLayerClient(client::HRN catalog,
+                                           std::string layer_id,
+                                           client::OlpClientSettings settings)
     : impl_(std::make_unique<VersionedLayerClientImpl>(
-          std::move(catalog), std::move(layer_id),
-          std::move(client_settings))) {}
+          std::move(catalog), std::move(layer_id), std::move(settings))) {}
 
 VersionedLayerClient::~VersionedLayerClient() = default;
 
-olp::client::CancellationToken VersionedLayerClient::GetData(
+client::CancellationToken VersionedLayerClient::GetData(
     DataRequest data_request, Callback callback) {
   return impl_->GetData(std::move(data_request), std::move(callback));
 }
@@ -46,7 +45,7 @@ client::CancellationToken VersionedLayerClient::GetPartitions(
                               std::move(callback));
 }
 
-olp::client::CancellationToken VersionedLayerClient::PrefetchTiles(
+client::CancellationToken VersionedLayerClient::PrefetchTiles(
     PrefetchTilesRequest request, PrefetchTilesResponseCallback callback) {
   return impl_->PrefetchTiles(std::move(request), std::move(callback));
 }
diff --git a/olp-cpp-sdk-dataservice-read/src/VersionedLayerClientImpl.cpp b/olp-cpp-sdk-dataservice-read/src/VersionedLayerClientImpl.cpp
@@ -37,16 +37,15 @@ namespace dataservice {
 namespace read {
 
 VersionedLayerClientImpl::VersionedLayerClientImpl(
-    olp::client::HRN catalog, std::string layer_id,
-    olp::client::OlpClientSettings client_settings)
+    client::HRN catalog, std::string layer_id,
+    client::OlpClientSettings settings)
     : catalog_(std::move(catalog)),
       layer_id_(std::move(layer_id)),
-      settings_(std::make_shared<olp::client::OlpClientSettings>(
-          std::move(client_settings))),
+      settings_(
+          std::make_shared<client::OlpClientSettings>(std::move(settings))),
       pending_requests_(std::make_shared<PendingRequests>()) {
   if (!settings_->cache) {
-    settings_->cache =
-        olp::client::OlpClientSettingsFactory::CreateDefaultCache({});
+    settings_->cache = client::OlpClientSettingsFactory::CreateDefaultCache({});
   }
   // to avoid capturing task scheduler inside a task, we need a copy of settings
   // without the scheduler
@@ -79,7 +78,7 @@ VersionedLayerClientImpl::~VersionedLayerClientImpl() {
 client::CancellationToken VersionedLayerClientImpl::GetPartitions(
     PartitionsRequest partitions_request, PartitionsCallback callback) const {
   partitions_request.WithLayerId(layer_id_);
-  olp::client::CancellationToken token;
+  client::CancellationToken token;
   int64_t request_key = pending_requests_->GenerateRequestPlaceholder();
   auto pending_requests = pending_requests_;
   auto request_callback = [pending_requests, request_key,
@@ -107,48 +106,48 @@ client::CancellationToken VersionedLayerClientImpl::GetPartitions(
   return token;
 }
 
-olp::client::CancellationToken VersionedLayerClientImpl::GetData(
-    DataRequest data_request, Callback callback) const {
-  auto fetch_option = data_request.GetFetchOption();
-  if (fetch_option == CacheWithUpdate) {
-    auto cache_token = AddGetDataTask(data_request.WithFetchOption(CacheOnly),
-                                      std::move(callback));
-    auto online_token =
-        AddGetDataTask(data_request.WithFetchOption(OnlineIfNotFound), nullptr);
-    return client::CancellationToken([cache_token, online_token]() {
-      cache_token.cancel();
-      online_token.cancel();
-    });
-  } else {
-    return AddGetDataTask(data_request, std::move(callback));
-  }
-}
-
-client::CancellationToken VersionedLayerClientImpl::AddGetDataTask(
+client::CancellationToken VersionedLayerClientImpl::GetData(
     DataRequest request, Callback callback) const {
-  auto catalog = catalog_;
-  auto layer_id = layer_id_;
-  auto settings = *settings_;
-  auto pending_requests = pending_requests_;
+  auto add_task = [&](DataRequest& request, Callback callback) {
+    auto catalog = catalog_;
+    auto layer_id = layer_id_;
+    auto settings = *settings_;
+    auto pending_requests = pending_requests_;
 
-  auto data_task = [=](client::CancellationContext context) {
-    return repository::DataRepository::GetVersionedData(
-        catalog, layer_id, settings, request, context);
-  };
+    auto data_task = [=](client::CancellationContext context) {
+      return repository::DataRepository::GetVersionedData(
+          catalog, layer_id, request, context, settings);
+    };
 
-  auto context = TaskContext::Create(std::move(data_task), std::move(callback));
+    auto context =
+        TaskContext::Create(std::move(data_task), std::move(callback));
 
-  pending_requests->Insert(context);
+    pending_requests->Insert(context);
 
-  repository::ExecuteOrSchedule(task_scheduler_, [=]() {
-    context.Execute();
-    pending_requests->Remove(context);
-  });
+    repository::ExecuteOrSchedule(task_scheduler_, [=]() {
+      context.Execute();
+      pending_requests->Remove(context);
+    });
+
+    return context.CancelToken();
+  };
+
+  if (request.GetFetchOption() == FetchOptions::CacheWithUpdate) {
+    auto cache_token = add_task(
+        request.WithFetchOption(FetchOptions::CacheOnly), std::move(callback));
+    auto online_token =
+        add_task(request.WithFetchOption(FetchOptions::OnlineOnly), nullptr);
 
-  return context.CancelToken();
+    return client::CancellationToken([=]() {
+      cache_token.cancel();
+      online_token.cancel();
+    });
+  } else {
+    return add_task(request, std::move(callback));
+  }
 }
 
-olp::client::CancellationToken VersionedLayerClientImpl::PrefetchTiles(
+client::CancellationToken VersionedLayerClientImpl::PrefetchTiles(
     PrefetchTilesRequest request, PrefetchTilesResponseCallback callback) {
   const int64_t request_key = pending_requests_->GenerateRequestPlaceholder();
   auto pending_requests = pending_requests_;
diff --git a/olp-cpp-sdk-dataservice-read/src/VersionedLayerClientImpl.h b/olp-cpp-sdk-dataservice-read/src/VersionedLayerClientImpl.h
@@ -70,28 +70,24 @@ class VersionedLayerClientImpl {
   using PrefetchTilesResponseCallback =
       std::function<void(const PrefetchTilesResponse& response)>;
 
-  VersionedLayerClientImpl(olp::client::HRN catalog, std::string layer_id,
-                           olp::client::OlpClientSettings client_settings);
+  VersionedLayerClientImpl(client::HRN catalog, std::string layer_id,
+                           client::OlpClientSettings settings);
 
   virtual ~VersionedLayerClientImpl();
 
-  virtual olp::client::CancellationToken GetData(DataRequest data_request,
-                                                 Callback callback) const;
+  virtual client::CancellationToken GetData(DataRequest data_request,
+                                            Callback callback) const;
 
   virtual client::CancellationToken GetPartitions(
       PartitionsRequest partitions_request, PartitionsCallback callback) const;
 
-  virtual olp::client::CancellationToken PrefetchTiles(
+  virtual client::CancellationToken PrefetchTiles(
       PrefetchTilesRequest request, PrefetchTilesResponseCallback callback);
 
- private:
-  olp::client::CancellationToken AddGetDataTask(DataRequest data_request,
-                                                Callback callback) const;
-
  protected:
-  olp::client::HRN catalog_;
+  client::HRN catalog_;
   std::string layer_id_;
-  std::shared_ptr<olp::client::OlpClientSettings> settings_;
+  std::shared_ptr<client::OlpClientSettings> settings_;
   std::shared_ptr<thread::TaskScheduler> task_scheduler_;
   std::shared_ptr<PendingRequests> pending_requests_;
   std::shared_ptr<repository::PartitionsRepository> partition_repo_;
diff --git a/olp-cpp-sdk-dataservice-read/src/VolatileLayerClient.cpp b/olp-cpp-sdk-dataservice-read/src/VolatileLayerClient.cpp
@@ -26,16 +26,15 @@ namespace olp {
 namespace dataservice {
 namespace read {
 
-VolatileLayerClient::VolatileLayerClient(
-    olp::client::HRN catalog, std::string layer_id,
-    olp::client::OlpClientSettings settings)
+VolatileLayerClient::VolatileLayerClient(client::HRN catalog,
+                                         std::string layer_id,
+                                         client::OlpClientSettings settings)
     : impl_(std::make_unique<VolatileLayerClientImpl>(
-          std::move(catalog), std::move(layer_id),
-          std::move(settings))) {}
+          std::move(catalog), std::move(layer_id), std::move(settings))) {}
 
 VolatileLayerClient::~VolatileLayerClient() = default;
 
-olp::client::CancellationToken VolatileLayerClient::GetData(
+client::CancellationToken VolatileLayerClient::GetData(
     DataRequest request, Callback<DataResponse> callback) {
   return impl_->GetData(std::move(request), std::move(callback));
 }
diff --git a/olp-cpp-sdk-dataservice-read/src/VolatileLayerClientImpl.cpp b/olp-cpp-sdk-dataservice-read/src/VolatileLayerClientImpl.cpp
@@ -92,9 +92,9 @@ client::CancellationToken VolatileLayerClientImpl::GetData(
       cache_token.cancel();
       online_token.cancel();
     });
+  } else {
+    return add_task(request, std::move(callback));
   }
-
-  return add_task(request, std::move(callback));
 }
 
 }  // namespace read
diff --git a/olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.cpp b/olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.cpp
@@ -43,6 +43,7 @@ using namespace olp::client;
 namespace {
 constexpr auto kDataInlinePrefix = "data:";
 constexpr auto kLogTag = "DataRepository";
+constexpr auto kBlobService = "blob";
 constexpr auto kVolatileBlobService = "volatile-blob";
 
 void GetDataInternal(std::shared_ptr<CancellationContext> cancellationContext,
@@ -330,40 +331,47 @@ void DataRepository::GetData(
 }
 
 DataRepository::DataResponse DataRepository::GetVersionedData(
-    HRN catalog, std::string layer_id, OlpClientSettings client_settings,
-    DataRequest data_request, CancellationContext context) {
-  if (!data_request.GetDataHandle()) {
-    if (!data_request.GetVersion()) {
+    const client::HRN& catalog, const std::string& layer_id,
+    DataRequest request, client::CancellationContext context,
+    client::OlpClientSettings settings) {
+  if (!request.GetDataHandle()) {
+    if (!request.GetVersion()) {
       // get latest version of the layer if it wasn't set by the user
       auto latest_version_response =
-          repository::CatalogRepository::GetLatestVersion(
-              catalog, context, data_request, client_settings);
+          repository::CatalogRepository::GetLatestVersion(catalog, context,
+                                                          request, settings);
       if (!latest_version_response.IsSuccessful()) {
         return latest_version_response.GetError();
       }
-      data_request.WithVersion(
-          latest_version_response.GetResult().GetVersion());
+      request.WithVersion(latest_version_response.GetResult().GetVersion());
     }
 
     // get data handle for a partition to be queried
     auto partitions_response =
         repository::PartitionsRepository::GetPartitionById(
-            catalog, layer_id, context, data_request, client_settings);
+            catalog, layer_id, context, request, settings);
+
     if (!partitions_response.IsSuccessful()) {
       return partitions_response.GetError();
     }
-    auto partitions = partitions_response.GetResult().GetPartitions();
+
+    const auto& partitions = partitions_response.GetResult().GetPartitions();
     if (partitions.empty()) {
+      OLP_SDK_LOG_INFO_F(kLogTag, "Partition %s not found",
+                         request.GetPartitionId()
+                             ? request.GetPartitionId().get().c_str()
+                             : "<none>");
+
       return client::ApiError(client::ErrorCode::NotFound,
                               "Partition not found");
     }
-    data_request.WithDataHandle(partitions.front().GetDataHandle());
+
+    request.WithDataHandle(partitions.front().GetDataHandle());
   }
 
   // finally get the data using a data handle
-  auto data_response = repository::DataRepository::GetBlobData(
-      catalog, layer_id, "blob", data_request, context, client_settings);
-  return data_response;
+  return repository::DataRepository::GetBlobData(
+      catalog, layer_id, kBlobService, request, context, settings);
 }
 
 DataRepository::DataResponse DataRepository::GetBlobData(
@@ -476,18 +484,19 @@ DataRepository::DataResponse DataRepository::GetBlobData(
 }
 
 DataRepository::DataResponse DataRepository::GetVolatileData(
-    const client::HRN& catalog, const std::string& layer_id, DataRequest request,
-    client::CancellationContext context,
+    const client::HRN& catalog, const std::string& layer_id,
+    DataRequest request, client::CancellationContext context,
     client::OlpClientSettings settings) {
   if (!request.GetDataHandle()) {
-    auto response = repository::PartitionsRepository::GetPartitionById(
-        catalog, layer_id, context, request, settings);
+    auto partitions_response =
+        repository::PartitionsRepository::GetPartitionById(
+            catalog, layer_id, context, request, settings);
 
-    if (!response.IsSuccessful()) {
-      return response.GetError();
+    if (!partitions_response.IsSuccessful()) {
+      return partitions_response.GetError();
     }
 
-    const auto& partitions = response.GetResult().GetPartitions();
+    const auto& partitions = partitions_response.GetResult().GetPartitions();
     if (partitions.empty()) {
       OLP_SDK_LOG_INFO_F(kLogTag, "Partition %s not found",
                          request.GetPartitionId()
@@ -501,10 +510,8 @@ DataRepository::DataResponse DataRepository::GetVolatileData(
     request.WithDataHandle(partitions.front().GetDataHandle());
   }
 
-  auto blob_response = repository::DataRepository::GetBlobData(
+  return repository::DataRepository::GetBlobData(
       catalog, layer_id, kVolatileBlobService, request, context, settings);
-
-  return std::move(blob_response);
 }
 
 }  // namespace repository
diff --git a/olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.h b/olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.h
@@ -54,10 +54,11 @@ class DataRepository final {
                const std::string& layerType, const read::DataRequest& request,
                const read::DataResponseCallback& callback);
 
-  static DataResponse GetVersionedData(
-      olp::client::HRN catalog, std::string layer_id,
-      olp::client::OlpClientSettings client_settings, DataRequest data_request,
-      olp::client::CancellationContext context);
+  static DataResponse GetVersionedData(const client::HRN& catalog,
+                                       const std::string& layer_id,
+                                       DataRequest data_request,
+                                       client::CancellationContext context,
+                                       client::OlpClientSettings settings);
 
   static DataResponse GetVolatileData(const client::HRN& catalog,
                                       const std::string& layer_id,

Original file line number	Diff line number	Diff line change
`@@ -92,9 +92,9 @@ client::CancellationToken VolatileLayerClientImpl::GetData(`
`92`	`92`	`cache_token.cancel();`
`93`	`93`	`online_token.cancel();`
`94`	`94`	`});`
	`95`	`+ } else {`
	`96`	`+ return add_task(request, std::move(callback));`
`95`	`97`	`}`
`96`		`-`
`97`		`- return add_task(request, std::move(callback));`
`98`	`98`	`}`
`99`	`99`
`100`	`100`	`} // namespace read`