Must include `->name()` when dispatching actions as jobs if `->onOneServer()` is specified

[travisaustin]

If you attempt to schedule more than one Laravel Action as a job dispatched from the scheduler, only the first job will fire if the option onOneServer() is selected.

To replicate this issue:

1) Create two new Laravel Actions (I've called them App\Actions\FirstLaravelActionAsAJob and App\Actions\SecondLaravelActionAsAJob) 2) Configure the Laravel Scheduler as follows:

<?php

namespace App\Console;

use App\Actions\FirstLaravelActionAsAJob;
use App\Actions\SecondLaravelActionAsAJob;
use Illuminate\Console\Scheduling\Schedule;
use Illuminate\Foundation\Console\Kernel as ConsoleKernel;
use Illuminate\Support\Facades\DB;

class Kernel extends ConsoleKernel
{
    /**
     * Define the application's command schedule.
     *
     * @param  \Illuminate\Console\Scheduling\Schedule  $schedule
     * @return void
     */
    protected function schedule(Schedule $schedule)
    {
        $schedule->job(FirstLaravelActionAsAJob::makeJob())
            ->everyMinute()
            ->onOneServer();

        $schedule->job(SecondLaravelActionAsAJob::makeJob())
            ->everyMinute()
            ->onOneServer();
    }
}

3) Execute the following command: php artisan schedule:run

The output of the schedule:run will be:

[2022-01-25T19:26:05+00:00] Running scheduled command: Lorisleiva\Actions\Decorators\JobDecorator
Skipping command (has already run on another server): Lorisleiva\Actions\Decorators\JobDecorator

Notice the second job was not dispatched.

The fix for this is simple, but it would be great if this fix wasn't necessary. Simply add a ->name() parameter to each job in the scheduler, like this:

<?php

namespace App\Console;

use App\Actions\FirstLaravelActionAsAJob;
use App\Actions\SecondLaravelActionAsAJob;
use Illuminate\Console\Scheduling\Schedule;
use Illuminate\Foundation\Console\Kernel as ConsoleKernel;
use Illuminate\Support\Facades\DB;

class Kernel extends ConsoleKernel
{
    /**
     * Define the application's command schedule.
     *
     * @param  \Illuminate\Console\Scheduling\Schedule  $schedule
     * @return void
     */
    protected function schedule(Schedule $schedule)
    {
        $schedule->job(FirstLaravelActionAsAJob::makeJob())
            ->name(FirstLaravelActionAsAJob::class)
            ->everyMinute()
            ->onOneServer();

        $schedule->job(SecondLaravelActionAsAJob::makeJob())
            ->name(SecondLaravelActionAsAJob::class)
            ->everyMinute()
            ->onOneServer();
    }
}

You can see that I used the name of the class as the name for the dispatched job, but you can use any text you want in the ->name() function, so long as the name is unique to the job.

When you run php artisan schedule:run again, you will see that both jobs did run:

[2022-01-25T19:34:25+00:00] Running scheduled command: App\Actions\FirstLaravelActionAsAJob
[2022-01-25T19:34:25+00:00] Running scheduled command: App\Actions\SecondLaravelActionAsAJob

[lorisleiva]

Hi there 👋

Thanks for raising this. I've had a look and I think there's a way to automate this on the package.

Take a look at the job method on the Schedule class.

public function job($job, $queue = null, $connection = null)
{
    return $this->call(function () use ($job, $queue, $connection) {
        $job = is_string($job) ? Container::getInstance()->make($job) : $job;

        if ($job instanceof ShouldQueue) {
            $this->dispatchToQueue($job, $queue ?? $job->queue, $connection ?? $job->connection);
        } else {
            $this->dispatchNow($job);
        }
    })->name(is_string($job) ? $job : get_class($job));
}

Line 4 will try to resolve the Job from the container if it is given as a string.

$job = is_string($job) ? Container::getInstance()->make($job) : $job;

So far Laravel Actions does not have a DesignPattern class for jobs because it didn't need to but if I were to add one and we would give the job as a string instead of using makeJob() then this would fix the issue.

[lorisleiva]

Okay, I've tried to implement that today and it's more problematic than I anticipated because Jobs can, of course, have parameters and therefore the fix I suggested above will only work for jobs without parameters.

Thus, the only way to fix this is to have a better default name on the framework which, at the moment, is using the class name of the job decorator.

->name(is_string($job) ? $job : get_class($job));

But I don't think it's the framework's responsibility to handle that so I think we're a bit stuck I'm afraid.

For now, using the name method explicitly is the best fix.

[travisaustin]

Thanks for checking it out!