Make the default formatter mandatory

sonata-project · Dec 3, 2016 · 9de44ff · 9de44ff
1 parent 4d29bea
commit 9de44ff
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 59 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')->isRequired()->end()
                 ->arrayNode('formatters')
                     ->useAttributeAsKey('name')
                     ->prototype('array')

diff --git a/DependencyInjection/SonataFormatterExtension.php b/DependencyInjection/SonataFormatterExtension.php
@@ -56,25 +56,29 @@ public function load(array $configs, ContainerBuilder $container)
             $loader->load('ckeditor.xml');
         }
 
-        // TODO: To be removed when major version is changed
-        if (!isset($config['default_formatter'])) {
-            reset($config['formatters']);
-            $config['default_formatter'] = key($config['formatters']);
-        }
-
         if (!array_key_exists($config['default_formatter'], $config['formatters'])) {
-            throw new \InvalidArgumentException(sprintf('SonataFormatterBundle - Invalid default formatter : %s, available : %s', $config['default_formatter'], json_encode(array_keys($config['formatters']))));
+            throw new \InvalidArgumentException(sprintf(
+                'SonataFormatterBundle - Invalid default formatter : %s, available : %s',
+                $config['default_formatter'],
+                json_encode(array_keys($config['formatters']))
+            ));
         }
 
-        $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']);
+        $container->setParameter(
+            'sonata.formater.pool.default_formatter',
+            $config['default_formatter']
+        );
 
         foreach ($config['formatters'] as $code => $configuration) {
             if (count($configuration['extensions']) == 0) {
                 $env = null;
             } else {
-                $env = new Reference($this->createEnvironment($container, $code, $container->getDefinition($configuration['service']), $configuration['extensions']));
+                $env = new Reference($this->createEnvironment(
+                    $container,
+                    $code,
+                    $container->getDefinition($configuration['service']),
+                    $configuration['extensions']
+                ));
             }
 
             $pool->addMethodCall('add', array($code, new Reference($configuration['service']), $env));

diff --git a/Formatter/Pool.php b/Formatter/Pool.php
@@ -34,15 +34,13 @@ class Pool
     protected $logger;
 
     /**
-     * @param LoggerInterface|null $logger
-     * @param string|null          $defaultFormatter
+     * @param string          $defaultFormatter
+     * @param LoggerInterface $logger
      */
-    public function __construct(LoggerInterface $logger = null, $defaultFormatter = null)
+    public function __construct($defaultFormatter, LoggerInterface $logger)
     {
-        $this->logger = $logger;
-
-        // TODO: This should become a required first parameter when the major version is changed
         $this->defaultFormatter = $defaultFormatter;
+        $this->logger = $logger;
     }
 
     /**
@@ -127,13 +125,6 @@ public function getFormatters()
      */
     public function getDefaultFormatter()
     {
-        // TODO: This should be removed when the major version is changed
-        if (is_null($this->defaultFormatter)) {
-            reset($this->formatters);
-
-            $this->defaultFormatter = key($this->formatters);
-        }
-
         return $this->defaultFormatter;
     }
 }
diff --git a/Resources/config/formatter.xml b/Resources/config/formatter.xml
@@ -8,6 +8,7 @@
     </parameters>
     <services>
         <service id="sonata.formatter.pool" class="Sonata\FormatterBundle\Formatter\Pool">
+            <argument>%sonata.formatter.pool.default_formatter%</argument>
             <argument type="service" id="logger"/>
         </service>
         <service id="sonata.formatter.text.markdown" class="%sonata.formatter.text.markdown.class%">

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 = new Pool('whatever');
 
         $listener = new FormatterListener($pool, '[format]', '[source]', '[target]');
 
@@ -41,7 +41,7 @@ public function testWithValidFormatter()
             return strtoupper($text);
         }));
 
-        $pool = new Pool();
+        $pool = new Pool('whatever');
         $pool->add('myformat', $formatter);
 
         $listener = new FormatterListener($pool, '[format]', '[source]', '[target]');

diff --git a/Tests/Form/Type/FormatterTypeTest.php b/Tests/Form/Type/FormatterTypeTest.php
@@ -24,7 +24,7 @@ class FormatterTypeTest extends \PHPUnit_Framework_TestCase
 {
     public function testBuildFormOneChoice()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
 
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
@@ -56,7 +56,7 @@ public function testBuildFormOneChoice()
 
     public function testBuildFormSeveralChoices()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -86,7 +86,7 @@ public function testBuildFormSeveralChoices()
 
     public function testBuildFormWithCustomFormatter()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -130,7 +130,7 @@ public function testBuildFormWithCustomFormatter()
 
     public function testBuildFormWithDefaultFormatter()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -173,7 +173,7 @@ public function testBuildFormWithDefaultFormatter()
 
     public function testBuildFormWithDefaultFormatterAndPluginManager()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
         $pluginManager = $this->getMock('Ivory\CKEditorBundle\Model\PluginManagerInterface');
@@ -217,7 +217,7 @@ public function testBuildFormWithDefaultFormatterAndPluginManager()
 
     public function testBuildViewWithDefaultConfig()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -249,7 +249,7 @@ public function testBuildViewWithDefaultConfig()
 
     public function testBuildViewWithoutDefaultConfig()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -282,7 +282,7 @@ public function testBuildViewWithoutDefaultConfig()
 
     public function testBuildViewWithDefaultConfigAndWithToolbarIcons()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -316,7 +316,7 @@ public function testBuildViewWithDefaultConfigAndWithToolbarIcons()
 
     public function testBuildViewWithFormatter()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
 
@@ -353,7 +353,7 @@ public function testBuildViewWithFormatter()
 
     public function testBuildViewWithDefaultConfigAndPluginManager()
     {
-        $pool = $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')->disableOriginalConstructor()->getMock();
+        $pool = $this->getPool();
         $translator = $this->getMock('Symfony\Component\Translation\TranslatorInterface');
         $configManager = $this->getMock('Ivory\CKEditorBundle\Model\ConfigManagerInterface');
         $pluginManager = $this->getMock('Ivory\CKEditorBundle\Model\PluginManagerInterface');
@@ -383,4 +383,11 @@ public function testBuildViewWithDefaultConfigAndPluginManager()
 
         $this->assertSame($view->vars['ckeditor_configuration'], $defaultConfigValues);
     }
+
+    private function getPool()
+    {
+        return $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
+    }
 }
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 = new Pool('whatever');
 
         $this->assertFalse($pool->has('foo'));
 
@@ -37,7 +37,7 @@ public function testNonExistantFormatter()
     {
         $this->setExpectedException('RuntimeException');
 
-        $pool = new Pool();
+        $pool = new Pool('whatever');
         $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 = new Pool('whatever');
         $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 = new Pool('whatever');
         $pool->add('foo', $formatter, $env);
 
         $this->assertSame('Salut', $pool->transform('foo', 'Salut'));
@@ -73,29 +73,16 @@ public function testUnexpectedException()
         $env = $this->getMock('\Twig_Environment');
         $env->expects($this->once())->method('render')->will($this->throwException(new \RuntimeException('Error')));
 
-        $pool = new Pool();
+        $pool = new Pool('whatever');
         $pool->add('foo', $formatter, $env);
 
         $pool->transform('foo', 'Salut');
     }
 
     public function testDefaultFormatter()
     {
-        $pool = new Pool(null, 'default');
+        $pool = new Pool('default', null);
 
         $this->assertSame('default', $pool->getDefaultFormatter());
     }
-
-    // TODO: This should be removed when the major version is changed
-    public function testBcDefaultFormatter()
-    {
-        $formatter = new RawFormatter();
-        $env = $this->getMock('\Twig_Environment');
-
-        $pool = new Pool();
-
-        $pool->add('foo', $formatter, $env);
-
-        $this->assertSame('foo', $pool->getDefaultFormatter());
-    }
 }
diff --git a/Tests/Validator/Constraints/FormatterValidatorTest.php b/Tests/Validator/Constraints/FormatterValidatorTest.php
@@ -27,7 +27,7 @@ protected function setUp()
 
     public function testValidator()
     {
-        $pool = $this->getMock('Sonata\FormatterBundle\Formatter\Pool');
+        $pool = $this->getPool();
 
         $validator = new FormatterValidator($pool);
         $this->assertInstanceOf('Symfony\Component\Validator\ConstraintValidator', $validator);
@@ -38,7 +38,7 @@ public function testValidator()
      */
     public function testInvalidCase()
     {
-        $pool = $this->getMock('Sonata\FormatterBundle\Formatter\Pool');
+        $pool = $this->getPool();
         $pool->expects($this->any())
             ->method('has')
             ->will($this->returnValue(false));
@@ -61,7 +61,7 @@ public function testInvalidCase()
 
     public function testValidCase()
     {
-        $pool = $this->getMock('Sonata\FormatterBundle\Formatter\Pool');
+        $pool = $this->getPool();
         $pool->expects($this->any())
             ->method('has')
             ->will($this->returnValue(true));
@@ -80,4 +80,11 @@ public function testValidCase()
 
         $validator->validate('existingFormatter', $constraint);
     }
+
+    private function getPool()
+    {
+        return $this->getMockBuilder('Sonata\FormatterBundle\Formatter\Pool')
+            ->disableOriginalConstructor()
+            ->getMock();
+    }
 }