diff --git a/.travis.yml b/.travis.yml index 2d1bcdd1..80a473b5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -48,7 +48,7 @@ jobs: before_install: - if [ "$USE_COMPOSER_JSON" != "1" ]; then composer remove friendsofphp/php-cs-fixer --dev --no-update; fi; - - if [ "$USE_COMPOSER_JSON" != "1" ]; then composer require laravel/framework:$LARAVEL illuminate/support:$LARAVEL orchestra/testbench:$TESTBENCH phpunit/phpunit:$PHPUNIT php-http/curl-client guzzlehttp/psr7 --no-update --no-interaction --dev; fi; + - if [ "$USE_COMPOSER_JSON" != "1" ]; then composer require laravel/framework:$LARAVEL illuminate/support:$LARAVEL orchestra/testbench:$TESTBENCH phpunit/phpunit:$PHPUNIT --no-update --no-interaction --dev; fi; install: - travis_retry composer install --no-suggest --no-interaction --prefer-dist --no-progress diff --git a/CHANGELOG.md b/CHANGELOG.md index 963f987e..8f931596 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Fix the configuration syntax for the sql bindings in breadcrumbs configuration option to be compatible with Laravel (#207) + ## 1.0.0 - This version requires `sentry/sentry` `>= 2.0` and also PHP `>= 7.1` @@ -8,10 +12,6 @@ Please see [Docs](https://docs.sentry.io/platforms/php/laravel/) for detailed usage. -## 0.12.0 (unreleased) - -- ... - ## 0.11.0 - Correctly merge the user config with the default configuration file (#163) diff --git a/composer.json b/composer.json index 7ce0a9ae..b56c1414 100644 --- a/composer.json +++ b/composer.json @@ -27,6 +27,11 @@ "Sentry\\Laravel\\": "src/" } }, + "autoload-dev": { + "psr-4": { + "Sentry\\Laravel\\Tests\\": "test/Sentry/" + } + }, "scripts": { "tests": [ "vendor/bin/phpunit --verbose" diff --git a/config/sentry.php b/config/sentry.php index ad01f9bb..c80e174b 100644 --- a/config/sentry.php +++ b/config/sentry.php @@ -6,6 +6,10 @@ // capture release as git sha // 'release' => trim(exec('git --git-dir ' . base_path('.git') . ' log --pretty="%h" -n1 HEAD')), - // Capture bindings on SQL queries - 'breadcrumbs.sql_bindings' => true, + 'breadcrumbs' => [ + + // Capture bindings on SQL queries logged in breadcrumbs + 'sql_bindings' => true, + + ], ); diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index 06d2714c..9aba7356 100644 --- a/src/Sentry/Laravel/EventHandler.php +++ b/src/Sentry/Laravel/EventHandler.php @@ -56,7 +56,7 @@ class EventHandler * * @var bool */ - private $sqlBindings; + private $recordSqlBindings; /** * EventHandler constructor. @@ -65,7 +65,7 @@ class EventHandler */ public function __construct(array $config) { - $this->sqlBindings = isset($config['breadcrumbs.sql_bindings']) ? $config['breadcrumbs.sql_bindings'] === true : true; + $this->recordSqlBindings = ($config['breadcrumbs']['sql_bindings'] ?? $config['breadcrumbs.sql_bindings'] ?? false) === true; } /** @@ -161,7 +161,7 @@ protected function queryHandler($query, $bindings, $time, $connectionName) { $data = array('connectionName' => $connectionName); - if ($this->sqlBindings) { + if ($this->recordSqlBindings) { $data['bindings'] = $bindings; } @@ -183,7 +183,7 @@ protected function queryExecutedHandler(QueryExecuted $query) { $data = array('connectionName' => $query->connectionName); - if ($this->sqlBindings) { + if ($this->recordSqlBindings) { $data['bindings'] = $query->bindings; } diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index 9a80b383..9e144ce6 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/src/Sentry/Laravel/ServiceProvider.php @@ -96,7 +96,11 @@ protected function configureAndRegisterClient(): void $userConfig = $this->app['config'][static::$abstract]; // We do not want this setting to hit our main client because it's Laravel specific - unset($userConfig['breadcrumbs.sql_bindings']); + unset( + $userConfig['breadcrumbs'], + // this is kept for backwards compatibilty and can be dropped in a breaking release + $userConfig['breadcrumbs.sql_bindings'] + ); $options = \array_merge( [ @@ -126,7 +130,9 @@ protected function configureAndRegisterClient(): void */ protected function hasDsnSet(): bool { - return !empty($this->app['config'][static::$abstract]['dsn'] ?? null); + $config = $this->app['config'][static::$abstract]; + + return !empty($config['dsn']); } /** diff --git a/test/Sentry/EventHandlerTest.php b/test/Sentry/EventHandlerTest.php index add556aa..2e6db05f 100644 --- a/test/Sentry/EventHandlerTest.php +++ b/test/Sentry/EventHandlerTest.php @@ -1,5 +1,7 @@ resetApplicationWithConfig([ /* config */ ]);` helper method + ]; + + protected function getEnvironmentSetUp($app) + { + foreach ($this->setupConfig as $key => $value) { + $app['config']->set($key, $value); + } + } + + protected function getPackageProviders($app) + { + return [ + ServiceProvider::class, + ]; + } + + protected function resetApplicationWithConfig(array $config) + { + $this->setupConfig = $config; + + $this->refreshApplication(); + } + + protected function dispatchLaravelEvent($event, array $payload = []) + { + $dispatcher = $this->app['events']; + + // Laravel 5.4+ uses the dispatch method to dispatch/fire events + return method_exists($dispatcher, 'dispatch') + ? $dispatcher->dispatch($event, $payload) + : $dispatcher->fire($event, $payload); + } + + protected function getScope(HubInterface $hub): Scope + { + $method = new \ReflectionMethod($hub, 'getScope'); + + $method->setAccessible(true); + + return $method->invoke($hub); + } +} diff --git a/test/Sentry/ServiceProviderTest.php b/test/Sentry/ServiceProviderTest.php index 0c2e6fe5..fc05dc2e 100644 --- a/test/Sentry/ServiceProviderTest.php +++ b/test/Sentry/ServiceProviderTest.php @@ -1,5 +1,7 @@ set('sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); + + parent::getEnvironmentSetUp($app); + } + + public function testIsBound() + { + $this->assertTrue(app()->bound('sentry')); + $this->assertInstanceOf(Hub::class, app('sentry')); + } + + /** + * @depends testIsBound + */ + public function testSqlBindingsAreRecordedWhenEnabled() + { + $this->resetApplicationWithConfig([ + 'sentry.breadcrumbs.sql_bindings' => true, + ]); + + $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); + + $this->dispatchLaravelEvent('illuminate.query', [ + $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', + $bindings = ['1'], + 10, + 'test', + ]); + + $breadcrumbs = $this->getScope(Hub::getCurrent())->getBreadcrumbs(); + + /** @var \Sentry\Breadcrumb $lastBreadcrumb */ + $lastBreadcrumb = end($breadcrumbs); + + $this->assertEquals($query, $lastBreadcrumb->getMessage()); + $this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']); + } + + /** + * @depends testIsBound + */ + public function testSqlBindingsAreRecordedWhenDisabled() + { + $this->resetApplicationWithConfig([ + 'sentry.breadcrumbs.sql_bindings' => false, + ]); + + $this->assertFalse($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); + + $this->dispatchLaravelEvent('illuminate.query', [ + $query = 'SELECT * FROM breadcrumbs WHERE bindings <> ?;', + $bindings = ['1'], + 10, + 'test', + ]); + + $breadcrumbs = $this->getScope(Hub::getCurrent())->getBreadcrumbs(); + + /** @var \Sentry\Breadcrumb $lastBreadcrumb */ + $lastBreadcrumb = end($breadcrumbs); + + $this->assertEquals($query, $lastBreadcrumb->getMessage()); + $this->assertFalse(isset($lastBreadcrumb->getMetadata()['bindings'])); + } +} diff --git a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyTest.php b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyTest.php new file mode 100644 index 00000000..fc48cf6f --- /dev/null +++ b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyTest.php @@ -0,0 +1,53 @@ +set('sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); + + $config = $app['config']->all(); + + $config['sentry']['breadcrumbs.sql_bindings'] = true; + + unset($config['sentry']['breadcrumbs']); + + $app['config'] = new Repository($config); + } + + public function testIsBound() + { + $this->assertTrue(app()->bound('sentry')); + $this->assertInstanceOf(Hub::class, app('sentry')); + } + + /** + * @depends testIsBound + */ + public function testSqlBindingsAreRecordedWhenEnabledByOldConfigKey() + { + $this->assertTrue($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']); + + $this->dispatchLaravelEvent('illuminate.query', [ + $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', + $bindings = ['1'], + 10, + 'test', + ]); + + $breadcrumbs = $this->getScope(Hub::getCurrent())->getBreadcrumbs(); + + /** @var \Sentry\Breadcrumb $lastBreadcrumb */ + $lastBreadcrumb = end($breadcrumbs); + + $this->assertEquals($query, $lastBreadcrumb->getMessage()); + $this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']); + } +} diff --git a/test/bootstrap.php b/test/bootstrap.php index 800d7ca8..10301536 100644 --- a/test/bootstrap.php +++ b/test/bootstrap.php @@ -3,3 +3,5 @@ error_reporting(E_ALL | E_STRICT); session_start(); + +require_once __DIR__ . '/../vendor/autoload.php';