From 5465c6c90fb41bd9f040716f902f015abb710453 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 6 Jun 2018 14:38:36 -0700 Subject: [PATCH] Removed the logic of using BLOB columns for BINARY type of fields --- UPGRADE.md | 6 +++ .../DBAL/Platforms/AbstractPlatform.php | 15 ------- lib/Doctrine/DBAL/Types/BinaryType.php | 13 ++---- .../DBAL/Functional/Types/BinaryTest.php | 13 +----- .../AbstractMySQLPlatformTestCase.php | 42 ++++++++----------- .../Platforms/AbstractPlatformTestCase.php | 5 --- .../AbstractSQLServerPlatformTestCase.php | 30 ++++++------- .../Tests/DBAL/Platforms/DB2PlatformTest.php | 10 ----- .../DBAL/Platforms/OraclePlatformTest.php | 30 ++++++------- .../Platforms/SQLAnywherePlatformTest.php | 28 ++++++------- .../Doctrine/Tests/DBAL/Types/BinaryTest.php | 19 ++++++--- tests/Doctrine/Tests/DBAL/Types/BlobTest.php | 33 --------------- 12 files changed, 82 insertions(+), 162 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 3c804ca3fb0..9334ba11b23 100644 --- 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 DB-generated UUIDs diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index 20e7be7b96b..9cca366bcc5 100644 --- 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 index 418ed2ff0cc..2559cc9f635 100644 --- 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 index f4e6ff1a84d..650e36b73f4 100644 --- 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 index b8fcfb35ede..b085645ed14 100644 --- 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 index 821405f7217..a7271d6a637 100644 --- 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 index 6271e778844..5e1801a912b 100644 --- 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 index 5373b3f640d..9e85d21d34c 100644 --- 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 index 5e270b4a1e7..fe7f541f5a1 100644 --- 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 index 6142459fc71..e7548b6ba5e 100644 --- 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 index 9881c897960..b8b74523a92 100644 --- 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,11 +54,10 @@ 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() @@ -64,7 +65,15 @@ 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 index 1ee3fefc43c..f23ad69a671 100644 --- 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; - } }