Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Configuration consistency fixes #10

Merged
merged 9 commits into from Apr 11, 2018
Merged

Configuration consistency fixes #10

merged 9 commits into from Apr 11, 2018

Conversation

michalbundyra
Copy link
Member

It should be compatible with https://github.com/weierophinney/zend-container-config-test and docs https://docs.zendframework.com/zend-expressive/v3/features/container/config/

The main issue is that container passed to the factory is not the same instance as the original container. This problem is because we configure pimple container (non-psr-11) and we wrap it with PSR-11 container when we need and when we pass it into the factories:
https://github.com/webimpress/zend-pimple-config/blob/hotfix/config-consistency/src/Config.php#L93

This PR is based on #6

By default all services created through the `get` method were shared,
first call creates an instance, and every another call return the
previously created instance.
Providing service name in "shared" allows to disable this functionality
for a specific service. Then every call of the `get` method creates
brand new instance of the service.
It is possoble to disable shared services by default for all services
by using `shared_by_default` option to boolean `false`.
@weierophinney
Copy link
Member

I suggest overriding testFactoryIsProvidedContainerAndServiceNameAsArguments within the test class in order to change the assertions around the container.

Alternately, we can simply check that the property is a PSR-11 container, as there's little question how that property is set.

@michalbundyra
Copy link
Member Author

I think I would prefer the 2nd option, if we override test here it will be hacky solution.
Maybe we should then add final for all our test cases in zend-container-config-test?
If someone what have library compatible with our test it must pass all our test without hacking ;)

@weierophinney
Copy link
Member

I think I would prefer the 2nd option, if we override test here it will be hacky solution.

Okay, those changes are made in the testing repository.

Allows removing repository, pinning to a (semi-) stable version, and
using the shipped abstract class.
@weierophinney
Copy link
Member

I've updated your branch to pin to the stable 0.1.0 release of zend-container-config-test, and updated the shipped test as well to extend AbstractExpressiveContainerConfigTest.

@weierophinney weierophinney merged commit 5624b6a into zendframework:master Apr 11, 2018
weierophinney added a commit that referenced this pull request Apr 11, 2018
weierophinney added a commit that referenced this pull request Apr 11, 2018
weierophinney added a commit that referenced this pull request Apr 11, 2018
Forward port #10
Forward port #6

Conflicts:
	CHANGELOG.md
@weierophinney
Copy link
Member

Thanks, @webimpress!

@michalbundyra michalbundyra deleted the hotfix/config-consistency branch April 11, 2018 20:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants