Entities: Updated repos to act on refreshed clones

Changes to core entity models are now done on clones to ensure clean
state before save, and those clones are returned back if changes are
needed after that action.

Author: ssddanbrown

app/Entities/Controllers/ChapterController.php

@@ -130,7 +130,7 @@ public function update(Request $request, string $bookSlug, string $chapterSlug)
         $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug);
         $this->checkOwnablePermission(Permission::ChapterUpdate, $chapter);
 
-        $this->chapterRepo->update($chapter, $validated);
+        $chapter = $this->chapterRepo->update($chapter, $validated);
 
         return redirect($chapter->getUrl());
     }

app/Entities/Models/Book.php

@@ -59,6 +59,9 @@ public function getBookCover(int $width = 440, int $height = 250): string
         }
     }
 
+    // TODO - Still handle cover as relation through containerData (since it's used in code)
+    // TODO - Remove above since we can access that via containerData
+
     /**
      * Get the Page that is used as default template for newly created pages within this Book.
      */

app/Entities/Models/BookChild.php

@@ -3,7 +3,6 @@
 namespace BookStack\Entities\Models;
 
 use BookStack\References\ReferenceUpdater;
-use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
 /**
@@ -27,25 +26,25 @@ public function book(): BelongsTo
     /**
      * Change the book that this entity belongs to.
      */
-    public function changeBook(int $newBookId): Entity
+    public function changeBook(int $newBookId): self
     {
-        $oldUrl = $this->getUrl();
-        $this->book_id = $newBookId;
-        $this->refreshSlug();
-        $this->save();
-        $this->refresh();
+        $altered = $this->clone()->refresh();
+        $oldUrl = $altered->getUrl();
+        $altered->book_id = $newBookId;
+        $altered->refreshSlug();
+        $altered->save();
 
-        if ($oldUrl !== $this->getUrl()) {
-            app()->make(ReferenceUpdater::class)->updateEntityReferences($this, $oldUrl);
+        if ($oldUrl !== $altered->getUrl()) {
+            app()->make(ReferenceUpdater::class)->updateEntityReferences($altered, $oldUrl);
         }
 
         // Update all child pages if a chapter
-        if ($this instanceof Chapter) {
-            foreach ($this->pages()->withTrashed()->get() as $page) {
+        if ($altered instanceof Chapter) {
+            foreach ($altered->pages()->withTrashed()->get() as $page) {
                 $page->changeBook($newBookId);
             }
         }
 
-        return $this;
+        return $altered;
     }
 }

app/Entities/Models/Bookshelf.php

@@ -59,6 +59,9 @@ public function getBookCover(int $width = 440, int $height = 250): string
         }
     }
 
+    // TODO - Still handle cover as relation through containerData (since it's used in code)
+    // TODO - Remove above since we can access that via containerData
+
     /**
      * Check if this shelf contains the given book.
      */

app/Entities/Models/Entity.php

@@ -101,7 +101,7 @@ public function shouldHaveContainerData(): bool
     }
 
     /**
-     * Get the container-specific data for this page.
+     * Get the container-specific data for this item.
      */
     public function containerData(): HasOne
     {

app/Entities/Models/EntityContainerData.php

@@ -4,6 +4,7 @@
 
 use BookStack\Uploads\Image;
 use BookStack\Util\HtmlContentFilter;
+use Exception;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\HasOne;
 
@@ -27,6 +28,22 @@ public function cover(): HasOne
         return $this->hasOne(Image::class, 'image_id');
     }
 
+    /**
+     * Returns a shelf cover image URL, if cover not exists return default cover image.
+     */
+    public function getCoverUrl(int $width = 440, int $height = 250, string|null $default = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='): string|null
+    {
+        if (!$this->image_id) {
+            return $default;
+        }
+
+        try {
+            return $this->cover->getThumb($width, $height, false) ?? $default;
+        } catch (Exception $err) {
+            return $default;
+        }
+    }
+
     /**
      * Check if this data supports having a default template assigned.
      */
@@ -46,7 +63,7 @@ public function supportsCoverImage(): bool
     /**
      * Get the description as a cleaned/handled HTML string.
      */
-    public function descriptionHtml(bool $raw = false): string
+    public function getDescriptionHtml(bool $raw = false): string
     {
         $html = $this->description_html ?: '<p>' . nl2br(e($this->description)) . '</p>';
         if ($raw) {
@@ -69,7 +86,7 @@ public function setDescriptionHtml(string $html, string|null $plaintext = null):
         }
 
         if (empty($html) && !empty($plaintext)) {
-            $this->description_html = $this->descriptionHtml();
+            $this->description_html = $this->getDescriptionHtml();
         }
     }
 }

app/Entities/Repos/BaseRepo.php

@@ -3,9 +3,7 @@
 namespace BookStack\Entities\Repos;
 
 use BookStack\Activity\TagRepo;
-use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\BookChild;
-use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\EntityContainerData;
 use BookStack\Entities\Queries\PageQueries;
@@ -31,9 +29,13 @@ public function __construct(
 
     /**
      * Create a new entity in the system.
+     * @template T of Entity
+     * @param T $entity
+     * @return T
      */
-    public function create(Entity $entity, array $input): void
+    public function create(Entity $entity, array $input): Entity
     {
+        $entity = $entity->clone()->refresh();
         $entityInput = array_intersect_key($input, ['name', 'priority']);
         $entity->forceFill($entityInput);
         $entity->forceFill([
@@ -59,13 +61,19 @@ public function create(Entity $entity, array $input): void
         $entity->indexForSearch();
 
         $this->referenceStore->updateForEntity($entity);
+
+        return $entity;
     }
 
     /**
      * Update the given entity.
+     * @template T of Entity
+     * @param T $entity
+     * @return T
      */
-    public function update(Entity $entity, array $input): void
+    public function update(Entity $entity, array $input): Entity
     {
+        $entity = $entity->clone()->refresh();
         $oldUrl = $entity->getUrl();
 
         $entity->fill($input);
@@ -78,6 +86,7 @@ public function update(Entity $entity, array $input): void
         $entity->save();
         if ($entity->shouldHaveContainerData() && $entity->containerData) {
             $this->updateContainerDescription($entity->containerData, $input);
+            $entity->containerData->save();
         }
 
         if (isset($input['tags'])) {
@@ -91,6 +100,8 @@ public function update(Entity $entity, array $input): void
         if ($oldUrl !== $entity->getUrl()) {
             $this->referenceUpdater->updateEntityReferences($entity, $oldUrl);
         }
+
+        return $entity;
     }
 
     /**
@@ -147,7 +158,7 @@ public function updateDefaultTemplate(EntityContainerData $containerData, int $t
     }
 
     /**
-     * Sort the parent of the given entity, if any auto sort actions are set for it.
+     * Sort the parent of the given entity if any auto sort actions are set for it.
      * Typically ran during create/update/insert events.
      */
     public function sortParent(Entity $entity): void
@@ -158,6 +169,9 @@ public function sortParent(Entity $entity): void
         }
     }
 
+    /**
+     * Update the description of the given container data from input data.
+     */
     protected function updateContainerDescription(EntityContainerData $data, array $input): void
     {
         if (isset($input['description_html'])) {

app/Entities/Repos/BookRepo.php

@@ -30,9 +30,7 @@ public function __construct(
     public function create(array $input): Book
     {
         return (new DatabaseTransaction(function () use ($input) {
-            $book = new Book();
-
-            $this->baseRepo->create($book, $input);
+            $book = $this->baseRepo->create(new Book(), $input);
             $this->baseRepo->updateCoverImage($book->containerData, $input['image'] ?? null);
             $this->baseRepo->updateDefaultTemplate($book->containerData, intval($input['default_template_id'] ?? null));
             Activity::add(ActivityType::BOOK_CREATE, $book);
@@ -52,7 +50,7 @@ public function create(array $input): Book
      */
     public function update(Book $book, array $input): Book
     {
-        $this->baseRepo->update($book, $input);
+        $book = $this->baseRepo->update($book, $input);
 
         if (array_key_exists('default_template_id', $input)) {
             $this->baseRepo->updateDefaultTemplate($book->containerData, intval($input['default_template_id']));

app/Entities/Repos/BookshelfRepo.php

@@ -25,8 +25,7 @@ public function __construct(
     public function create(array $input, array $bookIds): Bookshelf
     {
         return (new DatabaseTransaction(function () use ($input, $bookIds) {
-            $shelf = new Bookshelf();
-            $this->baseRepo->create($shelf, $input);
+            $shelf = $this->baseRepo->create(new Bookshelf(), $input);
             $this->baseRepo->updateCoverImage($shelf->containerData, $input['image'] ?? null);
             $this->updateBooks($shelf, $bookIds);
             Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf);
@@ -39,7 +38,7 @@ public function create(array $input, array $bookIds): Bookshelf
      */
     public function update(Bookshelf $shelf, array $input, ?array $bookIds): Bookshelf
     {
-        $this->baseRepo->update($shelf, $input);
+        $shelf = $this->baseRepo->update($shelf, $input);
 
         if (!is_null($bookIds)) {
             $this->updateBooks($shelf, $bookIds);

app/Entities/Repos/ChapterRepo.php

@@ -33,7 +33,8 @@ public function create(array $input, Book $parentBook): Chapter
             $chapter = new Chapter();
             $chapter->book_id = $parentBook->id;
             $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1;
-            $this->baseRepo->create($chapter, $input);
+
+            $chapter = $this->baseRepo->create($chapter, $input);
             $this->baseRepo->updateDefaultTemplate($chapter->containerData, intval($input['default_template_id'] ?? null));
             Activity::add(ActivityType::CHAPTER_CREATE, $chapter);
 
@@ -48,7 +49,7 @@ public function create(array $input, Book $parentBook): Chapter
      */
     public function update(Chapter $chapter, array $input): Chapter
     {
-        $this->baseRepo->update($chapter, $input);
+        $chapter = $this->baseRepo->update($chapter, $input);
 
         if (array_key_exists('default_template_id', $input)) {
             $this->baseRepo->updateDefaultTemplate($chapter->containerData, intval($input['default_template_id']));
@@ -93,7 +94,7 @@ public function move(Chapter $chapter, string $parentIdentifier): Book
         }
 
         return (new DatabaseTransaction(function () use ($chapter, $parent) {
-            $chapter->changeBook($parent->id);
+            $chapter = $chapter->changeBook($parent->id);
             $chapter->rebuildPermissions();
             Activity::add(ActivityType::CHAPTER_MOVE, $chapter);