From a70ed81908a8bcd67e6c449eb7d0cdd6f26ef998 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 4 Feb 2024 14:39:01 +0000 Subject: [PATCH 01/12] DB: Started update of entity loading to avoid global selects Removes page/chpater addSelect global query, to load book slug, and instead extracts base queries to be managed in new static class, while updating specific entitiy relation loading to use our more efficient MixedEntityListLoader where appropriate. Related to #4823 --- app/Activity/ActivityQueries.php | 14 ++++---- app/App/HomeController.php | 10 +++--- app/Entities/Models/BookChild.php | 14 -------- app/Entities/Queries/EntityQuery.php | 6 ++++ app/Entities/Queries/PageQueries.php | 31 ++++++++++++++++++ app/Entities/Queries/RecentlyViewed.php | 12 ++++--- app/Entities/Queries/TopFavourites.php | 10 +++--- app/Entities/Tools/MixedEntityListLoader.php | 25 ++++++++++++--- app/References/ReferenceFetcher.php | 2 +- ...4_02_04_141358_add_views_updated_index.php | 32 +++++++++++++++++++ 10 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 app/Entities/Queries/PageQueries.php create mode 100644 database/migrations/2024_02_04_141358_add_views_updated_index.php diff --git a/app/Activity/ActivityQueries.php b/app/Activity/ActivityQueries.php index c69cf7a36..dae0791b1 100644 --- a/app/Activity/ActivityQueries.php +++ b/app/Activity/ActivityQueries.php @@ -7,6 +7,7 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; +use BookStack\Entities\Tools\MixedEntityListLoader; use BookStack\Permissions\PermissionApplicator; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; @@ -14,11 +15,10 @@ use Illuminate\Database\Eloquent\Relations\Relation; class ActivityQueries { - protected PermissionApplicator $permissions; - - public function __construct(PermissionApplicator $permissions) - { - $this->permissions = $permissions; + public function __construct( + protected PermissionApplicator $permissions, + protected MixedEntityListLoader $listLoader, + ) { } /** @@ -29,11 +29,13 @@ class ActivityQueries $activityList = $this->permissions ->restrictEntityRelationQuery(Activity::query(), 'activities', 'entity_id', 'entity_type') ->orderBy('created_at', 'desc') - ->with(['user', 'entity']) + ->with(['user']) ->skip($count * $page) ->take($count) ->get(); + $this->listLoader->loadIntoRelations($activityList->all(), 'entity', false); + return $this->filterSimilar($activityList); } diff --git a/app/App/HomeController.php b/app/App/HomeController.php index 8188ad010..48d60b8e4 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -5,6 +5,7 @@ namespace BookStack\App; use BookStack\Activity\ActivityQueries; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Queries\RecentlyViewed; use BookStack\Entities\Queries\TopFavourites; use BookStack\Entities\Repos\BookRepo; @@ -26,9 +27,7 @@ class HomeController extends Controller $draftPages = []; if ($this->isSignedIn()) { - $draftPages = Page::visible() - ->where('draft', '=', true) - ->where('created_by', '=', user()->id) + $draftPages = PageQueries::currentUserDraftsForList() ->orderBy('updated_at', 'desc') ->with('book') ->take(6) @@ -40,11 +39,10 @@ class HomeController extends Controller (new RecentlyViewed())->run(12 * $recentFactor, 1) : Book::visible()->orderBy('created_at', 'desc')->take(12 * $recentFactor)->get(); $favourites = (new TopFavourites())->run(6); - $recentlyUpdatedPages = Page::visible()->with('book') + $recentlyUpdatedPages = PageQueries::visibleForList() ->where('draft', false) ->orderBy('updated_at', 'desc') ->take($favourites->count() > 0 ? 5 : 10) - ->select(Page::$listAttributes) ->get(); $homepageOptions = ['default', 'books', 'bookshelves', 'page']; @@ -95,7 +93,7 @@ class HomeController extends Controller $homepageSetting = setting('app-homepage', '0:'); $id = intval(explode(':', $homepageSetting)[0]); /** @var Page $customHomepage */ - $customHomepage = Page::query()->where('draft', '=', false)->findOrFail($id); + $customHomepage = PageQueries::start()->where('draft', '=', false)->findOrFail($id); $pageContent = new PageContent($customHomepage); $customHomepage->html = $pageContent->render(false); diff --git a/app/Entities/Models/BookChild.php b/app/Entities/Models/BookChild.php index 18735e56b..d19a2466a 100644 --- a/app/Entities/Models/BookChild.php +++ b/app/Entities/Models/BookChild.php @@ -18,20 +18,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; */ abstract class BookChild extends Entity { - protected static function boot() - { - parent::boot(); - - // Load book slugs onto these models by default during query-time - static::addGlobalScope('book_slug', function (Builder $builder) { - $builder->addSelect(['book_slug' => function ($builder) { - $builder->select('slug') - ->from('books') - ->whereColumn('books.id', '=', 'book_id'); - }]); - }); - } - /** * Scope a query to find items where the child has the given childSlug * where its parent has the bookSlug. diff --git a/app/Entities/Queries/EntityQuery.php b/app/Entities/Queries/EntityQuery.php index 2246e13b1..bd7a98b5e 100644 --- a/app/Entities/Queries/EntityQuery.php +++ b/app/Entities/Queries/EntityQuery.php @@ -3,10 +3,16 @@ namespace BookStack\Entities\Queries; use BookStack\Entities\EntityProvider; +use BookStack\Entities\Tools\MixedEntityListLoader; use BookStack\Permissions\PermissionApplicator; abstract class EntityQuery { + protected function mixedEntityListLoader(): MixedEntityListLoader + { + return app()->make(MixedEntityListLoader::class); + } + protected function permissionService(): PermissionApplicator { return app()->make(PermissionApplicator::class); diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php new file mode 100644 index 000000000..7b7ac1e5e --- /dev/null +++ b/app/Entities/Queries/PageQueries.php @@ -0,0 +1,31 @@ +select(array_merge(Page::$listAttributes, ['book_slug' => function ($builder) { + $builder->select('slug') + ->from('books') + ->whereColumn('books.id', '=', 'pages.book_id'); + }])); + } + + public static function currentUserDraftsForList(): Builder + { + return static::visibleForList() + ->where('draft', '=', true) + ->where('created_by', '=', user()->id); + } +} diff --git a/app/Entities/Queries/RecentlyViewed.php b/app/Entities/Queries/RecentlyViewed.php index 5895b97a2..fed15ca5a 100644 --- a/app/Entities/Queries/RecentlyViewed.php +++ b/app/Entities/Queries/RecentlyViewed.php @@ -10,7 +10,7 @@ class RecentlyViewed extends EntityQuery public function run(int $count, int $page): Collection { $user = user(); - if ($user === null || $user->isGuest()) { + if ($user->isGuest()) { return collect(); } @@ -23,11 +23,13 @@ class RecentlyViewed extends EntityQuery ->orderBy('views.updated_at', 'desc') ->where('user_id', '=', user()->id); - return $query->with('viewable') + $views = $query ->skip(($page - 1) * $count) ->take($count) - ->get() - ->pluck('viewable') - ->filter(); + ->get(); + + $this->mixedEntityListLoader()->loadIntoRelations($views->all(), 'viewable', false); + + return $views->pluck('viewable')->filter(); } } diff --git a/app/Entities/Queries/TopFavourites.php b/app/Entities/Queries/TopFavourites.php index a2f8d9ea1..47d4b77f7 100644 --- a/app/Entities/Queries/TopFavourites.php +++ b/app/Entities/Queries/TopFavourites.php @@ -25,11 +25,13 @@ class TopFavourites extends EntityQuery ->orderBy('views.views', 'desc') ->where('favourites.user_id', '=', user()->id); - return $query->with('favouritable') + $favourites = $query ->skip($skip) ->take($count) - ->get() - ->pluck('favouritable') - ->filter(); + ->get(); + + $this->mixedEntityListLoader()->loadIntoRelations($favourites->all(), 'favouritable', false); + + return $favourites->pluck('favouritable')->filter(); } } diff --git a/app/Entities/Tools/MixedEntityListLoader.php b/app/Entities/Tools/MixedEntityListLoader.php index 50079e3bf..a0df791db 100644 --- a/app/Entities/Tools/MixedEntityListLoader.php +++ b/app/Entities/Tools/MixedEntityListLoader.php @@ -26,7 +26,7 @@ class MixedEntityListLoader * This will look for a model id and type via 'name_id' and 'name_type'. * @param Model[] $relations */ - public function loadIntoRelations(array $relations, string $relationName): void + public function loadIntoRelations(array $relations, string $relationName, bool $loadParents): void { $idsByType = []; foreach ($relations as $relation) { @@ -40,7 +40,7 @@ class MixedEntityListLoader $idsByType[$type][] = $id; } - $modelMap = $this->idsByTypeToModelMap($idsByType); + $modelMap = $this->idsByTypeToModelMap($idsByType, $loadParents); foreach ($relations as $relation) { $type = $relation->getAttribute($relationName . '_type'); @@ -56,7 +56,7 @@ class MixedEntityListLoader * @param array $idsByType * @return array> */ - protected function idsByTypeToModelMap(array $idsByType): array + protected function idsByTypeToModelMap(array $idsByType, bool $eagerLoadParents): array { $modelMap = []; @@ -67,10 +67,10 @@ class MixedEntityListLoader $instance = $this->entityProvider->get($type); $models = $instance->newQuery() - ->select($this->listAttributes[$type]) + ->select(array_merge($this->listAttributes[$type], $this->getSubSelectsForQuery($type))) ->scopes('visible') ->whereIn('id', $ids) - ->with($this->getRelationsToEagerLoad($type)) + ->with($eagerLoadParents ? $this->getRelationsToEagerLoad($type) : []) ->get(); if (count($models) > 0) { @@ -100,4 +100,19 @@ class MixedEntityListLoader return $toLoad; } + + protected function getSubSelectsForQuery(string $type): array + { + $subSelects = []; + + if ($type === 'chapter' || $type === 'page') { + $subSelects['book_slug'] = function ($builder) { + $builder->select('slug') + ->from('books') + ->whereColumn('books.id', '=', 'book_id'); + }; + } + + return $subSelects; + } } diff --git a/app/References/ReferenceFetcher.php b/app/References/ReferenceFetcher.php index 0d9883a3e..655ea7c09 100644 --- a/app/References/ReferenceFetcher.php +++ b/app/References/ReferenceFetcher.php @@ -23,7 +23,7 @@ class ReferenceFetcher public function getReferencesToEntity(Entity $entity): Collection { $references = $this->queryReferencesToEntity($entity)->get(); - $this->mixedEntityListLoader->loadIntoRelations($references->all(), 'from'); + $this->mixedEntityListLoader->loadIntoRelations($references->all(), 'from', true); return $references; } diff --git a/database/migrations/2024_02_04_141358_add_views_updated_index.php b/database/migrations/2024_02_04_141358_add_views_updated_index.php new file mode 100644 index 000000000..a643b3a1e --- /dev/null +++ b/database/migrations/2024_02_04_141358_add_views_updated_index.php @@ -0,0 +1,32 @@ +index(['updated_at'], 'views_updated_at_index'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('views', function (Blueprint $table) { + $table->dropIndex('views_updated_at_index'); + }); + } +}; From 1559b0acd1018edff3f173df0c87f631549462fa Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 4 Feb 2024 17:35:16 +0000 Subject: [PATCH 02/12] Queries: Migrated BookRepo queries to new query class Also moved to a non-static approach, and added a high-level class to allow easy access to all other entity queries, for use in mixed-entity scenarios and easier/simpler injection. --- app/App/HomeController.php | 18 ++++-- app/Entities/Controllers/BookController.php | 30 +++++----- .../Controllers/BookExportController.php | 24 +++----- .../Controllers/BookSortController.php | 16 +++--- app/Entities/Queries/BookQueries.php | 56 ++++++++++++++++++ app/Entities/Queries/EntityQueries.php | 12 ++++ app/Entities/Queries/PageQueries.php | 10 ++-- app/Entities/Repos/BookRepo.php | 57 ------------------- 8 files changed, 118 insertions(+), 105 deletions(-) create mode 100644 app/Entities/Queries/BookQueries.php create mode 100644 app/Entities/Queries/EntityQueries.php diff --git a/app/App/HomeController.php b/app/App/HomeController.php index 48d60b8e4..dacd4bcc2 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -5,10 +5,9 @@ namespace BookStack\App; use BookStack\Activity\ActivityQueries; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Page; -use BookStack\Entities\Queries\PageQueries; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Queries\RecentlyViewed; use BookStack\Entities\Queries\TopFavourites; -use BookStack\Entities\Repos\BookRepo; use BookStack\Entities\Repos\BookshelfRepo; use BookStack\Entities\Tools\PageContent; use BookStack\Http\Controller; @@ -18,6 +17,11 @@ use Illuminate\Http\Request; class HomeController extends Controller { + public function __construct( + protected EntityQueries $queries, + ) { + } + /** * Display the homepage. */ @@ -27,7 +31,7 @@ class HomeController extends Controller $draftPages = []; if ($this->isSignedIn()) { - $draftPages = PageQueries::currentUserDraftsForList() + $draftPages = $this->queries->pages->currentUserDraftsForList() ->orderBy('updated_at', 'desc') ->with('book') ->take(6) @@ -39,7 +43,7 @@ class HomeController extends Controller (new RecentlyViewed())->run(12 * $recentFactor, 1) : Book::visible()->orderBy('created_at', 'desc')->take(12 * $recentFactor)->get(); $favourites = (new TopFavourites())->run(6); - $recentlyUpdatedPages = PageQueries::visibleForList() + $recentlyUpdatedPages = $this->queries->pages->visibleForList() ->where('draft', false) ->orderBy('updated_at', 'desc') ->take($favourites->count() > 0 ? 5 : 10) @@ -83,7 +87,9 @@ class HomeController extends Controller } if ($homepageOption === 'books') { - $books = app()->make(BookRepo::class)->getAllPaginated(18, $commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder()); + $books = $this->queries->books->visibleForListWithCover() + ->orderBy($commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder()) + ->paginate(18); $data = array_merge($commonData, ['books' => $books]); return view('home.books', $data); @@ -93,7 +99,7 @@ class HomeController extends Controller $homepageSetting = setting('app-homepage', '0:'); $id = intval(explode(':', $homepageSetting)[0]); /** @var Page $customHomepage */ - $customHomepage = PageQueries::start()->where('draft', '=', false)->findOrFail($id); + $customHomepage = $this->queries->pages->start()->where('draft', '=', false)->findOrFail($id); $pageContent = new PageContent($customHomepage); $customHomepage->html = $pageContent->render(false); diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index 412feca2f..a956a47d6 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -7,6 +7,7 @@ use BookStack\Activity\ActivityType; use BookStack\Activity\Models\View; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Repos\BookRepo; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\Cloner; @@ -27,7 +28,8 @@ class BookController extends Controller public function __construct( protected ShelfContext $shelfContext, protected BookRepo $bookRepo, - protected ReferenceFetcher $referenceFetcher + protected BookQueries $queries, + protected ReferenceFetcher $referenceFetcher, ) { } @@ -43,10 +45,12 @@ class BookController extends Controller 'updated_at' => trans('common.sort_updated_at'), ]); - $books = $this->bookRepo->getAllPaginated(18, $listOptions->getSort(), $listOptions->getOrder()); - $recents = $this->isSignedIn() ? $this->bookRepo->getRecentlyViewed(4) : false; - $popular = $this->bookRepo->getPopular(4); - $new = $this->bookRepo->getRecentlyCreated(4); + $books = $this->queries->visibleForListWithCover() + ->orderBy($listOptions->getSort(), $listOptions->getOrder()) + ->paginate(18); + $recents = $this->isSignedIn() ? $this->queries->recentlyViewedForCurrentUser()->take(4)->get() : false; + $popular = $this->queries->popularForList()->take(4)->get(); + $new = $this->queries->visibleForList()->orderBy('created_at', 'desc')->take(4)->get(); $this->shelfContext->clearShelfContext(); @@ -120,7 +124,7 @@ class BookController extends Controller */ public function show(Request $request, ActivityQueries $activities, string $slug) { - $book = $this->bookRepo->getBySlug($slug); + $book = $this->queries->findVisibleBySlug($slug); $bookChildren = (new BookContents($book))->getTree(true); $bookParentShelves = $book->shelves()->scopes('visible')->get(); @@ -147,7 +151,7 @@ class BookController extends Controller */ public function edit(string $slug) { - $book = $this->bookRepo->getBySlug($slug); + $book = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('book-update', $book); $this->setPageTitle(trans('entities.books_edit_named', ['bookName' => $book->getShortName()])); @@ -163,7 +167,7 @@ class BookController extends Controller */ public function update(Request $request, string $slug) { - $book = $this->bookRepo->getBySlug($slug); + $book = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('book-update', $book); $validated = $this->validate($request, [ @@ -190,7 +194,7 @@ class BookController extends Controller */ public function showDelete(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-delete', $book); $this->setPageTitle(trans('entities.books_delete_named', ['bookName' => $book->getShortName()])); @@ -204,7 +208,7 @@ class BookController extends Controller */ public function destroy(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-delete', $book); $this->bookRepo->destroy($book); @@ -219,7 +223,7 @@ class BookController extends Controller */ public function showCopy(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-view', $book); session()->flashInput(['name' => $book->name]); @@ -236,7 +240,7 @@ class BookController extends Controller */ public function copy(Request $request, Cloner $cloner, string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-view', $book); $this->checkPermission('book-create-all'); @@ -252,7 +256,7 @@ class BookController extends Controller */ public function convertToShelf(HierarchyTransformer $transformer, string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-update', $book); $this->checkOwnablePermission('book-delete', $book); $this->checkPermission('bookshelf-create-all'); diff --git a/app/Entities/Controllers/BookExportController.php b/app/Entities/Controllers/BookExportController.php index 1a6b20db9..6540df978 100644 --- a/app/Entities/Controllers/BookExportController.php +++ b/app/Entities/Controllers/BookExportController.php @@ -2,23 +2,17 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Repos\BookRepo; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\Controller; use Throwable; class BookExportController extends Controller { - protected $bookRepo; - protected $exportFormatter; - - /** - * BookExportController constructor. - */ - public function __construct(BookRepo $bookRepo, ExportFormatter $exportFormatter) - { - $this->bookRepo = $bookRepo; - $this->exportFormatter = $exportFormatter; + public function __construct( + protected BookQueries $queries, + protected ExportFormatter $exportFormatter, + ) { $this->middleware('can:content-export'); } @@ -29,7 +23,7 @@ class BookExportController extends Controller */ public function pdf(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $pdfContent = $this->exportFormatter->bookToPdf($book); return $this->download()->directly($pdfContent, $bookSlug . '.pdf'); @@ -42,7 +36,7 @@ class BookExportController extends Controller */ public function html(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $htmlContent = $this->exportFormatter->bookToContainedHtml($book); return $this->download()->directly($htmlContent, $bookSlug . '.html'); @@ -53,7 +47,7 @@ class BookExportController extends Controller */ public function plainText(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $textContent = $this->exportFormatter->bookToPlainText($book); return $this->download()->directly($textContent, $bookSlug . '.txt'); @@ -64,7 +58,7 @@ class BookExportController extends Controller */ public function markdown(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $textContent = $this->exportFormatter->bookToMarkdown($book); return $this->download()->directly($textContent, $bookSlug . '.md'); diff --git a/app/Entities/Controllers/BookSortController.php b/app/Entities/Controllers/BookSortController.php index f2310e205..5d7024952 100644 --- a/app/Entities/Controllers/BookSortController.php +++ b/app/Entities/Controllers/BookSortController.php @@ -3,7 +3,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Activity\ActivityType; -use BookStack\Entities\Repos\BookRepo; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\BookSortMap; use BookStack\Facades\Activity; @@ -12,11 +12,9 @@ use Illuminate\Http\Request; class BookSortController extends Controller { - protected $bookRepo; - - public function __construct(BookRepo $bookRepo) - { - $this->bookRepo = $bookRepo; + public function __construct( + protected BookQueries $queries, + ) { } /** @@ -24,7 +22,7 @@ class BookSortController extends Controller */ public function show(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-update', $book); $bookChildren = (new BookContents($book))->getTree(false); @@ -40,7 +38,7 @@ class BookSortController extends Controller */ public function showItem(string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $bookChildren = (new BookContents($book))->getTree(); return view('books.parts.sort-box', ['book' => $book, 'bookChildren' => $bookChildren]); @@ -51,7 +49,7 @@ class BookSortController extends Controller */ public function update(Request $request, string $bookSlug) { - $book = $this->bookRepo->getBySlug($bookSlug); + $book = $this->queries->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('book-update', $book); // Return if no map sent diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php new file mode 100644 index 000000000..6de28f0c2 --- /dev/null +++ b/app/Entities/Queries/BookQueries.php @@ -0,0 +1,56 @@ +start() + ->scopes('visible') + ->where('slug', '=', $slug) + ->first(); + + if ($book === null) { + throw new NotFoundException(trans('errors.book_not_found')); + } + + return $book; + } + + public function visibleForList(): Builder + { + return $this->start()->scopes('visible'); + } + + public function visibleForListWithCover(): Builder + { + return $this->visibleForList()->with('cover'); + } + + public function recentlyViewedForCurrentUser(): Builder + { + return $this->visibleForList() + ->scopes('withLastView') + ->having('last_viewed_at', '>', 0) + ->orderBy('last_viewed_at', 'desc'); + } + + public function popularForList(): Builder + { + return $this->visibleForList() + ->scopes('withViewCount') + ->having('view_count', '>', 0) + ->orderBy('view_count', 'desc'); + } +} diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php new file mode 100644 index 000000000..417db455c --- /dev/null +++ b/app/Entities/Queries/EntityQueries.php @@ -0,0 +1,12 @@ +start() ->select(array_merge(Page::$listAttributes, ['book_slug' => function ($builder) { $builder->select('slug') ->from('books') @@ -22,9 +22,9 @@ class PageQueries }])); } - public static function currentUserDraftsForList(): Builder + public function currentUserDraftsForList(): Builder { - return static::visibleForList() + return $this->visibleForList() ->where('draft', '=', true) ->where('created_by', '=', user()->id); } diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index bf765b22d..26b9414fb 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -5,16 +5,12 @@ namespace BookStack\Entities\Repos; use BookStack\Activity\ActivityType; use BookStack\Activity\TagRepo; use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Page; use BookStack\Entities\Tools\TrashCan; use BookStack\Exceptions\ImageUploadException; -use BookStack\Exceptions\NotFoundException; use BookStack\Facades\Activity; use BookStack\Uploads\ImageRepo; use Exception; -use Illuminate\Contracts\Pagination\LengthAwarePaginator; use Illuminate\Http\UploadedFile; -use Illuminate\Support\Collection; class BookRepo { @@ -25,59 +21,6 @@ class BookRepo ) { } - /** - * Get all books in a paginated format. - */ - public function getAllPaginated(int $count = 20, string $sort = 'name', string $order = 'asc'): LengthAwarePaginator - { - return Book::visible()->with('cover')->orderBy($sort, $order)->paginate($count); - } - - /** - * Get the books that were most recently viewed by this user. - */ - public function getRecentlyViewed(int $count = 20): Collection - { - return Book::visible()->withLastView() - ->having('last_viewed_at', '>', 0) - ->orderBy('last_viewed_at', 'desc') - ->take($count)->get(); - } - - /** - * Get the most popular books in the system. - */ - public function getPopular(int $count = 20): Collection - { - return Book::visible()->withViewCount() - ->having('view_count', '>', 0) - ->orderBy('view_count', 'desc') - ->take($count)->get(); - } - - /** - * Get the most recently created books from the system. - */ - public function getRecentlyCreated(int $count = 20): Collection - { - return Book::visible()->orderBy('created_at', 'desc') - ->take($count)->get(); - } - - /** - * Get a book by its slug. - */ - public function getBySlug(string $slug): Book - { - $book = Book::visible()->where('slug', '=', $slug)->first(); - - if ($book === null) { - throw new NotFoundException(trans('errors.book_not_found')); - } - - return $book; - } - /** * Create a new book in the system. */ From 3886aedf54873a1bf90cfdaa9cafc6f22c1d03fd Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 4 Feb 2024 19:32:19 +0000 Subject: [PATCH 03/12] Queries: Migrated bookshelf repo queries to new class --- app/App/HomeController.php | 5 +- .../Controllers/BookshelfController.php | 27 ++++--- app/Entities/Queries/BookshelfQueries.php | 56 +++++++++++++++ app/Entities/Queries/EntityQueries.php | 1 + app/Entities/Repos/BookshelfRepo.php | 70 +------------------ 5 files changed, 80 insertions(+), 79 deletions(-) create mode 100644 app/Entities/Queries/BookshelfQueries.php diff --git a/app/App/HomeController.php b/app/App/HomeController.php index dacd4bcc2..f5a3c7ed6 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -8,7 +8,6 @@ use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Queries\RecentlyViewed; use BookStack\Entities\Queries\TopFavourites; -use BookStack\Entities\Repos\BookshelfRepo; use BookStack\Entities\Tools\PageContent; use BookStack\Http\Controller; use BookStack\Uploads\FaviconHandler; @@ -80,7 +79,9 @@ class HomeController extends Controller } if ($homepageOption === 'bookshelves') { - $shelves = app()->make(BookshelfRepo::class)->getAllPaginated(18, $commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder()); + $shelves = $this->queries->shelves->visibleForListWithCover() + ->orderBy($commonData['listOptions']->getSort(), $commonData['listOptions']->getOrder()) + ->paginate(18); $data = array_merge($commonData, ['shelves' => $shelves]); return view('home.shelves', $data); diff --git a/app/Entities/Controllers/BookshelfController.php b/app/Entities/Controllers/BookshelfController.php index 2f5461cdb..bc3fc2a6f 100644 --- a/app/Entities/Controllers/BookshelfController.php +++ b/app/Entities/Controllers/BookshelfController.php @@ -5,6 +5,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Activity\ActivityQueries; use BookStack\Activity\Models\View; use BookStack\Entities\Models\Book; +use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Repos\BookshelfRepo; use BookStack\Entities\Tools\ShelfContext; use BookStack\Exceptions\ImageUploadException; @@ -20,8 +21,9 @@ class BookshelfController extends Controller { public function __construct( protected BookshelfRepo $shelfRepo, + protected BookshelfQueries $queries, protected ShelfContext $shelfContext, - protected ReferenceFetcher $referenceFetcher + protected ReferenceFetcher $referenceFetcher, ) { } @@ -37,10 +39,15 @@ class BookshelfController extends Controller 'updated_at' => trans('common.sort_updated_at'), ]); - $shelves = $this->shelfRepo->getAllPaginated(18, $listOptions->getSort(), $listOptions->getOrder()); - $recents = $this->isSignedIn() ? $this->shelfRepo->getRecentlyViewed(4) : false; - $popular = $this->shelfRepo->getPopular(4); - $new = $this->shelfRepo->getRecentlyCreated(4); + $shelves = $this->queries->visibleForListWithCover() + ->orderBy($listOptions->getSort(), $listOptions->getOrder()) + ->paginate(18); + $recents = $this->isSignedIn() ? $this->queries->recentlyViewedForCurrentUser()->get() : false; + $popular = $this->queries->popularForList()->get(); + $new = $this->queries->visibleForList() + ->orderBy('created_at', 'desc') + ->take(4) + ->get(); $this->shelfContext->clearShelfContext(); $this->setPageTitle(trans('entities.shelves')); @@ -96,7 +103,7 @@ class BookshelfController extends Controller */ public function show(Request $request, ActivityQueries $activities, string $slug) { - $shelf = $this->shelfRepo->getBySlug($slug); + $shelf = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('bookshelf-view', $shelf); $listOptions = SimpleListOptions::fromRequest($request, 'shelf_books')->withSortOptions([ @@ -134,7 +141,7 @@ class BookshelfController extends Controller */ public function edit(string $slug) { - $shelf = $this->shelfRepo->getBySlug($slug); + $shelf = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('bookshelf-update', $shelf); $shelfBookIds = $shelf->books()->get(['id'])->pluck('id'); @@ -157,7 +164,7 @@ class BookshelfController extends Controller */ public function update(Request $request, string $slug) { - $shelf = $this->shelfRepo->getBySlug($slug); + $shelf = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('bookshelf-update', $shelf); $validated = $this->validate($request, [ 'name' => ['required', 'string', 'max:255'], @@ -183,7 +190,7 @@ class BookshelfController extends Controller */ public function showDelete(string $slug) { - $shelf = $this->shelfRepo->getBySlug($slug); + $shelf = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->setPageTitle(trans('entities.shelves_delete_named', ['name' => $shelf->getShortName()])); @@ -198,7 +205,7 @@ class BookshelfController extends Controller */ public function destroy(string $slug) { - $shelf = $this->shelfRepo->getBySlug($slug); + $shelf = $this->queries->findVisibleBySlug($slug); $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->shelfRepo->destroy($shelf); diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php new file mode 100644 index 000000000..7edff45b9 --- /dev/null +++ b/app/Entities/Queries/BookshelfQueries.php @@ -0,0 +1,56 @@ +start() + ->scopes('visible') + ->where('slug', '=', $slug) + ->first(); + + if ($shelf === null) { + throw new NotFoundException(trans('errors.bookshelf_not_found')); + } + + return $shelf; + } + + public function visibleForList(): Builder + { + return $this->start()->scopes('visible'); + } + + public function visibleForListWithCover(): Builder + { + return $this->visibleForList()->with('cover'); + } + + public function recentlyViewedForCurrentUser(): Builder + { + return $this->visibleForList() + ->scopes('withLastView') + ->having('last_viewed_at', '>', 0) + ->orderBy('last_viewed_at', 'desc'); + } + + public function popularForList(): Builder + { + return $this->visibleForList() + ->scopes('withViewCount') + ->having('view_count', '>', 0) + ->orderBy('view_count', 'desc'); + } +} diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php index 417db455c..b1973b0ea 100644 --- a/app/Entities/Queries/EntityQueries.php +++ b/app/Entities/Queries/EntityQueries.php @@ -5,6 +5,7 @@ namespace BookStack\Entities\Queries; class EntityQueries { public function __construct( + public BookshelfQueries $shelves, public BookQueries $books, public PageQueries $pages, ) { diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 27333b5b1..479e6178f 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -6,78 +6,14 @@ use BookStack\Activity\ActivityType; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Tools\TrashCan; -use BookStack\Exceptions\NotFoundException; use BookStack\Facades\Activity; use Exception; -use Illuminate\Contracts\Pagination\LengthAwarePaginator; -use Illuminate\Support\Collection; class BookshelfRepo { - protected $baseRepo; - - /** - * BookshelfRepo constructor. - */ - public function __construct(BaseRepo $baseRepo) - { - $this->baseRepo = $baseRepo; - } - - /** - * Get all bookshelves in a paginated format. - */ - public function getAllPaginated(int $count = 20, string $sort = 'name', string $order = 'asc'): LengthAwarePaginator - { - return Bookshelf::visible() - ->with(['visibleBooks', 'cover']) - ->orderBy($sort, $order) - ->paginate($count); - } - - /** - * Get the bookshelves that were most recently viewed by this user. - */ - public function getRecentlyViewed(int $count = 20): Collection - { - return Bookshelf::visible()->withLastView() - ->having('last_viewed_at', '>', 0) - ->orderBy('last_viewed_at', 'desc') - ->take($count)->get(); - } - - /** - * Get the most popular bookshelves in the system. - */ - public function getPopular(int $count = 20): Collection - { - return Bookshelf::visible()->withViewCount() - ->having('view_count', '>', 0) - ->orderBy('view_count', 'desc') - ->take($count)->get(); - } - - /** - * Get the most recently created bookshelves from the system. - */ - public function getRecentlyCreated(int $count = 20): Collection - { - return Bookshelf::visible()->orderBy('created_at', 'desc') - ->take($count)->get(); - } - - /** - * Get a shelf by its slug. - */ - public function getBySlug(string $slug): Bookshelf - { - $shelf = Bookshelf::visible()->where('slug', '=', $slug)->first(); - - if ($shelf === null) { - throw new NotFoundException(trans('errors.bookshelf_not_found')); - } - - return $shelf; + public function __construct( + protected BaseRepo $baseRepo, + ) { } /** From 8e78b4c43eb980f47d9c207a0ce3330699d54103 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 5 Feb 2024 15:59:20 +0000 Subject: [PATCH 04/12] Queries: Extracted chapter repo queries to class Updated query classes to align to interface for common aligned operations. Extracted repeated string-identifier-based finding from page/chapter repos to shared higher-level entity queries. --- .../Controllers/ChapterController.php | 39 ++++++++------ .../Controllers/ChapterExportController.php | 24 ++++----- app/Entities/Controllers/PageController.php | 8 ++- app/Entities/Models/Chapter.php | 1 - app/Entities/Queries/BookQueries.php | 7 ++- app/Entities/Queries/BookshelfQueries.php | 7 ++- app/Entities/Queries/ChapterQueries.php | 53 +++++++++++++++++++ app/Entities/Queries/EntityQueries.php | 30 +++++++++++ app/Entities/Queries/PageQueries.php | 7 ++- .../Queries/ProvidesEntityQueries.php | 12 +++++ app/Entities/Repos/ChapterRepo.php | 46 ++-------------- app/Entities/Repos/PageRepo.php | 28 ++-------- 12 files changed, 160 insertions(+), 102 deletions(-) create mode 100644 app/Entities/Queries/ChapterQueries.php create mode 100644 app/Entities/Queries/ProvidesEntityQueries.php diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 00616888a..4b0a525eb 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -5,6 +5,8 @@ namespace BookStack\Entities\Controllers; use BookStack\Activity\Models\View; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Entities\Models\Book; +use BookStack\Entities\Queries\ChapterQueries; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Repos\ChapterRepo; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\Cloner; @@ -24,7 +26,9 @@ class ChapterController extends Controller { public function __construct( protected ChapterRepo $chapterRepo, - protected ReferenceFetcher $referenceFetcher + protected ChapterQueries $queries, + protected EntityQueries $entityQueries, + protected ReferenceFetcher $referenceFetcher, ) { } @@ -33,12 +37,15 @@ class ChapterController extends Controller */ public function create(string $bookSlug) { - $book = Book::visible()->where('slug', '=', $bookSlug)->firstOrFail(); + $book = $this->entityQueries->books->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('chapter-create', $book); $this->setPageTitle(trans('entities.chapters_create')); - return view('chapters.create', ['book' => $book, 'current' => $book]); + return view('chapters.create', [ + 'book' => $book, + 'current' => $book, + ]); } /** @@ -55,7 +62,7 @@ class ChapterController extends Controller 'default_template_id' => ['nullable', 'integer'], ]); - $book = Book::visible()->where('slug', '=', $bookSlug)->firstOrFail(); + $book = $this->entityQueries->books->findVisibleBySlug($bookSlug); $this->checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($validated, $book); @@ -68,7 +75,7 @@ class ChapterController extends Controller */ public function show(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); $sidebarTree = (new BookContents($chapter->book))->getTree(); @@ -96,7 +103,7 @@ class ChapterController extends Controller */ public function edit(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->setPageTitle(trans('entities.chapters_edit_named', ['chapterName' => $chapter->getShortName()])); @@ -118,7 +125,7 @@ class ChapterController extends Controller 'default_template_id' => ['nullable', 'integer'], ]); - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->chapterRepo->update($chapter, $validated); @@ -133,7 +140,7 @@ class ChapterController extends Controller */ public function showDelete(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-delete', $chapter); $this->setPageTitle(trans('entities.chapters_delete_named', ['chapterName' => $chapter->getShortName()])); @@ -149,7 +156,7 @@ class ChapterController extends Controller */ public function destroy(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-delete', $chapter); $this->chapterRepo->destroy($chapter); @@ -164,7 +171,7 @@ class ChapterController extends Controller */ public function showMove(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->setPageTitle(trans('entities.chapters_move_named', ['chapterName' => $chapter->getShortName()])); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); @@ -182,7 +189,7 @@ class ChapterController extends Controller */ public function move(Request $request, string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); @@ -211,7 +218,7 @@ class ChapterController extends Controller */ public function showCopy(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); session()->flashInput(['name' => $chapter->name]); @@ -230,13 +237,13 @@ class ChapterController extends Controller */ public function copy(Request $request, Cloner $cloner, string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); $entitySelection = $request->get('entity_selection') ?: null; - $newParentBook = $entitySelection ? $this->chapterRepo->findParentByIdentifier($entitySelection) : $chapter->getParent(); + $newParentBook = $entitySelection ? $this->entityQueries->findVisibleByStringIdentifier($entitySelection) : $chapter->getParent(); - if (is_null($newParentBook)) { + if (!$newParentBook instanceof Book) { $this->showErrorNotification(trans('errors.selected_book_not_found')); return redirect($chapter->getUrl('/copy')); @@ -256,7 +263,7 @@ class ChapterController extends Controller */ public function convertToBook(HierarchyTransformer $transformer, string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); $this->checkPermission('book-create-all'); diff --git a/app/Entities/Controllers/ChapterExportController.php b/app/Entities/Controllers/ChapterExportController.php index b67ec9b37..0e071fc82 100644 --- a/app/Entities/Controllers/ChapterExportController.php +++ b/app/Entities/Controllers/ChapterExportController.php @@ -2,7 +2,7 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Repos\ChapterRepo; +use BookStack\Entities\Queries\ChapterQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Exceptions\NotFoundException; use BookStack\Http\Controller; @@ -10,16 +10,10 @@ use Throwable; class ChapterExportController extends Controller { - protected $chapterRepo; - protected $exportFormatter; - - /** - * ChapterExportController constructor. - */ - public function __construct(ChapterRepo $chapterRepo, ExportFormatter $exportFormatter) - { - $this->chapterRepo = $chapterRepo; - $this->exportFormatter = $exportFormatter; + public function __construct( + protected ChapterQueries $queries, + protected ExportFormatter $exportFormatter, + ) { $this->middleware('can:content-export'); } @@ -31,7 +25,7 @@ class ChapterExportController extends Controller */ public function pdf(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $pdfContent = $this->exportFormatter->chapterToPdf($chapter); return $this->download()->directly($pdfContent, $chapterSlug . '.pdf'); @@ -45,7 +39,7 @@ class ChapterExportController extends Controller */ public function html(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $containedHtml = $this->exportFormatter->chapterToContainedHtml($chapter); return $this->download()->directly($containedHtml, $chapterSlug . '.html'); @@ -58,7 +52,7 @@ class ChapterExportController extends Controller */ public function plainText(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $chapterText = $this->exportFormatter->chapterToPlainText($chapter); return $this->download()->directly($chapterText, $chapterSlug . '.txt'); @@ -71,7 +65,7 @@ class ChapterExportController extends Controller */ public function markdown(string $bookSlug, string $chapterSlug) { - $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); $chapterText = $this->exportFormatter->chapterToMarkdown($chapter); return $this->download()->directly($chapterText, $chapterSlug . '.md'); diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index eaad3c0b7..f2cd729c6 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -8,6 +8,8 @@ use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\Cloner; @@ -29,6 +31,8 @@ class PageController extends Controller { public function __construct( protected PageRepo $pageRepo, + protected PageQueries $pageQueries, + protected EntityQueries $entityQueries, protected ReferenceFetcher $referenceFetcher ) { } @@ -435,9 +439,9 @@ class PageController extends Controller $this->checkOwnablePermission('page-view', $page); $entitySelection = $request->get('entity_selection') ?: null; - $newParent = $entitySelection ? $this->pageRepo->findParentByIdentifier($entitySelection) : $page->getParent(); + $newParent = $entitySelection ? $this->entityQueries->findVisibleByStringIdentifier($entitySelection) : $page->getParent(); - if (is_null($newParent)) { + if (!$newParent instanceof Book && !$newParent instanceof Chapter) { $this->showErrorNotification(trans('errors.selected_book_chapter_not_found')); return redirect($page->getUrl('/copy')); diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php index d3a710111..422c82442 100644 --- a/app/Entities/Models/Chapter.php +++ b/app/Entities/Models/Chapter.php @@ -11,7 +11,6 @@ use Illuminate\Support\Collection; * Class Chapter. * * @property Collection $pages - * @property string $description * @property ?int $default_template_id * @property ?Page $defaultTemplate */ diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 6de28f0c2..5e85523cf 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -6,13 +6,18 @@ use BookStack\Entities\Models\Book; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; -class BookQueries +class BookQueries implements ProvidesEntityQueries { public function start(): Builder { return Book::query(); } + public function findVisibleById(int $id): ?Book + { + return $this->start()->scopes('visible')->find($id); + } + public function findVisibleBySlug(string $slug): Book { /** @var ?Book $book */ diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index 7edff45b9..52e123087 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -6,13 +6,18 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; -class BookshelfQueries +class BookshelfQueries implements ProvidesEntityQueries { public function start(): Builder { return Bookshelf::query(); } + public function findVisibleById(int $id): ?Bookshelf + { + return $this->start()->scopes('visible')->find($id); + } + public function findVisibleBySlug(string $slug): Bookshelf { /** @var ?Bookshelf $shelf */ diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php new file mode 100644 index 000000000..dcfc4aad3 --- /dev/null +++ b/app/Entities/Queries/ChapterQueries.php @@ -0,0 +1,53 @@ +start()->scopes('visible')->find($id); + } + + public function findVisibleBySlugs(string $bookSlug, string $chapterSlug): Chapter + { + /** @var ?Chapter $chapter */ + $chapter = $this->start()->with('book') + ->whereHas('book', function (Builder $query) use ($bookSlug) { + $query->where('slug', '=', $bookSlug); + }) + ->where('slug', '=', $chapterSlug) + ->first(); + + if ($chapter === null) { + throw new NotFoundException(trans('errors.chapter_not_found')); + } + + return $chapter; + } + + public function visibleForList(): Builder + { + return $this->start() + ->select(array_merge(static::$listAttributes, ['book_slug' => function ($builder) { + $builder->select('slug') + ->from('books') + ->whereColumn('books.id', '=', 'chapters.book_id'); + }])); + } +} diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php index b1973b0ea..39a21c913 100644 --- a/app/Entities/Queries/EntityQueries.php +++ b/app/Entities/Queries/EntityQueries.php @@ -2,12 +2,42 @@ namespace BookStack\Entities\Queries; +use BookStack\Entities\Models\Entity; + class EntityQueries { public function __construct( public BookshelfQueries $shelves, public BookQueries $books, + public ChapterQueries $chapters, public PageQueries $pages, ) { } + + /** + * Find an entity via an identifier string in the format: + * {type}:{id} + * Example: (book:5). + */ + public function findVisibleByStringIdentifier(string $identifier): ?Entity + { + $explodedId = explode(':', $identifier); + $entityType = $explodedId[0]; + $entityId = intval($explodedId[1]); + + /** @var ?ProvidesEntityQueries $queries */ + $queries = match ($entityType) { + 'page' => $this->pages, + 'chapter' => $this->chapters, + 'book' => $this->books, + 'bookshelf' => $this->shelves, + default => null, + }; + + if (is_null($queries)) { + return null; + } + + return $queries->findVisibleById($entityId); + } } diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index b9c64f63c..f1991626f 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -5,13 +5,18 @@ namespace BookStack\Entities\Queries; use BookStack\Entities\Models\Page; use Illuminate\Database\Eloquent\Builder; -class PageQueries +class PageQueries implements ProvidesEntityQueries { public function start(): Builder { return Page::query(); } + public function findVisibleById(int $id): ?Page + { + return $this->start()->scopes('visible')->find($id); + } + public function visibleForList(): Builder { return $this->start() diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php new file mode 100644 index 000000000..5c37b02e4 --- /dev/null +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -0,0 +1,12 @@ +whereSlugs($bookSlug, $chapterSlug)->first(); - - if ($chapter === null) { - throw new NotFoundException(trans('errors.chapter_not_found')); - } - - return $chapter; - } - /** * Create a new chapter in the system. */ @@ -91,8 +75,8 @@ class ChapterRepo */ public function move(Chapter $chapter, string $parentIdentifier): Book { - $parent = $this->findParentByIdentifier($parentIdentifier); - if (is_null($parent)) { + $parent = $this->entityQueries->findVisibleByStringIdentifier($parentIdentifier); + if (!$parent instanceof Book) { throw new MoveOperationException('Book to move chapter into not found'); } @@ -106,24 +90,4 @@ class ChapterRepo return $parent; } - - /** - * Find a page parent entity via an identifier string in the format: - * {type}:{id} - * Example: (book:5). - * - * @throws MoveOperationException - */ - public function findParentByIdentifier(string $identifier): ?Book - { - $stringExploded = explode(':', $identifier); - $entityType = $stringExploded[0]; - $entityId = intval($stringExploded[1]); - - if ($entityType !== 'book') { - throw new MoveOperationException('Chapters can only be in books'); - } - - return Book::visible()->where('id', '=', $entityId)->first(); - } } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 85237a752..929de528d 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -8,6 +8,7 @@ use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; use BookStack\Entities\Models\PageRevision; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\PageContent; use BookStack\Entities\Tools\PageEditorData; @@ -26,6 +27,7 @@ class PageRepo public function __construct( protected BaseRepo $baseRepo, protected RevisionRepo $revisionRepo, + protected EntityQueries $entityQueries, protected ReferenceStore $referenceStore, protected ReferenceUpdater $referenceUpdater ) { @@ -324,8 +326,8 @@ class PageRepo */ public function move(Page $page, string $parentIdentifier): Entity { - $parent = $this->findParentByIdentifier($parentIdentifier); - if (is_null($parent)) { + $parent = $this->entityQueries->findVisibleByStringIdentifier($parentIdentifier); + if (!$parent instanceof Chapter && !$parent instanceof Book) { throw new MoveOperationException('Book or chapter to move page into not found'); } @@ -343,28 +345,6 @@ class PageRepo return $parent; } - /** - * Find a page parent entity via an identifier string in the format: - * {type}:{id} - * Example: (book:5). - * - * @throws MoveOperationException - */ - public function findParentByIdentifier(string $identifier): ?Entity - { - $stringExploded = explode(':', $identifier); - $entityType = $stringExploded[0]; - $entityId = intval($stringExploded[1]); - - if ($entityType !== 'book' && $entityType !== 'chapter') { - throw new MoveOperationException('Pages can only be in books or chapters'); - } - - $parentClass = $entityType === 'book' ? Book::class : Chapter::class; - - return $parentClass::visible()->where('id', '=', $entityId)->first(); - } - /** * Get a new priority for a page. */ From 222c665018cd7fc231d2970307e3a7423e4a377f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 5 Feb 2024 17:35:49 +0000 Subject: [PATCH 05/12] Queries: Extracted PageRepo queries to own class Started new class for PageRevisions too as part of these changes --- app/Entities/Controllers/BookController.php | 16 ++-- .../Controllers/BookExportController.php | 8 +- .../Controllers/BookSortController.php | 6 +- .../Controllers/BookshelfController.php | 10 +-- .../Controllers/ChapterController.php | 24 ++--- .../Controllers/ChapterExportController.php | 8 +- .../Controllers/PageApiController.php | 10 ++- app/Entities/Controllers/PageController.php | 57 +++++++----- .../Controllers/PageExportController.php | 24 ++--- .../Controllers/PageRevisionController.php | 14 +-- .../Controllers/PageTemplateController.php | 29 ++++--- app/Entities/Queries/BookQueries.php | 2 +- app/Entities/Queries/BookshelfQueries.php | 2 +- app/Entities/Queries/ChapterQueries.php | 4 +- app/Entities/Queries/EntityQueries.php | 1 + app/Entities/Queries/PageQueries.php | 35 ++++++++ app/Entities/Queries/PageRevisionQueries.php | 41 +++++++++ .../Queries/ProvidesEntityQueries.php | 11 ++- app/Entities/Repos/PageRepo.php | 87 ------------------- app/Entities/Tools/PageEditorData.php | 14 +-- .../Controllers/AttachmentController.php | 12 +-- 21 files changed, 219 insertions(+), 196 deletions(-) create mode 100644 app/Entities/Queries/PageRevisionQueries.php diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index a956a47d6..0e9346dbd 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -124,7 +124,7 @@ class BookController extends Controller */ public function show(Request $request, ActivityQueries $activities, string $slug) { - $book = $this->queries->findVisibleBySlug($slug); + $book = $this->queries->findVisibleBySlugOrFail($slug); $bookChildren = (new BookContents($book))->getTree(true); $bookParentShelves = $book->shelves()->scopes('visible')->get(); @@ -151,7 +151,7 @@ class BookController extends Controller */ public function edit(string $slug) { - $book = $this->queries->findVisibleBySlug($slug); + $book = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('book-update', $book); $this->setPageTitle(trans('entities.books_edit_named', ['bookName' => $book->getShortName()])); @@ -167,7 +167,7 @@ class BookController extends Controller */ public function update(Request $request, string $slug) { - $book = $this->queries->findVisibleBySlug($slug); + $book = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('book-update', $book); $validated = $this->validate($request, [ @@ -194,7 +194,7 @@ class BookController extends Controller */ public function showDelete(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-delete', $book); $this->setPageTitle(trans('entities.books_delete_named', ['bookName' => $book->getShortName()])); @@ -208,7 +208,7 @@ class BookController extends Controller */ public function destroy(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-delete', $book); $this->bookRepo->destroy($book); @@ -223,7 +223,7 @@ class BookController extends Controller */ public function showCopy(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-view', $book); session()->flashInput(['name' => $book->name]); @@ -240,7 +240,7 @@ class BookController extends Controller */ public function copy(Request $request, Cloner $cloner, string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-view', $book); $this->checkPermission('book-create-all'); @@ -256,7 +256,7 @@ class BookController extends Controller */ public function convertToShelf(HierarchyTransformer $transformer, string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-update', $book); $this->checkOwnablePermission('book-delete', $book); $this->checkPermission('bookshelf-create-all'); diff --git a/app/Entities/Controllers/BookExportController.php b/app/Entities/Controllers/BookExportController.php index 6540df978..5c1a964c1 100644 --- a/app/Entities/Controllers/BookExportController.php +++ b/app/Entities/Controllers/BookExportController.php @@ -23,7 +23,7 @@ class BookExportController extends Controller */ public function pdf(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $pdfContent = $this->exportFormatter->bookToPdf($book); return $this->download()->directly($pdfContent, $bookSlug . '.pdf'); @@ -36,7 +36,7 @@ class BookExportController extends Controller */ public function html(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $htmlContent = $this->exportFormatter->bookToContainedHtml($book); return $this->download()->directly($htmlContent, $bookSlug . '.html'); @@ -47,7 +47,7 @@ class BookExportController extends Controller */ public function plainText(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $textContent = $this->exportFormatter->bookToPlainText($book); return $this->download()->directly($textContent, $bookSlug . '.txt'); @@ -58,7 +58,7 @@ class BookExportController extends Controller */ public function markdown(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $textContent = $this->exportFormatter->bookToMarkdown($book); return $this->download()->directly($textContent, $bookSlug . '.md'); diff --git a/app/Entities/Controllers/BookSortController.php b/app/Entities/Controllers/BookSortController.php index 5d7024952..5aefc5832 100644 --- a/app/Entities/Controllers/BookSortController.php +++ b/app/Entities/Controllers/BookSortController.php @@ -22,7 +22,7 @@ class BookSortController extends Controller */ public function show(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-update', $book); $bookChildren = (new BookContents($book))->getTree(false); @@ -38,7 +38,7 @@ class BookSortController extends Controller */ public function showItem(string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $bookChildren = (new BookContents($book))->getTree(); return view('books.parts.sort-box', ['book' => $book, 'bookChildren' => $bookChildren]); @@ -49,7 +49,7 @@ class BookSortController extends Controller */ public function update(Request $request, string $bookSlug) { - $book = $this->queries->findVisibleBySlug($bookSlug); + $book = $this->queries->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('book-update', $book); // Return if no map sent diff --git a/app/Entities/Controllers/BookshelfController.php b/app/Entities/Controllers/BookshelfController.php index bc3fc2a6f..3118325da 100644 --- a/app/Entities/Controllers/BookshelfController.php +++ b/app/Entities/Controllers/BookshelfController.php @@ -103,7 +103,7 @@ class BookshelfController extends Controller */ public function show(Request $request, ActivityQueries $activities, string $slug) { - $shelf = $this->queries->findVisibleBySlug($slug); + $shelf = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('bookshelf-view', $shelf); $listOptions = SimpleListOptions::fromRequest($request, 'shelf_books')->withSortOptions([ @@ -141,7 +141,7 @@ class BookshelfController extends Controller */ public function edit(string $slug) { - $shelf = $this->queries->findVisibleBySlug($slug); + $shelf = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('bookshelf-update', $shelf); $shelfBookIds = $shelf->books()->get(['id'])->pluck('id'); @@ -164,7 +164,7 @@ class BookshelfController extends Controller */ public function update(Request $request, string $slug) { - $shelf = $this->queries->findVisibleBySlug($slug); + $shelf = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('bookshelf-update', $shelf); $validated = $this->validate($request, [ 'name' => ['required', 'string', 'max:255'], @@ -190,7 +190,7 @@ class BookshelfController extends Controller */ public function showDelete(string $slug) { - $shelf = $this->queries->findVisibleBySlug($slug); + $shelf = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->setPageTitle(trans('entities.shelves_delete_named', ['name' => $shelf->getShortName()])); @@ -205,7 +205,7 @@ class BookshelfController extends Controller */ public function destroy(string $slug) { - $shelf = $this->queries->findVisibleBySlug($slug); + $shelf = $this->queries->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->shelfRepo->destroy($shelf); diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 4b0a525eb..2e36a84b9 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -37,7 +37,7 @@ class ChapterController extends Controller */ public function create(string $bookSlug) { - $book = $this->entityQueries->books->findVisibleBySlug($bookSlug); + $book = $this->entityQueries->books->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('chapter-create', $book); $this->setPageTitle(trans('entities.chapters_create')); @@ -62,7 +62,7 @@ class ChapterController extends Controller 'default_template_id' => ['nullable', 'integer'], ]); - $book = $this->entityQueries->books->findVisibleBySlug($bookSlug); + $book = $this->entityQueries->books->findVisibleBySlugOrFail($bookSlug); $this->checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($validated, $book); @@ -75,7 +75,7 @@ class ChapterController extends Controller */ public function show(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); $sidebarTree = (new BookContents($chapter->book))->getTree(); @@ -103,7 +103,7 @@ class ChapterController extends Controller */ public function edit(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->setPageTitle(trans('entities.chapters_edit_named', ['chapterName' => $chapter->getShortName()])); @@ -125,7 +125,7 @@ class ChapterController extends Controller 'default_template_id' => ['nullable', 'integer'], ]); - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->chapterRepo->update($chapter, $validated); @@ -140,7 +140,7 @@ class ChapterController extends Controller */ public function showDelete(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-delete', $chapter); $this->setPageTitle(trans('entities.chapters_delete_named', ['chapterName' => $chapter->getShortName()])); @@ -156,7 +156,7 @@ class ChapterController extends Controller */ public function destroy(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-delete', $chapter); $this->chapterRepo->destroy($chapter); @@ -171,7 +171,7 @@ class ChapterController extends Controller */ public function showMove(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->setPageTitle(trans('entities.chapters_move_named', ['chapterName' => $chapter->getShortName()])); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); @@ -189,7 +189,7 @@ class ChapterController extends Controller */ public function move(Request $request, string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); @@ -218,7 +218,7 @@ class ChapterController extends Controller */ public function showCopy(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); session()->flashInput(['name' => $chapter->name]); @@ -237,7 +237,7 @@ class ChapterController extends Controller */ public function copy(Request $request, Cloner $cloner, string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-view', $chapter); $entitySelection = $request->get('entity_selection') ?: null; @@ -263,7 +263,7 @@ class ChapterController extends Controller */ public function convertToBook(HierarchyTransformer $transformer, string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-update', $chapter); $this->checkOwnablePermission('chapter-delete', $chapter); $this->checkPermission('book-create-all'); diff --git a/app/Entities/Controllers/ChapterExportController.php b/app/Entities/Controllers/ChapterExportController.php index 0e071fc82..ead601ab4 100644 --- a/app/Entities/Controllers/ChapterExportController.php +++ b/app/Entities/Controllers/ChapterExportController.php @@ -25,7 +25,7 @@ class ChapterExportController extends Controller */ public function pdf(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $pdfContent = $this->exportFormatter->chapterToPdf($chapter); return $this->download()->directly($pdfContent, $chapterSlug . '.pdf'); @@ -39,7 +39,7 @@ class ChapterExportController extends Controller */ public function html(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $containedHtml = $this->exportFormatter->chapterToContainedHtml($chapter); return $this->download()->directly($containedHtml, $chapterSlug . '.html'); @@ -52,7 +52,7 @@ class ChapterExportController extends Controller */ public function plainText(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $chapterText = $this->exportFormatter->chapterToPlainText($chapter); return $this->download()->directly($chapterText, $chapterSlug . '.txt'); @@ -65,7 +65,7 @@ class ChapterExportController extends Controller */ public function markdown(string $bookSlug, string $chapterSlug) { - $chapter = $this->queries->findVisibleBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $chapterText = $this->exportFormatter->chapterToMarkdown($chapter); return $this->download()->directly($chapterText, $chapterSlug . '.md'); diff --git a/app/Entities/Controllers/PageApiController.php b/app/Entities/Controllers/PageApiController.php index d2947f1bb..6e3880aed 100644 --- a/app/Entities/Controllers/PageApiController.php +++ b/app/Entities/Controllers/PageApiController.php @@ -5,6 +5,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Exceptions\PermissionsException; use BookStack\Http\ApiController; @@ -35,7 +36,8 @@ class PageApiController extends ApiController ]; public function __construct( - protected PageRepo $pageRepo + protected PageRepo $pageRepo, + protected PageQueries $queries, ) { } @@ -97,7 +99,7 @@ class PageApiController extends ApiController */ public function read(string $id) { - $page = $this->pageRepo->getById($id, []); + $page = $this->queries->findVisibleByIdOrFail($id); return response()->json($page->forJsonDisplay()); } @@ -113,7 +115,7 @@ class PageApiController extends ApiController { $requestData = $this->validate($request, $this->rules['update']); - $page = $this->pageRepo->getById($id, []); + $page = $this->queries->findVisibleByIdOrFail($id); $this->checkOwnablePermission('page-update', $page); $parent = null; @@ -148,7 +150,7 @@ class PageApiController extends ApiController */ public function delete(string $id) { - $page = $this->pageRepo->getById($id, []); + $page = $this->queries->findVisibleByIdOrFail($id); $this->checkOwnablePermission('page-delete', $page); $this->pageRepo->destroy($page); diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index f2cd729c6..3a5bdbd0b 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -31,7 +31,7 @@ class PageController extends Controller { public function __construct( protected PageRepo $pageRepo, - protected PageQueries $pageQueries, + protected PageQueries $queries, protected EntityQueries $entityQueries, protected ReferenceFetcher $referenceFetcher ) { @@ -44,7 +44,12 @@ class PageController extends Controller */ public function create(string $bookSlug, string $chapterSlug = null) { - $parent = $this->pageRepo->getParentFromSlugs($bookSlug, $chapterSlug); + if ($chapterSlug) { + $parent = $this->entityQueries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); + } else { + $parent = $this->entityQueries->books->findVisibleBySlugOrFail($bookSlug); + } + $this->checkOwnablePermission('page-create', $parent); // Redirect to draft edit screen if signed in @@ -71,7 +76,12 @@ class PageController extends Controller 'name' => ['required', 'string', 'max:255'], ]); - $parent = $this->pageRepo->getParentFromSlugs($bookSlug, $chapterSlug); + if ($chapterSlug) { + $parent = $this->entityQueries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); + } else { + $parent = $this->entityQueries->books->findVisibleBySlugOrFail($bookSlug); + } + $this->checkOwnablePermission('page-create', $parent); $page = $this->pageRepo->getNewDraftPage($parent); @@ -89,10 +99,10 @@ class PageController extends Controller */ public function editDraft(Request $request, string $bookSlug, int $pageId) { - $draft = $this->pageRepo->getById($pageId); + $draft = $this->queries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-create', $draft->getParent()); - $editorData = new PageEditorData($draft, $this->pageRepo, $request->query('editor', '')); + $editorData = new PageEditorData($draft, $this->entityQueries, $request->query('editor', '')); $this->setPageTitle(trans('entities.pages_edit_draft')); return view('pages.edit', $editorData->getViewData()); @@ -109,7 +119,7 @@ class PageController extends Controller $this->validate($request, [ 'name' => ['required', 'string', 'max:255'], ]); - $draftPage = $this->pageRepo->getById($pageId); + $draftPage = $this->queries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-create', $draftPage->getParent()); $page = $this->pageRepo->publishDraft($draftPage, $request->all()); @@ -126,11 +136,12 @@ class PageController extends Controller public function show(string $bookSlug, string $pageSlug) { try { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); } catch (NotFoundException $e) { - $page = $this->pageRepo->getByOldSlug($bookSlug, $pageSlug); + $revision = $this->entityQueries->revisions->findLatestVersionBySlugs($bookSlug, $pageSlug); + $page = $revision->page ?? null; - if ($page === null) { + if (is_null($page)) { throw $e; } @@ -171,7 +182,7 @@ class PageController extends Controller */ public function getPageAjax(int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->queries->findVisibleByIdOrFail($pageId); $page->setHidden(array_diff($page->getHidden(), ['html', 'markdown'])); $page->makeHidden(['book']); @@ -185,10 +196,10 @@ class PageController extends Controller */ public function edit(Request $request, string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-update', $page); - $editorData = new PageEditorData($page, $this->pageRepo, $request->query('editor', '')); + $editorData = new PageEditorData($page, $this->entityQueries, $request->query('editor', '')); if ($editorData->getWarnings()) { $this->showWarningNotification(implode("\n", $editorData->getWarnings())); } @@ -209,7 +220,7 @@ class PageController extends Controller $this->validate($request, [ 'name' => ['required', 'string', 'max:255'], ]); - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-update', $page); $this->pageRepo->update($page, $request->all()); @@ -224,7 +235,7 @@ class PageController extends Controller */ public function saveDraft(Request $request, int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->queries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-update', $page); if (!$this->isSignedIn()) { @@ -249,7 +260,7 @@ class PageController extends Controller */ public function redirectFromLink(int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->queries->findVisibleByIdOrFail($pageId); return redirect($page->getUrl()); } @@ -261,7 +272,7 @@ class PageController extends Controller */ public function showDelete(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-delete', $page); $this->setPageTitle(trans('entities.pages_delete_named', ['pageName' => $page->getShortName()])); $usedAsTemplate = @@ -283,7 +294,7 @@ class PageController extends Controller */ public function showDeleteDraft(string $bookSlug, int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->queries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-update', $page); $this->setPageTitle(trans('entities.pages_delete_draft_named', ['pageName' => $page->getShortName()])); $usedAsTemplate = @@ -306,7 +317,7 @@ class PageController extends Controller */ public function destroy(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-delete', $page); $parent = $page->getParent(); @@ -323,7 +334,7 @@ class PageController extends Controller */ public function destroyDraft(string $bookSlug, int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->queries->findVisibleByIdOrFail($pageId); $book = $page->book; $chapter = $page->chapter; $this->checkOwnablePermission('page-update', $page); @@ -370,7 +381,7 @@ class PageController extends Controller */ public function showMove(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-update', $page); $this->checkOwnablePermission('page-delete', $page); @@ -388,7 +399,7 @@ class PageController extends Controller */ public function move(Request $request, string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-update', $page); $this->checkOwnablePermission('page-delete', $page); @@ -417,7 +428,7 @@ class PageController extends Controller */ public function showCopy(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-view', $page); session()->flashInput(['name' => $page->name]); @@ -435,7 +446,7 @@ class PageController extends Controller */ public function copy(Request $request, Cloner $cloner, string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-view', $page); $entitySelection = $request->get('entity_selection') ?: null; diff --git a/app/Entities/Controllers/PageExportController.php b/app/Entities/Controllers/PageExportController.php index 31862c8ac..be97f1930 100644 --- a/app/Entities/Controllers/PageExportController.php +++ b/app/Entities/Controllers/PageExportController.php @@ -2,7 +2,7 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Repos\PageRepo; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Entities\Tools\PageContent; use BookStack\Exceptions\NotFoundException; @@ -11,16 +11,10 @@ use Throwable; class PageExportController extends Controller { - protected $pageRepo; - protected $exportFormatter; - - /** - * PageExportController constructor. - */ - public function __construct(PageRepo $pageRepo, ExportFormatter $exportFormatter) - { - $this->pageRepo = $pageRepo; - $this->exportFormatter = $exportFormatter; + public function __construct( + protected PageQueries $queries, + protected ExportFormatter $exportFormatter, + ) { $this->middleware('can:content-export'); } @@ -33,7 +27,7 @@ class PageExportController extends Controller */ public function pdf(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $page->html = (new PageContent($page))->render(); $pdfContent = $this->exportFormatter->pageToPdf($page); @@ -48,7 +42,7 @@ class PageExportController extends Controller */ public function html(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $page->html = (new PageContent($page))->render(); $containedHtml = $this->exportFormatter->pageToContainedHtml($page); @@ -62,7 +56,7 @@ class PageExportController extends Controller */ public function plainText(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $pageText = $this->exportFormatter->pageToPlainText($page); return $this->download()->directly($pageText, $pageSlug . '.txt'); @@ -75,7 +69,7 @@ class PageExportController extends Controller */ public function markdown(string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->queries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $pageText = $this->exportFormatter->pageToMarkdown($page); return $this->download()->directly($pageText, $pageSlug . '.md'); diff --git a/app/Entities/Controllers/PageRevisionController.php b/app/Entities/Controllers/PageRevisionController.php index a3190a0fc..232d40668 100644 --- a/app/Entities/Controllers/PageRevisionController.php +++ b/app/Entities/Controllers/PageRevisionController.php @@ -4,6 +4,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Activity\ActivityType; use BookStack\Entities\Models\PageRevision; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Entities\Repos\RevisionRepo; use BookStack\Entities\Tools\PageContent; @@ -18,6 +19,7 @@ class PageRevisionController extends Controller { public function __construct( protected PageRepo $pageRepo, + protected PageQueries $pageQueries, protected RevisionRepo $revisionRepo, ) { } @@ -29,7 +31,7 @@ class PageRevisionController extends Controller */ public function index(Request $request, string $bookSlug, string $pageSlug) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->pageQueries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $listOptions = SimpleListOptions::fromRequest($request, 'page_revisions', true)->withSortOptions([ 'id' => trans('entities.pages_revisions_sort_number') ]); @@ -60,7 +62,7 @@ class PageRevisionController extends Controller */ public function show(string $bookSlug, string $pageSlug, int $revisionId) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->pageQueries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); /** @var ?PageRevision $revision */ $revision = $page->revisions()->where('id', '=', $revisionId)->first(); if ($revision === null) { @@ -89,7 +91,7 @@ class PageRevisionController extends Controller */ public function changes(string $bookSlug, string $pageSlug, int $revisionId) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->pageQueries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); /** @var ?PageRevision $revision */ $revision = $page->revisions()->where('id', '=', $revisionId)->first(); if ($revision === null) { @@ -121,7 +123,7 @@ class PageRevisionController extends Controller */ public function restore(string $bookSlug, string $pageSlug, int $revisionId) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->pageQueries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-update', $page); $page = $this->pageRepo->restoreRevision($page, $revisionId); @@ -136,7 +138,7 @@ class PageRevisionController extends Controller */ public function destroy(string $bookSlug, string $pageSlug, int $revId) { - $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); + $page = $this->pageQueries->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('page-delete', $page); $revision = $page->revisions()->where('id', '=', $revId)->first(); @@ -162,7 +164,7 @@ class PageRevisionController extends Controller */ public function destroyUserDraft(string $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->revisionRepo->deleteDraftsForCurrentUser($page); return response('', 200); diff --git a/app/Entities/Controllers/PageTemplateController.php b/app/Entities/Controllers/PageTemplateController.php index e4e7b5680..c0b972148 100644 --- a/app/Entities/Controllers/PageTemplateController.php +++ b/app/Entities/Controllers/PageTemplateController.php @@ -2,6 +2,7 @@ namespace BookStack\Entities\Controllers; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Exceptions\NotFoundException; use BookStack\Http\Controller; @@ -9,14 +10,10 @@ use Illuminate\Http\Request; class PageTemplateController extends Controller { - protected $pageRepo; - - /** - * PageTemplateController constructor. - */ - public function __construct(PageRepo $pageRepo) - { - $this->pageRepo = $pageRepo; + public function __construct( + protected PageRepo $pageRepo, + protected PageQueries $pageQueries, + ) { } /** @@ -26,7 +23,19 @@ class PageTemplateController extends Controller { $page = $request->get('page', 1); $search = $request->get('search', ''); - $templates = $this->pageRepo->getTemplates(10, $page, $search); + $count = 10; + + $query = $this->pageQueries->visibleTemplates() + ->orderBy('name', 'asc') + ->skip(($page - 1) * $count) + ->take($count); + + if ($search) { + $query->where('name', 'like', '%' . $search . '%'); + } + + $templates = $query->paginate($count, ['*'], 'page', $page); + $templates->withPath('/templates'); if ($search) { $templates->appends(['search' => $search]); @@ -44,7 +53,7 @@ class PageTemplateController extends Controller */ public function get(int $templateId) { - $page = $this->pageRepo->getById($templateId); + $page = $this->pageQueries->findVisibleByIdOrFail($templateId); if (!$page->template) { throw new NotFoundException(); diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 5e85523cf..3d7474b3d 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -18,7 +18,7 @@ class BookQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } - public function findVisibleBySlug(string $slug): Book + public function findVisibleBySlugOrFail(string $slug): Book { /** @var ?Book $book */ $book = $this->start() diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index 52e123087..d61607e7a 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -18,7 +18,7 @@ class BookshelfQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } - public function findVisibleBySlug(string $slug): Bookshelf + public function findVisibleBySlugOrFail(string $slug): Bookshelf { /** @var ?Bookshelf $shelf */ $shelf = $this->start() diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php index dcfc4aad3..200514932 100644 --- a/app/Entities/Queries/ChapterQueries.php +++ b/app/Entities/Queries/ChapterQueries.php @@ -24,7 +24,7 @@ class ChapterQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } - public function findVisibleBySlugs(string $bookSlug, string $chapterSlug): Chapter + public function findVisibleBySlugsOrFail(string $bookSlug, string $chapterSlug): Chapter { /** @var ?Chapter $chapter */ $chapter = $this->start()->with('book') @@ -34,7 +34,7 @@ class ChapterQueries implements ProvidesEntityQueries ->where('slug', '=', $chapterSlug) ->first(); - if ($chapter === null) { + if (is_null($chapter)) { throw new NotFoundException(trans('errors.chapter_not_found')); } diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php index 39a21c913..31e5b913a 100644 --- a/app/Entities/Queries/EntityQueries.php +++ b/app/Entities/Queries/EntityQueries.php @@ -11,6 +11,7 @@ class EntityQueries public BookQueries $books, public ChapterQueries $chapters, public PageQueries $pages, + public PageRevisionQueries $revisions, ) { } diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index f1991626f..1640dc2db 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -3,6 +3,7 @@ namespace BookStack\Entities\Queries; use BookStack\Entities\Models\Page; +use BookStack\Exceptions\NotFoundException; use Illuminate\Database\Eloquent\Builder; class PageQueries implements ProvidesEntityQueries @@ -17,6 +18,34 @@ class PageQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } + public function findVisibleByIdOrFail(int $id): Page + { + $page = $this->findVisibleById($id); + + if (is_null($page)) { + throw new NotFoundException(trans('errors.page_not_found')); + } + + return $page; + } + + public function findVisibleBySlugsOrFail(string $bookSlug, string $pageSlug): Page + { + /** @var ?Page $page */ + $page = $this->start()->with('book') + ->whereHas('book', function (Builder $query) use ($bookSlug) { + $query->where('slug', '=', $bookSlug); + }) + ->where('slug', '=', $pageSlug) + ->first(); + + if (is_null($page)) { + throw new NotFoundException(trans('errors.chapter_not_found')); + } + + return $page; + } + public function visibleForList(): Builder { return $this->start() @@ -33,4 +62,10 @@ class PageQueries implements ProvidesEntityQueries ->where('draft', '=', true) ->where('created_by', '=', user()->id); } + + public function visibleTemplates(): Builder + { + return $this->visibleForList() + ->where('template', '=', true); + } } diff --git a/app/Entities/Queries/PageRevisionQueries.php b/app/Entities/Queries/PageRevisionQueries.php new file mode 100644 index 000000000..2dcd428f5 --- /dev/null +++ b/app/Entities/Queries/PageRevisionQueries.php @@ -0,0 +1,41 @@ +whereHas('page', function (Builder $query) { + $query->scopes('visible'); + }) + ->where('slug', '=', $pageSlug) + ->where('type', '=', 'version') + ->where('book_slug', '=', $bookSlug) + ->orderBy('created_at', 'desc') + ->first(); + } + + public function findLatestCurrentUserDraftsForPageId(int $pageId): ?PageRevision + { + return $this->latestCurrentUserDraftsForPageId($pageId)->first(); + } + + public function latestCurrentUserDraftsForPageId(int $pageId): Builder + { + return $this->start() + ->where('created_by', '=', user()->id) + ->where('type', 'update_draft') + ->where('page_id', '=', $pageId) + ->orderBy('created_at', 'desc'); + } +} diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php index 5c37b02e4..ea83d6cdd 100644 --- a/app/Entities/Queries/ProvidesEntityQueries.php +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -2,9 +2,18 @@ namespace BookStack\Entities\Queries; -use BookStack\Entities\Models\Entity; +use BookStack\App\Model; use Illuminate\Database\Eloquent\Builder; +/** + * Interface for our classes which provide common queries for our + * entity objects. Ideally all queries for entities should run through + * these classes. + * Any added methods should return a builder instances to allow extension + * via building on the query, unless the method starts with 'find' + * in which case an entity object should be returned. + * (nullable unless it's a *OrFail method). + */ interface ProvidesEntityQueries { public function start(): Builder; diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 929de528d..74aed072a 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -14,13 +14,11 @@ use BookStack\Entities\Tools\PageContent; use BookStack\Entities\Tools\PageEditorData; use BookStack\Entities\Tools\TrashCan; use BookStack\Exceptions\MoveOperationException; -use BookStack\Exceptions\NotFoundException; use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; use BookStack\References\ReferenceStore; use BookStack\References\ReferenceUpdater; use Exception; -use Illuminate\Pagination\LengthAwarePaginator; class PageRepo { @@ -33,91 +31,6 @@ class PageRepo ) { } - /** - * Get a page by ID. - * - * @throws NotFoundException - */ - public function getById(int $id, array $relations = ['book']): Page - { - /** @var Page $page */ - $page = Page::visible()->with($relations)->find($id); - - if (!$page) { - throw new NotFoundException(trans('errors.page_not_found')); - } - - return $page; - } - - /** - * Get a page its book and own slug. - * - * @throws NotFoundException - */ - public function getBySlug(string $bookSlug, string $pageSlug): Page - { - $page = Page::visible()->whereSlugs($bookSlug, $pageSlug)->first(); - - if (!$page) { - throw new NotFoundException(trans('errors.page_not_found')); - } - - return $page; - } - - /** - * Get a page by its old slug but checking the revisions table - * for the last revision that matched the given page and book slug. - */ - public function getByOldSlug(string $bookSlug, string $pageSlug): ?Page - { - $revision = $this->revisionRepo->getBySlugs($bookSlug, $pageSlug); - - return $revision->page ?? null; - } - - /** - * Get pages that have been marked as a template. - */ - public function getTemplates(int $count = 10, int $page = 1, string $search = ''): LengthAwarePaginator - { - $query = Page::visible() - ->where('template', '=', true) - ->orderBy('name', 'asc') - ->skip(($page - 1) * $count) - ->take($count); - - if ($search) { - $query->where('name', 'like', '%' . $search . '%'); - } - - $paginator = $query->paginate($count, ['*'], 'page', $page); - $paginator->withPath('/templates'); - - return $paginator; - } - - /** - * Get a parent item via slugs. - */ - public function getParentFromSlugs(string $bookSlug, string $chapterSlug = null): Entity - { - if ($chapterSlug !== null) { - return Chapter::visible()->whereSlugs($bookSlug, $chapterSlug)->firstOrFail(); - } - - return Book::visible()->where('slug', '=', $bookSlug)->firstOrFail(); - } - - /** - * Get the draft copy of the given page for the current user. - */ - public function getUserDraft(Page $page): ?PageRevision - { - return $this->revisionRepo->getLatestDraftForCurrentUser($page); - } - /** * Get a new draft page belonging to the given parent entity. */ diff --git a/app/Entities/Tools/PageEditorData.php b/app/Entities/Tools/PageEditorData.php index 3c7c9e2ea..20bf19eb2 100644 --- a/app/Entities/Tools/PageEditorData.php +++ b/app/Entities/Tools/PageEditorData.php @@ -4,7 +4,7 @@ namespace BookStack\Entities\Tools; use BookStack\Activity\Tools\CommentTree; use BookStack\Entities\Models\Page; -use BookStack\Entities\Repos\PageRepo; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Tools\Markdown\HtmlToMarkdown; use BookStack\Entities\Tools\Markdown\MarkdownToHtml; @@ -15,7 +15,7 @@ class PageEditorData public function __construct( protected Page $page, - protected PageRepo $pageRepo, + protected EntityQueries $queries, protected string $requestedEditor ) { $this->viewData = $this->build(); @@ -35,7 +35,11 @@ class PageEditorData { $page = clone $this->page; $isDraft = boolval($this->page->draft); - $templates = $this->pageRepo->getTemplates(10); + $templates = $this->queries->pages->visibleTemplates() + ->orderBy('name', 'asc') + ->take(10) + ->get(); + $draftsEnabled = auth()->check(); $isDraftRevision = false; @@ -47,8 +51,8 @@ class PageEditorData } // Check for a current draft version for this user - $userDraft = $this->pageRepo->getUserDraft($page); - if ($userDraft !== null) { + $userDraft = $this->queries->revisions->findLatestCurrentUserDraftsForPageId($page->id)->first(); + if (!is_null($userDraft)) { $page->forceFill($userDraft->only(['name', 'html', 'markdown'])); $isDraftRevision = true; $this->warnings[] = $editActivity->getEditingActiveDraftMessage($userDraft); diff --git a/app/Uploads/Controllers/AttachmentController.php b/app/Uploads/Controllers/AttachmentController.php index e61c10338..809cdfa58 100644 --- a/app/Uploads/Controllers/AttachmentController.php +++ b/app/Uploads/Controllers/AttachmentController.php @@ -2,6 +2,7 @@ namespace BookStack\Uploads\Controllers; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Exceptions\FileUploadException; use BookStack\Exceptions\NotFoundException; @@ -18,6 +19,7 @@ class AttachmentController extends Controller { public function __construct( protected AttachmentService $attachmentService, + protected PageQueries $pageQueries, protected PageRepo $pageRepo ) { } @@ -36,7 +38,7 @@ class AttachmentController extends Controller ]); $pageId = $request->get('uploaded_to'); - $page = $this->pageRepo->getById($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->checkPermission('attachment-create-all'); $this->checkOwnablePermission('page-update', $page); @@ -152,7 +154,7 @@ class AttachmentController extends Controller ]), 422); } - $page = $this->pageRepo->getById($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->checkPermission('attachment-create-all'); $this->checkOwnablePermission('page-update', $page); @@ -173,7 +175,7 @@ class AttachmentController extends Controller */ public function listForPage(int $pageId) { - $page = $this->pageRepo->getById($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-view', $page); return view('attachments.manager-list', [ @@ -192,7 +194,7 @@ class AttachmentController extends Controller $this->validate($request, [ 'order' => ['required', 'array'], ]); - $page = $this->pageRepo->getById($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-update', $page); $attachmentOrder = $request->get('order'); @@ -213,7 +215,7 @@ class AttachmentController extends Controller $attachment = Attachment::query()->findOrFail($attachmentId); try { - $page = $this->pageRepo->getById($attachment->uploaded_to); + $page = $this->pageQueries->findVisibleByIdOrFail($attachment->uploaded_to); } catch (NotFoundException $exception) { throw new NotFoundException(trans('errors.attachment_not_found')); } From c95f4ca40fecf0584eccb5f89d49b245d4ec7369 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 7 Feb 2024 15:09:16 +0000 Subject: [PATCH 06/12] Queries: Migrated revision repo queries to new class --- app/Entities/Queries/PageRevisionQueries.php | 5 +- .../Queries/ProvidesEntityQueries.php | 2 +- app/Entities/Repos/RevisionRepo.php | 50 +++---------------- 3 files changed, 11 insertions(+), 46 deletions(-) diff --git a/app/Entities/Queries/PageRevisionQueries.php b/app/Entities/Queries/PageRevisionQueries.php index 2dcd428f5..6e017a742 100644 --- a/app/Entities/Queries/PageRevisionQueries.php +++ b/app/Entities/Queries/PageRevisionQueries.php @@ -27,7 +27,10 @@ class PageRevisionQueries public function findLatestCurrentUserDraftsForPageId(int $pageId): ?PageRevision { - return $this->latestCurrentUserDraftsForPageId($pageId)->first(); + /** @var ?PageRevision $revision */ + $revision = $this->latestCurrentUserDraftsForPageId($pageId)->first(); + + return $revision; } public function latestCurrentUserDraftsForPageId(int $pageId): Builder diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php index ea83d6cdd..103352244 100644 --- a/app/Entities/Queries/ProvidesEntityQueries.php +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -2,7 +2,7 @@ namespace BookStack\Entities\Queries; -use BookStack\App\Model; +use BookStack\Entities\Models\Entity; use Illuminate\Database\Eloquent\Builder; /** diff --git a/app/Entities/Repos/RevisionRepo.php b/app/Entities/Repos/RevisionRepo.php index 064327ee9..daf55777c 100644 --- a/app/Entities/Repos/RevisionRepo.php +++ b/app/Entities/Repos/RevisionRepo.php @@ -4,39 +4,13 @@ namespace BookStack\Entities\Repos; use BookStack\Entities\Models\Page; use BookStack\Entities\Models\PageRevision; -use Illuminate\Database\Eloquent\Builder; +use BookStack\Entities\Queries\PageRevisionQueries; class RevisionRepo { - /** - * Get a revision by its stored book and page slug values. - */ - public function getBySlugs(string $bookSlug, string $pageSlug): ?PageRevision - { - /** @var ?PageRevision $revision */ - $revision = PageRevision::query() - ->whereHas('page', function (Builder $query) { - $query->scopes('visible'); - }) - ->where('slug', '=', $pageSlug) - ->where('type', '=', 'version') - ->where('book_slug', '=', $bookSlug) - ->orderBy('created_at', 'desc') - ->with('page') - ->first(); - - return $revision; - } - - /** - * Get the latest draft revision, for the given page, belonging to the current user. - */ - public function getLatestDraftForCurrentUser(Page $page): ?PageRevision - { - /** @var ?PageRevision $revision */ - $revision = $this->queryForCurrentUserDraft($page->id)->first(); - - return $revision; + public function __construct( + protected PageRevisionQueries $queries, + ) { } /** @@ -44,7 +18,7 @@ class RevisionRepo */ public function deleteDraftsForCurrentUser(Page $page): void { - $this->queryForCurrentUserDraft($page->id)->delete(); + $this->queries->latestCurrentUserDraftsForPageId($page->id)->delete(); } /** @@ -53,7 +27,7 @@ class RevisionRepo */ public function getNewDraftForCurrentUser(Page $page): PageRevision { - $draft = $this->getLatestDraftForCurrentUser($page); + $draft = $this->queries->findLatestCurrentUserDraftsForPageId($page->id); if ($draft) { return $draft; @@ -116,16 +90,4 @@ class RevisionRepo PageRevision::query()->whereIn('id', $revisionsToDelete->pluck('id'))->delete(); } } - - /** - * Query update draft revisions for the current user. - */ - protected function queryForCurrentUserDraft(int $pageId): Builder - { - return PageRevision::query() - ->where('created_by', '=', user()->id) - ->where('type', 'update_draft') - ->where('page_id', '=', $pageId) - ->orderBy('created_at', 'desc'); - } } From 483410749bdfb03d1e3fb98db82c092583a88449 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 7 Feb 2024 16:37:36 +0000 Subject: [PATCH 07/12] Queries: Updated all app book static query uses --- app/App/HomeController.php | 2 +- .../Controllers/BookApiController.php | 12 +++--- .../Controllers/BookExportApiController.php | 19 +++++---- .../Controllers/BookshelfController.php | 10 +++-- .../Controllers/ChapterApiController.php | 17 ++++---- .../Controllers/PageApiController.php | 15 ++++--- app/Entities/Controllers/PageController.php | 8 ++-- .../Controllers/RecycleBinController.php | 4 +- app/Entities/Models/Book.php | 11 +---- app/Entities/Queries/BookQueries.php | 5 +++ app/Entities/Queries/ChapterQueries.php | 19 ++++++++- app/Entities/Queries/PageQueries.php | 22 ++++++++++ app/Entities/Repos/BookRepo.php | 8 ++-- app/Entities/Repos/BookshelfRepo.php | 11 ++--- app/Entities/Repos/ChapterRepo.php | 6 +-- app/Entities/Repos/PageRepo.php | 8 ++-- app/Entities/Tools/BookContents.php | 38 ++++++++++-------- app/Entities/Tools/Cloner.php | 2 +- app/Entities/Tools/SiblingFetcher.php | 12 ++++-- app/Entities/Tools/TrashCan.php | 12 +++++- app/Permissions/JointPermissionBuilder.php | 13 ++++-- app/Permissions/PermissionsController.php | 40 +++++++++---------- app/References/CrossLinkParser.php | 13 +++--- .../ModelResolvers/BookLinkModelResolver.php | 8 +++- .../BookshelfLinkModelResolver.php | 7 +++- .../ChapterLinkModelResolver.php | 8 +++- .../ModelResolvers/PageLinkModelResolver.php | 8 +++- .../PagePermalinkModelResolver.php | 8 +++- app/References/ReferenceController.php | 12 +++--- app/Search/SearchController.php | 4 +- app/Settings/MaintenanceController.php | 4 +- app/Uploads/ImageService.php | 8 ++-- .../Controllers/UserProfileController.php | 19 ++++++--- app/Users/Queries/UserContentCounts.php | 19 +++++---- .../Queries/UserRecentlyCreatedContent.php | 18 +++++---- tests/Entity/BookTest.php | 6 +-- tests/Entity/EntitySearchTest.php | 4 +- 37 files changed, 278 insertions(+), 162 deletions(-) diff --git a/app/App/HomeController.php b/app/App/HomeController.php index f5a3c7ed6..6a4cb0176 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -40,7 +40,7 @@ class HomeController extends Controller $recentFactor = count($draftPages) > 0 ? 0.5 : 1; $recents = $this->isSignedIn() ? (new RecentlyViewed())->run(12 * $recentFactor, 1) - : Book::visible()->orderBy('created_at', 'desc')->take(12 * $recentFactor)->get(); + : $this->queries->books->visibleForList()->orderBy('created_at', 'desc')->take(12 * $recentFactor)->get(); $favourites = (new TopFavourites())->run(6); $recentlyUpdatedPages = $this->queries->pages->visibleForList() ->where('draft', false) diff --git a/app/Entities/Controllers/BookApiController.php b/app/Entities/Controllers/BookApiController.php index aa21aea47..955bd707b 100644 --- a/app/Entities/Controllers/BookApiController.php +++ b/app/Entities/Controllers/BookApiController.php @@ -6,6 +6,7 @@ use BookStack\Api\ApiEntityListFormatter; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Repos\BookRepo; use BookStack\Entities\Tools\BookContents; use BookStack\Http\ApiController; @@ -15,7 +16,8 @@ use Illuminate\Validation\ValidationException; class BookApiController extends ApiController { public function __construct( - protected BookRepo $bookRepo + protected BookRepo $bookRepo, + protected BookQueries $queries, ) { } @@ -24,7 +26,7 @@ class BookApiController extends ApiController */ public function list() { - $books = Book::visible(); + $books = $this->queries->visibleForList(); return $this->apiListingResponse($books, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', @@ -56,7 +58,7 @@ class BookApiController extends ApiController */ public function read(string $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail(intval($id)); $book = $this->forJsonDisplay($book); $book->load(['createdBy', 'updatedBy', 'ownedBy']); @@ -83,7 +85,7 @@ class BookApiController extends ApiController */ public function update(Request $request, string $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('book-update', $book); $requestData = $this->validate($request, $this->rules()['update']); @@ -100,7 +102,7 @@ class BookApiController extends ApiController */ public function delete(string $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('book-delete', $book); $this->bookRepo->destroy($book); diff --git a/app/Entities/Controllers/BookExportApiController.php b/app/Entities/Controllers/BookExportApiController.php index 5b6826c19..1161ddb88 100644 --- a/app/Entities/Controllers/BookExportApiController.php +++ b/app/Entities/Controllers/BookExportApiController.php @@ -2,18 +2,17 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Models\Book; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\ApiController; use Throwable; class BookExportApiController extends ApiController { - protected $exportFormatter; - - public function __construct(ExportFormatter $exportFormatter) - { - $this->exportFormatter = $exportFormatter; + public function __construct( + protected ExportFormatter $exportFormatter, + protected BookQueries $queries, + ) { $this->middleware('can:content-export'); } @@ -24,7 +23,7 @@ class BookExportApiController extends ApiController */ public function exportPdf(int $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail($id); $pdfContent = $this->exportFormatter->bookToPdf($book); return $this->download()->directly($pdfContent, $book->slug . '.pdf'); @@ -37,7 +36,7 @@ class BookExportApiController extends ApiController */ public function exportHtml(int $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail($id); $htmlContent = $this->exportFormatter->bookToContainedHtml($book); return $this->download()->directly($htmlContent, $book->slug . '.html'); @@ -48,7 +47,7 @@ class BookExportApiController extends ApiController */ public function exportPlainText(int $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail($id); $textContent = $this->exportFormatter->bookToPlainText($book); return $this->download()->directly($textContent, $book->slug . '.txt'); @@ -59,7 +58,7 @@ class BookExportApiController extends ApiController */ public function exportMarkdown(int $id) { - $book = Book::visible()->findOrFail($id); + $book = $this->queries->findVisibleByIdOrFail($id); $markdown = $this->exportFormatter->bookToMarkdown($book); return $this->download()->directly($markdown, $book->slug . '.md'); diff --git a/app/Entities/Controllers/BookshelfController.php b/app/Entities/Controllers/BookshelfController.php index 3118325da..6cedd23e7 100644 --- a/app/Entities/Controllers/BookshelfController.php +++ b/app/Entities/Controllers/BookshelfController.php @@ -4,7 +4,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Activity\ActivityQueries; use BookStack\Activity\Models\View; -use BookStack\Entities\Models\Book; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Repos\BookshelfRepo; use BookStack\Entities\Tools\ShelfContext; @@ -22,6 +22,7 @@ class BookshelfController extends Controller public function __construct( protected BookshelfRepo $shelfRepo, protected BookshelfQueries $queries, + protected BookQueries $bookQueries, protected ShelfContext $shelfContext, protected ReferenceFetcher $referenceFetcher, ) { @@ -68,7 +69,7 @@ class BookshelfController extends Controller public function create() { $this->checkPermission('bookshelf-create-all'); - $books = Book::visible()->orderBy('name')->get(['name', 'id', 'slug', 'created_at', 'updated_at']); + $books = $this->bookQueries->visibleForList()->orderBy('name')->get(['name', 'id', 'slug', 'created_at', 'updated_at']); $this->setPageTitle(trans('entities.shelves_create')); return view('shelves.create', ['books' => $books]); @@ -145,7 +146,10 @@ class BookshelfController extends Controller $this->checkOwnablePermission('bookshelf-update', $shelf); $shelfBookIds = $shelf->books()->get(['id'])->pluck('id'); - $books = Book::visible()->whereNotIn('id', $shelfBookIds)->orderBy('name')->get(['name', 'id', 'slug', 'created_at', 'updated_at']); + $books = $this->bookQueries->visibleForList() + ->whereNotIn('id', $shelfBookIds) + ->orderBy('name') + ->get(['name', 'id', 'slug', 'created_at', 'updated_at']); $this->setPageTitle(trans('entities.shelves_edit_named', ['name' => $shelf->getShortName()])); diff --git a/app/Entities/Controllers/ChapterApiController.php b/app/Entities/Controllers/ChapterApiController.php index 3fbe85222..fb484b85d 100644 --- a/app/Entities/Controllers/ChapterApiController.php +++ b/app/Entities/Controllers/ChapterApiController.php @@ -2,8 +2,9 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; +use BookStack\Entities\Queries\BookQueries; +use BookStack\Entities\Queries\ChapterQueries; use BookStack\Entities\Repos\ChapterRepo; use BookStack\Exceptions\PermissionsException; use BookStack\Http\ApiController; @@ -35,7 +36,9 @@ class ChapterApiController extends ApiController ]; public function __construct( - protected ChapterRepo $chapterRepo + protected ChapterRepo $chapterRepo, + protected ChapterQueries $queries, + protected BookQueries $bookQueries, ) { } @@ -44,7 +47,7 @@ class ChapterApiController extends ApiController */ public function list() { - $chapters = Chapter::visible(); + $chapters = $this->queries->visibleForList(); return $this->apiListingResponse($chapters, [ 'id', 'book_id', 'name', 'slug', 'description', 'priority', @@ -60,7 +63,7 @@ class ChapterApiController extends ApiController $requestData = $this->validate($request, $this->rules['create']); $bookId = $request->get('book_id'); - $book = Book::visible()->findOrFail($bookId); + $book = $this->bookQueries->findVisibleByIdOrFail(intval($bookId)); $this->checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($requestData, $book); @@ -73,7 +76,7 @@ class ChapterApiController extends ApiController */ public function read(string $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $chapter = $this->forJsonDisplay($chapter); $chapter->load([ @@ -94,7 +97,7 @@ class ChapterApiController extends ApiController public function update(Request $request, string $id) { $requestData = $this->validate($request, $this->rules()['update']); - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('chapter-update', $chapter); if ($request->has('book_id') && $chapter->book_id !== intval($requestData['book_id'])) { @@ -122,7 +125,7 @@ class ChapterApiController extends ApiController */ public function delete(string $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('chapter-delete', $chapter); $this->chapterRepo->destroy($chapter); diff --git a/app/Entities/Controllers/PageApiController.php b/app/Entities/Controllers/PageApiController.php index 6e3880aed..d2a5a3ee3 100644 --- a/app/Entities/Controllers/PageApiController.php +++ b/app/Entities/Controllers/PageApiController.php @@ -2,9 +2,7 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; use BookStack\Exceptions\PermissionsException; @@ -38,6 +36,7 @@ class PageApiController extends ApiController public function __construct( protected PageRepo $pageRepo, protected PageQueries $queries, + protected EntityQueries $entityQueries, ) { } @@ -46,7 +45,7 @@ class PageApiController extends ApiController */ public function list() { - $pages = Page::visible(); + $pages = $this->queries->visibleForList(); return $this->apiListingResponse($pages, [ 'id', 'book_id', 'chapter_id', 'name', 'slug', 'priority', @@ -72,9 +71,9 @@ class PageApiController extends ApiController $this->validate($request, $this->rules['create']); if ($request->has('chapter_id')) { - $parent = Chapter::visible()->findOrFail($request->get('chapter_id')); + $parent = $this->entityQueries->chapters->findVisibleByIdOrFail(intval($request->get('chapter_id'))); } else { - $parent = Book::visible()->findOrFail($request->get('book_id')); + $parent = $this->entityQueries->books->findVisibleByIdOrFail(intval($request->get('book_id'))); } $this->checkOwnablePermission('page-create', $parent); @@ -120,9 +119,9 @@ class PageApiController extends ApiController $parent = null; if ($request->has('chapter_id')) { - $parent = Chapter::visible()->findOrFail($request->get('chapter_id')); + $parent = $this->entityQueries->chapters->findVisibleByIdOrFail(intval($request->get('chapter_id'))); } elseif ($request->has('book_id')) { - $parent = Book::visible()->findOrFail($request->get('book_id')); + $parent = $this->entityQueries->books->findVisibleByIdOrFail(intval($request->get('book_id'))); } if ($parent && !$parent->matches($page->getParent())) { diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index 3a5bdbd0b..471df8184 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -276,8 +276,8 @@ class PageController extends Controller $this->checkOwnablePermission('page-delete', $page); $this->setPageTitle(trans('entities.pages_delete_named', ['pageName' => $page->getShortName()])); $usedAsTemplate = - Book::query()->where('default_template_id', '=', $page->id)->count() > 0 || - Chapter::query()->where('default_template_id', '=', $page->id)->count() > 0; + $this->entityQueries->books->start()->where('default_template_id', '=', $page->id)->count() > 0 || + $this->entityQueries->chapters->start()->where('default_template_id', '=', $page->id)->count() > 0; return view('pages.delete', [ 'book' => $page->book, @@ -298,8 +298,8 @@ class PageController extends Controller $this->checkOwnablePermission('page-update', $page); $this->setPageTitle(trans('entities.pages_delete_draft_named', ['pageName' => $page->getShortName()])); $usedAsTemplate = - Book::query()->where('default_template_id', '=', $page->id)->count() > 0 || - Chapter::query()->where('default_template_id', '=', $page->id)->count() > 0; + $this->entityQueries->books->start()->where('default_template_id', '=', $page->id)->count() > 0 || + $this->entityQueries->chapters->start()->where('default_template_id', '=', $page->id)->count() > 0; return view('pages.delete', [ 'book' => $page->book, diff --git a/app/Entities/Controllers/RecycleBinController.php b/app/Entities/Controllers/RecycleBinController.php index 78f86a5ae..d11dde4dd 100644 --- a/app/Entities/Controllers/RecycleBinController.php +++ b/app/Entities/Controllers/RecycleBinController.php @@ -116,9 +116,9 @@ class RecycleBinController extends Controller * * @throws \Exception */ - public function empty() + public function empty(TrashCan $trash) { - $deleteCount = (new TrashCan())->empty(); + $deleteCount = $trash->empty(); $this->logActivity(ActivityType::RECYCLE_BIN_EMPTY); $this->showSuccessNotification(trans('settings.recycle_bin_destroy_notification', ['count' => $deleteCount])); diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index 14cb790c5..c1644dcf5 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -117,20 +117,11 @@ class Book extends Entity implements HasCoverImage /** * Get the direct child items within this book. */ - public function getDirectChildren(): Collection + public function getDirectVisibleChildren(): Collection { $pages = $this->directPages()->scopes('visible')->get(); $chapters = $this->chapters()->scopes('visible')->get(); return $pages->concat($chapters)->sortBy('priority')->sortByDesc('draft'); } - - /** - * Get a visible book by its slug. - * @throws \Illuminate\Database\Eloquent\ModelNotFoundException - */ - public static function getBySlug(string $slug): self - { - return static::visible()->where('slug', '=', $slug)->firstOrFail(); - } } diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 3d7474b3d..2ffff5eca 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -18,6 +18,11 @@ class BookQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } + public function findVisibleByIdOrFail(int $id): Book + { + return $this->start()->scopes('visible')->findOrFail($id); + } + public function findVisibleBySlugOrFail(string $slug): Book { /** @var ?Book $book */ diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php index 200514932..31193c7ea 100644 --- a/app/Entities/Queries/ChapterQueries.php +++ b/app/Entities/Queries/ChapterQueries.php @@ -24,10 +24,17 @@ class ChapterQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } + public function findVisibleByIdOrFail(int $id): Chapter + { + return $this->start()->scopes('visible')->findOrFail($id); + } + public function findVisibleBySlugsOrFail(string $bookSlug, string $chapterSlug): Chapter { /** @var ?Chapter $chapter */ - $chapter = $this->start()->with('book') + $chapter = $this->start() + ->scopes('visible') + ->with('book') ->whereHas('book', function (Builder $query) use ($bookSlug) { $query->where('slug', '=', $bookSlug); }) @@ -41,9 +48,19 @@ class ChapterQueries implements ProvidesEntityQueries return $chapter; } + public function usingSlugs(string $bookSlug, string $chapterSlug): Builder + { + return $this->start() + ->where('slug', '=', $chapterSlug) + ->whereHas('book', function (Builder $query) use ($bookSlug) { + $query->where('slug', '=', $bookSlug); + }); + } + public function visibleForList(): Builder { return $this->start() + ->scopes('visible') ->select(array_merge(static::$listAttributes, ['book_slug' => function ($builder) { $builder->select('slug') ->from('books') diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index 1640dc2db..e22769c3a 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -33,6 +33,7 @@ class PageQueries implements ProvidesEntityQueries { /** @var ?Page $page */ $page = $this->start()->with('book') + ->scopes('visible') ->whereHas('book', function (Builder $query) use ($bookSlug) { $query->where('slug', '=', $bookSlug); }) @@ -46,9 +47,19 @@ class PageQueries implements ProvidesEntityQueries return $page; } + public function usingSlugs(string $bookSlug, string $pageSlug): Builder + { + return $this->start() + ->where('slug', '=', $pageSlug) + ->whereHas('book', function (Builder $query) use ($bookSlug) { + $query->where('slug', '=', $bookSlug); + }); + } + public function visibleForList(): Builder { return $this->start() + ->scopes('visible') ->select(array_merge(Page::$listAttributes, ['book_slug' => function ($builder) { $builder->select('slug') ->from('books') @@ -56,6 +67,17 @@ class PageQueries implements ProvidesEntityQueries }])); } + public function visibleWithContents(): Builder + { + return $this->start() + ->scopes('visible') + ->select(array_merge(Page::$contentAttributes, ['book_slug' => function ($builder) { + $builder->select('slug') + ->from('books') + ->whereColumn('books.id', '=', 'pages.book_id'); + }])); + } + public function currentUserDraftsForList(): Builder { return $this->visibleForList() diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index 26b9414fb..19d159eb1 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -17,7 +17,8 @@ class BookRepo public function __construct( protected BaseRepo $baseRepo, protected TagRepo $tagRepo, - protected ImageRepo $imageRepo + protected ImageRepo $imageRepo, + protected TrashCan $trashCan, ) { } @@ -73,10 +74,9 @@ class BookRepo */ public function destroy(Book $book) { - $trashCan = new TrashCan(); - $trashCan->softDestroyBook($book); + $this->trashCan->softDestroyBook($book); Activity::add(ActivityType::BOOK_DELETE, $book); - $trashCan->autoClearOld(); + $this->trashCan->autoClearOld(); } } diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 479e6178f..a00349ef1 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -3,8 +3,8 @@ namespace BookStack\Entities\Repos; use BookStack\Activity\ActivityType; -use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\TrashCan; use BookStack\Facades\Activity; use Exception; @@ -13,6 +13,8 @@ class BookshelfRepo { public function __construct( protected BaseRepo $baseRepo, + protected BookQueries $bookQueries, + protected TrashCan $trashCan, ) { } @@ -60,7 +62,7 @@ class BookshelfRepo return intval($id); }); - $syncData = Book::visible() + $syncData = $this->bookQueries->visibleForList() ->whereIn('id', $bookIds) ->pluck('id') ->mapWithKeys(function ($bookId) use ($numericIDs) { @@ -77,9 +79,8 @@ class BookshelfRepo */ public function destroy(Bookshelf $shelf) { - $trashCan = new TrashCan(); - $trashCan->softDestroyShelf($shelf); + $this->trashCan->softDestroyShelf($shelf); Activity::add(ActivityType::BOOKSHELF_DELETE, $shelf); - $trashCan->autoClearOld(); + $this->trashCan->autoClearOld(); } } diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index cacca93f6..17cbccd41 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -18,6 +18,7 @@ class ChapterRepo public function __construct( protected BaseRepo $baseRepo, protected EntityQueries $entityQueries, + protected TrashCan $trashCan, ) { } @@ -59,10 +60,9 @@ class ChapterRepo */ public function destroy(Chapter $chapter) { - $trashCan = new TrashCan(); - $trashCan->softDestroyChapter($chapter); + $this->trashCan->softDestroyChapter($chapter); Activity::add(ActivityType::CHAPTER_DELETE, $chapter); - $trashCan->autoClearOld(); + $this->trashCan->autoClearOld(); } /** diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 74aed072a..2526b6c44 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -27,7 +27,8 @@ class PageRepo protected RevisionRepo $revisionRepo, protected EntityQueries $entityQueries, protected ReferenceStore $referenceStore, - protected ReferenceUpdater $referenceUpdater + protected ReferenceUpdater $referenceUpdater, + protected TrashCan $trashCan, ) { } @@ -184,10 +185,9 @@ class PageRepo */ public function destroy(Page $page) { - $trashCan = new TrashCan(); - $trashCan->softDestroyPage($page); + $this->trashCan->softDestroyPage($page); Activity::add(ActivityType::PAGE_DELETE, $page); - $trashCan->autoClearOld(); + $this->trashCan->autoClearOld(); } /** diff --git a/app/Entities/Tools/BookContents.php b/app/Entities/Tools/BookContents.php index f45bdfcc1..7fa2134b7 100644 --- a/app/Entities/Tools/BookContents.php +++ b/app/Entities/Tools/BookContents.php @@ -7,15 +7,17 @@ use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use Illuminate\Support\Collection; class BookContents { - protected Book $book; + protected EntityQueries $queries; - public function __construct(Book $book) - { - $this->book = $book; + public function __construct( + protected Book $book, + ) { + $this->queries = app()->make(EntityQueries::class); } /** @@ -23,10 +25,12 @@ class BookContents */ public function getLastPriority(): int { - $maxPage = Page::visible()->where('book_id', '=', $this->book->id) + $maxPage = $this->book->pages() ->where('draft', '=', false) - ->where('chapter_id', '=', 0)->max('priority'); - $maxChapter = Chapter::visible()->where('book_id', '=', $this->book->id) + ->where('chapter_id', '=', 0) + ->max('priority'); + + $maxChapter = $this->book->chapters() ->max('priority'); return max($maxChapter, $maxPage, 1); @@ -38,7 +42,7 @@ class BookContents public function getTree(bool $showDrafts = false, bool $renderPages = false): Collection { $pages = $this->getPages($showDrafts, $renderPages); - $chapters = Chapter::visible()->where('book_id', '=', $this->book->id)->get(); + $chapters = $this->book->chapters()->scopes('visible')->get(); $all = collect()->concat($pages)->concat($chapters); $chapterMap = $chapters->keyBy('id'); $lonePages = collect(); @@ -87,15 +91,17 @@ class BookContents */ protected function getPages(bool $showDrafts = false, bool $getPageContent = false): Collection { - $query = Page::visible() - ->select($getPageContent ? Page::$contentAttributes : Page::$listAttributes) - ->where('book_id', '=', $this->book->id); + if ($getPageContent) { + $query = $this->queries->pages->visibleWithContents(); + } else { + $query = $this->queries->pages->visibleForList(); + } if (!$showDrafts) { $query->where('draft', '=', false); } - return $query->get(); + return $query->where('book_id', '=', $this->book->id)->get(); } /** @@ -126,7 +132,7 @@ class BookContents /** @var Book[] $booksInvolved */ $booksInvolved = array_values(array_filter($modelMap, function (string $key) { - return strpos($key, 'book:') === 0; + return str_starts_with($key, 'book:'); }, ARRAY_FILTER_USE_KEY)); // Update permissions of books involved @@ -279,7 +285,7 @@ class BookContents } } - $pages = Page::visible()->whereIn('id', array_unique($ids['page']))->get(Page::$listAttributes); + $pages = $this->queries->pages->visibleForList()->whereIn('id', array_unique($ids['page']))->get(); /** @var Page $page */ foreach ($pages as $page) { $modelMap['page:' . $page->id] = $page; @@ -289,14 +295,14 @@ class BookContents } } - $chapters = Chapter::visible()->whereIn('id', array_unique($ids['chapter']))->get(); + $chapters = $this->queries->chapters->visibleForList()->whereIn('id', array_unique($ids['chapter']))->get(); /** @var Chapter $chapter */ foreach ($chapters as $chapter) { $modelMap['chapter:' . $chapter->id] = $chapter; $ids['book'][] = $chapter->book_id; } - $books = Book::visible()->whereIn('id', array_unique($ids['book']))->get(); + $books = $this->queries->books->visibleForList()->whereIn('id', array_unique($ids['book']))->get(); /** @var Book $book */ foreach ($books as $book) { $modelMap['book:' . $book->id] = $book; diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index f7ed4b72d..2030b050c 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -77,7 +77,7 @@ class Cloner $copyBook = $this->bookRepo->create($bookDetails); // Clone contents - $directChildren = $original->getDirectChildren(); + $directChildren = $original->getDirectVisibleChildren(); foreach ($directChildren as $child) { if ($child instanceof Chapter && userCan('chapter-create', $copyBook)) { $this->cloneChapter($child, $copyBook, $child->name); diff --git a/app/Entities/Tools/SiblingFetcher.php b/app/Entities/Tools/SiblingFetcher.php index 617ef4a62..7b8ac37ad 100644 --- a/app/Entities/Tools/SiblingFetcher.php +++ b/app/Entities/Tools/SiblingFetcher.php @@ -7,10 +7,16 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use Illuminate\Support\Collection; class SiblingFetcher { + public function __construct( + protected EntityQueries $queries, + ) { + } + /** * Search among the siblings of the entity of given type and id. */ @@ -26,7 +32,7 @@ class SiblingFetcher // Page in book or chapter if (($entity instanceof Page && !$entity->chapter) || $entity instanceof Chapter) { - $entities = $entity->book->getDirectChildren(); + $entities = $entity->book->getDirectVisibleChildren(); } // Book @@ -36,13 +42,13 @@ class SiblingFetcher if ($contextShelf) { $entities = $contextShelf->visibleBooks()->get(); } else { - $entities = Book::visible()->get(); + $entities = $this->queries->books->visibleForList()->get(); } } // Shelf if ($entity instanceof Bookshelf) { - $entities = Bookshelf::visible()->get(); + $entities = $this->queries->shelves->visibleForList()->get(); } return $entities; diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index 8e9f010df..39c982cdc 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -10,6 +10,7 @@ use BookStack\Entities\Models\Deletion; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\HasCoverImage; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; use BookStack\Uploads\AttachmentService; @@ -20,6 +21,11 @@ use Illuminate\Support\Carbon; class TrashCan { + public function __construct( + protected EntityQueries $queries, + ) { + } + /** * Send a shelf to the recycle bin. * @@ -203,11 +209,13 @@ class TrashCan } // Remove book template usages - Book::query()->where('default_template_id', '=', $page->id) + $this->queries->books->start() + ->where('default_template_id', '=', $page->id) ->update(['default_template_id' => null]); // Remove chapter template usages - Chapter::query()->where('default_template_id', '=', $page->id) + $this->queries->chapters->start() + ->where('default_template_id', '=', $page->id) ->update(['default_template_id' => null]); $page->forceDelete(); diff --git a/app/Permissions/JointPermissionBuilder.php b/app/Permissions/JointPermissionBuilder.php index 8c961fb13..49eaf0774 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -8,6 +8,7 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Permissions\Models\JointPermission; use BookStack\Users\Models\Role; use Illuminate\Database\Eloquent\Builder; @@ -20,6 +21,12 @@ use Illuminate\Support\Facades\DB; */ class JointPermissionBuilder { + public function __construct( + protected EntityQueries $queries, + ) { + } + + /** * Re-generate all entity permission from scratch. */ @@ -36,7 +43,7 @@ class JointPermissionBuilder }); // Chunk through all bookshelves - Bookshelf::query()->withTrashed()->select(['id', 'owned_by']) + $this->queries->shelves->start()->withTrashed()->select(['id', 'owned_by']) ->chunk(50, function (EloquentCollection $shelves) use ($roles) { $this->createManyJointPermissions($shelves->all(), $roles); }); @@ -88,7 +95,7 @@ class JointPermissionBuilder }); // Chunk through all bookshelves - Bookshelf::query()->select(['id', 'owned_by']) + $this->queries->shelves->start()->select(['id', 'owned_by']) ->chunk(100, function ($shelves) use ($roles) { $this->createManyJointPermissions($shelves->all(), $roles); }); @@ -99,7 +106,7 @@ class JointPermissionBuilder */ protected function bookFetchQuery(): Builder { - return Book::query()->withTrashed() + return $this->queries->books->start()->withTrashed() ->select(['id', 'owned_by'])->with([ 'chapters' => function ($query) { $query->withTrashed()->select(['id', 'owned_by', 'book_id']); diff --git a/app/Permissions/PermissionsController.php b/app/Permissions/PermissionsController.php index f2014ea73..5d2035870 100644 --- a/app/Permissions/PermissionsController.php +++ b/app/Permissions/PermissionsController.php @@ -2,10 +2,7 @@ namespace BookStack\Permissions; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Bookshelf; -use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Tools\PermissionsUpdater; use BookStack\Http\Controller; use BookStack\Permissions\Models\EntityPermission; @@ -14,19 +11,18 @@ use Illuminate\Http\Request; class PermissionsController extends Controller { - protected PermissionsUpdater $permissionsUpdater; - - public function __construct(PermissionsUpdater $permissionsUpdater) - { - $this->permissionsUpdater = $permissionsUpdater; + public function __construct( + protected PermissionsUpdater $permissionsUpdater, + protected EntityQueries $queries, + ) { } /** - * Show the Permissions view for a page. + * Show the permissions view for a page. */ public function showForPage(string $bookSlug, string $pageSlug) { - $page = Page::getBySlugs($bookSlug, $pageSlug); + $page = $this->queries->pages->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('restrictions-manage', $page); $this->setPageTitle(trans('entities.pages_permissions')); @@ -41,7 +37,7 @@ class PermissionsController extends Controller */ public function updateForPage(Request $request, string $bookSlug, string $pageSlug) { - $page = Page::getBySlugs($bookSlug, $pageSlug); + $page = $this->queries->pages->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('restrictions-manage', $page); $this->permissionsUpdater->updateFromPermissionsForm($page, $request); @@ -52,11 +48,11 @@ class PermissionsController extends Controller } /** - * Show the Restrictions view for a chapter. + * Show the permissions view for a chapter. */ public function showForChapter(string $bookSlug, string $chapterSlug) { - $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('restrictions-manage', $chapter); $this->setPageTitle(trans('entities.chapters_permissions')); @@ -67,11 +63,11 @@ class PermissionsController extends Controller } /** - * Set the restrictions for a chapter. + * Set the permissions for a chapter. */ public function updateForChapter(Request $request, string $bookSlug, string $chapterSlug) { - $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('restrictions-manage', $chapter); $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request); @@ -86,7 +82,7 @@ class PermissionsController extends Controller */ public function showForBook(string $slug) { - $book = Book::getBySlug($slug); + $book = $this->queries->books->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $book); $this->setPageTitle(trans('entities.books_permissions')); @@ -97,11 +93,11 @@ class PermissionsController extends Controller } /** - * Set the restrictions for a book. + * Set the permissions for a book. */ public function updateForBook(Request $request, string $slug) { - $book = Book::getBySlug($slug); + $book = $this->queries->books->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $book); $this->permissionsUpdater->updateFromPermissionsForm($book, $request); @@ -116,7 +112,7 @@ class PermissionsController extends Controller */ public function showForShelf(string $slug) { - $shelf = Bookshelf::getBySlug($slug); + $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); $this->setPageTitle(trans('entities.shelves_permissions')); @@ -131,7 +127,7 @@ class PermissionsController extends Controller */ public function updateForShelf(Request $request, string $slug) { - $shelf = Bookshelf::getBySlug($slug); + $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request); @@ -146,7 +142,7 @@ class PermissionsController extends Controller */ public function copyShelfPermissionsToBooks(string $slug) { - $shelf = Bookshelf::getBySlug($slug); + $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); $updateCount = $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf); diff --git a/app/References/CrossLinkParser.php b/app/References/CrossLinkParser.php index b9c3ad205..1fd4c1b3e 100644 --- a/app/References/CrossLinkParser.php +++ b/app/References/CrossLinkParser.php @@ -3,6 +3,7 @@ namespace BookStack\References; use BookStack\App\Model; +use BookStack\Entities\Queries\EntityQueries; use BookStack\References\ModelResolvers\BookLinkModelResolver; use BookStack\References\ModelResolvers\BookshelfLinkModelResolver; use BookStack\References\ModelResolvers\ChapterLinkModelResolver; @@ -85,12 +86,14 @@ class CrossLinkParser */ public static function createWithEntityResolvers(): self { + $queries = app()->make(EntityQueries::class); + return new self([ - new PagePermalinkModelResolver(), - new PageLinkModelResolver(), - new ChapterLinkModelResolver(), - new BookLinkModelResolver(), - new BookshelfLinkModelResolver(), + new PagePermalinkModelResolver($queries->pages), + new PageLinkModelResolver($queries->pages), + new ChapterLinkModelResolver($queries->chapters), + new BookLinkModelResolver($queries->books), + new BookshelfLinkModelResolver($queries->shelves), ]); } } diff --git a/app/References/ModelResolvers/BookLinkModelResolver.php b/app/References/ModelResolvers/BookLinkModelResolver.php index d404fe2fd..a671fbf57 100644 --- a/app/References/ModelResolvers/BookLinkModelResolver.php +++ b/app/References/ModelResolvers/BookLinkModelResolver.php @@ -4,9 +4,15 @@ namespace BookStack\References\ModelResolvers; use BookStack\App\Model; use BookStack\Entities\Models\Book; +use BookStack\Entities\Queries\BookQueries; class BookLinkModelResolver implements CrossLinkModelResolver { + public function __construct( + protected BookQueries $queries + ) { + } + public function resolve(string $link): ?Model { $pattern = '/^' . preg_quote(url('/books'), '/') . '\/([\w-]+)' . '([#?\/]|$)/'; @@ -19,7 +25,7 @@ class BookLinkModelResolver implements CrossLinkModelResolver $bookSlug = $matches[1]; /** @var ?Book $model */ - $model = Book::query()->where('slug', '=', $bookSlug)->first(['id']); + $model = $this->queries->start()->where('slug', '=', $bookSlug)->first(['id']); return $model; } diff --git a/app/References/ModelResolvers/BookshelfLinkModelResolver.php b/app/References/ModelResolvers/BookshelfLinkModelResolver.php index cc9df034e..d79c760e0 100644 --- a/app/References/ModelResolvers/BookshelfLinkModelResolver.php +++ b/app/References/ModelResolvers/BookshelfLinkModelResolver.php @@ -4,9 +4,14 @@ namespace BookStack\References\ModelResolvers; use BookStack\App\Model; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookshelfQueries; class BookshelfLinkModelResolver implements CrossLinkModelResolver { + public function __construct( + protected BookshelfQueries $queries + ) { + } public function resolve(string $link): ?Model { $pattern = '/^' . preg_quote(url('/shelves'), '/') . '\/([\w-]+)' . '([#?\/]|$)/'; @@ -19,7 +24,7 @@ class BookshelfLinkModelResolver implements CrossLinkModelResolver $shelfSlug = $matches[1]; /** @var ?Bookshelf $model */ - $model = Bookshelf::query()->where('slug', '=', $shelfSlug)->first(['id']); + $model = $this->queries->start()->where('slug', '=', $shelfSlug)->first(['id']); return $model; } diff --git a/app/References/ModelResolvers/ChapterLinkModelResolver.php b/app/References/ModelResolvers/ChapterLinkModelResolver.php index 4733c68ee..318b5ed35 100644 --- a/app/References/ModelResolvers/ChapterLinkModelResolver.php +++ b/app/References/ModelResolvers/ChapterLinkModelResolver.php @@ -4,9 +4,15 @@ namespace BookStack\References\ModelResolvers; use BookStack\App\Model; use BookStack\Entities\Models\Chapter; +use BookStack\Entities\Queries\ChapterQueries; class ChapterLinkModelResolver implements CrossLinkModelResolver { + public function __construct( + protected ChapterQueries $queries + ) { + } + public function resolve(string $link): ?Model { $pattern = '/^' . preg_quote(url('/books'), '/') . '\/([\w-]+)' . '\/chapter\/' . '([\w-]+)' . '([#?\/]|$)/'; @@ -20,7 +26,7 @@ class ChapterLinkModelResolver implements CrossLinkModelResolver $chapterSlug = $matches[2]; /** @var ?Chapter $model */ - $model = Chapter::query()->whereSlugs($bookSlug, $chapterSlug)->first(['id']); + $model = $this->queries->usingSlugs($bookSlug, $chapterSlug)->first(['id']); return $model; } diff --git a/app/References/ModelResolvers/PageLinkModelResolver.php b/app/References/ModelResolvers/PageLinkModelResolver.php index 736382bec..9a01416a4 100644 --- a/app/References/ModelResolvers/PageLinkModelResolver.php +++ b/app/References/ModelResolvers/PageLinkModelResolver.php @@ -4,9 +4,15 @@ namespace BookStack\References\ModelResolvers; use BookStack\App\Model; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; class PageLinkModelResolver implements CrossLinkModelResolver { + public function __construct( + protected PageQueries $queries + ) { + } + public function resolve(string $link): ?Model { $pattern = '/^' . preg_quote(url('/books'), '/') . '\/([\w-]+)' . '\/page\/' . '([\w-]+)' . '([#?\/]|$)/'; @@ -20,7 +26,7 @@ class PageLinkModelResolver implements CrossLinkModelResolver $pageSlug = $matches[2]; /** @var ?Page $model */ - $model = Page::query()->whereSlugs($bookSlug, $pageSlug)->first(['id']); + $model = $this->queries->usingSlugs($bookSlug, $pageSlug)->first(['id']); return $model; } diff --git a/app/References/ModelResolvers/PagePermalinkModelResolver.php b/app/References/ModelResolvers/PagePermalinkModelResolver.php index 9ed00b36d..59d509f33 100644 --- a/app/References/ModelResolvers/PagePermalinkModelResolver.php +++ b/app/References/ModelResolvers/PagePermalinkModelResolver.php @@ -4,9 +4,15 @@ namespace BookStack\References\ModelResolvers; use BookStack\App\Model; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; class PagePermalinkModelResolver implements CrossLinkModelResolver { + public function __construct( + protected PageQueries $queries + ) { + } + public function resolve(string $link): ?Model { $pattern = '/^' . preg_quote(url('/link'), '/') . '\/(\d+)/'; @@ -18,7 +24,7 @@ class PagePermalinkModelResolver implements CrossLinkModelResolver $id = intval($matches[1]); /** @var ?Page $model */ - $model = Page::query()->find($id, ['id']); + $model = $this->queries->start()->find($id, ['id']); return $model; } diff --git a/app/References/ReferenceController.php b/app/References/ReferenceController.php index 991f47225..57dbcce3c 100644 --- a/app/References/ReferenceController.php +++ b/app/References/ReferenceController.php @@ -6,12 +6,14 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Http\Controller; class ReferenceController extends Controller { public function __construct( - protected ReferenceFetcher $referenceFetcher + protected ReferenceFetcher $referenceFetcher, + protected EntityQueries $queries, ) { } @@ -20,7 +22,7 @@ class ReferenceController extends Controller */ public function page(string $bookSlug, string $pageSlug) { - $page = Page::getBySlugs($bookSlug, $pageSlug); + $page = $this->queries->pages->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $references = $this->referenceFetcher->getReferencesToEntity($page); return view('pages.references', [ @@ -34,7 +36,7 @@ class ReferenceController extends Controller */ public function chapter(string $bookSlug, string $chapterSlug) { - $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug); + $chapter = $this->queries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $references = $this->referenceFetcher->getReferencesToEntity($chapter); return view('chapters.references', [ @@ -48,7 +50,7 @@ class ReferenceController extends Controller */ public function book(string $slug) { - $book = Book::getBySlug($slug); + $book = $this->queries->books->findVisibleBySlugOrFail($slug); $references = $this->referenceFetcher->getReferencesToEntity($book); return view('books.references', [ @@ -62,7 +64,7 @@ class ReferenceController extends Controller */ public function shelf(string $slug) { - $shelf = Bookshelf::getBySlug($slug); + $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $references = $this->referenceFetcher->getReferencesToEntity($shelf); return view('shelves.references', [ diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php index 6cf12a579..bc3f2ddb4 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -130,12 +130,12 @@ class SearchController extends Controller /** * Search siblings items in the system. */ - public function searchSiblings(Request $request) + public function searchSiblings(Request $request, SiblingFetcher $siblingFetcher) { $type = $request->get('entity_type', null); $id = $request->get('entity_id', null); - $entities = (new SiblingFetcher())->fetch($type, $id); + $entities = $siblingFetcher->fetch($type, $id); return view('entities.list-basic', ['entities' => $entities, 'style' => 'compact']); } diff --git a/app/Settings/MaintenanceController.php b/app/Settings/MaintenanceController.php index 62eeecf39..0382ae08a 100644 --- a/app/Settings/MaintenanceController.php +++ b/app/Settings/MaintenanceController.php @@ -14,7 +14,7 @@ class MaintenanceController extends Controller /** * Show the page for application maintenance. */ - public function index() + public function index(TrashCan $trashCan) { $this->checkPermission('settings-manage'); $this->setPageTitle(trans('settings.maint')); @@ -23,7 +23,7 @@ class MaintenanceController extends Controller $version = trim(file_get_contents(base_path('version'))); // Recycle bin details - $recycleStats = (new TrashCan())->getTrashedCounts(); + $recycleStats = $trashCan->getTrashedCounts(); return view('settings.maintenance', [ 'version' => $version, diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 1655a4cc3..ee2cb8a35 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -5,6 +5,7 @@ namespace BookStack\Uploads; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Exceptions\ImageUploadException; use Exception; use Illuminate\Support\Facades\DB; @@ -20,6 +21,7 @@ class ImageService public function __construct( protected ImageStorage $storage, protected ImageResizer $resizer, + protected EntityQueries $queries, ) { } @@ -278,15 +280,15 @@ class ImageService } if ($imageType === 'gallery' || $imageType === 'drawio') { - return Page::visible()->where('id', '=', $image->uploaded_to)->exists(); + return $this->queries->pages->visibleForList()->where('id', '=', $image->uploaded_to)->exists(); } if ($imageType === 'cover_book') { - return Book::visible()->where('id', '=', $image->uploaded_to)->exists(); + return $this->queries->books->visibleForList()->where('id', '=', $image->uploaded_to)->exists(); } if ($imageType === 'cover_bookshelf') { - return Bookshelf::visible()->where('id', '=', $image->uploaded_to)->exists(); + return $this->queries->shelves->visibleForList()->where('id', '=', $image->uploaded_to)->exists(); } return false; diff --git a/app/Users/Controllers/UserProfileController.php b/app/Users/Controllers/UserProfileController.php index bdf268260..963d69a62 100644 --- a/app/Users/Controllers/UserProfileController.php +++ b/app/Users/Controllers/UserProfileController.php @@ -10,16 +10,25 @@ use BookStack\Users\UserRepo; class UserProfileController extends Controller { + public function __construct( + protected UserRepo $userRepo, + protected ActivityQueries $activityQueries, + protected UserContentCounts $contentCounts, + protected UserRecentlyCreatedContent $recentlyCreatedContent + ) { + } + + /** * Show the user profile page. */ - public function show(UserRepo $repo, ActivityQueries $activities, string $slug) + public function show(string $slug) { - $user = $repo->getBySlug($slug); + $user = $this->userRepo->getBySlug($slug); - $userActivity = $activities->userActivity($user); - $recentlyCreated = (new UserRecentlyCreatedContent())->run($user, 5); - $assetCounts = (new UserContentCounts())->run($user); + $userActivity = $this->activityQueries->userActivity($user); + $recentlyCreated = $this->recentlyCreatedContent->run($user, 5); + $assetCounts = $this->contentCounts->run($user); $this->setPageTitle($user->name); diff --git a/app/Users/Queries/UserContentCounts.php b/app/Users/Queries/UserContentCounts.php index 178d8536b..af38bfa7e 100644 --- a/app/Users/Queries/UserContentCounts.php +++ b/app/Users/Queries/UserContentCounts.php @@ -2,10 +2,7 @@ namespace BookStack\Users\Queries; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Bookshelf; -use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Users\Models\User; /** @@ -13,6 +10,12 @@ use BookStack\Users\Models\User; */ class UserContentCounts { + public function __construct( + protected EntityQueries $queries, + ) { + } + + /** * @return array{pages: int, chapters: int, books: int, shelves: int} */ @@ -21,10 +24,10 @@ class UserContentCounts $createdBy = ['created_by' => $user->id]; return [ - 'pages' => Page::visible()->where($createdBy)->count(), - 'chapters' => Chapter::visible()->where($createdBy)->count(), - 'books' => Book::visible()->where($createdBy)->count(), - 'shelves' => Bookshelf::visible()->where($createdBy)->count(), + 'pages' => $this->queries->pages->visibleForList()->where($createdBy)->count(), + 'chapters' => $this->queries->chapters->visibleForList()->where($createdBy)->count(), + 'books' => $this->queries->books->visibleForList()->where($createdBy)->count(), + 'shelves' => $this->queries->shelves->visibleForList()->where($createdBy)->count(), ]; } } diff --git a/app/Users/Queries/UserRecentlyCreatedContent.php b/app/Users/Queries/UserRecentlyCreatedContent.php index 23db2c1f1..23850e072 100644 --- a/app/Users/Queries/UserRecentlyCreatedContent.php +++ b/app/Users/Queries/UserRecentlyCreatedContent.php @@ -2,10 +2,7 @@ namespace BookStack\Users\Queries; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Bookshelf; -use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Collection; @@ -15,6 +12,11 @@ use Illuminate\Database\Eloquent\Collection; */ class UserRecentlyCreatedContent { + public function __construct( + protected EntityQueries $queries, + ) { + } + /** * @return array{pages: Collection, chapters: Collection, books: Collection, shelves: Collection} */ @@ -28,10 +30,10 @@ class UserRecentlyCreatedContent }; return [ - 'pages' => $query(Page::visible()->where('draft', '=', false)), - 'chapters' => $query(Chapter::visible()), - 'books' => $query(Book::visible()), - 'shelves' => $query(Bookshelf::visible()), + 'pages' => $query($this->queries->pages->visibleForList()->where('draft', '=', false)), + 'chapters' => $query($this->queries->chapters->visibleForList()), + 'books' => $query($this->queries->books->visibleForList()), + 'shelves' => $query($this->queries->shelves->visibleForList()), ]; } } diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php index 374089246..04dff293f 100644 --- a/tests/Entity/BookTest.php +++ b/tests/Entity/BookTest.php @@ -317,7 +317,7 @@ class BookTest extends TestCase $copy = Book::query()->where('name', '=', 'My copy book')->first(); $resp->assertRedirect($copy->getUrl()); - $this->assertEquals($book->getDirectChildren()->count(), $copy->getDirectChildren()->count()); + $this->assertEquals($book->getDirectVisibleChildren()->count(), $copy->getDirectVisibleChildren()->count()); $this->get($copy->getUrl())->assertSee($book->description_html, false); } @@ -329,7 +329,7 @@ class BookTest extends TestCase // Hide child content /** @var BookChild $page */ - foreach ($book->getDirectChildren() as $child) { + foreach ($book->getDirectVisibleChildren() as $child) { $this->permissions->setEntityPermissions($child, [], []); } @@ -337,7 +337,7 @@ class BookTest extends TestCase /** @var Book $copy */ $copy = Book::query()->where('name', '=', 'My copy book')->first(); - $this->assertEquals(0, $copy->getDirectChildren()->count()); + $this->assertEquals(0, $copy->getDirectVisibleChildren()->count()); } public function test_copy_does_not_copy_pages_or_chapters_if_user_cant_create() diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php index 9b77a32ab..dcc062044 100644 --- a/tests/Entity/EntitySearchTest.php +++ b/tests/Entity/EntitySearchTest.php @@ -303,7 +303,7 @@ class EntitySearchTest extends TestCase public function test_sibling_search_for_pages_without_chapter() { $page = $this->entities->pageNotWithinChapter(); - $bookChildren = $page->book->getDirectChildren(); + $bookChildren = $page->book->getDirectVisibleChildren(); $this->assertGreaterThan(2, count($bookChildren), 'Ensure we\'re testing with at least 1 sibling'); $search = $this->actingAs($this->users->viewer())->get("/search/entity/siblings?entity_id={$page->id}&entity_type=page"); @@ -318,7 +318,7 @@ class EntitySearchTest extends TestCase public function test_sibling_search_for_chapters() { $chapter = $this->entities->chapter(); - $bookChildren = $chapter->book->getDirectChildren(); + $bookChildren = $chapter->book->getDirectVisibleChildren(); $this->assertGreaterThan(2, count($bookChildren), 'Ensure we\'re testing with at least 1 sibling'); $search = $this->actingAs($this->users->viewer())->get("/search/entity/siblings?entity_id={$chapter->id}&entity_type=chapter"); From 546cfb0dcc801c4fa6560ca0e6d18b4f1edd830f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 7 Feb 2024 21:58:27 +0000 Subject: [PATCH 08/12] Queries: Extracted static page,chapter,shelf queries to classes --- .../Controllers/CommentController.php | 6 ++++-- .../Commands/CopyShelfPermissionsCommand.php | 7 ++++--- app/Entities/Controllers/BookController.php | 6 ++++-- .../Controllers/BookshelfApiController.php | 12 ++++++----- .../ChapterExportApiController.php | 21 ++++++++----------- app/Entities/Controllers/PageController.php | 4 +++- .../Controllers/PageExportApiController.php | 18 ++++++++-------- app/Entities/Queries/BookshelfQueries.php | 11 ++++++++++ app/Entities/Repos/BaseRepo.php | 5 +++-- app/Entities/Tools/PageContent.php | 6 +++++- app/Entities/Tools/ShelfContext.php | 15 ++++++++----- app/Search/SearchController.php | 9 ++++---- .../Controllers/AttachmentApiController.php | 8 ++++--- .../Controllers/ImageGalleryApiController.php | 6 ++++-- app/Uploads/ImageRepo.php | 19 +++++++++-------- 15 files changed, 93 insertions(+), 60 deletions(-) diff --git a/app/Activity/Controllers/CommentController.php b/app/Activity/Controllers/CommentController.php index 340524cd0..0e8db9eaf 100644 --- a/app/Activity/Controllers/CommentController.php +++ b/app/Activity/Controllers/CommentController.php @@ -4,6 +4,7 @@ namespace BookStack\Activity\Controllers; use BookStack\Activity\CommentRepo; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Http\Controller; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -11,7 +12,8 @@ use Illuminate\Validation\ValidationException; class CommentController extends Controller { public function __construct( - protected CommentRepo $commentRepo + protected CommentRepo $commentRepo, + protected PageQueries $pageQueries, ) { } @@ -27,7 +29,7 @@ class CommentController extends Controller 'parent_id' => ['nullable', 'integer'], ]); - $page = Page::visible()->find($pageId); + $page = $this->pageQueries->findVisibleById($pageId); if ($page === null) { return response('Not found', 404); } diff --git a/app/Console/Commands/CopyShelfPermissionsCommand.php b/app/Console/Commands/CopyShelfPermissionsCommand.php index fc11484bd..8463ccd03 100644 --- a/app/Console/Commands/CopyShelfPermissionsCommand.php +++ b/app/Console/Commands/CopyShelfPermissionsCommand.php @@ -3,6 +3,7 @@ namespace BookStack\Console\Commands; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Tools\PermissionsUpdater; use Illuminate\Console\Command; @@ -28,7 +29,7 @@ class CopyShelfPermissionsCommand extends Command /** * Execute the console command. */ - public function handle(PermissionsUpdater $permissionsUpdater): int + public function handle(PermissionsUpdater $permissionsUpdater, BookshelfQueries $queries): int { $shelfSlug = $this->option('slug'); $cascadeAll = $this->option('all'); @@ -51,11 +52,11 @@ class CopyShelfPermissionsCommand extends Command return 0; } - $shelves = Bookshelf::query()->get(['id']); + $shelves = $queries->start()->get(['id']); } if ($shelfSlug) { - $shelves = Bookshelf::query()->where('slug', '=', $shelfSlug)->get(['id']); + $shelves = $queries->start()->where('slug', '=', $shelfSlug)->get(['id']); if ($shelves->count() === 0) { $this->info('No shelves found with the given slug.'); } diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index 0e9346dbd..a0b98636f 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -8,6 +8,7 @@ use BookStack\Activity\Models\View; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Queries\BookQueries; +use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Repos\BookRepo; use BookStack\Entities\Tools\BookContents; use BookStack\Entities\Tools\Cloner; @@ -29,6 +30,7 @@ class BookController extends Controller protected ShelfContext $shelfContext, protected BookRepo $bookRepo, protected BookQueries $queries, + protected BookshelfQueries $shelfQueries, protected ReferenceFetcher $referenceFetcher, ) { } @@ -75,7 +77,7 @@ class BookController extends Controller $bookshelf = null; if ($shelfSlug !== null) { - $bookshelf = Bookshelf::visible()->where('slug', '=', $shelfSlug)->firstOrFail(); + $bookshelf = $this->shelfQueries->findVisibleBySlugOrFail($shelfSlug); $this->checkOwnablePermission('bookshelf-update', $bookshelf); } @@ -105,7 +107,7 @@ class BookController extends Controller $bookshelf = null; if ($shelfSlug !== null) { - $bookshelf = Bookshelf::visible()->where('slug', '=', $shelfSlug)->firstOrFail(); + $bookshelf = $this->shelfQueries->findVisibleBySlugOrFail($shelfSlug); $this->checkOwnablePermission('bookshelf-update', $bookshelf); } diff --git a/app/Entities/Controllers/BookshelfApiController.php b/app/Entities/Controllers/BookshelfApiController.php index a12dc90ac..9170226a5 100644 --- a/app/Entities/Controllers/BookshelfApiController.php +++ b/app/Entities/Controllers/BookshelfApiController.php @@ -3,6 +3,7 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Repos\BookshelfRepo; use BookStack\Http\ApiController; use Exception; @@ -13,7 +14,8 @@ use Illuminate\Validation\ValidationException; class BookshelfApiController extends ApiController { public function __construct( - protected BookshelfRepo $bookshelfRepo + protected BookshelfRepo $bookshelfRepo, + protected BookshelfQueries $queries, ) { } @@ -22,7 +24,7 @@ class BookshelfApiController extends ApiController */ public function list() { - $shelves = Bookshelf::visible(); + $shelves = $this->queries->visibleForList(); return $this->apiListingResponse($shelves, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', @@ -54,7 +56,7 @@ class BookshelfApiController extends ApiController */ public function read(string $id) { - $shelf = Bookshelf::visible()->findOrFail($id); + $shelf = $this->queries->findVisibleByIdOrFail(intval($id)); $shelf = $this->forJsonDisplay($shelf); $shelf->load([ 'createdBy', 'updatedBy', 'ownedBy', @@ -78,7 +80,7 @@ class BookshelfApiController extends ApiController */ public function update(Request $request, string $id) { - $shelf = Bookshelf::visible()->findOrFail($id); + $shelf = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('bookshelf-update', $shelf); $requestData = $this->validate($request, $this->rules()['update']); @@ -97,7 +99,7 @@ class BookshelfApiController extends ApiController */ public function delete(string $id) { - $shelf = Bookshelf::visible()->findOrFail($id); + $shelf = $this->queries->findVisibleByIdOrFail(intval($id)); $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->bookshelfRepo->destroy($shelf); diff --git a/app/Entities/Controllers/ChapterExportApiController.php b/app/Entities/Controllers/ChapterExportApiController.php index d1523e665..08aa959b3 100644 --- a/app/Entities/Controllers/ChapterExportApiController.php +++ b/app/Entities/Controllers/ChapterExportApiController.php @@ -3,20 +3,17 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Chapter; +use BookStack\Entities\Queries\ChapterQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\ApiController; use Throwable; class ChapterExportApiController extends ApiController { - protected $exportFormatter; - - /** - * ChapterExportController constructor. - */ - public function __construct(ExportFormatter $exportFormatter) - { - $this->exportFormatter = $exportFormatter; + public function __construct( + protected ExportFormatter $exportFormatter, + protected ChapterQueries $queries, + ) { $this->middleware('can:content-export'); } @@ -27,7 +24,7 @@ class ChapterExportApiController extends ApiController */ public function exportPdf(int $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail($id); $pdfContent = $this->exportFormatter->chapterToPdf($chapter); return $this->download()->directly($pdfContent, $chapter->slug . '.pdf'); @@ -40,7 +37,7 @@ class ChapterExportApiController extends ApiController */ public function exportHtml(int $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail($id); $htmlContent = $this->exportFormatter->chapterToContainedHtml($chapter); return $this->download()->directly($htmlContent, $chapter->slug . '.html'); @@ -51,7 +48,7 @@ class ChapterExportApiController extends ApiController */ public function exportPlainText(int $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail($id); $textContent = $this->exportFormatter->chapterToPlainText($chapter); return $this->download()->directly($textContent, $chapter->slug . '.txt'); @@ -62,7 +59,7 @@ class ChapterExportApiController extends ApiController */ public function exportMarkdown(int $id) { - $chapter = Chapter::visible()->findOrFail($id); + $chapter = $this->queries->findVisibleByIdOrFail($id); $markdown = $this->exportFormatter->chapterToMarkdown($chapter); return $this->download()->directly($markdown, $chapter->slug . '.md'); diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index 471df8184..44ba999e8 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -359,7 +359,9 @@ class PageController extends Controller $query->scopes('visible'); }; - $pages = Page::visible()->with(['updatedBy', 'book' => $visibleBelongsScope, 'chapter' => $visibleBelongsScope]) + $pages = $this->queries->visibleForList() + ->addSelect('updated_by') + ->with(['updatedBy', 'book' => $visibleBelongsScope, 'chapter' => $visibleBelongsScope]) ->orderBy('updated_at', 'desc') ->paginate(20) ->setPath(url('/pages/recently-updated')); diff --git a/app/Entities/Controllers/PageExportApiController.php b/app/Entities/Controllers/PageExportApiController.php index d936a0de2..8737f2f3e 100644 --- a/app/Entities/Controllers/PageExportApiController.php +++ b/app/Entities/Controllers/PageExportApiController.php @@ -3,17 +3,17 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\ApiController; use Throwable; class PageExportApiController extends ApiController { - protected $exportFormatter; - - public function __construct(ExportFormatter $exportFormatter) - { - $this->exportFormatter = $exportFormatter; + public function __construct( + protected ExportFormatter $exportFormatter, + protected PageQueries $queries, + ) { $this->middleware('can:content-export'); } @@ -24,7 +24,7 @@ class PageExportApiController extends ApiController */ public function exportPdf(int $id) { - $page = Page::visible()->findOrFail($id); + $page = $this->queries->findVisibleByIdOrFail($id); $pdfContent = $this->exportFormatter->pageToPdf($page); return $this->download()->directly($pdfContent, $page->slug . '.pdf'); @@ -37,7 +37,7 @@ class PageExportApiController extends ApiController */ public function exportHtml(int $id) { - $page = Page::visible()->findOrFail($id); + $page = $this->queries->findVisibleByIdOrFail($id); $htmlContent = $this->exportFormatter->pageToContainedHtml($page); return $this->download()->directly($htmlContent, $page->slug . '.html'); @@ -48,7 +48,7 @@ class PageExportApiController extends ApiController */ public function exportPlainText(int $id) { - $page = Page::visible()->findOrFail($id); + $page = $this->queries->findVisibleByIdOrFail($id); $textContent = $this->exportFormatter->pageToPlainText($page); return $this->download()->directly($textContent, $page->slug . '.txt'); @@ -59,7 +59,7 @@ class PageExportApiController extends ApiController */ public function exportMarkdown(int $id) { - $page = Page::visible()->findOrFail($id); + $page = $this->queries->findVisibleByIdOrFail($id); $markdown = $this->exportFormatter->pageToMarkdown($page); return $this->download()->directly($markdown, $page->slug . '.md'); diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index d61607e7a..f2fe50531 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -18,6 +18,17 @@ class BookshelfQueries implements ProvidesEntityQueries return $this->start()->scopes('visible')->find($id); } + public function findVisibleByIdOrFail(int $id): Bookshelf + { + $shelf = $this->findVisibleById($id); + + if (is_null($shelf)) { + throw new NotFoundException(trans('errors.bookshelf_not_found')); + } + + return $shelf; + } + public function findVisibleBySlugOrFail(string $slug): Bookshelf { /** @var ?Bookshelf $shelf */ diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 17208ae03..6674f559a 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -9,6 +9,7 @@ use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\HasCoverImage; use BookStack\Entities\Models\HasHtmlDescription; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\ImageUploadException; use BookStack\References\ReferenceStore; use BookStack\References\ReferenceUpdater; @@ -23,6 +24,7 @@ class BaseRepo protected ImageRepo $imageRepo, protected ReferenceUpdater $referenceUpdater, protected ReferenceStore $referenceStore, + protected PageQueries $pageQueries, ) { } @@ -125,8 +127,7 @@ class BaseRepo return; } - $templateExists = Page::query()->visible() - ->where('template', '=', true) + $templateExists = $this->pageQueries->visibleTemplates() ->where('id', '=', $templateId) ->exists(); diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 6a89ff626..88987f054 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -3,6 +3,7 @@ namespace BookStack\Entities\Tools; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Tools\Markdown\MarkdownToHtml; use BookStack\Exceptions\ImageUploadException; use BookStack\Facades\Theme; @@ -21,9 +22,12 @@ use Illuminate\Support\Str; class PageContent { + protected PageQueries $pageQueries; + public function __construct( protected Page $page ) { + $this->pageQueries = app()->make(PageQueries::class); } /** @@ -331,7 +335,7 @@ class PageContent return PageIncludeContent::fromHtmlAndTag('', $tag); } - $matchedPage = Page::visible()->find($tag->getPageId()); + $matchedPage = $this->pageQueries->findVisibleById($tag->getPageId()); $content = PageIncludeContent::fromHtmlAndTag($matchedPage->html ?? '', $tag); if (Theme::hasListeners(ThemeEvents::PAGE_INCLUDE_PARSE)) { diff --git a/app/Entities/Tools/ShelfContext.php b/app/Entities/Tools/ShelfContext.php index 50c7047d9..5ed334878 100644 --- a/app/Entities/Tools/ShelfContext.php +++ b/app/Entities/Tools/ShelfContext.php @@ -4,10 +4,16 @@ namespace BookStack\Entities\Tools; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Queries\BookshelfQueries; class ShelfContext { - protected $KEY_SHELF_CONTEXT_ID = 'context_bookshelf_id'; + protected string $KEY_SHELF_CONTEXT_ID = 'context_bookshelf_id'; + + public function __construct( + protected BookshelfQueries $shelfQueries, + ) { + } /** * Get the current bookshelf context for the given book. @@ -20,8 +26,7 @@ class ShelfContext return null; } - /** @var Bookshelf $shelf */ - $shelf = Bookshelf::visible()->find($contextBookshelfId); + $shelf = $this->shelfQueries->findVisibleById($contextBookshelfId); $shelfContainsBook = $shelf && $shelf->contains($book); return $shelfContainsBook ? $shelf : null; @@ -30,7 +35,7 @@ class ShelfContext /** * Store the current contextual shelf ID. */ - public function setShelfContext(int $shelfId) + public function setShelfContext(int $shelfId): void { session()->put($this->KEY_SHELF_CONTEXT_ID, $shelfId); } @@ -38,7 +43,7 @@ class ShelfContext /** * Clear the session stored shelf context id. */ - public function clearShelfContext() + public function clearShelfContext(): void { session()->forget($this->KEY_SHELF_CONTEXT_ID); } diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php index bc3f2ddb4..08f0826dd 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -3,6 +3,7 @@ namespace BookStack\Search; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Queries\Popular; use BookStack\Entities\Tools\SiblingFetcher; use BookStack\Http\Controller; @@ -11,7 +12,8 @@ use Illuminate\Http\Request; class SearchController extends Controller { public function __construct( - protected SearchRunner $searchRunner + protected SearchRunner $searchRunner, + protected PageQueries $pageQueries, ) { } @@ -95,12 +97,11 @@ class SearchController extends Controller $searchOptions->setFilter('is_template'); $entities = $this->searchRunner->searchEntities($searchOptions, 'page', 1, 20)['results']; } else { - $entities = Page::visible() - ->where('template', '=', true) + $entities = $this->pageQueries->visibleTemplates() ->where('draft', '=', false) ->orderBy('updated_at', 'desc') ->take(20) - ->get(Page::$listAttributes); + ->get(); } return view('search.parts.entity-selector-list', [ diff --git a/app/Uploads/Controllers/AttachmentApiController.php b/app/Uploads/Controllers/AttachmentApiController.php index 2e6d16205..2832fa8e1 100644 --- a/app/Uploads/Controllers/AttachmentApiController.php +++ b/app/Uploads/Controllers/AttachmentApiController.php @@ -3,6 +3,7 @@ namespace BookStack\Uploads\Controllers; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\FileUploadException; use BookStack\Http\ApiController; use BookStack\Uploads\Attachment; @@ -15,7 +16,8 @@ use Illuminate\Validation\ValidationException; class AttachmentApiController extends ApiController { public function __construct( - protected AttachmentService $attachmentService + protected AttachmentService $attachmentService, + protected PageQueries $pageQueries, ) { } @@ -48,7 +50,7 @@ class AttachmentApiController extends ApiController $requestData = $this->validate($request, $this->rules()['create']); $pageId = $request->get('uploaded_to'); - $page = Page::visible()->findOrFail($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $this->checkOwnablePermission('page-update', $page); if ($request->hasFile('file')) { @@ -132,7 +134,7 @@ class AttachmentApiController extends ApiController $page = $attachment->page; if ($requestData['uploaded_to'] ?? false) { $pageId = $request->get('uploaded_to'); - $page = Page::visible()->findOrFail($pageId); + $page = $this->pageQueries->findVisibleByIdOrFail($pageId); $attachment->uploaded_to = $requestData['uploaded_to']; } diff --git a/app/Uploads/Controllers/ImageGalleryApiController.php b/app/Uploads/Controllers/ImageGalleryApiController.php index ec96e4593..cec47feb0 100644 --- a/app/Uploads/Controllers/ImageGalleryApiController.php +++ b/app/Uploads/Controllers/ImageGalleryApiController.php @@ -3,6 +3,7 @@ namespace BookStack\Uploads\Controllers; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Http\ApiController; use BookStack\Uploads\Image; use BookStack\Uploads\ImageRepo; @@ -18,6 +19,7 @@ class ImageGalleryApiController extends ApiController public function __construct( protected ImageRepo $imageRepo, protected ImageResizer $imageResizer, + protected PageQueries $pageQueries, ) { } @@ -66,9 +68,9 @@ class ImageGalleryApiController extends ApiController { $this->checkPermission('image-create-all'); $data = $this->validate($request, $this->rules()['create']); - Page::visible()->findOrFail($data['uploaded_to']); + $page = $this->pageQueries->findVisibleByIdOrFail($data['uploaded_to']); - $image = $this->imageRepo->saveNew($data['image'], $data['type'], $data['uploaded_to']); + $image = $this->imageRepo->saveNew($data['image'], $data['type'], $page->id); if (isset($data['name'])) { $image->refresh(); diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 0e312d883..160a02fa1 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -3,6 +3,7 @@ namespace BookStack\Uploads; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\ImageUploadException; use BookStack\Permissions\PermissionApplicator; use Exception; @@ -15,6 +16,7 @@ class ImageRepo protected ImageService $imageService, protected PermissionApplicator $permissions, protected ImageResizer $imageResizer, + protected PageQueries $pageQueries, ) { } @@ -77,14 +79,13 @@ class ImageRepo */ public function getEntityFiltered( string $type, - string $filterType = null, - int $page = 0, - int $pageSize = 24, - int $uploadedTo = null, - string $search = null + ?string $filterType, + int $page, + int $pageSize, + int $uploadedTo, + ?string $search ): array { - /** @var Page $contextPage */ - $contextPage = Page::visible()->findOrFail($uploadedTo); + $contextPage = $this->pageQueries->findVisibleByIdOrFail($uploadedTo); $parentFilter = null; if ($filterType === 'book' || $filterType === 'page') { @@ -225,9 +226,9 @@ class ImageRepo */ public function getPagesUsingImage(Image $image): array { - $pages = Page::visible() + $pages = $this->pageQueries->visibleForList() ->where('html', 'like', '%' . $image->url . '%') - ->get(['id', 'name', 'slug', 'book_id']); + ->get(); foreach ($pages as $page) { $page->setAttribute('url', $page->getUrl()); From b77ab6f3af996f924bd2ddbac6e88f61a8bf6cf9 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 7 Feb 2024 22:41:45 +0000 Subject: [PATCH 09/12] Queries: Moved out or removed some class-level items Also ran auto-removal of unused imports across app folder. --- .../Controllers/CommentController.php | 1 - app/Api/ApiDocsGenerator.php | 1 - app/App/HomeController.php | 1 - app/App/Providers/ThemeServiceProvider.php | 1 - .../Commands/CopyShelfPermissionsCommand.php | 1 - app/Entities/Controllers/BookController.php | 1 - .../ChapterExportApiController.php | 1 - app/Entities/Controllers/PageController.php | 1 - .../Controllers/PageExportApiController.php | 1 - app/Entities/Models/BookChild.php | 15 ------ app/Entities/Models/Chapter.php | 9 ---- app/Entities/Models/Page.php | 12 ----- app/Entities/Queries/BookQueries.php | 7 ++- app/Entities/Queries/BookshelfQueries.php | 6 ++- app/Entities/Queries/ChapterQueries.php | 3 +- app/Entities/Queries/EntityQueries.php | 24 ++++++++-- app/Entities/Queries/PageQueries.php | 30 ++++++++---- .../Queries/ProvidesEntityQueries.php | 13 +++++ app/Entities/Repos/BaseRepo.php | 1 - app/Permissions/JointPermissionBuilder.php | 1 - app/References/ReferenceController.php | 4 -- app/Search/SearchController.php | 1 - app/Search/SearchRunner.php | 48 ++++++++----------- app/Uploads/AttachmentService.php | 1 - .../Controllers/AttachmentApiController.php | 1 - .../Controllers/GalleryImageController.php | 2 - .../Controllers/ImageGalleryApiController.php | 1 - app/Uploads/ImageRepo.php | 1 - app/Uploads/ImageService.php | 3 -- 29 files changed, 85 insertions(+), 107 deletions(-) diff --git a/app/Activity/Controllers/CommentController.php b/app/Activity/Controllers/CommentController.php index 0e8db9eaf..52ccc8238 100644 --- a/app/Activity/Controllers/CommentController.php +++ b/app/Activity/Controllers/CommentController.php @@ -3,7 +3,6 @@ namespace BookStack\Activity\Controllers; use BookStack\Activity\CommentRepo; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Http\Controller; use Illuminate\Http\Request; diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php index bffc38623..9cae80617 100644 --- a/app/Api/ApiDocsGenerator.php +++ b/app/Api/ApiDocsGenerator.php @@ -7,7 +7,6 @@ use Exception; use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Route; use Illuminate\Support\Str; use Illuminate\Validation\Rules\Password; diff --git a/app/App/HomeController.php b/app/App/HomeController.php index 6a4cb0176..56f3d81a8 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -3,7 +3,6 @@ namespace BookStack\App; use BookStack\Activity\ActivityQueries; -use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Queries\RecentlyViewed; diff --git a/app/App/Providers/ThemeServiceProvider.php b/app/App/Providers/ThemeServiceProvider.php index c15b43a6b..4c657d912 100644 --- a/app/App/Providers/ThemeServiceProvider.php +++ b/app/App/Providers/ThemeServiceProvider.php @@ -4,7 +4,6 @@ namespace BookStack\App\Providers; use BookStack\Theming\ThemeEvents; use BookStack\Theming\ThemeService; -use Illuminate\Support\Facades\Route; use Illuminate\Support\ServiceProvider; class ThemeServiceProvider extends ServiceProvider diff --git a/app/Console/Commands/CopyShelfPermissionsCommand.php b/app/Console/Commands/CopyShelfPermissionsCommand.php index 8463ccd03..c5e2d504e 100644 --- a/app/Console/Commands/CopyShelfPermissionsCommand.php +++ b/app/Console/Commands/CopyShelfPermissionsCommand.php @@ -2,7 +2,6 @@ namespace BookStack\Console\Commands; -use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Tools\PermissionsUpdater; use Illuminate\Console\Command; diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index a0b98636f..a1c586f47 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -6,7 +6,6 @@ use BookStack\Activity\ActivityQueries; use BookStack\Activity\ActivityType; use BookStack\Activity\Models\View; use BookStack\Activity\Tools\UserEntityWatchOptions; -use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Queries\BookshelfQueries; use BookStack\Entities\Repos\BookRepo; diff --git a/app/Entities/Controllers/ChapterExportApiController.php b/app/Entities/Controllers/ChapterExportApiController.php index 08aa959b3..ceb2522b2 100644 --- a/app/Entities/Controllers/ChapterExportApiController.php +++ b/app/Entities/Controllers/ChapterExportApiController.php @@ -2,7 +2,6 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Models\Chapter; use BookStack\Entities\Queries\ChapterQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\ApiController; diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index 44ba999e8..eab53bb25 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -7,7 +7,6 @@ use BookStack\Activity\Tools\CommentTree; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Repos\PageRepo; diff --git a/app/Entities/Controllers/PageExportApiController.php b/app/Entities/Controllers/PageExportApiController.php index 8737f2f3e..693760bc8 100644 --- a/app/Entities/Controllers/PageExportApiController.php +++ b/app/Entities/Controllers/PageExportApiController.php @@ -2,7 +2,6 @@ namespace BookStack\Entities\Controllers; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Tools\ExportFormatter; use BookStack\Http\ApiController; diff --git a/app/Entities/Models/BookChild.php b/app/Entities/Models/BookChild.php index d19a2466a..ad54fb926 100644 --- a/app/Entities/Models/BookChild.php +++ b/app/Entities/Models/BookChild.php @@ -13,24 +13,9 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; * @property int $priority * @property string $book_slug * @property Book $book - * - * @method Builder whereSlugs(string $bookSlug, string $childSlug) */ abstract class BookChild extends Entity { - /** - * Scope a query to find items where the child has the given childSlug - * where its parent has the bookSlug. - */ - public function scopeWhereSlugs(Builder $query, string $bookSlug, string $childSlug) - { - return $query->with('book') - ->whereHas('book', function (Builder $query) use ($bookSlug) { - $query->where('slug', '=', $bookSlug); - }) - ->where('slug', '=', $childSlug); - } - /** * Get the book this page sits in. */ diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php index 422c82442..c926aaa64 100644 --- a/app/Entities/Models/Chapter.php +++ b/app/Entities/Models/Chapter.php @@ -69,13 +69,4 @@ class Chapter extends BookChild ->orderBy('priority', 'asc') ->get(); } - - /** - * Get a visible chapter by its book and page slugs. - * @throws \Illuminate\Database\Eloquent\ModelNotFoundException - */ - public static function getBySlugs(string $bookSlug, string $chapterSlug): self - { - return static::visible()->whereSlugs($bookSlug, $chapterSlug)->firstOrFail(); - } } diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php index 17d6f9a01..3a433338b 100644 --- a/app/Entities/Models/Page.php +++ b/app/Entities/Models/Page.php @@ -32,9 +32,6 @@ class Page extends BookChild { use HasFactory; - public static $listAttributes = ['name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', 'template', 'text', 'created_at', 'updated_at', 'priority']; - public static $contentAttributes = ['name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', 'template', 'html', 'text', 'created_at', 'updated_at', 'priority']; - protected $fillable = ['name', 'priority']; public string $textField = 'text'; @@ -145,13 +142,4 @@ class Page extends BookChild return $refreshed; } - - /** - * Get a visible page by its book and page slugs. - * @throws \Illuminate\Database\Eloquent\ModelNotFoundException - */ - public static function getBySlugs(string $bookSlug, string $pageSlug): self - { - return static::visible()->whereSlugs($bookSlug, $pageSlug)->firstOrFail(); - } } diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 2ffff5eca..8f62e513c 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -8,6 +8,10 @@ use Illuminate\Database\Eloquent\Builder; class BookQueries implements ProvidesEntityQueries { + protected static array $listAttributes = [ + 'id', 'slug', 'name', 'description', 'created_at', 'updated_at', 'image_id' + ]; + public function start(): Builder { return Book::query(); @@ -40,7 +44,8 @@ class BookQueries implements ProvidesEntityQueries public function visibleForList(): Builder { - return $this->start()->scopes('visible'); + return $this->start()->scopes('visible') + ->select(static::$listAttributes); } public function visibleForListWithCover(): Builder diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index f2fe50531..47ff068a9 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -8,6 +8,10 @@ use Illuminate\Database\Eloquent\Builder; class BookshelfQueries implements ProvidesEntityQueries { + protected static array $listAttributes = [ + 'id', 'slug', 'name', 'description', 'created_at', 'updated_at', 'image_id' + ]; + public function start(): Builder { return Bookshelf::query(); @@ -46,7 +50,7 @@ class BookshelfQueries implements ProvidesEntityQueries public function visibleForList(): Builder { - return $this->start()->scopes('visible'); + return $this->start()->scopes('visible')->select(static::$listAttributes); } public function visibleForListWithCover(): Builder diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php index 31193c7ea..34f762a15 100644 --- a/app/Entities/Queries/ChapterQueries.php +++ b/app/Entities/Queries/ChapterQueries.php @@ -10,8 +10,7 @@ class ChapterQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ 'id', 'slug', 'name', 'description', 'priority', - 'created_at', 'updated_at', - 'created_by', 'updated_by', 'owned_by', + 'created_at', 'updated_at' ]; public function start(): Builder diff --git a/app/Entities/Queries/EntityQueries.php b/app/Entities/Queries/EntityQueries.php index 31e5b913a..36dc6c0bc 100644 --- a/app/Entities/Queries/EntityQueries.php +++ b/app/Entities/Queries/EntityQueries.php @@ -3,6 +3,8 @@ namespace BookStack\Entities\Queries; use BookStack\Entities\Models\Entity; +use Illuminate\Database\Eloquent\Builder; +use InvalidArgumentException; class EntityQueries { @@ -25,9 +27,25 @@ class EntityQueries $explodedId = explode(':', $identifier); $entityType = $explodedId[0]; $entityId = intval($explodedId[1]); + $queries = $this->getQueriesForType($entityType); + return $queries->findVisibleById($entityId); + } + + /** + * Start a query of visible entities of the given type, + * suitable for listing display. + */ + public function visibleForList(string $entityType): Builder + { + $queries = $this->getQueriesForType($entityType); + return $queries->visibleForList(); + } + + protected function getQueriesForType(string $type): ProvidesEntityQueries + { /** @var ?ProvidesEntityQueries $queries */ - $queries = match ($entityType) { + $queries = match ($type) { 'page' => $this->pages, 'chapter' => $this->chapters, 'book' => $this->books, @@ -36,9 +54,9 @@ class EntityQueries }; if (is_null($queries)) { - return null; + throw new InvalidArgumentException("No entity query class configured for {$type}"); } - return $queries->findVisibleById($entityId); + return $queries; } } diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index e22769c3a..ec27ba4aa 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -8,6 +8,15 @@ use Illuminate\Database\Eloquent\Builder; class PageQueries implements ProvidesEntityQueries { + protected static array $contentAttributes = [ + 'name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', + 'template', 'html', 'text', 'created_at', 'updated_at', 'priority' + ]; + protected static array $listAttributes = [ + 'name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', + 'template', 'text', 'created_at', 'updated_at', 'priority' + ]; + public function start(): Builder { return Page::query(); @@ -60,22 +69,14 @@ class PageQueries implements ProvidesEntityQueries { return $this->start() ->scopes('visible') - ->select(array_merge(Page::$listAttributes, ['book_slug' => function ($builder) { - $builder->select('slug') - ->from('books') - ->whereColumn('books.id', '=', 'pages.book_id'); - }])); + ->select($this->mergeBookSlugForSelect(static::$listAttributes)); } public function visibleWithContents(): Builder { return $this->start() ->scopes('visible') - ->select(array_merge(Page::$contentAttributes, ['book_slug' => function ($builder) { - $builder->select('slug') - ->from('books') - ->whereColumn('books.id', '=', 'pages.book_id'); - }])); + ->select($this->mergeBookSlugForSelect(static::$contentAttributes)); } public function currentUserDraftsForList(): Builder @@ -90,4 +91,13 @@ class PageQueries implements ProvidesEntityQueries return $this->visibleForList() ->where('template', '=', true); } + + protected function mergeBookSlugForSelect(array $columns): array + { + return array_merge($columns, ['book_slug' => function ($builder) { + $builder->select('slug') + ->from('books') + ->whereColumn('books.id', '=', 'pages.book_id'); + }]); + } } diff --git a/app/Entities/Queries/ProvidesEntityQueries.php b/app/Entities/Queries/ProvidesEntityQueries.php index 103352244..611d0ae52 100644 --- a/app/Entities/Queries/ProvidesEntityQueries.php +++ b/app/Entities/Queries/ProvidesEntityQueries.php @@ -16,6 +16,19 @@ use Illuminate\Database\Eloquent\Builder; */ interface ProvidesEntityQueries { + /** + * Start a new query for this entity type. + */ public function start(): Builder; + + /** + * Find the entity of the given ID, or return null if not found. + */ public function findVisibleById(int $id): ?Entity; + + /** + * Start a query for items that are visible, with selection + * configured for list display of this item. + */ + public function visibleForList(): Builder; } diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 6674f559a..033350743 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -8,7 +8,6 @@ use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\HasCoverImage; use BookStack\Entities\Models\HasHtmlDescription; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\ImageUploadException; use BookStack\References\ReferenceStore; diff --git a/app/Permissions/JointPermissionBuilder.php b/app/Permissions/JointPermissionBuilder.php index 49eaf0774..c2922cdc9 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -4,7 +4,6 @@ namespace BookStack\Permissions; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\BookChild; -use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; diff --git a/app/References/ReferenceController.php b/app/References/ReferenceController.php index 57dbcce3c..2fe86fd59 100644 --- a/app/References/ReferenceController.php +++ b/app/References/ReferenceController.php @@ -2,10 +2,6 @@ namespace BookStack\References; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Bookshelf; -use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Http\Controller; diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php index 08f0826dd..b94ec1ec7 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -2,7 +2,6 @@ namespace BookStack\Search; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Entities\Queries\Popular; use BookStack\Entities\Tools\SiblingFetcher; diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php index aac9d1000..94518dbf7 100644 --- a/app/Search/SearchRunner.php +++ b/app/Search/SearchRunner.php @@ -3,9 +3,9 @@ namespace BookStack\Search; use BookStack\Entities\EntityProvider; -use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Permissions\PermissionApplicator; use BookStack\Users\Models\User; use Illuminate\Database\Connection; @@ -20,9 +20,6 @@ use SplObjectStorage; class SearchRunner { - protected EntityProvider $entityProvider; - protected PermissionApplicator $permissions; - /** * Acceptable operators to be used in a query. * @@ -38,10 +35,11 @@ class SearchRunner */ protected $termAdjustmentCache; - public function __construct(EntityProvider $entityProvider, PermissionApplicator $permissions) - { - $this->entityProvider = $entityProvider; - $this->permissions = $permissions; + public function __construct( + protected EntityProvider $entityProvider, + protected PermissionApplicator $permissions, + protected EntityQueries $entityQueries, + ) { $this->termAdjustmentCache = new SplObjectStorage(); } @@ -72,10 +70,9 @@ class SearchRunner continue; } - $entityModelInstance = $this->entityProvider->get($entityType); - $searchQuery = $this->buildQuery($searchOpts, $entityModelInstance); + $searchQuery = $this->buildQuery($searchOpts, $entityType); $entityTotal = $searchQuery->count(); - $searchResults = $this->getPageOfDataFromQuery($searchQuery, $entityModelInstance, $page, $count); + $searchResults = $this->getPageOfDataFromQuery($searchQuery, $entityType, $page, $count); if ($entityTotal > ($page * $count)) { $hasMore = true; @@ -108,8 +105,7 @@ class SearchRunner continue; } - $entityModelInstance = $this->entityProvider->get($entityType); - $search = $this->buildQuery($opts, $entityModelInstance)->where('book_id', '=', $bookId)->take(20)->get(); + $search = $this->buildQuery($opts, $entityType)->where('book_id', '=', $bookId)->take(20)->get(); $results = $results->merge($search); } @@ -122,8 +118,7 @@ class SearchRunner public function searchChapter(int $chapterId, string $searchString): Collection { $opts = SearchOptions::fromString($searchString); - $entityModelInstance = $this->entityProvider->get('page'); - $pages = $this->buildQuery($opts, $entityModelInstance)->where('chapter_id', '=', $chapterId)->take(20)->get(); + $pages = $this->buildQuery($opts, 'page')->where('chapter_id', '=', $chapterId)->take(20)->get(); return $pages->sortByDesc('score'); } @@ -131,17 +126,17 @@ class SearchRunner /** * Get a page of result data from the given query based on the provided page parameters. */ - protected function getPageOfDataFromQuery(EloquentBuilder $query, Entity $entityModelInstance, int $page = 1, int $count = 20): EloquentCollection + protected function getPageOfDataFromQuery(EloquentBuilder $query, string $entityType, int $page = 1, int $count = 20): EloquentCollection { $relations = ['tags']; - if ($entityModelInstance instanceof BookChild) { + if ($entityType === 'page' || $entityType === 'chapter') { $relations['book'] = function (BelongsTo $query) { $query->scopes('visible'); }; } - if ($entityModelInstance instanceof Page) { + if ($entityType === 'page') { $relations['chapter'] = function (BelongsTo $query) { $query->scopes('visible'); }; @@ -157,18 +152,13 @@ class SearchRunner /** * Create a search query for an entity. */ - protected function buildQuery(SearchOptions $searchOpts, Entity $entityModelInstance): EloquentBuilder + protected function buildQuery(SearchOptions $searchOpts, string $entityType): EloquentBuilder { - $entityQuery = $entityModelInstance->newQuery()->scopes('visible'); - - if ($entityModelInstance instanceof Page) { - $entityQuery->select(array_merge($entityModelInstance::$listAttributes, ['owned_by'])); - } else { - $entityQuery->select(['*']); - } + $entityModelInstance = $this->entityProvider->get($entityType); + $entityQuery = $this->entityQueries->visibleForList($entityType); // Handle normal search terms - $this->applyTermSearch($entityQuery, $searchOpts, $entityModelInstance); + $this->applyTermSearch($entityQuery, $searchOpts, $entityType); // Handle exact term matching foreach ($searchOpts->exacts as $inputTerm) { @@ -198,7 +188,7 @@ class SearchRunner /** * For the given search query, apply the queries for handling the regular search terms. */ - protected function applyTermSearch(EloquentBuilder $entityQuery, SearchOptions $options, Entity $entity): void + protected function applyTermSearch(EloquentBuilder $entityQuery, SearchOptions $options, string $entityType): void { $terms = $options->searches; if (count($terms) === 0) { @@ -216,7 +206,7 @@ class SearchRunner $subQuery->addBinding($scoreSelect['bindings'], 'select'); - $subQuery->where('entity_type', '=', $entity->getMorphClass()); + $subQuery->where('entity_type', '=', $entityType); $subQuery->where(function (Builder $query) use ($terms) { foreach ($terms as $inputTerm) { $inputTerm = str_replace('\\', '\\\\', $inputTerm); diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index 72f78e347..bd319fbd7 100644 --- a/app/Uploads/AttachmentService.php +++ b/app/Uploads/AttachmentService.php @@ -4,7 +4,6 @@ namespace BookStack\Uploads; use BookStack\Exceptions\FileUploadException; use Exception; -use Illuminate\Contracts\Filesystem\FileNotFoundException; use Illuminate\Contracts\Filesystem\Filesystem as Storage; use Illuminate\Filesystem\FilesystemManager; use Illuminate\Support\Facades\Log; diff --git a/app/Uploads/Controllers/AttachmentApiController.php b/app/Uploads/Controllers/AttachmentApiController.php index 2832fa8e1..9040ba6d3 100644 --- a/app/Uploads/Controllers/AttachmentApiController.php +++ b/app/Uploads/Controllers/AttachmentApiController.php @@ -2,7 +2,6 @@ namespace BookStack\Uploads\Controllers; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\FileUploadException; use BookStack\Http\ApiController; diff --git a/app/Uploads/Controllers/GalleryImageController.php b/app/Uploads/Controllers/GalleryImageController.php index 258f2bef6..1bc9da2d7 100644 --- a/app/Uploads/Controllers/GalleryImageController.php +++ b/app/Uploads/Controllers/GalleryImageController.php @@ -8,8 +8,6 @@ use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageResizer; use BookStack\Util\OutOfMemoryHandler; use Illuminate\Http\Request; -use Illuminate\Support\Facades\App; -use Illuminate\Support\Facades\Log; use Illuminate\Validation\ValidationException; class GalleryImageController extends Controller diff --git a/app/Uploads/Controllers/ImageGalleryApiController.php b/app/Uploads/Controllers/ImageGalleryApiController.php index cec47feb0..6d4657a7a 100644 --- a/app/Uploads/Controllers/ImageGalleryApiController.php +++ b/app/Uploads/Controllers/ImageGalleryApiController.php @@ -2,7 +2,6 @@ namespace BookStack\Uploads\Controllers; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Http\ApiController; use BookStack\Uploads\Image; diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 160a02fa1..1e58816a4 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -2,7 +2,6 @@ namespace BookStack\Uploads; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\PageQueries; use BookStack\Exceptions\ImageUploadException; use BookStack\Permissions\PermissionApplicator; diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index ee2cb8a35..8d8da61ec 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -2,9 +2,6 @@ namespace BookStack\Uploads; -use BookStack\Entities\Models\Book; -use BookStack\Entities\Models\Bookshelf; -use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Exceptions\ImageUploadException; use Exception; From ed21a6d798a5a7b4148bf388051c016a2b3315e4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 8 Feb 2024 16:39:59 +0000 Subject: [PATCH 10/12] Queries: Updated old use-specific entity query classes - Updated name to align, and differentate from new 'XQueries' clases. - Removed old sketchy base class with app resolving workarounds, to a proper injection-based approach. - Also fixed wrong translation text used in PageQueries. --- .../Controllers/FavouriteController.php | 4 +-- app/App/HomeController.php | 16 +++++---- app/Entities/Queries/EntityQuery.php | 25 ------------- app/Entities/Queries/PageQueries.php | 2 +- .../Queries/{Popular.php => QueryPopular.php} | 16 ++++++--- ...ntlyViewed.php => QueryRecentlyViewed.php} | 14 ++++++-- ...pFavourites.php => QueryTopFavourites.php} | 14 ++++++-- app/Entities/Tools/MixedEntityListLoader.php | 35 ++----------------- app/Search/SearchController.php | 6 ++-- resources/views/errors/404.blade.php | 8 ++--- 10 files changed, 57 insertions(+), 83 deletions(-) delete mode 100644 app/Entities/Queries/EntityQuery.php rename app/Entities/Queries/{Popular.php => QueryPopular.php} (80%) rename app/Entities/Queries/{RecentlyViewed.php => QueryRecentlyViewed.php} (61%) rename app/Entities/Queries/{TopFavourites.php => QueryTopFavourites.php} (71%) diff --git a/app/Activity/Controllers/FavouriteController.php b/app/Activity/Controllers/FavouriteController.php index b3aff1cef..3125a25ad 100644 --- a/app/Activity/Controllers/FavouriteController.php +++ b/app/Activity/Controllers/FavouriteController.php @@ -2,7 +2,7 @@ namespace BookStack\Activity\Controllers; -use BookStack\Entities\Queries\TopFavourites; +use BookStack\Entities\Queries\QueryTopFavourites; use BookStack\Entities\Tools\MixedEntityRequestHelper; use BookStack\Http\Controller; use Illuminate\Http\Request; @@ -21,7 +21,7 @@ class FavouriteController extends Controller { $viewCount = 20; $page = intval($request->get('page', 1)); - $favourites = (new TopFavourites())->run($viewCount + 1, (($page - 1) * $viewCount)); + $favourites = (new QueryTopFavourites())->run($viewCount + 1, (($page - 1) * $viewCount)); $hasMoreLink = ($favourites->count() > $viewCount) ? url('/favourites?page=' . ($page + 1)) : null; diff --git a/app/App/HomeController.php b/app/App/HomeController.php index 56f3d81a8..116f5c8a4 100644 --- a/app/App/HomeController.php +++ b/app/App/HomeController.php @@ -5,8 +5,8 @@ namespace BookStack\App; use BookStack\Activity\ActivityQueries; use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; -use BookStack\Entities\Queries\RecentlyViewed; -use BookStack\Entities\Queries\TopFavourites; +use BookStack\Entities\Queries\QueryRecentlyViewed; +use BookStack\Entities\Queries\QueryTopFavourites; use BookStack\Entities\Tools\PageContent; use BookStack\Http\Controller; use BookStack\Uploads\FaviconHandler; @@ -23,8 +23,12 @@ class HomeController extends Controller /** * Display the homepage. */ - public function index(Request $request, ActivityQueries $activities) - { + public function index( + Request $request, + ActivityQueries $activities, + QueryRecentlyViewed $recentlyViewed, + QueryTopFavourites $topFavourites, + ) { $activity = $activities->latest(10); $draftPages = []; @@ -38,9 +42,9 @@ class HomeController extends Controller $recentFactor = count($draftPages) > 0 ? 0.5 : 1; $recents = $this->isSignedIn() ? - (new RecentlyViewed())->run(12 * $recentFactor, 1) + $recentlyViewed->run(12 * $recentFactor, 1) : $this->queries->books->visibleForList()->orderBy('created_at', 'desc')->take(12 * $recentFactor)->get(); - $favourites = (new TopFavourites())->run(6); + $favourites = $topFavourites->run(6); $recentlyUpdatedPages = $this->queries->pages->visibleForList() ->where('draft', false) ->orderBy('updated_at', 'desc') diff --git a/app/Entities/Queries/EntityQuery.php b/app/Entities/Queries/EntityQuery.php deleted file mode 100644 index bd7a98b5e..000000000 --- a/app/Entities/Queries/EntityQuery.php +++ /dev/null @@ -1,25 +0,0 @@ -make(MixedEntityListLoader::class); - } - - protected function permissionService(): PermissionApplicator - { - return app()->make(PermissionApplicator::class); - } - - protected function entityProvider(): EntityProvider - { - return app()->make(EntityProvider::class); - } -} diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index ec27ba4aa..8fb02075a 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -50,7 +50,7 @@ class PageQueries implements ProvidesEntityQueries ->first(); if (is_null($page)) { - throw new NotFoundException(trans('errors.chapter_not_found')); + throw new NotFoundException(trans('errors.page_not_found')); } return $page; diff --git a/app/Entities/Queries/Popular.php b/app/Entities/Queries/QueryPopular.php similarity index 80% rename from app/Entities/Queries/Popular.php rename to app/Entities/Queries/QueryPopular.php index a934f346b..b2ca565eb 100644 --- a/app/Entities/Queries/Popular.php +++ b/app/Entities/Queries/QueryPopular.php @@ -3,24 +3,32 @@ namespace BookStack\Entities\Queries; use BookStack\Activity\Models\View; +use BookStack\Entities\EntityProvider; use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Entity; +use BookStack\Permissions\PermissionApplicator; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; -class Popular extends EntityQuery +class QueryPopular { + public function __construct( + protected PermissionApplicator $permissions, + protected EntityProvider $entityProvider, + ) { + } + public function run(int $count, int $page, array $filterModels = null) { - $query = $this->permissionService() + $query = $this->permissions ->restrictEntityRelationQuery(View::query(), 'views', 'viewable_id', 'viewable_type') ->select('*', 'viewable_id', 'viewable_type', DB::raw('SUM(views) as view_count')) ->groupBy('viewable_id', 'viewable_type') ->orderBy('view_count', 'desc'); if ($filterModels) { - $query->whereIn('viewable_type', $this->entityProvider()->getMorphClasses($filterModels)); + $query->whereIn('viewable_type', $this->entityProvider->getMorphClasses($filterModels)); } $entities = $query->with('viewable') @@ -35,7 +43,7 @@ class Popular extends EntityQuery return $entities; } - protected function loadBooksForChildren(Collection $entities) + protected function loadBooksForChildren(Collection $entities): void { $bookChildren = $entities->filter(fn(Entity $entity) => $entity instanceof BookChild); $eloquent = (new \Illuminate\Database\Eloquent\Collection($bookChildren)); diff --git a/app/Entities/Queries/RecentlyViewed.php b/app/Entities/Queries/QueryRecentlyViewed.php similarity index 61% rename from app/Entities/Queries/RecentlyViewed.php rename to app/Entities/Queries/QueryRecentlyViewed.php index fed15ca5a..f28b8f865 100644 --- a/app/Entities/Queries/RecentlyViewed.php +++ b/app/Entities/Queries/QueryRecentlyViewed.php @@ -3,10 +3,18 @@ namespace BookStack\Entities\Queries; use BookStack\Activity\Models\View; +use BookStack\Entities\Tools\MixedEntityListLoader; +use BookStack\Permissions\PermissionApplicator; use Illuminate\Support\Collection; -class RecentlyViewed extends EntityQuery +class QueryRecentlyViewed { + public function __construct( + protected PermissionApplicator $permissions, + protected MixedEntityListLoader $listLoader, + ) { + } + public function run(int $count, int $page): Collection { $user = user(); @@ -14,7 +22,7 @@ class RecentlyViewed extends EntityQuery return collect(); } - $query = $this->permissionService()->restrictEntityRelationQuery( + $query = $this->permissions->restrictEntityRelationQuery( View::query(), 'views', 'viewable_id', @@ -28,7 +36,7 @@ class RecentlyViewed extends EntityQuery ->take($count) ->get(); - $this->mixedEntityListLoader()->loadIntoRelations($views->all(), 'viewable', false); + $this->listLoader->loadIntoRelations($views->all(), 'viewable', false); return $views->pluck('viewable')->filter(); } diff --git a/app/Entities/Queries/TopFavourites.php b/app/Entities/Queries/QueryTopFavourites.php similarity index 71% rename from app/Entities/Queries/TopFavourites.php rename to app/Entities/Queries/QueryTopFavourites.php index 47d4b77f7..6340e35ef 100644 --- a/app/Entities/Queries/TopFavourites.php +++ b/app/Entities/Queries/QueryTopFavourites.php @@ -3,10 +3,18 @@ namespace BookStack\Entities\Queries; use BookStack\Activity\Models\Favourite; +use BookStack\Entities\Tools\MixedEntityListLoader; +use BookStack\Permissions\PermissionApplicator; use Illuminate\Database\Query\JoinClause; -class TopFavourites extends EntityQuery +class QueryTopFavourites { + public function __construct( + protected PermissionApplicator $permissions, + protected MixedEntityListLoader $listLoader, + ) { + } + public function run(int $count, int $skip = 0) { $user = user(); @@ -14,7 +22,7 @@ class TopFavourites extends EntityQuery return collect(); } - $query = $this->permissionService() + $query = $this->permissions ->restrictEntityRelationQuery(Favourite::query(), 'favourites', 'favouritable_id', 'favouritable_type') ->select('favourites.*') ->leftJoin('views', function (JoinClause $join) { @@ -30,7 +38,7 @@ class TopFavourites extends EntityQuery ->take($count) ->get(); - $this->mixedEntityListLoader()->loadIntoRelations($favourites->all(), 'favouritable', false); + $this->listLoader->loadIntoRelations($favourites->all(), 'favouritable', false); return $favourites->pluck('favouritable')->filter(); } diff --git a/app/Entities/Tools/MixedEntityListLoader.php b/app/Entities/Tools/MixedEntityListLoader.php index a0df791db..f9a940b98 100644 --- a/app/Entities/Tools/MixedEntityListLoader.php +++ b/app/Entities/Tools/MixedEntityListLoader.php @@ -3,20 +3,13 @@ namespace BookStack\Entities\Tools; use BookStack\App\Model; -use BookStack\Entities\EntityProvider; +use BookStack\Entities\Queries\EntityQueries; use Illuminate\Database\Eloquent\Relations\Relation; class MixedEntityListLoader { - protected array $listAttributes = [ - 'page' => ['id', 'name', 'slug', 'book_id', 'chapter_id', 'text', 'draft'], - 'chapter' => ['id', 'name', 'slug', 'book_id', 'description'], - 'book' => ['id', 'name', 'slug', 'description'], - 'bookshelf' => ['id', 'name', 'slug', 'description'], - ]; - public function __construct( - protected EntityProvider $entityProvider + protected EntityQueries $queries, ) { } @@ -61,14 +54,7 @@ class MixedEntityListLoader $modelMap = []; foreach ($idsByType as $type => $ids) { - if (!isset($this->listAttributes[$type])) { - continue; - } - - $instance = $this->entityProvider->get($type); - $models = $instance->newQuery() - ->select(array_merge($this->listAttributes[$type], $this->getSubSelectsForQuery($type))) - ->scopes('visible') + $models = $this->queries->visibleForList($type) ->whereIn('id', $ids) ->with($eagerLoadParents ? $this->getRelationsToEagerLoad($type) : []) ->get(); @@ -100,19 +86,4 @@ class MixedEntityListLoader return $toLoad; } - - protected function getSubSelectsForQuery(string $type): array - { - $subSelects = []; - - if ($type === 'chapter' || $type === 'page') { - $subSelects['book_slug'] = function ($builder) { - $builder->select('slug') - ->from('books') - ->whereColumn('books.id', '=', 'book_id'); - }; - } - - return $subSelects; - } } diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php index b94ec1ec7..2fce6a3d5 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -3,7 +3,7 @@ namespace BookStack\Search; use BookStack\Entities\Queries\PageQueries; -use BookStack\Entities\Queries\Popular; +use BookStack\Entities\Queries\QueryPopular; use BookStack\Entities\Tools\SiblingFetcher; use BookStack\Http\Controller; use Illuminate\Http\Request; @@ -67,7 +67,7 @@ class SearchController extends Controller * Search for a list of entities and return a partial HTML response of matching entities. * Returns the most popular entities if no search is provided. */ - public function searchForSelector(Request $request) + public function searchForSelector(Request $request, QueryPopular $queryPopular) { $entityTypes = $request->filled('types') ? explode(',', $request->get('types')) : ['page', 'chapter', 'book']; $searchTerm = $request->get('term', false); @@ -78,7 +78,7 @@ class SearchController extends Controller $searchTerm .= ' {type:' . implode('|', $entityTypes) . '}'; $entities = $this->searchRunner->searchEntities(SearchOptions::fromString($searchTerm), 'all', 1, 20)['results']; } else { - $entities = (new Popular())->run(20, 0, $entityTypes); + $entities = $queryPopular->run(20, 0, $entityTypes); } return view('search.parts.entity-selector-list', ['entities' => $entities, 'permission' => $permission]); diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php index 27d66b30b..dc24a558d 100644 --- a/resources/views/errors/404.blade.php +++ b/resources/views/errors/404.blade.php @@ -1,5 +1,5 @@ @extends('layouts.simple') - +@inject('popular', \BookStack\Entities\Queries\QueryPopular::class) @section('content')
@@ -28,7 +28,7 @@

{{ trans('entities.pages_popular') }}

- @include('entities.list', ['entities' => (new \BookStack\Entities\Queries\Popular)->run(10, 0, ['page']), 'style' => 'compact']) + @include('entities.list', ['entities' => $popular->run(10, 0, ['page']), 'style' => 'compact'])
@@ -36,7 +36,7 @@

{{ trans('entities.books_popular') }}

- @include('entities.list', ['entities' => (new \BookStack\Entities\Queries\Popular)->run(10, 0, ['book']), 'style' => 'compact']) + @include('entities.list', ['entities' => $popular->run(10, 0, ['book']), 'style' => 'compact'])
@@ -44,7 +44,7 @@

{{ trans('entities.chapters_popular') }}

- @include('entities.list', ['entities' => (new \BookStack\Entities\Queries\Popular)->run(10, 0, ['chapter']), 'style' => 'compact']) + @include('entities.list', ['entities' => $popular->run(10, 0, ['chapter']), 'style' => 'compact'])
From ed9c013f6e7ee7a7bfe1c67b4cb1a84ca198b7a6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 8 Feb 2024 17:18:03 +0000 Subject: [PATCH 11/12] Queries: Addressed failing test cases from recent changes --- app/Activity/Controllers/FavouriteController.php | 4 ++-- app/Entities/Queries/BookQueries.php | 3 ++- app/Entities/Queries/BookshelfQueries.php | 3 ++- app/Entities/Queries/ChapterQueries.php | 2 +- app/Entities/Queries/PageQueries.php | 5 +++-- app/Entities/Tools/PageContent.php | 5 +++-- app/Entities/Tools/PageEditorData.php | 5 +++-- app/Entities/Tools/SiblingFetcher.php | 3 ++- 8 files changed, 18 insertions(+), 12 deletions(-) diff --git a/app/Activity/Controllers/FavouriteController.php b/app/Activity/Controllers/FavouriteController.php index 3125a25ad..deeb4b0af 100644 --- a/app/Activity/Controllers/FavouriteController.php +++ b/app/Activity/Controllers/FavouriteController.php @@ -17,11 +17,11 @@ class FavouriteController extends Controller /** * Show a listing of all favourite items for the current user. */ - public function index(Request $request) + public function index(Request $request, QueryTopFavourites $topFavourites) { $viewCount = 20; $page = intval($request->get('page', 1)); - $favourites = (new QueryTopFavourites())->run($viewCount + 1, (($page - 1) * $viewCount)); + $favourites = $topFavourites->run($viewCount + 1, (($page - 1) * $viewCount)); $hasMoreLink = ($favourites->count() > $viewCount) ? url('/favourites?page=' . ($page + 1)) : null; diff --git a/app/Entities/Queries/BookQueries.php b/app/Entities/Queries/BookQueries.php index 8f62e513c..534640621 100644 --- a/app/Entities/Queries/BookQueries.php +++ b/app/Entities/Queries/BookQueries.php @@ -9,7 +9,8 @@ use Illuminate\Database\Eloquent\Builder; class BookQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ - 'id', 'slug', 'name', 'description', 'created_at', 'updated_at', 'image_id' + 'id', 'slug', 'name', 'description', + 'created_at', 'updated_at', 'image_id', 'owned_by', ]; public function start(): Builder diff --git a/app/Entities/Queries/BookshelfQueries.php b/app/Entities/Queries/BookshelfQueries.php index 47ff068a9..19717fb7c 100644 --- a/app/Entities/Queries/BookshelfQueries.php +++ b/app/Entities/Queries/BookshelfQueries.php @@ -9,7 +9,8 @@ use Illuminate\Database\Eloquent\Builder; class BookshelfQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ - 'id', 'slug', 'name', 'description', 'created_at', 'updated_at', 'image_id' + 'id', 'slug', 'name', 'description', + 'created_at', 'updated_at', 'image_id', 'owned_by', ]; public function start(): Builder diff --git a/app/Entities/Queries/ChapterQueries.php b/app/Entities/Queries/ChapterQueries.php index 34f762a15..53c5bc9d8 100644 --- a/app/Entities/Queries/ChapterQueries.php +++ b/app/Entities/Queries/ChapterQueries.php @@ -10,7 +10,7 @@ class ChapterQueries implements ProvidesEntityQueries { protected static array $listAttributes = [ 'id', 'slug', 'name', 'description', 'priority', - 'created_at', 'updated_at' + 'book_id', 'created_at', 'updated_at', 'owned_by', ]; public function start(): Builder diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index 8fb02075a..a5938f754 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -10,11 +10,12 @@ class PageQueries implements ProvidesEntityQueries { protected static array $contentAttributes = [ 'name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', - 'template', 'html', 'text', 'created_at', 'updated_at', 'priority' + 'template', 'html', 'text', 'created_at', 'updated_at', 'priority', + 'created_by', 'updated_by', 'owned_by', ]; protected static array $listAttributes = [ 'name', 'id', 'slug', 'book_id', 'chapter_id', 'draft', - 'template', 'text', 'created_at', 'updated_at', 'priority' + 'template', 'text', 'created_at', 'updated_at', 'priority', 'owned_by', ]; public function start(): Builder diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 88987f054..4f68b828f 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -329,13 +329,14 @@ class PageContent protected function getContentProviderClosure(bool $blankIncludes): Closure { $contextPage = $this->page; + $queries = $this->pageQueries; - return function (PageIncludeTag $tag) use ($blankIncludes, $contextPage): PageIncludeContent { + return function (PageIncludeTag $tag) use ($blankIncludes, $contextPage, $queries): PageIncludeContent { if ($blankIncludes) { return PageIncludeContent::fromHtmlAndTag('', $tag); } - $matchedPage = $this->pageQueries->findVisibleById($tag->getPageId()); + $matchedPage = $queries->findVisibleById($tag->getPageId()); $content = PageIncludeContent::fromHtmlAndTag($matchedPage->html ?? '', $tag); if (Theme::hasListeners(ThemeEvents::PAGE_INCLUDE_PARSE)) { diff --git a/app/Entities/Tools/PageEditorData.php b/app/Entities/Tools/PageEditorData.php index 20bf19eb2..f0bd23589 100644 --- a/app/Entities/Tools/PageEditorData.php +++ b/app/Entities/Tools/PageEditorData.php @@ -38,7 +38,8 @@ class PageEditorData $templates = $this->queries->pages->visibleTemplates() ->orderBy('name', 'asc') ->take(10) - ->get(); + ->paginate() + ->withPath('/templates'); $draftsEnabled = auth()->check(); @@ -51,7 +52,7 @@ class PageEditorData } // Check for a current draft version for this user - $userDraft = $this->queries->revisions->findLatestCurrentUserDraftsForPageId($page->id)->first(); + $userDraft = $this->queries->revisions->findLatestCurrentUserDraftsForPageId($page->id); if (!is_null($userDraft)) { $page->forceFill($userDraft->only(['name', 'html', 'markdown'])); $isDraftRevision = true; diff --git a/app/Entities/Tools/SiblingFetcher.php b/app/Entities/Tools/SiblingFetcher.php index 7b8ac37ad..34d0fc6b0 100644 --- a/app/Entities/Tools/SiblingFetcher.php +++ b/app/Entities/Tools/SiblingFetcher.php @@ -14,6 +14,7 @@ class SiblingFetcher { public function __construct( protected EntityQueries $queries, + protected ShelfContext $shelfContext, ) { } @@ -38,7 +39,7 @@ class SiblingFetcher // Book // Gets just the books in a shelf if shelf is in context if ($entity instanceof Book) { - $contextShelf = (new ShelfContext())->getContextualShelfForBook($entity); + $contextShelf = $this->shelfContext->getContextualShelfForBook($entity); if ($contextShelf) { $entities = $contextShelf->visibleBooks()->get(); } else { From 1ea2ac864aaa7e4ee3995ec675fd92db7b2722cd Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 11 Feb 2024 15:42:37 +0000 Subject: [PATCH 12/12] Queries: Update API to align data with previous versions Ensures fields returned match API docs and previous versions of BookStack where we were accidentally returning more fields than expected. Updates tests to cover many of these. Also updated clockwork to ignore image requests for less noisy debugging. Also updated chapter page query to not be loading all page data, via new query in PageQueries. --- app/Config/clockwork.php | 2 ++ .../Controllers/BookApiController.php | 4 +++- .../Controllers/BookshelfApiController.php | 4 +++- .../Controllers/ChapterApiController.php | 23 +++++++++++-------- .../Controllers/ChapterController.php | 3 ++- .../Controllers/PageApiController.php | 3 ++- app/Entities/Queries/PageQueries.php | 8 +++++++ tests/Api/BooksApiTest.php | 3 +++ tests/Api/ChaptersApiTest.php | 13 +++++++++++ tests/Api/PagesApiTest.php | 4 ++++ tests/Api/ShelvesApiTest.php | 3 +++ 11 files changed, 56 insertions(+), 14 deletions(-) diff --git a/app/Config/clockwork.php b/app/Config/clockwork.php index 394af8451..bd59eaf71 100644 --- a/app/Config/clockwork.php +++ b/app/Config/clockwork.php @@ -173,6 +173,8 @@ return [ // List of URIs that should not be collected 'except' => [ + '/uploads/images/.*', // BookStack image requests + '/horizon/.*', // Laravel Horizon requests '/telescope/.*', // Laravel Telescope requests '/_debugbar/.*', // Laravel DebugBar requests diff --git a/app/Entities/Controllers/BookApiController.php b/app/Entities/Controllers/BookApiController.php index 955bd707b..15e67a0f7 100644 --- a/app/Entities/Controllers/BookApiController.php +++ b/app/Entities/Controllers/BookApiController.php @@ -26,7 +26,9 @@ class BookApiController extends ApiController */ public function list() { - $books = $this->queries->visibleForList(); + $books = $this->queries + ->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($books, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', diff --git a/app/Entities/Controllers/BookshelfApiController.php b/app/Entities/Controllers/BookshelfApiController.php index 9170226a5..a665bcb6b 100644 --- a/app/Entities/Controllers/BookshelfApiController.php +++ b/app/Entities/Controllers/BookshelfApiController.php @@ -24,7 +24,9 @@ class BookshelfApiController extends ApiController */ public function list() { - $shelves = $this->queries->visibleForList(); + $shelves = $this->queries + ->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($shelves, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', diff --git a/app/Entities/Controllers/ChapterApiController.php b/app/Entities/Controllers/ChapterApiController.php index fb484b85d..430654330 100644 --- a/app/Entities/Controllers/ChapterApiController.php +++ b/app/Entities/Controllers/ChapterApiController.php @@ -3,8 +3,8 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Queries\ChapterQueries; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Repos\ChapterRepo; use BookStack\Exceptions\PermissionsException; use BookStack\Http\ApiController; @@ -38,7 +38,7 @@ class ChapterApiController extends ApiController public function __construct( protected ChapterRepo $chapterRepo, protected ChapterQueries $queries, - protected BookQueries $bookQueries, + protected EntityQueries $entityQueries, ) { } @@ -47,7 +47,8 @@ class ChapterApiController extends ApiController */ public function list() { - $chapters = $this->queries->visibleForList(); + $chapters = $this->queries->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($chapters, [ 'id', 'book_id', 'name', 'slug', 'description', 'priority', @@ -63,7 +64,7 @@ class ChapterApiController extends ApiController $requestData = $this->validate($request, $this->rules['create']); $bookId = $request->get('book_id'); - $book = $this->bookQueries->findVisibleByIdOrFail(intval($bookId)); + $book = $this->entityQueries->books->findVisibleByIdOrFail(intval($bookId)); $this->checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($requestData, $book); @@ -79,12 +80,14 @@ class ChapterApiController extends ApiController $chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $chapter = $this->forJsonDisplay($chapter); - $chapter->load([ - 'createdBy', 'updatedBy', 'ownedBy', - 'pages' => function (HasMany $query) { - $query->scopes('visible')->get(['id', 'name', 'slug']); - } - ]); + $chapter->load(['createdBy', 'updatedBy', 'ownedBy']); + + // Note: More fields than usual here, for backwards compatibility, + // due to previously accidentally including more fields that desired. + $pages = $this->entityQueries->pages->visibleForChapterList($chapter->id) + ->addSelect(['created_by', 'updated_by', 'revision_count', 'editor']) + ->get(); + $chapter->setRelation('pages', $pages); return response()->json($chapter); } diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 2e36a84b9..4274589e2 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -79,7 +79,8 @@ class ChapterController extends Controller $this->checkOwnablePermission('chapter-view', $chapter); $sidebarTree = (new BookContents($chapter->book))->getTree(); - $pages = $chapter->getVisiblePages(); + $pages = $this->entityQueries->pages->visibleForChapterList($chapter->id)->get(); + $nextPreviousLocator = new NextPreviousContentLocator($chapter, $sidebarTree); View::incrementFor($chapter); diff --git a/app/Entities/Controllers/PageApiController.php b/app/Entities/Controllers/PageApiController.php index d2a5a3ee3..40598e209 100644 --- a/app/Entities/Controllers/PageApiController.php +++ b/app/Entities/Controllers/PageApiController.php @@ -45,7 +45,8 @@ class PageApiController extends ApiController */ public function list() { - $pages = $this->queries->visibleForList(); + $pages = $this->queries->visibleForList() + ->addSelect(['created_by', 'updated_by', 'revision_count', 'editor']); return $this->apiListingResponse($pages, [ 'id', 'book_id', 'chapter_id', 'name', 'slug', 'priority', diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index a5938f754..06298f470 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -73,6 +73,14 @@ class PageQueries implements ProvidesEntityQueries ->select($this->mergeBookSlugForSelect(static::$listAttributes)); } + public function visibleForChapterList(int $chapterId): Builder + { + return $this->visibleForList() + ->where('chapter_id', '=', $chapterId) + ->orderBy('draft', 'desc') + ->orderBy('priority', 'asc'); + } + public function visibleWithContents(): Builder { return $this->start() diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php index b31bd7d37..b8c2b6133 100644 --- a/tests/Api/BooksApiTest.php +++ b/tests/Api/BooksApiTest.php @@ -24,6 +24,9 @@ class BooksApiTest extends TestCase 'id' => $firstBook->id, 'name' => $firstBook->name, 'slug' => $firstBook->slug, + 'owned_by' => $firstBook->owned_by, + 'created_by' => $firstBook->created_by, + 'updated_by' => $firstBook->updated_by, ], ]]); } diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php index e2d6cfc81..9698d4dd9 100644 --- a/tests/Api/ChaptersApiTest.php +++ b/tests/Api/ChaptersApiTest.php @@ -28,6 +28,9 @@ class ChaptersApiTest extends TestCase 'book_id' => $firstChapter->book->id, 'priority' => $firstChapter->priority, 'book_slug' => $firstChapter->book->slug, + 'owned_by' => $firstChapter->owned_by, + 'created_by' => $firstChapter->created_by, + 'updated_by' => $firstChapter->updated_by, ], ]]); } @@ -149,6 +152,16 @@ class ChaptersApiTest extends TestCase 'id' => $page->id, 'slug' => $page->slug, 'name' => $page->name, + 'owned_by' => $page->owned_by, + 'created_by' => $page->created_by, + 'updated_by' => $page->updated_by, + 'book_id' => $page->id, + 'chapter_id' => $chapter->id, + 'priority' => $page->priority, + 'book_slug' => $chapter->book->slug, + 'draft' => $page->draft, + 'template' => $page->template, + 'editor' => $page->editor, ], ], 'default_template_id' => null, diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php index 0d084472d..22659d5bb 100644 --- a/tests/Api/PagesApiTest.php +++ b/tests/Api/PagesApiTest.php @@ -27,6 +27,10 @@ class PagesApiTest extends TestCase 'slug' => $firstPage->slug, 'book_id' => $firstPage->book->id, 'priority' => $firstPage->priority, + 'owned_by' => $firstPage->owned_by, + 'created_by' => $firstPage->created_by, + 'updated_by' => $firstPage->updated_by, + 'revision_count' => $firstPage->revision_count, ], ]]); } diff --git a/tests/Api/ShelvesApiTest.php b/tests/Api/ShelvesApiTest.php index f1b8ed985..be276e110 100644 --- a/tests/Api/ShelvesApiTest.php +++ b/tests/Api/ShelvesApiTest.php @@ -25,6 +25,9 @@ class ShelvesApiTest extends TestCase 'id' => $firstBookshelf->id, 'name' => $firstBookshelf->name, 'slug' => $firstBookshelf->slug, + 'owned_by' => $firstBookshelf->owned_by, + 'created_by' => $firstBookshelf->created_by, + 'updated_by' => $firstBookshelf->updated_by, ], ]]); }