Flag of Ukraine
SymfonyCasts stands united with the people of Ukraine
TRACK

Symfony 5 >

COURSE

All about Uploading Files in Symfony

Buy Access to Course

Flysystem: Streaming & Defensive Coding

Keep on Learning!

If you liked what you've learned so far, dive in!
Subscribe to get access to this tutorial plus
video, code and script downloads.

Start your All-Access Pass
Buy just this tutorial for $10.00
Previous Chapter

Next Chapter
Edit on GitHub

With a Subscription, click any sentence in the script to jump to that part of the video!

Login Subscribe

There are a few minor problems with our new Flysystem integration. Let's clean them up before they bite us!

Streaming

The first is that using file_get_contents() eats memory: it reads the entire contents of the file into PHP's memory. That's not a huge deal for tiny files, but it could be a big deal if you start uploading bigger stuff. And, it's just not necessary.

For that reason, in general, when you use Flysystem, instead of using methods like ->write() or ->update(), you should use ->writeStream() or ->updateStream().

53 lines src/Service/UploaderHelper.php
... lines 1 - 10
class UploaderHelper
{
... lines 13 - 24
public function uploadArticleImage(File $file): string
{
... lines 27 - 34
$this->filesystem->writeStream(
... lines 36 - 37
);
... lines 39 - 43
}
... lines 45 - 51
}

It works the same, except that we need to pass a stream instead of the contents. Create the stream with $stream = fopen($file->getPathname()) and, because we just need to read the file, use the r flag. Now, pass stream instead of the contents.

53 lines src/Service/UploaderHelper.php
... lines 1 - 10
class UploaderHelper
{
... lines 13 - 24
public function uploadArticleImage(File $file): string
{
... lines 27 - 33
$stream = fopen($file->getPathname(), 'r');
$this->filesystem->writeStream(
self::ARTICLE_IMAGE.'/'.$newFilename,
$stream
);
... lines 39 - 43
}
... lines 45 - 51
}

Yea... that's it! Same thing, but no memory issues. But we do need to add one more detail after: if is_resource($stream), then fclose($stream). The "if" is needed because some Flysystem adapters close the stream by themselves.

53 lines src/Service/UploaderHelper.php
... lines 1 - 10
class UploaderHelper
{
... lines 13 - 24
public function uploadArticleImage(File $file): string
{
... lines 27 - 33
$stream = fopen($file->getPathname(), 'r');
$this->filesystem->writeStream(
self::ARTICLE_IMAGE.'/'.$newFilename,
$stream
);
if (is_resource($stream)) {
fclose($stream);
}
... lines 42 - 43
}
... lines 45 - 51
}

Deleting the Old File

Ok, for problem number two, go back to /admin/article. Log back in with password engage, edit an article, and go select an image - how about astronaut.jpg. Hit update and... it works! So what's the problem? Well, we just replaced an existing image with this new one. Does the old file still exist in our uploads directory? Absolutely! But it probably shouldn't. When an article image is updated, let's delete the old file.

In UploaderHelper, add a second argument - a nullable string argument called $existingFilename.

57 lines src/Service/UploaderHelper.php
... lines 1 - 10
class UploaderHelper
{
... lines 13 - 24
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 27 - 47
}
... lines 49 - 55
}

This is nullable because sometimes there may not be an existing file to delete. At the bottom, it's beautifully simple: if an $existingFilename was passed, then $this->filesystem->delete() and pass that the full path, which will be self::ARTICLE_IMAGE.'/'.$existingFilename.

57 lines src/Service/UploaderHelper.php
... lines 1 - 10
class UploaderHelper
{
... lines 13 - 24
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 27 - 42
if ($existingFilename) {
$this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
}
... lines 46 - 47
}
... lines 49 - 55
}

Done! You can see the astronaut file that we're using right now. Oh, but first, head over to ArticleAdminController: we need to pass this new argument. Let's see - this is the edit() action - so pass $article->getImageFilename().

126 lines src/Controller/ArticleAdminController.php
... lines 1 - 17
class ArticleAdminController extends BaseController
{
... lines 20 - 57
public function edit(Article $article, Request $request, EntityManagerInterface $em, UploaderHelper $uploaderHelper)
{
... lines 60 - 64
if ($form->isSubmitted() && $form->isValid()) {
... lines 66 - 67
if ($uploadedFile) {
$newFilename = $uploaderHelper->uploadArticleImage($uploadedFile, $article->getImageFilename());
... line 70
}
... lines 72 - 80
}
... lines 82 - 85
}
... lines 87 - 124
}

In new(), you can really just pass null - there will not be an article image. But I'll pass getImageFilename() to be consistent.

126 lines src/Controller/ArticleAdminController.php
... lines 1 - 17
class ArticleAdminController extends BaseController
{
... lines 20 - 23
public function new(EntityManagerInterface $em, Request $request, UploaderHelper $uploaderHelper)
{
... lines 26 - 28
if ($form->isSubmitted() && $form->isValid()) {
... lines 30 - 35
if ($uploadedFile) {
$newFilename = $uploaderHelper->uploadArticleImage($uploadedFile, $article->getImageFilename());
... line 38
}
... lines 40 - 46
}
... lines 48 - 51
}
... lines 53 - 124
}

Oh, and there's one other place we need update: ArticleFixtures. Down here, just pass null: we are never updating.

102 lines src/DataFixtures/ArticleFixtures.php
... lines 1 - 13
class ArticleFixtures extends BaseFixture implements DependentFixtureInterface
{
... lines 16 - 90
private function fakeUploadImage(): string
{
... lines 93 - 97
return $this->uploaderHelper
->uploadArticleImage(new File($targetPath), null);
}
}

Try it! Here is the current astronaut image. Now, move over, upload rocket.jpg this time and update! Back in the directory... there's rocket and astronaut is gone! Love it!

Avoiding Errors

In a perfect system, the existing file will always exist, right? I mean, how could a filename get set on the entity... without being uploaded? Well, what if we're developing locally... and maybe we clear out the uploads directory to test something - or we clear out the uploads directory in our automated tests. What would happen?

Let's find it! Empty uploads/. Back in our browser, the image preview still shows up because this is rendering a thumbnail file - which we didn't delete - but the original image is totally gone. Select earth.jpeg, update and... it fails! It fails on $this->filesystem->delete().

This may be the behavior you want: if something weird happens and the old file is gone, please explode so that I know. But, I'm going to propose something slightly less hardcore. If the old file doesn't exist for some reason, I don't want the entire process to fail... it really doesn't need to.

The error from Flysystem is a FileNotFoundException from League\Flysystem. In UploaderHelper wrap that line in a try-catch. Let's catch that FileNotFoundException - the one from League\Flysystem

66 lines src/Service/UploaderHelper.php
... lines 1 - 5
use League\Flysystem\FileNotFoundException;
... lines 7 - 12
class UploaderHelper
{
... lines 15 - 29
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 32 - 47
if ($existingFilename) {
try {
$this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
} catch (FileNotFoundException $e) {
... line 52
}
}
... lines 55 - 56
}
... lines 58 - 64
}

Logging Problems

That'll fix that problem... but I don't love doing this. Honestly, I hate silencing errors. One of the benefits of throwing an exception is that we can configure Symfony to notify us of errors via the logger. At SymfonyCasts, we send all errors to a Slack channel so we know if something weird is going on... not that we ever have bugs. Pfff.

Here's what I propose: a soft failure: we don't fail, but we do log that an error happened. Back on the constructor, autowire a new argument: LoggerInterface $logger. I'll hit Alt + Enter and select initialize fields to create that property and set it.

66 lines src/Service/UploaderHelper.php
... lines 1 - 7
use Psr\Log\LoggerInterface;
... lines 9 - 12
class UploaderHelper
{
... lines 15 - 20
private $logger;
public function __construct(FilesystemInterface $publicUploadsFilesystem, RequestStackContext $requestStackContext, LoggerInterface $logger)
{
... lines 25 - 26
$this->logger = $logger;
}
... lines 29 - 64
}

Now, down in the catch, say $this->logger->alert() - alert is one of the highest log levels and I usually send all logs that are this level or higher to a Slack channel. Inside, how about: "Old uploaded file %s was missing when trying to delete" - and pass $existingFilename.

66 lines src/Service/UploaderHelper.php
... lines 1 - 12
class UploaderHelper
{
... lines 15 - 29
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 32 - 47
if ($existingFilename) {
try {
$this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
} catch (FileNotFoundException $e) {
$this->logger->alert(sprintf('Old uploaded file "%s" was missing when trying to delete', $existingFilename));
}
}
... lines 55 - 56
}
... lines 58 - 64
}

Thanks to this, the user gets a smooth experience, but we get notified so we can figure out how the heck the old file disappeared.

Move over and re-POST the form. Now it works. And to prove the log worked, check out the terminal tab where we're running the Symfony web server: it's streaming all of our logs here. Scroll up and... there it is!

Old uploaded file "rocket..." was missing when trying to delete

Checking for Filesystem Failure

Ok, there's one more thing I want to tighten up. If one of the calls to the Filesystem object fails... what do you think will happen? An exception? Hold Command or Ctrl and click on writeStream(). Check out the docs: we will get an exception if we pass an invalid stream or if the file already exists. But for any other type of failure, maybe a network error... instead of an exception, the method just returns false!

Actually, that's not completely true - it depends on your adapter. For example, if you're using the S3 adapter and there's a network error, it may throw its own type of exception. But the point is this: if any of the Filesystem methods fail, you might not get an exception: it might just return false.

For that reason, I like to code defensively. Assign this to a $result variable.

75 lines src/Service/UploaderHelper.php
... lines 1 - 12
class UploaderHelper
{
... lines 15 - 29
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 32 - 39
$result = $this->filesystem->writeStream(
... lines 41 - 42
);
... lines 44 - 65
}
... lines 67 - 73
}

Then say: if ($result === false), let's throw our own exception - I do want to know that something failed:

Could not write uploaded file "%s"

and pass $newFilename.

75 lines src/Service/UploaderHelper.php
... lines 1 - 12
class UploaderHelper
{
... lines 15 - 29
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 32 - 39
$result = $this->filesystem->writeStream(
... lines 41 - 42
);
... line 44
if ($result === false) {
throw new \Exception(sprintf('Could not write uploaded file "%s"', $newFilename));
}
... lines 48 - 65
}
... lines 67 - 73
}

Copy that and do the same for delete:

Could not delete old uploaded file "%s"

with $existingFilename.

75 lines src/Service/UploaderHelper.php
... lines 1 - 12
class UploaderHelper
{
... lines 15 - 29
public function uploadArticleImage(File $file, ?string $existingFilename): string
{
... lines 32 - 52
if ($existingFilename) {
try {
$result = $this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
if ($result === false) {
throw new \Exception(sprintf('Could not delete old uploaded file "%s"', $existingFilename));
}
} catch (FileNotFoundException $e) {
... line 61
}
}
... lines 64 - 65
}
... lines 67 - 73
}

I'm throwing this error instead of just logging something because this would truly be an exceptional case - we shouldn't let things continue. But, it's your call.

Let's make sure this all works: move over and select the stars file - or... actually the "Earth from Moon" photo. Update and... got it!

Next: let's teach LiipImagineBundle to play nice with Flysytem. After all, if we move Flysystem to S3, but LiipImagineBundle is still looking for the source files locally... well... we're not going to have a great time.

Leave a comment!

Most Useful Newest Oldest
9
Login or Register to join the conversation
Kirill Avatar

Hey! You've noticed about symfonycasts where you have sent error logs to Slack, can you please give the name or link of this tutorial? Thanks!

Reply
MolloKhan Avatar

Hey Kirill,

I believe this is the video you're looking for https://symfonycasts.com/screencast/symfony4-fundamentals/logger-trait
By the way, at the top of the page, there's a search box where you can look for specific content just in this tutorial or through the whole site

Cheers!

1 Reply
Kiuega Avatar
Kiuega Avatar Kiuega | posted 2 years ago | edited

Hello, since we need to use FlysystemOperator, the "write" and "delete" methods return "void". Because of this, we are unable to run these snippets:


       $result = $this->filesystem->writeStream(
            self::ARTICLE_IMAGE.'/'.$newFilename,
            $stream
        );
        
        if ($result === false) {
            throw new \Exception(sprintf('Could not write uploaded file "%s"', $newFilename));
        }

        if (is_resource($stream)) {
            fclose($stream);
        }

        if ($existingFilename) {
            try {
                $result = $this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
                if ($result === false) {
                    throw new \Exception(sprintf('Could not delete old uploaded file "%s"', $existingFilename));
                }
            } catch (FileNotFoundException $e) {
                $this->logger->alert(sprintf('Old uploaded file "%s" was missing when trying to delete', $existingFilename));
            }
        }

We therefore cannot apply the "defensive" strategy demonstrated in the video. Is there an alternative?

I replaced it by :


       try {
            $this->filesystem->writeStream(
                self::ARTICLE_IMAGE.'/'.$newFilename,
                $stream
            );
        } catch (FilesystemException $e) {
            throw new \Exception($e->getMessage());
        }

        if (is_resource($stream)) {
            fclose($stream);
        }

        if ($existingFilename) {
            try {
                $this->filesystem->delete(self::ARTICLE_IMAGE.'/'.$existingFilename);
            } catch (FileNotFoundException $e) {
                $this->logger->alert(sprintf('Old uploaded file "%s" was missing when trying to delete', $existingFilename));
            }
        }

What do you think ?

Reply
weaverryan Avatar

Hey Kiuega !

Ah yes - a difference between Flysystem v1 and v2. I need to see if we need some notes to the video related to this!

I think what you're referring to, specifically, is this: https://flysystem.thephpleague.com/v2/docs/what-is-new/#no-more-success-result-booleans

So, in v2, life is actually simpler. You don't need to code as defensively because the operation will work or it will NOT work and you will get an exception (without us needing to if it worked).

The code you put above, I think, is close - but we can simplify a bit more :).

A) For the writeStream, there's no need to have a try/catch around this. Just call the method. Either it will work, or an exception will be thrown. So, it will have the same behavior that you see now (a different exception instance will be thrown, but that's fine).

B) For the delete, you technically don't need the try/catch here either, unless you wanted to avoid failing (500 error) if the file is not found. If that's what you want to accomplish, then your code is perfect: you're catching just that one type of error. I don't think you need the if ($existingFilename) part, but that's minor - and maybe there is more to your code that I'm not seeing.

Cheers!

1 Reply
sadikoff Avatar

Hey Kiuega

Looks like you did a good investigation! The code you did is a good alternative! Sorry that you got difficulties with this part of course.

Cheers!

1 Reply
Cameron Avatar
Cameron Avatar Cameron | posted 2 years ago

What is meant by "some streams close themselves"? At -7.49? Why are we closing something that already closes itself?

Reply
Victor Avatar

Hey Cameron,

> Why are we closing something that already closes itself?

Actually, it's the reverse... Ryan says:

> The "if" is needed because some Flysystem adapters close the stream by themselves.

So we need that "if" to find *those* steams that were not closed by themselves... and if they were not - we close it ourselves in that if :)

Makes sense for you now?

Cheers!

1 Reply
Krzysztof K. Avatar
Krzysztof K. Avatar Krzysztof K. | posted 4 years ago

I have noticed that Exception for the stream write result is thrown before we check if stream was closed, does it matter? Maybe we should throw it after?

Reply
MolloKhan Avatar

Hey Krzysztof K.

That's a good question. I'm not sure if after throwing an error the resource will be closed automatically, or if it may introduce any bugs. So, if you want to play defensively you may want to close it first and then check for errors.

Cheers!

Reply
Cat in space

"Houston: no signs of life"
Start the conversation!

This tutorial is built on Symfony 4 but works great in Symfony 5!
Symfony 4

What PHP libraries does this tutorial use?

// composer.json
{
    "require": {
        "php": "^7.1.3",
        "ext-iconv": "*",
        "aws/aws-sdk-php": "^3.87", // 3.87.10
        "composer/package-versions-deprecated": "^1.11", // 1.11.99
        "knplabs/knp-markdown-bundle": "^1.7", // 1.7.1
        "knplabs/knp-paginator-bundle": "^2.7", // v2.8.0
        "knplabs/knp-time-bundle": "^1.8", // 1.9.0
        "league/flysystem-aws-s3-v3": "^1.0", // 1.0.22
        "league/flysystem-cached-adapter": "^1.0", // 1.0.9
        "liip/imagine-bundle": "^2.1", // 2.1.0
        "nexylan/slack-bundle": "^2.0,<2.2.0", // v2.1.0
        "oneup/flysystem-bundle": "^3.0", // 3.0.3
        "php-http/guzzle6-adapter": "^1.1", // v1.1.1
        "sensio/framework-extra-bundle": "^5.1", // v5.2.4
        "stof/doctrine-extensions-bundle": "^1.3", // v1.3.0
        "symfony/asset": "^4.0", // v4.2.3
        "symfony/console": "^4.0", // v4.2.3
        "symfony/flex": "^1.9", // v1.17.6
        "symfony/form": "^4.0", // v4.2.3
        "symfony/framework-bundle": "^4.0", // v4.2.3
        "symfony/orm-pack": "^1.0", // v1.0.6
        "symfony/security-bundle": "^4.0", // v4.2.3
        "symfony/serializer-pack": "^1.0", // v1.0.2
        "symfony/twig-bundle": "^4.0", // v4.2.3
        "symfony/validator": "^4.0", // v4.2.3
        "symfony/web-server-bundle": "^4.0", // v4.2.3
        "symfony/yaml": "^4.0", // v4.2.3
        "twig/extensions": "^1.5" // v1.5.4
    },
    "require-dev": {
        "doctrine/doctrine-fixtures-bundle": "^3.0", // 3.1.0
        "easycorp/easy-log-handler": "^1.0.2", // v1.0.7
        "fzaninotto/faker": "^1.7", // v1.8.0
        "symfony/debug-bundle": "^3.3|^4.0", // v4.2.3
        "symfony/dotenv": "^4.0", // v4.2.3
        "symfony/maker-bundle": "^1.0", // v1.11.3
        "symfony/monolog-bundle": "^3.0", // v3.3.1
        "symfony/phpunit-bridge": "^3.3|^4.0", // v4.2.3
        "symfony/profiler-pack": "^1.0", // v1.0.4
        "symfony/var-dumper": "^3.3|^4.0" // v4.2.3
    }
}
Edit on GitHub
Previous Chapter
Next Chapter
userVoice