From 524ab12a87cd5873a9d1fcf3b227c3f8b22ed66f Mon Sep 17 00:00:00 2001 From: javer Date: Tue, 31 Jul 2018 02:57:10 +0300 Subject: [PATCH 1/3] Add Symfony 4 support and ability to configure all Rollbar parameters * Add Symfony 4 support * Get default values for bundle configuration from Rollbar library * Allow using non-scalar values in configuration * Removed unused uses * Removed FQCN references * Added phpdoc to methods * Configured travis to test with Symfony 3.4, 4.0, 4.1 --- .travis.yml | 32 ++++- DependencyInjection/Configuration.php | 66 +++++----- DependencyInjection/RollbarExtension.php | 17 +-- EventListener/AbstractListener.php | 42 +++--- EventListener/ErrorListener.php | 22 ++-- EventListener/ExceptionListener.php | 15 ++- Factories/RollbarHandlerFactory.php | 56 ++++---- Payload/ErrorItem.php | 14 +- Payload/Generator.php | 47 ++++--- Payload/TraceChain.php | 7 + Payload/TraceItem.php | 7 + RollbarBundle.php | 1 + .../DependencyInjection/ConfigurationTest.php | 49 +++---- .../RollbarExtensionTest.php | 30 +++-- Tests/EventListener/ErrorListenerTest.php | 122 +++++++++++------- Tests/EventListener/ExceptionListenerTest.php | 30 +++-- Tests/Fixtures/ErrorHandler.php | 19 ++- Tests/Fixtures/app/autoload.php | 11 -- Tests/Fixtures/app/config/config_test.yml | 8 +- Tests/Payload/ErrorItemTest.php | 16 ++- Tests/Payload/GeneratorTest.php | 118 ++++++++++------- Tests/Payload/TraceChainTest.php | 9 +- Tests/Payload/TraceItemTest.php | 8 +- Tests/RollbarBundleTest.php | 39 +++--- composer.json | 8 +- phpunit.xml | 2 +- 26 files changed, 487 insertions(+), 308 deletions(-) delete mode 100755 Tests/Fixtures/app/autoload.php diff --git a/.travis.yml b/.travis.yml index 442254f..acaf364 100755 --- a/.travis.yml +++ b/.travis.yml @@ -4,14 +4,38 @@ php: - 5.6 - 7.0 - 7.1 + - 7.2 + +env: + - SYMFONY_VERSION=3.4.* + - SYMFONY_VERSION=4.0.* + - SYMFONY_VERSION=4.1.* + +sudo: false +dist: trusty + +cache: + directories: + - $HOME/.composer/cache/files + +matrix: + exclude: + - php: 5.6 + env: SYMFONY_VERSION=4.0.* + - php: 5.6 + env: SYMFONY_VERSION=4.1.* + - php: 7.0 + env: SYMFONY_VERSION=4.0.* + - php: 7.0 + env: SYMFONY_VERSION=4.1.* before_install: - - curl -s https://getcomposer.org/installer | php - - php composer.phar install + - composer self-update + - composer require -n --prefer-dist "symfony/symfony:${SYMFONY_VERSION}" script: - php -v - - php composer.phar test + - composer test after_success: - - bash <(curl -s https://codecov.io/bash) + - bash <(curl -s https://codecov.io/bash) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index aa9b734..aff603f 100755 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -2,59 +2,61 @@ namespace Rollbar\Symfony\RollbarBundle\DependencyInjection; +use Rollbar\Config; +use Rollbar\Defaults; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; /** * Class Configuration + * * @link https://rollbar.com/docs/notifier/rollbar-php/#configuration-reference + * * @package Rollbar\Symfony\RollbarBundle\DependencyInjection */ class Configuration implements ConfigurationInterface { /** - * @inheritdoc + * {@inheritdoc} */ public function getConfigTreeBuilder() { $treeBuilder = new TreeBuilder(); $rollbarConfigNode = $treeBuilder->root(RollbarExtension::ALIAS); + $rollbarConfigNodeChildren = $rollbarConfigNode->children(); - // the intendation in this method reflects the structure of the rootNode - // for convenience - - foreach (\Rollbar\Config::listOptions() as $option) { - // TODO: this is duplicated code from - // https://github.com/rollbar/rollbar-php-wordpress/blob/master/src/Plugin.php#L359-L366 - // It needs to get replaced with a native rollbar/rollbar-php method - // as pointed out here https://github.com/rollbar/rollbar-php/issues/344 - $method = lcfirst(str_replace('_', '', ucwords($option, '_'))); - - // Handle the "branch" exception - switch ($method) { - case "branch": - $method = "gitBranch"; - break; - case "includeErrorCodeContext": - $method = 'includeCodeContext'; + $configOptions = Config::listOptions(); + $rollbarDefaults = Defaults::get(); + + foreach ($configOptions as $option) { + switch ($option) { + case 'branch': + $method = 'gitBranch'; break; - case "includeExceptionCodeContext": - $method = 'includeExcCodeContext'; + default: + $method = $option; break; } - - $default = method_exists(\Rollbar\Defaults::get(), $method) ? - \Rollbar\Defaults::get()->$method() : - null; - - $rollbarConfigNode - ->children() - ->scalarNode($option) - ->defaultValue($default) - ->end(); + + try { + $default = $rollbarDefaults->fromSnakeCase($method); + } catch (\Exception $e) { + $default = null; + } + + if (is_array($default)) { + $rollbarConfigNodeChildren + ->arrayNode($option) + ->scalarPrototype()->end() + ->defaultValue($default) + ->end(); + } else { + $rollbarConfigNodeChildren + ->scalarNode($option) + ->defaultValue($default) + ->end(); + } } - - $rollbarConfigNode->end(); return $treeBuilder; } diff --git a/DependencyInjection/RollbarExtension.php b/DependencyInjection/RollbarExtension.php index ba6cc74..ca13c60 100755 --- a/DependencyInjection/RollbarExtension.php +++ b/DependencyInjection/RollbarExtension.php @@ -7,13 +7,9 @@ use Symfony\Component\DependencyInjection\Loader; use Symfony\Component\HttpKernel\DependencyInjection\Extension; -use Rollbar\Rollbar; -use Rollbar\Monolog\Handler\RollbarHandler; -use Monolog\Logger; -use Psr\Logger\LoggerInterface; - /** - * Class Extension + * Class RollbarExtension + * * @package Rollbar\Symfony\RollbarBundle\DependencyInjection */ class RollbarExtension extends Extension @@ -21,12 +17,7 @@ class RollbarExtension extends Extension const ALIAS = 'rollbar'; /** - * Loads a specific configuration. - * - * @param array $configs An array of configuration values - * @param ContainerBuilder $container A ContainerBuilder instance - * - * @throws \InvalidArgumentException When provided tag is not defined in this extension + * {@inheritdoc} */ public function load(array $configs, ContainerBuilder $container) { @@ -42,7 +33,7 @@ public function load(array $configs, ContainerBuilder $container) } /** - * @return string + * {@inheritdoc} */ public function getAlias() { diff --git a/EventListener/AbstractListener.php b/EventListener/AbstractListener.php index 7e0e072..acfe41a 100755 --- a/EventListener/AbstractListener.php +++ b/EventListener/AbstractListener.php @@ -2,51 +2,57 @@ namespace Rollbar\Symfony\RollbarBundle\EventListener; +use Monolog\Logger; +use Psr\Log\LoggerInterface; +use Rollbar\Symfony\RollbarBundle\Payload\Generator; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Monolog\Logger; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\KernelEvents; -use Rollbar\Symfony\RollbarBundle\DependencyInjection\SymfonyRollbarExtension; -use Rollbar\Symfony\RollbarBundle\Payload\Generator; -use Psr\Log\LoggerInterface; /** * Class AbstractListener + * * @package Rollbar\Symfony\RollbarBundle\EventListener */ abstract class AbstractListener implements EventSubscriberInterface { /** - * @var \Monolog\Logger + * @var Logger */ protected $logger; /** - * @var \Symfony\Component\DependencyInjection\ContainerInterface + * @var ContainerInterface */ protected $container; /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator + * @var Generator */ protected $generator; + /** + * AbstractListener constructor. + * + * @param ContainerInterface $container + * @param LoggerInterface $logger + * @param Generator $generator + */ public function __construct( ContainerInterface $container, LoggerInterface $logger, Generator $generator ) { - /** - * @var \Rollbar\Symfony\RollbarBundle\Provider\RollbarHandler $rbProvider - */ $this->container = $container; $this->logger = $logger; $this->generator = $generator; } /** - * @return \Monolog\Logger + * Get logger. + * + * @return Logger */ public function getLogger() { @@ -54,7 +60,7 @@ public function getLogger() } /** - * @inheritdoc + * {@inheritdoc} */ public static function getSubscribedEvents() { @@ -64,7 +70,9 @@ public static function getSubscribedEvents() } /** - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * On kernel exception event handler. + * + * @param GetResponseForExceptionEvent $event */ public function onKernelException(GetResponseForExceptionEvent $event) { @@ -72,7 +80,9 @@ public function onKernelException(GetResponseForExceptionEvent $event) } /** - * @return \Symfony\Component\DependencyInjection\ContainerInterface + * Get container. + * + * @return ContainerInterface */ public function getContainer() { @@ -80,7 +90,9 @@ public function getContainer() } /** - * @return \Rollbar\Symfony\RollbarBundle\Payload\Generator + * Get generator. + * + * @return Generator */ public function getGenerator() { diff --git a/EventListener/ErrorListener.php b/EventListener/ErrorListener.php index 9095c01..2d3b925 100755 --- a/EventListener/ErrorListener.php +++ b/EventListener/ErrorListener.php @@ -2,20 +2,24 @@ namespace Rollbar\Symfony\RollbarBundle\EventListener; -use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; -use Rollbar\Symfony\RollbarBundle\DependencyInjection\RollbarExtension; use Psr\Log\LoggerInterface; +use Rollbar\Symfony\RollbarBundle\DependencyInjection\RollbarExtension; use Rollbar\Symfony\RollbarBundle\Payload\Generator; +use Symfony\Component\DependencyInjection\ContainerInterface; +/** + * Class ErrorListener + * + * @package Rollbar\Symfony\RollbarBundle\EventListener + */ class ErrorListener extends AbstractListener { /** * ErrorListener constructor. * - * @param \Symfony\Component\DependencyInjection\ContainerInterface $container - * @param \Psr\Log\LoggerInterface $logger - * @param \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator + * @param ContainerInterface $container + * @param LoggerInterface $logger + * @param Generator $generator */ public function __construct( ContainerInterface $container, @@ -73,9 +77,11 @@ public function handleFatalError() /** * Wrap php error_get_last() to get more testable code + * * @link: http://php.net/manual/en/function.error-get-last.php * * @return array|null + * * @codeCoverageIgnore */ protected function getLastError() @@ -86,13 +92,13 @@ protected function getLastError() /** * Check do we need to report error or skip * - * @param $code + * @param mixed $code * * @return int */ protected function isReportable($code) { - $code = (int)$code; + $code = (int) $code; $config = $this->getContainer()->getParameter(RollbarExtension::ALIAS . '.config'); return true diff --git a/EventListener/ExceptionListener.php b/EventListener/ExceptionListener.php index fbb5ecb..ec62d88 100755 --- a/EventListener/ExceptionListener.php +++ b/EventListener/ExceptionListener.php @@ -2,17 +2,19 @@ namespace Rollbar\Symfony\RollbarBundle\EventListener; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; -use Psr\Log\LoggerInterface; -use Rollbar\Symfony\RollbarBundle\Payload\Generator; +/** + * Class ExceptionListener + * + * @package Rollbar\Symfony\RollbarBundle\EventListener + */ class ExceptionListener extends AbstractListener { /** * Process exception * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * @param GetResponseForExceptionEvent $event */ public function onKernelException(GetResponseForExceptionEvent $event) { @@ -28,15 +30,16 @@ public function onKernelException(GetResponseForExceptionEvent $event) /** * Handle provided exception * - * @param $exception + * @param mixed $exception */ public function handleException($exception) { // generate payload and log data list($message, $payload) = $this->getGenerator()->getExceptionPayload($exception); - + $this->getLogger()->error($message, [ 'payload' => $payload, + 'exception' => $exception, ]); } } diff --git a/Factories/RollbarHandlerFactory.php b/Factories/RollbarHandlerFactory.php index 317cd1c..d1787d6 100755 --- a/Factories/RollbarHandlerFactory.php +++ b/Factories/RollbarHandlerFactory.php @@ -1,49 +1,57 @@ config = $container->getParameter(RollbarExtension::ALIAS . '.config'); - + $config = $container->getParameter(RollbarExtension::ALIAS . '.config'); + if (isset($_ENV['ROLLBAR_TEST_TOKEN']) && $_ENV['ROLLBAR_TEST_TOKEN']) { - $this->config['access_token'] = $_ENV['ROLLBAR_TEST_TOKEN']; + $config['access_token'] = $_ENV['ROLLBAR_TEST_TOKEN']; } - - if (empty($this->config['person'])) { + + if (empty($config['person'])) { try { if ($token = $container->get('security.token_storage')->getToken()) { - $this->config['person'] = $token->getUser(); + $config['person'] = $token->getUser(); } } catch (\Exception $exception) { + // Ignore } } - - if (!empty($this->config['person_fn']) && - is_callable($this->config['person_fn']) ) { - $this->config['person'] = null; + + if (!empty($config['person_fn']) && is_callable($config['person_fn'])) { + $config['person'] = null; } - - Rollbar::init($this->config, false, false, false); + + Rollbar::init($config, false, false, false); } - + + /** + * Create RollbarHandler + * + * @return RollbarHandler + */ public function createRollbarHandler() { - return new RollbarMonologHandler( - Rollbar::logger(), - Logger::ERROR - ); + return new RollbarHandler(Rollbar::logger(), LogLevel::ERROR); } } diff --git a/Payload/ErrorItem.php b/Payload/ErrorItem.php index d6023bb..2285c20 100755 --- a/Payload/ErrorItem.php +++ b/Payload/ErrorItem.php @@ -2,6 +2,11 @@ namespace Rollbar\Symfony\RollbarBundle\Payload; +/** + * Class ErrorItem + * + * @package Rollbar\Symfony\RollbarBundle\Payload + */ class ErrorItem { /** @@ -29,6 +34,8 @@ class ErrorItem ]; /** + * Invoke. + * * @param int $code * @param string $message * @param string $file @@ -62,17 +69,16 @@ public function __invoke($code, $message, $file, $line) return $record; } - /** * Map error code to human format * - * @param null $code + * @param mixed $code * - * @return mixed|string + * @return string */ protected function mapError($code) { - $code = (int)$code; + $code = (int) $code; return !empty(static::$map[$code]) ? static::$map[$code] : 'E_UNDEFINED'; } diff --git a/Payload/Generator.php b/Payload/Generator.php index 3e1fe19..39ea395 100755 --- a/Payload/Generator.php +++ b/Payload/Generator.php @@ -4,19 +4,30 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Kernel; +/** + * Class Generator + * + * @package Rollbar\Symfony\RollbarBundle\Payload + */ class Generator { /** - * @var \Symfony\Component\DependencyInjection\ContainerInterface + * @var ContainerInterface */ protected $container; /** - * @var \Symfony\Component\HttpKernel\Kernel + * @var Kernel */ protected $kernel; + /** + * Generator constructor. + * + * @param ContainerInterface $container + */ public function __construct(ContainerInterface $container) { $this->container = $container; @@ -38,7 +49,7 @@ public function getExceptionPayload($exception) */ $payload = [ 'body' => [], - 'framework' => \Symfony\Component\HttpKernel\Kernel::VERSION, + 'framework' => Kernel::VERSION, 'server' => $this->getServerInfo(), 'language_version' => phpversion(), 'request' => $this->getRequestInfo(), @@ -65,9 +76,11 @@ public function getExceptionPayload($exception) } /** - * @param $object - * @param $file - * @param $line + * Build generator error. + * + * @param object $object + * @param string $file + * @param int $line * * @return array */ @@ -79,6 +92,8 @@ protected function buildGeneratorError($object, $file, $line) } /** + * Get error payload. + * * @param int $code * @param string $message * @param string $file @@ -88,16 +103,13 @@ protected function buildGeneratorError($object, $file, $line) */ public function getErrorPayload($code, $message, $file, $line) { - /** - * @var \Symfony\Component\HttpFoundation\Request $request - */ $item = new ErrorItem(); $payload = [ 'body' => ['trace' => $item($code, $message, $file, $line)], 'request' => $this->getRequestInfo(), 'environment' => $this->getKernel()->getEnvironment(), - 'framework' => \Symfony\Component\HttpKernel\Kernel::VERSION, + 'framework' => Kernel::VERSION, 'language_version' => phpversion(), 'server' => $this->getServerInfo(), ]; @@ -106,13 +118,12 @@ public function getErrorPayload($code, $message, $file, $line) } /** + * Get request info. + * * @return array */ protected function getRequestInfo() { - /** - * @var \Symfony\Component\HttpFoundation\Request $request - */ $request = $this->getContainer()->get('request_stack')->getCurrentRequest(); if (empty($request)) { $request = new Request(); @@ -129,6 +140,8 @@ protected function getRequestInfo() } /** + * Get server info. + * * @return array */ protected function getServerInfo() @@ -146,7 +159,9 @@ protected function getServerInfo() } /** - * @return \Symfony\Component\DependencyInjection\ContainerInterface + * Get container. + * + * @return ContainerInterface */ public function getContainer() { @@ -154,7 +169,9 @@ public function getContainer() } /** - * @return \Symfony\Component\HttpKernel\Kernel + * Get kernel. + * + * @return Kernel */ public function getKernel() { diff --git a/Payload/TraceChain.php b/Payload/TraceChain.php index abbecae..a1ff9d9 100755 --- a/Payload/TraceChain.php +++ b/Payload/TraceChain.php @@ -2,9 +2,16 @@ namespace Rollbar\Symfony\RollbarBundle\Payload; +/** + * Class TraceChain + * + * @package Rollbar\Symfony\RollbarBundle\Payload + */ class TraceChain { /** + * Invoke. + * * @param \Exception $exception * * @return array diff --git a/Payload/TraceItem.php b/Payload/TraceItem.php index 3439383..1f6c530 100755 --- a/Payload/TraceItem.php +++ b/Payload/TraceItem.php @@ -2,9 +2,16 @@ namespace Rollbar\Symfony\RollbarBundle\Payload; +/** + * Class TraceItem + * + * @package Rollbar\Symfony\RollbarBundle\Payload + */ class TraceItem { /** + * Invoke. + * * @param \Exception $exception * * @return array diff --git a/RollbarBundle.php b/RollbarBundle.php index fa272d8..0805352 100755 --- a/RollbarBundle.php +++ b/RollbarBundle.php @@ -1,4 +1,5 @@ getContainer(); + $container = isset(static::$container) ? static::$container : static::$kernel->getContainer(); + + $config = $container->getParameter(RollbarExtension::ALIAS . '.config'); + + $configOptions = Config::listOptions(); + $rollbarDefaults = Defaults::get(); - $config = $container->getParameter(RollbarExtension::ALIAS . '.config'); - $defaults = []; - foreach (\Rollbar\Config::listOptions() as $option) { - // TODO: this is duplicated code from - // https://github.com/rollbar/rollbar-php-wordpress/blob/master/src/Plugin.php#L359-L366 - // It needs to get replaced with a native rollbar/rollbar-php method - // as pointed out here https://github.com/rollbar/rollbar-php/issues/344 - $method = lcfirst(str_replace('_', '', ucwords($option, '_'))); - + foreach ($configOptions as $option) { // Handle the "branch" exception - switch ($method) { + switch ($option) { case "branch": $method = "gitBranch"; break; - case "includeErrorCodeContext": - $method = 'includeCodeContext'; - break; - case "includeExceptionCodeContext": - $method = 'includeExcCodeContext'; + default: + $method = $option; break; } - - $default = method_exists(\Rollbar\Defaults::get(), $method) ? - \Rollbar\Defaults::get()->$method() : - null; - + + try { + $default = $rollbarDefaults->fromSnakeCase($method); + } catch (\Exception $e) { + $default = null; + } + $defaults[$option] = $default; } diff --git a/Tests/DependencyInjection/RollbarExtensionTest.php b/Tests/DependencyInjection/RollbarExtensionTest.php index f5ea4fa..a8dc35d 100755 --- a/Tests/DependencyInjection/RollbarExtensionTest.php +++ b/Tests/DependencyInjection/RollbarExtensionTest.php @@ -1,22 +1,22 @@ load(); $param = $this->container->getParameter($var); - + foreach ($expected as $key => $value) { $this->assertEquals($param[$key], $value); } } + /** + * Data provider generatorConfigVars. + * + * @return array + */ public function generatorConfigVars() { return [ @@ -51,24 +58,29 @@ public function generatorConfigVars() } /** + * Test config disabled vars. + * * @dataProvider generatorConfigVars * * @expectedException \PHPUnit_Framework_ExpectationFailedException * * @param string $var - * @param mixed $value + * @param array $expected */ public function testConfigDisabledVars($var, $expected) { $this->load(['enabled' => false]); $param = $this->container->getParameter($var); - + foreach ($expected as $key => $value) { $this->assertEquals($param[$key], $value); } } + /** + * Test alias. + */ public function testAlias() { $extension = new RollbarExtension(); diff --git a/Tests/EventListener/ErrorListenerTest.php b/Tests/EventListener/ErrorListenerTest.php index a986c1c..60cc24a 100755 --- a/Tests/EventListener/ErrorListenerTest.php +++ b/Tests/EventListener/ErrorListenerTest.php @@ -1,17 +1,25 @@ getContainer(); + $message = 'Fatal error - ' . time(); - /** - * @var TraceableEventDispatcher $eventDispatcher - */ + $container = $this->getContainer(); $eventDispatcher = $container->get('event_dispatcher'); $listeners = $eventDispatcher->getListeners('kernel.exception'); - $handler = \Tests\Fixtures\ErrorHandler::getInstance(); + $handler = ErrorHandler::getInstance(); $handler->setAssert(function (array $record) use ($message) { $this->assertNotEmpty($record); @@ -44,9 +52,6 @@ public function testUserError() }); foreach ($listeners as $listener) { - /** - * @var AbstractListener $listener - */ if (!$listener[0] instanceof AbstractListener) { continue; } @@ -55,95 +60,112 @@ public function testUserError() } trigger_error($message, E_USER_ERROR); + + $handler->setAssert(null); } /** + * Test fatal error parser. + * * @dataProvider generateFatalError * * @param array $error - * @param bool $called + * @param bool $called */ public function testFatalErrorParser($error, $called) { $mock = $this->getMockBuilder(ErrorListener::class) - ->setMethods(['getLastError', 'handleError']) - ->disableOriginalConstructor() - ->getMock(); + ->setMethods(['getLastError', 'handleError']) + ->disableOriginalConstructor() + ->getMock(); $mock->method('getLastError') - ->willReturn($error); + ->willReturn($error); $mock->expects($called ? $this->once() : $this->never()) - ->method('handleError') - ->with( - $this->equalTo($error['type']), - $this->stringContains($error['message']), - $this->stringContains($error['file']), - $this->equalTo($error['line']) - ); - - /** - * @var ErrorListener $mock - */ + ->method('handleError') + ->with( + $this->equalTo($error['type']), + $this->stringContains($error['message']), + $this->stringContains($error['file']), + $this->equalTo($error['line']) + ); + + /** @var ErrorListener $mock */ $mock->handleFatalError(); } /** + * Data provider generate fatal error. + * * @return array */ public function generateFatalError() { return [ - [['type' => E_ERROR, 'message' => 'Error message', 'file' => __DIR__, 'line' => rand(10, 100)], true], - [null, false] + [['type' => E_ERROR, 'message' => 'Error message', 'file' => __DIR__, 'line' => rand(10, 100)], true], + [null, false] ]; } /** + * Test isReportable. + * * @dataProvider generateIsReportable + * * @param bool $called */ public function testIsReportable($called) { - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); - - $logger = $this->getMockBuilder(\Monolog\Logger::class) - ->setMethods(['error']) - ->setConstructorArgs(['test-alias']) - ->getMock(); + $container = $this->getContainer(); + $generator = $container->get('test.' . Generator::class); + + $logger = $this->getMockBuilder(Logger::class) + ->setMethods(['error']) + ->setConstructorArgs(['test-alias']) + ->getMock(); $logger->method('error') - ->willReturn(true); + ->willReturn(true); - $mock = $this->getMockBuilder(ErrorListener::class) - ->setMethods(['isReportable', 'getGenerator', 'getLogger']) - ->disableOriginalConstructor() - ->getMock(); + $mock = $this->getMockBuilder(ErrorListener::class) + ->setMethods(['isReportable', 'getGenerator', 'getLogger']) + ->disableOriginalConstructor() + ->getMock(); $mock->method('isReportable') - ->willReturn($called); + ->willReturn($called); $mock->method('getGenerator') - ->willReturn($generator); + ->willReturn($generator); $mock->method('getLogger') - ->willReturn($logger); + ->willReturn($logger); - /** - * @var ErrorListener $mock - */ + /** @var ErrorListener $mock */ $mock->handleError(E_ERROR, 'Message', __FILE__, rand(1, 10)); } /** + * Data provider for testIsReportable. + * * @return array */ public function generateIsReportable() { return [ - [true], - [false] + [true], + [false] ]; } + + /** + * Get container. + * + * @return ContainerInterface + */ + private function getContainer() + { + return isset(static::$container) ? static::$container : static::$kernel->getContainer(); + } } diff --git a/Tests/EventListener/ExceptionListenerTest.php b/Tests/EventListener/ExceptionListenerTest.php index d58a904..55d3853 100755 --- a/Tests/EventListener/ExceptionListenerTest.php +++ b/Tests/EventListener/ExceptionListenerTest.php @@ -1,20 +1,23 @@ getContainer(); - - /** - * @var TraceableEventDispatcher $eventDispatcher - */ + $container = $this->getContainer(); $eventDispatcher = $container->get('event_dispatcher'); $exception = new \Exception('This is new exception'); $event = new GetResponseForExceptionEvent( @@ -40,4 +42,14 @@ public function testException() $eventDispatcher->dispatch('kernel.exception', $event); } + + /** + * Get container. + * + * @return ContainerInterface + */ + private function getContainer() + { + return isset(static::$container) ? static::$container : static::$kernel->getContainer(); + } } diff --git a/Tests/Fixtures/ErrorHandler.php b/Tests/Fixtures/ErrorHandler.php index 81fc826..b928b17 100755 --- a/Tests/Fixtures/ErrorHandler.php +++ b/Tests/Fixtures/ErrorHandler.php @@ -5,20 +5,27 @@ use Monolog\Handler\AbstractProcessingHandler; use Monolog\Logger; +/** + * Class ErrorHandler + * + * @package Tests\Fixtures + */ class ErrorHandler extends AbstractProcessingHandler { /** - * @var \Tests\Fixtures\ErrorHandler + * @var ErrorHandler */ protected static $instance; /** - * @var Callable + * @var callable */ protected $assert; /** - * @return \Tests\Fixtures\ErrorHandler + * Get instance. + * + * @return ErrorHandler */ public static function getInstance() { @@ -30,7 +37,9 @@ public static function getInstance() } /** - * @param Callable $assert + * Set assert. + * + * @param callable $assert */ public function setAssert($assert = null) { @@ -40,7 +49,7 @@ public function setAssert($assert = null) /** * Writes the record down to the log of the implementing handler * - * @param array $record + * @param array $record * * @return void */ diff --git a/Tests/Fixtures/app/autoload.php b/Tests/Fixtures/app/autoload.php deleted file mode 100755 index cbeb80e..0000000 --- a/Tests/Fixtures/app/autoload.php +++ /dev/null @@ -1,11 +0,0 @@ -assertEquals($mapped, $exception['class']); $this->assertContains($message, $exception['message']); - $this->assertEquals(1, count($data['frames'])); + $this->assertCount(1, $data['frames']); $frame = $data['frames'][0]; $this->assertEquals($file, $frame['filename']); @@ -39,6 +43,8 @@ public function testInvoke($code, $message, $file, $line, $mapped) } /** + * Data provider for testInvoke. + * * @return array */ public function generateInvoke() diff --git a/Tests/Payload/GeneratorTest.php b/Tests/Payload/GeneratorTest.php index a8a3a1f..d60e732 100755 --- a/Tests/Payload/GeneratorTest.php +++ b/Tests/Payload/GeneratorTest.php @@ -1,18 +1,25 @@ getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $container = $this->getContainer(); + $generator = $this->getGenerator(); $result = $generator->getContainer(); $this->assertEquals($container, $result); } + /** + * Test getKernel. + */ public function testGetKernel() { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $generator = $this->getGenerator(); $kernel = $generator->getKernel(); @@ -47,8 +53,10 @@ public function testGetKernel() } /** - * @param $class - * @param $method + * Get class method. + * + * @param string $class + * @param string $method * * @return \ReflectionMethod */ @@ -61,14 +69,12 @@ protected static function getClassMethod($class, $method) return $method; } - + /** + * Test getServerInfo. + */ public function testGetServerInfo() { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $generator = $this->getGenerator(); $method = static::getClassMethod(Generator::class, 'getServerInfo'); $data = $method->invoke($generator); @@ -84,17 +90,14 @@ public function testGetServerInfo() $this->assertEquals($data['user'], get_current_user()); } + /** + * Test getRequestInfo. + */ public function testGetRequestInfo() { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); - - /** - * @var \Symfony\Component\HttpFoundation\Request $request - */ + $container = $this->getContainer(); + $generator = $this->getGenerator(); + $request = $container->get('request_stack')->getCurrentRequest(); if (empty($request)) { $request = new Request(); @@ -118,13 +121,12 @@ public function testGetRequestInfo() $this->assertEquals($data['user_ip'], $request->getClientIp()); } + /** + * Test getErrorPayload. + */ public function testGetErrorPayload() { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $generator = $this->getGenerator(); $serverMethod = static::getClassMethod(Generator::class, 'getServerInfo'); $serverInfo = $serverMethod->invoke($generator); @@ -154,18 +156,17 @@ public function testGetErrorPayload() $this->assertEquals($body, $payload['body']); $this->assertEquals($requestInfo, $payload['request']); $this->assertEquals(static::$kernel->getEnvironment(), $payload['environment']); - $this->assertEquals(\Symfony\Component\HttpKernel\Kernel::VERSION, $payload['framework']); + $this->assertEquals(Kernel::VERSION, $payload['framework']); $this->assertEquals(phpversion(), $payload['language_version']); $this->assertEquals($serverInfo, $payload['server']); } + /** + * Test getExceptionPayload. + */ public function testGetExceptionPayload() { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $generator = $this->getGenerator(); $serverMethod = static::getClassMethod(Generator::class, 'getServerInfo'); $serverInfo = $serverMethod->invoke($generator); @@ -194,22 +195,21 @@ public function testGetExceptionPayload() $this->assertEquals($body, $payload['body']); $this->assertEquals($requestInfo, $payload['request']); $this->assertEquals(static::$kernel->getEnvironment(), $payload['environment']); - $this->assertEquals(\Symfony\Component\HttpKernel\Kernel::VERSION, $payload['framework']); + $this->assertEquals(Kernel::VERSION, $payload['framework']); $this->assertEquals(phpversion(), $payload['language_version']); $this->assertEquals($serverInfo, $payload['server']); } /** + * Test strange exception. + * * @dataProvider generatorStrangeData + * * @param mixed $data */ public function testStrangeException($data) { - /** - * @var \Rollbar\Symfony\RollbarBundle\Payload\Generator $generator - */ - $container = static::$kernel->getContainer(); - $generator = $container->get('Rollbar\\Symfony\\RollbarBundle\\Payload\\Generator'); + $generator = $this->getGenerator(); list($message, $payload) = $generator->getExceptionPayload($data); @@ -218,6 +218,8 @@ public function testStrangeException($data) } /** + * Data provider for testStrangeException. + * * @return array */ public function generatorStrangeData() @@ -227,9 +229,29 @@ public function generatorStrangeData() [1234], [0.2345], [null], - [(object)['p' => 'a']], + [(object) ['p' => 'a']], [['s' => 'app', 'd' => 'web']], [new ErrorItem()], ]; } + + /** + * Get container. + * + * @return ContainerInterface + */ + private function getContainer() + { + return isset(static::$container) ? static::$container : static::$kernel->getContainer(); + } + + /** + * Get generator. + * + * @return Generator + */ + private function getGenerator() + { + return $this->getContainer()->get('test.' . Generator::class); + } } diff --git a/Tests/Payload/TraceChainTest.php b/Tests/Payload/TraceChainTest.php index a432ff7..f217394 100755 --- a/Tests/Payload/TraceChainTest.php +++ b/Tests/Payload/TraceChainTest.php @@ -1,15 +1,20 @@ assertEquals(3, count($chain)); + $this->assertCount(3, $chain); foreach ($chain as $item) { $this->assertArrayHasKey('exception', $item); diff --git a/Tests/Payload/TraceItemTest.php b/Tests/Payload/TraceItemTest.php index cd8d7a4..df9a113 100755 --- a/Tests/Payload/TraceItemTest.php +++ b/Tests/Payload/TraceItemTest.php @@ -1,16 +1,20 @@ getContainer(); - - /** - * @var TraceableEventDispatcher $eventDispatcher - */ + $container = isset(static::$container) ? static::$container : static::$kernel->getContainer(); $eventDispatcher = $container->get('event_dispatcher'); $listeners = $eventDispatcher->getListeners('kernel.exception'); $listeners = array_merge( @@ -34,14 +36,19 @@ public function testListeners() $eventDispatcher->getListeners('kernel.controller') ); - $expected = [ - \Rollbar\Symfony\RollbarBundle\EventListener\ErrorListener::class, - \Rollbar\Symfony\RollbarBundle\EventListener\ExceptionListener::class + $expectedListeners = [ + ErrorListener::class, + ExceptionListener::class, ]; - + foreach ($listeners as $listener) { - $ok = $listener[0] instanceof $expected[0] || $listener[0] instanceof $expected[1]; - $this->assertTrue($ok, 'Listeners were not registered'); + foreach ($expectedListeners as $key => $expectedListener) { + if ($listener[0] instanceof $expectedListener) { + unset($expectedListeners[$key]); + } + } } + + $this->assertEmpty($expectedListeners, 'Listeners were not registered'); } } diff --git a/composer.json b/composer.json index 5004132..45ccd5a 100755 --- a/composer.json +++ b/composer.json @@ -29,14 +29,14 @@ "require": { "php": ">=5.6", "rollbar/rollbar": "^1", - "symfony/dependency-injection": "^3.2", - "symfony/config": "^3.2", - "symfony/http-kernel": "^3.2", + "symfony/dependency-injection": "^3.4 | ^4.0", + "symfony/config": "^3.4 | ^4.0", + "symfony/http-kernel": "^3.4 | ^4.0", "symfony/monolog-bundle": "*" }, "require-dev": { "phpunit/phpunit": "^5.7", - "doctrine/doctrine-fixtures-bundle": "^2.3", + "symfony/framework-bundle": "^3.4 | ^4.0", "squizlabs/php_codesniffer": "^2.7", "matthiasnoback/symfony-dependency-injection-test": "^1.1" }, diff --git a/phpunit.xml b/phpunit.xml index 587e85e..d70f03b 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,7 +6,7 @@ backupGlobals="false" colors="true" convertErrorsToExceptions="false" - bootstrap="Tests/Fixtures/app/autoload.php" + bootstrap="vendor/autoload.php" > From 7d4e5a5336065ed6719c5a6c4f47a295695386fc Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Sat, 18 Aug 2018 06:21:22 +0000 Subject: [PATCH 2/3] Change Exception strong-typing to weak-typing so the bundle is versatile between different versions of PHP --- Payload/TraceChain.php | 10 +++++----- Payload/TraceItem.php | 18 +++++++++--------- README.md | 25 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/Payload/TraceChain.php b/Payload/TraceChain.php index a1ff9d9..04a6054 100755 --- a/Payload/TraceChain.php +++ b/Payload/TraceChain.php @@ -12,18 +12,18 @@ class TraceChain /** * Invoke. * - * @param \Exception $exception + * @param $throwable * * @return array */ - public function __invoke(\Exception $exception) + public function __invoke($throwable) { $chain = []; $item = new TraceItem(); - while (!empty($exception)) { - $chain[] = $item($exception); - $exception = $exception->getPrevious(); + while (!empty($throwable)) { + $chain[] = $item($throwable); + $throwable = $throwable->getPrevious(); } return $chain; diff --git a/Payload/TraceItem.php b/Payload/TraceItem.php index 1f6c530..8059819 100755 --- a/Payload/TraceItem.php +++ b/Payload/TraceItem.php @@ -12,15 +12,15 @@ class TraceItem /** * Invoke. * - * @param \Exception $exception + * @param $throwable * * @return array */ - public function __invoke(\Exception $exception) + public function __invoke($throwable) { $frames = []; - foreach ($exception->getTrace() as $row) { + foreach ($throwable->getTrace() as $row) { // prepare initial frame $frame = [ 'filename' => empty($row['file']) ? null : $row['file'], @@ -44,17 +44,17 @@ public function __invoke(\Exception $exception) $record = [ 'exception' => [ - 'class' => get_class($exception), + 'class' => get_class($throwable), 'message' => implode(' ', [ - "'\\" . get_class($exception) . "'", + "'\\" . get_class($throwable) . "'", 'with message', - "'" . $exception->getMessage() . "'", + "'" . $throwable->getMessage() . "'", 'occurred in file', - "'" . $exception->getFile() . "'", + "'" . $throwable->getFile() . "'", 'line', - "'" . $exception->getLine() . "'", + "'" . $throwable->getLine() . "'", 'with code', - "'" . $exception->getCode() . "'", + "'" . $throwable->getCode() . "'", ]), ], 'frames' => $frames, diff --git a/README.md b/README.md index 19f0a08..5f2648e 100755 --- a/README.md +++ b/README.md @@ -19,6 +19,31 @@ See [real companies improving their development workflow thanks to Rollbar](http This bundle depends on [symfony/monolog-bundle](https://github.com/symfony/monolog-bundle). ## Installation + +### Symfony 4 +1. Add `Rollbar for Symfony` with composer: `composer require rollbar/rollbar-php-symfony3-bundle` +2. Register `Rollbar\Symfony\RollbarBundle\RollbarBundle` in `config/bundles.php` by adding the following entry at the end of your bundle array: +```php + ['all' => true] +?> +``` +3. Add RollbarHandler for appropriate environments in `config/packages/dev/monolog.yaml`, `config/packages/test/monolog.yaml` and `config/packages/prod/monolog.yaml` by adding the following in the `handlers` section: +```yaml + rollbar: + type: service + id: Rollbar\Monolog\Handler\RollbarHandler +``` +4. Configure your Rollbar setup in `config/rollbar.yaml` or in any of the environment subdirectories (i.e. `config/prod/rollbar.yaml`: +```yaml +rollbar: + enabled: true + access_token: [your access token] + environment: [environment name] + [other Rollbar config options] +``` + +### Symfony 3 1. Add `Rollbar for Symfony` with composer: `composer require rollbar/rollbar-php-symfony3-bundle` 2. Register `Rollbar\Symfony\RollbarBundle\RollbarBundle` in `AppKernel::registerBundles()` **after** registering the `MonologBundle` (`new Symfony\Bundle\MonologBundle\MonologBundle()`). From e2a62601230d38d5c317684d047293c17bdef204 Mon Sep 17 00:00:00 2001 From: Artur Moczulski Date: Sat, 18 Aug 2018 07:20:38 +0000 Subject: [PATCH 3/3] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5f2648e..e7f34a8 100755 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ rollbar: ``` -3. Configure Rollbar and Monolog in your `app/config.yml` or `app/config_*.yml`. +3. Configure Rollbar and Monolog in your `app/config/config.yml` or `app/config/config_*.yml`. ```yaml