From 3f54b8a034aa8dff9b5557457fbf10178c65f199 Mon Sep 17 00:00:00 2001 From: "Vladyslav G." Date: Wed, 2 Sep 2026 03:56:06 +0200 Subject: [PATCH 1/2] feat: bring the plugin up to the current framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto #1, which moved the plugin to Tempest 3 and the current Plugin contract. What remained was the parts that still could not work. ragnarok/fenrir was still required and Registry still implemented its Extension, so registerExtension() was handed something the current library's signature does not accept. The plugin no longer registers an extension at all: registerExtension() calls initialize() immediately, so routing through it bought indirection and no ordering. The registry is started from the plugin's own boot instead. composer.json pinned tempcord/framework ^0.7, which under Composer's reading of a 0.x caret excludes the 0.10 the plugin is meant to extend, and carried a hardcoded version, which is the thing we took out of the other repos so Packagist reads tags. #[Task] now goes on an invokable class as well as a method, so a task that is one class reads like every other Tempcord attribute. Tasks compile to a TaskDefinition rather than the attribute being handed a reflector and passed around. An attribute that mutates cannot survive the discovery cache. Each turn runs in a fiber, so a task may await the REST API — which for a Discord bot is most of what a task wants to do. A turn is skipped while the previous one is still running, and a task that throws is logged and keeps its place instead of cancelling its own timer. Two cron bugs: a step over a range counted from the bottom of the field, so 1-10/2 gave 2,4,6,8,10 rather than 1,3,5,7,9; and restricting both day fields meant "and" where cron means "or", so 0 0 1 * 1 ran only on Mondays that fell on the first. Cron tasks are armed for the exact wait until the next matching minute rather than waking every minute, which drifts until a matching minute is stepped over entirely. An ordinary turn logs at debug: a task running every ten seconds was writing eight thousand info lines a day to say nothing happened. 67 tests driving a fake loop, phpstan at the level the framework uses, and the CI, release and contribution setup the other three repos have. --- .github/dependabot.yml | 20 ++ .github/workflows/pr-title.yml | 35 +++ .github/workflows/release.yml | 42 ++++ .github/workflows/static-analysis.yml | 23 ++ .github/workflows/tests.yml | 37 ++++ .releaserc.json | 32 +++ CONTRIBUTING.md | 58 +++++ README.md | 176 +++++++++------ composer.json | 19 +- phpunit.xml | 6 +- src/Attributes/Task.php | 101 ++++----- src/Compiler/TaskCompiler.php | 77 +++++++ src/ConsoleCommands/TasksListCommand.php | 33 +-- src/Definitions/TaskDefinition.php | 62 ++++++ src/Discoveries/TasksDiscovery.php | 24 +- src/Registry.php | 83 +++---- src/Runner.php | 209 +++++++++--------- src/Support/CronExpression.php | 165 ++++++++------ src/TasksPlugin.php | 21 +- src/functions.php | 11 +- tests/Doubles/FakeDiscord.php | 33 --- tests/Doubles/FakeLoop.php | 102 +++++++++ tests/Doubles/FakeTimer.php | 36 +++ tests/Doubles/RecordingLogger.php | 35 +++ tests/Fixtures/BootTask.php | 22 ++ tests/Fixtures/DisabledTask.php | 18 ++ tests/Fixtures/FailingTask.php | 17 ++ tests/Fixtures/HandlerlessTask.php | 12 + tests/Fixtures/Housekeeping.php | 32 +++ tests/Fixtures/MinutelyTask.php | 18 ++ tests/Fixtures/SlowTask.php | 29 +++ tests/Fixtures/SweepMessages.php | 21 ++ tests/Fixtures/UnreadableCronTask.php | 13 ++ tests/RegistryTest.php | 72 ------ tests/RunnerTest.php | 113 ---------- tests/TaskTest.php | 99 --------- tests/TasksDiscoveryTest.php | 107 --------- tests/TasksPluginTest.php | 56 ----- tests/Unit/CronExpressionTest.php | 195 +++++++++++++++++ tests/Unit/CronSchedulingTest.php | 106 +++++++++ tests/Unit/RegistryTest.php | 119 ++++++++++ tests/Unit/RunnerTest.php | 268 +++++++++++++++++++++++ tests/Unit/TaskCompilerTest.php | 141 ++++++++++++ tests/Unit/TasksDiscoveryTest.php | 126 +++++++++++ tests/Unit/TasksPluginTest.php | 52 +++++ 45 files changed, 2190 insertions(+), 886 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/pr-title.yml create mode 100644 .github/workflows/release.yml create mode 100644 .github/workflows/static-analysis.yml create mode 100644 .github/workflows/tests.yml create mode 100644 .releaserc.json create mode 100644 CONTRIBUTING.md create mode 100644 src/Compiler/TaskCompiler.php create mode 100644 src/Definitions/TaskDefinition.php delete mode 100644 tests/Doubles/FakeDiscord.php create mode 100644 tests/Doubles/FakeLoop.php create mode 100644 tests/Doubles/FakeTimer.php create mode 100644 tests/Doubles/RecordingLogger.php create mode 100644 tests/Fixtures/BootTask.php create mode 100644 tests/Fixtures/DisabledTask.php create mode 100644 tests/Fixtures/FailingTask.php create mode 100644 tests/Fixtures/HandlerlessTask.php create mode 100644 tests/Fixtures/Housekeeping.php create mode 100644 tests/Fixtures/MinutelyTask.php create mode 100644 tests/Fixtures/SlowTask.php create mode 100644 tests/Fixtures/SweepMessages.php create mode 100644 tests/Fixtures/UnreadableCronTask.php delete mode 100644 tests/RegistryTest.php delete mode 100644 tests/RunnerTest.php delete mode 100644 tests/TaskTest.php delete mode 100644 tests/TasksDiscoveryTest.php delete mode 100644 tests/TasksPluginTest.php create mode 100644 tests/Unit/CronExpressionTest.php create mode 100644 tests/Unit/CronSchedulingTest.php create mode 100644 tests/Unit/RegistryTest.php create mode 100644 tests/Unit/RunnerTest.php create mode 100644 tests/Unit/TaskCompilerTest.php create mode 100644 tests/Unit/TasksDiscoveryTest.php create mode 100644 tests/Unit/TasksPluginTest.php diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..c780c60 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,20 @@ +version: 2 + +updates: + # Composer bumps arrive as their own pull request, so a dependency moving is + # a release of its own rather than something folded into unrelated work. + - package-ecosystem: composer + directory: / + schedule: + interval: weekly + commit-message: + prefix: fix + prefix-development: chore + include: scope + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + commit-message: + prefix: ci diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml new file mode 100644 index 0000000..dc0c434 --- /dev/null +++ b/.github/workflows/pr-title.yml @@ -0,0 +1,35 @@ +name: PR Title + +# Merges are squashed with the pull request title as the commit subject, so the +# title is what semantic-release will read. Checking it here is the difference +# between a release that happens and one that silently does not. +on: + pull_request_target: + types: [opened, edited, synchronize, reopened] + +permissions: + pull-requests: read + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: amannn/action-semantic-pull-request@v6 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + types: | + feat + fix + perf + refactor + docs + test + build + ci + chore + revert + requireScope: false + subjectPattern: ^(?![A-Z]).+[^.]$ + subjectPatternError: | + The subject "{subject}" should start lower case and not end with a period. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..e692eae --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,42 @@ +name: Release + +# Every merge to main is a candidate. semantic-release reads the commit subjects +# since the last tag and decides whether there is anything to cut — a docs or +# chore change releases nothing at all. +on: + push: + branches: [main] + +# The tag, the GitHub release, and the CHANGELOG commit all need writing. +permissions: + contents: write + issues: write + pull-requests: write + +concurrency: + group: release + cancel-in-progress: false + +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + # Tags are how semantic-release knows the current version, so the + # whole history has to be there. + fetch-depth: 0 + persist-credentials: false + + - uses: actions/setup-node@v7 + with: + node-version: 22 + + - uses: cycjimmy/semantic-release-action@v6 + with: + extra_plugins: | + @semantic-release/changelog@6 + @semantic-release/git@10 + conventional-changelog-conventionalcommits@8 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml new file mode 100644 index 0000000..e3366ab --- /dev/null +++ b/.github/workflows/static-analysis.yml @@ -0,0 +1,23 @@ +name: Static Analysis + +on: + pull_request: + push: + branches: [main] + +jobs: + phpstan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.5 + + - name: Install packages + run: composer install --prefer-dist --no-progress + + - name: Run PHPStan + run: composer analyse diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..9345764 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,37 @@ +name: Tests + +on: + pull_request: + push: + branches: [main] + +jobs: + tests: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + php: ['8.5'] + experimental: [false] + include: + # 8.6 is still in development and parts of the Tempest dependency tree + # do not install on it yet, so it reports without blocking a merge. + - php: '8.6' + experimental: true + + continue-on-error: ${{ matrix.experimental }} + + steps: + - uses: actions/checkout@v7 + + - name: Install PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + + - name: Install packages + run: composer install --prefer-dist --no-progress + + - name: Run tests + run: composer test diff --git a/.releaserc.json b/.releaserc.json new file mode 100644 index 0000000..8164edd --- /dev/null +++ b/.releaserc.json @@ -0,0 +1,32 @@ +{ + "branches": ["main"], + "tagFormat": "v${version}", + "plugins": [ + [ + "@semantic-release/commit-analyzer", + { + "preset": "conventionalcommits" + } + ], + [ + "@semantic-release/release-notes-generator", + { + "preset": "conventionalcommits" + } + ], + [ + "@semantic-release/changelog", + { + "changelogFile": "CHANGELOG.md" + } + ], + "@semantic-release/github", + [ + "@semantic-release/git", + { + "assets": ["CHANGELOG.md"], + "message": "chore(release): ${nextRelease.version} [skip ci]\n\n${nextRelease.notes}" + } + ] + ] +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..d78ab02 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,58 @@ +# Contributing + +## Commit messages decide releases + +Merges are squashed, and the pull request title becomes the commit subject on +`main`. That subject is read by semantic-release, which cuts the tag and the +GitHub release — so the title is not a formality, it is the version bump. + +Titles follow [Conventional Commits](https://www.conventionalcommits.org): + +``` +feat(cron): support step values over a range +fix(runner): keep a task that throws in the schedule +docs: explain why both day fields restricted means either +``` + +| Prefix | Release | +| --- | --- | +| `fix`, `perf` | patch — `1.2.3` → `1.2.4` | +| `feat` | minor — `1.2.3` → `1.3.0` | +| `feat!`, or `BREAKING CHANGE:` in the body | major — `1.2.3` → `2.0.0` | +| `docs`, `test`, `refactor`, `build`, `ci`, `chore` | none | + +A pull request whose title does not parse is rejected by a check before it can +be merged, because a subject semantic-release cannot read is a release that +silently never happens. + +Put the reasoning in the pull request body. It becomes the commit body, and it +is the part someone reads in a year when they are trying to work out why. + +## Breaking changes + +Mark them, and say what to do instead: + +``` +feat(task)!: name a method task after its class as well + +BREAKING CHANGE: an unnamed method task is now "Housekeeping::sweep" +rather than "sweep". Pass name: to keep the old one. +``` + +## Before opening a pull request + +```bash +composer test # PHPUnit +composer analyse # PHPStan +``` + +## Testing a scheduler + +The suite drives a fake loop rather than waiting out the schedules it exercises +— an interval of a second is the smallest the attribute allows, and a cron +task's next turn can be an hour away. `FakeLoop` records what was armed and +fires it on demand, so a test can say exactly when a turn happens and assert on +the wait that was chosen. + +Nothing here should need `sleep`. If a test seems to, the thing it is testing +probably wants a seam rather than the test wanting patience. diff --git a/README.md b/README.md index cd1fe0c..9e1eaaf 100644 --- a/README.md +++ b/README.md @@ -1,20 +1,9 @@ -# Tempcord Tasks Plugin +# Tempcord Tasks -Scheduled tasks plugin for the Tempcord Discord bot framework. Provides cron-based and interval-based task scheduling for your Discord bot. - -## Features - -- **Interval-based tasks** - Run tasks at regular intervals (every X seconds) -- **Cron-based tasks** - Schedule tasks using standard cron expressions -- **Task statistics** - Track execution times, success rates, and failures -- **Automatic discovery** - Tasks are auto-discovered via PHP attributes, in your bot or in - any package you install -- **Run on boot** - Optionally run tasks immediately when the bot starts - -## Requirements - -- PHP 8.5 or newer -- `tempcord/framework` 0.7 or newer +Scheduled tasks for the [Tempcord](https://github.com/Tempcord/framework) Discord bot framework: +work the bot does on a timer rather than in reply to anything — sweeping rows that have run +their course, lifting blocks that have expired, polling a service with no gateway event of +its own. ## Installation @@ -22,98 +11,145 @@ Scheduled tasks plugin for the Tempcord Discord bot framework. Provides cron-bas composer require tempcord/tasks ``` -That is the whole setup. Tempest discovers the package, `TasksPlugin` is picked up because -it implements `Tempcord\Plugins\Plugin`, and the scheduler starts with the bot — there is -nothing to register by hand. +## Declaring a task -## Usage - -### Basic Interval Task +On the class, the way every other Tempcord handler is declared: ```php use Tempcord\Plugins\Tasks\Attributes\Task; -class MyTasks +#[Task(interval: 10)] +final readonly class SweepTemporaryMessages { - #[Task(interval: 60)] // Run every 60 seconds - public function myTask(): void + public function __construct(private TempMessages $messages) {} + + public function __invoke(): void { - // Your task logic here + $this->messages->sweep(); } } ``` -### Cron-based Task +Or on a method, when several chores belong together and would only be split across classes +to satisfy the attribute: ```php -use Tempcord\Plugins\Tasks\Attributes\Task; - -class MyTasks +final readonly class Housekeeping { - #[Task(cron: '0 * * * *')] // Run every hour at minute 0 - public function hourlyTask(): void - { - // Your task logic here - } + #[Task(interval: 10)] + public function sweepMessages(): void {} - #[Task(cron: '@daily')] // Run once per day at midnight - public function dailyTask(): void - { - // Your task logic here - } + #[Task(cron: '@daily')] + public function pruneStatistics(): void {} } ``` -### Task Options +That is the whole registration. Tasks are discovered like commands and listeners, are built +by the container so they may take whatever dependencies they need, and are put on the event +loop before the gateway opens. + +## What the plugin guarantees + +A timer is less forgiving than an event listener: it fires again whether or not the last +turn finished or threw, forever. Three things are handled so you do not write them into +every task. + +- **A task that throws is logged and keeps its place.** Without that the exception travels + into the event loop, and the usual result is a cancelled timer and nothing ever swept + again — silently, because the bot carries on answering commands and looks healthy. +- **A task is never started alongside itself.** A turn still running when the next is due is + skipped, and the skip logged. Otherwise a task slower than its own interval makes every + following turn slower until nothing else gets a look in. +- **Each turn runs in a fiber**, so a task may `await` the REST API exactly as a command + handler does. + +An ordinary turn is logged at debug rather than info: a task running every ten seconds would +otherwise write eight thousand lines a day saying nothing happened. + +## Options ```php #[Task( - interval: 300, // Run every 5 minutes - runOnBoot: true, // Run immediately when bot starts - name: 'custom-name', // Custom task name (optional) - enabled: true // Enable/disable the task + interval: 300, // run every this many seconds + cron: '0 * * * *', // or on a cron schedule; the two are mutually exclusive + runOnBoot: true, // also take a turn as the bot starts, rather than waiting out + // the first interval + name: 'custom-name', // what to call it in the logs and in tasks:list + enabled: true, // false leaves it out of the schedule entirely )] -public function myTask(): void -{ - // Task logic -} ``` -### Supported Cron Expressions +A task with neither an interval nor a cron expression, with both, or with an interval under +a second is refused while the bot is starting rather than at the moment it would have run. +So is a cron expression that cannot be read. + +Left unnamed, a task is called after its class — `SweepTemporaryMessages` — or after the +class and method for a method-level task — `Housekeeping::sweepMessages`. The class is part +of it because two classes may well have a `sweep()`, and tasks sharing a name would share +their statistics and could not be cancelled apart. + +### The first turn + +The first turn comes after the interval unless `runOnBoot` says otherwise. A task is a +repeating chore; one-off startup work — reconciling against whatever changed while the bot +was down — usually belongs in a plugin's `boot()`, where its ordering against everything +else is visible. + +## Cron expressions + +Standard five-field format: -Standard 5-field cron format: ``` * * * * * │ │ │ │ │ -│ │ │ │ └─ Day of week (0-6, Sunday = 0) -│ │ │ └─── Month (1-12) -│ │ └───── Day of month (1-31) -│ └─────── Hour (0-23) -└───────── Minute (0-59) +│ │ │ │ └─ day of week (0-6, Sunday = 0) +│ │ │ └─── month (1-12) +│ │ └───── day of month (1-31) +│ └─────── hour (0-23) +└───────── minute (0-59) ``` -Aliases: -- `@yearly` or `@annually` - Run once a year (0 0 1 1 *) -- `@monthly` - Run once a month (0 0 1 * *) -- `@weekly` - Run once a week (0 0 * * 0) -- `@daily` or `@midnight` - Run once a day (0 0 * * *) -- `@hourly` - Run once an hour (0 * * * *) +Wildcards (`*`), lists (`1,15,30`), ranges (`9-17`) and steps (`*/5`, `1-10/2`) are +supported. A step over a range counts from where the range starts, so `1-10/2` is +1,3,5,7,9. + +Aliases: `@yearly` / `@annually`, `@monthly`, `@weekly`, `@daily` / `@midnight`, `@hourly`. + +**Both day fields restricted means either.** `0 0 1 * 1` runs on the first of the month +*and* on every Monday, which is what cron does and what anyone writing it expects. When only +one of the two names particular days, it simply narrows. -## Console Commands +A cron task is armed one turn at a time, for the exact wait until the next matching minute, +and re-armed from the turn just taken. Waking every minute to ask whether it is time yet +drifts, and once the drift crosses a minute boundary a matching minute is stepped over and +the task silently does not run that hour. + +## Console -List all registered tasks: ```bash php tempcord tasks:list ``` +Lists every registered task with its schedule and where it is declared. + +## Reaching the schedule at runtime + +```php +use function Tempcord\Plugins\Tasks\tasks; +use function Tempcord\Plugins\Tasks\cancelTask; +use function Tempcord\Plugins\Tasks\taskStats; + +taskStats()['SweepTemporaryMessages']->getSuccessRate(); +cancelTask('SweepTemporaryMessages'); +``` + +`Registry` is injectable, and is the better way in anything the container builds. + ## Requirements -- PHP 8.5 or higher -- Tempcord Framework ^0.6 -- Ragnarok Fenrir ^1 -- Tempest Console ^2 -- Tempest Core ^2 +- PHP 8.5 +- Tempcord Framework >= 0.10 ## License -MIT License +MIT diff --git a/composer.json b/composer.json index f009b13..453c2cc 100644 --- a/composer.json +++ b/composer.json @@ -1,16 +1,21 @@ { "name": "tempcord/tasks", "description": "Scheduled tasks plugin for Tempcord framework", - "version": "0.7.0", "type": "library", + "license": "MIT", "require": { "php": "^8.5", - "tempcord/framework": "^0.7", - "ragnarok/fenrir": "^1.0", + "tempcord/framework": ">=0.10 <1.0", + "react/event-loop": "^1.3", + "react/async": "^4.0", "tempest/console": "^3.18", - "tempest/core": "^3.18" + "tempest/core": "^3.18", + "tempest/log": "^3.18" + }, + "require-dev": { + "phpunit/phpunit": "^13.0", + "phpstan/phpstan": "^2.0" }, - "license": "MIT", "authors": [ { "name": "CyberWolf.Studio", @@ -43,10 +48,6 @@ "Tempcord\\Plugins\\Tasks\\": "src/" } }, - "require-dev": { - "phpunit/phpunit": "^13.0", - "phpstan/phpstan": "^2.1" - }, "autoload-dev": { "psr-4": { "Tempcord\\Plugins\\Tasks\\Tests\\": "tests/" diff --git a/phpunit.xml b/phpunit.xml index 6df8b57..e2757cf 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -3,13 +3,13 @@ xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="vendor/autoload.php" colors="true" + cacheDirectory=".phpunit.cache" failOnWarning="true" failOnNotice="true" - failOnDeprecation="true" - cacheDirectory=".phpunit.cache"> + failOnDeprecation="true"> - tests + tests/Unit diff --git a/src/Attributes/Task.php b/src/Attributes/Task.php index bff8a21..dc3a51f 100644 --- a/src/Attributes/Task.php +++ b/src/Attributes/Task.php @@ -6,54 +6,63 @@ use Attribute; use InvalidArgumentException; -use Tempest\Reflection\MethodReflector; /** - * Runs a method on a schedule, either every so many seconds or on a cron - * expression. + * Declares work the bot does on a timer. + * + * Goes on an invokable class, the way every other Tempcord attribute does, or + * on a method when several chores belong together and would only be split + * across classes to satisfy the attribute: + * + * #[Task(interval: 60)] + * final readonly class SweepTemporaryMessages { public function __invoke(): void {} } + * + * final readonly class Housekeeping + * { + * #[Task(interval: 10)] + * public function sweepMessages(): void {} + * + * #[Task(cron: '@daily')] + * public function pruneStatistics(): void {} + * } */ -#[Attribute(Attribute::TARGET_METHOD)] -final class Task +#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD)] +final readonly class Task { - public ?MethodReflector $reflector = null; - /** - * @param int|null $interval run every this many seconds; mutually exclusive with cron - * @param string|null $cron a cron expression; mutually exclusive with interval - * @param bool $runOnBoot also run once as soon as the bot starts - * @param string|null $name defaults to the method's own name - * @param bool $enabled a disabled task is discovered but never scheduled + * @param int|null $interval run every this many seconds; mutually exclusive with $cron + * @param string|null $cron a five field cron expression, or an alias such as '@daily' + * @param bool $runOnBoot also take a turn as soon as the bot starts, rather than + * waiting out the first interval + * @param string|null $name what to call it in the logs and in tasks:list; defaults to + * the class, or the class and method for a method level task + * @param bool $enabled a task registered but left out of the schedule */ public function __construct( - public readonly ?int $interval = null, - public readonly ?string $cron = null, - public readonly bool $runOnBoot = false, - public readonly ?string $name = null, - public readonly bool $enabled = true, + public ?int $interval = null, + public ?string $cron = null, + public bool $runOnBoot = false, + public ?string $name = null, + public bool $enabled = true, ) { if ($interval === null && $cron === null) { - throw new InvalidArgumentException('Task must have either an interval or cron expression'); + throw new InvalidArgumentException('A task must be given either an interval or a cron expression.'); } if ($interval !== null && $cron !== null) { - throw new InvalidArgumentException('Task cannot have both interval and cron expression'); + throw new InvalidArgumentException('A task cannot be given both an interval and a cron expression.'); } + /* + * A zero or negative interval asks the loop to run the task as fast as + * it can, which starves everything else the bot is doing, including the + * gateway heartbeat. + */ if ($interval !== null && $interval < 1) { - throw new InvalidArgumentException('Task interval must be at least 1 second'); + throw new InvalidArgumentException('A task interval must be at least one second.'); } } - public function setReflector(MethodReflector $reflector): void - { - $this->reflector = $reflector; - } - - public function getName(): string - { - return $this->name ?? $this->reflector?->getName() ?? 'unknown'; - } - public function isInterval(): bool { return $this->interval !== null; @@ -63,36 +72,4 @@ public function isCron(): bool { return $this->cron !== null; } - - public function getScheduleDescription(): string - { - if ($this->interval !== null) { - return $this->formatInterval($this->interval); - } - - return "cron: {$this->cron}"; - } - - private function formatInterval(int $seconds): string - { - if ($seconds < 60) { - return 'every ' . $seconds . ' second' . ($seconds > 1 ? 's' : ''); - } - - if ($seconds < 3600) { - $minutes = intdiv($seconds, 60); - - return 'every ' . $minutes . ' minute' . ($minutes > 1 ? 's' : ''); - } - - if ($seconds < 86400) { - $hours = intdiv($seconds, 3600); - - return 'every ' . $hours . ' hour' . ($hours > 1 ? 's' : ''); - } - - $days = intdiv($seconds, 86400); - - return 'every ' . $days . ' day' . ($days > 1 ? 's' : ''); - } } diff --git a/src/Compiler/TaskCompiler.php b/src/Compiler/TaskCompiler.php new file mode 100644 index 0000000..ea95a37 --- /dev/null +++ b/src/Compiler/TaskCompiler.php @@ -0,0 +1,77 @@ +getReflection()->hasMethod('__invoke')) { + throw new RuntimeException( + 'Class [' . $class->getName() . '] should declare an __invoke method', + ); + } + + return $this->build($task, $class, $class->getMethod('__invoke'), $class->getShortName()); + } + + /** + * A task declared on one method of a class that holds several. + */ + public function compileMethod(ClassReflector $class, MethodReflector $method, Task $task): TaskDefinition + { + return $this->build( + $task, + $class, + $method, + $class->getShortName() . '::' . $method->getName(), + ); + } + + private function build( + Task $task, + ClassReflector $class, + MethodReflector $method, + string $defaultName, + ): TaskDefinition { + /* + * Parsed here rather than when the timer is armed, so an expression + * nobody can read fails while the bot is starting and says which task + * it came from — not four hours later inside a timer callback. + */ + if ($task->cron !== null) { + try { + new CronExpression($task->cron); + } catch (Throwable $throwable) { + throw new RuntimeException( + 'Task [' . $defaultName . '] has an unreadable cron expression: ' . $throwable->getMessage(), + previous: $throwable, + ); + } + } + + return new TaskDefinition( + name: $task->name ?? $defaultName, + handler: $class->getName(), + method: $method, + interval: $task->interval, + cron: $task->cron, + runOnBoot: $task->runOnBoot, + enabled: $task->enabled, + ); + } +} diff --git a/src/ConsoleCommands/TasksListCommand.php b/src/ConsoleCommands/TasksListCommand.php index 5bc24da..0092142 100644 --- a/src/ConsoleCommands/TasksListCommand.php +++ b/src/ConsoleCommands/TasksListCommand.php @@ -11,52 +11,41 @@ final readonly class TasksListCommand { public function __construct( - private Registry $tasksRegistry, - private Console $console + private Registry $registry, + private Console $console, ) {} #[ConsoleCommand(name: 'tasks:list', description: 'List all registered scheduled tasks')] public function __invoke(): void { - $tasks = $this->tasksRegistry; - - if ($tasks->count() === 0) { + if ($this->registry->count() === 0) { $this->console->writeln("No tasks registered"); + return; } $this->console->info('Registered Tasks:'); - foreach ($tasks->getAllTasks() as $index => $task) { + foreach ($this->registry->all() as $task) { $status = $task->enabled ? '✓' : '✗'; - $name = $task->getName(); - $schedule = $task->getScheduleDescription(); $runOnBoot = $task->runOnBoot ? " (runs on boot)" : ''; - $class = $task->reflector?->getDeclaringClass()->getShortName() ?? 'Unknown'; $this->console->writeln(sprintf( ' %s %s - %s%s', $status, - $name, - $schedule, - $runOnBoot + $task->name, + $task->schedule(), + $runOnBoot, )); $this->console->writeln(sprintf( " %s::%s()", - $class, - $task->reflector?->getName() ?? 'unknown' + $task->handler, + $task->method->getName(), )); - - if ($index < $tasks->count() - 1) { - $this->console->writeln(''); - } } $this->console->writeln(''); - $this->console->writeln(sprintf( - 'Total: %d task(s)', - $tasks->count() - )); + $this->console->writeln(sprintf('Total: %d task(s)', $this->registry->count())); } } diff --git a/src/Definitions/TaskDefinition.php b/src/Definitions/TaskDefinition.php new file mode 100644 index 0000000..8c48522 --- /dev/null +++ b/src/Definitions/TaskDefinition.php @@ -0,0 +1,62 @@ +interval !== null; + } + + public function isCron(): bool + { + return $this->cron !== null; + } + + /** + * How the schedule reads in the logs and in tasks:list. + */ + public function schedule(): string + { + if ($this->cron !== null) { + return 'cron: ' . $this->cron; + } + + return 'every ' . $this->humanInterval((int) $this->interval); + } + + private function humanInterval(int $seconds): string + { + [$size, $unit] = match (true) { + $seconds < 60 => [$seconds, 'second'], + $seconds < 3600 => [intdiv($seconds, 60), 'minute'], + $seconds < 86400 => [intdiv($seconds, 3600), 'hour'], + default => [intdiv($seconds, 86400), 'day'], + }; + + return $size . ' ' . $unit . ($size === 1 ? '' : 's'); + } +} diff --git a/src/Discoveries/TasksDiscovery.php b/src/Discoveries/TasksDiscovery.php index 574830a..16df650 100644 --- a/src/Discoveries/TasksDiscovery.php +++ b/src/Discoveries/TasksDiscovery.php @@ -5,45 +5,39 @@ namespace Tempcord\Plugins\Tasks\Discoveries; use Tempcord\Plugins\Tasks\Attributes\Task; +use Tempcord\Plugins\Tasks\Compiler\TaskCompiler; use Tempcord\Plugins\Tasks\Registry; use Tempest\Discovery\Discovery; use Tempest\Discovery\DiscoveryLocation; use Tempest\Discovery\IsDiscovery; use Tempest\Reflection\ClassReflector; -/** - * Finds every #[Task] method, wherever it lives. - * - * Discovery reaches the bot's own code and any installed package alike, so a - * package can ship scheduled tasks of its own. - */ final class TasksDiscovery implements Discovery { use IsDiscovery; public function __construct( private readonly Registry $registry, + private readonly TaskCompiler $compiler = new TaskCompiler(), ) {} public function discover(DiscoveryLocation $location, ClassReflector $class): void { - foreach ($class->getPublicMethods() as $method) { - $task = $method->getAttribute(Task::class); + foreach ($class->getAttributes(Task::class) as $task) { + $this->discoveryItems->add($location, $this->compiler->compileClass($class, $task)); + } - if ($task === null) { - continue; + foreach ($class->getPublicMethods() as $method) { + foreach ($method->getAttributes(Task::class) as $task) { + $this->discoveryItems->add($location, $this->compiler->compileMethod($class, $method, $task)); } - - $task->setReflector($method); - - $this->discoveryItems->add($location, $task); } } public function apply(): void { foreach ($this->discoveryItems as $task) { - $this->registry->register($task); + $this->registry->add($task); } } } diff --git a/src/Registry.php b/src/Registry.php index f37299c..ff8ff37 100644 --- a/src/Registry.php +++ b/src/Registry.php @@ -4,94 +4,99 @@ namespace Tempcord\Plugins\Tasks; -use Ragnarok\Fenrir\Discord; -use Ragnarok\Fenrir\Extension\Extension; -use React\EventLoop\Loop; -use Tempcord\Plugins\Tasks\Attributes\Task; +use React\EventLoop\LoopInterface; +use Tempcord\Plugins\Tasks\Definitions\TaskDefinition; use Tempcord\Plugins\Tasks\Support\TaskStats; use Tempest\Container\Container; use Tempest\Container\Singleton; use Tempest\Log\Logger; /** - * Holds every discovered task and starts them once Discord is ready. + * Holds every discovered task and puts it on the event loop. */ #[Singleton] -final class Registry implements Extension +final class Registry { - /** @var array */ + /** @var array keyed by name, so a task discovered twice is scheduled once */ private array $tasks = []; private ?Runner $runner = null; public function __construct( private readonly Container $container, - private readonly Logger $logger, ) {} - public function register(Task $task): void + public function add(TaskDefinition $task): void { - // Keyed by name so a task discovered twice is scheduled once. - $this->tasks[$task->getName()] = $task; + $this->tasks[$task->name] = $task; } /** - * Called by Fenrir once the extension is registered, which the plugin does - * as the bot boots. + * @return list */ - public function initialize(Discord $discord): void + public function all(): array + { + return array_values($this->tasks); + } + + public function count(): int + { + return count($this->tasks); + } + + /** + * Arms every task's timer. Nothing takes a turn until the loop itself runs, + * which is after the gateway opens. + * + * @return list what was scheduled, for the caller to report + */ + public function start(LoopInterface $loop): array { if ($this->tasks === []) { - return; + return []; } - $this->runner ??= new Runner(Loop::get(), $this->logger, $this->container); + /* + * Resolved here rather than in the constructor: discovery builds this + * registry while the container is still being assembled, before the + * initializers that provide the logger have themselves been found. + */ + $this->runner = new Runner($loop, $this->container, $this->container->get(Logger::class)); + + $scheduled = []; foreach ($this->tasks as $task) { - $this->runner->schedule($task); + if ($this->runner->schedule($task)) { + $scheduled[] = $task->name . ' (' . $task->schedule() . ')'; + } } - $this->logger->info('Task scheduler initialized', ['tasks' => count($this->tasks)]); + return $scheduled; } - public function cancelTask(string $taskName): bool + public function cancel(string $taskName): bool { return $this->runner?->cancel($taskName) ?? false; } - public function cancelAllTasks(): void + public function cancelAll(): void { $this->runner?->cancelAll(); } /** - * Execution statistics per task, empty until the scheduler has started. - * * @return array */ - public function getStats(): array + public function stats(): array { - return $this->runner?->getStats() ?? []; + return $this->runner?->stats() ?? []; } /** * @return list */ - public function getScheduledTasks(): array - { - return $this->runner?->getScheduledTasks() ?? []; - } - - public function count(): int - { - return count($this->tasks); - } - - /** - * @return list - */ - public function getAllTasks(): array + public function scheduled(): array { - return array_values($this->tasks); + return $this->runner?->scheduled() ?? []; } } diff --git a/src/Runner.php b/src/Runner.php index 821055a..9086b2b 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -4,150 +4,154 @@ namespace Tempcord\Plugins\Tasks; -use DateTimeImmutable; use React\EventLoop\LoopInterface; -use Tempcord\Plugins\Tasks\Attributes\Task; +use React\EventLoop\TimerInterface; +use Tempcord\Plugins\Tasks\Definitions\TaskDefinition; use Tempcord\Plugins\Tasks\Support\CronExpression; use Tempcord\Plugins\Tasks\Support\TaskStats; use Tempest\Container\Container; use Tempest\Log\Logger; use Throwable; +use function React\Async\async; + /** - * Manages the execution of scheduled tasks + * Runs scheduled tasks, and keeps them running. + * + * A timer is less forgiving than an event listener: it fires again whether or + * not the last turn finished or threw, forever. Both are contained here, so + * that a task cannot quietly stop the bot doing its other work. */ final class Runner { - /** @var array Timer references for cleanup */ + /** @var array */ private array $timers = []; - /** @var array Last run times for cron tasks */ - private array $lastRunTimes = []; + /** @var array tasks whose previous turn has not finished */ + private array $running = []; - /** @var array Task execution statistics */ + /** @var array */ private array $stats = []; public function __construct( private readonly LoopInterface $loop, - private readonly Logger $logger, private readonly Container $container, + private readonly Logger $logger, ) {} /** - * Schedule a task for execution + * @return bool whether the task was put on the schedule */ - public function schedule(Task $task): void + public function schedule(TaskDefinition $task): bool { if (!$task->enabled) { - $this->logger->info("Task '{$task->getName()}' is disabled, skipping"); - return; + $this->logger->info('Task ' . $task->name . ' is disabled and was left out of the schedule.'); + + return false; } - $taskName = $task->getName(); - $this->stats[$taskName] = new TaskStats($taskName); + $this->stats[$task->name] = new TaskStats($task->name); - if ($task->isInterval()) { - $this->scheduleIntervalTask($task); - } else { - $this->scheduleCronTask($task); + if ($task->runOnBoot) { + /* + * On the next tick rather than now, so a task cannot run before the + * rest of the bot has finished being wired together. + */ + $this->loop->futureTick(fn() => $this->run($task)); } - $this->logger->info("Scheduled task '{$taskName}'", [ - 'schedule' => $task->getScheduleDescription(), - 'runOnBoot' => $task->runOnBoot, - ]); + $task->isInterval() + ? $this->armInterval($task) + : $this->armCron($task, new CronExpression((string) $task->cron)); + + return true; } - /** - * Schedule an interval-based task - */ - private function scheduleIntervalTask(Task $task): void + private function armInterval(TaskDefinition $task): void { - $taskName = $task->getName(); - $interval = $task->interval; - - // Run immediately if configured - if ($task->runOnBoot) { - $this->loop->futureTick(fn() => $this->executeTask($task)); - } - - // Schedule periodic execution - $timer = $this->loop->addPeriodicTimer($interval, fn() => $this->executeTask($task)); - $this->timers[$taskName] = $timer; + $this->timers[$task->name] = $this->loop->addPeriodicTimer( + (int) $task->interval, + fn() => $this->run($task), + ); } /** - * Schedule a cron-based task + * Cron is armed one turn at a time, for the exact number of seconds until + * the next minute that matches. + * + * Waking every minute instead would drift: the first tick lands at whatever + * offset within the minute the bot happened to start at, and once the drift + * crosses a minute boundary a matching minute is stepped over entirely and + * the task silently does not run that hour. */ - private function scheduleCronTask(Task $task): void + private function armCron(TaskDefinition $task, CronExpression $cron): void { - $taskName = $task->getName(); - $cron = new CronExpression($task->cron); - - // Run immediately if configured - if ($task->runOnBoot) { - $this->loop->futureTick(fn() => $this->executeTask($task)); - } + $seconds = max(1, $cron->getSecondsUntilNextRun()); - // Check every minute if cron matches - $timer = $this->loop->addPeriodicTimer(60, function () use ($task, $cron, $taskName) { - $now = new DateTimeImmutable(); - $currentMinute = $now->format('Y-m-d H:i'); + $this->timers[$task->name] = $this->loop->addTimer($seconds, function () use ($task, $cron): void { + $this->run($task); - // Avoid running twice in the same minute - $lastRun = $this->lastRunTimes[$taskName] ?? null; - if ($lastRun !== null && $lastRun->format('Y-m-d H:i') === $currentMinute) { - return; - } - - if ($cron->matches($now)) { - $this->lastRunTimes[$taskName] = $now; - $this->executeTask($task); - } + // Re-armed from the turn just taken, so the wait is recomputed + // rather than accumulated. + $this->armCron($task, $cron); }); - - $this->timers[$taskName] = $timer; } - /** - * Execute a task - */ - private function executeTask(Task $task): void + private function run(TaskDefinition $task): void { - $taskName = $task->getName(); - $startTime = microtime(true); - - $this->logger->debug("Running task '{$taskName}'"); - - try { - $instance = $this->container->get($task->reflector->getDeclaringClass()->getName()); - - $task->reflector->invokeArgs($instance); + /* + * A task that takes longer than its own interval would otherwise be + * started again alongside itself, and each turn would make the next one + * slower until nothing else got a look in. + */ + if (isset($this->running[$task->name])) { + $this->logger->warning( + 'Task ' . $task->name . ' is still busy from its last turn; skipping this one.', + ); - $duration = round((microtime(true) - $startTime) * 1000, 2); - - $this->stats[$taskName]->recordSuccess($duration); + return; + } - $this->logger->info("Task '{$taskName}' completed", [ - 'duration' => "{$duration}ms", - 'runs' => $this->stats[$taskName]->totalRuns, - ]); - } catch (Throwable $e) { - $duration = round((microtime(true) - $startTime) * 1000, 2); + $this->running[$task->name] = true; + $startedAt = microtime(true); + + /* + * In a fiber, so a task may await the REST API the way a command + * handler does, and inside a catch, so one that throws is logged rather + * than travelling up into the event loop and taking the process with it. + */ + async(function () use ($task, $startedAt): void { + try { + $task->method->invokeArgs($this->container->get($task->handler), []); + + $this->record($task)?->recordSuccess($this->msSince($startedAt)); + + // Debug, not info: a task running every ten seconds would + // otherwise write eight thousand lines a day saying nothing. + $this->logger->debug('Task ' . $task->name . ' finished.'); + } catch (Throwable $throwable) { + $this->record($task)?->recordFailure($this->msSince($startedAt), $throwable->getMessage()); + + $this->logger->error( + 'Task ' . $task->name . ' failed: ' . $throwable->getMessage(), + ['exception' => $throwable], + ); + } finally { + unset($this->running[$task->name]); + } + })(); + } - $this->stats[$taskName]->recordFailure($duration, $e->getMessage()); + private function record(TaskDefinition $task): ?TaskStats + { + return $this->stats[$task->name] ?? null; + } - $this->logger->error("Task '{$taskName}' failed", [ - 'error' => $e->getMessage(), - 'duration' => "{$duration}ms", - 'failures' => $this->stats[$taskName]->failures, - ]); - } + private function msSince(float $startedAt): float + { + return round((microtime(true) - $startedAt) * 1000, 2); } - /** - * Cancel a scheduled task - */ public function cancel(string $taskName): bool { if (!isset($this->timers[$taskName])) { @@ -157,14 +161,11 @@ public function cancel(string $taskName): bool $this->loop->cancelTimer($this->timers[$taskName]); unset($this->timers[$taskName]); - $this->logger->info("Cancelled task '{$taskName}'"); + $this->logger->info('Task ' . $taskName . ' was cancelled.'); return true; } - /** - * Cancel all scheduled tasks - */ public function cancelAll(): void { foreach (array_keys($this->timers) as $taskName) { @@ -172,10 +173,7 @@ public function cancelAll(): void } } - /** - * Get statistics for a specific task - */ - public function getTaskStats(string $taskName): ?TaskStats + public function statsFor(string $taskName): ?TaskStats { return $this->stats[$taskName] ?? null; } @@ -183,16 +181,15 @@ public function getTaskStats(string $taskName): ?TaskStats /** * @return array */ - public function getStats(): array + public function stats(): array { return $this->stats; } /** - * Get list of scheduled task names - * @return array + * @return list */ - public function getScheduledTasks(): array + public function scheduled(): array { return array_keys($this->timers); } diff --git a/src/Support/CronExpression.php b/src/Support/CronExpression.php index 005d4d5..84b9950 100644 --- a/src/Support/CronExpression.php +++ b/src/Support/CronExpression.php @@ -7,93 +7,117 @@ use DateTimeImmutable; use DateTimeInterface; use InvalidArgumentException; +use RuntimeException; /** - * Simple cron expression parser + * A five field cron expression. * - * Supports standard 5-field cron format: - * ┌───────────── minute (0-59) - * │ ┌───────────── hour (0-23) - * │ │ ┌───────────── day of month (1-31) - * │ │ │ ┌───────────── month (1-12) - * │ │ │ │ ┌───────────── day of week (0-6, Sunday = 0) - * │ │ │ │ │ - * * * * * * + * ┌───────────── minute (0-59) + * │ ┌───────────── hour (0-23) + * │ │ ┌───────────── day of month (1-31) + * │ │ │ ┌───────────── month (1-12) + * │ │ │ │ ┌───────────── day of week (0-6, Sunday = 0) + * │ │ │ │ │ + * * * * * * */ final class CronExpression { + /** @var list */ private array $minutes; + + /** @var list */ private array $hours; + + /** @var list */ private array $daysOfMonth; + + /** @var list */ private array $months; + + /** @var list */ private array $daysOfWeek; + /** + * Whether each of the two day fields names particular days rather than + * standing open. Cron reads the pair as "or" when both are restricted. + */ + private bool $dayOfMonthRestricted; + + private bool $dayOfWeekRestricted; + public function __construct( - public readonly string $expression + public readonly string $expression, ) { $this->parse($expression); } + public function matches(DateTimeInterface $moment): bool + { + $matchesTime = in_array((int) $moment->format('i'), $this->minutes, true) + && in_array((int) $moment->format('G'), $this->hours, true) + && in_array((int) $moment->format('n'), $this->months, true); + + return $matchesTime && $this->matchesDay($moment); + } + /** - * Check if the cron expression matches the given time + * Cron's one genuine oddity: when a line restricts the day of the month + * *and* the day of the week, it runs on days matching either, not both. So + * `0 0 1 * 1` is the first of the month and every Monday, which is what + * anyone writing it expects and not what an "and" would give them. */ - public function matches(DateTimeInterface $dateTime): bool + private function matchesDay(DateTimeInterface $moment): bool { - $minute = (int) $dateTime->format('i'); - $hour = (int) $dateTime->format('G'); - $dayOfMonth = (int) $dateTime->format('j'); - $month = (int) $dateTime->format('n'); - $dayOfWeek = (int) $dateTime->format('w'); - - return in_array($minute, $this->minutes, true) - && in_array($hour, $this->hours, true) - && in_array($dayOfMonth, $this->daysOfMonth, true) - && in_array($month, $this->months, true) - && in_array($dayOfWeek, $this->daysOfWeek, true); + $dayOfMonth = in_array((int) $moment->format('j'), $this->daysOfMonth, true); + $dayOfWeek = in_array((int) $moment->format('w'), $this->daysOfWeek, true); + + if ($this->dayOfMonthRestricted && $this->dayOfWeekRestricted) { + return $dayOfMonth || $dayOfWeek; + } + + return $dayOfMonth && $dayOfWeek; } /** - * Get the next run time after the given time + * The first minute at or after the one following the given moment that this + * expression matches. */ public function getNextRunDate(DateTimeInterface $from): DateTimeImmutable { - $next = DateTimeImmutable::createFromInterface($from); - $next = $next->modify('+1 minute')->setTime( - (int) $next->modify('+1 minute')->format('G'), - (int) $next->modify('+1 minute')->format('i'), - 0 - ); + $next = DateTimeImmutable::createFromInterface($from) + ->modify('+1 minute') + ->setTime( + (int) DateTimeImmutable::createFromInterface($from)->modify('+1 minute')->format('G'), + (int) DateTimeImmutable::createFromInterface($from)->modify('+1 minute')->format('i'), + 0, + ); - // Search for up to 4 years to find next match - $maxIterations = 60 * 24 * 366 * 4; + // A day of the month that never comes round in a given month — the 31st + // of February — still resolves within four years, or not at all. + $limit = 60 * 24 * 366 * 4; - for ($i = 0; $i < $maxIterations; $i++) { + for ($minute = 0; $minute < $limit; $minute++) { if ($this->matches($next)) { return $next; } + $next = $next->modify('+1 minute'); } - throw new \RuntimeException('Could not find next run date within 4 years'); + throw new RuntimeException( + 'The expression "' . $this->expression . '" does not come round within four years.', + ); } - /** - * Get seconds until the next run - */ public function getSecondsUntilNextRun(?DateTimeInterface $from = null): int { $from ??= new DateTimeImmutable(); - $next = $this->getNextRunDate($from); - return $next->getTimestamp() - $from->getTimestamp(); + return $this->getNextRunDate($from)->getTimestamp() - $from->getTimestamp(); } - /** - * Parse the cron expression - */ private function parse(string $expression): void { - // Handle common aliases $expression = match (strtolower(trim($expression))) { '@yearly', '@annually' => '0 0 1 1 *', '@monthly' => '0 0 1 * *', @@ -103,11 +127,11 @@ private function parse(string $expression): void default => trim($expression), }; - $parts = preg_split('/\s+/', $expression); + $parts = preg_split('/\s+/', $expression) ?: []; if (count($parts) !== 5) { throw new InvalidArgumentException( - "Invalid cron expression '{$expression}'. Expected 5 fields: minute hour day month weekday" + 'Invalid cron expression "' . $expression . '". Expected 5 fields: minute hour day month weekday', ); } @@ -118,68 +142,71 @@ private function parse(string $expression): void $this->daysOfMonth = $this->parseField($dayOfMonth, 1, 31); $this->months = $this->parseField($month, 1, 12); $this->daysOfWeek = $this->parseField($dayOfWeek, 0, 6); + + $this->dayOfMonthRestricted = trim($dayOfMonth) !== '*'; + $this->dayOfWeekRestricted = trim($dayOfWeek) !== '*'; } /** - * Parse a single cron field + * @return list */ private function parseField(string $field, int $min, int $max): array { $values = []; - // Handle comma-separated values - $parts = explode(',', $field); - - foreach ($parts as $part) { - $values = array_merge($values, $this->parsePart($part, $min, $max)); + foreach (explode(',', $field) as $part) { + $values = [...$values, ...$this->parsePart($part, $min, $max)]; } - $values = array_unique($values); + $values = array_values(array_unique($values)); sort($values); return $values; } /** - * Parse a single part of a cron field + * @return list */ private function parsePart(string $part, int $min, int $max): array { - // Handle wildcard (*) if ($part === '*') { return range($min, $max); } - // Handle step values (*/5, 1-10/2) if (str_contains($part, '/')) { [$range, $step] = explode('/', $part, 2); $step = (int) $step; - if ($range === '*') { - $rangeValues = range($min, $max); - } else { - $rangeValues = $this->parsePart($range, $min, $max); + if ($step < 1) { + throw new InvalidArgumentException('A cron step must be at least 1, got "' . $part . '".'); } + $values = $range === '*' ? range($min, $max) : $this->parsePart($range, $min, $max); + + /* + * Counted from where the range starts, not from the bottom of the + * field: 1-10/2 is 1,3,5,7,9 — every second value beginning at one + * — and not 2,4,6,8,10. + */ + $start = $values[0]; + return array_values(array_filter( - $rangeValues, - fn($v) => ($v - $min) % $step === 0 + $values, + static fn(int $value) => ($value - $start) % $step === 0, )); } - // Handle ranges (1-5) if (str_contains($part, '-')) { - [$start, $end] = explode('-', $part, 2); - $start = max($min, (int) $start); - $end = min($max, (int) $end); - return range($start, $end); + [$from, $to] = explode('-', $part, 2); + + return range(max($min, (int) $from), min($max, (int) $to)); } - // Handle single value $value = (int) $part; - if ($value < $min || $value > $max) { + + if ($value < $min || $value > $max || !ctype_digit(trim($part))) { throw new InvalidArgumentException( - "Value {$value} is out of range [{$min}-{$max}]" + 'Value "' . $part . '" is out of range [' . $min . '-' . $max . ']', ); } diff --git a/src/TasksPlugin.php b/src/TasksPlugin.php index 934b453..4a20472 100644 --- a/src/TasksPlugin.php +++ b/src/TasksPlugin.php @@ -4,24 +4,35 @@ namespace Tempcord\Plugins\Tasks; +use React\EventLoop\Loop; use Tempcord\Plugins\Plugin; use Tempcord\Tempcord; +use Tempest\Log\Logger; /** - * Interval and cron scheduling for Tempcord bots. + * Puts every discovered task on the event loop. * - * The #[Task] attribute is found by TasksDiscovery wherever it appears, in the - * bot's own code or in another package. This only has to hand the registry to - * Discord, so its timers start with the gateway. + * A plugin boots after commands and events are bound and before the gateway + * opens, which is exactly when timers want arming: nothing fires until the loop + * itself runs, so no task can take a turn against a half-built bot. */ final readonly class TasksPlugin implements Plugin { public function __construct( private Registry $registry, + private Logger $logger, ) {} public function boot(Tempcord $tempcord): void { - $tempcord->discord->registerExtension($this->registry); + $scheduled = $this->registry->start(Loop::get()); + + if ($scheduled === []) { + return; + } + + $this->logger->info( + 'Scheduled ' . count($scheduled) . ' task(s): ' . implode(', ', $scheduled), + ); } } diff --git a/src/functions.php b/src/functions.php index a61a09c..c8d12ba 100644 --- a/src/functions.php +++ b/src/functions.php @@ -5,14 +5,13 @@ namespace Tempcord\Plugins\Tasks; use Tempcord\Plugins\Tasks\Support\TaskStats; + use function Tempest\Container\get; if (!function_exists('Tempcord\Plugins\Tasks\tasks')) { /** - * The task registry, for inspecting or cancelling scheduled tasks. - * - * Prefer injecting Registry where you can; this exists for the places a - * container is awkward to reach, such as a closure in configuration. + * The task registry, for reaching the schedule from somewhere the container + * does not inject into. */ function tasks(): Registry { @@ -21,7 +20,7 @@ function tasks(): Registry function cancelTask(string $taskName): bool { - return tasks()->cancelTask($taskName); + return tasks()->cancel($taskName); } /** @@ -29,6 +28,6 @@ function cancelTask(string $taskName): bool */ function taskStats(): array { - return tasks()->getStats(); + return tasks()->stats(); } } diff --git a/tests/Doubles/FakeDiscord.php b/tests/Doubles/FakeDiscord.php deleted file mode 100644 index 9a968d3..0000000 --- a/tests/Doubles/FakeDiscord.php +++ /dev/null @@ -1,33 +0,0 @@ -newInstanceWithoutConstructor(); - $gateway->events = new EventHandler($mapper); - - $this->gateway = $gateway; - } -} diff --git a/tests/Doubles/FakeLoop.php b/tests/Doubles/FakeLoop.php new file mode 100644 index 0000000..7e40a5e --- /dev/null +++ b/tests/Doubles/FakeLoop.php @@ -0,0 +1,102 @@ + */ + public array $timers = []; + + /** @var list */ + public array $futureTicks = []; + + public function addTimer($interval, $callback): TimerInterface + { + $timer = new FakeTimer((float) $interval, $callback, periodic: false); + $this->timers[] = $timer; + + return $timer; + } + + public function addPeriodicTimer($interval, $callback): TimerInterface + { + $timer = new FakeTimer((float) $interval, $callback, periodic: true); + $this->timers[] = $timer; + + return $timer; + } + + public function cancelTimer(TimerInterface $timer): void + { + $this->timers = array_values(array_filter( + $this->timers, + static fn(FakeTimer $known) => $known !== $timer, + )); + } + + public function futureTick($listener): void + { + $this->futureTicks[] = $listener; + } + + /** + * Fires every timer currently armed, once. + * + * The list is copied first: a cron task re-arms itself from inside its own + * callback, and the new timer belongs to the next turn rather than this one. + */ + public function tick(int $times = 1): void + { + for ($turn = 0; $turn < $times; $turn++) { + foreach ($this->timers as $timer) { + $timer->fire(); + } + } + } + + public function drainFutureTicks(): void + { + $ticks = $this->futureTicks; + $this->futureTicks = []; + + foreach ($ticks as $tick) { + $tick(); + } + } + + /** + * The most recently armed timer, which for a cron task is the one waiting + * for the next matching minute. + */ + public function lastTimer(): ?FakeTimer + { + return $this->timers === [] ? null : $this->timers[count($this->timers) - 1]; + } + + public function addReadStream($stream, $listener): void {} + + public function addWriteStream($stream, $listener): void {} + + public function removeReadStream($stream): void {} + + public function removeWriteStream($stream): void {} + + public function addSignal($signal, $listener): void {} + + public function removeSignal($signal, $listener): void {} + + public function run(): void {} + + public function stop(): void {} +} diff --git a/tests/Doubles/FakeTimer.php b/tests/Doubles/FakeTimer.php new file mode 100644 index 0000000..66983d7 --- /dev/null +++ b/tests/Doubles/FakeTimer.php @@ -0,0 +1,36 @@ +interval; + } + + public function getCallback(): callable + { + return $this->callback; + } + + public function isPeriodic(): bool + { + return $this->periodic; + } + + public function fire(): void + { + ($this->callback)($this); + } +} diff --git a/tests/Doubles/RecordingLogger.php b/tests/Doubles/RecordingLogger.php new file mode 100644 index 0000000..d525618 --- /dev/null +++ b/tests/Doubles/RecordingLogger.php @@ -0,0 +1,35 @@ + */ + public array $messages = []; + + /** @var list the level each message was logged at, in step with $messages */ + public array $levels = []; + + public function log($level, string|Stringable $message, array $context = []): void + { + $this->messages[] = (string) $message; + $this->levels[] = (string) $level; + } + + public function has(string $needle): bool + { + foreach ($this->messages as $message) { + if (str_contains($message, $needle)) { + return true; + } + } + + return false; + } +} diff --git a/tests/Fixtures/BootTask.php b/tests/Fixtures/BootTask.php new file mode 100644 index 0000000..d5a2c5d --- /dev/null +++ b/tests/Fixtures/BootTask.php @@ -0,0 +1,22 @@ +promise()); + } +} diff --git a/tests/Fixtures/SweepMessages.php b/tests/Fixtures/SweepMessages.php new file mode 100644 index 0000000..7d4f5ce --- /dev/null +++ b/tests/Fixtures/SweepMessages.php @@ -0,0 +1,21 @@ +createStub(Logger::class)); - } - - private function task(string $method, ?string $name = null): Task - { - $task = new Task(interval: 60, name: $name); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod($method)); - - return $task; - } - - public function test_it_holds_registered_tasks(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - - $this->assertSame(1, $registry->count()); - $this->assertSame('everyMinute', $registry->getAllTasks()[0]->getName()); - } - - /** - * Discovery can reach the same method through more than one location, and - * scheduling it twice would run it twice on every tick. - */ - public function test_the_same_task_registered_twice_is_held_once(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - $registry->register($this->task('everyMinute')); - - $this->assertSame(1, $registry->count()); - } - - /** - * Statistics come from the runner, which does not exist until the scheduler - * starts. This used to return the runner itself, so callers expecting a map - * of statistics got an object. - */ - public function test_statistics_are_empty_before_the_scheduler_starts(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - - $this->assertSame([], $registry->getStats()); - $this->assertSame([], $registry->getScheduledTasks()); - } - - public function test_cancelling_before_the_scheduler_starts_reports_failure(): void - { - $this->assertFalse($this->registry()->cancelTask('everyMinute')); - } -} diff --git a/tests/RunnerTest.php b/tests/RunnerTest.php deleted file mode 100644 index 8c74e7a..0000000 --- a/tests/RunnerTest.php +++ /dev/null @@ -1,113 +0,0 @@ -createStub(Logger::class), new GenericContainer()); - } - - private function task(string $method, ?int $interval = 60, ?string $cron = null, bool $enabled = true, bool $runOnBoot = false): Task - { - $task = new Task(interval: $interval, cron: $cron, runOnBoot: $runOnBoot, enabled: $enabled); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod($method)); - - return $task; - } - - public function test_it_schedules_an_interval_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $this->assertSame(['everyMinute'], $runner->getScheduledTasks()); - - $runner->cancelAll(); - } - - public function test_a_disabled_task_is_never_scheduled(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('disabled', enabled: false)); - - $this->assertSame([], $runner->getScheduledTasks()); - $this->assertNull($runner->getTaskStats('disabled')); - } - - public function test_a_cron_task_is_scheduled_on_a_ticking_timer(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('report', interval: null, cron: '0 * * * *')); - - $this->assertSame(['report'], $runner->getScheduledTasks()); - - $runner->cancelAll(); - } - - public function test_cancelling_removes_a_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $this->assertTrue($runner->cancel('everyMinute')); - $this->assertSame([], $runner->getScheduledTasks()); - $this->assertFalse($runner->cancel('everyMinute')); - } - - /** - * Statistics exist from the moment a task is scheduled, so a task that has - * not run yet still reports zero runs rather than nothing at all. - */ - public function test_statistics_exist_from_scheduling(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $stats = $runner->getStats(); - - $this->assertArrayHasKey('everyMinute', $stats); - $this->assertSame(0, $stats['everyMinute']->totalRuns); - - $runner->cancelAll(); - } - - /** - * runOnBoot queues the task on the next tick rather than running it inline, - * so scheduling never blocks the boot sequence. - */ - public function test_run_on_boot_executes_the_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute', runOnBoot: true)); - - $this->assertSame([], ScheduledCommands::$ran, 'should not have run inline'); - - Loop::get()->futureTick(static fn() => Loop::get()->stop()); - Loop::get()->run(); - - $this->assertSame(['everyMinute'], ScheduledCommands::$ran); - $this->assertSame(1, $runner->getStats()['everyMinute']->totalRuns); - - $runner->cancelAll(); - } -} diff --git a/tests/TaskTest.php b/tests/TaskTest.php deleted file mode 100644 index b5ab3dd..0000000 --- a/tests/TaskTest.php +++ /dev/null @@ -1,99 +0,0 @@ -expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('either an interval or cron expression'); - - new Task(); - } - - public function test_a_task_cannot_have_both_kinds_of_schedule(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('cannot have both'); - - new Task(interval: 60, cron: '* * * * *'); - } - - public function test_an_interval_below_a_second_is_rejected(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('at least 1 second'); - - new Task(interval: 0); - } - - /** - * The name falls back to the method it sits on, which is only known once - * discovery has attached the reflector. - */ - public function test_the_name_falls_back_to_the_method(): void - { - $task = new Task(interval: 60); - - $this->assertSame('unknown', $task->getName()); - - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod('everyMinute')); - - $this->assertSame('everyMinute', $task->getName()); - } - - public function test_an_explicit_name_wins(): void - { - $task = new Task(cron: '0 * * * *', name: 'hourly-report'); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod('report')); - - $this->assertSame('hourly-report', $task->getName()); - } - - /** @return array */ - public static function intervals(): array - { - return [ - 'one second' => [1, 'every 1 second'], - 'seconds' => [30, 'every 30 seconds'], - 'one minute' => [60, 'every 1 minute'], - 'minutes' => [300, 'every 5 minutes'], - 'hours' => [7200, 'every 2 hours'], - 'days' => [172800, 'every 2 days'], - ]; - } - - #[DataProvider('intervals')] - public function test_it_describes_an_interval_in_words(int $seconds, string $expected): void - { - $this->assertSame($expected, new Task(interval: $seconds)->getScheduleDescription()); - } - - public function test_it_describes_a_cron_schedule(): void - { - $this->assertSame('cron: 0 * * * *', new Task(cron: '0 * * * *')->getScheduleDescription()); - } - - public function test_it_knows_which_kind_of_schedule_it_has(): void - { - $interval = new Task(interval: 60); - $cron = new Task(cron: '* * * * *'); - - $this->assertTrue($interval->isInterval()); - $this->assertFalse($interval->isCron()); - $this->assertTrue($cron->isCron()); - $this->assertFalse($cron->isInterval()); - } -} diff --git a/tests/TasksDiscoveryTest.php b/tests/TasksDiscoveryTest.php deleted file mode 100644 index 89db097..0000000 --- a/tests/TasksDiscoveryTest.php +++ /dev/null @@ -1,107 +0,0 @@ -location = new DiscoveryLocation( - namespace: 'Tempcord\\Plugins\\Tasks\\Tests\\Fixtures\\', - path: __DIR__ . '/Fixtures', - ); - } - - private function discovery(Registry $registry): TasksDiscovery - { - $discovery = new TasksDiscovery($registry); - $discovery->setItems(new DiscoveryItems()); - - return $discovery; - } - - private function registry(): Registry - { - return new Registry(new GenericContainer(), $this->createStub(Logger::class)); - } - - public function test_it_finds_every_annotated_method(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - $names = array_map(static fn($task) => $task->getName(), $registry->getAllTasks()); - - sort($names); - - $this->assertSame(['disabled', 'everyMinute', 'hourly-report'], $names); - } - - /** - * A method without the attribute is left alone, so helpers can sit beside - * scheduled work. - */ - public function test_it_ignores_methods_without_the_attribute(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - $names = array_map(static fn($task) => $task->getName(), $registry->getAllTasks()); - - $this->assertNotContains('notATask', $names); - } - - /** - * The reflector is what lets the runner find and call the method later, so - * discovery has to attach it. - */ - public function test_it_attaches_the_method_to_each_task(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - foreach ($registry->getAllTasks() as $task) { - $this->assertNotNull($task->reflector); - $this->assertSame(ScheduledCommands::class, $task->reflector->getDeclaringClass()->getName()); - } - } - - /** - * Registration happens in apply, not while discovering, so a discovery run - * that is thrown away leaves no trace. - */ - public function test_nothing_is_registered_until_apply(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - - $this->assertSame(0, $registry->count()); - } -} diff --git a/tests/TasksPluginTest.php b/tests/TasksPluginTest.php deleted file mode 100644 index 9bcc007..0000000 --- a/tests/TasksPluginTest.php +++ /dev/null @@ -1,56 +0,0 @@ -assertInstanceOf( - Plugin::class, - new TasksPlugin(new Registry(new GenericContainer(), $this->createStub(Logger::class))), - ); - } - - /** - * The registry is a Fenrir extension: registering it is what gets its - * timers started once the gateway is up. - */ - public function test_booting_registers_the_registry_as_a_discord_extension(): void - { - $registry = new Registry(new GenericContainer(), $this->createStub(Logger::class)); - $discord = new FakeDiscord(); - - new TasksPlugin($registry)->boot($this->tempcord($discord)); - - $this->assertTrue($discord->hasExtension(Registry::class)); - $this->assertSame($registry, $discord->getExtension(Registry::class)); - } - - private function tempcord(Discord $discord): Tempcord - { - /* - * Only the discord property is touched during boot, so the rest is - * built without a container. - */ - $tempcord = new \ReflectionClass(Tempcord::class)->newInstanceWithoutConstructor(); - - new \ReflectionProperty(Tempcord::class, 'discord')->setValue($tempcord, $discord); - - return $tempcord; - } -} diff --git a/tests/Unit/CronExpressionTest.php b/tests/Unit/CronExpressionTest.php new file mode 100644 index 0000000..53bb861 --- /dev/null +++ b/tests/Unit/CronExpressionTest.php @@ -0,0 +1,195 @@ +assertTrue($cron->matches($this->at('2026-09-02 13:37:00'))); + $this->assertTrue($cron->matches($this->at('2026-01-01 00:00:00'))); + } + + public function test_a_fixed_minute_and_hour_matches_only_then(): void + { + $cron = new CronExpression('30 14 * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:30:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:31:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 15:30:00'))); + } + + #[DataProvider('aliases')] + public function test_an_alias_stands_for_its_expression(string $alias, string $matches, string $misses): void + { + $cron = new CronExpression($alias); + + $this->assertTrue($cron->matches($this->at($matches)), $alias . ' should match ' . $matches); + $this->assertFalse($cron->matches($this->at($misses)), $alias . ' should not match ' . $misses); + } + + /** + * @return array + */ + public static function aliases(): array + { + return [ + '@hourly' => ['@hourly', '2026-09-02 14:00:00', '2026-09-02 14:01:00'], + '@daily' => ['@daily', '2026-09-02 00:00:00', '2026-09-02 01:00:00'], + '@midnight' => ['@midnight', '2026-09-02 00:00:00', '2026-09-02 12:00:00'], + '@weekly' => ['@weekly', '2026-09-06 00:00:00', '2026-09-07 00:00:00'], + '@monthly' => ['@monthly', '2026-09-01 00:00:00', '2026-09-02 00:00:00'], + '@yearly' => ['@yearly', '2026-01-01 00:00:00', '2026-02-01 00:00:00'], + ]; + } + + public function test_a_list_matches_any_of_its_values(): void + { + $cron = new CronExpression('0,15,30,45 * * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:15:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 14:45:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:20:00'))); + } + + public function test_a_range_matches_within_it(): void + { + $cron = new CronExpression('0 9-17 * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 09:00:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 17:00:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 18:00:00'))); + } + + public function test_a_step_over_a_wildcard_counts_from_the_bottom_of_the_field(): void + { + $cron = new CronExpression('*/15 * * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:00:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 14:30:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:10:00'))); + } + + /** + * A step over a range counts from where the range starts: 1-10/2 is every + * second value beginning at one, so 1,3,5,7,9 — not 2,4,6,8,10. + */ + public function test_a_step_over_a_range_counts_from_the_start_of_the_range(): void + { + $cron = new CronExpression('1-10/2 * * * *'); + + foreach ([1, 3, 5, 7, 9] as $minute) { + $this->assertTrue($cron->matches($this->at(sprintf('2026-09-02 14:%02d:00', $minute)))); + } + + foreach ([2, 4, 10] as $minute) { + $this->assertFalse($cron->matches($this->at(sprintf('2026-09-02 14:%02d:00', $minute)))); + } + } + + /** + * Cron's one genuine oddity: with both day fields restricted the line runs + * on days matching either. + */ + public function test_restricting_both_day_fields_matches_either_of_them(): void + { + $cron = new CronExpression('0 0 1 * 1'); + + // The first of the month, which in July 2026 is a Wednesday. + $this->assertTrue($cron->matches($this->at('2026-07-01 00:00:00'))); + // An ordinary Monday, which is not the first. + $this->assertTrue($cron->matches($this->at('2026-07-06 00:00:00'))); + // Neither. + $this->assertFalse($cron->matches($this->at('2026-07-07 00:00:00'))); + } + + /** + * With only one of them restricted the other stands open and the usual + * reading applies. + */ + public function test_restricting_one_day_field_still_narrows(): void + { + $cron = new CronExpression('0 0 1 * *'); + + $this->assertTrue($cron->matches($this->at('2026-07-01 00:00:00'))); + $this->assertFalse($cron->matches($this->at('2026-07-06 00:00:00'))); + } + + public function test_the_next_run_is_the_following_matching_minute(): void + { + $next = new CronExpression('0 * * * *')->getNextRunDate($this->at('2026-09-02 14:30:12')); + + $this->assertSame('2026-09-02 15:00:00', $next->format('Y-m-d H:i:s')); + } + + /** + * The minute the expression is asked from is behind it; asking at exactly + * the matching minute gives the next one rather than answering with now. + */ + public function test_the_next_run_is_never_the_minute_it_was_asked_in(): void + { + $next = new CronExpression('* * * * *')->getNextRunDate($this->at('2026-09-02 14:30:00')); + + $this->assertSame('2026-09-02 14:31:00', $next->format('Y-m-d H:i:s')); + } + + public function test_it_says_how_long_until_the_next_run(): void + { + $seconds = new CronExpression('0 * * * *') + ->getSecondsUntilNextRun($this->at('2026-09-02 14:59:30')); + + $this->assertSame(30, $seconds); + } + + public function test_an_expression_with_the_wrong_number_of_fields_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected 5 fields'); + + new CronExpression('0 0 *'); + } + + public function test_a_value_outside_its_field_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('out of range'); + + new CronExpression('99 * * * *'); + } + + /** + * Anything that is not a number would otherwise be cast to zero, and a task + * written for noon would quietly run at midnight. + */ + public function test_something_that_is_not_a_number_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + + new CronExpression('noon * * * *'); + } + + public function test_a_step_of_zero_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('step must be at least 1'); + + new CronExpression('*/0 * * * *'); + } +} diff --git a/tests/Unit/CronSchedulingTest.php b/tests/Unit/CronSchedulingTest.php new file mode 100644 index 0000000..1820297 --- /dev/null +++ b/tests/Unit/CronSchedulingTest.php @@ -0,0 +1,106 @@ +loop = new FakeLoop(); + $this->runner = new Runner($this->loop, new GenericContainer(), new RecordingLogger()); + + $reflector = new ClassReflector(MinutelyTask::class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + $this->runner->schedule(new TaskCompiler()->compileClass($reflector, $attribute)); + } + + /** + * A periodic timer is exactly what drifts, so a cron task must not be given + * one. + */ + public function test_a_cron_task_is_armed_one_turn_at_a_time(): void + { + $this->assertCount(1, $this->loop->timers); + $this->assertFalse($this->loop->timers[0]->isPeriodic()); + } + + public function test_it_waits_only_until_the_next_matching_minute(): void + { + $wait = $this->loop->timers[0]->getInterval(); + + $this->assertGreaterThan(0, $wait); + $this->assertLessThanOrEqual(60, $wait); + } + + public function test_the_turn_runs_when_its_moment_arrives(): void + { + $this->loop->timers[0]->fire(); + + $this->assertSame(1, MinutelyTask::$turns); + } + + /** + * Re-armed from the turn just taken, so the wait is recomputed rather than + * accumulated. + */ + public function test_taking_a_turn_arms_the_next_one(): void + { + $first = $this->loop->timers[0]; + $first->fire(); + + $armed = $this->loop->lastTimer(); + + $this->assertNotNull($armed); + $this->assertNotSame($first, $armed); + $this->assertFalse($armed->isPeriodic()); + $this->assertGreaterThan(0, $armed->getInterval()); + $this->assertLessThanOrEqual(60, $armed->getInterval()); + } + + public function test_it_keeps_taking_turns(): void + { + for ($turn = 0; $turn < 3; $turn++) { + $this->loop->lastTimer()?->fire(); + } + + $this->assertSame(3, MinutelyTask::$turns); + } + + public function test_a_cron_task_can_be_cancelled(): void + { + $this->assertTrue($this->runner->cancel('MinutelyTask')); + $this->assertSame([], $this->runner->scheduled()); + } +} diff --git a/tests/Unit/RegistryTest.php b/tests/Unit/RegistryTest.php new file mode 100644 index 0000000..de0a41d --- /dev/null +++ b/tests/Unit/RegistryTest.php @@ -0,0 +1,119 @@ +container = new GenericContainer(); + $this->container->singleton(Logger::class, new RecordingLogger()); + } + + private function registry(string ...$classes): Registry + { + $registry = new Registry($this->container); + + foreach ($classes as $class) { + $registry->add($this->definition($class)); + } + + return $registry; + } + + private function definition(string $class): TaskDefinition + { + $reflector = new ClassReflector($class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + public function test_it_holds_registered_tasks(): void + { + $registry = $this->registry(SweepMessages::class, DisabledTask::class); + + $this->assertSame(2, $registry->count()); + $this->assertSame( + ['SweepMessages', 'DisabledTask'], + array_map(static fn($task) => $task->name, $registry->all()), + ); + } + + /** + * Discovery can reach the same class twice — from the bot's own code and + * from a package that ships it — and the task must still run once. + */ + public function test_the_same_task_registered_twice_is_held_once(): void + { + $registry = $this->registry(SweepMessages::class, SweepMessages::class); + + $this->assertSame(1, $registry->count()); + } + + public function test_starting_puts_every_enabled_task_on_the_loop(): void + { + $loop = new FakeLoop(); + $scheduled = $this->registry(SweepMessages::class, DisabledTask::class)->start($loop); + + $this->assertCount(1, $scheduled); + $this->assertStringContainsString('SweepMessages', $scheduled[0]); + $this->assertStringContainsString('every 10 seconds', $scheduled[0]); + $this->assertCount(1, $loop->timers); + } + + /** + * A bot with nothing scheduled must not build a runner, and must not report + * having started one. + */ + public function test_a_bot_with_no_tasks_starts_nothing(): void + { + $this->assertSame([], $this->registry()->start(new FakeLoop())); + } + + public function test_statistics_are_empty_before_the_scheduler_starts(): void + { + $this->assertSame([], $this->registry(SweepMessages::class)->stats()); + } + + public function test_cancelling_before_the_scheduler_starts_reports_failure(): void + { + $this->assertFalse($this->registry(SweepMessages::class)->cancel('SweepMessages')); + } + + public function test_a_started_task_can_be_cancelled_through_the_registry(): void + { + $registry = $this->registry(SweepMessages::class); + $loop = new FakeLoop(); + $registry->start($loop); + + $this->assertTrue($registry->cancel('SweepMessages')); + + $loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + } +} diff --git a/tests/Unit/RunnerTest.php b/tests/Unit/RunnerTest.php new file mode 100644 index 0000000..1ad29df --- /dev/null +++ b/tests/Unit/RunnerTest.php @@ -0,0 +1,268 @@ +loop = new FakeLoop(); + $this->logger = new RecordingLogger(); + $this->container = new GenericContainer(); + } + + private function definition(string $class): TaskDefinition + { + $reflector = new ClassReflector($class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + private function runner(): Runner + { + return new Runner($this->loop, $this->container, $this->logger); + } + + private function schedule(string ...$classes): Runner + { + $runner = $this->runner(); + + foreach ($classes as $class) { + $runner->schedule($this->definition($class)); + } + + return $runner; + } + + public function test_an_interval_task_takes_a_turn_when_its_timer_fires(): void + { + $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $this->assertSame(3, SweepMessages::$turns); + } + + public function test_an_interval_task_is_armed_for_the_interval_it_declared(): void + { + $this->schedule(SweepMessages::class); + + $this->assertSame(10.0, $this->loop->timers[0]->getInterval()); + $this->assertTrue($this->loop->timers[0]->isPeriodic()); + } + + /** + * A task is a repeating chore, so nothing happens until the first interval + * has passed — unless it says otherwise. + */ + public function test_a_task_does_not_take_a_turn_merely_by_being_scheduled(): void + { + $this->schedule(SweepMessages::class); + + $this->assertSame(0, SweepMessages::$turns); + } + + /** + * Catching up on what expired while the bot was down cannot wait out the + * first interval — but it still waits for the bot to finish starting. + */ + public function test_a_task_that_runs_on_boot_takes_its_first_turn_on_the_next_tick(): void + { + $this->schedule(BootTask::class); + + $this->assertSame(0, BootTask::$turns); + + $this->loop->drainFutureTicks(); + + $this->assertSame(1, BootTask::$turns); + } + + public function test_a_disabled_task_is_left_out_of_the_schedule_entirely(): void + { + $scheduled = $this->runner()->schedule($this->definition(DisabledTask::class)); + + $this->loop->tick(); + $this->loop->drainFutureTicks(); + + $this->assertFalse($scheduled); + $this->assertSame([], $this->loop->timers); + $this->assertSame(0, DisabledTask::$turns); + } + + /** + * A timer fires again whether or not the last turn threw. Without + * containment the exception travels into the event loop and takes the + * process with it. + */ + public function test_a_task_that_throws_is_reported_and_keeps_its_place(): void + { + $this->schedule(FailingTask::class, SweepMessages::class); + + $this->loop->tick(2); + + $this->assertSame(2, SweepMessages::$turns); + $this->assertTrue($this->logger->has('the database went away')); + } + + public function test_a_failure_is_counted_against_the_task(): void + { + $runner = $this->schedule(FailingTask::class); + + $this->loop->tick(2); + + $stats = $runner->statsFor('FailingTask'); + + $this->assertNotNull($stats); + $this->assertSame(2, $stats->totalRuns); + $this->assertSame(2, $stats->failures); + $this->assertSame('the database went away', $stats->lastError); + $this->assertSame(0.0, $stats->getSuccessRate()); + } + + public function test_a_turn_that_worked_is_counted_too(): void + { + $runner = $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $stats = $runner->statsFor('SweepMessages'); + + $this->assertNotNull($stats); + $this->assertSame(3, $stats->totalRuns); + $this->assertSame(3, $stats->successfulRuns); + $this->assertSame(100.0, $stats->getSuccessRate()); + } + + /** + * A task running every ten seconds would otherwise write eight thousand + * lines a day saying nothing happened. + */ + public function test_an_ordinary_turn_is_not_announced(): void + { + $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $this->assertNotContains('info', $this->logger->levels); + $this->assertNotContains('error', $this->logger->levels); + } + + /** + * A task slower than its own interval must not be started alongside itself, + * or each turn makes the next one slower until nothing else gets a look in. + */ + public function test_a_task_still_busy_from_its_last_turn_is_skipped(): void + { + $this->schedule(SlowTask::class); + + $this->loop->tick(4); + + $this->assertSame(1, SlowTask::$started); + $this->assertTrue($this->logger->has('still busy')); + } + + public function test_a_task_that_catches_up_is_run_again(): void + { + $this->schedule(SlowTask::class); + + $this->loop->tick(); + SlowTask::$holding->resolve(null); + $this->loop->tick(); + + $this->assertSame(2, SlowTask::$started); + } + + public function test_a_cancelled_task_stops_taking_turns(): void + { + $runner = $this->schedule(SweepMessages::class); + + $this->assertTrue($runner->cancel('SweepMessages')); + + $this->loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + $this->assertSame([], $runner->scheduled()); + } + + public function test_cancelling_something_that_was_never_scheduled_says_so(): void + { + $this->assertFalse($this->schedule(SweepMessages::class)->cancel('NoSuchTask')); + } + + public function test_every_task_can_be_cancelled_at_once(): void + { + $runner = $this->schedule(SweepMessages::class, FailingTask::class); + + $runner->cancelAll(); + $this->loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + $this->assertSame([], $runner->scheduled()); + } + + /** + * Several chores on one class each get their own place in the schedule. + */ + public function test_tasks_declared_on_methods_are_scheduled_separately(): void + { + $reflector = new ClassReflector(Housekeeping::class); + $compiler = new TaskCompiler(); + $runner = $this->runner(); + + foreach (['sweepMessages', 'pruneStatistics'] as $methodName) { + $method = $reflector->getMethod($methodName); + + /** @var Task $attribute */ + $attribute = $method->getAttribute(Task::class); + + $runner->schedule($compiler->compileMethod($reflector, $method, $attribute)); + } + + $this->assertSame( + ['Housekeeping::sweepMessages', 'nightly-prune'], + $runner->scheduled(), + ); + } +} diff --git a/tests/Unit/TaskCompilerTest.php b/tests/Unit/TaskCompilerTest.php new file mode 100644 index 0000000..cbe5d3c --- /dev/null +++ b/tests/Unit/TaskCompilerTest.php @@ -0,0 +1,141 @@ +getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + private function compileMethod(string $class, string $methodName): TaskDefinition + { + $reflector = new ClassReflector($class); + $method = $reflector->getMethod($methodName); + + /** @var Task $attribute */ + $attribute = $method->getAttribute(Task::class); + + return new TaskCompiler()->compileMethod($reflector, $method, $attribute); + } + + public function test_a_task_on_a_class_runs_through_its_invoke(): void + { + $definition = $this->compileClass(SweepMessages::class); + + $this->assertSame(SweepMessages::class, $definition->handler); + $this->assertSame('__invoke', $definition->method->getName()); + $this->assertSame(10, $definition->interval); + } + + public function test_a_task_on_a_method_runs_through_that_method(): void + { + $definition = $this->compileMethod(Housekeeping::class, 'sweepMessages'); + + $this->assertSame(Housekeeping::class, $definition->handler); + $this->assertSame('sweepMessages', $definition->method->getName()); + } + + /** + * A method name on its own would collide between classes, and two tasks + * sharing a name share their statistics and cannot be cancelled apart. + */ + public function test_a_method_task_is_named_after_its_class_as_well(): void + { + $this->assertSame( + 'Housekeeping::sweepMessages', + $this->compileMethod(Housekeeping::class, 'sweepMessages')->name, + ); + } + + public function test_a_class_task_is_named_after_its_class(): void + { + $this->assertSame('SweepMessages', $this->compileClass(SweepMessages::class)->name); + } + + public function test_a_name_given_in_the_attribute_wins(): void + { + $this->assertSame( + 'nightly-prune', + $this->compileMethod(Housekeeping::class, 'pruneStatistics')->name, + ); + } + + public function test_a_class_task_without_an_invoke_method_is_refused(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('should declare an __invoke method'); + + $this->compileClass(HandlerlessTask::class); + } + + /** + * An expression nobody can read should fail while the bot is starting and + * say which task it came from, not four hours later inside a timer. + */ + public function test_an_unreadable_cron_expression_is_refused_at_discovery(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('UnreadableCronTask'); + + $this->compileClass(UnreadableCronTask::class); + } + + public function test_a_task_must_say_when_it_runs(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('either an interval or a cron expression'); + + new Task(); + } + + public function test_a_task_cannot_be_scheduled_two_ways_at_once(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('cannot be given both'); + + new Task(interval: 60, cron: '@daily'); + } + + /** + * A zero interval asks the loop to run the task as fast as it can, which + * starves the gateway heartbeat and drops the connection. + */ + public function test_a_task_must_run_at_least_a_second_apart(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('at least one second'); + + new Task(interval: 0); + } + + public function test_an_interval_reads_as_something_a_person_can_check(): void + { + $this->assertSame('every 10 seconds', $this->compileClass(SweepMessages::class)->schedule()); + $this->assertSame('cron: @daily', $this->compileMethod(Housekeeping::class, 'pruneStatistics')->schedule()); + } +} diff --git a/tests/Unit/TasksDiscoveryTest.php b/tests/Unit/TasksDiscoveryTest.php new file mode 100644 index 0000000..da60fee --- /dev/null +++ b/tests/Unit/TasksDiscoveryTest.php @@ -0,0 +1,126 @@ +location = new DiscoveryLocation( + namespace: 'Tempcord\\Plugins\\Tasks\\Tests\\Fixtures\\', + path: dirname(__DIR__) . '/Fixtures', + ); + } + + private function registry(): Registry + { + return new Registry(new GenericContainer()); + } + + private function discover(Registry $registry, string ...$classes): void + { + $discovery = new TasksDiscovery($registry); + $discovery->setItems(new DiscoveryItems()); + + foreach ($classes as $class) { + $discovery->discover($this->location, new ClassReflector($class)); + } + + $discovery->apply(); + } + + /** + * @return list + */ + private function names(Registry $registry): array + { + $names = array_map(static fn($task) => $task->name, $registry->all()); + sort($names); + + return $names; + } + + public function test_it_finds_every_annotated_method(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + $this->assertSame( + ['ScheduledCommands::disabled', 'ScheduledCommands::everyMinute', 'hourly-report'], + $this->names($registry), + ); + } + + /** + * A task on the class itself is found the same way, so a bot may declare + * one task per class the way it declares commands and listeners. + */ + public function test_it_finds_a_task_declared_on_the_class(): void + { + $registry = $this->registry(); + $this->discover($registry, SweepMessages::class); + + $this->assertSame(['SweepMessages'], $this->names($registry)); + } + + /** + * A method without the attribute is left alone, so helpers can sit beside + * scheduled work. + */ + public function test_it_ignores_methods_without_the_attribute(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + $this->assertNotContains('ScheduledCommands::notATask', $this->names($registry)); + } + + /** + * The method is what the runner calls later, so discovery has to carry it + * through. + */ + public function test_each_task_knows_what_to_call(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + foreach ($registry->all() as $task) { + $this->assertSame(ScheduledCommands::class, $task->handler); + $this->assertSame( + ScheduledCommands::class, + $task->method->getDeclaringClass()->getName(), + ); + } + } + + /** + * Registration happens in apply, not while discovering, so a discovery run + * that is thrown away leaves no trace. + */ + public function test_nothing_is_registered_until_apply(): void + { + $registry = $this->registry(); + + $discovery = new TasksDiscovery($registry); + $discovery->setItems(new DiscoveryItems()); + $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); + + $this->assertSame(0, $registry->count()); + } +} diff --git a/tests/Unit/TasksPluginTest.php b/tests/Unit/TasksPluginTest.php new file mode 100644 index 0000000..d2d9c53 --- /dev/null +++ b/tests/Unit/TasksPluginTest.php @@ -0,0 +1,52 @@ +singleton(Logger::class, new RecordingLogger()); + + $plugin = new TasksPlugin(new Registry($container), new RecordingLogger()); + + $this->assertInstanceOf(Plugin::class, $plugin); + } + + /** + * A bot that installed the plugin but declared no tasks should say nothing + * on the way up. + */ + public function test_a_bot_with_no_tasks_is_not_told_about_it(): void + { + $container = new GenericContainer(); + $logger = new RecordingLogger(); + $container->singleton(Logger::class, $logger); + + // Tempcord is final, and boot() only needs something of that type. + $tempcord = new ReflectionClass(Tempcord::class)->newInstanceWithoutConstructor(); + + new TasksPlugin(new Registry($container), $logger)->boot($tempcord); + + $this->assertSame([], $logger->messages); + } +} From c1283d66330260170b85fefa664ba091389d1b45 Mon Sep 17 00:00:00 2001 From: "Vladyslav G." Date: Wed, 2 Sep 2026 03:57:10 +0200 Subject: [PATCH 2/2] ci: name the test jobs after the PHP they run --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9345764..f7efb72 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,6 +7,7 @@ on: jobs: tests: + name: PHP ${{ matrix.php }} runs-on: ubuntu-latest strategy: