From d170b83e0d29b93d3de7c508281cfeae38ae141e Mon Sep 17 00:00:00 2001 From: ncteisen Date: Tue, 19 Jun 2018 08:11:13 -0700 Subject: [PATCH] Allow AsyncServer with one generic to compile --- .../grpcpp/impl/codegen/rpc_service_method.h | 45 ++++++++++++++++++- include/grpcpp/impl/codegen/service_type.h | 30 ++++++++----- test/cpp/codegen/compiler_test_golden | 2 +- .../end2end/codegen_generic_end2end_test.cc | 29 ++++++++---- 4 files changed, 84 insertions(+), 22 deletions(-) diff --git a/include/grpcpp/impl/codegen/rpc_service_method.h b/include/grpcpp/impl/codegen/rpc_service_method.h index 1b4e57d80ce..c38ea2b0625 100644 --- a/include/grpcpp/impl/codegen/rpc_service_method.h +++ b/include/grpcpp/impl/codegen/rpc_service_method.h @@ -31,6 +31,8 @@ #include #include +#include + namespace grpc { class ServerContext; @@ -59,18 +61,57 @@ class RpcServiceMethod : public RpcMethod { /// Takes ownership of the handler RpcServiceMethod(const char* name, RpcMethod::RpcType type, MethodHandler* handler) - : RpcMethod(name, type), server_tag_(nullptr), handler_(handler) {} + : RpcMethod(name, type), + server_tag_(nullptr), + async_type_(AsyncType::UNSET), + handler_(handler) {} + + enum class AsyncType { + UNSET, + ASYNC, + CODEGEN_GENERIC, + }; void set_server_tag(void* tag) { server_tag_ = tag; } void* server_tag() const { return server_tag_; } /// if MethodHandler is nullptr, then this is an async method MethodHandler* handler() const { return handler_.get(); } - void ResetHandler() { handler_.reset(); } void SetHandler(MethodHandler* handler) { handler_.reset(handler); } + void SetServerAsyncType(RpcServiceMethod::AsyncType type) { + if (async_type_ == AsyncType::UNSET) { + // this marks this method as async + handler_.reset(); + } else { + // this is not an error condition, as it allows users to declare a server + // like WithCodegenGenericMethod_foo. However since it + // overwrites behavior, it should be logged. + gpr_log( + GPR_INFO, + "You are marking method %s as '%s', even though it was " + "previously marked '%s'. This behavior will overwrite the original " + "behavior. If you expected this then ignore this message.", + name(), TypeToString(async_type_), TypeToString(type)); + } + async_type_ = type; + } private: void* server_tag_; + RpcServiceMethod::AsyncType async_type_; std::unique_ptr handler_; + + const char* TypeToString(RpcServiceMethod::AsyncType type) { + switch (type) { + case AsyncType::UNSET: + return "unset"; + case AsyncType::ASYNC: + return "async"; + case AsyncType::CODEGEN_GENERIC: + return "codegen generic"; + default: + GPR_UNREACHABLE_CODE(return "unknown"); + } + } }; } // namespace internal diff --git a/include/grpcpp/impl/codegen/service_type.h b/include/grpcpp/impl/codegen/service_type.h index af94d653f07..dc63b9dc21c 100644 --- a/include/grpcpp/impl/codegen/service_type.h +++ b/include/grpcpp/impl/codegen/service_type.h @@ -124,30 +124,40 @@ class Service { } void MarkMethodAsync(int index) { + // This does not have to be a hard error, however no one has approached us + // with a use case yet. Please file an issue if you believe you have one. GPR_CODEGEN_ASSERT( methods_[index].get() != nullptr && "Cannot mark the method as 'async' because it has already been " "marked as 'generic'."); - methods_[index]->ResetHandler(); + methods_[index]->SetServerAsyncType( + internal::RpcServiceMethod::AsyncType::ASYNC); + } + + void MarkMethodCodegenGeneric(int index) { + // This does not have to be a hard error, however no one has approached us + // with a use case yet. Please file an issue if you believe you have one. + GPR_CODEGEN_ASSERT( + methods_[index].get() != nullptr && + "Cannot mark the method as 'codegen generic' because it has already " + "been marked as 'generic'."); + methods_[index]->SetServerAsyncType( + internal::RpcServiceMethod::AsyncType::CODEGEN_GENERIC); } void MarkMethodGeneric(int index) { + // This does not have to be a hard error, however no one has approached us + // with a use case yet. Please file an issue if you believe you have one. GPR_CODEGEN_ASSERT( methods_[index]->handler() != nullptr && "Cannot mark the method as 'generic' because it has already been " - "marked as 'async'."); + "marked as 'async' or 'codegen generic'."); methods_[index].reset(); } - void MarkMethodCodegenGeneric(int index) { - GPR_CODEGEN_ASSERT(methods_[index]->handler() != nullptr && - "Cannot mark the method as 'codegen generic' because it " - "has already been " - "marked as 'async' or 'generic'."); - methods_[index]->ResetHandler(); - } - void MarkMethodStreamed(int index, internal::MethodHandler* streamed_method) { + // This does not have to be a hard error, however no one has approached us + // with a use case yet. Please file an issue if you believe you have one. GPR_CODEGEN_ASSERT(methods_[index] && methods_[index]->handler() && "Cannot mark an async or generic method Streamed"); methods_[index]->SetHandler(streamed_method); diff --git a/test/cpp/codegen/compiler_test_golden b/test/cpp/codegen/compiler_test_golden index 033c963dc4a..90dabb5cb74 100644 --- a/test/cpp/codegen/compiler_test_golden +++ b/test/cpp/codegen/compiler_test_golden @@ -26,7 +26,7 @@ #include "src/proto/grpc/testing/compiler_test.pb.h" -#include +#include #include #include #include diff --git a/test/cpp/end2end/codegen_generic_end2end_test.cc b/test/cpp/end2end/codegen_generic_end2end_test.cc index f8f8a702a70..6bb5712eb26 100644 --- a/test/cpp/end2end/codegen_generic_end2end_test.cc +++ b/test/cpp/end2end/codegen_generic_end2end_test.cc @@ -311,9 +311,10 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerUnary) { // Client uses proto, server uses generic codegen, client streaming TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerClientStreaming) { - typedef grpc::testing::EchoTestService::WithCodegenGenericMethod_RequestStream< - grpc::testing::EchoTestService::Service> - SType; + typedef grpc::testing::EchoTestService:: + WithCodegenGenericMethod_RequestStream< + grpc::testing::EchoTestService::Service> + SType; ResetStub(); auto service = BuildAndStartServer(); @@ -324,7 +325,7 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerClientStreaming) { stub_->AsyncRequestStream(&cli_ctx_, &recv_response_, cq_.get(), tag(1))); service->RequestRequestStream(&srv_ctx_, &srv_stream, cq_.get(), cq_.get(), - tag(2)); + tag(2)); Verifier().Expect(2, true).Expect(1, true).Verify(cq_.get()); @@ -357,9 +358,10 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerClientStreaming) { // Client uses proto, server uses generic codegen, server streaming TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerServerStreaming) { - typedef grpc::testing::EchoTestService::WithCodegenGenericMethod_ResponseStream< - grpc::testing::EchoTestService::Service> - SType; + typedef grpc::testing::EchoTestService:: + WithCodegenGenericMethod_ResponseStream< + grpc::testing::EchoTestService::Service> + SType; ResetStub(); auto service = BuildAndStartServer(); grpc::GenericServerAsyncWriter srv_stream(&srv_ctx_); @@ -369,7 +371,7 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerServerStreaming) { stub_->AsyncResponseStream(&cli_ctx_, send_request_, cq_.get(), tag(1))); service->RequestResponseStream(&srv_ctx_, &recv_request_buffer_, &srv_stream, - cq_.get(), cq_.get(), tag(2)); + cq_.get(), cq_.get(), tag(2)); Verifier().Expect(1, true).Expect(2, true).Verify(cq_.get()); ParseFromByteBuffer(&recv_request_buffer_, &recv_request_); @@ -412,7 +414,7 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerBidiStreaming) { cli_stream(stub_->AsyncBidiStream(&cli_ctx_, cq_.get(), tag(1))); service->RequestBidiStream(&srv_ctx_, &srv_stream, cq_.get(), cq_.get(), - tag(2)); + tag(2)); Verifier().Expect(1, true).Expect(2, true).Verify(cq_.get()); @@ -440,6 +442,15 @@ TEST_F(CodegenGenericEnd2EndTest, CodegenGenericServerBidiStreaming) { EXPECT_TRUE(recv_status_.ok()); } +// Testing that this pattern compiles +TEST_F(CodegenGenericEnd2EndTest, CompileTest) { + typedef grpc::testing::EchoTestService::WithCodegenGenericMethod_Echo< + grpc::testing::EchoTestService::AsyncService> + SType; + ResetStub(); + auto service = BuildAndStartServer(); +} + } // namespace } // namespace testing } // namespace grpc