Validate shortcode controllername service configurations (Case 173693) (#40)

Resolves #35

---------

Co-authored-by: Malte Wunsch <[email protected]>

Author: FabianSchmick

.gitignore

@@ -1,6 +1,7 @@
 composer.lock
 vendor/
 phpunit.xml
+.phpunit.cache/
 .phpunit.result.cache
 .php-cs-fixer.cache
 tests/Fixtures/cache/

src/Handler/EmbeddedShortcodeHandler.php

@@ -2,6 +2,7 @@
 
 namespace Webfactory\ShortcodeBundle\Handler;
 
+use InvalidArgumentException;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -33,17 +34,15 @@ class EmbeddedShortcodeHandler
     /** @var RequestStack */
     private $requestStack;
 
-    /**
-     * @param string $controllerName
-     * @param string $renderer
-     */
     public function __construct(
         FragmentHandler $fragmentHandler,
-        $controllerName,
-        $renderer,
+        string $controllerName,
+        string $renderer,
         RequestStack $requestStack,
         ?LoggerInterface $logger = null
     ) {
+        $this->validateControllerName($controllerName);
+
         $this->fragmentHandler = $fragmentHandler;
         $this->controllerName = $controllerName;
         $this->renderer = $renderer;
@@ -90,4 +89,22 @@ public function getControllerName(): string
     {
         return $this->controllerName;
     }
+
+    private function validateControllerName(string $controllerName): void
+    {
+        if (class_exists($controllerName)) {
+            // Check with method_exists instead of is_callable, because is_callable would need an object instance to
+            // positively test an invokable classes
+            if (method_exists($controllerName, '__invoke')) {
+                return;
+            }
+
+            throw new InvalidArgumentException('The configured controller "'.$controllerName.'" does not refer a method. Although a class "'.$controllerName.'" exists, but has no __invoke method.');
+        }
+
+        $callableFragments = explode('::', $controllerName);
+        if (!\is_array($callableFragments) || 2 !== \count($callableFragments) || !method_exists($callableFragments[0], $callableFragments[1])) {
+            throw new InvalidArgumentException('The controller method: "'.$controllerName.'" does not exist.');
+        }
+    }
 }

tests/Fixtures/Controller/InvokableShortcodeTestController.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Webfactory\ShortcodeBundle\Tests\Fixtures\Controller;
+
+use Symfony\Component\HttpFoundation\Response;
+
+final class InvokableShortcodeTestController
+{
+    public function __invoke(): Response
+    {
+        return new Response('invokable-controller-response');
+    }
+}

tests/Fixtures/config/config.yml

@@ -14,11 +14,11 @@ webfactory_shortcode:
         test-config-esi:
             controller: 'Webfactory\ShortcodeBundle\Tests\Fixtures\Controller\ShortcodeTestController::test'
             method: esi
+        test-config-invokable: 'Webfactory\ShortcodeBundle\Tests\Fixtures\Controller\InvokableShortcodeTestController'
         test-shortcode-guide:
             controller: 'Webfactory\ShortcodeBundle\Tests\Fixtures\Controller\ShortcodeTestController::test'
             description: "Description for the 'test-shortcode-guide' shortcode"
             example: "test-shortcode-guide test=true"
-        test-config-invalid-controller: 'Foo\Bar::baz'
 
 services:
     Webfactory\ShortcodeBundle\Tests\Fixtures\Controller\:

tests/Functional/EmbeddedShortcodeHandlerTest.php

@@ -3,10 +3,14 @@
 namespace Webfactory\ShortcodeBundle\Tests\Functional;
 
 use Generator;
+use InvalidArgumentException;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpKernel\Fragment\FragmentHandler;
+use Webfactory\ShortcodeBundle\Handler\EmbeddedShortcodeHandler;
 use Webfactory\ShortcodeBundle\Test\EndToEndTestHelper;
+use Webfactory\ShortcodeBundle\Tests\Fixtures\Controller\ShortcodeTestController;
 
 /**
  * Test shortcode processing using EmbeddedShortcodeHandler and a fixture ShortodeTestController,
@@ -64,6 +68,34 @@ public static function provideEsiShortcodes(): Generator
         yield 'ESI-based shortcode defined in service configuration' => ['test-service-esi'];
     }
 
+    /** @test */
+    public function invokable_controller_can_be_used(): void
+    {
+        self::assertSame('invokable-controller-response', $this->processShortcodes('<p>[test-config-invokable]</p>'));
+    }
+
+    /**
+     * @test
+     *
+     * @dataProvider provideControllerNames
+     */
+    public function throws_exception_on_invalid_controller_names(string $controllerName): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        new EmbeddedShortcodeHandler($this->createMock(FragmentHandler::class), $controllerName, 'inline', $this->createMock(RequestStack::class));
+    }
+
+    public static function provideControllerNames(): Generator
+    {
+        yield 'Empty string' => [''];
+        yield 'Not existing controller' => ['Foo\Bar::baz'];
+        yield 'Missing method name' => [ShortcodeTestController::class];
+        yield 'Not existing method' => [ShortcodeTestController::class.'_notExistingMethod'];
+        yield 'Missing class' => ['ThisClassDoesNotExist'];
+        yield 'Valid reference followed by a second scope resolution operator' => [ShortcodeTestController::class.'::test::'];
+    }
+
     private function processShortcodes(string $content, ?Request $request = null): string
     {
         self::bootKernel();

tests/Functional/ShortcodeDefinitionTestHelperTest.php

@@ -2,7 +2,6 @@
 
 namespace Webfactory\ShortcodeBundle\Tests\Functional;
 
-use InvalidArgumentException;
 use RuntimeException;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Thunder\Shortcode\Handler\PlaceholderHandler;
@@ -31,15 +30,6 @@ public function throws_exception_for_handlers_that_do_not_use_controllers(): voi
         $this->helper->resolveShortcodeController('placeholder'); // uses the \Thunder\Shortcode\Handler\PlaceholderHandler handler class directly
     }
 
-    /**
-     * @test
-     */
-    public function throws_exception_for_shortcode_with_unresolvable_controller(): void
-    {
-        self::expectException(InvalidArgumentException::class);
-        $this->helper->resolveShortcodeController('test-config-invalid-controller');
-    }
-
     /**
      * @test
      */