Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#869 cleanups and hardening of tests around date-related types #902

Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9f72045
#869 - DBAL-1293 - simplified conversion logic for readability
Ocramius Sep 5, 2015
35a4dfb
#869 - DBAL-1293 - simplified conversion logic for readability (TimeT…
Ocramius Sep 5, 2015
7e80ad8
#869 - DBAL-1293 - Correcting tests around the invalid date/time form…
Ocramius Sep 5, 2015
e5202c0
#869 - DBAL-1293 - Using a data provider for invalid type conversions
Ocramius Sep 5, 2015
b6d34a6
#869 - DBAL-1293 - Adding tests for array values
Ocramius Sep 5, 2015
4e42e4c
#869 - DBAL-1293 - converting all tests to use a data provider
Ocramius Sep 5, 2015
00595fc
#869 - DBAL-1293 - `assertInternalType` over `assertTrue(is_string())`
Ocramius Sep 5, 2015
bc0a4a4
#869 - DBAL-1293 - restoring logic around summer time tests
Ocramius Sep 5, 2015
1a6a796
#869 - DBAL-1293 - abstract test case for date-based types
Ocramius Sep 5, 2015
ef4bd5d
#869 - DBAL-1293 - removing unused imports
Ocramius Sep 5, 2015
dcc18f0
#869 - DBAL-1293 - cleaning up error message generation
Ocramius Sep 5, 2015
ae0a205
#869 - DBAL-1293 - aligning datetime types to the new conversion exce…
Ocramius Sep 5, 2015
f0534f3
#869 - DBAL-1293 - basic coverage for the conversion exception
Ocramius Sep 5, 2015
beb2c7c
#869 - DBAL-1293 - covering remaining conversion exception logic
Ocramius Sep 5, 2015
ae67ace
#869 - DBAL-1293 - testing `DateIntervalType` logic with invalid type…
Ocramius Sep 6, 2015
6c867a0
#869 - DBAL-1293 - `DateIntervalType` should throw on invalid types
Ocramius Sep 6, 2015
52fe0e4
#869 - DBAL-1293 - Removing whitespace
Ocramius Sep 6, 2015
a348e33
#869 - DBAL-1293 - Previous exceptions should be preserved by the `Co…
Ocramius Sep 6, 2015
5eb3e04
#869 - DBAL-1293 - Preserving exceptions for invalid conversions
Ocramius Sep 6, 2015
73f3d2c
#869 - DBAL-1293 - Removing unused assignment
Ocramius Sep 6, 2015
8bfa9bd
#869 - DBAL-1293 - Correcting class names caused by incorrect refacto…
Ocramius Sep 6, 2015
155f5d7
#869 - DBAL-1293 - Making sure `Type::getType('dateinterval')` retrie…
Ocramius Sep 6, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions lib/Doctrine/DBAL/Types/ConversionException.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ConversionException extends \Doctrine\DBAL\DBALException
*/
static public function conversionFailed($value, $toType)
{
$value = (strlen($value) > 32) ? substr($value, 0, 20) . "..." : $value;
$value = (strlen($value) > 32) ? substr($value, 0, 20) . '...' : $value;

return new self('Could not convert database value "' . $value . '" to Doctrine Type ' . $toType);
}
Expand All @@ -50,44 +50,53 @@ static public function conversionFailed($value, $toType)
* Thrown when a Database to Doctrine Type Conversion fails and we can make a statement
* about the expected format.
*
* @param string $value
* @param string $toType
* @param string $expectedFormat
* @param string $value
* @param string $toType
* @param string $expectedFormat
* @param \Exception|null $previous
*
* @return \Doctrine\DBAL\Types\ConversionException
*/
static public function conversionFailedFormat($value, $toType, $expectedFormat)
static public function conversionFailedFormat($value, $toType, $expectedFormat, \Exception $previous = null)
{
$value = (strlen($value) > 32) ? substr($value, 0, 20) . "..." : $value;
$value = (strlen($value) > 32) ? substr($value, 0, 20) . '...' : $value;

return new self(
'Could not convert database value "' . $value . '" to Doctrine Type ' .
$toType . '. Expected format: ' . $expectedFormat
$toType . '. Expected format: ' . $expectedFormat,
0,
$previous
);
}

/**
* Thrown when the PHP value passed to the converter was not of the expected type.
*
* @param mixed $value
* @param string $fromType
* @param mixed $value
* @param string $toType
* @param string[] $possibleTypes
*
* @return \Doctrine\DBAL\Types\ConversionException
*/
static public function conversionFailedInvalidType($value, $toType, $fromType)
static public function conversionFailedInvalidType($value, $toType, array $possibleTypes)
{
$actualType = gettype($value);
if ($actualType === 'object') {
$actualType .= " (" . get_class($value) . ")";
if (!method_exists($value, '__toString')) {
$value = 'object';
}
$actualType = is_object($value) ? get_class($value) : gettype($value);

if (is_scalar($value)) {
return new self(sprintf(
"Could not convert PHP value '%s' of type '%s' to type '%s'. Expected one of the following types: %s",
$value,
$actualType,
$toType,
implode(', ', $possibleTypes)
));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes vastly more sense than the nonsense I wrote.

}
$value = (string)$value;
$value = (strlen($value) > 32) ? substr($value, 0, 20) . "..." : $value;

return new self(
"Could not convert PHP value '$value' of type '$actualType' to type $toType. Expected type: $fromType"
);
return new self(sprintf(
"Could not convert PHP value of type '%s' to type '%s'. Expected one of the following types: %s",
$actualType,
$toType,
implode(', ', $possibleTypes)
));
}
}
28 changes: 11 additions & 17 deletions lib/Doctrine/DBAL/Types/DateIntervalType.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,17 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform)
{
$spec = null;
if ($value !== null) {
/** @var \DateInterval $value */
$spec = 'P'
if (null === $value) {
return null;
}

if ($value instanceof \DateInterval) {
return 'P'
. str_pad($value->y, 4, '0', STR_PAD_LEFT) . '-'
. $value->format('%M') . '-'
. $value->format('%D') . 'T'
. $value->format('%H') . ':'
. $value->format('%I') . ':'
. $value->format('%S')
;
. $value->format('%M-%DT%H:%I:%S');
}

return $spec;
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'DateInterval']);
}

/**
Expand All @@ -59,13 +56,10 @@ public function convertToPHPValue($value, AbstractPlatform $platform)
}

try {
$interval = new \DateInterval($value);
} catch (\Exception $e) {
throw ConversionException::conversionFailedFormat($value, $this->getName(), 'PY-m-dTH:i:s');

return new \DateInterval($value);
} catch (\Exception $exception) {
throw ConversionException::conversionFailedFormat($value, $this->getName(), 'PY-m-dTH:i:s', $exception);
}

return $interval;
}

/**
Expand Down
11 changes: 7 additions & 4 deletions lib/Doctrine/DBAL/Types/DateTimeType.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform)
{
if ($value !== null && !$value instanceof \DateTime) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), "DateTime");
if (null === $value) {
return $value;
}

if ($value instanceof \DateTime) {
return $value->format($platform->getDateTimeFormatString());
}

return ($value !== null)
? $value->format($platform->getDateTimeFormatString()) : null;
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'DateTime']);
}

/**
Expand Down
13 changes: 8 additions & 5 deletions lib/Doctrine/DBAL/Types/DateTimeTzType.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform)
{
if ($value !== null && !$value instanceof \DateTime) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), "DateTime");
if (null === $value) {
return $value;
}

if ($value instanceof \DateTime) {
return $value->format($platform->getDateTimeTzFormatString());
}

return ($value !== null)
? $value->format($platform->getDateTimeTzFormatString()) : null;

throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'DateTime']);
}

/**
Expand Down
11 changes: 7 additions & 4 deletions lib/Doctrine/DBAL/Types/DateType.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform)
{
if ($value !== null && !$value instanceof \DateTime) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), "DateTime");
if (null === $value) {
return $value;
}

if ($value instanceof \DateTime) {
return $value->format($platform->getDateFormatString());
}

return ($value !== null)
? $value->format($platform->getDateFormatString()) : null;
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'DateTime']);
}

/**
Expand Down
11 changes: 7 additions & 4 deletions lib/Doctrine/DBAL/Types/TimeType.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform)
{
if ($value !== null && !$value instanceof \DateTime) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), "DateTime");
if (null === $value) {
return $value;
}

if ($value instanceof \DateTime) {
return $value->format($platform->getTimeFormatString());
}

return ($value !== null)
? $value->format($platform->getTimeFormatString()) : null;
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'DateTime']);
}

/**
Expand Down
94 changes: 94 additions & 0 deletions tests/Doctrine/Tests/DBAL/Types/BaseDateTypeTestCase.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php

namespace Doctrine\Tests\DBAL\Types;

use Doctrine\Tests\DBAL\Mocks\MockPlatform;
use PHPUnit_Framework_TestCase;

abstract class BaseDateTypeTestCase extends PHPUnit_Framework_TestCase
{
/**
* @var MockPlatform
*/
protected $platform;

/**
* @var \Doctrine\DBAL\Types\Type
*/
protected $type;

/**
* @var string
*/
private $currentTimezone;

/**
* {@inheritDoc}
*/
protected function setUp()
{
$this->platform = new MockPlatform();
$this->currentTimezone = date_default_timezone_get();

$this->assertInstanceOf('Doctrine\DBAL\Types\Type', $this->type);
}

/**
* {@inheritDoc}
*/
protected function tearDown()
{
date_default_timezone_set($this->currentTimezone);
}

public function testDateConvertsToDatabaseValue()
{
$this->assertInternalType('string', $this->type->convertToDatabaseValue(new \DateTime(), $this->platform));
}

/**
* @dataProvider invalidPHPValuesProvider
*
* @param mixed $value
*/
public function testInvalidTypeConversionToDatabaseValue($value)
{
$this->setExpectedException('Doctrine\DBAL\Types\ConversionException');

$this->type->convertToDatabaseValue($value, $this->platform);
}

public function testNullConversion()
{
$this->assertNull($this->type->convertToPHPValue(null, $this->platform));
}

public function testConvertDateTimeToPHPValue()
{
$date = new \DateTime('now');

$this->assertSame($date, $this->type->convertToPHPValue($date, $this->platform));
}

/**
* @return mixed[][]
*/
public function invalidPHPValuesProvider()
{
return [
[0],
[''],
['foo'],
['10:11:12'],
['2015-01-31'],
['2015-01-31 10:11:12'],
[new \stdClass()],
[$this],
[27],
[-1],
[1.2],
[[]],
[['an array']],
];
}
}
83 changes: 83 additions & 0 deletions tests/Doctrine/Tests/DBAL/Types/ConversionExceptionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php

namespace Doctrine\Tests\DBAL\Types;

use Doctrine\DBAL\Types\ConversionException;
use PHPUnit_Framework_TestCase;

class ConversionExceptionTest extends PHPUnit_Framework_TestCase
{
/**
* @dataProvider scalarsProvider
*
* @param mixed $scalarValue
*/
public function testConversionFailedInvalidTypeWithScalar($scalarValue)
{
$exception = ConversionException::conversionFailedInvalidType($scalarValue, 'foo', ['bar', 'baz']);

$this->assertInstanceOf('Doctrine\DBAL\Types\ConversionException', $exception);
$this->assertRegExp(
'/^Could not convert PHP value \'.*\' of type \'(string|boolean|float|double|integer)\' to type \'foo\'. '
. 'Expected one of the following types: bar, baz$/',
$exception->getMessage()
);
}
/**
* @dataProvider nonScalarsProvider
*
* @param mixed $nonScalar
*/
public function testConversionFailedInvalidTypeWithNonScalar($nonScalar)
{
$exception = ConversionException::conversionFailedInvalidType($nonScalar, 'foo', ['bar', 'baz']);

$this->assertInstanceOf('Doctrine\DBAL\Types\ConversionException', $exception);
$this->assertRegExp(
'/^Could not convert PHP value of type \'(.*)\' to type \'foo\'. '
. 'Expected one of the following types: bar, baz$/',
$exception->getMessage()
);
}

public function testConversionFailedFormatPreservesPreviousException()
{
$previous = new \Exception();

$exception = ConversionException::conversionFailedFormat('foo', 'bar', 'baz', $previous);

$this->assertInstanceOf('Doctrine\DBAL\Types\ConversionException', $exception);
$this->assertSame($previous, $exception->getPrevious());
}

/**
* @return mixed[][]
*/
public function nonScalarsProvider()
{
return [
[[]],
[['foo']],
[null],
[$this],
[new \stdClass()],
[tmpfile()],
];
}

/**
* @return mixed[][]
*/
public function scalarsProvider()
{
return [
[''],
['foo'],
[123],
[-123],
[12.34],
[true],
[false],
];
}
}
Loading