From e3c6df4478fb6c83ff72d59bb3a0340e885f4e3e Mon Sep 17 00:00:00 2001 From: Brad Fol Date: Fri, 20 Sep 2024 14:51:53 -0700 Subject: [PATCH] Detect and validate duplicate registrations within an Assembly We are able to detect duplicates from within a single module at parse/compile time, so it will be good to fail early and provide an error to the developer. --- Sources/KnitCodeGen/ConfigurationSet.swift | 69 +++++++ Sources/KnitCommand/GenCommand.swift | 3 + .../ConfigurationSetTests.swift | 177 ++++++++++++++++++ 3 files changed, 249 insertions(+) diff --git a/Sources/KnitCodeGen/ConfigurationSet.swift b/Sources/KnitCodeGen/ConfigurationSet.swift index 5219004..dfe9733 100644 --- a/Sources/KnitCodeGen/ConfigurationSet.swift +++ b/Sources/KnitCodeGen/ConfigurationSet.swift @@ -145,3 +145,72 @@ public extension ConfigurationSet { """ } + +// MARK: - Validation + +/// Validate that there are not duplicate registrations _within_ this ConfigurationSet +/// which represents a single module. +/// Note that this will not find duplicate registrations across modules. +extension ConfigurationSet { + + private struct Key: Hashable { + let service: String + let name: String? + let arguments: [String] + } + + public func validateNoDuplicateRegistrations() throws { + var registrationSetPerTargetResolver = [String: Set]() + + try assemblies + // Get all registrations across all assemblies + .forEach { assembly in + let targetResolver = assembly.targetResolver + + // First make sure there is a Set assigned for this assembly's TargetResolver + if registrationSetPerTargetResolver[targetResolver] == nil { + registrationSetPerTargetResolver[targetResolver] = Set() + } + + try assembly.registrations.forEach { registration in + let key = Key( + service: registration.service, + name: registration.name, + arguments: registration.arguments.map { $0.type } + ) + + guard let registrationSet = registrationSetPerTargetResolver[targetResolver], + registrationSet.contains(key) == false else { + throw ConfigurationSetParsingError.detectedDuplicateRegistration( + service: key.service, + name: key.name, + arguments: key.arguments + ) + } + + var set = registrationSetPerTargetResolver[targetResolver]! + set.insert(key) + registrationSetPerTargetResolver[targetResolver] = set + } + } + } + +} + +enum ConfigurationSetParsingError: LocalizedError { + + case detectedDuplicateRegistration(service: String, name: String?, arguments: [String]) + + var errorDescription: String? { + switch self { + case .detectedDuplicateRegistration(let service, let name, let arguments): + return """ + Detected a duplicated registration: + Service type: \(service) + Name (optional): \(name ?? "`nil`") + Arguments: \(arguments) + """ + } + } + +} diff --git a/Sources/KnitCommand/GenCommand.swift b/Sources/KnitCommand/GenCommand.swift index 97a79f9..dda9275 100644 --- a/Sources/KnitCommand/GenCommand.swift +++ b/Sources/KnitCommand/GenCommand.swift @@ -89,6 +89,9 @@ struct GenCommand: ParsableCommand { externalTestingAssemblies: expandedTestingPaths, moduleDependencies: dependencyModuleNames ) + + try parsedConfig.validateNoDuplicateRegistrations() + if let jsonDataOutputPath { let data = try JSONEncoder().encode(parsedConfig.allAssemblies) try data.write(to: URL(fileURLWithPath: jsonDataOutputPath)) diff --git a/Tests/KnitCodeGenTests/ConfigurationSetTests.swift b/Tests/KnitCodeGenTests/ConfigurationSetTests.swift index 6102ed8..4ff447c 100644 --- a/Tests/KnitCodeGenTests/ConfigurationSetTests.swift +++ b/Tests/KnitCodeGenTests/ConfigurationSetTests.swift @@ -253,6 +253,94 @@ final class ConfigurationSetTests: XCTestCase { } + func testValidateDuplicates() { + + var configSet = Factory.makeConfigSet( + duplicateService: "DuplicateService", + serviceName: nil, + serviceArguments: [] + ) + + XCTAssertThrowsError( + try configSet.validateNoDuplicateRegistrations(), + "Should throw error for duplicated registration", + { error in + if case let ConfigurationSetParsingError.detectedDuplicateRegistration(service: service, name: name, arguments: arguments) = error { + XCTAssertEqual(service, "DuplicateService") + XCTAssertNil(name) + XCTAssertEqual(arguments, []) + } else { + XCTFail("Incorrect error") + } + } + ) + + // Test with a service name + configSet = Factory.makeConfigSet( + duplicateService: "DuplicateNamedService", + serviceName: "aName", + serviceArguments: [] + ) + + XCTAssertThrowsError( + try configSet.validateNoDuplicateRegistrations(), + "Should throw error for duplicated registration", + { error in + if case let ConfigurationSetParsingError.detectedDuplicateRegistration(service: service, name: name, arguments: arguments) = error { + XCTAssertEqual(service, "DuplicateNamedService") + XCTAssertEqual(name, "aName") + XCTAssertEqual(arguments, []) + } else { + XCTFail("Incorrect error") + } + } + ) + + // Test with service argument + configSet = Factory.makeConfigSet( + duplicateService: "DuplicateServiceArguments", + serviceName: nil, + serviceArguments: ["Argument"] + ) + + XCTAssertThrowsError( + try configSet.validateNoDuplicateRegistrations(), + "Should throw error for duplicated registration", + { error in + if case let ConfigurationSetParsingError.detectedDuplicateRegistration(service: service, name: name, arguments: arguments) = error { + XCTAssertEqual(service, "DuplicateServiceArguments") + XCTAssertNil(name) + XCTAssertEqual(arguments, ["Argument"]) + } else { + XCTFail("Incorrect error") + } + } + ) + + // No duplicates + configSet = ConfigurationSet( + assemblies: [ + .init(assemblyName: "TestAssembly", moduleName: "TestModule", registrations: [Registration(service: "Service")], targetResolver: "TestResolver") + ], + externalTestingAssemblies: [], + moduleDependencies: [] + ) + XCTAssertNoThrow(try configSet.validateNoDuplicateRegistrations()) + } + + func testValidateDuplicates_multipleTargetResolvers() { + // Registrations should only be compared to other registrations for the same TargetResolver + // It is allowed to make the same registration on two different TargetResolvers + + let configSet = Factory.makeConfigSetAcrossTwoTargetResolvers( + duplicateService: "DuplicateService", + serviceName: nil, + serviceArguments: [] + ) + + XCTAssertNoThrow(try configSet.validateNoDuplicateRegistrations()) + } + } private enum Factory { @@ -297,4 +385,93 @@ private enum Factory { ], targetResolver: "Resolver" ) + + static func makeConfigSet( + duplicateService: String, + serviceName: String?, + serviceArguments: [String] + ) -> ConfigurationSet { + let config1 = Configuration( + assemblyName: "Assembly1", + moduleName: "Module1", + registrations: [ + Factory.makeRegistration( + duplicateService: duplicateService, + duplicateServiceName: serviceName, + duplicateArguments: serviceArguments + ) + ], + targetResolver: "TestResolver" + ) + let config2 = Configuration( + assemblyName: "Assembly2", + moduleName: "Module2", + registrations: [ + Factory.makeRegistration( + duplicateService: duplicateService, + duplicateServiceName: serviceName, + duplicateArguments: serviceArguments + ) + ], + targetResolver: "TestResolver" + ) + return ConfigurationSet( + assemblies: [ + config1, + config2, + ], externalTestingAssemblies: [], + moduleDependencies: [] + ) + } + + static func makeConfigSetAcrossTwoTargetResolvers( + duplicateService: String, + serviceName: String?, + serviceArguments: [String] + ) -> ConfigurationSet { + let config1 = Configuration( + assemblyName: "Assembly1", + moduleName: "Module1", + registrations: [ + Factory.makeRegistration( + duplicateService: duplicateService, + duplicateServiceName: serviceName, + duplicateArguments: serviceArguments + ) + ], + targetResolver: "TestResolver" + ) + let config2 = Configuration( + assemblyName: "Assembly2", + moduleName: "Module2", + registrations: [ + Factory.makeRegistration( + duplicateService: duplicateService, + duplicateServiceName: serviceName, + duplicateArguments: serviceArguments + ) + ], + targetResolver: "OtherTestResolver" + ) + return ConfigurationSet( + assemblies: [ + config1, + config2, + ], externalTestingAssemblies: [], + moduleDependencies: [] + ) + } + + static func makeRegistration( + duplicateService: String, + duplicateServiceName: String?, + duplicateArguments: [String] + ) -> Registration { + Registration( + service: duplicateService, + name: duplicateServiceName, + arguments: duplicateArguments.map { .init(type: $0) } + ) + } + }