Merge pull request #222 from greg0ire/add_missing_deprecations

Add missing deprecations
sonata-project · Dec 13, 2016 · e443906 · e443906
2 parents 93aa458 + 32b654b
commit e443906
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 36 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -32,7 +32,7 @@ public function getConfigTreeBuilder()
 
         $rootNode
             ->children()
-                ->scalarNode('default_formatter')->end() // TODO: make it required when the major version is changed
+                ->scalarNode('default_formatter')->end() // NEXT_MAJOR: make this required
                 ->arrayNode('formatters')
                     ->useAttributeAsKey('name')
                     ->prototype('array')

diff --git a/DependencyInjection/SonataFormatterExtension.php b/DependencyInjection/SonataFormatterExtension.php
@@ -56,8 +56,13 @@ public function load(array $configs, ContainerBuilder $container)
             $loader->load('ckeditor.xml');
         }
 
-        // TODO: To be removed when major version is changed
+        // NEXT_MAJOR: remove this if block
         if (!isset($config['default_formatter'])) {
+            @trigger_error(
+                'Not setting the default_formatter configuration node is deprecated since 3.x,'.
+                ' and will no longer be supported in 4.0.',
+                E_USER_DEPRECATED
+            );
             reset($config['formatters']);
             $config['default_formatter'] = key($config['formatters']);
         }
@@ -67,7 +72,6 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         $pool = $container->getDefinition('sonata.formatter.pool');
-        // TODO: This should become the first (zero-indexed) argument when the major version is changed
         $pool->addArgument($config['default_formatter']);
 
         foreach ($config['formatters'] as $code => $configuration) {

diff --git a/Formatter/Pool.php b/Formatter/Pool.php
@@ -11,12 +11,14 @@
 
 namespace Sonata\FormatterBundle\Formatter;
 
+use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Twig_Environment;
 use Twig_Error_Syntax;
 use Twig_Sandbox_SecurityError;
 
-class Pool
+class Pool implements LoggerAwareInterface
 {
     /**
      * @var array
@@ -29,20 +31,60 @@ class Pool
     protected $defaultFormatter;
 
     /**
+     * NEXT_MAJOR: use LoggerAwareTrait.
+     *
      * @var LoggerInterface
      */
     protected $logger;
 
     /**
-     * @param LoggerInterface|null $logger
-     * @param string|null          $defaultFormatter
+     * @param string|null $defaultFormatter
      */
-    public function __construct(LoggerInterface $logger = null, $defaultFormatter = null)
+    public function __construct($defaultFormatter = null)
     {
-        $this->logger = $logger;
+        if (func_num_args() == 2) {
+            list($logger, $defaultFormatter) = func_get_args();
+            $this->logger = $logger;
+            $this->defaultFormatter = $defaultFormatter;
+        } elseif (func_num_args() == 1) {
+            if ($defaultFormatter instanceof LoggerInterface) {
+                $this->logger = $defaultFormatter;
+            } else {
+                // NEXT_MAJOR: Only keep this block
+                $this->defaultFormatter = $defaultFormatter;
+            }
+        }
 
-        // TODO: This should become a required first parameter when the major version is changed
-        $this->defaultFormatter = $defaultFormatter;
+        // NEXT_MAJOR: keep the else block only
+        if ($this->logger) {
+            @trigger_error(sprintf(
+                'Providing a logger to %s through the constructor is deprecated since 3.x'.
+                ' and will no longer be possible in 4.0'.
+                ' This argument should be provided through the setLogger() method.',
+                __CLASS__
+            ), E_USER_DEPRECATED);
+        } else {
+            $this->logger = new NullLogger();
+        }
+
+        // NEXT_MAJOR: make defaultFormatter required
+        if (is_null($this->defaultFormatter)) {
+            @trigger_error(sprintf(
+                'Not providing the defaultFormatter argument to %s is deprecated since 3.x.'.
+                ' This argument will become mandatory in 4.0.',
+                __METHOD__
+            ), E_USER_DEPRECATED);
+        }
+    }
+
+    /**
+     * NEXT_MAJOR: use Psr\Log\LoggerAwareTrait.
+     *
+     * @param LoggerInterface will be used to report errors
+     */
+    public function setLogger(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
     }
 
     /**
@@ -98,17 +140,23 @@ public function transform($code, $text)
                 $text = $env->render($text);
             }
         } catch (Twig_Error_Syntax $e) {
-            if ($this->logger) {
-                $this->logger->critical(sprintf('[FormatterBundle::transform] %s - Error while parsing twig template : %s', $code, $e->getMessage()), array(
-                    'text' => $text,
-                ));
-            }
+            $this->logger->critical(sprintf(
+                '[FormatterBundle::transform] %s - Error while parsing twig template : %s',
+                $code,
+                $e->getMessage()
+            ), array(
+                'text' => $text,
+                'exception' => $e,
+            ));
         } catch (Twig_Sandbox_SecurityError $e) {
-            if ($this->logger) {
-                $this->logger->critical(sprintf('[FormatterBundle::transform] %s - the user try an non white-listed keyword : %s', $code, $e->getMessage()), array(
-                    'text' => $text,
-                ));
-            }
+            $this->logger->critical(sprintf(
+                '[FormatterBundle::transform] %s - the user try an non white-listed keyword : %s',
+                $code,
+                $e->getMessage()
+            ), array(
+                'text' => $text,
+                'exception' => $e,
+            ));
         }
 
         return $text;
@@ -127,7 +175,7 @@ public function getFormatters()
      */
     public function getDefaultFormatter()
     {
-        // TODO: This should be removed when the major version is changed
+        // NEXT_MAJOR: This should be removed
         if (is_null($this->defaultFormatter)) {
             reset($this->formatters);
 

diff --git a/Resources/config/formatter.xml b/Resources/config/formatter.xml
@@ -8,7 +8,9 @@
     </parameters>
     <services>
         <service id="sonata.formatter.pool" class="Sonata\FormatterBundle\Formatter\Pool">
-            <argument type="service" id="logger"/>
+            <call method="setLogger">
+                <argument type="service" id="logger"/>
+            </call>
         </service>
         <service id="sonata.formatter.text.markdown" class="%sonata.formatter.text.markdown.class%">
             <tag name="sonata.text.formatter" code="markdown"/>

diff --git a/Tests/Form/EventListener/FormatterListenerTest.php b/Tests/Form/EventListener/FormatterListenerTest.php
@@ -21,7 +21,7 @@ public function testWithInvalidFormatter()
     {
         $this->setExpectedException('RuntimeException');
 
-        $pool = new Pool();
+        $pool = $this->getPool();
 
         $listener = new FormatterListener($pool, '[format]', '[source]', '[target]');
 
@@ -41,7 +41,7 @@ public function testWithValidFormatter()
             return strtoupper($text);
         }));
 
-        $pool = new Pool();
+        $pool = $this->getPool();
         $pool->add('myformat', $formatter);
 
         $listener = new FormatterListener($pool, '[format]', '[source]', '[target]');
@@ -62,4 +62,12 @@ public function testWithValidFormatter()
 
         $this->assertSame($expected, $event->getData());
     }
+
+    private function getPool()
+    {
+        $pool = new Pool('whatever');
+        $pool->setLogger($this->getMock('Psr\Log\LoggerInterface'));
+
+        return $pool;
+    }
 }
diff --git a/Tests/Form/Type/FormatterTypeTest.php b/Tests/Form/Type/FormatterTypeTest.php
@@ -86,7 +86,9 @@ public function testBuildFormSeveralChoices()
 
     public function testBuildFormWithCustomFormatter()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -130,7 +132,9 @@ public function testBuildFormWithCustomFormatter()
 
     public function testBuildFormWithDefaultFormatter()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -173,7 +177,9 @@ public function testBuildFormWithDefaultFormatter()
 
     public function testBuildFormWithDefaultFormatterAndPluginManager()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
         $pluginManager = $this->getMock('Ivory\CKEditorBundle\Model\PluginManagerInterface');

diff --git a/Tests/Formatter/PoolTest.php b/Tests/Formatter/PoolTest.php
@@ -22,7 +22,7 @@ public function testPool()
         $env = $this->getMock('\Twig_Environment');
         $env->expects($this->once())->method('render')->will($this->returnValue('Salut'));
 
-        $pool = new Pool();
+        $pool = $this->getPool();
 
         $this->assertFalse($pool->has('foo'));
 
@@ -37,7 +37,7 @@ public function testNonExistantFormatter()
     {
         $this->setExpectedException('RuntimeException');
 
-        $pool = new Pool();
+        $pool = $this->getPool();
         $pool->get('foo');
     }
 
@@ -47,7 +47,7 @@ public function testSyntaxError()
         $env = $this->getMock('\Twig_Environment');
         $env->expects($this->once())->method('render')->will($this->throwException(new \Twig_Error_Syntax('Error')));
 
-        $pool = new Pool();
+        $pool = $this->getPool();
         $pool->add('foo', $formatter, $env);
 
         $this->assertSame('Salut', $pool->transform('foo', 'Salut'));
@@ -59,7 +59,7 @@ public function testTwig_Sandbox_SecurityError()
         $env = $this->getMock('\Twig_Environment');
         $env->expects($this->once())->method('render')->will($this->throwException(new \Twig_Sandbox_SecurityError('Error')));
 
-        $pool = new Pool();
+        $pool = $this->getPool();
         $pool->add('foo', $formatter, $env);
 
         $this->assertSame('Salut', $pool->transform('foo', 'Salut'));
@@ -73,20 +73,25 @@ public function testUnexpectedException()
         $env = $this->getMock('\Twig_Environment');
         $env->expects($this->once())->method('render')->will($this->throwException(new \RuntimeException('Error')));
 
-        $pool = new Pool();
+        $pool = $this->getPool();
         $pool->add('foo', $formatter, $env);
 
         $pool->transform('foo', 'Salut');
     }
 
     public function testDefaultFormatter()
     {
-        $pool = new Pool(null, 'default');
+        $pool = new Pool('default');
+        $pool->setLogger($this->getMock('Psr\Log\LoggerInterface'));
 
         $this->assertSame('default', $pool->getDefaultFormatter());
     }
 
-    // TODO: This should be removed when the major version is changed
+    /**
+     * NEXT_MAJOR: This should be removed.
+     *
+     * @group legacy
+     */
     public function testBcDefaultFormatter()
     {
         $formatter = new RawFormatter();
@@ -98,4 +103,32 @@ public function testBcDefaultFormatter()
 
         $this->assertSame('foo', $pool->getDefaultFormatter());
     }
+
+    /**
+     * NEXT_MAJOR: This should be removed.
+     *
+     * @group legacy
+     */
+    public function testLoggerProvidedThroughConstuctor()
+    {
+        $formatter = new RawFormatter();
+        $pool = new Pool($logger = $this->getMock('Psr\Log\LoggerInterface'));
+        $env = $this->getMock('\Twig_Environment');
+        $env->expects($this->once())->method('render')->will(
+            $this->throwException(new \Twig_Sandbox_SecurityError('Error'))
+        );
+
+        $pool->add('foo', $formatter, $env);
+        $logger->expects($this->once())->method('critical');
+
+        $pool->transform('foo', 'whatever');
+    }
+
+    private function getPool()
+    {
+        $pool = new Pool('whatever');
+        $pool->setLogger($this->getMock('Psr\Log\LoggerInterface'));
+
+        return $pool;
+    }
 }
diff --git a/Tests/Validator/Constraints/FormatterValidatorTest.php b/Tests/Validator/Constraints/FormatterValidatorTest.php
@@ -27,7 +27,9 @@ protected function setUp()
 
     public function testValidator()
     {
-        $pool = $this->getMock('Sonata\FormatterBundle\Formatter\Pool');
+        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
 
         $validator = new FormatterValidator($pool);
         $this->assertInstanceOf('Symfony\Component\Validator\ConstraintValidator', $validator);
@@ -61,7 +63,9 @@ public function testInvalidCase()
 
     public function testValidCase()
     {
-        $pool = $this->getMock('Sonata\FormatterBundle\Formatter\Pool');
+        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
         $pool->expects($this->any())
             ->method('has')
             ->will($this->returnValue(true));