laminas-servicemanager icon indicating copy to clipboard operation
laminas-servicemanager copied to clipboard

Zend\ServiceManager\Factory\FactoryInterface does not use Psr\Container\ContainerInterface yet

Open weierophinney opened this issue 5 years ago • 30 comments

I want to make my factories to use Psr\Container\ContainerInterface in an Zend\Expressive project. But the Zend\ServiceManager\Factory\FactoryInterface does not use Psr\Container\ContainerInterface yet.

What are the plans about this?


Originally posted by @RalfEggert at https://github.com/zendframework/zend-servicemanager/issues/184

weierophinney avatar Dec 31 '19 22:12 weierophinney

psr-11.md says this is coming in v4. Makes sense - it's a BC break - but an over-eager search-and-replace at work broke lots of stuff for me ;)


Originally posted by @kynx at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-285903010

weierophinney avatar Dec 31 '19 22:12 weierophinney

Ok, then I guess this should include a warning:

http://zendframework.github.io/zend-expressive/reference/migration/to-v2/#psr-11-support


Originally posted by @RalfEggert at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-285923823

weierophinney avatar Dec 31 '19 22:12 weierophinney

Added an issue for Zend\Expressive : zendframework/zend-expressive#466


Originally posted by @RalfEggert at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-285923921

weierophinney avatar Dec 31 '19 22:12 weierophinney

Note; probably an assumption (at least on my side) but I'd expect DelegatorFactoryInterface and AbstractFactoryInterface should be updated to use Psr\Container\* at the same time as this.


Originally posted by @asgrim at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-394658611

weierophinney avatar Dec 31 '19 22:12 weierophinney

Just a couple notes:

  • We pin to a version of container-interop that extends the official PSR-11 interfaces.
  • zend-servicemanager v3 pins to container-interop, as it pre-dates the completion of PSR-11.
  • zend-servicemanager allows arbitrary callables for factories and delegators, so long as they have the minimum required arguments (a factory requires a container; delegators require a container, the service name, and a callback). The arguments do not need to follow the same typehints, however, which means they can typehint against the PSR-11 interface. (This is true for initializers as well, but you should NEVER use those.)
  • Abstract factories are the only ones in wide use in Expressive that do not fit this paradigm, as we require an interface implementation; this is due to the additional method for testing if the factory can create an instance of the requested service.

Thus, the migration path is to:

  • remove implementation of zend-servicemanager interfaces in your factories and delegators.
  • change ContainerInterface imports to use PSR-11 in those classes.

When it comes to abstract factories, I'd argue you should likely try not to use them. They are one of the configuration types that are non-portable between different containers. Consider using the expressive factory:create tool for each class that you were previously using an abstract factory for, or, alternately, mapping each class that can be served by the abstract factory directly to it as a factory (as the implementations we directly support all accept the service name argument to the factory).

zend-servicemanager v4 will implement PSR-11 directly, and the various interfaces consuming a container will typehint against it as well. Most likely, we will allow v4 within Expressive as well as v3 once it is released, as the consumer aspects will be the same. (That said, we may do something different with aliases and invokables for v4; if we do, we'll have a v3 minor release that provides forwards compatibility, just as we did when prepping v3.)


Originally posted by @weierophinney at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-394709630

weierophinney avatar Dec 31 '19 22:12 weierophinney

Encountered this as well when trying to use PHP-DI as a DIC. PHP-DI already uses PSR-11, while https://github.com/zendframework/zend-servicemanager/blob/release-2.7.11/src/AbstractPluginManager.php#L12 does not, causing an error.

zend-servicemanager v4 will implement PSR-11 directly, and the various interfaces consuming a container will typehint against it as well.

This should do the trick, any plans for v4 already? It seems the development branch is not yet migrated to using PSR-11...

https://github.com/zendframework/zend-servicemanager/blob/dev-4.0/src/AbstractPluginManager.php#L10

See https://github.com/elie29/zend-di-config/issues/25 for a complete description


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-433705384

weierophinney avatar Dec 31 '19 22:12 weierophinney

@holtkamp if you need compatibility with libraries that still use Interop Container you can decorate your PSR container:

<?php

declare(strict_types=1);

namespace App;

use Interop\Container\ContainerInterface;
use Psr\Container\ContainerInterface as PsrContainer;

class InteropContainerDecorator implements ContainerInterface
{
    /** @var PsrContainer */
    private $container;

    public function __construct(PsrContainer $container)
    {
        $this->container = $container;
    }

    public function get($id)
    {
        return $this->container->get($id);
    }
    
    public function has($id)
    {
        return $this->container->has($id);
    }
}

And register it as with a factory:

return [
    'dependencies' => [
        'aliases' => [
            \Interop\Container\ContainerInterface::class => \App\InteropContainerDecorator::class,
        ],
    ],
];

Originally posted by @thomasvargiu at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-433970422

weierophinney avatar Dec 31 '19 22:12 weierophinney

Thanks for the suggestions, tried a bit and this seems not to work.

Apparently Zend\Expressive\ZendView\HelperPluginManagerFactory::__construct() already adopted PSR-11 and therefore injects a Psr\Container\ContainerInterface instance, while ZendServiceManager does not.

Array
(
    [type] => 1
    [message] => Uncaught Zend\ServiceManager\Exception\InvalidArgumentException: Zend\ServiceManager\AbstractPluginManager expects a ConfigInterface or ContainerInterface instance as the first argument; received DI\Container in /user/project/vendor/zendframework/zend-servicemanager/src/AbstractPluginManager.php:59
Stack trace:
#0 /user/project/vendor/zendframework/zend-view/src/HelperPluginManager.php(253): Zend\ServiceManager\AbstractPluginManager->__construct(Object(DI\Container), Array)
#1 /user/project/vendor/zendframework/zend-expressive-zendviewrenderer/src/HelperPluginManagerFactory.php(20): Zend\View\HelperPluginManager->__construct(Object(DI\Container))
#2 [internal function]: Zend\Expressive\ZendView\HelperPluginManagerFactory->__invoke(Object(DI\Container))
#3 /user/project/vendor/p
    [file] => /user/project/vendor/zendframework/zend-servicemanager/src/AbstractPluginManager.php
    [line] => 59
)

For now using an fork of DI\Container which ALSO implements \Interop\Container\ContainerInterface does the "trick" / hack.

class Container implements \Psr\Container\ContainerInterface, \Interop\Container\ContainerInterface, FactoryInterface, InvokerInterface {
 /* rest of code */
}

A minor new release of ZendServiceManager which also accepts a PSR-11 container would be nice.


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434015174

weierophinney avatar Dec 31 '19 22:12 weierophinney

Oh, you're right. I think the bug is here Because HelperPluginManagerFactory is accepting a PSR container and injecting it to the HelperPluginManager which can accept only the Interop container.

So, you can use zend-view HelperPluginManager only with zend-servicemanager.

The only thing you can do at this moment is to try to use the decorated container and create your factory to instantiate the HelperPluginManager.


Originally posted by @thomasvargiu at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434034593

weierophinney avatar Dec 31 '19 22:12 weierophinney

You can add in the container an alias from interop container to Psr container without the need of decorator.


Originally posted by @elie29 at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434039381

weierophinney avatar Dec 31 '19 22:12 weierophinney

@elie29 with PHP-DI 6.0 the container only implements the Psr container, while zend-view and other services (zend-hydrator for me) requires the interop container. Just an alias is not enough.

The decorator allowed me to use an interop container for factories that requires an interop container, but in this case I think is more a bug of zend-expressive-zendviewrenderer which its factory use a psr container but inject it to a service that requires a interop container.


Originally posted by @thomasvargiu at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434049496

weierophinney avatar Dec 31 '19 22:12 weierophinney

@thomasvargiu thanks for the effort and ideas!

@elie29 yeah, tried that in dependencies.global.php of the skeleton Zend Expressive application:

'aliases' => [
            // Fully\Qualified\ClassOrInterfaceName::class => Fully\Qualified\ClassName::class,
            //\Interop\Container\ContainerInterface::class => \Psr\Container\ContainerInterface::class,
            \Psr\Container\ContainerInterface::class => \Interop\Container\ContainerInterface::class,
        ],

Hower it seems that Zend\Expressive\ZendView\HelperPluginManagerFactory is instantiated at the moment that the DI Container is being assembled / built by the DI ContainerBuilder => at that time the alias is not available yet...

What did function is to use the PHP-DI ContainerBuilder->wrapContainer() functionality and use the InteropContainerDecorator:

namespace Zend\DI\Config;

use App\InteropContainerDecorator;
use DI\ContainerBuilder;
use Psr\Container\ContainerInterface;
use Zend\DI\Config\ConfigInterface;

class ContainerFactory
{

    public function __invoke(ConfigInterface $config): ContainerInterface
    {
        $builder = new ContainerBuilder();
        $config->configureContainer($builder);

        $wrapperContainer = new InteropContainerDecorator();
        $builder->wrapContainer($wrapperContainer);

        $psrContainer = $builder->build();

        $wrapperContainer->setPsrContainer($psrContainer);

        return $wrapperContainer;
    }
}

But that is quite specific to https://github.com/elie29/zend-di-config, I think for now I should leave this and wait for a update to ZendServiceManager to actually use PSR-11.


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434052493

weierophinney avatar Dec 31 '19 22:12 weierophinney

You can kick off the container in the front controller so it is ready before other dependencies. However, phpdi is Psr compliant but not interop. So we need to wait for zend service v4


Originally posted by @elie29 at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434054548

weierophinney avatar Dec 31 '19 22:12 weierophinney

@elie29 indeed. Another option would be to add support for "PHP-DI wrapContainer functionality" in https://github.com/elie29/zend-di-config

However to me this seems like a waste of time 😄 . I am still in the orientation phase of adopting Zend Expressive with PHP-DI, so this is not a big issue for me. Will be patient 👍


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434055521

weierophinney avatar Dec 31 '19 22:12 weierophinney

@holtkamp I have been using zend expressive with twig and Json repsonse for almost 2 years. And I have no issues with php di v6


Originally posted by @elie29 at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434057504

weierophinney avatar Dec 31 '19 22:12 weierophinney

@holtkamp PHP-DI 6.0 was released on February XD

The problem is when some factory requires an interop container (that extends psr container from v1.2), but if you use a non interop compatible container, you can't inject it to that factory.

So:

  • you can use an interop container with a factory that requires a psr container (from v1.2)
  • you can't use a psr container with a factory that requires an interop container

If someone want to use old libraries that requires an interop container they should decorate its container to make it compatible.

But probably we are going out of topic here.


Originally posted by @thomasvargiu at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434060225

weierophinney avatar Dec 31 '19 22:12 weierophinney

https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434057504 Ok, I believe that, this only occurs wih ZendView.

https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434060225 I understand the cause of the issue and I also understand the possible solutions. Since decorating the container did not succeed easily, for now my preferred solution is the simplest: have the PHI-DI Container implement both ContainerInterfaces.

But probably we are going out of topic here.

I agree, let's leave this rest for now 😄. I think I will submit an issue + PR to also "allow" a Psr\Container\ContainerInterface here https://github.com/zendframework/zend-servicemanager/blob/release-2.7.11/src/AbstractPluginManager.php#L78-L81 to enable "forward" compatibility with pure PSR-11 containers.


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-434077138

weierophinney avatar Dec 31 '19 22:12 weierophinney

The last release of https://github.com/elie29/zend-di-config supports Interop Container in order to be used with zend view.


Originally posted by @elie29 at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-435353081

weierophinney avatar Dec 31 '19 22:12 weierophinney

So, 3 days ago the container-interop package was marked as abandonded (see https://github.com/container-interop/container-interop/issues/94 ). Will zend-servicemanager do this BC with allowing PSR\Cotntainer\ContainerInterface in FactoryInterface?

Please no hack or new library etc. Walkarounds are ot useful for a problem, which exists since more than 2 years.


Originally posted by @dpauli at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-557468948

weierophinney avatar Dec 31 '19 22:12 weierophinney

Please no hack or new library etc. Walkarounds are ot useful for a problem, which exists since more than 2 years.

What would you suggest as a (backward compatible) solution?


Originally posted by @holtkamp at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-557470249

weierophinney avatar Dec 31 '19 22:12 weierophinney

@dpauli We will be doing a new major release that targets the final specification; it has to be a major release, though, because it changes signatures for any code that extends our classes or implements our interfaces. (I mean, the actual signature remains the same, it's the inheritance tree that changes, but PHP doesn't make a distinction there.)

This will not happen until we transition to Laminas (which is soon). We've not done it sooner as it affects any components that provide plugin managers as well, which means those will also require new major versions. As such, it's a non-trivial change for both our libraries and our end-users.


Originally posted by @weierophinney at https://github.com/zendframework/zend-servicemanager/issues/184#issuecomment-557541762

weierophinney avatar Dec 31 '19 22:12 weierophinney

@weierophinney you handled the original discussion on this topic, do you have any suggestions on how we should proceed with this please?

GeeH avatar Jun 08 '20 10:06 GeeH

Since I opened that issue originally and it still annoys me from time to time. Yes please proceed on this one.

RalfEggert avatar Jun 08 '20 10:06 RalfEggert

I don't really understand what needs to be done here. @RalfEggert would you be open to creating an initial PR on this please?

GeeH avatar Jun 08 '20 10:06 GeeH

Well, the FactoryInterface is still using Interop\Container\ContainerInterface that should be changed to Psr\Container\ContainerInterface.

https://github.com/laminas/laminas-servicemanager/blob/master/src/Factory/FactoryInterface.php#L11

The Interop\Container\ContainerInterface already just extends Psr\Container\ContainerInterface, so a namespace change should solve it. But I am not sure if that Interop Interface is not used somewhere else.

https://github.com/container-interop/container-interop/blob/master/src/Interop/Container/ContainerInterface.php

RalfEggert avatar Jun 08 '20 11:06 RalfEggert

@RalfEggert There is an RFC regarding this issue. It was opened by @weierophinney and if you have some things to add, feel free to comment on discourse 👍

Sadly, its not that trivial than just replacing the interfaces.

boesing avatar Jun 08 '20 16:06 boesing

Thanks @boesing. Actually I have nothing to add. Thanks for pointing out.

RalfEggert avatar Jun 26 '20 09:06 RalfEggert

The problem, as I noted, is that any factories implementing FactoryInterface would break on the next update, as the signature technically changes (the typehint changes, essentially). As such, a new major version is needed for switching to the PSR-11 interfaces.

weierophinney avatar Jun 26 '20 14:06 weierophinney

Any updates on this? I've created a feature request in Laminas/ApiTools/Doctrine to update usages but realized this would also need updating.

https://github.com/laminas-api-tools/api-tools-doctrine/issues/27

javabudd avatar Jul 13 '21 15:07 javabudd

@javabudd yes, there is some information here and here.

TBH, bumping the servicemanager to a new major will need a huge amount of work in downstream components. We are currently lacking time and developers to handle such a largely coordinated release just for the sake of getting rid of an abandoned interface. Just because its abandoned doesn't mean that it stops working anytime soon.

Due to the lack of final in most factories, a removal of the interop interface in several packages is not possible as well.

boesing avatar Jul 13 '21 15:07 boesing