Fix wrapping of the DBAL driver connection when implementing the `Doc…

…trine\DBAL\Driver\ServerInfoAwareConnection` interface (#567)
getsentry · Oct 19, 2021 · f3594df · f3594df
1 parent 977755e
commit f3594df
Show file tree

Hide file tree

Showing 17 changed files with 751 additions and 86 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - Add return typehints to the methods of the `SentryExtension` class to prepare for Symfony 6 (#563)
 - Fix setting the IP address on the user context when it's not available (#565)
 - Fix wrong method existence check in `TracingDriverConnection::errorCode()` (#568)
+- Fix decoration of the Doctrine DBAL connection when it implemented the `ServerInfoAwareConnection` interface (#567)
 
 ## 4.2.3 (2021-09-21)
 

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -35,6 +35,26 @@ parameters:
 			count: 1
 			path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php
 
+		-
+			message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingServerInfoAwareDriverConnection\\:\\:errorInfo\\(\\) return type has no value type specified in iterable type array\\.$#"
+			count: 1
+			path: src/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnection.php
+
+		-
+			message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingServerInfoAwareDriverConnection\\:\\:exec\\(\\) has parameter \\$sql with no typehint specified\\.$#"
+			count: 1
+			path: src/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnection.php
+
+		-
+			message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingServerInfoAwareDriverConnection\\:\\:prepare\\(\\) has parameter \\$sql with no typehint specified\\.$#"
+			count: 1
+			path: src/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnection.php
+
+		-
+			message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingServerInfoAwareDriverConnection\\:\\:query\\(\\) has parameter \\$args with no typehint specified\\.$#"
+			count: 1
+			path: src/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnection.php
+
 		-
 			message: "#^Class Symfony\\\\Bundle\\\\FrameworkBundle\\\\Client not found\\.$#"
 			count: 1
@@ -170,6 +190,26 @@ parameters:
 			count: 1
 			path: tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php
 
+		-
+			message: "#^Parameter \\#1 \\$hubOrConnectionFactory of class Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverMiddleware constructor expects Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnectionFactoryInterface\\|Sentry\\\\State\\\\HubInterface, null given\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php
+
+		-
+			message: "#^Trying to mock an undefined method errorCode\\(\\) on class Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnectionInterface\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnectionTest.php
+
+		-
+			message: "#^Trying to mock an undefined method errorInfo\\(\\) on class Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnectionInterface\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnectionTest.php
+
+		-
+			message: "#^Trying to mock an undefined method requiresQueryForServerVersion\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\ServerInfoAwareConnectionStub\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnectionTest.php
+
 		-
 			message: "#^Trying to mock an undefined method closeCursor\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
 			count: 1

diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml
@@ -91,10 +91,16 @@
             <tag name="console.command" />
         </service>
 
-        <service id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware" class="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware">
+        <service id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnectionFactoryInterface" alias="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnectionFactory" />
+
+        <service id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnectionFactory" class="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnectionFactory">
             <argument type="service" id="Sentry\State\HubInterface" />
         </service>
 
+        <service id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware" class="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware">
+            <argument type="service" id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnectionFactoryInterface" />
+        </service>
+
         <service id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator" class="Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator">
             <argument type="service" id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware" />
         </service>

diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php
@@ -16,7 +16,7 @@
  * capabilities to Doctrine DBAL. This implementation IS and MUST be compatible
  * with all versions of Doctrine DBAL >= 2.10.
  */
-final class TracingDriverConnection implements DriverConnectionInterface
+final class TracingDriverConnection implements TracingDriverConnectionInterface
 {
     /**
      * @internal

diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnectionFactory.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnectionFactory.php
@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;
+
+use Doctrine\DBAL\Driver\Connection;
+use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Sentry\State\HubInterface;
+
+/**
+ * @internal
+ */
+final class TracingDriverConnectionFactory implements TracingDriverConnectionFactoryInterface
+{
+    /**
+     * @var HubInterface The current hub
+     */
+    private $hub;
+
+    /**
+     * Constructor.
+     *
+     * @param HubInterface $hub The current hub
+     */
+    public function __construct(HubInterface $hub)
+    {
+        $this->hub = $hub;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function create(Connection $connection, AbstractPlatform $databasePlatform, array $params): TracingDriverConnectionInterface
+    {
+        $tracingDriverConnection = new TracingDriverConnection(
+            $this->hub,
+            $connection,
+            $databasePlatform->getName(),
+            $params
+        );
+
+        if ($connection instanceof ServerInfoAwareConnection) {
+            $tracingDriverConnection = new TracingServerInfoAwareDriverConnection($tracingDriverConnection);
+        }
+
+        return $tracingDriverConnection;
+    }
+}
diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnectionFactoryInterface.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnectionFactoryInterface.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;
+
+use Doctrine\DBAL\Driver\Connection;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+
+interface TracingDriverConnectionFactoryInterface
+{
+    /**
+     * Creates an instance of a driver connection which is decorated to trace
+     * the performances of the queries.
+     *
+     * @param Connection           $connection       The connection to wrap
+     * @param AbstractPlatform     $databasePlatform The database platform
+     * @param array<string, mixed> $params           The params of the connection
+     */
+    public function create(Connection $connection, AbstractPlatform $databasePlatform, array $params): TracingDriverConnectionInterface;
+}
diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnectionInterface.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnectionInterface.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;
+
+use Doctrine\DBAL\Driver\Connection;
+
+interface TracingDriverConnectionInterface extends Connection
+{
+    public function getWrappedConnection(): Connection;
+}
diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverForV2.php b/src/Tracing/Doctrine/DBAL/TracingDriverForV2.php
@@ -12,7 +12,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
-use Sentry\State\HubInterface;
 
 /**
  * This is a simple implementation of the {@see Driver} interface that decorates
@@ -24,34 +23,33 @@
 final class TracingDriverForV2 implements Driver, VersionAwarePlatformDriver, ExceptionConverterDriver
 {
     /**
-     * @var HubInterface The current hub
+     * @var TracingDriverConnectionFactoryInterface
      */
-    private $hub;
+    private $connectionFactory;
 
     /**
      * @var Driver|VersionAwarePlatformDriver|ExceptionConverterDriver The instance of the decorated driver
      */
     private $decoratedDriver;
 
     /**
-     * @param HubInterface $hub             The current hub
-     * @param Driver       $decoratedDriver The instance of the driver to decorate
+     * @param TracingDriverConnectionFactoryInterface $connectionFactory The connection factory
+     * @param Driver                                  $decoratedDriver   The instance of the driver to decorate
      */
-    public function __construct(HubInterface $hub, Driver $decoratedDriver)
+    public function __construct(TracingDriverConnectionFactoryInterface $connectionFactory, Driver $decoratedDriver)
     {
-        $this->hub = $hub;
         $this->decoratedDriver = $decoratedDriver;
+        $this->connectionFactory = $connectionFactory;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function connect(array $params, $username = null, $password = null, array $driverOptions = []): TracingDriverConnection
+    public function connect(array $params, $username = null, $password = null, array $driverOptions = []): TracingDriverConnectionInterface
     {
-        return new TracingDriverConnection(
-            $this->hub,
+        return $this->connectionFactory->create(
             $this->decoratedDriver->connect($params, $username, $password, $driverOptions),
-            $this->decoratedDriver->getDatabasePlatform()->getName(),
+            $this->decoratedDriver->getDatabasePlatform(),
             $params
         );
     }

diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverForV3.php b/src/Tracing/Doctrine/DBAL/TracingDriverForV3.php
@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
-use Sentry\State\HubInterface;
 
 /**
  * This is a simple implementation of the {@see Driver} interface that decorates
@@ -22,34 +21,35 @@
 final class TracingDriverForV3 implements Driver, VersionAwarePlatformDriver
 {
     /**
-     * @var HubInterface The current hub
+     * @var TracingDriverConnectionFactoryInterface The connection factory
      */
-    private $hub;
+    private $connectionFactory;
 
     /**
      * @var Driver|VersionAwarePlatformDriver The instance of the decorated driver
      */
     private $decoratedDriver;
 
     /**
-     * @param HubInterface $hub             The current hub
-     * @param Driver       $decoratedDriver The instance of the driver to decorate
+     * Constructor.
+     *
+     * @param TracingDriverConnectionFactoryInterface $connectionFactory The connection factory
+     * @param Driver                                  $decoratedDriver   The instance of the driver to decorate
      */
-    public function __construct(HubInterface $hub, Driver $decoratedDriver)
+    public function __construct(TracingDriverConnectionFactoryInterface $connectionFactory, Driver $decoratedDriver)
     {
-        $this->hub = $hub;
+        $this->connectionFactory = $connectionFactory;
         $this->decoratedDriver = $decoratedDriver;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function connect(array $params): TracingDriverConnection
+    public function connect(array $params): TracingDriverConnectionInterface
     {
-        return new TracingDriverConnection(
-            $this->hub,
+        return $this->connectionFactory->create(
             $this->decoratedDriver->connect($params),
-            $this->decoratedDriver->getDatabasePlatform()->getName(),
+            $this->decoratedDriver->getDatabasePlatform(),
             $params
         );
     }

diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php b/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php
@@ -11,29 +11,39 @@
 /**
  * This middleware wraps a {@see Driver} instance into one that
  * supports the distributed tracing feature of Sentry.
+ *
+ * @internal since version 4.2
  */
 final class TracingDriverMiddleware implements Middleware
 {
     /**
-     * @var HubInterface The current hub
+     * @var TracingDriverConnectionFactoryInterface
      */
-    private $hub;
+    private $connectionFactory;
 
     /**
      * Constructor.
      *
-     * @param HubInterface $hub The current hub
+     * @param HubInterface|TracingDriverConnectionFactoryInterface $hubOrConnectionFactory The current hub (deprecated) or the connection factory
      */
-    public function __construct(HubInterface $hub)
+    public function __construct($hubOrConnectionFactory)
     {
-        $this->hub = $hub;
+        if ($hubOrConnectionFactory instanceof TracingDriverConnectionFactoryInterface) {
+            $this->connectionFactory = $hubOrConnectionFactory;
+        } elseif ($hubOrConnectionFactory instanceof HubInterface) {
+            @trigger_error(sprintf('Not passing an instance of the "%s" interface as argument of the constructor is deprecated since version 4.2 and will not work since version 5.0.', TracingDriverConnectionFactoryInterface::class), \E_USER_DEPRECATED);
+
+            $this->connectionFactory = new TracingDriverConnectionFactory($hubOrConnectionFactory);
+        } else {
+            throw new \InvalidArgumentException(sprintf('The constructor requires either an instance of the "%s" interface or an instance of the "%s" interface.', HubInterface::class, TracingDriverConnectionFactoryInterface::class));
+        }
     }
 
     /**
      * {@inheritdoc}
      */
     public function wrap(Driver $driver): Driver
     {
-        return new TracingDriver($this->hub, $driver);
+        return new TracingDriver($this->connectionFactory, $driver);
     }
 }