Skip to content

Commit ede7dbb

Browse files
rmcdanielrmcdaniel
authored
Fix retry (#322)
1 parent e0b7814 commit ede7dbb

10 files changed

Lines changed: 311 additions & 1 deletion

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ coverage.xml
88
.phpunit.result.cache
99
.php_cs.cache
1010
.php-cs-fixer.cache
11+
.vs

‎src/WorkflowStub.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,15 @@ private function dispatch(): void
381381
);
382382
}
383383

384-
$this->storedWorkflow->status->transitionTo(WorkflowPendingStatus::class);
384+
try {
385+
$this->storedWorkflow->status->transitionTo(WorkflowPendingStatus::class);
386+
} catch (\Spatie\ModelStates\Exceptions\TransitionNotFound $exception) {
387+
$this->storedWorkflow->refresh();
388+
389+
if ($this->status() !== WorkflowPendingStatus::class) {
390+
throw $exception;
391+
}
392+
}
385393

386394
$dispatch = static::faked() ? 'dispatchSync' : 'dispatch';
387395

‎tests/.env.feature‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ DB_PORT=5432
77
DB_USERNAME=laravel
88
DB_PASSWORD=laravel
99

10+
CACHE_DRIVER=redis
11+
CACHE_STORE=redis
12+
1013
QUEUE_CONNECTION=redis
1114
QUEUE_FAILED_DRIVER=null
1215

‎tests/Feature/NestedSignalRaceConditionTest.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Feature;
6+
7+
use RuntimeException;
8+
use Tests\Fixtures\TestNestedSignalLeafWorkflow;
9+
use Tests\Fixtures\TestNestedSignalParentWorkflow;
10+
use Tests\TestCase;
11+
use Workflow\Models\StoredWorkflow;
12+
use Workflow\States\WorkflowCompletedStatus;
13+
use Workflow\WorkflowStub;
14+
15+
final class NestedSignalRaceConditionTest extends TestCase
16+
{
17+
public function testNestedChildWorkflowsWithDuplicateSignalsDoNotGetStuckPending(): void
18+
{
19+
$runId = (int) now()
20+
->format('Uu');
21+
$middleCount = 12;
22+
$leafCount = 3;
23+
$duplicateSignals = 4;
24+
$expectedLeafCount = $middleCount * $leafCount;
25+
26+
$workflow = WorkflowStub::make(TestNestedSignalParentWorkflow::class);
27+
$workflow->start($runId, $middleCount, $leafCount);
28+
29+
$creationDeadline = now()
30+
->addSeconds(30);
31+
$leafIds = [];
32+
while (now()->lt($creationDeadline)) {
33+
$leafIds = StoredWorkflow::query()
34+
->where('class', TestNestedSignalLeafWorkflow::class)
35+
->pluck('id')
36+
->all();
37+
38+
if (count($leafIds) === $expectedLeafCount) {
39+
break;
40+
}
41+
42+
usleep(50000);
43+
}
44+
45+
$this->assertCount($expectedLeafCount, $leafIds, 'Timed out waiting for all nested leaf workflows');
46+
47+
for ($round = 0; $round < $duplicateSignals; $round++) {
48+
foreach ($leafIds as $leafId) {
49+
WorkflowStub::load((int) $leafId)->respond();
50+
}
51+
}
52+
53+
$completionDeadline = now()
54+
->addSeconds(120);
55+
while ($workflow->running() && now()->lt($completionDeadline)) {
56+
usleep(50000);
57+
$workflow->fresh();
58+
}
59+
60+
if ($workflow->running()) {
61+
throw new RuntimeException(sprintf(
62+
'Nested signal run %d did not complete before timeout. Current status: %s',
63+
$runId,
64+
(string) $workflow->status()
65+
));
66+
}
67+
68+
$this->assertSame(WorkflowCompletedStatus::class, $workflow->status());
69+
$this->assertSame([
70+
'run_id' => $runId,
71+
'middle_count' => $middleCount,
72+
'leaf_count' => $leafCount,
73+
'resolved_leaf_count' => $expectedLeafCount,
74+
], $workflow->output());
75+
}
76+
}

‎tests/Fixtures/TestNestedSignalLeafWorkflow.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Fixtures;
6+
7+
use Generator;
8+
use function Workflow\awaitWithTimeout;
9+
use Workflow\SignalMethod;
10+
use Workflow\Workflow;
11+
12+
final class TestNestedSignalLeafWorkflow extends Workflow
13+
{
14+
private bool $responded = false;
15+
16+
#[SignalMethod]
17+
public function respond(): void
18+
{
19+
$this->responded = true;
20+
}
21+
22+
public function execute(int $runId, int $middleIndex, int $leafIndex): Generator
23+
{
24+
$resolved = yield awaitWithTimeout(30, fn (): bool => $this->responded);
25+
26+
return [
27+
'run_id' => $runId,
28+
'middle_index' => $middleIndex,
29+
'leaf_index' => $leafIndex,
30+
'resolved' => $resolved,
31+
];
32+
}
33+
}

‎tests/Fixtures/TestNestedSignalMiddleWorkflow.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Fixtures;
6+
7+
use Generator;
8+
use function Workflow\all;
9+
use function Workflow\child;
10+
use Workflow\Workflow;
11+
12+
final class TestNestedSignalMiddleWorkflow extends Workflow
13+
{
14+
public function execute(int $runId, int $middleIndex, int $leafCount = 3): Generator
15+
{
16+
$promises = [];
17+
18+
for ($leafIndex = 0; $leafIndex < $leafCount; $leafIndex++) {
19+
$promises[] = child(TestNestedSignalLeafWorkflow::class, $runId, $middleIndex, $leafIndex);
20+
}
21+
22+
$results = yield all($promises);
23+
24+
return count(array_filter(
25+
$results,
26+
static fn (array $result): bool => (bool) ($result['resolved'] ?? false)
27+
));
28+
}
29+
}

‎tests/Fixtures/TestNestedSignalParentWorkflow.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Fixtures;
6+
7+
use Generator;
8+
use function Workflow\all;
9+
use function Workflow\child;
10+
use Workflow\Workflow;
11+
12+
final class TestNestedSignalParentWorkflow extends Workflow
13+
{
14+
public function execute(int $runId, int $middleCount = 10, int $leafCount = 3): Generator
15+
{
16+
$promises = [];
17+
18+
for ($middleIndex = 0; $middleIndex < $middleCount; $middleIndex++) {
19+
$promises[] = child(TestNestedSignalMiddleWorkflow::class, $runId, $middleIndex, $leafCount);
20+
}
21+
22+
$resolvedPerMiddle = yield all($promises);
23+
24+
return [
25+
'run_id' => $runId,
26+
'middle_count' => $middleCount,
27+
'leaf_count' => $leafCount,
28+
'resolved_leaf_count' => array_sum($resolvedPerMiddle),
29+
];
30+
}
31+
}

‎tests/Unit/ChildWorkflowStubTest.php‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Tests\Unit;
66

7+
use Mockery;
8+
use Spatie\ModelStates\Exceptions\TransitionNotFound;
79
use Tests\Fixtures\TestChildWorkflow;
810
use Tests\Fixtures\TestParentWorkflow;
911
use Tests\TestCase;
@@ -89,6 +91,77 @@ public function testLoadsChildWorkflow(): void
8991
$this->assertNull($result);
9092
}
9193

94+
public function testIgnoresTransitionNotFoundWhenChildResumeThrows(): void
95+
{
96+
$logs = Mockery::mock();
97+
$logs->shouldReceive('whereIndex')
98+
->once()
99+
->with(0)
100+
->andReturnSelf();
101+
$logs->shouldReceive('first')
102+
->once()
103+
->andReturn(null);
104+
105+
$childWorkflow = new class() {
106+
public function running(): bool
107+
{
108+
return true;
109+
}
110+
111+
public function created(): bool
112+
{
113+
return false;
114+
}
115+
116+
public function resume(): void
117+
{
118+
throw TransitionNotFound::make('running', 'pending', StoredWorkflow::class);
119+
}
120+
121+
public function completed(): bool
122+
{
123+
return false;
124+
}
125+
126+
public function startAsChild(...$arguments): void
127+
{
128+
}
129+
};
130+
131+
$storedChildWorkflow = Mockery::mock();
132+
$storedChildWorkflow->shouldReceive('toWorkflow')
133+
->once()
134+
->andReturn($childWorkflow);
135+
136+
$children = Mockery::mock();
137+
$children->shouldReceive('wherePivot')
138+
->once()
139+
->with('parent_index', 0)
140+
->andReturnSelf();
141+
$children->shouldReceive('first')
142+
->once()
143+
->andReturn($storedChildWorkflow);
144+
145+
$storedWorkflow = Mockery::mock();
146+
$storedWorkflow->shouldReceive('logs')
147+
->once()
148+
->andReturn($logs);
149+
$storedWorkflow->shouldReceive('children')
150+
->once()
151+
->andReturn($children);
152+
153+
WorkflowStub::setContext([
154+
'storedWorkflow' => $storedWorkflow,
155+
'index' => 0,
156+
'now' => now(),
157+
'replaying' => false,
158+
]);
159+
160+
ChildWorkflowStub::make(TestChildWorkflow::class);
161+
162+
$this->assertSame(1, WorkflowStub::getContext()->index);
163+
}
164+
92165
public function testAll(): void
93166
{
94167
$workflow = WorkflowStub::load(WorkflowStub::make(TestParentWorkflow::class)->id());

‎tests/Unit/ChildWorkflowTest.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Unit;
6+
7+
use Tests\Fixtures\TestChildWorkflow;
8+
use Tests\Fixtures\TestWorkflow;
9+
use Tests\TestCase;
10+
use Workflow\ChildWorkflow;
11+
use Workflow\Models\StoredWorkflow;
12+
use Workflow\Serializers\Serializer;
13+
use Workflow\States\WorkflowRunningStatus;
14+
use Workflow\WorkflowStub;
15+
16+
final class ChildWorkflowTest extends TestCase
17+
{
18+
public function testHandleReleasesWhenParentWorkflowIsRunning(): void
19+
{
20+
$parent = WorkflowStub::make(TestWorkflow::class);
21+
$storedParent = StoredWorkflow::findOrFail($parent->id());
22+
$storedParent->update([
23+
'arguments' => Serializer::serialize([]),
24+
'status' => WorkflowRunningStatus::class,
25+
]);
26+
27+
$storedChild = StoredWorkflow::create([
28+
'class' => TestChildWorkflow::class,
29+
'arguments' => Serializer::serialize([]),
30+
]);
31+
32+
$job = new ChildWorkflow(0, now()->toDateTimeString(), $storedChild, true, $storedParent);
33+
34+
$job->handle();
35+
36+
$this->assertSame(1, $storedParent->logs()->count());
37+
$this->assertSame(WorkflowRunningStatus::class, $storedParent->refresh()->status::class);
38+
}
39+
}

‎tests/Unit/WorkflowStubTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,23 @@ public function testHandlesDuplicateLogInsertionProperly(): void
258258
Queue::assertPushed(TestWorkflow::class, 1);
259259
}
260260

261+
public function testResumeWhilePendingDoesNotThrowAndStillDispatches(): void
262+
{
263+
Queue::fake();
264+
265+
$workflow = WorkflowStub::make(TestWorkflow::class);
266+
$storedWorkflow = StoredWorkflow::findOrFail($workflow->id());
267+
$storedWorkflow->update([
268+
'arguments' => Serializer::serialize([]),
269+
'status' => WorkflowPendingStatus::$name,
270+
]);
271+
272+
$workflow->resume();
273+
274+
$this->assertSame(WorkflowPendingStatus::class, $workflow->status());
275+
Queue::assertPushed(TestWorkflow::class, 1);
276+
}
277+
261278
public function testIsUpdateMethodReturnsTrueForUpdateMethods(): void
262279
{
263280
$this->assertTrue(WorkflowStub::isUpdateMethod(TestChatBotWorkflow::class, 'receive'));

0 commit comments

Comments
 (0)