This repository has been archived by the owner on Feb 6, 2020. It is now read-only.
Remove the need for the $checkAbstractFactories flag #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
: : Please enter the commit message for your changes. Lines starting : with '#' will be ignored, and an empty message aborts the commit. : : Date: Tue Jul 21 14:13:04 2015 +0200 : : qrebase in progress; onto afcc387 : You are currently editing a commit while rebasing branch 'rebase' on 'afcc387'. : : Changes to be committed: : deleted: src/AbstractFactoryInterface.php : deleted: src/Config.php : deleted: src/ConfigInterface.php : deleted: src/DelegatorFactoryInterface.php : deleted: src/Di/DiAbstractServiceFactory.php : deleted: src/Di/DiInstanceManagerProxy.php : deleted: src/Di/DiServiceFactory.php : deleted: src/Di/DiServiceInitializer.php : deleted: src/Exception/CircularDependencyFoundException.php : deleted: src/Exception/CircularReferenceException.php : deleted: src/Exception/InvalidServiceNameException.php : deleted: src/Exception/RuntimeException.php : deleted: src/Exception/ServiceLocatorUsageException.php : deleted: src/FactoryInterface.php : deleted: src/InitializerInterface.php : deleted: src/MutableCreationOptionsInterface.php : deleted: src/MutableCreationOptionsTrait.php : deleted: src/Proxy/LazyServiceFactoryFactory.php : deleted: src/ServiceLocatorAwareInterface.php : deleted: src/ServiceLocatorAwareTrait.php : deleted: src/ServiceManagerAwareInterface.php : deleted: test/Di/DiAbstractServiceFactoryTest.php : deleted: test/Di/DiServiceFactoryTest.php : deleted: test/Di/DiServiceInitializerTest.php : deleted: test/Exception/ServiceLocatorUsageExceptionTest.php : deleted: test/MutableCreationOptionsTraitTest.php : deleted: test/Proxy/LazyServiceFactoryFactoryTest.php : deleted: test/Proxy/LazyServiceFactoryTest.php : deleted: test/ServiceLocatorAwareTraitTest.php : deleted: test/TestAsset/AbstractFactoryWithMutableCreationOptions.php : deleted: test/TestAsset/Bar.php : deleted: test/TestAsset/BarAbstractFactory.php : deleted: test/TestAsset/CallableWithMutableCreationOptions.php : deleted: test/TestAsset/CircularDependencyAbstractFactory.php : deleted: test/TestAsset/ExceptionThrowingFactory.php : deleted: test/TestAsset/Foo.php : deleted: test/TestAsset/FooAbstractFactory.php : deleted: test/TestAsset/FooCounterAbstractFactory.php : deleted: test/TestAsset/FooException.php : deleted: test/TestAsset/FooFactory.php : deleted: test/TestAsset/FooFake.php : deleted: test/TestAsset/FooFakeAbstractFactory.php : deleted: test/TestAsset/FooInitializer.php : deleted: test/TestAsset/FooPluginManager.php : deleted: test/TestAsset/GlobIteratorService.php : deleted: test/TestAsset/MockSelfReturningDelegatorFactory.php : deleted: test/TestAsset/TrollAbstractFactory.php : deleted: test/TestAsset/WaitingAbstractFactory.php : deleted: test/bootstrap.php :
First docs
- Fixes two tests for options usage to use `build()` when providing options.
s/have/has for singular
- Provides tests covering remaining uncovered functionality of the LazyServiceFactoryFactory.
- FactoryInterface does this already; InvokableFactory should as well.
This commit achieves 100% test coverage, and attempts to test all behaviors of the refactored ServiceManager. In particular: - Re-added 'services' to the configurable options; this was done for feature parity with the original, and because some cases, such as configuration, will not need factories. - Ensured that invalid delegator factory types now raise exceptions. - Tested that lazy-services are created and proxy as expected.
- Proper grammar, plus now matches what bookdown is looking for.
- Updated `configure()` docblock to properly reflect what it accepts; also updated `__construct()` and `withConfig()` docblocks to link to `configure()` for purposes of documenting the configuration array. - Changed verbiage of inline comment in `configure()` to refer to service construction. - Ensure that the `$options` argument is nullable. If null, it should *not* be passed to constructors!
- Wrote delegators and lazy-services chapters, based on originals from ZF2 manual and adapted for new usage. - Edited all other chapters.
Lazy services were added after the ServiceManager had an established API, and as such had to rely on optional integration. This patch alters that relationship to make lazy service configuration a first-class citizen of the ServiceManager. This allows: - Removing the `LazyServiceFactoryFactory`. - Passing `lazy_services` configuration to the constructor or `withConfig()` method. - Direct instantiation of the `LazyServiceFactory` when first encountered as a delegator factory, using the `lazy_services` configuration composed in the ServiceManager itself. The usage is thus simplified, as all configuration for all service types is now directly handled by the ServiceManager itself.
- Often necessary for determining if configuration or dependencies required for creating instances exist.
Previous behavior of the service manager allowed specifying aliases when creating the `shared` configuration. Prior to this patch, that did not work with the refactored code. This patch updates the code to check for a cached service under the _requested_ service name, not the _resolved_ service name, which allows specifying sharing configuration based on aliases. Internally, it caches up to twice: - If the resolved service name is supposed to be shared, it will cache under the resolved name. - If the requested service name is supposed to be shared, it will cache under the resolved name. This allows having separate instances based on service name.
…aliasing Cache by requested service name
Prior to this patch, calling `withConfig()` on a plugin manager would reset the `creationContext` to the plugin manager itself — instead of retaining the original creation context. This patch updates `ServiceManager::withConfig()` to ensure that if the creation context is different than the original instance, it gets injected into the clone.
…manager-with-config Ensure plugin manager creation context travels with clone
With the most recent changes to allow unshared aliases, aliases to explicitly registered services no longer worked. This patch adds logic to get() to check if: - the alias is shared, AND - the resolved service is present and, if so, registers the service under the alias, returning it.
…-service Ensure aliases can reference explicit services
Add the benchmarks
Forward port zendframework#39
…ases Missing test cases
…ers-to-config-test Feature/add covers to config test
|
Ping @bakura10 … |
The flag breaks the LSP, and requires that a user know how a given service might be configured inside the container. Removing it provides easier interoperability, and removes unexpected false negative lookups.
|
Closing; I opened against the wrong branch. |
|
I can't fight against a breakage of the Liskov law =). More seriously, no problem with that. Best practices encourage not to use many abstract factories so the introduced overweight should be negligeable. Go on! ;) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In updating zend-mvc to v3 of zend-servicemanager, I ran into a subtle issue:
has()does not look in abstract factories by default, and requires that you pass an additional flag,$checkAbstractFactories, with a boolean true value, if you want to test if an abstract factory can resolve the given$name.This breaks the Liskov Substitution Principal, as it means that the behavior of
has()depends on how theServiceManageris configured. Users then must be aware that an abstract factory might supply a dependency, and cannot rely onhas()per the container-interop contract to work for the specific use case of zend-servicemanager.As such, I'd like to remove the flag, and check abstract factories whenever a service or factory matching the provided
$nameis not found.