Drawings now generate revisions, not replace

Updated drawing update test to accomodate.
Image deletion system now takes revisions into account.

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);
         }

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'));
+    }
 }

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;
     }
 

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);
         }
     }
 

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;
     }
 

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');

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');

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