Merge pull request #625 from lucasvanlierop/fix-pg-boolean-conversion

Fix pg boolean conversion

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -2562,9 +2562,13 @@ abstract class AbstractPlatform
      *
      * The default conversion in this implementation converts to integers (false => 0, true => 1).
      *
-     * @param mixed $item
+     * Note: if the input is not a boolean the original input might be returned.
+     *
+     * There are two contexts when converting booleans: Literals and Prepared Statements.
+     * This method should handle the literal case
      *
-     * @return mixed
+     * @param mixed $item A boolean or an array of them.
+     * @return mixed A boolean database value or an array of them.
      */
     public function convertBooleans($item)
     {
@@ -2580,6 +2584,34 @@ abstract class AbstractPlatform
 
         return $item;
     }
+    
+    /**
+     * Some platforms have boolean literals that needs to be correctly converted
+     *
+     * The default conversion tries to convert value into bool "(bool)$item"
+     *
+     * @param mixed $item
+     *
+     * @return bool|null
+     */
+    public function convertFromBoolean($item)
+    {
+        return null === $item ? null: (bool) $item ;
+    }
+
+    /**
+     * This method should handle the prepared statements case. When there is no
+     * distinction, it's OK to use the same method.
+     *
+     * Note: if the input is not a boolean the original input might be returned.
+     *
+     * @param mixed $item A boolean or an array of them.
+     * @return mixed A boolean database value or an array of them.
+     */
+    public function convertBooleansToDatabaseValue($item)
+    {
+        return $this->convertBooleans($item);
+    }
 
     /**
      * Returns the SQL specific for the platform to get the current date.

lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

@@ -44,6 +44,28 @@ class PostgreSqlPlatform extends AbstractPlatform
      */
     private $useBooleanTrueFalseStrings = true;
 
+    /**
+     * @var array PostgreSQL booleans literals
+     */
+    private $booleanLiterals = array(
+        'true' => array(
+            't',
+            'true',
+            'y',
+            'yes',
+            'on',
+            '1'
+        ),
+        'false' => array(
+            'f',
+            'false',
+            'n',
+            'no',
+            'off',
+            '0'
+        )
+    );
+
     /**
      * PostgreSQL has different behavior with some drivers
      * with regard to how booleans have to be handled.
@@ -685,6 +707,71 @@ class PostgreSqlPlatform extends AbstractPlatform
         return $sql;
     }
 
+    /**
+     * Converts a single boolean value.
+     *
+     * First converts the value to its native PHP boolean type
+     * and passes it to the given callback function to be reconverted
+     * into any custom representation.
+     *
+     * @param mixed    $value    The value to convert.
+     * @param callable $callback The callback function to use for converting the real boolean value.
+     *
+     * @return mixed
+     */
+    private function convertSingleBooleanValue($value, $callback)
+    {
+        if (null === $value) {
+            return $callback(false);
+        }
+
+        if (is_bool($value) || is_numeric($value)) {
+            return $callback($value ? true : false);
+        }
+
+        if (!is_string($value)) {
+            return $callback(true);
+        }
+
+        /**
+         * Better safe than sorry: http://php.net/in_array#106319
+         */
+        if (in_array(trim(strtolower($value)), $this->booleanLiterals['false'], true)) {
+            return $callback(false);
+        }
+
+        if (in_array(trim(strtolower($value)), $this->booleanLiterals['true'], true)) {
+            return $callback(true);
+        }
+
+        throw new \UnexpectedValueException("Unrecognized boolean literal '${value}'");
+    }
+
+    /**
+     * Converts one or multiple boolean values.
+     *
+     * First converts the value(s) to their native PHP boolean type
+     * and passes them to the given callback function to be reconverted
+     * into any custom representation.
+     *
+     * @param  $item     The value(s) to convert.
+     * @param  $callback The callback function to use for converting the real boolean value(s).
+     *
+     * @return mixed
+     */
+    private function doConvertBooleans($item, $callback)
+    {
+        if (is_array($item)) {
+            foreach ($item as $key => $value) {
+                $item[$key] = $this->convertSingleBooleanValue($value, $callback);
+            }
+
+            return $item;
+        }
+
+        return $this->convertSingleBooleanValue($item, $callback);
+    }
+
     /**
      * {@inheritDoc}
      *
@@ -696,19 +783,41 @@ class PostgreSqlPlatform extends AbstractPlatform
             return parent::convertBooleans($item);
         }
 
-        if (is_array($item)) {
-            foreach ($item as $key => $value) {
-                if (is_bool($value) || is_numeric($item)) {
-                    $item[$key] = ($value) ? 'true' : 'false';
-                }
+        return $this->doConvertBooleans(
+            $item,
+            function ($boolean) {
+                return true === $boolean ? 'true' : 'false';
             }
-        } else {
-           if (is_bool($item) || is_numeric($item)) {
-               $item = ($item) ? 'true' : 'false';
-           }
+        );
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function convertBooleansToDatabaseValue($item)
+    {
+        if ( ! $this->useBooleanTrueFalseStrings) {
+            return parent::convertBooleansToDatabaseValue($item);
         }
 
-        return $item;
+        return $this->doConvertBooleans(
+            $item,
+            function ($boolean) {
+                return (int) $boolean;
+            }
+        );
+    }
+    
+    /**
+     * {@inheritDoc}
+     */
+    public function convertFromBoolean($item)
+    {
+        if (in_array(strtolower($item), $this->booleanLiterals['false'], true)) {
+            return false;
+        } 
+          
+        return parent::convertFromBoolean($item);
     }
 
     /**

lib/Doctrine/DBAL/Types/BooleanType.php

@@ -41,7 +41,7 @@ class BooleanType extends Type
      */
     public function convertToDatabaseValue($value, AbstractPlatform $platform)
     {
-        return $platform->convertBooleans($value);
+        return $platform->convertBooleansToDatabaseValue($value);
     }
 
     /**
@@ -49,7 +49,7 @@ class BooleanType extends Type
      */
     public function convertToPHPValue($value, AbstractPlatform $platform)
     {
-        return (null === $value) ? null : (bool) $value;
+        return $platform->convertFromBoolean($value);
     }
 
     /**

tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php

@@ -33,6 +33,12 @@ class DBAL630Test extends \Doctrine\Tests\DbalFunctionalTestCase
     {
         if ($this->running) {
             $this->_conn->getWrappedConnection()->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
+
+            // PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT is deprecated in php 5.6. PDO::ATTR_EMULATE_PREPARES should
+            // be used instead. so should only it be set when it is supported.
+            if (PHP_VERSION_ID < 50600) {
+                $this->_conn->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT, false);
+            }
         }
     }
 
@@ -60,12 +66,18 @@ class DBAL630Test extends \Doctrine\Tests\DbalFunctionalTestCase
 
     public function testBooleanConversionBoolParamEmulatedPrepares()
     {
-        $this->markTestIncomplete('There is something missing here, on some machines it fails on some it passes.');
-
         $this->_conn->getWrappedConnection()->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
 
+        // PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT is deprecated in php 5.6. PDO::ATTR_EMULATE_PREPARES should
+        // be used instead. so should only it be set when it is supported.
+        if (PHP_VERSION_ID < 50600) {
+            $this->_conn->getWrappedConnection()->setAttribute(PDO::PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT, true);
+        }
+
+        $platform = $this->_conn->getDatabasePlatform();
+
         $stmt = $this->_conn->prepare('INSERT INTO dbal630 (bool_col) VALUES(?)');
-        $stmt->bindValue(1, 'false', PDO::PARAM_BOOL);
+        $stmt->bindValue(1, $platform->convertBooleansToDatabaseValue('false'), PDO::PARAM_BOOL);
         $stmt->execute();
 
         $id = $this->_conn->lastInsertId('dbal630_id_seq');
@@ -74,6 +86,6 @@ class DBAL630Test extends \Doctrine\Tests\DbalFunctionalTestCase
 
         $row = $this->_conn->fetchAssoc('SELECT bool_col FROM dbal630 WHERE id = ?', array($id));
 
-        $this->assertTrue($row['bool_col']);
+        $this->assertFalse($row['bool_col']);
     }
 }

tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php

@@ -290,25 +290,94 @@ abstract class AbstractPostgreSqlPlatformTestCase extends AbstractPlatformTestCa
 
     /**
      * @group DBAL-457
+     * @dataProvider pgBooleanProvider
+     *
+     * @param string $databaseValue
+     * @param string $prepareStatementValue
+     * @param integer $integerValue
+     * @param boolean $booleanValue
      */
-    public function testConvertBooleanAsStrings()
-    {
+    public function testConvertBooleanAsLiteralStrings(
+        $databaseValue,
+        $preparedStatementValue,
+        $integerValue,
+        $booleanValue
+    ) {
         $platform = $this->createPlatform();
 
-        $this->assertEquals('true', $platform->convertBooleans(true));
-        $this->assertEquals('false', $platform->convertBooleans(false));
+        $this->assertEquals($preparedStatementValue, $platform->convertBooleans($databaseValue));
     }
 
     /**
      * @group DBAL-457
      */
-    public function testConvertBooleanAsIntegers()
+    public function testConvertBooleanAsLiteralIntegers()
+    {
+        $platform = $this->createPlatform();
+        $platform->setUseBooleanTrueFalseStrings(false);
+
+        $this->assertEquals(1, $platform->convertBooleans(true));
+        $this->assertEquals(1, $platform->convertBooleans('1'));
+
+        $this->assertEquals(0, $platform->convertBooleans(false));
+        $this->assertEquals(0, $platform->convertBooleans('0'));
+    }
+
+    /**
+     * @group DBAL-630
+     * @dataProvider pgBooleanProvider
+     *
+     * @param string $databaseValue
+     * @param string $prepareStatementValue
+     * @param integer $integerValue
+     * @param boolean $booleanValue
+     */
+    public function testConvertBooleanAsDatabaseValueStrings(
+        $databaseValue,
+        $preparedStatementValue,
+        $integerValue,
+        $booleanValue
+    )
+    {
+        $platform = $this->createPlatform();
+
+        $this->assertSame($integerValue, $platform->convertBooleansToDatabaseValue($booleanValue));
+    }
+
+    /**
+     * @group DBAL-630
+     */
+    public function testConvertBooleanAsDatabaseValueIntegers()
     {
         $platform = $this->createPlatform();
         $platform->setUseBooleanTrueFalseStrings(false);
 
-        $this->assertEquals('1', $platform->convertBooleans(true));
-        $this->assertEquals('0', $platform->convertBooleans(false));
+        $this->assertSame(1, $platform->convertBooleansToDatabaseValue(true));
+        $this->assertSame(0, $platform->convertBooleansToDatabaseValue(false));
+    }
+
+    /**
+     * @dataProvider pgBooleanProvider
+     *
+     * @param string $databaseValue
+     * @param string $prepareStatementValue
+     * @param integer $integerValue
+     * @param boolean $booleanValue
+     */
+    public function testConvertFromBoolean($databaseValue, $prepareStatementValue, $integerValue, $booleanValue)
+    {
+        $platform = $this->createPlatform();
+
+        $this->assertSame($booleanValue, $platform->convertFromBoolean($databaseValue));
+    }
+
+    /**
+     * @expectedException        UnexpectedValueException
+     * @expectedExceptionMessage Unrecognized boolean literal 'my-bool'
+     */
+    public function testThrowsExceptionWithInvalidBooleanLiteral()
+    {
+        $platform = $this->createPlatform()->convertBooleansToDatabaseValue("my-bool");
     }
 
     public function testGetCreateSchemaSQL()
@@ -521,6 +590,32 @@ abstract class AbstractPostgreSqlPlatformTestCase extends AbstractPlatformTestCa
         );
     }
 
+    /**
+     * PostgreSQL boolean strings provider
+     * @return array
+     */
+    public function pgBooleanProvider()
+    {
+        return array(
+            // Database value, prepared statement value, boolean integer value, boolean value.
+            array(true, 'true', 1, true),
+            array('t', 'true', 1, true),
+            array('true', 'true', 1, true),
+            array('y', 'true', 1, true),
+            array('yes', 'true', 1, true),
+            array('on', 'true', 1, true),
+            array('1', 'true', 1, true),
+
+            array(false, 'false', 0, false),
+            array('f', 'false', 0, false),
+            array('false', 'false', 0, false),
+            array( 'n', 'false', 0, false),
+            array('no', 'false', 0, false),
+            array('off', 'false', 0, false),
+            array('0', 'false', 0, false),
+        );
+    }
+
     /**
      * {@inheritdoc}
      */