[C++][FlightRPC] Pass ServerCallContext instead of CallHeaders to ServerMiddlewareFactory::StartCall()

[kou]

Describe the enhancement requested

I want to use arrow::flight::ServerCallContext::peer() to get client address in https://github.com/apache/arrow-flight-sql-postgresql because PostgreSQL can specify authentication configurations per client address.
See also: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html

Is it acceptable that we change StartCall() signature to

virtual Status ServerMiddlewareFactory::StartCall(const CallInfo& info,
                                                  const ServerCallContext& context,
                                                  std::shared_ptr<ServerMiddleware>* middleware)

from

virtual Status ServerMiddlewareFactory::StartCall(const CallInfo& info,
                                                  const CallHeaders& incoming_headers,
                                                  std::shared_ptr<ServerMiddleware>* middleware)

?

Here is an incomplete diff that illustrates this proposal:

diff --git a/cpp/src/arrow/flight/server_middleware.h b/cpp/src/arrow/flight/server_middleware.h
index 26431aff0..b8669a293 100644
--- a/cpp/src/arrow/flight/server_middleware.h
+++ b/cpp/src/arrow/flight/server_middleware.h
@@ -30,6 +30,8 @@
 namespace arrow {
 namespace flight {
 
+class ServerCallContext;
+
 /// \brief Server-side middleware for a call, instantiated per RPC.
 ///
 /// Middleware should be fast and must be infallible: there is no way
@@ -61,6 +63,28 @@ class ARROW_FLIGHT_EXPORT ServerMiddlewareFactory {
  public:
   virtual ~ServerMiddlewareFactory() = default;
 
+  /// \brief A callback for the start of a new call.
+  ///
+  /// Return a non-OK status to reject the call with the given status.
+  ///
+  /// \param[in] info Information about the call.
+  /// \param[in] context The call context.
+  /// \param[out] middleware The middleware instance for this call. If
+  ///     null, no middleware will be added to this call instance from
+  ///     this factory.
+  /// \return Status A non-OK status will reject the call with the
+  ///     given status. Middleware previously in the chain will have
+  ///     their CallCompleted callback called. Other middleware
+  ///     factories will not be called.
+  virtual Status StartCall(const CallInfo& info, const ServerCallContext& context, ,
+                           std::shared_ptr<ServerMiddleware>* middleware) {
+    // TODO: We can make this pure virtual function when we remove
+    // the deprecated version.
+    ARROW_SUPPRESS_DEPRECATION_WARNING
+    return StartCall(info, context.incoming_headers(), middleware);
+    ARROW_UNSUPPRESS_DEPRECATION_WARNING
+  }
+
   /// \brief A callback for the start of a new call.
   ///
   /// Return a non-OK status to reject the call with the given status.
@@ -75,8 +99,12 @@ class ARROW_FLIGHT_EXPORT ServerMiddlewareFactory {
   ///     given status. Middleware previously in the chain will have
   ///     their CallCompleted callback called. Other middleware
   ///     factories will not be called.
+  ARROW_DEPRECATED("Deprecated in 13.0.0. Use ServerCallContext overload instead.")
   virtual Status StartCall(const CallInfo& info, const CallHeaders& incoming_headers,
-                           std::shared_ptr<ServerMiddleware>* middleware) = 0;
+                           std::shared_ptr<ServerMiddleware>* middleware) {
+    return Status::NotImplemented(typeid(this).name(),
+                                  "::StartCall() isn't implemented");
+  }
 };
 
 }  // namespace flight
diff --git a/cpp/src/arrow/flight/transport/grpc/grpc_server.cc b/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
index 09d702cd8..d0d618993 100644
--- a/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
+++ b/cpp/src/arrow/flight/transport/grpc/grpc_server.cc
@@ -324,7 +324,7 @@ class GrpcServiceHandler final : public FlightService::Service {
     for (const auto& factory : middleware_) {
       std::shared_ptr<ServerMiddleware> instance;
       Status result =
-          factory.second->StartCall(info, flight_context.incoming_headers(), &instance);
+          factory.second->StartCall(info, flight_context, &instance);
       if (!result.ok()) {
         // Interceptor rejected call, end the request on all existing
         // interceptors

Component(s)

C++, FlightRPC

[kou]

@lidavidm What do you think about this?

[lidavidm]

I think this is reasonable.