Allow explicit null value for dsn configuration option (#457)

* Allow explicit null value for dsn configuration option

* Add tests for falsy dsn configuration values

* Add changelog entry

Co-authored-by: Alessandro Lai <[email protected]>

Author: bobvandevijver

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+## 4.0.3 (2021-03-03)
+- Fix regression from #454 for `null` value on DSN not disabling Sentry (#457)
+
 ## 4.0.2 (2021-03-03)
 
 - Add `kernel.project_dir` to `prefixes` default value to trim paths to the project root (#434)

src/DependencyInjection/SentryExtension.php

@@ -67,7 +67,7 @@ private function registerConfiguration(ContainerBuilder $container, array $confi
     {
         $options = $config['options'];
 
-        if (isset($config['dsn'])) {
+        if (\array_key_exists('dsn', $config)) {
             $options['dsn'] = $config['dsn'];
         }
 

tests/DependencyInjection/Fixtures/php/dsn_empty_string.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/** @var ContainerBuilder $container */
+$container->loadFromExtension('sentry', [
+    'dsn' => '',
+]);

tests/DependencyInjection/Fixtures/php/dsn_false.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/** @var ContainerBuilder $container */
+$container->loadFromExtension('sentry', [
+    'dsn' => false,
+]);

tests/DependencyInjection/Fixtures/php/dsn_null.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/** @var ContainerBuilder $container */
+$container->loadFromExtension('sentry', [
+    'dsn' => null,
+]);

tests/DependencyInjection/Fixtures/xml/dsn_empty_string.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sentry="https://sentry.io/schema/dic/sentry-symfony"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+                               https://sentry.io/schema/dic/sentry-symfony https://sentry.io/schema/dic/sentry-symfony/sentry-1.0.xsd">
+
+    <sentry:config dsn="">
+    </sentry:config>
+</container>

tests/DependencyInjection/Fixtures/xml/dsn_false.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sentry="https://sentry.io/schema/dic/sentry-symfony"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+                               https://sentry.io/schema/dic/sentry-symfony https://sentry.io/schema/dic/sentry-symfony/sentry-1.0.xsd">
+
+    <sentry:config dsn="false">
+    </sentry:config>
+</container>

tests/DependencyInjection/Fixtures/xml/dsn_null.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:sentry="https://sentry.io/schema/dic/sentry-symfony"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+                               https://sentry.io/schema/dic/sentry-symfony https://sentry.io/schema/dic/sentry-symfony/sentry-1.0.xsd">
+
+    <sentry:config dsn="null">
+    </sentry:config>
+</container>

tests/DependencyInjection/Fixtures/yml/dsn_empty_string.yml

@@ -0,0 +1,2 @@
+sentry:
+  dsn: ''

tests/DependencyInjection/Fixtures/yml/dsn_false.yml

@@ -0,0 +1,2 @@
+sentry:
+  dsn: false

tests/DependencyInjection/Fixtures/yml/dsn_null.yml

@@ -0,0 +1,2 @@
+sentry:
+  dsn: ~

tests/DependencyInjection/SentryExtensionTest.php

@@ -263,6 +263,21 @@ public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured():
         $this->assertSame(1, $ignoreErrorsIntegrationsCount);
     }
 
+    public function testEmptyDsnIsPropagatedToOptions(): void
+    {
+        $this->assertDsnPropagation('dsn_empty_string', '');
+    }
+
+    public function testFalseDsnIsPropagatedToOptions(): void
+    {
+        $this->assertDsnPropagation('dsn_false', false);
+    }
+
+    public function testNullDsnIsPropagatedToOptions(): void
+    {
+        $this->assertDsnPropagation('dsn_null', null);
+    }
+
     private function createContainerFromFixture(string $fixtureFile): ContainerBuilder
     {
         $container = new ContainerBuilder(new EnvPlaceholderParameterBag([
@@ -292,4 +307,17 @@ private function assertDefinitionMethodCallAt(array $methodCall, string $method,
         $this->assertSame($method, $methodCall[0]);
         $this->assertEquals($arguments, $methodCall[1]);
     }
+
+    /**
+     * @param mixed $result
+     */
+    private function assertDsnPropagation(string $fixtureFile, $result): void
+    {
+        $container = $this->createContainerFromFixture($fixtureFile);
+        $optionsDefinition = $container->getDefinition('sentry.client.options');
+
+        $this->assertSame(Options::class, $optionsDefinition->getClass());
+        $this->assertTrue(\array_key_exists('dsn', $optionsDefinition->getArgument(0)));
+        $this->assertSame($result, $optionsDefinition->getArgument(0)['dsn']);
+    }
 }