Skip to content

Commit 50b439d

Browse files
claudeclaude
committed
Implement Phases 2, 5, 6, 8: ConditionEvaluator, retry, timeout, state enforcement
Phase 2 — Deduplicate condition evaluation: - Extract ConditionEvaluator to Support namespace - Both Step and WorkflowDefinition delegate to it - Throw on unparseable conditions instead of silently passing Phase 5 — Retry logic: - Add executeActionWithRetry() to Executor with exponential backoff - Dispatch StepRetriedEvent on each retry attempt - Respects Step::getRetryAttempts() configuration Phase 6 — Timeout enforcement: - Add executeWithTimeout() using pcntl_alarm (graceful fallback) - Add Timeout helper for parsing duration strings (30s, 5m, 2h, 1d) - Wrap action execution with timeout in Executor Phase 8 — State transition enforcement: - WorkflowInstance::setState() now validates via canTransitionTo() - Add PENDING→FAILED and PAUSED→FAILED transitions - Executor handles PAUSED→RUNNING and WAITING→RUNNING on resume - WorkflowEngine::cancel() guards against terminal states Additional fixes: - Replace ALL remaining data_get/data_set/class_basename calls (WorkflowContext, ActionResult, ConditionAction, SimpleWorkflow, ActionNotFoundException, StepExecutionException) - Add Arr::classBasename() helper - Clean up stale PHPStan ignore patterns - Fix DocumentApproval test conditions to proper format CI: 93 tests, 224 assertions, all green. PHPStan level 6 clean. https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG
1 parent e9fdb22 commit 50b439d

29 files changed

Lines changed: 757 additions & 99 deletions

‎phpstan.neon.dist‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,5 @@ parameters:
66
ignoreErrors:
77
- '#has no value type specified in iterable type array#'
88
- '#Match arm comparison between .+ and .+ is always true#'
9-
- '#has parameter .+ with no type specified#'
10-
- '#has no return type specified#'
119
- '#PHPDoc tag @param for parameter .+ with type .+ is not subtype of native type#'
1210
- '#Parameter .+ has invalid type#'

‎src/Actions/ConditionAction.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use SolutionForest\WorkflowEngine\Attributes\WorkflowStep;
77
use SolutionForest\WorkflowEngine\Core\ActionResult;
88
use SolutionForest\WorkflowEngine\Core\WorkflowContext;
9+
use SolutionForest\WorkflowEngine\Support\Arr;
910

1011
/**
1112
* Condition evaluation action with advanced expression parsing
@@ -116,7 +117,7 @@ private function getValue(string $expression, array $data): mixed
116117
'true', 'yes' => true,
117118
'false', 'no' => false,
118119
'null', 'empty' => null,
119-
default => data_get($data, $expression)
120+
default => Arr::get($data, $expression)
120121
};
121122
}
122123
}

‎src/Core/ActionResult.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace SolutionForest\WorkflowEngine\Core;
44

5+
use SolutionForest\WorkflowEngine\Support\Arr;
6+
57
/**
68
* Represents the result of a workflow action execution.
79
*
@@ -375,7 +377,7 @@ public function toArray(): array
375377
*/
376378
public function get(string $key, $default = null)
377379
{
378-
return data_get($this->data, $key, $default);
380+
return Arr::get($this->data, $key, $default);
379381
}
380382

381383
/**

‎src/Core/DefinitionParser.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ private function validateStep(string $stepId, array $stepData, array $fullDefini
396396

397397
foreach ($stepData['conditions'] as $conditionIndex => $condition) {
398398
if (! is_string($condition) || empty(trim($condition))) {
399-
throw InvalidWorkflowDefinitionException::invalidCondition($condition);
399+
throw InvalidWorkflowDefinitionException::invalidCondition($condition, 'Condition cannot be empty.');
400400
}
401401
}
402402
}
@@ -512,7 +512,7 @@ private function validateTransition(array $transition, array $steps, int $transi
512512
// Validate optional condition field
513513
if (isset($transition['condition'])) {
514514
if (! is_string($transition['condition']) || empty(trim($transition['condition']))) {
515-
throw InvalidWorkflowDefinitionException::invalidCondition($transition['condition']);
515+
throw InvalidWorkflowDefinitionException::invalidCondition($transition['condition'], 'Condition cannot be empty.');
516516
}
517517
}
518518

‎src/Core/Executor.php‎

Lines changed: 121 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
use SolutionForest\WorkflowEngine\Contracts\WorkflowAction;
99
use SolutionForest\WorkflowEngine\Events\StepCompletedEvent;
1010
use SolutionForest\WorkflowEngine\Events\StepFailedEvent;
11+
use SolutionForest\WorkflowEngine\Events\StepRetriedEvent;
1112
use SolutionForest\WorkflowEngine\Events\WorkflowCompletedEvent;
1213
use SolutionForest\WorkflowEngine\Events\WorkflowFailedEvent;
1314
use SolutionForest\WorkflowEngine\Exceptions\ActionNotFoundException;
1415
use SolutionForest\WorkflowEngine\Exceptions\StepExecutionException;
1516
use SolutionForest\WorkflowEngine\Support\NullEventDispatcher;
1617
use SolutionForest\WorkflowEngine\Support\NullLogger;
18+
use SolutionForest\WorkflowEngine\Support\Timeout;
1719

1820
/**
1921
* Workflow executor responsible for running workflow steps and managing execution flow.
@@ -150,8 +152,8 @@ public function execute(WorkflowInstance $instance): void
150152
*/
151153
private function processWorkflow(WorkflowInstance $instance): void
152154
{
153-
// If workflow is not running, start it
154-
if ($instance->getState() === WorkflowState::PENDING) {
155+
// If workflow is not running, transition it to running
156+
if (in_array($instance->getState(), [WorkflowState::PENDING, WorkflowState::PAUSED, WorkflowState::WAITING])) {
155157
$instance->setState(WorkflowState::RUNNING);
156158
$this->stateManager->save($instance);
157159
}
@@ -217,7 +219,7 @@ private function executeStep(WorkflowInstance $instance, Step $step): void
217219

218220
try {
219221
if ($step->hasAction()) {
220-
$this->executeAction($instance, $step);
222+
$this->executeActionWithRetry($instance, $step);
221223
}
222224

223225
// Mark step as completed
@@ -227,7 +229,6 @@ private function executeStep(WorkflowInstance $instance, Step $step): void
227229
$this->logger->info('Workflow step completed successfully', [
228230
'workflow_id' => $instance->getId(),
229231
'step_id' => $step->getId(),
230-
'step_duration' => 'calculated_in_future_version', // TODO: Add timing
231232
]);
232233

233234
// Continue execution recursively
@@ -268,6 +269,112 @@ private function executeStep(WorkflowInstance $instance, Step $step): void
268269
}
269270
}
270271

272+
/**
273+
* Execute a step's action with retry logic.
274+
*
275+
* @param WorkflowInstance $instance The workflow instance
276+
* @param Step $step The step to execute
277+
*
278+
* @throws ActionNotFoundException If the action class doesn't exist
279+
* @throws StepExecutionException If all retry attempts are exhausted
280+
*/
281+
private function executeActionWithRetry(WorkflowInstance $instance, Step $step): void
282+
{
283+
$maxAttempts = $step->getRetryAttempts() + 1; // +1 for initial attempt
284+
285+
if ($maxAttempts <= 1) {
286+
// No retries configured, execute directly
287+
$this->executeAction($instance, $step);
288+
289+
return;
290+
}
291+
292+
$lastException = null;
293+
294+
for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) {
295+
try {
296+
$this->executeAction($instance, $step);
297+
298+
return; // Success — exit retry loop
299+
} catch (\Exception $e) {
300+
$lastException = $e;
301+
302+
if ($attempt === $maxAttempts) {
303+
$this->logger->error('Step failed after all retry attempts', [
304+
'workflow_id' => $instance->getId(),
305+
'step_id' => $step->getId(),
306+
'attempts' => $attempt,
307+
'max_attempts' => $maxAttempts,
308+
'error' => $e->getMessage(),
309+
]);
310+
311+
throw $e; // Final attempt failed — propagate
312+
}
313+
314+
$this->logger->warning('Step failed, retrying', [
315+
'workflow_id' => $instance->getId(),
316+
'step_id' => $step->getId(),
317+
'attempt' => $attempt,
318+
'max_attempts' => $maxAttempts,
319+
'error' => $e->getMessage(),
320+
]);
321+
322+
$this->eventDispatcher->dispatch(new StepRetriedEvent(
323+
$instance,
324+
$step,
325+
$attempt,
326+
$maxAttempts,
327+
$e
328+
));
329+
330+
// Exponential backoff: 100ms, 200ms, 400ms... (keep short for a library)
331+
$backoffMicroseconds = (int) (100000 * pow(2, $attempt - 1));
332+
usleep($backoffMicroseconds);
333+
}
334+
}
335+
}
336+
337+
/**
338+
* Execute a callback with a timeout constraint.
339+
*
340+
* Uses pcntl_alarm when available, otherwise logs a warning and executes without timeout.
341+
*
342+
* @param callable $callback The callback to execute
343+
* @param int $timeoutSeconds Maximum execution time in seconds
344+
* @return mixed The callback's return value
345+
*
346+
* @throws StepExecutionException If the timeout is exceeded
347+
*/
348+
private function executeWithTimeout(callable $callback, int $timeoutSeconds): mixed
349+
{
350+
if (! function_exists('pcntl_alarm') || ! function_exists('pcntl_signal')) {
351+
$this->logger->warning('pcntl extension not available, timeout not enforced', [
352+
'timeout_seconds' => $timeoutSeconds,
353+
]);
354+
355+
return $callback();
356+
}
357+
358+
pcntl_signal(SIGALRM, function () use ($timeoutSeconds) {
359+
throw new \RuntimeException("Step execution timed out after {$timeoutSeconds} seconds");
360+
});
361+
362+
pcntl_alarm($timeoutSeconds);
363+
364+
try {
365+
$result = $callback();
366+
pcntl_alarm(0);
367+
368+
return $result;
369+
} catch (\Exception $e) {
370+
pcntl_alarm(0);
371+
372+
throw $e;
373+
} finally {
374+
pcntl_signal(SIGALRM, SIG_DFL);
375+
}
376+
}
377+
271378
/**
272379
* Execute the action associated with a workflow step.
273380
*
@@ -319,7 +426,16 @@ private function executeAction(WorkflowInstance $instance, Step $step): void
319426
instance: $instance
320427
);
321428

322-
$result = $action->execute($context);
429+
$timeoutValue = $step->getTimeout();
430+
if ($timeoutValue !== null) {
431+
$timeoutSeconds = Timeout::toSeconds($timeoutValue);
432+
$result = $this->executeWithTimeout(
433+
fn () => $action->execute($context),
434+
$timeoutSeconds
435+
);
436+
} else {
437+
$result = $action->execute($context);
438+
}
323439

324440
if ($result->isSuccess()) {
325441
// Merge any output data from the action

‎src/Core/Step.php‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace SolutionForest\WorkflowEngine\Core;
44

5-
use SolutionForest\WorkflowEngine\Support\Arr;
5+
use SolutionForest\WorkflowEngine\Support\ConditionEvaluator;
66

77
/**
88
* Represents a single step in a workflow with complete configuration and execution metadata.
@@ -222,28 +222,7 @@ public function canExecute(array $data): bool
222222
*/
223223
private function evaluateCondition(string $condition, array $data): bool
224224
{
225-
// Enhanced condition evaluation with support for more operators
226-
if (preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) {
227-
$key = $matches[1];
228-
$operator = $matches[2];
229-
$value = trim($matches[3], '"\'');
230-
231-
$dataValue = Arr::get($data, $key);
232-
233-
return match ($operator) {
234-
'===' => $dataValue === $value,
235-
'!==' => $dataValue !== $value,
236-
'>=' => $dataValue >= $value,
237-
'<=' => $dataValue <= $value,
238-
'==' => $dataValue == $value,
239-
'!=' => $dataValue != $value,
240-
'>' => $dataValue > $value,
241-
'<' => $dataValue < $value,
242-
default => false,
243-
};
244-
}
245-
246-
return true; // Default to true if condition cannot be parsed
225+
return ConditionEvaluator::evaluate($condition, $data);
247226
}
248227

249228
/**

‎src/Core/WorkflowBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ public function then(
281281
public function when(string $condition, callable $callback): self
282282
{
283283
if (empty(trim($condition))) {
284-
throw InvalidWorkflowDefinitionException::invalidCondition($condition);
284+
throw InvalidWorkflowDefinitionException::invalidCondition($condition, 'Condition cannot be empty.');
285285
}
286286

287287
$originalStepsCount = count($this->steps);

‎src/Core/WorkflowContext.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace SolutionForest\WorkflowEngine\Core;
44

55
use DateTime;
6+
use SolutionForest\WorkflowEngine\Support\Arr;
67

78
/**
89
* Immutable workflow execution context using PHP 8.3+ readonly properties.
@@ -111,7 +112,7 @@ public function getData(?string $key = null, mixed $default = null): mixed
111112
return $this->data;
112113
}
113114

114-
return data_get($this->data, $key, $default);
115+
return Arr::get($this->data, $key, $default);
115116
}
116117

117118
/**
@@ -173,7 +174,7 @@ public function withData(array $newData): static
173174
public function with(string $key, mixed $value): static
174175
{
175176
$newData = $this->data;
176-
data_set($newData, $key, $value);
177+
Arr::set($newData, $key, $value);
177178

178179
return new self(
179180
workflowId: $this->workflowId,
@@ -206,7 +207,7 @@ public function with(string $key, mixed $value): static
206207
*/
207208
public function hasData(string $key): bool
208209
{
209-
return data_get($this->data, $key) !== null;
210+
return Arr::get($this->data, $key) !== null;
210211
}
211212

212213
/**
@@ -229,7 +230,7 @@ public function getConfig(?string $key = null, mixed $default = null): mixed
229230
return $this->config;
230231
}
231232

232-
return data_get($this->config, $key, $default);
233+
return Arr::get($this->config, $key, $default);
233234
}
234235

235236
/**

‎src/Core/WorkflowDefinition.php‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
namespace SolutionForest\WorkflowEngine\Core;
44

55
use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException;
6-
use SolutionForest\WorkflowEngine\Support\Arr;
6+
use SolutionForest\WorkflowEngine\Support\ConditionEvaluator;
77

88
/**
99
* Represents a complete workflow definition with steps, transitions, and metadata.
@@ -323,28 +323,7 @@ private function processSteps(array $stepsData): array
323323
*/
324324
private function evaluateCondition(string $condition, array $data): bool
325325
{
326-
// Enhanced condition evaluation with comprehensive operator support
327-
if (preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) {
328-
$key = $matches[1];
329-
$operator = $matches[2];
330-
$value = trim($matches[3], '"\'');
331-
332-
$dataValue = Arr::get($data, $key);
333-
334-
return match ($operator) {
335-
'===' => $dataValue === $value,
336-
'!==' => $dataValue !== $value,
337-
'>=' => $dataValue >= $value,
338-
'<=' => $dataValue <= $value,
339-
'==' => $dataValue == $value,
340-
'!=' => $dataValue != $value,
341-
'>' => $dataValue > $value,
342-
'<' => $dataValue < $value,
343-
default => false,
344-
};
345-
}
346-
347-
return false; // Default to false if condition cannot be parsed
326+
return ConditionEvaluator::evaluate($condition, $data);
348327
}
349328

350329
/**

‎src/Core/WorkflowEngine.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use SolutionForest\WorkflowEngine\Contracts\StorageAdapter;
77
use SolutionForest\WorkflowEngine\Events\WorkflowCancelledEvent;
88
use SolutionForest\WorkflowEngine\Events\WorkflowStartedEvent;
9-
use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException;
109
use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowStateException;
1110
use SolutionForest\WorkflowEngine\Exceptions\WorkflowInstanceNotFoundException;
1211

@@ -95,7 +94,7 @@ public function __construct(
9594
* @param array<string, mixed> $context Initial context data for the workflow
9695
* @return string The workflow instance ID
9796
*
98-
* @throws InvalidWorkflowDefinitionException If the workflow definition is invalid
97+
* @throws \SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException If the workflow definition is invalid
9998
* @throws \RuntimeException If the workflow cannot be started due to system issues
10099
*
101100
* @example Starting a simple workflow
@@ -250,6 +249,16 @@ public function getInstances(array $filters = []): array
250249
public function cancel(string $instanceId, string $reason = ''): WorkflowInstance
251250
{
252251
$instance = $this->stateManager->load($instanceId);
252+
253+
if ($instance->getState()->isFinished()) {
254+
throw new InvalidWorkflowStateException(
255+
"Cannot cancel workflow '{$instanceId}' because it is in '{$instance->getState()->value}' state",
256+
$instance->getState(),
257+
WorkflowState::CANCELLED,
258+
$instanceId
259+
);
260+
}
261+
253262
$instance->setState(WorkflowState::CANCELLED);
254263
$this->stateManager->save($instance);
255264

0 commit comments

Comments
 (0)