Use ServiceDescriptor within MethodDescriptor

glbrntt · glbrntt · commit ba8122e64ed1 · 2024-11-21T13:21:20.000Z
Motivation:

MethodDescriptor uses a string to represent the fully qualified name of
the service the method belongs to. ServiceDescriptor also represents a
fully qualified name of a service. It'd make more sense for
MethodDescriptor to use a ServiceDescriptor instead of a string to
describe the service.

Modifications:

- ServiceDescriptor now stores the fully qualified service name (as this
  is generally more useful) and computes the package and unqualified
  service name.
- ServiceDescriptor can now be created from the fully qualified name or
  from a separate package and service name.
- MethodDescriptor now holds a ServiceDescriptor instead of a String for
  the service

Result:

More coherent API
diff --git a/Sources/GRPCCore/Call/Client/ClientInterceptorPipelineOperation.swift b/Sources/GRPCCore/Call/Client/ClientInterceptorPipelineOperation.swift
@@ -61,7 +61,7 @@ public struct ClientInterceptorPipelineOperation: Sendable {
         return true
 
       case .services(let services):
-        return services.map({ $0.fullyQualifiedService }).contains(descriptor.service)
+        return services.contains(descriptor.service)
 
       case .methods(let methods):
         return methods.contains(descriptor)
diff --git a/Sources/GRPCCore/Call/Server/ServerInterceptorPipelineOperation.swift b/Sources/GRPCCore/Call/Server/ServerInterceptorPipelineOperation.swift
@@ -61,7 +61,7 @@ public struct ServerInterceptorPipelineOperation: Sendable {
         return true
 
       case .services(let services):
-        return services.map({ $0.fullyQualifiedService }).contains(descriptor.service)
+        return services.contains(descriptor.service)
 
       case .methods(let methods):
         return methods.contains(descriptor)
diff --git a/Sources/GRPCCore/Internal/MethodConfigs.swift b/Sources/GRPCCore/Internal/MethodConfigs.swift
@@ -51,7 +51,10 @@ package struct MethodConfigs: Sendable, Hashable {
   ///  - descriptor: The ``MethodDescriptor`` for which to get or set a ``MethodConfig``.
   package subscript(_ descriptor: MethodDescriptor) -> MethodConfig? {
     get {
-      var name = MethodConfig.Name(service: descriptor.service, method: descriptor.method)
+      var name = MethodConfig.Name(
+        service: descriptor.service.fullyQualifiedService,
+        method: descriptor.method
+      )
 
       if let configuration = self.elements[name] {
         return configuration
@@ -70,7 +73,10 @@ package struct MethodConfigs: Sendable, Hashable {
     }
 
     set {
-      let name = MethodConfig.Name(service: descriptor.service, method: descriptor.method)
+      let name = MethodConfig.Name(
+        service: descriptor.service.fullyQualifiedService,
+        method: descriptor.method
+      )
       self.elements[name] = newValue
     }
   }
diff --git a/Sources/GRPCCore/MethodDescriptor.swift b/Sources/GRPCCore/MethodDescriptor.swift
@@ -16,11 +16,8 @@
 
 /// A description of a method on a service.
 public struct MethodDescriptor: Sendable, Hashable {
-  /// The name of the service, including the package name.
-  ///
-  /// For example, the name of the "Greeter" service in "helloworld" package
-  /// is "helloworld.Greeter".
-  public var service: String
+  /// A description of the service, including its package name.
+  public var service: ServiceDescriptor
 
   /// The name of the method in the service, excluding the service name.
   public var method: String
@@ -39,8 +36,30 @@ public struct MethodDescriptor: Sendable, Hashable {
   ///   - service: The name of the service, including the package name. For example,
   ///       "helloworld.Greeter".
   ///   - method: The name of the method. For example, "SayHello".
-  public init(service: String, method: String) {
+  public init(service: ServiceDescriptor, method: String) {
     self.service = service
     self.method = method
   }
+
+  /// Creates a new method descriptor.
+  ///
+  /// - Parameters:
+  ///   - fullyQualifiedService: The fully qualified name of the service, including the package
+  ///       name. For example, "helloworld.Greeter".
+  ///   - method: The name of the method. For example, "SayHello".
+  public init(fullyQualifiedService: String, method: String) {
+    self.service = ServiceDescriptor(fullyQualifiedService: fullyQualifiedService)
+    self.method = method
+  }
+
+  @available(*, deprecated, renamed: "init(fullyQualifiedService:method:)")
+  /// Creates a new method descriptor.
+  ///
+  /// - Parameters:
+  ///   - service: The fully qualified name of the service, including the package
+  ///       name. For example, "helloworld.Greeter".
+  ///   - method: The name of the method. For example, "SayHello".
+  public init(service: String, method: String) {
+    self.init(fullyQualifiedService: service, method: method)
+  }
 }
diff --git a/Sources/GRPCCore/ServiceDescriptor.swift b/Sources/GRPCCore/ServiceDescriptor.swift
@@ -18,28 +18,44 @@
 public struct ServiceDescriptor: Sendable, Hashable {
   /// The name of the package the service belongs to. For example, "helloworld".
   /// An empty string means that the service does not belong to any package.
-  public var package: String
+  public var package: String {
+    if let index = self.fullyQualifiedService.utf8.lastIndex(of: UInt8(ascii: ".")) {
+      return String(self.fullyQualifiedService[..<index])
+    } else {
+      return ""
+    }
+  }
 
   /// The name of the service. For example, "Greeter".
-  public var service: String
+  public var service: String {
+    if var index = self.fullyQualifiedService.utf8.lastIndex(of: UInt8(ascii: ".")) {
+      self.fullyQualifiedService.utf8.formIndex(after: &index)
+      return String(self.fullyQualifiedService[index...])
+    } else {
+      return self.fullyQualifiedService
+    }
+  }
 
   /// The fully qualified service name in the format:
   /// - "package.service": if a package name is specified. For example, "helloworld.Greeter".
   /// - "service": if a package name is not specified. For example, "Greeter".
-  public var fullyQualifiedService: String {
-    if self.package.isEmpty {
-      return self.service
-    }
+  public var fullyQualifiedService: String
 
-    return "\(self.package).\(self.service)"
+  /// Create a new descriptor from the fully qualified service name.
+  /// - Parameter fullyQualifiedService: The fully qualified service name.
+  public init(fullyQualifiedService: String) {
+    self.fullyQualifiedService = fullyQualifiedService
   }
 
   /// - Parameters:
   ///   - package: The name of the package the service belongs to. For example, "helloworld".
   ///   An empty string means that the service does not belong to any package.
   ///   - service: The name of the service. For example, "Greeter".
   public init(package: String, service: String) {
-    self.package = package
-    self.service = service
+    if package.isEmpty {
+      self.fullyQualifiedService = service
+    } else {
+      self.fullyQualifiedService = package + "." + service
+    }
   }
 }
diff --git a/Tests/GRPCCoreTests/Call/Client/ClientInterceptorPipelineOperationTests.swift b/Tests/GRPCCoreTests/Call/Client/ClientInterceptorPipelineOperationTests.swift
@@ -61,8 +61,8 @@ struct ClientInterceptorPipelineOperationTests {
 }
 
 extension MethodDescriptor {
-  fileprivate static let fooBar = Self(service: "pkg.foo", method: "bar")
-  fileprivate static let fooBaz = Self(service: "pkg.foo", method: "baz")
-  fileprivate static let barFoo = Self(service: "pkg.bar", method: "foo")
-  fileprivate static let barBaz = Self(service: "pkg.bar", method: "Baz")
+  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")
 }
diff --git a/Tests/GRPCCoreTests/Call/Client/Internal/ClientRPCExecutorTestSupport/ClientRPCExecutorTestHarness.swift b/Tests/GRPCCoreTests/Call/Client/Internal/ClientRPCExecutorTestSupport/ClientRPCExecutorTestHarness.swift
@@ -136,7 +136,7 @@ struct ClientRPCExecutorTestHarness {
       // Execute the request.
       try await ClientRPCExecutor.execute(
         request: request,
-        method: MethodDescriptor(service: "foo", method: "bar"),
+        method: MethodDescriptor(fullyQualifiedService: "foo", method: "bar"),
         options: options,
         serializer: serializer,
         deserializer: deserializer,
diff --git a/Tests/GRPCCoreTests/Call/Server/Internal/ServerRPCExecutorTestSupport/ServerRPCExecutorTestHarness.swift b/Tests/GRPCCoreTests/Call/Server/Internal/ServerRPCExecutorTestSupport/ServerRPCExecutorTestHarness.swift
@@ -101,7 +101,7 @@ struct ServerRPCExecutorTestHarness {
       group.addTask {
         await withServerContextRPCCancellationHandle { cancellation in
           let context = ServerContext(
-            descriptor: MethodDescriptor(service: "foo", method: "bar"),
+            descriptor: MethodDescriptor(fullyQualifiedService: "foo", method: "bar"),
             cancellation: cancellation
           )
 
diff --git a/Tests/GRPCCoreTests/Call/Server/RPCRouterTests.swift b/Tests/GRPCCoreTests/Call/Server/RPCRouterTests.swift
@@ -22,13 +22,17 @@ final class RPCRouterTests: XCTestCase {
     var router = RPCRouter()
     XCTAssertEqual(router.count, 0)
     XCTAssertEqual(router.methods, [])
-    XCTAssertFalse(router.hasHandler(forMethod: MethodDescriptor(service: "foo", method: "bar")))
-    XCTAssertFalse(router.removeHandler(forMethod: MethodDescriptor(service: "foo", method: "bar")))
+    XCTAssertFalse(
+      router.hasHandler(forMethod: MethodDescriptor(fullyQualifiedService: "foo", method: "bar"))
+    )
+    XCTAssertFalse(
+      router.removeHandler(forMethod: MethodDescriptor(fullyQualifiedService: "foo", method: "bar"))
+    )
   }
 
   func testRegisterMethod() async throws {
     var router = RPCRouter()
-    let method = MethodDescriptor(service: "foo", method: "bar")
+    let method = MethodDescriptor(fullyQualifiedService: "foo", method: "bar")
     router.registerHandler(
       forMethod: method,
       deserializer: IdentityDeserializer(),
@@ -44,7 +48,7 @@ final class RPCRouterTests: XCTestCase {
 
   func testRemoveMethod() async throws {
     var router = RPCRouter()
-    let method = MethodDescriptor(service: "foo", method: "bar")
+    let method = MethodDescriptor(fullyQualifiedService: "foo", method: "bar")
     router.registerHandler(
       forMethod: method,
       deserializer: IdentityDeserializer(),
diff --git a/Tests/GRPCCoreTests/Call/Server/ServerInterceptorPipelineOperation.swift b/Tests/GRPCCoreTests/Call/Server/ServerInterceptorPipelineOperation.swift
@@ -61,8 +61,8 @@ struct ServerInterceptorPipelineOperationTests {
 }
 
 extension MethodDescriptor {
-  fileprivate static let fooBar = Self(service: "pkg.foo", method: "bar")
-  fileprivate static let fooBaz = Self(service: "pkg.foo", method: "baz")
-  fileprivate static let barFoo = Self(service: "pkg.bar", method: "foo")
-  fileprivate static let barBaz = Self(service: "pkg.bar", method: "Baz")
+  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")
 }
diff --git a/Tests/GRPCCoreTests/GRPCClientTests.swift b/Tests/GRPCCoreTests/GRPCClientTests.swift
@@ -26,8 +26,8 @@ final class GRPCClientTests: XCTestCase {
     _ body: (GRPCClient, GRPCServer) async throws -> Void
   ) async throws {
     let inProcess = InProcessTransport()
-    let client = GRPCClient(transport: inProcess.client, interceptorPipeline: interceptorPipeline)
-    let server = GRPCServer(transport: inProcess.server, services: services)
+    _ = GRPCClient(transport: inProcess.client, interceptorPipeline: interceptorPipeline)
+    _ = GRPCServer(transport: inProcess.server, services: services)
 
     try await withGRPCServer(
       transport: inProcess.server,
@@ -137,7 +137,7 @@ final class GRPCClientTests: XCTestCase {
     try await self.withInProcessConnectedClient(services: [BinaryEcho()]) { client, _ in
       try await client.unary(
         request: .init(message: [3, 1, 4, 1, 5]),
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         serializer: IdentitySerializer(),
         deserializer: IdentityDeserializer(),
         options: .defaults
@@ -157,7 +157,7 @@ final class GRPCClientTests: XCTestCase {
             try await writer.write([byte])
           }
         }),
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         serializer: IdentitySerializer(),
         deserializer: IdentityDeserializer(),
         options: .defaults
@@ -173,7 +173,7 @@ final class GRPCClientTests: XCTestCase {
     try await self.withInProcessConnectedClient(services: [BinaryEcho()]) { client, _ in
       try await client.serverStreaming(
         request: .init(message: [3, 1, 4, 1, 5]),
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         serializer: IdentitySerializer(),
         deserializer: IdentityDeserializer(),
         options: .defaults
@@ -193,7 +193,7 @@ final class GRPCClientTests: XCTestCase {
             try await writer.write([byte])
           }
         }),
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         serializer: IdentitySerializer(),
         deserializer: IdentityDeserializer(),
         options: .defaults
diff --git a/Tests/GRPCCoreTests/GRPCServerTests.swift b/Tests/GRPCCoreTests/GRPCServerTests.swift
@@ -164,7 +164,7 @@ final class GRPCServerTests: XCTestCase {
   func testUnimplementedMethod() async throws {
     try await self.withInProcessClientConnectedToServer(services: [BinaryEcho()]) { client, _ in
       try await client.withStream(
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         options: .defaults
       ) { stream in
         try await stream.outbound.write(.metadata([:]))
@@ -248,7 +248,7 @@ final class GRPCServerTests: XCTestCase {
       interceptorPipeline: [.apply(.requestCounter(counter), to: .all)]
     ) { client, _ in
       try await client.withStream(
-        descriptor: MethodDescriptor(service: "not", method: "implemented"),
+        descriptor: MethodDescriptor(fullyQualifiedService: "not", method: "implemented"),
         options: .defaults
       ) { stream in
         try await stream.outbound.write(.metadata([:]))
diff --git a/Tests/GRPCCoreTests/Internal/MethodConfigsTests.swift b/Tests/GRPCCoreTests/Internal/MethodConfigsTests.swift
@@ -26,7 +26,7 @@ final class MethodConfigsTests: XCTestCase {
     let defaultConfiguration = MethodConfig(names: [], executionPolicy: .hedge(policy))
     var configurations = MethodConfigs()
     configurations.setDefaultConfig(defaultConfiguration)
-    let descriptor = MethodDescriptor(service: "test", method: "first")
+    let descriptor = MethodDescriptor(fullyQualifiedService: "test", method: "first")
     let retryPolicy = RetryPolicy(
       maxAttempts: 10,
       initialBackoff: .seconds(1),
@@ -49,7 +49,7 @@ final class MethodConfigsTests: XCTestCase {
     let defaultConfiguration = MethodConfig(names: [], executionPolicy: .hedge(policy))
     var configurations = MethodConfigs()
     configurations.setDefaultConfig(defaultConfiguration)
-    let firstDescriptor = MethodDescriptor(service: "test", method: "")
+    let firstDescriptor = MethodDescriptor(fullyQualifiedService: "test", method: "")
     let retryPolicy = RetryPolicy(
       maxAttempts: 10,
       initialBackoff: .seconds(1),
@@ -60,7 +60,7 @@ final class MethodConfigsTests: XCTestCase {
     let overrideConfiguration = MethodConfig(names: [], executionPolicy: .retry(retryPolicy))
     configurations[firstDescriptor] = overrideConfiguration
 
-    let secondDescriptor = MethodDescriptor(service: "test", method: "second")
+    let secondDescriptor = MethodDescriptor(fullyQualifiedService: "test", method: "second")
     XCTAssertEqual(configurations[secondDescriptor], overrideConfiguration)
   }
 
@@ -73,7 +73,7 @@ final class MethodConfigsTests: XCTestCase {
     let defaultConfiguration = MethodConfig(names: [], executionPolicy: .hedge(policy))
     var configurations = MethodConfigs()
     configurations.setDefaultConfig(defaultConfiguration)
-    let firstDescriptor = MethodDescriptor(service: "test1", method: "first")
+    let firstDescriptor = MethodDescriptor(fullyQualifiedService: "test1", method: "first")
     let retryPolicy = RetryPolicy(
       maxAttempts: 10,
       initialBackoff: .seconds(1),
@@ -84,7 +84,7 @@ final class MethodConfigsTests: XCTestCase {
     let overrideConfiguration = MethodConfig(names: [], executionPolicy: .retry(retryPolicy))
     configurations[firstDescriptor] = overrideConfiguration
 
-    let secondDescriptor = MethodDescriptor(service: "test2", method: "second")
+    let secondDescriptor = MethodDescriptor(fullyQualifiedService: "test2", method: "second")
     XCTAssertEqual(configurations[secondDescriptor], defaultConfiguration)
   }
 }
diff --git a/Tests/GRPCCoreTests/MethodDescriptorTests.swift b/Tests/GRPCCoreTests/MethodDescriptorTests.swift
@@ -14,13 +14,15 @@
  * limitations under the License.
  */
 import GRPCCore
-import XCTest
+import Testing
 
-final class MethodDescriptorTests: XCTestCase {
+@Suite
+struct MethodDescriptorTests {
+  @Test("Fully qualified name")
   func testFullyQualifiedName() {
-    let descriptor = MethodDescriptor(service: "foo.bar", method: "Baz")
-    XCTAssertEqual(descriptor.service, "foo.bar")
-    XCTAssertEqual(descriptor.method, "Baz")
-    XCTAssertEqual(descriptor.fullyQualifiedMethod, "foo.bar/Baz")
+    let descriptor = MethodDescriptor(fullyQualifiedService: "foo.bar", method: "Baz")
+    #expect(descriptor.service == ServiceDescriptor(fullyQualifiedService: "foo.bar"))
+    #expect(descriptor.method == "Baz")
+    #expect(descriptor.fullyQualifiedMethod == "foo.bar/Baz")
   }
 }
diff --git a/Tests/GRPCCoreTests/ServiceDescriptorTests.swift b/Tests/GRPCCoreTests/ServiceDescriptorTests.swift
@@ -14,13 +14,25 @@
  * limitations under the License.
  */
 import GRPCCore
-import XCTest
+import Testing
 
-final class ServiceDescriptorTests: XCTestCase {
-  func testFullyQualifiedName() {
-    let descriptor = ServiceDescriptor(package: "foo.bar", service: "Baz")
-    XCTAssertEqual(descriptor.package, "foo.bar")
-    XCTAssertEqual(descriptor.service, "Baz")
-    XCTAssertEqual(descriptor.fullyQualifiedService, "foo.bar.Baz")
+@Suite
+struct ServiceDescriptorTests {
+  @Test(
+    "Decompose fully qualified service name",
+    arguments: [
+      ("foo.bar.baz", "foo.bar", "baz"),
+      ("foo.bar", "foo", "bar"),
+      ("foo", "", "foo"),
+      ("..", ".", ""),
+      (".", "", ""),
+      ("", "", ""),
+    ]
+  )
+  func packageAndService(fullyQualified: String, package: String, service: String) {
+    let descriptor = ServiceDescriptor(fullyQualifiedService: fullyQualified)
+    #expect(descriptor.fullyQualifiedService == fullyQualified)
+    #expect(descriptor.package == package)
+    #expect(descriptor.service == service)
   }
 }
diff --git a/Tests/GRPCCoreTests/Test Utilities/Services/BinaryEcho.swift b/Tests/GRPCCoreTests/Test Utilities/Services/BinaryEcho.swift
diff --git a/Tests/GRPCCoreTests/Test Utilities/Services/HelloWorld.swift b/Tests/GRPCCoreTests/Test Utilities/Services/HelloWorld.swift
diff --git a/Tests/GRPCInProcessTransportTests/InProcessClientTransportTests.swift b/Tests/GRPCInProcessTransportTests/InProcessClientTransportTests.swift
diff --git a/Tests/GRPCInProcessTransportTests/InProcessServerTransportTests.swift b/Tests/GRPCInProcessTransportTests/InProcessServerTransportTests.swift
diff --git a/Tests/GRPCInProcessTransportTests/InProcessTransportTests.swift b/Tests/GRPCInProcessTransportTests/InProcessTransportTests.swift

Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,8 @@ struct ClientInterceptorPipelineOperationTests {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`extension MethodDescriptor {`
`64`		`- fileprivate static let fooBar = Self(service: "pkg.foo", method: "bar")`
`65`		`- fileprivate static let fooBaz = Self(service: "pkg.foo", method: "baz")`
`66`		`- fileprivate static let barFoo = Self(service: "pkg.bar", method: "foo")`
`67`		`- fileprivate static let barBaz = Self(service: "pkg.bar", method: "Baz")`
	`64`	`+ fileprivate static let fooBar = Self(fullyQualifiedService: "pkg.foo", method: "bar")`
	`65`	`+ fileprivate static let fooBaz = Self(fullyQualifiedService: "pkg.foo", method: "baz")`
	`66`	`+ fileprivate static let barFoo = Self(fullyQualifiedService: "pkg.bar", method: "foo")`
	`67`	`+ fileprivate static let barBaz = Self(fullyQualifiedService: "pkg.bar", method: "Baz")`
`68`	`68`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ struct ServerRPCExecutorTestHarness {`
`101`	`101`	`group.addTask {`
`102`	`102`	`await withServerContextRPCCancellationHandle { cancellation in`
`103`	`103`	`let context = ServerContext(`
`104`		`- descriptor: MethodDescriptor(service: "foo", method: "bar"),`
	`104`	`+ descriptor: MethodDescriptor(fullyQualifiedService: "foo", method: "bar"),`
`105`	`105`	`cancellation: cancellation`
`106`	`106`	`)`
`107`	`107`
Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,8 @@ struct ServerInterceptorPipelineOperationTests {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`extension MethodDescriptor {`
`64`		`- fileprivate static let fooBar = Self(service: "pkg.foo", method: "bar")`
`65`		`- fileprivate static let fooBaz = Self(service: "pkg.foo", method: "baz")`
`66`		`- fileprivate static let barFoo = Self(service: "pkg.bar", method: "foo")`
`67`		`- fileprivate static let barBaz = Self(service: "pkg.bar", method: "Baz")`
	`64`	`+ fileprivate static let fooBar = Self(fullyQualifiedService: "pkg.foo", method: "bar")`
	`65`	`+ fileprivate static let fooBaz = Self(fullyQualifiedService: "pkg.foo", method: "baz")`
	`66`	`+ fileprivate static let barFoo = Self(fullyQualifiedService: "pkg.bar", method: "foo")`
	`67`	`+ fileprivate static let barBaz = Self(fullyQualifiedService: "pkg.bar", method: "Baz")`
`68`	`68`	`}`