Allow AsyncServer with one generic to compile

include/grpcpp/impl/codegen/rpc_service_method.h

@@ -31,6 +31,8 @@
 #include <grpcpp/impl/codegen/rpc_method.h>
 #include <grpcpp/impl/codegen/status.h>
 
+#include <grpc/support/log.h>
+
 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<AsyncService>. 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<MethodHandler> 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
 

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);

test/cpp/codegen/compiler_test_golden

@@ -26,7 +26,7 @@
 
 #include "src/proto/grpc/testing/compiler_test.pb.h"
 
-#include <grpcpp/generic/async_generic_service.h>
+#include <grpcpp/impl/codegen/async_generic_service.h>
 #include <grpcpp/impl/codegen/async_stream.h>
 #include <grpcpp/impl/codegen/async_unary_call.h>
 #include <grpcpp/impl/codegen/method_handler_impl.h>

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<SType>();
 
@@ -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<SType>();
   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<SType>();
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc