Untested Updates Part Two - Now We Can Actually Update
Towards the end of the previous video we had a somewhat-ok, kinda working Wallpaper Edit feature.
We added in a Data Transformer and a custom form theme to help us fix a visual strangeness that we are getting via EasyAdminBundle. We saw that our upload process still worked - but only for the 'Create' new wallpaper journey.
We also saw that when we go to Edit an existing Wallpaper, we could edit all the text fields (and if there were number fields, or text areas, or any of the typical fields) and these updated in our database just fine.
Now we need to figure out why the image is not updating, and also... fix that problem :)
The most obvious place to start is the WallpaperUploadListener
. We've already covered that we need to use the prePersist
method call when Doctrine is pass over some insert
instructions to our database.
It's called prePersist
because it only affects insert
statements.
When updating, we need to use update
statements :)
Doctrine has our backs here. We can listen for the preUpdate
event. But that won't work unless we explicitly instruct it how to do so.
Pre Update
Our existing WallpaperUploadListener
implementation is as follows:
<?php
// src/AppBundle/Event/Listener/WallpaperUploadListener.php
namespace AppBundle\Event\Listener;
use AppBundle\Entity\Wallpaper;
use AppBundle\Service\FileMover;
use AppBundle\Service\WallpaperFilePathHelper;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;
class WallpaperUploadListener
{
/**
* @var FileMover
*/
private $fileMover;
/**
* @var WallpaperFilePathHelper
*/
private $wallpaperFilePathHelper;
public function __construct(FileMover $fileMover, WallpaperFilePathHelper $wallpaperFilePathHelper)
{
$this->fileMover = $fileMover;
$this->wallpaperFilePathHelper = $wallpaperFilePathHelper;
}
public function prePersist(LifecycleEventArgs $eventArgs)
{
$entity = $eventArgs->getEntity();
// if not Wallpaper entity, return false
if (false === $entity instanceof Wallpaper) {
return false;
}
/**
* @var $entity Wallpaper
*/
// get access to the file
$file = $entity->getFile();
$temporaryLocation = $file->getPathname();
$newFileLocation = $this->wallpaperFilePathHelper->getNewFilePath(
$file->getClientOriginalName()
);
// todo:
// - move the uploaded file
$this->fileMover->move($temporaryLocation, $newFileLocation);
// - update the entity with additional info
[
0 => $width,
1 => $height,
] = getimagesize($newFileLocation);
$entity
->setFilename(
$file->getClientOriginalName()
)
->setHeight($height)
->setWidth($width)
;
return true;
}
public function preUpdate(PreUpdateEventArgs $eventArgs)
{
}
}
We'd already stubbed out the preUpdate
event. That's handy.
Before we forget, let's add in a new entry for listening to the preUpdate
event, which in turn will call our preUpdate
method on our WallpaperUploadListener
:
# app/config/services.yml
services:
app.doctrine_event_listener.wallpaper_upload_listener:
class: AppBundle\Event\Listener\WallpaperUploadListener
arguments:
- "@app.service.local_filesystem_file_mover"
- "@app.service.wallpaper_file_path_helper"
tags:
- { name: doctrine.event_listener, event: prePersist }
- { name: doctrine.event_listener, event: preUpdate }
As we covered in earlier videos, we were really only concerned with prePersist
up until now.
As it stands though, most of what's in the WallpaperUploadListener::prePersist
method is going to be identical to the logic we want to also happen for preUpdate
. Let's extract that chunk out:
<?php
// src/AppBundle/Event/Listener/WallpaperUploadListener.php
namespace AppBundle\Event\Listener;
use AppBundle\Entity\Wallpaper;
use AppBundle\Service\FileMover;
use AppBundle\Service\WallpaperFilePathHelper;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\Event\PreUpdateEventArgs;
class WallpaperUploadListener
{
/**
* @var FileMover
*/
private $fileMover;
/**
* @var WallpaperFilePathHelper
*/
private $wallpaperFilePathHelper;
public function __construct(FileMover $fileMover, WallpaperFilePathHelper $wallpaperFilePathHelper)
{
$this->fileMover = $fileMover;
$this->wallpaperFilePathHelper = $wallpaperFilePathHelper;
}
public function prePersist(LifecycleEventArgs $eventArgs)
{
$entity = $eventArgs->getEntity();
$this->uploadFile($entity);
}
public function preUpdate(PreUpdateEventArgs $eventArgs)
{
/**
* same thing as prePersist, only slightly
* different syntax
*/
$this->uploadFile(
$eventArgs->getEntity()
);
}
private function uploadFile($entity)
{
// if not Wallpaper entity, return false
if (false === $entity instanceof Wallpaper) {
return false;
}
/**
* @var $entity Wallpaper
*/
// get access to the file
$file = $entity->getFile();
$temporaryLocation = $file->getPathname();
$newFileLocation = $this->wallpaperFilePathHelper->getNewFilePath(
$file->getClientOriginalName()
);
// todo:
// - move the uploaded file
$this->fileMover->move($temporaryLocation, $newFileLocation);
// - update the entity with additional info
[
0 => $width,
1 => $height,
] = getimagesize($newFileLocation);
$entity
->setFilename(
$file->getClientOriginalName()
)
->setHeight($height)
->setWidth($width)
;
return true;
}
}
Note that the uploadFile
method is private. The only way to upload a file is through one of the exposed public
methods.
There's a strange issue to address here though.
At this point you could feasibly think we have solved our problem, and that the Wallpaper image file should update as expected. Not so.
At least, not unless you follow a specific set of instructions.
Ok, so if we load up the site now and add a new wallpaper, everything should be good. The image should upload, the Wallpaper entity is created, the db is happy, life is good.
Then, we come to make a change to our Wallpaper. Let's say we uploaded the wrong image for whatever reason.
We edit the wallpaper and choose our new image. We don't change any other fields. Clicking 'Save changes' appears to work, but the image file itself is not changed.
Yet, if we go back in, choose a new image AND change the slug, then when we 'Save changes', superficially everything behaves as expected. The slug is changed. The new wallpaper is uploaded.
However, if you look closely you will notice that the Wallpaper we replaced is not actually deleted. It still resides on disk. And that's a problem as our image files won't be overwritten - so now we have ourselves an orphan to take care of.
Now, the issue causing the image to not update when uploaded in isolation is because from Doctrine's perspective, there are no changes to the Wallpaper entity, so the preUpdate
event is not being fired. We are unfortunately quite tightly coupled to Doctrine here, and are paying a price for this coupling.
There is a hack to fix this problem. We simply need to have a field that is updated on our entity in order for our wallpaper image to be uploaded as expected. Typically this is done using a timestamp field (think: createdAt
, or in our case, updatedAt
). The one reason this hack isn't so bad is that most of the time having an entity that has timestamps is actually a benefit. However, as we aren't tracking what update actually happened at any given timestamp (that's a different can of worms), the overall benefit here is quite minimal.
As adding timestamps to entities is such a common thing to do, there are inevitably a bunch of ways to do this. Here are two that I have covered in the past.
For the sake of simplicity here, I am going to opt for a very basic approach of adding a couple of properties to the Wallpaper
entity, and the associated getter
/ setter
combo:
<?php
// src/AppBundle/Entity/Wallpaper.php
namespace AppBundle\Entity;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;
/**
* Wallpaper
*
* @ORM\Table(name="wallpaper")
* @ORM\Entity(repositoryClass="AppBundle\Repository\WallpaperRepository")
*
* ---- IMPORTANT LINE HERE -----
* @ORM\HasLifecycleCallbacks()
* ------------------------------
*/
class Wallpaper
{
/**
* created Time/Date
*
* @var \DateTime
*
* @ORM\Column(name="created_at", type="datetime", nullable=false)
*/
private $createdAt;
/**
* updated Time/Date
*
* @var \DateTime
*
* @ORM\Column(name="updated_at", type="datetime", nullable=false)
*/
private $updatedAt;
// * snip *
/**
* Set createdAt
*
* @ORM\PrePersist
*/
public function setCreatedAt()
{
$this->createdAt = new \DateTime();
$this->updatedAt = new \DateTime();
}
/**
* Get createdAt
*
* @return \DateTime
*/
public function getCreatedAt()
{
return $this->createdAt;
}
/**
* Set updatedAt
*
* @ORM\PreUpdate
*/
public function setUpdatedAt()
{
$this->updatedAt = new \DateTime();
}
/**
* Get updatedAt
*
* @return \DateTime
*/
public function getUpdatedAt()
{
return $this->updatedAt;
}
}
I've only left in the additions.
What we will also do is manually call setUpdatedAt
whenever an UploadedFile
is set on our Wallpaper
entity:
// src/AppBundle/Entity/Wallpaper.php
class Wallpaper
{
// * snip *
public function setFile($file)
{
$this->file = $file;
if ($file) {
$this->setUpdatedAt();
}
return $this;
}
This way we can guarantee our entity has been updated, even if no "real" data has changed. The downside to this approach is that we only know when our entity was last updated. We don't know what changed. Again, that's a different discussion, so for the moment, let's just ignore that fact.
Note here that adding new properties to our entity has also brought in some annotations that talk of new database columns. Therefore, we need to generate a new migration:
php bin/console doctrine:migrations:diff
And then migrate:
php bin/console doctrine:migrations:migrate
Now we should be able to both add new, and edit existing wallpapers. This includes when we only want to change the uploaded image file.
There's two further tasks that remain.
First, we want to display a 'preview' of the image when editing. We already have this data, so it would be nice to use it.
Secondly, we need to handle the annoying situation of deleting an image file when we delete a Wallpaper entity.
Both of these tasks are up next.