Buy
Buy

Flysystem: Streaming & Defensive Coding

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().

... 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.

... 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.

... 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.

... 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.

... 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().

... 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.

... 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.

... 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

... 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.

... 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.

... 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.

... 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.

... 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.

... 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!