$deleteWhenMissingModels on Jobs is ignored when Models use SoftDeletes trait

olml89 commented 9 months ago

Laravel Version

10.3.2

PHP Version

8.2.5

Database Driver & Version

No response

Description

When using the SoftDeletes trait, deleted models are ignored in Jobs which are using the $deleteWhenMissingModels = true feature, and the Jobs are still processed when they should be discarded.

The reason is that SoftDeletes trait is only taken into account when using Eloquent, but Jobs use the DB query builder to retrieve the serialized Models from the database, without checking the possible deleted_at column:

https://github.com/laravel/framework/blob/d7616a176afc641e9693266920cb11e84c352241/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php#L119

https://github.com/laravel/framework/blob/d7616a176afc641e9693266920cb11e84c352241/src/Illuminate/Database/Eloquent/Model.php#L1555

Possible fix

One possible way to solve it is to overwrite the newQueryForRestoration from Models on the SoftDeletes trait, making it check that the deleted_at column is null:

    /**
     * Get a new query to restore one or more models by their queueable IDs.
     *
     * @param  array|int  $ids
     * @return \Illuminate\Database\Eloquent\Builder
     */
    public function newQueryForRestoration($ids): Builder
    {
        return $this
            ->newQueryWithoutScopes()
            ->whereKey($ids)
            ->whereNull($this->getDeletedAtColumn());
    }

Steps To Reproduce

    class ExampleModel extends \Illuminate\Database\Eloquent\Model
    {
        use \Illuminate\Database\Eloquent\SoftDeletes;
    }

    final class Job implements \Illuminate\Contracts\Queue\ShouldQueue
    {
        public bool $deleteWhenMissingModels = true;

        public function __construct(
            public readonly ExampleModel $exampleModel,
        ) {
        }
    }

If we create a Job injecting an ExampleModel, and then we delete the model and dispatch job:

    $model = new ExampleModel;
    $job = new Job($exampleModel);

    $model->delete();

    dispatch($job);

The job should be automatically discarded, but it is processed anyway.

Repository demonstrating the issue

A minimal reproducible example is available here.

The example is dockerized. To set the docker container up, run:

docker-compose -up -d

Then, prepare the database running the needed migrations:

php artisan migrate

To test the issue:

php artisan test --filter=JobTest

This test should pass, but it fails, because the Job is handled instead of being discarded.

github-actions[bot] commented 9 months ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

olml89 commented 9 months ago

I have refactored JobTest to add a comparison with the behaviour of no soft-deletable Models.

Also, I have submitted a PR to fix the issue: https://github.com/laravel/framework/pull/49807

driesvints commented 9 months ago

Hi there, I'm sorry but it seems we're not going to take any action on this right now. Please see taylor's response here: https://github.com/laravel/framework/pull/49807#issuecomment-1908241983

laravel / framework