From c2775b7310800b744bfe2f36d0a89ec162ae0fd5 Mon Sep 17 00:00:00 2001 From: Maelan LE BORGNE Date: Mon, 25 Mar 2024 11:22:26 +0100 Subject: [PATCH 1/8] Add new make:webhook command --- src/Maker/MakeWebhook.php | 288 ++++++++++++++++++ src/Resources/config/makers.xml | 6 + src/Resources/help/MakeWebhook.txt | 8 + .../skeleton/webhook/RequestParser.tpl.php | 52 ++++ .../skeleton/webhook/WebhookHandler.tpl.php | 20 ++ tests/Maker/MakeWebhookTest.php | 203 ++++++++++++ .../RemoteServiceRequestParser.php | 52 ++++ .../RemoteServiceWebhookHandler.php | 20 ++ tests/fixtures/make-webhook/webhook.yaml | 6 + 9 files changed, 655 insertions(+) create mode 100644 src/Maker/MakeWebhook.php create mode 100644 src/Resources/help/MakeWebhook.txt create mode 100644 src/Resources/skeleton/webhook/RequestParser.tpl.php create mode 100644 src/Resources/skeleton/webhook/WebhookHandler.tpl.php create mode 100644 tests/Maker/MakeWebhookTest.php create mode 100644 tests/fixtures/make-webhook/RemoteServiceRequestParser.php create mode 100644 tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php create mode 100644 tests/fixtures/make-webhook/webhook.yaml diff --git a/src/Maker/MakeWebhook.php b/src/Maker/MakeWebhook.php new file mode 100644 index 000000000..4dbec5b99 --- /dev/null +++ b/src/Maker/MakeWebhook.php @@ -0,0 +1,288 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Maker; + +use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Bundle\MakerBundle\DependencyBuilder; +use Symfony\Bundle\MakerBundle\FileManager; +use Symfony\Bundle\MakerBundle\Generator; +use Symfony\Bundle\MakerBundle\InputAwareMakerInterface; +use Symfony\Bundle\MakerBundle\InputConfiguration; +use Symfony\Bundle\MakerBundle\Str; +use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; +use Symfony\Bundle\MakerBundle\Util\UseStatementGenerator; +use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator; +use Symfony\Bundle\MakerBundle\Validator; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Question\ChoiceQuestion; +use Symfony\Component\Console\Question\Question; +use Symfony\Component\ExpressionLanguage\Expression; +use Symfony\Component\ExpressionLanguage\ExpressionLanguage; +use Symfony\Component\HttpFoundation\ChainRequestMatcher; +use Symfony\Component\HttpFoundation\Exception\JsonException; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestMatcher\AttributesRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\IpsRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcher\SchemeRequestMatcher; +use Symfony\Component\HttpFoundation\RequestMatcherInterface; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\RemoteEvent\Consumer\ConsumerInterface; +use Symfony\Component\RemoteEvent\RemoteEvent; +use Symfony\Component\Webhook\Client\AbstractRequestParser; +use Symfony\Component\Webhook\Exception\RejectWebhookException; +use Symfony\Component\Yaml\Yaml; + +/** + * @author Maelan LE BORGNE + */ +final class MakeWebhook extends AbstractMaker implements InputAwareMakerInterface +{ + + private const WEBHOOK_CONFIG_PATH = 'config/packages/webhook.yaml'; + private YamlSourceManipulator $ysm; + + public function __construct( + private FileManager $fileManager, + private Generator $generator, + ) { + } + + public static function getCommandName(): string + { + return 'make:webhook'; + } + + public static function getCommandDescription(): string + { + return 'Create a new Webhook'; + } + + public function configureCommand(Command $command, InputConfiguration $inputConfig): void + { + $command + ->addArgument('name', InputArgument::OPTIONAL, sprintf('Name of the webhook to create (e.g. github, stripe, ...)')) + ->setHelp(file_get_contents(__DIR__ . '/../Resources/help/MakeWebhook.txt')) + ; + + $inputConfig->setArgumentAsNonInteractive('name'); + } + + public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null) + { + $dependencies->addClassDependency( + AbstractRequestParser::class, + 'webhook' + ); + $dependencies->addClassDependency( + ConsumerInterface::class, + 'remote-event' + ); + $dependencies->addClassDependency( + Yaml::class, + 'yaml' + ); + } + + public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void + { + if ($input->getArgument('name')) { + if (!$this->verifyWebhookName($input->getArgument('name'))) { + throw new \InvalidArgumentException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); + } + + return; + } + + $argument = $command->getDefinition()->getArgument('name'); + $question = new Question($argument->getDescription()); + $question->setValidator(Validator::notBlank(...)); + $webhookName = $io->askQuestion($question); + + while (!$this->verifyWebhookName($webhookName)) { + $io->error('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); + $webhookName = $io->askQuestion($question); + } + + $input->setArgument('name', $webhookName); + } + + private function verifyWebhookName(string $entityName): bool + { + return preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_.\-\x7f-\xff]*$/', $entityName); + } + + public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void + { + $webhookName = $input->getArgument('name'); + $requestParserDetails = $this->generator->createClassNameDetails( + Str::asClassName($webhookName . 'RequestParser'), + 'Webhook\\' + ); + $remoteEventHandlerDetails = $this->generator->createClassNameDetails( + Str::asClassName($webhookName . 'WebhookHandler'), + 'RemoteEvent\\' + ); + + $this->addToYamlConfig($webhookName, $requestParserDetails); + + $this->generateRequestParser($io, $requestParserDetails); + + $this->generator->generateClass( + $remoteEventHandlerDetails->getFullName(), + 'webhook/WebhookHandler.tpl.php', + [ + 'webhook_name' => $webhookName, + ] + ); + + $this->generator->writeChanges(); + $this->fileManager->dumpFile(self::WEBHOOK_CONFIG_PATH, $this->ysm->getContents()); + } + + private function addToYamlConfig(string $webhookName, ClassNameDetails $requestParserDetails): void + { + if (!$this->fileManager->fileExists(self::WEBHOOK_CONFIG_PATH)) { + $yamlConfig = Yaml::dump(['framework' => ['webhook' => ['routing' => []]]], 4, 2); + } else { + $yamlConfig = $this->fileManager->getFileContents(self::WEBHOOK_CONFIG_PATH); + } + $this->ysm = new YamlSourceManipulator($yamlConfig); + $arrayConfig = $this->ysm->getData(); + if (key_exists($webhookName, $arrayConfig['framework']['webhook']['routing'] ?? [])) { + throw new \InvalidArgumentException('A webhook with this name already exists'); + } + + $arrayConfig['framework']['webhook']['routing'][$webhookName] = [ + 'service' => $requestParserDetails->getFullName(), + 'secret' => 'your_secret_here', + ]; + $this->ysm->setData( + $arrayConfig + ); + } + + /** + * @param ConsoleStyle $io + * @param ClassNameDetails $requestParserDetails + * @return void + * @throws \Exception + */ + public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void + { + $useStatements = new UseStatementGenerator([ + JsonException::class, + Request::class, + Response::class, + RemoteEvent::class, + AbstractRequestParser::class, + RejectWebhookException::class, + RequestMatcherInterface::class, + ]); + $requestMatchers = []; + while (true) { + $newRequestMatcher = $this->askForNextRequestMatcher($io, $requestMatchers, $requestParserDetails->getFullName(), empty($requestMatchers)); + if (null === $newRequestMatcher) { + break; + } + $requestMatchers[] = $newRequestMatcher; + } + $useChainRequestsMatcher = false; + // Use a ChainRequestMatcher if multiple matchers have been added OR if none (will be printed with an empty array) + if (1 !== \count($requestMatchers)) { + $useChainRequestsMatcher = true; + $useStatements->addUseStatement(ChainRequestMatcher::class); + } + $requestMatcherArguments = []; + foreach ($requestMatchers as $requestMatcherClass) { + $useStatements->addUseStatement($requestMatcherClass); + $requestMatcherArguments[$requestMatcherClass] = $this->getRequestMatcherArguments($requestMatcherClass); + if (ExpressionRequestMatcher::class === $requestMatcherClass) { + if (!class_exists(Expression::class)) { + throw new \Exception('The ExpressionRequestMatcher requires the symfony/expression-language package.'); + } + $useStatements->addUseStatement(Expression::class); + $useStatements->addUseStatement(ExpressionLanguage::class); + } + } + + $this->generator->generateClass( + $requestParserDetails->getFullName(), + 'webhook/RequestParser.tpl.php', + [ + 'use_statements' => $useStatements, + 'use_chained_requests_matcher' => $useChainRequestsMatcher, + 'request_matchers' => $requestMatchers, + 'request_matcher_arguments' => $requestMatcherArguments, + ] + ); + } + + private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers, string $entityClass, bool $isFirstMatcher): string|null + { + $io->writeln(''); + $availableMatchers = $this->getAvailableRequestMatchers(); + $matcherName = null; + while (null === $matcherName) { + if ($isFirstMatcher) { + $questionText = 'Add a RequestMatcher (press to skip this step)'; + } else { + $questionText = 'Add another RequestMatcher? Enter the RequestMatcher name (or press to stop adding matchers)'; + } + + $choices = array_diff($availableMatchers, $addedMatchers); + $question = new ChoiceQuestion($questionText, array_values([''] + $choices), 0); + $matcherName = $io->askQuestion($question); + if ('' === $matcherName) { + return null; + } + } + return $matcherName; + } + + private function getAvailableRequestMatchers(): array + { + return [ + AttributesRequestMatcher::class, + ExpressionRequestMatcher::class, + HostRequestMatcher::class, + IpsRequestMatcher::class, + IsJsonRequestMatcher::class, + MethodRequestMatcher::class, + PathRequestMatcher::class, + PortRequestMatcher::class, + SchemeRequestMatcher::class, + ]; + } + + private function getRequestMatcherArguments(string $requestMatcherClass) + { + return match ($requestMatcherClass) { + AttributesRequestMatcher::class => '[\'attributeName\' => \'regex\']', + ExpressionRequestMatcher::class => 'new ExpressionLanguage(), new Expression(\'expression\')', + HostRequestMatcher::class, PathRequestMatcher::class => '\'regex\'', + IpsRequestMatcher::class => '[\'127.0.0.1\']', + IsJsonRequestMatcher::class => '', + MethodRequestMatcher::class => '\'POST\'', + PortRequestMatcher::class => '443', + SchemeRequestMatcher::class => 'https', + default => '[]', + }; + } +} diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index 6575767af..7062f826e 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -157,5 +157,11 @@ + + + + + + diff --git a/src/Resources/help/MakeWebhook.txt b/src/Resources/help/MakeWebhook.txt new file mode 100644 index 000000000..52096f221 --- /dev/null +++ b/src/Resources/help/MakeWebhook.txt @@ -0,0 +1,8 @@ +The %command.name% command creates a RequestParser, a WebhookHandler and adds the necessary configuration +for a new Webhook. + +php %command.full_name% stripe + +If the argument is missing, the command will ask for the webhook name interactively. + +It will also interactively ask for the RequestMatchers to use for the RequestParser's getRequestMatcher function. diff --git a/src/Resources/skeleton/webhook/RequestParser.tpl.php b/src/Resources/skeleton/webhook/RequestParser.tpl.php new file mode 100644 index 000000000..98610b5d6 --- /dev/null +++ b/src/Resources/skeleton/webhook/RequestParser.tpl.php @@ -0,0 +1,52 @@ + + +namespace ; + + + +final class extends AbstractRequestParser +{ + protected function getRequestMatcher(): RequestMatcherInterface + { + + return new ChainRequestMatcher([ + + + + new (), + + ]); + + return new (); + + } + + /** + * @throws JsonException + */ + protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?RemoteEvent + { + // Implement your own logic to validate and parse the request, and return a RemoteEvent object. + + // Validate the request against $secret. + $authToken = $request->headers->get('X-Authentication-Token'); + if (is_null($authToken) || $authToken !== $secret) { + throw new RejectWebhookException(Response::HTTP_UNAUTHORIZED, 'Invalid authentication token.'); + } + + // Validate the request payload. + if (!$request->getPayload()->has('name') + || !$request->getPayload()->has('id')) { + throw new RejectWebhookException(Response::HTTP_BAD_REQUEST, 'Request payload does not contain required fields.'); + } + + // Parse the request payload and return a RemoteEvent object. + $payload = $request->getPayload()->getIterator()->getArrayCopy(); + + return new RemoteEvent( + $payload['name'], + $payload['id'], + $payload, + ); + } +} diff --git a/src/Resources/skeleton/webhook/WebhookHandler.tpl.php b/src/Resources/skeleton/webhook/WebhookHandler.tpl.php new file mode 100644 index 000000000..ab627124b --- /dev/null +++ b/src/Resources/skeleton/webhook/WebhookHandler.tpl.php @@ -0,0 +1,20 @@ + + +namespace ; + +use Symfony\Component\RemoteEvent\Attribute\AsRemoteEventConsumer; +use Symfony\Component\RemoteEvent\Consumer\ConsumerInterface; +use Symfony\Component\RemoteEvent\RemoteEvent; + +#[AsRemoteEventConsumer('')] +final class implements ConsumerInterface +{ + public function __construct() + { + } + + public function consume(RemoteEvent $event): void + { + // Implement your own logic here + } +} diff --git a/tests/Maker/MakeWebhookTest.php b/tests/Maker/MakeWebhookTest.php new file mode 100644 index 000000000..b8dc0db48 --- /dev/null +++ b/tests/Maker/MakeWebhookTest.php @@ -0,0 +1,203 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Tests\Maker; + +use Symfony\Bundle\MakerBundle\Maker\MakeWebhook; +use Symfony\Bundle\MakerBundle\Test\MakerTestCase; +use Symfony\Bundle\MakerBundle\Test\MakerTestRunner; + +class MakeWebhookTest extends MakerTestCase +{ + protected function getMakerClass(): string + { + return MakeWebhook::class; + } + + public function getTestDetails(): \Generator + { + yield 'it_makes_webhook_whit_no_prior_config_file' => [$this->createMakerTest() + ->run(function (MakerTestRunner $runner) { + $output = $runner->runMaker([ + // webhook name + 'remote_service', + // skip adding matchers + '', + ]); + $this->assertStringContainsString('created:', $output); + $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertStringContainsString( + 'use Symfony\Component\Webhook\Client\AbstractRequestParser;', + file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')) + ); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $this->assertStringContainsString( + '#[AsRemoteEventConsumer(\'remote_service\')]', + file_get_contents($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')), + ); + $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); + $this->assertEquals( + 'App\\Webhook\\RemoteServiceRequestParser', + $securityConfig['framework']['webhook']['routing']['remote_service']['service'] + ); + $this->assertEquals( + 'your_secret_here', + $securityConfig['framework']['webhook']['routing']['remote_service']['secret'] + ); + }), + ]; + + yield 'it_makes_webhook_with_prior_webhook' => [$this->createMakerTest() + ->run(function (MakerTestRunner $runner) { + $runner->copy('make-webhook/webhook.yaml', 'config/packages/webhook.yaml'); + $runner->copy('make-webhook/RemoteServiceRequestParser.php', 'src/Webhook/RemoteServiceRequestParser.php'); + $runner->copy('make-webhook/RemoteServiceWebhookHandler.php', 'src/RemoteEvent/RemoteServiceWebhookHandler.php'); + $output = $runner->runMaker([ + // webhook name + 'another_remote_service', + // skip adding matchers + '', + ]); + $this->assertStringContainsString('created:', $output); + $this->assertFileExists($runner->getPath('src/Webhook/AnotherRemoteServiceRequestParser.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/AnotherRemoteServiceWebhookHandler.php')); + $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); + // original config should not be modified + $this->assertArrayHasKey('remote_service', $securityConfig['framework']['webhook']['routing']); + $this->assertEquals( + 'App\\Webhook\\RemoteServiceRequestParser', + $securityConfig['framework']['webhook']['routing']['remote_service']['service'] + ); + $this->assertEquals( + '%env(REMOTE_SERVICE_WEBHOOK_SECRET)%', + $securityConfig['framework']['webhook']['routing']['remote_service']['secret'] + ); + // new config should be added + $this->assertArrayHasKey('another_remote_service', $securityConfig['framework']['webhook']['routing']); + $this->assertEquals( + 'App\\Webhook\\AnotherRemoteServiceRequestParser', + $securityConfig['framework']['webhook']['routing']['another_remote_service']['service'] + ); + $this->assertEquals( + 'your_secret_here', + $securityConfig['framework']['webhook']['routing']['another_remote_service']['secret'] + ); + }), + ]; + + yield 'it_makes_webhook_with_single_matcher' => [$this->createMakerTest() + ->run(function (MakerTestRunner $runner) { + $output = $runner->runMaker([ + // webhook name + 'remote_service', + // add a matcher + 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', + ]); + $this->assertStringContainsString('created:', $output); + $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'return new IsJsonRequestMatcher();', + $requestParserSource + ); + }), + ]; + + yield 'it_makes_webhook_with_multiple_matchers' => [$this->createMakerTest() + ->run(function (MakerTestRunner $runner) { + $output = $runner->runMaker([ + // webhook name + 'remote_service', + // add matchers + 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', + 'Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher', + ]); + $this->assertStringContainsString('created:', $output); + $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\ChainRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + << [$this->createMakerTest() + ->addExtraDependencies('symfony/expression-language') + ->run(function (MakerTestRunner $runner) { + + $output = $runner->runMaker([ + // webhook name + 'remote_service', + // add matchers + 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', + 'Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher', + ]); + $this->assertStringContainsString('created:', $output); + $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\HttpFoundation\ChainRequestMatcher;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\ExpressionLanguage\Expression;', + $requestParserSource + ); + $this->assertStringContainsString( + 'use Symfony\Component\ExpressionLanguage\ExpressionLanguage;', + $requestParserSource + ); + $this->assertStringContainsString( + <<headers->get('X-Authentication-Token'); + + if (null === $authToken || $authToken !== $secret) { + throw new RejectWebhookException(Response::HTTP_UNAUTHORIZED, 'Invalid authentication token.'); + } + + // Validate the request payload. + if (!$request->getPayload()->has('name') + || !$request->getPayload()->has('id')) { + throw new RejectWebhookException(Response::HTTP_BAD_REQUEST, 'Request payload does not contain required fields.'); + } + + // Parse the request payload and return a RemoteEvent object. + $payload = $request->getPayload()->getIterator()->getArrayCopy(); + + return new RemoteEvent( + $payload['name'], + $payload['id'], + $payload, + ); + } +} diff --git a/tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php b/tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php new file mode 100644 index 000000000..0a0e35d21 --- /dev/null +++ b/tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php @@ -0,0 +1,20 @@ + Date: Mon, 25 Mar 2024 11:41:43 +0100 Subject: [PATCH 2/8] CS Fixer --- src/Maker/MakeWebhook.php | 17 +++++++---------- tests/Maker/MakeWebhookTest.php | 21 ++++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/Maker/MakeWebhook.php b/src/Maker/MakeWebhook.php index 4dbec5b99..bb3f442c6 100644 --- a/src/Maker/MakeWebhook.php +++ b/src/Maker/MakeWebhook.php @@ -54,7 +54,6 @@ */ final class MakeWebhook extends AbstractMaker implements InputAwareMakerInterface { - private const WEBHOOK_CONFIG_PATH = 'config/packages/webhook.yaml'; private YamlSourceManipulator $ysm; @@ -77,8 +76,8 @@ public static function getCommandDescription(): string public function configureCommand(Command $command, InputConfiguration $inputConfig): void { $command - ->addArgument('name', InputArgument::OPTIONAL, sprintf('Name of the webhook to create (e.g. github, stripe, ...)')) - ->setHelp(file_get_contents(__DIR__ . '/../Resources/help/MakeWebhook.txt')) + ->addArgument('name', InputArgument::OPTIONAL, 'Name of the webhook to create (e.g. github, stripe, ...)') + ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeWebhook.txt')) ; $inputConfig->setArgumentAsNonInteractive('name'); @@ -132,11 +131,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen { $webhookName = $input->getArgument('name'); $requestParserDetails = $this->generator->createClassNameDetails( - Str::asClassName($webhookName . 'RequestParser'), + Str::asClassName($webhookName.'RequestParser'), 'Webhook\\' ); $remoteEventHandlerDetails = $this->generator->createClassNameDetails( - Str::asClassName($webhookName . 'WebhookHandler'), + Str::asClassName($webhookName.'WebhookHandler'), 'RemoteEvent\\' ); @@ -165,7 +164,7 @@ private function addToYamlConfig(string $webhookName, ClassNameDetails $requestP } $this->ysm = new YamlSourceManipulator($yamlConfig); $arrayConfig = $this->ysm->getData(); - if (key_exists($webhookName, $arrayConfig['framework']['webhook']['routing'] ?? [])) { + if (\array_key_exists($webhookName, $arrayConfig['framework']['webhook']['routing'] ?? [])) { throw new \InvalidArgumentException('A webhook with this name already exists'); } @@ -179,9 +178,6 @@ private function addToYamlConfig(string $webhookName, ClassNameDetails $requestP } /** - * @param ConsoleStyle $io - * @param ClassNameDetails $requestParserDetails - * @return void * @throws \Exception */ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void @@ -234,7 +230,7 @@ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reques ); } - private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers, string $entityClass, bool $isFirstMatcher): string|null + private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers, string $entityClass, bool $isFirstMatcher): ?string { $io->writeln(''); $availableMatchers = $this->getAvailableRequestMatchers(); @@ -253,6 +249,7 @@ private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers return null; } } + return $matcherName; } diff --git a/tests/Maker/MakeWebhookTest.php b/tests/Maker/MakeWebhookTest.php index b8dc0db48..efd84ca21 100644 --- a/tests/Maker/MakeWebhookTest.php +++ b/tests/Maker/MakeWebhookTest.php @@ -143,11 +143,11 @@ public function getTestDetails(): \Generator ); $this->assertStringContainsString( << [$this->createMakerTest() ->addExtraDependencies('symfony/expression-language') ->run(function (MakerTestRunner $runner) { - $output = $runner->runMaker([ // webhook name 'remote_service', @@ -190,11 +189,11 @@ public function getTestDetails(): \Generator ); $this->assertStringContainsString( << Date: Fri, 29 Mar 2024 19:23:37 +0100 Subject: [PATCH 3/8] Fixes after review --- src/Maker/MakeWebhook.php | 56 +++++++++++++++++++-------------- tests/Maker/MakeWebhookTest.php | 2 +- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/Maker/MakeWebhook.php b/src/Maker/MakeWebhook.php index bb3f442c6..e537f560e 100644 --- a/src/Maker/MakeWebhook.php +++ b/src/Maker/MakeWebhook.php @@ -13,6 +13,7 @@ use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; +use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputAwareMakerInterface; @@ -51,11 +52,16 @@ /** * @author Maelan LE BORGNE + * + * @internal */ final class MakeWebhook extends AbstractMaker implements InputAwareMakerInterface { + /** @see https://regex101.com/r/S3BWkx/1 */ + public const WEBHOOK_NAME_PATTERN = '/^[a-zA-Z_.\-\x80-\xff][a-zA-Z0-9_.\-\x80-\xff]*$/u'; private const WEBHOOK_CONFIG_PATH = 'config/packages/webhook.yaml'; private YamlSourceManipulator $ysm; + private ?string $name; public function __construct( private FileManager $fileManager, @@ -83,7 +89,7 @@ public function configureCommand(Command $command, InputConfiguration $inputConf $inputConfig->setArgumentAsNonInteractive('name'); } - public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null) + public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null): void { $dependencies->addClassDependency( AbstractRequestParser::class, @@ -101,9 +107,9 @@ public function configureDependencies(DependencyBuilder $dependencies, ?InputInt public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void { - if ($input->getArgument('name')) { - if (!$this->verifyWebhookName($input->getArgument('name'))) { - throw new \InvalidArgumentException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); + if ($this->name = $input->getArgument('name')) { + if (!$this->verifyWebhookName($this->name)) { + throw new RuntimeCommandException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); } return; @@ -112,34 +118,26 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma $argument = $command->getDefinition()->getArgument('name'); $question = new Question($argument->getDescription()); $question->setValidator(Validator::notBlank(...)); - $webhookName = $io->askQuestion($question); - while (!$this->verifyWebhookName($webhookName)) { + $this->name = $io->askQuestion($question); + while (!$this->verifyWebhookName($this->name)) { $io->error('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); - $webhookName = $io->askQuestion($question); + $this->name = $io->askQuestion($question); } - - $input->setArgument('name', $webhookName); - } - - private function verifyWebhookName(string $entityName): bool - { - return preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_.\-\x7f-\xff]*$/', $entityName); } public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator): void { - $webhookName = $input->getArgument('name'); $requestParserDetails = $this->generator->createClassNameDetails( - Str::asClassName($webhookName.'RequestParser'), + Str::asClassName($this->name.'RequestParser'), 'Webhook\\' ); $remoteEventHandlerDetails = $this->generator->createClassNameDetails( - Str::asClassName($webhookName.'WebhookHandler'), + Str::asClassName($this->name.'WebhookHandler'), 'RemoteEvent\\' ); - $this->addToYamlConfig($webhookName, $requestParserDetails); + $this->addToYamlConfig($this->name, $requestParserDetails); $this->generateRequestParser($io, $requestParserDetails); @@ -147,7 +145,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $remoteEventHandlerDetails->getFullName(), 'webhook/WebhookHandler.tpl.php', [ - 'webhook_name' => $webhookName, + 'webhook_name' => $this->name, ] ); @@ -155,15 +153,21 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->fileManager->dumpFile(self::WEBHOOK_CONFIG_PATH, $this->ysm->getContents()); } + private function verifyWebhookName(string $entityName): bool + { + return preg_match(self::WEBHOOK_NAME_PATTERN, $entityName); + } + private function addToYamlConfig(string $webhookName, ClassNameDetails $requestParserDetails): void { - if (!$this->fileManager->fileExists(self::WEBHOOK_CONFIG_PATH)) { - $yamlConfig = Yaml::dump(['framework' => ['webhook' => ['routing' => []]]], 4, 2); - } else { + $yamlConfig = Yaml::dump(['framework' => ['webhook' => ['routing' => []]]], 4, 2); + if ($this->fileManager->fileExists(self::WEBHOOK_CONFIG_PATH)) { $yamlConfig = $this->fileManager->getFileContents(self::WEBHOOK_CONFIG_PATH); } + $this->ysm = new YamlSourceManipulator($yamlConfig); $arrayConfig = $this->ysm->getData(); + if (\array_key_exists($webhookName, $arrayConfig['framework']['webhook']['routing'] ?? [])) { throw new \InvalidArgumentException('A webhook with this name already exists'); } @@ -180,7 +184,7 @@ private function addToYamlConfig(string $webhookName, ClassNameDetails $requestP /** * @throws \Exception */ - public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void + private function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void { $useStatements = new UseStatementGenerator([ JsonException::class, @@ -191,6 +195,7 @@ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reques RejectWebhookException::class, RequestMatcherInterface::class, ]); + $requestMatchers = []; while (true) { $newRequestMatcher = $this->askForNextRequestMatcher($io, $requestMatchers, $requestParserDetails->getFullName(), empty($requestMatchers)); @@ -199,12 +204,14 @@ public function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reques } $requestMatchers[] = $newRequestMatcher; } - $useChainRequestsMatcher = false; + // Use a ChainRequestMatcher if multiple matchers have been added OR if none (will be printed with an empty array) + $useChainRequestsMatcher = false; if (1 !== \count($requestMatchers)) { $useChainRequestsMatcher = true; $useStatements->addUseStatement(ChainRequestMatcher::class); } + $requestMatcherArguments = []; foreach ($requestMatchers as $requestMatcherClass) { $useStatements->addUseStatement($requestMatcherClass); @@ -245,6 +252,7 @@ private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers $choices = array_diff($availableMatchers, $addedMatchers); $question = new ChoiceQuestion($questionText, array_values([''] + $choices), 0); $matcherName = $io->askQuestion($question); + if ('' === $matcherName) { return null; } diff --git a/tests/Maker/MakeWebhookTest.php b/tests/Maker/MakeWebhookTest.php index efd84ca21..4fcb5d4bb 100644 --- a/tests/Maker/MakeWebhookTest.php +++ b/tests/Maker/MakeWebhookTest.php @@ -24,7 +24,7 @@ protected function getMakerClass(): string public function getTestDetails(): \Generator { - yield 'it_makes_webhook_whit_no_prior_config_file' => [$this->createMakerTest() + yield 'it_makes_webhook_with_no_prior_config_file' => [$this->createMakerTest() ->run(function (MakerTestRunner $runner) { $output = $runner->runMaker([ // webhook name From 35f8652708bedee14078de430cd777d91fee2b52 Mon Sep 17 00:00:00 2001 From: Maelan LE BORGNE Date: Tue, 2 Apr 2024 09:48:45 +0200 Subject: [PATCH 4/8] Add tests for webhook name regex --- tests/RegexTest.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/RegexTest.php b/tests/RegexTest.php index e11f3e462..aee93eed5 100644 --- a/tests/RegexTest.php +++ b/tests/RegexTest.php @@ -12,6 +12,7 @@ namespace Symfony\Bundle\MakerBundle\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\MakerBundle\Maker\MakeWebhook; use Symfony\Bundle\MakerBundle\Test\MakerTestEnvironment; /** @@ -66,4 +67,27 @@ private function generatedFilesRegexDataProvider(): \Generator ['tests/FooBarTest.php'], ]; } + + /** @dataProvider webhookNameRegexDataProvider */ + public function testWebhookNameRegex(string $subjectData, bool $expectedResult): void + { + $result = preg_match(MakeWebhook::WEBHOOK_NAME_PATTERN, $subjectData); + + self::assertSame($expectedResult, (bool) $result); + } + + private function webhookNameRegexDataProvider(): \Generator + { + // Valid cases + yield 'Simple word' => ['mywebhook', true]; + yield 'With underscore' => ['my_webhook', true]; + yield 'With hyphen' => ['my-webhook', true]; + yield 'With extend ascii chars' => ['éÿù', true]; + yield 'With numbers' => ['mywebh00k', true]; + + // Invalid cases + yield 'Leading number' => ['1mywebh00k', false]; + yield 'With space' => ['my webhook', false]; + yield 'With non-ascii characters' => ['web🪝', false]; + } } From 5582b7b540f10bfb6f899bd3590391f0db094a27 Mon Sep 17 00:00:00 2001 From: Maelan LE BORGNE Date: Wed, 3 Apr 2024 11:40:29 +0200 Subject: [PATCH 5/8] Rename WebhookHandler to WebhookConsumer and all() method on payload --- src/Maker/MakeWebhook.php | 8 ++++---- .../skeleton/webhook/RequestParser.tpl.php | 2 +- ...hookHandler.tpl.php => WebhookConsumer.tpl.php} | 0 tests/Maker/MakeWebhookTest.php | 14 +++++++------- ...andler.php => RemoteServiceWebhookConsumer.php} | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) rename src/Resources/skeleton/webhook/{WebhookHandler.tpl.php => WebhookConsumer.tpl.php} (100%) rename tests/fixtures/make-webhook/{RemoteServiceWebhookHandler.php => RemoteServiceWebhookConsumer.php} (85%) diff --git a/src/Maker/MakeWebhook.php b/src/Maker/MakeWebhook.php index e537f560e..300c15731 100644 --- a/src/Maker/MakeWebhook.php +++ b/src/Maker/MakeWebhook.php @@ -132,8 +132,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen Str::asClassName($this->name.'RequestParser'), 'Webhook\\' ); - $remoteEventHandlerDetails = $this->generator->createClassNameDetails( - Str::asClassName($this->name.'WebhookHandler'), + $remoteEventConsumerDetails = $this->generator->createClassNameDetails( + Str::asClassName($this->name.'WebhookConsumer'), 'RemoteEvent\\' ); @@ -142,8 +142,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->generateRequestParser($io, $requestParserDetails); $this->generator->generateClass( - $remoteEventHandlerDetails->getFullName(), - 'webhook/WebhookHandler.tpl.php', + $remoteEventConsumerDetails->getFullName(), + 'webhook/WebhookConsumer.tpl.php', [ 'webhook_name' => $this->name, ] diff --git a/src/Resources/skeleton/webhook/RequestParser.tpl.php b/src/Resources/skeleton/webhook/RequestParser.tpl.php index 98610b5d6..9a60fc972 100644 --- a/src/Resources/skeleton/webhook/RequestParser.tpl.php +++ b/src/Resources/skeleton/webhook/RequestParser.tpl.php @@ -41,7 +41,7 @@ protected function doParse(Request $request, #[\SensitiveParameter] string $secr } // Parse the request payload and return a RemoteEvent object. - $payload = $request->getPayload()->getIterator()->getArrayCopy(); + $payload = $request->getPayload()->all(); return new RemoteEvent( $payload['name'], diff --git a/src/Resources/skeleton/webhook/WebhookHandler.tpl.php b/src/Resources/skeleton/webhook/WebhookConsumer.tpl.php similarity index 100% rename from src/Resources/skeleton/webhook/WebhookHandler.tpl.php rename to src/Resources/skeleton/webhook/WebhookConsumer.tpl.php diff --git a/tests/Maker/MakeWebhookTest.php b/tests/Maker/MakeWebhookTest.php index 4fcb5d4bb..830386ab3 100644 --- a/tests/Maker/MakeWebhookTest.php +++ b/tests/Maker/MakeWebhookTest.php @@ -38,10 +38,10 @@ public function getTestDetails(): \Generator 'use Symfony\Component\Webhook\Client\AbstractRequestParser;', file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')) ); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); $this->assertStringContainsString( '#[AsRemoteEventConsumer(\'remote_service\')]', - file_get_contents($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')), + file_get_contents($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')), ); $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); $this->assertEquals( @@ -59,7 +59,7 @@ public function getTestDetails(): \Generator ->run(function (MakerTestRunner $runner) { $runner->copy('make-webhook/webhook.yaml', 'config/packages/webhook.yaml'); $runner->copy('make-webhook/RemoteServiceRequestParser.php', 'src/Webhook/RemoteServiceRequestParser.php'); - $runner->copy('make-webhook/RemoteServiceWebhookHandler.php', 'src/RemoteEvent/RemoteServiceWebhookHandler.php'); + $runner->copy('make-webhook/RemoteServiceWebhookConsumer.php', 'src/RemoteEvent/RemoteServiceWebhookConsumer.php'); $output = $runner->runMaker([ // webhook name 'another_remote_service', @@ -68,7 +68,7 @@ public function getTestDetails(): \Generator ]); $this->assertStringContainsString('created:', $output); $this->assertFileExists($runner->getPath('src/Webhook/AnotherRemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/AnotherRemoteServiceWebhookHandler.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/AnotherRemoteServiceWebhookConsumer.php')); $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); // original config should not be modified $this->assertArrayHasKey('remote_service', $securityConfig['framework']['webhook']['routing']); @@ -103,7 +103,7 @@ public function getTestDetails(): \Generator ]); $this->assertStringContainsString('created:', $output); $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', @@ -127,7 +127,7 @@ public function getTestDetails(): \Generator ]); $this->assertStringContainsString('created:', $output); $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', @@ -165,7 +165,7 @@ public function getTestDetails(): \Generator ]); $this->assertStringContainsString('created:', $output); $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookHandler.php')); + $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', diff --git a/tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php b/tests/fixtures/make-webhook/RemoteServiceWebhookConsumer.php similarity index 85% rename from tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php rename to tests/fixtures/make-webhook/RemoteServiceWebhookConsumer.php index 0a0e35d21..fe9dd0c4c 100644 --- a/tests/fixtures/make-webhook/RemoteServiceWebhookHandler.php +++ b/tests/fixtures/make-webhook/RemoteServiceWebhookConsumer.php @@ -7,7 +7,7 @@ use Symfony\Component\RemoteEvent\RemoteEvent; #[AsRemoteEventConsumer('remote_service')] -final class RemoteServiceWebhookHandler implements ConsumerInterface +final class RemoteServiceWebhookConsumer implements ConsumerInterface { public function __construct() { From 64622cfcfdc6960549d0ce061540d4a2c6269417 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Sat, 6 Apr 2024 09:09:12 -0400 Subject: [PATCH 6/8] ask questions in interact, install depends automatically, cleanup --- src/Maker/Common/InstallDependencyTrait.php | 37 ++++ src/Maker/MakeWebhook.php | 90 +++++----- tests/Maker/MakeWebhookTest.php | 178 ++++++++++++++------ tools/phpstan/includes/composer.json | 4 +- 4 files changed, 217 insertions(+), 92 deletions(-) create mode 100644 src/Maker/Common/InstallDependencyTrait.php diff --git a/src/Maker/Common/InstallDependencyTrait.php b/src/Maker/Common/InstallDependencyTrait.php new file mode 100644 index 000000000..f74a7f10d --- /dev/null +++ b/src/Maker/Common/InstallDependencyTrait.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Maker\Common; + +use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Component\Process\Process; + +trait InstallDependencyTrait +{ + /** + * @param string $composerPackage Fully qualified composer package to install e.g. symfony/maker-bundle + */ + public function installDependencyIfNeeded(ConsoleStyle $io, string $expectedClassToExist, string $composerPackage): ConsoleStyle + { + if (class_exists($expectedClassToExist)) { + return $io; + } + + $io->writeln(sprintf('Running: composer require %s', $composerPackage)); + + Process::fromShellCommandline(sprintf('composer require %s', $composerPackage))->run(); + + $io->writeln(sprintf('%s successfully installed!', $composerPackage)); + $io->newLine(); + + return $io; + } +} diff --git a/src/Maker/MakeWebhook.php b/src/Maker/MakeWebhook.php index 300c15731..3fe1d1931 100644 --- a/src/Maker/MakeWebhook.php +++ b/src/Maker/MakeWebhook.php @@ -18,6 +18,7 @@ use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputAwareMakerInterface; use Symfony\Bundle\MakerBundle\InputConfiguration; +use Symfony\Bundle\MakerBundle\Maker\Common\InstallDependencyTrait; use Symfony\Bundle\MakerBundle\Str; use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; use Symfony\Bundle\MakerBundle\Util\UseStatementGenerator; @@ -44,7 +45,6 @@ use Symfony\Component\HttpFoundation\RequestMatcher\SchemeRequestMatcher; use Symfony\Component\HttpFoundation\RequestMatcherInterface; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\RemoteEvent\Consumer\ConsumerInterface; use Symfony\Component\RemoteEvent\RemoteEvent; use Symfony\Component\Webhook\Client\AbstractRequestParser; use Symfony\Component\Webhook\Exception\RejectWebhookException; @@ -57,11 +57,18 @@ */ final class MakeWebhook extends AbstractMaker implements InputAwareMakerInterface { - /** @see https://regex101.com/r/S3BWkx/1 */ + use InstallDependencyTrait; + public const WEBHOOK_NAME_PATTERN = '/^[a-zA-Z_.\-\x80-\xff][a-zA-Z0-9_.\-\x80-\xff]*$/u'; private const WEBHOOK_CONFIG_PATH = 'config/packages/webhook.yaml'; + + private ConsoleStyle $io; + private YamlSourceManipulator $ysm; - private ?string $name; + private string $name; + + /** @var array */ + private array $requestMatchers = []; public function __construct( private FileManager $fileManager, @@ -91,14 +98,6 @@ public function configureCommand(Command $command, InputConfiguration $inputConf public function configureDependencies(DependencyBuilder $dependencies, ?InputInterface $input = null): void { - $dependencies->addClassDependency( - AbstractRequestParser::class, - 'webhook' - ); - $dependencies->addClassDependency( - ConsumerInterface::class, - 'remote-event' - ); $dependencies->addClassDependency( Yaml::class, 'yaml' @@ -107,7 +106,11 @@ public function configureDependencies(DependencyBuilder $dependencies, ?InputInt public function interact(InputInterface $input, ConsoleStyle $io, Command $command): void { - if ($this->name = $input->getArgument('name')) { + $this->io = $io; + + $this->installDependencyIfNeeded($io, AbstractRequestParser::class, 'symfony/webhook'); + + if ($this->name = $input->getArgument('name') ?? '') { if (!$this->verifyWebhookName($this->name)) { throw new RuntimeCommandException('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); } @@ -119,10 +122,25 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma $question = new Question($argument->getDescription()); $question->setValidator(Validator::notBlank(...)); - $this->name = $io->askQuestion($question); + $this->name = $this->io->askQuestion($question); + while (!$this->verifyWebhookName($this->name)) { - $io->error('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); - $this->name = $io->askQuestion($question); + $this->io->error('A webhook name can only have alphanumeric characters, underscores, dots, and dashes.'); + $this->name = $this->io->askQuestion($question); + } + + while (true) { + $newRequestMatcher = $this->askForNextRequestMatcher(isFirstMatcher: empty($this->requestMatchers)); + + if (null === $newRequestMatcher) { + break; + } + + $this->requestMatchers[] = $newRequestMatcher; + } + + if (\in_array(ExpressionRequestMatcher::class, $this->requestMatchers, true)) { + $this->installDependencyIfNeeded($this->io, Expression::class, 'symfony/expression-language'); } } @@ -139,7 +157,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->addToYamlConfig($this->name, $requestParserDetails); - $this->generateRequestParser($io, $requestParserDetails); + $this->generateRequestParser(requestParserDetails: $requestParserDetails); $this->generator->generateClass( $remoteEventConsumerDetails->getFullName(), @@ -151,6 +169,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->generator->writeChanges(); $this->fileManager->dumpFile(self::WEBHOOK_CONFIG_PATH, $this->ysm->getContents()); + + $this->writeSuccessMessage($io); } private function verifyWebhookName(string $entityName): bool @@ -184,7 +204,7 @@ private function addToYamlConfig(string $webhookName, ClassNameDetails $requestP /** * @throws \Exception */ - private function generateRequestParser(ConsoleStyle $io, ClassNameDetails $requestParserDetails): void + private function generateRequestParser(ClassNameDetails $requestParserDetails): void { $useStatements = new UseStatementGenerator([ JsonException::class, @@ -196,30 +216,21 @@ private function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reque RequestMatcherInterface::class, ]); - $requestMatchers = []; - while (true) { - $newRequestMatcher = $this->askForNextRequestMatcher($io, $requestMatchers, $requestParserDetails->getFullName(), empty($requestMatchers)); - if (null === $newRequestMatcher) { - break; - } - $requestMatchers[] = $newRequestMatcher; - } - // Use a ChainRequestMatcher if multiple matchers have been added OR if none (will be printed with an empty array) $useChainRequestsMatcher = false; - if (1 !== \count($requestMatchers)) { + + if (1 !== \count($this->requestMatchers)) { $useChainRequestsMatcher = true; $useStatements->addUseStatement(ChainRequestMatcher::class); } $requestMatcherArguments = []; - foreach ($requestMatchers as $requestMatcherClass) { + + foreach ($this->requestMatchers as $requestMatcherClass) { $useStatements->addUseStatement($requestMatcherClass); - $requestMatcherArguments[$requestMatcherClass] = $this->getRequestMatcherArguments($requestMatcherClass); + $requestMatcherArguments[$requestMatcherClass] = $this->getRequestMatcherArguments(requestMatcherClass: $requestMatcherClass); + if (ExpressionRequestMatcher::class === $requestMatcherClass) { - if (!class_exists(Expression::class)) { - throw new \Exception('The ExpressionRequestMatcher requires the symfony/expression-language package.'); - } $useStatements->addUseStatement(Expression::class); $useStatements->addUseStatement(ExpressionLanguage::class); } @@ -231,17 +242,19 @@ private function generateRequestParser(ConsoleStyle $io, ClassNameDetails $reque [ 'use_statements' => $useStatements, 'use_chained_requests_matcher' => $useChainRequestsMatcher, - 'request_matchers' => $requestMatchers, + 'request_matchers' => $this->requestMatchers, 'request_matcher_arguments' => $requestMatcherArguments, ] ); } - private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers, string $entityClass, bool $isFirstMatcher): ?string + private function askForNextRequestMatcher(bool $isFirstMatcher): ?string { - $io->writeln(''); + $this->io->newLine(); + $availableMatchers = $this->getAvailableRequestMatchers(); $matcherName = null; + while (null === $matcherName) { if ($isFirstMatcher) { $questionText = 'Add a RequestMatcher (press to skip this step)'; @@ -249,9 +262,9 @@ private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers $questionText = 'Add another RequestMatcher? Enter the RequestMatcher name (or press to stop adding matchers)'; } - $choices = array_diff($availableMatchers, $addedMatchers); + $choices = array_diff($availableMatchers, $this->requestMatchers); $question = new ChoiceQuestion($questionText, array_values([''] + $choices), 0); - $matcherName = $io->askQuestion($question); + $matcherName = $this->io->askQuestion($question); if ('' === $matcherName) { return null; @@ -261,6 +274,7 @@ private function askForNextRequestMatcher(ConsoleStyle $io, array $addedMatchers return $matcherName; } + /** @return string[] */ private function getAvailableRequestMatchers(): array { return [ @@ -276,7 +290,7 @@ private function getAvailableRequestMatchers(): array ]; } - private function getRequestMatcherArguments(string $requestMatcherClass) + private function getRequestMatcherArguments(string $requestMatcherClass): string { return match ($requestMatcherClass) { AttributesRequestMatcher::class => '[\'attributeName\' => \'regex\']', diff --git a/tests/Maker/MakeWebhookTest.php b/tests/Maker/MakeWebhookTest.php index 830386ab3..f9bc6b05e 100644 --- a/tests/Maker/MakeWebhookTest.php +++ b/tests/Maker/MakeWebhookTest.php @@ -27,27 +27,34 @@ public function getTestDetails(): \Generator yield 'it_makes_webhook_with_no_prior_config_file' => [$this->createMakerTest() ->run(function (MakerTestRunner $runner) { $output = $runner->runMaker([ - // webhook name - 'remote_service', - // skip adding matchers - '', + 'remote_service', // webhook name + '', // skip adding matchers ]); - $this->assertStringContainsString('created:', $output); - $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertStringContainsString( - 'use Symfony\Component\Webhook\Client\AbstractRequestParser;', - file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')) - ); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); - $this->assertStringContainsString( - '#[AsRemoteEventConsumer(\'remote_service\')]', - file_get_contents($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')), - ); + + $this->assertStringContainsString('Success', $output); + + $outputExpectations = [ + 'src/Webhook/RemoteServiceRequestParser.php' => 'use Symfony\Component\Webhook\Client\AbstractRequestParser;', + 'src/RemoteEvent/RemoteServiceWebhookConsumer.php' => '#[AsRemoteEventConsumer(\'remote_service\')]', + ]; + + $this->assertStringContainsString('created: ', $output); + + foreach ($outputExpectations as $expectedFileName => $expectedContent) { + $path = $runner->getPath($expectedFileName); + + $this->assertStringContainsString($expectedFileName, $output); + $this->assertFileExists($runner->getPath($expectedFileName)); + $this->assertStringContainsString($expectedContent, file_get_contents($path)); + } + $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); + $this->assertEquals( 'App\\Webhook\\RemoteServiceRequestParser', $securityConfig['framework']['webhook']['routing']['remote_service']['service'] ); + $this->assertEquals( 'your_secret_here', $securityConfig['framework']['webhook']['routing']['remote_service']['secret'] @@ -56,36 +63,57 @@ public function getTestDetails(): \Generator ]; yield 'it_makes_webhook_with_prior_webhook' => [$this->createMakerTest() + ->addExtraDependencies('symfony/webhook') ->run(function (MakerTestRunner $runner) { $runner->copy('make-webhook/webhook.yaml', 'config/packages/webhook.yaml'); $runner->copy('make-webhook/RemoteServiceRequestParser.php', 'src/Webhook/RemoteServiceRequestParser.php'); $runner->copy('make-webhook/RemoteServiceWebhookConsumer.php', 'src/RemoteEvent/RemoteServiceWebhookConsumer.php'); + $output = $runner->runMaker([ - // webhook name - 'another_remote_service', - // skip adding matchers - '', + 'another_remote_service', // webhook name + '', // skip adding matchers ]); - $this->assertStringContainsString('created:', $output); - $this->assertFileExists($runner->getPath('src/Webhook/AnotherRemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/AnotherRemoteServiceWebhookConsumer.php')); + + $this->assertStringContainsString('Success', $output); + + $outputExpectations = [ + 'src/Webhook/AnotherRemoteServiceRequestParser.php' => 'use Symfony\Component\Webhook\Client\AbstractRequestParser;', + 'src/RemoteEvent/AnotherRemoteServiceWebhookConsumer.php' => '#[AsRemoteEventConsumer(\'another_remote_service\')]', + ]; + + $this->assertStringContainsString('created: ', $output); + + foreach ($outputExpectations as $expectedFileName => $expectedContent) { + $path = $runner->getPath($expectedFileName); + + $this->assertStringContainsString($expectedFileName, $output); + $this->assertFileExists($runner->getPath($expectedFileName)); + $this->assertStringContainsString($expectedContent, file_get_contents($path)); + } + $securityConfig = $runner->readYaml('config/packages/webhook.yaml'); + // original config should not be modified $this->assertArrayHasKey('remote_service', $securityConfig['framework']['webhook']['routing']); + $this->assertEquals( 'App\\Webhook\\RemoteServiceRequestParser', $securityConfig['framework']['webhook']['routing']['remote_service']['service'] ); + $this->assertEquals( '%env(REMOTE_SERVICE_WEBHOOK_SECRET)%', $securityConfig['framework']['webhook']['routing']['remote_service']['secret'] ); + // new config should be added $this->assertArrayHasKey('another_remote_service', $securityConfig['framework']['webhook']['routing']); + $this->assertEquals( 'App\\Webhook\\AnotherRemoteServiceRequestParser', $securityConfig['framework']['webhook']['routing']['another_remote_service']['service'] ); + $this->assertEquals( 'your_secret_here', $securityConfig['framework']['webhook']['routing']['another_remote_service']['secret'] @@ -96,22 +124,30 @@ public function getTestDetails(): \Generator yield 'it_makes_webhook_with_single_matcher' => [$this->createMakerTest() ->run(function (MakerTestRunner $runner) { $output = $runner->runMaker([ - // webhook name - 'remote_service', - // add a matcher - 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', + 'remote_service', // webhook name + '4', // 'IsJsonRequestMatcher', ]); - $this->assertStringContainsString('created:', $output); - $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); - $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertStringContainsString( - 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', - $requestParserSource - ); + + $this->assertStringContainsString('Success', $output); + + $outputExpectations = [ + $parserFileName = 'src/Webhook/RemoteServiceRequestParser.php' => 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + 'src/RemoteEvent/RemoteServiceWebhookConsumer.php' => '#[AsRemoteEventConsumer(\'remote_service\')]', + ]; + + $this->assertStringContainsString('created: ', $output); + + foreach ($outputExpectations as $expectedFileName => $expectedContent) { + $path = $runner->getPath($expectedFileName); + + $this->assertStringContainsString($expectedFileName, $output); + $this->assertFileExists($runner->getPath($expectedFileName)); + $this->assertStringContainsString($expectedContent, file_get_contents($path)); + } + $this->assertStringContainsString( 'return new IsJsonRequestMatcher();', - $requestParserSource + file_get_contents($runner->getPath($parserFileName)) ); }), ]; @@ -119,28 +155,45 @@ public function getTestDetails(): \Generator yield 'it_makes_webhook_with_multiple_matchers' => [$this->createMakerTest() ->run(function (MakerTestRunner $runner) { $output = $runner->runMaker([ - // webhook name - 'remote_service', - // add matchers - 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', - 'Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher', + 'remote_service', // webhook name + '4', // 'IsJsonRequestMatcher', + '6', // 'PortRequestMatcher', ]); - $this->assertStringContainsString('created:', $output); - $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); - $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + + $this->assertStringContainsString('Success', $output); + + $outputExpectations = [ + $parserFileName = 'src/Webhook/RemoteServiceRequestParser.php' => 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + 'src/RemoteEvent/RemoteServiceWebhookConsumer.php' => '#[AsRemoteEventConsumer(\'remote_service\')]', + ]; + + $this->assertStringContainsString('created: ', $output); + + foreach ($outputExpectations as $expectedFileName => $expectedContent) { + $path = $runner->getPath($expectedFileName); + + $this->assertStringContainsString($expectedFileName, $output); + $this->assertFileExists($runner->getPath($expectedFileName)); + $this->assertStringContainsString($expectedContent, file_get_contents($path)); + } + + $requestParserSource = file_get_contents($runner->getPath($parserFileName)); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\ChainRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( <<addExtraDependencies('symfony/expression-language') ->run(function (MakerTestRunner $runner) { $output = $runner->runMaker([ - // webhook name - 'remote_service', - // add matchers - 'Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher', - 'Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher', + 'remote_service', // webhook name + '4', // 'IsJsonRequestMatcher', + '1', // 'ExpressionRequestMatcher', ]); - $this->assertStringContainsString('created:', $output); - $this->assertFileExists($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); - $this->assertFileExists($runner->getPath('src/RemoteEvent/RemoteServiceWebhookConsumer.php')); - $requestParserSource = file_get_contents($runner->getPath('src/Webhook/RemoteServiceRequestParser.php')); + + $this->assertStringContainsString('Success', $output); + + $outputExpectations = [ + $parserFileName = 'src/Webhook/RemoteServiceRequestParser.php' => 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', + 'src/RemoteEvent/RemoteServiceWebhookConsumer.php' => '#[AsRemoteEventConsumer(\'remote_service\')]', + ]; + + $this->assertStringContainsString('created: ', $output); + + foreach ($outputExpectations as $expectedFileName => $expectedContent) { + $path = $runner->getPath($expectedFileName); + + $this->assertStringContainsString($expectedFileName, $output); + $this->assertFileExists($runner->getPath($expectedFileName)); + $this->assertStringContainsString($expectedContent, file_get_contents($path)); + } + + $requestParserSource = file_get_contents($runner->getPath($parserFileName)); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\HttpFoundation\ChainRequestMatcher;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\ExpressionLanguage\Expression;', $requestParserSource ); + $this->assertStringContainsString( 'use Symfony\Component\ExpressionLanguage\ExpressionLanguage;', $requestParserSource ); + $this->assertStringContainsString( << Date: Sat, 6 Apr 2024 10:02:47 -0400 Subject: [PATCH 7/8] dump timeout because of windows... --- .github/workflows/ci-windows.yaml | 2 +- src/Test/MakerTestEnvironment.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-windows.yaml b/.github/workflows/ci-windows.yaml index e35abb645..e6c23e16f 100644 --- a/.github/workflows/ci-windows.yaml +++ b/.github/workflows/ci-windows.yaml @@ -67,4 +67,4 @@ jobs: run: vendor/bin/simple-phpunit --version - name: Run Tests - run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }} + run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }} --filter MakeWebhook diff --git a/src/Test/MakerTestEnvironment.php b/src/Test/MakerTestEnvironment.php index 2e224feec..635c031f9 100644 --- a/src/Test/MakerTestEnvironment.php +++ b/src/Test/MakerTestEnvironment.php @@ -336,7 +336,7 @@ public function createInteractiveCommandProcess(string $commandName, array $user commandLine: sprintf('php bin/console %s %s --no-ansi', $commandName, $argumentsString), cwd: $this->path, envVars: $envVars, - timeout: 10 + timeout: 30 ); if ($userInputs) { From 53d4c204bb6c06bd87078d3e8e7cd351b57e523a Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Sat, 6 Apr 2024 10:07:24 -0400 Subject: [PATCH 8/8] fixed... run all the tests in windows --- .github/workflows/ci-windows.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-windows.yaml b/.github/workflows/ci-windows.yaml index e6c23e16f..e35abb645 100644 --- a/.github/workflows/ci-windows.yaml +++ b/.github/workflows/ci-windows.yaml @@ -67,4 +67,4 @@ jobs: run: vendor/bin/simple-phpunit --version - name: Run Tests - run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }} --filter MakeWebhook + run: vendor/bin/simple-phpunit ${{ env.PHPUNIT_FLAGS }}