Do not generate SID or SERVICE_NAME when dbname or service name is not specified

Refactored the logic of connection string generation

lib/Doctrine/DBAL/Driver/AbstractOracleDriver.php

@@ -20,6 +20,7 @@
 namespace Doctrine\DBAL\Driver;
 
 use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\AbstractOracleDriver\EasyConnectString;
 use Doctrine\DBAL\Exception;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Schema\OracleSchemaManager;
@@ -108,48 +109,9 @@ abstract class AbstractOracleDriver implements Driver, ExceptionConverterDriver
      * @param array $params The connection parameters to return the Easy Connect STring for.
      *
      * @return string
-     *
-     * @link https://docs.oracle.com/database/121/NETAG/naming.htm
      */
     protected function getEasyConnectString(array $params)
     {
-        if ( ! empty($params['connectstring'])) {
-            return $params['connectstring'];
-        }
-
-        if ( ! empty($params['host'])) {
-            if ( ! isset($params['port'])) {
-                $params['port'] = 1521;
-            }
-
-            $serviceName = $params['dbname'];
-
-            if ( ! empty($params['servicename'])) {
-                $serviceName = $params['servicename'];
-            }
-
-            $service = 'SID=' . $serviceName;
-            $pooled  = '';
-            $instance = '';
-
-            if (isset($params['service']) && $params['service'] == true) {
-                $service = 'SERVICE_NAME=' . $serviceName;
-            }
-
-            if (isset($params['instancename']) && ! empty($params['instancename'])) {
-                $instance = '(INSTANCE_NAME = ' . $params['instancename'] . ')';
-            }
-
-            if (isset($params['pooled']) && $params['pooled'] == true) {
-                $pooled = '(SERVER=POOLED)';
-            }
-
-            return '(DESCRIPTION=' .
-                     '(ADDRESS=(PROTOCOL=TCP)(HOST=' . $params['host'] . ')(PORT=' . $params['port'] . '))' .
-                     '(CONNECT_DATA=(' . $service . ')' . $instance . $pooled . '))';
-
-        }
-
-        return $params['dbname'] ?? '';
+        return (string) EasyConnectString::fromConnectionParameters($params);
     }
 }

lib/Doctrine/DBAL/Driver/AbstractOracleDriver/EasyConnectString.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\AbstractOracleDriver;
+
+use function implode;
+use function is_array;
+use function sprintf;
+
+/**
+ * Represents an Oracle Easy Connect string
+ *
+ * @link https://docs.oracle.com/database/121/NETAG/naming.htm
+ */
+final class EasyConnectString
+{
+    /** @var string */
+    private $string;
+
+    private function __construct(string $string)
+    {
+        $this->string = $string;
+    }
+
+    public function __toString() : string
+    {
+        return $this->string;
+    }
+
+    /**
+     * Creates the object from an array representation
+     *
+     * @param mixed[] $params
+     */
+    public static function fromArray(array $params) : self
+    {
+        return new self(self::renderParams($params));
+    }
+
+    /**
+     * Creates the object from the given DBAL connection parameters.
+     *
+     * @param mixed[] $params
+     */
+    public static function fromConnectionParameters(array $params) : self
+    {
+        if (! empty($params['connectstring'])) {
+            return new self($params['connectstring']);
+        }
+
+        if (empty($params['host'])) {
+            return new self($params['dbname'] ?? '');
+        }
+
+        $connectData = [];
+
+        if (isset($params['servicename']) || isset($params['dbname'])) {
+            $serviceKey = 'SID';
+
+            if (! empty($params['service'])) {
+                $serviceKey = 'SERVICE_NAME';
+            }
+
+            $serviceName = $params['servicename'] ?? $params['dbname'];
+
+            $connectData[$serviceKey] = $serviceName;
+        }
+
+        if (! empty($params['instancename'])) {
+            $connectData['INSTANCE_NAME'] = $params['instancename'];
+        }
+
+        if (! empty($params['pooled'])) {
+            $connectData['SERVER'] = 'POOLED';
+        }
+
+        return self::fromArray([
+            'DESCRIPTION' => [
+                'ADDRESS' => [
+                    'PROTOCOL' => 'TCP',
+                    'HOST' => $params['host'],
+                    'PORT' => $params['port'] ?? 1521,
+                ],
+                'CONNECT_DATA' => $connectData,
+            ],
+        ]);
+    }
+
+    /**
+     * @param mixed[] $params
+     */
+    private static function renderParams(array $params) : string
+    {
+        $chunks = [];
+
+        foreach ($params as $key => $value) {
+            $string = self::renderValue($value);
+
+            if ($string === '') {
+                continue;
+            }
+
+            $chunks[] = sprintf('(%s=%s)', $key, $string);
+        }
+
+        return implode('', $chunks);
+    }
+
+    /**
+     * @param mixed $value
+     */
+    private static function renderValue($value) : string
+    {
+        if (is_array($value)) {
+            return self::renderParams($value);
+        }
+
+        return (string) $value;
+    }
+}

tests/Doctrine/Tests/DBAL/Driver/AbstractOracleDriver/EasyConnectStringTest.php

+<?php
+
+namespace Doctrine\Tests\DBAL\Driver\AbstractOracleDriver;
+
+use Doctrine\DBAL\Driver\AbstractOracleDriver\EasyConnectString;
+use PHPUnit\Framework\TestCase;
+
+class EasyConnectStringTest extends TestCase
+{
+    /**
+     * @param mixed[] $params
+     * @dataProvider connectionParametersProvider
+     */
+    public function testFromConnectionParameters(array $params, string $expected) : void
+    {
+        $string = EasyConnectString::fromConnectionParameters($params);
+
+        $this->assertSame($expected, (string) $string);
+    }
+
+    /**
+     * @return mixed[]
+     */
+    public static function connectionParametersProvider() : iterable
+    {
+        return [
+            'empty-params' => [[],''],
+            'common-params' => [
+                [
+                    'host' => 'oracle.example.com',
+                    'port' => 1521,
+                    'dbname' => 'XE',
+                ],
+                '(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=oracle.example.com)(PORT=1521))(CONNECT_DATA=(SID=XE)))',
+            ],
+            'no-db-name' => [
+                ['host' => 'localhost'],
+                '(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=1521)))',
+            ],
+            'service' => [
+                [
+                    'host' => 'localhost',
+                    'port' => 1521,
+                    'service' => true,
+                    'servicename' => 'BILLING',
+                ],
+                '(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=BILLING)))',
+            ],
+            'advanced-params' => [
+                [
+                    'host' => 'localhost',
+                    'port' => 41521,
+                    'dbname' => 'XE',
+                    'instancename' => 'SALES',
+                    'pooled' => true,
+                ],
+                '(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=41521))(CONNECT_DATA=(SID=XE)(INSTANCE_NAME=SALES)(SERVER=POOLED)))',
+            ],
+        ];
+    }
+}