From e858c2c37d079f5aa8d2bf2a809c1d98240eb707 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Sat, 13 May 2017 08:59:01 +0000 Subject: [PATCH 01/16] github-38: truncate mehod for databuilder --- src/DataBuilder.php | 38 ++++++++++++++++++++++++++++++++++++++ src/RollbarLogger.php | 15 +++++++++++++-- tests/DataBuilderTest.php | 23 +++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/DataBuilder.php b/src/DataBuilder.php index 2fcf311a..c75d0a73 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -17,6 +17,15 @@ class DataBuilder implements DataBuilderInterface { + const MAX_PAYLOAD_SIZE = 524288; // 512 * 1024 + + protected static $truncationStrategies = array( + Truncation\RawStrategy::class, + Truncation\FramesStrategy::class, + Truncation\StringsStrategy::class, + Truncation\MinBodyStrategy::class + ); + protected static $defaults; protected $environment; @@ -849,4 +858,33 @@ protected static function uuid4() mt_rand(0, 0xffff) ); } + + /** + * Applies truncation strategies in order to keep the payload size under + * configured limit. + * + * @param array $payload + * @param string $strategy + * + * @return array + */ + public function truncate(array $payload) + { + + foreach (static::$truncationStrategies as $strategy) { + + if (strlen(json_encode($payload)) <= DataBuilder::MAX_PAYLOAD_SIZE) { + break; + } + + $strategy = new $strategy(); + + $payload = $strategy->execute($payload); + + } + + return $payload; + } + + } diff --git a/src/RollbarLogger.php b/src/RollbarLogger.php index 6754e854..86cbf1c1 100644 --- a/src/RollbarLogger.php +++ b/src/RollbarLogger.php @@ -45,8 +45,9 @@ public function log($level, $toLog, array $context = array()) if ($this->config->checkIgnored($payload, $accessToken, $toLog, $isUncaught)) { $response = new Response(0, "Ignored"); } else { - $scrubbed = $this->scrub($payload); - $response = $this->config->send($scrubbed, $accessToken); + $toSend = $this->scrub($payload); + $toSend = $this->truncate($toSend); + $response = $this->config->send($toSend, $accessToken); } $this->handleResponse($payload, $response); @@ -80,4 +81,14 @@ protected function scrub(Payload $payload) $serialized['data'] = $this->config->getDataBuilder()->scrub($serialized['data']); return $serialized; } + + /** + * @param array $payload + * @return array + */ + protected function truncate(array $payload) + { + return $this->config->getDataBuilder()->truncate($payload); + } + } diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index 2242437e..e18c9f3d 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -527,4 +527,27 @@ public function testSetRequestBody() stream_wrapper_restore("php"); } + + /** + * @dataProvider truncateProvider + */ + public function testTruncate($data) + { + $result = $this->dataBuilder->truncate($data); + + $size = strlen(json_encode($result)); + + $this->assertTrue($size <= DataBuilder::MAX_PAYLOAD_SIZE, "Truncation failed. Payload size exceeds MAX_PAYLOAD_SIZE."); + } + + public function truncateProvider() + { + return array( + array( // test 1 + array( + 'test' => str_repeat('A', DataBuilder::MAX_PAYLOAD_SIZE+1) + ) + ) + ); + } } From 6d40ec05d8de2d0106606796ac01cce320ac5684 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Sat, 13 May 2017 08:59:23 +0000 Subject: [PATCH 02/16] github-38: raw and frames truncation strategies --- src/Truncation/FramesStrategy.php | 36 ++++++++++++++ src/Truncation/IStrategy.php | 12 +++++ src/Truncation/MinBodyStrategy.php | 9 ++++ src/Truncation/RawStrategy.php | 9 ++++ src/Truncation/StringsStrategy.php | 9 ++++ tests/Truncation/FramesStrategyTest.php | 63 +++++++++++++++++++++++++ tests/Truncation/RawStrategyTest.php | 19 ++++++++ 7 files changed, 157 insertions(+) create mode 100644 src/Truncation/FramesStrategy.php create mode 100644 src/Truncation/IStrategy.php create mode 100644 src/Truncation/MinBodyStrategy.php create mode 100644 src/Truncation/RawStrategy.php create mode 100644 src/Truncation/StringsStrategy.php create mode 100644 tests/Truncation/FramesStrategyTest.php create mode 100644 tests/Truncation/RawStrategyTest.php diff --git a/src/Truncation/FramesStrategy.php b/src/Truncation/FramesStrategy.php new file mode 100644 index 00000000..75c6db47 --- /dev/null +++ b/src/Truncation/FramesStrategy.php @@ -0,0 +1,36 @@ +selectFrames($frames); + } + + protected function selectFrames($frames, $range = self::FRAMES_OPTIMIZATION_RANGE) + { + if (count($frames) <= $range * 2) { + return $frames; + } + + return array_merge( + array_splice($frames, 0, $range), + array_splice($frames, -$range, $range) + ); + } +} \ No newline at end of file diff --git a/src/Truncation/IStrategy.php b/src/Truncation/IStrategy.php new file mode 100644 index 00000000..78b1f892 --- /dev/null +++ b/src/Truncation/IStrategy.php @@ -0,0 +1,12 @@ +execute($data); + + $this->assertEquals($expected, $result); + } + + public function executeProvider() + { + $data = array( + 'nothing to truncate using trace key' => array( + array('data' => array('body' => + array('trace' => array('frames' => range(1,6))) + )), + range(1,6) + ), + 'nothing to truncate using trace_chain key' => array( + array('data' => array('body' => + array('trace_chain' => array('frames' => range(1,6))) + )), + range(1,6) + ), + 'truncate middle using trace key' => array( + array('data' => array('body' => + array( + 'trace' => array( + 'frames' => range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + ) + ) + )), + array_merge( + range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE), + range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + ) + ), + 'truncate middle using trace_chain key' => array( + array('data' => array('body' => + array( + 'trace_chain' => array( + 'frames' => range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + ) + ) + )), + array_merge( + range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE), + range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + ) + ) + ); + + return $data; + } +} diff --git a/tests/Truncation/RawStrategyTest.php b/tests/Truncation/RawStrategyTest.php new file mode 100644 index 00000000..14ffafa4 --- /dev/null +++ b/tests/Truncation/RawStrategyTest.php @@ -0,0 +1,19 @@ + 'test data'); + + $strategy = new RawStrategy(); + $result = $strategy->execute($payload); + + $this->assertEquals( + strlen(json_encode($payload)), + strlen(json_encode($result)) + ); + } +} From 8f4daa858e2c42aaca4c0c2dfd2d9ce457ec604c Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 11:17:46 +0000 Subject: [PATCH 03/16] github-38: put excessive payload identification logic in a method --- src/DataBuilder.php | 15 ++++- src/Truncation/AbstractStrategy.php | 16 +++++ tests/Truncation/StringsStrategyTest.php | 78 ++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 src/Truncation/AbstractStrategy.php create mode 100644 tests/Truncation/StringsStrategyTest.php diff --git a/src/DataBuilder.php b/src/DataBuilder.php index c75d0a73..ae744ea2 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -873,11 +873,11 @@ public function truncate(array $payload) foreach (static::$truncationStrategies as $strategy) { - if (strlen(json_encode($payload)) <= DataBuilder::MAX_PAYLOAD_SIZE) { + if (!$this->needsTruncating($payload)) { break; } - $strategy = new $strategy(); + $strategy = new $strategy($this); $payload = $strategy->execute($payload); @@ -886,5 +886,16 @@ public function truncate(array $payload) return $payload; } + /** + * Check if the payload is too big to be sent + * + * @param array $payload + * + * @return boolean + */ + public function needsTruncating(array $payload) + { + return strlen(json_encode($payload)) > self::MAX_PAYLOAD_SIZE; + } } diff --git a/src/Truncation/AbstractStrategy.php b/src/Truncation/AbstractStrategy.php new file mode 100644 index 00000000..e161bf29 --- /dev/null +++ b/src/Truncation/AbstractStrategy.php @@ -0,0 +1,16 @@ +databuilder = $databuilder; + } + + public function execute(array $payload) + { + return $payload; + } +} \ No newline at end of file diff --git a/tests/Truncation/StringsStrategyTest.php b/tests/Truncation/StringsStrategyTest.php new file mode 100644 index 00000000..c0f151df --- /dev/null +++ b/tests/Truncation/StringsStrategyTest.php @@ -0,0 +1,78 @@ + 'abcd1234efef5678abcd1234567890be', + 'environment' => 'tests' + )); + + $strategy = new StringsStrategy($dataBuilder); + $result = $strategy->execute($data); + + $this->assertEquals($expected, $result); + } + + public function executeProvider() + { + $data = array(); + $thresholds = StringsStrategy::getThresholds(); + + // $data["truncate nothing"] = array( + // $this->payloadStructureProvider(str_repeat("A", 10)), + // $this->payloadStructureProvider(str_repeat("A", 10)) + // ); + + // $threshold = $tresholds[0]; + // $data['truncate strings to ' . $threshold] = array( + // $this->payloadStructureProvider(str_repeat('A', DataBuilder::MAX_PAYLOAD_SIZE+1)), + // $this->payloadStructureProvider(str_repeat('A', $threshold)) + // ); + + $threshold = $thresholds[1]; + $stringLengthToTrim = $threshold+1; + + $payloadStrings = $expectedStrings = array(); + + $numStrings = floor(DataBuilder::MAX_PAYLOAD_SIZE / $stringLengthToTrim); + + $staticNoise = strlen(json_encode($this->payloadStructureProvider(array("")))); + $dynamicNoise = strlen(json_encode($this->payloadStructureProvider(array("","")))) - $staticNoise; + + for ($i = 0; $i < $numStrings; $i++) { + $payloadStrings []= str_repeat('A', $stringLengthToTrim); + $expectedStrings []= str_repeat('A', $threshold); + } + + $payload = $this->payloadStructureProvider($payloadStrings); + $expected = $this->payloadStructureProvider($expectedStrings); + + $data['truncate strings to ' . $threshold] = array($payload,$expected); + + return $data; + } + + protected function payloadStructureProvider($message) + { + return array( + "data" => array( + "body" => array( + "message" => array( + "body" => array( + "value" => $message + ) + ) + ) + ) + ); + } +} From cde6d39970921ee115a45dd5dd945a8c1e87d355 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 11:18:33 +0000 Subject: [PATCH 04/16] github-38: use abstract strategy --- src/Truncation/FramesStrategy.php | 2 +- src/Truncation/MinBodyStrategy.php | 2 +- src/Truncation/RawStrategy.php | 2 +- src/Truncation/StringsStrategy.php | 26 ++++++++++++-- tests/Truncation/FramesStrategyTest.php | 7 +++- tests/Truncation/RawStrategyTest.php | 7 +++- tests/Truncation/StringsStrategyTest.php | 45 ++++++++++-------------- 7 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/Truncation/FramesStrategy.php b/src/Truncation/FramesStrategy.php index 75c6db47..846f0a4e 100644 --- a/src/Truncation/FramesStrategy.php +++ b/src/Truncation/FramesStrategy.php @@ -1,6 +1,6 @@ databuilder->needsTruncating($payload)) { + break; + } + + array_walk_recursive($payload, function(&$value) use ($threshold) { + + if (is_string($value) && strlen($value) > $threshold) { + $value = substr($value, 0, $threshold); + } + + }); + + } + + return $payload; } } \ No newline at end of file diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index 75ddd262..f56be895 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -9,7 +9,12 @@ class FramesStrategyTest extends \PHPUnit_Framework_TestCase */ public function testExecute($data, $expected) { - $strategy = new FramesStrategy(); + $dataBuilder = new DataBuilder(array( + 'accessToken' => 'abcd1234efef5678abcd1234567890be', + 'environment' => 'tests' + )); + + $strategy = new FramesStrategy($databuilder); $result = $strategy->execute($data); $this->assertEquals($expected, $result); diff --git a/tests/Truncation/RawStrategyTest.php b/tests/Truncation/RawStrategyTest.php index 14ffafa4..8a26daa8 100644 --- a/tests/Truncation/RawStrategyTest.php +++ b/tests/Truncation/RawStrategyTest.php @@ -7,8 +7,13 @@ class RawStrategyTest extends \PHPUnit_Framework_TestCase public function testExecute() { $payload = array('test' => 'test data'); + + $dataBuilder = new DataBuilder(array( + 'accessToken' => 'abcd1234efef5678abcd1234567890be', + 'environment' => 'tests' + )); - $strategy = new RawStrategy(); + $strategy = new RawStrategy($dataBuilder); $result = $strategy->execute($payload); $this->assertEquals( diff --git a/tests/Truncation/StringsStrategyTest.php b/tests/Truncation/StringsStrategyTest.php index c0f151df..ce105a6e 100644 --- a/tests/Truncation/StringsStrategyTest.php +++ b/tests/Truncation/StringsStrategyTest.php @@ -25,40 +25,33 @@ public function testExecute($data, $expected) public function executeProvider() { $data = array(); - $thresholds = StringsStrategy::getThresholds(); - // $data["truncate nothing"] = array( - // $this->payloadStructureProvider(str_repeat("A", 10)), - // $this->payloadStructureProvider(str_repeat("A", 10)) - // ); + $data["truncate nothing"] = array( + $this->payloadStructureProvider(str_repeat("A", 10)), + $this->payloadStructureProvider(str_repeat("A", 10)) + ); - // $threshold = $tresholds[0]; - // $data['truncate strings to ' . $threshold] = array( - // $this->payloadStructureProvider(str_repeat('A', DataBuilder::MAX_PAYLOAD_SIZE+1)), - // $this->payloadStructureProvider(str_repeat('A', $threshold)) - // ); + $thresholds = StringsStrategy::getThresholds(); + foreach ($thresholds as $threshold) { + $data['truncate strings to ' . $threshold] = $this->thresholdTestProvider($threshold); + } - $threshold = $thresholds[1]; + return $data; + } + + protected function thresholdTestProvider($threshold) + { $stringLengthToTrim = $threshold+1; - $payloadStrings = $expectedStrings = array(); - - $numStrings = floor(DataBuilder::MAX_PAYLOAD_SIZE / $stringLengthToTrim); - - $staticNoise = strlen(json_encode($this->payloadStructureProvider(array("")))); - $dynamicNoise = strlen(json_encode($this->payloadStructureProvider(array("","")))) - $staticNoise; + $payload = $this->payloadStructureProvider(array()); + $expected = $this->payloadStructureProvider(array()); - for ($i = 0; $i < $numStrings; $i++) { - $payloadStrings []= str_repeat('A', $stringLengthToTrim); - $expectedStrings []= str_repeat('A', $threshold); + while (strlen(json_encode($payload)) < DataBuilder::MAX_PAYLOAD_SIZE) { + $payload['data']['body']['message']['body']['value'] []= str_repeat('A', $stringLengthToTrim); + $expected['data']['body']['message']['body']['value'] []= str_repeat('A', $threshold); } - $payload = $this->payloadStructureProvider($payloadStrings); - $expected = $this->payloadStructureProvider($expectedStrings); - - $data['truncate strings to ' . $threshold] = array($payload,$expected); - - return $data; + return array($payload,$expected); } protected function payloadStructureProvider($message) From 325eb9b8d7dfd7c5081f5b2ca4c494a0f19e043c Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 13:36:49 +0000 Subject: [PATCH 05/16] github-38: min body strategy --- src/Truncation/FramesStrategy.php | 2 +- src/Truncation/MinBodyStrategy.php | 42 +++++++++++++++- tests/Truncation/MinBodyStrategyTest.php | 64 ++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/Truncation/MinBodyStrategyTest.php diff --git a/src/Truncation/FramesStrategy.php b/src/Truncation/FramesStrategy.php index 846f0a4e..883bb4e3 100644 --- a/src/Truncation/FramesStrategy.php +++ b/src/Truncation/FramesStrategy.php @@ -22,7 +22,7 @@ public function execute(array $payload) return $this->selectFrames($frames); } - protected function selectFrames($frames, $range = self::FRAMES_OPTIMIZATION_RANGE) + public function selectFrames($frames, $range = self::FRAMES_OPTIMIZATION_RANGE) { if (count($frames) <= $range * 2) { return $frames; diff --git a/src/Truncation/MinBodyStrategy.php b/src/Truncation/MinBodyStrategy.php index 2eb72009..a6eea553 100644 --- a/src/Truncation/MinBodyStrategy.php +++ b/src/Truncation/MinBodyStrategy.php @@ -2,8 +2,48 @@ class MinBodyStrategy extends AbstractStrategy { + + const EXCEPTION_MESSAGE_LIMIT = 256; + const EXCEPTION_FRAMES_RANGE = 1; + public function execute(array $payload) { - throw new \Exception('implement MinBodyStrategy::execute'); + + $traceData = null; + + if (isset($payload['data']['body']['trace'])) { + + $traceData = &$payload['data']['body']['trace']; + + } elseif (isset($payload['data']['body']['trace_chain'])) { + + $traceData = &$payload['data']['body']['trace_chain']; + + } + + /** + * Delete exception description + */ + unset($traceData['exception']['description']); + + /** + * Truncate exception message + */ + $traceData['exception']['message'] = substr( + $traceData['exception']['message'], + 0, + static::EXCEPTION_MESSAGE_LIMIT + ); + + /** + * Limit trace frames + */ + $framesStrategy = new FramesStrategy($this->databuilder); + $traceData['frames'] = $framesStrategy->selectFrames( + $traceData['frames'], + static::EXCEPTION_FRAMES_RANGE + ); + + return $payload; } } \ No newline at end of file diff --git a/tests/Truncation/MinBodyStrategyTest.php b/tests/Truncation/MinBodyStrategyTest.php new file mode 100644 index 00000000..94f1672c --- /dev/null +++ b/tests/Truncation/MinBodyStrategyTest.php @@ -0,0 +1,64 @@ + 'abcd1234efef5678abcd1234567890be', + 'environment' => 'tests' + )); + + $strategy = new MinBodyStrategy($dataBuilder); + $result = $strategy->execute($data); + + $this->assertEquals($expected, $result); + } + + public function executeProvider() + { + $data = array(); + + $traceData = array( + 'exception' => array( + 'description' => 'Test description', + 'message' => str_repeat('A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT+1) + ), + 'frames' => array('Frame 1', 'Frame 2', 'Frame 3') + ); + + $expected = $traceData; + unset($expected['exception']['description']); + $expected['exception']['message'] = str_repeat('A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT); + $expected['frames'] = array('Frame 1', 'Frame 3'); + + + $data['trace data set'] = array( + $this->payloadStructureProvider(array('trace' => $traceData)), + $this->payloadStructureProvider(array('trace' => $expected)) + ); + + $data['trace_chain data set'] = array( + $this->payloadStructureProvider(array('trace_chain' => $traceData)), + $this->payloadStructureProvider(array('trace_chain' => $expected)) + ); + return $data; + } + + protected function payloadStructureProvider($body) + { + return array( + "data" => array( + "body" => $body + ) + ); + } +} From af58a72ddee4196602a5793e0a9ab93b51bb5c9a Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 14:11:53 +0000 Subject: [PATCH 06/16] github-38: add use DataBuilder and change property name to follow convention --- src/Truncation/AbstractStrategy.php | 6 +++--- src/Truncation/MinBodyStrategy.php | 2 +- src/Truncation/StringsStrategy.php | 2 +- tests/Truncation/FramesStrategyTest.php | 2 ++ tests/Truncation/RawStrategyTest.php | 2 ++ 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Truncation/AbstractStrategy.php b/src/Truncation/AbstractStrategy.php index e161bf29..c6283ce4 100644 --- a/src/Truncation/AbstractStrategy.php +++ b/src/Truncation/AbstractStrategy.php @@ -2,11 +2,11 @@ class AbstractStrategy implements IStrategy { - protected $databuilder; + protected $dataBuilder; - public function __construct($databuilder) + public function __construct($dataBuilder) { - $this->databuilder = $databuilder; + $this->dataBuilder = $dataBuilder; } public function execute(array $payload) diff --git a/src/Truncation/MinBodyStrategy.php b/src/Truncation/MinBodyStrategy.php index a6eea553..d37a7a35 100644 --- a/src/Truncation/MinBodyStrategy.php +++ b/src/Truncation/MinBodyStrategy.php @@ -38,7 +38,7 @@ public function execute(array $payload) /** * Limit trace frames */ - $framesStrategy = new FramesStrategy($this->databuilder); + $framesStrategy = new FramesStrategy($this->dataBuilder); $traceData['frames'] = $framesStrategy->selectFrames( $traceData['frames'], static::EXCEPTION_FRAMES_RANGE diff --git a/src/Truncation/StringsStrategy.php b/src/Truncation/StringsStrategy.php index 1dc87617..d4da6437 100644 --- a/src/Truncation/StringsStrategy.php +++ b/src/Truncation/StringsStrategy.php @@ -12,7 +12,7 @@ public function execute(array $payload) { foreach (static::getThresholds() as $threshold) { - if (!$this->databuilder->needsTruncating($payload)) { + if (!$this->dataBuilder->needsTruncating($payload)) { break; } diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index f56be895..cf77b1fd 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -2,6 +2,8 @@ namespace Rollbar\Truncation; +use Rollbar\DataBuilder; + class FramesStrategyTest extends \PHPUnit_Framework_TestCase { /** diff --git a/tests/Truncation/RawStrategyTest.php b/tests/Truncation/RawStrategyTest.php index 8a26daa8..65c7c7d6 100644 --- a/tests/Truncation/RawStrategyTest.php +++ b/tests/Truncation/RawStrategyTest.php @@ -2,6 +2,8 @@ namespace Rollbar\Truncation; +use Rollbar\DataBuilder; + class RawStrategyTest extends \PHPUnit_Framework_TestCase { public function testExecute() From 2c189c091fb9506225be3ac6e1fb83663c79cec2 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 14:12:31 +0000 Subject: [PATCH 07/16] github-38: composer fix --- src/DataBuilder.php | 13 +++++-------- src/RollbarLogger.php | 1 - src/Truncation/AbstractStrategy.php | 2 +- src/Truncation/FramesStrategy.php | 6 +----- src/Truncation/IStrategy.php | 3 +-- src/Truncation/MinBodyStrategy.php | 10 +++------- src/Truncation/RawStrategy.php | 2 +- src/Truncation/StringsStrategy.php | 7 ++----- tests/Truncation/FramesStrategyTest.php | 16 ++++++++-------- 9 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/DataBuilder.php b/src/DataBuilder.php index ae744ea2..8ce52cae 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -862,17 +862,16 @@ protected static function uuid4() /** * Applies truncation strategies in order to keep the payload size under * configured limit. - * + * * @param array $payload * @param string $strategy - * + * * @return array */ public function truncate(array $payload) { foreach (static::$truncationStrategies as $strategy) { - if (!$this->needsTruncating($payload)) { break; } @@ -880,7 +879,6 @@ public function truncate(array $payload) $strategy = new $strategy($this); $payload = $strategy->execute($payload); - } return $payload; @@ -888,14 +886,13 @@ public function truncate(array $payload) /** * Check if the payload is too big to be sent - * + * * @param array $payload - * + * * @return boolean */ public function needsTruncating(array $payload) { - return strlen(json_encode($payload)) > self::MAX_PAYLOAD_SIZE; + return strlen(json_encode($payload)) > self::MAX_PAYLOAD_SIZE; } - } diff --git a/src/RollbarLogger.php b/src/RollbarLogger.php index 86cbf1c1..d7d5edbb 100644 --- a/src/RollbarLogger.php +++ b/src/RollbarLogger.php @@ -90,5 +90,4 @@ protected function truncate(array $payload) { return $this->config->getDataBuilder()->truncate($payload); } - } diff --git a/src/Truncation/AbstractStrategy.php b/src/Truncation/AbstractStrategy.php index c6283ce4..7eb371c1 100644 --- a/src/Truncation/AbstractStrategy.php +++ b/src/Truncation/AbstractStrategy.php @@ -13,4 +13,4 @@ public function execute(array $payload) { return $payload; } -} \ No newline at end of file +} diff --git a/src/Truncation/FramesStrategy.php b/src/Truncation/FramesStrategy.php index 883bb4e3..9f671f38 100644 --- a/src/Truncation/FramesStrategy.php +++ b/src/Truncation/FramesStrategy.php @@ -10,13 +10,9 @@ public function execute(array $payload) $frames = array(); if (isset($payload['data']['body']['trace_chain']['frames'])) { - $frames = $payload['data']['body']['trace_chain']['frames']; - } elseif (isset($payload['data']['body']['trace']['frames'])) { - $frames = $payload['data']['body']['trace']['frames']; - } return $this->selectFrames($frames); @@ -33,4 +29,4 @@ public function selectFrames($frames, $range = self::FRAMES_OPTIMIZATION_RANGE) array_splice($frames, -$range, $range) ); } -} \ No newline at end of file +} diff --git a/src/Truncation/IStrategy.php b/src/Truncation/IStrategy.php index 78b1f892..7c4dbe2b 100644 --- a/src/Truncation/IStrategy.php +++ b/src/Truncation/IStrategy.php @@ -8,5 +8,4 @@ interface IStrategy * @return array */ public function execute(array $payload); - -} \ No newline at end of file +} diff --git a/src/Truncation/MinBodyStrategy.php b/src/Truncation/MinBodyStrategy.php index d37a7a35..4b9c9f12 100644 --- a/src/Truncation/MinBodyStrategy.php +++ b/src/Truncation/MinBodyStrategy.php @@ -12,13 +12,9 @@ public function execute(array $payload) $traceData = null; if (isset($payload['data']['body']['trace'])) { - $traceData = &$payload['data']['body']['trace']; - } elseif (isset($payload['data']['body']['trace_chain'])) { - $traceData = &$payload['data']['body']['trace_chain']; - } /** @@ -30,8 +26,8 @@ public function execute(array $payload) * Truncate exception message */ $traceData['exception']['message'] = substr( - $traceData['exception']['message'], - 0, + $traceData['exception']['message'], + 0, static::EXCEPTION_MESSAGE_LIMIT ); @@ -46,4 +42,4 @@ public function execute(array $payload) return $payload; } -} \ No newline at end of file +} diff --git a/src/Truncation/RawStrategy.php b/src/Truncation/RawStrategy.php index dbff75d8..f34782ad 100644 --- a/src/Truncation/RawStrategy.php +++ b/src/Truncation/RawStrategy.php @@ -6,4 +6,4 @@ public function execute(array $payload) { return $payload; } -} \ No newline at end of file +} diff --git a/src/Truncation/StringsStrategy.php b/src/Truncation/StringsStrategy.php index d4da6437..aaee1d91 100644 --- a/src/Truncation/StringsStrategy.php +++ b/src/Truncation/StringsStrategy.php @@ -11,21 +11,18 @@ public static function getThresholds() public function execute(array $payload) { foreach (static::getThresholds() as $threshold) { - if (!$this->dataBuilder->needsTruncating($payload)) { break; } - array_walk_recursive($payload, function(&$value) use ($threshold) { + array_walk_recursive($payload, function (&$value) use ($threshold) { if (is_string($value) && strlen($value) > $threshold) { $value = substr($value, 0, $threshold); } - }); - } return $payload; } -} \ No newline at end of file +} diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index cf77b1fd..4bf0696f 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -27,26 +27,26 @@ public function executeProvider() $data = array( 'nothing to truncate using trace key' => array( array('data' => array('body' => - array('trace' => array('frames' => range(1,6))) + array('trace' => array('frames' => range(1, 6))) )), - range(1,6) + range(1, 6) ), 'nothing to truncate using trace_chain key' => array( array('data' => array('body' => - array('trace_chain' => array('frames' => range(1,6))) + array('trace_chain' => array('frames' => range(1, 6))) )), - range(1,6) + range(1, 6) ), 'truncate middle using trace key' => array( array('data' => array('body' => array( 'trace' => array( - 'frames' => range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + 'frames' => range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) ) ) )), array_merge( - range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE), + range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) ) ), @@ -54,12 +54,12 @@ public function executeProvider() array('data' => array('body' => array( 'trace_chain' => array( - 'frames' => range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + 'frames' => range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) ) ) )), array_merge( - range(1,FramesStrategy::FRAMES_OPTIMIZATION_RANGE), + range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) ) ) From 9c4618185e91d7b74f880e7353844e86d5709cb9 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:21:48 +0000 Subject: [PATCH 08/16] github-38: cut in half the frames optimization range --- src/Truncation/FramesStrategy.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Truncation/FramesStrategy.php b/src/Truncation/FramesStrategy.php index 9f671f38..78feabcf 100644 --- a/src/Truncation/FramesStrategy.php +++ b/src/Truncation/FramesStrategy.php @@ -3,7 +3,7 @@ class FramesStrategy extends AbstractStrategy { - const FRAMES_OPTIMIZATION_RANGE = 150; + const FRAMES_OPTIMIZATION_RANGE = 75; public function execute(array $payload) { From 20356f7eac637b65fbf58ee3573852cf01d46970 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:23:29 +0000 Subject: [PATCH 09/16] github-38: fix a typo --- tests/Truncation/FramesStrategyTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index 4bf0696f..327d39fa 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -16,7 +16,7 @@ public function testExecute($data, $expected) 'environment' => 'tests' )); - $strategy = new FramesStrategy($databuilder); + $strategy = new FramesStrategy($dataBuilder); $result = $strategy->execute($data); $this->assertEquals($expected, $result); From 7139935bb1bd9825f29b5f65f89f04303b0e014a Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:36:32 +0000 Subject: [PATCH 10/16] github-38: clean up long lines --- tests/Truncation/FramesStrategyTest.php | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index 327d39fa..e6bd88e5 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -41,26 +41,38 @@ public function executeProvider() array('data' => array('body' => array( 'trace' => array( - 'frames' => range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + 'frames' => range( + 1, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 + ) ) ) )), array_merge( range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), - range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + range( + FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 + ) ) ), 'truncate middle using trace_chain key' => array( array('data' => array('body' => array( 'trace_chain' => array( - 'frames' => range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + 'frames' => range( + 1, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 + ) ) ) )), array_merge( range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), - range(FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1) + range( + FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 + ) ) ) ); From 8b1a797a4cd649c2fef7f6cc0b368c5237f9ef7f Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:37:40 +0000 Subject: [PATCH 11/16] github-38: clean up long lines --- tests/Truncation/MinBodyStrategyTest.php | 10 ++++++++-- tests/Truncation/StringsStrategyTest.php | 9 +++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/Truncation/MinBodyStrategyTest.php b/tests/Truncation/MinBodyStrategyTest.php index 94f1672c..f24e8a5d 100644 --- a/tests/Truncation/MinBodyStrategyTest.php +++ b/tests/Truncation/MinBodyStrategyTest.php @@ -30,14 +30,20 @@ public function executeProvider() $traceData = array( 'exception' => array( 'description' => 'Test description', - 'message' => str_repeat('A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT+1) + 'message' => str_repeat( + 'A', + MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT+1 + ) ), 'frames' => array('Frame 1', 'Frame 2', 'Frame 3') ); $expected = $traceData; unset($expected['exception']['description']); - $expected['exception']['message'] = str_repeat('A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT); + $expected['exception']['message'] = str_repeat( + 'A', + MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT + ); $expected['frames'] = array('Frame 1', 'Frame 3'); diff --git a/tests/Truncation/StringsStrategyTest.php b/tests/Truncation/StringsStrategyTest.php index ce105a6e..f18eceee 100644 --- a/tests/Truncation/StringsStrategyTest.php +++ b/tests/Truncation/StringsStrategyTest.php @@ -47,8 +47,13 @@ protected function thresholdTestProvider($threshold) $expected = $this->payloadStructureProvider(array()); while (strlen(json_encode($payload)) < DataBuilder::MAX_PAYLOAD_SIZE) { - $payload['data']['body']['message']['body']['value'] []= str_repeat('A', $stringLengthToTrim); - $expected['data']['body']['message']['body']['value'] []= str_repeat('A', $threshold); + + $payload['data']['body']['message']['body']['value'] []= + str_repeat('A', $stringLengthToTrim); + + $expected['data']['body']['message']['body']['value'] []= + str_repeat('A', $threshold); + } return array($payload,$expected); From 4d966a1742751e28bce7d8b13b7dfe416d9bba90 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:46:07 +0000 Subject: [PATCH 12/16] github-38: use strings instead of ::class for 5.3 compatibility --- src/DataBuilder.php | 8 ++++---- tests/Truncation/FramesStrategyTest.php | 6 +++--- tests/Truncation/MinBodyStrategyTest.php | 4 ++-- tests/Truncation/StringsStrategyTest.php | 6 ++---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/DataBuilder.php b/src/DataBuilder.php index 8ce52cae..0b1e582e 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -20,10 +20,10 @@ class DataBuilder implements DataBuilderInterface const MAX_PAYLOAD_SIZE = 524288; // 512 * 1024 protected static $truncationStrategies = array( - Truncation\RawStrategy::class, - Truncation\FramesStrategy::class, - Truncation\StringsStrategy::class, - Truncation\MinBodyStrategy::class + "Rollbar\Truncation\RawStrategy", + "Rollbar\Truncation\FramesStrategy", + "Rollbar\Truncation\StringsStrategy", + "Rollbar\Truncation\MinBodyStrategy" ); protected static $defaults; diff --git a/tests/Truncation/FramesStrategyTest.php b/tests/Truncation/FramesStrategyTest.php index e6bd88e5..be1b4758 100644 --- a/tests/Truncation/FramesStrategyTest.php +++ b/tests/Truncation/FramesStrategyTest.php @@ -51,7 +51,7 @@ public function executeProvider() array_merge( range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), range( - FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 ) ) @@ -61,7 +61,7 @@ public function executeProvider() array( 'trace_chain' => array( 'frames' => range( - 1, + 1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 ) ) @@ -70,7 +70,7 @@ public function executeProvider() array_merge( range(1, FramesStrategy::FRAMES_OPTIMIZATION_RANGE), range( - FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, + FramesStrategy::FRAMES_OPTIMIZATION_RANGE + 2, FramesStrategy::FRAMES_OPTIMIZATION_RANGE * 2 + 1 ) ) diff --git a/tests/Truncation/MinBodyStrategyTest.php b/tests/Truncation/MinBodyStrategyTest.php index f24e8a5d..de708305 100644 --- a/tests/Truncation/MinBodyStrategyTest.php +++ b/tests/Truncation/MinBodyStrategyTest.php @@ -31,7 +31,7 @@ public function executeProvider() 'exception' => array( 'description' => 'Test description', 'message' => str_repeat( - 'A', + 'A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT+1 ) ), @@ -41,7 +41,7 @@ public function executeProvider() $expected = $traceData; unset($expected['exception']['description']); $expected['exception']['message'] = str_repeat( - 'A', + 'A', MinBodyStrategy::EXCEPTION_MESSAGE_LIMIT ); $expected['frames'] = array('Frame 1', 'Frame 3'); diff --git a/tests/Truncation/StringsStrategyTest.php b/tests/Truncation/StringsStrategyTest.php index f18eceee..ee20322f 100644 --- a/tests/Truncation/StringsStrategyTest.php +++ b/tests/Truncation/StringsStrategyTest.php @@ -47,13 +47,11 @@ protected function thresholdTestProvider($threshold) $expected = $this->payloadStructureProvider(array()); while (strlen(json_encode($payload)) < DataBuilder::MAX_PAYLOAD_SIZE) { - - $payload['data']['body']['message']['body']['value'] []= + $payload['data']['body']['message']['body']['value'] []= str_repeat('A', $stringLengthToTrim); - $expected['data']['body']['message']['body']['value'] []= + $expected['data']['body']['message']['body']['value'] []= str_repeat('A', $threshold); - } return array($payload,$expected); From 479603247bffda825490e90d7a9b6fe9be4a9fc3 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:52:37 +0000 Subject: [PATCH 13/16] github-38: add test cases to testTruncate --- tests/DataBuilderTest.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index e18c9f3d..bd07cc9a 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -4,6 +4,7 @@ use Rollbar\Payload\Level; use Rollbar\TestHelpers\MockPhpStream; +use Rollbar\Truncation\FramesStrategy; class DataBuilderTest extends \PHPUnit_Framework_TestCase { @@ -542,12 +543,26 @@ public function testTruncate($data) public function truncateProvider() { - return array( - array( // test 1 + + $stringsTest = new Truncation\StringsStrategyTest(); + $framesTest = new Truncation\StringsStrategyTest(); + $minBodyTest = new Truncation\StringsStrategyTest(); + + $data = array( + 'string truncation' => array( // test 1 array( 'test' => str_repeat('A', DataBuilder::MAX_PAYLOAD_SIZE+1) ) ) ); + + $data = array_merge( + $data, + $stringsTest->executeProvider(), + $framesTest->executeProvider(), + $minBodyTest->executeProvider() + ); + + return $data; } } From e0ed7d729a1eb79504e25fe1b939b0e645d72838 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:54:36 +0000 Subject: [PATCH 14/16] github-38: clean up long lines --- tests/DataBuilderTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index bd07cc9a..e8579c78 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -538,7 +538,10 @@ public function testTruncate($data) $size = strlen(json_encode($result)); - $this->assertTrue($size <= DataBuilder::MAX_PAYLOAD_SIZE, "Truncation failed. Payload size exceeds MAX_PAYLOAD_SIZE."); + $this->assertTrue + $size <= DataBuilder::MAX_PAYLOAD_SIZE, + "Truncation failed. Payload size exceeds MAX_PAYLOAD_SIZE." + ); } public function truncateProvider() From 8983d9a2fdaa8f2c3b6ff9e9b354b5f1ac96d159 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 22:55:05 +0000 Subject: [PATCH 15/16] github-38: composer fix --- tests/DataBuilderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index e8579c78..a2149548 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -539,7 +539,7 @@ public function testTruncate($data) $size = strlen(json_encode($result)); $this->assertTrue - $size <= DataBuilder::MAX_PAYLOAD_SIZE, + $size <= DataBuilder::MAX_PAYLOAD_SIZE, "Truncation failed. Payload size exceeds MAX_PAYLOAD_SIZE." ); } From 4721b15acc25ca7a4ebfff9c65abb24c8d95f12b Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Tue, 16 May 2017 23:00:25 +0000 Subject: [PATCH 16/16] github-38: fix typos --- tests/DataBuilderTest.php | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index a2149548..7218f2f3 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -538,7 +538,7 @@ public function testTruncate($data) $size = strlen(json_encode($result)); - $this->assertTrue + $this->assertTrue( $size <= DataBuilder::MAX_PAYLOAD_SIZE, "Truncation failed. Payload size exceeds MAX_PAYLOAD_SIZE." ); @@ -548,19 +548,10 @@ public function truncateProvider() { $stringsTest = new Truncation\StringsStrategyTest(); - $framesTest = new Truncation\StringsStrategyTest(); - $minBodyTest = new Truncation\StringsStrategyTest(); - - $data = array( - 'string truncation' => array( // test 1 - array( - 'test' => str_repeat('A', DataBuilder::MAX_PAYLOAD_SIZE+1) - ) - ) - ); + $framesTest = new Truncation\FramesStrategyTest(); + $minBodyTest = new Truncation\MinBodyStrategyTest(); $data = array_merge( - $data, $stringsTest->executeProvider(), $framesTest->executeProvider(), $minBodyTest->executeProvider()