Skip to content

Commit

Permalink
fix: FilesystemAdapter could not make 0777 directories (#93)
Browse files Browse the repository at this point in the history
  • Loading branch information
taka-oyama committed May 1, 2023
1 parent 4516732 commit b8613d8
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ Added

Fixed
- Fixed bug where running `Connection::statement` with DDLs was not logging and was not triggering events. (#86)
- FilesystemAdapter was not creating the directory for the cache file with proper permissions. (#93)

Changed
- Use google-cloud-php's CacheSessionPool since the [concerned bug](https://github.com/googleapis/google-cloud-php/issues/5567) has been fixed in [v1.53](https://github.com/googleapis/google-cloud-php-spanner/releases/tag/v1.58.2). (#90)
- Separate session pool and authentication per connection so transaction works properly. (#89)
- SessionPool and AuthCache now writes to `storage/framework/spanner/{$name}-{auth|session}`. (#93)

# v5.0.0

Expand Down
112 changes: 112 additions & 0 deletions src/FileCacheAdapter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php
/**
* Copyright 2019 Colopl Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Colopl\Spanner;

use Generator;
use RuntimeException;
use Symfony\Component\Cache\Adapter\AbstractAdapter;
use Symfony\Component\Cache\Marshaller\DefaultMarshaller;
use Symfony\Component\Cache\Marshaller\MarshallerInterface;
use Symfony\Component\Cache\PruneableInterface;
use Symfony\Component\Cache\Traits\FilesystemCommonTrait;
use Symfony\Component\Cache\Traits\FilesystemTrait;
use function is_dir;
use function mkdir;
use function scandir;
use function sprintf;
use function umask;
use const DIRECTORY_SEPARATOR;
use const SCANDIR_SORT_NONE;

class FileCacheAdapter extends AbstractAdapter implements PruneableInterface
{
use FilesystemTrait;

/**
* @var string
*/
protected string $name;

/**
* @param string $name
* @param string $directory
* @param MarshallerInterface|null $marshaller
*/
public function __construct(string $name, string $directory, MarshallerInterface $marshaller = null)
{
$this->marshaller = $marshaller ?? new DefaultMarshaller();
parent::__construct($name);
$this->name = $name;
$this->directory = $directory.DIRECTORY_SEPARATOR;
$this->ensureDirectory();
}

/**
* @return void
*/
protected function ensureDirectory(): void
{
if (!is_dir($this->directory)) {
// umask must be set to 0000 inorder to create cache directory with 0777 permission.
// writing as 0777 ensures that web-server user and cli user can both read/write to the same cache directory.
// This is important because we want to be able to warm up the cache from cli and use it from web-server.
$umask = umask(0);
@mkdir($this->directory, 0777, true);
umask($umask);
if (!is_dir($this->directory)) {
throw new RuntimeException(sprintf('Impossible to create the root directory "%s".', $this->directory));
}
}
}

/**
* OVERRIDE implementation from FilesystemCommonTrait since our directory structure differs from one provided
* @see FilesystemCommonTrait::getFile()
*
* @param string $id
* @param bool $mkdir
* @return string
*/
protected function getFile(string $id, bool $mkdir = false): string
{
if ($mkdir) {
$this->ensureDirectory();
}
return $this->directory.$this->name;
}

/**
* OVERRIDE implementation from FilesystemCommonTrait since our directory structure differs from one provided
* @see FilesystemCommonTrait::scanHashDir()
*
* @param string $directory
* @return Generator
*/
protected function scanHashDir(string $directory): Generator
{
if (!is_dir($directory)) {
return;
}

foreach (@scandir($directory, SCANDIR_SORT_NONE) ?: [] as $file) {
if ('.' !== $file && '..' !== $file) {
yield $directory . DIRECTORY_SEPARATOR . $file;
}
}
}
}
10 changes: 4 additions & 6 deletions src/SpannerServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
use Illuminate\Support\Facades\DB;
use Illuminate\Support\ServiceProvider;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Cache\Adapter\FilesystemAdapter;

class SpannerServiceProvider extends ServiceProvider
{
Expand Down Expand Up @@ -93,9 +92,8 @@ protected function parseConfig(array $config, string $name): array
*/
protected function createSessionPool(string $name, array $sessionPoolConfig): SessionPoolInterface
{
$cachePath = storage_path(implode(DIRECTORY_SEPARATOR, ['framework', 'spanner', $name]));
$adapter = new FilesystemAdapter('session', 0, $cachePath);
return new CacheSessionPool($adapter, $sessionPoolConfig);
$cachePath = $this->app->storagePath(implode(DIRECTORY_SEPARATOR, ['framework', 'spanner']));
return new CacheSessionPool(new FileCacheAdapter("{$name}-session", $cachePath), $sessionPoolConfig);
}

/**
Expand All @@ -104,8 +102,8 @@ protected function createSessionPool(string $name, array $sessionPoolConfig): Se
*/
protected function createAuthCache(string $name): CacheItemPoolInterface
{
$cachePath = storage_path(implode(DIRECTORY_SEPARATOR, ['framework', 'spanner', $name]));
return new FilesystemAdapter('auth', 0, $cachePath);
$cachePath = $this->app->storagePath(implode(DIRECTORY_SEPARATOR, ['framework', 'spanner']));
return new FileCacheAdapter("{$name}-auth", $cachePath);
}

protected function closeSessionAfterEachQueueJob(): void
Expand Down
21 changes: 19 additions & 2 deletions tests/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Event;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use function dirname;
use function fileperms;
use function mkdir;
use function sprintf;
use function substr;

class ConnectionTest extends TestCase
{
Expand Down Expand Up @@ -247,9 +252,15 @@ public function test_AuthCache_works(): void

public function test_AuthCache_with_FileSystemAdapter(): void
{
$this->app->useStoragePath('/tmp/laravel-spanner');

$conn = $this->getDefaultConnection();
$conn->select('SELECT 1');
self::assertDirectoryExists(storage_path("framework/spanner/{$conn->getName()}/auth"));

$outputPath = $this->app->storagePath("framework/spanner/{$conn->getName()}-auth");
self::assertFileExists($outputPath);
self::assertSame('0777', substr(sprintf('%o', fileperms(dirname($outputPath))), -4));
self::assertSame('0644', substr(sprintf('%o', fileperms($outputPath)), -4));
}

public function testSessionPool(): void
Expand All @@ -270,9 +281,15 @@ public function testSessionPool(): void

public function test_session_pool_with_FileSystemAdapter(): void
{
$this->app->useStoragePath('/tmp/laravel-spanner');

$conn = $this->getDefaultConnection();
$conn->select('SELECT 1');
self::assertDirectoryExists(storage_path("framework/spanner/{$conn->getName()}/session"));

$outputPath = $this->app->storagePath("framework/spanner/{$conn->getName()}-session");
self::assertFileExists($outputPath);
self::assertSame('0777', substr(sprintf('%o', fileperms(dirname($outputPath))), -4));
self::assertSame('0644', substr(sprintf('%o', fileperms($outputPath)), -4));
}

public function test_clearSessionPool(): void
Expand Down
3 changes: 3 additions & 0 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
use Illuminate\Foundation\Application;
use Ramsey\Uuid\Uuid;

/**
* @property Application $app
*/
abstract class TestCase extends \Orchestra\Testbench\TestCase
{
protected const TABLE_NAME_TEST = 'Test';
Expand Down

0 comments on commit b8613d8

Please sign in to comment.