From 85ddf5c71351faae8d269339bbc8be638a3d52c1 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 29 Oct 2018 00:14:05 -0700 Subject: [PATCH 1/3] Get ClientContext included with ChannelInterface, and slight more cleanup --- include/grpcpp/impl/codegen/channel_interface.h | 1 + include/grpcpp/impl/codegen/client_interceptor.h | 1 - include/grpcpp/impl/codegen/interceptor.h | 2 +- include/grpcpp/impl/codegen/server_interceptor.h | 1 - test/cpp/end2end/client_interceptors_end2end_test.cc | 12 +++++++++--- test/cpp/end2end/server_interceptors_end2end_test.cc | 4 +++- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/include/grpcpp/impl/codegen/channel_interface.h b/include/grpcpp/impl/codegen/channel_interface.h index 6fd1dd1d9be..e957ea6aab7 100644 --- a/include/grpcpp/impl/codegen/channel_interface.h +++ b/include/grpcpp/impl/codegen/channel_interface.h @@ -21,6 +21,7 @@ #include #include +#include #include #include diff --git a/include/grpcpp/impl/codegen/client_interceptor.h b/include/grpcpp/impl/codegen/client_interceptor.h index 00113f04aaa..0e08a7ce01d 100644 --- a/include/grpcpp/impl/codegen/client_interceptor.h +++ b/include/grpcpp/impl/codegen/client_interceptor.h @@ -21,7 +21,6 @@ #include -#include #include #include diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index cdd34b80d17..15cab711e5c 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -21,13 +21,13 @@ #include #include -#include #include #include #include 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 c39e9a988d5..5fb5df28b70 100644 --- a/include/grpcpp/impl/codegen/server_interceptor.h +++ b/include/grpcpp/impl/codegen/server_interceptor.h @@ -22,7 +22,6 @@ #include #include -#include #include #include diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 5720e87478f..e8ffd463442 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::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits::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::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits::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::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits::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 44ba2a60093..e08a4493d3d 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::Deserialize(&copied_buffer, &req); + EXPECT_TRUE( + SerializationTraits::Deserialize(&copied_buffer, &req) + .ok()); EXPECT_TRUE(req.message().find("Hello") == 0); } if (methods->QueryInterceptionHookPoint( From bfd1935afc34fc1cc1a3ddf357f62ad32aff249e Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 29 Oct 2018 01:56:12 -0700 Subject: [PATCH 2/3] Add note on the experimental nature of the additional method in ServerInterface --- include/grpcpp/impl/codegen/server_interface.h | 7 ++++++- include/grpcpp/server.h | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/grpcpp/impl/codegen/server_interface.h b/include/grpcpp/impl/codegen/server_interface.h index 92c87a5f7ec..3967e96cfe5 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>* interceptor_creators() { return nullptr; diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h index 2b89ffd317a..76a4e56f06e 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>* + std::vector>* interceptor_creators() override { return &interceptor_creators_; } From 145789e9ff5b579d2e31c84cb6a5b378a20aa89a Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 29 Oct 2018 13:15:29 -0700 Subject: [PATCH 3/3] interceptor_creators_ should be destroyed after health_check_service_ --- include/grpcpp/server.h | 11 ++++++++--- src/cpp/server/server_cc.cc | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h index 76a4e56f06e..82d60b02183 100644 --- a/include/grpcpp/server.h +++ b/include/grpcpp/server.h @@ -225,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> + interceptor_creators_; + const int max_receive_message_size_; /// The following completion queues are ONLY used in case of Sync API @@ -260,9 +268,6 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { // A special handler for resource exhausted in sync case std::unique_ptr resource_exhausted_handler_; - - std::vector> - interceptor_creators_; }; } // namespace grpc diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 59a531e2725..04717a013bb 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -446,7 +446,8 @@ Server::Server( std::vector< std::unique_ptr> 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), @@ -454,8 +455,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;