From 823edbc01556d7c1d37ddaf27c12fdf1848f238f Mon Sep 17 00:00:00 2001 From: Andrey Konoplyankin Date: Tue, 30 Mar 2021 11:39:40 +0300 Subject: [PATCH 1/5] Added duplicate singleton check --- Sources/EasyDi.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/EasyDi.swift b/Sources/EasyDi.swift index 03dc89d..47bfed9 100644 --- a/Sources/EasyDi.swift +++ b/Sources/EasyDi.swift @@ -417,8 +417,12 @@ open class Assembly: AssemblyInternal { } // And save singletons - if context.singletons[key] == nil, scope == .lazySingleton { - context.singletons[key] = result + if scope == .lazySingleton { + if context.singletons[key] == nil { + context.singletons[key] = result + } else { + fatalError("Singleton already exist, inspect your dependencies graph") + } } if context.weakSingletons[key] == nil, scope == .weakSingleton { From f8063d6a7e7a9b541eee3f1f26ca1fc661e789dd Mon Sep 17 00:00:00 2001 From: Andrey Konoplyankin Date: Tue, 30 Mar 2021 12:17:08 +0300 Subject: [PATCH 2/5] Also needs check pointers --- Sources/EasyDi.swift | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Sources/EasyDi.swift b/Sources/EasyDi.swift index 47bfed9..c0189bc 100644 --- a/Sources/EasyDi.swift +++ b/Sources/EasyDi.swift @@ -421,7 +421,13 @@ open class Assembly: AssemblyInternal { if context.singletons[key] == nil { context.singletons[key] = result } else { - fatalError("Singleton already exist, inspect your dependencies graph") + var current = context.singletons[key] as! ObjectType + let currentPointer = withUnsafePointer(to: ¤t, { unsafeBitCast($0.pointee, to: Int.self) }) + let resultPointer = withUnsafePointer(to: &result, { unsafeBitCast($0.pointee, to: Int.self) }) + + if currentPointer != resultPointer { + fatalError("Singleton already exist, inspect your dependencies graph") + } } } From 74953047e6f9f758eb8850d1a5a9006276fc0e42 Mon Sep 17 00:00:00 2001 From: Andrey Konoplyankin Date: Tue, 30 Mar 2021 12:19:22 +0300 Subject: [PATCH 3/5] Add Test_SingletonDuplication --- EasyDi.xcodeproj/project.pbxproj | 4 ++ Sources/EasyDi.swift | 15 +++--- Tests/Test_SingletonDuplication.swift | 67 +++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 Tests/Test_SingletonDuplication.swift diff --git a/EasyDi.xcodeproj/project.pbxproj b/EasyDi.xcodeproj/project.pbxproj index 509f0e9..2507234 100644 --- a/EasyDi.xcodeproj/project.pbxproj +++ b/EasyDi.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 283175422612173700C09104 /* Test_SingletonDuplication.swift in Sources */ = {isa = PBXBuildFile; fileRef = 283175412612173700C09104 /* Test_SingletonDuplication.swift */; }; 5C88753F236FD22500019260 /* Test_CrossAssemblyInjections_WeakSingletonCycle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C88753E236FD22500019260 /* Test_CrossAssemblyInjections_WeakSingletonCycle.swift */; }; 5C887540236FD22500019260 /* Test_CrossAssemblyInjections_WeakSingletonCycle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C88753E236FD22500019260 /* Test_CrossAssemblyInjections_WeakSingletonCycle.swift */; }; 8BEE13521F9A27C800A02331 /* EasyDi.h in Headers */ = {isa = PBXBuildFile; fileRef = 8BEE13501F9A27C800A02331 /* EasyDi.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -54,6 +55,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 283175412612173700C09104 /* Test_SingletonDuplication.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Test_SingletonDuplication.swift; sourceTree = ""; }; 5C88753E236FD22500019260 /* Test_CrossAssemblyInjections_WeakSingletonCycle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Test_CrossAssemblyInjections_WeakSingletonCycle.swift; sourceTree = ""; }; 8BEE13501F9A27C800A02331 /* EasyDi.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = EasyDi.h; sourceTree = ""; }; 8BEE13511F9A27C800A02331 /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; @@ -144,6 +146,7 @@ C3614B4A1F1C8B5F00B1F4A1 /* Test_Scope.swift */, C3614B4B1F1C8B5F00B1F4A1 /* Test_StructsInjection.swift */, A5ABB84221A5522400C96320 /* Test_Threadsafety.swift */, + 283175412612173700C09104 /* Test_SingletonDuplication.swift */, ); path = Tests; sourceTree = ""; @@ -367,6 +370,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 283175422612173700C09104 /* Test_SingletonDuplication.swift in Sources */, C3614B571F1C8B6800B1F4A1 /* Test_Patches.swift in Sources */, C3614B551F1C8B6800B1F4A1 /* Test_CrossAssemblyInjections.swift in Sources */, D2B16C972123116500CF69E8 /* Test_ImplicitlyUnwrappedOptional.swift in Sources */, diff --git a/Sources/EasyDi.swift b/Sources/EasyDi.swift index c0189bc..8444461 100644 --- a/Sources/EasyDi.swift +++ b/Sources/EasyDi.swift @@ -24,6 +24,9 @@ struct WeakSingletonWrapper { weak var instance: AnyObject? } +// For handle fatalError calls in tests +var fatalError = Swift.fatalError + /// This class is used to join assembly instances into separated shared group. /// /// All assemblies with one context shares object graph stack. @@ -355,7 +358,7 @@ open class Assembly: AssemblyInternal { definitionClosure: DefinitionClosure? = nil) -> ResultType { guard let context = self.context else { - fatalError("Associated context doesn't exists anymore") + fatalError("Associated context doesn't exists anymore", #file, #line) } context.locker.lock(); defer { context.locker.unlock() } @@ -371,7 +374,7 @@ open class Assembly: AssemblyInternal { let substitutionObject = substitutionClosure() guard let object = substitutionObject as? ResultType else { - fatalError("Expected type: \(ResultType.self), received: \(type(of: substitutionObject))") + fatalError("Expected type: \(ResultType.self), received: \(type(of: substitutionObject))", #file, #line) } return object @@ -396,7 +399,7 @@ open class Assembly: AssemblyInternal { context.objectGraphStackDepth += 1 guard var object = definition.initObject() else { - fatalError("Failed to initialize object") + fatalError("Failed to initialize object", #file, #line) } context.objectGraphStackDepth -= 1 @@ -426,7 +429,7 @@ open class Assembly: AssemblyInternal { let resultPointer = withUnsafePointer(to: &result, { unsafeBitCast($0.pointee, to: Int.self) }) if currentPointer != resultPointer { - fatalError("Singleton already exist, inspect your dependencies graph") + fatalError("Singleton already exist, inspect your dependencies graph", #file, #line) } } } @@ -441,7 +444,7 @@ open class Assembly: AssemblyInternal { } guard let finalResult = result as? ResultType else { - fatalError("Failed to build result object. Expected \(ResultType.self) received: \(result)") + fatalError("Failed to build result object. Expected \(ResultType.self) received: \(result)", #file, #line) } return finalResult @@ -469,7 +472,7 @@ public final class Definition: DefinitionInternal func injectObject(object: InjectableObject) -> InjectableObject { guard let injectableObject = object as? ObjectType else { - fatalError() + fatalError("Failed to build result object. Expected \(ObjectType.self) received: \(object)", #file, #line) } guard let actualInjectClosure = self.injectClosure else { diff --git a/Tests/Test_SingletonDuplication.swift b/Tests/Test_SingletonDuplication.swift new file mode 100644 index 0000000..f50074f --- /dev/null +++ b/Tests/Test_SingletonDuplication.swift @@ -0,0 +1,67 @@ +// +// Test_SingletoneDuplication.swift +// EasyDi-iOS-Tests +// +// Created by Andrey Konoplyankin on 29.03.2021. +// Copyright © 2021 AndreyZarembo. All rights reserved. +// + +import XCTest +@testable import EasyDi + +fileprivate class SomeObjectA { + let objectB: SomeObjectB + + init(objectB: SomeObjectB) { + self.objectB = objectB + } +} + +fileprivate class SomeObjectB { + var objectA: SomeObjectA? +} + +fileprivate class TestAssembly: Assembly { + var objectA: SomeObjectA { + return define(scope: .lazySingleton, init: SomeObjectA(objectB: self.objectB)) + } + + var objectB: SomeObjectB { + return define(scope: .lazySingleton, init: SomeObjectB()) { + $0.objectA = self.objectA + return $0 + } + } +} + +final class Test_SingletonDuplication: XCTestCase { + func testSingletonDuplication() { + let context = DIContext() + let assembly = TestAssembly.instance(from: context) + + let error = FalatErrorHandler(test: self).catchFatalError { + let _ = assembly.objectA + let _ = assembly.objectB + } + XCTAssertEqual(error, "Singleton already exist, inspect your dependencies graph") + } +} + +private struct FalatErrorHandler { + let test: XCTestCase + + func catchFatalError(handler: @escaping () -> Void) -> String? { + let expectation = test.expectation(description: "fatal_error") + var result: String? + EasyDi.fatalError = { message, _, _ in + result = message() + expectation.fulfill() + while (true) { RunLoop.current.run() } + } + + DispatchQueue.global(qos: .background).async(execute: handler) + test.waitForExpectations(timeout: 0.1, handler: nil) + EasyDi.fatalError = Swift.fatalError + return result + } +} From 68c1cb17c66a01c106212bf77815bb7b395de64c Mon Sep 17 00:00:00 2001 From: Andrey Konoplyankin Date: Thu, 8 Apr 2021 10:10:45 +0300 Subject: [PATCH 4/5] Value types currently doesn't works... --- Sources/EasyDi.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Sources/EasyDi.swift b/Sources/EasyDi.swift index 8444461..98bf592 100644 --- a/Sources/EasyDi.swift +++ b/Sources/EasyDi.swift @@ -424,12 +424,14 @@ open class Assembly: AssemblyInternal { if context.singletons[key] == nil { context.singletons[key] = result } else { - var current = context.singletons[key] as! ObjectType - let currentPointer = withUnsafePointer(to: ¤t, { unsafeBitCast($0.pointee, to: Int.self) }) - let resultPointer = withUnsafePointer(to: &result, { unsafeBitCast($0.pointee, to: Int.self) }) + let current = context.singletons[key] as! ObjectType - if currentPointer != resultPointer { - fatalError("Singleton already exist, inspect your dependencies graph", #file, #line) + if type(of: current) is AnyClass { + if unsafeBitCast(current, to: Int.self) != unsafeBitCast(result, to: Int.self) { + fatalError("Singleton already exist, inspect your dependencies graph", #file, #line) + } + } else { + // Skip value types } } } From cdfb91e8438e0fbf8c84d748a95faeafe1480a8f Mon Sep 17 00:00:00 2001 From: Andrey Konoplyankin Date: Fri, 11 Jun 2021 11:47:52 +0300 Subject: [PATCH 5/5] Use NSException --- Sources/EasyDi.swift | 16 +++++----- Tests/Test_SingletonDuplication.swift | 42 +++++++++++++++------------ 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/Sources/EasyDi.swift b/Sources/EasyDi.swift index 98bf592..a558a71 100644 --- a/Sources/EasyDi.swift +++ b/Sources/EasyDi.swift @@ -24,9 +24,6 @@ struct WeakSingletonWrapper { weak var instance: AnyObject? } -// For handle fatalError calls in tests -var fatalError = Swift.fatalError - /// This class is used to join assembly instances into separated shared group. /// /// All assemblies with one context shares object graph stack. @@ -358,7 +355,7 @@ open class Assembly: AssemblyInternal { definitionClosure: DefinitionClosure? = nil) -> ResultType { guard let context = self.context else { - fatalError("Associated context doesn't exists anymore", #file, #line) + fatalError("Associated context doesn't exists anymore") } context.locker.lock(); defer { context.locker.unlock() } @@ -374,7 +371,7 @@ open class Assembly: AssemblyInternal { let substitutionObject = substitutionClosure() guard let object = substitutionObject as? ResultType else { - fatalError("Expected type: \(ResultType.self), received: \(type(of: substitutionObject))", #file, #line) + fatalError("Expected type: \(ResultType.self), received: \(type(of: substitutionObject))") } return object @@ -399,7 +396,7 @@ open class Assembly: AssemblyInternal { context.objectGraphStackDepth += 1 guard var object = definition.initObject() else { - fatalError("Failed to initialize object", #file, #line) + fatalError("Failed to initialize object") } context.objectGraphStackDepth -= 1 @@ -428,7 +425,8 @@ open class Assembly: AssemblyInternal { if type(of: current) is AnyClass { if unsafeBitCast(current, to: Int.self) != unsafeBitCast(result, to: Int.self) { - fatalError("Singleton already exist, inspect your dependencies graph", #file, #line) + let reason = "Singleton already exist, inspect your dependencies graph" + NSException(name: .internalInconsistencyException, reason: reason, userInfo: nil).raise() } } else { // Skip value types @@ -446,7 +444,7 @@ open class Assembly: AssemblyInternal { } guard let finalResult = result as? ResultType else { - fatalError("Failed to build result object. Expected \(ResultType.self) received: \(result)", #file, #line) + fatalError("Failed to build result object. Expected \(ResultType.self) received: \(result)") } return finalResult @@ -474,7 +472,7 @@ public final class Definition: DefinitionInternal func injectObject(object: InjectableObject) -> InjectableObject { guard let injectableObject = object as? ObjectType else { - fatalError("Failed to build result object. Expected \(ObjectType.self) received: \(object)", #file, #line) + fatalError("Failed to build result object. Expected \(ObjectType.self) received: \(object)") } guard let actualInjectClosure = self.injectClosure else { diff --git a/Tests/Test_SingletonDuplication.swift b/Tests/Test_SingletonDuplication.swift index f50074f..cc46950 100644 --- a/Tests/Test_SingletonDuplication.swift +++ b/Tests/Test_SingletonDuplication.swift @@ -36,32 +36,36 @@ fileprivate class TestAssembly: Assembly { final class Test_SingletonDuplication: XCTestCase { func testSingletonDuplication() { + NSException.test_swizzleRaise() + let context = DIContext() let assembly = TestAssembly.instance(from: context) - let error = FalatErrorHandler(test: self).catchFatalError { - let _ = assembly.objectA - let _ = assembly.objectB - } - XCTAssertEqual(error, "Singleton already exist, inspect your dependencies graph") + let _ = assembly.objectA + let _ = assembly.objectB + + XCTAssertEqual(NSException.last?.reason, "Singleton already exist, inspect your dependencies graph") + NSException.last = nil } } -private struct FalatErrorHandler { - let test: XCTestCase +extension NSException { + static var last: NSException? + static var alreadySwizzled = false - func catchFatalError(handler: @escaping () -> Void) -> String? { - let expectation = test.expectation(description: "fatal_error") - var result: String? - EasyDi.fatalError = { message, _, _ in - result = message() - expectation.fulfill() - while (true) { RunLoop.current.run() } - } + static func test_swizzleRaise() { + guard !alreadySwizzled else { return } - DispatchQueue.global(qos: .background).async(execute: handler) - test.waitForExpectations(timeout: 0.1, handler: nil) - EasyDi.fatalError = Swift.fatalError - return result + let origin = class_getInstanceMethod(NSException.self, NSSelectorFromString("raise")) + let new = class_getInstanceMethod(NSException.self, NSSelectorFromString("test_raise")) + + if let origin = origin, let new = new { + method_exchangeImplementations(origin, new) + alreadySwizzled = true + } + } + + @objc func test_raise() { + NSException.last = self } }