Focused base Entity class cleanup

Removed some common functions from other entities.
Aligned implementation of getUrl()
Cleaned phpdocs and added typehinting.
Also extracted sibling search logic out of controller.

app/Entities/EntityProvider.php

@@ -42,21 +42,14 @@ class EntityProvider
      */
     public $pageRevision;
 
-    /**
-     * EntityProvider constructor.
-     */
-    public function __construct(
-        Bookshelf $bookshelf,
-        Book $book,
-        Chapter $chapter,
-        Page $page,
-        PageRevision $pageRevision
-    ) {
-        $this->bookshelf = $bookshelf;
-        $this->book = $book;
-        $this->chapter = $chapter;
-        $this->page = $page;
-        $this->pageRevision = $pageRevision;
+
+    public function __construct()
+    {
+        $this->bookshelf = new Bookshelf();
+        $this->book = new Book();
+        $this->chapter = new Chapter();
+        $this->page = new Page();
+        $this->pageRevision = new PageRevision();
     }
 
     /**

app/Entities/Models/Book.php

@@ -1,10 +1,5 @@
 <?php namespace BookStack\Entities\Models;
 
-use BookStack\Entities\Models\Bookshelf;
-use BookStack\Entities\Models\Chapter;
-use BookStack\Entities\Models\Entity;
-use BookStack\Entities\Models\HasCoverImage;
-use BookStack\Entities\Models\Page;
 use BookStack\Uploads\Image;
 use Exception;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
@@ -27,15 +22,10 @@ class Book extends Entity implements HasCoverImage
 
     /**
      * Get the url for this book.
-     * @param string|bool $path
-     * @return string
      */
-    public function getUrl($path = false)
+    public function getUrl(string $path = ''): string
     {
-        if ($path !== false) {
-            return url('/books/' . urlencode($this->slug) . '/' . trim($path, '/'));
-        }
-        return url('/books/' . urlencode($this->slug));
+        return url('/books/' . implode('/', [urlencode($this->slug), trim($path, '/')]));
     }
 
     /**
@@ -121,15 +111,4 @@ class Book extends Entity implements HasCoverImage
         $chapters = $this->chapters()->visible()->get();
         return $pages->concat($chapters)->sortBy('priority')->sortByDesc('draft');
     }
-
-    /**
-     * Get an excerpt of this book's description to the specified length or less.
-     * @param int $length
-     * @return string
-     */
-    public function getExcerpt(int $length = 100)
-    {
-        $description = $this->description;
-        return mb_strlen($description) > $length ? mb_substr($description, 0, $length-3) . '...' : $description;
-    }
 }

app/Entities/Models/Bookshelf.php

@@ -1,8 +1,5 @@
 <?php namespace BookStack\Entities\Models;
 
-use BookStack\Entities\Models\Entity;
-use BookStack\Entities\Models\HasCoverImage;
-use BookStack\Entities\Models\Book;
 use BookStack\Uploads\Image;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
@@ -39,15 +36,10 @@ class Bookshelf extends Entity implements HasCoverImage
 
     /**
      * Get the url for this bookshelf.
-     * @param string|bool $path
-     * @return string
      */
-    public function getUrl($path = false)
+    public function getUrl(string $path = ''): string
     {
-        if ($path !== false) {
-            return url('/shelves/' . urlencode($this->slug) . '/' . trim($path, '/'));
-        }
-        return url('/shelves/' . urlencode($this->slug));
+        return url('/shelves/' . implode('/', [urlencode($this->slug), trim($path, '/')]));
     }
 
     /**
@@ -88,17 +80,6 @@ class Bookshelf extends Entity implements HasCoverImage
         return 'cover_shelf';
     }
 
-    /**
-     * Get an excerpt of this book's description to the specified length or less.
-     * @param int $length
-     * @return string
-     */
-    public function getExcerpt(int $length = 100)
-    {
-        $description = $this->description;
-        return mb_strlen($description) > $length ? mb_substr($description, 0, $length-3) . '...' : $description;
-    }
-
     /**
      * Check if this shelf contains the given book.
      * @param Book $book

app/Entities/Models/Chapter.php

@@ -1,7 +1,5 @@
 <?php namespace BookStack\Entities\Models;
 
-use BookStack\Entities\Models\BookChild;
-use BookStack\Entities\Models\Page;
 use Illuminate\Support\Collection;
 
 /**
@@ -27,30 +25,18 @@ class Chapter extends BookChild
 
     /**
      * Get the url of this chapter.
-     * @param string|bool $path
-     * @return string
      */
-    public function getUrl($path = false)
+    public function getUrl($path = ''): string
     {
-        $bookSlug = $this->getAttribute('bookSlug') ? $this->getAttribute('bookSlug') : $this->book->slug;
-        $fullPath = '/books/' . urlencode($bookSlug) . '/chapter/' . urlencode($this->slug);
+        $parts = [
+            'books',
+            urlencode($this->getAttribute('bookSlug') ?? $this->book->slug),
+            'chapter',
+            urlencode($this->slug),
+            trim($path, '/'),
+        ];
 
-        if ($path !== false) {
-            $fullPath .= '/' . trim($path, '/');
-        }
-
-        return url($fullPath);
-    }
-
-    /**
-     * Get an excerpt of this chapter's description to the specified length or less.
-     * @param int $length
-     * @return string
-     */
-    public function getExcerpt(int $length = 100)
-    {
-        $description = $this->text ?? $this->description;
-        return mb_strlen($description) > $length ? mb_substr($description, 0, $length-3) . '...' : $description;
+        return url('/' . implode('/', $parts));
     }
 
     /**

app/Entities/Models/Entity.php

@@ -35,7 +35,7 @@ use Illuminate\Database\Eloquent\SoftDeletes;
  * @method static Builder withLastView()
  * @method static Builder withViewCount()
  */
-class Entity extends Ownable
+abstract class Entity extends Ownable
 {
     use SoftDeletes;
 
@@ -52,7 +52,7 @@ class Entity extends Ownable
     /**
      * Get the entities that are visible to the current user.
      */
-    public function scopeVisible(Builder $query)
+    public function scopeVisible(Builder $query): Builder
     {
         return $this->scopeHasPermission($query, 'view');
     }
@@ -94,24 +94,18 @@ class Entity extends Ownable
     /**
      * Compares this entity to another given entity.
      * Matches by comparing class and id.
-     * @param $entity
-     * @return bool
      */
-    public function matches($entity)
+    public function matches(Entity $entity): bool
     {
         return [get_class($this), $this->id] === [get_class($entity), $entity->id];
     }
 
     /**
-     * Checks if an entity matches or contains another given entity.
-     * @param Entity $entity
-     * @return bool
+     * Checks if the current entity matches or contains the given.
      */
-    public function matchesOrContains(Entity $entity)
+    public function matchesOrContains(Entity $entity): bool
     {
-        $matches = [get_class($this), $this->id] === [get_class($entity), $entity->id];
-
-        if ($matches) {
+        if ($this->matches($entity)) {
             return true;
         }
 
@@ -128,9 +122,8 @@ class Entity extends Ownable
 
     /**
      * Gets the activity objects for this entity.
-     * @return MorphMany
      */
-    public function activity()
+    public function activity(): MorphMany
     {
         return $this->morphMany(Activity::class, 'entity')
             ->orderBy('created_at', 'desc');
@@ -139,26 +132,23 @@ class Entity extends Ownable
     /**
      * Get View objects for this entity.
      */
-    public function views()
+    public function views(): MorphMany
     {
         return $this->morphMany(View::class, 'viewable');
     }
 
     /**
      * Get the Tag models that have been user assigned to this entity.
-     * @return MorphMany
      */
-    public function tags()
+    public function tags(): MorphMany
     {
         return $this->morphMany(Tag::class, 'entity')->orderBy('order', 'asc');
     }
 
     /**
      * Get the comments for an entity
-     * @param bool $orderByCreated
-     * @return MorphMany
      */
-    public function comments($orderByCreated = true)
+    public function comments(bool $orderByCreated = true): MorphMany
     {
         $query = $this->morphMany(Comment::class, 'entity');
         return $orderByCreated ? $query->orderBy('created_at', 'asc') : $query;
@@ -166,9 +156,8 @@ class Entity extends Ownable
 
     /**
      * Get the related search terms.
-     * @return MorphMany
      */
-    public function searchTerms()
+    public function searchTerms(): MorphMany
     {
         return $this->morphMany(SearchTerm::class, 'entity');
     }
@@ -176,18 +165,15 @@ class Entity extends Ownable
     /**
      * Get this entities restrictions.
      */
-    public function permissions()
+    public function permissions(): MorphMany
     {
         return $this->morphMany(EntityPermission::class, 'restrictable');
     }
 
     /**
      * Check if this entity has a specific restriction set against it.
-     * @param $role_id
-     * @param $action
-     * @return bool
      */
-    public function hasRestriction($role_id, $action)
+    public function hasRestriction(int $role_id, string $action): bool
     {
         return $this->permissions()->where('role_id', '=', $role_id)
             ->where('action', '=', $action)->count() > 0;
@@ -227,21 +213,6 @@ class Entity extends Ownable
         return strtolower(static::getClassName());
     }
 
-    /**
-     * Get an instance of an entity of the given type.
-     * TODO - Refactor out
-     */
-    public static function getEntityInstance(string $type): ?Entity
-    {
-        $types = ['Page', 'Book', 'Chapter', 'Bookshelf'];
-        $className = str_replace([' ', '-', '_'], '', ucwords($type));
-        if (!in_array($className, $types)) {
-            return null;
-        }
-
-        return app('BookStack\\Entities\\Models\\' . $className);
-    }
-
     /**
      * Gets a limited-length version of the entities name.
      */
@@ -255,36 +226,30 @@ class Entity extends Ownable
 
     /**
      * Get the body text of this entity.
-     * @return mixed
      */
-    public function getText()
+    public function getText(): string
     {
-        return $this->{$this->textField};
+        return $this->{$this->textField} ?? '';
     }
 
     /**
      * Get an excerpt of this entity's descriptive content to the specified length.
-     * @param int $length
-     * @return mixed
      */
-    public function getExcerpt(int $length = 100)
+    public function getExcerpt(int $length = 100): string
     {
         $text = $this->getText();
+
         if (mb_strlen($text) > $length) {
             $text = mb_substr($text, 0, $length-3) . '...';
         }
+
         return trim($text);
     }
 
     /**
      * Get the url of this entity
-     * @param $path
-     * @return string
      */
-    public function getUrl($path = '/')
-    {
-        return $path;
-    }
+    abstract public function getUrl(string $path = '/'): string;
 
     /**
      * Get the parent entity if existing.

app/Entities/Models/Page.php

@@ -1,8 +1,5 @@
 <?php namespace BookStack\Entities\Models;
 
-use BookStack\Entities\Models\BookChild;
-use BookStack\Entities\Models\Chapter;
-use BookStack\Entities\Models\PageRevision;
 use BookStack\Uploads\Attachment;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
@@ -35,7 +32,7 @@ class Page extends BookChild
     /**
      * Get the entities that are visible to the current user.
      */
-    public function scopeVisible(Builder $query)
+    public function scopeVisible(Builder $query): Builder
     {
         $query = Permissions::enforceDraftVisiblityOnQuery($query);
         return parent::scopeVisible($query);
@@ -89,22 +86,19 @@ class Page extends BookChild
     }
 
     /**
-     * Get the url for this page.
-     * @param string|bool $path
-     * @return string
+     * Get the url of this page.
      */
-    public function getUrl($path = false)
+    public function getUrl($path = ''): string
     {
-        $bookSlug = $this->getAttribute('bookSlug') ? $this->getAttribute('bookSlug') : $this->book->slug;
-        $midText = $this->draft ? '/draft/' : '/page/';
-        $idComponent = $this->draft ? $this->id : urlencode($this->slug);
+        $parts = [
+            'books',
+            urlencode($this->getAttribute('bookSlug') ?? $this->book->slug),
+            $this->draft ? 'draft' : 'page',
+            $this->draft ? $this->id : urlencode($this->slug),
+            trim($path, '/'),
+        ];
 
-        $url = '/books/' . urlencode($bookSlug) . $midText . $idComponent;
-        if ($path !== false) {
-            $url .= '/' . trim($path, '/');
-        }
-
-        return url($url);
+        return url('/' . implode('/', $parts));
     }
 
     /**

app/Entities/Tools/SearchIndex.php

@@ -31,7 +31,7 @@ class SearchIndex
     {
         $this->deleteEntityTerms($entity);
         $nameTerms = $this->generateTermArrayFromText($entity->name, 5 * $entity->searchFactor);
-        $bodyTerms = $this->generateTermArrayFromText($entity->getText() ?? '', 1 * $entity->searchFactor);
+        $bodyTerms = $this->generateTermArrayFromText($entity->getText(), 1 * $entity->searchFactor);
         $terms = array_merge($nameTerms, $bodyTerms);
         foreach ($terms as $index => $term) {
             $terms[$index]['entity_type'] = $entity->getMorphClass();

app/Entities/Tools/SiblingFetcher.php

@@ -0,0 +1,47 @@
+<?php namespace BookStack\Entities\Tools;
+
+use BookStack\Entities\EntityProvider;
+use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Bookshelf;
+use Illuminate\Support\Collection;
+
+class SiblingFetcher
+{
+
+    /**
+     * Search among the siblings of the entity of given type and id.
+     */
+    public function fetch(string $entityType, int $entityId): Collection
+    {
+        $entity = (new EntityProvider)->get($entityType)->visible()->findOrFail($entityId);
+        $entities = [];
+
+        // Page in chapter
+        if ($entity->isA('page') && $entity->chapter) {
+            $entities = $entity->chapter->getVisiblePages();
+        }
+
+        // Page in book or chapter
+        if (($entity->isA('page') && !$entity->chapter) || $entity->isA('chapter')) {
+            $entities = $entity->book->getDirectChildren();
+        }
+
+        // Book
+        // Gets just the books in a shelf if shelf is in context
+        if ($entity->isA('book')) {
+            $contextShelf = (new ShelfContext)->getContextualShelfForBook($entity);
+            if ($contextShelf) {
+                $entities = $contextShelf->visibleBooks()->get();
+            } else {
+                $entities = Book::visible()->get();
+            }
+        }
+
+        // Shelve
+        if ($entity->isA('bookshelf')) {
+            $entities = Bookshelf::visible()->get();
+        }
+
+        return $entities;
+    }
+}

app/Entities/Tools/TrashCan.php

@@ -168,11 +168,10 @@ class TrashCan
      */
     public function getTrashedCounts(): array
     {
-        $provider = app(EntityProvider::class);
         $counts = [];
 
         /** @var Entity $instance */
-        foreach ($provider->all() as $key => $instance) {
+        foreach ((new EntityProvider)->all() as $key => $instance) {
             $counts[$key] = $instance->newQuery()->onlyTrashed()->count();
         }
 

app/Http/Controllers/SearchController.php

@@ -7,6 +7,7 @@ use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Tools\SearchRunner;
 use BookStack\Entities\Tools\ShelfContext;
 use BookStack\Entities\Tools\SearchOptions;
+use BookStack\Entities\Tools\SiblingFetcher;
 use Illuminate\Http\Request;
 
 class SearchController extends Controller
@@ -98,39 +99,7 @@ class SearchController extends Controller
         $type = $request->get('entity_type', null);
         $id = $request->get('entity_id', null);
 
-        $entity = Entity::getEntityInstance($type)->newQuery()->visible()->find($id);
-        if (!$entity) {
-            return $this->jsonError(trans('errors.entity_not_found'), 404);
-        }
-
-        $entities = [];
-
-        // Page in chapter
-        if ($entity->isA('page') && $entity->chapter) {
-            $entities = $entity->chapter->getVisiblePages();
-        }
-
-        // Page in book or chapter
-        if (($entity->isA('page') && !$entity->chapter) || $entity->isA('chapter')) {
-            $entities = $entity->book->getDirectChildren();
-        }
-
-        // Book
-        // Gets just the books in a shelf if shelf is in context
-        if ($entity->isA('book')) {
-            $contextShelf = $this->entityContextManager->getContextualShelfForBook($entity);
-            if ($contextShelf) {
-                $entities = $contextShelf->visibleBooks()->get();
-            } else {
-                $entities = Book::visible()->get();
-            }
-        }
-
-        // Shelve
-        if ($entity->isA('bookshelf')) {
-            $entities = Bookshelf::visible()->get();
-        }
-
+        $entities = (new SiblingFetcher)->fetch($type, $id);
         return view('partials.entity-list-basic', ['entities' => $entities, 'style' => 'compact']);
     }
 }