DI with job Invokable classes

[jenky]

https://chevere.org/packages/workflow.html#example-with-invokable-class-dependencies

It doesn’t work properly when the properties have default values.

With an object default

public function __construct(
    private readonly LoggerInterface $logger = new NullLogger(),
) {
}

[Chevere\Container\Exceptions\ContainerException]                                                              
  [logger]: Failed to instantiate Psr\Log\LoggerInterface: Cannot instantiate interface Psr\Log\LoggerInterface

With primitive default values

public function __construct(
      private readonly LoggerInterface $logger,
      private readonly string $channel = 'default',
  ) {
  }

[OutOfBoundsException]                         
  Dependency `channel` not defined in container

I’m not sure whether this is a bug, but I think the container shouldn’t be required to bind values that already have defaults.
If you have multiple containers for different workflow groups, this becomes quite repetitive, especially as the class dependencies grow later.

Another thing I’d suggest is that the container might already contain the invokable class definition.

if (is_string($action)) {
      $dependencies = $this->run->workflow()->dependencies()->extract(
          $action,
          $this->run->container()
      );
      /** @var ActionInterface $action */
      $action = new $action(...$dependencies);
  }

We could try resolving $action through the container before extracting dependencies:

if (is_string($action)) {
  if ($this->run->container()->has($action)) {
    $action = $this->run->container()->get($action);
  }
// ...
}

This might look like a hack because you can pass the invokable object directly. However, it would mean you need to instantiate your invokable with all dependencies before passing it to the workflow, which defeats the purpose of using the container.

In reality, some compiled containers (e.g., Symfony) can create a service locator (a separate lazy-loaded container that provides access to a set of predefined services while instantiating them only when actually needed) that contains all invokable action definitions. Then I can keep using invokable class names in the workflow sync/async and simply pass the locator as the container to run().

[rodber]

It doesn’t work properly when the properties have default values.

Opened a fix in chevere/container: chevere/container#2

In reality, some compiled containers (e.g., Symfony) can create a....

Chevere Workflow works with any PSR-11 container. If chevere/container doesn't fit your needs, you can bring your own, including a Symfony service locator or any other compliant implementation.

---

Another thing I’d suggest is that the container might already contain the invokable class definition.

True. There may also be cases where the constructor runs something expensive that you'd want to reuse rather than reconstruct on every job execution. I'll add support for this, if the container has an entry for the action class name, it will be resolved from there before falling back to dependency extraction.

[jenky]

I think the action should be check if it is callable. This is what I've done

if (is_string($action) && $this->run->container()->has($action)) {
    $service = $this->run->container()->get($action);

    if (is_callable($service)) {
        $action = $service;
    }
}
if (is_string($action)) {
    $dependencies = $this->run->workflow()->dependencies()->extract(
        $action,
        $this->run->container()
    );
    /** @var ActionInterface $action */
    $action = new $action(...$dependencies);
}

Also the Dependencies need to update the assert method to bypass the invokable class parameters check

public function assert(ContainerInterface $container): void
{
    foreach ($this->map as $className => $parameters) {
        if ($container->has($className)) {
            $this->parameters = $this->parameters->without(...$parameters->keys());
        }
    }

    // ....
}

[rodber]

I think the action should be check if it is callable.

No. When a string reaches that point it is a invokable class because the job converted anything else to a closure. Your callable check is double checking something that another system already did.

[rodber]

Also the Dependencies need to update the assert method to bypass the invokable class parameters check

Yes, but that surfaces a problem that happens before. The extract method is calling autoInject, which ignored the default values at all. You can check this commit which fixed the issue: chevere/container@797ace5

Autoinject also needed to ignore interface names as it was trying to initiate these objects. Now it will skip these: chevere/container@854f929

[rodber]

Pushed a new tag for chevere/container, please give it a try.

[jenky]

chevere/container@797ace5

This fixed the issue with default values

chevere/container@854f929

But this one, it won't work because I don't use your container. I have my own PSR 11 container that binds all the invokable classes. However the Dependencies still extract the parameters and then assert them. So I think the bypass should still needed, whether in extract or assert. If you allow the container to get the invokable action, then why bother collect their parameters and validate something that already done by the container itself.

[rodber]

If you allow the container to get the invokable action, then why bother collect their parameters and validate something that already done by the container itself.

... Because you asked for that. I never pass a job action to the container but you asked for it "it should take the bla bla".

Anyway, thanks.