From dc9c474927047d9e932634226def8e6cdc4549fa Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Tue, 1 Jul 2025 13:58:07 +0000
Subject: [PATCH 01/10] create otel tracing decorator for upload_file

---
 .../opentelemetry/quickstart/quickstart.cc    | 66 -------------
 google/cloud/storage/client.cc                | 97 ------------------
 google/cloud/storage/client.h                 | 19 ++--
 .../cloud/storage/internal/connection_impl.cc | 99 +++++++++++++++++++
 .../cloud/storage/internal/connection_impl.h  |  6 ++
 .../storage/internal/storage_connection.h     |  6 ++
 .../storage/internal/tracing_connection.cc    | 20 ++++
 .../storage/internal/tracing_connection.h     |  7 ++
 8 files changed, 147 insertions(+), 173 deletions(-)
 delete mode 100644 google/cloud/opentelemetry/quickstart/quickstart.cc

diff --git a/google/cloud/opentelemetry/quickstart/quickstart.cc b/google/cloud/opentelemetry/quickstart/quickstart.cc
deleted file mode 100644
index 369cd8d2494ce..0000000000000
--- a/google/cloud/opentelemetry/quickstart/quickstart.cc
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2023 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// https://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! [all] [START storage_enable_otel_tracing]
-#include "google/cloud/opentelemetry/configure_basic_tracing.h"
-#include "google/cloud/storage/client.h"
-#include "google/cloud/opentelemetry_options.h"
-#include <iostream>
-
-int main(int argc, char* argv[]) {
-  if (argc != 3) {
-    std::cerr << "Usage: " << argv[0] << " <bucket-name> <project-id>\n";
-    return 1;
-  }
-  std::string const bucket_name = argv[1];
-  std::string const project_id = argv[2];
-
-  // Create aliases to make the code easier to read.
-  namespace gc = ::google::cloud;
-  namespace gcs = ::google::cloud::storage;
-
-  // Instantiate a basic tracing configuration which exports traces to Cloud
-  // Trace. By default, spans are sent in batches and always sampled.
-  auto project = gc::Project(project_id);
-  auto configuration = gc::otel::ConfigureBasicTracing(project);
-
-  // Create a client with OpenTelemetry tracing enabled.
-  auto options = gc::Options{}.set<gc::OpenTelemetryTracingOption>(true);
-  auto client = gcs::Client(options);
-
-  auto writer = client.WriteObject(bucket_name, "quickstart.txt");
-  writer << "Hello World!";
-  writer.Close();
-  if (!writer.metadata()) {
-    std::cerr << "Error creating object: " << writer.metadata().status()
-              << "\n";
-    return 1;
-  }
-  std::cout << "Successfully created object: " << *writer.metadata() << "\n";
-
-  auto reader = client.ReadObject(bucket_name, "quickstart.txt");
-  if (!reader) {
-    std::cerr << "Error reading object: " << reader.status() << "\n";
-    return 1;
-  }
-
-  std::string contents{std::istreambuf_iterator<char>{reader}, {}};
-  std::cout << contents << "\n";
-
-  // The basic tracing configuration object goes out of scope. The collected
-  // spans are flushed to Cloud Trace.
-
-  return 0;
-}
-//! [all] [END storage_enable_otel_tracing]
diff --git a/google/cloud/storage/client.cc b/google/cloud/storage/client.cc
index b1b7bd4e4158a..325a9d8cc71a8 100644
--- a/google/cloud/storage/client.cc
+++ b/google/cloud/storage/client.cc
@@ -127,103 +127,6 @@ bool Client::UseSimpleUpload(std::string const& file_name, std::size_t& size) {
   return false;
 }
 
-StatusOr<ObjectMetadata> Client::UploadFileSimple(
-    std::string const& file_name, std::size_t file_size,
-    internal::InsertObjectMediaRequest request) {
-  auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
-  if (file_size < upload_offset) {
-    std::ostringstream os;
-    os << __func__ << "(" << request << ", " << file_name
-       << "): UploadFromOffset (" << upload_offset
-       << ") is bigger than the size of file source (" << file_size << ")";
-    return google::cloud::internal::InvalidArgumentError(std::move(os).str(),
-                                                         GCP_ERROR_INFO());
-  }
-  auto upload_size = (std::min)(
-      request.GetOption<UploadLimit>().value_or(file_size - upload_offset),
-      file_size - upload_offset);
-
-  std::ifstream is(file_name, std::ios::binary);
-  if (!is.is_open()) {
-    std::ostringstream os;
-    os << __func__ << "(" << request << ", " << file_name
-       << "): cannot open upload file source";
-    return google::cloud::internal::NotFoundError(std::move(os).str(),
-                                                  GCP_ERROR_INFO());
-  }
-
-  std::string payload(static_cast<std::size_t>(upload_size), char{});
-  is.seekg(upload_offset, std::ios::beg);
-  // We need to use `&payload[0]` until C++17
-  // NOLINTNEXTLINE(readability-container-data-pointer)
-  is.read(&payload[0], payload.size());
-  if (static_cast<std::size_t>(is.gcount()) < payload.size()) {
-    std::ostringstream os;
-    os << __func__ << "(" << request << ", " << file_name << "): Actual read ("
-       << is.gcount() << ") is smaller than upload_size (" << payload.size()
-       << ")";
-    return google::cloud::internal::InternalError(std::move(os).str(),
-                                                  GCP_ERROR_INFO());
-  }
-  is.close();
-  request.set_payload(payload);
-
-  return connection_->InsertObjectMedia(request);
-}
-
-StatusOr<ObjectMetadata> Client::UploadFileResumable(
-    std::string const& file_name,
-    google::cloud::storage::internal::ResumableUploadRequest request) {
-  auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
-  auto status = google::cloud::internal::status(file_name);
-  if (!is_regular(status)) {
-    GCP_LOG(WARNING) << "Trying to upload " << file_name
-                     << R"""( which is not a regular file.
-This is often a problem because:
-  - Some non-regular files are infinite sources of data, and the load will
-    never complete.
-  - Some non-regular files can only be read once, and UploadFile() may need to
-    read the file more than once to compute the checksum and hashes needed to
-    preserve data integrity.
-
-Consider using UploadLimit option or Client::WriteObject(). You may also need to disable data
-integrity checks using the DisableMD5Hash() and DisableCrc32cChecksum() options.
-)""";
-  } else {
-    std::error_code size_err;
-    auto file_size = google::cloud::internal::file_size(file_name, size_err);
-    if (size_err) {
-      return google::cloud::internal::NotFoundError(size_err.message(),
-                                                    GCP_ERROR_INFO());
-    }
-    if (file_size < upload_offset) {
-      std::ostringstream os;
-      os << __func__ << "(" << request << ", " << file_name
-         << "): UploadFromOffset (" << upload_offset
-         << ") is bigger than the size of file source (" << file_size << ")";
-      return google::cloud::internal::InvalidArgumentError(std::move(os).str(),
-                                                           GCP_ERROR_INFO());
-    }
-
-    auto upload_size = (std::min)(
-        request.GetOption<UploadLimit>().value_or(file_size - upload_offset),
-        file_size - upload_offset);
-    request.set_option(UploadContentLength(upload_size));
-  }
-  std::ifstream source(file_name, std::ios::binary);
-  if (!source.is_open()) {
-    std::ostringstream os;
-    os << __func__ << "(" << request << ", " << file_name
-       << "): cannot open upload file source";
-    return google::cloud::internal::NotFoundError(std::move(os).str(),
-                                                  GCP_ERROR_INFO());
-  }
-  // We set its offset before passing it to `UploadStreamResumable` so we don't
-  // need to compute `UploadFromOffset` again.
-  source.seekg(upload_offset, std::ios::beg);
-  return UploadStreamResumable(source, request);
-}
-
 StatusOr<ObjectMetadata> Client::UploadStreamResumable(
     std::istream& source,
     internal::ResumableUploadRequest const& request) const {
diff --git a/google/cloud/storage/client.h b/google/cloud/storage/client.h
index d0b01d03da5b1..065fa57b20fd9 100644
--- a/google/cloud/storage/client.h
+++ b/google/cloud/storage/client.h
@@ -3511,7 +3511,9 @@ class Client {
         SpanOptions(std::forward<Options>(options)...));
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return UploadFileResumable(file_name, std::move(request));
+    auto source = connection_->UploadFileResumable(file_name, request);
+    if (!source) return source.status();
+    return UploadStreamResumable(*source.value(), request);
   }
 
   // The version of UploadFile() where UseResumableUploadSession is *not* one of
@@ -3530,22 +3532,19 @@ class Client {
       internal::InsertObjectMediaRequest request(bucket_name, object_name,
                                                  std::string{});
       request.set_multiple_options(std::forward<Options>(options)...);
-      return UploadFileSimple(file_name, file_size, request);
+      auto status = connection_->UploadFileSimple(file_name, file_size, request);
+      if (!status) return status.status();
+      return connection_->InsertObjectMedia(request);
     }
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return UploadFileResumable(file_name, std::move(request));
+    auto source = connection_->UploadFileResumable(file_name, request);
+    if (!source) return source.status();
+    return UploadStreamResumable(*source.value(), request);
   }
 
   static bool UseSimpleUpload(std::string const& file_name, std::size_t& size);
 
-  StatusOr<ObjectMetadata> UploadFileSimple(
-      std::string const& file_name, std::size_t file_size,
-      internal::InsertObjectMediaRequest request);
-
-  StatusOr<ObjectMetadata> UploadFileResumable(
-      std::string const& file_name, internal::ResumableUploadRequest request);
-
   StatusOr<ObjectMetadata> UploadStreamResumable(
       std::istream& source,
       internal::ResumableUploadRequest const& request) const;
diff --git a/google/cloud/storage/internal/connection_impl.cc b/google/cloud/storage/internal/connection_impl.cc
index 8be3ac7c8d17b..dedcd449f4c8b 100644
--- a/google/cloud/storage/internal/connection_impl.cc
+++ b/google/cloud/storage/internal/connection_impl.cc
@@ -14,10 +14,13 @@
 
 #include "google/cloud/storage/internal/connection_impl.h"
 #include "google/cloud/storage/internal/retry_object_read_source.h"
+#include "google/cloud/internal/filesystem.h"
 #include "google/cloud/internal/opentelemetry.h"
 #include "google/cloud/internal/rest_retry_loop.h"
+#include "google/cloud/log.h"
 #include "absl/strings/match.h"
 #include <chrono>
+#include <fstream>
 #include <functional>
 #include <memory>
 #include <sstream>
@@ -754,6 +757,102 @@ StatusOr<QueryResumableUploadResponse> StorageConnectionImpl::UploadChunk(
   return RetryError(last_status, *retry_policy, __func__);
 }
 
+StatusOr<EmptyResponse> StorageConnectionImpl::UploadFileSimple(
+    std::string const& file_name, std::size_t file_size,
+    InsertObjectMediaRequest& request) {
+  auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
+  if (file_size < upload_offset) {
+    std::ostringstream os;
+    os << __func__ << "(" << request << ", " << file_name
+       << "): UploadFromOffset (" << upload_offset
+       << ") is bigger than the size of file source (" << file_size << ")";
+    return google::cloud::internal::InvalidArgumentError(std::move(os).str(),
+                                                         GCP_ERROR_INFO());
+  }
+  auto upload_size = (std::min)(request.GetOption<UploadLimit>().value_or(
+                                    file_size - upload_offset),
+                                file_size - upload_offset);
+
+  std::ifstream is(file_name, std::ios::binary);
+  if (!is.is_open()) {
+    std::ostringstream os;
+    os << __func__ << "(" << request << ", " << file_name
+       << "): cannot open upload file source";
+    return google::cloud::internal::NotFoundError(std::move(os).str(),
+                                                  GCP_ERROR_INFO());
+  }
+
+  std::string payload(static_cast<std::size_t>(upload_size), char{});
+  is.seekg(upload_offset, std::ios::beg);
+  // We need to use `&payload[0]` until C++17
+  // NOLINTNEXTLINE(readability-container-data-pointer)
+  is.read(&payload[0], payload.size());
+  if (static_cast<std::size_t>(is.gcount()) < payload.size()) {
+    std::ostringstream os;
+    os << __func__ << "(" << request << ", " << file_name << "): Actual read ("
+       << is.gcount() << ") is smaller than upload_size (" << payload.size()
+       << ")";
+    return google::cloud::internal::InternalError(std::move(os).str(),
+                                                  GCP_ERROR_INFO());
+  }
+  is.close();
+  request.set_payload(payload);
+
+  return EmptyResponse();
+}
+
+StatusOr<std::unique_ptr<std::istream>> StorageConnectionImpl::UploadFileResumable(
+    std::string const& file_name, ResumableUploadRequest& request) {
+  auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
+  auto status = google::cloud::internal::status(file_name);
+  if (!is_regular(status)) {
+    GCP_LOG(WARNING) << "Trying to upload " << file_name
+                     << R"""( which is not a regular file.
+This is often a problem because:
+  - Some non-regular files are infinite sources of data, and the load will
+    never complete.
+  - Some non-regular files can only be read once, and UploadFile() may need to
+    read the file more than once to compute the checksum and hashes needed to
+    preserve data integrity.
+
+Consider using UploadLimit option or Client::WriteObject(). You may also need to disable data
+integrity checks using the DisableMD5Hash() and DisableCrc32cChecksum() options.
+)""";
+  } else {
+    std::error_code size_err;
+    auto file_size = google::cloud::internal::file_size(file_name, size_err);
+    if (size_err) {
+      return google::cloud::internal::NotFoundError(size_err.message(),
+                                                    GCP_ERROR_INFO());
+    }
+    if (file_size < upload_offset) {
+      std::ostringstream os;
+      os << __func__ << "(" << request << ", " << file_name
+         << "): UploadFromOffset (" << upload_offset
+         << ") is bigger than the size of file source (" << file_size << ")";
+      return google::cloud::internal::InvalidArgumentError(std::move(os).str(),
+                                                           GCP_ERROR_INFO());
+    }
+
+    auto upload_size = (std::min)(request.GetOption<UploadLimit>().value_or(
+                                      file_size - upload_offset),
+                                  file_size - upload_offset);
+    request.set_option(UploadContentLength(upload_size));
+  }
+  auto source = std::make_unique<std::ifstream>(file_name, std::ios::binary);
+  if (!source->is_open()) {
+    std::ostringstream os;
+    os << __func__ << "(" << request << ", " << file_name
+       << "): cannot open upload file source";
+    return google::cloud::internal::NotFoundError(std::move(os).str(),
+                                                  GCP_ERROR_INFO());
+  }
+  // We set its offset before passing it to `UploadStreamResumable` so we don't
+  // need to compute `UploadFromOffset` again.
+  source->seekg(upload_offset, std::ios::beg);
+  return std::unique_ptr<std::istream>(std::move(source));
+}
+
 StatusOr<ListBucketAclResponse> StorageConnectionImpl::ListBucketAcl(
     ListBucketAclRequest const& request) {
   auto const idempotency = current_idempotency_policy().IsIdempotent(request)
diff --git a/google/cloud/storage/internal/connection_impl.h b/google/cloud/storage/internal/connection_impl.h
index ab510c5db8ed2..b65751c2c9117 100644
--- a/google/cloud/storage/internal/connection_impl.h
+++ b/google/cloud/storage/internal/connection_impl.h
@@ -99,6 +99,12 @@ class StorageConnectionImpl
       DeleteResumableUploadRequest const& request) override;
   StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) override;
+  StatusOr<EmptyResponse> UploadFileSimple(
+      std::string const& file_name, std::size_t file_size,
+      InsertObjectMediaRequest& request) override;
+  StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
+      std::string const& file_name,
+      ResumableUploadRequest& request) override;
 
   StatusOr<ListBucketAclResponse> ListBucketAcl(
       ListBucketAclRequest const& request) override;
diff --git a/google/cloud/storage/internal/storage_connection.h b/google/cloud/storage/internal/storage_connection.h
index 72e121bbb08d5..5f713fa72833c 100644
--- a/google/cloud/storage/internal/storage_connection.h
+++ b/google/cloud/storage/internal/storage_connection.h
@@ -110,6 +110,12 @@ class StorageConnection {
       DeleteResumableUploadRequest const& request) = 0;
   virtual StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) = 0;
+  virtual StatusOr<EmptyResponse> UploadFileSimple(
+      std::string const& file_name, std::size_t file_size,
+      InsertObjectMediaRequest& request) = 0;
+  virtual StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
+      std::string const& file_name,
+      ResumableUploadRequest& request) = 0;
   ///@}
 
   ///@{
diff --git a/google/cloud/storage/internal/tracing_connection.cc b/google/cloud/storage/internal/tracing_connection.cc
index 8e5b1fafd8479..e358f636f0e95 100644
--- a/google/cloud/storage/internal/tracing_connection.cc
+++ b/google/cloud/storage/internal/tracing_connection.cc
@@ -235,6 +235,26 @@ TracingConnection::UploadChunk(
   return internal::EndSpan(*span, impl_->UploadChunk(request));
 }
 
+StatusOr<storage::internal::EmptyResponse> TracingConnection::UploadFileSimple(
+    std::string const& file_name, std::size_t file_size,
+    storage::internal::InsertObjectMediaRequest& request) {
+  auto span =
+      internal::MakeSpan("storage::Client::UploadFile/UploadFileSimple");
+  auto scope = opentelemetry::trace::Scope(span);
+  return internal::EndSpan(
+      *span, impl_->UploadFileSimple(file_name, file_size, request));
+}
+
+StatusOr<std::unique_ptr<std::istream>> TracingConnection::UploadFileResumable(
+    std::string const& file_name,
+    storage::internal::ResumableUploadRequest& request) {
+  auto span =
+      internal::MakeSpan("storage::Client::UploadFile/UploadFileResumable");
+  auto scope = opentelemetry::trace::Scope(span);
+  return internal::EndSpan(*span,
+                           impl_->UploadFileResumable(file_name, request));
+}
+
 StatusOr<storage::internal::ListBucketAclResponse>
 TracingConnection::ListBucketAcl(
     storage::internal::ListBucketAclRequest const& request) {
diff --git a/google/cloud/storage/internal/tracing_connection.h b/google/cloud/storage/internal/tracing_connection.h
index 4778aa1cf4dc3..6f4aaddc1f6e3 100644
--- a/google/cloud/storage/internal/tracing_connection.h
+++ b/google/cloud/storage/internal/tracing_connection.h
@@ -98,6 +98,13 @@ class TracingConnection : public storage::internal::StorageConnection {
       storage::internal::DeleteResumableUploadRequest const& request) override;
   StatusOr<storage::internal::QueryResumableUploadResponse> UploadChunk(
       storage::internal::UploadChunkRequest const& request) override;
+  StatusOr<storage::internal::EmptyResponse> UploadFileSimple(
+      std::string const& file_name,
+      std::size_t file_size,
+      storage::internal::InsertObjectMediaRequest& request) override;
+  StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
+      std::string const& file_name,
+      storage::internal::ResumableUploadRequest& request) override;
 
   StatusOr<storage::internal::ListBucketAclResponse> ListBucketAcl(
       storage::internal::ListBucketAclRequest const& request) override;

From 8ea291202fbf8075a6d446838d53da43896d0d5b Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Tue, 1 Jul 2025 14:06:56 +0000
Subject: [PATCH 02/10] don't delete otel quickstart

---
 .../opentelemetry/quickstart/quickstart.cc    | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 google/cloud/opentelemetry/quickstart/quickstart.cc

diff --git a/google/cloud/opentelemetry/quickstart/quickstart.cc b/google/cloud/opentelemetry/quickstart/quickstart.cc
new file mode 100644
index 0000000000000..ca8f46a933ab3
--- /dev/null
+++ b/google/cloud/opentelemetry/quickstart/quickstart.cc
@@ -0,0 +1,66 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! [all] [START storage_enable_otel_tracing]
+#include "google/cloud/opentelemetry/configure_basic_tracing.h"
+#include "google/cloud/storage/client.h"
+#include "google/cloud/opentelemetry_options.h"
+#include <iostream>
+
+int main(int argc, char* argv[]) {
+  if (argc != 3) {
+    std::cerr << "Usage: " << argv[0] << " <bucket-name> <project-id>\n";
+    return 1;
+  }
+  std::string const bucket_name = argv[1];
+  std::string const project_id = argv[2];
+
+  // Create aliases to make the code easier to read.
+  namespace gc = ::google::cloud;
+  namespace gcs = ::google::cloud::storage;
+
+  // Instantiate a basic tracing configuration which exports traces to Cloud
+  // Trace. By default, spans are sent in batches and always sampled.
+  auto project = gc::Project(project_id);
+  auto configuration = gc::otel::ConfigureBasicTracing(project);
+
+  // Create a client with OpenTelemetry tracing enabled.
+  auto options = gc::Options{}.set<gc::OpenTelemetryTracingOption>(true);
+  auto client = gcs::Client(options);
+
+  auto writer = client.WriteObject(bucket_name, "quickstart.txt");
+  writer << "Hello World!";
+  writer.Close();
+  if (!writer.metadata()) {
+    std::cerr << "Error creating object: " << writer.metadata().status()
+              << "\n";
+    return 1;
+  }
+  std::cout << "Successfully created object: " << *writer.metadata() << "\n";
+
+  auto reader = client.ReadObject(bucket_name, "quickstart.txt");
+  if (!reader) {
+    std::cerr << "Error reading object: " << reader.status() << "\n";
+    return 1;
+  }
+
+  std::string contents{std::istreambuf_iterator<char>{reader}, {}};
+  std::cout << contents << "\n";
+
+  // The basic tracing configuration object goes out of scope. The collected
+  // spans are flushed to Cloud Trace.
+
+  return 0;
+}
+//! [all] [END storage_enable_otel_tracing]
\ No newline at end of file

From 8ac9d840e72f28eefc9333cff5f18c52ed374535 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Wed, 9 Jul 2025 11:32:22 +0000
Subject: [PATCH 03/10] resolving comments

---
 .../opentelemetry/quickstart/quickstart.cc    |  2 +-
 google/cloud/storage/client.cc                | 15 +++++++++++++++
 google/cloud/storage/client.h                 | 19 ++++++++++---------
 .../cloud/storage/internal/connection_impl.cc |  5 +++--
 .../storage/internal/storage_connection.h     |  8 ++++++--
 5 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/google/cloud/opentelemetry/quickstart/quickstart.cc b/google/cloud/opentelemetry/quickstart/quickstart.cc
index ca8f46a933ab3..369cd8d2494ce 100644
--- a/google/cloud/opentelemetry/quickstart/quickstart.cc
+++ b/google/cloud/opentelemetry/quickstart/quickstart.cc
@@ -63,4 +63,4 @@ int main(int argc, char* argv[]) {
 
   return 0;
 }
-//! [all] [END storage_enable_otel_tracing]
\ No newline at end of file
+//! [all] [END storage_enable_otel_tracing]
diff --git a/google/cloud/storage/client.cc b/google/cloud/storage/client.cc
index 325a9d8cc71a8..90cbc89baa824 100644
--- a/google/cloud/storage/client.cc
+++ b/google/cloud/storage/client.cc
@@ -127,6 +127,21 @@ bool Client::UseSimpleUpload(std::string const& file_name, std::size_t& size) {
   return false;
 }
 
+StatusOr<ObjectMetadata> Client::UploadFileSimple(
+    std::string const& file_name, std::size_t file_size,
+    internal::InsertObjectMediaRequest& request) {
+  auto status = connection_->UploadFileSimple(file_name, file_size, request);
+  if (!status) return status.status();
+  return connection_->InsertObjectMedia(request);
+}
+
+StatusOr<ObjectMetadata> Client::UploadFileResumable(
+    std::string const& file_name, internal::ResumableUploadRequest& request) {
+  auto source = connection_->UploadFileResumable(file_name, request);
+  if (!source) return source.status();
+  return UploadStreamResumable(*source.value(), request);
+}
+
 StatusOr<ObjectMetadata> Client::UploadStreamResumable(
     std::istream& source,
     internal::ResumableUploadRequest const& request) const {
diff --git a/google/cloud/storage/client.h b/google/cloud/storage/client.h
index 065fa57b20fd9..870408a8aa87f 100644
--- a/google/cloud/storage/client.h
+++ b/google/cloud/storage/client.h
@@ -3511,9 +3511,7 @@ class Client {
         SpanOptions(std::forward<Options>(options)...));
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    auto source = connection_->UploadFileResumable(file_name, request);
-    if (!source) return source.status();
-    return UploadStreamResumable(*source.value(), request);
+    return UploadFileResumable(file_name, request);
   }
 
   // The version of UploadFile() where UseResumableUploadSession is *not* one of
@@ -3532,19 +3530,22 @@ class Client {
       internal::InsertObjectMediaRequest request(bucket_name, object_name,
                                                  std::string{});
       request.set_multiple_options(std::forward<Options>(options)...);
-      auto status = connection_->UploadFileSimple(file_name, file_size, request);
-      if (!status) return status.status();
-      return connection_->InsertObjectMedia(request);
+      return UploadFileSimple(file_name, file_size, request);
     }
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    auto source = connection_->UploadFileResumable(file_name, request);
-    if (!source) return source.status();
-    return UploadStreamResumable(*source.value(), request);
+    return UploadFileResumable(file_name, request);
   }
 
   static bool UseSimpleUpload(std::string const& file_name, std::size_t& size);
 
+  StatusOr<ObjectMetadata> UploadFileSimple(
+      std::string const& file_name, std::size_t file_size,
+      internal::InsertObjectMediaRequest request);
+
+  StatusOr<ObjectMetadata> UploadFileResumable(
+      std::string const& file_name, internal::ResumableUploadRequest request);
+
   StatusOr<ObjectMetadata> UploadStreamResumable(
       std::istream& source,
       internal::ResumableUploadRequest const& request) const;
diff --git a/google/cloud/storage/internal/connection_impl.cc b/google/cloud/storage/internal/connection_impl.cc
index dedcd449f4c8b..f414be48c5394 100644
--- a/google/cloud/storage/internal/connection_impl.cc
+++ b/google/cloud/storage/internal/connection_impl.cc
@@ -801,8 +801,9 @@ StatusOr<EmptyResponse> StorageConnectionImpl::UploadFileSimple(
   return EmptyResponse();
 }
 
-StatusOr<std::unique_ptr<std::istream>> StorageConnectionImpl::UploadFileResumable(
-    std::string const& file_name, ResumableUploadRequest& request) {
+StatusOr<std::unique_ptr<std::istream>>
+StorageConnectionImpl::UploadFileResumable(std::string const& file_name,
+                                           ResumableUploadRequest& request) {
   auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
   auto status = google::cloud::internal::status(file_name);
   if (!is_regular(status)) {
diff --git a/google/cloud/storage/internal/storage_connection.h b/google/cloud/storage/internal/storage_connection.h
index 5f713fa72833c..038909e3e6973 100644
--- a/google/cloud/storage/internal/storage_connection.h
+++ b/google/cloud/storage/internal/storage_connection.h
@@ -112,10 +112,14 @@ class StorageConnection {
       UploadChunkRequest const& request) = 0;
   virtual StatusOr<EmptyResponse> UploadFileSimple(
       std::string const& file_name, std::size_t file_size,
-      InsertObjectMediaRequest& request) = 0;
+      InsertObjectMediaRequest& request) {
+    return Status(StatusCode::kUnimplemented, "unimplemented");
+  }
   virtual StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
       std::string const& file_name,
-      ResumableUploadRequest& request) = 0;
+      ResumableUploadRequest& request) {
+    return Status(StatusCode::kUnimplemented, "unimplemented");
+  }
   ///@}
 
   ///@{

From d67d9cc42e9a37305e0683e0f92e4d72550a8235 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 11:18:00 +0000
Subject: [PATCH 04/10] adding tests

---
 google/cloud/storage/client.cc                |  9 ++--
 google/cloud/storage/client.h                 |  6 +--
 google/cloud/storage/client_object_test.cc    | 46 +++++++++++++++++
 .../cloud/storage/internal/connection_impl.cc | 14 +++---
 .../cloud/storage/internal/connection_impl.h  |  2 +-
 .../storage/internal/storage_connection.h     |  2 +-
 .../storage/internal/tracing_connection.cc    |  2 +-
 .../storage/internal/tracing_connection.h     |  2 +-
 .../internal/tracing_connection_test.cc       | 50 +++++++++++++++++++
 google/cloud/storage/testing/mock_client.h    |  8 +++
 10 files changed, 123 insertions(+), 18 deletions(-)

diff --git a/google/cloud/storage/client.cc b/google/cloud/storage/client.cc
index 90cbc89baa824..20054ee9097ec 100644
--- a/google/cloud/storage/client.cc
+++ b/google/cloud/storage/client.cc
@@ -129,14 +129,15 @@ bool Client::UseSimpleUpload(std::string const& file_name, std::size_t& size) {
 
 StatusOr<ObjectMetadata> Client::UploadFileSimple(
     std::string const& file_name, std::size_t file_size,
-    internal::InsertObjectMediaRequest& request) {
-  auto status = connection_->UploadFileSimple(file_name, file_size, request);
-  if (!status) return status.status();
+    internal::InsertObjectMediaRequest request) {
+  auto payload = connection_->UploadFileSimple(file_name, file_size, request);
+  if (!payload) return payload.status();
+  request.set_payload(*payload.value());
   return connection_->InsertObjectMedia(request);
 }
 
 StatusOr<ObjectMetadata> Client::UploadFileResumable(
-    std::string const& file_name, internal::ResumableUploadRequest& request) {
+    std::string const& file_name, internal::ResumableUploadRequest request) {
   auto source = connection_->UploadFileResumable(file_name, request);
   if (!source) return source.status();
   return UploadStreamResumable(*source.value(), request);
diff --git a/google/cloud/storage/client.h b/google/cloud/storage/client.h
index 870408a8aa87f..6cd946b3d7615 100644
--- a/google/cloud/storage/client.h
+++ b/google/cloud/storage/client.h
@@ -3511,7 +3511,7 @@ class Client {
         SpanOptions(std::forward<Options>(options)...));
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return UploadFileResumable(file_name, request);
+    return UploadFileResumable(file_name, std::move(request));
   }
 
   // The version of UploadFile() where UseResumableUploadSession is *not* one of
@@ -3530,11 +3530,11 @@ class Client {
       internal::InsertObjectMediaRequest request(bucket_name, object_name,
                                                  std::string{});
       request.set_multiple_options(std::forward<Options>(options)...);
-      return UploadFileSimple(file_name, file_size, request);
+      return UploadFileSimple(file_name, file_size, std::move(request));
     }
     internal::ResumableUploadRequest request(bucket_name, object_name);
     request.set_multiple_options(std::forward<Options>(options)...);
-    return UploadFileResumable(file_name, request);
+    return UploadFileResumable(file_name, std::move(request));
   }
 
   static bool UseSimpleUpload(std::string const& file_name, std::size_t& size);
diff --git a/google/cloud/storage/client_object_test.cc b/google/cloud/storage/client_object_test.cc
index 8b6591adb03fc..53b814b7540f8 100644
--- a/google/cloud/storage/client_object_test.cc
+++ b/google/cloud/storage/client_object_test.cc
@@ -254,6 +254,52 @@ TEST_F(ObjectTest, UploadFile) {
   EXPECT_EQ(expected, *actual);
 }
 
+TEST_F(ObjectTest, UploadFileSimple) {
+  std::string text = R"""({
+      "name": "test-bucket-name/test-object-name/1"
+})""";
+  auto expected =
+      storage::internal::ObjectMetadataParser::FromString(text).value();
+  auto const contents = std::string{"some simple contents"};
+
+  EXPECT_CALL(*mock_, InsertObjectMedia)
+      .WillOnce([&](internal::InsertObjectMediaRequest const& request) {
+        EXPECT_EQ("test-bucket-name", request.bucket_name());
+        EXPECT_EQ("test-object-name", request.object_name());
+        EXPECT_EQ(contents, request.payload());
+        return make_status_or(expected);
+      });
+
+  TempFile temp(contents);
+  auto client = ClientForMock();
+  auto actual =
+      client.UploadFile(temp.name(), "test-bucket-name", "test-object-name");
+  ASSERT_STATUS_OK(actual);
+  EXPECT_EQ(expected, *actual);
+}
+
+TEST_F(ObjectTest, UploadFileResumable) {
+  std::string text = R"""({
+      "name": "test-bucket-name/test-object-name/1"
+})""";
+  auto expected =
+      storage::internal::ObjectMetadataParser::FromString(text).value();
+  auto const contents = std::string{"some not so simple contents"};
+
+  EXPECT_CALL(*mock_, CreateResumableUpload)
+      .WillOnce(Return(internal::CreateResumableUploadResponse{"test-upload-id"}));
+  EXPECT_CALL(*mock_, UploadChunk).WillOnce(Return(make_status_or(
+      internal::QueryResumableUploadResponse{absl::nullopt, expected})));
+
+  TempFile temp(contents);
+  auto client = ClientForMock();
+  auto actual = client.UploadFile(
+      temp.name(), "test-bucket-name", "test-object-name",
+      UseResumableUploadSession(""));
+  ASSERT_STATUS_OK(actual);
+  EXPECT_EQ(expected, *actual);
+}
+
 TEST_F(ObjectTest, DeleteResumableUpload) {
   EXPECT_CALL(*mock_, DeleteResumableUpload)
       .WillOnce(Return(StatusOr<internal::EmptyResponse>(TransientError())))
diff --git a/google/cloud/storage/internal/connection_impl.cc b/google/cloud/storage/internal/connection_impl.cc
index f414be48c5394..c3b0d1456171c 100644
--- a/google/cloud/storage/internal/connection_impl.cc
+++ b/google/cloud/storage/internal/connection_impl.cc
@@ -757,7 +757,7 @@ StatusOr<QueryResumableUploadResponse> StorageConnectionImpl::UploadChunk(
   return RetryError(last_status, *retry_policy, __func__);
 }
 
-StatusOr<EmptyResponse> StorageConnectionImpl::UploadFileSimple(
+StatusOr<std::unique_ptr<std::string>> StorageConnectionImpl::UploadFileSimple(
     std::string const& file_name, std::size_t file_size,
     InsertObjectMediaRequest& request) {
   auto upload_offset = request.GetOption<UploadFromOffset>().value_or(0);
@@ -782,23 +782,23 @@ StatusOr<EmptyResponse> StorageConnectionImpl::UploadFileSimple(
                                                   GCP_ERROR_INFO());
   }
 
-  std::string payload(static_cast<std::size_t>(upload_size), char{});
+  auto payload =
+      std::make_unique<std::string>(static_cast<std::size_t>(upload_size), char{});
   is.seekg(upload_offset, std::ios::beg);
   // We need to use `&payload[0]` until C++17
   // NOLINTNEXTLINE(readability-container-data-pointer)
-  is.read(&payload[0], payload.size());
-  if (static_cast<std::size_t>(is.gcount()) < payload.size()) {
+  is.read(&(*payload)[0], payload->size());
+  if (static_cast<std::size_t>(is.gcount()) < payload->size()) {
     std::ostringstream os;
     os << __func__ << "(" << request << ", " << file_name << "): Actual read ("
-       << is.gcount() << ") is smaller than upload_size (" << payload.size()
+       << is.gcount() << ") is smaller than upload_size (" << payload->size()
        << ")";
     return google::cloud::internal::InternalError(std::move(os).str(),
                                                   GCP_ERROR_INFO());
   }
   is.close();
-  request.set_payload(payload);
 
-  return EmptyResponse();
+  return std::move(payload);
 }
 
 StatusOr<std::unique_ptr<std::istream>>
diff --git a/google/cloud/storage/internal/connection_impl.h b/google/cloud/storage/internal/connection_impl.h
index b65751c2c9117..cb6f9c441c0ae 100644
--- a/google/cloud/storage/internal/connection_impl.h
+++ b/google/cloud/storage/internal/connection_impl.h
@@ -99,7 +99,7 @@ class StorageConnectionImpl
       DeleteResumableUploadRequest const& request) override;
   StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) override;
-  StatusOr<EmptyResponse> UploadFileSimple(
+  StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
       std::string const& file_name, std::size_t file_size,
       InsertObjectMediaRequest& request) override;
   StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
diff --git a/google/cloud/storage/internal/storage_connection.h b/google/cloud/storage/internal/storage_connection.h
index 038909e3e6973..811402bc29d4d 100644
--- a/google/cloud/storage/internal/storage_connection.h
+++ b/google/cloud/storage/internal/storage_connection.h
@@ -110,7 +110,7 @@ class StorageConnection {
       DeleteResumableUploadRequest const& request) = 0;
   virtual StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) = 0;
-  virtual StatusOr<EmptyResponse> UploadFileSimple(
+  virtual StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
       std::string const& file_name, std::size_t file_size,
       InsertObjectMediaRequest& request) {
     return Status(StatusCode::kUnimplemented, "unimplemented");
diff --git a/google/cloud/storage/internal/tracing_connection.cc b/google/cloud/storage/internal/tracing_connection.cc
index e358f636f0e95..661a1a2c5d976 100644
--- a/google/cloud/storage/internal/tracing_connection.cc
+++ b/google/cloud/storage/internal/tracing_connection.cc
@@ -235,7 +235,7 @@ TracingConnection::UploadChunk(
   return internal::EndSpan(*span, impl_->UploadChunk(request));
 }
 
-StatusOr<storage::internal::EmptyResponse> TracingConnection::UploadFileSimple(
+StatusOr<std::unique_ptr<std::string>> TracingConnection::UploadFileSimple(
     std::string const& file_name, std::size_t file_size,
     storage::internal::InsertObjectMediaRequest& request) {
   auto span =
diff --git a/google/cloud/storage/internal/tracing_connection.h b/google/cloud/storage/internal/tracing_connection.h
index 6f4aaddc1f6e3..f6c148a59c4c7 100644
--- a/google/cloud/storage/internal/tracing_connection.h
+++ b/google/cloud/storage/internal/tracing_connection.h
@@ -98,7 +98,7 @@ class TracingConnection : public storage::internal::StorageConnection {
       storage::internal::DeleteResumableUploadRequest const& request) override;
   StatusOr<storage::internal::QueryResumableUploadResponse> UploadChunk(
       storage::internal::UploadChunkRequest const& request) override;
-  StatusOr<storage::internal::EmptyResponse> UploadFileSimple(
+  StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
       std::string const& file_name,
       std::size_t file_size,
       storage::internal::InsertObjectMediaRequest& request) override;
diff --git a/google/cloud/storage/internal/tracing_connection_test.cc b/google/cloud/storage/internal/tracing_connection_test.cc
index c5a613de835e0..2dd2f3cfab691 100644
--- a/google/cloud/storage/internal/tracing_connection_test.cc
+++ b/google/cloud/storage/internal/tracing_connection_test.cc
@@ -694,6 +694,56 @@ TEST(TracingClientTest, UploadChunk) {
                       "gl-cpp.status_code", code_str)))));
 }
 
+TEST(TracingClientTest, UploadFileSimple) {
+  auto span_catcher = InstallSpanCatcher();
+  auto mock = std::make_shared<MockClient>();
+  EXPECT_CALL(*mock, UploadFileSimple).WillOnce([](std::string const&, std::size_t, storage::internal::InsertObjectMediaRequest&) {
+    EXPECT_TRUE(ThereIsAnActiveSpan());
+    return PermanentError();
+  });
+  auto under_test = TracingConnection(mock);
+  storage::internal::InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  auto actual = under_test.UploadFileSimple("test-file.txt", 1234, request);
+
+  auto const code = PermanentError().code();
+  auto const code_str = StatusCodeToString(code);
+  auto const msg = PermanentError().message();
+  EXPECT_THAT(actual, StatusIs(code));
+  EXPECT_THAT(
+      span_catcher->GetSpans(),
+      ElementsAre(AllOf(
+          SpanNamed("storage::Client::UploadFile/UploadFileSimple"),
+          SpanHasInstrumentationScope(), SpanKindIsClient(),
+          SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
+          SpanHasAttributes(
+              OTelAttribute<std::string>("gl-cpp.status_code", code_str)))));
+}
+
+TEST(TracingClientTest, UploadFileResumable) {
+  auto span_catcher = InstallSpanCatcher();
+  auto mock = std::make_shared<MockClient>();
+  EXPECT_CALL(*mock, UploadFileResumable).WillOnce([](auto const&, auto&) {
+    EXPECT_TRUE(ThereIsAnActiveSpan());
+    return PermanentError();
+  });
+  auto under_test = TracingConnection(mock);
+  storage::internal::ResumableUploadRequest request("test-bucket", "test-object");
+  auto actual = under_test.UploadFileResumable("test-file.txt", request);
+
+  auto const code = PermanentError().code();
+  auto const code_str = StatusCodeToString(code);
+  auto const msg = PermanentError().message();
+  EXPECT_THAT(actual, StatusIs(code));
+  EXPECT_THAT(
+      span_catcher->GetSpans(),
+      ElementsAre(AllOf(
+          SpanNamed("storage::Client::UploadFile/UploadFileResumable"),
+          SpanHasInstrumentationScope(), SpanKindIsClient(),
+          SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
+          SpanHasAttributes(
+              OTelAttribute<std::string>("gl-cpp.status_code", code_str)))));
+}
+
 TEST(TracingClientTest, ListBucketAcl) {
   auto span_catcher = InstallSpanCatcher();
   auto mock = std::make_shared<MockClient>();
diff --git a/google/cloud/storage/testing/mock_client.h b/google/cloud/storage/testing/mock_client.h
index 3462e7d68795e..a4b9d5eeee95f 100644
--- a/google/cloud/storage/testing/mock_client.h
+++ b/google/cloud/storage/testing/mock_client.h
@@ -101,6 +101,14 @@ class MockClient : public google::cloud::storage::internal::StorageConnection {
               (internal::DeleteResumableUploadRequest const&), (override));
   MOCK_METHOD(StatusOr<internal::QueryResumableUploadResponse>, UploadChunk,
               (internal::UploadChunkRequest const&), (override));
+  MOCK_METHOD(StatusOr<std::unique_ptr<std::string>>, UploadFileSimple,
+              (std::string const&, std::size_t,
+               storage::internal::InsertObjectMediaRequest&),
+              (override));
+  MOCK_METHOD(StatusOr<std::unique_ptr<std::istream>>, UploadFileResumable,
+              (std::string const&,
+               storage::internal::ResumableUploadRequest&),
+              (override));
 
   MOCK_METHOD(StatusOr<internal::ListBucketAclResponse>, ListBucketAcl,
               (internal::ListBucketAclRequest const&), (override));

From 51185aa29d0aaa92a3d742ab6ed265aad740fd4b Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 12:10:51 +0000
Subject: [PATCH 05/10] fixing file formatting

---
 google/cloud/storage/client_object_test.cc    | 14 +++---
 .../cloud/storage/internal/connection_impl.cc |  6 +--
 .../cloud/storage/internal/connection_impl.h  |  3 +-
 .../storage/internal/storage_connection.h     |  9 ++--
 .../storage/internal/tracing_connection.h     |  3 +-
 .../internal/tracing_connection_test.cc       | 46 ++++++++++---------
 google/cloud/storage/testing/mock_client.h    |  3 +-
 7 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/google/cloud/storage/client_object_test.cc b/google/cloud/storage/client_object_test.cc
index 53b814b7540f8..35fae9f255af0 100644
--- a/google/cloud/storage/client_object_test.cc
+++ b/google/cloud/storage/client_object_test.cc
@@ -287,15 +287,17 @@ TEST_F(ObjectTest, UploadFileResumable) {
   auto const contents = std::string{"some not so simple contents"};
 
   EXPECT_CALL(*mock_, CreateResumableUpload)
-      .WillOnce(Return(internal::CreateResumableUploadResponse{"test-upload-id"}));
-  EXPECT_CALL(*mock_, UploadChunk).WillOnce(Return(make_status_or(
-      internal::QueryResumableUploadResponse{absl::nullopt, expected})));
+      .WillOnce(
+          Return(internal::CreateResumableUploadResponse{"test-upload-id"}));
+  EXPECT_CALL(*mock_, UploadChunk)
+      .WillOnce(Return(make_status_or(
+          internal::QueryResumableUploadResponse{absl::nullopt, expected})));
 
   TempFile temp(contents);
   auto client = ClientForMock();
-  auto actual = client.UploadFile(
-      temp.name(), "test-bucket-name", "test-object-name",
-      UseResumableUploadSession(""));
+  auto actual =
+      client.UploadFile(temp.name(), "test-bucket-name", "test-object-name",
+                        UseResumableUploadSession(""));
   ASSERT_STATUS_OK(actual);
   EXPECT_EQ(expected, *actual);
 }
diff --git a/google/cloud/storage/internal/connection_impl.cc b/google/cloud/storage/internal/connection_impl.cc
index c3b0d1456171c..6225dea4016a4 100644
--- a/google/cloud/storage/internal/connection_impl.cc
+++ b/google/cloud/storage/internal/connection_impl.cc
@@ -782,8 +782,8 @@ StatusOr<std::unique_ptr<std::string>> StorageConnectionImpl::UploadFileSimple(
                                                   GCP_ERROR_INFO());
   }
 
-  auto payload =
-      std::make_unique<std::string>(static_cast<std::size_t>(upload_size), char{});
+  auto payload = std::make_unique<std::string>(
+      static_cast<std::size_t>(upload_size), char{});
   is.seekg(upload_offset, std::ios::beg);
   // We need to use `&payload[0]` until C++17
   // NOLINTNEXTLINE(readability-container-data-pointer)
@@ -798,7 +798,7 @@ StatusOr<std::unique_ptr<std::string>> StorageConnectionImpl::UploadFileSimple(
   }
   is.close();
 
-  return std::move(payload);
+  return payload;
 }
 
 StatusOr<std::unique_ptr<std::istream>>
diff --git a/google/cloud/storage/internal/connection_impl.h b/google/cloud/storage/internal/connection_impl.h
index cb6f9c441c0ae..c42d806f78d84 100644
--- a/google/cloud/storage/internal/connection_impl.h
+++ b/google/cloud/storage/internal/connection_impl.h
@@ -103,8 +103,7 @@ class StorageConnectionImpl
       std::string const& file_name, std::size_t file_size,
       InsertObjectMediaRequest& request) override;
   StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
-      std::string const& file_name,
-      ResumableUploadRequest& request) override;
+      std::string const& file_name, ResumableUploadRequest& request) override;
 
   StatusOr<ListBucketAclResponse> ListBucketAcl(
       ListBucketAclRequest const& request) override;
diff --git a/google/cloud/storage/internal/storage_connection.h b/google/cloud/storage/internal/storage_connection.h
index 811402bc29d4d..9f754a3019cee 100644
--- a/google/cloud/storage/internal/storage_connection.h
+++ b/google/cloud/storage/internal/storage_connection.h
@@ -111,13 +111,14 @@ class StorageConnection {
   virtual StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) = 0;
   virtual StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
-      std::string const& file_name, std::size_t file_size,
-      InsertObjectMediaRequest& request) {
+      [[maybe_unused]] std::string const& file_name,
+      [[maybe_unused]] std::size_t file_size,
+      [[maybe_unused]] InsertObjectMediaRequest& request) {
     return Status(StatusCode::kUnimplemented, "unimplemented");
   }
   virtual StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
-      std::string const& file_name,
-      ResumableUploadRequest& request) {
+      [[maybe_unused]] std::string const& file_name,
+      [[maybe_unused]] ResumableUploadRequest& request) {
     return Status(StatusCode::kUnimplemented, "unimplemented");
   }
   ///@}
diff --git a/google/cloud/storage/internal/tracing_connection.h b/google/cloud/storage/internal/tracing_connection.h
index f6c148a59c4c7..60bf303074214 100644
--- a/google/cloud/storage/internal/tracing_connection.h
+++ b/google/cloud/storage/internal/tracing_connection.h
@@ -99,8 +99,7 @@ class TracingConnection : public storage::internal::StorageConnection {
   StatusOr<storage::internal::QueryResumableUploadResponse> UploadChunk(
       storage::internal::UploadChunkRequest const& request) override;
   StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
-      std::string const& file_name,
-      std::size_t file_size,
+      std::string const& file_name, std::size_t file_size,
       storage::internal::InsertObjectMediaRequest& request) override;
   StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
       std::string const& file_name,
diff --git a/google/cloud/storage/internal/tracing_connection_test.cc b/google/cloud/storage/internal/tracing_connection_test.cc
index 2dd2f3cfab691..5e6f7dc47f261 100644
--- a/google/cloud/storage/internal/tracing_connection_test.cc
+++ b/google/cloud/storage/internal/tracing_connection_test.cc
@@ -697,26 +697,28 @@ TEST(TracingClientTest, UploadChunk) {
 TEST(TracingClientTest, UploadFileSimple) {
   auto span_catcher = InstallSpanCatcher();
   auto mock = std::make_shared<MockClient>();
-  EXPECT_CALL(*mock, UploadFileSimple).WillOnce([](std::string const&, std::size_t, storage::internal::InsertObjectMediaRequest&) {
-    EXPECT_TRUE(ThereIsAnActiveSpan());
-    return PermanentError();
-  });
+  EXPECT_CALL(*mock, UploadFileSimple)
+      .WillOnce([](std::string const&, std::size_t,
+                   storage::internal::InsertObjectMediaRequest&) {
+        EXPECT_TRUE(ThereIsAnActiveSpan());
+        return PermanentError();
+      });
   auto under_test = TracingConnection(mock);
-  storage::internal::InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  storage::internal::InsertObjectMediaRequest request("test-bucket",
+                                                      "test-object", "");
   auto actual = under_test.UploadFileSimple("test-file.txt", 1234, request);
 
   auto const code = PermanentError().code();
   auto const code_str = StatusCodeToString(code);
   auto const msg = PermanentError().message();
   EXPECT_THAT(actual, StatusIs(code));
-  EXPECT_THAT(
-      span_catcher->GetSpans(),
-      ElementsAre(AllOf(
-          SpanNamed("storage::Client::UploadFile/UploadFileSimple"),
-          SpanHasInstrumentationScope(), SpanKindIsClient(),
-          SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
-          SpanHasAttributes(
-              OTelAttribute<std::string>("gl-cpp.status_code", code_str)))));
+  EXPECT_THAT(span_catcher->GetSpans(),
+              ElementsAre(AllOf(
+                  SpanNamed("storage::Client::UploadFile/UploadFileSimple"),
+                  SpanHasInstrumentationScope(), SpanKindIsClient(),
+                  SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
+                  SpanHasAttributes(OTelAttribute<std::string>(
+                      "gl-cpp.status_code", code_str)))));
 }
 
 TEST(TracingClientTest, UploadFileResumable) {
@@ -727,21 +729,21 @@ TEST(TracingClientTest, UploadFileResumable) {
     return PermanentError();
   });
   auto under_test = TracingConnection(mock);
-  storage::internal::ResumableUploadRequest request("test-bucket", "test-object");
+  storage::internal::ResumableUploadRequest request("test-bucket",
+                                                    "test-object");
   auto actual = under_test.UploadFileResumable("test-file.txt", request);
 
   auto const code = PermanentError().code();
   auto const code_str = StatusCodeToString(code);
   auto const msg = PermanentError().message();
   EXPECT_THAT(actual, StatusIs(code));
-  EXPECT_THAT(
-      span_catcher->GetSpans(),
-      ElementsAre(AllOf(
-          SpanNamed("storage::Client::UploadFile/UploadFileResumable"),
-          SpanHasInstrumentationScope(), SpanKindIsClient(),
-          SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
-          SpanHasAttributes(
-              OTelAttribute<std::string>("gl-cpp.status_code", code_str)))));
+  EXPECT_THAT(span_catcher->GetSpans(),
+              ElementsAre(AllOf(
+                  SpanNamed("storage::Client::UploadFile/UploadFileResumable"),
+                  SpanHasInstrumentationScope(), SpanKindIsClient(),
+                  SpanWithStatus(opentelemetry::trace::StatusCode::kError, msg),
+                  SpanHasAttributes(OTelAttribute<std::string>(
+                      "gl-cpp.status_code", code_str)))));
 }
 
 TEST(TracingClientTest, ListBucketAcl) {
diff --git a/google/cloud/storage/testing/mock_client.h b/google/cloud/storage/testing/mock_client.h
index a4b9d5eeee95f..5aa1d2fa05de5 100644
--- a/google/cloud/storage/testing/mock_client.h
+++ b/google/cloud/storage/testing/mock_client.h
@@ -106,8 +106,7 @@ class MockClient : public google::cloud::storage::internal::StorageConnection {
                storage::internal::InsertObjectMediaRequest&),
               (override));
   MOCK_METHOD(StatusOr<std::unique_ptr<std::istream>>, UploadFileResumable,
-              (std::string const&,
-               storage::internal::ResumableUploadRequest&),
+              (std::string const&, storage::internal::ResumableUploadRequest&),
               (override));
 
   MOCK_METHOD(StatusOr<internal::ListBucketAclResponse>, ListBucketAcl,

From e7c56d8bdc0bd81af3401cd3bdadd3043a8df0d8 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 12:27:52 +0000
Subject: [PATCH 06/10] fixing file formatting

---
 google/cloud/storage/internal/connection_impl.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/google/cloud/storage/internal/connection_impl.cc b/google/cloud/storage/internal/connection_impl.cc
index 6225dea4016a4..158dbdeb2e58d 100644
--- a/google/cloud/storage/internal/connection_impl.cc
+++ b/google/cloud/storage/internal/connection_impl.cc
@@ -769,9 +769,9 @@ StatusOr<std::unique_ptr<std::string>> StorageConnectionImpl::UploadFileSimple(
     return google::cloud::internal::InvalidArgumentError(std::move(os).str(),
                                                          GCP_ERROR_INFO());
   }
-  auto upload_size = (std::min)(request.GetOption<UploadLimit>().value_or(
-                                    file_size - upload_offset),
-                                file_size - upload_offset);
+  auto upload_size = (std::min)(
+      request.GetOption<UploadLimit>().value_or(file_size - upload_offset),
+      file_size - upload_offset);
 
   std::ifstream is(file_name, std::ios::binary);
   if (!is.is_open()) {
@@ -835,9 +835,9 @@ integrity checks using the DisableMD5Hash() and DisableCrc32cChecksum() options.
                                                            GCP_ERROR_INFO());
     }
 
-    auto upload_size = (std::min)(request.GetOption<UploadLimit>().value_or(
-                                      file_size - upload_offset),
-                                  file_size - upload_offset);
+    auto upload_size = (std::min)(
+        request.GetOption<UploadLimit>().value_or(file_size - upload_offset),
+        file_size - upload_offset);
     request.set_option(UploadContentLength(upload_size));
   }
   auto source = std::make_unique<std::ifstream>(file_name, std::ios::binary);

From 4e32737cf0f43033b186b186d3f57c378193ed7e Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 13:20:25 +0000
Subject: [PATCH 07/10] adding more tests

---
 .../storage/google_cloud_cpp_storage.cmake    |   2 +
 .../connection_impl_file_upload_test.cc       | 154 ++++++++++++++++
 .../internal/storage_connection_test.cc       | 166 ++++++++++++++++++
 .../storage/storage_client_unit_tests.bzl     |   2 +
 4 files changed, 324 insertions(+)
 create mode 100644 google/cloud/storage/internal/connection_impl_file_upload_test.cc
 create mode 100644 google/cloud/storage/internal/storage_connection_test.cc

diff --git a/google/cloud/storage/google_cloud_cpp_storage.cmake b/google/cloud/storage/google_cloud_cpp_storage.cmake
index 337cf12a80e71..d1fe20b4fa382 100644
--- a/google/cloud/storage/google_cloud_cpp_storage.cmake
+++ b/google/cloud/storage/google_cloud_cpp_storage.cmake
@@ -462,6 +462,7 @@ if (BUILD_TESTING)
         internal/connection_impl_bucket_acl_test.cc
         internal/connection_impl_bucket_test.cc
         internal/connection_impl_default_object_acl_test.cc
+        internal/connection_impl_file_upload_test.cc
         internal/connection_impl_notifications_test.cc
         internal/connection_impl_object_acl_test.cc
         internal/connection_impl_object_copy_test.cc
@@ -499,6 +500,7 @@ if (BUILD_TESTING)
         internal/service_account_requests_test.cc
         internal/sign_blob_requests_test.cc
         internal/signed_url_requests_test.cc
+        internal/storage_connection_test.cc
         internal/tracing_connection_test.cc
         internal/tracing_object_read_source_test.cc
         internal/tuple_filter_test.cc
diff --git a/google/cloud/storage/internal/connection_impl_file_upload_test.cc b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
new file mode 100644
index 0000000000000..fb1b524a2e317
--- /dev/null
+++ b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
@@ -0,0 +1,154 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "google/cloud/storage/internal/connection_impl.h"
+#include "google/cloud/storage/testing/mock_generic_stub.h"
+#include "google/cloud/storage/testing/temp_file.h"
+#include "google/cloud/testing_util/status_matchers.h"
+#include <gmock/gmock.h>
+#include <iterator>
+
+namespace google {
+namespace cloud {
+namespace storage {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+namespace internal {
+namespace {
+
+using ::google::cloud::storage::testing::MockGenericStub;
+using ::google::cloud::storage::testing::TempFile;
+using ::google::cloud::testing_util::IsOk;
+using ::google::cloud::testing_util::StatusIs;
+using ::testing::_;
+using ::testing::Return;
+
+class ConnectionImplFileUploadTest : public ::testing::Test {
+ protected:
+  std::shared_ptr<StorageConnectionImpl> MakeConnection() {
+    auto mock = std::make_unique<MockGenericStub>();
+    EXPECT_CALL(*mock, options).WillRepeatedly(Return(Options{}));
+    return StorageConnectionImpl::Create(std::move(mock));
+  }
+};
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleSuccess) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some simple contents"};
+  TempFile temp(contents);
+  InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  auto payload =
+      connection->UploadFileSimple(temp.name(), contents.size(), request);
+  ASSERT_STATUS_OK(payload);
+  EXPECT_EQ(*payload.value(), contents);
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleNonExistentFile) {
+  auto connection = MakeConnection();
+  InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  auto payload =
+      connection->UploadFileSimple("/no/such/file/exists.txt", 123, request);
+  EXPECT_THAT(payload, StatusIs(StatusCode::kNotFound));
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleOffsetLargerThanSize) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some simple contents"};
+  TempFile temp(contents);
+  InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  request.set_option(UploadFromOffset(contents.size() + 1));
+  auto payload =
+      connection->UploadFileSimple(temp.name(), contents.size(), request);
+  EXPECT_THAT(payload, StatusIs(StatusCode::kInvalidArgument));
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleWithLimit) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"0123456789"};
+  TempFile temp(contents);
+  InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  request.set_option(UploadLimit(5));
+  auto payload =
+      connection->UploadFileSimple(temp.name(), contents.size(), request);
+  ASSERT_STATUS_OK(payload);
+  EXPECT_EQ(*payload.value(), "01234");
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleReadError) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some simple contents"};
+  TempFile temp(contents);
+  // Pass a size larger than the file to trigger a read error.
+  InsertObjectMediaRequest request("test-bucket", "test-object", "");
+  auto payload =
+      connection->UploadFileSimple(temp.name(), contents.size() + 1, request);
+  EXPECT_THAT(payload, StatusIs(StatusCode::kInternal));
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileResumableSuccess) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some resumable contents"};
+  TempFile temp(contents);
+  ResumableUploadRequest request("test-bucket", "test-object");
+  auto stream = connection->UploadFileResumable(temp.name(), request);
+  ASSERT_STATUS_OK(stream);
+  EXPECT_TRUE(request.HasOption<UploadContentLength>());
+  EXPECT_EQ(request.GetOption<UploadContentLength>().value(), contents.size());
+  std::string actual(std::istreambuf_iterator<char>(**stream), {});
+  EXPECT_EQ(contents, actual);
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileResumableNonExistentFile) {
+  auto connection = MakeConnection();
+  ResumableUploadRequest request("test-bucket", "test-object");
+  auto stream =
+      connection->UploadFileResumable("/no/such/file/exists.txt", request);
+  EXPECT_THAT(stream, StatusIs(StatusCode::kNotFound));
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileResumableOffsetLargerThanSize) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some resumable contents"};
+  TempFile temp(contents);
+  ResumableUploadRequest request("test-bucket", "test-object");
+  request.set_option(UploadFromOffset(contents.size() + 1));
+  auto stream = connection->UploadFileResumable(temp.name(), request);
+  EXPECT_THAT(stream, StatusIs(StatusCode::kInvalidArgument));
+}
+
+TEST_F(ConnectionImplFileUploadTest, UploadFileResumableWithLimit) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"0123456789"};
+  TempFile temp(contents);
+  ResumableUploadRequest request("test-bucket", "test-object");
+  auto const limit = 5;
+  request.set_option(UploadLimit(limit));
+  auto stream = connection->UploadFileResumable(temp.name(), request);
+  ASSERT_STATUS_OK(stream);
+  EXPECT_TRUE(request.HasOption<UploadContentLength>());
+  EXPECT_EQ(request.GetOption<UploadContentLength>().value(), limit);
+
+  // Read only the limited number of bytes from the stream.
+  std::vector<char> buffer(limit);
+  (*stream)->read(buffer.data(), buffer.size());
+  std::string actual(buffer.begin(), buffer.end());
+
+  EXPECT_EQ("01234", actual);
+}
+
+}  // namespace
+}  // namespace internal
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace storage
+}  // namespace cloud
+}  // namespace google
diff --git a/google/cloud/storage/internal/storage_connection_test.cc b/google/cloud/storage/internal/storage_connection_test.cc
new file mode 100644
index 0000000000000..83135beb45dad
--- /dev/null
+++ b/google/cloud/storage/internal/storage_connection_test.cc
@@ -0,0 +1,166 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "google/cloud/storage/internal/storage_connection.h"
+#include "google/cloud/testing_util/status_matchers.h"
+#include <gmock/gmock.h>
+
+namespace google {
+namespace cloud {
+namespace storage {
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
+namespace internal {
+namespace {
+
+using ::google::cloud::testing_util::StatusIs;
+
+// A minimal implementation of StorageConnection to test the default
+// implementation of some of its virtual functions.
+class TestStorageConnection : public StorageConnection {
+ public:
+  ~TestStorageConnection() override = default;
+  MOCK_METHOD(ClientOptions const&, client_options, (), (const, override));
+  MOCK_METHOD(StatusOr<ListBucketsResponse>, ListBuckets,
+              (ListBucketsRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketMetadata>, CreateBucket,
+              (CreateBucketRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketMetadata>, GetBucketMetadata,
+              (GetBucketMetadataRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteBucket,
+              (DeleteBucketRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketMetadata>, UpdateBucket,
+              (UpdateBucketRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketMetadata>, PatchBucket,
+              (PatchBucketRequest const&), (override));
+  MOCK_METHOD(StatusOr<NativeIamPolicy>, GetNativeBucketIamPolicy,
+              (GetBucketIamPolicyRequest const&), (override));
+  MOCK_METHOD(StatusOr<NativeIamPolicy>, SetNativeBucketIamPolicy,
+              (SetNativeBucketIamPolicyRequest const&), (override));
+  MOCK_METHOD(StatusOr<TestBucketIamPermissionsResponse>,
+              TestBucketIamPermissions,
+              (TestBucketIamPermissionsRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketMetadata>, LockBucketRetentionPolicy,
+              (LockBucketRetentionPolicyRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, InsertObjectMedia,
+              (InsertObjectMediaRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, CopyObject, (CopyObjectRequest const&),
+              (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, GetObjectMetadata,
+              (GetObjectMetadataRequest const&), (override));
+  MOCK_METHOD(StatusOr<std::unique_ptr<ObjectReadSource>>, ReadObject,
+              (ReadObjectRangeRequest const&), (override));
+  MOCK_METHOD(StatusOr<ListObjectsResponse>, ListObjects,
+              (ListObjectsRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteObject,
+              (DeleteObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, UpdateObject,
+              (UpdateObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, MoveObject, (MoveObjectRequest const&),
+              (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, PatchObject,
+              (PatchObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, ComposeObject,
+              (ComposeObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<RewriteObjectResponse>, RewriteObject,
+              (RewriteObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectMetadata>, RestoreObject,
+              (RestoreObjectRequest const&), (override));
+  MOCK_METHOD(StatusOr<CreateResumableUploadResponse>, CreateResumableUpload,
+              (ResumableUploadRequest const&), (override));
+  MOCK_METHOD(StatusOr<QueryResumableUploadResponse>, QueryResumableUpload,
+              (QueryResumableUploadRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteResumableUpload,
+              (DeleteResumableUploadRequest const&), (override));
+  MOCK_METHOD(StatusOr<QueryResumableUploadResponse>, UploadChunk,
+              (UploadChunkRequest const&), (override));
+  MOCK_METHOD(StatusOr<ListBucketAclResponse>, ListBucketAcl,
+              (ListBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketAccessControl>, CreateBucketAcl,
+              (CreateBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteBucketAcl,
+              (DeleteBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketAccessControl>, GetBucketAcl,
+              (GetBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketAccessControl>, UpdateBucketAcl,
+              (UpdateBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<BucketAccessControl>, PatchBucketAcl,
+              (PatchBucketAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ListObjectAclResponse>, ListObjectAcl,
+              (ListObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, CreateObjectAcl,
+              (CreateObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteObjectAcl,
+              (DeleteObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, GetObjectAcl,
+              (GetObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, UpdateObjectAcl,
+              (UpdateObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, PatchObjectAcl,
+              (PatchObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ListDefaultObjectAclResponse>, ListDefaultObjectAcl,
+              (ListDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, CreateDefaultObjectAcl,
+              (CreateDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteDefaultObjectAcl,
+              (DeleteDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, GetDefaultObjectAcl,
+              (GetDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, UpdateDefaultObjectAcl,
+              (UpdateDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ObjectAccessControl>, PatchDefaultObjectAcl,
+              (PatchDefaultObjectAclRequest const&), (override));
+  MOCK_METHOD(StatusOr<ServiceAccount>, GetServiceAccount,
+              (GetProjectServiceAccountRequest const&), (override));
+  MOCK_METHOD(StatusOr<ListHmacKeysResponse>, ListHmacKeys,
+              (ListHmacKeysRequest const&), (override));
+  MOCK_METHOD(StatusOr<CreateHmacKeyResponse>, CreateHmacKey,
+              (CreateHmacKeyRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteHmacKey,
+              (DeleteHmacKeyRequest const&), (override));
+  MOCK_METHOD(StatusOr<HmacKeyMetadata>, GetHmacKey, (GetHmacKeyRequest const&),
+              (override));
+  MOCK_METHOD(StatusOr<HmacKeyMetadata>, UpdateHmacKey,
+              (UpdateHmacKeyRequest const&), (override));
+  MOCK_METHOD(StatusOr<SignBlobResponse>, SignBlob, (SignBlobRequest const&),
+              (override));
+  MOCK_METHOD(StatusOr<ListNotificationsResponse>, ListNotifications,
+              (ListNotificationsRequest const&), (override));
+  MOCK_METHOD(StatusOr<NotificationMetadata>, CreateNotification,
+              (CreateNotificationRequest const&), (override));
+  MOCK_METHOD(StatusOr<NotificationMetadata>, GetNotification,
+              (GetNotificationRequest const&), (override));
+  MOCK_METHOD(StatusOr<EmptyResponse>, DeleteNotification,
+              (DeleteNotificationRequest const&), (override));
+};
+
+TEST(StorageConnectionTest, UploadFileSimpleUnimplemented) {
+  TestStorageConnection connection;
+  InsertObjectMediaRequest request;
+  auto response = connection.UploadFileSimple("test-file.txt", 1234, request);
+  EXPECT_THAT(response, StatusIs(StatusCode::kUnimplemented));
+}
+
+TEST(StorageConnectionTest, UploadFileResumableUnimplemented) {
+  TestStorageConnection connection;
+  ResumableUploadRequest request;
+  auto response = connection.UploadFileResumable("test-file.txt", request);
+  EXPECT_THAT(response, StatusIs(StatusCode::kUnimplemented));
+}
+
+}  // namespace
+}  // namespace internal
+GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
+}  // namespace storage
+}  // namespace cloud
+}  // namespace google
diff --git a/google/cloud/storage/storage_client_unit_tests.bzl b/google/cloud/storage/storage_client_unit_tests.bzl
index 3087aec96896a..a9b757d1058fa 100644
--- a/google/cloud/storage/storage_client_unit_tests.bzl
+++ b/google/cloud/storage/storage_client_unit_tests.bzl
@@ -53,6 +53,7 @@ storage_client_unit_tests = [
     "internal/connection_impl_bucket_acl_test.cc",
     "internal/connection_impl_bucket_test.cc",
     "internal/connection_impl_default_object_acl_test.cc",
+    "internal/connection_impl_file_upload_test.cc",
     "internal/connection_impl_notifications_test.cc",
     "internal/connection_impl_object_acl_test.cc",
     "internal/connection_impl_object_copy_test.cc",
@@ -90,6 +91,7 @@ storage_client_unit_tests = [
     "internal/service_account_requests_test.cc",
     "internal/sign_blob_requests_test.cc",
     "internal/signed_url_requests_test.cc",
+    "internal/storage_connection_test.cc",
     "internal/tracing_connection_test.cc",
     "internal/tracing_object_read_source_test.cc",
     "internal/tuple_filter_test.cc",

From 6622ccc3a40732f4d4a9e55e1a08602babd52dd5 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 14:33:31 +0000
Subject: [PATCH 08/10] fixing clang tidy

---
 .../storage/internal/connection_impl_file_upload_test.cc      | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/google/cloud/storage/internal/connection_impl_file_upload_test.cc b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
index fb1b524a2e317..eabf4c3bd3e41 100644
--- a/google/cloud/storage/internal/connection_impl_file_upload_test.cc
+++ b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
@@ -28,14 +28,12 @@ namespace {
 
 using ::google::cloud::storage::testing::MockGenericStub;
 using ::google::cloud::storage::testing::TempFile;
-using ::google::cloud::testing_util::IsOk;
 using ::google::cloud::testing_util::StatusIs;
-using ::testing::_;
 using ::testing::Return;
 
 class ConnectionImplFileUploadTest : public ::testing::Test {
  protected:
-  std::shared_ptr<StorageConnectionImpl> MakeConnection() {
+  static std::shared_ptr<StorageConnectionImpl> MakeConnection() {
     auto mock = std::make_unique<MockGenericStub>();
     EXPECT_CALL(*mock, options).WillRepeatedly(Return(Options{}));
     return StorageConnectionImpl::Create(std::move(mock));

From bc03d1e7c702be43845874bf2b1bf6f240a63c54 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 15:17:14 +0000
Subject: [PATCH 09/10] fixing code coverage

---
 .../connection_impl_file_upload_test.cc       | 22 +++++++++++++++++++
 .../internal/storage_connection_test.cc       |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/google/cloud/storage/internal/connection_impl_file_upload_test.cc b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
index eabf4c3bd3e41..619c175459e9d 100644
--- a/google/cloud/storage/internal/connection_impl_file_upload_test.cc
+++ b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
@@ -18,6 +18,9 @@
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <iterator>
+#if !defined(_WIN32)
+#include <sys/stat.h>
+#endif  // !defined(_WIN32)
 
 namespace google {
 namespace cloud {
@@ -114,6 +117,25 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileResumableNonExistentFile) {
   EXPECT_THAT(stream, StatusIs(StatusCode::kNotFound));
 }
 
+#if !defined(_WIN32)
+TEST_F(ConnectionImplFileUploadTest, UploadFileResumableUnreadable) {
+  auto connection = MakeConnection();
+  auto const contents = std::string{"some resumable contents"};
+  TempFile temp(contents);
+
+  // Make the file unreadable.
+  ASSERT_EQ(0, chmod(temp.name().c_str(), 0000));
+
+  // The implementation should detect that it cannot open the file.
+  ResumableUploadRequest request("test-bucket", "test-object");
+  auto stream = connection->UploadFileResumable(temp.name(), request);
+  EXPECT_THAT(stream, StatusIs(StatusCode::kNotFound));
+
+  // Restore permissions so the TempFile destructor can delete it.
+  (void)chmod(temp.name().c_str(), 0600);
+}
+#endif  // !defined(_WIN32)
+
 TEST_F(ConnectionImplFileUploadTest, UploadFileResumableOffsetLargerThanSize) {
   auto connection = MakeConnection();
   auto const contents = std::string{"some resumable contents"};
diff --git a/google/cloud/storage/internal/storage_connection_test.cc b/google/cloud/storage/internal/storage_connection_test.cc
index 83135beb45dad..6e56c82a1dbdb 100644
--- a/google/cloud/storage/internal/storage_connection_test.cc
+++ b/google/cloud/storage/internal/storage_connection_test.cc
@@ -30,6 +30,7 @@ using ::google::cloud::testing_util::StatusIs;
 class TestStorageConnection : public StorageConnection {
  public:
   ~TestStorageConnection() override = default;
+  // LCOV_EXCL_START
   MOCK_METHOD(ClientOptions const&, client_options, (), (const, override));
   MOCK_METHOD(StatusOr<ListBucketsResponse>, ListBuckets,
               (ListBucketsRequest const&), (override));
@@ -142,6 +143,7 @@ class TestStorageConnection : public StorageConnection {
               (GetNotificationRequest const&), (override));
   MOCK_METHOD(StatusOr<EmptyResponse>, DeleteNotification,
               (DeleteNotificationRequest const&), (override));
+  // LCOV_EXCL_STOP
 };
 
 TEST(StorageConnectionTest, UploadFileSimpleUnimplemented) {

From 4524398fcc4dd296f1b3e51db1ac46b919896d65 Mon Sep 17 00:00:00 2001
From: Shubham Kaushal <shubhamkaushal@google.com>
Date: Fri, 11 Jul 2025 16:49:18 +0000
Subject: [PATCH 10/10] resolving comments

---
 google/cloud/storage/client_object_test.cc    | 12 ++--
 .../connection_impl_file_upload_test.cc       | 56 ++++++-------------
 .../storage/internal/storage_connection.h     |  7 +--
 3 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/google/cloud/storage/client_object_test.cc b/google/cloud/storage/client_object_test.cc
index 35fae9f255af0..ab743c2aafda4 100644
--- a/google/cloud/storage/client_object_test.cc
+++ b/google/cloud/storage/client_object_test.cc
@@ -258,9 +258,9 @@ TEST_F(ObjectTest, UploadFileSimple) {
   std::string text = R"""({
       "name": "test-bucket-name/test-object-name/1"
 })""";
-  auto expected =
+  ObjectMetadata expected =
       storage::internal::ObjectMetadataParser::FromString(text).value();
-  auto const contents = std::string{"some simple contents"};
+  std::string const contents = std::string{"some simple contents"};
 
   EXPECT_CALL(*mock_, InsertObjectMedia)
       .WillOnce([&](internal::InsertObjectMediaRequest const& request) {
@@ -272,7 +272,7 @@ TEST_F(ObjectTest, UploadFileSimple) {
 
   TempFile temp(contents);
   auto client = ClientForMock();
-  auto actual =
+  StatusOr<ObjectMetadata> actual =
       client.UploadFile(temp.name(), "test-bucket-name", "test-object-name");
   ASSERT_STATUS_OK(actual);
   EXPECT_EQ(expected, *actual);
@@ -282,9 +282,9 @@ TEST_F(ObjectTest, UploadFileResumable) {
   std::string text = R"""({
       "name": "test-bucket-name/test-object-name/1"
 })""";
-  auto expected =
+  ObjectMetadata expected =
       storage::internal::ObjectMetadataParser::FromString(text).value();
-  auto const contents = std::string{"some not so simple contents"};
+  std::string const contents = std::string{"some not so simple contents"};
 
   EXPECT_CALL(*mock_, CreateResumableUpload)
       .WillOnce(
@@ -295,7 +295,7 @@ TEST_F(ObjectTest, UploadFileResumable) {
 
   TempFile temp(contents);
   auto client = ClientForMock();
-  auto actual =
+  StatusOr<ObjectMetadata> actual =
       client.UploadFile(temp.name(), "test-bucket-name", "test-object-name",
                         UseResumableUploadSession(""));
   ASSERT_STATUS_OK(actual);
diff --git a/google/cloud/storage/internal/connection_impl_file_upload_test.cc b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
index 619c175459e9d..208f394a4c4b9 100644
--- a/google/cloud/storage/internal/connection_impl_file_upload_test.cc
+++ b/google/cloud/storage/internal/connection_impl_file_upload_test.cc
@@ -18,9 +18,6 @@
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <iterator>
-#if !defined(_WIN32)
-#include <sys/stat.h>
-#endif  // !defined(_WIN32)
 
 namespace google {
 namespace cloud {
@@ -44,8 +41,8 @@ class ConnectionImplFileUploadTest : public ::testing::Test {
 };
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleSuccess) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some simple contents"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"some simple contents"};
   TempFile temp(contents);
   InsertObjectMediaRequest request("test-bucket", "test-object", "");
   auto payload =
@@ -55,7 +52,7 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleSuccess) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleNonExistentFile) {
-  auto connection = MakeConnection();
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
   InsertObjectMediaRequest request("test-bucket", "test-object", "");
   auto payload =
       connection->UploadFileSimple("/no/such/file/exists.txt", 123, request);
@@ -63,8 +60,8 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleNonExistentFile) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleOffsetLargerThanSize) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some simple contents"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"some simple contents"};
   TempFile temp(contents);
   InsertObjectMediaRequest request("test-bucket", "test-object", "");
   request.set_option(UploadFromOffset(contents.size() + 1));
@@ -74,8 +71,8 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleOffsetLargerThanSize) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleWithLimit) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"0123456789"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"0123456789"};
   TempFile temp(contents);
   InsertObjectMediaRequest request("test-bucket", "test-object", "");
   request.set_option(UploadLimit(5));
@@ -86,8 +83,8 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleWithLimit) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleReadError) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some simple contents"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"some simple contents"};
   TempFile temp(contents);
   // Pass a size larger than the file to trigger a read error.
   InsertObjectMediaRequest request("test-bucket", "test-object", "");
@@ -97,8 +94,8 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileSimpleReadError) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileResumableSuccess) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some resumable contents"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"some resumable contents"};
   TempFile temp(contents);
   ResumableUploadRequest request("test-bucket", "test-object");
   auto stream = connection->UploadFileResumable(temp.name(), request);
@@ -110,35 +107,16 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileResumableSuccess) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileResumableNonExistentFile) {
-  auto connection = MakeConnection();
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
   ResumableUploadRequest request("test-bucket", "test-object");
   auto stream =
       connection->UploadFileResumable("/no/such/file/exists.txt", request);
   EXPECT_THAT(stream, StatusIs(StatusCode::kNotFound));
 }
 
-#if !defined(_WIN32)
-TEST_F(ConnectionImplFileUploadTest, UploadFileResumableUnreadable) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some resumable contents"};
-  TempFile temp(contents);
-
-  // Make the file unreadable.
-  ASSERT_EQ(0, chmod(temp.name().c_str(), 0000));
-
-  // The implementation should detect that it cannot open the file.
-  ResumableUploadRequest request("test-bucket", "test-object");
-  auto stream = connection->UploadFileResumable(temp.name(), request);
-  EXPECT_THAT(stream, StatusIs(StatusCode::kNotFound));
-
-  // Restore permissions so the TempFile destructor can delete it.
-  (void)chmod(temp.name().c_str(), 0600);
-}
-#endif  // !defined(_WIN32)
-
 TEST_F(ConnectionImplFileUploadTest, UploadFileResumableOffsetLargerThanSize) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"some resumable contents"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"some resumable contents"};
   TempFile temp(contents);
   ResumableUploadRequest request("test-bucket", "test-object");
   request.set_option(UploadFromOffset(contents.size() + 1));
@@ -147,11 +125,11 @@ TEST_F(ConnectionImplFileUploadTest, UploadFileResumableOffsetLargerThanSize) {
 }
 
 TEST_F(ConnectionImplFileUploadTest, UploadFileResumableWithLimit) {
-  auto connection = MakeConnection();
-  auto const contents = std::string{"0123456789"};
+  std::shared_ptr<StorageConnectionImpl> connection = MakeConnection();
+  std::string const contents = std::string{"0123456789"};
   TempFile temp(contents);
   ResumableUploadRequest request("test-bucket", "test-object");
-  auto const limit = 5;
+  int const limit = 5;
   request.set_option(UploadLimit(limit));
   auto stream = connection->UploadFileResumable(temp.name(), request);
   ASSERT_STATUS_OK(stream);
diff --git a/google/cloud/storage/internal/storage_connection.h b/google/cloud/storage/internal/storage_connection.h
index 9f754a3019cee..e33638fad695f 100644
--- a/google/cloud/storage/internal/storage_connection.h
+++ b/google/cloud/storage/internal/storage_connection.h
@@ -111,14 +111,11 @@ class StorageConnection {
   virtual StatusOr<QueryResumableUploadResponse> UploadChunk(
       UploadChunkRequest const& request) = 0;
   virtual StatusOr<std::unique_ptr<std::string>> UploadFileSimple(
-      [[maybe_unused]] std::string const& file_name,
-      [[maybe_unused]] std::size_t file_size,
-      [[maybe_unused]] InsertObjectMediaRequest& request) {
+      std::string const&, std::size_t, InsertObjectMediaRequest&) {
     return Status(StatusCode::kUnimplemented, "unimplemented");
   }
   virtual StatusOr<std::unique_ptr<std::istream>> UploadFileResumable(
-      [[maybe_unused]] std::string const& file_name,
-      [[maybe_unused]] ResumableUploadRequest& request) {
+      std::string const&, ResumableUploadRequest&) {
     return Status(StatusCode::kUnimplemented, "unimplemented");
   }
   ///@}