diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php index 277c27069..b156a8425 100644 --- a/app/Http/Controllers/ImageController.php +++ b/app/Http/Controllers/ImageController.php @@ -170,7 +170,7 @@ class ImageController extends Controller * @param Request $request * @return \Illuminate\Contracts\Routing\ResponseFactory|\Illuminate\Http\JsonResponse|\Symfony\Component\HttpFoundation\Response */ - public function replaceDrawing(string $id, Request $request) + public function updateDrawing(string $id, Request $request) { $this->validate($request, [ 'image' => 'required|string' @@ -182,7 +182,7 @@ class ImageController extends Controller $this->checkOwnablePermission('image-update', $image); try { - $image = $this->imageRepo->replaceDrawingContent($image, $imageBase64Data); + $image = $this->imageRepo->updateDrawing($image, $imageBase64Data); } catch (ImageUploadException $e) { return response($e->getMessage(), 500); } diff --git a/app/Image.php b/app/Image.php index 30bbe21e2..ac94d9bf0 100644 --- a/app/Image.php +++ b/app/Image.php @@ -28,4 +28,15 @@ class Image extends Ownable { return $this->hasMany(ImageRevision::class); } + + /** + * Get the count of revisions made to this image. + * Based off numbers on revisions rather than raw count of attached revisions + * as they may be cleared up or revisions deleted at some point. + * @return int + */ + public function revisionCount() + { + return intval($this->revisions()->max('revision')); + } } diff --git a/app/Repos/ImageRepo.php b/app/Repos/ImageRepo.php index 245c0f27b..eff856872 100644 --- a/app/Repos/ImageRepo.php +++ b/app/Repos/ImageRepo.php @@ -160,9 +160,9 @@ class ImageRepo * @return Image * @throws \BookStack\Exceptions\ImageUploadException */ - public function replaceDrawingContent(Image $image, string $base64Uri) + public function updateDrawing(Image $image, string $base64Uri) { - return $this->imageService->replaceImageDataFromBase64Uri($image, $base64Uri); + return $this->imageService->updateImageFromBase64Uri($image, $base64Uri); } /** @@ -183,13 +183,14 @@ class ImageRepo /** - * Destroys an Image object along with its files and thumbnails. + * Destroys an Image object along with its revisions, files and thumbnails. * @param Image $image * @return bool + * @throws \Exception */ public function destroyImage(Image $image) { - $this->imageService->destroyImage($image); + $this->imageService->destroy($image); return true; } diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 3cfd61d27..d113b676a 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -166,7 +166,7 @@ class UserRepo // Delete user profile images $profileImages = $images = Image::where('type', '=', 'user')->where('created_by', '=', $user->id)->get(); foreach ($profileImages as $image) { - Images::destroyImage($image); + Images::destroy($image); } } diff --git a/app/Services/ImageService.php b/app/Services/ImageService.php index 06ef3a0f0..e83c1860b 100644 --- a/app/Services/ImageService.php +++ b/app/Services/ImageService.php @@ -2,12 +2,12 @@ use BookStack\Exceptions\ImageUploadException; use BookStack\Image; +use BookStack\ImageRevision; use BookStack\User; use Exception; use Intervention\Image\Exception\NotSupportedException; use Intervention\Image\ImageManager; use Illuminate\Contracts\Filesystem\Factory as FileSystem; -use Illuminate\Contracts\Filesystem\Filesystem as FileSystemInstance; use Illuminate\Contracts\Cache\Repository as Cache; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -83,28 +83,19 @@ class ImageService extends UploadService } /** - * Replace the data for an image via a Base64 encoded string. * @param Image $image * @param string $base64Uri * @return Image * @throws ImageUploadException */ - public function replaceImageDataFromBase64Uri(Image $image, string $base64Uri) + public function updateImageFromBase64Uri(Image $image, string $base64Uri) { $splitData = explode(';base64,', $base64Uri); if (count($splitData) < 2) { throw new ImageUploadException("Invalid base64 image data provided"); } $data = base64_decode($splitData[1]); - $storage = $this->getStorage(); - - try { - $storage->put($image->path, $data); - } catch (Exception $e) { - throw new ImageUploadException(trans('errors.path_not_writable', ['filePath' => $image->path])); - } - - return $image; + return $this->update($image, $data); } /** @@ -178,13 +169,57 @@ class ImageService extends UploadService } /** - * Get the storage path, Dependant of storage type. + * Update the content of an existing image. + * Uploaded the new image content and creates a revision for the old image content. * @param Image $image - * @return mixed|string + * @param $imageData + * @return Image + * @throws ImageUploadException */ - protected function getPath(Image $image) + private function update(Image $image, $imageData) { - return $image->path; + // Save image revision if not previously exists to ensure we always have + // a reference to the image files being uploaded. + if ($image->revisions()->count() === 0) { + $this->saveImageRevision($image); + } + + $pathInfo = pathinfo($image->path); + $revisionCount = $image->revisionCount() + 1; + $newFileName = preg_replace('/^(.+?)(-v\d+)?$/', '$1-v' . $revisionCount, $pathInfo['filename']); + + $image->path = str_replace_last($pathInfo['filename'], $newFileName, $image->path); + $image->url = $this->getPublicUrl($image->path); + $image->updated_by = user()->id; + + $storage = $this->getStorage(); + + try { + $storage->put($image->path, $imageData); + $storage->setVisibility($image->path, 'public'); + $image->save(); + $this->saveImageRevision($image); + } catch (Exception $e) { + throw new ImageUploadException(trans('errors.path_not_writable', ['filePath' => $image->path])); + } + return $image; + } + + /** + * Save a new revision for an image. + * @param Image $image + * @return ImageRevision + */ + protected function saveImageRevision(Image $image) + { + $revision = new ImageRevision(); + $revision->image_id = $image->id; + $revision->path = $image->path; + $revision->url = $image->url; + $revision->created_by = user()->id; + $revision->revision = $image->revisionCount() + 1; + $revision->save(); + return $revision; } /** @@ -194,7 +229,7 @@ class ImageService extends UploadService */ protected function isGif(Image $image) { - return strtolower(pathinfo($this->getPath($image), PATHINFO_EXTENSION)) === 'gif'; + return strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'gif'; } /** @@ -212,11 +247,11 @@ class ImageService extends UploadService public function getThumbnail(Image $image, $width = 220, $height = 220, $keepRatio = false) { if ($keepRatio && $this->isGif($image)) { - return $this->getPublicUrl($this->getPath($image)); + return $this->getPublicUrl($image->path); } $thumbDirName = '/' . ($keepRatio ? 'scaled-' : 'thumbs-') . $width . '-' . $height . '/'; - $imagePath = $this->getPath($image); + $imagePath = $image->path; $thumbFilePath = dirname($imagePath) . $thumbDirName . basename($imagePath); if ($this->cache->has('images-' . $image->id . '-' . $thumbFilePath) && $this->cache->get('images-' . $thumbFilePath)) { @@ -262,43 +297,58 @@ class ImageService extends UploadService */ public function getImageData(Image $image) { - $imagePath = $this->getPath($image); + $imagePath = $image->path; $storage = $this->getStorage(); return $storage->get($imagePath); } /** - * Destroys an Image object along with its files and thumbnails. + * Destroy an image along with its revisions, thumbnails and remaining folders. * @param Image $image - * @return bool * @throws Exception */ - public function destroyImage(Image $image) + public function destroy(Image $image) + { + // Destroy image revisions + foreach ($image->revisions as $revision) { + $this->destroyImagesFromPath($revision->path); + $revision->delete(); + } + + // Destroy main image + $this->destroyImagesFromPath($image->path); + $image->delete(); + } + + /** + * Destroys an image at the given path. + * Searches for image thumbnails in addition to main provided path.. + * @param string $path + * @return bool + */ + protected function destroyImagesFromPath(string $path) { $storage = $this->getStorage(); - $imageFolder = dirname($this->getPath($image)); - $imageFileName = basename($this->getPath($image)); + $imageFolder = dirname($path); + $imageFileName = basename($path); $allImages = collect($storage->allFiles($imageFolder)); + // Delete image files $imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) { $expectedIndex = strlen($imagePath) - strlen($imageFileName); return strpos($imagePath, $imageFileName) === $expectedIndex; }); - $storage->delete($imagesToDelete->all()); // Cleanup of empty folders - foreach ($storage->directories($imageFolder) as $directory) { + $foldersInvolved = array_merge([$imageFolder], $storage->directories($imageFolder)); + foreach ($foldersInvolved as $directory) { if ($this->isFolderEmpty($directory)) { $storage->deleteDirectory($directory); } } - if ($this->isFolderEmpty($imageFolder)) { - $storage->deleteDirectory($imageFolder); - } - $image->delete(); return true; } diff --git a/database/migrations/2018_05_13_090521_create_image_revisions_table.php b/database/migrations/2018_05_13_090521_create_image_revisions_table.php index 968773a86..d3032258f 100644 --- a/database/migrations/2018_05_13_090521_create_image_revisions_table.php +++ b/database/migrations/2018_05_13_090521_create_image_revisions_table.php @@ -16,6 +16,7 @@ class CreateImageRevisionsTable extends Migration Schema::create('image_revisions', function (Blueprint $table) { $table->increments('id'); $table->integer('image_id'); + $table->integer('revision'); $table->string('path'); $table->string('url'); $table->integer('created_by'); diff --git a/routes/web.php b/routes/web.php index 0efd19efe..40b00c75d 100644 --- a/routes/web.php +++ b/routes/web.php @@ -96,7 +96,7 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/base64/{id}', 'ImageController@getBase64Image'); Route::put('/update/{imageId}', 'ImageController@update'); Route::post('/drawing/upload', 'ImageController@uploadDrawing'); - Route::put('/drawing/upload/{id}', 'ImageController@replaceDrawing'); + Route::put('/drawing/upload/{id}', 'ImageController@updateDrawing'); Route::get('/usage/{id}', 'ImageController@usage'); Route::post('/{type}/upload', 'ImageController@uploadByType'); Route::get('/{type}/all', 'ImageController@getAllByType'); diff --git a/tests/ImageTest.php b/tests/ImageTest.php index 49912ec4c..fd1aa010e 100644 --- a/tests/ImageTest.php +++ b/tests/ImageTest.php @@ -210,7 +210,7 @@ class ImageTest extends TestCase $this->assertTrue($testImageData === $uploadedImageData, "Uploaded image file data does not match our test image as expected"); } - public function test_drawing_replacing() + public function test_drawing_updating() { $page = Page::first(); $editor = $this->getEditor(); @@ -235,6 +235,15 @@ class ImageTest extends TestCase 'updated_by' => $editor->id, ]); + // Check a revision has been created + $this->assertDatabaseHas('image_revisions', [ + 'image_id' => $image->id, + 'revision' => 2, + 'created_by' => $editor->id, + ]); + + $image = Image::find($image->id); + $this->assertTrue(file_exists(public_path($image->path)), 'Uploaded image not found at path: '. public_path($image->path)); $testImageData = file_get_contents($this->getTestImageFilePath());