Improved sort efficiency by a factor of 10

Fixes #145

diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php
index 5b97fbdafd18b628f97f6deb43a09b09b7436abb..8ada594335292b93c5ffb7a6d7a89dd5e440bdcb 100644 (file)
--- a/app/Http/Controllers/BookController.php
+++ b/app/Http/Controllers/BookController.php
@@ -3,7 +3,6 @@
 use Activity;
 use BookStack\Repos\UserRepo;
 use Illuminate\Http\Request;
 use Activity;
 use BookStack\Repos\UserRepo;
 use Illuminate\Http\Request;

-use Illuminate\Support\Facades\Auth;

 use BookStack\Http\Requests;
 use BookStack\Repos\BookRepo;
 use BookStack\Repos\ChapterRepo;
 use BookStack\Http\Requests;
 use BookStack\Repos\BookRepo;
 use BookStack\Repos\ChapterRepo;


@@ -180,21 +179,31 @@ class BookController extends Controller
             return redirect($book->getUrl());
         }
 
             return redirect($book->getUrl());
         }
 

-        $sortedBooks = [];

         // Sort pages and chapters
         // Sort pages and chapters

+        $sortedBooks = [];
+        $updatedModels = collect();

         $sortMap = json_decode($request->get('sort-tree'));
         $defaultBookId = $book->id;
         $sortMap = json_decode($request->get('sort-tree'));
         $defaultBookId = $book->id;

-        foreach ($sortMap as $index => $bookChild) {
-            $id = $bookChild->id;
+
+        // Loop through contents of provided map and update entities accordingly
+        foreach ($sortMap as $bookChild) {
+            $priority = $bookChild->sort;
+            $id = intval($bookChild->id);

             $isPage = $bookChild->type == 'page';
             $isPage = $bookChild->type == 'page';

-            $bookId = $this->bookRepo->exists($bookChild->book) ? $bookChild->book : $defaultBookId;
+            $bookId = $this->bookRepo->exists($bookChild->book) ? intval($bookChild->book) : $defaultBookId;
+            $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter);

             $model = $isPage ? $this->pageRepo->getById($id) : $this->chapterRepo->getById($id);
             $model = $isPage ? $this->pageRepo->getById($id) : $this->chapterRepo->getById($id);

-            $isPage ? $this->pageRepo->changeBook($bookId, $model) : $this->chapterRepo->changeBook($bookId, $model);
-            $model->priority = $index;
-            if ($isPage) {
-                $model->chapter_id = ($bookChild->parentChapter === false) ? 0 : $bookChild->parentChapter;
+
+            // Update models only if there's a change in parent chain or ordering.
+            if ($model->priority !== $priority || $model->book_id !== $bookId || ($isPage && $model->chapter_id !== $chapterId)) {
+                $isPage ? $this->pageRepo->changeBook($bookId, $model) : $this->chapterRepo->changeBook($bookId, $model);
+                $model->priority = $priority;
+                if ($isPage) $model->chapter_id = $chapterId;
+                $model->save();
+                $updatedModels->push($model);

             }
             }

-            $model->save();
+
+            // Store involved books to be sorted later

             if (!in_array($bookId, $sortedBooks)) {
                 $sortedBooks[] = $bookId;
             }
             if (!in_array($bookId, $sortedBooks)) {
                 $sortedBooks[] = $bookId;
             }


@@ -203,10 +212,12 @@ class BookController extends Controller
         // Add activity for books
         foreach ($sortedBooks as $bookId) {
             $updatedBook = $this->bookRepo->getById($bookId);
         // Add activity for books
         foreach ($sortedBooks as $bookId) {
             $updatedBook = $this->bookRepo->getById($bookId);

-            $this->bookRepo->updateBookPermissions($updatedBook);

             Activity::add($updatedBook, 'book_sort', $updatedBook->id);
         }
 
             Activity::add($updatedBook, 'book_sort', $updatedBook->id);
         }
 

+        // Update permissions on changed models
+        $this->bookRepo->buildJointPermissions($updatedModels);
+

         return redirect($book->getUrl());
     }
 
         return redirect($book->getUrl());
     }
 

diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php
index 3c9050bf6b4b705aeb371a09b8273ed9ea581245..03ec2c1109973ed3013fb0d579e65a6abfdbb558 100644 (file)
--- a/app/Http/Controllers/ChapterController.php
+++ b/app/Http/Controllers/ChapterController.php
@@ -204,7 +204,7 @@ class ChapterController extends Controller
             return redirect()->back();
         }
 
             return redirect()->back();
         }
 

-        $this->chapterRepo->changeBook($parent->id, $chapter);
+        $this->chapterRepo->changeBook($parent->id, $chapter, true);

         Activity::add($chapter, 'chapter_move', $chapter->book->id);
         session()->flash('success', sprintf('Chapter moved to "%s"', $parent->name));
 
         Activity::add($chapter, 'chapter_move', $chapter->book->id);
         session()->flash('success', sprintf('Chapter moved to "%s"', $parent->name));
 

diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php
index 58816d738578a8c1e6d784a48b6159dc7633a514..fdc4dd8d47fc9c2611e6dba07f4c1fb6bf470da2 100644 (file)
--- a/app/Repos/BookRepo.php
+++ b/app/Repos/BookRepo.php
@@ -2,6 +2,7 @@
 
 use Alpha\B;
 use BookStack\Exceptions\NotFoundException;
 
 use Alpha\B;
 use BookStack\Exceptions\NotFoundException;

+use Illuminate\Database\Eloquent\Collection;

 use Illuminate\Support\Str;
 use BookStack\Book;
 use Views;
 use Illuminate\Support\Str;
 use BookStack\Book;
 use Views;


@@ -173,15 +174,6 @@ class BookRepo extends EntityRepo
         $book->delete();
     }
 
         $book->delete();
     }
 

-    /**
-     * Alias method to update the book jointPermissions in the PermissionService.
-     * @param Book $book
-     */
-    public function updateBookPermissions(Book $book)
-    {
-        $this->permissionService->buildJointPermissionsForEntity($book);
-    }
-

     /**
      * Get the next child element priority.
      * @param Book $book
     /**
      * Get the next child element priority.
      * @param Book $book

diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php
index 1a8cbdf0fdada690eae187ad5a6c9db4fd5e37ba..c12a9f0f206811eec5b14dd1e27e04f20ccf5275 100644 (file)
--- a/app/Repos/ChapterRepo.php
+++ b/app/Repos/ChapterRepo.php
@@ -195,11 +195,12 @@ class ChapterRepo extends EntityRepo
 
     /**
      * Changes the book relation of this chapter.
 
     /**
      * Changes the book relation of this chapter.

-     * @param         $bookId
+     * @param $bookId

      * @param Chapter $chapter
      * @param Chapter $chapter

+     * @param bool $rebuildPermissions

      * @return Chapter
      */
      * @return Chapter
      */

-    public function changeBook($bookId, Chapter $chapter)
+    public function changeBook($bookId, Chapter $chapter, $rebuildPermissions = false)

     {
         $chapter->book_id = $bookId;
         // Update related activity
     {
         $chapter->book_id = $bookId;
         // Update related activity


@@ -213,9 +214,12 @@ class ChapterRepo extends EntityRepo
         foreach ($chapter->pages as $page) {
             $this->pageRepo->changeBook($bookId, $page);
         }
         foreach ($chapter->pages as $page) {
             $this->pageRepo->changeBook($bookId, $page);
         }

-        // Update permissions
-        $chapter->load('book');
-        $this->permissionService->buildJointPermissionsForEntity($chapter->book);
+
+        // Update permissions if applicable
+        if ($rebuildPermissions) {
+            $chapter->load('book');
+            $this->permissionService->buildJointPermissionsForEntity($chapter->book);
+        }

 
         return $chapter;
     }
 
         return $chapter;
     }

diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php
index 28f4c0ae8466a805b456cb8ba2eb08e5e7b98038..c94601738dc2f926b8ea0775982a8c3467efbba6 100644 (file)
--- a/app/Repos/EntityRepo.php
+++ b/app/Repos/EntityRepo.php
@@ -6,6 +6,7 @@ use BookStack\Entity;
 use BookStack\Page;
 use BookStack\Services\PermissionService;
 use BookStack\User;
 use BookStack\Page;
 use BookStack\Services\PermissionService;
 use BookStack\User;

+use Illuminate\Support\Collection;

 use Illuminate\Support\Facades\Log;
 
 class EntityRepo
 use Illuminate\Support\Facades\Log;
 
 class EntityRepo


@@ -260,6 +261,15 @@ class EntityRepo
         return $query;
     }
 
         return $query;
     }
 

+    /**
+     * Alias method to update the book jointPermissions in the PermissionService.
+     * @param Collection $collection collection on entities
+     */
+    public function buildJointPermissions(Collection $collection)
+    {
+        $this->permissionService->buildJointPermissionsForEntities($collection);
+    }
+

 }
 
 
 }
 
 

diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php
index 0fffe60f29d523153516fc33b12431af8f9a5660..cee074cd7cd09b1f36278cef042443cb05166181 100644 (file)
--- a/app/Services/PermissionService.php
+++ b/app/Services/PermissionService.php
@@ -8,7 +8,7 @@ use BookStack\Ownable;
 use BookStack\Page;
 use BookStack\Role;
 use BookStack\User;
 use BookStack\Page;
 use BookStack\Role;
 use BookStack\User;

-use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Support\Collection;

 
 class PermissionService
 {
 
 class PermissionService
 {


@@ -25,6 +25,8 @@ class PermissionService
     protected $jointPermission;
     protected $role;
 
     protected $jointPermission;
     protected $role;
 

+    protected $entityCache;
+

     /**
      * PermissionService constructor.
      * @param JointPermission $jointPermission
     /**
      * PermissionService constructor.
      * @param JointPermission $jointPermission


@@ -48,6 +50,57 @@ class PermissionService
         $this->page = $page;
     }
 
         $this->page = $page;
     }
 

+    /**
+     * Prepare the local entity cache and ensure it's empty
+     */
+    protected function readyEntityCache()
+    {
+        $this->entityCache = [
+            'books' => collect(),
+            'chapters' => collect()
+        ];
+    }
+
+    /**
+     * Get a book via ID, Checks local cache
+     * @param $bookId
+     * @return Book
+     */
+    protected function getBook($bookId)
+    {
+        if (isset($this->entityCache['books']) && $this->entityCache['books']->has($bookId)) {
+            return $this->entityCache['books']->get($bookId);
+        }
+
+        $book = $this->book->find($bookId);
+        if ($book === null) $book = false;
+        if (isset($this->entityCache['books'])) {
+            $this->entityCache['books']->put($bookId, $book);
+        }
+
+        return $book;
+    }
+
+    /**
+     * Get a chapter via ID, Checks local cache
+     * @param $chapterId
+     * @return Book
+     */
+    protected function getChapter($chapterId)
+    {
+        if (isset($this->entityCache['chapters']) && $this->entityCache['chapters']->has($chapterId)) {
+            return $this->entityCache['chapters']->get($chapterId);
+        }
+
+        $chapter = $this->chapter->find($chapterId);
+        if ($chapter === null) $chapter = false;
+        if (isset($this->entityCache['chapters'])) {
+            $this->entityCache['chapters']->put($chapterId, $chapter);
+        }
+
+        return $chapter;
+    }
+

     /**
      * Get the roles for the current user;
      * @return array|bool
     /**
      * Get the roles for the current user;
      * @return array|bool


@@ -76,6 +129,7 @@ class PermissionService
     public function buildJointPermissions()
     {
         $this->jointPermission->truncate();
     public function buildJointPermissions()
     {
         $this->jointPermission->truncate();

+        $this->readyEntityCache();

 
         // Get all roles (Should be the most limited dimension)
         $roles = $this->role->with('permissions')->get();
 
         // Get all roles (Should be the most limited dimension)
         $roles = $this->role->with('permissions')->get();


@@ -97,7 +151,7 @@ class PermissionService
     }
 
     /**
     }
 
     /**

-     * Create the entity jointPermissions for a particular entity.
+     * Rebuild the entity jointPermissions for a particular entity.

      * @param Entity $entity
      */
     public function buildJointPermissionsForEntity(Entity $entity)
      * @param Entity $entity
      */
     public function buildJointPermissionsForEntity(Entity $entity)


@@ -116,6 +170,17 @@ class PermissionService
         $this->createManyJointPermissions($entities, $roles);
     }
 
         $this->createManyJointPermissions($entities, $roles);
     }
 

+    /**
+     * Rebuild the entity jointPermissions for a collection of entities.
+     * @param Collection $entities
+     */
+    public function buildJointPermissionsForEntities(Collection $entities)
+    {
+        $roles = $this->role->with('jointPermissions')->get();
+        $this->deleteManyJointPermissionsForEntities($entities);
+        $this->createManyJointPermissions($entities, $roles);
+    }
+

     /**
      * Build the entity jointPermissions for a particular role.
      * @param Role $role
     /**
      * Build the entity jointPermissions for a particular role.
      * @param Role $role


@@ -177,9 +242,14 @@ class PermissionService
      */
     protected function deleteManyJointPermissionsForEntities($entities)
     {
      */
     protected function deleteManyJointPermissionsForEntities($entities)
     {

+        $query = $this->jointPermission->newQuery();

         foreach ($entities as $entity) {
         foreach ($entities as $entity) {

-            $entity->jointPermissions()->delete();
+            $query->orWhere(function($query) use ($entity) {
+                $query->where('entity_id', '=', $entity->id)
+                    ->where('entity_type', '=', $entity->getMorphClass());
+            });

         }
         }

+        $query->delete();

     }
 
     /**
     }
 
     /**


@@ -189,6 +259,7 @@ class PermissionService
      */
     protected function createManyJointPermissions($entities, $roles)
     {
      */
     protected function createManyJointPermissions($entities, $roles)
     {

+        $this->readyEntityCache();

         $jointPermissions = [];
         foreach ($entities as $entity) {
             foreach ($roles as $role) {
         $jointPermissions = [];
         foreach ($entities as $entity) {
             foreach ($roles as $role) {


@@ -248,8 +319,9 @@ class PermissionService
         } elseif ($entity->isA('chapter')) {
 
             if (!$entity->restricted) {
         } elseif ($entity->isA('chapter')) {
 
             if (!$entity->restricted) {

-                $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction);
-                $hasPermissiveAccessToBook = !$entity->book->restricted;
+                $book = $this->getBook($entity->book_id);
+                $hasExplicitAccessToBook = $book->hasActiveRestriction($role->id, $restrictionAction);
+                $hasPermissiveAccessToBook = !$book->restricted;

                 return $this->createJointPermissionDataArray($entity, $role, $action,
                     ($hasExplicitAccessToBook || ($roleHasPermission && $hasPermissiveAccessToBook)),
                     ($hasExplicitAccessToBook || ($roleHasPermissionOwn && $hasPermissiveAccessToBook)));
                 return $this->createJointPermissionDataArray($entity, $role, $action,
                     ($hasExplicitAccessToBook || ($roleHasPermission && $hasPermissiveAccessToBook)),
                     ($hasExplicitAccessToBook || ($roleHasPermissionOwn && $hasPermissiveAccessToBook)));


@@ -261,11 +333,14 @@ class PermissionService
         } elseif ($entity->isA('page')) {
 
             if (!$entity->restricted) {
         } elseif ($entity->isA('page')) {
 
             if (!$entity->restricted) {

-                $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction);
-                $hasPermissiveAccessToBook = !$entity->book->restricted;
-                $hasExplicitAccessToChapter = $entity->chapter && $entity->chapter->hasActiveRestriction($role->id, $restrictionAction);
-                $hasPermissiveAccessToChapter = $entity->chapter && !$entity->chapter->restricted;
-                $acknowledgeChapter = ($entity->chapter && $entity->chapter->restricted);
+                $book = $this->getBook($entity->book_id);
+                $hasExplicitAccessToBook = $book->hasActiveRestriction($role->id, $restrictionAction);
+                $hasPermissiveAccessToBook = !$book->restricted;
+
+                $chapter = $this->getChapter($entity->chapter_id);
+                $hasExplicitAccessToChapter = $chapter && $chapter->hasActiveRestriction($role->id, $restrictionAction);
+                $hasPermissiveAccessToChapter = $chapter && !$chapter->restricted;
+                $acknowledgeChapter = ($chapter && $chapter->restricted);

 
                 $hasExplicitAccessToParents = $acknowledgeChapter ? $hasExplicitAccessToChapter : $hasExplicitAccessToBook;
                 $hasPermissiveAccessToParents = $acknowledgeChapter ? $hasPermissiveAccessToChapter : $hasPermissiveAccessToBook;
 
                 $hasExplicitAccessToParents = $acknowledgeChapter ? $hasExplicitAccessToChapter : $hasExplicitAccessToBook;
                 $hasPermissiveAccessToParents = $acknowledgeChapter ? $hasPermissiveAccessToChapter : $hasPermissiveAccessToBook;

diff --git a/resources/views/books/sort.blade.php b/resources/views/books/sort.blade.php
index 3849dfcfce5f0de8f3a0d019cd7d570a39fdcb58..984db0ce68e5e8c67e8b508a96b63ae25079cc7f 100644 (file)
--- a/resources/views/books/sort.blade.php
+++ b/resources/views/books/sort.blade.php
@@ -50,7 +50,7 @@
             var sortableOptions = {
                 group: 'serialization',
                 onDrop: function($item, container, _super) {
             var sortableOptions = {
                 group: 'serialization',
                 onDrop: function($item, container, _super) {

-                    var pageMap = buildPageMap();
+                    var pageMap = buildEntityMap();

                     $('#sort-tree-input').val(JSON.stringify(pageMap));
                     _super($item, container);
                 },
                     $('#sort-tree-input').val(JSON.stringify(pageMap));
                     _super($item, container);
                 },


@@ -74,29 +74,42 @@
                 $link.remove();
             });
 
                 $link.remove();
             });
 

-            function buildPageMap() {
-                var pageMap = [];
+            /**
+             * Build up a mapping of entities with their ordering and nesting.
+             * @returns {Array}
+             */
+            function buildEntityMap() {
+                var entityMap = [];

                 var $lists = $('.sort-list');
                 $lists.each(function(listIndex) {
                     var list = $(this);
                     var bookId = list.closest('[data-type="book"]').attr('data-id');
                 var $lists = $('.sort-list');
                 $lists.each(function(listIndex) {
                     var list = $(this);
                     var bookId = list.closest('[data-type="book"]').attr('data-id');

-                    var $childElements = list.find('[data-type="page"], [data-type="chapter"]');
-                    $childElements.each(function(childIndex) {
+                    var $directChildren = list.find('> [data-type="page"], > [data-type="chapter"]');
+                    $directChildren.each(function(directChildIndex) {

                         var $childElem = $(this);
                         var type = $childElem.attr('data-type');
                         var parentChapter = false;
                         var $childElem = $(this);
                         var type = $childElem.attr('data-type');
                         var parentChapter = false;

-                        if(type === 'page' && $childElem.closest('[data-type="chapter"]').length === 1) {
-                            parentChapter = $childElem.closest('[data-type="chapter"]').attr('data-id');
-                        }
-                        pageMap.push({
-                            id: $childElem.attr('data-id'),
+                        var childId = $childElem.attr('data-id');
+                        entityMap.push({
+                            id: childId,
+                            sort: directChildIndex,

                             parentChapter: parentChapter,
                             type: type,
                             book: bookId
                         });
                             parentChapter: parentChapter,
                             type: type,
                             book: bookId
                         });

+                        $chapterChildren = $childElem.find('[data-type="page"]').each(function(pageIndex) {
+                            var $chapterChild = $(this);
+                            entityMap.push({
+                                id: $chapterChild.attr('data-id'),
+                                sort: pageIndex,
+                                parentChapter: childId,
+                                type: 'page',
+                                book: bookId
+                            });
+                        });

                     });
                 });
                     });
                 });

-                return pageMap;
+                return entityMap;

             }
 
         });
             }
 
         });