Skip to content

Commit

Permalink
Merge pull request #215 from cashapp/bradfol/placeholder-name
Browse files Browse the repository at this point in the history
Fix AbstractRegistration placeholder bug
  • Loading branch information
bradfol authored Nov 12, 2024
2 parents 98fa6a0 + 1f9497a commit 66d9194
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
6 changes: 3 additions & 3 deletions Sources/Knit/Module/Container+AbstractRegistration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)")
}
}
Expand Down
36 changes: 34 additions & 2 deletions Tests/KnitTests/ModuleAssemblerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//

@testable import Knit
@testable import Swinject
import XCTest

final class ModuleAssemblerTests: XCTestCase {
Expand Down Expand Up @@ -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)
}

}
Expand Down Expand Up @@ -150,6 +181,7 @@ private struct AbstractAssembly5: AbstractAssembly {

func assemble(container: Swinject.Container) {
container.registerAbstract(Assembly5Protocol.self)
container.registerAbstract(Assembly5Protocol.self, name: "testName")
}

}
Expand Down

0 comments on commit 66d9194

Please sign in to comment.