enl / flysystem-cloudinary Goto Github PK

There are list of functions which can possibly throw exceptions in Cloudinary API and Adapter MUST handle them internally. Sometimes it does not. I need to find all these places and fix it.

Hi,

The delete method isn't working for me. It seems to fail on the method where you normalizeMetaData as a result of calling the has method. The FlySystem delete method does the 'has' check before deleting.

The has method calls to the getMetadata method, which in turn grabs the resource via the Cloudinary API, and then the result of that is passed to the normalizeMetadata method.

The normalizeMetadata method receives a Cloudinary\Api\Response, but if that parameter is not an array, then the normalizeMetadata method simply returns false. This obviously rattles back up the chain and results in the has method returning false, meaning the file wasnt found, meaning the delete method doesnt get called.

Am I missing something here or is this a bug?

Thanks
Lee

Use this as a guide: http://keepachangelog.com/en/1.0.0/

Hi,

I'm using your package with Laravel Medialibrary, it would be interesting if the package had some classes to integrate with Laravel Medialibrary. Example: Utils\CloudinaryServiceProvider and Utils\CloudinaryUrlGenerator.

This options will be used to create transformed image when somebody calls Filesystem::read($path).

Since I cannot touch AdapterInterface which is just:

    /**
     * Read a file.
     *
     * @param string $path
     *
     * @return array|false
     */
    public function read($path);

I can only create this transformations option only for the whole Adapter.

I can just copy these parts from flysystem's travis config:

env:
  - COMPOSER_OPTS=""
  - COMPOSER_OPTS="--prefer-lowest"

install:
  - travis_retry composer update --prefer-dist $COMPOSER_OPTS

Cloudinary supports specifying the public HTTP endpoint for an image upload source. Your library assumes all uploaded images must come from a local source.

Hi

I have a strange issue where the image is not saving into Cloudinary. I'm getting back a URL from $this->photo->storeAs('folder', $filename, 'cloudinary'); but when accessing, I get a 404. Can you offer any advice as to where to try and troubleshoot?

I've tried through the SDK directly and all OK.

PHP: 7.4.8
"enl/flysystem-cloudinary": "~1.0"
Laravel: 7.21.0

First this notice is thrown:

PHP Notice:  Undefined index: files in /private/var/www/enl/flysystem-cloudinary/src/CloudinaryAdapter.php on line 229

and finally:

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to League\Flysystem\Util\ContentListingFormatter::formatListing() must be of the type array, null given, called in /private/var/www/enl/flysystem-cloudinary/vendor/league/flysystem/src/Filesystem.php on line 273 and defined in /private/var/www/enl/flysystem-cloudinary/vendor/league/flysystem/src/Util/ContentListingFormatter.php:38
Stack trace:
#0 /private/var/www/enl/flysystem-cloudinary/vendor/league/flysystem/src/Filesystem.php(273): League\Flysystem\Util\ContentListingFormatter->formatListing(NULL)
#1 /private/var/www/enl/flysystem-cloudinary/test.php(20): League\Flysystem\Filesystem->listContents('tests', true)
#2 {main}
  thrown in /private/var/www/enl/flysystem-cloudinary/vendor/league/flysystem/src/Util/ContentListingFormatter.php on line 38

Expected behavior:

return empty array, like local adapter does.

According to the interface, we can not define the expected behaviour of the deleteDir method, but by looking at the FTP adapter for instance, it seems like every files under the specified directory should be removed as well as the directory.

public function deleteDir($dirname)
{
    $connection = $this->getConnection();
    $contents = array_reverse($this->listDirectoryContents($dirname));
    foreach ($contents as $object) {
        if ($object['type'] === 'file') {
            if (! ftp_delete($connection, $object['path'])) {
                return false;
            }
        } elseif (! ftp_rmdir($connection, $object['path'])) {
            return false;
        }
    }
    return ftp_rmdir($connection, $dirname);
}

For Cloudinary, directories are not supported, so the current implementation is raising an Exception (which by the way is not declared in the interface docblock).

/**
 * Delete a directory.
 *
 * @param string $dirname
 * @return bool
 * @throws NotSupportedException
 */
public function deleteDir($dirname)
{
    throw new NotSupportedException('Cloudinary API does not support folders management.');
}

vs

/**
 * Delete a directory.
 *
 * @param string $dirname
 *
 * @return bool
 */
public function deleteDir($dirname);

The problem is when someone tries to use the adapter to replace another one, the exception will be thrown where one expects to have a return false; or see all the files under the directory be deleted.

I can make a PR but I first need to know what direction we should consider: either return false or delete all files from the path and return true. In the latter case, should we include an optional PSR-3 LoggerInterface using the LoggerAwareInterface and the LoggerAwareTrait? Optionally we could initialise the logger to NullLogger to avoid having to test whether a logger has been set.

Code coverage badge just lies: it shows the data from 2 years old build.

The idea is to filter out paths which look like file in a subfolder (e.g. contains "/" symbol) if $recursive is false.

This is necessary in order to get rid of unnecessary file writes in test cases when resource is needed.

PHPUnit of 5.* branch support goes to its end, so that we need consider upgrading to the new version. The problem is that new version of PHPUnit supports only php 7. So we will need to bump PHP version minimum to at least 7.0. This introduces BC-break so that it will be in 2.0 release whenever it will be released.

This seems to only work for images, as there is no way to set resource_type.

Seemingly, resource_type can be "auto", which might be sensible default so one could upload all types.

Thanks so much for this library!

Working great for me, except overwrite seems to have no effect.

I can upload, copy, move, and delete fine regardless of the setting.

However, If I try to upload a file that exists in Cloudinary (even with overwrite: true), I get a FileExistsException thrown by assertAbsent: https://github.com/thephpleague/flysystem/blob/master/src/Filesystem.php#L82

Any ideas?

I'm attempting to use this package with laravel-medialibrary in order to upload my videos to Cloudinary. However, after a lot of digging around and logging, it seems that this package assumes all files are images and offers no way to define the file as a video.

Is there any way around this?

We introduced this conversion a private function of CloudinaryAdapter, but this solution has several drawbacks:

1. First of all, exactly the same function will be necessary for Plugins (#16), since FilesystemInterface just do not have a function getAdapter to get adapter instance and make this conversion for plugin.
2. Secondly, this conversion is possible extension point, like it was mentioned by @cwey here: #17 (review)

So, as for me, the best solution is to introduce new class with pretty straight-forward interface:

interface PathConverterInterface
{
    public function toId(string $path): string;
    public function toPath(string $id): string;
}

In order not to break BC it will be better to add it to adapter as nullable parameter with internal instantiation of default converter which will simply execute exactly the same code as CloudinaryAdapter::pathToPublicId().

This was exactly what I was looking for, it would be awesome if you could add at least a short README.

Keep up the good work.

Hi,

Could you enable the project both on travis-ci and coveralls, so all the tests and coverage are automatically triggered?

I have already started to work on the configurations here: engineor/flysystem-cloudinary, and as soon as you enable both of them I'll be able to change the links and create a PR (style and ci config only for now).

Steps to replicate:

$client = new CloudinaryClient([
    'cloud_name' => 'your-cloudname-here',
    'api_key' => 'api-key',
    'api_secret' => 'You-know-what-to-do',
    'overwrite' => true, // set this to true if you want to overwrite existing files using $filesystem->write();
]);

$adapter = new CloudinaryAdapter($client);
$filesystem = new FileSystem($adapter);

$filesystem->write('test.png', file_get_contents('test.png');
$filesystem->write('test.png', file_get_contents('test.png');

This script will fail on the second call of write with File already exists at path: test.png.