Skip to content

Commit 88031e6

Browse files
coryancoryan
authored
Refactor AdminClient to support multiple services. (#444)
This is a large change. It is part of the fixes for #433 and for #418. The current implementation of bigtable::*Client makes it really hard to write tests that use multiple services, and when using long running operations you need to access at least two services. The good news is that the tests and some of the helpers get easier to write, and this does not affect any of the APIs exposed to the user.
1 parent de7dd90 commit 88031e6

13 files changed

Lines changed: 558 additions & 351 deletions

‎bigtable/client/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,12 @@ add_library(bigtable_client_testing
123123
testing/embedded_server_test_fixture.cc
124124
testing/internal_table_test_fixture.h
125125
testing/internal_table_test_fixture.cc
126+
testing/mock_admin_client.h
126127
testing/mock_data_client.h
127128
testing/mock_instance_admin_client.h
128129
testing/inprocess_data_client.h
129130
testing/inprocess_admin_client.h
131+
testing/inprocess_admin_client.cc
130132
testing/mock_mutate_rows_reader.h
131133
testing/mock_read_rows_reader.h
132134
testing/mock_response_reader.h

‎bigtable/client/admin_client.cc‎

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#include "bigtable/client/internal/common_client.h"
1717

1818
namespace {
19+
namespace btadmin = google::bigtable::admin::v2;
20+
1921
/**
2022
* An AdminClient for single-threaded programs that refreshes credentials on all
2123
* gRPC errors.
@@ -39,8 +41,8 @@ class DefaultAdminClient : public bigtable::AdminClient {
3941
}
4042
};
4143

42-
using Impl = bigtable::internal::CommonClient<
43-
AdminTraits, ::google::bigtable::admin::v2::BigtableTableAdmin>;
44+
using Impl = bigtable::internal::CommonClient<AdminTraits,
45+
btadmin::BigtableTableAdmin>;
4446

4547
public:
4648
using AdminStubPtr = Impl::StubPtr;
@@ -49,9 +51,92 @@ class DefaultAdminClient : public bigtable::AdminClient {
4951
: project_(std::move(project)), impl_(std::move(options)) {}
5052

5153
std::string const& project() const override { return project_; }
52-
AdminStubPtr Stub() override { return impl_.Stub(); }
54+
std::shared_ptr<grpc::Channel> Channel() override { return impl_.Channel(); }
5355
void reset() override { return impl_.reset(); }
54-
void on_completion(grpc::Status const& status) override {}
56+
57+
grpc::Status CreateTable(grpc::ClientContext* context,
58+
btadmin::CreateTableRequest const& request,
59+
btadmin::Table* response) override {
60+
return impl_.Stub()->CreateTable(context, request, response);
61+
}
62+
63+
grpc::Status CreateTableFromSnapshot(
64+
grpc::ClientContext* context,
65+
btadmin::CreateTableFromSnapshotRequest const& request,
66+
google::longrunning::Operation* response) override {
67+
return impl_.Stub()->CreateTableFromSnapshot(context, request, response);
68+
}
69+
70+
grpc::Status ListTables(grpc::ClientContext* context,
71+
btadmin::ListTablesRequest const& request,
72+
btadmin::ListTablesResponse* response) override {
73+
return impl_.Stub()->ListTables(context, request, response);
74+
}
75+
76+
grpc::Status GetTable(grpc::ClientContext* context,
77+
btadmin::GetTableRequest const& request,
78+
btadmin::Table* response) override {
79+
return impl_.Stub()->GetTable(context, request, response);
80+
}
81+
82+
grpc::Status DeleteTable(grpc::ClientContext* context,
83+
btadmin::DeleteTableRequest const& request,
84+
google::protobuf::Empty* response) override {
85+
return impl_.Stub()->DeleteTable(context, request, response);
86+
}
87+
88+
grpc::Status ModifyColumnFamilies(
89+
grpc::ClientContext* context,
90+
btadmin::ModifyColumnFamiliesRequest const& request,
91+
btadmin::Table* response) override {
92+
return impl_.Stub()->ModifyColumnFamilies(context, request, response);
93+
}
94+
95+
grpc::Status DropRowRange(grpc::ClientContext* context,
96+
btadmin::DropRowRangeRequest const& request,
97+
google::protobuf::Empty* response) override {
98+
return impl_.Stub()->DropRowRange(context, request, response);
99+
}
100+
101+
grpc::Status GenerateConsistencyToken(
102+
grpc::ClientContext* context,
103+
btadmin::GenerateConsistencyTokenRequest const& request,
104+
btadmin::GenerateConsistencyTokenResponse* response) override {
105+
return impl_.Stub()->GenerateConsistencyToken(context, request, response);
106+
}
107+
108+
grpc::Status CheckConsistency(
109+
grpc::ClientContext* context,
110+
btadmin::CheckConsistencyRequest const& request,
111+
btadmin::CheckConsistencyResponse* response) override {
112+
return impl_.Stub()->CheckConsistency(context, request, response);
113+
}
114+
115+
grpc::Status SnapshotTable(
116+
grpc::ClientContext* context,
117+
btadmin::SnapshotTableRequest const& request,
118+
google::longrunning::Operation* response) override {
119+
return impl_.Stub()->SnapshotTable(context, request, response);
120+
}
121+
122+
grpc::Status GetSnapshot(grpc::ClientContext* context,
123+
btadmin::GetSnapshotRequest const& request,
124+
btadmin::Snapshot* response) override {
125+
return impl_.Stub()->GetSnapshot(context, request, response);
126+
}
127+
128+
grpc::Status ListSnapshots(
129+
grpc::ClientContext* context,
130+
btadmin::ListSnapshotsRequest const& request,
131+
btadmin::ListSnapshotsResponse* response) override {
132+
return impl_.Stub()->ListSnapshots(context, request, response);
133+
}
134+
135+
grpc::Status DeleteSnapshot(grpc::ClientContext* context,
136+
btadmin::DeleteSnapshotRequest const& request,
137+
google::protobuf::Empty* response) override {
138+
return impl_.Stub()->DeleteSnapshot(context, request, response);
139+
}
55140

56141
private:
57142
std::string project_;

‎bigtable/client/admin_client.h‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,27 +42,81 @@ class AdminClient {
4242
/// The project that this AdminClient works on.
4343
virtual std::string const& project() const = 0;
4444

45-
/// Return a new stub to handle admin operations.
46-
virtual std::shared_ptr<
47-
::google::bigtable::admin::v2::BigtableTableAdmin::StubInterface>
48-
Stub() = 0;
45+
/**
46+
* Return a new channel to handle admin operations.
47+
*
48+
* Intended to access rarely used services in the same endpoints as the
49+
* Bigtable admin interfaces, for example, the google.longrunning.Operations.
50+
*/
51+
virtual std::shared_ptr<grpc::Channel> Channel() = 0;
4952

5053
/**
51-
* Reset and create a new Stub().
54+
* Reset and create new Channels.
5255
*
5356
* Currently this is only used in testing. In the future, we expect this,
5457
* or a similar member function, will be needed to handle errors that require
5558
* a new connection, or an explicit refresh of the credentials.
5659
*/
5760
virtual void reset() = 0;
5861

59-
/**
60-
* A callback for completed RPCs.
61-
*
62-
* Currently this is only used in testing. In the future, we expect that
63-
* some errors may require the class to update its state.
64-
*/
65-
virtual void on_completion(grpc::Status const&) = 0;
62+
//@{
63+
/// @name the google.bigtable.admin.v2.TableAdmin operations.
64+
virtual grpc::Status CreateTable(
65+
grpc::ClientContext* context,
66+
google::bigtable::admin::v2::CreateTableRequest const& request,
67+
google::bigtable::admin::v2::Table* response) = 0;
68+
virtual grpc::Status CreateTableFromSnapshot(
69+
grpc::ClientContext* context,
70+
google::bigtable::admin::v2::CreateTableFromSnapshotRequest const&
71+
request,
72+
google::longrunning::Operation* response) = 0;
73+
virtual grpc::Status ListTables(
74+
grpc::ClientContext* context,
75+
google::bigtable::admin::v2::ListTablesRequest const& request,
76+
google::bigtable::admin::v2::ListTablesResponse* response) = 0;
77+
virtual grpc::Status GetTable(
78+
grpc::ClientContext* context,
79+
google::bigtable::admin::v2::GetTableRequest const& request,
80+
google::bigtable::admin::v2::Table* response) = 0;
81+
virtual grpc::Status DeleteTable(
82+
grpc::ClientContext* context,
83+
google::bigtable::admin::v2::DeleteTableRequest const& request,
84+
google::protobuf::Empty* response) = 0;
85+
virtual grpc::Status ModifyColumnFamilies(
86+
grpc::ClientContext* context,
87+
google::bigtable::admin::v2::ModifyColumnFamiliesRequest const& request,
88+
google::bigtable::admin::v2::Table* response) = 0;
89+
virtual grpc::Status DropRowRange(
90+
grpc::ClientContext* context,
91+
google::bigtable::admin::v2::DropRowRangeRequest const& request,
92+
google::protobuf::Empty* response) = 0;
93+
virtual grpc::Status GenerateConsistencyToken(
94+
grpc::ClientContext* context,
95+
google::bigtable::admin::v2::GenerateConsistencyTokenRequest const&
96+
request,
97+
google::bigtable::admin::v2::GenerateConsistencyTokenResponse*
98+
response) = 0;
99+
virtual grpc::Status CheckConsistency(
100+
grpc::ClientContext* context,
101+
google::bigtable::admin::v2::CheckConsistencyRequest const& request,
102+
google::bigtable::admin::v2::CheckConsistencyResponse* response) = 0;
103+
virtual grpc::Status SnapshotTable(
104+
grpc::ClientContext* context,
105+
google::bigtable::admin::v2::SnapshotTableRequest const& request,
106+
google::longrunning::Operation* response) = 0;
107+
virtual grpc::Status GetSnapshot(
108+
grpc::ClientContext* context,
109+
google::bigtable::admin::v2::GetSnapshotRequest const& request,
110+
google::bigtable::admin::v2::Snapshot* response) = 0;
111+
virtual grpc::Status ListSnapshots(
112+
grpc::ClientContext* context,
113+
google::bigtable::admin::v2::ListSnapshotsRequest const& request,
114+
google::bigtable::admin::v2::ListSnapshotsResponse* response) = 0;
115+
virtual grpc::Status DeleteSnapshot(
116+
grpc::ClientContext* context,
117+
google::bigtable::admin::v2::DeleteSnapshotRequest const& request,
118+
google::protobuf::Empty* response) = 0;
119+
//@}
66120
};
67121

68122
/// Create a new admin client configured via @p options.

‎bigtable/client/admin_client_test.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ TEST(AdminClientTest, Default) {
2121
ASSERT_TRUE(admin_client);
2222
EXPECT_EQ("test-project", admin_client->project());
2323

24-
auto stub0 = admin_client->Stub();
25-
EXPECT_TRUE(stub0);
24+
auto channel0 = admin_client->Channel();
25+
EXPECT_TRUE(channel0);
2626

27-
auto stub1 = admin_client->Stub();
28-
EXPECT_EQ(stub0.get(), stub1.get());
27+
auto channel1 = admin_client->Channel();
28+
EXPECT_EQ(channel0.get(), channel1.get());
2929

3030
admin_client->reset();
31-
stub1 = admin_client->Stub();
32-
EXPECT_TRUE(stub1);
33-
EXPECT_NE(stub0.get(), stub1.get());
31+
channel1 = admin_client->Channel();
32+
EXPECT_TRUE(channel1);
33+
EXPECT_NE(channel0.get(), channel1.get());
3434
}

‎bigtable/client/instance_admin.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
#include "bigtable/client/instance_admin_client.h"
1919
#include "bigtable/client/internal/instance_admin.h"
20-
#include "bigtable/client/internal/unary_rpc_utils.h"
2120
#include <memory>
2221

2322
namespace bigtable {

0 commit comments

Comments
 (0)