Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Timestamp log filter #6058

Closed
91 changes: 91 additions & 0 deletions library/Zend/Log/Filter/Timestamp.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php
namespace Zend\Log\Filter;

use Zend\Log\Exception;
use DateTime;

/**
* Filters log events based on the time when they were triggered.
*
* @author Nikola Posa <posa.nikola@gmail.com>
*/
class Timestamp implements FilterInterface
{
/**
* DateTime instance or desired value based on $dateFormatChar.
*
* @var int|DateTime
*/
protected $value;

/**
* PHP idate()-compliant format character.
*
* @var string|null
*/
protected $dateFormatChar = null;

/**
* @var string
*/
protected $operator;

/**
* @param int|DateTime|array|Traversable $value DateTime instance or desired value based on $dateFormatChar
* @param string $dateFormatChar PHP idate()-compliant format character
* @param string $operator Comparison operator
* @return Timestamp
* @throws Exception\InvalidArgumentException
*/
public function __construct($value, $dateFormatChar = null, $operator = null)
{
if ($value instanceof Traversable) {
$value = iterator_to_array($value);
}
if (is_array($value)) {
extract($value, EXTR_IF_EXISTS);
Copy link
Member

Choose a reason for hiding this comment

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

Please do not use extract; test for values in the array instead, and set the locally declared variables based on what you find in the array.

}

if ($value instanceof DateTime) {
$this->value = $value;
} else {
if (!is_int($value)) {
throw new Exception\InvalidArgumentException(sprintf(
'Value must be either DateTime instance or integer; received "%s"',
gettype($value)
));
} elseif (!is_string($dateFormatChar)) {
Copy link
Member

Choose a reason for hiding this comment

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

elseif can be avoided here.

throw new Exception\InvalidArgumentException(sprintf(
'Date format character must be supplied as string; received "%s"',
gettype($dateFormatChar)
));
}
$this->value = $value;
$this->dateFormatChar = $dateFormatChar;
}

$this->operator = ($operator) ?: '<=';
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the operator be validated?

Copy link
Author

Choose a reason for hiding this comment

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

I frankly haven't seen any validation on $operator in Zend\Log\Filter\Priority, which is also based on the version_compare() function. Do you think that I should at least add some type check (is string)?

Copy link
Member

Choose a reason for hiding this comment

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

@nikolaposa what are the allowed operator here? The PHP docs mention The possible operators are: <, lt, <=, le, >, gt, >=, ge, ==, =, eq, !=, <>, ne respectively..

The fact that the priority filter doesn't have strict checks doesn't mean that we can't apply them here.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, no problem, I'll put some validation there.

Copy link
Member

Choose a reason for hiding this comment

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

The ternary is unnecessary due to the previous in_array() test. If you want to allow a default value, the conditional starting on line 77 should first test !empty($operator) && ....

}

/**
* Returns TRUE if timestamp is accepted, otherwise FALSE is returned.
*
* @param array $event event data
* @return bool
*/
public function filter(array $event)
{
$datetime = $event['timestamp'];
$timestamp = $datetime->getTimestamp();

if ($this->value instanceof DateTime) {
return version_compare((string) $timestamp, (string) $this->value->getTimestamp(), $this->operator);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need else, just return directly because already return early in previous if

Copy link
Author

Choose a reason for hiding this comment

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

I do understand your point, but I prefer to use explicit else in these kind of situations, in order to "highlight" that two different strategies/directions. On the other hand, in case of raw boolean return statements, I use that "no else" approach, with that last, fallback return statement at the very end of a method declaration.

But if you insist, I'll alter this...

Copy link
Contributor

Choose a reason for hiding this comment

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

I already did a lot of work at the pass for removing else because already return early, see #3286

return version_compare(
idate($this->dateFormatChar, $timestamp),
$this->value,
$this->operator
);
}
}
}
99 changes: 99 additions & 0 deletions tests/ZendTest/Log/Filter/TimestampTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php
namespace ZendTest\Log\Filter;

use PHPUnit_Framework_TestCase as TestCase;
use Zend\Log\Filter\Timestamp as TimestampFilter;

/**
* @group Zend_Log
*
* @author Nikola Posa <posa.nikola@gmail.com>
*/
class TimestampTest extends TestCase
{
/**
* @dataProvider dateTimeDataProvider
*/
public function testComparisonWhenValueIsSuppliedAsDateTimeObject($timestamp, $dateTimeValue, $operator, $expectation)
{
$filter = new TimestampFilter($dateTimeValue, null, $operator);

$result = $filter->filter(array('timestamp' => $timestamp));

if ($expectation === true) {
$this->assertTrue($result);
} else {
$this->assertFalse($result);
}
}

public function dateTimeDataProvider()
{
return array(
array(new \DateTime('2014-03-03'), new \DateTime('2014-03-03'), '>=', true),
array(new \DateTime('2014-10-10'), new \DateTime('2014-03-03'),'>=', true),
array(new \DateTime('2013-03-03'), new \DateTime('2014-03-03'), '>=', false),
array(new \DateTime('2014-03-03'), new \DateTime('2014-03-03'), '==', true),
array(new \DateTime('2014-02-02'), new \DateTime('2014-03-03'), '<', true),
array(new \DateTime('2014-03-03'), new \DateTime('2014-03-03'), '<', false)
);
Copy link
Member

Choose a reason for hiding this comment

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

You should add some data sets that use the string versions of comparison operators as well (e.g., 'gt', 'lt', etc.).

}

/**
* @dataProvider datePartDataProvider
*/
public function testComparisonWhenValueIsSuppliedAsDatePartValue($timestamp, $datePartVal, $datePartChar, $operator, $expectation)
{
$filter = new TimestampFilter($datePartVal, $datePartChar, $operator);

$result = $filter->filter(array('timestamp' => $timestamp));

if ($expectation === true) {
$this->assertTrue($result);
} else {
$this->assertFalse($result);
}
}

public function datePartDataProvider()
{
return array(
array(new \DateTime('2014-03-03 10:15:00'), 10, 'H', '==', true),
array(new \DateTime('2013-03-03 22:00:00'), 10, 'H', '==', false),
array(new \DateTime('2014-03-04 10:15:00'), 3, 'd', '>', true),
array(new \DateTime('2014-03-04 10:15:00'), 10, 'd', '<', true),
array(new \DateTime('2014-03-03 10:15:00'), 1, 'm', '==', false),
array(new \DateTime('2014-03-03 10:15:00'), 2, 'm', '>=', true),
);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as for line 47.

}

/**
* @expectedException Zend\Log\Exception\InvalidArgumentException
*/
public function testConstructorThrowsOnInvalidValue()
{
new TimestampFilter('foo');
}

/**
* @expectedException Zend\Log\Exception\InvalidArgumentException
*/
public function testConstructorThrowsWhenDateFormatCharIsMissing()
{
new TimestampFilter(3);
}

public function testFilterCreatedFromArray()
{
$config = array(
'value' => 10,
'dateFormatChar' => 'm',
'operator' => '==',
);
$filter = new TimestampFilter($config);

$this->assertAttributeEquals($config['value'], 'value', $filter);
$this->assertAttributeEquals($config['dateFormatChar'], 'dateFormatChar', $filter);
$this->assertAttributeEquals($config['operator'], 'operator', $filter);
}
}