trim leading slash from schemeless connection URL path with default driver connection parameters

- includes extended default driver connection parameter handling
- minor refactoring

fixes #1183

lib/Doctrine/DBAL/DBALException.php

@@ -76,10 +76,22 @@ class DBALException extends \Exception
     }
 
     /**
+     * @param bool|null $url The URL that was provided in the connection parameters (if any).
+     *
      * @return \Doctrine\DBAL\DBALException
      */
-    public static function driverRequired()
+    public static function driverRequired($url = null)
     {
+        if ($url) {
+            return new self(
+                sprintf(
+                    "The options 'driver' or 'driverClass' are mandatory if a connection URL without scheme " .
+                    "is given to DriverManager::getConnection(). Given URL: %s",
+                    $url
+                )
+            );
+        }
+
         return new self("The options 'driver' or 'driverClass' are mandatory if no PDO ".
             "instance is given to DriverManager::getConnection().");
     }

lib/Doctrine/DBAL/DriverManager.php

@@ -212,6 +212,19 @@ final class DriverManager
         }
     }
 
+    /**
+     * Normalizes the given connection URL path.
+     *
+     * @param string $urlPath
+     *
+     * @return string The normalized connection URL path
+     */
+    private static function normalizeDatabaseUrlPath($urlPath)
+    {
+        // Trim leading slash from URL path.
+        return substr($urlPath, 1);
+    }
+
     /**
      * Extracts parts from a database URL, if present, and returns an
      * updated list of parameters.
@@ -237,12 +250,11 @@ final class DriverManager
             throw new DBALException('Malformed parameter "url".');
         }
 
-        if (isset($url['scheme'])) {
-            $params['driver'] = str_replace('-', '_', $url['scheme']); // URL schemes must not contain underscores, but dashes are ok
-            if (isset(self::$driverSchemeAliases[$params['driver']])) {
-                $params['driver'] = self::$driverSchemeAliases[$params['driver']]; // use alias like "postgres", else we just let checkParams decide later if the driver exists (for literal "pdo-pgsql" etc)
-            }
-        }
+        // If we have a connection URL, we have to unset the default PDO instance connection parameter (if any)
+        // as we cannot merge connection details from the URL into the PDO instance (URL takes precedence).
+        unset($params['pdo']);
+
+        $params = self::parseDatabaseUrlScheme($url, $params);
 
         if (isset($url['host'])) {
             $params['host'] = $url['host'];
@@ -257,53 +269,97 @@ final class DriverManager
             $params['password'] = $url['pass'];
         }
 
-        if (isset($url['path'])) {
-            $params = self::parseDatabaseUrlPath($url, $params);
-        }
-
-        if (isset($url['query'])) {
-            $query = array();
-            parse_str($url['query'], $query); // simply ingest query as extra params, e.g. charset or sslmode
-            $params = array_merge($params, $query); // parse_str wipes existing array elements
-        }
+        $params = self::parseDatabaseUrlPath($url, $params);
+        $params = self::parseDatabaseUrlQuery($url, $params);
 
         return $params;
     }
 
     /**
-     * Parses the given URL and resolves the given connection parameters.
+     * Parses the given connection URL and resolves the given connection parameters.
+     *
+     * Assumes that the connection URL scheme is already parsed and resolved into the given connection parameters
+     * via {@link parseDatabaseUrlScheme}.
      *
      * @param array $url    The URL parts to evaluate.
      * @param array $params The connection parameters to resolve.
      *
      * @return array The resolved connection parameters.
+     *
+     * @see parseDatabaseUrlScheme
      */
     private static function parseDatabaseUrlPath(array $url, array $params)
     {
-        if (!isset($url['scheme'])) {
-            $params['dbname'] = $url['path'];
-
+        if (! isset($url['path'])) {
             return $params;
         }
 
-        $url['path'] = substr($url['path'], 1);
+        $url['path'] = self::normalizeDatabaseUrlPath($url['path']);
 
-        if (strpos($url['scheme'], 'sqlite') !== false) {
+        // If we do not have a known DBAL driver, we do not know any connection URL path semantics to evaluate
+        // and therefore treat the path as regular DBAL connection URL path.
+        if (! isset($params['driver'])) {
+            return self::parseRegularDatabaseUrlPath($url, $params);
+        }
+
+        if (strpos($params['driver'], 'sqlite') !== false) {
             return self::parseSqliteDatabaseUrlPath($url, $params);
         }
 
+        return self::parseRegularDatabaseUrlPath($url, $params);
+    }
+
+    /**
+     * Parses the query part of the given connection URL and resolves the given connection parameters.
+     *
+     * @param array $url    The connection URL parts to evaluate.
+     * @param array $params The connection parameters to resolve.
+     *
+     * @return array The resolved connection parameters.
+     */
+    private static function parseDatabaseUrlQuery(array $url, array $params)
+    {
+        if (! isset($url['query'])) {
+            return $params;
+        }
+
+        $query = array();
+
+        parse_str($url['query'], $query); // simply ingest query as extra params, e.g. charset or sslmode
+
+        return array_merge($params, $query); // parse_str wipes existing array elements
+    }
+
+    /**
+     * Parses the given regular connection URL and resolves the given connection parameters.
+     *
+     * Assumes that the "path" URL part is already normalized via {@link normalizeDatabaseUrlPath}.
+     *
+     * @param array $url    The regular connection URL parts to evaluate.
+     * @param array $params The connection parameters to resolve.
+     *
+     * @return array The resolved connection parameters.
+     *
+     * @see normalizeDatabaseUrlPath
+     */
+    private static function parseRegularDatabaseUrlPath(array $url, array $params)
+    {
         $params['dbname'] = $url['path'];
 
         return $params;
     }
 
     /**
-     * Parses the given SQLite URL and resolves the given connection parameters.
+     * Parses the given SQLite connection URL and resolves the given connection parameters.
      *
-     * @param array $url    The SQLite URL parts to evaluate.
+     * Assumes that the "path" URL part is already normalized via {@link normalizeDatabaseUrlPath}.
+     *
+     * @param array $url    The SQLite connection URL parts to evaluate.
      * @param array $params The connection parameters to resolve.
      *
      * @return array The resolved connection parameters.
+     *
+     * @see normalizeDatabaseUrlPath
      */
     private static function parseSqliteDatabaseUrlPath(array $url, array $params)
     {
@@ -317,4 +373,44 @@ final class DriverManager
 
         return $params;
     }
+
+    /**
+     * Parses the scheme part from given connection URL and resolves the given connection parameters.
+     *
+     * @param array $url    The connection URL parts to evaluate.
+     * @param array $params The connection parameters to resolve.
+     *
+     * @return array The resolved connection parameters.
+     *
+     * @throws DBALException if parsing failed or resolution is not possible.
+     */
+    private static function parseDatabaseUrlScheme(array $url, array $params)
+    {
+        if (isset($url['scheme'])) {
+            // The requested driver from the URL scheme takes precedence
+            // over the default custom driver from the connection parameters (if any).
+            unset($params['driverClass']);
+
+            // URL schemes must not contain underscores, but dashes are ok
+            $driver = str_replace('-', '_', $url['scheme']);
+
+            // The requested driver from the URL scheme takes precedence
+            // over the default driver from the connection parameters (if any).
+            $params['driver'] = isset(self::$driverSchemeAliases[$driver])
+                // use alias like "postgres", else we just let checkParams decide later
+                // if the driver exists (for literal "pdo-pgsql" etc)
+                ? self::$driverSchemeAliases[$driver]
+                : $driver;
+
+            return $params;
+        }
+
+        // If a schemeless connection URL is given, we require a default driver or default custom driver
+        // as connection parameter.
+        if (! isset($params['driverClass']) && ! isset($params['driver'])) {
+            throw DBALException::driverRequired($params['url']);
+        }
+
+        return $params;
+    }
 }

tests/Doctrine/Tests/DBAL/DBALExceptionTest.php

@@ -12,4 +12,28 @@ class DBALExceptionTest extends \Doctrine\Tests\DbalTestCase
         $e = DBALException::driverExceptionDuringQuery($driver, new \Exception, '', array('ABC', chr(128)));
         $this->assertContains('with params ["ABC", "\x80"]', $e->getMessage());
     }
+
+    public function testAvoidOverWrappingOnDriverException()
+    {
+        $driver = $this->createMock('\Doctrine\DBAL\Driver');
+        $ex = new DriverException('', $this->createMock('\Doctrine\DBAL\Driver\DriverException'));
+        $e = DBALException::driverExceptionDuringQuery($driver, $ex, '');
+        $this->assertSame($ex, $e);
+    }
+
+    public function testDriverRequiredWithUrl()
+    {
+        $url = 'mysql://localhost';
+        $exception = DBALException::driverRequired($url);
+
+        $this->assertInstanceOf('Doctrine\DBAL\DBALException', $exception);
+        $this->assertSame(
+            sprintf(
+                "The options 'driver' or 'driverClass' are mandatory if a connection URL without scheme " .
+                "is given to DriverManager::getConnection(). Given URL: %s",
+                $url
+            ),
+            $exception->getMessage()
+        );
+    }
 }

tests/Doctrine/Tests/DBAL/DriverManagerTest.php

@@ -132,7 +132,7 @@ class DriverManagerTest extends \Doctrine\Tests\DbalTestCase
         
         $params = $conn->getParams();
         foreach ($expected as $key => $value) {
-            if ($key == 'driver') {
+            if (in_array($key, array('pdo', 'driver', 'driverClass'), true)) {
                 $this->assertInstanceOf($value, $conn->getDriver());
             } else {
                 $this->assertEquals($value, $params[$key]);
@@ -142,6 +142,10 @@ class DriverManagerTest extends \Doctrine\Tests\DbalTestCase
     
     public function databaseUrls()
     {
+        $pdoMock = $this->getMockBuilder('PDO')
+            ->disableOriginalConstructor()
+            ->getMock();
+
         return array(
             'simple URL' => array(
                 'mysql://foo:bar@localhost/baz',
@@ -199,6 +203,56 @@ class DriverManagerTest extends \Doctrine\Tests\DbalTestCase
                 'drizzle-pdo-mysql://foo:bar@localhost/baz',
                 array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\DrizzlePDOMySql\Driver'),
             ),
+
+            // DBAL-1234
+            'URL without scheme and without any driver information' => array(
+                array('url' => '//foo:bar@localhost/baz'),
+                false,
+            ),
+            'URL without scheme but default PDO driver' => array(
+                array('url' => '//foo:bar@localhost/baz', 'pdo' => $pdoMock),
+                false,
+            ),
+            'URL without scheme but default driver' => array(
+                array('url' => '//foo:bar@localhost/baz', 'driver' => 'pdo_mysql'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL without scheme but custom driver' => array(
+                array('url' => '//foo:bar@localhost/baz', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+            ),
+            'URL without scheme but default PDO driver and default driver' => array(
+                array('url' => '//foo:bar@localhost/baz', 'pdo' => $pdoMock, 'driver' => 'pdo_mysql'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL without scheme but driver and custom driver' => array(
+                array('url' => '//foo:bar@localhost/baz', 'driver' => 'pdo_mysql', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+            ),
+            'URL with default PDO driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'pdo' => $pdoMock),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL with default driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'driver' => 'sqlite'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL with default custom driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL with default PDO driver and default driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'pdo' => $pdoMock, 'driver' => 'sqlite'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL with default driver and default custom driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'driver' => 'sqlite',  'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
+            'URL with default PDO driver and default driver and default custom driver' => array(
+                array('url' => 'mysql://foo:bar@localhost/baz', 'pdo' => $pdoMock, 'driver' => 'sqlite', 'driverClass' => 'Doctrine\Tests\Mocks\DriverMock'),
+                array('user' => 'foo', 'password' => 'bar', 'host' => 'localhost', 'dbname' => 'baz', 'driver' => 'Doctrine\DBAL\Driver\PDOMySQL\Driver'),
+            ),
         );
     }
 }