zendframework / zend-pimple-config

Pimple container configurator based on ZF ServiceManager configuration
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

Ability to disable shared services with Pimple container #6

Closed michalbundyra closed 6 years ago

michalbundyra commented 6 years ago

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.

See:

michalbundyra commented 6 years ago

Just a note, tests are failing because it's not ready yet. Please see related PR. I'll note one more time: I think we should extract tests into separate repository, with abstract test case with method createContainer and then require that repository in require-dev section of composer and implement that test case. The repository should be used with all container packages (servicemanager, pimple, auradi, ...) to have exactly the same behaviour of the container with the same configuration and to not duplicate these tests across all repositories. Unless you have better idea?

I'm not sure if this PR should be treated as feature or hotfix because the repository has not been used with expressive 2.0, but it could be, so would be nice to have it there in place. But in other hands we can say always "all services are shared and we are not supporting others with pimple" so adding it is a feature...

geerteltink commented 6 years ago

I'll note one more time: I think we should extract tests into separate repository, with abstract test case with method createContainer and then require that repository in require-dev section of composer and implement that test case. The repository should be used with all container packages (servicemanager, pimple, auradi, ...) to have exactly the same behaviour of the container with the same configuration and to not duplicate these tests across all repositories.

I prefer to keep the tests into the same repo. Moving this into its own repo makes sense as they can be shared between other similar packages. However you might loose overview. Also where does it end? Should we do this for other similar packages as well like the expressive routers or template handlers. Before you know it, it is getting a big mess. I'd like to think that tests belong in the same repo as the source code it is testing.

michalbundyra commented 6 years ago

@xtreamwayz

I prefer to keep the tests into the same repo.

I would prefer the same, but in that case we have a lot of code duplicated, and adding more/new tests will require update all repositories. Also external libraries compatible with expressive would be able to use the library with these tests. I was even thinking about creating expressive-common-test repository which will contains common tests for container/routers/template renderers. But for templates/routers we have a bit different situation, because we could have these tests in zendframework/zend-expressive-router (for router related repos) and zendframework/zend-expressive-template (for template related repos). We don't have just common repository for containers.

weierophinney commented 6 years ago

As discussed on zendframework/zend-expressive#547, this may not even be necessary.

For services we do not want as shared, we can create "virtual services" that use the same factory to generate them. Every container I've surveyed uses a "shared by default" strategy, while very few have the ability to mark a service as unshared. However, when multiple services resolve to the same factory, none of them cache and re-use the result for different service names, but instead invoke the factory the first time each discrete service is retrieved. This allows you to create unshared services by using different service names.

Now, if a container does support altering shared service settings, we should likely find a way to accommodate that, but I'm not 100% certain yet if we want to make that a general requirement for all services, particularly if we have a viable alternative with existing configuration.

michalbundyra commented 6 years ago

I've created common repository (still WIP) with common container test cases: https://github.com/webimpress/zend-container-test

weierophinney commented 6 years ago

Thanks, @webimpress!