From a8f1dd184c23ca03dc8b235f67a72af9185301ed Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Wed, 11 Feb 2026 05:47:23 +0000 Subject: [PATCH 1/7] Part 1: Core get/set changes for object contexts --- .../storage/google_cloud_cpp_storage.bzl | 2 + .../storage/google_cloud_cpp_storage.cmake | 2 + .../internal/object_metadata_parser.cc | 55 +++++++++++ google/cloud/storage/object_contexts.cc | 46 +++++++++ google/cloud/storage/object_contexts.h | 94 +++++++++++++++++++ google/cloud/storage/object_metadata.cc | 6 ++ google/cloud/storage/object_metadata.h | 25 +++++ google/cloud/storage/object_metadata_test.cc | 67 +++++++++++++ .../object_basic_crud_integration_test.cc | 49 ++++++++++ 9 files changed, 346 insertions(+) create mode 100644 google/cloud/storage/object_contexts.cc create mode 100644 google/cloud/storage/object_contexts.h diff --git a/google/cloud/storage/google_cloud_cpp_storage.bzl b/google/cloud/storage/google_cloud_cpp_storage.bzl index 7f244903705ab..ed40ddb04ab5f 100644 --- a/google/cloud/storage/google_cloud_cpp_storage.bzl +++ b/google/cloud/storage/google_cloud_cpp_storage.bzl @@ -116,6 +116,7 @@ google_cloud_cpp_storage_hdrs = [ "notification_metadata.h", "notification_payload_format.h", "object_access_control.h", + "object_contexts.h", "object_metadata.h", "object_read_stream.h", "object_retention.h", @@ -219,6 +220,7 @@ google_cloud_cpp_storage_srcs = [ "list_objects_reader.cc", "notification_metadata.cc", "object_access_control.cc", + "object_contexts.cc", "object_metadata.cc", "object_read_stream.cc", "object_retention.cc", diff --git a/google/cloud/storage/google_cloud_cpp_storage.cmake b/google/cloud/storage/google_cloud_cpp_storage.cmake index f24107bca2cd9..f3d70b6766817 100644 --- a/google/cloud/storage/google_cloud_cpp_storage.cmake +++ b/google/cloud/storage/google_cloud_cpp_storage.cmake @@ -197,6 +197,8 @@ add_library( notification_payload_format.h object_access_control.cc object_access_control.h + object_contexts.cc + object_contexts.h object_metadata.cc object_metadata.h object_read_stream.cc diff --git a/google/cloud/storage/internal/object_metadata_parser.cc b/google/cloud/storage/internal/object_metadata_parser.cc index 4f5e7efa20795..7e56c2c9872dc 100644 --- a/google/cloud/storage/internal/object_metadata_parser.cc +++ b/google/cloud/storage/internal/object_metadata_parser.cc @@ -44,6 +44,29 @@ void SetIfNotEmpty(nlohmann::json& json, char const* key, json[key] = value; } +/** + * Populates the "contexts" field in the JSON object from the given metadata. + */ +void SetJsonContextsIfNotEmpty(nlohmann::json& json, + ObjectMetadata const& meta) { + if (!meta.has_contexts()) { + return; + } + + nlohmann::json custom_json; + for (auto const& kv : meta.contexts().custom) { + custom_json[kv.first] = nlohmann::json{ + {"value", kv.second.value}, + {"createTime", + google::cloud::internal::FormatRfc3339(kv.second.create_time)}, + {"updateTime", + google::cloud::internal::FormatRfc3339(kv.second.update_time)}, + }; + } + + json["contexts"] = nlohmann::json{{"custom", std::move(custom_json)}}; +} + Status ParseAcl(ObjectMetadata& meta, nlohmann::json const& json) { auto i = json.find("acl"); if (i == json.end()) return Status{}; @@ -160,6 +183,33 @@ Status ParseRetention(ObjectMetadata& meta, nlohmann::json const& json) { return Status{}; } +Status ParseContexts(ObjectMetadata& meta, nlohmann::json const& json) { + auto f_contexts = json.find("contexts"); + if (f_contexts == json.end()) return Status{}; + + auto f_custom = f_contexts->find("custom"); + if (f_custom == f_contexts->end()) return Status{}; + + ObjectContexts contexts; + for (auto const& kv : f_custom->items()) { + ObjectCustomContextPayload payload; + auto value = kv.value().value("value", ""); + payload.value = value; + + auto create_time = internal::ParseTimestampField(kv.value(), "createTime"); + if (!create_time) return std::move(create_time).status(); + payload.create_time = *create_time; + + auto update_time = internal::ParseTimestampField(kv.value(), "updateTime"); + if (!update_time) return std::move(update_time).status(); + payload.update_time = *update_time; + + contexts.custom.emplace(kv.key(), std::move(payload)); + } + meta.set_contexts(std::move(contexts)); + return Status{}; +} + Status ParseSize(ObjectMetadata& meta, nlohmann::json const& json) { auto v = internal::ParseUnsignedLongField(json, "size"); if (!v) return std::move(v).status(); @@ -296,6 +346,7 @@ StatusOr ObjectMetadataParser::FromJson( ParseOwner, ParseRetentionExpirationTime, ParseRetention, + ParseContexts, [](ObjectMetadata& meta, nlohmann::json const& json) { return SetStringField(meta, json, "selfLink", &ObjectMetadata::set_self_link); @@ -372,6 +423,8 @@ nlohmann::json ObjectMetadataJsonForCompose(ObjectMetadata const& meta) { meta.retention().retain_until_time)}}; } + SetJsonContextsIfNotEmpty(metadata_as_json, meta); + return metadata_as_json; } @@ -430,6 +483,8 @@ nlohmann::json ObjectMetadataJsonForUpdate(ObjectMetadata const& meta) { meta.retention().retain_until_time)}}; } + SetJsonContextsIfNotEmpty(metadata_as_json, meta); + return metadata_as_json; } diff --git a/google/cloud/storage/object_contexts.cc b/google/cloud/storage/object_contexts.cc new file mode 100644 index 0000000000000..47253a9826b44 --- /dev/null +++ b/google/cloud/storage/object_contexts.cc @@ -0,0 +1,46 @@ +// Copyright 2026 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/object_contexts.h" +#include "google/cloud/internal/format_time_point.h" +#include + +namespace google { +namespace cloud { +namespace storage { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +std::ostream& operator<<(std::ostream& os, + ObjectCustomContextPayload const& rhs) { + return os << "ObjectCustomContextPayload={value=" << rhs.value + << ", create_time=" + << google::cloud::internal::FormatRfc3339(rhs.create_time) + << ", update_time=" + << google::cloud::internal::FormatRfc3339(rhs.update_time) << "}"; +} + +std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs) { + os << "ObjectContexts={custom={"; + char const* sep = ""; + for (auto const& kv : rhs.custom) { + os << sep << kv.first << "=" << kv.second; + sep = ",\n"; + } + return os << "}}"; +} + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace storage +} // namespace cloud +} // namespace google diff --git a/google/cloud/storage/object_contexts.h b/google/cloud/storage/object_contexts.h new file mode 100644 index 0000000000000..bff11421d627c --- /dev/null +++ b/google/cloud/storage/object_contexts.h @@ -0,0 +1,94 @@ +// Copyright 2026 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. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_OBJECT_CONTEXTS_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_OBJECT_CONTEXTS_H + +#include "google/cloud/storage/version.h" +#include +#include +#include + +namespace google { +namespace cloud { +namespace storage { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +/** + * Represents the payload of a user-defined object context. + */ +struct ObjectCustomContextPayload { + std::string value; + + std::chrono::system_clock::time_point create_time; + + std::chrono::system_clock::time_point update_time; +}; + +inline bool operator==(ObjectCustomContextPayload const& lhs, + ObjectCustomContextPayload const& rhs) { + return std::tie(lhs.value, lhs.create_time, lhs.update_time) == + std::tie(rhs.value, rhs.create_time, rhs.update_time); +}; + +inline bool operator!=(ObjectCustomContextPayload const& lhs, + ObjectCustomContextPayload const& rhs) { + return !(lhs == rhs); +} + +std::ostream& operator<<(std::ostream& os, + ObjectCustomContextPayload const& rhs); + +/** + * Specifies the custom contexts of an object. + */ +struct ObjectContexts { + /** + * Represents the map of user-defined object contexts, keyed by a string + * value. + */ + std::map custom; + + /** + * A set of helper functions to handle the custom. + */ + bool has_custom(std::string const& key) const { + return custom.end() != custom.find(key); + } + ObjectCustomContextPayload const& get_custom(std::string const& key) const { + return custom.at(key); + } + void upsert_custom(std::string const& key, + ObjectCustomContextPayload const& value) { + custom[key] = value; + } + void delete_custom(std::string const& key) { custom.erase(key); } +}; + +inline bool operator==(ObjectContexts const& lhs, ObjectContexts const& rhs) { + return lhs.custom == rhs.custom; +}; + +inline bool operator!=(ObjectContexts const& lhs, ObjectContexts const& rhs) { + return !(lhs == rhs); +} + +std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs); + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace storage +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_OBJECT_CONTEXTS_H diff --git a/google/cloud/storage/object_metadata.cc b/google/cloud/storage/object_metadata.cc index 68e18262ce9d9..171324dfd5064 100644 --- a/google/cloud/storage/object_metadata.cc +++ b/google/cloud/storage/object_metadata.cc @@ -73,6 +73,7 @@ bool operator==(ObjectMetadata const& lhs, ObjectMetadata const& rhs) { && lhs.updated_ == rhs.updated_ // && lhs.soft_delete_time_ == rhs.soft_delete_time_ // && lhs.hard_delete_time_ == rhs.hard_delete_time_ // + && lhs.contexts_ == rhs.contexts_ // ; } @@ -133,6 +134,11 @@ std::ostream& operator<<(std::ostream& os, ObjectMetadata const& rhs) { if (rhs.has_hard_delete_time()) { os << ", hard_delete_time=" << FormatRfc3339(rhs.hard_delete_time()); } + + if (rhs.has_contexts()) { + os << ", contexts=" << rhs.contexts(); + } + return os << "}"; } diff --git a/google/cloud/storage/object_metadata.h b/google/cloud/storage/object_metadata.h index bc40bb6706899..a3266fbd0c9b5 100644 --- a/google/cloud/storage/object_metadata.h +++ b/google/cloud/storage/object_metadata.h @@ -17,6 +17,7 @@ #include "google/cloud/storage/internal/complex_option.h" #include "google/cloud/storage/object_access_control.h" +#include "google/cloud/storage/object_contexts.h" #include "google/cloud/storage/object_retention.h" #include "google/cloud/storage/owner.h" #include "google/cloud/storage/version.h" @@ -450,6 +451,29 @@ class ObjectMetadata { return *this; } + /// Returns `true` if the object has user-defined contexts. + bool has_contexts() const { return contexts_.has_value(); } + + /** + * The object's user custom contexts. + * + * It is undefined behavior to call this member function if + * `has_contexts() == false`. + */ + ObjectContexts const& contexts() const { return *contexts_; } + + /// Change or set the object's custom contexts. + ObjectMetadata& set_contexts(ObjectContexts v) { + contexts_ = std::move(v); + return *this; + } + + /// Reset the object's custom contexts. + ObjectMetadata& reset_contexts() { + contexts_.reset(); + return *this; + } + /// An HTTPS link to the object metadata. std::string const& self_link() const { return self_link_; } @@ -612,6 +636,7 @@ class ObjectMetadata { std::string md5_hash_; std::string media_link_; std::map metadata_; + absl::optional contexts_; std::string name_; absl::optional owner_; std::chrono::system_clock::time_point retention_expiration_time_; diff --git a/google/cloud/storage/object_metadata_test.cc b/google/cloud/storage/object_metadata_test.cc index 1cc291cb4cc90..6f94fe2d66529 100644 --- a/google/cloud/storage/object_metadata_test.cc +++ b/google/cloud/storage/object_metadata_test.cc @@ -120,6 +120,15 @@ ObjectMetadata CreateObjectMetadataForTest() { "mode": "Unlocked", "retainUntilTime": "2024-07-18T00:00:00Z" }, + "contexts": { + "custom": { + "environment": { + "value": "prod", + "createTime": "2024-07-18T00:00:00Z", + "updateTime": "2024-07-18T00:00:00Z" + } + } + }, "selfLink": "https://storage.googleapis.com/storage/v1/b/foo-bar/o/baz", "size": 102400, "storageClass": "STANDARD", @@ -207,6 +216,14 @@ TEST(ObjectMetadataTest, Parse) { EXPECT_EQ(actual.hard_delete_time(), std::chrono::system_clock::from_time_t(1710160496L) + std::chrono::milliseconds(789)); + ASSERT_TRUE(actual.has_contexts()); + EXPECT_EQ( + actual.contexts().custom.at("environment"), + (ObjectCustomContextPayload{ + "prod", + google::cloud::internal::ParseRfc3339("2024-07-18T00:00:00Z").value(), + google::cloud::internal::ParseRfc3339("2024-07-18T00:00:00Z") + .value()})); } /// @test Verify that the IOStream operator works as expected. @@ -267,6 +284,11 @@ TEST(ObjectMetadataTest, JsonForCompose) { {"customTime", "2020-08-10T12:34:56Z"}, {"retention", {{"mode", "Unlocked"}, {"retainUntilTime", "2024-07-18T00:00:00Z"}}}, + {"contexts", + {{"custom", nlohmann::json{{"environment", + {{"createTime", "2024-07-18T00:00:00Z"}, + {"updateTime", "2024-07-18T00:00:00Z"}, + {"value", "prod"}}}}}}}, }; EXPECT_EQ(expected, actual) << "diff=" << nlohmann::json::diff(expected, actual); @@ -306,7 +328,13 @@ TEST(ObjectMetadataTest, JsonForCopy) { {"customTime", "2020-08-10T12:34:56Z"}, {"retention", {{"mode", "Unlocked"}, {"retainUntilTime", "2024-07-18T00:00:00Z"}}}, + {"contexts", + {{"custom", nlohmann::json{{"environment", + {{"createTime", "2024-07-18T00:00:00Z"}, + {"updateTime", "2024-07-18T00:00:00Z"}, + {"value", "prod"}}}}}}}, }; + EXPECT_EQ(expected, actual) << "diff=" << nlohmann::json::diff(expected, actual); } @@ -348,6 +376,11 @@ TEST(ObjectMetadataTest, JsonForInsert) { {"customTime", "2020-08-10T12:34:56Z"}, {"retention", {{"mode", "Unlocked"}, {"retainUntilTime", "2024-07-18T00:00:00Z"}}}, + {"contexts", + {{"custom", nlohmann::json{{"environment", + {{"createTime", "2024-07-18T00:00:00Z"}, + {"updateTime", "2024-07-18T00:00:00Z"}, + {"value", "prod"}}}}}}}, }; EXPECT_EQ(expected, actual) << "diff=" << nlohmann::json::diff(expected, actual); @@ -388,6 +421,11 @@ TEST(ObjectMetadataTest, JsonForRewrite) { {"customTime", "2020-08-10T12:34:56Z"}, {"retention", {{"mode", "Unlocked"}, {"retainUntilTime", "2024-07-18T00:00:00Z"}}}, + {"contexts", + {{"custom", nlohmann::json{{"environment", + {{"createTime", "2024-07-18T00:00:00Z"}, + {"updateTime", "2024-07-18T00:00:00Z"}, + {"value", "prod"}}}}}}}, }; EXPECT_EQ(expected, actual) << "diff=" << nlohmann::json::diff(expected, actual); @@ -429,6 +467,11 @@ TEST(ObjectMetadataTest, JsonForUpdate) { {"customTime", "2020-08-10T12:34:56Z"}, {"retention", {{"mode", "Unlocked"}, {"retainUntilTime", "2024-07-18T00:00:00Z"}}}, + {"contexts", + {{"custom", nlohmann::json{{"environment", + {{"createTime", "2024-07-18T00:00:00Z"}, + {"updateTime", "2024-07-18T00:00:00Z"}, + {"value", "prod"}}}}}}}, }; EXPECT_EQ(expected, actual) << "diff=" << nlohmann::json::diff(expected, actual); @@ -645,6 +688,30 @@ TEST(ObjectMetadataTest, ResetRetention) { EXPECT_NE(expected, copy); } +/// @test Verify we can change the `contexts` field. +TEST(ObjectMetadataTest, SetContexts) { + auto const expected = CreateObjectMetadataForTest(); + auto copy = expected; + auto const context_payload = ObjectCustomContextPayload{"engineering", {}, {}}; + std::map custom{ + {"department", context_payload}}; + auto const contexts = ObjectContexts{custom}; + copy.set_contexts(contexts); + EXPECT_TRUE(expected.has_contexts()); + EXPECT_TRUE(copy.has_contexts()); + EXPECT_EQ(contexts, copy.contexts()); + EXPECT_NE(expected, copy); +} + +/// @test Verify we can reset the `contexts` field. +TEST(ObjectMetadataTest, ResetContexts) { + auto const expected = CreateObjectMetadataForTest(); + auto copy = expected; + copy.reset_contexts(); + EXPECT_FALSE(copy.has_contexts()); + EXPECT_NE(expected, copy); +} + TEST(ObjectMetadataPatchBuilder, SetAcl) { ObjectMetadataPatchBuilder builder; builder.SetAcl({internal::ObjectAccessControlParser::FromString( diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index a1c6e454f47c3..c356d7e793696 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -31,6 +31,13 @@ #include #include +namespace { +// Helper function to check if a time point is set (i.e. not the default value). +bool IsSet(std::chrono::system_clock::time_point tp) { + return tp != std::chrono::system_clock::time_point{}; +} +} + namespace google { namespace cloud { namespace storage { @@ -60,6 +67,32 @@ struct ObjectBasicCRUDIntegrationTest } return MakeIntegrationTestClient(std::move(options)); } + + void SetUp() override { + // 1. Run the base class SetUp first. This initializes 'bucket_name_' + // from the environment variable. + ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); + + // 2. Create a client to interact with the emulator/backend. + auto client = MakeIntegrationTestClient(); + + // 3. Check if the bucket exists. + auto metadata = client.GetBucketMetadata(bucket_name_); + + // 4. If it's missing (kNotFound), create it. + if (metadata.status().code() == StatusCode::kNotFound) { + // Use a default project ID if the env var isn't set (common in local emulators). + auto project_id = + google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT").value_or("test-project"); + + auto created = client.CreateBucketForProject( + bucket_name_, project_id, BucketMetadata()); + ASSERT_STATUS_OK(created) << "Failed to auto-create missing bucket: " << bucket_name_; + } else { + // If it exists (or failed for another reason), assert it is OK. + ASSERT_STATUS_OK(metadata) << "Failed to verify bucket existence: " << bucket_name_; + } + } }; /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations. @@ -104,6 +137,16 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { .set_content_language("en") .set_content_type("plain/text"); update.mutable_metadata().emplace("updated", "true"); + + // Adds custom contexts with value only. + ObjectContexts contexts; + ObjectCustomContextPayload payload; + payload.value = "test-custom-value"; + contexts.upsert_custom("test-key", payload); + update.set_contexts(contexts); + EXPECT_TRUE(!IsSet(update.contexts().get_custom("test-key").update_time)); + EXPECT_TRUE(!IsSet(update.contexts().get_custom("test-key").create_time)); + StatusOr updated_meta = client.UpdateObject( bucket_name_, object_name, update, Projection("full")); ASSERT_STATUS_OK(updated_meta); @@ -135,6 +178,12 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { EXPECT_EQ(update.content_type(), updated_meta->content_type()) << *updated_meta; EXPECT_EQ(update.metadata(), updated_meta->metadata()) << *updated_meta; + // Verify the custom contexts are updated correctly with valid timestamps. + EXPECT_EQ( + "test-custom-value", + updated_meta->contexts().get_custom("test-key").value) << *updated_meta; + EXPECT_TRUE(IsSet(updated_meta->contexts().get_custom("test-key").update_time)) << *updated_meta; + EXPECT_TRUE(IsSet(updated_meta->contexts().get_custom("test-key").create_time)) << *updated_meta; ObjectMetadata desired_patch = *updated_meta; desired_patch.set_content_language("en"); From 0991d6c8b05a3aa31de65b06fd552d223e710c52 Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Wed, 11 Feb 2026 08:28:40 +0000 Subject: [PATCH 2/7] remove set up code in object_basic_crud_integration_test --- .../object_basic_crud_integration_test.cc | 42 +++++-------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index c356d7e793696..390b228ce8232 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -36,7 +36,7 @@ namespace { bool IsSet(std::chrono::system_clock::time_point tp) { return tp != std::chrono::system_clock::time_point{}; } -} +} // namespace namespace google { namespace cloud { @@ -67,32 +67,6 @@ struct ObjectBasicCRUDIntegrationTest } return MakeIntegrationTestClient(std::move(options)); } - - void SetUp() override { - // 1. Run the base class SetUp first. This initializes 'bucket_name_' - // from the environment variable. - ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); - - // 2. Create a client to interact with the emulator/backend. - auto client = MakeIntegrationTestClient(); - - // 3. Check if the bucket exists. - auto metadata = client.GetBucketMetadata(bucket_name_); - - // 4. If it's missing (kNotFound), create it. - if (metadata.status().code() == StatusCode::kNotFound) { - // Use a default project ID if the env var isn't set (common in local emulators). - auto project_id = - google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT").value_or("test-project"); - - auto created = client.CreateBucketForProject( - bucket_name_, project_id, BucketMetadata()); - ASSERT_STATUS_OK(created) << "Failed to auto-create missing bucket: " << bucket_name_; - } else { - // If it exists (or failed for another reason), assert it is OK. - ASSERT_STATUS_OK(metadata) << "Failed to verify bucket existence: " << bucket_name_; - } - } }; /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations. @@ -179,11 +153,15 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { << *updated_meta; EXPECT_EQ(update.metadata(), updated_meta->metadata()) << *updated_meta; // Verify the custom contexts are updated correctly with valid timestamps. - EXPECT_EQ( - "test-custom-value", - updated_meta->contexts().get_custom("test-key").value) << *updated_meta; - EXPECT_TRUE(IsSet(updated_meta->contexts().get_custom("test-key").update_time)) << *updated_meta; - EXPECT_TRUE(IsSet(updated_meta->contexts().get_custom("test-key").create_time)) << *updated_meta; + EXPECT_EQ("test-custom-value", + updated_meta->contexts().get_custom("test-key").value) + << *updated_meta; + EXPECT_TRUE( + IsSet(updated_meta->contexts().get_custom("test-key").update_time)) + << *updated_meta; + EXPECT_TRUE( + IsSet(updated_meta->contexts().get_custom("test-key").create_time)) + << *updated_meta; ObjectMetadata desired_patch = *updated_meta; desired_patch.set_content_language("en"); From 5a9fcdec557c6f2753564660b7238988741c73c7 Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:10:18 +0000 Subject: [PATCH 3/7] Fix data structure and add integration test --- .../internal/object_metadata_parser.cc | 57 +++--- google/cloud/storage/object_contexts.cc | 17 +- google/cloud/storage/object_contexts.h | 70 +++++-- google/cloud/storage/object_metadata.cc | 2 - google/cloud/storage/object_metadata.h | 4 +- google/cloud/storage/object_metadata_test.cc | 18 +- .../object_basic_crud_integration_test.cc | 171 ++++++++++++++++-- 7 files changed, 261 insertions(+), 78 deletions(-) diff --git a/google/cloud/storage/internal/object_metadata_parser.cc b/google/cloud/storage/internal/object_metadata_parser.cc index 7e56c2c9872dc..71066daae9a5f 100644 --- a/google/cloud/storage/internal/object_metadata_parser.cc +++ b/google/cloud/storage/internal/object_metadata_parser.cc @@ -52,19 +52,25 @@ void SetJsonContextsIfNotEmpty(nlohmann::json& json, if (!meta.has_contexts()) { return; } - - nlohmann::json custom_json; - for (auto const& kv : meta.contexts().custom) { - custom_json[kv.first] = nlohmann::json{ - {"value", kv.second.value}, - {"createTime", - google::cloud::internal::FormatRfc3339(kv.second.create_time)}, - {"updateTime", - google::cloud::internal::FormatRfc3339(kv.second.update_time)}, - }; + if (meta.contexts().has_custom()) { + nlohmann::json custom_json; + for (auto const& kv : meta.contexts().custom()) { + if (kv.second.has_value()) { + nlohmann::json item; + item["value"] = kv.second.value().value; + item["createTime"] = google::cloud::internal::FormatRfc3339( + kv.second.value().create_time); + item["updateTime"] = google::cloud::internal::FormatRfc3339( + kv.second.value().update_time); + custom_json[kv.first] = std::move(item); + } else { + custom_json[kv.first] = nullptr; + } + } + json["contexts"] = nlohmann::json{{"custom", std::move(custom_json)}}; + } else { + json["contexts"] = nlohmann::json{{"custom", nullptr}}; } - - json["contexts"] = nlohmann::json{{"custom", std::move(custom_json)}}; } Status ParseAcl(ObjectMetadata& meta, nlohmann::json const& json) { @@ -192,19 +198,26 @@ Status ParseContexts(ObjectMetadata& meta, nlohmann::json const& json) { ObjectContexts contexts; for (auto const& kv : f_custom->items()) { - ObjectCustomContextPayload payload; - auto value = kv.value().value("value", ""); - payload.value = value; + if (kv.value().is_null()) { + contexts.upsert_custom_context(kv.key(), absl::nullopt); + + } else { + ObjectCustomContextPayload payload; + auto value = kv.value().value("value", ""); + payload.value = value; - auto create_time = internal::ParseTimestampField(kv.value(), "createTime"); - if (!create_time) return std::move(create_time).status(); - payload.create_time = *create_time; + auto create_time = + internal::ParseTimestampField(kv.value(), "createTime"); + if (!create_time) return std::move(create_time).status(); + payload.create_time = *create_time; - auto update_time = internal::ParseTimestampField(kv.value(), "updateTime"); - if (!update_time) return std::move(update_time).status(); - payload.update_time = *update_time; + auto update_time = + internal::ParseTimestampField(kv.value(), "updateTime"); + if (!update_time) return std::move(update_time).status(); + payload.update_time = *update_time; - contexts.custom.emplace(kv.key(), std::move(payload)); + contexts.upsert_custom_context(kv.key(), std::move(payload)); + } } meta.set_contexts(std::move(contexts)); return Status{}; diff --git a/google/cloud/storage/object_contexts.cc b/google/cloud/storage/object_contexts.cc index 47253a9826b44..181495442569a 100644 --- a/google/cloud/storage/object_contexts.cc +++ b/google/cloud/storage/object_contexts.cc @@ -32,10 +32,19 @@ std::ostream& operator<<(std::ostream& os, std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs) { os << "ObjectContexts={custom={"; - char const* sep = ""; - for (auto const& kv : rhs.custom) { - os << sep << kv.first << "=" << kv.second; - sep = ",\n"; + if (rhs.has_custom()) { + char const* sep = ""; + for (auto const& kv : rhs.custom()) { + os << sep << kv.first << "="; + if (kv.second.has_value()) { + os << kv.second.value(); + } else { + os << "null"; + } + sep = ",\n"; + } + } else { + os << "null"; } return os << "}}"; } diff --git a/google/cloud/storage/object_contexts.h b/google/cloud/storage/object_contexts.h index bff11421d627c..77cd7da03642f 100644 --- a/google/cloud/storage/object_contexts.h +++ b/google/cloud/storage/object_contexts.h @@ -16,6 +16,7 @@ #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_OBJECT_CONTEXTS_H #include "google/cloud/storage/version.h" +#include "absl/types/optional.h" #include #include #include @@ -54,36 +55,67 @@ std::ostream& operator<<(std::ostream& os, * Specifies the custom contexts of an object. */ struct ObjectContexts { + public: + // Returns true if the map itself exists. + bool has_custom() const { return custom_map_.has_value(); } + /** - * Represents the map of user-defined object contexts, keyed by a string - * value. + * Returns true if the map exists AND the key is present AND the value is + * a valid value. */ - std::map custom; + bool has_custom(std::string const& key) const { + if (!has_custom()) { + return false; + } + return custom_map_->find(key) != custom_map_->end() && + custom_map_->at(key).has_value(); + } /** - * A set of helper functions to handle the custom. + * The `custom` attribute of the object contexts. + * Values are now absl::optional. + * + * It is undefined behavior to call this member function if + * `has_custom() == false`. */ - bool has_custom(std::string const& key) const { - return custom.end() != custom.find(key); + std::map> const& + custom() const { + return *custom_map_; + } + + /** + * Upserts a context. Passing absl::nullopt for the value + * represents a "null" entry in the map. + */ + void upsert_custom_context(std::string const& key, + absl::optional value) { + if (!has_custom()) { + custom_map_.emplace(); + } + + (*custom_map_)[key] = std::move(value); } - ObjectCustomContextPayload const& get_custom(std::string const& key) const { - return custom.at(key); + + void reset_custom() { custom_map_.reset(); } + + bool operator==(ObjectContexts const& other) const { + return custom_map_ == other.custom_map_; } - void upsert_custom(std::string const& key, - ObjectCustomContextPayload const& value) { - custom[key] = value; + + bool operator!=(ObjectContexts const& other) const { + return !(*this == other); } - void delete_custom(std::string const& key) { custom.erase(key); } -}; -inline bool operator==(ObjectContexts const& lhs, ObjectContexts const& rhs) { - return lhs.custom == rhs.custom; + private: + /** + * Represents the map of user-defined object contexts. + * Inner optional allows keys to point to a "null" value. + */ + absl::optional< + std::map>> + custom_map_; }; -inline bool operator!=(ObjectContexts const& lhs, ObjectContexts const& rhs) { - return !(lhs == rhs); -} - std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs); GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/storage/object_metadata.cc b/google/cloud/storage/object_metadata.cc index 171324dfd5064..deecb96b0298c 100644 --- a/google/cloud/storage/object_metadata.cc +++ b/google/cloud/storage/object_metadata.cc @@ -134,11 +134,9 @@ std::ostream& operator<<(std::ostream& os, ObjectMetadata const& rhs) { if (rhs.has_hard_delete_time()) { os << ", hard_delete_time=" << FormatRfc3339(rhs.hard_delete_time()); } - if (rhs.has_contexts()) { os << ", contexts=" << rhs.contexts(); } - return os << "}"; } diff --git a/google/cloud/storage/object_metadata.h b/google/cloud/storage/object_metadata.h index a3266fbd0c9b5..c3ad0621ffee7 100644 --- a/google/cloud/storage/object_metadata.h +++ b/google/cloud/storage/object_metadata.h @@ -451,7 +451,7 @@ class ObjectMetadata { return *this; } - /// Returns `true` if the object has user-defined contexts. + /// Returns `true` if the object has custom contexts. bool has_contexts() const { return contexts_.has_value(); } /** @@ -468,7 +468,7 @@ class ObjectMetadata { return *this; } - /// Reset the object's custom contexts. + /// Reset the object contexts. ObjectMetadata& reset_contexts() { contexts_.reset(); return *this; diff --git a/google/cloud/storage/object_metadata_test.cc b/google/cloud/storage/object_metadata_test.cc index 6f94fe2d66529..8fd26eb9214b4 100644 --- a/google/cloud/storage/object_metadata_test.cc +++ b/google/cloud/storage/object_metadata_test.cc @@ -216,9 +216,9 @@ TEST(ObjectMetadataTest, Parse) { EXPECT_EQ(actual.hard_delete_time(), std::chrono::system_clock::from_time_t(1710160496L) + std::chrono::milliseconds(789)); - ASSERT_TRUE(actual.has_contexts()); + ASSERT_TRUE(actual.has_contexts() && actual.contexts().has_custom()); EXPECT_EQ( - actual.contexts().custom.at("environment"), + actual.contexts().custom().at("environment"), (ObjectCustomContextPayload{ "prod", google::cloud::internal::ParseRfc3339("2024-07-18T00:00:00Z").value(), @@ -692,19 +692,19 @@ TEST(ObjectMetadataTest, ResetRetention) { TEST(ObjectMetadataTest, SetContexts) { auto const expected = CreateObjectMetadataForTest(); auto copy = expected; - auto const context_payload = ObjectCustomContextPayload{"engineering", {}, {}}; - std::map custom{ - {"department", context_payload}}; - auto const contexts = ObjectContexts{custom}; + auto const context_payload = + ObjectCustomContextPayload{"engineering", {}, {}}; + ObjectContexts contexts; + contexts.upsert_custom_context("department", context_payload); copy.set_contexts(contexts); - EXPECT_TRUE(expected.has_contexts()); - EXPECT_TRUE(copy.has_contexts()); + EXPECT_TRUE(expected.contexts().has_custom()); + EXPECT_TRUE(copy.contexts().has_custom()); EXPECT_EQ(contexts, copy.contexts()); EXPECT_NE(expected, copy); } /// @test Verify we can reset the `contexts` field. -TEST(ObjectMetadataTest, ResetContexts) { +TEST(ObjectMetadataTest, DeleteContexts) { auto const expected = CreateObjectMetadataForTest(); auto copy = expected; copy.reset_contexts(); diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index 390b228ce8232..bd227869c4d01 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -67,6 +67,35 @@ struct ObjectBasicCRUDIntegrationTest } return MakeIntegrationTestClient(std::move(options)); } + + void SetUp() override { + // 1. Run the base class SetUp first. This initializes 'bucket_name_' + // from the environment variable. + ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); + + // 2. Create a client to interact with the emulator/backend. + auto client = MakeIntegrationTestClient(); + + // 3. Check if the bucket exists. + auto metadata = client.GetBucketMetadata(bucket_name_); + + // 4. If it's missing (kNotFound), create it. + if (metadata.status().code() == StatusCode::kNotFound) { + // Use a default project ID if the env var isn't set (common in local + // emulators). + auto project_id = google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT") + .value_or("test-project"); + + auto created = client.CreateBucketForProject(bucket_name_, project_id, + BucketMetadata()); + ASSERT_STATUS_OK(created) + << "Failed to auto-create missing bucket: " << bucket_name_; + } else { + // If it exists (or failed for another reason), assert it is OK. + ASSERT_STATUS_OK(metadata) + << "Failed to verify bucket existence: " << bucket_name_; + } + } }; /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations. @@ -100,6 +129,7 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { Projection("full")); ASSERT_STATUS_OK(get_meta); EXPECT_EQ(*get_meta, *insert_meta); + EXPECT_FALSE(insert_meta->has_contexts()) << *insert_meta; ObjectMetadata update = *get_meta; update.mutable_acl().emplace_back( @@ -111,16 +141,6 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { .set_content_language("en") .set_content_type("plain/text"); update.mutable_metadata().emplace("updated", "true"); - - // Adds custom contexts with value only. - ObjectContexts contexts; - ObjectCustomContextPayload payload; - payload.value = "test-custom-value"; - contexts.upsert_custom("test-key", payload); - update.set_contexts(contexts); - EXPECT_TRUE(!IsSet(update.contexts().get_custom("test-key").update_time)); - EXPECT_TRUE(!IsSet(update.contexts().get_custom("test-key").create_time)); - StatusOr updated_meta = client.UpdateObject( bucket_name_, object_name, update, Projection("full")); ASSERT_STATUS_OK(updated_meta); @@ -152,16 +172,6 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { EXPECT_EQ(update.content_type(), updated_meta->content_type()) << *updated_meta; EXPECT_EQ(update.metadata(), updated_meta->metadata()) << *updated_meta; - // Verify the custom contexts are updated correctly with valid timestamps. - EXPECT_EQ("test-custom-value", - updated_meta->contexts().get_custom("test-key").value) - << *updated_meta; - EXPECT_TRUE( - IsSet(updated_meta->contexts().get_custom("test-key").update_time)) - << *updated_meta; - EXPECT_TRUE( - IsSet(updated_meta->contexts().get_custom("test-key").create_time)) - << *updated_meta; ObjectMetadata desired_patch = *updated_meta; desired_patch.set_content_language("en"); @@ -183,6 +193,127 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { EXPECT_THAT(list_object_names(), Not(Contains(object_name))); } +/// @test Verify the Object CRUD operations with object contexts. +TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { + auto client = MakeIntegrationTestClient(); + + auto list_object_names = [&client, this] { + std::vector names; + for (auto o : client.ListObjects(bucket_name_)) { + EXPECT_STATUS_OK(o); + if (!o) break; + names.push_back(o->name()); + } + return names; + }; + + auto object_name = MakeRandomObjectName(); + ASSERT_THAT(list_object_names(), Not(Contains(object_name))) + << "Test aborted. The object <" << object_name << "> already exists." + << "This is unexpected as the test generates a random object name."; + + // Create the object, but only if it does not exist already, inserting + // a custom context {"department": "engineering"}. + ObjectContexts contexts; + ObjectCustomContextPayload payload; + payload.value = "engineering"; + contexts.upsert_custom_context("department", std::move(payload)); + StatusOr insert_meta = client.InsertObject( + bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), + Projection("full"), + WithObjectMetadata(ObjectMetadata().set_contexts(contexts))); + ASSERT_STATUS_OK(insert_meta); + EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); + + // Verify the response ObjectMetadata has the custom contexts we set. + StatusOr get_meta = client.GetObjectMetadata( + bucket_name_, object_name, Generation(insert_meta->generation()), + Projection("full")); + ASSERT_STATUS_OK(get_meta); + EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; + EXPECT_TRUE(get_meta->contexts().has_custom("department")) << *get_meta; + EXPECT_EQ("engineering", + get_meta->contexts().custom().at("department")->value) + << *get_meta; + EXPECT_TRUE( + IsSet(get_meta->contexts().custom().at("department")->update_time)) + << *get_meta; + EXPECT_TRUE( + IsSet(get_meta->contexts().custom().at("department")->create_time)) + << *get_meta; + + // Update object with a new value "engineering and research" for the existing + // custom context, and also add another custom context {"region": + // "Asia Pacific"}. + ObjectMetadata update = *get_meta; + ObjectContexts contexts_updated; + ObjectCustomContextPayload payload_updated; + payload_updated.value = "engineering and research"; + contexts_updated.upsert_custom_context("department", + std::move(payload_updated)); + ObjectCustomContextPayload payload_another; + payload_another.value = "Asia Pacific"; + contexts_updated.upsert_custom_context("region", std::move(payload_another)); + update.set_contexts(contexts_updated); + StatusOr updated_meta = client.UpdateObject( + bucket_name_, object_name, update, Projection("full")); + ASSERT_STATUS_OK(updated_meta); + + // Verify the response ObjectMetadata has the updated custom contexts. + EXPECT_TRUE(updated_meta->has_contexts()) << *updated_meta; + EXPECT_TRUE(updated_meta->contexts().has_custom("department")) + << *updated_meta; + EXPECT_EQ("engineering and research", + updated_meta->contexts().custom().at("department")->value) + << *updated_meta; + EXPECT_TRUE(updated_meta->contexts().has_custom("region")) << *updated_meta; + EXPECT_EQ("Asia Pacific", + updated_meta->contexts().custom().at("region")->value) + << *updated_meta; + EXPECT_TRUE( + IsSet(updated_meta->contexts().custom().at("region")->update_time)) + << *updated_meta; + EXPECT_TRUE( + IsSet(updated_meta->contexts().custom().at("region")->create_time)) + << *updated_meta; + + // Update object with deletion of the "department" custom context. + ObjectContexts contexts_deleted; + contexts_deleted.upsert_custom_context("department", absl::nullopt); + update.set_contexts(contexts_deleted); + StatusOr deleted_meta = client.UpdateObject( + bucket_name_, object_name, update, Projection("full")); + ASSERT_STATUS_OK(deleted_meta); + + // Verify the response ObjectMetadata has the "department" key is set to null, + // but the "region" key still present with value. + EXPECT_TRUE(deleted_meta->has_contexts()) << *deleted_meta; + EXPECT_FALSE(deleted_meta->contexts().has_custom("department")) + << *deleted_meta; + EXPECT_TRUE(deleted_meta->contexts().has_custom("region")) << *deleted_meta; + EXPECT_TRUE(deleted_meta->contexts().custom().at("region").has_value()) + << *deleted_meta; + EXPECT_EQ("Asia Pacific", + deleted_meta->contexts().custom().at("region")->value) + << *deleted_meta; + + // Update object with reset of the custom field. + ObjectContexts contexts_reset; + update.set_contexts(contexts_reset); + StatusOr reset_meta = client.UpdateObject( + bucket_name_, object_name, update, Projection("full")); + ASSERT_STATUS_OK(reset_meta); + + // Verify the response ObjectMetadata that no custom contexts exists. This + // is the default behavior as if the custom field is never set. + EXPECT_FALSE(reset_meta->has_contexts()) << *reset_meta; + + // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. + auto status = client.DeleteObject(bucket_name_, object_name); + ASSERT_STATUS_OK(status); + EXPECT_THAT(list_object_names(), Not(Contains(object_name))); +} + /// @test Verify that the client works with non-default endpoints. TEST_F(ObjectBasicCRUDIntegrationTest, NonDefaultEndpointInsert) { auto client = MakeNonDefaultClient(); From f5c2c753ef83fd27ea28c2b94e0e3ee8d936d930 Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Fri, 13 Feb 2026 10:45:40 +0000 Subject: [PATCH 4/7] try again --- .../internal/grpc/object_request_parser.cc | 39 +++++++++++++++++++ .../object_basic_crud_integration_test.cc | 22 +++++------ 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/google/cloud/storage/internal/grpc/object_request_parser.cc b/google/cloud/storage/internal/grpc/object_request_parser.cc index 934dded5ce5b5..7ee9239152b63 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser.cc @@ -155,6 +155,17 @@ Status SetObjectMetadata(google::storage::v2::Object& resource, *resource.mutable_custom_time() = google::cloud::internal::ToProtoTimestamp(metadata.custom_time()); } + if (metadata.has_contexts()) { + auto& contexts_proto = *resource.mutable_contexts(); + if (metadata.contexts().has_custom()) { + auto& custom_map = *contexts_proto.mutable_custom(); + for (auto const& kv : metadata.contexts().custom()) { + if (kv.second.has_value()) { + custom_map[kv.first].set_value(kv.second->value); + } + } + } + } return Status{}; } @@ -288,6 +299,17 @@ StatusOr ToProto( *destination.mutable_custom_time() = google::cloud::internal::ToProtoTimestamp(metadata.custom_time()); } + if (metadata.has_contexts()) { + auto& contexts_proto = *destination.mutable_contexts(); + if (metadata.contexts().has_custom()) { + auto& custom_map = *contexts_proto.mutable_custom(); + for (auto const& kv : metadata.contexts().custom()) { + if (kv.second.has_value()) { + custom_map[kv.first].set_value(kv.second->value); + } + } + } + } } for (auto const& s : request.source_objects()) { google::storage::v2::ComposeObjectRequest::SourceObject source; @@ -516,6 +538,23 @@ StatusOr ToProto( request.metadata().custom_time()); } + if (request.metadata().has_contexts()) { + result.mutable_update_mask()->add_paths("contexts"); + auto& contexts_proto = *object.mutable_contexts(); + if (request.metadata().contexts().has_custom()) { + auto& custom_map = *contexts_proto.mutable_custom(); + for (auto const& kv : request.metadata().contexts().custom()) { + if (kv.second.has_value()) { + custom_map[kv.first].set_value(kv.second->value); + } else { + custom_map.erase(kv.first); + } + } + } else { + contexts_proto.clear_custom(); + } + } + // We need to check each modifiable field. result.mutable_update_mask()->add_paths("cache_control"); object.set_cache_control(request.metadata().cache_control()); diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index bd227869c4d01..50bff5eb53477 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -263,13 +263,13 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { EXPECT_TRUE(updated_meta->has_contexts()) << *updated_meta; EXPECT_TRUE(updated_meta->contexts().has_custom("department")) << *updated_meta; - EXPECT_EQ("engineering and research", - updated_meta->contexts().custom().at("department")->value) - << *updated_meta; + // EXPECT_EQ("engineering and research", + // updated_meta->contexts().custom().at("department")->value) + // << *updated_meta; EXPECT_TRUE(updated_meta->contexts().has_custom("region")) << *updated_meta; - EXPECT_EQ("Asia Pacific", - updated_meta->contexts().custom().at("region")->value) - << *updated_meta; + // EXPECT_EQ("Asia Pacific", + // updated_meta->contexts().custom().at("region")->value) + // << *updated_meta; EXPECT_TRUE( IsSet(updated_meta->contexts().custom().at("region")->update_time)) << *updated_meta; @@ -291,11 +291,11 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { EXPECT_FALSE(deleted_meta->contexts().has_custom("department")) << *deleted_meta; EXPECT_TRUE(deleted_meta->contexts().has_custom("region")) << *deleted_meta; - EXPECT_TRUE(deleted_meta->contexts().custom().at("region").has_value()) - << *deleted_meta; - EXPECT_EQ("Asia Pacific", - deleted_meta->contexts().custom().at("region")->value) - << *deleted_meta; + // EXPECT_TRUE(deleted_meta->contexts().custom().at("region").has_value()) + // << *deleted_meta; + // EXPECT_EQ("Asia Pacific", + // deleted_meta->contexts().custom().at("region")->value) + // << *deleted_meta; // Update object with reset of the custom field. ObjectContexts contexts_reset; From 4ebe151f4fe49f996673b76e023288854c4db3b4 Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Mon, 16 Feb 2026 04:32:11 +0000 Subject: [PATCH 5/7] Refactor data structure and update with grpc and json parsing --- .../internal/grpc/object_metadata_parser.cc | 13 ++ .../grpc/object_metadata_parser_test.cc | 40 +++++ .../internal/grpc/object_request_parser.cc | 81 +++++---- .../grpc/object_request_parser_test.cc | 85 ++++++++- .../internal/object_metadata_parser.cc | 57 +++---- .../cloud/storage/internal/object_requests.cc | 38 +++++ .../storage/internal/object_requests_test.cc | 69 ++++++++ .../storage/internal/patch_builder_details.cc | 9 + .../storage/internal/patch_builder_details.h | 2 + google/cloud/storage/object_contexts.cc | 18 +- google/cloud/storage/object_contexts.h | 59 +++---- google/cloud/storage/object_metadata.cc | 30 ++++ google/cloud/storage/object_metadata.h | 7 + google/cloud/storage/object_metadata_test.cc | 6 +- .../object_basic_crud_integration_test.cc | 161 ++++++++---------- 15 files changed, 447 insertions(+), 228 deletions(-) diff --git a/google/cloud/storage/internal/grpc/object_metadata_parser.cc b/google/cloud/storage/internal/grpc/object_metadata_parser.cc index 230cbfdcbde8b..b22bc21dd11ed 100644 --- a/google/cloud/storage/internal/grpc/object_metadata_parser.cc +++ b/google/cloud/storage/internal/grpc/object_metadata_parser.cc @@ -171,6 +171,19 @@ storage::ObjectMetadata FromProto(google::storage::v2::Object object, metadata.set_hard_delete_time( google::cloud::internal::ToChronoTimePoint(object.hard_delete_time())); } + if (object.has_contexts()) { + storage::ObjectContexts contexts; + for (auto const& kv : object.contexts().custom()) { + storage::ObjectCustomContextPayload payload; + payload.value = kv.second.value(); + payload.create_time = + google::cloud::internal::ToChronoTimePoint(kv.second.create_time()); + payload.update_time = + google::cloud::internal::ToChronoTimePoint(kv.second.update_time()); + contexts.upsert(kv.first, std::move(payload)); + } + metadata.set_contexts(std::move(contexts)); + } return metadata; } diff --git a/google/cloud/storage/internal/grpc/object_metadata_parser_test.cc b/google/cloud/storage/internal/grpc/object_metadata_parser_test.cc index a497a5b0ef7cc..b52f28122e4eb 100644 --- a/google/cloud/storage/internal/grpc/object_metadata_parser_test.cc +++ b/google/cloud/storage/internal/grpc/object_metadata_parser_test.cc @@ -79,6 +79,32 @@ TEST(GrpcClientFromProto, ObjectSimple) { } metadata: { key: "test-key-1" value: "test-value-1" } metadata: { key: "test-key-2" value: "test-value-2" } + contexts: { + custom: { + key: "custom-key-1" value: { + value: "custom-value-1" + create_time: { + seconds: 1565194924 + nanos: 456789012 + } + update_time: { + seconds: 1565194924 + nanos: 456789012 + } + }} + custom: { + key: "custom-key-2" value: { + value: "custom-value-2" + create_time: { + seconds: 1565194924 + nanos: 456789012 + } + update_time: { + seconds: 1709555696 + nanos: 987654321 + } + }} + } event_based_hold: true name: "test-object-name" bucket: "test-bucket" @@ -141,6 +167,20 @@ TEST(GrpcClientFromProto, ObjectSimple) { "test-key-1": "test-value-1", "test-key-2": "test-value-2" }, + "contexts": { + "custom": { + "custom-key-1": { + "value": "custom-value-1", + "createTime": "2019-08-07T16:22:04.456789012Z", + "updateTime": "2019-08-07T16:22:04.456789012Z" + }, + "custom-key-2": { + "value": "custom-value-2", + "createTime": "2019-08-07T16:22:04.456789012Z", + "updateTime": "2024-03-04T12:34:56.987654321Z" + } + } + }, "eventBasedHold": true, "name": "test-object-name", "id": "test-bucket/test-object-name/2345", diff --git a/google/cloud/storage/internal/grpc/object_request_parser.cc b/google/cloud/storage/internal/grpc/object_request_parser.cc index 7ee9239152b63..0e92b3d3106ef 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser.cc @@ -23,6 +23,7 @@ #include "google/cloud/internal/make_status.h" #include "google/cloud/internal/time_utils.h" #include "absl/strings/str_cat.h" +#include #include namespace google { @@ -156,16 +157,14 @@ Status SetObjectMetadata(google::storage::v2::Object& resource, google::cloud::internal::ToProtoTimestamp(metadata.custom_time()); } if (metadata.has_contexts()) { - auto& contexts_proto = *resource.mutable_contexts(); - if (metadata.contexts().has_custom()) { - auto& custom_map = *contexts_proto.mutable_custom(); - for (auto const& kv : metadata.contexts().custom()) { - if (kv.second.has_value()) { - custom_map[kv.first].set_value(kv.second->value); - } - } + auto& custom_map = *resource.mutable_contexts()->mutable_custom(); + for (auto const& kv : metadata.contexts().custom()) { + // In request, the create_time and update_time are ignored by the server, + // hence there is no need to parse them. + custom_map[kv.first].set_value(kv.second.value); } } + return Status{}; } @@ -287,6 +286,13 @@ StatusOr ToProto( for (auto const& kv : metadata.metadata()) { (*destination.mutable_metadata())[kv.first] = kv.second; } + if (metadata.has_contexts()) { + for (auto const& kv : metadata.contexts().custom()) { + auto& payload = + (*destination.mutable_contexts()->mutable_custom())[kv.first]; + payload.set_value(kv.second.value); + } + } destination.set_content_encoding(metadata.content_encoding()); destination.set_content_disposition(metadata.content_disposition()); destination.set_cache_control(metadata.cache_control()); @@ -299,17 +305,6 @@ StatusOr ToProto( *destination.mutable_custom_time() = google::cloud::internal::ToProtoTimestamp(metadata.custom_time()); } - if (metadata.has_contexts()) { - auto& contexts_proto = *destination.mutable_contexts(); - if (metadata.contexts().has_custom()) { - auto& custom_map = *contexts_proto.mutable_custom(); - for (auto const& kv : metadata.contexts().custom()) { - if (kv.second.has_value()) { - custom_map[kv.first].set_value(kv.second->value); - } - } - } - } } for (auto const& s : request.source_objects()) { google::storage::v2::ComposeObjectRequest::SourceObject source; @@ -478,6 +473,24 @@ StatusOr ToProto( } } + auto const& contexts_subpatch = + storage::internal::PatchBuilderDetails::GetCustomContextsSubPatch( + request.patch()); + if (contexts_subpatch.is_null()) { + object.clear_contexts(); + result.mutable_update_mask()->add_paths("contexts.custom"); + } else { + for (auto const& kv : contexts_subpatch.items()) { + result.mutable_update_mask()->add_paths("contexts.custom." + kv.key()); + auto const& v = kv.value(); + if (v.is_object() && v.contains("value")) { + auto& payload = + (*object.mutable_contexts()->mutable_custom())[kv.key()]; + payload.set_value(v["value"].get()); + } + } + } + // We need to check each modifiable field. struct StringField { char const* json_name; @@ -532,29 +545,25 @@ StatusOr ToProto( (*object.mutable_metadata())[kv.first] = kv.second; } + if (request.metadata().has_contexts()) { + result.mutable_update_mask()->add_paths("contexts"); + auto& custom_map = *object.mutable_contexts()->mutable_custom(); + + for (auto const& kv : request.metadata().contexts().custom()) { + google::storage::v2::ObjectCustomContextPayload& payload_ref = + custom_map[kv.first]; + // In request, the create_time and update_time are ignored by + // the server, hence there is no need to parse them. + payload_ref.set_value(kv.second.value); + } + } + if (request.metadata().has_custom_time()) { result.mutable_update_mask()->add_paths("custom_time"); *object.mutable_custom_time() = google::cloud::internal::ToProtoTimestamp( request.metadata().custom_time()); } - if (request.metadata().has_contexts()) { - result.mutable_update_mask()->add_paths("contexts"); - auto& contexts_proto = *object.mutable_contexts(); - if (request.metadata().contexts().has_custom()) { - auto& custom_map = *contexts_proto.mutable_custom(); - for (auto const& kv : request.metadata().contexts().custom()) { - if (kv.second.has_value()) { - custom_map[kv.first].set_value(kv.second->value); - } else { - custom_map.erase(kv.first); - } - } - } else { - contexts_proto.clear_custom(); - } - } - // We need to check each modifiable field. result.mutable_update_mask()->add_paths("cache_control"); object.set_cache_control(request.metadata().cache_control()); diff --git a/google/cloud/storage/internal/grpc/object_request_parser_test.cc b/google/cloud/storage/internal/grpc/object_request_parser_test.cc index 8b91d9b8018f9..ce5d01194bff9 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser_test.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser_test.cc @@ -88,6 +88,12 @@ google::storage::v2::Object ExpectedFullObjectMetadata() { temporary_hold: true metadata: { key: "test-metadata-key1" value: "test-value1" } metadata: { key: "test-metadata-key2" value: "test-value2" } + contexts: { + custom: { + key: "custom-key-1" + value: { value: "custom-value-1" } + } + } event_based_hold: true custom_time { seconds: 1643126687 nanos: 123000000 } )pb"; @@ -113,6 +119,8 @@ storage::ObjectMetadata FullObjectMetadata() { .set_temporary_hold(true) .upsert_metadata("test-metadata-key1", "test-value1") .upsert_metadata("test-metadata-key2", "test-value2") + .set_contexts( + storage::ObjectContexts().upsert("custom-key-1", {"custom-value-1"})) .set_event_based_hold(true) .set_custom_time(std::chrono::system_clock::time_point{} + std::chrono::seconds(1643126687) + @@ -479,6 +487,7 @@ TEST(GrpcObjectRequestParser, PatchObjectRequestAllOptions) { .SetContentType("test-content-type") .SetMetadata("test-metadata-key1", "test-value1") .SetMetadata("test-metadata-key2", "test-value2") + .SetContext("custom-key-1", "custom-value-1") .SetTemporaryHold(true) .SetAcl({ storage::ObjectAccessControl{} @@ -505,12 +514,13 @@ TEST(GrpcObjectRequestParser, PatchObjectRequestAllOptions) { ASSERT_STATUS_OK(actual); // First check the paths. We do not care about their order, so checking them // with IsProtoEqual does not work. - EXPECT_THAT(actual->update_mask().paths(), - UnorderedElementsAre( - "acl", "content_encoding", "content_disposition", - "cache_control", "content_language", "content_type", - "metadata.test-metadata-key1", "metadata.test-metadata-key2", - "temporary_hold", "event_based_hold", "custom_time")); + EXPECT_THAT( + actual->update_mask().paths(), + UnorderedElementsAre( + "acl", "content_encoding", "content_disposition", "cache_control", + "content_language", "content_type", "metadata.test-metadata-key1", + "metadata.test-metadata-key2", "temporary_hold", "event_based_hold", + "custom_time", "contexts.custom.custom-key-1")); // Clear the paths, which we already compared, and compare the proto. actual->mutable_update_mask()->clear_paths(); EXPECT_THAT(*actual, IsProtoEqual(expected)); @@ -535,6 +545,7 @@ TEST(GrpcObjectRequestParser, PatchObjectRequestAllResets) { .ResetContentType() .ResetEventBasedHold() .ResetMetadata() + .ResetContexts() .ResetTemporaryHold() .ResetCustomTime()); @@ -547,7 +558,7 @@ TEST(GrpcObjectRequestParser, PatchObjectRequestAllResets) { UnorderedElementsAre("acl", "content_encoding", "content_disposition", "cache_control", "content_language", "content_type", "metadata", "temporary_hold", "event_based_hold", - "custom_time")); + "custom_time", "contexts.custom")); // Clear the paths, which we already compared, and compare the proto. actual->mutable_update_mask()->clear_paths(); EXPECT_THAT(*actual, IsProtoEqual(expected)); @@ -604,6 +615,64 @@ TEST(GrpcObjectRequestParser, PatchObjectRequestResetMetadata) { EXPECT_THAT(*actual, IsProtoEqual(expected)); } +TEST(GrpcObjectRequestParser, PatchObjectRequestContexts) { + auto constexpr kTextProto = R"pb( + object { + bucket: "projects/_/buckets/bucket-name" + name: "object-name" + contexts: { + custom: { + key: "custom-key-1" + value: { value: "custom-value-1" } + } + } + } + update_mask {} + )pb"; + google::storage::v2::UpdateObjectRequest expected; + ASSERT_TRUE(TextFormat::ParseFromString(kTextProto, &expected)); + + storage::internal::PatchObjectRequest req( + "bucket-name", "object-name", + storage::ObjectMetadataPatchBuilder{} + .SetContext("custom-key-1", "custom-value-1") + .ResetContext("custom-key-2")); + + auto actual = ToProto(req); + ASSERT_STATUS_OK(actual); + // First check the paths. We do not care about their order, so checking them + // with IsProtoEqual does not work. + EXPECT_THAT(actual->update_mask().paths(), + UnorderedElementsAre("contexts.custom.custom-key-1", + "contexts.custom.custom-key-2")); + // Clear the paths, which we already compared, and compare the proto. + actual->mutable_update_mask()->clear_paths(); + EXPECT_THAT(*actual, IsProtoEqual(expected)); +} + +TEST(GrpcObjectRequestParser, PatchObjectRequestResetContexts) { + auto constexpr kTextProto = R"pb( + object { bucket: "projects/_/buckets/bucket-name" name: "object-name" } + update_mask {} + )pb"; + google::storage::v2::UpdateObjectRequest expected; + ASSERT_TRUE(TextFormat::ParseFromString(kTextProto, &expected)); + + storage::internal::PatchObjectRequest req( + "bucket-name", "object-name", + storage::ObjectMetadataPatchBuilder{}.ResetContexts()); + + auto actual = ToProto(req); + ASSERT_STATUS_OK(actual); + // First check the paths. We do not care about their order, so checking them + // with IsProtoEqual does not work. + EXPECT_THAT(actual->update_mask().paths(), + UnorderedElementsAre("contexts.custom")); + // Clear the paths, which we already compared, and compare the proto. + actual->mutable_update_mask()->clear_paths(); + EXPECT_THAT(*actual, IsProtoEqual(expected)); +} + TEST(GrpcObjectRequestParser, UpdateObjectRequestAllOptions) { auto constexpr kTextProto = R"pb( predefined_acl: "projectPrivate" @@ -643,7 +712,7 @@ TEST(GrpcObjectRequestParser, UpdateObjectRequestAllOptions) { UnorderedElementsAre("acl", "content_encoding", "content_disposition", "cache_control", "content_language", "content_type", "metadata", "temporary_hold", "event_based_hold", - "custom_time")); + "custom_time", "contexts")); // Clear the paths, which we already compared, and test the rest actual->mutable_update_mask()->clear_paths(); EXPECT_THAT(*actual, IsProtoEqual(expected)); diff --git a/google/cloud/storage/internal/object_metadata_parser.cc b/google/cloud/storage/internal/object_metadata_parser.cc index 71066daae9a5f..9e61ac22268db 100644 --- a/google/cloud/storage/internal/object_metadata_parser.cc +++ b/google/cloud/storage/internal/object_metadata_parser.cc @@ -52,25 +52,18 @@ void SetJsonContextsIfNotEmpty(nlohmann::json& json, if (!meta.has_contexts()) { return; } - if (meta.contexts().has_custom()) { - nlohmann::json custom_json; - for (auto const& kv : meta.contexts().custom()) { - if (kv.second.has_value()) { - nlohmann::json item; - item["value"] = kv.second.value().value; - item["createTime"] = google::cloud::internal::FormatRfc3339( - kv.second.value().create_time); - item["updateTime"] = google::cloud::internal::FormatRfc3339( - kv.second.value().update_time); - custom_json[kv.first] = std::move(item); - } else { - custom_json[kv.first] = nullptr; - } - } - json["contexts"] = nlohmann::json{{"custom", std::move(custom_json)}}; - } else { - json["contexts"] = nlohmann::json{{"custom", nullptr}}; + + nlohmann::json custom_json; + for (auto const& kv : meta.contexts().custom()) { + nlohmann::json item; + item["value"] = kv.second.value; + item["createTime"] = + google::cloud::internal::FormatRfc3339(kv.second.create_time); + item["updateTime"] = + google::cloud::internal::FormatRfc3339(kv.second.update_time); + custom_json[kv.first] = std::move(item); } + json["contexts"] = nlohmann::json{{"custom", std::move(custom_json)}}; } Status ParseAcl(ObjectMetadata& meta, nlohmann::json const& json) { @@ -198,26 +191,22 @@ Status ParseContexts(ObjectMetadata& meta, nlohmann::json const& json) { ObjectContexts contexts; for (auto const& kv : f_custom->items()) { - if (kv.value().is_null()) { - contexts.upsert_custom_context(kv.key(), absl::nullopt); + auto payload_json = kv.value(); + ObjectCustomContextPayload payload; - } else { - ObjectCustomContextPayload payload; - auto value = kv.value().value("value", ""); - payload.value = value; + payload.value = payload_json.value("value", ""); - auto create_time = - internal::ParseTimestampField(kv.value(), "createTime"); - if (!create_time) return std::move(create_time).status(); - payload.create_time = *create_time; + auto create_time = + internal::ParseTimestampField(payload_json, "createTime"); + if (!create_time) return std::move(create_time).status(); + payload.create_time = *create_time; - auto update_time = - internal::ParseTimestampField(kv.value(), "updateTime"); - if (!update_time) return std::move(update_time).status(); - payload.update_time = *update_time; + auto update_time = + internal::ParseTimestampField(payload_json, "updateTime"); + if (!update_time) return std::move(update_time).status(); + payload.update_time = *update_time; - contexts.upsert_custom_context(kv.key(), std::move(payload)); - } + contexts.upsert(kv.key(), std::move(payload)); } meta.set_contexts(std::move(contexts)); return Status{}; diff --git a/google/cloud/storage/internal/object_requests.cc b/google/cloud/storage/internal/object_requests.cc index 37857aff7bfdc..192f21bf83488 100644 --- a/google/cloud/storage/internal/object_requests.cc +++ b/google/cloud/storage/internal/object_requests.cc @@ -97,6 +97,44 @@ ObjectMetadataPatchBuilder DiffObjectMetadata(ObjectMetadata const& original, } } + if (original.has_contexts() && updated.has_contexts()) { + if (original.contexts() != updated.contexts()) { + // Find the keys in the original map that are not in the new map, so as + // to reset them in patch builder. + std::map deleted_entries; + std::set_difference( + original.contexts().custom().begin(), + original.contexts().custom().end(), + updated.contexts().custom().begin(), + updated.contexts().custom().end(), + std::inserter(deleted_entries, deleted_entries.end()), + [](auto const& a, auto const& b) { return a.first < b.first; }); + for (auto&& d : deleted_entries) { + builder.ResetContext(d.first); + } + + // Find the entries in the updated map that are either not in the original + // map or have different values, so as to upsert them in the patch + // builder. + std::map changed_entries; + std::set_difference( + updated.contexts().custom().begin(), + updated.contexts().custom().end(), + original.contexts().custom().begin(), + original.contexts().custom().end(), + std::inserter(changed_entries, changed_entries.end())); + for (auto&& d : changed_entries) { + builder.SetContext(d.first, d.second.value); + } + } + } else if (original.has_contexts() && !updated.has_contexts()) { + builder.ResetContexts(); + } else if (!original.has_contexts() && updated.has_contexts()) { + for (auto const& c : updated.contexts().custom()) { + builder.SetContext(c.first, c.second.value); + } + } + if (original.temporary_hold() != updated.temporary_hold()) { builder.SetTemporaryHold(updated.temporary_hold()); } diff --git a/google/cloud/storage/internal/object_requests_test.cc b/google/cloud/storage/internal/object_requests_test.cc index 4e25fc2d11036..ae035195f260f 100644 --- a/google/cloud/storage/internal/object_requests_test.cc +++ b/google/cloud/storage/internal/object_requests_test.cc @@ -749,6 +749,15 @@ ObjectMetadata CreateObjectMetadataForTest() { "foo": "bar", "baz": "qux" }, + "contexts": { + "custom": { + "environment": { + "value": "prod", + "createTime": "2024-07-18T00:00:00Z", + "updateTime": "2024-07-18T00:00:00Z" + } + } + }, "metageneration": "4", "name": "baz", "owner": { @@ -964,6 +973,66 @@ TEST(PatchObjectRequestTest, DiffResetMetadata) { EXPECT_EQ(expected, patch); } +TEST(PatchObjectRequestTest, DiffSetContexts) { + ObjectMetadata original = CreateObjectMetadataForTest(); + + ObjectMetadata updated = original; + ObjectContexts contexts; + contexts.upsert("department", {"engineering"}) + .upsert("environment", {"preprod"}); + updated.set_contexts(contexts); + + PatchObjectRequest request("test-bucket", "test-object", original, updated); + + auto patch = nlohmann::json::parse(request.payload()); + auto expected = nlohmann::json::parse(R"""({ + "contexts": { + "custom": { + "environment": { + "value": "preprod" + }, + "department": { + "value": "engineering" + } + } + } + })"""); + EXPECT_EQ(expected, patch); +} + +TEST(PatchObjectRequestTest, DiffResetOneContext) { + ObjectMetadata original = CreateObjectMetadataForTest(); + + ObjectMetadata updated = original; + ObjectContexts contexts; + contexts.delete_key("environment"); + updated.set_contexts(contexts); + + PatchObjectRequest request("test-bucket", "test-object", original, updated); + + auto patch = nlohmann::json::parse(request.payload()); + auto expected = nlohmann::json::parse(R"""({ + "contexts": { + "custom": { + "environment": null + } + } + })"""); + EXPECT_EQ(expected, patch); +} + +TEST(PatchObjectRequestTest, DiffResetContexts) { + ObjectMetadata original = CreateObjectMetadataForTest(); + ObjectMetadata updated = original; + updated.reset_contexts(); + PatchObjectRequest request("test-bucket", "test-object", original, updated); + + auto patch = nlohmann::json::parse(request.payload()); + auto expected = + nlohmann::json::parse(R"""({"contexts": {"custom": null}})"""); + EXPECT_EQ(expected, patch); +} + TEST(PatchObjectRequestTest, DiffSetTemporaryHold) { ObjectMetadata original = CreateObjectMetadataForTest(); original.set_temporary_hold(false); diff --git a/google/cloud/storage/internal/patch_builder_details.cc b/google/cloud/storage/internal/patch_builder_details.cc index 9a4ab7c8db5a9..fe634a54b717d 100644 --- a/google/cloud/storage/internal/patch_builder_details.cc +++ b/google/cloud/storage/internal/patch_builder_details.cc @@ -62,6 +62,15 @@ nlohmann::json const& PatchBuilderDetails::GetMetadataSubPatch( return GetPatch(patch.metadata_subpatch_); } +nlohmann::json const& PatchBuilderDetails::GetCustomContextsSubPatch( + storage::ObjectMetadataPatchBuilder const& patch) { + static auto const* const kEmpty = [] { + return new nlohmann::json(nlohmann::json::object()); + }(); + if (!patch.contexts_subpatch_dirty_) return *kEmpty; + return GetPatch(patch.contexts_custom_subpatch_); +} + } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage diff --git a/google/cloud/storage/internal/patch_builder_details.h b/google/cloud/storage/internal/patch_builder_details.h index 4231b156f5560..681cc8e92ba58 100644 --- a/google/cloud/storage/internal/patch_builder_details.h +++ b/google/cloud/storage/internal/patch_builder_details.h @@ -53,6 +53,8 @@ struct PatchBuilderDetails { storage::ObjectMetadataPatchBuilder const& patch); static nlohmann::json const& GetMetadataSubPatch( storage::ObjectMetadataPatchBuilder const& patch); + static nlohmann::json const& GetCustomContextsSubPatch( + storage::ObjectMetadataPatchBuilder const& patch); static nlohmann::json const& GetPatch(PatchBuilder const& patch); }; diff --git a/google/cloud/storage/object_contexts.cc b/google/cloud/storage/object_contexts.cc index 181495442569a..757c4c213b49b 100644 --- a/google/cloud/storage/object_contexts.cc +++ b/google/cloud/storage/object_contexts.cc @@ -32,20 +32,12 @@ std::ostream& operator<<(std::ostream& os, std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs) { os << "ObjectContexts={custom={"; - if (rhs.has_custom()) { - char const* sep = ""; - for (auto const& kv : rhs.custom()) { - os << sep << kv.first << "="; - if (kv.second.has_value()) { - os << kv.second.value(); - } else { - os << "null"; - } - sep = ",\n"; - } - } else { - os << "null"; + char const* sep = ""; + for (auto const& kv : rhs.custom()) { + os << sep << kv.first << "=" << kv.second.value; + sep = ",\n"; } + return os << "}}"; } diff --git a/google/cloud/storage/object_contexts.h b/google/cloud/storage/object_contexts.h index 77cd7da03642f..f4e16800df6e9 100644 --- a/google/cloud/storage/object_contexts.h +++ b/google/cloud/storage/object_contexts.h @@ -30,10 +30,13 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN * Represents the payload of a user-defined object context. */ struct ObjectCustomContextPayload { + // The value of the object context. std::string value; + // The time at which the object context was created. Output only. std::chrono::system_clock::time_point create_time; + // The time at which the object context was last updated. Output only. std::chrono::system_clock::time_point update_time; }; @@ -48,6 +51,11 @@ inline bool operator!=(ObjectCustomContextPayload const& lhs, return !(lhs == rhs); } +inline bool operator<(ObjectCustomContextPayload const& lhs, + ObjectCustomContextPayload const& rhs) { + return lhs.value < rhs.value; +} + std::ostream& operator<<(std::ostream& os, ObjectCustomContextPayload const& rhs); @@ -56,50 +64,24 @@ std::ostream& operator<<(std::ostream& os, */ struct ObjectContexts { public: - // Returns true if the map itself exists. - bool has_custom() const { return custom_map_.has_value(); } - - /** - * Returns true if the map exists AND the key is present AND the value is - * a valid value. - */ - bool has_custom(std::string const& key) const { - if (!has_custom()) { - return false; - } - return custom_map_->find(key) != custom_map_->end() && - custom_map_->at(key).has_value(); + bool has_key(std::string const& key) const { + return custom_.find(key) != custom_.end(); } - /** - * The `custom` attribute of the object contexts. - * Values are now absl::optional. - * - * It is undefined behavior to call this member function if - * `has_custom() == false`. - */ - std::map> const& - custom() const { - return *custom_map_; + ObjectContexts& upsert(std::string const& key, + ObjectCustomContextPayload const& value) { + custom_[key] = value; + return *this; } - /** - * Upserts a context. Passing absl::nullopt for the value - * represents a "null" entry in the map. - */ - void upsert_custom_context(std::string const& key, - absl::optional value) { - if (!has_custom()) { - custom_map_.emplace(); - } + bool delete_key(std::string const& key) { return custom_.erase(key) > 0; } - (*custom_map_)[key] = std::move(value); + std::map const& custom() const { + return custom_; } - void reset_custom() { custom_map_.reset(); } - bool operator==(ObjectContexts const& other) const { - return custom_map_ == other.custom_map_; + return custom_ == other.custom_; } bool operator!=(ObjectContexts const& other) const { @@ -109,11 +91,8 @@ struct ObjectContexts { private: /** * Represents the map of user-defined object contexts. - * Inner optional allows keys to point to a "null" value. */ - absl::optional< - std::map>> - custom_map_; + std::map custom_; }; std::ostream& operator<<(std::ostream& os, ObjectContexts const& rhs); diff --git a/google/cloud/storage/object_metadata.cc b/google/cloud/storage/object_metadata.cc index deecb96b0298c..82590845179ea 100644 --- a/google/cloud/storage/object_metadata.cc +++ b/google/cloud/storage/object_metadata.cc @@ -149,6 +149,15 @@ std::string ObjectMetadataPatchBuilder::BuildPatch() const { tmp.AddSubPatch("metadata", metadata_subpatch_); } } + if (contexts_subpatch_dirty_) { + if (contexts_custom_subpatch_.empty()) { + tmp.AddSubPatch("contexts", + internal::PatchBuilder().RemoveField("custom")); + } else { + tmp.AddSubPatch("contexts", internal::PatchBuilder().AddSubPatch( + "custom", contexts_custom_subpatch_)); + } + } return tmp.ToString(); } @@ -275,6 +284,27 @@ ObjectMetadataPatchBuilder& ObjectMetadataPatchBuilder::ResetMetadata() { return *this; } +ObjectMetadataPatchBuilder& ObjectMetadataPatchBuilder::SetContext( + std::string const& key, std::string const& value) { + contexts_custom_subpatch_.AddSubPatch( + key.c_str(), internal::PatchBuilder().SetStringField("value", value)); + contexts_subpatch_dirty_ = true; + return *this; +} + +ObjectMetadataPatchBuilder& ObjectMetadataPatchBuilder::ResetContext( + std::string const& key) { + contexts_custom_subpatch_.RemoveField(key.c_str()); + contexts_subpatch_dirty_ = true; + return *this; +} + +ObjectMetadataPatchBuilder& ObjectMetadataPatchBuilder::ResetContexts() { + contexts_custom_subpatch_.clear(); + contexts_subpatch_dirty_ = true; + return *this; +} + ObjectMetadataPatchBuilder& ObjectMetadataPatchBuilder::SetTemporaryHold( bool v) { impl_.SetBoolField("temporaryHold", v); diff --git a/google/cloud/storage/object_metadata.h b/google/cloud/storage/object_metadata.h index c3ad0621ffee7..c5e367fd46962 100644 --- a/google/cloud/storage/object_metadata.h +++ b/google/cloud/storage/object_metadata.h @@ -700,6 +700,11 @@ class ObjectMetadataPatchBuilder { ObjectMetadataPatchBuilder& ResetMetadata(std::string const& key); ObjectMetadataPatchBuilder& ResetMetadata(); + ObjectMetadataPatchBuilder& SetContext(std::string const& key, + std::string const& value); + ObjectMetadataPatchBuilder& ResetContext(std::string const& key); + ObjectMetadataPatchBuilder& ResetContexts(); + ObjectMetadataPatchBuilder& SetTemporaryHold(bool v); ObjectMetadataPatchBuilder& ResetTemporaryHold(); @@ -728,6 +733,8 @@ class ObjectMetadataPatchBuilder { internal::PatchBuilder impl_; bool metadata_subpatch_dirty_{false}; internal::PatchBuilder metadata_subpatch_; + bool contexts_subpatch_dirty_{false}; + internal::PatchBuilder contexts_custom_subpatch_; }; /** diff --git a/google/cloud/storage/object_metadata_test.cc b/google/cloud/storage/object_metadata_test.cc index 8fd26eb9214b4..89171a42bcaf2 100644 --- a/google/cloud/storage/object_metadata_test.cc +++ b/google/cloud/storage/object_metadata_test.cc @@ -216,7 +216,6 @@ TEST(ObjectMetadataTest, Parse) { EXPECT_EQ(actual.hard_delete_time(), std::chrono::system_clock::from_time_t(1710160496L) + std::chrono::milliseconds(789)); - ASSERT_TRUE(actual.has_contexts() && actual.contexts().has_custom()); EXPECT_EQ( actual.contexts().custom().at("environment"), (ObjectCustomContextPayload{ @@ -695,10 +694,9 @@ TEST(ObjectMetadataTest, SetContexts) { auto const context_payload = ObjectCustomContextPayload{"engineering", {}, {}}; ObjectContexts contexts; - contexts.upsert_custom_context("department", context_payload); + contexts.upsert("department", context_payload); copy.set_contexts(contexts); - EXPECT_TRUE(expected.contexts().has_custom()); - EXPECT_TRUE(copy.contexts().has_custom()); + EXPECT_TRUE(copy.has_contexts()); EXPECT_EQ(contexts, copy.contexts()); EXPECT_NE(expected, copy); } diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index 50bff5eb53477..218f825efef9f 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -68,34 +68,35 @@ struct ObjectBasicCRUDIntegrationTest return MakeIntegrationTestClient(std::move(options)); } - void SetUp() override { - // 1. Run the base class SetUp first. This initializes 'bucket_name_' - // from the environment variable. - ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); - - // 2. Create a client to interact with the emulator/backend. - auto client = MakeIntegrationTestClient(); - - // 3. Check if the bucket exists. - auto metadata = client.GetBucketMetadata(bucket_name_); - - // 4. If it's missing (kNotFound), create it. - if (metadata.status().code() == StatusCode::kNotFound) { - // Use a default project ID if the env var isn't set (common in local - // emulators). - auto project_id = google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT") - .value_or("test-project"); - - auto created = client.CreateBucketForProject(bucket_name_, project_id, - BucketMetadata()); - ASSERT_STATUS_OK(created) - << "Failed to auto-create missing bucket: " << bucket_name_; - } else { - // If it exists (or failed for another reason), assert it is OK. - ASSERT_STATUS_OK(metadata) - << "Failed to verify bucket existence: " << bucket_name_; - } - } + // void SetUp() override { + // // 1. Run the base class SetUp first. This initializes 'bucket_name_' + // // from the environment variable. + // ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); + + // // 2. Create a client to interact with the emulator/backend. + // auto client = MakeIntegrationTestClient(); + + // // 3. Check if the bucket exists. + // auto metadata = client.GetBucketMetadata(bucket_name_); + + // // 4. If it's missing (kNotFound), create it. + // if (metadata.status().code() == StatusCode::kNotFound) { + // // Use a default project ID if the env var isn't set (common in local + // // emulators). + // auto project_id = + // google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT") + // .value_or("test-project"); + + // auto created = client.CreateBucketForProject(bucket_name_, project_id, + // BucketMetadata()); + // ASSERT_STATUS_OK(created) + // << "Failed to auto-create missing bucket: " << bucket_name_; + // } else { + // // If it exists (or failed for another reason), assert it is OK. + // ASSERT_STATUS_OK(metadata) + // << "Failed to verify bucket existence: " << bucket_name_; + // } + // } }; /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations. @@ -214,14 +215,11 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { // Create the object, but only if it does not exist already, inserting // a custom context {"department": "engineering"}. - ObjectContexts contexts; - ObjectCustomContextPayload payload; - payload.value = "engineering"; - contexts.upsert_custom_context("department", std::move(payload)); StatusOr insert_meta = client.InsertObject( bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), Projection("full"), - WithObjectMetadata(ObjectMetadata().set_contexts(contexts))); + WithObjectMetadata(ObjectMetadata().set_contexts( + ObjectContexts().upsert("department", {"engineering"})))); ASSERT_STATUS_OK(insert_meta); EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); @@ -231,81 +229,58 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { Projection("full")); ASSERT_STATUS_OK(get_meta); EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; - EXPECT_TRUE(get_meta->contexts().has_custom("department")) << *get_meta; - EXPECT_EQ("engineering", - get_meta->contexts().custom().at("department")->value) - << *get_meta; - EXPECT_TRUE( - IsSet(get_meta->contexts().custom().at("department")->update_time)) - << *get_meta; - EXPECT_TRUE( - IsSet(get_meta->contexts().custom().at("department")->create_time)) - << *get_meta; - - // Update object with a new value "engineering and research" for the existing - // custom context, and also add another custom context {"region": - // "Asia Pacific"}. + EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + // EXPECT_EQ("engineering", + // get_meta->contexts().custom().at("department").value) + // << *get_meta; + // EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").update_time)) + // << *get_meta; + // EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").create_time)) + // << *get_meta; + + // Update object with two keys. ObjectMetadata update = *get_meta; - ObjectContexts contexts_updated; - ObjectCustomContextPayload payload_updated; - payload_updated.value = "engineering and research"; - contexts_updated.upsert_custom_context("department", - std::move(payload_updated)); - ObjectCustomContextPayload payload_another; - payload_another.value = "Asia Pacific"; - contexts_updated.upsert_custom_context("region", std::move(payload_another)); - update.set_contexts(contexts_updated); + update.set_contexts(ObjectContexts() + .upsert("department", {"engineering and research"}) + .upsert("region", {"Asia Pacific"})); StatusOr updated_meta = client.UpdateObject( bucket_name_, object_name, update, Projection("full")); ASSERT_STATUS_OK(updated_meta); // Verify the response ObjectMetadata has the updated custom contexts. EXPECT_TRUE(updated_meta->has_contexts()) << *updated_meta; - EXPECT_TRUE(updated_meta->contexts().has_custom("department")) - << *updated_meta; + EXPECT_TRUE(updated_meta->contexts().has_key("department")) << *updated_meta; // EXPECT_EQ("engineering and research", - // updated_meta->contexts().custom().at("department")->value) + // updated_meta->contexts().custom().at("department").value) // << *updated_meta; - EXPECT_TRUE(updated_meta->contexts().has_custom("region")) << *updated_meta; + EXPECT_TRUE(updated_meta->contexts().has_key("region")) << *updated_meta; // EXPECT_EQ("Asia Pacific", - // updated_meta->contexts().custom().at("region")->value) + // updated_meta->contexts().custom().at("region").value) // << *updated_meta; - EXPECT_TRUE( - IsSet(updated_meta->contexts().custom().at("region")->update_time)) - << *updated_meta; - EXPECT_TRUE( - IsSet(updated_meta->contexts().custom().at("region")->create_time)) - << *updated_meta; - // Update object with deletion of the "department" custom context. - ObjectContexts contexts_deleted; - contexts_deleted.upsert_custom_context("department", absl::nullopt); - update.set_contexts(contexts_deleted); - StatusOr deleted_meta = client.UpdateObject( - bucket_name_, object_name, update, Projection("full")); - ASSERT_STATUS_OK(deleted_meta); - - // Verify the response ObjectMetadata has the "department" key is set to null, - // but the "region" key still present with value. - EXPECT_TRUE(deleted_meta->has_contexts()) << *deleted_meta; - EXPECT_FALSE(deleted_meta->contexts().has_custom("department")) - << *deleted_meta; - EXPECT_TRUE(deleted_meta->contexts().has_custom("region")) << *deleted_meta; - // EXPECT_TRUE(deleted_meta->contexts().custom().at("region").has_value()) - // << *deleted_meta; - // EXPECT_EQ("Asia Pacific", - // deleted_meta->contexts().custom().at("region")->value) - // << *deleted_meta; + // Patch object with reset of the "department" key and update of the "region" + // key. + StatusOr patched_meta = client.PatchObject( + bucket_name_, object_name, + ObjectMetadataPatchBuilder() + .ResetContext("department") + .SetContext("region", {"Asia Pacific - Singapore"}), + Projection("full")); + ASSERT_STATUS_OK(patched_meta); - // Update object with reset of the custom field. - ObjectContexts contexts_reset; - update.set_contexts(contexts_reset); - StatusOr reset_meta = client.UpdateObject( - bucket_name_, object_name, update, Projection("full")); + // Verify the response ObjectMetadata has the "department" key reset to null + // and the "region" key updated. + EXPECT_TRUE(patched_meta->has_contexts()) << *patched_meta; + EXPECT_FALSE(patched_meta->contexts().has_key("department")) << *patched_meta; + EXPECT_TRUE(patched_meta->contexts().has_key("region")) << *patched_meta; + + // Patch object with reset of all contexts. + StatusOr reset_meta = client.PatchObject( + bucket_name_, object_name, ObjectMetadataPatchBuilder().ResetContexts(), + Projection("full")); ASSERT_STATUS_OK(reset_meta); - // Verify the response ObjectMetadata that no custom contexts exists. This - // is the default behavior as if the custom field is never set. + // Verify the response ObjectMetadata that no custom contexts exists. EXPECT_FALSE(reset_meta->has_contexts()) << *reset_meta; // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. From 6deb70e5d055b0fdbcda0d00307cdeec5470f596 Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Wed, 18 Feb 2026 13:54:53 +0000 Subject: [PATCH 6/7] fix cxx20 compilation problem --- .../grpc/object_request_parser_test.cc | 4 +- .../storage/internal/object_requests_test.cc | 4 +- .../object_basic_crud_integration_test.cc | 170 +++++++++++++++--- 3 files changed, 151 insertions(+), 27 deletions(-) diff --git a/google/cloud/storage/internal/grpc/object_request_parser_test.cc b/google/cloud/storage/internal/grpc/object_request_parser_test.cc index ce5d01194bff9..a429dd70f2718 100644 --- a/google/cloud/storage/internal/grpc/object_request_parser_test.cc +++ b/google/cloud/storage/internal/grpc/object_request_parser_test.cc @@ -119,8 +119,8 @@ storage::ObjectMetadata FullObjectMetadata() { .set_temporary_hold(true) .upsert_metadata("test-metadata-key1", "test-value1") .upsert_metadata("test-metadata-key2", "test-value2") - .set_contexts( - storage::ObjectContexts().upsert("custom-key-1", {"custom-value-1"})) + .set_contexts(storage::ObjectContexts().upsert( + "custom-key-1", {"custom-value-1", {}, {}})) .set_event_based_hold(true) .set_custom_time(std::chrono::system_clock::time_point{} + std::chrono::seconds(1643126687) + diff --git a/google/cloud/storage/internal/object_requests_test.cc b/google/cloud/storage/internal/object_requests_test.cc index ae035195f260f..791dd73fc1ecb 100644 --- a/google/cloud/storage/internal/object_requests_test.cc +++ b/google/cloud/storage/internal/object_requests_test.cc @@ -978,8 +978,8 @@ TEST(PatchObjectRequestTest, DiffSetContexts) { ObjectMetadata updated = original; ObjectContexts contexts; - contexts.upsert("department", {"engineering"}) - .upsert("environment", {"preprod"}); + contexts.upsert("department", {"engineering", {}, {}}) + .upsert("environment", {"preprod", {}, {}}); updated.set_contexts(contexts); PatchObjectRequest request("test-bucket", "test-object", original, updated); diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index 218f825efef9f..34774044c221c 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -195,7 +195,7 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { } /// @test Verify the Object CRUD operations with object contexts. -TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { +TEST_F(ObjectBasicCRUDIntegrationTest, BasicInsertAndGetWithObjectContexts) { auto client = MakeIntegrationTestClient(); auto list_object_names = [&client, this] { @@ -219,7 +219,55 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), Projection("full"), WithObjectMetadata(ObjectMetadata().set_contexts( - ObjectContexts().upsert("department", {"engineering"})))); + ObjectContexts().upsert("department", {"engineering", {}, {}})))); + ASSERT_STATUS_OK(insert_meta); + EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); + + // Verify the response ObjectMetadata has the custom contexts we set. + StatusOr get_meta = client.GetObjectMetadata( + bucket_name_, object_name, Generation(insert_meta->generation()), + Projection("full")); + ASSERT_STATUS_OK(get_meta); + EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; + EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + EXPECT_EQ("engineering", get_meta->contexts().custom().at("department").value) + << *get_meta; + EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").update_time)) + << *get_meta; + EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").create_time)) + << *get_meta; + + auto status = client.DeleteObject(bucket_name_, object_name); + ASSERT_STATUS_OK(status); + EXPECT_THAT(list_object_names(), Not(Contains(object_name))); +} + +/// @test Verify the Object CRUD operations with object contexts. +TEST_F(ObjectBasicCRUDIntegrationTest, BasicUpdateWithObjectContexts) { + auto client = MakeIntegrationTestClient(); + + auto list_object_names = [&client, this] { + std::vector names; + for (auto o : client.ListObjects(bucket_name_)) { + EXPECT_STATUS_OK(o); + if (!o) break; + names.push_back(o->name()); + } + return names; + }; + + auto object_name = MakeRandomObjectName(); + ASSERT_THAT(list_object_names(), Not(Contains(object_name))) + << "Test aborted. The object <" << object_name << "> already exists." + << "This is unexpected as the test generates a random object name."; + + // Create the object, but only if it does not exist already, inserting + // a custom context {"department": "engineering"}. + StatusOr insert_meta = client.InsertObject( + bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), + Projection("full"), + WithObjectMetadata(ObjectMetadata().set_contexts( + ObjectContexts().upsert("department", {"engineering", {}, {}})))); ASSERT_STATUS_OK(insert_meta); EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); @@ -230,21 +278,16 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { ASSERT_STATUS_OK(get_meta); EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; - // EXPECT_EQ("engineering", - // get_meta->contexts().custom().at("department").value) - // << *get_meta; - // EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").update_time)) - // << *get_meta; - // EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").create_time)) - // << *get_meta; // Update object with two keys. ObjectMetadata update = *get_meta; - update.set_contexts(ObjectContexts() - .upsert("department", {"engineering and research"}) - .upsert("region", {"Asia Pacific"})); + update.set_contexts( + ObjectContexts() + .upsert("department", {"engineering and research", {}, {}}) + .upsert("region", {"Asia Pacific", {}, {}})); StatusOr updated_meta = client.UpdateObject( - bucket_name_, object_name, update, Projection("full")); + bucket_name_, object_name, update, Generation(get_meta->generation()), + Projection("full")); ASSERT_STATUS_OK(updated_meta); // Verify the response ObjectMetadata has the updated custom contexts. @@ -258,22 +301,103 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { // updated_meta->contexts().custom().at("region").value) // << *updated_meta; - // Patch object with reset of the "department" key and update of the "region" - // key. - StatusOr patched_meta = client.PatchObject( - bucket_name_, object_name, - ObjectMetadataPatchBuilder() - .ResetContext("department") - .SetContext("region", {"Asia Pacific - Singapore"}), + // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. + auto status = client.DeleteObject(bucket_name_, object_name); + ASSERT_STATUS_OK(status); + EXPECT_THAT(list_object_names(), Not(Contains(object_name))); +} + +/// @test Verify the Object CRUD operations with object contexts. +TEST_F(ObjectBasicCRUDIntegrationTest, BasicPatchWithObjectContexts) { + auto client = MakeIntegrationTestClient(); + + auto list_object_names = [&client, this] { + std::vector names; + for (auto o : client.ListObjects(bucket_name_)) { + EXPECT_STATUS_OK(o); + if (!o) break; + names.push_back(o->name()); + } + return names; + }; + + auto object_name = MakeRandomObjectName(); + ASSERT_THAT(list_object_names(), Not(Contains(object_name))) + << "Test aborted. The object <" << object_name << "> already exists." + << "This is unexpected as the test generates a random object name."; + + // Create the object, but only if it does not exist already, inserting + // a custom context {"department": "engineering"}. + StatusOr insert_meta = client.InsertObject( + bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), + Projection("full"), + WithObjectMetadata(ObjectMetadata().set_contexts( + ObjectContexts().upsert("department", {"engineering", {}, {}})))); + ASSERT_STATUS_OK(insert_meta); + EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); + + // Verify the response ObjectMetadata has the custom contexts we set. + StatusOr get_meta = client.GetObjectMetadata( + bucket_name_, object_name, Generation(insert_meta->generation()), Projection("full")); + ASSERT_STATUS_OK(get_meta); + EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; + EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + + StatusOr patched_meta = + client.PatchObject(bucket_name_, object_name, + ObjectMetadataPatchBuilder().SetContext( + "region", {"Asia Pacific - Singapore"}), + Projection("full")); ASSERT_STATUS_OK(patched_meta); - // Verify the response ObjectMetadata has the "department" key reset to null - // and the "region" key updated. EXPECT_TRUE(patched_meta->has_contexts()) << *patched_meta; - EXPECT_FALSE(patched_meta->contexts().has_key("department")) << *patched_meta; + EXPECT_TRUE(patched_meta->contexts().has_key("department")) << *patched_meta; EXPECT_TRUE(patched_meta->contexts().has_key("region")) << *patched_meta; + // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. + auto status = client.DeleteObject(bucket_name_, object_name); + ASSERT_STATUS_OK(status); + EXPECT_THAT(list_object_names(), Not(Contains(object_name))); +} + +/// @test Verify the Object CRUD operations with object contexts. +TEST_F(ObjectBasicCRUDIntegrationTest, BasicResetWithObjectContexts) { + auto client = MakeIntegrationTestClient(); + + auto list_object_names = [&client, this] { + std::vector names; + for (auto o : client.ListObjects(bucket_name_)) { + EXPECT_STATUS_OK(o); + if (!o) break; + names.push_back(o->name()); + } + return names; + }; + + auto object_name = MakeRandomObjectName(); + ASSERT_THAT(list_object_names(), Not(Contains(object_name))) + << "Test aborted. The object <" << object_name << "> already exists." + << "This is unexpected as the test generates a random object name."; + + // Create the object, but only if it does not exist already, inserting + // a custom context {"department": "engineering"}. + StatusOr insert_meta = client.InsertObject( + bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), + Projection("full"), + WithObjectMetadata(ObjectMetadata().set_contexts( + ObjectContexts().upsert("department", {"engineering", {}, {}})))); + ASSERT_STATUS_OK(insert_meta); + EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); + + // Verify the response ObjectMetadata has the custom contexts we set. + StatusOr get_meta = client.GetObjectMetadata( + bucket_name_, object_name, Generation(insert_meta->generation()), + Projection("full")); + ASSERT_STATUS_OK(get_meta); + EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; + EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + // Patch object with reset of all contexts. StatusOr reset_meta = client.PatchObject( bucket_name_, object_name, ObjectMetadataPatchBuilder().ResetContexts(), From 86f4b87098005735813d936a951760fc25210bad Mon Sep 17 00:00:00 2001 From: Olivia Xiaoni Lai <5503815+xlai20@users.noreply.github.com> Date: Thu, 19 Feb 2026 03:12:56 +0000 Subject: [PATCH 7/7] Fix the integration test with storage-testbench --- .../object_basic_crud_integration_test.cc | 259 +++++------------- 1 file changed, 70 insertions(+), 189 deletions(-) diff --git a/google/cloud/storage/tests/object_basic_crud_integration_test.cc b/google/cloud/storage/tests/object_basic_crud_integration_test.cc index 34774044c221c..aef1b057b814e 100644 --- a/google/cloud/storage/tests/object_basic_crud_integration_test.cc +++ b/google/cloud/storage/tests/object_basic_crud_integration_test.cc @@ -36,6 +36,27 @@ namespace { bool IsSet(std::chrono::system_clock::time_point tp) { return tp != std::chrono::system_clock::time_point{}; } + +void AssertHasCustomContext(google::cloud::storage::ObjectMetadata const& meta, + std::string const& key, + std::string const& expected_value) { + EXPECT_TRUE(meta.has_contexts()) + << "Missing contexts entirely in metadata: " << meta; + EXPECT_TRUE(meta.contexts().has_key(key)) + << "Missing expected context key '" << key << "' in metadata: " << meta; + if (meta.contexts().has_key(key)) { + EXPECT_EQ(expected_value, meta.contexts().custom().at(key).value) + << "Mismatch in context value for key '" << key << "'\n" + << "Expecting value '" << expected_value << "'\n" + << "Actual metadata: " << meta; + EXPECT_TRUE(IsSet(meta.contexts().custom().at(key).create_time)) + << "The create_time of key '" << key << "' is not set.\n" + << "Actual metadata: " << meta; + EXPECT_TRUE(IsSet(meta.contexts().custom().at(key).update_time)) + << "The update_time of key '" << key << "' is not set.\n" + << "Actual metadata: " << meta; + } +} } // namespace namespace google { @@ -68,35 +89,34 @@ struct ObjectBasicCRUDIntegrationTest return MakeIntegrationTestClient(std::move(options)); } - // void SetUp() override { - // // 1. Run the base class SetUp first. This initializes 'bucket_name_' - // // from the environment variable. - // ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); - - // // 2. Create a client to interact with the emulator/backend. - // auto client = MakeIntegrationTestClient(); - - // // 3. Check if the bucket exists. - // auto metadata = client.GetBucketMetadata(bucket_name_); - - // // 4. If it's missing (kNotFound), create it. - // if (metadata.status().code() == StatusCode::kNotFound) { - // // Use a default project ID if the env var isn't set (common in local - // // emulators). - // auto project_id = - // google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT") - // .value_or("test-project"); - - // auto created = client.CreateBucketForProject(bucket_name_, project_id, - // BucketMetadata()); - // ASSERT_STATUS_OK(created) - // << "Failed to auto-create missing bucket: " << bucket_name_; - // } else { - // // If it exists (or failed for another reason), assert it is OK. - // ASSERT_STATUS_OK(metadata) - // << "Failed to verify bucket existence: " << bucket_name_; - // } - // } + void SetUp() override { + // 1. Run the base class SetUp first. This initializes 'bucket_name_' + // from the environment variable. + ::google::cloud::storage::testing::ObjectIntegrationTest::SetUp(); + + // 2. Create a client to interact with the emulator/backend. + auto client = MakeIntegrationTestClient(); + + // 3. Check if the bucket exists. + auto metadata = client.GetBucketMetadata(bucket_name_); + + // 4. If it's missing (kNotFound), create it. + if (metadata.status().code() == StatusCode::kNotFound) { + // Use a default project ID if the env var isn't set (common in local + // emulators). + auto project_id = google::cloud::internal::GetEnv("GOOGLE_CLOUD_PROJECT") + .value_or("test-project"); + + auto created = client.CreateBucketForProject(bucket_name_, project_id, + BucketMetadata()); + ASSERT_STATUS_OK(created) + << "Failed to auto-create missing bucket: " << bucket_name_; + } else { + // If it exists (or failed for another reason), assert it is OK. + ASSERT_STATUS_OK(metadata) + << "Failed to verify bucket existence: " << bucket_name_; + } + } }; /// @test Verify the Object CRUD (Create, Get, Update, Delete, List) operations. @@ -195,55 +215,7 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUD) { } /// @test Verify the Object CRUD operations with object contexts. -TEST_F(ObjectBasicCRUDIntegrationTest, BasicInsertAndGetWithObjectContexts) { - auto client = MakeIntegrationTestClient(); - - auto list_object_names = [&client, this] { - std::vector names; - for (auto o : client.ListObjects(bucket_name_)) { - EXPECT_STATUS_OK(o); - if (!o) break; - names.push_back(o->name()); - } - return names; - }; - - auto object_name = MakeRandomObjectName(); - ASSERT_THAT(list_object_names(), Not(Contains(object_name))) - << "Test aborted. The object <" << object_name << "> already exists." - << "This is unexpected as the test generates a random object name."; - - // Create the object, but only if it does not exist already, inserting - // a custom context {"department": "engineering"}. - StatusOr insert_meta = client.InsertObject( - bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), - Projection("full"), - WithObjectMetadata(ObjectMetadata().set_contexts( - ObjectContexts().upsert("department", {"engineering", {}, {}})))); - ASSERT_STATUS_OK(insert_meta); - EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); - - // Verify the response ObjectMetadata has the custom contexts we set. - StatusOr get_meta = client.GetObjectMetadata( - bucket_name_, object_name, Generation(insert_meta->generation()), - Projection("full")); - ASSERT_STATUS_OK(get_meta); - EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; - EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; - EXPECT_EQ("engineering", get_meta->contexts().custom().at("department").value) - << *get_meta; - EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").update_time)) - << *get_meta; - EXPECT_TRUE(IsSet(get_meta->contexts().custom().at("department").create_time)) - << *get_meta; - - auto status = client.DeleteObject(bucket_name_, object_name); - ASSERT_STATUS_OK(status); - EXPECT_THAT(list_object_names(), Not(Contains(object_name))); -} - -/// @test Verify the Object CRUD operations with object contexts. -TEST_F(ObjectBasicCRUDIntegrationTest, BasicUpdateWithObjectContexts) { +TEST_F(ObjectBasicCRUDIntegrationTest, BasicCRUDWithObjectContexts) { auto client = MakeIntegrationTestClient(); auto list_object_names = [&client, this] { @@ -261,8 +233,7 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicUpdateWithObjectContexts) { << "Test aborted. The object <" << object_name << "> already exists." << "This is unexpected as the test generates a random object name."; - // Create the object, but only if it does not exist already, inserting - // a custom context {"department": "engineering"}. + // 1. Insert Object with custom contexts. StatusOr insert_meta = client.InsertObject( bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), Projection("full"), @@ -270,16 +241,13 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicUpdateWithObjectContexts) { ObjectContexts().upsert("department", {"engineering", {}, {}})))); ASSERT_STATUS_OK(insert_meta); EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); - - // Verify the response ObjectMetadata has the custom contexts we set. StatusOr get_meta = client.GetObjectMetadata( bucket_name_, object_name, Generation(insert_meta->generation()), Projection("full")); ASSERT_STATUS_OK(get_meta); - EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; - EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + AssertHasCustomContext(*get_meta, "department", "engineering"); - // Update object with two keys. + // 2. Update object with two keys. ObjectMetadata update = *get_meta; update.set_contexts( ObjectContexts() @@ -289,125 +257,38 @@ TEST_F(ObjectBasicCRUDIntegrationTest, BasicUpdateWithObjectContexts) { bucket_name_, object_name, update, Generation(get_meta->generation()), Projection("full")); ASSERT_STATUS_OK(updated_meta); + AssertHasCustomContext(*updated_meta, "department", + "engineering and research"); + AssertHasCustomContext(*updated_meta, "region", "Asia Pacific"); - // Verify the response ObjectMetadata has the updated custom contexts. - EXPECT_TRUE(updated_meta->has_contexts()) << *updated_meta; - EXPECT_TRUE(updated_meta->contexts().has_key("department")) << *updated_meta; - // EXPECT_EQ("engineering and research", - // updated_meta->contexts().custom().at("department").value) - // << *updated_meta; - EXPECT_TRUE(updated_meta->contexts().has_key("region")) << *updated_meta; - // EXPECT_EQ("Asia Pacific", - // updated_meta->contexts().custom().at("region").value) - // << *updated_meta; - - // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. - auto status = client.DeleteObject(bucket_name_, object_name); - ASSERT_STATUS_OK(status); - EXPECT_THAT(list_object_names(), Not(Contains(object_name))); -} - -/// @test Verify the Object CRUD operations with object contexts. -TEST_F(ObjectBasicCRUDIntegrationTest, BasicPatchWithObjectContexts) { - auto client = MakeIntegrationTestClient(); - - auto list_object_names = [&client, this] { - std::vector names; - for (auto o : client.ListObjects(bucket_name_)) { - EXPECT_STATUS_OK(o); - if (!o) break; - names.push_back(o->name()); - } - return names; - }; - - auto object_name = MakeRandomObjectName(); - ASSERT_THAT(list_object_names(), Not(Contains(object_name))) - << "Test aborted. The object <" << object_name << "> already exists." - << "This is unexpected as the test generates a random object name."; - - // Create the object, but only if it does not exist already, inserting - // a custom context {"department": "engineering"}. - StatusOr insert_meta = client.InsertObject( - bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), - Projection("full"), - WithObjectMetadata(ObjectMetadata().set_contexts( - ObjectContexts().upsert("department", {"engineering", {}, {}})))); - ASSERT_STATUS_OK(insert_meta); - EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); - - // Verify the response ObjectMetadata has the custom contexts we set. - StatusOr get_meta = client.GetObjectMetadata( - bucket_name_, object_name, Generation(insert_meta->generation()), - Projection("full")); - ASSERT_STATUS_OK(get_meta); - EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; - EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; - + // 3. Patch the object contexts by updating one key's value. StatusOr patched_meta = client.PatchObject(bucket_name_, object_name, ObjectMetadataPatchBuilder().SetContext( "region", {"Asia Pacific - Singapore"}), Projection("full")); ASSERT_STATUS_OK(patched_meta); + AssertHasCustomContext(*patched_meta, "department", + "engineering and research"); + AssertHasCustomContext(*patched_meta, "region", "Asia Pacific - Singapore"); - EXPECT_TRUE(patched_meta->has_contexts()) << *patched_meta; - EXPECT_TRUE(patched_meta->contexts().has_key("department")) << *patched_meta; - EXPECT_TRUE(patched_meta->contexts().has_key("region")) << *patched_meta; - - // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. - auto status = client.DeleteObject(bucket_name_, object_name); - ASSERT_STATUS_OK(status); - EXPECT_THAT(list_object_names(), Not(Contains(object_name))); -} - -/// @test Verify the Object CRUD operations with object contexts. -TEST_F(ObjectBasicCRUDIntegrationTest, BasicResetWithObjectContexts) { - auto client = MakeIntegrationTestClient(); - - auto list_object_names = [&client, this] { - std::vector names; - for (auto o : client.ListObjects(bucket_name_)) { - EXPECT_STATUS_OK(o); - if (!o) break; - names.push_back(o->name()); - } - return names; - }; - - auto object_name = MakeRandomObjectName(); - ASSERT_THAT(list_object_names(), Not(Contains(object_name))) - << "Test aborted. The object <" << object_name << "> already exists." - << "This is unexpected as the test generates a random object name."; - - // Create the object, but only if it does not exist already, inserting - // a custom context {"department": "engineering"}. - StatusOr insert_meta = client.InsertObject( - bucket_name_, object_name, LoremIpsum(), IfGenerationMatch(0), - Projection("full"), - WithObjectMetadata(ObjectMetadata().set_contexts( - ObjectContexts().upsert("department", {"engineering", {}, {}})))); - ASSERT_STATUS_OK(insert_meta); - EXPECT_THAT(list_object_names(), Contains(object_name).Times(1)); - - // Verify the response ObjectMetadata has the custom contexts we set. - StatusOr get_meta = client.GetObjectMetadata( - bucket_name_, object_name, Generation(insert_meta->generation()), - Projection("full")); - ASSERT_STATUS_OK(get_meta); - EXPECT_TRUE(get_meta->has_contexts()) << *get_meta; - EXPECT_TRUE(get_meta->contexts().has_key("department")) << *get_meta; + // 4. Patch object contexts by deleting one existing key. + StatusOr reset_key_meta = client.PatchObject( + bucket_name_, object_name, + ObjectMetadataPatchBuilder().ResetContext("region"), Projection("full")); + ASSERT_STATUS_OK(reset_key_meta); + AssertHasCustomContext(*reset_key_meta, "department", + "engineering and research"); + EXPECT_FALSE(reset_key_meta->contexts().has_key("region")); - // Patch object with reset of all contexts. + // 5. Patch object with reset of all contexts. StatusOr reset_meta = client.PatchObject( bucket_name_, object_name, ObjectMetadataPatchBuilder().ResetContexts(), Projection("full")); ASSERT_STATUS_OK(reset_meta); - - // Verify the response ObjectMetadata that no custom contexts exists. EXPECT_FALSE(reset_meta->has_contexts()) << *reset_meta; - // This is the test for Object CRUD, we cannot rely on `ScheduleForDelete()`. + // 6. Delete the object away to clean up. auto status = client.DeleteObject(bucket_name_, object_name); ASSERT_STATUS_OK(status); EXPECT_THAT(list_object_names(), Not(Contains(object_name)));