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.

Sign up for GitHub

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

Jump to bottom

Fix warnings in case of incorrect data #482

Closed
wants to merge 5 commits into from
Closed

Fix warnings in case of incorrect data #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c35779
Fix warningns in case of invalid data
vkhramtsov Dec 23, 2014
59ec3bb
Add unittests for fix, some refactoring
vkhramtsov Dec 23, 2014
5bd158f
Switch to old definition for arrays
vkhramtsov Dec 23, 2014
e8dc0aa
Fix mistake in checking that something is array
vkhramtsov Dec 23, 2014
aa680c2
Code review: Throw an exception, fix unittests
vkhramtsov Dec 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/Monolog/Formatter/GelfMessageFormatter.php
View file
Open in desktop
Expand Up @@ -67,6 +67,10 @@ public function __construct($systemName = null, $extraPrefix = null, $contextPre
public function format(array $record)
{
$record = parent::format($record);
if (!$this->isInputValid($record))
{
throw new \InvalidArgumentException('Please check keys in input array');
}
$message = new Message();
$message
->setTimestamp($record['datetime'])
Expand Down Expand Up @@ -98,4 +102,24 @@ public function format(array $record)

return $message;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a DocBlock here?

Copy link
Author

Choose a reason for hiding this comment

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

Added

/**
* @param array $input
* @return bool is input valid of not
*/
private function isInputValid(array $input)
{
return (
!empty($input) &&
isset($input['datetime']) &&
isset($input['message']) &&
isset($input['channel']) &&
isset($input['level']) &&
isset($this->logLevels[$input['level']]) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand the PHP documentation in the right way, this will cause a parse error.

Copy link
Author

Choose a reason for hiding this comment

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

Actually not, at least build machine checks this code and reports that everything is ok.

isset($input['extra']) &&
is_array($input['extra']) &&
isset($input['context']) &&
is_array($input['context'])
);
}
}
45 changes: 35 additions & 10 deletions tests/Monolog/Formatter/GelfMessageFormatterTest.php
View file
Open in desktop
Expand Up @@ -15,19 +15,48 @@

class GelfMessageFormatterTest extends \PHPUnit_Framework_TestCase
{
/** @var GelfMessageFormatter */
private $formatter = null;

public function setUp()
{
if (!class_exists('\Gelf\Message')) {
$this->markTestSkipped("graylog2/gelf-php or mlehner/gelf-php is not installed");
}
$this->formatter = new GelfMessageFormatter();
}

public function formatterIncorrectInputDataProvider()
{
return array(
array(array()),
array(array('datetime' => 1)),
array(array('datetime' => 1, 'message' => 1)),
array(array('datetime' => 1, 'message' => 1, 'channel' => 1)),
array(array('datetime' => 1, 'message' => 1, 'channel' => 1, 'level' => 255)),
array(array('datetime' => 1, 'message' => 1, 'channel' => 1, 'level' => Logger::EMERGENCY)),
array(array('datetime' => 1, 'message' => 1, 'channel' => 1, 'level' => Logger::EMERGENCY, 'extra' => 1)),
array(array('datetime' => 1, 'message' => 1, 'channel' => 1, 'level' => Logger::EMERGENCY, 'extra' => array(), 'context' => 1)),
);
}

/**
* @param array $input
* @covers Monolog\Formatter\GelfMessageFormatter::format
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage Please check keys in input array
* @dataProvider formatterIncorrectInputDataProvider
*/
public function testFormatterWithIncorrectInput(array $input)
{
$this->assertEquals($input, $this->formatter->format($input));
}

/**
* @covers Monolog\Formatter\GelfMessageFormatter::format
*/
public function testDefaultFormatter()
{
$formatter = new GelfMessageFormatter();
$record = array(
'level' => Logger::ERROR,
'level_name' => 'ERROR',
Expand All @@ -38,7 +67,7 @@ public function testDefaultFormatter()
'message' => 'log',
);

$message = $formatter->format($record);
$message = $this->formatter->format($record);

$this->assertInstanceOf('Gelf\Message', $message);
$this->assertEquals(0, $message->getTimestamp());
Expand All @@ -62,7 +91,6 @@ public function testDefaultFormatter()
*/
public function testFormatWithFileAndLine()
{
$formatter = new GelfMessageFormatter();
$record = array(
'level' => Logger::ERROR,
'level_name' => 'ERROR',
Expand All @@ -73,7 +101,7 @@ public function testFormatWithFileAndLine()
'message' => 'log',
);

$message = $formatter->format($record);
$message = $this->formatter->format($record);

$this->assertInstanceOf('Gelf\Message', $message);
$this->assertEquals('test', $message->getFile());
Expand All @@ -85,7 +113,6 @@ public function testFormatWithFileAndLine()
*/
public function testFormatWithContext()
{
$formatter = new GelfMessageFormatter();
$record = array(
'level' => Logger::ERROR,
'level_name' => 'ERROR',
Expand All @@ -96,7 +123,7 @@ public function testFormatWithContext()
'message' => 'log'
);

$message = $formatter->format($record);
$message = $this->formatter->format($record);

$this->assertInstanceOf('Gelf\Message', $message);

Expand All @@ -122,7 +149,6 @@ public function testFormatWithContext()
*/
public function testFormatWithContextContainingException()
{
$formatter = new GelfMessageFormatter();
$record = array(
'level' => Logger::ERROR,
'level_name' => 'ERROR',
Expand All @@ -137,7 +163,7 @@ public function testFormatWithContextContainingException()
'message' => 'log'
);

$message = $formatter->format($record);
$message = $this->formatter->format($record);

$this->assertInstanceOf('Gelf\Message', $message);

Expand All @@ -150,7 +176,6 @@ public function testFormatWithContextContainingException()
*/
public function testFormatWithExtra()
{
$formatter = new GelfMessageFormatter();
$record = array(
'level' => Logger::ERROR,
'level_name' => 'ERROR',
Expand All @@ -161,7 +186,7 @@ public function testFormatWithExtra()
'message' => 'log'
);

$message = $formatter->format($record);
$message = $this->formatter->format($record);

$this->assertInstanceOf('Gelf\Message', $message);

Expand Down