From f31dc7ae74bd9695cb827c087a0540700516a073 Mon Sep 17 00:00:00 2001 From: dbolduc Date: Sat, 18 Jun 2022 20:07:23 -0400 Subject: [PATCH] impl(bigtable): Table accepts Options --- google/cloud/bigtable/table.cc | 13 +++++ google/cloud/bigtable/table.h | 20 +++++-- google/cloud/bigtable/table_test.cc | 85 +++++++++++++++++------------ 3 files changed, 78 insertions(+), 40 deletions(-) diff --git a/google/cloud/bigtable/table.cc b/google/cloud/bigtable/table.cc index 3361800918bd9..07d4b229def13 100644 --- a/google/cloud/bigtable/table.cc +++ b/google/cloud/bigtable/table.cc @@ -48,6 +48,7 @@ static_assert(std::is_copy_assignable::value, Status Table::Apply(SingleRowMutation mut) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->Apply(app_profile_id_, table_name_, std::move(mut)); } @@ -94,6 +95,7 @@ Status Table::Apply(SingleRowMutation mut) { future Table::AsyncApply(SingleRowMutation mut) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncApply(app_profile_id_, table_name_, std::move(mut)); } @@ -138,6 +140,7 @@ future Table::AsyncApply(SingleRowMutation mut) { std::vector Table::BulkApply(BulkMutation mut) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->BulkApply(app_profile_id_, table_name_, std::move(mut)); } @@ -171,6 +174,7 @@ std::vector Table::BulkApply(BulkMutation mut) { future> Table::AsyncBulkApply(BulkMutation mut) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncBulkApply(app_profile_id_, table_name_, std::move(mut)); } @@ -191,6 +195,7 @@ RowReader Table::ReadRows(RowSet row_set, Filter filter) { RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, Filter filter) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->ReadRows(app_profile_id_, table_name_, std::move(row_set), rows_limit, std::move(filter)); @@ -207,6 +212,7 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit, StatusOr> Table::ReadRow(std::string row_key, Filter filter) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->ReadRow(app_profile_id_, table_name_, std::move(row_key), std::move(filter)); } @@ -236,6 +242,7 @@ StatusOr Table::CheckAndMutateRow( std::string row_key, Filter filter, std::vector true_mutations, std::vector false_mutations) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->CheckAndMutateRow( app_profile_id_, table_name_, std::move(row_key), std::move(filter), std::move(true_mutations), std::move(false_mutations)); @@ -272,6 +279,7 @@ future> Table::AsyncCheckAndMutateRow( std::string row_key, Filter filter, std::vector true_mutations, std::vector false_mutations) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncCheckAndMutateRow( app_profile_id_, table_name_, std::move(row_key), std::move(filter), std::move(true_mutations), std::move(false_mutations)); @@ -325,6 +333,7 @@ future> Table::AsyncCheckAndMutateRow( // samples otherwise the result is an inconsistent set of sample row keys. StatusOr> Table::SampleRows() { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->SampleRows(app_profile_id_, table_name_); } @@ -370,6 +379,7 @@ StatusOr> Table::SampleRows() { future>> Table::AsyncSampleRows() { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncSampleRows(app_profile_id_, table_name_); } @@ -385,6 +395,7 @@ StatusOr Table::ReadModifyWriteRowImpl( ::google::bigtable::v2::ReadModifyWriteRowRequest>( request, app_profile_id_, table_name_); if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->ReadModifyWriteRow(std::move(request)); } @@ -406,6 +417,7 @@ future> Table::AsyncReadModifyWriteRowImpl( ::google::bigtable::v2::ReadModifyWriteRowRequest>( request, app_profile_id_, table_name_); if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncReadModifyWriteRow(std::move(request)); } @@ -435,6 +447,7 @@ future> Table::AsyncReadModifyWriteRowImpl( future>> Table::AsyncReadRow(std::string row_key, Filter filter) { if (connection_) { + google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncReadRow(app_profile_id_, table_name_, std::move(row_key), std::move(filter)); } diff --git a/google/cloud/bigtable/table.h b/google/cloud/bigtable/table.h index 0cc18390610b8..f1e5bcb3eec44 100644 --- a/google/cloud/bigtable/table.h +++ b/google/cloud/bigtable/table.h @@ -20,6 +20,7 @@ #include "google/cloud/bigtable/filters.h" #include "google/cloud/bigtable/idempotent_mutation_policy.h" #include "google/cloud/bigtable/internal/data_connection.h" +#include "google/cloud/bigtable/internal/defaults.h" #include "google/cloud/bigtable/internal/legacy_async_row_reader.h" #include "google/cloud/bigtable/mutation_branch.h" #include "google/cloud/bigtable/mutations.h" @@ -52,7 +53,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN // public. bigtable::Table MakeTable(std::shared_ptr conn, std::string project_id, std::string instance_id, - std::string app_profile_id, std::string table_id); + std::string app_profile_id, std::string table_id, + Options options = {}); GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace bigtable_internal @@ -888,6 +890,7 @@ class Table { "RowFunctor should return a future."); if (connection_) { + google::cloud::internal::OptionsSpan span(options_); connection_->AsyncReadRows( app_profile_id_, table_name_, std::move(on_row), std::move(on_finish), std::move(row_set), rows_limit, std::move(filter)); @@ -937,10 +940,11 @@ class Table { private: friend Table bigtable_internal::MakeTable( std::shared_ptr, std::string, - std::string, std::string, std::string); + std::string, std::string, std::string, Options); explicit Table(std::shared_ptr conn, std::string project_id, std::string instance_id, - std::string app_profile_id, std::string table_id) + std::string app_profile_id, std::string table_id, + Options options = {}) : app_profile_id_(std::move(app_profile_id)), project_id_(std::move(project_id)), instance_id_(std::move(instance_id)), @@ -948,7 +952,10 @@ class Table { table_id_(std::move(table_id)), metadata_update_policy_( MetadataUpdatePolicy(table_name_, MetadataParamTypes::TABLE_NAME)), - connection_(std::move(conn)) {} + connection_(std::move(conn)), + options_(google::cloud::internal::MergeOptions( + std::move(options), + internal::DefaultDataOptions(connection_->options()))) {} /** * Send request ReadModifyWriteRowRequest to modify the row and get it back @@ -1021,6 +1028,7 @@ class Table { std::shared_ptr idempotent_mutation_policy_; std::shared_ptr background_threads_; std::shared_ptr connection_; + Options options_; }; GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END @@ -1032,10 +1040,10 @@ inline bigtable::Table MakeTable(std::shared_ptr conn, std::string project_id, std::string instance_id, std::string app_profile_id, - std::string table_id) { + std::string table_id, Options options) { return bigtable::Table(std::move(conn), std::move(project_id), std::move(instance_id), std::move(app_profile_id), - std::move(table_id)); + std::move(table_id), std::move(options)); } GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/bigtable/table_test.cc b/google/cloud/bigtable/table_test.cc index 160a192f5b925..2739805e443fa 100644 --- a/google/cloud/bigtable/table_test.cc +++ b/google/cloud/bigtable/table_test.cc @@ -34,6 +34,7 @@ using ::testing::Eq; using ::testing::Matcher; using ::testing::MockFunction; using ::testing::Property; +using ::testing::Return; auto const* const kProjectId = "test-project"; auto const* const kInstanceId = "test-instance"; @@ -110,10 +111,26 @@ Matcher MatchRule( r.increment_amount())); } +struct TestOption { + using Type = bool; +}; + +Options TableOptions() { return Options{}.set(true); } + +Table TestTable(std::shared_ptr mock) { + EXPECT_CALL(*mock, options).WillOnce(Return(Options{})); + return bigtable_internal::MakeTable(std::move(mock), kProjectId, kInstanceId, + kAppProfileId, kTableId, TableOptions()); +} + +void CheckCurrentOptions() { + auto const& options = google::cloud::internal::CurrentOptions(); + EXPECT_TRUE(options.has()); +} + TEST(TableTest, ConnectionConstructor) { auto conn = std::make_shared(); - auto table = bigtable_internal::MakeTable(conn, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(conn)); EXPECT_EQ(kAppProfileId, table.app_profile_id()); EXPECT_EQ(kProjectId, table.project_id()); EXPECT_EQ(kInstanceId, table.instance_id()); @@ -127,14 +144,14 @@ TEST(TableTest, Apply) { .WillOnce([](std::string const& app_profile_id, std::string const& table_name, bigtable::SingleRowMutation const& mut) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ(mut.row_key(), "row"); return PermanentError(); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto status = table.Apply(IdempotentMutation()); EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); } @@ -145,14 +162,14 @@ TEST(TableTest, AsyncApply) { .WillOnce([](std::string const& app_profile_id, std::string const& table_name, bigtable::SingleRowMutation const& mut) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ(mut.row_key(), "row"); return make_ready_future(PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto status = table.AsyncApply(IdempotentMutation()).get(); EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); } @@ -165,14 +182,14 @@ TEST(TableTest, BulkApply) { .WillOnce([&expected](std::string const& app_profile_id, std::string const& table_name, bigtable::BulkMutation const& mut) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ(mut.size(), 2); return expected; }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto actual = table.BulkApply( BulkMutation(IdempotentMutation(), NonIdempotentMutation())); CheckFailedMutations(actual, expected); @@ -186,14 +203,14 @@ TEST(TableTest, AsyncBulkApply) { .WillOnce([&expected](std::string const& app_profile_id, std::string const& table_name, bigtable::BulkMutation const& mut) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ(mut.size(), 2); return make_ready_future(expected); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto actual = table.AsyncBulkApply( BulkMutation(IdempotentMutation(), NonIdempotentMutation())); CheckFailedMutations(actual.get(), expected); @@ -206,6 +223,7 @@ TEST(TableTest, ReadRows) { std::string const& table_name, bigtable::RowSet const& row_set, std::int64_t rows_limit, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_THAT(row_set, IsTestRowSet()); @@ -215,8 +233,7 @@ TEST(TableTest, ReadRows) { PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto reader = table.ReadRows(TestRowSet(), TestFilter()); auto it = reader.begin(); EXPECT_THAT(*it, StatusIs(StatusCode::kPermissionDenied)); @@ -230,6 +247,7 @@ TEST(TableTest, ReadRowsWithRowLimit) { std::string const& table_name, bigtable::RowSet const& row_set, std::int64_t rows_limit, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_THAT(row_set, IsTestRowSet()); @@ -239,8 +257,7 @@ TEST(TableTest, ReadRowsWithRowLimit) { PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto reader = table.ReadRows(TestRowSet(), 42, TestFilter()); auto it = reader.begin(); EXPECT_THAT(*it, StatusIs(StatusCode::kPermissionDenied)); @@ -253,6 +270,7 @@ TEST(TableTest, ReadRow) { .WillOnce([](std::string const& app_profile_id, std::string const& table_name, std::string const& row_key, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ("row", row_key); @@ -260,8 +278,7 @@ TEST(TableTest, ReadRow) { return PermanentError(); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto resp = table.ReadRow("row", TestFilter()); EXPECT_THAT(resp, StatusIs(StatusCode::kPermissionDenied)); } @@ -274,6 +291,7 @@ TEST(TableTest, CheckAndMutateRow) { Filter const& filter, std::vector const& true_mutations, std::vector const& false_mutations) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ("row", row_key); @@ -288,8 +306,7 @@ TEST(TableTest, CheckAndMutateRow) { auto t1 = bigtable::SetCell("f1", "c1", ms(0), "true1"); auto f1 = bigtable::SetCell("f1", "c1", ms(0), "false1"); auto f2 = bigtable::SetCell("f2", "c2", ms(0), "false2"); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto row = table.CheckAndMutateRow("row", TestFilter(), {t1}, {f1, f2}); EXPECT_THAT(row, StatusIs(StatusCode::kPermissionDenied)); } @@ -302,6 +319,7 @@ TEST(TableTest, AsyncCheckAndMutateRow) { Filter const& filter, std::vector const& true_mutations, std::vector const& false_mutations) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ("row", row_key); @@ -317,8 +335,7 @@ TEST(TableTest, AsyncCheckAndMutateRow) { auto t1 = bigtable::SetCell("f1", "c1", ms(0), "true1"); auto f1 = bigtable::SetCell("f1", "c1", ms(0), "false1"); auto f2 = bigtable::SetCell("f2", "c2", ms(0), "false2"); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto row = table.AsyncCheckAndMutateRow("row", TestFilter(), {t1}, {f1, f2}).get(); EXPECT_THAT(row, StatusIs(StatusCode::kPermissionDenied)); @@ -329,13 +346,13 @@ TEST(TableTest, SampleRows) { EXPECT_CALL(*mock, SampleRows) .WillOnce( [](std::string const& app_profile_id, std::string const& table_name) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); return PermanentError(); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto samples = table.SampleRows(); EXPECT_THAT(samples, StatusIs(StatusCode::kPermissionDenied)); } @@ -345,14 +362,14 @@ TEST(TableTest, AsyncSampleRows) { EXPECT_CALL(*mock, AsyncSampleRows) .WillOnce( [](std::string const& app_profile_id, std::string const& table_name) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); return make_ready_future>>( PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto samples = table.AsyncSampleRows().get(); EXPECT_THAT(samples, StatusIs(StatusCode::kPermissionDenied)); } @@ -361,6 +378,7 @@ TEST(TableTest, ReadModifyWriteRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, ReadModifyWriteRow) .WillOnce([](v2::ReadModifyWriteRowRequest const& request) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, request.app_profile_id()); EXPECT_EQ(kTableName, request.table_name()); EXPECT_THAT(request.rules(), @@ -369,8 +387,7 @@ TEST(TableTest, ReadModifyWriteRow) { return PermanentError(); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto row = table.ReadModifyWriteRow("row", TestAppendRule(), TestIncrementRule()); EXPECT_THAT(row, StatusIs(StatusCode::kPermissionDenied)); @@ -380,6 +397,7 @@ TEST(TableTest, AsyncReadModifyWriteRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncReadModifyWriteRow) .WillOnce([](v2::ReadModifyWriteRowRequest const& request) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, request.app_profile_id()); EXPECT_EQ(kTableName, request.table_name()); EXPECT_THAT(request.rules(), @@ -388,8 +406,7 @@ TEST(TableTest, AsyncReadModifyWriteRow) { return make_ready_future>(PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto row = table.AsyncReadModifyWriteRow("row", TestAppendRule(), TestIncrementRule()); EXPECT_THAT(row.get(), StatusIs(StatusCode::kPermissionDenied)); @@ -404,6 +421,7 @@ TEST(TableTest, AsyncReadRows) { std::function const& on_finish, bigtable::RowSet const& row_set, std::int64_t rows_limit, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_THAT(row_set, IsTestRowSet()); @@ -432,8 +450,7 @@ TEST(TableTest, AsyncReadRows) { EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); table.AsyncReadRows(on_row.AsStdFunction(), on_finish.AsStdFunction(), TestRowSet(), TestFilter()); } @@ -447,6 +464,7 @@ TEST(TableTest, AsyncReadRowsWithRowLimit) { std::function const& on_finish, bigtable::RowSet const& row_set, std::int64_t rows_limit, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_THAT(row_set, IsTestRowSet()); @@ -475,8 +493,7 @@ TEST(TableTest, AsyncReadRowsWithRowLimit) { EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); table.AsyncReadRows(on_row.AsStdFunction(), on_finish.AsStdFunction(), TestRowSet(), 42, TestFilter()); } @@ -487,6 +504,7 @@ TEST(TableTest, AsyncReadRow) { .WillOnce([](std::string const& app_profile_id, std::string const& table_name, std::string const& row_key, bigtable::Filter const& filter) { + CheckCurrentOptions(); EXPECT_EQ(kAppProfileId, app_profile_id); EXPECT_EQ(kTableName, table_name); EXPECT_EQ("row", row_key); @@ -495,8 +513,7 @@ TEST(TableTest, AsyncReadRow) { PermanentError()); }); - auto table = bigtable_internal::MakeTable(mock, kProjectId, kInstanceId, - kAppProfileId, kTableId); + auto table = TestTable(std::move(mock)); auto resp = table.AsyncReadRow("row", TestFilter()).get(); EXPECT_THAT(resp, StatusIs(StatusCode::kPermissionDenied)); }