diff --git a/.gitignore b/.gitignore index 4d84bc3..c36cea3 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ coverage.xml .phpunit.result.cache .php_cs.cache .php-cs-fixer.cache +.vs \ No newline at end of file diff --git a/src/WorkflowStub.php b/src/WorkflowStub.php index 76b828b..84e33ac 100644 --- a/src/WorkflowStub.php +++ b/src/WorkflowStub.php @@ -381,7 +381,15 @@ private function dispatch(): void ); } - $this->storedWorkflow->status->transitionTo(WorkflowPendingStatus::class); + try { + $this->storedWorkflow->status->transitionTo(WorkflowPendingStatus::class); + } catch (\Spatie\ModelStates\Exceptions\TransitionNotFound $exception) { + $this->storedWorkflow->refresh(); + + if ($this->status() !== WorkflowPendingStatus::class) { + throw $exception; + } + } $dispatch = static::faked() ? 'dispatchSync' : 'dispatch'; diff --git a/tests/.env.feature b/tests/.env.feature index 406cbc8..e6a598c 100644 --- a/tests/.env.feature +++ b/tests/.env.feature @@ -7,6 +7,9 @@ DB_PORT=5432 DB_USERNAME=laravel DB_PASSWORD=laravel +CACHE_DRIVER=redis +CACHE_STORE=redis + QUEUE_CONNECTION=redis QUEUE_FAILED_DRIVER=null diff --git a/tests/Feature/NestedSignalRaceConditionTest.php b/tests/Feature/NestedSignalRaceConditionTest.php new file mode 100644 index 0000000..c6612bd --- /dev/null +++ b/tests/Feature/NestedSignalRaceConditionTest.php @@ -0,0 +1,76 @@ +format('Uu'); + $middleCount = 12; + $leafCount = 3; + $duplicateSignals = 4; + $expectedLeafCount = $middleCount * $leafCount; + + $workflow = WorkflowStub::make(TestNestedSignalParentWorkflow::class); + $workflow->start($runId, $middleCount, $leafCount); + + $creationDeadline = now() + ->addSeconds(30); + $leafIds = []; + while (now()->lt($creationDeadline)) { + $leafIds = StoredWorkflow::query() + ->where('class', TestNestedSignalLeafWorkflow::class) + ->pluck('id') + ->all(); + + if (count($leafIds) === $expectedLeafCount) { + break; + } + + usleep(50000); + } + + $this->assertCount($expectedLeafCount, $leafIds, 'Timed out waiting for all nested leaf workflows'); + + for ($round = 0; $round < $duplicateSignals; $round++) { + foreach ($leafIds as $leafId) { + WorkflowStub::load((int) $leafId)->respond(); + } + } + + $completionDeadline = now() + ->addSeconds(120); + while ($workflow->running() && now()->lt($completionDeadline)) { + usleep(50000); + $workflow->fresh(); + } + + if ($workflow->running()) { + throw new RuntimeException(sprintf( + 'Nested signal run %d did not complete before timeout. Current status: %s', + $runId, + (string) $workflow->status() + )); + } + + $this->assertSame(WorkflowCompletedStatus::class, $workflow->status()); + $this->assertSame([ + 'run_id' => $runId, + 'middle_count' => $middleCount, + 'leaf_count' => $leafCount, + 'resolved_leaf_count' => $expectedLeafCount, + ], $workflow->output()); + } +} diff --git a/tests/Fixtures/TestNestedSignalLeafWorkflow.php b/tests/Fixtures/TestNestedSignalLeafWorkflow.php new file mode 100644 index 0000000..8897680 --- /dev/null +++ b/tests/Fixtures/TestNestedSignalLeafWorkflow.php @@ -0,0 +1,33 @@ +responded = true; + } + + public function execute(int $runId, int $middleIndex, int $leafIndex): Generator + { + $resolved = yield awaitWithTimeout(30, fn (): bool => $this->responded); + + return [ + 'run_id' => $runId, + 'middle_index' => $middleIndex, + 'leaf_index' => $leafIndex, + 'resolved' => $resolved, + ]; + } +} diff --git a/tests/Fixtures/TestNestedSignalMiddleWorkflow.php b/tests/Fixtures/TestNestedSignalMiddleWorkflow.php new file mode 100644 index 0000000..4714e56 --- /dev/null +++ b/tests/Fixtures/TestNestedSignalMiddleWorkflow.php @@ -0,0 +1,29 @@ + (bool) ($result['resolved'] ?? false) + )); + } +} diff --git a/tests/Fixtures/TestNestedSignalParentWorkflow.php b/tests/Fixtures/TestNestedSignalParentWorkflow.php new file mode 100644 index 0000000..6a63cf2 --- /dev/null +++ b/tests/Fixtures/TestNestedSignalParentWorkflow.php @@ -0,0 +1,31 @@ + $runId, + 'middle_count' => $middleCount, + 'leaf_count' => $leafCount, + 'resolved_leaf_count' => array_sum($resolvedPerMiddle), + ]; + } +} diff --git a/tests/Unit/ChildWorkflowStubTest.php b/tests/Unit/ChildWorkflowStubTest.php index 61d51fd..f3d90dc 100644 --- a/tests/Unit/ChildWorkflowStubTest.php +++ b/tests/Unit/ChildWorkflowStubTest.php @@ -4,6 +4,8 @@ namespace Tests\Unit; +use Mockery; +use Spatie\ModelStates\Exceptions\TransitionNotFound; use Tests\Fixtures\TestChildWorkflow; use Tests\Fixtures\TestParentWorkflow; use Tests\TestCase; @@ -89,6 +91,77 @@ public function testLoadsChildWorkflow(): void $this->assertNull($result); } + public function testIgnoresTransitionNotFoundWhenChildResumeThrows(): void + { + $logs = Mockery::mock(); + $logs->shouldReceive('whereIndex') + ->once() + ->with(0) + ->andReturnSelf(); + $logs->shouldReceive('first') + ->once() + ->andReturn(null); + + $childWorkflow = new class() { + public function running(): bool + { + return true; + } + + public function created(): bool + { + return false; + } + + public function resume(): void + { + throw TransitionNotFound::make('running', 'pending', StoredWorkflow::class); + } + + public function completed(): bool + { + return false; + } + + public function startAsChild(...$arguments): void + { + } + }; + + $storedChildWorkflow = Mockery::mock(); + $storedChildWorkflow->shouldReceive('toWorkflow') + ->once() + ->andReturn($childWorkflow); + + $children = Mockery::mock(); + $children->shouldReceive('wherePivot') + ->once() + ->with('parent_index', 0) + ->andReturnSelf(); + $children->shouldReceive('first') + ->once() + ->andReturn($storedChildWorkflow); + + $storedWorkflow = Mockery::mock(); + $storedWorkflow->shouldReceive('logs') + ->once() + ->andReturn($logs); + $storedWorkflow->shouldReceive('children') + ->once() + ->andReturn($children); + + WorkflowStub::setContext([ + 'storedWorkflow' => $storedWorkflow, + 'index' => 0, + 'now' => now(), + 'replaying' => false, + ]); + + ChildWorkflowStub::make(TestChildWorkflow::class); + + $this->assertSame(1, WorkflowStub::getContext()->index); + } + public function testAll(): void { $workflow = WorkflowStub::load(WorkflowStub::make(TestParentWorkflow::class)->id()); diff --git a/tests/Unit/ChildWorkflowTest.php b/tests/Unit/ChildWorkflowTest.php new file mode 100644 index 0000000..49da05b --- /dev/null +++ b/tests/Unit/ChildWorkflowTest.php @@ -0,0 +1,39 @@ +id()); + $storedParent->update([ + 'arguments' => Serializer::serialize([]), + 'status' => WorkflowRunningStatus::class, + ]); + + $storedChild = StoredWorkflow::create([ + 'class' => TestChildWorkflow::class, + 'arguments' => Serializer::serialize([]), + ]); + + $job = new ChildWorkflow(0, now()->toDateTimeString(), $storedChild, true, $storedParent); + + $job->handle(); + + $this->assertSame(1, $storedParent->logs()->count()); + $this->assertSame(WorkflowRunningStatus::class, $storedParent->refresh()->status::class); + } +} diff --git a/tests/Unit/WorkflowStubTest.php b/tests/Unit/WorkflowStubTest.php index fbc2218..fde6f76 100644 --- a/tests/Unit/WorkflowStubTest.php +++ b/tests/Unit/WorkflowStubTest.php @@ -258,6 +258,23 @@ public function testHandlesDuplicateLogInsertionProperly(): void Queue::assertPushed(TestWorkflow::class, 1); } + public function testResumeWhilePendingDoesNotThrowAndStillDispatches(): void + { + Queue::fake(); + + $workflow = WorkflowStub::make(TestWorkflow::class); + $storedWorkflow = StoredWorkflow::findOrFail($workflow->id()); + $storedWorkflow->update([ + 'arguments' => Serializer::serialize([]), + 'status' => WorkflowPendingStatus::$name, + ]); + + $workflow->resume(); + + $this->assertSame(WorkflowPendingStatus::class, $workflow->status()); + Queue::assertPushed(TestWorkflow::class, 1); + } + public function testIsUpdateMethodReturnsTrueForUpdateMethods(): void { $this->assertTrue(WorkflowStub::isUpdateMethod(TestChatBotWorkflow::class, 'receive'));