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/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/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'; } 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]); +});