Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(storage): support per-request options #11445

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions google/cloud/internal/curl_rest_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,24 @@ CurlRestClient::CurlRestClient(std::string endpoint_address,
}

StatusOr<std::unique_ptr<CurlImpl>> CurlRestClient::CreateCurlImpl(
RestContext const& context, RestRequest const& request) {
RestContext const& context, RestRequest const& request,
Options const& options) {
auto handle = CurlHandle::MakeFromPool(*handle_factory_);
auto impl =
std::make_unique<CurlImpl>(std::move(handle), handle_factory_, options_);
std::make_unique<CurlImpl>(std::move(handle), handle_factory_, options);
if (credentials_) {
auto auth_header = oauth2_internal::AuthorizationHeader(*credentials_);
if (!auth_header.ok()) return std::move(auth_header).status();
impl->SetHeader(auth_header.value());
}
impl->SetHeader(HostHeader(options_, endpoint_address_));
impl->SetHeader(HostHeader(options, endpoint_address_));
impl->SetHeader(x_goog_api_client_header_);
impl->SetHeaders(context, request);
RestRequest::HttpParameters additional_parameters;
// The UserIp option has been deprecated in favor of quotaUser. Only add the
// parameter if the option has been set.
if (options_.has<UserIpOption>()) {
auto user_ip = options_.get<UserIpOption>();
if (options.has<UserIpOption>()) {
auto user_ip = options.get<UserIpOption>();
if (user_ip.empty()) user_ip = impl->LastClientIpAddress();
if (!user_ip.empty()) additional_parameters.emplace_back("userIp", user_ip);
}
Expand All @@ -140,53 +141,58 @@ StatusOr<std::unique_ptr<CurlImpl>> CurlRestClient::CreateCurlImpl(
// CreateCurlImpl relies heavily on member variables.
StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Delete(
RestContext& context, RestRequest const& request) {
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
auto response = (*impl)->MakeRequest(CurlImpl::HttpMethod::kDelete, context);
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Get(
RestContext& context, RestRequest const& request) {
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
auto response = (*impl)->MakeRequest(CurlImpl::HttpMethod::kGet, context);
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Patch(
RestContext& context, RestRequest const& request,
std::vector<absl::Span<char const>> const& payload) {
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
Status response = MakeRequestWithPayload(CurlImpl::HttpMethod::kPatch,
context, request, **impl, payload);
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Post(
RestContext& context, RestRequest const& request,
std::vector<absl::Span<char const>> const& payload) {
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
Status response = MakeRequestWithPayload(CurlImpl::HttpMethod::kPost, context,
request, **impl, payload);
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Post(
RestContext& context, RestRequest const& request,
std::vector<std::pair<std::string, std::string>> const& form_data) {
context.AddHeader("content-type", "application/x-www-form-urlencoded");
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
std::string form_payload = absl::StrJoin(
form_data, "&",
Expand All @@ -198,19 +204,20 @@ StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Post(
request, **impl, {form_payload});
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

StatusOr<std::unique_ptr<RestResponse>> CurlRestClient::Put(
RestContext& context, RestRequest const& request,
std::vector<absl::Span<char const>> const& payload) {
auto impl = CreateCurlImpl(context, request);
auto options = internal::MergeOptions(context.options(), options_);
auto impl = CreateCurlImpl(context, request, options);
if (!impl.ok()) return impl.status();
Status response = MakeRequestWithPayload(CurlImpl::HttpMethod::kPut, context,
request, **impl, payload);
if (!response.ok()) return response;
return {std::unique_ptr<CurlRestResponse>(
new CurlRestResponse(options_, std::move(*impl)))};
new CurlRestResponse(std::move(options), std::move(*impl)))};
}

std::unique_ptr<RestClient> MakeDefaultRestClient(std::string endpoint_address,
Expand All @@ -226,7 +233,6 @@ std::unique_ptr<RestClient> MakePooledRestClient(std::string endpoint_address,
if (options.has<ConnectionPoolSizeOption>()) {
pool_size = options.get<ConnectionPoolSizeOption>();
}

if (pool_size > 0) {
auto pool = std::make_shared<PooledCurlHandleFactory>(pool_size, options);
return MakeTracingRestClient(std::make_unique<CurlRestClient>(
Expand Down
5 changes: 3 additions & 2 deletions google/cloud/internal/curl_rest_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ class CurlRestClient : public RestClient {
std::vector<absl::Span<char const>> const& payload) override;

private:
StatusOr<std::unique_ptr<CurlImpl>> CreateCurlImpl(
RestContext const& context, RestRequest const& request);
StatusOr<std::unique_ptr<CurlImpl>> CreateCurlImpl(RestContext const& context,
RestRequest const& request,
Options const& options);

std::string endpoint_address_;
std::shared_ptr<CurlHandleFactory> handle_factory_;
Expand Down
25 changes: 25 additions & 0 deletions google/cloud/internal/curl_rest_client_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,31 @@ TEST_F(RestClientIntegrationTest, CaptureMetadata) {
ASSERT_TRUE(parsed_response.is_object()) << "body=" << *body;
}

TEST_F(RestClientIntegrationTest, PerRequestOptions) {
auto client = MakeDefaultRestClient(url_, {});
RestRequest request;
request.SetPath("anything");
auto const version = google::cloud::version_string();
auto const p1 = "p1/" + google::cloud::version_string();
auto const p2 = "p2/" + google::cloud::version_string();
auto response_status = RetryRestRequest([&] {
rest_internal::RestContext context(
Options{}.set<UserAgentProductsOption>({p1, p2}));
return client->Get(context, request);
});
ASSERT_STATUS_OK(response_status);
auto response = *std::move(response_status);
ASSERT_THAT(response->StatusCode(), Eq(HttpStatusCode::kOk));
auto body = ReadAll(std::move(*response).ExtractPayload());
ASSERT_STATUS_OK(body);
auto parsed_response = nlohmann::json::parse(*body, nullptr, false);
ASSERT_TRUE(parsed_response.is_object()) << "body=" << *body;
auto headers = parsed_response.find("headers");
ASSERT_TRUE(headers != parsed_response.end()) << "body=" << *body;
EXPECT_THAT(headers->value("User-Agent", ""), HasSubstr(p1));
EXPECT_THAT(headers->value("User-Agent", ""), HasSubstr(p2));
}

} // namespace
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace rest_internal
Expand Down
10 changes: 9 additions & 1 deletion google/cloud/internal/rest_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_REST_CONTEXT_H
#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_INTERNAL_REST_CONTEXT_H

#include "google/cloud/options.h"
#include "google/cloud/version.h"
#include "absl/types/optional.h"
#include <chrono>
Expand All @@ -36,7 +37,13 @@ class RestContext {
public:
using HttpHeaders = std::unordered_map<std::string, std::vector<std::string>>;
RestContext() = default;
explicit RestContext(HttpHeaders headers) : headers_(std::move(headers)) {}
explicit RestContext(Options options, HttpHeaders headers)
: options_(std::move(options)), headers_(std::move(headers)) {}
explicit RestContext(Options options) : RestContext(std::move(options), {}) {}
explicit RestContext(HttpHeaders headers)
: RestContext({}, std::move(headers)) {}

Options const& options() const { return options_; }

HttpHeaders const& headers() const { return headers_; }

Expand Down Expand Up @@ -104,6 +111,7 @@ class RestContext {

private:
friend bool operator==(RestContext const& lhs, RestContext const& rhs);
Options options_;
HttpHeaders headers_;
absl::optional<std::string> local_ip_address_;
absl::optional<std::int32_t> local_port_;
Expand Down
1 change: 1 addition & 0 deletions google/cloud/storage/internal/bucket_acl_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ std::ostream& operator<<(std::ostream& os, UpdateBucketAclRequest const& r);
class PatchBucketAclRequest
: public GenericBucketAclRequest<PatchBucketAclRequest> {
public:
PatchBucketAclRequest() = default;
PatchBucketAclRequest(std::string bucket, std::string entity,
BucketAccessControl const& original,
BucketAccessControl const& new_acl);
Expand Down
4 changes: 4 additions & 0 deletions google/cloud/storage/internal/bucket_requests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ std::ostream& operator<<(std::ostream& os, GetBucketIamPolicyRequest const& r) {
return os << "}";
}

SetNativeBucketIamPolicyRequest::SetNativeBucketIamPolicyRequest()
: SetNativeBucketIamPolicyRequest(
std::string{}, NativeIamPolicy(std::vector<NativeIamBinding>{})) {}

SetNativeBucketIamPolicyRequest::SetNativeBucketIamPolicyRequest(
std::string bucket_name, NativeIamPolicy const& policy)
: bucket_name_(std::move(bucket_name)),
Expand Down
1 change: 1 addition & 0 deletions google/cloud/storage/internal/bucket_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ std::ostream& operator<<(std::ostream& os, GetBucketIamPolicyRequest const& r);
class SetNativeBucketIamPolicyRequest
: public GenericRequest<SetNativeBucketIamPolicyRequest, UserProject> {
public:
SetNativeBucketIamPolicyRequest();
explicit SetNativeBucketIamPolicyRequest(std::string bucket_name,
NativeIamPolicy const& policy);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ std::ostream& operator<<(std::ostream& os,
class PatchDefaultObjectAclRequest
: public GenericDefaultObjectAclRequest<PatchDefaultObjectAclRequest> {
public:
PatchDefaultObjectAclRequest() = default;
PatchDefaultObjectAclRequest(std::string bucket, std::string entity,
ObjectAccessControl const& original,
ObjectAccessControl const& new_acl);
Expand Down
4 changes: 4 additions & 0 deletions google/cloud/storage/internal/hmac_key_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class ListHmacKeysRequest
: public GenericHmacKeyRequest<ListHmacKeysRequest, Deleted, MaxResults,
ServiceAccountFilter> {
public:
ListHmacKeysRequest() = default;
explicit ListHmacKeysRequest(std::string project_id)
: GenericHmacKeyRequest(std::move(project_id)) {}

Expand Down Expand Up @@ -150,6 +151,7 @@ std::ostream& operator<<(std::ostream& os, ListHmacKeysResponse const& r);
class DeleteHmacKeyRequest
: public GenericHmacKeyRequest<DeleteHmacKeyRequest> {
public:
DeleteHmacKeyRequest() = default;
explicit DeleteHmacKeyRequest(std::string project_id, std::string access_id)
: GenericHmacKeyRequest(std::move(project_id)),
access_id_(std::move(access_id)) {}
Expand All @@ -165,6 +167,7 @@ std::ostream& operator<<(std::ostream& os, DeleteHmacKeyRequest const& r);
/// Represents a request to call the `HmacKeys: get` API.
class GetHmacKeyRequest : public GenericHmacKeyRequest<GetHmacKeyRequest> {
public:
GetHmacKeyRequest() = default;
explicit GetHmacKeyRequest(std::string project_id, std::string access_id)
: GenericHmacKeyRequest(std::move(project_id)),
access_id_(std::move(access_id)) {}
Expand All @@ -181,6 +184,7 @@ std::ostream& operator<<(std::ostream& os, GetHmacKeyRequest const& r);
class UpdateHmacKeyRequest
: public GenericHmacKeyRequest<UpdateHmacKeyRequest> {
public:
UpdateHmacKeyRequest() = default;
explicit UpdateHmacKeyRequest(std::string project_id, std::string access_id,
HmacKeyMetadata resource)
: GenericHmacKeyRequest(std::move(project_id)),
Expand Down
1 change: 1 addition & 0 deletions google/cloud/storage/internal/object_acl_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ std::ostream& operator<<(std::ostream& os, UpdateObjectAclRequest const& r);
class PatchObjectAclRequest
: public GenericObjectAclRequest<PatchObjectAclRequest> {
public:
PatchObjectAclRequest() = default;
PatchObjectAclRequest(std::string bucket, std::string object,
std::string entity, ObjectAccessControl const& original,
ObjectAccessControl const& new_acl);
Expand Down
3 changes: 3 additions & 0 deletions google/cloud/storage/internal/object_requests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ std::ostream& operator<<(std::ostream& os, GetObjectMetadataRequest const& r) {
return os << "}";
}

InsertObjectMediaRequest::InsertObjectMediaRequest()
: hash_function_(CreateHashFunction(*this)) {}

InsertObjectMediaRequest::InsertObjectMediaRequest(std::string bucket_name,
std::string object_name,
absl::string_view payload)
Expand Down
3 changes: 1 addition & 2 deletions google/cloud/storage/internal/object_requests.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ class InsertObjectMediaRequest
MD5HashValue, PredefinedAcl, Projection, UserProject,
UploadFromOffset, UploadLimit, WithObjectMetadata> {
public:
InsertObjectMediaRequest() = default;

InsertObjectMediaRequest();
InsertObjectMediaRequest(std::string bucket_name, std::string object_name,
absl::string_view payload);

Expand Down
Loading