Venture support for decorated jobs

[ksassnowski]

Wall of text incoming!

Hi! I'm the maintainer of Venture.

I'm currently trying to integrate this package into a project that's also using Venture to create workflows from jobs. There are a couple of incompatibilities between the two packages that I've encountered that I hope we can work out.

Decorated jobs don't implement a __call method

First, some context. Every job used in a Venture workflow needs to use the WorkflowStep trait provided by the package. This trait adds a couple of public methods and fields that Venture then uses internally to configure the job object, as can be seen here.

The JobDecorator class obviously doesn't use this trait and as such doesn't expose these methods. This leads to an error when trying to add a decorated job to a workflow like so.

Workflow::define('My workflow')
    // Blows up because Venture tries to call a `withStepId` method on the
    // `JobDecorator` class.
    ->addJob(MyJob::makeJob());

A possible solution to this problem (and one that I've already tested) is to add a __call method to the JobDecorator and UniqueJobDecorator classes. This method will simply forward the call to the decorated action.

public function __call(string $method, array $args): mixed
{
    return $this->getAction()->{$method}(...$args);
}

Decorated Jobs lead to DuplicateJobException

The next problem is a little trickier. Venture assigns an id to every job inside a workflow in order to build up the dependency graph. If no explicit id is provided by the user, Venture will default to using the class name of the provided job instance.

Workflow::define('My workflow')
    // No explicit id is provided, so Venture will call `get_class($job)` internally
    // and use that as the job's id. In this case, this would be something like
    // App\Jobs\Job1 .
    ->addJob(new Job1())

    // This will always use `job2-id` as the job's id, since it was explicitly
    // provided by the user.
    ->addJob(new Job2(), id: 'job2-id')

The job id has to be unique within the workflow, so dependencies can be unambiguously resolved. Otherwise, something like this could happen

Workflow::define('My workflow')
     // Neither `Job1` job has an explicit id, so the would both 
     // fall back to `get_class($job)`...
    ->addJob(new Job1('some-param'))
    ->addJob(new Job1('some-other-param'))
    // Wait, which `Job1` are you referring to?
    ->addJob(new Job2(), dependencies: [Job1::class])

For this reason, Venture throws an exception when trying to add a job with a duplicate id to the same workflow. Which finally leads me to the problem in integrating our two packages.

When calling the makeJob static method on an action, the result will always be an instance of JobDecorator or UniqueJobDecorator. This becomes problematic when a user is trying to multiple actions as jobs in a workflow like so:

Workflow::define('My workflow')
    ->addJob(MyJob::makeJob())
    // Boom. A job with id `JobDecorator` already exists in this workflow.
    ->addJob(MyOtherJob::makeJob());

One possible workaround is, of course, to provide explicit ids for every single job. However, that's pretty horrible DX in my opinion.

This isn't really a problem with either package and I'm not quite sure what the solution to this is. I'm just putting this out there in case you (or someone else) has an idea on how to tackle this. I'd like to come up with a solution that wouldn't cause either package to have to know about the inner working of the other package. Ideally, each package wouldn't even know that the other one exists. The solution might just be to create a separate package that acts like a bridge between the two packages which a user would have to install if they want to use both Venture and Laravel Actions together.

---

Anyways, let me know what you think. I'd be happy to contribute PRs for the __call method if you decide that this is a good solution.

Cheers!

[shealan]

I'd just like to add that both these packages are incredible, thank you @ksassnowski and @lorisleiva for all your hard work. If we can get them working in harmony together then the possibilities are endless!

[ksassnowski]

I managed to get the two libraries playing together in one of our projects at work. It required a few changes in Venture to be able to override certain behaviors. The good thing is, that I didn't really have to change this package, although I did have to add decorators for both the JobDecorator and UniqueJobDecorator classes so I could add the WorkflowStep trait to them.

So ideally, the only change needed in this package is to make the decorator classes configurable that an action gets wrapped in when calling ::makeJob. Everything else could be done in a venture-laravel-actions-bridge package that I would write.

What do you think, @lorisleiva?

[lorisleiva]

Hi there 👋

Sorry for the late reply, I wanted to make sure I had enough time to allocate to this issue to read it properly.

I think the best way to approach this would be to implement a VentureJobDecorator that extends the JobDecorator and uses the WorkflowStep trait. Since the VentureJobDecorator would have access to the underlying $action you could implement the required methods so they delegate directly to the action. Something like that:

<?php
class VentureJobDecorator extends JobDecorator
{
    use WorkflowStep;

    public function withStepId()
    {
        // This will first try $this->action->getVentureStepId().
        // Otherwise, it will try $this->action->ventureStepId.
        // Otherwise, it will fallback to get_class($this->action).
        return $this->fromActionMethodOrProperty('getVentureStepId', 'ventureStepId', get_class($this->action));
    }
}

Then, you could offer a AsVentureJob trait that would provide a makeVentureJob static method that would wrap the action in a VentureJobDecorator instead of a typical JobDecorator.

trait AsVentureJob
{
    public static function makeVentureJob(...$arguments): VentureJobDecorator
    {
        return new VentureJobDecorator(static::class, ...$arguments);
    }
}

I hope this helps. 🍀

[lorisleiva]

Closing this due to inactivity. Feel free to reopen if you have more questions. 🙂

[shealan]

Sorry @lorisleiva I forgot to reply. I'm looking forward to trying this approach out. Thank you!