Resolve comments

tmatsuo · May 6, 2019 · 152a7cc · 152a7cc
1 parent abd97b9
commit 152a7cc
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 35 deletions.
diff --git a/include/grpcpp/server_builder_impl.h b/include/grpcpp/server_builder_impl.h
@@ -56,6 +56,9 @@ namespace experimental {
 class CallbackGenericService;
 }
 
+// EXPERIMENTAL API:
+// Interface for a grpc server to handle connections created out of band.
+// See ServerBuilder's AddExternalConnectionAcceptor API for usage.
 class ExternalConnectionAcceptor {
  public:
   struct NewConnectionParameters {
@@ -262,6 +265,16 @@ class ServerBuilder {
     ServerBuilder& RegisterCallbackGenericService(
         grpc::experimental::CallbackGenericService* service);
 
+    enum ExternalConnectionType {
+      CONNECTION_FROM_FD = 0  // in the form of a file descriptor
+    };
+
+    // Create an acceptor to take in external connections and pass them to the
+    // gRPC server.
+    std::unique_ptr<grpc::ExternalConnectionAcceptor>
+    AddExternalConnectionAcceptor(ExternalConnectionType type,
+                                  std::shared_ptr<ServerCredentials> creds);
+
    private:
     ServerBuilder* builder_;
   };
@@ -271,16 +284,6 @@ class ServerBuilder {
   /// at any time.
   experimental_type experimental() { return experimental_type(this); }
 
-  enum ExternalConnectionType {
-    CONNECTION_FROM_FD = 0  // in the form of a file descriptor
-  };
-  // EXPERIMENTAL API:
-  // Create an acceptor to take in external connections and pass them to the
-  // gRPC server.
-  std::unique_ptr<grpc::ExternalConnectionAcceptor>
-  AddExternalConnectionAcceptor(ExternalConnectionType type,
-                                std::shared_ptr<ServerCredentials> creds);
-
  protected:
   /// Experimental, to be deprecated
   struct Port {

diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc
@@ -297,7 +297,6 @@ static grpc_error* chttp2_server_add_acceptor(grpc_server* server,
   server_state* state = nullptr;
   const grpc_arg* arg = nullptr;
   grpc_core::TcpServerFdHandler** arg_val = nullptr;
-
   state = static_cast<server_state*>(gpr_zalloc(sizeof(*state)));
   GRPC_CLOSURE_INIT(&state->tcp_server_shutdown_complete,
                     tcp_server_shutdown_complete, state,
@@ -307,23 +306,19 @@ static grpc_error* chttp2_server_add_acceptor(grpc_server* server,
   if (err != GRPC_ERROR_NONE) {
     goto error;
   }
-
   state->server = server;
   state->tcp_server = tcp_server;
   state->args = args;
   state->shutdown = true;
   gpr_mu_init(&state->mu);
-
   // TODO(yangg) channelz
-
   arg = grpc_channel_args_find(args, name);
   GPR_ASSERT(arg->type == GRPC_ARG_POINTER);
   arg_val = static_cast<grpc_core::TcpServerFdHandler**>(arg->value.pointer.p);
   *arg_val = grpc_tcp_server_create_fd_handler(tcp_server);
 
   grpc_server_add_listener(server, state, server_start_listener,
                            server_destroy_listener, /* socket_uuid */ 0);
-
   return err;
 
 /* Error path: cleanup and return */

diff --git a/src/cpp/server/external_connection_acceptor_impl.cc b/src/cpp/server/external_connection_acceptor_impl.cc
@@ -40,10 +40,12 @@ class InternalAcceptor : public grpc::ExternalConnectionAcceptor {
 }  // namespace
 
 ExternalConnectionAcceptorImpl::ExternalConnectionAcceptorImpl(
-    const grpc::string& name, ServerBuilder::ExternalConnectionType type,
+    const grpc::string& name,
+    ServerBuilder::experimental_type::ExternalConnectionType type,
     std::shared_ptr<ServerCredentials> creds)
     : name_(name), creds_(std::move(creds)) {
-  GPR_ASSERT(type == ServerBuilder::ExternalConnectionType::CONNECTION_FROM_FD);
+  GPR_ASSERT(type == ServerBuilder::experimental_type::ExternalConnectionType::
+                         CONNECTION_FROM_FD);
 }
 
 std::unique_ptr<grpc::ExternalConnectionAcceptor>

diff --git a/src/cpp/server/external_connection_acceptor_impl.h b/src/cpp/server/external_connection_acceptor_impl.h
@@ -37,9 +37,10 @@ typedef void (*RawConnectionHandler)(int fd, grpc_byte_buffer* buffer);
 class ExternalConnectionAcceptorImpl
     : public std::enable_shared_from_this<ExternalConnectionAcceptorImpl> {
  public:
-  ExternalConnectionAcceptorImpl(const grpc::string& name,
-                                 ServerBuilder::ExternalConnectionType type,
-                                 std::shared_ptr<ServerCredentials> creds);
+  ExternalConnectionAcceptorImpl(
+      const grpc::string& name,
+      ServerBuilder::experimental_type::ExternalConnectionType type,
+      std::shared_ptr<ServerCredentials> creds);
   // Should only be called once.
   std::unique_ptr<grpc::ExternalConnectionAcceptor> GetAcceptor();
 

diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc
@@ -116,6 +116,19 @@ ServerBuilder& ServerBuilder::experimental_type::RegisterCallbackGenericService(
   return *builder_;
 }
 
+std::unique_ptr<grpc::ExternalConnectionAcceptor>
+ServerBuilder::experimental_type::AddExternalConnectionAcceptor(
+    experimental_type::ExternalConnectionType type,
+    std::shared_ptr<ServerCredentials> creds) {
+  grpc::string name_prefix("external:");
+  char count_str[GPR_LTOA_MIN_BUFSIZE];
+  gpr_ltoa(static_cast<long>(builder_->acceptors_.size()), count_str);
+  builder_->acceptors_.emplace_back(
+      std::make_shared<ExternalConnectionAcceptorImpl>(
+          name_prefix.append(count_str), type, creds));
+  return builder_->acceptors_.back()->GetAcceptor();
+}
+
 ServerBuilder& ServerBuilder::SetOption(
     std::unique_ptr<grpc::ServerBuilderOption> option) {
   options_.push_back(std::move(option));
@@ -411,15 +424,4 @@ ServerBuilder& ServerBuilder::EnableWorkaround(grpc_workaround_list id) {
   }
 }
 
-std::unique_ptr<grpc::ExternalConnectionAcceptor>
-ServerBuilder::AddExternalConnectionAcceptor(
-    ExternalConnectionType type, std::shared_ptr<ServerCredentials> creds) {
-  grpc::string name_prefix("external:");
-  char count_str[GPR_LTOA_MIN_BUFSIZE];
-  gpr_ltoa(static_cast<long>(acceptors_.size()), count_str);
-  acceptors_.emplace_back(std::make_shared<ExternalConnectionAcceptorImpl>(
-      name_prefix.append(count_str), type, creds));
-  return acceptors_.back()->GetAcceptor();
-}
-
 }  // namespace grpc_impl
diff --git a/test/cpp/end2end/port_sharing_end2end_test.cc b/test/cpp/end2end/port_sharing_end2end_test.cc
@@ -220,12 +220,14 @@ class PortSharingEnd2endTest : public ::testing::TestWithParam<TestScenario> {
     }
     auto server_creds = GetCredentialsProvider()->GetServerCredentials(
         GetParam().credentials_type);
-    auto acceptor1 = builder.AddExternalConnectionAcceptor(
-        ServerBuilder::ExternalConnectionType::CONNECTION_FROM_FD,
+    auto acceptor1 = builder.experimental().AddExternalConnectionAcceptor(
+        ServerBuilder::experimental_type::ExternalConnectionType::
+            CONNECTION_FROM_FD,
         server_creds);
     tcp_server1_.SetAcceptor(std::move(acceptor1));
-    auto acceptor2 = builder.AddExternalConnectionAcceptor(
-        ServerBuilder::ExternalConnectionType::CONNECTION_FROM_FD,
+    auto acceptor2 = builder.experimental().AddExternalConnectionAcceptor(
+        ServerBuilder::experimental_type::ExternalConnectionType::
+            CONNECTION_FROM_FD,
         server_creds);
     tcp_server2_.SetAcceptor(std::move(acceptor2));
     builder.RegisterService(&service_);