Skip to content

Path prefix fixes, directory exists fix, SqlServer compability #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Path prefix fixes, directory exists fix, SqlServer compability #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions schema/mssql.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
create table flysystem_files
(
id bigint identity primary key,
path nvarchar(255) not null,
type nvarchar(4) not null,
contents varbinary(max),
size int default 0 not null,
level int not null,
mimetype nvarchar(127),
visibility nvarchar(7) default 'public' not null,
timestamp int default 0 not null
);
50 changes: 25 additions & 25 deletions src/DoctrineDBALAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use DateTimeImmutable;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\ParameterType;
use Doctrine\DBAL\Platforms\SQLServerPlatform;
use Doctrine\DBAL\Types\Types;
use League\Flysystem\Config;
use League\Flysystem\DirectoryAttributes;
Expand Down Expand Up @@ -84,14 +85,14 @@ public function directoryExists(string $path): bool
private function exists(string $prefixedPath, string $type): bool
{
return (bool) $this->connection->executeQuery(<<<SQL
SELECT EXISTS (
SELECT CASE WHEN EXISTS (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXISTS already a boolean operator in SQL.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but for mssql that is inncorect syntax. You can check this on (https://sqliteonline.com/)

SELECT EXISTS (SELECT 1)

throws: Token error: 'Incorrect syntax near the keyword 'exists'.'

When on Sqlite, PostgreSql, MariaDb, Mysql works like a charm.
Case-When syntax should work on every engine.

But on Oracle still missing from DUAL. On this could be patched with simple

$this->connection->getDatabasePlatform()->getDummySelectSQL('CASE WHEN EXISTS(...`)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDummySelect wont work. E.g.: DB2.
Currently the bundle supports: sqlite, mysql , mariadb and pgsql.
If we need to add more platform, a refactor is needed to handle platform-specific queries i guess. Not just some if-else in the adapter.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package is great, I just miss support for SQLserver.I'm not intrested with others like Oracle or DB2.

I'm not familiar with DB2. Doctrine do sprintf('SELECT %s FROM sysibm.sysdummy1', $expression); for this platform.

SELECT
1
FROM {$this->table}
WHERE
path = :path AND
type = :type
)
) THEN 1 ELSE 0 END
SQL,
[
'path' => $prefixedPath,
Expand All @@ -116,9 +117,6 @@ public function write(string $path, string $contents, Config $config): void
$this->writeStream($path, $resource, $config);
}

/**
* {@inheritDoc}
*/
public function writeStream(string $path, $contents, Config $config): void
{
try {
Expand All @@ -130,31 +128,31 @@ public function writeStream(string $path, $contents, Config $config): void
* UPDATE if file exists
* INSERT if not
*/
$pathPrefixed = $this->prefixer->prefixPath($path);
if ($this->fileExists($path)) {
$path = $this->prefixer->prefixPath($path);
$this->connection->update($this->table,
[
'contents' => $contents,
'timestamp' => $config->get('timestamp', time()),
'visibility' => $config->get(Config::OPTION_VISIBILITY, Visibility::PUBLIC),
],
[
'path' => $path,
'path' => $pathPrefixed,
],
[
'contents' => Types::BINARY,
'timestamp' => Types::INTEGER,
]);
]
);
} else {
$path = $this->prefixer->prefixPath($path);
/* @var int|string $timestamp */
$this->connection->insert($this->table, [
'path' => $path,
'path' => $pathPrefixed,
'type' => self::TYPE_FILE,
'timestamp' => $config->get('timestamp', time()),
'level' => $this->directoryLevel($path),
'level' => $this->directoryLevel($pathPrefixed),
'contents' => $contents,
'mimetype' => $this->mimeTypeDetector->detectMimeType($path, $contents),
'mimetype' => $this->mimeTypeDetector->detectMimeType($pathPrefixed, $contents),
'visibility' => $config->get(Config::OPTION_VISIBILITY, Visibility::PUBLIC),
], [
'path' => ParameterType::STRING,
Expand All @@ -167,16 +165,23 @@ public function writeStream(string $path, $contents, Config $config): void
]);
}

$this->connection->executeStatement(<<<SQL
if ($this->connection->getDatabasePlatform() instanceof SQLServerPlatform) {
$lengthFnName = 'LEN';
} else {
$lengthFnName = 'LENGTH';
}

$this->connection->executeStatement(
<<<SQL
UPDATE
{$this->table}
SET
size = LENGTH(contents)
size = {$lengthFnName}(contents)
WHERE
path = :path
SQL,
[
'path' => $path,
'path' => $pathPrefixed,
],
[
'path' => ParameterType::STRING,
Expand Down Expand Up @@ -210,9 +215,6 @@ public function read(string $path): string
}
}

/**
* {@inheritDoc}
*/
public function readStream(string $path): mixed
{
try {
Expand Down Expand Up @@ -254,7 +256,7 @@ public function delete(string $path): void
->delete(
$this->table,
[
'path' => $path,
'path' => $this->prefixer->prefixPath($path),
'type' => self::TYPE_FILE,
],
);
Expand Down Expand Up @@ -406,8 +408,8 @@ public function listContents(string $path, bool $deep): iterable

try {
$queryBuilder = $this->connection->createQueryBuilder()
->from($this->table)
->select('path, size, mimetype, timestamp, type, visibility');
->from($this->table)
->select('path, size, mimetype, timestamp, type, visibility');

if (!empty($path)) {
$expressionBuilder = $this->connection->createExpressionBuilder();
Expand All @@ -430,10 +432,8 @@ public function listContents(string $path, bool $deep): iterable
ParameterType::INTEGER),
);
}
} else {
if (!$deep) {
$queryBuilder->andWhere('level = 0');
}
} elseif (!$deep) {
$queryBuilder->andWhere('level = 0');
}
$queryBuilder->orderBy('path', 'ASC');

Expand Down
27 changes: 27 additions & 0 deletions tests/DoctrineDBALAdapterWithPrefixTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace WGG\Flysystem\Doctrine\Tests;

use Doctrine\DBAL\DriverManager;
use League\Flysystem\AdapterTestUtilities\FilesystemAdapterTestCase;
use League\Flysystem\FilesystemAdapter;
use WGG\Flysystem\Doctrine\DoctrineDBALAdapter;

use function dirname;

/**
* @covers \WGG\Flysystem\Doctrine\DoctrineDBALAdapter
*/
class DoctrineDBALAdapterWithPrefixTest extends FilesystemAdapterTestCase
{
protected static function createFilesystemAdapter(): FilesystemAdapter
{
$connection = DriverManager::getConnection([
'url' => 'sqlite:///:memory:',
]);

$connection->executeStatement((string) file_get_contents(dirname(__DIR__).'/schema/sqlite.sql'));

return new DoctrineDBALAdapter(connection: $connection, prefix: 'prefix_test');
}
}