Browse Source

Deduplicate interceptor pipeline operation (#2157)

Motivation:

ClientInterceptorPipelineOperation is the same as
ServerInterceptorPipelineOperation apart from the type of interceptor
being used. The duplication here is unnecessary.

Modifications:

- Add a `ConditionalInterceptor` which is roughly the same as
`*InterceptorPipelineOperation` but is generic.
- The init is private and can only be initialized via static methods
when the generic type is appropriate (i.e. client/server interceptor).

Result:

Less duplication, less code

---------

Co-authored-by: Gus Cairo <me@gustavocairo.com>
George Barnett 1 year ago
parent
commit
eb7ed6fc35

+ 1 - 1
Sources/GRPCCore/Call/Client/ClientInterceptor.swift

@@ -23,7 +23,7 @@
 /// received from the transport. They are typically used for cross-cutting concerns like injecting
 /// metadata, validating messages, logging additional data, and tracing.
 ///
-/// Interceptors are registered with the client via ``ClientInterceptorPipelineOperation``s.
+/// Interceptors are registered with the client via ``ConditionalInterceptor``s.
 /// You may register them for all services registered with a server, for RPCs directed to specific services, or
 /// for RPCs directed to specific methods. If you need to modify the behavior of an interceptor on a
 /// per-RPC basis in more detail, then you can use the ``ClientContext/descriptor`` to determine

+ 36 - 23
Sources/GRPCCore/Call/Client/ClientInterceptorPipelineOperation.swift → Sources/GRPCCore/Call/ConditionalInterceptor.swift

@@ -14,18 +14,16 @@
  * limitations under the License.
  */
 
-/// A `ClientInterceptorPipelineOperation` describes to which RPCs a client interceptor should be applied.
+/// Describes the conditions under which an interceptor should be applied.
 ///
-/// You can configure a client interceptor to be applied to:
+/// You can configure interceptors to be applied to:
 /// - all RPCs and services;
 /// - requests directed only to specific services; or
 /// - requests directed only to specific methods (of a specific service).
 ///
-/// - SeeAlso: ``ClientInterceptor`` for more information on client interceptors, and
-///  ``ServerInterceptorPipelineOperation`` for the server-side version of this type.
-public struct ClientInterceptorPipelineOperation: Sendable {
-  /// The subject of a ``ClientInterceptorPipelineOperation``.
-  /// The subject of an interceptor can either be all services and methods, only specific services, or only specific methods.
+/// - SeeAlso: ``ClientInterceptor`` and ``ServerInterceptor`` for more information on client and
+///   server interceptors, respectively.
+public struct ConditionalInterceptor<Interceptor: Sendable>: Sendable {
   public struct Subject: Sendable {
     internal enum Wrapped: Sendable {
       case all
@@ -41,7 +39,6 @@ public struct ClientInterceptorPipelineOperation: Sendable {
     /// An operation subject specifying an interceptor that will be applied only to RPCs directed to the specified services.
     /// - Parameters:
     ///   - services: The list of service names for which this interceptor should intercept RPCs.
-    /// - Returns: A ``ClientInterceptorPipelineOperation``.
     public static func services(_ services: Set<ServiceDescriptor>) -> Self {
       Self(wrapped: .services(services))
     }
@@ -49,13 +46,12 @@ public struct ClientInterceptorPipelineOperation: Sendable {
     /// An operation subject specifying an interceptor that will be applied only to RPCs directed to the specified service methods.
     /// - Parameters:
     ///   - methods: The list of method descriptors for which this interceptor should intercept RPCs.
-    /// - Returns: A ``ClientInterceptorPipelineOperation``.
     public static func methods(_ methods: Set<MethodDescriptor>) -> Self {
       Self(wrapped: .methods(methods))
     }
 
     @usableFromInline
-    internal func applies(to descriptor: MethodDescriptor) -> Bool {
+    package func applies(to descriptor: MethodDescriptor) -> Bool {
       switch self.wrapped {
       case .all:
         return true
@@ -69,24 +65,15 @@ public struct ClientInterceptorPipelineOperation: Sendable {
     }
   }
 
-  /// The interceptor specified for this operation.
-  public let interceptor: any ClientInterceptor
+  /// The interceptor.
+  public let interceptor: Interceptor
 
   @usableFromInline
   internal let subject: Subject
 
-  private init(interceptor: any ClientInterceptor, appliesTo: Subject) {
+  fileprivate init(interceptor: Interceptor, subject: Subject) {
     self.interceptor = interceptor
-    self.subject = appliesTo
-  }
-
-  /// Create an operation, specifying which ``ClientInterceptor`` to apply and to which ``Subject``.
-  /// - Parameters:
-  ///   - interceptor: The ``ClientInterceptor`` to register with the client.
-  ///   - subject: The ``Subject`` to which the `interceptor` applies.
-  /// - Returns: A ``ClientInterceptorPipelineOperation``.
-  public static func apply(_ interceptor: any ClientInterceptor, to subject: Subject) -> Self {
-    Self(interceptor: interceptor, appliesTo: subject)
+    self.subject = subject
   }
 
   /// Returns whether this ``ClientInterceptorPipelineOperation`` applies to the given `descriptor`.
@@ -97,3 +84,29 @@ public struct ClientInterceptorPipelineOperation: Sendable {
     self.subject.applies(to: descriptor)
   }
 }
+
+extension ConditionalInterceptor where Interceptor == any ClientInterceptor {
+  /// Create an operation, specifying which ``ClientInterceptor`` to apply and to which ``Subject``.
+  /// - Parameters:
+  ///   - interceptor: The ``ClientInterceptor`` to register with the client.
+  ///   - subject: The ``Subject`` to which the `interceptor` applies.
+  public static func apply(
+    _ interceptor: any ClientInterceptor,
+    to subject: Subject
+  ) -> Self {
+    Self(interceptor: interceptor, subject: subject)
+  }
+}
+
+extension ConditionalInterceptor where Interceptor == any ServerInterceptor {
+  /// Create an operation, specifying which ``ServerInterceptor`` to apply and to which ``Subject``.
+  /// - Parameters:
+  ///   - interceptor: The ``ServerInterceptor`` to register with the server.
+  ///   - subject: The ``Subject`` to which the `interceptor` applies.
+  public static func apply(
+    _ interceptor: any ServerInterceptor,
+    to subject: Subject
+  ) -> Self {
+    Self(interceptor: interceptor, subject: subject)
+  }
+}

+ 3 - 2
Sources/GRPCCore/Call/Server/RPCRouter.swift

@@ -155,9 +155,10 @@ public struct RPCRouter: Sendable {
   ///    only call this method _after_ you have registered all handlers.
   /// - Parameter pipeline: The interceptor pipeline operations to register to all currently-registered handlers. The order of the
   ///  interceptors matters.
-  /// - SeeAlso: ``ServerInterceptorPipelineOperation``.
   @inlinable
-  public mutating func registerInterceptors(pipeline: [ServerInterceptorPipelineOperation]) {
+  public mutating func registerInterceptors(
+    pipeline: [ConditionalInterceptor<any ServerInterceptor>]
+  ) {
     for descriptor in self.handlers.keys {
       let applicableOperations = pipeline.filter { $0.applies(to: descriptor) }
       if !applicableOperations.isEmpty {

+ 1 - 1
Sources/GRPCCore/Call/Server/ServerInterceptor.swift

@@ -21,7 +21,7 @@
 /// been returned from a service. They are typically used for cross-cutting concerns like filtering
 /// requests, validating messages, logging additional data, and tracing.
 ///
-/// Interceptors can be registered with the server either directly or  via ``ServerInterceptorPipelineOperation``s.
+/// Interceptors can be registered with the server either directly or via ``ConditionalInterceptor``s.
 /// You may register them for all services registered with a server, for RPCs directed to specific services, or
 /// for RPCs directed to specific methods. If you need to modify the behavior of an interceptor on a
 /// per-RPC basis in more detail, then you can use the ``ServerContext/descriptor`` to determine

+ 0 - 99
Sources/GRPCCore/Call/Server/ServerInterceptorPipelineOperation.swift

@@ -1,99 +0,0 @@
-/*
- * Copyright 2024, gRPC Authors All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/// A `ServerInterceptorPipelineOperation` describes to which RPCs a server interceptor should be applied.
-///
-/// You can configure a server interceptor to be applied to:
-/// - all RPCs and services;
-/// - requests directed only to specific services registered with your server; or
-/// - requests directed only to specific methods (of a specific service).
-///
-/// - SeeAlso: ``ServerInterceptor`` for more information on server interceptors, and
-///  ``ClientInterceptorPipelineOperation`` for the client-side version of this type.
-public struct ServerInterceptorPipelineOperation: Sendable {
-  /// The subject of a ``ServerInterceptorPipelineOperation``.
-  /// The subject of an interceptor can either be all services and methods, only specific services, or only specific methods.
-  public struct Subject: Sendable {
-    internal enum Wrapped: Sendable {
-      case all
-      case services(Set<ServiceDescriptor>)
-      case methods(Set<MethodDescriptor>)
-    }
-
-    private let wrapped: Wrapped
-
-    /// An operation subject specifying an interceptor that applies to all RPCs across all services will be registered with this server.
-    public static var all: Self { .init(wrapped: .all) }
-
-    /// An operation subject specifying an interceptor that will be applied only to RPCs directed to the specified services.
-    /// - Parameters:
-    ///   - services: The list of service names for which this interceptor should intercept RPCs.
-    /// - Returns: A ``ServerInterceptorPipelineOperation``.
-    public static func services(_ services: Set<ServiceDescriptor>) -> Self {
-      Self(wrapped: .services(services))
-    }
-
-    /// An operation subject specifying an interceptor that will be applied only to RPCs directed to the specified service methods.
-    /// - Parameters:
-    ///   - methods: The list of method descriptors for which this interceptor should intercept RPCs.
-    /// - Returns: A ``ServerInterceptorPipelineOperation``.
-    public static func methods(_ methods: Set<MethodDescriptor>) -> Self {
-      Self(wrapped: .methods(methods))
-    }
-
-    @usableFromInline
-    internal func applies(to descriptor: MethodDescriptor) -> Bool {
-      switch self.wrapped {
-      case .all:
-        return true
-
-      case .services(let services):
-        return services.contains(descriptor.service)
-
-      case .methods(let methods):
-        return methods.contains(descriptor)
-      }
-    }
-  }
-
-  /// The interceptor specified for this operation.
-  public let interceptor: any ServerInterceptor
-
-  @usableFromInline
-  internal let subject: Subject
-
-  private init(interceptor: any ServerInterceptor, appliesTo: Subject) {
-    self.interceptor = interceptor
-    self.subject = appliesTo
-  }
-
-  /// Create an operation, specifying which ``ServerInterceptor`` to apply and to which ``Subject``.
-  /// - Parameters:
-  ///   - interceptor: The ``ServerInterceptor`` to register with the server.
-  ///   - subject: The ``Subject`` to which the `interceptor` applies.
-  /// - Returns: A ``ServerInterceptorPipelineOperation``.
-  public static func apply(_ interceptor: any ServerInterceptor, to subject: Subject) -> Self {
-    Self(interceptor: interceptor, appliesTo: subject)
-  }
-
-  /// Returns whether this ``ServerInterceptorPipelineOperation`` applies to the given `descriptor`.
-  /// - Parameter descriptor: A ``MethodDescriptor`` for which to test whether this interceptor applies.
-  /// - Returns: `true` if this interceptor applies to the given `descriptor`, or `false` otherwise.
-  @inlinable
-  internal func applies(to descriptor: MethodDescriptor) -> Bool {
-    self.subject.applies(to: descriptor)
-  }
-}

+ 1 - 2
Sources/GRPCCore/Documentation.docc/Documentation.md

@@ -96,8 +96,7 @@ Resources for developers working on gRPC Swift:
 - ``ServerInterceptor``
 - ``ClientContext``
 - ``ServerContext``
-- ``ClientInterceptorPipelineOperation``
-- ``ServerInterceptorPipelineOperation``
+- ``ConditionalInterceptor``
 
 ### RPC descriptors
 

+ 6 - 6
Sources/GRPCCore/GRPCClient.swift

@@ -129,7 +129,7 @@ public final class GRPCClient: Sendable {
   private struct StateMachine {
     var state: State
 
-    private let interceptorPipeline: [ClientInterceptorPipelineOperation]
+    private let interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>]
 
     /// A collection of interceptors providing cross-cutting functionality to each accepted RPC, keyed by the method to which they apply.
     ///
@@ -142,7 +142,7 @@ public final class GRPCClient: Sendable {
     /// the appropriate handler.
     var interceptorsPerMethod: [MethodDescriptor: [any ClientInterceptor]]
 
-    init(interceptorPipeline: [ClientInterceptorPipelineOperation]) {
+    init(interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>]) {
       self.state = .notStarted
       self.interceptorPipeline = interceptorPipeline
       self.interceptorsPerMethod = [:]
@@ -188,14 +188,14 @@ public final class GRPCClient: Sendable {
   ///
   /// - Parameters:
   ///   - transport: The transport used to establish a communication channel with a server.
-  ///   - interceptorPipeline: A collection of ``ClientInterceptorPipelineOperation`` providing cross-cutting
+  ///   - interceptorPipeline: A collection of ``ConditionalInterceptor``s providing cross-cutting
   ///       functionality to each accepted RPC. Only applicable interceptors from the pipeline will be applied to each RPC.
   ///       The order in which interceptors are added reflects the order in which they are called.
   ///       The first interceptor added will be the first interceptor to intercept each request.
   ///       The last interceptor added will be the final interceptor to intercept each request before calling the appropriate handler.
   public init(
     transport: some ClientTransport,
-    interceptorPipeline: [ClientInterceptorPipelineOperation]
+    interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>]
   ) {
     self.transport = transport
     self.stateMachine = Mutex(StateMachine(interceptorPipeline: interceptorPipeline))
@@ -416,7 +416,7 @@ public func withGRPCClient<Result: Sendable>(
 ///
 /// - Parameters:
 ///   - transport: The transport used to establish a communication channel with a server.
-///   - interceptorPipeline: A collection of ``ClientInterceptorPipelineOperation`` providing cross-cutting
+///   - interceptorPipeline: A collection of ``ConditionalInterceptor``s providing cross-cutting
 ///       functionality to each accepted RPC. Only applicable interceptors from the pipeline will be applied to each RPC.
 ///       The order in which interceptors are added reflects the order in which they are called.
 ///       The first interceptor added will be the first interceptor to intercept each request.
@@ -428,7 +428,7 @@ public func withGRPCClient<Result: Sendable>(
 /// - Returns: The result of the `handleClient` closure.
 public func withGRPCClient<Result: Sendable>(
   transport: some ClientTransport,
-  interceptorPipeline: [ClientInterceptorPipelineOperation],
+  interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>],
   isolation: isolated (any Actor)? = #isolation,
   handleClient: (GRPCClient) async throws -> Result
 ) async throws -> Result {

+ 2 - 2
Sources/GRPCCore/GRPCServer.swift

@@ -171,7 +171,7 @@ public final class GRPCServer: Sendable {
   public convenience init(
     transport: any ServerTransport,
     services: [any RegistrableRPCService],
-    interceptorPipeline: [ServerInterceptorPipelineOperation]
+    interceptorPipeline: [ConditionalInterceptor<any ServerInterceptor>]
   ) {
     var router = RPCRouter()
     for service in services {
@@ -290,7 +290,7 @@ public func withGRPCServer<Result: Sendable>(
 public func withGRPCServer<Result: Sendable>(
   transport: any ServerTransport,
   services: [any RegistrableRPCService],
-  interceptorPipeline: [ServerInterceptorPipelineOperation],
+  interceptorPipeline: [ConditionalInterceptor<any ServerInterceptor>],
   isolation: isolated (any Actor)? = #isolation,
   handleServer: (GRPCServer) async throws -> Result
 ) async throws -> Result {

+ 7 - 13
Tests/GRPCCoreTests/Call/Client/ClientInterceptorPipelineOperationTests.swift → Tests/GRPCCoreTests/Call/ConditionalInterceptorTests.swift

@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 
+import GRPCCore
 import Testing
 
-@testable import GRPCCore
-
-@Suite("ClientInterceptorPipelineOperation")
-struct ClientInterceptorPipelineOperationTests {
+@Suite("ConditionalInterceptor")
+struct ConditionalInterceptorTests {
   @Test(
     "Applies to",
     arguments: [
@@ -38,24 +37,19 @@ struct ClientInterceptorPipelineOperationTests {
         [.barFoo],
         [.fooBar, .fooBaz, .barBaz]
       ),
-    ] as [(ClientInterceptorPipelineOperation.Subject, [MethodDescriptor], [MethodDescriptor])]
+    ] as [(ConditionalInterceptor<any Sendable>.Subject, [MethodDescriptor], [MethodDescriptor])]
   )
   func appliesTo(
-    operationSubject: ClientInterceptorPipelineOperation.Subject,
+    target: ConditionalInterceptor<any Sendable>.Subject,
     applicableMethods: [MethodDescriptor],
     notApplicableMethods: [MethodDescriptor]
   ) {
-    let operation = ClientInterceptorPipelineOperation.apply(
-      .requestCounter(.init()),
-      to: operationSubject
-    )
-
     for applicableMethod in applicableMethods {
-      #expect(operation.applies(to: applicableMethod))
+      #expect(target.applies(to: applicableMethod))
     }
 
     for notApplicableMethod in notApplicableMethods {
-      #expect(!operation.applies(to: notApplicableMethod))
+      #expect(!target.applies(to: notApplicableMethod))
     }
   }
 }

+ 0 - 68
Tests/GRPCCoreTests/Call/Server/ServerInterceptorPipelineOperation.swift

@@ -1,68 +0,0 @@
-/*
- * Copyright 2024, gRPC Authors All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import Testing
-
-@testable import GRPCCore
-
-@Suite("ServerInterceptorPipelineOperation")
-struct ServerInterceptorPipelineOperationTests {
-  @Test(
-    "Applies to",
-    arguments: [
-      (
-        .all,
-        [.fooBar, .fooBaz, .barFoo, .barBaz],
-        []
-      ),
-      (
-        .services([ServiceDescriptor(package: "pkg", service: "foo")]),
-        [.fooBar, .fooBaz],
-        [.barFoo, .barBaz]
-      ),
-      (
-        .methods([.barFoo]),
-        [.barFoo],
-        [.fooBar, .fooBaz, .barBaz]
-      ),
-    ] as [(ServerInterceptorPipelineOperation.Subject, [MethodDescriptor], [MethodDescriptor])]
-  )
-  func appliesTo(
-    operationSubject: ServerInterceptorPipelineOperation.Subject,
-    applicableMethods: [MethodDescriptor],
-    notApplicableMethods: [MethodDescriptor]
-  ) {
-    let operation = ServerInterceptorPipelineOperation.apply(
-      .requestCounter(.init()),
-      to: operationSubject
-    )
-
-    for applicableMethod in applicableMethods {
-      #expect(operation.applies(to: applicableMethod))
-    }
-
-    for notApplicableMethod in notApplicableMethods {
-      #expect(!operation.applies(to: notApplicableMethod))
-    }
-  }
-}
-
-extension MethodDescriptor {
-  fileprivate static let fooBar = Self(fullyQualifiedService: "pkg.foo", method: "bar")
-  fileprivate static let fooBaz = Self(fullyQualifiedService: "pkg.foo", method: "baz")
-  fileprivate static let barFoo = Self(fullyQualifiedService: "pkg.bar", method: "foo")
-  fileprivate static let barBaz = Self(fullyQualifiedService: "pkg.bar", method: "Baz")
-}

+ 2 - 2
Tests/GRPCCoreTests/GRPCClientTests.swift

@@ -22,7 +22,7 @@ import XCTest
 final class GRPCClientTests: XCTestCase {
   func withInProcessConnectedClient(
     services: [any RegistrableRPCService],
-    interceptorPipeline: [ClientInterceptorPipelineOperation] = [],
+    interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>] = [],
     _ body: (GRPCClient, GRPCServer) async throws -> Void
   ) async throws {
     let inProcess = InProcessTransport()
@@ -538,7 +538,7 @@ struct ClientTests {
 
   func withInProcessConnectedClient(
     services: [any RegistrableRPCService],
-    interceptorPipeline: [ClientInterceptorPipelineOperation] = [],
+    interceptorPipeline: [ConditionalInterceptor<any ClientInterceptor>] = [],
     _ body: (GRPCClient, GRPCServer) async throws -> Void
   ) async throws {
     let inProcess = InProcessTransport()

+ 2 - 2
Tests/GRPCCoreTests/GRPCServerTests.swift

@@ -22,7 +22,7 @@ import XCTest
 final class GRPCServerTests: XCTestCase {
   func withInProcessClientConnectedToServer(
     services: [any RegistrableRPCService],
-    interceptorPipeline: [ServerInterceptorPipelineOperation] = [],
+    interceptorPipeline: [ConditionalInterceptor<any ServerInterceptor>] = [],
     _ body: (InProcessTransport.Client, GRPCServer) async throws -> Void
   ) async throws {
     let inProcess = InProcessTransport()
@@ -553,7 +553,7 @@ struct ServerTests {
 
   func withInProcessClientConnectedToServer(
     services: [any RegistrableRPCService],
-    interceptorPipeline: [ServerInterceptorPipelineOperation] = [],
+    interceptorPipeline: [ConditionalInterceptor<any ServerInterceptor>] = [],
     _ body: (InProcessTransport.Client, GRPCServer) async throws -> Void
   ) async throws {
     let inProcess = InProcessTransport()