Skip to content

Commit

Permalink
no issue - fix filename and :memory: detection in dsn
Browse files Browse the repository at this point in the history
  • Loading branch information
@pounard
pounard committed Apr 23, 2024
1 parent cf29a2c commit 7b6293b
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 26 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 1.5.5

* [fix] Handle `mysqli` in `Dsn` class.
* [fix] Handle `:memory:` in `Dsn` class.
* [fix] Better filename detection in `Dsn` class.

## 1.5.4

* [feature] Add `MakinaCorpus\QueryBuilder\Dsn` class as a public API for third
Expand Down
62 changes: 50 additions & 12 deletions src/Dsn.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,20 @@ class Dsn
private readonly bool $isFile;
private readonly string $driver;
private readonly string $vendor;
private readonly ?string $host;
private readonly ?string $filename;
private readonly ?string $database;

public function __construct(
/** Database vendor, eg. "mysql", "pgsql", ... */
string $vendor,
/** Host or local filename (unix socket, database file) */
private readonly string $host,
?string $host = null,
?string $filename = null,
bool $isFile = false,
/** Driver, eg. "pdo", "ext", ... */
?string $driver = null,
private readonly ?string $database = null,
?string $database = null,
private readonly ?string $user = null,
#[\SensitiveParameter]
private readonly ?string $password = null,
Expand All @@ -54,6 +58,7 @@ public function __construct(
$this->driver = self::DRIVER_SQLITE3;
$this->vendor = Vendor::SQLITE;
$this->scheme = $vendor;
$isFile = true;
break;
default:
$matches = [];
Expand All @@ -68,7 +73,28 @@ public function __construct(
}
break;
}
$this->isFile = $isFile || '/' === $host[0];

if (!$host && !$filename) {
throw new \InvalidArgumentException("Either one of \$host or \$filename parameter must be provided.");
}

$isFile = $isFile || $filename || '/' === $host[0] || Vendor::SQLITE === $this->vendor;

// Fix an edge case where sqlite is not detected.
if ($isFile) {
if (!$filename) {
$filename = $database ?? $host;
$database = $host = null;
}
$isFile = true;
} else {
$database = $database ? \trim($database, '/') : null;
}

$this->isFile = $isFile;
$this->database = $database;
$this->filename = $filename;
$this->host = $host;
}

public static function fromString(#[\SensitiveParameter] string $dsn): self
Expand Down Expand Up @@ -98,21 +124,32 @@ public static function fromString(#[\SensitiveParameter] string $dsn): self
throw new \InvalidArgumentException('The database DSN is invalid.');
}

$database = $host = $filename = null;

if ($isFile) {
if (empty($params['path'])) {
throw new \InvalidArgumentException('The database DSN must contain a path when a targetting a local filename.');
}
$host = $params['path'];
$database = $params['path'];
$filename = $params['path'];
} else {
if (empty($params['host'])) {
throw new \InvalidArgumentException('The database DSN must contain a host (use "default" by default).');
}
if (empty($params['path'])) {
throw new \InvalidArgumentException('The database DSN must contain a database name.');
if (isset($params['path'])) {
// Strip only one '/', if path was absolute then this should
// take the form of "//some/path". This is weird and barely
// maintainable but tests are here to cover us.
$database = '/' === $params['path'][0] ? \substr($params['path'], 1) : $params['path'];
}
$host = $params['host'];
$database = \trim($params['path'], '/');
}

// parse_url() few edge cases.
if (':memory' === $host) {
$filename = ':memory:';
$host = null;
} else if ('/:memory:' === $filename) {
$filename = ':memory:';
}

$port = $params['port'] ?? null;
Expand All @@ -124,6 +161,7 @@ public static function fromString(#[\SensitiveParameter] string $dsn): self

return new self(
database: $database,
filename: $filename,
host: $host,
isFile: $isFile,
password: $password,
Expand All @@ -146,22 +184,22 @@ public function getVendor(): string

public function getHost(): string
{
return $this->host;
return $this->host ?? $this->filename;
}

public function isFile(): bool
{
return $this->isFile;
}

public function getDatabase(): ?string
public function getDatabase(): string
{
return $this->database;
return $this->database ?? 'default';
}

public function getFilename(): ?string
{
return $this->isFile ? $this->host : null;
return $this->filename;
}

public function getUser(): ?string
Expand Down
101 changes: 87 additions & 14 deletions tests/DsnTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,26 @@ public function testBasics(): void
self::assertNull($dsn->getOption('non_existing_option'));
}

public function testUserPassAreDecoded(): void
{
$dsn = Dsn::fromString('pdo_mysql://some%20user:some%[email protected]:1234/some_database');

self::assertSame('some user', $dsn->getUser());
self::assertSame('some password', $dsn->getPassword());
}

public function testNoSchemeRaiseException(): void
{
self::expectException(\InvalidArgumentException::class);
Dsn::fromString('some%20user:some%[email protected]/bla');
}

public function testNoHostRaiseException(): void
{
self::expectException(\InvalidArgumentException::class);
Dsn::fromString('oauth://some%20user:some%20password@?foo=bar');
}

public function testWithFilename(): void
{
$dsn = Dsn::fromString('sqlite:///some/path.db?server=sqlite-3');
Expand All @@ -34,33 +54,86 @@ public function testWithFilename(): void
self::assertSame('any', $dsn->getDriver());
self::assertSame('sqlite-3', $dsn->getOption('server'));
self::assertSame('/some/path.db', $dsn->getHost());
self::assertSame('/some/path.db', $dsn->getDatabase());
self::assertSame('/some/path.db', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());
}

public function testUserPassAreDecoded(): void
public function testSQLiteEdgeCaseRelative(): void
{
$dsn = Dsn::fromString('pdo_mysql://some%20user:some%[email protected]:1234/some_database');
$dsn = Dsn::fromString('pdo-sqlite://ignored:ignored@ignored:1234/somedb.sqlite');

self::assertSame('some user', $dsn->getUser());
self::assertSame('some password', $dsn->getPassword());
self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame('somedb.sqlite', $dsn->getFilename());
self::assertSame('somedb.sqlite', $dsn->getHost());
self::assertSame('default', $dsn->getDatabase());
}

public function testNoDatabaseRaiseException(): void
public function testSQLiteEdgeCaseAbsolute(): void
{
self::expectException(\InvalidArgumentException::class);
Dsn::fromString('pdo_mysql://some%20user:some%[email protected]');
$dsn = Dsn::fromString('pdo-sqlite://ignored:ignored@ignored:1234//somedb.sqlite');

self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame('/somedb.sqlite', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());
self::assertSame('/somedb.sqlite', $dsn->getHost());
}

public function testNoSchemeRaiseException(): void
public function testSQLiteEdgeCaseNoHostRelative(): void
{
self::expectException(\InvalidArgumentException::class);
Dsn::fromString('some%20user:some%[email protected]/bla');
$dsn = Dsn::fromString('pdo-sqlite://ignored:[email protected]');

self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame('somedb.sqlite', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());

self::assertSame('somedb.sqlite', $dsn->getHost());
}

public function testNoHostRaiseException(): void
public function testSQLiteEdgeCaseNoHostAbsolute(): void
{
self::expectException(\InvalidArgumentException::class);
Dsn::fromString('oauth://some%20user:some%20password@?foo=bar');
self::markTestSkipped("We sure cannot handle every typo error from our users.");

/*
$dsn = Dsn::fromString('pdo-sqlite://ignored:ignored@/somedb.sqlite');
self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame('/somedb.sqlite', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());
self::assertSame('/somedb.sqlite', $dsn->getHost());
*/
}

public function testSQLiteEdgeCaseMemoryRelative(): void
{
$dsn = Dsn::fromString('pdo-sqlite://:memory:');

self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame(':memory:', $dsn->getHost());
self::assertSame(':memory:', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());
}

public function testSQLiteEdgeCaseMemoryAbsolute(): void
{
$dsn = Dsn::fromString('pdo-sqlite:///:memory:');

self::assertTrue($dsn->isFile());
self::assertSame('sqlite', $dsn->getVendor());
self::assertSame('pdo', $dsn->getDriver());
self::assertSame(':memory:', $dsn->getHost());
self::assertSame(':memory:', $dsn->getFilename());
self::assertSame('default', $dsn->getDatabase());
}

public function testToUrl(): void
Expand Down

0 comments on commit 7b6293b

Please sign in to comment.