From c290d9a408d006b372792bd5e7262c965a59ebe6 Mon Sep 17 00:00:00 2001 From: Darren Bolduc Date: Wed, 29 Jun 2022 23:19:08 -0400 Subject: [PATCH] fix(bigtable)!: pass app profile id to connection as options (#9388) --- google/cloud/bigtable/data_connection.cc | 30 ++- google/cloud/bigtable/data_connection.h | 41 ++-- .../bigtable/internal/data_connection_impl.cc | 67 +++--- .../bigtable/internal/data_connection_impl.h | 41 ++-- .../internal/data_connection_impl_test.cc | 194 +++++++++++------- .../bigtable/mocks/mock_data_connection.h | 44 ++-- google/cloud/bigtable/table.cc | 51 +++-- google/cloud/bigtable/table.h | 35 ++-- google/cloud/bigtable/table_test.cc | 80 +++----- 9 files changed, 282 insertions(+), 301 deletions(-) diff --git a/google/cloud/bigtable/data_connection.cc b/google/cloud/bigtable/data_connection.cc index 7c162ff6a6fb2..b3b80f375e5ca 100644 --- a/google/cloud/bigtable/data_connection.cc +++ b/google/cloud/bigtable/data_connection.cc @@ -43,53 +43,52 @@ std::vector MakeUnimplementedFailedMutations(std::size_t n) { DataConnection::~DataConnection() = default; -Status DataConnection::Apply( - // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, SingleRowMutation) { +// NOLINTNEXTLINE(performance-unnecessary-value-param) +Status DataConnection::Apply(std::string const&, SingleRowMutation) { return Status(StatusCode::kUnimplemented, "not implemented"); } future DataConnection::AsyncApply( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, SingleRowMutation) { + std::string const&, SingleRowMutation) { return make_ready_future( Status(StatusCode::kUnimplemented, "not implemented")); } std::vector DataConnection::BulkApply( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, BulkMutation mut) { + std::string const&, BulkMutation mut) { return MakeUnimplementedFailedMutations(mut.size()); } future> DataConnection::AsyncBulkApply( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, BulkMutation mut) { + std::string const&, BulkMutation mut) { return make_ready_future(MakeUnimplementedFailedMutations(mut.size())); } RowReader DataConnection::ReadRows( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, RowSet, std::int64_t, Filter) { + std::string const&, RowSet, std::int64_t, Filter) { return MakeRowReader(std::make_shared( Status(StatusCode::kUnimplemented, "not implemented"))); } StatusOr> DataConnection::ReadRow( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, std::string, Filter) { + std::string const&, std::string, Filter) { return Status(StatusCode::kUnimplemented, "not implemented"); } StatusOr DataConnection::CheckAndMutateRow( - std::string const&, std::string const&, + std::string const&, // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string, Filter, std::vector, std::vector) { return Status(StatusCode::kUnimplemented, "not implemented"); } future> DataConnection::AsyncCheckAndMutateRow( - std::string const&, std::string const&, + std::string const&, // NOLINTNEXTLINE(performance-unnecessary-value-param) std::string, Filter, std::vector, std::vector) { return make_ready_future>( @@ -97,12 +96,12 @@ future> DataConnection::AsyncCheckAndMutateRow( } StatusOr> DataConnection::SampleRows( - std::string const&, std::string const&) { + std::string const&) { return Status(StatusCode::kUnimplemented, "not implemented"); } future>> DataConnection::AsyncSampleRows( - std::string const&, std::string const&) { + std::string const&) { return make_ready_future>>( Status(StatusCode::kUnimplemented, "not implemented")); } @@ -121,17 +120,16 @@ future> DataConnection::AsyncReadModifyWriteRow( } void DataConnection::AsyncReadRows( - std::string const&, std::string const&, // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::function(Row)>, std::function on_finish, + std::string const&, std::function(Row)>, // NOLINTNEXTLINE(performance-unnecessary-value-param) - RowSet, std::int64_t, Filter) { + std::function on_finish, RowSet, std::int64_t, Filter) { on_finish(Status(StatusCode::kUnimplemented, "not implemented")); } future>> DataConnection::AsyncReadRow( // NOLINTNEXTLINE(performance-unnecessary-value-param) - std::string const&, std::string const&, std::string, Filter) { + std::string const&, std::string, Filter) { return make_ready_future>>( Status(StatusCode::kUnimplemented, "not implemented")); } diff --git a/google/cloud/bigtable/data_connection.h b/google/cloud/bigtable/data_connection.h index 646fae78ed576..04ba5bf51cf32 100644 --- a/google/cloud/bigtable/data_connection.h +++ b/google/cloud/bigtable/data_connection.h @@ -52,44 +52,39 @@ class DataConnection { virtual Options options() { return Options{}; } - virtual Status Apply(std::string const& app_profile_id, - std::string const& table_name, SingleRowMutation mut); + virtual Status Apply(std::string const& table_name, SingleRowMutation mut); - virtual future AsyncApply(std::string const& app_profile_id, - std::string const& table_name, + virtual future AsyncApply(std::string const& table_name, SingleRowMutation mut); - virtual std::vector BulkApply( - std::string const& app_profile_id, std::string const& table_name, - BulkMutation mut); + virtual std::vector BulkApply(std::string const& table_name, + BulkMutation mut); virtual future> AsyncBulkApply( - std::string const& app_profile_id, std::string const& table_name, - BulkMutation mut); + std::string const& table_name, BulkMutation mut); - virtual RowReader ReadRows(std::string const& app_profile_id, - std::string const& table_name, RowSet row_set, + virtual RowReader ReadRows(std::string const& table_name, RowSet row_set, std::int64_t rows_limit, Filter filter); - virtual StatusOr> ReadRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, Filter filter); + virtual StatusOr> ReadRow(std::string const& table_name, + std::string row_key, + Filter filter); virtual StatusOr CheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, Filter filter, std::vector true_mutations, + std::string const& table_name, std::string row_key, Filter filter, + std::vector true_mutations, std::vector false_mutations); virtual future> AsyncCheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, Filter filter, std::vector true_mutations, + std::string const& table_name, std::string row_key, Filter filter, + std::vector true_mutations, std::vector false_mutations); virtual StatusOr> SampleRows( - std::string const& app_profile_id, std::string const& table_name); + std::string const& table_name); virtual future>> AsyncSampleRows( - std::string const& app_profile_id, std::string const& table_name); + std::string const& table_name); virtual StatusOr ReadModifyWriteRow( google::bigtable::v2::ReadModifyWriteRowRequest request); @@ -97,16 +92,14 @@ class DataConnection { virtual future> AsyncReadModifyWriteRow( google::bigtable::v2::ReadModifyWriteRowRequest request); - virtual void AsyncReadRows(std::string const& app_profile_id, - std::string const& table_name, + virtual void AsyncReadRows(std::string const& table_name, std::function(Row)> on_row, std::function on_finish, RowSet row_set, std::int64_t rows_limit, Filter filter); virtual future>> AsyncReadRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, Filter filter); + std::string const& table_name, std::string row_key, Filter filter); }; /** diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc index 14652b2eb2681..bf635e87ec125 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.cc +++ b/google/cloud/bigtable/internal/data_connection_impl.cc @@ -59,11 +59,10 @@ DataConnectionImpl::DataConnectionImpl( std::move(options), bigtable::internal::DefaultDataOptions(DataConnection::options()))) {} -Status DataConnectionImpl::Apply(std::string const& app_profile_id, - std::string const& table_name, +Status DataConnectionImpl::Apply(std::string const& table_name, bigtable::SingleRowMutation mut) { google::bigtable::v2::MutateRowRequest request; - request.set_app_profile_id(app_profile_id); + request.set_app_profile_id(app_profile_id()); request.set_table_name(table_name); mut.MoveTo(request); @@ -86,11 +85,10 @@ Status DataConnectionImpl::Apply(std::string const& app_profile_id, return Status{}; } -future DataConnectionImpl::AsyncApply(std::string const& app_profile_id, - std::string const& table_name, +future DataConnectionImpl::AsyncApply(std::string const& table_name, bigtable::SingleRowMutation mut) { google::bigtable::v2::MutateRowRequest request; - request.set_app_profile_id(app_profile_id); + request.set_app_profile_id(app_profile_id()); request.set_table_name(table_name); mut.MoveTo(request); @@ -121,11 +119,10 @@ future DataConnectionImpl::AsyncApply(std::string const& app_profile_id, } std::vector DataConnectionImpl::BulkApply( - std::string const& app_profile_id, std::string const& table_name, - bigtable::BulkMutation mut) { + std::string const& table_name, bigtable::BulkMutation mut) { if (mut.empty()) return {}; bigtable::internal::BulkMutator mutator( - app_profile_id, table_name, *idempotency_policy(), std::move(mut)); + app_profile_id(), table_name, *idempotency_policy(), std::move(mut)); // We wait to allocate the policies until they are needed as a // micro-optimization. std::unique_ptr retry; @@ -143,31 +140,30 @@ std::vector DataConnectionImpl::BulkApply( } future> -DataConnectionImpl::AsyncBulkApply(std::string const& app_profile_id, - std::string const& table_name, +DataConnectionImpl::AsyncBulkApply(std::string const& table_name, bigtable::BulkMutation mut) { return AsyncBulkApplier::Create(background_->cq(), stub_, retry_policy(), backoff_policy(), *idempotency_policy(), - app_profile_id, table_name, std::move(mut)); + app_profile_id(), table_name, std::move(mut)); } -bigtable::RowReader DataConnectionImpl::ReadRows( - std::string const& app_profile_id, std::string const& table_name, - bigtable::RowSet row_set, std::int64_t rows_limit, - bigtable::Filter filter) { +bigtable::RowReader DataConnectionImpl::ReadRows(std::string const& table_name, + bigtable::RowSet row_set, + std::int64_t rows_limit, + bigtable::Filter filter) { auto impl = std::make_shared( - stub_, app_profile_id, table_name, std::move(row_set), rows_limit, + stub_, app_profile_id(), table_name, std::move(row_set), rows_limit, std::move(filter), retry_policy(), backoff_policy()); return MakeRowReader(std::move(impl)); } StatusOr> DataConnectionImpl::ReadRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter) { + std::string const& table_name, std::string row_key, + bigtable::Filter filter) { bigtable::RowSet row_set(std::move(row_key)); std::int64_t const rows_limit = 1; - auto reader = ReadRows(app_profile_id, table_name, std::move(row_set), - rows_limit, std::move(filter)); + auto reader = + ReadRows(table_name, std::move(row_set), rows_limit, std::move(filter)); auto it = reader.begin(); if (it == reader.end()) return std::make_pair(false, bigtable::Row("", {})); @@ -182,12 +178,11 @@ StatusOr> DataConnectionImpl::ReadRow( } StatusOr DataConnectionImpl::CheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, + std::string const& table_name, std::string row_key, bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations) { google::bigtable::v2::CheckAndMutateRowRequest request; - request.set_app_profile_id(app_profile_id); + request.set_app_profile_id(app_profile_id()); request.set_table_name(table_name); request.set_row_key(std::move(row_key)); *request.mutable_predicate_filter() = std::move(filter).as_proto(); @@ -216,12 +211,11 @@ StatusOr DataConnectionImpl::CheckAndMutateRow( future> DataConnectionImpl::AsyncCheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, + std::string const& table_name, std::string row_key, bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations) { google::bigtable::v2::CheckAndMutateRowRequest request; - request.set_app_profile_id(app_profile_id); + request.set_app_profile_id(app_profile_id()); request.set_table_name(table_name); request.set_row_key(std::move(row_key)); *request.mutable_predicate_filter() = std::move(filter).as_proto(); @@ -258,9 +252,9 @@ DataConnectionImpl::AsyncCheckAndMutateRow( } StatusOr> DataConnectionImpl::SampleRows( - std::string const& app_profile_id, std::string const& table_name) { + std::string const& table_name) { google::bigtable::v2::SampleRowKeysRequest request; - request.set_app_profile_id(app_profile_id); + request.set_app_profile_id(app_profile_id()); request.set_table_name(table_name); Status status; @@ -307,10 +301,10 @@ StatusOr> DataConnectionImpl::SampleRows( } future>> -DataConnectionImpl::AsyncSampleRows(std::string const& app_profile_id, - std::string const& table_name) { +DataConnectionImpl::AsyncSampleRows(std::string const& table_name) { return AsyncRowSampler::Create(background_->cq(), stub_, retry_policy(), - backoff_policy(), app_profile_id, table_name); + backoff_policy(), app_profile_id(), + table_name); } StatusOr DataConnectionImpl::ReadModifyWriteRow( @@ -350,19 +344,18 @@ future> DataConnectionImpl::AsyncReadModifyWriteRow( } void DataConnectionImpl::AsyncReadRows( - std::string const& app_profile_id, std::string const& table_name, + std::string const& table_name, std::function(bigtable::Row)> on_row, std::function on_finish, bigtable::RowSet row_set, std::int64_t rows_limit, bigtable::Filter filter) { bigtable_internal::AsyncRowReader::Create( - background_->cq(), stub_, app_profile_id, table_name, std::move(on_row), + background_->cq(), stub_, app_profile_id(), table_name, std::move(on_row), std::move(on_finish), std::move(row_set), rows_limit, std::move(filter), retry_policy(), backoff_policy()); } future>> -DataConnectionImpl::AsyncReadRow(std::string const& app_profile_id, - std::string const& table_name, +DataConnectionImpl::AsyncReadRow(std::string const& table_name, std::string row_key, bigtable::Filter filter) { class AsyncReadRowHandler { public: @@ -409,7 +402,7 @@ DataConnectionImpl::AsyncReadRow(std::string const& app_profile_id, std::int64_t const rows_limit = 1; auto handler = std::make_shared(); AsyncReadRows( - app_profile_id, table_name, + table_name, [handler](bigtable::Row row) { return handler->OnRow(std::move(row)); }, [handler](Status status) { handler->OnStreamFinished(std::move(status)); diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h index 579f71886fcfc..51c831bfd90d3 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.h +++ b/google/cloud/bigtable/internal/data_connection_impl.h @@ -41,49 +41,41 @@ class DataConnectionImpl : public bigtable::DataConnection { Options options() override { return options_; } - Status Apply(std::string const& app_profile_id, std::string const& table_name, + Status Apply(std::string const& table_name, bigtable::SingleRowMutation mut) override; - future AsyncApply(std::string const& app_profile_id, - std::string const& table_name, + future AsyncApply(std::string const& table_name, bigtable::SingleRowMutation mut) override; std::vector BulkApply( - std::string const& app_profile_id, std::string const& table_name, - bigtable::BulkMutation mut) override; + std::string const& table_name, bigtable::BulkMutation mut) override; future> AsyncBulkApply( - std::string const& app_profile_id, std::string const& table_name, - bigtable::BulkMutation mut) override; + std::string const& table_name, bigtable::BulkMutation mut) override; - bigtable::RowReader ReadRows(std::string const& app_profile_id, - std::string const& table_name, + bigtable::RowReader ReadRows(std::string const& table_name, bigtable::RowSet row_set, std::int64_t rows_limit, bigtable::Filter filter) override; StatusOr> ReadRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter) override; + std::string const& table_name, std::string row_key, + bigtable::Filter filter) override; StatusOr CheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, - std::vector true_mutations, + std::string const& table_name, std::string row_key, + bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations) override; future> AsyncCheckAndMutateRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, - std::vector true_mutations, + std::string const& table_name, std::string row_key, + bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations) override; StatusOr> SampleRows( - std::string const& app_profile_id, std::string const& table_name) override; future>> AsyncSampleRows( - std::string const& app_profile_id, std::string const& table_name) override; StatusOr ReadModifyWriteRow( @@ -92,18 +84,21 @@ class DataConnectionImpl : public bigtable::DataConnection { future> AsyncReadModifyWriteRow( google::bigtable::v2::ReadModifyWriteRowRequest request) override; - void AsyncReadRows(std::string const& app_profile_id, - std::string const& table_name, + void AsyncReadRows(std::string const& table_name, std::function(bigtable::Row)> on_row, std::function on_finish, bigtable::RowSet row_set, std::int64_t rows_limit, bigtable::Filter filter) override; future>> AsyncReadRow( - std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter) override; + std::string const& table_name, std::string row_key, + bigtable::Filter filter) override; private: + static std::string const& app_profile_id() { + return internal::CurrentOptions().get(); + } + std::unique_ptr retry_policy() { auto const& options = internal::CurrentOptions(); if (options.has()) { diff --git a/google/cloud/bigtable/internal/data_connection_impl_test.cc b/google/cloud/bigtable/internal/data_connection_impl_test.cc index e631f166deff8..9a0889c0ac528 100644 --- a/google/cloud/bigtable/internal/data_connection_impl_test.cc +++ b/google/cloud/bigtable/internal/data_connection_impl_test.cc @@ -32,6 +32,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN namespace { namespace v2 = ::google::bigtable::v2; +using ::google::cloud::bigtable::AppProfileIdOption; using ::google::cloud::bigtable::DataBackoffPolicyOption; using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy; using ::google::cloud::bigtable::DataRetryPolicyOption; @@ -281,7 +282,7 @@ TEST(DataConnectionTest, CallOptionsOverrideConnectionOptions) { auto conn = std::make_shared(nullptr, mock, std::move(conn_opts)); internal::OptionsSpan span(call_opts); - (void)conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + (void)conn->Apply(kTableName, IdempotentMutation("row")); } /** @@ -313,7 +314,7 @@ TEST(DataConnectionTest, UseConnectionOptionsIfNoCallOptions) { auto conn = std::make_shared(nullptr, mock, std::move(conn_opts)); - (void)conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + (void)conn->Apply(kTableName, IdempotentMutation("row")); } TEST(DataConnectionTest, ApplySuccess) { @@ -326,8 +327,9 @@ TEST(DataConnectionTest, ApplySuccess) { return v2::MutateRowResponse{}; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->Apply(kTableName, IdempotentMutation("row")); ASSERT_STATUS_OK(status); } @@ -341,8 +343,9 @@ TEST(DataConnectionTest, ApplyPermanentFailure) { return PermanentError(); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->Apply(kTableName, IdempotentMutation("row")); EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); } @@ -362,8 +365,9 @@ TEST(DataConnectionTest, ApplyRetryThenSuccess) { return v2::MutateRowResponse{}; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->Apply(kTableName, IdempotentMutation("row")); ASSERT_STATUS_OK(status); } @@ -387,9 +391,11 @@ TEST(DataConnectionTest, ApplyRetryExhausted) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_b))); + Options{} + .set(std::move(mock_b)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->Apply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->Apply(kTableName, IdempotentMutation("row")); EXPECT_THAT(status, StatusIs(StatusCode::kUnavailable)); } @@ -412,10 +418,11 @@ TEST(DataConnectionTest, ApplyRetryIdempotency) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_i))); + Options{} + .set(std::move(mock_i)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = - conn->Apply(kAppProfile, kTableName, NonIdempotentMutation("row")); + auto status = conn->Apply(kTableName, NonIdempotentMutation("row")); EXPECT_THAT(status, StatusIs(StatusCode::kUnavailable)); } @@ -431,9 +438,9 @@ TEST(DataConnectionTest, AsyncApplySuccess) { return make_ready_future(make_status_or(v2::MutateRowResponse{})); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = - conn->AsyncApply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->AsyncApply(kTableName, IdempotentMutation("row")); ASSERT_STATUS_OK(status.get()); } @@ -450,9 +457,9 @@ TEST(DataConnectionTest, AsyncApplyPermanentFailure) { PermanentError()); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = - conn->AsyncApply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->AsyncApply(kTableName, IdempotentMutation("row")); EXPECT_THAT(status.get(), StatusIs(StatusCode::kPermissionDenied)); } @@ -478,10 +485,11 @@ TEST(DataConnectionTest, AsyncApplyRetryExhausted) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_b))); + Options{} + .set(std::move(mock_b)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = - conn->AsyncApply(kAppProfile, kTableName, IdempotentMutation("row")); + auto status = conn->AsyncApply(kTableName, IdempotentMutation("row")); EXPECT_THAT(status.get(), StatusIs(StatusCode::kUnavailable)); } @@ -507,18 +515,18 @@ TEST(DataConnectionTest, AsyncApplyRetryIdempotency) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_i))); + Options{} + .set(std::move(mock_i)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = - conn->AsyncApply(kAppProfile, kTableName, NonIdempotentMutation("row")); + auto status = conn->AsyncApply(kTableName, NonIdempotentMutation("row")); EXPECT_THAT(status.get(), StatusIs(StatusCode::kUnavailable)); } TEST(DataConnectionTest, BulkApplyEmpty) { auto mock = std::make_shared(); auto conn = TestConnection(std::move(mock)); - auto actual = - conn->BulkApply(kAppProfile, kTableName, bigtable::BulkMutation()); + auto actual = conn->BulkApply(kTableName, bigtable::BulkMutation()); CheckFailedMutations(actual, {}); } @@ -541,8 +549,9 @@ TEST(DataConnectionTest, BulkApplySuccess) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->BulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual, {}); } @@ -585,8 +594,9 @@ TEST(DataConnectionTest, BulkApplyRetryMutationPolicy) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->BulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual, expected); } @@ -620,8 +630,9 @@ TEST(DataConnectionTest, BulkApplyIncompleteStreamRetried) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->BulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual, {}); } @@ -650,9 +661,11 @@ TEST(DataConnectionTest, BulkApplyStreamRetryExhausted) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_b))); + Options{} + .set(std::move(mock_b)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->BulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual, expected); } @@ -671,8 +684,9 @@ TEST(DataConnectionTest, BulkApplyStreamPermanentError) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->BulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual, expected); } @@ -699,9 +713,11 @@ TEST(DataConnectionTest, BulkApplyNoSleepIfNoPendingMutations) { EXPECT_CALL(*mock_b, clone).Times(0); internal::OptionsSpan span( - Options{}.set(std::move(mock_b))); + Options{} + .set(std::move(mock_b)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - (void)conn->BulkApply(kAppProfile, kTableName, std::move(mut)); + (void)conn->BulkApply(kTableName, std::move(mut)); } // The `AsyncBulkApplier` is tested extensively in `async_bulk_apply_test.cc`. @@ -721,8 +737,9 @@ TEST(DataConnectionTest, AsyncBulkApply) { return absl::make_unique(PermanentError()); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto actual = conn->AsyncBulkApply(kAppProfile, kTableName, std::move(mut)); + auto actual = conn->AsyncBulkApply(kTableName, std::move(mut)); CheckFailedMutations(actual.get(), expected); } @@ -744,9 +761,9 @@ TEST(DataConnectionTest, ReadRows) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto reader = - conn->ReadRows(kAppProfile, kTableName, TestRowSet(), 42, TestFilter()); + auto reader = conn->ReadRows(kTableName, TestRowSet(), 42, TestFilter()); EXPECT_EQ(reader.begin(), reader.end()); } @@ -766,8 +783,9 @@ TEST(DataConnectionTest, ReadRowEmpty) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = conn->ReadRow(kAppProfile, kTableName, "row", TestFilter()); + auto resp = conn->ReadRow(kTableName, "row", TestFilter()); ASSERT_STATUS_OK(resp); EXPECT_FALSE(resp->first); } @@ -798,8 +816,9 @@ TEST(DataConnectionTest, ReadRowSuccess) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = conn->ReadRow(kAppProfile, kTableName, "row", TestFilter()); + auto resp = conn->ReadRow(kTableName, "row", TestFilter()); ASSERT_STATUS_OK(resp); EXPECT_TRUE(resp->first); EXPECT_EQ("row", resp->second.row_key()); @@ -821,8 +840,9 @@ TEST(DataConnectionTest, ReadRowFailure) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = conn->ReadRow(kAppProfile, kTableName, "row", TestFilter()); + auto resp = conn->ReadRow(kTableName, "row", TestFilter()); EXPECT_THAT(resp, StatusIs(StatusCode::kPermissionDenied)); } @@ -865,14 +885,15 @@ TEST(DataConnectionTest, CheckAndMutateRowSuccess) { return resp; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto predicate = conn->CheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto predicate = conn->CheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); ASSERT_STATUS_OK(predicate); EXPECT_EQ(*predicate, bigtable::MutationBranch::kPredicateMatched); - predicate = conn->CheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + predicate = conn->CheckAndMutateRow(kTableName, "row", TestFilter(), {t1, t2}, + {f1, f2}); ASSERT_STATUS_OK(predicate); EXPECT_EQ(*predicate, bigtable::MutationBranch::kPredicateNotMatched); } @@ -908,10 +929,12 @@ TEST(DataConnectionTest, CheckAndMutateRowIdempotency) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_i))); + Options{} + .set(std::move(mock_i)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->CheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->CheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status, StatusIs(StatusCode::kUnavailable)); } @@ -936,9 +959,10 @@ TEST(DataConnectionTest, CheckAndMutateRowPermanentError) { return PermanentError(); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->CheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->CheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); } @@ -975,10 +999,11 @@ TEST(DataConnectionTest, CheckAndMutateRowRetryExhausted) { Options{} .set(std::move(mock_b)) .set( - bigtable::AlwaysRetryMutationPolicy().clone())); + bigtable::AlwaysRetryMutationPolicy().clone()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->CheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->CheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status, StatusIs(StatusCode::kUnavailable)); } @@ -1023,16 +1048,16 @@ TEST(DataConnectionTest, AsyncCheckAndMutateRowSuccess) { return make_ready_future(make_status_or(resp)); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto predicate = - conn->AsyncCheckAndMutateRow(kAppProfile, kTableName, "row", TestFilter(), - {t1, t2}, {f1, f2}) - .get(); + auto predicate = conn->AsyncCheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}) + .get(); ASSERT_STATUS_OK(predicate); EXPECT_EQ(*predicate, bigtable::MutationBranch::kPredicateMatched); - predicate = conn->AsyncCheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}) + predicate = conn->AsyncCheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}) .get(); ASSERT_STATUS_OK(predicate); EXPECT_EQ(*predicate, bigtable::MutationBranch::kPredicateNotMatched); @@ -1071,10 +1096,12 @@ TEST(DataConnectionTest, AsyncCheckAndMutateRowIdempotency) { }); internal::OptionsSpan span( - Options{}.set(std::move(mock_i))); + Options{} + .set(std::move(mock_i)) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->AsyncCheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->AsyncCheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status.get(), StatusIs(StatusCode::kUnavailable)); } @@ -1101,9 +1128,10 @@ TEST(DataConnectionTest, AsyncCheckAndMutateRowPermanentError) { PermanentError()); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->AsyncCheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->AsyncCheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status.get(), StatusIs(StatusCode::kPermissionDenied)); } @@ -1142,10 +1170,11 @@ TEST(DataConnectionTest, AsyncCheckAndMutateRowRetryExhausted) { Options{} .set(std::move(mock_b)) .set( - bigtable::AlwaysRetryMutationPolicy().clone())); + bigtable::AlwaysRetryMutationPolicy().clone()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto status = conn->AsyncCheckAndMutateRow(kAppProfile, kTableName, "row", - TestFilter(), {t1, t2}, {f1, f2}); + auto status = conn->AsyncCheckAndMutateRow(kTableName, "row", TestFilter(), + {t1, t2}, {f1, f2}); EXPECT_THAT(status.get(), StatusIs(StatusCode::kUnavailable)); } @@ -1168,9 +1197,11 @@ TEST(DataConnectionTest, SampleRowsSuccess) { EXPECT_CALL(mock_setup, Call).Times(1); internal::OptionsSpan span( - Options{}.set(mock_setup.AsStdFunction())); + Options{} + .set(mock_setup.AsStdFunction()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto samples = conn->SampleRows(kAppProfile, kTableName); + auto samples = conn->SampleRows(kTableName); ASSERT_STATUS_OK(samples); auto actual = RowKeySampleVectors(*samples); EXPECT_THAT(actual.offset_bytes, ElementsAre(11, 22)); @@ -1205,9 +1236,11 @@ TEST(DataConnectionTest, SampleRowsRetryResetsSamples) { EXPECT_CALL(mock_setup, Call).Times(2); internal::OptionsSpan span( - Options{}.set(mock_setup.AsStdFunction())); + Options{} + .set(mock_setup.AsStdFunction()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto samples = conn->SampleRows(kAppProfile, kTableName); + auto samples = conn->SampleRows(kTableName); ASSERT_STATUS_OK(samples); auto actual = RowKeySampleVectors(*samples); EXPECT_THAT(actual.offset_bytes, ElementsAre(22)); @@ -1239,9 +1272,10 @@ TEST(DataConnectionTest, SampleRowsRetryExhausted) { internal::OptionsSpan span( Options{} .set(std::move(mock_b)) - .set(mock_setup.AsStdFunction())); + .set(mock_setup.AsStdFunction()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto samples = conn->SampleRows(kAppProfile, kTableName); + auto samples = conn->SampleRows(kTableName); EXPECT_THAT(samples, StatusIs(StatusCode::kUnavailable)); } @@ -1261,9 +1295,11 @@ TEST(DataConnectionTest, SampleRowsPermanentError) { EXPECT_CALL(mock_setup, Call).Times(1); internal::OptionsSpan span( - Options{}.set(mock_setup.AsStdFunction())); + Options{} + .set(mock_setup.AsStdFunction()) + .set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto samples = conn->SampleRows(kAppProfile, kTableName); + auto samples = conn->SampleRows(kTableName); EXPECT_THAT(samples, StatusIs(StatusCode::kPermissionDenied)); } @@ -1281,8 +1317,9 @@ TEST(DataConnectionTest, AsyncSampleRows) { return absl::make_unique(PermanentError()); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto samples = conn->AsyncSampleRows(kAppProfile, kTableName).get(); + auto samples = conn->AsyncSampleRows(kTableName).get(); EXPECT_THAT(samples, StatusIs(StatusCode::kPermissionDenied)); } @@ -1496,8 +1533,9 @@ TEST(DataConnectionTest, AsyncReadRows) { EXPECT_THAT(status, StatusIs(StatusCode::kPermissionDenied)); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - conn->AsyncReadRows(kAppProfile, kTableName, on_row.AsStdFunction(), + conn->AsyncReadRows(kTableName, on_row.AsStdFunction(), on_finish.AsStdFunction(), TestRowSet(), 42, TestFilter()); } @@ -1526,9 +1564,9 @@ TEST(DataConnectionTest, AsyncReadRowEmpty) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = - conn->AsyncReadRow(kAppProfile, kTableName, "row", TestFilter()).get(); + auto resp = conn->AsyncReadRow(kTableName, "row", TestFilter()).get(); ASSERT_STATUS_OK(resp); EXPECT_FALSE(resp->first); } @@ -1568,9 +1606,9 @@ TEST(DataConnectionTest, AsyncReadRowSuccess) { return stream; }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = - conn->AsyncReadRow(kAppProfile, kTableName, "row", TestFilter()).get(); + auto resp = conn->AsyncReadRow(kTableName, "row", TestFilter()).get(); ASSERT_STATUS_OK(resp); EXPECT_TRUE(resp->first); EXPECT_EQ("row", resp->second.row_key()); @@ -1592,9 +1630,9 @@ TEST(DataConnectionTest, AsyncReadRowFailure) { return absl::make_unique(PermanentError()); }); + internal::OptionsSpan span(Options{}.set(kAppProfile)); auto conn = TestConnection(std::move(mock)); - auto resp = - conn->AsyncReadRow(kAppProfile, kTableName, "row", TestFilter()).get(); + auto resp = conn->AsyncReadRow(kTableName, "row", TestFilter()).get(); EXPECT_THAT(resp, StatusIs(StatusCode::kPermissionDenied)); } diff --git a/google/cloud/bigtable/mocks/mock_data_connection.h b/google/cloud/bigtable/mocks/mock_data_connection.h index 106c92df875ce..3ce7ff0cd2e00 100644 --- a/google/cloud/bigtable/mocks/mock_data_connection.h +++ b/google/cloud/bigtable/mocks/mock_data_connection.h @@ -37,61 +37,51 @@ class MockDataConnection : public bigtable::DataConnection { MOCK_METHOD(Options, options, (), (override)); MOCK_METHOD(Status, Apply, - (std::string const& app_profile_id, std::string const& table_name, - bigtable::SingleRowMutation mut), + (std::string const& table_name, bigtable::SingleRowMutation mut), (override)); MOCK_METHOD(future, AsyncApply, - (std::string const& app_profile_id, std::string const& table_name, - bigtable::SingleRowMutation mut), + (std::string const& table_name, bigtable::SingleRowMutation mut), (override)); MOCK_METHOD(std::vector, BulkApply, - (std::string const& app_profile_id, std::string const& table_name, - bigtable::BulkMutation mut), + (std::string const& table_name, bigtable::BulkMutation mut), (override)); MOCK_METHOD(future>, AsyncBulkApply, - (std::string const& app_profile_id, std::string const& table_name, - bigtable::BulkMutation mut), + (std::string const& table_name, bigtable::BulkMutation mut), (override)); MOCK_METHOD(bigtable::RowReader, ReadRows, - (std::string const& app_profile_id, std::string const& table_name, - bigtable::RowSet row_set, std::int64_t rows_limit, - bigtable::Filter filter), + (std::string const& table_name, bigtable::RowSet row_set, + std::int64_t rows_limit, bigtable::Filter filter), (override)); MOCK_METHOD((StatusOr>), ReadRow, - (std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter), + (std::string const& table_name, std::string row_key, + bigtable::Filter filter), (override)); MOCK_METHOD(StatusOr, CheckAndMutateRow, - (std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, + (std::string const& table_name, std::string row_key, + bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations), (override)); MOCK_METHOD(future>, AsyncCheckAndMutateRow, - (std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter, + (std::string const& table_name, std::string row_key, + bigtable::Filter filter, std::vector true_mutations, std::vector false_mutations), (override)); MOCK_METHOD(StatusOr>, SampleRows, - (std::string const& app_profile_id, - std::string const& table_name), - (override)); + (std::string const& table_name), (override)); MOCK_METHOD(future>>, - AsyncSampleRows, - (std::string const& app_profile_id, - std::string const& table_name), - (override)); + AsyncSampleRows, (std::string const& table_name), (override)); MOCK_METHOD(StatusOr, ReadModifyWriteRow, (google::bigtable::v2::ReadModifyWriteRowRequest request), @@ -102,15 +92,15 @@ class MockDataConnection : public bigtable::DataConnection { (override)); MOCK_METHOD(void, AsyncReadRows, - (std::string const& app_profile_id, std::string const& table_name, + (std::string const& table_name, std::function(bigtable::Row)> on_row, std::function on_finish, bigtable::RowSet row_set, std::int64_t rows_limit, bigtable::Filter filter), (override)); MOCK_METHOD((future>>), AsyncReadRow, - (std::string const& app_profile_id, std::string const& table_name, - std::string row_key, bigtable::Filter filter), + (std::string const& table_name, std::string row_key, + bigtable::Filter filter), (override)); }; diff --git a/google/cloud/bigtable/table.cc b/google/cloud/bigtable/table.cc index 07d4b229def13..d2184a01f676f 100644 --- a/google/cloud/bigtable/table.cc +++ b/google/cloud/bigtable/table.cc @@ -49,7 +49,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)); + return connection_->Apply(table_name_, std::move(mut)); } // Copy the policies in effect for this operation. Many policy classes change @@ -62,7 +62,7 @@ Status Table::Apply(SingleRowMutation mut) { // Build the RPC request, try to minimize copying. btproto::MutateRowRequest request; SetCommonTableOperationRequest( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); mut.MoveTo(request); bool const is_idempotent = @@ -96,13 +96,12 @@ 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)); + return connection_->AsyncApply(table_name_, std::move(mut)); } google::bigtable::v2::MutateRowRequest request; SetCommonTableOperationRequest( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); mut.MoveTo(request); auto context = absl::make_unique(); @@ -141,7 +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)); + return connection_->BulkApply(table_name_, std::move(mut)); } if (mut.empty()) return {}; @@ -154,7 +153,7 @@ std::vector Table::BulkApply(BulkMutation mut) { auto retry_policy = clone_rpc_retry_policy(); auto idempotent_policy = clone_idempotent_mutation_policy(); - bigtable::internal::BulkMutator mutator(app_profile_id_, table_name_, + bigtable::internal::BulkMutator mutator(app_profile_id(), table_name_, *idempotent_policy, std::move(mut)); while (true) { grpc::ClientContext client_context; @@ -175,8 +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)); + return connection_->AsyncBulkApply(table_name_, std::move(mut)); } auto cq = background_threads_->cq(); @@ -184,7 +182,7 @@ future> Table::AsyncBulkApply(BulkMutation mut) { return internal::AsyncRetryBulkApply::Create( cq, clone_rpc_retry_policy(), clone_rpc_backoff_policy(), *mutation_policy, clone_metadata_update_policy(), client_, - app_profile_id_, table_name(), std::move(mut)); + app_profile_id(), table_name(), std::move(mut)); } RowReader Table::ReadRows(RowSet row_set, Filter filter) { @@ -196,13 +194,12 @@ 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, + return connection_->ReadRows(table_name_, std::move(row_set), rows_limit, std::move(filter)); } auto impl = std::make_shared( - client_, app_profile_id_, table_name_, std::move(row_set), rows_limit, + client_, app_profile_id(), table_name_, std::move(row_set), rows_limit, std::move(filter), clone_rpc_retry_policy(), clone_rpc_backoff_policy(), metadata_update_policy_, absl::make_unique()); @@ -213,8 +210,8 @@ 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)); + return connection_->ReadRow(table_name_, std::move(row_key), + std::move(filter)); } RowSet row_set(std::move(row_key)); @@ -244,7 +241,7 @@ StatusOr Table::CheckAndMutateRow( if (connection_) { google::cloud::internal::OptionsSpan span(options_); return connection_->CheckAndMutateRow( - app_profile_id_, table_name_, std::move(row_key), std::move(filter), + table_name_, std::move(row_key), std::move(filter), std::move(true_mutations), std::move(false_mutations)); } @@ -252,7 +249,7 @@ StatusOr Table::CheckAndMutateRow( btproto::CheckAndMutateRowRequest request; request.set_row_key(std::move(row_key)); SetCommonTableOperationRequest( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); *request.mutable_predicate_filter() = std::move(filter).as_proto(); for (auto& m : true_mutations) { *request.add_true_mutations() = std::move(m.op); @@ -281,14 +278,14 @@ future> Table::AsyncCheckAndMutateRow( if (connection_) { google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncCheckAndMutateRow( - app_profile_id_, table_name_, std::move(row_key), std::move(filter), + table_name_, std::move(row_key), std::move(filter), std::move(true_mutations), std::move(false_mutations)); } btproto::CheckAndMutateRowRequest request; request.set_row_key(std::move(row_key)); SetCommonTableOperationRequest( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); *request.mutable_predicate_filter() = std::move(filter).as_proto(); for (auto& m : true_mutations) { *request.add_true_mutations() = std::move(m.op); @@ -334,7 +331,7 @@ future> Table::AsyncCheckAndMutateRow( StatusOr> Table::SampleRows() { if (connection_) { google::cloud::internal::OptionsSpan span(options_); - return connection_->SampleRows(app_profile_id_, table_name_); + return connection_->SampleRows(table_name_); } // Copy the policies in effect for this operation. @@ -346,7 +343,7 @@ StatusOr> Table::SampleRows() { btproto::SampleRowKeysRequest request; btproto::SampleRowKeysResponse response; SetCommonTableOperationRequest( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); while (true) { grpc::ClientContext client_context; @@ -380,20 +377,20 @@ StatusOr> Table::SampleRows() { future>> Table::AsyncSampleRows() { if (connection_) { google::cloud::internal::OptionsSpan span(options_); - return connection_->AsyncSampleRows(app_profile_id_, table_name_); + return connection_->AsyncSampleRows(table_name_); } auto cq = background_threads_->cq(); return internal::LegacyAsyncRowSampler::Create( cq, client_, clone_rpc_retry_policy(), clone_rpc_backoff_policy(), - metadata_update_policy_, app_profile_id_, table_name_); + metadata_update_policy_, app_profile_id(), table_name_); } StatusOr Table::ReadModifyWriteRowImpl( btproto::ReadModifyWriteRowRequest request) { SetCommonTableOperationRequest< ::google::bigtable::v2::ReadModifyWriteRowRequest>( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); if (connection_) { google::cloud::internal::OptionsSpan span(options_); return connection_->ReadModifyWriteRow(std::move(request)); @@ -415,7 +412,7 @@ future> Table::AsyncReadModifyWriteRowImpl( ::google::bigtable::v2::ReadModifyWriteRowRequest request) { SetCommonTableOperationRequest< ::google::bigtable::v2::ReadModifyWriteRowRequest>( - request, app_profile_id_, table_name_); + request, app_profile_id(), table_name_); if (connection_) { google::cloud::internal::OptionsSpan span(options_); return connection_->AsyncReadModifyWriteRow(std::move(request)); @@ -448,8 +445,8 @@ 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)); + return connection_->AsyncReadRow(table_name_, std::move(row_key), + std::move(filter)); } class AsyncReadRowHandler { diff --git a/google/cloud/bigtable/table.h b/google/cloud/bigtable/table.h index fae00d39bb05f..e121ddf368223 100644 --- a/google/cloud/bigtable/table.h +++ b/google/cloud/bigtable/table.h @@ -223,7 +223,6 @@ class Table { Table(std::shared_ptr client, std::string app_profile_id, std::string const& table_id) : client_(std::move(client)), - app_profile_id_(std::move(app_profile_id)), project_id_(client_->project_id()), instance_id_(client_->instance_id()), table_name_(TableName(project_id_, instance_id_, table_id)), @@ -236,7 +235,9 @@ class Table { MetadataUpdatePolicy(table_name_, MetadataParamTypes::TABLE_NAME)), idempotent_mutation_policy_( bigtable::DefaultIdempotentMutationPolicy()), - background_threads_(client_->BackgroundThreadsFactory()()) {} + background_threads_(client_->BackgroundThreadsFactory()()), + options_(Options{}.set(std::move(app_profile_id))) { + } /** * Constructor with explicit policies. @@ -364,7 +365,9 @@ class Table { } std::string const& table_name() const { return table_name_; } - std::string const& app_profile_id() const { return app_profile_id_; } + std::string const& app_profile_id() const { + return options_.get(); + } std::string const& project_id() const { return project_id_; } std::string const& instance_id() const { return instance_id_; } std::string const& table_id() const { return table_id_; } @@ -377,8 +380,15 @@ class Table { */ Table WithNewTarget(std::string project_id, std::string instance_id, std::string table_id) const { - return WithNewTarget(std::move(project_id), std::move(instance_id), - app_profile_id_, std::move(table_id)); + auto table = *this; + table.instance_id_ = std::move(instance_id); + table.project_id_ = std::move(project_id); + table.table_id_ = std::move(table_id); + table.table_name_ = + TableName(table.project_id_, table.instance_id_, table.table_id_); + table.metadata_update_policy_ = + MetadataUpdatePolicy(table.table_name_, MetadataParamTypes::TABLE_NAME); + return table; } /** @@ -391,11 +401,11 @@ class Table { table.instance_id_ = std::move(instance_id); table.project_id_ = std::move(project_id); table.table_id_ = std::move(table_id); - table.app_profile_id_ = std::move(app_profile_id); table.table_name_ = TableName(table.project_id_, table.instance_id_, table.table_id_); table.metadata_update_policy_ = MetadataUpdatePolicy(table.table_name_, MetadataParamTypes::TABLE_NAME); + table.options_.set(std::move(app_profile_id)); return table; } @@ -904,15 +914,14 @@ class Table { if (connection_) { google::cloud::internal::OptionsSpan span(options_); - connection_->AsyncReadRows(app_profile_id_, table_name_, - std::move(on_row_fn), std::move(on_finish_fn), - std::move(row_set), rows_limit, - std::move(filter)); + connection_->AsyncReadRows(table_name_, std::move(on_row_fn), + std::move(on_finish_fn), std::move(row_set), + rows_limit, std::move(filter)); return; } bigtable_internal::LegacyAsyncRowReader::Create( - background_threads_->cq(), client_, app_profile_id_, table_name_, + background_threads_->cq(), client_, app_profile_id(), table_name_, std::move(on_row_fn), std::move(on_finish_fn), std::move(row_set), rows_limit, std::move(filter), clone_rpc_retry_policy(), clone_rpc_backoff_policy(), metadata_update_policy_, @@ -958,8 +967,7 @@ class Table { explicit Table(std::shared_ptr conn, std::string project_id, std::string instance_id, std::string table_id, Options options = {}) - : app_profile_id_(options.get()), - project_id_(std::move(project_id)), + : project_id_(std::move(project_id)), instance_id_(std::move(instance_id)), table_name_(TableName(project_id_, instance_id_, table_id)), table_id_(std::move(table_id)), @@ -1030,7 +1038,6 @@ class Table { friend class MutationBatcher; std::shared_ptr client_; - std::string app_profile_id_; std::string project_id_; std::string instance_id_; std::string table_name_; diff --git a/google/cloud/bigtable/table_test.cc b/google/cloud/bigtable/table_test.cc index bf65d7805e5f0..0b297c9f55d0c 100644 --- a/google/cloud/bigtable/table_test.cc +++ b/google/cloud/bigtable/table_test.cc @@ -129,6 +129,7 @@ Table TestTable(std::shared_ptr mock) { void CheckCurrentOptions() { auto const& options = google::cloud::internal::CurrentOptions(); EXPECT_TRUE(options.has()); + EXPECT_EQ(kAppProfileId, options.get()); } TEST(TableTest, ConnectionConstructor) { @@ -156,11 +157,9 @@ TEST(TableTest, AppProfileId) { TEST(TableTest, Apply) { auto mock = std::make_shared(); EXPECT_CALL(*mock, Apply) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](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(); @@ -174,11 +173,9 @@ TEST(TableTest, Apply) { TEST(TableTest, AsyncApply) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncApply) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](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()); @@ -194,11 +191,9 @@ TEST(TableTest, BulkApply) { auto mock = std::make_shared(); EXPECT_CALL(*mock, BulkApply) - .WillOnce([&expected](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([&expected](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; @@ -215,11 +210,9 @@ TEST(TableTest, AsyncBulkApply) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncBulkApply) - .WillOnce([&expected](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([&expected](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); @@ -234,12 +227,10 @@ TEST(TableTest, AsyncBulkApply) { TEST(TableTest, ReadRows) { auto mock = std::make_shared(); EXPECT_CALL(*mock, ReadRows) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](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()); EXPECT_EQ(rows_limit, RowReader::NO_ROWS_LIMIT); @@ -257,12 +248,10 @@ TEST(TableTest, ReadRows) { TEST(TableTest, ReadRowsWithRowLimit) { auto mock = std::make_shared(); EXPECT_CALL(*mock, ReadRows) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](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()); EXPECT_EQ(rows_limit, 42); @@ -280,11 +269,9 @@ TEST(TableTest, ReadRowsWithRowLimit) { TEST(TableTest, ReadRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, ReadRow) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, std::string const& row_key, + .WillOnce([](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); EXPECT_THAT(filter, IsTestFilter()); @@ -299,13 +286,11 @@ TEST(TableTest, ReadRow) { TEST(TableTest, CheckAndMutateRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, CheckAndMutateRow) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, std::string const& row_key, + .WillOnce([](std::string const& table_name, std::string const& row_key, 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); EXPECT_THAT(filter, IsTestFilter()); @@ -327,13 +312,11 @@ TEST(TableTest, CheckAndMutateRow) { TEST(TableTest, AsyncCheckAndMutateRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncCheckAndMutateRow) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, std::string const& row_key, + .WillOnce([](std::string const& table_name, std::string const& row_key, 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); EXPECT_THAT(filter, IsTestFilter()); @@ -356,14 +339,11 @@ TEST(TableTest, AsyncCheckAndMutateRow) { TEST(TableTest, SampleRows) { auto mock = std::make_shared(); - 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(); - }); + EXPECT_CALL(*mock, SampleRows).WillOnce([](std::string const& table_name) { + CheckCurrentOptions(); + EXPECT_EQ(kTableName, table_name); + return PermanentError(); + }); auto table = TestTable(std::move(mock)); auto samples = table.SampleRows(); @@ -373,14 +353,12 @@ TEST(TableTest, SampleRows) { TEST(TableTest, AsyncSampleRows) { auto mock = std::make_shared(); 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()); - }); + .WillOnce([](std::string const& table_name) { + CheckCurrentOptions(); + EXPECT_EQ(kTableName, table_name); + return make_ready_future>>( + PermanentError()); + }); auto table = TestTable(std::move(mock)); auto samples = table.AsyncSampleRows().get(); @@ -392,7 +370,6 @@ TEST(TableTest, ReadModifyWriteRow) { 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(), ElementsAre(MatchRule(TestAppendRule()), @@ -411,7 +388,6 @@ TEST(TableTest, AsyncReadModifyWriteRow) { 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(), ElementsAre(MatchRule(TestAppendRule()), @@ -428,14 +404,12 @@ TEST(TableTest, AsyncReadModifyWriteRow) { TEST(TableTest, AsyncReadRows) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncReadRows) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](std::string const& table_name, std::function(bigtable::Row)> const& on_row, 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()); EXPECT_EQ(RowReader::NO_ROWS_LIMIT, rows_limit); @@ -471,14 +445,12 @@ TEST(TableTest, AsyncReadRows) { TEST(TableTest, AsyncReadRowsWithRowLimit) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncReadRows) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, + .WillOnce([](std::string const& table_name, std::function(bigtable::Row)> const& on_row, 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()); EXPECT_EQ(42, rows_limit); @@ -514,7 +486,7 @@ TEST(TableTest, AsyncReadRowsWithRowLimit) { TEST(TableTest, AsyncReadRowsAcceptsMoveOnlyTypes) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncReadRows) - .WillOnce([](Unused, Unused, + .WillOnce([](Unused, std::function(bigtable::Row)> const& on_row, std::function const& on_finish, Unused, Unused, Unused) { @@ -548,11 +520,9 @@ TEST(TableTest, AsyncReadRowsAcceptsMoveOnlyTypes) { TEST(TableTest, AsyncReadRow) { auto mock = std::make_shared(); EXPECT_CALL(*mock, AsyncReadRow) - .WillOnce([](std::string const& app_profile_id, - std::string const& table_name, std::string const& row_key, + .WillOnce([](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); EXPECT_THAT(filter, IsTestFilter());