Skip to content

Commit 0c6ccc8

Browse files
coryancoryan
authored
cleanup(GCS+gRPC): simplify support for resumable uploads (#7290)
1 parent 570ed91 commit 0c6ccc8

8 files changed

Lines changed: 14 additions & 83 deletions

‎google/cloud/storage/BUILD‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,6 @@ GOOGLE_CLOUD_STORAGE_WIN_COPTS = [
2626
load(":google_cloud_cpp_storage.bzl", "google_cloud_cpp_storage_hdrs", "google_cloud_cpp_storage_srcs")
2727
load(":google_cloud_cpp_storage_grpc.bzl", "google_cloud_cpp_storage_grpc_hdrs", "google_cloud_cpp_storage_grpc_srcs")
2828

29-
proto_library(
30-
name = "grpc_resumable_session_url_proto",
31-
srcs = [
32-
"internal/grpc_resumable_upload_session_url.proto",
33-
],
34-
)
35-
36-
cc_proto_library(
37-
name = "grpc_resumable_session_url_cc_proto",
38-
deps = [":grpc_resumable_session_url_proto"],
39-
)
40-
4129
cc_library(
4230
name = "google_cloud_cpp_storage_grpc",
4331
srcs = google_cloud_cpp_storage_grpc_srcs,
@@ -49,7 +37,6 @@ cc_library(
4937
defines = ["GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC"],
5038
deps = [
5139
":google_cloud_cpp_storage",
52-
":grpc_resumable_session_url_cc_proto",
5340
"//google/cloud:google_cloud_cpp_grpc_utils",
5441
"@boringssl//:crypto",
5542
"@boringssl//:ssl",

‎google/cloud/storage/CMakeLists.txt‎

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ set(DOXYGEN_EXCLUDE_PATTERNS
3232
set(DOXYGEN_EXCLUDE_SYMBOLS "internal" "storage_benchmarks")
3333

3434
# Creates the proto headers needed by doxygen.
35-
set(GOOGLE_CLOUD_CPP_DOXYGEN_DEPS google-cloud-cpp::storage_protos
36-
google_cloud_cpp_storage_internal_protos)
35+
set(GOOGLE_CLOUD_CPP_DOXYGEN_DEPS google-cloud-cpp::storage_protos)
3736

3837
include(GoogleCloudCppCommon)
3938

@@ -313,17 +312,6 @@ install(
313312

314313
if (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
315314
find_package(gRPC REQUIRED)
316-
google_cloud_cpp_proto_library(
317-
google_cloud_cpp_storage_internal_protos
318-
internal/grpc_resumable_upload_session_url.proto PROTO_PATH_DIRECTORIES
319-
${CMAKE_CURRENT_SOURCE_DIR}/internal LOCAL_INCLUDE)
320-
add_library(google-cloud-cpp::internal-storage-protos ALIAS
321-
google_cloud_cpp_storage_internal_protos)
322-
set_target_properties(
323-
google_cloud_cpp_storage_internal_protos
324-
PROPERTIES EXPORT_NAME "google-cloud-cpp::internal-storage-protos"
325-
VERSION ${PROJECT_VERSION} SOVERSION
326-
${PROJECT_VERSION_MAJOR})
327315
add_library(
328316
google_cloud_cpp_storage_grpc
329317
grpc_plugin.cc
@@ -349,7 +337,6 @@ if (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
349337
PUBLIC google-cloud-cpp::storage
350338
google-cloud-cpp::grpc_utils
351339
google-cloud-cpp::common
352-
google_cloud_cpp_storage_internal_protos
353340
google-cloud-cpp::storage_protos
354341
nlohmann_json::nlohmann_json
355342
gRPC::grpc++
@@ -378,20 +365,14 @@ if (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
378365

379366
create_bazel_config(google_cloud_cpp_storage_grpc)
380367
else ()
381-
add_library(google_cloud_cpp_storage_internal_protos INTERFACE)
382368
add_library(google_cloud_cpp_storage_grpc INTERFACE)
383369

384370
set_target_properties(
385371
google_cloud_cpp_storage_grpc
386372
PROPERTIES EXPORT_NAME "google-cloud-cpp::experimental-storage-grpc")
387-
set_target_properties(
388-
google_cloud_cpp_storage_internal_protos
389-
PROPERTIES EXPORT_NAME "google-cloud-cpp::internal-storage-protos")
390373
endif (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
391374
add_library(google-cloud-cpp::experimental-storage-grpc ALIAS
392375
google_cloud_cpp_storage_grpc)
393-
add_library(google-cloud-cpp::internal-storage-protos ALIAS
394-
google_cloud_cpp_storage_internal_protos)
395376

396377
# Setup global variables used in the following *.in files.
397378
set(GOOGLE_CLOUD_CPP_PC_NAME "The GCS (Google Cloud Storage) gRPC plugin")
@@ -407,8 +388,7 @@ string(
407388
" google_cloud_cpp_common"
408389
" libcurl"
409390
" openssl")
410-
string(CONCAT GOOGLE_CLOUD_CPP_PC_LIBS "-lgoogle_cloud_cpp_storage_grpc"
411-
" -lgoogle_cloud_cpp_storage_internal_protos")
391+
string(CONCAT GOOGLE_CLOUD_CPP_PC_LIBS "-lgoogle_cloud_cpp_storage_grpc")
412392

413393
# Create and install the pkg-config files.
414394
configure_file("${PROJECT_SOURCE_DIR}/google/cloud/config.pc.in"
@@ -648,7 +628,6 @@ endif ()
648628

649629
install(
650630
TARGETS google_cloud_cpp_storage google_cloud_cpp_storage_grpc
651-
google_cloud_cpp_storage_internal_protos
652631
EXPORT storage-targets
653632
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
654633
COMPONENT google_cloud_cpp_runtime
@@ -661,7 +640,6 @@ install(
661640
# duplication).
662641
install(
663642
TARGETS google_cloud_cpp_storage google_cloud_cpp_storage_grpc
664-
google_cloud_cpp_storage_internal_protos
665643
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
666644
COMPONENT google_cloud_cpp_development
667645
NAMELINK_ONLY

‎google/cloud/storage/internal/grpc_client.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,8 @@ GrpcClient::CreateResumableSession(ResumableUploadRequest const& request) {
390390
if (!response.ok()) return std::move(response).status();
391391

392392
auto self = shared_from_this();
393-
return std::unique_ptr<ResumableUploadSession>(new GrpcResumableUploadSession(
394-
self, request,
395-
{request.bucket_name(), request.object_name(), response->upload_id()}));
393+
return std::unique_ptr<ResumableUploadSession>(
394+
new GrpcResumableUploadSession(self, request, {response->upload_id()}));
396395
}
397396

398397
StatusOr<EmptyResponse> GrpcClient::DeleteResumableUpload(

‎google/cloud/storage/internal/grpc_client_failures_test.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,9 @@ TEST_P(GrpcClientFailuresTest, CreateResumableSession) {
318318
}
319319

320320
TEST_P(GrpcClientFailuresTest, DeleteResumableUpload) {
321-
auto actual = client_->DeleteResumableUpload(DeleteResumableUploadRequest(
322-
EncodeGrpcResumableUploadSessionUrl(ResumableUploadSessionGrpcParams{
323-
"test-bucket", "test-object", "test-upload-id"})));
321+
auto actual = client_->DeleteResumableUpload(
322+
DeleteResumableUploadRequest(EncodeGrpcResumableUploadSessionUrl(
323+
ResumableUploadSessionGrpcParams{"test-upload-id"})));
324324
EXPECT_THAT(actual, StatusIs(AnyOf(StatusCode::kUnavailable,
325325
StatusCode::kUnimplemented)));
326326
}

‎google/cloud/storage/internal/grpc_resumable_upload_session_test.cc‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ StatusOr<google::storage::v2::WriteObjectResponse> MockCloseError(Status s) {
7474
TEST(GrpcResumableUploadSessionTest, Simple) {
7575
auto mock = MockGrpcClient::Create();
7676
ResumableUploadRequest request("test-bucket", "test-object");
77-
GrpcResumableUploadSession session(
78-
mock, request, {"test-bucket", "test-object", "test-upload-id"});
77+
GrpcResumableUploadSession session(mock, request, {"test-upload-id"});
7978

8079
std::string const payload = "test payload";
8180
auto const size = payload.size();
@@ -140,8 +139,7 @@ TEST(GrpcResumableUploadSessionTest, Simple) {
140139
TEST(GrpcResumableUploadSessionTest, SingleStreamForLargeChunks) {
141140
auto mock = MockGrpcClient::Create();
142141
ResumableUploadRequest request("test-bucket", "test-object");
143-
GrpcResumableUploadSession session(
144-
mock, request, {"test-bucket", "test-object", "test-upload-id"});
142+
GrpcResumableUploadSession session(mock, request, {"test-upload-id"});
145143

146144
auto rng = google::cloud::internal::MakeDefaultPRNG();
147145
auto const payload = MakeRandomData(rng, 8 * 1024 * 1024L);
@@ -193,8 +191,7 @@ TEST(GrpcResumableUploadSessionTest, SingleStreamForLargeChunks) {
193191
TEST(GrpcResumableUploadSessionTest, Reset) {
194192
auto mock = MockGrpcClient::Create();
195193
ResumableUploadRequest request("test-bucket", "test-object");
196-
GrpcResumableUploadSession session(
197-
mock, request, {"test-bucket", "test-object", "test-upload-id"});
194+
GrpcResumableUploadSession session(mock, request, {"test-upload-id"});
198195

199196
std::string const payload = "test payload";
200197
auto const size = payload.size();
@@ -261,8 +258,7 @@ TEST(GrpcResumableUploadSessionTest, Reset) {
261258
TEST(GrpcResumableUploadSessionTest, ResumeFromEmpty) {
262259
auto mock = MockGrpcClient::Create();
263260
ResumableUploadRequest request("test-bucket", "test-object");
264-
GrpcResumableUploadSession session(
265-
mock, request, {"test-bucket", "test-object", "test-upload-id"});
261+
GrpcResumableUploadSession session(mock, request, {"test-upload-id"});
266262

267263
std::string const payload = "test payload";
268264
auto const size = payload.size();

‎google/cloud/storage/internal/grpc_resumable_upload_session_url.cc‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/storage/internal/grpc_resumable_upload_session_url.h"
16-
#include "google/cloud/storage/internal/grpc_resumable_upload_session_url.pb.h"
1716
#include "google/cloud/storage/internal/openssl_util.h"
1817
#include "absl/strings/match.h"
1918
#include <cstring>
@@ -24,19 +23,11 @@ namespace storage {
2423
inline namespace STORAGE_CLIENT_NS {
2524
namespace internal {
2625

27-
using ::google::storage::v1::internal::GrpcResumableUploadSessionUrl;
28-
2926
auto constexpr kUriScheme = "grpc://";
3027

3128
std::string EncodeGrpcResumableUploadSessionUrl(
3229
ResumableUploadSessionGrpcParams const& upload_session_params) {
33-
GrpcResumableUploadSessionUrl proto;
34-
proto.set_bucket_name(upload_session_params.bucket_name);
35-
proto.set_object_name(upload_session_params.object_name);
36-
proto.set_upload_id(upload_session_params.upload_id);
37-
std::string proto_rep;
38-
proto.SerializeToString(&proto_rep);
39-
return kUriScheme + UrlsafeBase64Encode(proto_rep);
30+
return kUriScheme + UrlsafeBase64Encode(upload_session_params.upload_id);
4031
}
4132

4233
StatusOr<ResumableUploadSessionGrpcParams> DecodeGrpcResumableUploadSessionUrl(
@@ -51,15 +42,8 @@ StatusOr<ResumableUploadSessionGrpcParams> DecodeGrpcResumableUploadSessionUrl(
5142
auto const payload = upload_session_url.substr(std::strlen(kUriScheme));
5243
auto decoded_vec = UrlsafeBase64Decode(payload);
5344
if (!decoded_vec) return std::move(decoded_vec).status();
54-
std::string decoded(decoded_vec->begin(), decoded_vec->end());
55-
56-
GrpcResumableUploadSessionUrl proto;
57-
if (!proto.ParseFromString(decoded)) {
58-
return Status(StatusCode::kInvalidArgument,
59-
"Malformed gRPC resumable upload session URL");
60-
}
6145
return ResumableUploadSessionGrpcParams{
62-
proto.bucket_name(), proto.object_name(), proto.upload_id()};
46+
std::string(decoded_vec->begin(), decoded_vec->end())};
6347
}
6448

6549
bool IsGrpcResumableSessionUrl(std::string const& upload_session_url) {

‎google/cloud/storage/internal/grpc_resumable_upload_session_url.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ namespace internal {
3838

3939
/// Parameters that should be bundled with a resumable upload session ID.
4040
struct ResumableUploadSessionGrpcParams {
41-
std::string bucket_name;
42-
std::string object_name;
4341
std::string upload_id;
4442
};
4543

‎google/cloud/storage/internal/grpc_resumable_upload_session_url_test.cc‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,11 @@ namespace internal {
2525
namespace {
2626

2727
TEST(GrpcUploadSessionTest, SimpleEncodeDecode) {
28-
ResumableUploadSessionGrpcParams input{"test-bucket", "test-object",
29-
"some-upload-id"};
28+
ResumableUploadSessionGrpcParams input{"some-upload-id"};
3029
auto encoded = EncodeGrpcResumableUploadSessionUrl(input);
3130
ASSERT_TRUE(IsGrpcResumableSessionUrl(encoded));
3231
auto decoded = DecodeGrpcResumableUploadSessionUrl(encoded);
3332
ASSERT_STATUS_OK(decoded) << "Failed to decode url: " << encoded;
34-
EXPECT_EQ(input.bucket_name, decoded->bucket_name);
35-
EXPECT_EQ(input.object_name, decoded->object_name);
3633
EXPECT_EQ(input.upload_id, decoded->upload_id);
3734
}
3835

@@ -47,14 +44,6 @@ TEST(GrpcUploadSessionTest, MalformedUri) {
4744
::testing::HasSubstr("different implementation"));
4845
}
4946

50-
TEST(GrpcUploadSessionTest, MalformedProto) {
51-
auto res = DecodeGrpcResumableUploadSessionUrl(
52-
"grpc://" + UrlsafeBase64Encode("somerubbish"));
53-
EXPECT_FALSE(res);
54-
EXPECT_EQ(StatusCode::kInvalidArgument, res.status().code());
55-
EXPECT_THAT(res.status().message(), ::testing::HasSubstr("Malformed gRPC"));
56-
}
57-
5847
} // namespace
5948
} // namespace internal
6049
} // namespace STORAGE_CLIENT_NS

0 commit comments

Comments
 (0)