Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting interface in the constructor gives InvalidDefinition error #657

Closed
dingo-d opened this issue May 9, 2019 · 2 comments
Closed

Setting interface in the constructor gives InvalidDefinition error #657

dingo-d opened this issue May 9, 2019 · 2 comments

Comments

@dingo-d
Copy link

dingo-d commented May 9, 2019

I'm trying to use php-di in my WordPress plugin. In my plugin class I have tried with

class Plugin {
    public function register() : void {
      add_action( 'plugins_loaded', [ $this, 'register_services' ] );
    }

  public function register_services() {
    $container = new \DI\Container();

    $this->services = array_map(
      function( $class ) use ( $container ) {
        return $container->get( $class );
      },
      $this->get_service_classes()
    );

    array_walk(
      $this->services,
      function( $class ) {
        $class->register();
      }
    );
  }
}

In my main plugin file I'm calling the (new Plugin())->register() method via plugin factory to kickstart it all.

The get_service_classes method returns array of classes that need to be instantiated and autowired. For instance:

private function get_service_classes() : array {
    return [
      My_Plugin\Admin\Admin_View::class,
      // ...
    ];
}

In one example I have

final class Admin_View implements Service {
  private $plugin_options;

  public function __construct( Plugin_Options $plugin_options ) {
    $this->plugin_options = $plugin_options;
  }
}

This works because I'm passing the exact class Plugin_Options as the constructor type hint (I've set the use My_Plugin\Options\Plugin_Options above the class definition).

But, this also tightly couples my code, because I'm depending on concretes not abstracts (interface).

So, since my Plugin_Options class implements the same Service interface as 99.8% of my classes (service interface only has one public method register for registering hooks in WordPress), I tried to type hint against the Service interface

final class Admin_View implements Service {
  private $plugin_options;

  public function __construct( Service $plugin_options ) {
    $this->plugin_options = $plugin_options;
  }
}

This threw an error:

[09-May-2019 13:35:01 UTC] PHP Fatal error: Uncaught DI\Definition\Exception\InvalidDefinition: Entry "My_Plugin\Admin\Admin_View" cannot be resolved: Entry "My_Plugin\Core\Service" cannot be resolved: the class is not instantiable
Full definition:
Object (
class = #NOT INSTANTIABLE# My_Plugin\Core\Service
lazy = false
)
Full definition:
Object (
class = My_Plugin\Admin\Admin_View
lazy = false
__construct(
$plugin_options = get(My_Plugin\Core\Service)
)
) in /public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Exception/InvalidDefinition.php:18
Stack trace:
#0 /public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Resolver/ObjectCreator.php(155): DI\Definition\Exception\InvalidDefinition::create(Object(DI\Definition\ObjectDefinition), 'Entry "My_Plugi...')
#1 /public_html/wp-content/plugins/my-plugin/vend in /public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Exception/InvalidDefinition.php on line 18

The issue is that Service is an interface.

I've tried by defining the ContainerBuilder

public function register_services() {
    // Implement PHP-DI.
    $builder = new ContainerBuilder();

    $builder->addDefinitions(
      [
        Service::class => \DI\autowire( OldAdmin\Admin_View::class ),
      ]
    );

    $container = $builder->build();
    //...
}

But that caused circular definitions error.

[09-May-2019 14:30:31 UTC] PHP Fatal error: Uncaught DI\DependencyException: Circular dependency detected while trying to resolve entry 'My_Plugin\Core\Service' in public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Container.php:371
Stack trace:
#0 public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Container.php(137): DI\Container->resolveDefinition(Object(DI\Definition\AutowireDefinition))
#1 public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Reference.php(53): DI\Container->get('Developer_Porta...')
#2 public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Resolver/ResolverDispatcher.php(59): DI\Definition\Reference->resolve(Object(DI\Container))
#3 public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Definition/Resolver/ParameterResolver in public_html/wp-content/plugins/my-plugin/vendor/php-di/php-di/src/Container.php on line 371

Is the problem that I'm using the same interface in most places? Should I create separate interface and then pass this one (even thought that interface may just implement the Service interface)?
Or am I doing something wrong completely? I've read the documentation but had no luck so far :/

Additional info

PHP Version: 7.2.14
php-di Version: 6.0.8

@SvenRtbg
Copy link
Contributor

SvenRtbg commented May 9, 2019

Your Admin_View implements Service and requires Service as a constructor parameter that is implemented by Admin_View?

How would you try to resolve this manually?
$admin_view = new Admin_View(new Admin_View(new Admin_View(...)));

I'd suggest that you try to write code without PHP-DI that resolves your class injection manually for one case first. That way you'll easily find why you are unintentionally creating circles. As a result you'll be able to create the interfaces and classes properly, and PHP-DI will work then.

@dingo-d
Copy link
Author

dingo-d commented May 9, 2019

Yeah, I've tried to resolve this issue for a couple of hours without luck. I guess I'll have to rewrite the Plugin_Options class so that I can inject it properly in the Admin_View class.

I'm doing a refactor of an old code I wrote and I'm trying to write everything with proper OOP. That's why I'm looking into a DI container, to make my life a bit easier. Both of those classes are not refactored (yet) 😬

Well back to the drawing board for me 😅

@dingo-d dingo-d closed this as completed May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants