From d915b9965351b5a2c8bcdedcaa01875601eeed94 Mon Sep 17 00:00:00 2001 From: Alexey Rogachev Date: Wed, 6 Apr 2022 00:57:02 +0600 Subject: [PATCH] Fix #17: Improve performance of SQL queries (#18) --- src/AssignmentsStorage.php | 16 +++++++++++----- src/Command/RbacCycleInit.php | 7 +------ src/ItemsStorage.php | 22 ++++++++++++---------- tests/ItemsStorageTest.php | 29 ++++++++++++++++------------- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/AssignmentsStorage.php b/src/AssignmentsStorage.php index a7b143c..18d4be9 100644 --- a/src/AssignmentsStorage.php +++ b/src/AssignmentsStorage.php @@ -69,10 +69,8 @@ public function get(string $itemName, string $userId): ?Assignment ->where(['itemName' => $itemName, 'userId' => $userId]) ->run() ->fetch(); - if (!empty($assignment)) { - return new Assignment($userId, $itemName, (int)$assignment['createdAt']); - } - return null; + + return empty($assignment) ? null : new Assignment($userId, $itemName, (int)$assignment['createdAt']); } /** @@ -97,7 +95,15 @@ public function add(string $itemName, string $userId): void */ public function hasItem(string $name): bool { - return $this->database->select('itemName')->from($this->tableName)->where(['itemName' => $name])->count() > 0; + $result = $this + ->database + ->select('1') + ->from($this->tableName) + ->where(['itemName' => $name]) + ->run() + ->fetch(); + + return $result !== false; } /** diff --git a/src/Command/RbacCycleInit.php b/src/Command/RbacCycleInit.php index 1c54072..bb9f24c 100644 --- a/src/Command/RbacCycleInit.php +++ b/src/Command/RbacCycleInit.php @@ -41,12 +41,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int { /** @var non-empty-string $itemsChildrenTable */ $itemsChildrenTable = $this->config['itemsChildrenTable'] ?? $this->config['itemsTable'] . '_child'; - $force = $input->getOption('force'); - if ($force === false) { - $reCreate = false; - } else { - $reCreate = true; - } + $reCreate = $input->getOption('force') !== false; /** @var Table $table */ if ($reCreate && $this->dbal->database()->hasTable($itemsChildrenTable) === true) { $this->dropTable($itemsChildrenTable); diff --git a/src/ItemsStorage.php b/src/ItemsStorage.php index 63299a9..8490964 100644 --- a/src/ItemsStorage.php +++ b/src/ItemsStorage.php @@ -66,10 +66,8 @@ public function getAll(): array public function get(string $name): ?Item { $item = $this->database->select()->from($this->tableName)->where(['name' => $name])->run()->fetch(); - if (!empty($item)) { - return $this->populateItem($item); - } - return null; + + return empty($item) ? null : $this->populateItem($item); } /** @@ -183,7 +181,15 @@ public function getChildren(string $name): array */ public function hasChildren(string $name): bool { - return $this->database->select('parent')->from($this->childrenTableName)->where(['parent' => $name])->count() > 0; + $result = $this + ->database + ->select('1') + ->from($this->childrenTableName) + ->where(['parent' => $name]) + ->run() + ->fetch(); + + return $result !== false; } /** @@ -229,11 +235,7 @@ private function getItemByTypeAndName(string $type, string $name): ?Item { $item = $this->database->select()->from($this->tableName)->where(['type' => $type, 'name' => $name])->run()->fetch(); - if (empty($item)) { - return null; - } - - return $this->populateItem($item); + return empty($item) ? null : $this->populateItem($item); } /** diff --git a/tests/ItemsStorageTest.php b/tests/ItemsStorageTest.php index 1c1a1e7..c3f9a7e 100644 --- a/tests/ItemsStorageTest.php +++ b/tests/ItemsStorageTest.php @@ -32,6 +32,7 @@ public function testGet(): void $this->assertInstanceOf(Permission::class, $item); $this->assertSame(Item::TYPE_PERMISSION, $item->getType()); + $this->assertSame('Parent 3', $item->getName()); } public function testGetPermission(): void @@ -40,6 +41,7 @@ public function testGetPermission(): void $permission = $storage->getPermission('Child 1'); $this->assertInstanceOf(Permission::class, $permission); + $this->assertSame('Child 1', $permission->getName()); } public function testAddChild(): void @@ -120,6 +122,7 @@ public function testGetRole(): void $this->assertNotEmpty($role); $this->assertInstanceOf(Role::class, $role); + $this->assertSame('Parent 1', $role->getName()); } public function testAdd(): void @@ -142,9 +145,8 @@ public function testRemoveChild(): void public function testGetAll(): void { $storage = $this->getStorage(); - $all = $storage->getAll(); - $this->assertCount(5, $all); + $this->assertCount(5, $storage->getAll()); } public function testHasChildren(): void @@ -173,36 +175,37 @@ public function testClearRoles(): void protected function populateDb(): void { + $time = time(); $items = [ [ 'name' => 'Parent 1', 'type' => Item::TYPE_ROLE, - 'createdAt' => time(), - 'updatedAt' => time(), + 'createdAt' => $time, + 'updatedAt' => $time, ], [ 'name' => 'Parent 2', 'type' => Item::TYPE_ROLE, - 'createdAt' => time(), - 'updatedAt' => time(), + 'createdAt' => $time, + 'updatedAt' => $time, ], [ 'name' => 'Parent 3', 'type' => Item::TYPE_PERMISSION, - 'createdAt' => time(), - 'updatedAt' => time(), + 'createdAt' => $time, + 'updatedAt' => $time, ], [ 'name' => 'Child 1', 'type' => Item::TYPE_PERMISSION, - 'createdAt' => time(), - 'updatedAt' => time(), + 'createdAt' => $time, + 'updatedAt' => $time, ], [ 'name' => 'Child 2', 'type' => Item::TYPE_ROLE, - 'createdAt' => time(), - 'updatedAt' => time(), + 'createdAt' => $time, + 'updatedAt' => $time, ], ]; $items_child = [ @@ -233,7 +236,7 @@ protected function populateDb(): void } } - private function getStorage() + private function getStorage(): ItemsStorage { return new ItemsStorage('auth_item', $this->getDbal()); }