hmlongco / Resolver

Swift Ultralight Dependency Injection / Service Locator framework
MIT License
2.14k stars 188 forks source link

After calling Resolver.cached.reset(), Resolver.optional() still returns value #113

Closed ChrisMash closed 3 years ago

ChrisMash commented 3 years ago

Hi, I seem to have stumbled upon a bug, or need a bit of guidance to achieve what I want!

This code shows the issue.

// Register a view to the cached scope
let view = UIView()
Resolver.register { view }.scope(Resolver.cached)

// Optionally resolve it, successfully (doesn't matter if this one is optional or not)
let resolvedView: UIView? = Resolver.optional()
XCTAssertEqual(view, resolvedView)

// Reset the cache to remove the registered view
Resolver.cached.reset()

// Optionally resolve it, still successfully (shouldn't be returned after the rest)
let reResolvedView: UIView? = Resolver.optional()
XCTAssertNil(reResolvedView) // will fail

// Non-optionally resolve it, crash because it's not in the cache anymore
let reResolvedView2: UIView? = Resolver.resolve() // will crash

Am I wrong in any of my assumptions?

Here's a test project with a unit test running this: ResolverTest.zip

This can be reproduce on both 1.2.1 and 1.4.2

hmlongco commented 3 years ago

Nope. All working as expected.

A registration factory provides an instance of an object. A cache is used to provide the same instance when requested.

If the object isn't in the cache, it asks the factory to build one, then caches a reference to it, and then returns it. If you clear the cache, then the same thing happens all over again. "Hey, factory, build me another one!"

In the above sample the cache is redundant, since the factory will always return the same strongly captured reference to the constructed UIView. (Congrats, you've made a singleton.)

Generally, you'd do the following to only construct the object when needed.

Resolver.register { UIView() }.scope(Resolver.cached)

The last resolution fails not because of a cache issue, but because of a type-mismatch as the inferred type is Optional\<UIView>, not UIView. Resolver uses the .optional() form to help disambiguate the type so resolution lookups work as expected.

ChrisMash commented 3 years ago

Thanks so much for the quick response and all the details, it's making more sense now, I was definitely getting tripped up on the optionals 😁

What I was trying to achieve was to have a unit test where I can register a dependency, but only for that test, so at the end I can reset it and then other tests that don't need that dependency (going through code paths that would use that dependency if it's registered, but otherwise wouldn't care) would behave as if it wasn't there if they weren't setup to register it. I noticed another open issue where you'd pointed out your Builder project which has an example of how to handle this, so I think I'm all sorted now!

I did find it a bit odd that setting Resolver.root to something still meant you needed to register directly to that instance, I thought that Resolver.register would automatically register to the root. But perhaps there's a good reason for that!

hmlongco commented 3 years ago

By default the global register function registers to .main. Global resolutions resolve based on .root (which usually points to .main).

In the builder example I'm registering to a specific container so I'm not changing anything in .main. It's just that with root swapped Resolver will "see" the other container first.