From 33ef9278df67259f09d33f15a5681bcd09d8bfd6 Mon Sep 17 00:00:00 2001 From: "Vladyslav G." Date: Thu, 3 Sep 2026 12:08:07 +0200 Subject: [PATCH] feat(commands): run middleware around a command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A command may now declare middleware, and so may a subcommand group and a subcommand. They are folded outermost first — the command's, then the group's, then the subcommand's own — and a middleware that answers the interaction instead of calling $next stops the handler from running. Discord's own permission field is a default: a guild administrator can override it in Server Settings, and it is scoped to the whole command, so a command with one subcommand for everybody and another for moderators cannot be described with it at all. RequiresPermissions ships for that, reading the permissions Discord has already computed for the channel. Arguments are resolved inside the chain rather than before it, so a command about to be refused does not pay for the REST call an option can cost. Co-Authored-By: Claude Opus 5 (1M context) --- docs/README.md | 2 + docs/index.json | 65 +++++++++ docs/reference/attributes/command.md | 1 + docs/reference/attributes/subcommand-group.md | 1 + docs/reference/attributes/subcommand.md | 1 + docs/reference/index.md | 5 + docs/reference/middleware/middleware.md | 14 ++ .../middleware/requires-permissions.md | 17 +++ src/Attributes/Command.php | 4 + src/Attributes/Subcommand.php | 5 + src/Attributes/SubcommandGroup.php | 5 + src/Compiler/CommandCompiler.php | 44 +++++++ src/Definitions/HandlerDefinition.php | 5 + src/Definitions/SubcommandDefinition.php | 4 + src/Definitions/SubcommandGroupDefinition.php | 5 + src/Interfaces/Middleware.php | 27 ++++ src/Middleware/RequiresPermissions.php | 80 +++++++++++ src/Runtime/CommandDispatcher.php | 18 ++- src/Runtime/MiddlewarePipeline.php | 52 ++++++++ tests/Fixtures/GuardedCommand.php | 25 ++++ tests/Fixtures/GuardedGroupCommand.php | 15 +++ tests/Fixtures/InlineGuardedCommand.php | 20 +++ tests/Fixtures/InnerMiddleware.php | 11 ++ tests/Fixtures/NotMiddlewareCommand.php | 14 ++ tests/Fixtures/OuterMiddleware.php | 11 ++ tests/Fixtures/RefusingMiddleware.php | 17 +++ tests/Fixtures/TrailMiddleware.php | 27 ++++ tests/Unit/Compiler/CommandMiddlewareTest.php | 80 +++++++++++ .../Compiler/DefinitionsAreCacheableTest.php | 35 +++++ .../Middleware/RequiresPermissionsTest.php | 124 ++++++++++++++++++ tests/Unit/Runtime/CommandDispatcherTest.php | 17 +++ tests/Unit/Runtime/MiddlewarePipelineTest.php | 113 ++++++++++++++++ tests/Unit/TestCase.php | 2 + tools/src/ApiReflector.php | 4 + 34 files changed, 867 insertions(+), 3 deletions(-) create mode 100644 docs/reference/middleware/middleware.md create mode 100644 docs/reference/middleware/requires-permissions.md create mode 100644 src/Interfaces/Middleware.php create mode 100644 src/Middleware/RequiresPermissions.php create mode 100644 src/Runtime/MiddlewarePipeline.php create mode 100644 tests/Fixtures/GuardedCommand.php create mode 100644 tests/Fixtures/GuardedGroupCommand.php create mode 100644 tests/Fixtures/InlineGuardedCommand.php create mode 100644 tests/Fixtures/InnerMiddleware.php create mode 100644 tests/Fixtures/NotMiddlewareCommand.php create mode 100644 tests/Fixtures/OuterMiddleware.php create mode 100644 tests/Fixtures/RefusingMiddleware.php create mode 100644 tests/Fixtures/TrailMiddleware.php create mode 100644 tests/Unit/Compiler/CommandMiddlewareTest.php create mode 100644 tests/Unit/Middleware/RequiresPermissionsTest.php create mode 100644 tests/Unit/Runtime/MiddlewarePipelineTest.php diff --git a/docs/README.md b/docs/README.md index 3ee6439..53a3ba6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -37,3 +37,5 @@ Generated from the source, so it describes what the framework actually does. **Plugins** — [Plugin](reference/plugins/plugin.md) +**Middleware** — [Middleware](reference/middleware/middleware.md), [RequiresPermissions](reference/middleware/requires-permissions.md) + diff --git a/docs/index.json b/docs/index.json index 57161c4..7adb743 100644 --- a/docs/index.json +++ b/docs/index.json @@ -105,6 +105,13 @@ "default": "null", "required": false, "summary": "the catalog key this command's translations live under. Keys for everything beneath it are derived from position, so \"commands.music\" gives commands.music.description for the command, commands.music.playlist.play.description for a subcommand, and commands.music.playlist.play.title.description for its option." + }, + { + "name": "middleware", + "type": "array", + "default": "[]", + "required": false, + "summary": "run in the order given, outermost first, before any handler this command has; any of them may answer instead of letting it run" } ], "cases": [], @@ -131,6 +138,13 @@ "default": null, "required": true, "summary": "" + }, + { + "name": "middleware", + "type": "array", + "default": "[]", + "required": false, + "summary": "run after whatever the command declares, before the subcommand's own" } ], "cases": [], @@ -157,6 +171,13 @@ "default": null, "required": true, "summary": "" + }, + { + "name": "middleware", + "type": "array", + "default": "[]", + "required": false, + "summary": "run after whatever the command and the group around it declare" } ], "cases": [], @@ -719,6 +740,50 @@ } ] } + ], + "middleware": [ + { + "name": "Middleware", + "fqcn": "Tempcord\\Interfaces\\Middleware", + "kind": "interface", + "target": null, + "summary": "Something that runs before a command, and may decide it never runs.", + "slug": "reference/middleware/middleware", + "parameters": [], + "cases": [], + "methods": [ + { + "signature": "__invoke(CommandInteraction $interaction, callable $next): void", + "summary": "" + } + ] + }, + { + "name": "RequiresPermissions", + "fqcn": "Tempcord\\Middleware\\RequiresPermissions", + "kind": "class", + "target": null, + "summary": "Refuses anyone whose permissions in the channel fall short.", + "slug": "reference/middleware/requires-permissions", + "parameters": [ + { + "name": "permissions", + "type": "array", + "default": null, + "required": true, + "summary": "every one of which the caller must hold; an administrator holds all of them by definition" + }, + { + "name": "refusal", + "type": "string", + "default": "'You are not allowed to use this command.'", + "required": false, + "summary": "what the caller is told, ephemerally" + } + ], + "cases": [], + "methods": [] + } ] } } diff --git a/docs/reference/attributes/command.md b/docs/reference/attributes/command.md index 3e761f9..78087ec 100644 --- a/docs/reference/attributes/command.md +++ b/docs/reference/attributes/command.md @@ -23,4 +23,5 @@ use Tempcord\Attributes\Command; | `type` | `ApplicationCommandTypes` | `ApplicationCommandTypes::CHAT_INPUT` | | | `handler` | `?EntryPointCommandHandlerType` | `null` | | | `translationKey` | `?string` | `null` | the catalog key this command's translations live under. Keys for everything beneath it are derived from position, so "commands.music" gives commands.music.description for the command, commands.music.playlist.play.description for a subcommand, and commands.music.playlist.play.title.description for its option. | +| `middleware` | `array` | `[]` | run in the order given, outermost first, before any handler this command has; any of them may answer instead of letting it run | diff --git a/docs/reference/attributes/subcommand-group.md b/docs/reference/attributes/subcommand-group.md index e1e60be..753c33c 100644 --- a/docs/reference/attributes/subcommand-group.md +++ b/docs/reference/attributes/subcommand-group.md @@ -16,4 +16,5 @@ use Tempcord\Attributes\SubcommandGroup; | --- | --- | --- | --- | | `name` | `BackedEnum\|string` | *required* | | | `description` | `string` | *required* | | +| `middleware` | `array` | `[]` | run after whatever the command declares, before the subcommand's own | diff --git a/docs/reference/attributes/subcommand.md b/docs/reference/attributes/subcommand.md index e882865..958848a 100644 --- a/docs/reference/attributes/subcommand.md +++ b/docs/reference/attributes/subcommand.md @@ -16,4 +16,5 @@ use Tempcord\Attributes\Subcommand; | --- | --- | --- | --- | | `name` | `BackedEnum\|string` | *required* | | | `description` | `string` | *required* | | +| `middleware` | `array` | `[]` | run after whatever the command and the group around it declare | diff --git a/docs/reference/index.md b/docs/reference/index.md index 5d80e45..37a8f60 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -47,3 +47,8 @@ - [Plugin](plugins/plugin.md) — A package that extends a bot with its own behaviour. +## Middleware + +- [Middleware](middleware/middleware.md) — Something that runs before a command, and may decide it never runs. +- [RequiresPermissions](middleware/requires-permissions.md) — Refuses anyone whose permissions in the channel fall short. + diff --git a/docs/reference/middleware/middleware.md b/docs/reference/middleware/middleware.md new file mode 100644 index 0000000..d254754 --- /dev/null +++ b/docs/reference/middleware/middleware.md @@ -0,0 +1,14 @@ + + +# Middleware + +Something that runs before a command, and may decide it never runs. + +```php +use Tempcord\Interfaces\Middleware; +``` + +## Methods + +### `__invoke(CommandInteraction $interaction, callable $next): void` + diff --git a/docs/reference/middleware/requires-permissions.md b/docs/reference/middleware/requires-permissions.md new file mode 100644 index 0000000..84516fc --- /dev/null +++ b/docs/reference/middleware/requires-permissions.md @@ -0,0 +1,17 @@ + + +# RequiresPermissions + +Refuses anyone whose permissions in the channel fall short. + +```php +use Tempcord\Middleware\RequiresPermissions; +``` + +## Parameters + +| Name | Type | Default | Description | +| --- | --- | --- | --- | +| `permissions` | `array` | *required* | every one of which the caller must hold; an administrator holds all of them by definition | +| `refusal` | `string` | `'You are not allowed to use this command.'` | what the caller is told, ephemerally | + diff --git a/src/Attributes/Command.php b/src/Attributes/Command.php index 13e2ae6..f4be6e1 100644 --- a/src/Attributes/Command.php +++ b/src/Attributes/Command.php @@ -34,6 +34,9 @@ * commands.music.description for the command, * commands.music.playlist.play.description for a subcommand, and * commands.music.playlist.play.title.description for its option. + * @param list<\Tempcord\Interfaces\Middleware|class-string<\Tempcord\Interfaces\Middleware>> $middleware + * run in the order given, outermost first, before any handler this + * command has; any of them may answer instead of letting it run */ public function __construct( public string|BackedEnum|null $name = null, @@ -45,6 +48,7 @@ public function __construct( public ApplicationCommandTypes $type = ApplicationCommandTypes::CHAT_INPUT, public ?EntryPointCommandHandlerType $handler = null, public ?string $translationKey = null, + public array $middleware = [], ) { $this->guildId = $guildId === null ? null : (string) $guildId; } diff --git a/src/Attributes/Subcommand.php b/src/Attributes/Subcommand.php index aeee54f..e6ec6de 100644 --- a/src/Attributes/Subcommand.php +++ b/src/Attributes/Subcommand.php @@ -11,8 +11,13 @@ #[Attribute(Attribute::TARGET_METHOD)] final readonly class Subcommand { + /** + * @param list<\Tempcord\Interfaces\Middleware|class-string<\Tempcord\Interfaces\Middleware>> $middleware + * run after whatever the command and the group around it declare + */ public function __construct( public string|BackedEnum $name, public string $description, + public array $middleware = [], ) {} } diff --git a/src/Attributes/SubcommandGroup.php b/src/Attributes/SubcommandGroup.php index 29054d4..cd8aa80 100644 --- a/src/Attributes/SubcommandGroup.php +++ b/src/Attributes/SubcommandGroup.php @@ -11,8 +11,13 @@ #[Attribute(Attribute::TARGET_CLASS)] final readonly class SubcommandGroup { + /** + * @param list<\Tempcord\Interfaces\Middleware|class-string<\Tempcord\Interfaces\Middleware>> $middleware + * run after whatever the command declares, before the subcommand's own + */ public function __construct( public string|BackedEnum $name, public string $description, + public array $middleware = [], ) {} } diff --git a/src/Compiler/CommandCompiler.php b/src/Compiler/CommandCompiler.php index 2be654c..7826097 100644 --- a/src/Compiler/CommandCompiler.php +++ b/src/Compiler/CommandCompiler.php @@ -23,6 +23,7 @@ use Tempcord\Definitions\SubcommandDefinition; use Tempcord\Definitions\SubcommandGroupDefinition; use Tempcord\Interfaces\Autocomplete; +use Tempcord\Interfaces\Middleware; use Tempcord\Localization\LocalizationProvider; use Tempcord\Localization\NullLocalizations; use ReflectionEnum; @@ -67,6 +68,7 @@ public function compile(ClassReflector $class, Command $command): CommandDefinit $handlers = []; $key = $command->translationKey; + $around = $this->middlewareOf($command->middleware, 'Command [' . $name . ']'); $group = $this->groupOf($class, $key); $subcommands = $this->subcommandsOf($class, $key); @@ -81,6 +83,7 @@ public function compile(ClassReflector $class, Command $command): CommandDefinit method: $subcommand->method, options: $subcommand->options, optionPath: $group->name . '.' . $subcommand->name, + middleware: [...$around, ...$group->middleware, ...$subcommand->middleware], ); } } elseif ($subcommands !== []) { @@ -93,6 +96,7 @@ public function compile(ClassReflector $class, Command $command): CommandDefinit method: $subcommand->method, options: $subcommand->options, optionPath: $subcommand->name, + middleware: [...$around, ...$subcommand->middleware], ); } } elseif (!$this->isLaunchedByDiscord($command)) { @@ -107,6 +111,7 @@ public function compile(ClassReflector $class, Command $command): CommandDefinit path: $name, method: $invoke, options: $options, + middleware: $around, ); } @@ -211,6 +216,7 @@ private function groupOf(ClassReflector $class, ?string $key): ?SubcommandGroupD subcommands: $this->subcommandsOf($class, $groupKey), nameLocalizations: $this->translate($groupKey, 'name'), descriptionLocalizations: $this->translate($groupKey, 'description'), + middleware: $this->middlewareOf($group->middleware, 'Subcommand group [' . $name . ']'), ); } @@ -239,6 +245,7 @@ private function subcommandsOf(ClassReflector $class, ?string $key): array method: $method, nameLocalizations: $this->translate($subcommandKey, 'name'), descriptionLocalizations: $this->translate($subcommandKey, 'description'), + middleware: $this->middlewareOf($subcommand->middleware, 'Subcommand [' . $name . ']'), ); } @@ -346,6 +353,43 @@ private function autocompleteFor(Option $option, ?MethodReflector $completer): ? return null; } + /** + * Middleware as declared, checked before anything is built out of it. + * + * A class name that turns out not to be middleware is a mistake worth + * catching here: discovery runs at start-up, so the bot refuses to boot + * rather than failing the first time somebody uses the command it was meant + * to guard — which, for a guard, is the worst moment to find out. + * + * @param array $declared + * + * @return list> + */ + private function middlewareOf(array $declared, string $where): array + { + $middleware = []; + + foreach ($declared as $entry) { + if ($entry instanceof Middleware) { + $middleware[] = $entry; + continue; + } + + if (is_string($entry) && is_subclass_of($entry, Middleware::class)) { + $middleware[] = $entry; + continue; + } + + throw new LogicException( + $where . ' declares middleware [' + . (is_string($entry) ? $entry : get_debug_type($entry)) + . '], which does not implement ' . Middleware::class, + ); + } + + return $middleware; + } + /** * The command's own methods that complete an option, keyed by the option * each one answers for. diff --git a/src/Definitions/HandlerDefinition.php b/src/Definitions/HandlerDefinition.php index c8c4d80..e09024d 100644 --- a/src/Definitions/HandlerDefinition.php +++ b/src/Definitions/HandlerDefinition.php @@ -2,6 +2,7 @@ namespace Tempcord\Definitions; +use Tempcord\Interfaces\Middleware; use Tempest\Reflection\MethodReflector; /** @@ -17,12 +18,16 @@ * @param array $options keyed by option name * @param string $optionPath the prefix getOption() needs to reach this * handler's options, empty for an invokable command + * @param list> $middleware everything + * declared around this handler, flattened outermost first — the + * command's, then its group's, then the subcommand's own */ public function __construct( public string $path, public MethodReflector $method, public array $options, public string $optionPath = '', + public array $middleware = [], ) {} /** diff --git a/src/Definitions/SubcommandDefinition.php b/src/Definitions/SubcommandDefinition.php index 6d45923..05899fb 100644 --- a/src/Definitions/SubcommandDefinition.php +++ b/src/Definitions/SubcommandDefinition.php @@ -2,6 +2,7 @@ namespace Tempcord\Definitions; +use Tempcord\Interfaces\Middleware; use Tempest\Reflection\MethodReflector; /** @@ -13,6 +14,8 @@ * @param array $options keyed by option name * @param array $nameLocalizations keyed by Discord locale * @param array $descriptionLocalizations keyed by Discord locale + * @param list> $middleware this + * subcommand's own, before anything declared around it is folded in */ public function __construct( public string $name, @@ -21,5 +24,6 @@ public function __construct( public MethodReflector $method, public array $nameLocalizations = [], public array $descriptionLocalizations = [], + public array $middleware = [], ) {} } diff --git a/src/Definitions/SubcommandGroupDefinition.php b/src/Definitions/SubcommandGroupDefinition.php index 9a12210..56509ce 100644 --- a/src/Definitions/SubcommandGroupDefinition.php +++ b/src/Definitions/SubcommandGroupDefinition.php @@ -2,6 +2,8 @@ namespace Tempcord\Definitions; +use Tempcord\Interfaces\Middleware; + /** * A named grouping of subcommands, which Discord renders as one more level of * nesting under the command itself. @@ -12,6 +14,8 @@ * @param array $subcommands keyed by subcommand name * @param array $nameLocalizations keyed by Discord locale * @param array $descriptionLocalizations keyed by Discord locale + * @param list> $middleware run around + * every subcommand in the group */ public function __construct( public string $name, @@ -19,5 +23,6 @@ public function __construct( public array $subcommands, public array $nameLocalizations = [], public array $descriptionLocalizations = [], + public array $middleware = [], ) {} } diff --git a/src/Interfaces/Middleware.php b/src/Interfaces/Middleware.php new file mode 100644 index 0000000..d6d851e --- /dev/null +++ b/src/Interfaces/Middleware.php @@ -0,0 +1,27 @@ + $permissions every one of which the caller must + * hold; an administrator holds all of them by definition + * @param string $refusal what the caller is told, ephemerally + */ + public function __construct( + public array $permissions, + public string $refusal = 'You are not allowed to use this command.', + ) {} + + public function __invoke(CommandInteraction $interaction, callable $next): void + { + if (!$this->allows($interaction)) { + $interaction->reply($this->refusal, ephemeral: true); + + return; + } + + $next($interaction); + } + + private function allows(CommandInteraction $interaction): bool + { + $held = $interaction->interaction->member?->permissions; + + /* + * No member means the command was used somewhere there are no guild + * permissions to hold — a direct message, most often. Nobody clears a + * permission check there. + */ + if ($held === null) { + return false; + } + + $held = (int) $held; + + if (($held & Permission::ADMINISTRATOR->value) === Permission::ADMINISTRATOR->value) { + return true; + } + + foreach ($this->permissions as $permission) { + if (($held & $permission->value) !== $permission->value) { + return false; + } + } + + return true; + } +} diff --git a/src/Runtime/CommandDispatcher.php b/src/Runtime/CommandDispatcher.php index 85f60d3..5d02cc4 100644 --- a/src/Runtime/CommandDispatcher.php +++ b/src/Runtime/CommandDispatcher.php @@ -19,6 +19,7 @@ public function __construct( private ArgumentResolver $arguments, private Container $container, private Logger $logger, + private MiddlewarePipeline $middleware, ) {} public function dispatch(HandlerDefinition $handler, CommandInteraction $interaction): void @@ -30,9 +31,20 @@ public function dispatch(HandlerDefinition $handler, CommandInteraction $interac */ async(function () use ($handler, $interaction): void { try { - $handler->method->invokeArgs( - $this->container->get($handler->method->getDeclaringClass()->getName()), - $this->arguments->resolve($handler, $interaction), + /* + * Arguments are resolved inside the chain rather than before + * it: resolving them can cost a REST call, and a command a + * middleware is about to refuse should not pay for one. + */ + $this->middleware->run( + $handler->middleware, + $interaction, + function (CommandInteraction $interaction) use ($handler): void { + $handler->method->invokeArgs( + $this->container->get($handler->method->getDeclaringClass()->getName()), + $this->arguments->resolve($handler, $interaction), + ); + }, ); } catch (Throwable $throwable) { $this->logger->error( diff --git a/src/Runtime/MiddlewarePipeline.php b/src/Runtime/MiddlewarePipeline.php new file mode 100644 index 0000000..dbffd3e --- /dev/null +++ b/src/Runtime/MiddlewarePipeline.php @@ -0,0 +1,52 @@ +> $middleware + * @param callable(CommandInteraction): void $handler + */ + public function run(array $middleware, CommandInteraction $interaction, callable $handler): void + { + $next = $handler; + + foreach (array_reverse($middleware) as $entry) { + $inner = $next; + + $next = function (CommandInteraction $interaction) use ($entry, $inner): void { + ($this->resolve($entry))($interaction, $inner); + }; + } + + $next($interaction); + } + + /** + * @param Middleware|class-string $entry + */ + private function resolve(Middleware|string $entry): Middleware + { + return $entry instanceof Middleware ? $entry : $this->container->get($entry); + } +} diff --git a/tests/Fixtures/GuardedCommand.php b/tests/Fixtures/GuardedCommand.php new file mode 100644 index 0000000..d952cd8 --- /dev/null +++ b/tests/Fixtures/GuardedCommand.php @@ -0,0 +1,25 @@ + */ + public static array $calls = []; + + #[Subcommand(name: 'open', description: 'Guarded by the command alone')] + public function open(): void + { + self::$calls[] = 'open'; + } + + #[Subcommand(name: 'shut', description: 'Guarded again', middleware: [InnerMiddleware::class])] + public function shut(): void + { + self::$calls[] = 'shut'; + } +} diff --git a/tests/Fixtures/GuardedGroupCommand.php b/tests/Fixtures/GuardedGroupCommand.php new file mode 100644 index 0000000..56b7430 --- /dev/null +++ b/tests/Fixtures/GuardedGroupCommand.php @@ -0,0 +1,15 @@ + */ + public static array $trail = []; + + public function __construct( + public string $label = 'anonymous', + ) {} + + public function __invoke(CommandInteraction $interaction, callable $next): void + { + self::$trail[] = $this->label; + + $next($interaction); + } +} diff --git a/tests/Unit/Compiler/CommandMiddlewareTest.php b/tests/Unit/Compiler/CommandMiddlewareTest.php new file mode 100644 index 0000000..43d4318 --- /dev/null +++ b/tests/Unit/Compiler/CommandMiddlewareTest.php @@ -0,0 +1,80 @@ +definition(GuardedCommand::class)->handlers; + + $this->assertSame([OuterMiddleware::class], $handlers['guarded.open']->middleware); + } + + public function test_a_subcommands_own_middleware_runs_inside_the_commands(): void + { + $handlers = $this->definition(GuardedCommand::class)->handlers; + + $this->assertSame( + [OuterMiddleware::class, InnerMiddleware::class], + $handlers['guarded.shut']->middleware, + ); + } + + public function test_a_group_sits_between_the_command_and_the_subcommand(): void + { + $handler = $this->definition(GuardedGroupCommand::class)->handlers['guarded_group.keys.cut']; + + $this->assertSame( + [OuterMiddleware::class, InnerMiddleware::class, RefusingMiddleware::class], + $handler->middleware, + ); + } + + public function test_middleware_written_inline_is_kept_as_the_object_it_is(): void + { + $middleware = $this->definition(InlineGuardedCommand::class)->handlers['inline_guarded']->middleware; + + $this->assertCount(1, $middleware); + $this->assertInstanceOf(RequiresPermissions::class, $middleware[0]); + $this->assertSame('Not for you.', $middleware[0]->refusal); + } + + public function test_a_command_declaring_none_carries_none(): void + { + $this->assertSame([], $this->definition(PingCommand::class)->handlers['ping']->middleware); + } + + /** + * The check belongs at discovery because that is start-up: a guard that is + * not a guard should stop the bot booting, not surface the first time + * somebody uses the command it was supposed to protect. + */ + public function test_a_class_that_is_not_middleware_is_refused(): void + { + $this->expectException(LogicException::class); + $this->expectExceptionMessage('does not implement'); + + $this->definition(NotMiddlewareCommand::class); + } +} diff --git a/tests/Unit/Compiler/DefinitionsAreCacheableTest.php b/tests/Unit/Compiler/DefinitionsAreCacheableTest.php index b8394b5..fece64b 100644 --- a/tests/Unit/Compiler/DefinitionsAreCacheableTest.php +++ b/tests/Unit/Compiler/DefinitionsAreCacheableTest.php @@ -6,6 +6,7 @@ use Symfony\Component\VarExporter\VarExporter; use Tempcord\Compiler\CommandCompiler; use Tempcord\Definitions\OptionDefinition; +use Tempcord\Tests\Fixtures\InlineGuardedCommand; use Tempcord\Tests\Fixtures\PlatformCommand; use Tempcord\Tests\Unit\TestCase; use Throwable; @@ -51,6 +52,40 @@ public function test_a_compiled_command_can_be_exported_as_php(): void $this->assertSame('platform', $restored->options['platform']->parameter()->getName()); } + /** + * Middleware written as an object inside an attribute is held in the + * definition as that object, so it goes through the cache the same way an + * inline autocomplete does — and would take the whole location's cache down + * with it if it could not be written back out as PHP. + */ + public function test_middleware_written_inline_survives_the_cache(): void + { + $definition = $this->definition(InlineGuardedCommand::class); + + $this->assertNotEmpty($definition->handlers['inline_guarded']->middleware); + + try { + $exported = VarExporter::export($definition); + } catch (Throwable $throwable) { + $this->fail('Middleware written inline must be exportable, but: ' . $throwable->getMessage()); + } + + $file = tempnam(sys_get_temp_dir(), 'definition') . '.php'; + file_put_contents($file, 'assertEquals($definition, $restored); + $this->assertSame( + 'Not for you.', + $restored->handlers['inline_guarded']->middleware[0]->refusal, + ); + } + public function test_an_option_still_reaches_the_parameter_it_feeds(): void { $option = $this->definition(PlatformCommand::class)->options['platform']; diff --git a/tests/Unit/Middleware/RequiresPermissionsTest.php b/tests/Unit/Middleware/RequiresPermissionsTest.php new file mode 100644 index 0000000..bf91073 --- /dev/null +++ b/tests/Unit/Middleware/RequiresPermissionsTest.php @@ -0,0 +1,124 @@ +http = new RecordingHttp(); + } + + /** + * Discord sends the caller's permissions as a decimal string, because the + * bitfield outgrew what JSON can carry as a number. + */ + private function interaction(?string $permissions): CommandInteraction + { + $interaction = new InteractionCreate(); + $interaction->id = '1'; + $interaction->token = 'token'; + $interaction->data = new InteractionData(); + + if ($permissions !== null) { + $member = new GuildMember(); + $member->roles = []; + $member->permissions = $permissions; + + $interaction->member = $member; + } + + return new CommandInteraction($interaction, new FakeDiscord($this->http)); + } + + private function reaches(RequiresPermissions $middleware, ?string $permissions): bool + { + $reached = false; + + $middleware($this->interaction($permissions), function () use (&$reached): void { + $reached = true; + }); + + return $reached; + } + + public function test_a_member_holding_the_permission_gets_through(): void + { + $this->assertTrue($this->reaches( + new RequiresPermissions([Permission::MANAGE_GUILD]), + (string) Permission::MANAGE_GUILD->value, + )); + } + + public function test_a_member_without_it_is_refused(): void + { + $this->assertFalse($this->reaches( + new RequiresPermissions([Permission::MANAGE_GUILD]), + (string) Permission::SEND_MESSAGES->value, + )); + } + + public function test_every_permission_named_has_to_be_held(): void + { + $this->assertFalse($this->reaches( + new RequiresPermissions([Permission::MANAGE_GUILD, Permission::BAN_MEMBERS]), + (string) Permission::MANAGE_GUILD->value, + )); + } + + public function test_an_administrator_holds_everything(): void + { + $this->assertTrue($this->reaches( + new RequiresPermissions([Permission::MANAGE_GUILD, Permission::BAN_MEMBERS]), + (string) Permission::ADMINISTRATOR->value, + )); + } + + /** + * A permission above the 31st bit is the reason the field arrives as a + * string at all, so one of those is worth checking on its own. + */ + public function test_a_permission_beyond_a_32_bit_field_is_read_correctly(): void + { + $this->assertTrue($this->reaches( + new RequiresPermissions([Permission::MODERATE_MEMBERS]), + (string) Permission::MODERATE_MEMBERS->value, + )); + } + + /** + * Nobody holds a guild permission in a direct message, so nobody clears + * a check for one. + */ + public function test_an_interaction_with_no_member_is_refused(): void + { + $this->assertFalse($this->reaches(new RequiresPermissions([Permission::MANAGE_GUILD]), null)); + } + + public function test_a_refusal_is_told_to_the_caller_and_nobody_else(): void + { + $this->reaches( + new RequiresPermissions([Permission::MANAGE_GUILD], 'Тільки для модерації.'), + (string) Permission::SEND_MESSAGES->value, + ); + + $data = $this->http->posts[0]['content']['data'] ?? []; + + $this->assertSame('Тільки для модерації.', $data['content'] ?? null); + $this->assertSame(64, $data['flags'] ?? null, 'an ephemeral reply carries the EPHEMERAL flag'); + } +} diff --git a/tests/Unit/Runtime/CommandDispatcherTest.php b/tests/Unit/Runtime/CommandDispatcherTest.php index 209a18b..8a7d09a 100644 --- a/tests/Unit/Runtime/CommandDispatcherTest.php +++ b/tests/Unit/Runtime/CommandDispatcherTest.php @@ -11,11 +11,14 @@ use Tempcord\Definitions\HandlerDefinition; use Tempcord\Runtime\ArgumentResolver; use Tempcord\Runtime\CommandDispatcher; +use Tempcord\Runtime\MiddlewarePipeline; use Tempcord\Runtime\OptionValueResolver; use Tempcord\Tests\Doubles\FakeDiscord; use Tempcord\Tests\Doubles\RecordingHttp; use Tempcord\Tests\Doubles\RecordingLogger; +use Tempcord\Tests\Fixtures\GuardedCommand; use Tempcord\Tests\Fixtures\RecordingCommand; +use Tempcord\Tests\Fixtures\TrailMiddleware; use Tempcord\Tests\Fixtures\ThrowingCommand; use Tempcord\Tests\Unit\TestCase; use Tempest\Container\GenericContainer; @@ -28,6 +31,8 @@ final class CommandDispatcherTest extends TestCase protected function setUp(): void { RecordingCommand::$calls = []; + GuardedCommand::$calls = []; + TrailMiddleware::$trail = []; $this->logger = new RecordingLogger(); } @@ -37,6 +42,7 @@ private function dispatcher(): CommandDispatcher new ArgumentResolver(new OptionValueResolver(new FakeDiscord(new RecordingHttp()))), new GenericContainer(), $this->logger, + new MiddlewarePipeline(new GenericContainer()), ); } @@ -81,6 +87,17 @@ public function test_it_invokes_the_command_with_its_resolved_arguments(): void $this->assertSame([], $this->logger->messages); } + public function test_it_runs_the_middleware_declared_around_a_handler(): void + { + $this->dispatcher()->dispatch( + $this->handler(GuardedCommand::class, 'guarded.shut'), + $this->interaction('guarded', []), + ); + + $this->assertSame(['outer', 'inner'], TrailMiddleware::$trail); + $this->assertSame(['shut'], GuardedCommand::$calls); + } + /** * A command that throws must be logged rather than allowed to take the * gateway connection down with it. diff --git a/tests/Unit/Runtime/MiddlewarePipelineTest.php b/tests/Unit/Runtime/MiddlewarePipelineTest.php new file mode 100644 index 0000000..478cd65 --- /dev/null +++ b/tests/Unit/Runtime/MiddlewarePipelineTest.php @@ -0,0 +1,113 @@ +id = '1'; + $interaction->token = 'token'; + $interaction->data = new InteractionData(); + + return new CommandInteraction($interaction, new FakeDiscord(new RecordingHttp())); + } + + private function record(string $what): callable + { + return static function () use ($what): void { + TrailMiddleware::$trail[] = $what; + }; + } + + public function test_a_handler_with_no_middleware_is_called_straight(): void + { + $this->pipeline()->run([], $this->interaction(), $this->record('handler')); + + $this->assertSame(['handler'], TrailMiddleware::$trail); + } + + public function test_the_first_middleware_listed_is_the_outermost(): void + { + $this->pipeline()->run( + [OuterMiddleware::class, InnerMiddleware::class], + $this->interaction(), + $this->record('handler'), + ); + + $this->assertSame(['outer', 'inner', 'handler'], TrailMiddleware::$trail); + } + + public function test_a_middleware_that_does_not_continue_stops_the_handler(): void + { + $this->pipeline()->run( + [OuterMiddleware::class, RefusingMiddleware::class, InnerMiddleware::class], + $this->interaction(), + $this->record('handler'), + ); + + $this->assertSame(['outer', 'refused'], TrailMiddleware::$trail); + } + + public function test_middleware_written_as_an_object_is_used_as_it_stands(): void + { + $this->pipeline()->run( + [new TrailMiddleware('inline')], + $this->interaction(), + $this->record('handler'), + ); + + $this->assertSame(['inline', 'handler'], TrailMiddleware::$trail); + } + + /** + * Nothing behind a refusal is built, which is the point of resolving each + * one only as it is reached: middleware that reads a database or calls an + * API costs nothing when the request never gets that far. + */ + public function test_middleware_behind_a_refusal_is_never_constructed(): void + { + $built = 0; + + $container = new GenericContainer(); + $container->singleton(InnerMiddleware::class, function () use (&$built): InnerMiddleware { + $built++; + + return new InnerMiddleware(); + }); + + new MiddlewarePipeline($container)->run( + [RefusingMiddleware::class, InnerMiddleware::class], + $this->interaction(), + $this->record('handler'), + ); + + $this->assertSame(0, $built); + } +} diff --git a/tests/Unit/TestCase.php b/tests/Unit/TestCase.php index 4370c05..dc2324f 100644 --- a/tests/Unit/TestCase.php +++ b/tests/Unit/TestCase.php @@ -19,6 +19,7 @@ use Tempcord\Runtime\ChoiceFactory; use Tempcord\Runtime\CommandBinder; use Tempcord\Runtime\CommandDispatcher; +use Tempcord\Runtime\MiddlewarePipeline; use Tempcord\Runtime\CommandRegistrar; use Tempcord\Runtime\ComponentArgumentResolver; use Tempcord\Runtime\ComponentBinder; @@ -79,6 +80,7 @@ protected function tempcord( new ArgumentResolver(new OptionValueResolver($discord)), new GenericContainer(), new RecordingLogger(), + new MiddlewarePipeline(new GenericContainer()), ), new AutocompleteResponder( new ChoiceFactory(), diff --git a/tools/src/ApiReflector.php b/tools/src/ApiReflector.php index 649e0df..c160b62 100644 --- a/tools/src/ApiReflector.php +++ b/tools/src/ApiReflector.php @@ -60,6 +60,10 @@ 'plugins' => [ \Tempcord\Plugins\Plugin::class, ], + 'middleware' => [ + \Tempcord\Interfaces\Middleware::class, + \Tempcord\Middleware\RequiresPermissions::class, + ], ]; /**