Merge pull request #223 from cashapp/bradfol/fix-abstract-optional

Fix the RegistrationKey for OptionalAbstractRegistrations
cashapp · Dec 17, 2024 · c9e2c30 · c9e2c30
2 parents 6c24faa + 73da53b
commit c9e2c30
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git a/Sources/Knit/Module/Container+AbstractRegistration.swift b/Sources/Knit/Module/Container+AbstractRegistration.swift
@@ -59,7 +59,7 @@ internal struct RegistrationKey: Hashable, Equatable {
 internal protocol AbstractRegistration {
     associatedtype ServiceType
 
-    var serviceType: ServiceType.Type { get }
+    var serviceDescription: String { get }
     var file: String { get }
     var name: String? { get }
     var key: RegistrationKey { get }
@@ -78,7 +78,7 @@ extension AbstractRegistration {
     // Convert the key into an error
     var error: Container.AbstractRegistrationError {
         return Container.AbstractRegistrationError(
-            serviceType: "\(serviceType)",
+            serviceType: serviceDescription,
             file: file,
             name: name
         )
@@ -91,7 +91,7 @@ fileprivate struct RealAbstractRegistration<ServiceType>: AbstractRegistration {
     // Source file used for debugging. Not included in hash calculation or equality
     let file: String
 
-    var serviceType: ServiceType.Type { ServiceType.self }
+    var serviceDescription: String { String(describing: ServiceType.self) }
 
     let concurrency: ConcurrencyAttribute
 
@@ -116,25 +116,33 @@ fileprivate struct RealAbstractRegistration<ServiceType>: AbstractRegistration {
 }
 
 /// An abstract registration for an optional service
-fileprivate struct OptionalAbstractRegistration<ServiceType>: AbstractRegistration {
+/// The `UnwrappedServiceType` represents the inner type of the Optional service type for the registration.
+fileprivate struct OptionalAbstractRegistration<UnwrappedServiceType>: AbstractRegistration {
     let name: String?
     // Source file used for debugging. Not included in hash calculation or equality
     let file: String
 
-    var serviceType: ServiceType.Type { ServiceType.self }
+    /// The actual service type added for this registration (includes the Optional wrapper).
+    typealias ServiceType = Optional<UnwrappedServiceType>
+
+    var serviceDescription: String { String(describing: ServiceType.self) }
 
     let concurrency: ConcurrencyAttribute
 
     var key: RegistrationKey {
-        return .init(typeIdentifier: ObjectIdentifier(ServiceType.self), name: name, concurrency: concurrency)
+        return .init(
+            typeIdentifier: ObjectIdentifier(ServiceType.self),
+            name: name,
+            concurrency: concurrency
+        )
     }
 
     func registerPlaceholder(
         container: Container,
         errorFormatter: ModuleAssemblerErrorFormatter,
         dependencyTree: DependencyTree
     ) {
-        container.register(Optional<ServiceType>.self, name: name) { _ in
+        container.register(ServiceType.self, name: name) { _ in
             return nil
         }
     }

diff --git a/Tests/KnitTests/AbstractRegistrationTests.swift b/Tests/KnitTests/AbstractRegistrationTests.swift
@@ -12,13 +12,15 @@ final class AbstractRegistrationTests: XCTestCase {
         let abstractRegistrations = container.registerAbstractContainer()
         container.registerAbstract(String.self)
         container.registerAbstract(String.self, name: "test")
+        container.registerAbstract(Optional<Int>.self)
 
         XCTAssertThrowsError(try abstractRegistrations.validate()) { error in
             XCTAssertEqual(
                 error.localizedDescription,
                 """
                 Unsatisfied abstract registration. Service: String, File: KnitTests/AbstractRegistrationTests.swift
                 Unsatisfied abstract registration. Service: String, File: KnitTests/AbstractRegistrationTests.swift, Name: test
+                Unsatisfied abstract registration. Service: Optional<Int>, File: KnitTests/AbstractRegistrationTests.swift
                 """
             )
         }
@@ -29,8 +31,14 @@ final class AbstractRegistrationTests: XCTestCase {
         let abstractRegistrations = container.registerAbstractContainer()
         container.registerAbstract(String.self)
         container.register(String.self) { _ in "Test" }
+
+        // Abstract registrations of Optional types are handled differently so test that as well
+        container.registerAbstract(Optional<Int>.self)
+        container.register(Optional<Int>.self) { _ in 1 }
+
         XCTAssertNoThrow(try abstractRegistrations.validate())
         XCTAssertEqual(container.resolve(String.self), "Test")
+        XCTAssertEqual(container.resolve(Optional<Int>.self), 1)
     }
 
     func testNamedRegistrations() {