Fixing config syntax

.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

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)

composer.json

@@ -27,6 +27,11 @@
       "Sentry\\Laravel\\": "src/"
     }
   },
+  "autoload-dev": {
+    "psr-4": {
+      "Sentry\\Laravel\\Tests\\": "test/Sentry/"
+    }
+  },
   "scripts": {
     "tests": [
       "vendor/bin/phpunit --verbose"

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,
+
+    ],
 );

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;
         }
 

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']);
     }
 
     /**

test/Sentry/EventHandlerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+namespace Sentry\Laravel\Tests;
+
 class EventHandlerTest extends \PHPUnit\Framework\TestCase
 {
     /**

test/Sentry/SentryLaravelTestCase.php

@@ -0,0 +1,56 @@
+<?php
+
+namespace Sentry\Laravel\Tests;
+
+use Sentry\State\Scope;
+use Sentry\State\HubInterface;
+use Sentry\Laravel\ServiceProvider;
+use Orchestra\Testbench\TestCase as LaravelTestCase;
+
+abstract class SentryLaravelTestCase extends LaravelTestCase
+{
+    protected $setupConfig = [
+        // Set config here before refreshing the app to set it in the container before Sentry is loaded
+        // or use the `$this->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);
+    }
+}

test/Sentry/ServiceProviderTest.php

@@ -1,5 +1,7 @@
 <?php
 
+namespace Sentry\Laravel\Tests;
+
 use Sentry\State\Hub;
 use Sentry\Laravel\Facade;
 use Sentry\Laravel\ServiceProvider;

test/Sentry/ServiceProviderWithoutDsnTest.php

@@ -1,5 +1,7 @@
 <?php
 
+namespace Sentry\Laravel\Tests;
+
 use Sentry\Laravel\ServiceProvider;
 use Illuminate\Routing\Events\RouteMatched;
 

test/Sentry/SqlBindingsInBreadcrumbsTest.php

@@ -0,0 +1,75 @@
+<?php
+
+namespace Sentry\Laravel\Tests;
+
+use Sentry\State\Hub;
+
+class SqlBindingsInBreadcrumbsTest extends SentryLaravelTestCase
+{
+    protected function getEnvironmentSetUp($app)
+    {
+        $app['config']->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']));
+    }
+}

test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyTest.php

@@ -0,0 +1,53 @@
+<?php
+
+namespace Sentry\Laravel\Tests;
+
+use Sentry\State\Hub;
+use Illuminate\Config\Repository;
+
+class SqlBindingsInBreadcrumbsWithOldConfigKeyTest extends SentryLaravelTestCase
+{
+    protected function getEnvironmentSetUp($app)
+    {
+        parent::getEnvironmentSetUp($app);
+
+        $app['config']->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']);
+    }
+}

test/bootstrap.php

@@ -3,3 +3,5 @@
 error_reporting(E_ALL | E_STRICT);
 
 session_start();
+
+require_once __DIR__ . '/../vendor/autoload.php';