Merge pull request #17544 from yashykt/factorydocs

Allow interceptor creators to return nullptr

5 files changed, 67 insertions, 4 deletions

diff --git a/include/grpcpp/impl/codegen/client_interceptor.h b/include/grpcpp/impl/codegen/client_interceptor.h
index 2bae11a251..da75bde499 100644
--- a/include/grpcpp/impl/codegen/client_interceptor.h
+++ b/include/grpcpp/impl/codegen/client_interceptor.h
@@ -38,9 +38,17 @@ class InterceptorBatchMethodsImpl;
 namespace experimental {
 class ClientRpcInfo;
 
+// A factory interface for creation of client interceptors. A vector of
+// factories can be provided at channel creation which will be used to create a
+// new vector of client interceptors per RPC. Client interceptor authors should
+// create a subclass of ClientInterceptorFactorInterface which creates objects
+// of their interceptors.
 class ClientInterceptorFactoryInterface {
  public:
   virtual ~ClientInterceptorFactoryInterface() {}
+  // Returns a pointer to an Interceptor object on successful creation, nullptr
+  // otherwise. If nullptr is returned, this server interceptor factory is
+  // ignored for the purposes of that RPC.
   virtual Interceptor* CreateClientInterceptor(ClientRpcInfo* info) = 0;
 };
 }  // namespace experimental
@@ -120,8 +128,11 @@ class ClientRpcInfo {
     }
     for (auto it = creators.begin() + interceptor_pos; it != creators.end();
          ++it) {
-      interceptors_.push_back(std::unique_ptr<experimental::Interceptor>(
-          (*it)->CreateClientInterceptor(this)));
+      auto* interceptor = (*it)->CreateClientInterceptor(this);
+      if (interceptor != nullptr) {
+        interceptors_.push_back(
+            std::unique_ptr<experimental::Interceptor>(interceptor));
+      }
     }
     if (internal::g_global_client_interceptor_factory != nullptr) {
       interceptors_.push_back(std::unique_ptr<experimental::Interceptor>(
diff --git a/include/grpcpp/impl/codegen/server_interceptor.h b/include/grpcpp/impl/codegen/server_interceptor.h
index afc3c198cc..8652ec5c64 100644
--- a/include/grpcpp/impl/codegen/server_interceptor.h
+++ b/include/grpcpp/impl/codegen/server_interceptor.h
@@ -37,9 +37,17 @@ class InterceptorBatchMethodsImpl;
 namespace experimental {
 class ServerRpcInfo;
 
+// A factory interface for creation of server interceptors. A vector of
+// factories can be provided to ServerBuilder which will be used to create a new
+// vector of server interceptors per RPC. Server interceptor authors should
+// create a subclass of ServerInterceptorFactorInterface which creates objects
+// of their interceptors.
 class ServerInterceptorFactoryInterface {
  public:
   virtual ~ServerInterceptorFactoryInterface() {}
+  // Returns a pointer to an Interceptor object on successful creation, nullptr
+  // otherwise. If nullptr is returned, this server interceptor factory is
+  // ignored for the purposes of that RPC.
   virtual Interceptor* CreateServerInterceptor(ServerRpcInfo* info) = 0;
 };
 
@@ -90,8 +98,11 @@ class ServerRpcInfo {
           std::unique_ptr<experimental::ServerInterceptorFactoryInterface>>&
           creators) {
     for (const auto& creator : creators) {
-      interceptors_.push_back(std::unique_ptr<experimental::Interceptor>(
-          creator->CreateServerInterceptor(this)));
+      auto* interceptor = creator->CreateServerInterceptor(this);
+      if (interceptor != nullptr) {
+        interceptors_.push_back(
+            std::unique_ptr<experimental::Interceptor>(interceptor));
+      }
     }
   }
 
diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc
index 34f5b93cab..8abf4eb3f4 100644
--- a/test/cpp/end2end/client_interceptors_end2end_test.cc
+++ b/test/cpp/end2end/client_interceptors_end2end_test.cc
@@ -467,6 +467,28 @@ TEST_F(ClientInterceptorsEnd2endTest,
   EXPECT_EQ(DummyInterceptor::GetNumTimesRun(), 20);
 }
 
+TEST_F(ClientInterceptorsEnd2endTest,
+       ClientInterceptorFactoryAllowsNullptrReturn) {
+  ChannelArguments args;
+  DummyInterceptor::Reset();
+  std::vector<std::unique_ptr<experimental::ClientInterceptorFactoryInterface>>
+      creators;
+  creators.push_back(std::unique_ptr<LoggingInterceptorFactory>(
+      new LoggingInterceptorFactory()));
+  // Add 20 dummy interceptors and 20 null interceptors
+  for (auto i = 0; i < 20; i++) {
+    creators.push_back(std::unique_ptr<DummyInterceptorFactory>(
+        new DummyInterceptorFactory()));
+    creators.push_back(
+        std::unique_ptr<NullInterceptorFactory>(new NullInterceptorFactory()));
+  }
+  auto channel = server_->experimental().InProcessChannelWithInterceptors(
+      args, std::move(creators));
+  MakeCallbackCall(channel);
+  // Make sure all 20 dummy interceptors were run
+  EXPECT_EQ(DummyInterceptor::GetNumTimesRun(), 20);
+}
+
 class ClientInterceptorsStreamingEnd2endTest : public ::testing::Test {
  protected:
   ClientInterceptorsStreamingEnd2endTest() {
diff --git a/test/cpp/end2end/interceptors_util.h b/test/cpp/end2end/interceptors_util.h
index d886e32494..659e613d2e 100644
--- a/test/cpp/end2end/interceptors_util.h
+++ b/test/cpp/end2end/interceptors_util.h
@@ -82,6 +82,22 @@ class DummyInterceptorFactory
   }
 };
 
+/* This interceptor factory returns nullptr on interceptor creation */
+class NullInterceptorFactory
+    : public experimental::ClientInterceptorFactoryInterface,
+      public experimental::ServerInterceptorFactoryInterface {
+ public:
+  virtual experimental::Interceptor* CreateClientInterceptor(
+      experimental::ClientRpcInfo* info) override {
+    return nullptr;
+  }
+
+  virtual experimental::Interceptor* CreateServerInterceptor(
+      experimental::ServerRpcInfo* info) override {
+    return nullptr;
+  }
+};
+
 class EchoTestServiceStreamingImpl : public EchoTestService::Service {
  public:
   ~EchoTestServiceStreamingImpl() override {}
diff --git a/test/cpp/end2end/server_interceptors_end2end_test.cc b/test/cpp/end2end/server_interceptors_end2end_test.cc
index 4efe13fdec..53d8c4dc96 100644
--- a/test/cpp/end2end/server_interceptors_end2end_test.cc
+++ b/test/cpp/end2end/server_interceptors_end2end_test.cc
@@ -176,9 +176,12 @@ class ServerInterceptorsEnd2endSyncUnaryTest : public ::testing::Test {
     creators.push_back(
         std::unique_ptr<experimental::ServerInterceptorFactoryInterface>(
             new LoggingInterceptorFactory()));
+    // Add 20 dummy interceptor factories and null interceptor factories
     for (auto i = 0; i < 20; i++) {
       creators.push_back(std::unique_ptr<DummyInterceptorFactory>(
           new DummyInterceptorFactory()));
+      creators.push_back(std::unique_ptr<NullInterceptorFactory>(
+          new NullInterceptorFactory()));
     }
     builder.experimental().SetInterceptorCreators(std::move(creators));
     server_ = builder.BuildAndStart();