Actually use LoggingClient. (#950)

The storage::Client was supposed to create a chain of decorated
RawClient objects: RetryClient -> LoggingClient -> CurlClient.
Somehow I dropped LoggingClient from the chain, created a test
and fixed the problem.

Author: coryan

google/cloud/storage/client.h

@@ -15,7 +15,7 @@
 #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_CLIENT_H_
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_CLIENT_H_
 
-#include "google/cloud/storage/internal/raw_client.h"
+#include "google/cloud/storage/internal/logging_client.h"
 #include "google/cloud/storage/internal/retry_client.h"
 #include "google/cloud/storage/list_buckets_reader.h"
 #include "google/cloud/storage/list_objects_reader.h"
@@ -28,7 +28,7 @@ inline namespace STORAGE_CLIENT_NS {
 /**
  * The Google Cloud Storage Client.
  *
- * @warning this implementation is incomplete, we are still prototyping.
+ * @warning this implementation is incomplete.
  */
 class Client {
  public:
@@ -52,12 +52,12 @@ class Client {
   template <typename... Policies>
   explicit Client(std::shared_ptr<internal::RawClient> client,
                   Policies&&... policies)
-      : raw_client_(new internal::RetryClient(
-            std::move(client), std::forward<Policies>(policies)...)) {}
+      : raw_client_(
+            Decorate(std::move(client), std::forward<Policies>(policies)...)) {}
 
-  /// Build a client with an specific RawClient, without retry policies.
-  struct NoRetry {};
-  explicit Client(std::shared_ptr<internal::RawClient> client, NoRetry)
+  /// Build a client with an specific RawClient, without decorations.
+  struct NoDecorations {};
+  explicit Client(std::shared_ptr<internal::RawClient> client, NoDecorations)
       : raw_client_(std::move(client)) {}
 
   /**
@@ -385,13 +385,26 @@ class Client {
     return raw_client_->UpdateObjectAcl(request).second;
   }
 
+  std::shared_ptr<internal::RawClient> raw_client() const {
+    return raw_client_;
+  }
+
  private:
   BucketMetadata GetBucketMetadataImpl(
       internal::GetBucketMetadataRequest const& request);
 
   ObjectMetadata InsertObjectMediaImpl(
       internal::InsertObjectMediaRequest const& request);
 
+  template <typename... Policies>
+  std::shared_ptr<internal::RawClient> Decorate(
+      std::shared_ptr<internal::RawClient> client, Policies&&... policies) {
+    auto logging = std::make_shared<internal::LoggingClient>(std::move(client));
+    auto retry = std::make_shared<internal::RetryClient>(
+        std::move(logging), std::forward<Policies>(policies)...);
+    return retry;
+  }
+
  private:
   std::shared_ptr<internal::RawClient> raw_client_;
 };

google/cloud/storage/client_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/storage/client.h"
+#include "google/cloud/storage/internal/curl_client.h"
 #include "google/cloud/storage/retry_policy.h"
 #include "google/cloud/storage/testing/canonical_errors.h"
 #include "google/cloud/storage/testing/mock_client.h"
@@ -125,6 +126,23 @@ TEST_F(ClientTest, OverrideBothPolicies) {
   EXPECT_LE(1, ObservableBackoffPolicy::on_completion_call_count);
 }
 
+/// @test Verify the constructor creates the right set of RawClient decorations.
+TEST_F(ClientTest, DefaultDecorators) {
+  // Create a client, use the insecure credentials because on the CI environment
+  // there may not be other credentials configured.
+  Client tested(CreateInsecureCredentials());
+
+  EXPECT_TRUE(tested.raw_client() != nullptr);
+  auto retry = dynamic_cast<internal::RetryClient*>(tested.raw_client().get());
+  ASSERT_TRUE(retry != nullptr);
+
+  auto logging = dynamic_cast<internal::LoggingClient*>(retry->client().get());
+  ASSERT_TRUE(logging != nullptr);
+
+  auto curl = dynamic_cast<internal::CurlClient*>(logging->client().get());
+  ASSERT_TRUE(curl != nullptr);
+}
+
 }  // namespace
 }  // namespace STORAGE_CLIENT_NS
 }  // namespace storage

google/cloud/storage/internal/logging_client.h

@@ -67,6 +67,8 @@ class LoggingClient : public RawClient {
   std::pair<Status, ObjectAccessControl> UpdateObjectAcl(
       UpdateObjectAclRequest const&) override;
 
+  std::shared_ptr<RawClient> client() const { return client_; }
+
  private:
   std::shared_ptr<RawClient> client_;
 };

google/cloud/storage/internal/retry_client.h

@@ -77,6 +77,8 @@ class RetryClient : public RawClient {
   std::pair<Status, ObjectAccessControl> UpdateObjectAcl(
       UpdateObjectAclRequest const&) override;
 
+  std::shared_ptr<RawClient> client() const { return client_; }
+
  private:
   void Apply(RetryPolicy& policy) { retry_policy_ = policy.clone(); }