Fix issue with shared scope when type was optional

Author: hmlongco

CHANGELOG

@@ -1,5 +1,9 @@
 # Factory Changelog
 
+### 1.2.1
+
+* Fix issue with shared scope when type was optional
+
 ### 1.2.0 - Stable release
 
 * Revised structure to better support ParameterFactory

README.md

@@ -28,7 +28,7 @@ Failure to find a matching type can lead to an application crash if we attempt t
  * **Safe:** Factory is compile-time safe; a factory for a given type *must* exist or the code simply will not compile.
  * **Flexible:** It's easy to override dependencies at runtime and for use in SwiftUI Previews.
  * **Powerful:** Like Resolver, Factory supports application, cached, shared, and custom scopes, custom containers, arguments, decorators, and more.
- * **Lightweight:** With all of that Factory is slim and trim, well under 400 lines of code and half the size of Resolver.
+ * **Lightweight:** With all of that Factory is slim and trim, just 400 lines of code and half the size of Resolver.
  * **Performant:** Little to no setup time is needed for the vast majority of your services, resolutions are extremely fast, and no compile-time scripts or build phases are needed.
  * **Concise:** Defining a registration usually takes just a single line of code. Same for resolution.
  * **Tested:** Unit tests ensure correct operation of registrations, resolutions, and scopes.

Sources/Factory/Factory.swift

@@ -196,22 +196,33 @@ open class SharedContainer {
             defer { lock.unlock() }
             lock.lock()
             if let box = cache[id], let instance = box.instance as? T {
-                return instance
+                if let optional = instance as? OptionalProtocol {
+                    if optional.hasWrappedValue {
+                        return instance
+                    }
+                } else {
+                    return instance
+                }
             }
             let instance: T = factory()
-            if let box = box(instance) {
+            if let optional = instance as? OptionalProtocol {
+                if optional.hasWrappedValue, let box = box(instance) {
+                    cache[id] = box
+                }
+            } else if let box = box(instance) {
                 cache[id] = box
             }
             return instance
         }
 
-        /// Internal reset function used by Factory
+       /// Internal reset function used by Factory
         fileprivate func reset(_ id: UUID) {
             defer { lock.unlock() }
             lock.lock()
             cache.removeValue(forKey: id)
         }
 
+        /// Internal function correctly boxes cache value depending upon scope type
         fileprivate func box<T>(_ instance: T) -> AnyBox? {
             StrongBox<T>(boxed: instance)
         }
@@ -247,7 +258,15 @@ extension SharedContainer.Scope {
             super.init()
         }
         fileprivate override func box<T>(_ instance: T) -> AnyBox? {
-            type(of: instance) is AnyClass ? WeakBox(boxed: instance as AnyObject) : nil
+            if let optional = instance as? OptionalProtocol {
+                // Actual wrapped type could be value, protocol, or something else so we need to check if wrapped instance is class type
+                if optional.hasWrappedValue, type(of: optional.unwrap()) is AnyObject.Type {
+                    return WeakBox(boxed: instance as AnyObject)
+                }
+            } else if type(of: instance) is AnyClass {
+                return WeakBox(boxed: instance as AnyObject)
+            }
+            return nil
         }
     }
 
@@ -343,6 +362,27 @@ private struct Registration<P,T> {
 
 }
 
+/// Internal protocol used to evaluate optional types for caching
+private protocol OptionalProtocol {
+    var hasWrappedValue: Bool { get }
+    func unwrap() -> Any
+}
+
+extension Optional : OptionalProtocol {
+    fileprivate var hasWrappedValue: Bool {
+        if case .some = self {
+            return true
+        }
+        return false
+    }
+    fileprivate func unwrap() -> Any {
+        if case .some(let unwrapped) = self {
+            return unwrapped
+        }
+        preconditionFailure("trying to unwrap nil")
+    }
+}
+
 /// Internal box protocol for scope functionality
 private protocol AnyBox {
     var instance: Any { get }

Tests/FactoryTests/FactoryDefectTests.swift

@@ -11,13 +11,13 @@ final class FactoryDefectTests: XCTestCase {
 
     // scope would not correctly resolve a factory with an optional type. e.g. Factory<MyType?>(scope: .cached) { nil }
     func testNilScopedService() throws {
-        Container.nilScopedService.reset()
-        let service1 = Container.nilScopedService()
+        Container.nilCachedService.reset()
+        let service1 = Container.nilCachedService()
         XCTAssertNil(service1)
-        Container.nilScopedService.register {
+        Container.nilCachedService.register {
             MyService()
         }
-        let service2 = Container.nilScopedService()
+        let service2 = Container.nilCachedService()
         XCTAssertNotNil(service2)
     }
 

Tests/FactoryTests/FactoryScopeTests.swift

@@ -42,12 +42,44 @@ final class FactoryScopeTests: XCTestCase {
     }
 
     func testSharedScope() throws {
-        var service1: MyService? = Container.sharedService()
-        var service2: MyService? = Container.sharedService()
+        var service1: MyServiceType? = Container.sharedService()
+        var service2: MyServiceType? = Container.sharedService()
+        XCTAssertNotNil(service1)
+        XCTAssertNotNil(service2)
+        // Shared cached item ids should match
         XCTAssertTrue(service1?.id == service2?.id)
         service1 = nil
         service2 = nil
-        let service3: MyService? = Container.sharedService()
+        let service3: MyServiceType? = Container.sharedService()
+        XCTAssertNotNil(service3)
+        XCTAssertTrue(service2?.id != service3?.id)
+    }
+
+    func testOptionalSharedScope() throws {
+        var service1: MyServiceType? = Container.optionalSharedService()
+        var service2: MyServiceType? = Container.optionalSharedService()
+        XCTAssertNotNil(service1)
+        XCTAssertNotNil(service2)
+        // Shared cached item ids should match
+        XCTAssertTrue(service1?.id == service2?.id)
+        service1 = nil
+        service2 = nil
+        let service3: MyServiceType? = Container.optionalSharedService()
+        XCTAssertNotNil(service3)
+        XCTAssertTrue(service2?.id != service3?.id)
+    }
+
+    func testOptionalValueSharedScope() throws {
+        var service1: MyServiceType? = Container.optionalValueService()
+        var service2: MyServiceType? = Container.optionalValueService()
+        XCTAssertNotNil(service1)
+        XCTAssertNotNil(service2)
+        // Value types aren't shared so cached item ids should NOT match
+        XCTAssertTrue(service1?.id != service2?.id)
+        service1 = nil
+        service2 = nil
+        let service3: MyServiceType? = Container.optionalValueService()
+        XCTAssertNotNil(service3)
         XCTAssertTrue(service2?.id != service3?.id)
     }
 
@@ -160,26 +192,51 @@ final class FactoryScopeTests: XCTestCase {
     }
 
     func testNilScopedServiceCaching() throws {
-        Container.nilScopedService.reset()
+        Container.nilCachedService.reset()
         XCTAssertTrue(Container.Scope.cached.isEmpty)
-        let service1 = Container.nilScopedService()
+        let service1 = Container.nilCachedService()
         XCTAssertNil(service1)
-        XCTAssertFalse(Container.Scope.cached.isEmpty) // nil was cached
-        let service2 = Container.nilScopedService()
-        XCTAssertNil(service2) // cached nil was returned
-        XCTAssertFalse(Container.Scope.cached.isEmpty)
-        Container.nilScopedService.register {
+        XCTAssertTrue(Container.Scope.cached.isEmpty) // nothing caches
+        let service2 = Container.nilCachedService()
+        XCTAssertNil(service2)
+        XCTAssertTrue(Container.Scope.cached.isEmpty) // nothing caches
+        Container.nilCachedService.register {
             MyService()
         }
-        let service3 = Container.nilScopedService()
+        let service3 = Container.nilCachedService()
         XCTAssertNotNil(service3)
-        XCTAssertFalse(Container.Scope.cached.isEmpty)
-        Container.nilScopedService.register {
+        XCTAssertFalse(Container.Scope.cached.isEmpty) // cached value
+        Container.nilCachedService.register {
             nil
         }
-        let service4 = Container.nilScopedService()
+        let service4 = Container.nilCachedService()
         XCTAssertNil(service4) // cache was reset by registration
-        XCTAssertFalse(Container.Scope.cached.isEmpty)
+        XCTAssertTrue(Container.Scope.cached.isEmpty) // nothing cached
     }
 
+    func testNilSharedServiceCaching() throws {
+        Container.nilSharedService.reset()
+        XCTAssertTrue(Container.Scope.shared.isEmpty)
+        let service1 = Container.nilSharedService()
+        XCTAssertNil(service1)
+        XCTAssertTrue(Container.Scope.shared.isEmpty) // nothing caches
+        let service2 = Container.nilSharedService()
+        XCTAssertNil(service2)
+        XCTAssertTrue(Container.Scope.shared.isEmpty) // nothing caches
+        Container.nilSharedService.register {
+            MyService()
+        }
+        let service3 = Container.nilSharedService()
+        XCTAssertNotNil(service3)
+        let service4 = Container.nilSharedService()
+        XCTAssertNotNil(service4)
+        XCTAssertTrue(service3?.id == service4?.id)
+        XCTAssertFalse(Container.Scope.shared.isEmpty) // cached value
+        Container.nilSharedService.register {
+            nil
+        }
+        let service5 = Container.nilSharedService()
+        XCTAssertNil(service5) // cache was reset by registration
+        XCTAssertTrue(Container.Scope.shared.isEmpty) // nothing cached
+    }
 }

Tests/FactoryTests/MockServices.swift

@@ -85,8 +85,11 @@ extension Container {
     static let sharedService = Factory(scope: .shared) { MyService() }
     static let singletonService = Factory(scope: .singleton) { MyService() }
     static let optionalService = Factory<MyServiceType?> { MyService() }
+    static let optionalSharedService = Factory<MyServiceType?>(scope: .shared) { MyService() }
+    static let optionalValueService = Factory<MyServiceType?> { ValueService() }
     static let nilSService = Factory<MyServiceType?> { nil }
-    static let nilScopedService = Factory<MyServiceType?>(scope: .cached) { nil }
+    static let nilCachedService = Factory<MyServiceType?>(scope: .cached) { nil }
+    static let nilSharedService = Factory<MyServiceType?>(scope: .shared) { nil }
     static let sessionService = Factory(scope: .session) { MyService() }
     static let valueService = Factory(scope: .cached) { ValueService() }
     static let sharedValueService = Factory(scope: .shared) { ValueService() }