-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New DateTimeFormatter Filter (#3617) #3632
Changes from 5 commits
668c84a
c41ec21
ebca3f6
e81ba2e
81ec65f
1f9de5d
a2a7712
90b0456
bef553b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
<?php | ||
/** | ||
* Zend Framework (http://framework.zend.com/) | ||
* | ||
* @link http://github.com/zendframework/zf2 for the canonical source repository | ||
* @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com) | ||
* @license http://framework.zend.com/license/new-bsd New BSD License | ||
*/ | ||
|
||
namespace Zend\Filter; | ||
|
||
use DateTime; | ||
use Exception; | ||
|
||
class DateTimeFormatter extends AbstractFilter | ||
{ | ||
/** | ||
* A valid format string accepted by date() | ||
* | ||
* @var string | ||
*/ | ||
protected $format = DateTime::ISO8601; | ||
|
||
/** | ||
* Sets filter options | ||
* | ||
* @param string|array|\Zend\Config\Config $options | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
*/ | ||
public function __construct($options = null) | ||
{ | ||
if ($options) { | ||
$this->setOptions($options); | ||
} | ||
} | ||
|
||
/** | ||
* Set the format string accepted by date() to use when formatting a string | ||
* | ||
* @param string $format | ||
* @return \Zend\Filter\DateTimeFormatter | ||
*/ | ||
public function setFormat($format) | ||
{ | ||
$this->format = $format; | ||
return $this; | ||
} | ||
|
||
/** | ||
* Filter a datetime string by normalizing it to the filters specified format | ||
* | ||
* @param string $value | ||
* @return string | ||
*/ | ||
public function filter($value) | ||
{ | ||
try { | ||
$result = $this->normalizeDateTime($value); | ||
} catch (Exception $ex) { | ||
// DateTime threw an exception, an invalid date string was provided | ||
return $value; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The filter should throw the exception else it returns not normalized dates
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marc-mabe this is not the behaviour of other filters, see https://github.com/zendframework/zf2/blob/master/library/Zend/Filter/UriNormalize.php#L105 Filters should not throw exceptions or they will cause problems with InputFilters There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats interesting because other filters throw exceptions - see https://github.com/zendframework/zf2/blob/master/library/Zend/Filter/HtmlEntities.php @weierophinney What's the right way to go here? For me a filter should throw exceptions on failures else it's impossible to detect such failures (see my example above). Only implementations of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm now re throwing the blanket DateTime \Exception as a InvalidArgumentException |
||
} | ||
|
||
return $result; | ||
} | ||
|
||
/** | ||
* Attempt to convert a string into a valid DateTime object | ||
* | ||
* @param string $value | ||
* @returns DateTime | ||
* @throws Exception | ||
*/ | ||
protected function normalizeDateTime($value) | ||
{ | ||
if (is_int($value)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm missing the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
$dateTime = new DateTime('@' . $value); | ||
} else { | ||
$dateTime = new DateTime($value); | ||
} | ||
|
||
return $dateTime->format($this->format); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
<?php | ||
/** | ||
* Zend Framework (http://framework.zend.com/) | ||
* | ||
* @link http://github.com/zendframework/zf2 for the canonical source repository | ||
* @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com) | ||
* @license http://framework.zend.com/license/new-bsd New BSD License | ||
* @package Zend_Filter | ||
*/ | ||
|
||
namespace ZendTest\Filter; | ||
|
||
use DateTime; | ||
use Zend\Filter\DateTimeFormatter; | ||
|
||
/** | ||
* @category Zend | ||
* @package Zend_Filter | ||
* @subpackage UnitTests | ||
* @group Zend_Filter | ||
*/ | ||
class DateTimeFormatterTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function setUp() | ||
{ | ||
date_default_timezone_set('UTC'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we also have a few tests with other timezones? Just to ensure the code isn't fixated on UTC. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, this is potentially destructive if the timezone is already set, and other tests rely on it. I'd recommend storing the current setting, and during |
||
} | ||
|
||
public function testDateTimeFormatted() | ||
{ | ||
$filter = new DateTimeFormatter(); | ||
$result = $filter->filter('2012-01-01'); | ||
$this->assertEquals('2012-01-01T00:00:00+0000', $result); | ||
} | ||
|
||
public function testSetFormat() | ||
{ | ||
$filter = new DateTimeFormatter(); | ||
$filter->setFormat(DateTime::RFC1036); | ||
$result = $filter->filter('2012-01-01'); | ||
$this->assertEquals('Sun, 01 Jan 12 00:00:00 +0000', $result); | ||
} | ||
|
||
public function testFormatDateTimeFromTimestamp() | ||
{ | ||
$filter = new DateTimeFormatter(); | ||
$result = $filter->filter(1359739801); | ||
$this->assertEquals('2013-02-01T17:30:01+0000', $result); | ||
} | ||
|
||
public function testOriginalValueReturnedOnInvalidInput() | ||
{ | ||
$filter = new DateTimeFormatter(); | ||
$result = $filter->filter('2013-31-31'); | ||
$this->assertEquals('2013-31-31', $result); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please use an exception that is part of the Zend\Filter component? If no suitable exception exists you could create it, and have it extend the appropriate SPL exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second the recommendation from @Freeaqingme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi both, I am "using" \Exception to catch the Exception thrown by DateTime (http://www.php.net/manual/en/datetime.construct.php) which is just \Exception. Are you saying I should create an Exception in Filter\Exception that Extends \Exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I'd remove the import, and use FQCN when catching:
This will make it more clear that you actually intend to catch a global exception -- which is a rare occurrence in the framework.