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

Conversation

michalbundyra
Copy link
Member

For now used with SM, but we can add it also to pimple, as it is proposed in:
zendframework/zend-pimple-config#6

@weierophinney
Copy link
Member

I'm not 100% sure we should try and enforce this.

As we discovered during the march to Expressive 3, we can accomplish this to a degree by having multiple services that resolve to the same factory, allowing discrete instances. Additionally, whenever a service should always return a discrete instance, we can instead have the service resolve to a factory (either a class or a PHP callable) that can produce an instance of the requested type. These will work regardless of sharing capabilities, and generally help us thing more clearly about why a service should not be shared.

As such, for now, I'm going to close this. We can revisit at a later date if we see demand for the capability as a general-purpose container requirement.

@michalbundyra
Copy link
Member Author

@weierophinney I don't want to enforce it for all other implementations, as we can't do it always (it was not possible to do in Aura.Di if I remember correctly).
I just want to have this as separate trait here, as some other implementation could support shared services - i.e. pimple (see: zendframework/zend-pimple-config#6) or Auryn (see: https://github.com/northwoods/container#service-definitions). In that case if someone decide to support shared services that trait should be included in the test case.

This is why I think we should change it slightly - I mean I would create here abstract class ExpressiveContainerConfigTest with bare minimum each container has satisfy (so basically nor it will be all traits included in AllTestTrait), and if any container decide to support more SM configuration just another trait should be added in specific implementation. Then we can remove AllTestTrait. After that test for SM is gonna look like:

class ServiceManagerTest extends ExpressiveContainerConfigTest
{
    use SharedTestTrait; // additional trait, as SM supports shared services, all other traits are in `ExpressiveContainerConfigTest`

    public function createContainer(array $config) : ContainerInterface
    {
        return new ServiceManager($config);
    }
}

where:

abstract class ExpressiveContainerConfigTest extends ContainerTest
{
    use AliasTestTrait;
    use DelegatorTestTrait;
    use FactoryTestTrait;
    use InvokableTestTrait;
    use ServiceTestTrait;
}

Then, I think we should also add here tests for abstract_factories. If only SM is gonna use it - fine, but if someone decide implement that feature in his container implementation it will be much easier to know how it should work. But for now I think shared services are more important because some containers (as mentioned above) already implement it, so we would like to have it consistent.

@weierophinney weierophinney reopened this Apr 10, 2018
@weierophinney weierophinney merged commit 590efda into master Apr 10, 2018
weierophinney added a commit that referenced this pull request Apr 10, 2018
Added "shared" and "shared_by_default" configuration tests
@michalbundyra michalbundyra deleted the feature/shared branch April 11, 2018 11:04
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.

2 participants