Skip to content
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

StorageFailure and FileNotFound exceptions for Flysystem adapter #519

Merged
merged 1 commit into from
Jul 11, 2017
Merged

StorageFailure and FileNotFound exceptions for Flysystem adapter #519

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
127 changes: 123 additions & 4 deletions spec/Gaufrette/Adapter/FlysystemSpec.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
namespace spec\Gaufrette\Adapter;

use PhpSpec\ObjectBehavior;
use Prophecy\Argument;
use League\Flysystem\AdapterInterface;
use League\Flysystem\Config;
use League\Flysystem\FileNotFoundException;

class FlysystemSpec extends ObjectBehavior
{
Expand All @@ -31,11 +31,46 @@ function it_reads_file(AdapterInterface $adapter)
$this->read('filename')->shouldReturn('Hello.');
}

function it_throws_file_not_found_exception_when_trying_to_read_an_unexisting_file(AdapterInterface $adapter)
{
$adapter->read('filename')->willThrow(new FileNotFoundException('filename'));

$this->shouldThrow('Gaufrette\Exception\FileNotFound')->duringread('filename');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the lower "r" a mistake or is it due to the pretty old version of phpspec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks surprising but yes, it's because of an old version of phpspec (2.4). BTW I ran into issues when trying to run the phpspec tests : the phpspec.yml file has a dependency on Akeneo\SkipExampleExtension but I couldn't get it installed via composer (unresolvable set of packages). So I had to comment that file to run the tests on my machine 😕 . I'm still wondering how the tests can be executed on the CI .

It would be a good point to bump up the tests tools to their latest version 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

I ran into issues when trying to run the phpspec tests : the phpspec.yml file has a dependency on Akeneo\SkipExampleExtension but I couldn't get it installed via composer (unresolvable set of packages). So I had to comment that file to run the tests on my machine 😕

Mh, that's strange! The akeneo/phpspec-skip-example-extension package is in our composer.json for some time now. Was there any error message/stacktrace?

It would be a good point to bump up the tests tools to their latest version 😉

Yep, I already upgraded phpunit to 5.7 and there's a task about upgrading phpspec in the github project ;)

Copy link
Contributor Author

@nicolasmure nicolasmure Jul 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There was no error during composer install, but the akeneo directory was not present in the vendor. However, when trying to manually install the package, I got the following error :

$ composer require --dev akeneo/phpspec-skip-example-extension
Using version ^3.0@dev for akeneo/phpspec-skip-example-extension
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - akeneo/phpspec-skip-example-extension 3.0.x-dev requires phpspec/phpspec ^4.0 -> satisfiable by phpspec/phpspec[4.0.x-dev].
    - akeneo/phpspec-skip-example-extension v3.0.0-alpha1 requires phpspec/phpspec ^4.0 -> satisfiable by phpspec/phpspec[4.0.x-dev].
    - Conclusion: don't install phpspec/phpspec 4.0.x-dev
    - Installation request for akeneo/phpspec-skip-example-extension ^3.0@dev -> satisfiable by akeneo/phpspec-skip-example-extension[3.0.x-dev, v3.0.0-alpha1].


Installation failed, reverting ./composer.json to its original content.

I haven't noticed the phpspec upgrade card, I'll try to give it a look 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you try to delete your local composer.lock first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shame on me 🔨 removing the composer.lock file and running composer install again did the trick :) Thank you 👍

}

function it_turns_exception_into_storage_failure_while_reading_a_file(AdapterInterface $adapter)
{
$adapter->read('filename')->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringread('filename');
}

function it_throws_storage_failure_when_the_adapter_returns_an_error_value_when_reading_file(AdapterInterface $adapter)
{
$adapter->read('filename')->willReturn(false);

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringread('filename');
}

function it_writes_file(AdapterInterface $adapter, Config $config)
{
$adapter->write('filename', 'Hello.', $config)->willReturn(array());

$this->write('filename', 'Hello.')->shouldReturn(array());
$this->write('filename', 'Hello.')->shouldReturn(null);
}

function it_turns_exception_into_storage_failure_while_writing_a_file(AdapterInterface $adapter, Config $config)
{
$adapter->write('filename', 'Hello.', $config)->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringwrite('filename', 'Hello.');
}

function it_throws_storage_failure_when_the_adapter_returns_an_error_value_when_writing_file(AdapterInterface $adapter, Config $config)
{
$adapter->write('filename', 'Hello.', $config)->willReturn(false);

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringwrite('filename', 'Hello.');
}

function it_checks_if_file_exists(AdapterInterface $adapter)
Expand All @@ -45,6 +80,13 @@ function it_checks_if_file_exists(AdapterInterface $adapter)
$this->exists('filename')->shouldReturn(true);
}

function it_turns_exception_into_storage_failure_while_checking_if_file_exists(AdapterInterface $adapter)
{
$adapter->has('filename')->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringexists('filename');
}

function it_fetches_keys(AdapterInterface $adapter)
{
$adapter->listContents()->willReturn([[
Expand All @@ -57,6 +99,13 @@ function it_fetches_keys(AdapterInterface $adapter)
$this->keys()->shouldReturn(['folder']);
}

function it_turns_exception_into_storage_failure_while_fetching_keys(AdapterInterface $adapter)
{
$adapter->listContents()->willThrow(new \Exception('contents'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringkeys();
}

function it_lists_keys(AdapterInterface $adapter)
{
$adapter->listContents()->willReturn([[
Expand All @@ -72,25 +121,95 @@ function it_lists_keys(AdapterInterface $adapter)
]);
}

function it_turns_exception_into_storage_failure_while_listing_keys(AdapterInterface $adapter)
{
$adapter->listContents()->willThrow(new \Exception('contents'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringlistKeys();
}

function it_fetches_mtime(AdapterInterface $adapter)
{
$adapter->getTimestamp('filename')->willReturn(1457104978);

$this->mtime('filename')->shouldReturn(1457104978);
}

function it_throws_file_not_found_exception_when_trying_to_fetch_the_mtime_of_an_unexisting_file(AdapterInterface $adapter)
{
$adapter->getTimestamp('filename')->willThrow(new FileNotFoundException('filename'));

$this->shouldThrow('Gaufrette\Exception\FileNotFound')->duringmtime('filename');
}

function it_turns_exception_into_storage_failure_while_getting_file_mtime(AdapterInterface $adapter)
{
$adapter->getTimestamp('filename')->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringmtime('filename');
}

function it_throws_storage_failure_when_the_adapter_returns_an_error_value_when_getting_file_mtime(AdapterInterface $adapter)
{
$adapter->getTimestamp('filename')->willReturn(false);

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringmtime('filename');
}

function it_deletes_file(AdapterInterface $adapter)
{
$adapter->delete('filename')->willReturn(true);

$this->delete('filename')->shouldReturn(true);
$this->delete('filename')->shouldReturn(null);
}

function it_throws_file_not_found_exception_when_trying_to_delete_an_unexisting_file(AdapterInterface $adapter)
{
$adapter->delete('filename')->willThrow(new FileNotFoundException('filename'));

$this->shouldThrow('Gaufrette\Exception\FileNotFound')->duringdelete('filename');
}

function it_turns_exception_into_storage_failure_while_deleting_a_file(AdapterInterface $adapter)
{
$adapter->delete('filename')->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringdelete('filename');
}

function it_throws_storage_failure_when_the_adapter_returns_an_error_value_when_deleting_file(AdapterInterface $adapter)
{
$adapter->delete('filename')->willReturn(false);

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringdelete('filename');
}

function it_renames_file(AdapterInterface $adapter)
{
$adapter->rename('oldfilename', 'newfilename')->willReturn(true);

$this->rename('oldfilename', 'newfilename')->shouldReturn(true);
$this->rename('oldfilename', 'newfilename')->shouldReturn(null);
}

function it_throws_file_not_found_exception_when_trying_to_rename_an_unexisting_file(AdapterInterface $adapter)
{
$adapter->rename('oldfilename', 'newfilename')->willThrow(new FileNotFoundException('filename'));

$this->shouldThrow('Gaufrette\Exception\FileNotFound')->duringrename('oldfilename', 'newfilename');
}

function it_turns_exception_into_storage_failure_while_renaming_a_file(AdapterInterface $adapter)
{
$adapter->rename('oldfilename', 'newfilename')->willThrow(new \Exception('filename'));

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringrename('oldfilename', 'newfilename');
}

function it_throws_storage_failure_when_the_adapter_returns_an_error_value_when_renaming_file(AdapterInterface $adapter)
{
$adapter->rename('oldfilename', 'newfilename')->willReturn(false);

$this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringrename('oldfilename', 'newfilename');
}

function it_does_not_support_is_directory(AdapterInterface $adapter)
Expand Down
123 changes: 108 additions & 15 deletions src/Gaufrette/Adapter/Flysystem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
namespace Gaufrette\Adapter;

use Gaufrette\Adapter;
use Gaufrette\Exception\FileNotFound;
use Gaufrette\Exception\StorageFailure;
use Gaufrette\Exception\UnsupportedAdapterMethodException;
use League\Flysystem\AdapterInterface;
use League\Flysystem\FileNotFoundException;
use League\Flysystem\Util;

class Flysystem implements Adapter, ListKeysAware
Expand Down Expand Up @@ -34,33 +37,70 @@ public function __construct(AdapterInterface $adapter, $config = null)
*/
public function read($key)
{
return $this->adapter->read($key)['contents'];
try {
$result = $this->adapter->read($key);
} catch (\Exception $e) {
if ($e instanceof FileNotFoundException) {
throw new FileNotFound($key, $e->getCode(), $e);
}

throw StorageFailure::unexpectedFailure('read', ['key' => $key], $e);
}

if (false === $result) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So Flysystem adapters are less stricter than us? Funny :)

throw StorageFailure::unexpectedFailure('read', ['key' => $key]);
}

return $result['contents'];
}

/**
* {@inheritdoc}
*/
public function write($key, $content)
{
return $this->adapter->write($key, $content, $this->config);
try {
$result = $this->adapter->write($key, $content, $this->config);
} catch (\Exception $e) {
throw StorageFailure::unexpectedFailure(
'write',
['key' => $key, 'content' => $content],
$e
);
}

if (false === $result) {
throw StorageFailure::unexpectedFailure(
'write',
['key' => $key, 'content' => $content]
);
}
}

/**
* {@inheritdoc}
*/
public function exists($key)
{
return $this->adapter->has($key);
try {
return $this->adapter->has($key);
} catch (\Exception $e) {
throw StorageFailure::unexpectedFailure('exists', ['key' => $key], $e);
}
}

/**
* {@inheritdoc}
*/
public function keys()
{
return array_map(function ($content) {
return $content['path'];
}, $this->adapter->listContents());
try {
return array_map(function ($content) {
return $content['path'];
}, $this->adapter->listContents());
} catch (\Exception $e) {
throw StorageFailure::unexpectedFailure('keys', [], $e);
}
}

/**
Expand All @@ -71,14 +111,22 @@ public function listKeys($prefix = '')
$dirs = [];
$keys = [];

foreach ($this->adapter->listContents() as $content) {
if (empty($prefix) || 0 === strpos($content['path'], $prefix)) {
if ('dir' === $content['type']) {
$dirs[] = $content['path'];
} else {
$keys[] = $content['path'];
try {
foreach ($this->adapter->listContents() as $content) {
if (empty($prefix) || 0 === strpos($content['path'], $prefix)) {
if ('dir' === $content['type']) {
$dirs[] = $content['path'];
} else {
$keys[] = $content['path'];
}
}
}
} catch (\Exception $e) {
throw StorageFailure::unexpectedFailure(
'listKeys',
['prefix' => $prefix],
$e
);
}

return [
Expand All @@ -92,23 +140,68 @@ public function listKeys($prefix = '')
*/
public function mtime($key)
{
return $this->adapter->getTimestamp($key);
try {
$result = $this->adapter->getTimestamp($key);
} catch (\Exception $e) {
if ($e instanceof FileNotFoundException) {
throw new FileNotFound($key, $e->getCode(), $e);
}

throw StorageFailure::unexpectedFailure('mtime', ['key' => $key], $e);
}

if (false === $result) {
throw StorageFailure::unexpectedFailure('mtime', ['key' => $key]);
}

return $result;
}

/**
* {@inheritdoc}
*/
public function delete($key)
{
return $this->adapter->delete($key);
try {
$result = $this->adapter->delete($key);
} catch (\Exception $e) {
if ($e instanceof FileNotFoundException) {
throw new FileNotFound($key, $e->getCode(), $e);
}

throw StorageFailure::unexpectedFailure('delete', ['key' => $key], $e);
}

if (false === $result) {
throw StorageFailure::unexpectedFailure('delete', ['key' => $key]);
}
}

/**
* {@inheritdoc}
*/
public function rename($sourceKey, $targetKey)
{
return $this->adapter->rename($sourceKey, $targetKey);
try {
$result = $this->adapter->rename($sourceKey, $targetKey);
} catch (\Exception $e) {
if ($e instanceof FileNotFoundException) {
throw new FileNotFound($sourceKey, $e->getCode(), $e);
}

throw StorageFailure::unexpectedFailure(
'rename',
['sourceKey' => $sourceKey, 'targetKey' => $targetKey],
$e
);
}

if (false === $result) {
throw StorageFailure::unexpectedFailure(
'rename',
['sourceKey' => $sourceKey, 'targetKey' => $targetKey]
);
}
}

/**
Expand Down