Fix AbstractRegistration placeholder bug

Registered placeholders should match the service type and name of the AbstractRegistration, otherwise there are incorrect placeholders created.
cashapp · Nov 12, 2024 · 1f9497a · 1f9497a
1 parent 98fa6a0
commit 1f9497a
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 deletions.
diff --git a/Sources/Knit/Module/Container+AbstractRegistration.swift b/Sources/Knit/Module/Container+AbstractRegistration.swift
@@ -8,7 +8,7 @@ extension Container {
 
     /// Register that a service is expected to exist but no implementation is currently available
     /// The concrete implementation must be registered or the dependency graph is considered invalid
-    /// We don't currently support abstract registrations with arguments
+    /// - NOTE: We don't currently support abstract registrations with arguments
     public func registerAbstract<Service>(
         _ serviceType: Service.Type,
         name: String? = nil,
@@ -47,7 +47,7 @@ internal protocol AbstractRegistration {
     var name: String? { get }
     var key: RegistrationKey { get }
 
-    /// Register a placeholder registration to fill the unfullfilled abstract registration
+    /// Register a placeholder registration to fill the unfulfilled abstract registration
     /// This placeholder cannot be resolved
     func registerPlaceholder(
         container: Container,
@@ -85,7 +85,7 @@ fileprivate struct RealAbstractRegistration<ServiceType>: AbstractRegistration {
         dependencyTree: DependencyTree
     ) {
         let message = errorFormatter.format(error: self.error, dependencyTree: dependencyTree)
-        container.register(ServiceType.self) { _ in
+        container.register(ServiceType.self, name: name) { _ in
             fatalError("Attempt to resolve unfulfilled abstract registration.\n\(message)")
         }
     }

diff --git a/Tests/KnitTests/ModuleAssemblerTests.swift b/Tests/KnitTests/ModuleAssemblerTests.swift
@@ -3,6 +3,7 @@
 //
 
 @testable import Knit
+@testable import Swinject
 import XCTest
 
 final class ModuleAssemblerTests: XCTestCase {
@@ -59,19 +60,49 @@ final class ModuleAssemblerTests: XCTestCase {
                     XCTFail("Incorrect error type \(error)")
                     return
                 }
-                XCTAssertEqual(abstractRegistrationErrors.errors.count, 1)
+                XCTAssertEqual(abstractRegistrationErrors.errors.count, 2)
+
+                // Abstract registration one
                 XCTAssertEqual(abstractRegistrationErrors.errors.first?.serviceType, "Assembly5Protocol")
+                XCTAssertNil(abstractRegistrationErrors.errors.first?.name)
+
+                // Abstract registration two
+                XCTAssertEqual(abstractRegistrationErrors.errors.last?.serviceType, "Assembly5Protocol")
+                XCTAssertEqual(abstractRegistrationErrors.errors.last?.name, "testName")
             }
         )
     }
 
     @MainActor
     func test_abstractAssemblyPlaceholders() throws {
         // No error is thrown as the graph is using abstract placeholders
-        _ = try ModuleAssembler(
+        let assembler = try ModuleAssembler(
             _modules: [ Assembly4() ],
             overrideBehavior: .init(allowDefaultOverrides: true, useAbstractPlaceholders: true)
         )
+
+        var services = assembler._container.services.filter { (key, value) in
+            // Filter out registrations for `AbstractRegistrationContainer` and `DependencyTree`
+            return key.serviceType != Container.AbstractRegistrationContainer.self &&
+            key.serviceType != DependencyTree.self
+        }
+        XCTAssertEqual(services.count, 2)
+
+        XCTAssertNotNil(services.removeValue(forKey: .init(
+            serviceType: Assembly5Protocol.self,
+            argumentsType: (Resolver).self,
+            name: nil
+        )), "Service entry for Assembly5Protocol without name should exist")
+        XCTAssertEqual(services.count, 1)
+
+        XCTAssertNotNil(services.removeValue(forKey: .init(
+            serviceType: Assembly5Protocol.self,
+            argumentsType: (Resolver).self,
+            name: "testName"
+        )), "Service entry for Assembly5Protocol with name should exist")
+
+        // No more registrations left
+        XCTAssertEqual(services.count, 0)
     }
 
 }
@@ -150,6 +181,7 @@ private struct AbstractAssembly5: AbstractAssembly {
 
     func assemble(container: Swinject.Container) {
         container.registerAbstract(Assembly5Protocol.self)
+        container.registerAbstract(Assembly5Protocol.self, name: "testName")
     }
 
 }