Fix loaded relation index lookups in StoredWorkflow (#361)

Author: rmcdaniel

src/Models/StoredWorkflow.php

@@ -146,7 +146,7 @@ public function findLogByIndex(int $index, bool $fresh = false): ?StoredWorkflow
         if ($this->relationLoaded('logs')) {
             /** @var Collection<int, StoredWorkflowLog> $logs */
             $logs = $this->getRelation('logs');
-            return $logs->firstWhere('index', $index);
+            return $logs->first(static fn (StoredWorkflowLog $log): bool => self::modelHasIndex($log, $index));
         }
 
         return $this->logs()
@@ -197,7 +197,9 @@ public function findTimerByIndex(int $index): ?StoredWorkflowTimer
         if ($this->relationLoaded('timers')) {
             /** @var Collection<int, StoredWorkflowTimer> $timers */
             $timers = $this->getRelation('timers');
-            return $timers->firstWhere('index', $index);
+            return $timers->first(
+                static fn (StoredWorkflowTimer $timer): bool => self::modelHasIndex($timer, $index)
+            );
         }
 
         return $this->timers()
@@ -334,4 +336,12 @@ protected function recursivePrune(self $workflow): void
             $workflow->delete();
         }
     }
+
+    private static function modelHasIndex(Model $model, int $index): bool
+    {
+        // Use raw attributes so loaded relations never fall back to Eloquent's magic relation lookup.
+        $attributes = $model->getAttributes();
+
+        return array_key_exists('index', $attributes) && (int) $attributes['index'] === $index;
+    }
 }

tests/Unit/Models/StoredWorkflowTest.php

@@ -323,6 +323,33 @@ public function testFindLogByIndexUsesLoadedLogsRelation(): void
         $this->assertCount(0, DB::getQueryLog());
     }
 
+    public function testFindLogByIndexDoesNotUseFirstWhereForLoadedLogsRelation(): void
+    {
+        $workflow = StoredWorkflow::create([
+            'class' => 'TestWorkflow',
+            'status' => 'running',
+        ]);
+
+        $log = $workflow->logs()
+            ->create([
+                'index' => 0,
+                'now' => now(),
+                'class' => 'test',
+            ]);
+
+        $workflow->setRelation('logs', new class([$log]) extends \Illuminate\Database\Eloquent\Collection {
+            public function firstWhere($key, $operator = null, $value = null)
+            {
+                throw new \BadMethodCallException('Loaded log lookup should not rely on firstWhere.');
+            }
+        });
+
+        $foundLog = $workflow->findLogByIndex(0);
+
+        $this->assertNotNull($foundLog);
+        $this->assertSame($log->id, $foundLog->id);
+    }
+
     public function testCreateLogSyncsLoadedLogsRelation(): void
     {
         $workflow = StoredWorkflow::create([
@@ -372,6 +399,33 @@ public function testFindTimerByIndexUsesLoadedTimersRelation(): void
         $this->assertCount(0, DB::getQueryLog());
     }
 
+    public function testFindTimerByIndexDoesNotUseFirstWhereForLoadedTimersRelation(): void
+    {
+        $workflow = StoredWorkflow::create([
+            'class' => 'TestWorkflow',
+            'status' => 'running',
+        ]);
+
+        $timer = $workflow->timers()
+            ->create([
+                'index' => 3,
+                'stop_at' => now()
+                    ->addSecond(),
+            ]);
+
+        $workflow->setRelation('timers', new class([$timer]) extends \Illuminate\Database\Eloquent\Collection {
+            public function firstWhere($key, $operator = null, $value = null)
+            {
+                throw new \BadMethodCallException('Loaded timer lookup should not rely on firstWhere.');
+            }
+        });
+
+        $foundTimer = $workflow->findTimerByIndex(3);
+
+        $this->assertNotNull($foundTimer);
+        $this->assertSame($timer->id, $foundTimer->id);
+    }
+
     public function testFindTimerByIndexQueriesWhenTimersRelationIsNotLoaded(): void
     {
         $workflow = StoredWorkflow::create([