From cf41764c222d3ad820fb853ef814800f3b1bdbb7 Mon Sep 17 00:00:00 2001 From: nuxsmin Date: Wed, 25 Jul 2018 01:03:47 +0200 Subject: [PATCH] * [FIX] Upgraded database schema to enforce constraints in UserToUserGroup table * [ADD] Unit testing. Work in progress * [MOD] Code refactoring --- .../UserGroup/UserToUserGroupRepository.php | 26 +- .../Upgrade/UpgradeDatabaseService.php | 2 +- .../UserGroup/UserToUserGroupService.php | 20 +- schemas/30018072501.sql | 1 + schemas/dbstructure.sql | 1 + .../UserToUserGroupRepositoryTest.php | 231 ++++++++++++++++++ .../UserGroup/UserToUserGroupServiceTest.php | 192 +++++++++++++++ test/res/datasets/syspass_userGroup.xml | 4 + 8 files changed, 458 insertions(+), 19 deletions(-) create mode 100644 schemas/30018072501.sql create mode 100644 test/SP/Repositories/UserToUserGroupRepositoryTest.php create mode 100644 test/SP/Services/UserGroup/UserToUserGroupServiceTest.php diff --git a/lib/SP/Repositories/UserGroup/UserToUserGroupRepository.php b/lib/SP/Repositories/UserGroup/UserToUserGroupRepository.php index 08dae784..34b0779a 100644 --- a/lib/SP/Repositories/UserGroup/UserToUserGroupRepository.php +++ b/lib/SP/Repositories/UserGroup/UserToUserGroupRepository.php @@ -62,7 +62,7 @@ class UserToUserGroupRepository extends Repository * * @param $userId * - * @return array + * @return \SP\Storage\Database\QueryResult * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -72,7 +72,7 @@ class UserToUserGroupRepository extends Repository $queryData->setQuery('SELECT userGroupId FROM UserToUserGroup WHERE userId = ?'); $queryData->addParam($userId); - return $this->db->doSelect($queryData)->getDataAsArray(); + return $this->db->doSelect($queryData); } /** @@ -81,7 +81,7 @@ class UserToUserGroupRepository extends Repository * @param int $id * @param array $users * - * @return UserToUserGroupRepository + * @return int * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -96,7 +96,7 @@ class UserToUserGroupRepository extends Repository * * @param $id int * - * @return $this + * @return int * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -107,9 +107,7 @@ class UserToUserGroupRepository extends Repository $queryData->addParam($id); $queryData->setOnErrorMessage(__u('Error al eliminar los usuarios del grupo')); - $this->db->doQuery($queryData); - - return $this; + return $this->db->doQuery($queryData)->getAffectedNumRows(); } /** @@ -118,12 +116,16 @@ class UserToUserGroupRepository extends Repository * @param int $groupId * @param array $users * - * @return UserToUserGroupRepository + * @return int * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ public function add($groupId, array $users) { + if (empty($users)) { + return 0; + } + $query = /** @lang SQL */ 'INSERT INTO UserToUserGroup (userId, userGroupId) VALUES ' . $this->getParamsFromArray($users, '(?,?)'); @@ -137,9 +139,7 @@ class UserToUserGroupRepository extends Repository $queryData->setOnErrorMessage(__u('Error al asignar los usuarios al grupo')); - $this->db->doQuery($queryData); - - return $this; + return $this->db->doQuery($queryData)->getAffectedNumRows(); } /** @@ -147,7 +147,7 @@ class UserToUserGroupRepository extends Repository * * @param $id int * - * @return UserToUserGroupData[] + * @return \SP\Storage\Database\QueryResult * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -158,6 +158,6 @@ class UserToUserGroupRepository extends Repository $queryData->setQuery('SELECT userGroupId, userId FROM UserToUserGroup WHERE userGroupId = ?'); $queryData->addParam($id); - return $this->db->doSelect($queryData)->getDataAsArray(); + return $this->db->doSelect($queryData); } } \ No newline at end of file diff --git a/lib/SP/Services/Upgrade/UpgradeDatabaseService.php b/lib/SP/Services/Upgrade/UpgradeDatabaseService.php index 0a4fa68e..64f400a8 100644 --- a/lib/SP/Services/Upgrade/UpgradeDatabaseService.php +++ b/lib/SP/Services/Upgrade/UpgradeDatabaseService.php @@ -45,7 +45,7 @@ class UpgradeDatabaseService extends Service implements UpgradeInterface /** * @var array Versiones actualizables */ - const UPGRADES = ['300.18010101', '300.18072302']; + const UPGRADES = ['300.18010101', '300.18072302', '300.18072501']; /** * @var Database diff --git a/lib/SP/Services/UserGroup/UserToUserGroupService.php b/lib/SP/Services/UserGroup/UserToUserGroupService.php index 5b0ed605..b30a590f 100644 --- a/lib/SP/Services/UserGroup/UserToUserGroupService.php +++ b/lib/SP/Services/UserGroup/UserToUserGroupService.php @@ -24,6 +24,8 @@ namespace SP\Services\UserGroup; +use SP\DataModel\UserToUserGroupData; +use SP\Repositories\NoSuchItemException; use SP\Repositories\UserGroup\UserToUserGroupRepository; use SP\Services\Service; @@ -43,19 +45,26 @@ class UserToUserGroupService extends Service * @param $id * * @return \SP\DataModel\UserToUserGroupData[] + * @throws NoSuchItemException * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ public function getById($id) { - return $this->userToUserGroupRepository->getById($id); + $result = $this->userToUserGroupRepository->getById($id); + + if ($result->getNumRows() === 0) { + throw new NoSuchItemException(__u('Grupo no encontrado'), NoSuchItemException::INFO); + } + + return $result->getDataAsArray(); } /** * @param $id * @param array $users * - * @return UserToUserGroupRepository + * @return int * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -68,7 +77,7 @@ class UserToUserGroupService extends Service * @param int $id * @param array $users * - * @return UserToUserGroupRepository + * @return int * @throws \SP\Core\Exceptions\ConstraintException * @throws \SP\Core\Exceptions\QueryException */ @@ -92,7 +101,8 @@ class UserToUserGroupService extends Service { $usersId = []; - foreach ($this->userToUserGroupRepository->getById($id) as $userToUserGroupData) { + /** @var UserToUserGroupData $userToUserGroupData */ + foreach ($this->userToUserGroupRepository->getById($id)->getDataAsArray() as $userToUserGroupData) { $usersId[] = $userToUserGroupData->getUserId(); } @@ -125,7 +135,7 @@ class UserToUserGroupService extends Service */ public function getGroupsForUser($userId) { - return $this->userToUserGroupRepository->getGroupsForUser($userId); + return $this->userToUserGroupRepository->getGroupsForUser($userId)->getDataAsArray(); } /** diff --git a/schemas/30018072501.sql b/schemas/30018072501.sql new file mode 100644 index 00000000..dab85aed --- /dev/null +++ b/schemas/30018072501.sql @@ -0,0 +1 @@ +ALTER TABLE `UserToUserGroup` ADD CONSTRAINT `uk_UserToUserGroup_01` UNIQUE (`userId`, `userGroupId`); \ No newline at end of file diff --git a/schemas/dbstructure.sql b/schemas/dbstructure.sql index 718ec1a6..638edaec 100644 --- a/schemas/dbstructure.sql +++ b/schemas/dbstructure.sql @@ -519,6 +519,7 @@ CREATE TABLE `UserToUserGroup` ( `userGroupId` smallint(5) unsigned NOT NULL, KEY `idx_UserToUserGroup_01` (`userId`), KEY `fk_UserToGroup_userGroupId` (`userGroupId`), + UNIQUE KEY `uk_UserToUserGroup_01` (`userId`, `userGroupId`), CONSTRAINT `fk_UserToGroup_userGroupId` FOREIGN KEY (`userGroupId`) REFERENCES `UserGroup` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, diff --git a/test/SP/Repositories/UserToUserGroupRepositoryTest.php b/test/SP/Repositories/UserToUserGroupRepositoryTest.php new file mode 100644 index 00000000..2907cd5d --- /dev/null +++ b/test/SP/Repositories/UserToUserGroupRepositoryTest.php @@ -0,0 +1,231 @@ +. + */ + +namespace SP\Tests\SP\Repositories; + +use SP\Core\Exceptions\ConstraintException; +use SP\DataModel\UserToUserGroupData; +use SP\Repositories\UserGroup\UserToUserGroupRepository; +use SP\Storage\Database\DatabaseConnectionData; +use SP\Test\DatabaseTestCase; +use function SP\Test\setupContext; + +/** + * Class UserToUserGroupRepositoryTest + * + * @package SP\Tests\SP\Repositories + */ +class UserToUserGroupRepositoryTest extends DatabaseTestCase +{ + /** + * @var UserToUserGroupRepository + */ + private static $repository; + + /** + * @throws \DI\DependencyException + * @throws \DI\NotFoundException + * @throws \SP\Core\Context\ContextException + */ + public static function setUpBeforeClass() + { + $dic = setupContext(); + + self::$dataset = 'syspass_userGroup.xml'; + + // Datos de conexión a la BBDD + self::$databaseConnectionData = $dic->get(DatabaseConnectionData::class); + + // Inicializar el repositorio + self::$repository = $dic->get(UserToUserGroupRepository::class); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testGetGroupsForUser() + { + $result = self::$repository->getGroupsForUser(3); + + $this->assertEquals(1, $result->getNumRows()); + + $data = $result->getDataAsArray(); + + $this->assertCount(1, $data); + $this->assertEquals(2, $data[0]->userGroupId); + + $result = self::$repository->getGroupsForUser(2); + + $this->assertEquals(2, $result->getNumRows()); + + $data = $result->getDataAsArray(); + + $this->assertCount(2, $data); + $this->assertEquals(1, $data[0]->userGroupId); + $this->assertEquals(3, $data[1]->userGroupId); + + $this->assertEquals(0, self::$repository->getGroupsForUser(10)->getNumRows()); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testUpdate() + { + $data = [3, 4]; + + self::$repository->update(1, $data); + + $result = self::$repository->getById(1); + + $this->assertEquals(2, $result->getNumRows()); + + /** @var UserToUserGroupData[] $data */ + $data = $result->getDataAsArray(); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[0]); + $this->assertEquals(1, $data[0]->getUserGroupId()); + $this->assertEquals(3, $data[0]->getUserId()); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[1]); + $this->assertEquals(1, $data[1]->getUserGroupId()); + $this->assertEquals(4, $data[1]->getUserId()); + + $this->expectException(ConstraintException::class); + + self::$repository->update(10, [3, 4]); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testGetById() + { + $result = self::$repository->getById(2); + + $this->assertEquals(2, $result->getNumRows()); + + /** @var UserToUserGroupData[] $data */ + $data = $result->getDataAsArray(); + + $this->assertCount(2, $data); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[0]); + $this->assertEquals(2, $data[0]->getUserGroupId()); + $this->assertEquals(1, $data[0]->getUserId()); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[1]); + $this->assertEquals(2, $data[1]->getUserGroupId()); + $this->assertEquals(3, $data[1]->getUserId()); + + $data = self::$repository->getById(3)->getDataAsArray(); + + $this->assertCount(1, $data); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[0]); + $this->assertEquals(3, $data[0]->getUserGroupId()); + $this->assertEquals(2, $data[0]->getUserId()); + + $this->assertEquals(0, self::$repository->getById(10)->getNumRows()); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testDelete() + { + $this->assertEquals(1, self::$repository->delete(1)); + + $this->assertEquals(2, self::$repository->delete(2)); + + $this->assertEquals(0, self::$repository->delete(10)); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testAdd() + { + $data = [3, 4]; + + self::$repository->add(1, $data); + + $result = self::$repository->getById(1); + + $this->assertEquals(3, $result->getNumRows()); + + /** @var UserToUserGroupData[] $data */ + $data = $result->getDataAsArray(); + + $this->assertCount(3, $data); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[0]); + + $this->assertEquals(1, $data[0]->getUserGroupId()); + $this->assertEquals(2, $data[0]->getUserId()); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[1]); + $this->assertEquals(1, $data[1]->getUserGroupId()); + $this->assertEquals(3, $data[1]->getUserId()); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[1]); + $this->assertEquals(1, $data[2]->getUserGroupId()); + $this->assertEquals(4, $data[2]->getUserId()); + + $this->expectException(ConstraintException::class); + + self::$repository->add(10, [3, 4]); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testAddDuplicated() + { + $data = [2, 3, 4]; + + $this->expectException(ConstraintException::class); + + self::$repository->add(1, $data); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testCheckUserInGroup() + { + $this->assertTrue(self::$repository->checkUserInGroup(1, 2)); + + $this->assertTrue(self::$repository->checkUserInGroup(2, 3)); + + $this->assertFalse(self::$repository->checkUserInGroup(3, 3)); + } +} diff --git a/test/SP/Services/UserGroup/UserToUserGroupServiceTest.php b/test/SP/Services/UserGroup/UserToUserGroupServiceTest.php new file mode 100644 index 00000000..4e43c341 --- /dev/null +++ b/test/SP/Services/UserGroup/UserToUserGroupServiceTest.php @@ -0,0 +1,192 @@ +. + */ + +namespace SP\Tests\SP\Services\UserGroup; + +use SP\Core\Exceptions\ConstraintException; +use SP\DataModel\UserToUserGroupData; +use SP\Repositories\NoSuchItemException; +use SP\Services\UserGroup\UserToUserGroupService; +use SP\Storage\Database\DatabaseConnectionData; +use SP\Test\DatabaseTestCase; +use function SP\Test\setupContext; + +/** + * Class UserToUserGroupServiceTest + * + * @package SP\Tests\SP\Services\UserGroup + */ +class UserToUserGroupServiceTest extends DatabaseTestCase +{ + + /** + * @var UserToUserGroupService + */ + private static $service; + + /** + * @throws \DI\NotFoundException + * @throws \SP\Core\Context\ContextException + * @throws \DI\DependencyException + * @throws \SP\Core\Exceptions\SPException + */ + public static function setUpBeforeClass() + { + $dic = setupContext(); + + self::$dataset = 'syspass_userGroup.xml'; + + // Datos de conexión a la BBDD + self::$databaseConnectionData = $dic->get(DatabaseConnectionData::class); + + // Inicializar el servicio + self::$service = $dic->get(UserToUserGroupService::class); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testCheckUserInGroup() + { + $this->assertTrue(self::$service->checkUserInGroup(1, 2)); + + $this->assertTrue(self::$service->checkUserInGroup(2, 3)); + + $this->assertFalse(self::$service->checkUserInGroup(3, 3)); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testGetGroupsForUser() + { + $data = self::$service->getGroupsForUser(3); + + $this->assertCount(1, $data); + $this->assertEquals(2, $data[0]->userGroupId); + + $data = self::$service->getGroupsForUser(2); + + $this->assertCount(2, $data); + $this->assertEquals(1, $data[0]->userGroupId); + $this->assertEquals(3, $data[1]->userGroupId); + + $data = self::$service->getGroupsForUser(10); + + $this->assertCount(0, $data); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testAdd() + { + $data = [3, 4]; + + self::$service->add(1, $data); + + $this->assertEquals([2, 3, 4], self::$service->getUsersByGroupId(1)); + + $this->expectException(ConstraintException::class); + + self::$service->add(10, $data); + } + + /** + * @throws \SP\Core\Exceptions\ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testAddDuplicated() + { + $data = [2, 3, 4]; + + $this->expectException(ConstraintException::class); + + self::$service->add(1, $data); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testUpdate() + { + $data = [3, 4]; + + self::$service->update(1, $data); + + $this->assertEquals($data, self::$service->getUsersByGroupId(1)); + + $this->expectException(ConstraintException::class); + + self::$service->update(10, $data); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + * @throws \SP\Repositories\NoSuchItemException + */ + public function testGetById() + { + $data = self::$service->getById(2); + + $this->assertCount(2, $data); + + $this->assertInstanceOf(UserToUserGroupData::class, $data[0]); + + $this->assertEquals(2, $data[0]->getUserGroupId()); + $this->assertEquals(1, $data[0]->getUserId()); + + $this->assertEquals(2, $data[1]->getUserGroupId()); + $this->assertEquals(3, $data[1]->getUserId()); + + $data = self::$service->getById(3); + + $this->assertCount(1, $data); + + $this->assertEquals(3, $data[0]->getUserGroupId()); + $this->assertEquals(2, $data[0]->getUserId()); + + $this->expectException(NoSuchItemException::class); + + self::$service->getById(10); + } + + /** + * @throws ConstraintException + * @throws \SP\Core\Exceptions\QueryException + */ + public function testGetUsersByGroupId() + { + $data = self::$service->getUsersByGroupId(2); + + $this->assertCount(2, $data); + + $this->assertEquals([1, 3], $data); + } +} diff --git a/test/res/datasets/syspass_userGroup.xml b/test/res/datasets/syspass_userGroup.xml index 420c837f..207598ba 100644 --- a/test/res/datasets/syspass_userGroup.xml +++ b/test/res/datasets/syspass_userGroup.xml @@ -81,6 +81,10 @@ 2 1 + + 2 + 3 + \ No newline at end of file