diff --git a/.clang-format b/.clang-format index 5d29807a7b7fe..394687f115afd 100644 --- a/.clang-format +++ b/.clang-format @@ -47,4 +47,4 @@ RawStringFormats: - 'proto' BasedOnStyle: Google -CommentPragmas: '(@copydoc|@copybrief|@see)' +CommentPragmas: '(@copydoc|@copybrief|@see|@overload)' diff --git a/google/cloud/storage/benchmarks/throughput_experiment.cc b/google/cloud/storage/benchmarks/throughput_experiment.cc index 86aef8f6f3813..ce2d168d58634 100644 --- a/google/cloud/storage/benchmarks/throughput_experiment.cc +++ b/google/cloud/storage/benchmarks/throughput_experiment.cc @@ -140,10 +140,10 @@ class SimpleUpload : public ThroughputExperiment { // truncate the object to the right size. auto const start = std::chrono::system_clock::now(); auto timer = Timer::PerThread(); - std::string data = - random_data_->substr(0, static_cast(config.object_size)); + auto data = absl::string_view{*random_data_}.substr( + 0, static_cast(config.object_size)); auto object_metadata = - client_.InsertObject(bucket_name, object_name, std::move(data), + client_.InsertObject(bucket_name, object_name, data, gcs::DisableCrc32cChecksum(!config.enable_crc32c), gcs::DisableMD5Hash(!config.enable_md5)); auto const usage = timer.Sample(); diff --git a/google/cloud/storage/client.cc b/google/cloud/storage/client.cc index 95b2018659c19..80c6132dba7a5 100644 --- a/google/cloud/storage/client.cc +++ b/google/cloud/storage/client.cc @@ -165,7 +165,7 @@ StatusOr Client::UploadFileSimple( return Status(StatusCode::kInternal, std::move(os).str()); } is.close(); - request.set_contents(std::move(payload)); + request.set_payload(payload); return raw_client_->InsertObjectMedia(request); } diff --git a/google/cloud/storage/client.h b/google/cloud/storage/client.h index 3510ae1857b1a..0bcae9e10823e 100644 --- a/google/cloud/storage/client.h +++ b/google/cloud/storage/client.h @@ -40,6 +40,7 @@ #include "google/cloud/status.h" #include "google/cloud/status_or.h" #include "absl/meta/type_traits.h" +#include "absl/strings/string_view.h" #include #include #include @@ -863,16 +864,38 @@ class Client { template StatusOr InsertObject(std::string const& bucket_name, std::string const& object_name, - std::string contents, + absl::string_view contents, Options&&... options) { google::cloud::internal::OptionsSpan const span( SpanOptions(std::forward(options)...)); internal::InsertObjectMediaRequest request(bucket_name, object_name, - std::move(contents)); + contents); request.set_multiple_options(std::forward(options)...); return raw_client_->InsertObjectMedia(request); } + /// @overload InsertObject(std::string const& bucket_name, std::string const& object_name, absl::string_view contents, Options&&... options) + template + StatusOr InsertObject(std::string const& bucket_name, + std::string const& object_name, + std::string const& contents, + Options&&... options) { + return InsertObject(bucket_name, object_name, absl::string_view(contents), + std::forward(options)...); + } + + /// @overload InsertObject(std::string const& bucket_name, std::string const& object_name, absl::string_view contents, Options&&... options) + template + StatusOr InsertObject(std::string const& bucket_name, + std::string const& object_name, + char const* contents, + Options&&... options) { + auto c = + contents == nullptr ? absl::string_view{} : absl::string_view{contents}; + return InsertObject(bucket_name, object_name, c, + std::forward(options)...); + } + /** * Copies an existing object. * diff --git a/google/cloud/storage/client_object_test.cc b/google/cloud/storage/client_object_test.cc index 7b447fd8eaf30..f4d9819c389ff 100644 --- a/google/cloud/storage/client_object_test.cc +++ b/google/cloud/storage/client_object_test.cc @@ -79,7 +79,7 @@ TEST_F(ObjectTest, InsertObjectMedia) { EXPECT_EQ(CurrentOptions().get(), "u-p-test"); EXPECT_EQ("test-bucket-name", request.bucket_name()); EXPECT_EQ("test-object-name", request.object_name()); - EXPECT_EQ("test object contents", request.contents()); + EXPECT_EQ("test object contents", request.payload()); return make_status_or(expected); }); @@ -358,7 +358,7 @@ TEST_F(ObjectTest, UploadFile) { EXPECT_EQ(CurrentOptions().get(), "u-p-test"); EXPECT_EQ("test-bucket-name", request.bucket_name()); EXPECT_EQ("test-object-name", request.object_name()); - EXPECT_EQ(contents, request.contents()); + EXPECT_EQ(contents, request.payload()); return make_status_or(expected); }); diff --git a/google/cloud/storage/compose_many_test.cc b/google/cloud/storage/compose_many_test.cc index 17b593964262d..5a1ca82e98c9d 100644 --- a/google/cloud/storage/compose_many_test.cc +++ b/google/cloud/storage/compose_many_test.cc @@ -73,7 +73,7 @@ TEST(ComposeMany, One) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); EXPECT_CALL(*mock, DeleteObject) @@ -112,7 +112,7 @@ TEST(ComposeMany, Three) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); EXPECT_CALL(*mock, DeleteObject) @@ -184,7 +184,7 @@ TEST(ComposeMany, ThreeLayers) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); EXPECT_CALL(*mock, DeleteObject) @@ -234,7 +234,7 @@ TEST(ComposeMany, ComposeFails) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); @@ -285,7 +285,7 @@ TEST(ComposeMany, CleanupFailsLoudly) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); @@ -324,7 +324,7 @@ TEST(ComposeMany, CleanupFailsSilently) { .WillOnce([](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ("test-bucket", request.bucket_name()); EXPECT_EQ("prefix", request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject("test-bucket", "prefix", 42)); }); diff --git a/google/cloud/storage/hashing_options.cc b/google/cloud/storage/hashing_options.cc index 92d68e740c8ab..0b447bca812cb 100644 --- a/google/cloud/storage/hashing_options.cc +++ b/google/cloud/storage/hashing_options.cc @@ -22,17 +22,25 @@ namespace cloud { namespace storage { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN -std::string ComputeMD5Hash(std::string const& payload) { +std::string ComputeMD5Hash(absl::string_view payload) { return internal::Base64Encode(internal::MD5Hash(payload)); } -std::string ComputeCrc32cChecksum(std::string const& payload) { +std::string ComputeMD5Hash(std::string const& payload) { + return ComputeMD5Hash(absl::string_view(payload)); +} + +std::string ComputeCrc32cChecksum(absl::string_view payload) { auto checksum = crc32c::Extend( 0, reinterpret_cast(payload.data()), payload.size()); std::string const hash = google::cloud::internal::EncodeBigEndian(checksum); return internal::Base64Encode(hash); } +std::string ComputeCrc32cChecksum(std::string const& payload) { + return ComputeCrc32cChecksum(absl::string_view(payload)); +} + GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage } // namespace cloud diff --git a/google/cloud/storage/hashing_options.h b/google/cloud/storage/hashing_options.h index 2c6520485a0ae..3fcac86aea5d4 100644 --- a/google/cloud/storage/hashing_options.h +++ b/google/cloud/storage/hashing_options.h @@ -17,6 +17,7 @@ #include "google/cloud/storage/internal/complex_option.h" #include "google/cloud/storage/version.h" +#include "absl/strings/string_view.h" #include namespace google { @@ -45,10 +46,19 @@ struct MD5HashValue }; /** - * Compute the MD5 Hash of a string in the format preferred by GCS. + * Compute the MD5 Hash of a buffer in the format preferred by GCS. */ +std::string ComputeMD5Hash(absl::string_view payload); + +/// @copydoc ComputeMD5Hash(absl::string_view) std::string ComputeMD5Hash(std::string const& payload); +/// @copydoc ComputeMD5Hash(absl::string_view) +inline std::string ComputeMD5Hash(char const* payload) { + return ComputeMD5Hash(payload == nullptr ? absl::string_view{} + : absl::string_view{payload}); +} + /** * Disable or enable MD5 Hashing computations. * @@ -99,10 +109,19 @@ struct Crc32cChecksumValue }; /** - * Compute the CRC32C checksum of a string in the format preferred by GCS. + * Compute the CRC32C checksum of a buffer in the format preferred by GCS. */ +std::string ComputeCrc32cChecksum(absl::string_view payload); + +/// @copydoc ComputeCrc32cChecksum(absl::string_view payload) std::string ComputeCrc32cChecksum(std::string const& payload); +/// @copydoc ComputeCrc32cChecksum(absl::string_view payload) +inline std::string ComputeCrc32cChecksum(char const* payload) { + return ComputeCrc32cChecksum(payload == nullptr ? absl::string_view{} + : absl::string_view{payload}); +} + /** * Disable CRC32C checksum computations. * diff --git a/google/cloud/storage/internal/curl_client.cc b/google/cloud/storage/internal/curl_client.cc index e9abc654c0357..d58f6b7cf59e5 100644 --- a/google/cloud/storage/internal/curl_client.cc +++ b/google/cloud/storage/internal/curl_client.cc @@ -1122,13 +1122,13 @@ StatusOr CurlClient::InsertObjectMediaMultipart( if (request.HasOption()) { metadata["md5Hash"] = request.GetOption().value(); } else if (!request.GetOption().value_or(false)) { - metadata["md5Hash"] = ComputeMD5Hash(request.contents()); + metadata["md5Hash"] = ComputeMD5Hash(request.payload()); } if (request.HasOption()) { metadata["crc32c"] = request.GetOption().value(); } else if (!request.GetOption().value_or(false)) { - metadata["crc32c"] = ComputeCrc32cChecksum(request.contents()); + metadata["crc32c"] = ComputeCrc32cChecksum(request.payload()); } std::string crlf = "\r\n"; @@ -1149,7 +1149,7 @@ StatusOr CurlClient::InsertObjectMediaMultipart( } else { writer << "content-type: application/octet-stream" << crlf; } - writer << crlf << request.contents() << crlf << marker << "--" << crlf; + writer << crlf << request.payload() << crlf << marker << "--" << crlf; // 6. Return the results as usual. auto contents = std::move(writer).str(); @@ -1179,9 +1179,9 @@ StatusOr CurlClient::InsertObjectMediaSimple( builder.AddQueryParameter("uploadType", "media"); builder.AddQueryParameter("name", request.object_name()); builder.AddHeader("Content-Length: " + - std::to_string(request.contents().size())); + std::to_string(request.payload().size())); return CheckedFromString( - std::move(builder).BuildRequest().MakeRequest(request.contents())); + std::move(builder).BuildRequest().MakeRequest(request.payload())); } } // namespace internal diff --git a/google/cloud/storage/internal/curl_request.cc b/google/cloud/storage/internal/curl_request.cc index 7cb083734168a..eac4ccb4c79d5 100644 --- a/google/cloud/storage/internal/curl_request.cc +++ b/google/cloud/storage/internal/curl_request.cc @@ -68,11 +68,11 @@ CurlRequest::~CurlRequest() { if (factory_) CurlHandle::ReturnToPool(*factory_, std::move(handle_)); } -StatusOr CurlRequest::MakeRequest(std::string const& payload) && { +StatusOr CurlRequest::MakeRequest(absl::string_view payload) && { handle_.SetOption(CURLOPT_UPLOAD, 0L); if (!payload.empty()) { - handle_.SetOption(CURLOPT_POSTFIELDSIZE, payload.length()); - handle_.SetOption(CURLOPT_POSTFIELDS, payload.c_str()); + handle_.SetOption(CURLOPT_POSTFIELDSIZE, payload.size()); + handle_.SetOption(CURLOPT_POSTFIELDS, payload.data()); } return MakeRequestImpl(); } diff --git a/google/cloud/storage/internal/curl_request.h b/google/cloud/storage/internal/curl_request.h index 2b93c04a99749..abd7b8da4b342 100644 --- a/google/cloud/storage/internal/curl_request.h +++ b/google/cloud/storage/internal/curl_request.h @@ -20,6 +20,7 @@ #include "google/cloud/storage/internal/curl_handle_factory.h" #include "google/cloud/storage/internal/http_response.h" #include "google/cloud/storage/version.h" +#include "absl/strings/string_view.h" #include namespace google { @@ -43,13 +44,22 @@ class CurlRequest { /** * Makes the prepared request. * - * This function can be called multiple times on the same request. - * * @return The response HTTP error code, the headers and an empty payload. */ - StatusOr MakeRequest(std::string const& payload) &&; + StatusOr MakeRequest(absl::string_view payload) &&; + + /// @overload MakeRequest(absl::string_view) + StatusOr MakeRequest(std::string const& payload) && { + return std::move(*this).MakeRequest(absl::string_view(payload)); + } + + /// @overload MakeRequest(absl::string_view) + StatusOr MakeRequest(char const* payload) && { + return std::move(*this).MakeRequest( + payload == nullptr ? absl::string_view{} : absl::string_view{payload}); + } - /// @copydoc MakeRequest(std::string const&) + /// @copydoc MakeRequest(absl::string_view) StatusOr MakeUploadRequest(ConstBufferSequence payload) &&; private: diff --git a/google/cloud/storage/internal/grpc_client.cc b/google/cloud/storage/internal/grpc_client.cc index 0856f70902175..dcbd883c78eae 100644 --- a/google/cloud/storage/internal/grpc_client.cc +++ b/google/cloud/storage/internal/grpc_client.cc @@ -449,7 +449,7 @@ StatusOr GrpcClient::InsertObjectMedia( using ContentType = std::remove_const_t() .content())>>; - auto splitter = SplitObjectWriteData(request.contents()); + auto splitter = SplitObjectWriteData(request.payload()); std::int64_t offset = 0; // This loop must run at least once because we need to send at least one diff --git a/google/cloud/storage/internal/grpc_object_metadata_parser.cc b/google/cloud/storage/internal/grpc_object_metadata_parser.cc index 11c03cd7a86e9..4b5f5ca64e3ae 100644 --- a/google/cloud/storage/internal/grpc_object_metadata_parser.cc +++ b/google/cloud/storage/internal/grpc_object_metadata_parser.cc @@ -68,7 +68,7 @@ StatusOr MD5ToProto(std::string const& v) { return std::string{binary->begin(), binary->end()}; } -std::string ComputeMD5Hash(std::string const& payload) { +std::string ComputeMD5Hash(absl::string_view payload) { auto b = storage::internal::MD5Hash(payload); return std::string{b.begin(), b.end()}; } diff --git a/google/cloud/storage/internal/grpc_object_metadata_parser.h b/google/cloud/storage/internal/grpc_object_metadata_parser.h index eb9f95e550ab6..0e7e11a590299 100644 --- a/google/cloud/storage/internal/grpc_object_metadata_parser.h +++ b/google/cloud/storage/internal/grpc_object_metadata_parser.h @@ -18,6 +18,7 @@ #include "google/cloud/storage/object_metadata.h" #include "google/cloud/storage/version.h" #include "google/cloud/options.h" +#include "absl/strings/string_view.h" #include namespace google { @@ -34,7 +35,7 @@ std::string Crc32cFromProto(std::uint32_t); StatusOr Crc32cToProto(std::string const&); std::string MD5FromProto(std::string const&); StatusOr MD5ToProto(std::string const&); -std::string ComputeMD5Hash(std::string const& payload); +std::string ComputeMD5Hash(absl::string_view payload); storage::ObjectMetadata FromProto(google::storage::v2::Object object, Options const& options); diff --git a/google/cloud/storage/internal/grpc_object_request_parser.cc b/google/cloud/storage/internal/grpc_object_request_parser.cc index 81c711fbf7531..252d60c2693ee 100644 --- a/google/cloud/storage/internal/grpc_object_request_parser.cc +++ b/google/cloud/storage/internal/grpc_object_request_parser.cc @@ -489,7 +489,8 @@ StatusOr ToProto( false)) { // Nothing to do, the option is disabled (mostly useful in tests). } else { - checksums.set_crc32c(crc32c::Crc32c(request.contents())); + checksums.set_crc32c( + crc32c::Crc32c(request.payload().data(), request.payload().size())); } if (request.HasOption()) { @@ -500,8 +501,7 @@ StatusOr ToProto( } else if (request.GetOption().value_or(false)) { // Nothing to do, the option is disabled. } else { - checksums.set_md5_hash( - storage_internal::ComputeMD5Hash(request.contents())); + checksums.set_md5_hash(storage_internal::ComputeMD5Hash(request.payload())); } return r; diff --git a/google/cloud/storage/internal/object_requests.cc b/google/cloud/storage/internal/object_requests.cc index fcbd865adcb17..869c650ced6d1 100644 --- a/google/cloud/storage/internal/object_requests.cc +++ b/google/cloud/storage/internal/object_requests.cc @@ -161,14 +161,32 @@ std::ostream& operator<<(std::ostream& os, GetObjectMetadataRequest const& r) { return os << "}"; } +void InsertObjectMediaRequest::set_payload(absl::string_view payload) { + payload_ = payload; + dirty_ = true; +} + +std::string const& InsertObjectMediaRequest::contents() const { + if (!dirty_) return contents_; + contents_ = std::string{payload_}; + dirty_ = false; + return contents_; +} + +void InsertObjectMediaRequest::set_contents(std::string v) { + contents_ = std::move(v); + payload_ = contents_; + dirty_ = false; +} + std::ostream& operator<<(std::ostream& os, InsertObjectMediaRequest const& r) { os << "InsertObjectMediaRequest={bucket_name=" << r.bucket_name() << ", object_name=" << r.object_name(); r.DumpOptions(os, ", "); std::size_t constexpr kMaxDumpSize = 128; + auto const payload = r.payload(); os << ", contents=" - << BinaryDataAsDebugString(r.contents().data(), r.contents().size(), - kMaxDumpSize); + << BinaryDataAsDebugString(payload.data(), payload.size(), kMaxDumpSize); return os << "}"; } diff --git a/google/cloud/storage/internal/object_requests.h b/google/cloud/storage/internal/object_requests.h index 55d54ede66669..e53e3b26ca7bd 100644 --- a/google/cloud/storage/internal/object_requests.h +++ b/google/cloud/storage/internal/object_requests.h @@ -26,6 +26,7 @@ #include "google/cloud/storage/upload_options.h" #include "google/cloud/storage/version.h" #include "google/cloud/storage/well_known_parameters.h" +#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/span.h" #include @@ -112,18 +113,32 @@ class InsertObjectMediaRequest explicit InsertObjectMediaRequest(std::string bucket_name, std::string object_name, - std::string contents) + absl::string_view payload) : GenericObjectRequest(std::move(bucket_name), std::move(object_name)), - contents_(std::move(contents)) {} + payload_(payload) {} - std::string const& contents() const { return contents_; } - InsertObjectMediaRequest& set_contents(std::string&& v) { - contents_ = std::move(v); - return *this; - } + absl::string_view payload() const { return payload_; } + void set_payload(absl::string_view payload); + + ///@{ + /** + * @name Backwards compatibility. + * + * While this class is in the internal namespace, the storage library + * requires applications to use parts of the internal namespace in mocks. + * + * These functions are only provided for backwards compatibility. The library + * no longer uses them, and mocks (if any) should migrate to payload() and + * set_payload(). + */ + [[deprecated("use payload() instead")]] std::string const& contents() const; + [[deprecated("use set_payload() instead")]] void set_contents(std::string v); + ///@} private: - std::string contents_; + absl::string_view payload_; + mutable std::string contents_; + mutable bool dirty_ = true; }; std::ostream& operator<<(std::ostream& os, InsertObjectMediaRequest const& r); diff --git a/google/cloud/storage/internal/object_requests_test.cc b/google/cloud/storage/internal/object_requests_test.cc index 13446887846c4..e357edecbcd67 100644 --- a/google/cloud/storage/internal/object_requests_test.cc +++ b/google/cloud/storage/internal/object_requests_test.cc @@ -176,11 +176,25 @@ TEST(ObjectRequestsTest, InsertObjectMedia) { TEST(ObjectRequestsTest, InsertObjectMediaUpdateContents) { InsertObjectMediaRequest request("my-bucket", "my-object", "object contents"); - EXPECT_EQ("object contents", request.contents()); - request.set_contents("new contents"); - EXPECT_EQ("new contents", request.contents()); + EXPECT_EQ("object contents", request.payload()); + request.set_payload("new contents"); + EXPECT_EQ("new contents", request.payload()); } +#include "google/cloud/internal/disable_deprecation_warnings.inc" +TEST(ObjectRequestsTest, InsertObjectBackwardsCompat) { + auto const payload = + std::string("The quick brown fox jumps over the lazy dog"); + auto const zebras = std::string("How quickly daft jumping zebras vex"); + InsertObjectMediaRequest request("my-bucket", "my-object", payload); + EXPECT_EQ(payload, request.payload()); + EXPECT_EQ(payload, request.contents()); + request.set_contents(zebras); + EXPECT_EQ(zebras, request.payload()); + EXPECT_EQ(zebras, request.contents()); +} +#include "google/cloud/internal/diagnostics_pop.inc" + TEST(ObjectRequestsTest, Copy) { CopyObjectRequest request("source-bucket", "source-object", "my-bucket", "my-object"); diff --git a/google/cloud/storage/internal/openssl_util.cc b/google/cloud/storage/internal/openssl_util.cc index 4b1d09f8fac60..3ea668d736904 100644 --- a/google/cloud/storage/internal/openssl_util.cc +++ b/google/cloud/storage/internal/openssl_util.cc @@ -170,7 +170,7 @@ StatusOr> UrlsafeBase64Decode( return Base64Decode(b64str); } -std::vector MD5Hash(std::string const& payload) { +std::vector MD5Hash(absl::string_view payload) { std::array digest; unsigned int size = 0; diff --git a/google/cloud/storage/internal/openssl_util.h b/google/cloud/storage/internal/openssl_util.h index 1297c469d25e7..67ea819bc0378 100644 --- a/google/cloud/storage/internal/openssl_util.h +++ b/google/cloud/storage/internal/openssl_util.h @@ -18,6 +18,7 @@ #include "google/cloud/storage/oauth2/credential_constants.h" #include "google/cloud/storage/version.h" #include "google/cloud/status_or.h" +#include "absl/strings/string_view.h" #include "absl/types/span.h" #include #include @@ -80,7 +81,12 @@ inline std::string UrlsafeBase64Encode(Collection const& bytes) { StatusOr> UrlsafeBase64Decode(std::string const& str); /// Compute the MD5 hash of @p payload -std::vector MD5Hash(std::string const& payload); +std::vector MD5Hash(absl::string_view payload); + +inline std::vector MD5Hash(char const* payload) { + return MD5Hash(payload == nullptr ? absl::string_view{} + : absl::string_view(payload)); +} } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/storage/internal/openssl_util_test.cc b/google/cloud/storage/internal/openssl_util_test.cc index 13426ad00ba59..9291e1af24b05 100644 --- a/google/cloud/storage/internal/openssl_util_test.cc +++ b/google/cloud/storage/internal/openssl_util_test.cc @@ -81,13 +81,23 @@ TEST(OpensslUtilTest, Base64DecodePadding) { } TEST(OpensslUtilTest, MD5HashEmpty) { - auto const actual = MD5Hash(""); // I used this command to get the expected value: // /bin/echo -n "" | openssl md5 auto const expected = std::vector{0xd4, 0x1d, 0x8c, 0xd9, 0x8f, 0x00, 0xb2, 0x04, 0xe9, 0x80, 0x09, 0x98, 0xec, 0xf8, 0x42, 0x7e}; - EXPECT_THAT(actual, ElementsAreArray(expected)); + + // There are many ways to represent the "empty" string: + std::vector> const values{ + MD5Hash({}), + MD5Hash(nullptr), + MD5Hash(""), + MD5Hash(absl::string_view{}), + }; + + for (auto const& actual : values) { + EXPECT_THAT(actual, ElementsAreArray(expected)); + } } TEST(OpensslUtilTest, MD5HashSimple) { diff --git a/google/cloud/storage/internal/rest_client.cc b/google/cloud/storage/internal/rest_client.cc index 8ad94a7032a46..83ff9efec77db 100644 --- a/google/cloud/storage/internal/rest_client.cc +++ b/google/cloud/storage/internal/rest_client.cc @@ -372,13 +372,13 @@ StatusOr RestClient::InsertObjectMediaMultipart( if (request.HasOption()) { metadata["md5Hash"] = request.GetOption().value(); } else if (!request.GetOption().value_or(false)) { - metadata["md5Hash"] = ComputeMD5Hash(request.contents()); + metadata["md5Hash"] = ComputeMD5Hash(request.payload()); } if (request.HasOption()) { metadata["crc32c"] = request.GetOption().value(); } else if (!request.GetOption().value_or(false)) { - metadata["crc32c"] = ComputeCrc32cChecksum(request.contents()); + metadata["crc32c"] = ComputeCrc32cChecksum(request.payload()); } std::string crlf = "\r\n"; @@ -407,7 +407,7 @@ StatusOr RestClient::InsertObjectMediaMultipart( // 6. Return the results as usual. return CheckedFromString(storage_rest_client_->Post( std::move(builder).BuildRequest(), - {absl::MakeConstSpan(header), absl::MakeConstSpan(request.contents()), + {absl::MakeConstSpan(header), absl::MakeConstSpan(request.payload()), absl::MakeConstSpan(trailer)})); } @@ -434,7 +434,7 @@ StatusOr RestClient::InsertObjectMediaSimple( builder.AddQueryParameter("name", request.object_name()); return CheckedFromString( storage_rest_client_->Post(std::move(builder).BuildRequest(), - {absl::MakeConstSpan(request.contents())})); + {absl::MakeConstSpan(request.payload())})); } StatusOr RestClient::InsertObjectMedia( diff --git a/google/cloud/storage/parallel_uploads_test.cc b/google/cloud/storage/parallel_uploads_test.cc index 737a054dfde39..38f87440ef617 100644 --- a/google/cloud/storage/parallel_uploads_test.cc +++ b/google/cloud/storage/parallel_uploads_test.cc @@ -329,7 +329,7 @@ auto expect_new_object = [](std::string const& object_name, int generation) { generation](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ(kBucketName, request.bucket_name()); EXPECT_EQ(object_name, request.object_name()); - EXPECT_EQ("", request.contents()); + EXPECT_EQ("", request.payload()); return make_status_or(MockObject(object_name, generation)); }; }; @@ -340,7 +340,7 @@ auto expect_persistent_state = [](std::string const& state_name, int generation, state](internal::InsertObjectMediaRequest const& request) { EXPECT_EQ(kBucketName, request.bucket_name()); EXPECT_EQ(state_name, request.object_name()); - EXPECT_EQ(state.dump(), request.contents()); + EXPECT_EQ(state.dump(), request.payload()); return make_status_or(MockObject(state_name, generation)); }; }; diff --git a/google/cloud/storage/tests/object_media_integration_test.cc b/google/cloud/storage/tests/object_media_integration_test.cc index 2ba440b258b00..f6f2aeb32d76f 100644 --- a/google/cloud/storage/tests/object_media_integration_test.cc +++ b/google/cloud/storage/tests/object_media_integration_test.cc @@ -604,6 +604,70 @@ TEST_F(ObjectMediaIntegrationTest, StreamingReadInternalError) { } } +TEST_F(ObjectMediaIntegrationTest, StringView) { + StatusOr client = MakeIntegrationTestClient(); + ASSERT_STATUS_OK(client); + + auto const object_name = MakeRandomObjectName(); + auto const contents = LoremIpsum(); + + // Create the object, but only if it does not exist already. + StatusOr meta = + client->InsertObject(bucket_name_, object_name, + absl::string_view(contents), IfGenerationMatch(0)); + ASSERT_STATUS_OK(meta); + ScheduleForDelete(*meta); + EXPECT_EQ(object_name, meta->name()); + EXPECT_EQ(bucket_name_, meta->bucket()); + + // Create an iostream to read the object back. + auto stream = client->ReadObject(bucket_name_, object_name); + std::string actual(std::istreambuf_iterator{stream}, {}); + EXPECT_EQ(contents, actual); +} + +TEST_F(ObjectMediaIntegrationTest, String) { + StatusOr client = MakeIntegrationTestClient(); + ASSERT_STATUS_OK(client); + + auto const object_name = MakeRandomObjectName(); + std::string const contents = LoremIpsum(); + + // Create the object, but only if it does not exist already. + StatusOr meta = client->InsertObject( + bucket_name_, object_name, contents, IfGenerationMatch(0)); + ASSERT_STATUS_OK(meta); + ScheduleForDelete(*meta); + EXPECT_EQ(object_name, meta->name()); + EXPECT_EQ(bucket_name_, meta->bucket()); + + // Create an iostream to read the object back. + auto stream = client->ReadObject(bucket_name_, object_name); + std::string actual(std::istreambuf_iterator{stream}, {}); + EXPECT_EQ(contents, actual); +} + +TEST_F(ObjectMediaIntegrationTest, CharConstPointer) { + StatusOr client = MakeIntegrationTestClient(); + ASSERT_STATUS_OK(client); + + auto const object_name = MakeRandomObjectName(); + std::string const contents = LoremIpsum(); + + // Create the object, but only if it does not exist already. + StatusOr meta = client->InsertObject( + bucket_name_, object_name, contents.c_str(), IfGenerationMatch(0)); + ASSERT_STATUS_OK(meta); + ScheduleForDelete(*meta); + EXPECT_EQ(object_name, meta->name()); + EXPECT_EQ(bucket_name_, meta->bucket()); + + // Create an iostream to read the object back. + auto stream = client->ReadObject(bucket_name_, object_name); + std::string actual(std::istreambuf_iterator{stream}, {}); + EXPECT_EQ(contents, actual); +} + } // anonymous namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage