Merge pull request #17034 from yashykt/interceptorcleanup

Get ClientContext included with ChannelInterface, and slight more cle…

9 files changed, 32 insertions, 16 deletions

diff --git a/include/grpcpp/impl/codegen/channel_interface.h b/include/grpcpp/impl/codegen/channel_interface.h
index 6fd1dd1d9b..e957ea6aab 100644
--- a/include/grpcpp/impl/codegen/channel_interface.h
+++ b/include/grpcpp/impl/codegen/channel_interface.h
@@ -21,6 +21,7 @@
 
 #include <grpc/impl/codegen/connectivity_state.h>
 #include <grpcpp/impl/codegen/call.h>
+#include <grpcpp/impl/codegen/client_context.h>
 #include <grpcpp/impl/codegen/status.h>
 #include <grpcpp/impl/codegen/time.h>
 
diff --git a/include/grpcpp/impl/codegen/client_interceptor.h b/include/grpcpp/impl/codegen/client_interceptor.h
index 00113f04aa..0e08a7ce01 100644
--- a/include/grpcpp/impl/codegen/client_interceptor.h
+++ b/include/grpcpp/impl/codegen/client_interceptor.h
@@ -21,7 +21,6 @@
 
 #include <vector>
 
-#include <grpc/impl/codegen/log.h>
 #include <grpcpp/impl/codegen/interceptor.h>
 #include <grpcpp/impl/codegen/string_ref.h>
 
diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h
index cdd34b80d1..15cab711e5 100644
--- a/include/grpcpp/impl/codegen/interceptor.h
+++ b/include/grpcpp/impl/codegen/interceptor.h
@@ -21,13 +21,13 @@
 
 #include <grpc/impl/codegen/grpc_types.h>
 #include <grpcpp/impl/codegen/byte_buffer.h>
-#include <grpcpp/impl/codegen/channel_interface.h>
 #include <grpcpp/impl/codegen/config.h>
 #include <grpcpp/impl/codegen/core_codegen_interface.h>
 #include <grpcpp/impl/codegen/metadata_map.h>
 
 namespace grpc {
 
+class ChannelInterface;
 class Status;
 
 namespace experimental {
diff --git a/include/grpcpp/impl/codegen/server_interceptor.h b/include/grpcpp/impl/codegen/server_interceptor.h
index c39e9a988d..5fb5df28b7 100644
--- a/include/grpcpp/impl/codegen/server_interceptor.h
+++ b/include/grpcpp/impl/codegen/server_interceptor.h
@@ -22,7 +22,6 @@
 #include <atomic>
 #include <vector>
 
-#include <grpc/impl/codegen/log.h>
 #include <grpcpp/impl/codegen/interceptor.h>
 #include <grpcpp/impl/codegen/string_ref.h>
 
diff --git a/include/grpcpp/impl/codegen/server_interface.h b/include/grpcpp/impl/codegen/server_interface.h
index 92c87a5f7e..3967e96cfe 100644
--- a/include/grpcpp/impl/codegen/server_interface.h
+++ b/include/grpcpp/impl/codegen/server_interface.h
@@ -333,7 +333,12 @@ class ServerInterface : public internal::CallHook {
   }
 
  private:
-  virtual const std::vector<
+  // EXPERIMENTAL
+  // Getter method for the vector of interceptor factory objects.
+  // Returns a nullptr (rather than being pure) since this is a new method and
+  // adding a new pure method to an interface would be a breaking change (even
+  // though this is private and non-API)
+  virtual std::vector<
       std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
   interceptor_creators() {
     return nullptr;
diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h
index 2b89ffd317..82d60b0218 100644
--- a/include/grpcpp/server.h
+++ b/include/grpcpp/server.h
@@ -191,8 +191,7 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
   grpc_server* server() override { return server_; };
 
  private:
-  const std::vector<
-      std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
+  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>*
   interceptor_creators() override {
     return &interceptor_creators_;
   }
@@ -226,6 +225,14 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
 
   ServerInitializer* initializer();
 
+  // A vector of interceptor factory objects.
+  // This should be destroyed after health_check_service_ and this requirement
+  // is satisfied by declaring interceptor_creators_ before
+  // health_check_service_. (C++ mandates that member objects be destroyed in
+  // the reverse order of initialization.)
+  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
+      interceptor_creators_;
+
   const int max_receive_message_size_;
 
   /// The following completion queues are ONLY used in case of Sync API
@@ -261,9 +268,6 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
 
   // A special handler for resource exhausted in sync case
   std::unique_ptr<internal::MethodHandler> resource_exhausted_handler_;
-
-  std::vector<std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
-      interceptor_creators_;
 };
 
 }  // namespace grpc
diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc
index 82a9d719fa..6f911e1392 100644
--- a/src/cpp/server/server_cc.cc
+++ b/src/cpp/server/server_cc.cc
@@ -447,7 +447,8 @@ Server::Server(
     std::vector<
         std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>
         interceptor_creators)
-    : max_receive_message_size_(max_receive_message_size),
+    : interceptor_creators_(std::move(interceptor_creators)),
+      max_receive_message_size_(max_receive_message_size),
       sync_server_cqs_(std::move(sync_server_cqs)),
       started_(false),
       shutdown_(false),
@@ -455,8 +456,7 @@ Server::Server(
       has_generic_service_(false),
       server_(nullptr),
       server_initializer_(new ServerInitializer(this)),
-      health_check_service_disabled_(false),
-      interceptor_creators_(std::move(interceptor_creators)) {
+      health_check_service_disabled_(false) {
   g_gli_initializer.summon();
   gpr_once_init(&g_once_init_callbacks, InitGlobalCallbacks);
   global_callbacks_ = g_callbacks;
diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc
index 5720e87478..e8ffd46344 100644
--- a/test/cpp/end2end/client_interceptors_end2end_test.cc
+++ b/test/cpp/end2end/client_interceptors_end2end_test.cc
@@ -152,7 +152,9 @@ class HijackingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_EQ(req.message(), "Hello");
     }
     if (methods->QueryInterceptionHookPoint(
@@ -255,7 +257,9 @@ class HijackingInterceptorMakesAnotherCall : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_EQ(req.message(), "Hello");
       req_ = req;
       stub_ = grpc::testing::EchoTestService::NewStub(
@@ -367,7 +371,9 @@ class LoggingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_TRUE(req.message().find("Hello") == 0);
     }
     if (methods->QueryInterceptionHookPoint(
diff --git a/test/cpp/end2end/server_interceptors_end2end_test.cc b/test/cpp/end2end/server_interceptors_end2end_test.cc
index 44ba2a6009..e08a4493d3 100644
--- a/test/cpp/end2end/server_interceptors_end2end_test.cc
+++ b/test/cpp/end2end/server_interceptors_end2end_test.cc
@@ -103,7 +103,9 @@ class LoggingInterceptor : public experimental::Interceptor {
       EchoRequest req;
       auto* buffer = methods->GetSendMessage();
       auto copied_buffer = *buffer;
-      SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req);
+      EXPECT_TRUE(
+          SerializationTraits<EchoRequest>::Deserialize(&copied_buffer, &req)
+              .ok());
       EXPECT_TRUE(req.message().find("Hello") == 0);
     }
     if (methods->QueryInterceptionHookPoint(