Add tests for type registration including deprecations

doctrine · Apr 5, 2019 · b122810 · b122810
1 parent dcd6086
commit b122810
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 15 deletions.
diff --git a/ConnectionFactory.php b/ConnectionFactory.php
@@ -105,13 +105,29 @@ private function initializeTypes(Connection $connection)
                 Type::addType($typeName, $typeConfig['class']);
             }
 
+            $type                   = Type::getType($typeName);
+            $requiresSQLCommentHint = $type->requiresSQLCommentHint($this->getDatabasePlatform($connection));
+
+            // Attribute is missing, make sure a type that doesn't require a comment is marked as commented
+            // This is deprecated behaviour that will be dropped in 2.0.
             if ($typeConfig['commented'] === null) {
+                if (! $requiresSQLCommentHint) {
+                    @trigger_error(
+                        sprintf(
+                            'The type "%s" was implicitly marked as commented due to the configuration. This is deprecated and will be removed in DoctrineBundle 2.0. Either set the "commented" attribute in the configuration to "false" or mark the type as commented in "%s::requiresSQLCommentHint()."',
+                            $typeName,
+                            get_class($type)
+                        ),
+                        E_USER_DEPRECATED
+                    );
+
+                    $this->commentedTypes[] = $typeName;
+                }
+
                 continue;
             }
 
-            $type                   = Type::getType($typeName);
-            $requiresSQLCommentHint = $type->requiresSQLCommentHint($this->getDatabasePlatform($connection));
-
+            // The following logic generates appropriate deprecation notices telling the user how to update their type configuration.
             if ($typeConfig['commented']) {
                 if (! $requiresSQLCommentHint) {
                     @trigger_error(
@@ -151,14 +167,6 @@ private function initializeTypes(Connection $connection)
 
                 continue;
             }
-
-            @trigger_error(
-                sprintf(
-                    'The "commented" option in the type configuration for type "%s" is deprecated and will be removed in DoctrineBundle 2.0. Please remove the "commented" attribute from the type configuration.',
-                    $typeName
-                ),
-                E_USER_DEPRECATED
-            );
         }
 
         $this->initialized = true;

diff --git a/Tests/ConnectionFactoryTest.php b/Tests/ConnectionFactoryTest.php
@@ -3,9 +3,13 @@
 namespace Doctrine\Bundle\DoctrineBundle\Tests;
 
 use Doctrine\Bundle\DoctrineBundle\ConnectionFactory;
+use Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\TestCommentedType;
+use Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\TestType;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Exception\DriverException;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
 use Doctrine\ORM\Version;
 use Exception;
 
@@ -29,11 +33,11 @@ public function testContainer()
     {
         $typesConfig  = [];
         $factory      = new ConnectionFactory($typesConfig);
-        $params       = ['driverClass' => '\\Doctrine\\Bundle\\DoctrineBundle\\Tests\\FakeDriver'];
+        $params       = ['driverClass' => FakeDriver::class];
         $config       = null;
         $eventManager = null;
         $mappingTypes = [0];
-        $exception    = new DriverException('', $this->getMockBuilder(Driver\AbstractDriverException::class)->disableOriginalConstructor()->getMock());
+        $exception    = new DriverException('', $this->createMock(Driver\AbstractDriverException::class));
 
         // put the mock into the fake driver
         FakeDriver::$exception = $exception;
@@ -43,8 +47,116 @@ public function testContainer()
         } catch (Exception $e) {
             $this->assertTrue(strpos($e->getMessage(), 'can circumvent this by setting') > 0);
             throw $e;
+        } finally {
+            FakeDriver::$exception = null;
         }
     }
+
+    /**
+     * @dataProvider getValidTypeConfigurations
+     */
+    public function testRegisterTypes(array $type, int $expectedCalls) : void
+    {
+        $factory      = new ConnectionFactory(['test' => $type]);
+        $params       = ['driverClass' => FakeDriver::class];
+        $config       = null;
+        $eventManager = null;
+        $mappingTypes = [];
+
+        $platform = $this->createMock(AbstractPlatform::class);
+        $platform
+            ->expects($this->exactly($expectedCalls))
+            ->method('markDoctrineTypeCommented')
+            ->with($this->isInstanceOf($type['class']));
+
+        FakeDriver::$platform = $platform;
+
+        try {
+            $factory->createConnection($params, $config, $eventManager, $mappingTypes);
+        } finally {
+            FakeDriver::$platform = null;
+        }
+    }
+
+    public static function getValidTypeConfigurations() : array
+    {
+        return [
+            'uncommentedTypeMarkedNotCommented' => [
+                'type' => [
+                    'class' => TestType::class,
+                    'commented' => false,
+                ],
+                'expectedCalls' => 0,
+            ],
+            'commentedTypeNotMarked' => [
+                'type' => [
+                    'class' => TestCommentedType::class,
+                    'commented' => null,
+                ],
+                'expectedCalls' => 0,
+            ],
+        ];
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation The type "test" was implicitly marked as commented due to the configuration. This is deprecated and will be removed in DoctrineBundle 2.0. Either set the "commented" attribute in the configuration to "false" or mark the type as commented in "Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\TestType::requiresSQLCommentHint()."
+     */
+    public function testRegisterUncommentedTypeNotMarked() : void
+    {
+        $this->testRegisterTypes(
+            [
+                'class' => TestType::class,
+                'commented' => null,
+            ],
+            1
+        );
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation The type "test" was marked as commented in its configuration but not in the type itself. This is deprecated and will be removed in DoctrineBundle 2.0. Please update the return value of "Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\TestType::requiresSQLCommentHint()."
+     */
+    public function testRegisterUncommentedTypeMarkedCommented() : void
+    {
+        $this->testRegisterTypes(
+            [
+                'class' => TestType::class,
+                'commented' => true,
+            ],
+            1
+        );
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation The type "test" was explicitly marked as commented in its configuration. This is no longer necessary and will be removed in DoctrineBundle 2.0. Please remove the "commented" attribute from the type configuration.
+     */
+    public function testRegisterCommentedTypeMarkedCommented() : void
+    {
+        $this->testRegisterTypes(
+            [
+                'class' => TestCommentedType::class,
+                'commented' => true,
+            ],
+            0
+        );
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation Disabling type commenting for the commented type "test" is deprecated and will be removed in DoctrineBundle 2.0. Please remove the "commented" attribute from the configuration and instead disable type commenting in "Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\TestCommentedType::requiresSQLCommentHint()" if you no longer want the type to be commented.
+     */
+    public function testRegisterCommentedTypeMarkedNotCommented() : void
+    {
+        $this->testRegisterTypes(
+            [
+                'class' => TestCommentedType::class,
+                'commented' => false,
+            ],
+            0
+        );
+    }
 }
 
 /**
@@ -62,6 +174,9 @@ class FakeDriver implements Driver
      */
     public static $exception;
 
+    /** @var AbstractPlatform|null */
+    public static $platform;
+
     /**
      * This method gets called to determine the database version which in our case leeds to the problem.
      * So we have to fake the exception a driver would normally throw.
@@ -70,7 +185,11 @@ class FakeDriver implements Driver
      */
     public function getDatabasePlatform()
     {
-        throw self::$exception;
+        if (self::$exception !== null) {
+            throw self::$exception;
+        }
+
+        return static::$platform ?? new MySqlPlatform();
     }
 
     // ----- below this line follow only dummy methods to satisfy the interface requirements ----

diff --git a/Tests/DependencyInjection/TestCommentedType.php b/Tests/DependencyInjection/TestCommentedType.php
@@ -0,0 +1,24 @@
+<?php
+
+namespace Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection;
+
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Types\Type;
+
+class TestCommentedType extends Type
+{
+    public function getName()
+    {
+        return 'test';
+    }
+
+    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
+    {
+        return '';
+    }
+
+    public function requiresSQLCommentHint(AbstractPlatform $platform)
+    {
+        return true;
+    }
+}
diff --git a/Tests/TestCase.php b/Tests/TestCase.php
@@ -48,7 +48,10 @@ public function createYamlBundleTestContainer()
                 ],
                 'default_connection' => 'default',
                 'types' => [
-                    'test' => TestType::class,
+                    'test' => [
+                        'class' => TestType::class,
+                        'commented' => false,
+                    ],
                 ],
             ], 'orm' => [
                 'default_entity_manager' => 'default',

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -17,4 +17,8 @@
             </exclude>
         </whitelist>
     </filter>
+
+    <listeners>
+        <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
+    </listeners>
 </phpunit>