Removed the logic of using BLOB columns for BINARY type of fields

doctrine · Jul 2, 2018 · 7e5fb71 · 7e5fb71
1 parent 2685e67
commit 7e5fb71
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 162 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -102,6 +102,12 @@ The Drizzle project is abandoned and is therefore not supported by Doctrine DBAL
 - `Configuration::getSQLLogger()` does not return `null` anymore, but a `NullLogger` implementation.
 - `Configuration::setSQLLogger()` does not allow `null` anymore.
 
+## BC BREAK: Changes to handling binary fields
+
+- Binary fields whose length exceeds the maximum field size on a given platform are no longer represented as `BLOB`s.
+  Use binary fields of a size which fits all target platforms, or use blob explicitly instead.
+- Binary fields are no longer represented as streams in PHP. They are represented as strings.
+
 # Upgrade to 2.8
 
 ## Deprecated usage of binary fields whose length exceeds the platform maximum

diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
@@ -26,7 +26,6 @@
 use Doctrine\DBAL\TransactionIsolationLevel;
 use Doctrine\DBAL\Types;
 use Doctrine\DBAL\Types\Type;
-use const E_USER_DEPRECATED;
 use function addcslashes;
 use function array_map;
 use function array_merge;
@@ -311,20 +310,6 @@ public function getBinaryTypeDeclarationSQL(array $field)
 
         $fixed = $field['fixed'] ?? false;
 
-        $maxLength = $this->getBinaryMaxLength();
-
-        if ($field['length'] > $maxLength) {
-            if ($maxLength > 0) {
-                @trigger_error(sprintf(
-                    'Binary field length %d is greater than supported by the platform (%d). Reduce the field length or use a BLOB field instead.',
-                    $field['length'],
-                    $maxLength
-                ), E_USER_DEPRECATED);
-            }
-
-            return $this->getBlobTypeDeclarationSQL($field);
-        }
-
         return $this->getBinaryTypeDeclarationSQLSnippet($field['length'], $fixed);
     }
 

diff --git a/lib/Doctrine/DBAL/Types/BinaryType.php b/lib/Doctrine/DBAL/Types/BinaryType.php
@@ -4,11 +4,9 @@
 
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use function fopen;
-use function fseek;
-use function fwrite;
 use function is_resource;
 use function is_string;
+use function stream_get_contents;
 
 /**
  * Type that maps ab SQL BINARY/VARBINARY to a PHP resource stream.
@@ -35,14 +33,11 @@ public function convertToPHPValue($value, AbstractPlatform $platform)
             return null;
         }
 
-        if (is_string($value)) {
-            $fp = fopen('php://temp', 'rb+');
-            fwrite($fp, $value);
-            fseek($fp, 0);
-            $value = $fp;
+        if (is_resource($value)) {
+            $value = stream_get_contents($value);
         }
 
-        if ( ! is_resource($value)) {
+        if (! is_string($value)) {
             throw ConversionException::conversionFailed($value, self::BINARY);
         }
 

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Types/BinaryTest.php b/tests/Doctrine/Tests/DBAL/Functional/Types/BinaryTest.php
@@ -7,11 +7,10 @@
 use Doctrine\DBAL\Driver\IBMDB2\DB2Driver;
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalFunctionalTestCase;
-use function is_resource;
 use function random_bytes;
 use function str_replace;
-use function stream_get_contents;
 
 class BinaryTest extends DbalFunctionalTestCase
 {
@@ -74,14 +73,6 @@ private function select(string $id)
             [ParameterType::BINARY]
         );
 
-        // Currently, `BinaryType` mistakenly converts string values fetched from the DB to a stream.
-        // It should be the opposite. Streams should be used to represent large objects, not binary
-        // strings. The confusion comes from the PostgreSQL's type system where binary strings and
-        // large objects are represented by the same BYTEA type
-        if (is_resource($value)) {
-            $value = stream_get_contents($value);
-        }
-
-        return $value;
+        return Type::getType('binary')->convertToPHPValue($value, $this->_conn->getDatabasePlatform());
     }
 }
diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php
@@ -516,30 +516,24 @@ protected function getBinaryMaxLength()
 
     public function testReturnsBinaryTypeDeclarationSQL()
     {
-        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array()));
-        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 0)));
-        self::assertSame('VARBINARY(65535)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 65535)));
-
-        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true)));
-        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 0)));
-        self::assertSame('BINARY(65535)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 65535)));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 65536 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     * @expectedDeprecation Binary field length 16777215 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     * @expectedDeprecation Binary field length 16777216 is greater than supported by the platform (65535). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        self::assertSame('MEDIUMBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 65536]));
-        self::assertSame('MEDIUMBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 16777215]));
-        self::assertSame('LONGBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 16777216]));
-
-        self::assertSame('MEDIUMBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 65536]));
-        self::assertSame('MEDIUMBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 16777215]));
-        self::assertSame('LONGBLOB', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 16777216]));
+        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL([]));
+        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 0]));
+        self::assertSame('VARBINARY(65535)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 65535]));
+        self::assertSame('VARBINARY(65536)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 65536]));
+
+        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
+        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(65535)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 65535,
+        ]));
+        self::assertSame('BINARY(65536)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 65536,
+        ]));
     }
 
     public function testDoesNotPropagateForeignKeyCreationForNonSupportingEngines()

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
@@ -831,11 +831,6 @@ public function testReturnsBinaryTypeDeclarationSQL()
         $this->_platform->getBinaryTypeDeclarationSQL(array());
     }
 
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        $this->markTestSkipped('Not applicable to the platform');
-    }
-
     /**
      * @group DBAL-553
      */

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php
@@ -978,23 +978,19 @@ protected function getBinaryMaxLength()
 
     public function testReturnsBinaryTypeDeclarationSQL()
     {
-        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array()));
-        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 0)));
-        self::assertSame('VARBINARY(8000)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 8000)));
-
-        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true)));
-        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 0)));
-        self::assertSame('BINARY(8000)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 8000)));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 8001 is greater than supported by the platform (8000). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        self::assertSame('VARBINARY(MAX)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 8001]));
-        self::assertSame('VARBINARY(MAX)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 8001]));
+        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL([]));
+        self::assertSame('VARBINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 0]));
+        self::assertSame('VARBINARY(8000)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 8000]));
+
+        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
+        self::assertSame('BINARY(255)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(8000)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 8000,
+        ]));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php
@@ -427,16 +427,6 @@ public function testReturnsBinaryTypeDeclarationSQL()
         self::assertSame('CHAR(254) FOR BIT DATA', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 0]));
     }
 
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 32705 is greater than supported by the platform (32704). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        self::assertSame('BLOB(1M)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 32705]));
-        self::assertSame('BLOB(1M)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 32705]));
-    }
-
     /**
      * @group DBAL-234
      */

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php
@@ -443,23 +443,19 @@ protected function getBinaryMaxLength()
 
     public function testReturnsBinaryTypeDeclarationSQL()
     {
-        self::assertSame('RAW(255)', $this->_platform->getBinaryTypeDeclarationSQL(array()));
-        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 0)));
-        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 2000)));
-
-        self::assertSame('RAW(255)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true)));
-        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 0)));
-        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 2000)));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 2001 is greater than supported by the platform (2000). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        self::assertSame('BLOB', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 2001]));
-        self::assertSame('BLOB', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 2001]));
+        self::assertSame('RAW(255)', $this->_platform->getBinaryTypeDeclarationSQL([]));
+        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 0]));
+        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 2000]));
+
+        self::assertSame('RAW(255)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
+        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('RAW(2000)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 2000,
+        ]));
     }
 
     public function testDoesNotPropagateUnnecessaryTableAlterationOnBinaryType()

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
@@ -935,23 +935,19 @@ protected function getBinaryMaxLength()
 
     public function testReturnsBinaryTypeDeclarationSQL()
     {
-        self::assertSame('VARBINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(array()));
-        self::assertSame('VARBINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 0)));
-        self::assertSame('VARBINARY(32767)', $this->_platform->getBinaryTypeDeclarationSQL(array('length' => 32767)));
+        self::assertSame('VARBINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL([]));
+        self::assertSame('VARBINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 0]));
+        self::assertSame('VARBINARY(32767)', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 32767]));
 
-        self::assertSame('BINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true)));
-        self::assertSame('BINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 0)));
-        self::assertSame('BINARY(32767)', $this->_platform->getBinaryTypeDeclarationSQL(array('fixed' => true, 'length' => 32767)));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Binary field length 32768 is greater than supported by the platform (32767). Reduce the field length or use a BLOB field instead.
-     */
-    public function testReturnsBinaryTypeLongerThanMaxDeclarationSQL()
-    {
-        self::assertSame('LONG BINARY', $this->_platform->getBinaryTypeDeclarationSQL(['length' => 32768]));
-        self::assertSame('LONG BINARY', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true, 'length' => 32768]));
+        self::assertSame('BINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL(['fixed' => true]));
+        self::assertSame('BINARY(1)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 0,
+        ]));
+        self::assertSame('BINARY(32767)', $this->_platform->getBinaryTypeDeclarationSQL([
+            'fixed' => true,
+            'length' => 32767,
+        ]));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Types/BinaryTest.php b/tests/Doctrine/Tests/DBAL/Types/BinaryTest.php
@@ -5,9 +5,11 @@
 use Doctrine\DBAL\ParameterType;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DBAL\Mocks\MockPlatform;
+use function array_map;
 use function base64_encode;
 use function fopen;
-use function stream_get_contents;
+use function implode;
+use function range;
 
 class BinaryTest extends \Doctrine\Tests\DbalTestCase
 {
@@ -52,19 +54,26 @@ public function testBinaryNullConvertsToPHPValue()
 
     public function testBinaryStringConvertsToPHPValue()
     {
-        $databaseValue = 'binary string';
+        $databaseValue = $this->getBinaryString();
         $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
 
-        self::assertInternalType('resource', $phpValue);
-        self::assertEquals($databaseValue, stream_get_contents($phpValue));
+        self::assertSame($databaseValue, $phpValue);
     }
 
     public function testBinaryResourceConvertsToPHPValue()
     {
         $databaseValue = fopen('data://text/plain;base64,' . base64_encode('binary string'), 'r');
         $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
 
-        self::assertSame($databaseValue, $phpValue);
+        self::assertSame('binary string', $phpValue);
+    }
+
+    /**
+     * Creates a binary string containing all possible byte values.
+     */
+    private function getBinaryString() : string
+    {
+        return implode(array_map('chr', range(0, 255)));
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Types/BlobTest.php b/tests/Doctrine/Tests/DBAL/Types/BlobTest.php
@@ -34,37 +34,4 @@ public function testBlobNullConvertsToPHPValue()
     {
         self::assertNull($this->type->convertToPHPValue(null, $this->platform));
     }
-
-    public function testBinaryStringConvertsToPHPValue()
-    {
-        $databaseValue = $this->getBinaryString();
-        $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
-
-        self::assertInternalType('resource', $phpValue);
-        self::assertSame($databaseValue, stream_get_contents($phpValue));
-    }
-
-    public function testBinaryResourceConvertsToPHPValue()
-    {
-        $databaseValue = fopen('data://text/plain;base64,' . base64_encode($this->getBinaryString()), 'r');
-        $phpValue      = $this->type->convertToPHPValue($databaseValue, $this->platform);
-
-        self::assertSame($databaseValue, $phpValue);
-    }
-
-    /**
-     * Creates a binary string containing all possible byte values.
-     *
-     * @return string
-     */
-    private function getBinaryString()
-    {
-        $string = '';
-
-        for ($i = 0; $i < 256; $i++) {
-            $string .= chr($i);
-        }
-
-        return $string;
-    }
 }