Skip to content

Commit 4640853

Browse files
coryancoryan
authored
refactor(storage): use g::c::Options in RawClients (#6282)
Change the classes derived from `RawClient` to use a `g::c::Options` object as its only configuration parameter. These classes assume the object is fully initialized, and contains the default values if needed. Add options to pass the retry, backoff, and idempotency policies as option values too.
1 parent d349a4e commit 4640853

25 files changed

Lines changed: 504 additions & 313 deletions

‎google/cloud/storage/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ add_library(
218218
object_rewriter.h
219219
object_stream.cc
220220
object_stream.h
221+
options.h
221222
override_default_project.h
222223
parallel_upload.cc
223224
parallel_upload.h

‎google/cloud/storage/benchmarks/throughput_experiment.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,9 @@ std::vector<std::unique_ptr<ThroughputExperiment>> CreateUploadExperiments(
321321
case ApiName::kApiRawGrpc: {
322322
gcs::Client grpc_client =
323323
google::cloud::storage_experimental::DefaultGrpcClient(
324-
client_options, thread_id);
324+
thread_id,
325+
google::cloud::storage::internal::MakeOptions(client_options))
326+
.value();
325327
result.push_back(
326328
absl::make_unique<UploadObject>(grpc_client, a, contents, false));
327329
result.push_back(
@@ -363,7 +365,9 @@ std::vector<std::unique_ptr<ThroughputExperiment>> CreateDownloadExperiments(
363365
case ApiName::kApiGrpc:
364366
result.push_back(absl::make_unique<DownloadObject>(
365367
google::cloud::storage_experimental::DefaultGrpcClient(
366-
client_options),
368+
thread_id,
369+
google::cloud::storage::internal::MakeOptions(client_options))
370+
.value(),
367371
a));
368372
break;
369373
#else

‎google/cloud/storage/client.cc‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "google/cloud/storage/internal/curl_handle.h"
1818
#include "google/cloud/storage/internal/openssl_util.h"
1919
#include "google/cloud/storage/oauth2/service_account_credentials.h"
20+
#include "google/cloud/internal/algorithm.h"
2021
#include "google/cloud/internal/filesystem.h"
2122
#include "google/cloud/log.h"
2223
#include "absl/memory/memory.h"
@@ -34,8 +35,20 @@ static_assert(std::is_copy_assignable<storage::Client>::value,
3435
"storage::Client must be assignable");
3536

3637
std::shared_ptr<internal::RawClient> Client::CreateDefaultInternalClient(
37-
ClientOptions options) {
38-
return internal::CurlClient::Create(std::move(options));
38+
Options const& opts, std::shared_ptr<internal::RawClient> client) {
39+
using google::cloud::internal::Contains;
40+
auto const& tracing_components = opts.get<TracingComponentsOption>();
41+
auto const enable_logging = Contains(tracing_components, "raw-client") ||
42+
Contains(tracing_components, "rpc");
43+
if (enable_logging) {
44+
client = std::make_shared<internal::LoggingClient>(std::move(client));
45+
}
46+
return std::make_shared<internal::RetryClient>(std::move(client), opts);
47+
}
48+
49+
std::shared_ptr<internal::RawClient> Client::CreateDefaultInternalClient(
50+
Options const& opts) {
51+
return CreateDefaultInternalClient(opts, internal::CurlClient::Create(opts));
3952
}
4053

4154
StatusOr<Client> Client::CreateDefaultClient() {

‎google/cloud/storage/client.h‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,9 @@ class Client {
225225
*/
226226
template <typename... Policies>
227227
explicit Client(ClientOptions options, Policies&&... policies)
228-
: Client(InternalOnly{}, CreateDefaultInternalClient(std::move(options)),
229-
std::forward<Policies>(policies)...) {}
228+
: Client(InternalOnly{}, internal::ApplyPolicies(
229+
internal::MakeOptions(std::move(options)),
230+
std::forward<Policies>(policies)...)) {}
230231

231232
/**
232233
* Creates the default client type given the credentials and policies.
@@ -246,8 +247,10 @@ class Client {
246247
template <typename... Policies>
247248
explicit Client(std::shared_ptr<oauth2::Credentials> credentials,
248249
Policies&&... policies)
249-
: Client(ClientOptions(std::move(credentials)),
250-
std::forward<Policies>(policies)...) {}
250+
: Client(InternalOnly{},
251+
internal::ApplyPolicies(
252+
internal::DefaultOptions(std::move(credentials), {}),
253+
std::forward<Policies>(policies)...)) {}
251254

252255
/// Create a Client using ClientOptions::CreateDefaultClientOptions().
253256
static StatusOr<Client> CreateDefaultClient();
@@ -3130,24 +3133,15 @@ class Client {
31303133
struct InternalOnly {};
31313134
struct InternalOnlyNoDecorations {};
31323135

3133-
template <typename... Policies>
3134-
Client(InternalOnly, std::shared_ptr<internal::RawClient> c, Policies&&... p)
3135-
: raw_client_(Decorate(std::move(c), std::forward<Policies>(p)...)) {}
3136+
Client(InternalOnly, Options const& opts)
3137+
: raw_client_(CreateDefaultInternalClient(opts)) {}
31363138
Client(InternalOnlyNoDecorations, std::shared_ptr<internal::RawClient> c)
31373139
: raw_client_(std::move(c)) {}
3138-
static std::shared_ptr<internal::RawClient> CreateDefaultInternalClient(
3139-
ClientOptions options);
31403140

3141-
template <typename... Policies>
3142-
std::shared_ptr<internal::RawClient> Decorate(
3143-
std::shared_ptr<internal::RawClient> client, Policies&&... policies) {
3144-
if (client->client_options().enable_raw_client_tracing()) {
3145-
client = std::make_shared<internal::LoggingClient>(std::move(client));
3146-
}
3147-
auto retry = std::make_shared<internal::RetryClient>(
3148-
std::move(client), std::forward<Policies>(policies)...);
3149-
return retry;
3150-
}
3141+
static std::shared_ptr<internal::RawClient> CreateDefaultInternalClient(
3142+
Options const& opts, std::shared_ptr<internal::RawClient> client);
3143+
static std::shared_ptr<internal::RawClient> CreateDefaultInternalClient(
3144+
Options const& opts);
31513145

31523146
ObjectReadStream ReadObjectImpl(
31533147
internal::ReadObjectRangeRequest const& request);
@@ -3263,13 +3257,25 @@ struct ClientImplDetails {
32633257
template <typename... Policies>
32643258
static Client CreateClient(std::shared_ptr<internal::RawClient> c,
32653259
Policies&&... p) {
3266-
return Client(Client::InternalOnly{}, std::move(c),
3267-
std::forward<Policies>(p)...);
3260+
auto opts =
3261+
internal::ApplyPolicies(internal::MakeOptions(c->client_options()),
3262+
std::forward<Policies>(p)...);
3263+
return Client(Client::InternalOnlyNoDecorations{},
3264+
Client::CreateDefaultInternalClient(opts, std::move(c)));
32683265
}
32693266
static Client CreateWithoutDecorations(
32703267
std::shared_ptr<internal::RawClient> c) {
32713268
return Client(Client::InternalOnlyNoDecorations{}, std::move(c));
32723269
}
3270+
3271+
// TODO(#6161) - this should become a public API and the *recommended* way to
3272+
// create a Client.
3273+
static Client CreateClient(std::shared_ptr<oauth2::Credentials> credentials,
3274+
Options opts = {}) {
3275+
opts = DefaultOptions(std::move(credentials), std::move(opts));
3276+
return Client(Client::InternalOnlyNoDecorations{},
3277+
Client::CreateDefaultInternalClient(opts));
3278+
}
32733279
};
32743280

32753281
// Just a wrapper to allow for using in `google::cloud::internal::apply`.

‎google/cloud/storage/client_options.cc‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,28 +78,47 @@ std::size_t DefaultConnectionPoolSize() {
7878
#define GOOGLE_CLOUD_CPP_STORAGE_DEFAULT_DOWNLOAD_STALL_TIMEOUT 120
7979
#endif // GOOGLE_CLOUD_CPP_STORAGE_DEFAULT_DOWNLOAD_STALL_TIMEOUT
8080

81+
// Define the defaults using a pre-processor macro, this allows the application
82+
// developers to change the defaults for their application by compiling with
83+
// different values.
84+
#ifndef STORAGE_CLIENT_DEFAULT_MAXIMUM_RETRY_PERIOD
85+
#define STORAGE_CLIENT_DEFAULT_MAXIMUM_RETRY_PERIOD std::chrono::minutes(15)
86+
#endif // STORAGE_CLIENT_DEFAULT_MAXIMUM_RETRY_PERIOD
87+
88+
#ifndef STORAGE_CLIENT_DEFAULT_INITIAL_BACKOFF_DELAY
89+
#define STORAGE_CLIENT_DEFAULT_INITIAL_BACKOFF_DELAY std::chrono::seconds(1)
90+
#endif // STORAGE_CLIENT_DEFAULT_INITIAL_BACKOFF_DELAY
91+
92+
#ifndef STORAGE_CLIENT_DEFAULT_MAXIMUM_BACKOFF_DELAY
93+
#define STORAGE_CLIENT_DEFAULT_MAXIMUM_BACKOFF_DELAY std::chrono::minutes(5)
94+
#endif // STORAGE_CLIENT_DEFAULT_MAXIMUM_BACKOFF_DELAY
95+
96+
#ifndef STORAGE_CLIENT_DEFAULT_BACKOFF_SCALING
97+
#define STORAGE_CLIENT_DEFAULT_BACKOFF_SCALING 2.0
98+
#endif // STORAGE_CLIENT_DEFAULT_BACKOFF_SCALING
99+
81100
} // namespace
82101

83102
namespace internal {
84103

85104
std::string JsonEndpoint(Options const& options) {
86-
return GetEmulator().value_or(options.get<GcsRestEndpointOption>()) +
105+
return GetEmulator().value_or(options.get<RestEndpointOption>()) +
87106
"/storage/" + options.get<TargetApiVersionOption>();
88107
}
89108

90109
std::string JsonUploadEndpoint(Options const& options) {
91-
return GetEmulator().value_or(options.get<GcsRestEndpointOption>()) +
110+
return GetEmulator().value_or(options.get<RestEndpointOption>()) +
92111
"/upload/storage/" + options.get<TargetApiVersionOption>();
93112
}
94113

95114
std::string XmlEndpoint(Options const& options) {
96-
return GetEmulator().value_or(options.get<GcsRestEndpointOption>());
115+
return GetEmulator().value_or(options.get<RestEndpointOption>());
97116
}
98117

99118
std::string IamEndpoint(Options const& options) {
100119
auto emulator = GetEmulator();
101120
if (emulator) return *emulator + "/iamapi";
102-
return options.get<GcsIamEndpointOption>();
121+
return options.get<IamEndpointOption>();
103122
}
104123

105124
Options MakeOptions(ClientOptions o) {
@@ -112,13 +131,28 @@ ClientOptions MakeBackwardsCompatibleClientOptions(Options opts) {
112131
return ClientOptions(std::move(opts));
113132
}
114133

134+
Options ApplyPolicy(Options opts, RetryPolicy const& p) {
135+
opts.set<RetryPolicyOption>(p.clone());
136+
return opts;
137+
}
138+
139+
Options ApplyPolicy(Options opts, BackoffPolicy const& p) {
140+
opts.set<BackoffPolicyOption>(p.clone());
141+
return opts;
142+
}
143+
144+
Options ApplyPolicy(Options opts, IdempotencyPolicy const& p) {
145+
opts.set<IdempotencyPolicyOption>(p.clone());
146+
return opts;
147+
}
148+
115149
Options DefaultOptions(std::shared_ptr<oauth2::Credentials> credentials,
116150
Options opts) {
117151
auto o =
118152
Options{}
119153
.set<Oauth2CredentialsOption>(std::move(credentials))
120-
.set<GcsRestEndpointOption>("https://storage.googleapis.com")
121-
.set<GcsIamEndpointOption>("https://iamcredentials.googleapis.com/v1")
154+
.set<RestEndpointOption>("https://storage.googleapis.com")
155+
.set<IamEndpointOption>("https://iamcredentials.googleapis.com/v1")
122156
.set<TargetApiVersionOption>("v1")
123157
.set<ConnectionPoolSizeOption>(DefaultConnectionPoolSize())
124158
.set<DownloadBufferSizeOption>(
@@ -132,13 +166,24 @@ Options DefaultOptions(std::shared_ptr<oauth2::Credentials> credentials,
132166
.set<MaximumCurlSocketRecvSizeOption>(0)
133167
.set<MaximumCurlSocketSendSizeOption>(0)
134168
.set<DownloadStallTimeoutOption>(std::chrono::seconds(
135-
GOOGLE_CLOUD_CPP_STORAGE_DEFAULT_DOWNLOAD_STALL_TIMEOUT));
169+
GOOGLE_CLOUD_CPP_STORAGE_DEFAULT_DOWNLOAD_STALL_TIMEOUT))
170+
.set<RetryPolicyOption>(
171+
LimitedTimeRetryPolicy(
172+
STORAGE_CLIENT_DEFAULT_MAXIMUM_RETRY_PERIOD)
173+
.clone())
174+
.set<BackoffPolicyOption>(
175+
ExponentialBackoffPolicy(
176+
STORAGE_CLIENT_DEFAULT_INITIAL_BACKOFF_DELAY,
177+
STORAGE_CLIENT_DEFAULT_MAXIMUM_BACKOFF_DELAY,
178+
STORAGE_CLIENT_DEFAULT_BACKOFF_SCALING)
179+
.clone())
180+
.set<IdempotencyPolicyOption>(AlwaysRetryIdempotencyPolicy().clone());
136181

137182
o = google::cloud::internal::MergeOptions(std::move(opts), std::move(o));
138183
auto emulator = GetEmulator();
139184
if (emulator.has_value()) {
140-
o.set<GcsRestEndpointOption>(*emulator).set<GcsIamEndpointOption>(
141-
*emulator + "/iamapi");
185+
o.set<RestEndpointOption>(*emulator).set<IamEndpointOption>(*emulator +
186+
"/iamapi");
142187
}
143188

144189
auto tracing =

‎google/cloud/storage/client_options.h‎

Lines changed: 17 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_CLIENT_OPTIONS_H
1717

1818
#include "google/cloud/storage/oauth2/credentials.h"
19+
#include "google/cloud/storage/options.h"
1920
#include "google/cloud/storage/version.h"
2021
#include "google/cloud/common_options.h"
2122
#include "google/cloud/options.h"
@@ -35,73 +36,23 @@ std::string XmlEndpoint(Options const&);
3536
std::string IamEndpoint(Options const&);
3637

3738
Options MakeOptions(ClientOptions);
38-
ClientOptions MakeBackwardsCompatibleClientOptions(Options);
39-
Options DefaultOptions(std::shared_ptr<oauth2::Credentials> credentials,
40-
Options opts = {});
41-
42-
/// Configure the IAM endpoint for the GCS client library.
43-
struct GcsRestEndpointOption {
44-
using Type = std::string;
45-
};
46-
47-
struct GcsIamEndpointOption {
48-
using Type = std::string;
49-
};
50-
51-
/// Configure oauth2::Credentials for the GCS client library.
52-
struct Oauth2CredentialsOption {
53-
using Type = std::shared_ptr<oauth2::Credentials>;
54-
};
55-
56-
/// This is only intended for testing against staging or development versions
57-
/// of the service. It should *remain* in the internal:: namespace.
58-
struct TargetApiVersionOption {
59-
using Type = std::string;
60-
};
61-
62-
struct ProjectIdOption {
63-
using Type = std::string;
64-
};
65-
66-
struct ConnectionPoolSizeOption {
67-
using Type = std::size_t;
68-
};
6939

70-
struct DownloadBufferSizeOption {
71-
using Type = std::size_t;
72-
};
73-
74-
struct UploadBufferSizeOption {
75-
using Type = std::size_t;
76-
};
77-
78-
struct MaximumSimpleUploadSizeOption {
79-
using Type = std::size_t;
80-
};
81-
82-
struct EnableCurlSslLockingOption {
83-
using Type = bool;
84-
};
85-
86-
struct EnableCurlSigpipeHandlerOption {
87-
using Type = bool;
88-
};
40+
ClientOptions MakeBackwardsCompatibleClientOptions(Options);
8941

90-
struct MaximumCurlSocketRecvSizeOption {
91-
using Type = std::size_t;
92-
};
42+
Options ApplyPolicy(Options opts, RetryPolicy const& p);
43+
Options ApplyPolicy(Options opts, BackoffPolicy const& p);
44+
Options ApplyPolicy(Options opts, IdempotencyPolicy const& p);
9345

94-
struct MaximumCurlSocketSendSizeOption {
95-
using Type = std::size_t;
96-
};
46+
inline Options ApplyPolicies(Options opts) { return opts; }
9747

98-
struct DownloadStallTimeoutOption {
99-
using Type = std::chrono::seconds;
100-
};
48+
template <typename P, typename... Policies>
49+
Options ApplyPolicies(Options opts, P&& head, Policies&&... tail) {
50+
opts = ApplyPolicy(std::move(opts), std::forward<P>(head));
51+
return ApplyPolicies(std::move(opts), std::forward<Policies>(tail)...);
52+
}
10153

102-
struct SslRootPathOption {
103-
using Type = std::string;
104-
};
54+
Options DefaultOptions(std::shared_ptr<oauth2::Credentials> credentials,
55+
Options opts = {});
10556

10657
} // namespace internal
10758

@@ -173,18 +124,18 @@ class ClientOptions {
173124
}
174125

175126
std::string const& endpoint() const {
176-
return opts_.get<internal::GcsRestEndpointOption>();
127+
return opts_.get<internal::RestEndpointOption>();
177128
}
178129
ClientOptions& set_endpoint(std::string endpoint) {
179-
opts_.set<internal::GcsRestEndpointOption>(std::move(endpoint));
130+
opts_.set<internal::RestEndpointOption>(std::move(endpoint));
180131
return *this;
181132
}
182133

183134
std::string const& iam_endpoint() const {
184-
return opts_.get<internal::GcsIamEndpointOption>();
135+
return opts_.get<internal::IamEndpointOption>();
185136
}
186137
ClientOptions& set_iam_endpoint(std::string endpoint) {
187-
opts_.set<internal::GcsIamEndpointOption>(std::move(endpoint));
138+
opts_.set<internal::IamEndpointOption>(std::move(endpoint));
188139
return *this;
189140
}
190141

0 commit comments

Comments
 (0)