Change names to StreamedUnary, ServerUnaryStreamer, etc. Use a templated method handler since most code shared between the new StreamedUnary and the existing BidiStreaming. Eliminate the separate enum case for streamed unary. Return a status failure if a StreamedUnary method handler doesn't actually do a write (since that is

violating the appearance of unary-ness)

8 files changed, 61 insertions, 76 deletions

diff --git a/include/grpc++/ext/reflection.grpc.pb.h b/include/grpc++/ext/reflection.grpc.pb.h
index 18ec2ea1ec..822c2e374a 100644
--- a/include/grpc++/ext/reflection.grpc.pb.h
+++ b/include/grpc++/ext/reflection.grpc.pb.h
@@ -74,10 +74,10 @@
 
 #include <grpc++/impl/codegen/async_stream.h>
 #include <grpc++/impl/codegen/async_unary_call.h>
-#include <grpc++/impl/codegen/fc_unary.h>
 #include <grpc++/impl/codegen/method_handler_impl.h>
 #include <grpc++/impl/codegen/proto_utils.h>
 #include <grpc++/impl/codegen/rpc_method.h>
+#include <grpc++/impl/codegen/server_streamed_unary.h>
 #include <grpc++/impl/codegen/service_type.h>
 #include <grpc++/impl/codegen/status.h>
 #include <grpc++/impl/codegen/stub_options.h>
diff --git a/include/grpc++/impl/codegen/completion_queue.h b/include/grpc++/impl/codegen/completion_queue.h
index 95ece77ce5..ea317a7a79 100644
--- a/include/grpc++/impl/codegen/completion_queue.h
+++ b/include/grpc++/impl/codegen/completion_queue.h
@@ -78,8 +78,6 @@ template <class ServiceType, class RequestType, class ResponseType>
 class ServerStreamingHandler;
 template <class ServiceType, class RequestType, class ResponseType>
 class BidiStreamingHandler;
-template <class ServiceType, class RequestType, class ResponseType>
-class FCUnaryMethodHandler;
 class UnknownMethodHandler;
 
 class Channel;
@@ -187,10 +185,8 @@ class CompletionQueue : private GrpcLibraryCodegen {
   friend class ClientStreamingHandler;
   template <class ServiceType, class RequestType, class ResponseType>
   friend class ServerStreamingHandler;
-  template <class ServiceType, class RequestType, class ResponseType>
-  friend class BidiStreamingHandler;
-  template <class ServiceType, class RequestType, class ResponseType>
-  friend class FCUnaryMethodHandler;
+  template <class Streamer, bool WriteNeeded>
+  friend class TemplatedBidiStreamingHandler;
   friend class UnknownMethodHandler;
   friend class ::grpc::Server;
   friend class ::grpc::ServerContext;
diff --git a/include/grpc++/impl/codegen/method_handler_impl.h b/include/grpc++/impl/codegen/method_handler_impl.h
index 9c3af53b3a..3a671fe830 100644
--- a/include/grpc++/impl/codegen/method_handler_impl.h
+++ b/include/grpc++/impl/codegen/method_handler_impl.h
@@ -35,8 +35,8 @@
 #define GRPCXX_IMPL_CODEGEN_METHOD_HANDLER_IMPL_H
 
 #include <grpc++/impl/codegen/core_codegen_interface.h>
-#include <grpc++/impl/codegen/fc_unary.h>
 #include <grpc++/impl/codegen/rpc_service_method.h>
+#include <grpc++/impl/codegen/server_streamed_unary.h>
 #include <grpc++/impl/codegen/sync_stream.h>
 
 namespace grpc {
@@ -168,20 +168,23 @@ class ServerStreamingHandler : public MethodHandler {
 };
 
 // A wrapper class of an application provided bidi-streaming handler.
-template <class ServiceType, class RequestType, class ResponseType>
-class BidiStreamingHandler : public MethodHandler {
+// This also applies to server-streamed implementation of a unary method
+// with the additional requirement that such methods must have done a
+// write for status to be ok
+// Since this is used by more than 1 class, the service is not passed in.
+// Instead, it is expected to be an implicitly-captured argument of func
+// (through bind or something along those lines)
+template <class Streamer, bool WriteNeeded>
+class TemplatedBidiStreamingHandler : public MethodHandler {
  public:
-  BidiStreamingHandler(
-      std::function<Status(ServiceType*, ServerContext*,
-                           ServerReaderWriter<ResponseType, RequestType>*)>
-          func,
-      ServiceType* service)
-      : func_(func), service_(service) {}
+  TemplatedBidiStreamingHandler(
+      std::function<Status(ServerContext*, Streamer*)>
+          func)
+    : func_(func), write_needed_(WriteNeeded) {}
 
   void RunHandler(const HandlerParameter& param) GRPC_FINAL {
-    ServerReaderWriter<ResponseType, RequestType> stream(param.call,
-                                                         param.server_context);
-    Status status = func_(service_, param.server_context, &stream);
+    Streamer stream(param.call, param.server_context);
+    Status status = func_(param.server_context, &stream);
 
     CallOpSet<CallOpSendInitialMetadata, CallOpServerSendStatus> ops;
     if (!param.server_context->sent_initial_metadata_) {
@@ -190,6 +193,12 @@ class BidiStreamingHandler : public MethodHandler {
       if (param.server_context->compression_level_set()) {
         ops.set_compression_level(param.server_context->compression_level());
       }
+      if (write_needed_ && status.ok()) {
+	// If we needed a write but never did one, we need to mark the
+	// status as a fail
+	status = Status(IMPROPER_IMPLEMENTATION,
+			"Service did not provide response message");
+      }
     }
     ops.ServerSendStatus(param.server_context->trailing_metadata_, status);
     param.call->PerformOps(&ops);
@@ -197,46 +206,24 @@ class BidiStreamingHandler : public MethodHandler {
   }
 
  private:
-  std::function<Status(ServiceType*, ServerContext*,
-                       ServerReaderWriter<ResponseType, RequestType>*)>
-      func_;
-  ServiceType* service_;
+  std::function<Status(ServerContext*, Streamer*)>
+    func_;
+  const bool write_needed_;
 };
 
-// A wrapper class of an application provided rpc method handler
-// specifically to apply to the flow-controlled implementation of a unary
-// method.
-/// The argument to the constructor should be a member function already
-/// bound to the appropriate service instance. The declaration gets too
-/// complicated
-/// otherwise.
 template <class ServiceType, class RequestType, class ResponseType>
-class FCUnaryMethodHandler : public MethodHandler {
+  class BidiStreamingHandler : public TemplatedBidiStreamingHandler<ServerReaderWriter<ResponseType, RequestType>, false> {
  public:
-  FCUnaryMethodHandler(
-      std::function<Status(ServerContext*, FCUnary<RequestType, ResponseType>*)>
-          func)
-      : func_(func) {}
+  BidiStreamingHandler(std::function<Status(ServiceType*, ServerContext*,
+					    ServerReaderWriter<ResponseType,RequestType>*)> func, ServiceType* service): TemplatedBidiStreamingHandler<ServerReaderWriter<ResponseType, RequestType>,false>(std::bind(func, service, std::placeholders::_1, std::placeholders::_2)) {}
+ };
 
-  void RunHandler(const HandlerParameter& param) GRPC_FINAL {
-    FCUnary<RequestType, ResponseType> fc_unary(param.call,
-                                                param.server_context);
-    Status status = func_(param.server_context, &fc_unary);
-    if (!param.server_context->sent_initial_metadata_) {
-      // means that the write never happened, which is bad
-    } else {
-      CallOpSet<CallOpServerSendStatus> ops;
-      ops.ServerSendStatus(param.server_context->trailing_metadata_, status);
-      param.call->PerformOps(&ops);
-      param.call->cq()->Pluck(&ops);
-    }
-  }
-
- private:
-  // Application provided rpc handler function, already bound to its service.
-  std::function<Status(ServerContext*, FCUnary<RequestType, ResponseType>*)>
-      func_;
-};
+ template <class RequestType, class ResponseType>
+   class StreamedUnaryHandler : public TemplatedBidiStreamingHandler<ServerUnaryStreamer<RequestType, ResponseType>, true> {
+ public:
+ explicit StreamedUnaryHandler(std::function<Status(ServerContext*,
+					    ServerUnaryStreamer<RequestType,ResponseType>*)> func): TemplatedBidiStreamingHandler<ServerUnaryStreamer<RequestType, ResponseType>, true>(func) {}
+ };
 
 // Handle unknown method by returning UNIMPLEMENTED error.
 class UnknownMethodHandler : public MethodHandler {
diff --git a/include/grpc++/impl/codegen/server_context.h b/include/grpc++/impl/codegen/server_context.h
index aadae893ad..8e101c3f15 100644
--- a/include/grpc++/impl/codegen/server_context.h
+++ b/include/grpc++/impl/codegen/server_context.h
@@ -75,8 +75,6 @@ template <class ServiceType, class RequestType, class ResponseType>
 class ServerStreamingHandler;
 template <class ServiceType, class RequestType, class ResponseType>
 class BidiStreamingHandler;
-template <class ServiceType, class RequestType, class ResponseType>
-class FCUnaryMethodHandler;
 class UnknownMethodHandler;
 
 class Call;
@@ -192,10 +190,8 @@ class ServerContext {
   friend class ClientStreamingHandler;
   template <class ServiceType, class RequestType, class ResponseType>
   friend class ServerStreamingHandler;
-  template <class ServiceType, class RequestType, class ResponseType>
-  friend class BidiStreamingHandler;
-  template <class ServiceType, class RequestType, class ResponseType>
-  friend class FCUnaryMethodHandler;
+  template <class Streamer, bool WriteNeeded>
+  friend class TemplatedBidiStreamingHandler;
   friend class UnknownMethodHandler;
   friend class ::grpc::ClientContext;
 
diff --git a/include/grpc++/impl/codegen/fc_unary.h b/include/grpc++/impl/codegen/server_streamed_unary.h
index a423f1f0b3..a23e6020ed 100644
--- a/include/grpc++/impl/codegen/fc_unary.h
+++ b/include/grpc++/impl/codegen/server_streamed_unary.h
@@ -31,8 +31,8 @@
  *
  */
 
-#ifndef GRPCXX_IMPL_CODEGEN_FC_UNARY_H
-#define GRPCXX_IMPL_CODEGEN_FC_UNARY_H
+#ifndef GRPCXX_IMPL_CODEGEN_SERVER_STREAMED_UNARY_H
+#define GRPCXX_IMPL_CODEGEN_SERVER_STREAMED_UNARY_H
 
 #include <grpc++/impl/codegen/call.h>
 #include <grpc++/impl/codegen/completion_queue.h>
@@ -47,19 +47,19 @@ namespace grpc {
 /// as though it were a single-ping-pong streaming call. The server can use
 /// the \a NextMessageSize method to determine an upper-bound on the size of
 /// the message.
-/// A key difference relative to streaming: an FCUnary must have exactly 1 Read
-/// and exactly 1 Write, in that order, to function correctly.
-/// Otherwise, the RPC is in error.
+/// A key difference relative to streaming: ServerUnaryStreamer
+///  must have exactly 1 Read and exactly 1 Write, in that order, to function
+/// correctly. Otherwise, the RPC is in error.
 template <class RequestType, class ResponseType>
-class FCUnary GRPC_FINAL
+class ServerUnaryStreamer GRPC_FINAL
     : public ServerReaderWriterInterface<ResponseType, RequestType> {
  public:
-  FCUnary(Call* call, ServerContext* ctx)
+  ServerUnaryStreamer(Call* call, ServerContext* ctx)
       : ServerReaderWriterInterface<ResponseType, RequestType>(call, ctx),
         read_done_(false),
         write_done_(false) {}
 
-  ~FCUnary() {}
+  ~ServerUnaryStreamer() {}
 
   bool Read(RequestType* request) GRPC_OVERRIDE {
     if (read_done_) {
@@ -87,4 +87,4 @@ class FCUnary GRPC_FINAL
 };
 }  // namespace grpc
 
-#endif  // GRPCXX_IMPL_CODEGEN_FC_UNARY_H
+#endif  // GRPCXX_IMPL_CODEGEN_SERVER_STREAMED_UNARY_H
diff --git a/include/grpc++/impl/codegen/service_type.h b/include/grpc++/impl/codegen/service_type.h
index 4af40422a1..9d2a80cbc4 100644
--- a/include/grpc++/impl/codegen/service_type.h
+++ b/include/grpc++/impl/codegen/service_type.h
@@ -147,10 +147,11 @@ class Service {
     methods_[index].reset();
   }
 
-  void MarkMethodFCUnary(int index, MethodHandler* fc_unary_method) {
+  void MarkMethodStreamedUnary(int index,
+			       MethodHandler* streamed_unary_method) {
     GPR_CODEGEN_ASSERT(methods_[index] && methods_[index]->handler() &&
-                       "Cannot mark an async or generic method as FCUnary");
-    methods_[index]->SetHandler(fc_unary_method);
+                       "Cannot mark an async or generic method Streamed Unary");
+    methods_[index]->SetHandler(streamed_unary_method);
 
     // From the server's point of view, streamed unary is a special
     // case of BIDI_STREAMING that has 1 read and 1 write, in that order.
diff --git a/include/grpc++/impl/codegen/status_code_enum.h b/include/grpc++/impl/codegen/status_code_enum.h
index 9a90a18e2a..0f18a22c36 100644
--- a/include/grpc++/impl/codegen/status_code_enum.h
+++ b/include/grpc++/impl/codegen/status_code_enum.h
@@ -143,6 +143,11 @@ enum StatusCode {
   /// Unrecoverable data loss or corruption.
   DATA_LOSS = 15,
 
+  // Service was improperly implemented, violated a gRPC API requirement
+  // Not quite the same as unimplemented since it could just be that the API
+  // requirement was violated in this particular circumstance
+  IMPROPER_IMPLEMENTATION = 16,
+
   /// Force users to include a default branch:
   DO_NOT_USE = -1
 };
diff --git a/include/grpc++/support/fc_unary.h b/include/grpc++/support/server_streamed_unary.h
index 7e7dea8feb..109dfd4bca 100644
--- a/include/grpc++/support/fc_unary.h
+++ b/include/grpc++/support/server_streamed_unary.h
@@ -31,9 +31,9 @@
  *
  */
 
-#ifndef GRPCXX_SUPPORT_FC_UNARY_H
-#define GRPCXX_SUPPORT_FC_UNARY_H
+#ifndef GRPCXX_SUPPORT_SERVER_STREAMED_UNARY_H
+#define GRPCXX_SUPPORT_SERVER_STREAMED_UNARY_H
 
-#include <grpc++/impl/codegen/fc_unary.h>
+#include <grpc++/impl/codegen/server_streamed_unary.h>
 
-#endif  // GRPCXX_SUPPORT_FC_UNARY_H
+#endif  // GRPCXX_SUPPORT_SERVER_STREAMED_UNARY_H