venture
venture copied to clipboard
Issue deleting Eloquent models in non Venture jobs on sync driver
This is a follow on from a previously closed/stale issue #65 as I am also experiencing the issue.
I have created a minimal repository on a fresh Laravel application with one test, and one job that simply deletes the user. This job does not extend WorkflowableJob or interacts with Laravel Venture.
As discussed #65 it just seems to be when delete/forceDelete
on Eloquent models that are passed in to jobs, and using QUEUE_CONNECTION=sync
I think the issue only occurs when forceDeleting
as this is what's happening in my application. All other jobs that do soft deleting inside jobs seem to be fine. This minimal repository, the User
model does not implement SoftDeletes
and so it's failing when just using ->delete()
Here is the minimal repository
laravel/laravel: v10.45.1
sassnowski/venture: v5.2.0
PHP 8.1.27
After forking you will need to run the test suit php artisan test
and you should expect this simple job to fail.
Normal Laravel job (not Workflowable job)
Test fails
As described in #65 seems to be an issue with the Base64WorkflowSerializer.php
when using QUEUE_CONNECTION=sync
, but this Base64WorkflowSerializer` is interfering with non Venture jobs :/
Can confirm it's just when the model is force deleted.
The results of soft deleting means the job is ran without issue, as the job is performed. Test is failing but that's because it found a soft deleted record.
@simple-hacker Can confirm this is indeed an issue, as the WorkflowEventSubscriber
doesn't differentiate between non-Venture workflow steps and regular queued jobs, which means it will always unserialize all queued jobs payload's regardless of it being a workflow step or not:
https://github.com/ksassnowski/venture/blob/73cdd0f05b32f161104d30f4e61c103b16845074/src/WorkflowEventSubscriber.php#L82-L91
https://github.com/ksassnowski/venture/blob/73cdd0f05b32f161104d30f4e61c103b16845074/src/WorkflowEventSubscriber.php#L96-L105
Only thing I can think of atm to prevent this would be to prevent unserializing the job if the job doesn't implement the WorkflowableJob
interface:
/**
* @param Closure(WorkflowableJob): void $callback
*/
private function withWorkflowJob(
JobProcessing|JobProcessed|JobFailed $event,
Closure $callback,
): void {
+ $jobName = $event->job->payload()['data']['commandName'];
+ if (! isset(class_implements($jobName)[WorkflowableJob::class])) {
+ return;
+ }
$jobInstance = $this->jobExtractor->extractWorkflowJob($event->job);
if (null !== $jobInstance) {
$callback($jobInstance);
}
}
Though this is indeed a weird edge-case, since the SerializesModels
trait is sort of the culprit here, as it listens for when the job is unserialized and attempts a findOrFail
on the deleted user. Unfortunately the Ignore Missing Models feature doesn't resolve this issue either (I've tested locally):
class DeleteUser ...
{
/**
* Delete the job if its models no longer exist.
*
* @var bool
*/
public $deleteWhenMissingModels = true;