fix(storage): preserve metadata in ReadAll() for 0-byte objects

[v-pratap]

Summary

storage::ReadAll(), and therefore AsyncClient::ReadObjectRange(), dropped the object metadata (ReadPayload::metadata() == std::nullopt) when reading a 0-byte object. This happened even though the service sends the full google.storage.v2.Object metadata in the first ReadObjectResponse.

Root cause

ReadPayloadImpl::Accumulate(lhs, rhs) had a fast path: "if lhs has no data, replace lhs with rhs". It only looked at the data, not the metadata or the other fields.

For a 0-byte object, ReadAll() receives:

1. The first payload: empty contents with metadata. accumulated_ is replaced by this payload, so it now holds the metadata.
2. The end-of-stream marker, which AsyncReader::Read() synthesizes as ReadPayload{} (no data, no metadata). Because accumulated_ still has no data, the fast path fires again and overwrites the metadata.

Objects with data are not affected: after the first chunk, lhs is no longer empty, so later chunks only append data.

Fix

The fast path now only replaces lhs when it is fully default-constructed, meaning it has no data, no metadata, no headers, no object hashes, and offset_ == 0:

-    if (lhs.impl_.empty()) {
+    if (IsDefault(lhs)) {

  static bool IsDefault(storage::ReadPayload const& p) {
    return p.impl_.empty() && p.offset_ == 0 && !p.metadata_.has_value() &&
           p.headers_.empty() && !p.object_hash_values_.has_value();
  }

This also covers an empty first payload followed by payloads with data: the metadata, headers, offset, and object hashes from the first payload are kept.

Accumulate() has exactly one caller (ReadAll()), so no other code paths change behavior.

Tests

New tests:

• read_payload_impl_test.cc: direct tests for Accumulate(), which had none before:
  • AccumulateIntoDefault, AccumulateAppendsData, AccumulateEmptyIntoEmpty (existing behavior)
  • AccumulateEmptyWithMetadataKeepsMetadata, AccumulateEmptyWithMetadataThenData, AccumulateEmptyWithoutMetadataKeepsOtherFields (regressions)
• read_all_test.cc: ZeroBytePayloadPreservesMetadata, EmptyPayloadWithMetadataThenData
• connection_impl_read_test.cc: ReadObjectRangeZeroByteObjectKeepsMetadata, an end-to-end test through the real ReadObjectRange() -> resume -> reader -> ReadAll() stack with a mocked gRPC stream returning a 0-byte object.

Results

Check	Result
New tests with the fix	✅ pass
New tests without the fix (read_payload_impl.h reverted to main)	❌ exactly the 6 regression tests fail; all other tests pass
All //google/cloud/storage unit tests (Bazel, integration tests excluded)	✅ 175 / 175 pass
clang-format on changed files	✅ clean

Manual verification against production GCS

I ran a small program using AsyncClient against a real regional bucket with hierarchical namespace enabled. For each of a 0-byte and an 11-byte object, it creates the object, reads it back with ReadObjectRange() and ReadAll(ReadObject()), checks the returned metadata, and deletes the object.

Read	With fix	Without fix
0-byte object, ReadObjectRange(bucket, name, 0, 0)	✅ metadata present, correct generation, size=0	❌ metadata missing (nullopt)
0-byte object, ReadAll(ReadObject(bucket, name))	✅ metadata present, correct generation	❌ metadata missing (nullopt)
11-byte object (control), both reads	✅ metadata present	✅ metadata present

Output with the fix:

0-byte object:
  [ReadObjectRange(0, 0)] status=OK bytes=0 metadata=present generation=1790844817631271 size=0  OK
  [ReadAll(ReadObject)] status=OK bytes=0 metadata=present generation=1790844817631271 size=0  OK

11-byte object (control, should always work):
  [ReadObjectRange(0, 0)] status=OK bytes=11 metadata=present generation=1790844817917947 size=11  OK
  [ReadAll(ReadObject)] status=OK bytes=11 metadata=present generation=1790844817917947 size=11  OK

RESULT: PASS

Output without the fix:

0-byte object:
  [ReadObjectRange(0, 0)] status=OK bytes=0 metadata=MISSING (nullopt)  <-- BUG
  [ReadAll(ReadObject)] status=OK bytes=0 metadata=MISSING (nullopt)  <-- BUG

11-byte object (control, should always work):
  [ReadObjectRange(0, 0)] status=OK bytes=11 metadata=present generation=1790585932937576 size=11  OK
  [ReadAll(ReadObject)] status=OK bytes=11 metadata=present generation=1790585932937576 size=11  OK

RESULT: FAIL

Internal bug: b/565852217

[gemini-code-assist]

Code Review

This pull request ensures that object metadata is preserved when reading 0-byte objects or when an empty payload containing metadata is followed by data. It updates ReadPayloadImpl::Accumulate to only replace the left-hand side payload if it contains neither data nor metadata, and adds comprehensive unit tests to verify this behavior. Feedback on the changes suggests extending this safety check in Accumulate to also verify other fields like headers, offset, and hashes to prevent them from being discarded if they are present in an otherwise empty payload.

[codecov]

Codecov Report

❌ Patch coverage is 99.31973% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.37%. Comparing base (b67a379) to head (1d6da94).

Files with missing lines	Patch %	Lines
google/cloud/storage/async/read_all_test.cc	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16499      +/-   ##
==========================================
- Coverage   92.37%   92.37%   -0.01%     
==========================================
  Files        2262     2262              
  Lines      217100   217246     +146     
==========================================
+ Hits       200540   200671     +131     
- Misses      16560    16575      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kalragauri]

nit: in the PR description, consider removing the notes about the first commit vs. the review update, since they won't make sense after the squash-merge.