From e802ceb587423e9614a1ca69534bd502d5d46335 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Wed, 30 Sep 2026 21:02:37 -0700 Subject: [PATCH 1/2] fix(scheduling): every #[Scheduled] duration is proved at boot, and ISO-8601 durations are read as Spring reads them lockTtl was parsed only inside the task closure, so a value Duration::parse refused threw on every tick, was reported and the task never ran while the application booted and looked healthy. ScheduleWiringPass now parses fixedRate, fixedDelay and lockTtl at boot, as it already parsed initialDelay, and refuses the boot naming the method. Duration::parse also accepts ISO-8601 durations (PT14M, PT1H30M, P1D, PT0.25S), the java.time.Duration form Spring's @Scheduled reads. Years, months and weeks are refused because their length is not fixed. Found in a Signature CRM deployment whose seven scheduled tasks declared lockTtl: 'PT14M' and never ran. --- docs/modules/resilience.md | 6 +- docs/modules/scheduling.md | 15 +++-- packages/resilience/src/Duration.php | 25 +++++-- packages/resilience/tests/DurationTest.php | 33 ++++++++++ .../scheduling/src/Attributes/Scheduled.php | 4 +- .../src/Boot/ScheduleWiringPass.php | 30 +++++++++ .../tests/Boot/ScheduleWiringPassTest.php | 65 +++++++++++++++++++ 7 files changed, 166 insertions(+), 12 deletions(-) diff --git a/docs/modules/resilience.md b/docs/modules/resilience.md index 5d928640..c394eff9 100644 --- a/docs/modules/resilience.md +++ b/docs/modules/resilience.md @@ -91,8 +91,10 @@ several of those example values are deliberately not the framework default (the ``` Duration-shaped keys (`wait-duration`, `max-wait`, `wait-duration-in-open`, `timeout`, …) accept either a -bare number of seconds or a `Firefly\Resilience\Duration`-parsed string: `250ms`, `30s`, `5m`, `1h`. `Duration` -is also reused by `firefly/scheduling` for `lock-ttl` and `fixedRate`/`fixedDelay`. +bare number of seconds or a `Firefly\Resilience\Duration`-parsed string: `250ms`, `30s`, `5m`, `1h`, or an ISO-8601 +duration such as `PT5M` (days, hours, minutes and seconds; years, months and weeks are refused because their length +is not fixed). `Duration` is also reused by `firefly/scheduling` for `lockTtl`, `initialDelay` and +`fixedRate`/`fixedDelay`. ## The six patterns diff --git a/docs/modules/scheduling.md b/docs/modules/scheduling.md index 837db9e1..b2245b37 100644 --- a/docs/modules/scheduling.md +++ b/docs/modules/scheduling.md @@ -83,14 +83,19 @@ Exactly one of `cron`, `fixedRate`, or `fixedDelay` must be set (the attribute's `InvalidArgumentException` otherwise): - **`cron`** — a Laravel/crontab expression, applied to the scheduled `Event` verbatim (`$event->cron(...)`). -- **`fixedRate`** / **`fixedDelay`** — a `Duration`-parsed string (`'250ms'`, `'30s'`, `'5m'`, `'1h'`, or a - bare number of seconds) mapped to the *nearest* native Laravel frequency method — see - [Known-latent](#known-latent). +- **`fixedRate`** / **`fixedDelay`** — a `Duration`-parsed string (`'250ms'`, `'30s'`, `'5m'`, `'1h'`, a bare + number of seconds, or an ISO-8601 duration such as `'PT5M'`, the form Spring's `@Scheduled` reads) mapped to the + *nearest* native Laravel frequency method — see [Known-latent](#known-latent). - **`lock`** — `true` shares a lock named `"Class::method"` (derived from the annotated method); a string is an explicit shared lock name (so several methods can share one lock); `null`/`false` (the default) runs unlocked, with no `DistributedLock` guard at all. -- **`lockTtl`** — a `Duration`-parsed string bounding how long the lock may be held; defaults to `30.0` - seconds when the trigger is locked and no `lockTtl` is given. +- **`lockTtl`** — a `Duration`-parsed string, in either form, bounding how long the lock may be held; defaults + to `30.0` seconds when the trigger is locked and no `lockTtl` is given. + +Every duration of a `#[Scheduled]` method (`fixedRate`, `fixedDelay`, `initialDelay`, `lockTtl`) is parsed when +the application boots, and one that does not parse refuses the boot with the method it belongs to. Before 26.09.11 +a `lockTtl` was only parsed when its task ran, so an unparseable value failed that task on every tick while the +application looked healthy. ```php diff --git a/packages/resilience/src/Duration.php b/packages/resilience/src/Duration.php index 22216de0..5174e9bb 100644 --- a/packages/resilience/src/Duration.php +++ b/packages/resilience/src/Duration.php @@ -7,17 +7,34 @@ use Firefly\Kernel\Exception\Framework\ConfigurationException; /** - * Parses a human duration string to a float number of seconds. Grammar: an optional-decimal magnitude with - * an optional unit suffix (ms/s/m/h); a bare number is seconds (pyfly parity). Shared by resilience config - * (wait-duration, timeout, …) and scheduling (lock-ttl, fixed-rate). + * Parses a duration string to a float number of seconds. Two grammars are accepted: + * + * - the short form: an optional-decimal magnitude with an optional unit suffix (ms/s/m/h); a bare number is + * seconds (pyfly parity); + * - ISO-8601 as java.time.Duration reads it (Spring parity): `P[nD][T[nH][nM][n[.n]S]]`, case-insensitive, with at + * least one component. Years, months and weeks are refused, as java.time.Duration refuses them: their length + * in seconds is not fixed. + * + * Shared by resilience config (wait-duration, timeout, …) and scheduling (lock-ttl, fixed-rate, initial-delay). */ final class Duration { + private const string ISO_8601 = '/^\s*P(?:(\d+)D)?(?:T(?=\d)(?:(\d+)H)?(?:(\d+)M)?(?:(\d+(?:\.\d+)?)S)?)?\s*$/i'; + public static function parse(string $value): float { + if (preg_match(self::ISO_8601, $value, $iso) === 1 && preg_match('/\d/', $value) === 1) { + // A trailing `T` with nothing after it is caught by the lookahead; `P` alone has no digit at all. + return (float) ($iso[1] ?? 0) * 86400.0 + + (float) ($iso[2] ?? 0) * 3600.0 + + (float) ($iso[3] ?? 0) * 60.0 + + (float) ($iso[4] ?? 0); + } + if (preg_match('/^\s*(\d+(?:\.\d+)?)\s*(ms|s|m|h)?\s*$/', $value, $matches) !== 1) { throw new ConfigurationException( - "Invalid duration [{$value}]. Use e.g. '250ms', '30s', '5m', '1h', or a bare number of seconds.", + "Invalid duration [{$value}]. Use e.g. '250ms', '30s', '5m', '1h', a bare number of seconds, " + ."or an ISO-8601 duration such as 'PT5M'.", ); } diff --git a/packages/resilience/tests/DurationTest.php b/packages/resilience/tests/DurationTest.php index 12a931f4..0cbe9ac8 100644 --- a/packages/resilience/tests/DurationTest.php +++ b/packages/resilience/tests/DurationTest.php @@ -21,3 +21,36 @@ it('rejects an unparseable duration', function (string $input) { expect(fn () => Duration::parse($input))->toThrow(ConfigurationException::class); })->with(['empty' => [''], 'letters' => ['abc'], 'bad unit' => ['5d'], 'negative' => ['-3s']]); + +/* + * SPRING'S DURATION FORMAT IS ACCEPTED TOO. `#[Scheduled]` claims @Scheduled parity, and Spring reads + * `fixedDelayString`, `initialDelayString` and lock durations as ISO-8601 (`PT14M`), the java.time.Duration + * form. Days, hours, minutes and fractional seconds are accepted case-insensitively; years, months and weeks + * are refused because their length in seconds is not fixed, as java.time.Duration refuses them. + */ +it('parses an ISO-8601 duration to seconds', function (string $input, float $seconds) { + expect(Duration::parse($input))->toBe($seconds); +})->with([ + 'minutes' => ['PT14M', 840.0], + 'hours' => ['PT1H', 3600.0], + 'seconds' => ['PT30S', 30.0], + 'fractional seconds' => ['PT0.25S', 0.25], + 'hours and minutes' => ['PT1H30M', 5400.0], + 'days' => ['P1D', 86400.0], + 'days and hours' => ['P1DT2H', 93600.0], + 'lower case' => ['pt5m', 300.0], + 'whitespace tolerated' => [' PT4M ', 240.0], +]); + +it('rejects an ISO-8601 duration without a fixed length or a component', function (string $input) { + expect(fn () => Duration::parse($input))->toThrow(ConfigurationException::class); +})->with([ + 'no component' => ['P'], + 'time marker only' => ['PT'], + 'years' => ['P1Y'], + 'months' => ['P1M'], + 'weeks' => ['P1W'], + 'negative' => ['-PT5M'], + 'unit missing' => ['PT5'], + 'minutes before hours' => ['PT5M1H'], +]); diff --git a/packages/scheduling/src/Attributes/Scheduled.php b/packages/scheduling/src/Attributes/Scheduled.php index 64ab3ffb..13f6a7c1 100644 --- a/packages/scheduling/src/Attributes/Scheduled.php +++ b/packages/scheduling/src/Attributes/Scheduled.php @@ -10,7 +10,9 @@ /** * Marks a public method as a scheduled task (pyfly/Spring @Scheduled parity). Exactly one trigger is required: * `cron` (a Laravel/crontab expression, applied verbatim), `fixedRate`, or `fixedDelay` (duration strings parsed - * by Duration::parse at wiring time and mapped to the nearest native Laravel frequency). `lock === true` shares a + * by Duration::parse at wiring time and mapped to the nearest native Laravel frequency). Durations — these two, + * `initialDelay` and `lockTtl` — take the short form ('30s', '5m') or ISO-8601 ('PT5M'), and a value that does not + * parse refuses to boot. `lock === true` shares a * lock named "Class::method"; a string is an explicit shared lock name; null/false runs unlocked. The scanner * (the package's sole reflection site) reads these into pure-array descriptors; nothing here parses durations. */ diff --git a/packages/scheduling/src/Boot/ScheduleWiringPass.php b/packages/scheduling/src/Boot/ScheduleWiringPass.php index 2436731a..4ae4f2e6 100644 --- a/packages/scheduling/src/Boot/ScheduleWiringPass.php +++ b/packages/scheduling/src/Boot/ScheduleWiringPass.php @@ -66,6 +66,7 @@ public function run(BootContext $context): void $lock = $container->make(DistributedLock::class); $config = $context->config; + $this->validateDurations($manifest); $this->validateDelays($manifest, $config); $container->afterResolving(Schedule::class, function (Schedule $schedule) use ($manifest, $lock, $container, $config): void { @@ -84,6 +85,35 @@ public function run(BootContext $context): void }); } + /** + * `fixedRate`, `fixedDelay` and `lockTtl` are PARSED HERE for their refusal, as validateDelays() parses + * `initialDelay`. Neither the attribute nor the scanner validates them. `lockTtl` used to be parsed only inside + * the task closure, so a string Duration::parse refused threw on every tick, was reported and the task never + * ran — while the application booted, served and looked healthy. A rate or delay was parsed when the Schedule + * was resolved, so one bad string stopped every task of the scheduler, not just its own. + */ + private function validateDurations(ScheduledManifest $manifest): void + { + foreach ($manifest->all() as $descriptor) { + $durations = ['fixedRate' => $descriptor->fixedRate, 'fixedDelay' => $descriptor->fixedDelay, 'lockTtl' => $descriptor->lockTtl]; + foreach ($durations as $parameter => $value) { + if ($value === null) { + continue; + } + + try { + Duration::parse($value); + } catch (ConfigurationException $exception) { + throw new ConfigurationException( + "#[Scheduled({$parameter}: '{$value}')] on {$descriptor->class}::{$descriptor->method} " + ."is not a duration this framework can parse. {$exception->getMessage()}", + previous: $exception, + ); + } + } + } + } + /** * `initialDelay` is APPLIED as a per-tick predicate (see InitialDelayGate) — or, when the gate is * switched off, REFUSED at boot. The one thing it must never do again is what it did for two releases: diff --git a/packages/scheduling/tests/Boot/ScheduleWiringPassTest.php b/packages/scheduling/tests/Boot/ScheduleWiringPassTest.php index 09f9cf17..484cf863 100644 --- a/packages/scheduling/tests/Boot/ScheduleWiringPassTest.php +++ b/packages/scheduling/tests/Boot/ScheduleWiringPassTest.php @@ -368,3 +368,68 @@ public function store($name = null) expect($logger->records)->toBe([]); }); + +/* + * A LOCK TTL THAT IS NOT A DURATION USED TO DISABLE ITS TASK FOR GOOD. `lockTtl` was parsed inside the task + * closure, so a value Duration::parse refused threw on every tick, was reported and the task never ran — while + * the application booted, served and looked healthy. Every duration of the manifest is now proved where + * initialDelay already was: at boot. + */ + +it('REFUSES TO BOOT on a lockTtl that is not a duration, rather than failing its task on every tick', function () { + $refuse = fn () => scheduleWithInitialDelayGate([ + new ScheduledDescriptor(class: ScheduledJobs::class, method: 'reconcile', cron: '*/5 * * * *', lockName: 'reconcile', lockTtl: 'four minutes'), + ], enabled: true); + + expect($refuse)->toThrow( + ConfigurationException::class, + "#[Scheduled(lockTtl: 'four minutes')] on ".ScheduledJobs::class.'::reconcile', + ); +}); + +it('REFUSES TO BOOT on a fixedRate or fixedDelay that is not a duration', function (string $parameter) { + $refuse = fn () => scheduleWithInitialDelayGate([ + new ScheduledDescriptor(...['class' => ScheduledJobs::class, 'method' => 'reconcile', $parameter => 'often']), + ], enabled: true); + + expect($refuse)->toThrow( + ConfigurationException::class, + "#[Scheduled({$parameter}: 'often')] on ".ScheduledJobs::class.'::reconcile', + ); +})->with(['fixedRate', 'fixedDelay']); + +it('acquires the lock for the ISO-8601 lockTtl Spring authors write', function () { + $lock = new class implements DistributedLock + { + /** @var array */ + public array $acquired = []; + + public function tryAcquire(string $name, float $ttlSeconds): bool + { + $this->acquired[$name] = $ttlSeconds; + + return true; + } + + public function release(string $name): void {} + }; + $container = new Container; + Container::setInstance($container); + $container->instance(CacheFactoryContract::class, new class implements CacheFactoryContract + { + public function store($name = null) + { + return new CacheRepository(new ArrayStore); + } + }); + $container->instance(DistributedLock::class, $lock); + $container->instance(ScheduledManifest::class, new ScheduledManifest([ + new ScheduledDescriptor(class: ScheduledJobs::class, method: 'reconcile', cron: '*/5 * * * *', lockName: 'reconcile', lockTtl: 'PT4M'), + ])); + (new ScheduleWiringPass)->run(scheduleWiringContext($container)); + + $event = $container->make(Schedule::class)->events()[0]; + $event->run($container); + + expect($lock->acquired)->toBe(['reconcile' => 240.0]); +}); From 245aa577fe5f041e8569a16f4723d9baaec2a12a Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Wed, 30 Sep 2026 21:02:37 -0700 Subject: [PATCH 2/2] chore(release): prepare LaraFly 26.09.11 --- CHANGELOG.md | 12 ++++++++++++ README.md | 2 +- docs/publishing.md | 2 +- docs/versioning.md | 2 +- packages/kernel/src/Version.php | 2 +- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffbdc3b8..6f39a146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to LaraFly are documented here. This project uses CalVer (`Y ## [Unreleased] +## [26.09.11] - 2026-09-30 + +### Fixed + +- **Scheduling:** every duration of a `#[Scheduled]` method — `fixedRate`, `fixedDelay` and `lockTtl`, as + `initialDelay` already was — is parsed at boot, and one that does not parse refuses the boot naming its method. + `lockTtl` used to be parsed only when its task ran, so an unparseable value failed that task on every tick and + the task never ran while the application booted and looked healthy. +- **Durations:** `Firefly\Resilience\Duration` also reads ISO-8601 durations (`PT14M`, `PT1H30M`, `P1D`, + `PT0.25S`), the form Spring's `@Scheduled` and `java.time.Duration` use. Years, months and weeks are refused + because their length is not fixed. Resilience settings and every scheduling duration accept it. + ## [26.09.10] - 2026-09-29 ### Fixed diff --git a/README.md b/README.md index 79993022..45b6ee2e 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ PHP 8.3+ Laravel 13 License: Apache 2.0 - Version: 26.09.10 + Version: 26.09.11 PHPStan: max Code Style: Pint

diff --git a/docs/publishing.md b/docs/publishing.md index 9e658f5a..74fbb0cf 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -78,7 +78,7 @@ CI runs these package checks on PHP 8.3, 8.4 and 8.5 for PRs to `main` and pushe To repeat the public install check locally: ```bash -RELEASE_TAG=v26.09.10 RELEASE_SHA="$(git rev-parse 'v26.09.10^{commit}')" php scripts/check-package-install.php --published +RELEASE_TAG=v26.09.11 RELEASE_SHA="$(git rev-parse 'v26.09.11^{commit}')" php scripts/check-package-install.php --published ``` An unmerged branch or a local consumer check is not a published release. diff --git a/docs/versioning.md b/docs/versioning.md index 5c52cd84..97a989eb 100644 --- a/docs/versioning.md +++ b/docs/versioning.md @@ -21,7 +21,7 @@ The single place the current version *is* asserted in code is: ```php final class Version { - public const string VERSION = '26.09.10'; + public const string VERSION = '26.09.11'; } ``` diff --git a/packages/kernel/src/Version.php b/packages/kernel/src/Version.php index 67c35840..1e773643 100644 --- a/packages/kernel/src/Version.php +++ b/packages/kernel/src/Version.php @@ -15,5 +15,5 @@ */ final class Version { - public const string VERSION = '26.09.10'; + public const string VERSION = '26.09.11'; }