From: Dan Brown Date: Sun, 4 Feb 2024 14:39:01 +0000 (+0000) Subject: DB: Started update of entity loading to avoid global selects X-Git-Tag: v24.02~1^2~16^2~11 X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/a70ed81908a8bcd67e6c449eb7d0cdd6f26ef998 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 --- 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'); + }); + } +};