[4.x] Fixes jobs saying "pending" forever (#1349)

* Fixes "pending" job status

* Apply fixes from StyleCI

* Style

* style

* Test failed updates

* Fixes contract

* More tests

* Fix type

* Fixes tests

* formatting

* formatting

---------

Co-authored-by: StyleCI Bot <bot@styleci.io>
Co-authored-by: Taylor Otwell <taylor@laravel.com>

src/Contracts/EntriesRepository.php

@@ -34,10 +34,10 @@ public function get($type, EntryQueryOptions $options);
     public function store(Collection $entries);
 
     /**
-     * Store the given entry updates.
+     * Store the given entry updates and return the failed updates.
      *
      * @param  \Illuminate\Support\Collection|\Laravel\Telescope\EntryUpdate[]  $updates
-     * @return void
+     * @return \Illuminate\Support\Collection|null
      */
     public function update(Collection $updates);
 

src/Jobs/ProcessPendingUpdates.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace Laravel\Telescope\Jobs;
+
+use Illuminate\Bus\Queueable;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Foundation\Bus\Dispatchable;
+use Illuminate\Queue\InteractsWithQueue;
+use Illuminate\Queue\SerializesModels;
+use Laravel\Telescope\Contracts\EntriesRepository;
+
+class ProcessPendingUpdates implements ShouldQueue
+{
+    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
+
+    /**
+     * The pending entry updates.
+     *
+     * @var \Illuminate\Support\Collection<int, \Laravel\Telescope\EntryUpdate>
+     */
+    public $pendingUpdates;
+
+    /**
+     * The number of times the job has been attempted.
+     *
+     * @var int
+     */
+    public $attempt;
+
+    /**
+     * Create a new job instance.
+     *
+     * @param  \Illuminate\Support\Collection<int, \Laravel\Telescope\EntryUpdate>
+     * @param  int  $attempt
+     * @return void
+     */
+    public function __construct($pendingUpdates, $attempt = 0)
+    {
+        $this->pendingUpdates = $pendingUpdates;
+        $this->attempt = $attempt;
+    }
+
+    /**
+     * Execute the job.
+     *
+     * @param  \Laravel\Telescope\Contracts\EntriesRepository  $repository
+     * @return void
+     */
+    public function handle(EntriesRepository $repository)
+    {
+        $this->attempt++;
+
+        $repository->update($this->pendingUpdates)->whenNotEmpty(
+            fn ($pendingUpdates) => static::dispatchIf(
+                $this->attempt < 3, $pendingUpdates, $this->attempt
+            )->delay(now()->addSeconds(10)),
+        );
+    }
+}

src/Storage/DatabaseEntriesRepository.php

@@ -201,20 +201,24 @@ protected function storeTags(Collection $results)
     }
 
     /**
-     * Store the given entry updates.
+     * Store the given entry updates and return the failed updates.
      *
      * @param  \Illuminate\Support\Collection|\Laravel\Telescope\EntryUpdate[]  $updates
-     * @return void
+     * @return \Illuminate\Support\Collection|null
      */
     public function update(Collection $updates)
     {
+        $failedUpdates = [];
+
         foreach ($updates as $update) {
             $entry = $this->table('telescope_entries')
                             ->where('uuid', $update->uuid)
                             ->where('type', $update->type)
                             ->first();
 
             if (! $entry) {
+                $failedUpdates[] = $update;
+
                 continue;
             }
 
@@ -229,6 +233,8 @@ public function update(Collection $updates)
 
             $this->updateTags($update);
         }
+
+        return collect($failedUpdates);
     }
 
     /**

src/Telescope.php

@@ -7,11 +7,13 @@
 use Illuminate\Contracts\Debug\ExceptionHandler;
 use Illuminate\Log\Events\MessageLogged;
 use Illuminate\Support\Arr;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Str;
 use Illuminate\Support\Testing\Fakes\EventFake;
 use Laravel\Telescope\Contracts\EntriesRepository;
 use Laravel\Telescope\Contracts\TerminableRepository;
+use Laravel\Telescope\Jobs\ProcessPendingUpdates;
 use Throwable;
 
 class Telescope
@@ -664,7 +666,11 @@ public static function store(EntriesRepository $storage)
                 $batchId = Str::orderedUuid()->toString();
 
                 $storage->store(static::collectEntries($batchId));
-                $storage->update(static::collectUpdates($batchId));
+
+                ($storage->update(static::collectUpdates($batchId)) ?: Collection::make())
+                    ->whenNotEmpty(fn ($pendingUpdates) => rescue(fn () => ProcessPendingUpdates::dispatch(
+                        $pendingUpdates,
+                    )->delay(now()->addSeconds(10))));
 
                 if ($storage instanceof TerminableRepository) {
                     $storage->terminate();

src/Watchers/JobWatcher.php

@@ -19,6 +19,16 @@
 
 class JobWatcher extends Watcher
 {
+    /**
+     * The list of ignored jobs classes.
+     *
+     * @var array<int, class-string>
+     */
+    protected $ignoredJobClasses = [
+        \Laravel\Scout\Jobs\MakeSearchable::class, // @phpstan-ignore-line
+        \Laravel\Telescope\Jobs\ProcessPendingUpdates::class,
+    ];
+
     /**
      * Register the watcher.
      *
@@ -49,8 +59,7 @@ public function recordJob($connection, $queue, array $payload)
             return;
         }
 
-        // Logging this job can cause extensive memory usage...
-        if (get_class($payload['data']['command']) === 'Laravel\Scout\Jobs\MakeSearchable') {
+        if (in_array(get_class($payload['data']['command']), $this->ignoredJobClasses)) {
             return;
         }
 

tests/Jobs/ProcessPendingUpdatesTest.php

@@ -0,0 +1,87 @@
+<?php
+
+namespace Laravel\Telescope\Tests\Storage;
+
+use Illuminate\Support\Facades\Bus;
+use Laravel\Telescope\Contracts\EntriesRepository;
+use Laravel\Telescope\Jobs\ProcessPendingUpdates;
+use Laravel\Telescope\Tests\FeatureTestCase;
+use Mockery as m;
+
+class ProcessPendingUpdatesTest extends FeatureTestCase
+{
+    public function test_pending_updates()
+    {
+        Bus::fake();
+
+        $pendingUpdates = collect([
+            ['id' => 1, 'content' => 'foo'],
+            ['id' => 2, 'content' => 'bar'],
+        ]);
+
+        $failedUpdates = collect();
+        $repository = m::mock(EntriesRepository::class);
+
+        $repository
+            ->shouldReceive('update')
+            ->once()
+            ->with($pendingUpdates)
+            ->andReturn($failedUpdates);
+
+        (new ProcessPendingUpdates($pendingUpdates))->handle($repository);
+
+        Bus::assertNothingDispatched();
+    }
+
+    public function test_pending_updates_may_stay_pending()
+    {
+        Bus::fake();
+
+        $pendingUpdates = collect([
+            ['id' => 1, 'content' => 'foo'],
+            ['id' => 2, 'content' => 'bar'],
+        ]);
+        $failedUpdates = collect([
+            $pendingUpdates->get(1),
+        ]);
+
+        $repository = m::mock(EntriesRepository::class);
+
+        $repository
+            ->shouldReceive('update')
+            ->once()
+            ->with($pendingUpdates)
+            ->andReturn($failedUpdates);
+
+        (new ProcessPendingUpdates($pendingUpdates))->handle($repository);
+
+        Bus::assertDispatched(ProcessPendingUpdates::class, function ($job) {
+            return $job->attempt == 1 && $job->pendingUpdates->toArray() == [['id' => 2, 'content' => 'bar']];
+        });
+    }
+
+    public function test_pending_updates_may_stay_pending_only_three_times()
+    {
+        Bus::fake();
+
+        $pendingUpdates = collect([
+            ['id' => 1, 'content' => 'foo'],
+            ['id' => 2, 'content' => 'bar'],
+        ]);
+        $failedUpdates = collect([
+            $pendingUpdates->get(1),
+        ]);
+
+        $repository = m::mock(EntriesRepository::class);
+
+        $repository
+            ->shouldReceive('update')
+            ->once()
+            ->with($pendingUpdates)
+            ->andReturn($failedUpdates);
+
+        (new ProcessPendingUpdates($pendingUpdates, 2))->handle($repository);
+
+        Bus::assertNothingDispatched();
+    }
+}

tests/Storage/DatabaseEntriesRepositoryTest.php

@@ -3,6 +3,7 @@
 namespace Laravel\Telescope\Tests\Storage;
 
 use Laravel\Telescope\Database\Factories\EntryModelFactory;
+use Laravel\Telescope\EntryUpdate;
 use Laravel\Telescope\Storage\DatabaseEntriesRepository;
 use Laravel\Telescope\Tests\FeatureTestCase;
 
@@ -24,4 +25,21 @@ public function test_find_entry_by_uuid()
         // Why is sequence always null? DatabaseEntriesRepository::class#L60
         $this->assertNull($result['sequence']);
     }
+
+    public function test_update()
+    {
+        $entry = EntryModelFactory::new()->create();
+
+        $repository = new DatabaseEntriesRepository('testbench');
+
+        $result = $repository->find($entry->uuid)->jsonSerialize();
+
+        $failedUpdates = $repository->update(collect([
+            new EntryUpdate($result['id'], $result['type'], ['content' => ['foo' => 'bar']]),
+            new EntryUpdate('missing-id', $result['type'], ['content' => ['foo' => 'bar']]),
+        ]));
+
+        $this->assertCount(1, $failedUpdates);
+        $this->assertSame('missing-id', $failedUpdates->first()->uuid);
+    }
 }