Merge pull request #2556 from acrobat/deprecate-templating-export-ser…

…vice

[AdminListBundle] Deprecate setter injection and usage of symfony templating

UPGRADE-5.4.md

@@ -5,7 +5,14 @@ AdminBundle
 -----------
 
 * The composer script class `Kunstmaan\AdminBundle\Composer\ScriptHandler` is deprecated and will be removed in 6.0. 
-  If you use this script handler, remove it from your composer.json scripts section.  
+  If you use this script handler, remove it from your composer.json scripts section.
+
+AdminListBundle
+---------------
+
+* Not passing the Twig service as the first argument of "Kunstmaan\AdminListBundle\Service\ExportService::__construct" is deprecated and will be required in 6.0. Injected the required services in the constructor instead.
+* Injecting the template renderer with "Kunstmaan\AdminListBundle\Service\ExportService::setRenderer" is deprecated and will be removed in 6.0. Inject Twig with constructor injection instead.
+* Injecting the translator with "Kunstmaan\AdminListBundle\Service\ExportService::setTranslator" is deprecated and will be removed in 6.0. Inject the Translator with constructor injection instead.
 
 ConfigBundle
 ------------

src/Kunstmaan/AdminListBundle/Resources/config/services.yml

@@ -8,6 +8,9 @@ services:
 
     kunstmaan_adminlist.service.export:
         class:  '%kunstmaan_adminlist.service.export.class%'
+        arguments:
+            - '@twig'
+            - '@translator'
         calls:
             -  [ setRenderer, [ '@templating' ] ]
             -  [ setTranslator, [ '@translator' ] ]

src/Kunstmaan/AdminListBundle/Service/ExportService.php

@@ -10,6 +10,9 @@
 use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;
 use Symfony\Component\HttpFoundation\StreamedResponse;
 use Symfony\Component\Translation\Translator;
+use Symfony\Component\Translation\TranslatorInterface as LegaceTranslatorInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
+use Twig\Environment;
 
 /**
  * class ExportService
@@ -31,7 +34,7 @@ public static function getSupportedExtensions()
     }
 
     /**
-     * @var EngineInterface
+     * @var EngineInterface|Environment
      */
     private $renderer;
 
@@ -40,6 +43,19 @@ public static function getSupportedExtensions()
      */
     private $translator;
 
+    public function __construct(Environment $twig = null, $translator = null)
+    {
+        if (null === $twig) {
+            @trigger_error(sprintf('Not passing the Twig service as the first argument of "%s" is deprecated since KunstmaanAdminListBundle 5.4 and will be required in KunstmaanAdminListBundle 6.0. Injected the required services in the constructor instead.', __METHOD__), E_USER_DEPRECATED);
+        }
+
+        if (null !== $translator && (!$translator instanceof LegaceTranslatorInterface && !$translator instanceof TranslatorInterface)) {
+            throw new \InvalidArgumentException(sprintf('Argument 2 passed to "%s" must be of the type "%s" or "%s", "%s" given', __METHOD__, LegaceTranslatorInterface::class, TranslatorInterface::class, get_class($translator)));
+        }
+
+        $this->renderer = $twig;
+    }
+
     /**
      * @param ExportableInterface $adminList
      * @param string              $format
@@ -95,7 +111,8 @@ protected function streamOutput(ExportableInterface $adminList, $format)
                     }
                     $data = $adminList->getStringValue($itemHelper, $columnName);
                     if (null !== $column->getTemplate()) {
-                        if (!$this->renderer->exists($column->getTemplate())) {
+                        // NEXT_MAJOR: Remove `templateExists` private method and call Twig exists check directly.
+                        if (!$this->templateExists($column->getTemplate())) {
                             throw new ExportException('No export template defined for ' . \get_class($data), $data);
                         }
 
@@ -115,18 +132,52 @@ protected function streamOutput(ExportableInterface $adminList, $format)
     }
 
     /**
+     * @deprecated Setter injection is deprecation since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Use constructor injection instead.
+     *
      * @param EngineInterface $renderer
      */
     public function setRenderer($renderer)
     {
-        $this->renderer = $renderer;
+        if (!$this->renderer instanceof Environment) {
+            if ($renderer instanceof EngineInterface) {
+                @trigger_error(
+                    sprintf('Injecting the template renderer with "%s" is deprecated since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Inject Twig with constructor injection instead.', __METHOD__),
+                    E_USER_DEPRECATED
+                );
+            }
+
+            // Renderer was not set in the constructor, so set it here to the deprecated templating renderer. Constructor
+            // value has precedence over the setter because the implementation is switched to twig.
+            $this->renderer = $renderer;
+        }
     }
 
     /**
+     * @deprecated Setter injection is deprecation since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Use constructor injection instead.
+     *
      * @param Translator $translator
      */
     public function setTranslator($translator)
     {
-        $this->translator = $translator;
+        if (!$this->translator instanceof LegaceTranslatorInterface && !$this->translator instanceof TranslatorInterface) {
+            if ($translator instanceof LegaceTranslatorInterface || $translator instanceof TranslatorInterface) {
+                //Trigger deprecation because setter is deprecated, translator should be injected in the constructor
+                @trigger_error(
+                    sprintf('Injecting the translator with "%s" is deprecated since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Inject the Translator with constructor injection instead.', __METHOD__),
+                    E_USER_DEPRECATED
+                );
+            }
+
+            $this->translator = $translator;
+        }
+    }
+
+    private function templateExists(string $template)
+    {
+        if ($this->renderer instanceof EngineInterface) {
+            return $this->renderer->exists($template);
+        }
+
+        return $this->renderer->getLoader()->exists($template);
     }
 }

src/Kunstmaan/AdminListBundle/Tests/unit/Service/ExportServiceTest.php

@@ -6,6 +6,10 @@
 use Kunstmaan\AdminListBundle\AdminList\ExportableInterface;
 use Kunstmaan\AdminListBundle\Service\ExportService;
 use PHPUnit\Framework\TestCase;
+use Symfony\Bundle\FrameworkBundle\Templating\EngineInterface;
+use Symfony\Component\Translation\Translator;
+use Symfony\Component\Translation\TranslatorInterface;
+use Twig\Environment;
 
 class ExportServiceTest extends TestCase
 {
@@ -14,13 +18,39 @@ class ExportServiceTest extends TestCase
      */
     protected $object;
 
+    protected function setUp()
+    {
+        $this->object = new ExportService($this->createMock(Environment::class), new Translator('nl'));
+    }
+
     /**
-     * Sets up the fixture, for example, opens a network connection.
-     * This method is called before a test is executed.
+     * @group legacy
+     * @expectedDeprecation Not passing the Twig service as the first argument of "Kunstmaan\AdminListBundle\Service\ExportService::__construct" is deprecated since KunstmaanAdminListBundle 5.4 and will be required in KunstmaanAdminListBundle 6.0. Injected the required services in the constructor instead.
      */
-    protected function setUp()
+    public function testWithoutConstructorInjection()
+    {
+        new ExportService();
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation Not passing the Twig service as the first argument of "Kunstmaan\AdminListBundle\Service\ExportService::__construct" is deprecated since KunstmaanAdminListBundle 5.4 and will be required in KunstmaanAdminListBundle 6.0. Injected the required services in the constructor instead.
+     * @expectedDeprecation Injecting the template renderer with "Kunstmaan\AdminListBundle\Service\ExportService::setRenderer" is deprecated since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Inject Twig with constructor injection instead.
+     * @expectedDeprecation Injecting the translator with "Kunstmaan\AdminListBundle\Service\ExportService::setTranslator" is deprecated since KunstmaanAdminListBundle 5.4 and will be removed in KunstmaanAdminListBundle 6.0. Inject the Translator with constructor injection instead.
+     */
+    public function testDeprecatedSetters()
     {
-        $this->object = new ExportService();
+        $obj = new ExportService();
+        $obj->setRenderer($this->createMock(EngineInterface::class));
+        $obj->setTranslator($this->createMock(TranslatorInterface::class));
+    }
+
+    public function testConstructorInvalidTranslatorlass()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Argument 2 passed to "Kunstmaan\AdminListBundle\Service\ExportService::__construct" must be of the type "Symfony\Component\Translation\TranslatorInterface" or "Symfony\Contracts\Translation\TranslatorInterface", "stdClass" given');
+
+        new ExportService($this->createMock(Environment::class), new \stdClass());
     }
 
     public function testGetSupportedExtensions()