8000 [HttpFoundation] adds getDate method to the ParameterBag class by maldoinc · Pull Request #19296 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpFoundation] adds getDate method to the ParameterBag class #19296

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions src/Symfony/Component/HttpFoundation/ParameterBag.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,39 @@ public function getBoolean($key, $default = false)
return $this->filter($key, $default, FILTER_VALIDATE_BOOLEAN);
}

/**
* Returns the parameter value converted to a DateTime object.
*
* @param string $key The parameter key
* @param string $format The expected date format
* @param \DateTimeInterface|string|null $default The default value to be converted to a DateTime object if the parameter key does not exist
* @param \DateTimeZone|null $timeZone A DateTimeZone object representing the desired time zone
*
* @return \DateTimeInterface|null
*/
public function getDate($key, $format, $default = null, \DateTimeZone $timeZone = null)
{
$time = $this->get($key, $default);

if ((null === $time) || $time instanceof \DateTimeInterface) {
Copy link
Member

Choose a reason for hiding this comment

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

(null === $time) parentheses unnecessary ?

return $time;
}

// if the user has specified a timezone then pass that
// otherwise do not even attempt to put a value but rather let the runtime decide
// the default value by itself
// this is in order to ensure compatibility with all php versions since
// some accept null as a TimeZone parameter and others do not
if (null !== $timeZone) {
$result = \DateTime::createFromFormat($format, $time, $timeZone);
Copy link
Member

Choose a reason for hiding this comment

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

If format is U, the timezone won't be applied.

} else {
$result = \DateTime::createFromFormat($format, $time);
}

// Failure to parse the date according to the specified format will return null
return false === $result ? null : $result;
Copy link
Contributor
@ro0NL ro0NL Aug 6, 2016

Choose a reason for hiding this comment

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

Just thought of this.. what about one last argument $throwOnInvalid. Basically to avoid boilerplate code in controllers, etc. (ie null checking). Defaulting to false is just fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just read @nicolas-grekas comment once more

returning null in case of parse failure looks fine to me: this is user input, ie potentially garbage

I guess your right. Letting the app error 500 based on user input is not desired. Maybe it should only throw InvalidArgumentException when $default as a string is invalid.. wdyt?

}

/**
* Filter key.
*
Expand Down
30 changes: 29 additions & 1 deletion src/Symfony/Component/HttpFoundation/Tests/ParameterBagTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,34 @@ public function testGetInt()
$this->assertEquals(0, $bag->getInt('unknown'), '->getInt() returns zero if a parameter is not defined');
}

public function testGetDate()
{
$isoDate = '2016-07-05T15:30:00UTC';
$bag = new ParameterBag(array(
'd1' => '2016-01-01',
'iso' => $isoDate,
));

$date = \DateTime::createFromFormat('Y-m-d', '2016-01-01');
$diff = $date->diff($bag->getDate('d1', 'Y-m-d'));

$this->assertEquals(0, $diff->days, '->getDate() returns a date via the format specified');
$this->assertNull($bag->getDate('d1', 'd/m/Y'), '->getDate() returns null if the format is not valid');
$this->assertNull($bag->getDate('d2', 'd/m/Y'), '->getDate() returns null if the parameter is not found');
$this->assertEquals($date->format('Ymd'), $bag->getDate('d1', 'Y-m-d', new \DateTime('2016-12-01'))->format('Ymd'), '->getDate() parses the value if the key is present');

$date = $bag->getDate('iso', \DateTime::ISO8601);
$this->assertEquals(new \DateTime($isoDate), $date);
$this->assertEquals('UTC', $date->getTimezone()->getName());

$this->assertEquals($date, $bag->getDate('nokey', \DateTime::ISO8601, $isoDate));
$this->assertInstanceOf(\DateTimeInterface::class, $bag->getDate('nokey', \DateTime::ISO8601, $date));
$this->assertNull($bag->getDate('nokey', 'd/m/Y', $isoDate), '->getDate() returns null when the default value is not in the specified format');

$tz = $bag->getDate('d1', 'Y-m-d', null, new \DateTimeZone('Europe/Tirane'))->getTimezone()->getName();
$this->assertEquals('Europe/Tirane', $tz, '->getDate() accepts a DateTimeZone object which specifies the preferred timezone');
}

public function testFilter()
{
$bag = new ParameterBag(array(
Expand All @@ -133,7 +161,7 @@ public function testFilter()
'dec' => '256',
'hex' => '0x100',
'array' => array('bang'),
));
));
Copy link
Contributor

Choose a reason for hiding this comment

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

This cs fix should be done in 2.7, not in master, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cs change was pushed accidentaly. Probably should be reverted.


$this->assertEmpty($bag->filter('nokey'), '->filter() should return empty by default if no key is found');

Expand Down
0