Made more efficiency improvements to permission system

diff --git a/app/Console/Commands/RegeneratePermissions.php b/app/Console/Commands/RegeneratePermissions.php
index 1dc25f9aa6e996f4a8db898b513dfe08207f83a3..9cd577a17864b5c3c759f24ae2a07e7fdb6f4f44 100644 (file)
--- a/app/Console/Commands/RegeneratePermissions.php
+++ b/app/Console/Commands/RegeneratePermissions.php
@@ -49,6 +49,7 @@ class RegeneratePermissions extends Command
         $connection = \DB::getDefaultConnection();
         if ($this->option('database') !== null) {
             \DB::setDefaultConnection($this->option('database'));
         $connection = \DB::getDefaultConnection();
         if ($this->option('database') !== null) {
             \DB::setDefaultConnection($this->option('database'));

+            $this->permissionService->setConnection(\DB::connection($this->option('database')));

         }
 
         $this->permissionService->buildJointPermissions();
         }
 
         $this->permissionService->buildJointPermissions();

diff --git a/app/Console/Commands/RegenerateSearch.php b/app/Console/Commands/RegenerateSearch.php
index 35ecd46c0817ddac6e201f161dec73068e9085d6..1757911a7ce1226063bf91098302d74b9552b722 100644 (file)
--- a/app/Console/Commands/RegenerateSearch.php
+++ b/app/Console/Commands/RegenerateSearch.php
@@ -44,6 +44,7 @@ class RegenerateSearch extends Command
         $connection = \DB::getDefaultConnection();
         if ($this->option('database') !== null) {
             \DB::setDefaultConnection($this->option('database'));
         $connection = \DB::getDefaultConnection();
         if ($this->option('database') !== null) {
             \DB::setDefaultConnection($this->option('database'));

+            $this->searchService->setConnection(\DB::connection($this->option('database')));

         }
 
         $this->searchService->indexAllEntities();
         }
 
         $this->searchService->indexAllEntities();

diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php
index 35e5640d63ccffce6c51aac1ac8a0b9392abed22..83905ec44df4928a0d761294f48382cd4dc8e5f3 100644 (file)
--- a/app/Services/PermissionService.php
+++ b/app/Services/PermissionService.php
@@ -11,8 +11,8 @@ use BookStack\Role;
 use BookStack\User;
 use Illuminate\Database\Connection;
 use Illuminate\Database\Eloquent\Builder;
 use BookStack\User;
 use Illuminate\Database\Connection;
 use Illuminate\Database\Eloquent\Builder;

+use Illuminate\Database\Query\Builder as QueryBuilder;

 use Illuminate\Support\Collection;
 use Illuminate\Support\Collection;

-use Illuminate\Support\Facades\Log;

 
 class PermissionService
 {
 
 class PermissionService
 {


@@ -56,6 +56,15 @@ class PermissionService
         // TODO - Update so admin still goes through filters
     }
 
         // TODO - Update so admin still goes through filters
     }
 

+    /**
+     * Set the database connection
+     * @param Connection $connection
+     */
+    public function setConnection(Connection $connection)
+    {
+        $this->db = $connection;
+    }
+

     /**
      * Prepare the local entity cache and ensure it's empty
      */
     /**
      * Prepare the local entity cache and ensure it's empty
      */


@@ -138,10 +147,14 @@ class PermissionService
         $this->readyEntityCache();
 
         // Get all roles (Should be the most limited dimension)
         $this->readyEntityCache();
 
         // Get all roles (Should be the most limited dimension)

-        $roles = $this->role->with('permissions')->get();
+        $roles = $this->role->with('permissions')->get()->all();

 
         // Chunk through all books
 
         // Chunk through all books

-        $this->book->newQuery()->with('chapters', 'pages')->chunk(5, function ($books) use ($roles) {
+        $this->book->newQuery()->select(['id', 'restricted', 'created_by'])->with(['chapters' => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id']);
+        }, 'pages'  => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id', 'chapter_id']);
+        }])->chunk(5, function ($books) use ($roles) {

             $this->buildJointPermissionsForBooks($books, $roles);
         });
     }
             $this->buildJointPermissionsForBooks($books, $roles);
         });
     }


@@ -149,17 +162,18 @@ class PermissionService
     /**
      * Build joint permissions for an array of books
      * @param Collection $books
     /**
      * Build joint permissions for an array of books
      * @param Collection $books

-     * @param Collection $roles
+     * @param array $roles

      * @param bool $deleteOld
      */
     protected function buildJointPermissionsForBooks($books, $roles, $deleteOld = false) {
         $entities = clone $books;
 
      * @param bool $deleteOld
      */
     protected function buildJointPermissionsForBooks($books, $roles, $deleteOld = false) {
         $entities = clone $books;
 

-        foreach ($books as $book) {
-            foreach ($book->chapters as $chapter) {
+        /** @var Book $book */
+        foreach ($books->all() as $book) {
+            foreach ($book->getRelation('chapters') as $chapter) {

                 $entities->push($chapter);
             }
                 $entities->push($chapter);
             }

-            foreach ($book->pages as $page) {
+            foreach ($book->getRelation('pages') as $page) {

                 $entities->push($page);
             }
         }
                 $entities->push($page);
             }
         }


@@ -176,7 +190,12 @@ class PermissionService
     {
         $roles = $this->role->newQuery()->get();
         $book = ($entity->isA('book')) ? $entity : $entity->book;
     {
         $roles = $this->role->newQuery()->get();
         $book = ($entity->isA('book')) ? $entity : $entity->book;

-        $this->buildJointPermissionsForBooks(collect([$book]), $roles, true);
+        $book = $this->book->newQuery()->select(['id', 'restricted', 'created_by'])->with(['chapters' => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id']);
+        }, 'pages'  => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id', 'chapter_id']);
+        }])->where('id', '=', $book->id)->get();
+        $this->buildJointPermissionsForBooks($book, $roles, true);

     }
 
     /**
     }
 
     /**


@@ -196,12 +215,15 @@ class PermissionService
      */
     public function buildJointPermissionForRole(Role $role)
     {
      */
     public function buildJointPermissionForRole(Role $role)
     {

-        $roles = collect([$role]);
-
+        $roles = [$role];

         $this->deleteManyJointPermissionsForRoles($roles);
 
         // Chunk through all books
         $this->deleteManyJointPermissionsForRoles($roles);
 
         // Chunk through all books

-        $this->book->with('chapters', 'pages')->chunk(5, function ($books) use ($roles) {
+        $this->book->newQuery()->select(['id', 'restricted', 'created_by'])->with(['chapters' => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id']);
+        }, 'pages'  => function($query) {
+            $query->select(['id', 'restricted', 'created_by', 'book_id', 'chapter_id']);
+        }])->chunk(5, function ($books) use ($roles) {

             $this->buildJointPermissionsForBooks($books, $roles);
         });
     }
             $this->buildJointPermissionsForBooks($books, $roles);
         });
     }


@@ -221,9 +243,10 @@ class PermissionService
      */
     protected function deleteManyJointPermissionsForRoles($roles)
     {
      */
     protected function deleteManyJointPermissionsForRoles($roles)
     {

-        foreach ($roles as $role) {
-            $role->jointPermissions()->delete();
-        }
+        $roleIds = array_map(function($role) {
+            return $role->id;
+        }, $roles);
+        $this->jointPermission->newQuery()->whereIn('id', $roleIds)->delete();

     }
 
     /**
     }
 
     /**


@@ -242,22 +265,27 @@ class PermissionService
     protected function deleteManyJointPermissionsForEntities($entities)
     {
         if (count($entities) === 0) return;
     protected function deleteManyJointPermissionsForEntities($entities)
     {
         if (count($entities) === 0) return;

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

 
 

-        foreach ($entities as $entity) {
-            $query->orWhere(function($query) use ($entity) {
-                $query->where('entity_id', '=', $entity->id)
-                    ->where('entity_type', '=', $entity->getMorphClass());
-            });
-        }
+        $this->db->transaction(function() use ($entities) {
+
+            foreach (array_chunk($entities, 1000) as $entityChunk) {
+                $query = $this->db->table('joint_permissions');
+                foreach ($entityChunk as $entity) {
+                    $query->orWhere(function(QueryBuilder $query) use ($entity) {
+                        $query->where('entity_id', '=', $entity->id)
+                            ->where('entity_type', '=', $entity->getMorphClass());
+                    });
+                }
+                $query->delete();
+            }

 
 

-        $query->delete();
+        });

     }
 
     /**
      * Create & Save entity jointPermissions for many entities and jointPermissions.
      * @param Collection $entities
     }
 
     /**
      * Create & Save entity jointPermissions for many entities and jointPermissions.
      * @param Collection $entities

-     * @param Collection $roles
+     * @param array $roles

      */
     protected function createManyJointPermissions($entities, $roles)
     {
      */
     protected function createManyJointPermissions($entities, $roles)
     {


@@ -299,9 +327,12 @@ class PermissionService
                 }
             }
         }
                 }
             }
         }

-        foreach (array_chunk($jointPermissions, 5000) as $jointPermissionChunk) {
-            $this->jointPermission->insert($jointPermissionChunk);
-        }
+
+        $this->db->transaction(function() use ($jointPermissions) {
+            foreach (array_chunk($jointPermissions, 1000) as $jointPermissionChunk) {
+                $this->db->table('joint_permissions')->insert($jointPermissionChunk);
+            }
+        });

     }
 
 
     }
 
 


@@ -494,7 +525,7 @@ class PermissionService
      * @param integer $book_id
      * @param bool $filterDrafts
      * @param bool $fetchPageContent
      * @param integer $book_id
      * @param bool $filterDrafts
      * @param bool $fetchPageContent

-     * @return \Illuminate\Database\Query\Builder
+     * @return QueryBuilder

      */
     public function bookChildrenQuery($book_id, $filterDrafts = false, $fetchPageContent = false) {
         $pageSelect = $this->db->table('pages')->selectRaw($this->page->entityRawQuery($fetchPageContent))->where('book_id', '=', $book_id)->where(function($query) use ($filterDrafts) {
      */
     public function bookChildrenQuery($book_id, $filterDrafts = false, $fetchPageContent = false) {
         $pageSelect = $this->db->table('pages')->selectRaw($this->page->entityRawQuery($fetchPageContent))->where('book_id', '=', $book_id)->where(function($query) use ($filterDrafts) {

diff --git a/app/Services/SearchService.php b/app/Services/SearchService.php
index 670c1545d263a1d9995d5f87fbede79616b7a5a1..3d1d45c3b77d47ab356285f0f45ffa9c58b90115 100644 (file)
--- a/app/Services/SearchService.php
+++ b/app/Services/SearchService.php
@@ -50,6 +50,15 @@ class SearchService
         $this->permissionService = $permissionService;
     }
 
         $this->permissionService = $permissionService;
     }
 

+    /**
+     * Set the database connection
+     * @param Connection $connection
+     */
+    public function setConnection(Connection $connection)
+    {
+        $this->db = $connection;
+    }
+

     /**
      * Search all entities in the system.
      * @param string $searchString
     /**
      * Search all entities in the system.
      * @param string $searchString

diff --git a/tests/BrowserKitTest.php b/tests/BrowserKitTest.php
index f8d60239db9f545058802787423d6c10a5a1c474..c665bfc231453308a0cd18d33752b834f590b71b 100644 (file)
--- a/tests/BrowserKitTest.php
+++ b/tests/BrowserKitTest.php
@@ -1,6 +1,7 @@
 <?php namespace Tests;
 
 use BookStack\Role;
 <?php namespace Tests;
 
 use BookStack\Role;

+use BookStack\Services\PermissionService;

 use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Laravel\BrowserKitTesting\TestCase;
 use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Laravel\BrowserKitTesting\TestCase;


@@ -105,11 +106,9 @@ abstract class BrowserKitTest extends TestCase
     {
         if ($updaterUser === false) $updaterUser = $creatorUser;
         $book = factory(\BookStack\Book::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]);
     {
         if ($updaterUser === false) $updaterUser = $creatorUser;
         $book = factory(\BookStack\Book::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]);

-        $chapter = factory(\BookStack\Chapter::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]);
-        $page = factory(\BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]);
-        $book->chapters()->saveMany([$chapter]);
-        $chapter->pages()->saveMany([$page]);
-        $restrictionService = $this->app[\BookStack\Services\PermissionService::class];
+        $chapter = factory(\BookStack\Chapter::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]);
+        $page = factory(\BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id, 'chapter_id' => $chapter->id]);
+        $restrictionService = $this->app[PermissionService::class];

         $restrictionService->buildJointPermissionsForEntity($book);
         return [
             'book' => $book,
         $restrictionService->buildJointPermissionsForEntity($book);
         return [
             'book' => $book,

diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php
index 24b8ae0f5fbc4717ff7fdc92b83cb68c2f7a15f6..83d1b98a8c1395a627a8b35053e08429465b9c19 100644 (file)
--- a/tests/Permissions/RolesTest.php
+++ b/tests/Permissions/RolesTest.php
@@ -1,5 +1,8 @@
 <?php namespace Tests;
 
 <?php namespace Tests;
 

+use BookStack\Repos\PermissionsRepo;
+use BookStack\Role;
+

 class RolesTest extends BrowserKitTest
 {
     protected $user;
 class RolesTest extends BrowserKitTest
 {
     protected $user;


@@ -34,11 +37,11 @@ class RolesTest extends BrowserKitTest
     /**
      * Create a new basic role for testing purposes.
      * @param array $permissions
     /**
      * Create a new basic role for testing purposes.
      * @param array $permissions

-     * @return static
+     * @return Role

      */
     protected function createNewRole($permissions = [])
     {
      */
     protected function createNewRole($permissions = [])
     {

-        $permissionRepo = app('BookStack\Repos\PermissionsRepo');
+        $permissionRepo = app(PermissionsRepo::class);

         $roleData = factory(\BookStack\Role::class)->make()->toArray();
         $roleData['permissions'] = array_flip($permissions);
         return $permissionRepo->saveNewRole($roleData);
         $roleData = factory(\BookStack\Role::class)->make()->toArray();
         $roleData['permissions'] = array_flip($permissions);
         return $permissionRepo->saveNewRole($roleData);


@@ -107,16 +110,16 @@ class RolesTest extends BrowserKitTest
 
     public function test_manage_user_permission()
     {
 
     public function test_manage_user_permission()
     {

-        $this->actingAs($this->user)->visit('/')->visit('/settings/users')
+        $this->actingAs($this->user)->visit('/settings/users')

             ->seePageIs('/');
         $this->giveUserPermissions($this->user, ['users-manage']);
             ->seePageIs('/');
         $this->giveUserPermissions($this->user, ['users-manage']);

-        $this->actingAs($this->user)->visit('/')->visit('/settings/users')
+        $this->actingAs($this->user)->visit('/settings/users')

             ->seePageIs('/settings/users');
     }
 
     public function test_user_roles_manage_permission()
     {
             ->seePageIs('/settings/users');
     }
 
     public function test_user_roles_manage_permission()
     {

-        $this->actingAs($this->user)->visit('/')->visit('/settings/roles')
+        $this->actingAs($this->user)->visit('/settings/roles')

             ->seePageIs('/')->visit('/settings/roles/1')->seePageIs('/');
         $this->giveUserPermissions($this->user, ['user-roles-manage']);
         $this->actingAs($this->user)->visit('/settings/roles')
             ->seePageIs('/')->visit('/settings/roles/1')->seePageIs('/');
         $this->giveUserPermissions($this->user, ['user-roles-manage']);
         $this->actingAs($this->user)->visit('/settings/roles')


@@ -126,10 +129,10 @@ class RolesTest extends BrowserKitTest
 
     public function test_settings_manage_permission()
     {
 
     public function test_settings_manage_permission()
     {

-        $this->actingAs($this->user)->visit('/')->visit('/settings')
+        $this->actingAs($this->user)->visit('/settings')

             ->seePageIs('/');
         $this->giveUserPermissions($this->user, ['settings-manage']);
             ->seePageIs('/');
         $this->giveUserPermissions($this->user, ['settings-manage']);

-        $this->actingAs($this->user)->visit('/')->visit('/settings')
+        $this->actingAs($this->user)->visit('/settings')

             ->seePageIs('/settings')->press('Save Settings')->see('Settings Saved');
     }
 
             ->seePageIs('/settings')->press('Save Settings')->see('Settings Saved');
     }
 


@@ -181,27 +184,26 @@ class RolesTest extends BrowserKitTest
      * @param string $permission
      * @param array $accessUrls Urls that are only accessible after having the permission
      * @param array $visibles Check this text, In the buttons toolbar, is only visible with the permission
      * @param string $permission
      * @param array $accessUrls Urls that are only accessible after having the permission
      * @param array $visibles Check this text, In the buttons toolbar, is only visible with the permission

-     * @param null $callback

      */
     private function checkAccessPermission($permission, $accessUrls = [], $visibles = [])
     {
         foreach ($accessUrls as $url) {
      */
     private function checkAccessPermission($permission, $accessUrls = [], $visibles = [])
     {
         foreach ($accessUrls as $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->seePageIs('/');
         }
         foreach ($visibles as $url => $text) {
                 ->seePageIs('/');
         }
         foreach ($visibles as $url => $text) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->dontSeeInElement('.action-buttons',$text);
         }
 
         $this->giveUserPermissions($this->user, [$permission]);
 
         foreach ($accessUrls as $url) {
                 ->dontSeeInElement('.action-buttons',$text);
         }
 
         $this->giveUserPermissions($this->user, [$permission]);
 
         foreach ($accessUrls as $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->seePageIs($url);
         }
         foreach ($visibles as $url => $text) {
                 ->seePageIs($url);
         }
         foreach ($visibles as $url => $text) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->see($text);
         }
     }
                 ->see($text);
         }
     }


@@ -391,8 +393,8 @@ class RolesTest extends BrowserKitTest
 
     public function test_page_create_own_permissions()
     {
 
     public function test_page_create_own_permissions()
     {

-        $book = \BookStack\Book::take(1)->get()->first();
-        $chapter = \BookStack\Chapter::take(1)->get()->first();
+        $book = \BookStack\Book::first();
+        $chapter = \BookStack\Chapter::first();

 
         $entities = $this->createEntityChainBelongingToUser($this->user);
         $ownBook = $entities['book'];
 
         $entities = $this->createEntityChainBelongingToUser($this->user);
         $ownBook = $entities['book'];


@@ -405,7 +407,7 @@ class RolesTest extends BrowserKitTest
         $accessUrls = [$createUrl, $createUrlChapter];
 
         foreach ($accessUrls as $url) {
         $accessUrls = [$createUrl, $createUrlChapter];
 
         foreach ($accessUrls as $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->seePageIs('/');
         }
 
                 ->seePageIs('/');
         }
 


@@ -417,7 +419,7 @@ class RolesTest extends BrowserKitTest
         $this->giveUserPermissions($this->user, ['page-create-own']);
 
         foreach ($accessUrls as $index => $url) {
         $this->giveUserPermissions($this->user, ['page-create-own']);
 
         foreach ($accessUrls as $index => $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url);
+            $this->actingAs($this->user)->visit($url);

             $expectedUrl = \BookStack\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl();
             $this->seePageIs($expectedUrl);
         }
             $expectedUrl = \BookStack\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl();
             $this->seePageIs($expectedUrl);
         }


@@ -449,7 +451,7 @@ class RolesTest extends BrowserKitTest
         $accessUrls = [$createUrl, $createUrlChapter];
 
         foreach ($accessUrls as $url) {
         $accessUrls = [$createUrl, $createUrlChapter];
 
         foreach ($accessUrls as $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url)
+            $this->actingAs($this->user)->visit($url)

                 ->seePageIs('/');
         }
 
                 ->seePageIs('/');
         }
 


@@ -461,7 +463,7 @@ class RolesTest extends BrowserKitTest
         $this->giveUserPermissions($this->user, ['page-create-all']);
 
         foreach ($accessUrls as $index => $url) {
         $this->giveUserPermissions($this->user, ['page-create-all']);
 
         foreach ($accessUrls as $index => $url) {

-            $this->actingAs($this->user)->visit('/')->visit($url);
+            $this->actingAs($this->user)->visit($url);

             $expectedUrl = \BookStack\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl();
             $this->seePageIs($expectedUrl);
         }
             $expectedUrl = \BookStack\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl();
             $this->seePageIs($expectedUrl);
         }