Fixed issue where more images than expected could be deleted

When deleting images, images within the same directory, that have
a suffix of the delete image name, would also be deleted.

Added test to cover.
This commit is contained in:
Dan Brown 2020-07-24 23:41:59 +01:00
parent b383f5776d
commit b6aa232205
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
2 changed files with 31 additions and 7 deletions

View File

@ -223,6 +223,7 @@ class ImageService extends UploadService
$storage->setVisibility($thumbFilePath, 'public'); $storage->setVisibility($thumbFilePath, 'public');
$this->cache->put('images-' . $image->id . '-' . $thumbFilePath, $thumbFilePath, 60 * 60 * 72); $this->cache->put('images-' . $image->id . '-' . $thumbFilePath, $thumbFilePath, 60 * 60 * 72);
return $this->getPublicUrl($thumbFilePath); return $this->getPublicUrl($thumbFilePath);
} }
@ -292,11 +293,9 @@ class ImageService extends UploadService
/** /**
* Destroys an image at the given path. * Destroys an image at the given path.
* Searches for image thumbnails in addition to main provided path.. * Searches for image thumbnails in addition to main provided path.
* @param string $path
* @return bool
*/ */
protected function destroyImagesFromPath(string $path) protected function destroyImagesFromPath(string $path): bool
{ {
$storage = $this->getStorage(); $storage = $this->getStorage();
@ -306,8 +305,7 @@ class ImageService extends UploadService
// Delete image files // Delete image files
$imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) { $imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) {
$expectedIndex = strlen($imagePath) - strlen($imageFileName); return basename($imagePath) === $imageFileName;
return strpos($imagePath, $imageFileName) === $expectedIndex;
}); });
$storage->delete($imagesToDelete->all()); $storage->delete($imagesToDelete->all());

View File

@ -262,10 +262,11 @@ class ImageTest extends TestCase
$page = Page::first(); $page = Page::first();
$this->asAdmin(); $this->asAdmin();
$imageName = 'first-image.png'; $imageName = 'first-image.png';
$relPath = $this->getTestImagePath('gallery', $imageName);
$this->deleteImage($relPath);
$this->uploadImage($imageName, $page->id); $this->uploadImage($imageName, $page->id);
$image = Image::first(); $image = Image::first();
$relPath = $this->getTestImagePath('gallery', $imageName);
$delete = $this->delete( '/images/' . $image->id); $delete = $this->delete( '/images/' . $image->id);
$delete->assertStatus(200); $delete->assertStatus(200);
@ -278,6 +279,31 @@ class ImageTest extends TestCase
$this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has not been deleted as expected'); $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has not been deleted as expected');
} }
public function test_image_delete_does_not_delete_similar_images()
{
$page = Page::first();
$this->asAdmin();
$imageName = 'first-image.png';
$relPath = $this->getTestImagePath('gallery', $imageName);
$this->deleteImage($relPath);
$this->uploadImage($imageName, $page->id);
$this->uploadImage($imageName, $page->id);
$this->uploadImage($imageName, $page->id);
$image = Image::first();
$folder = public_path(dirname($relPath));
$imageCount = count(glob($folder . '/*'));
$delete = $this->delete( '/images/' . $image->id);
$delete->assertStatus(200);
$newCount = count(glob($folder . '/*'));
$this->assertEquals($imageCount - 1, $newCount, 'More files than expected have been deleted');
$this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has not been deleted as expected');
}
protected function getTestProfileImage() protected function getTestProfileImage()
{ {
$imageName = 'profile.png'; $imageName = 'profile.png';