From 02eecb3a3fa5c7b14cb4ee3843204b8038ed92c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 27 Mar 2017 20:10:18 +0200 Subject: [PATCH] Use mapper classes for relational data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/Board.php | 1 + lib/Db/BoardMapper.php | 40 ++++++++++++++++++- lib/Db/Card.php | 1 + lib/Db/CardMapper.php | 12 +++++- lib/Db/RelationalEntity.php | 19 ++++++--- lib/Service/BoardService.php | 53 +++++++------------------ lib/Service/CardService.php | 3 +- templates/part.boardlist.php | 7 +--- templates/part.card.php | 2 +- tests/unit/Db/AclMapperTest.php | 11 ++++- tests/unit/Db/BoardMapperTest.php | 11 ++++- tests/unit/Service/BoardServiceTest.php | 13 +----- 12 files changed, 106 insertions(+), 67 deletions(-) diff --git a/lib/Db/Board.php b/lib/Db/Board.php index 4d3b5e42d..868b54fde 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -52,6 +52,7 @@ class Board extends RelationalEntity implements JsonSerializable { if ($this->shared === -1) { unset($json['shared']); } + $json['owner'] = $this->resolveOwner(); return $json; } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 38652eceb..9d982070d 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -24,19 +24,31 @@ namespace OCA\Deck\Db; use OCP\IDBConnection; - +use OCP\IUserManager; +use OCP\IGroupManager; class BoardMapper extends DeckMapper implements IPermissionMapper { private $labelMapper; private $aclMapper; private $stackMapper; + private $userManager; + private $groupManager; - public function __construct(IDBConnection $db, LabelMapper $labelMapper, AclMapper $aclMapper, StackMapper $stackMapper) { + public function __construct( + IDBConnection $db, + LabelMapper $labelMapper, + AclMapper $aclMapper, + StackMapper $stackMapper, + IUserManager $userManager, + IGroupManager $groupManager + ) { parent::__construct($db, 'deck_boards', '\OCA\Deck\Db\Board'); $this->labelMapper = $labelMapper; $this->aclMapper = $aclMapper; $this->stackMapper = $stackMapper; + $this->userManager = $userManager; + $this->groupManager = $groupManager; } @@ -149,5 +161,29 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $id; } + public function mapAcl(Acl &$acl) { + $userManager = $this->userManager; + $groupManager = $this->groupManager; + $acl->resolveRelation('participant', function($participant) use (&$acl, &$userManager, &$groupManager) { + if($acl->getType() === Acl::PERMISSION_TYPE_USER) { + return new User($userManager->get($acl->getParticipant($participant))); + } + if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { + return new Group($groupManager->get($acl->getParticipant($participant))); + } + throw new \Exception('Unknown permission type for mapping Acl'); + }); + } + + /** + * @param Board $board + */ + public function mapOwner(Board &$board) { + $userManager = $this->userManager; + $board->resolveRelation('owner', function($owner) use (&$userManager) { + return new User($userManager->get($owner)); + }); + } + } \ No newline at end of file diff --git a/lib/Db/Card.php b/lib/Db/Card.php index bdbde9cff..53f73771d 100644 --- a/lib/Db/Card.php +++ b/lib/Db/Card.php @@ -48,6 +48,7 @@ class Card extends RelationalEntity implements JsonSerializable { $this->addType('createdAt', 'integer'); $this->addType('archived', 'boolean'); $this->addRelation('labels'); + $this->addResolvable('owner'); } } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 961d77866..1542a95ea 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -25,15 +25,17 @@ namespace OCA\Deck\Db; use OCP\AppFramework\Db\Entity; use OCP\IDBConnection; +use OCP\IUserManager; class CardMapper extends DeckMapper implements IPermissionMapper { private $labelMapper; - public function __construct(IDBConnection $db, LabelMapper $labelMapper) { + public function __construct(IDBConnection $db, LabelMapper $labelMapper, IUserManager $userManager) { parent::__construct($db, 'deck_cards', '\OCA\Deck\Db\Card'); $this->labelMapper = $labelMapper; + $this->userManager = $userManager; } public function insert(Entity $entity) { @@ -57,6 +59,7 @@ class CardMapper extends DeckMapper implements IPermissionMapper { $card = $this->findEntity($sql, [$id]); $labels = $this->labelMapper->findAssignedLabelsForCard($card->id); $card->setLabels($labels); + $this->mapOwner($card); return $card; } @@ -125,5 +128,12 @@ class CardMapper extends DeckMapper implements IPermissionMapper { return $row['id']; } + public function mapOwner(Card &$card) { + $userManager = $this->userManager; + $card->resolveRelation('owner', function($owner) use (&$userManager) { + return new User($userManager->get($owner)); + }); + } + } \ No newline at end of file diff --git a/lib/Db/RelationalEntity.php b/lib/Db/RelationalEntity.php index eebd4576a..80895c963 100644 --- a/lib/Db/RelationalEntity.php +++ b/lib/Db/RelationalEntity.php @@ -23,7 +23,9 @@ namespace OCA\Deck\Db; -class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSerializable { +use OCP\AppFramework\Db\Entity; + +class RelationalEntity extends Entity implements \JsonSerializable { private $_relations = array(); private $_resolvedProperties = []; @@ -73,9 +75,15 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria } } } + foreach ($this->_resolvedProperties as $property => $value) { + if($value !== null) { + $json[$property] = $value; + } + } return $json; } - /* + + /* * Resolve relational data from external methods * * example usage: @@ -83,7 +91,7 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria * in Board::__construct() * $this->addResolvable('owner') * - * in BoardService + * in BoardMapper * $board->resolveRelation('owner', function($owner) use (&$userManager) { * return new \OCA\Deck\Db\User($userManager->get($owner)); * }); @@ -91,8 +99,6 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria * resolved values can be obtained by calling resolveProperty * e.g. $board->resolveOwner() * - * TODO: Maybe move from callable to a custom Resolver class that can be reused and use DI? - * * @param string $property name of the property * @param callable $resolver anonymous function to resolve relational * data defined by $property as unique identifier @@ -126,7 +132,8 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria if(!is_scalar($args[0])) { $args[0] = $args[0]['primaryKey']; } - return parent::setter($attr, $args); + parent::setter($attr, $args); + return null; } return parent::__call($methodName, $args); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 36b6ad89a..5a8af4f88 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -25,18 +25,11 @@ namespace OCA\Deck\Service; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; -use OCA\Deck\Db\Group; use OCA\Deck\Db\Label; - - -use OCA\Deck\Db\User; -use OCP\IGroupManager; use OCP\IL10N; - use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\LabelMapper; -use OCP\IUserManager; class BoardService { @@ -47,14 +40,12 @@ class BoardService { private $l10n; private $permissionService; - public function __construct(BoardMapper $boardMapper, IL10N $l10n, LabelMapper $labelMapper, AclMapper $aclMapper, PermissionService $permissionService, IUserManager $userManager, IGroupManager $groupManager) { + public function __construct(BoardMapper $boardMapper, IL10N $l10n, LabelMapper $labelMapper, AclMapper $aclMapper, PermissionService $permissionService) { $this->boardMapper = $boardMapper; $this->labelMapper = $labelMapper; $this->aclMapper = $aclMapper; $this->l10n = $l10n; $this->permissionService = $permissionService; - $this->userManager = $userManager; - $this->groupManager = $groupManager; } public function findAll($userInfo) { @@ -64,10 +55,10 @@ class BoardService { $result = []; foreach($complete as &$item) { if(!array_key_exists($item->getId(), $result)) { - $item = $this->mapOwner($item); + $this->boardMapper->mapOwner($item); if($item->getAcl() !== null) { foreach ($item->getAcl() as &$acl) { - $acl = $this->mapAcl($acl); + $this->boardMapper->mapAcl($acl); } } $result[$item->getId()] = $item; @@ -78,37 +69,18 @@ class BoardService { public function find($boardId) { $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); + /** @var Board $board */ $board = $this->boardMapper->find($boardId, true, true); - $board = $this->mapOwner($board); + $this->boardMapper->mapOwner($board); foreach ($board->getAcl() as &$acl) { if($acl !== null) { - $this->mapAcl($acl); + $this->boardMapper->mapAcl($acl); } } return $board; } - private function mapAcl(Acl &$acl) { - $userManager = $this->userManager; - $groupManager = $this->groupManager; - $acl->resolveRelation('participant', function($participant) use (&$acl, &$userManager, &$groupManager) { - if($acl->getType() === Acl::PERMISSION_TYPE_USER) { - return new User($userManager->get($acl->getParticipant($participant))); - } - if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { - return new Group($groupManager->get($acl->getParticipant($participant))); - } - }); - return $acl; - } - private function mapOwner(Board $board) { - $userManager = $this->userManager; - $board->resolveRelation('owner', function($owner) use (&$userManager) { - return new User($userManager->get($owner)); - }); - return $board; - } public function create($title, $userId, $color) { $board = new Board(); @@ -133,6 +105,7 @@ class BoardService { $labels[] = $this->labelMapper->insert($label); } $new_board->setLabels($labels); + $this->boardMapper->mapOwner($new_board); return $new_board; } @@ -147,6 +120,7 @@ class BoardService { $board = $this->find($id); $board->setTitle($title); $board->setColor($color); + $this->boardMapper->mapOwner($board); return $this->boardMapper->update($board); } @@ -160,24 +134,27 @@ class BoardService { $acl->setPermissionEdit($edit); $acl->setPermissionShare($share); $acl->setPermissionManage($manage); - $this->mapAcl($acl); - return $this->aclMapper->insert($acl); + $newAcl = $this->aclMapper->insert($acl); + $this->boardMapper->mapAcl($newAcl); + return $newAcl; } public function updateAcl($id, $edit, $share, $manage) { $this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE); + /** @var Acl $acl */ $acl = $this->aclMapper->find($id); $acl->setPermissionEdit($edit); $acl->setPermissionShare($share); $acl->setPermissionManage($manage); - $this->mapAcl($acl); + $this->boardMapper->mapAcl($acl); return $this->aclMapper->update($acl); } public function deleteAcl($id) { $this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE); + /** @var Acl $acl */ $acl = $this->aclMapper->find($id); - $this->mapAcl($acl); + $this->boardMapper->mapAcl($acl); return $this->aclMapper->delete($acl); } diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index bc7437c5b..7fd7a6b36 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -42,7 +42,8 @@ class CardService { public function find($cardId) { $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); - return $this->cardMapper->find($cardId); + $card = $this->cardMapper->find($cardId); + return $card; } /** diff --git a/templates/part.boardlist.php b/templates/part.boardlist.php index 98ae6a08a..f52e41564 100644 --- a/templates/part.boardlist.php +++ b/templates/part.boardlist.php @@ -17,11 +17,8 @@ {{ b.title }}
-
-
+
+
diff --git a/templates/part.card.php b/templates/part.card.php index ec2548455..5be912ea4 100644 --- a/templates/part.card.php +++ b/templates/part.card.php @@ -26,7 +26,7 @@ t('Modified:')); ?> {{ cardservice.getCurrent().lastModified|relativeDateFilter }} t('Created:')); ?> {{ cardservice.getCurrent().createdAt|relativeDateFilter }} t('by')); ?> - {{ cardservice.getCurrent().owner }} + {{ cardservice.getCurrent().owner.displayname }}
diff --git a/tests/unit/Db/AclMapperTest.php b/tests/unit/Db/AclMapperTest.php index 76c307301..ca2ea6b39 100644 --- a/tests/unit/Db/AclMapperTest.php +++ b/tests/unit/Db/AclMapperTest.php @@ -23,6 +23,8 @@ namespace OCA\Deck\Db; +use OCP\IGroupManager; +use OCP\IUserManager; use Test\AppFramework\Db\MapperTestUtility; /** @@ -33,6 +35,8 @@ class AclMapperTest extends MapperTestUtility { private $dbConnection; private $aclMapper; private $boardMapper; + private $userManager; + private $groupManager; // Data private $acls; @@ -43,11 +47,16 @@ class AclMapperTest extends MapperTestUtility { $this->dbConnection = \OC::$server->getDatabaseConnection(); $this->aclMapper = new AclMapper($this->dbConnection); + $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->boardMapper = new BoardMapper( $this->dbConnection, \OC::$server->query(LabelMapper::class), $this->aclMapper, - \OC::$server->query(StackMapper::class)); + \OC::$server->query(StackMapper::class), + $this->userManager, + $this->groupManager + ); $this->boards = [ $this->boardMapper->insert($this->getBoard('MyBoard 1', 'user1')), diff --git a/tests/unit/Db/BoardMapperTest.php b/tests/unit/Db/BoardMapperTest.php index b186c0e3b..9b3e87d33 100644 --- a/tests/unit/Db/BoardMapperTest.php +++ b/tests/unit/Db/BoardMapperTest.php @@ -23,6 +23,8 @@ namespace OCA\Deck\Db; +use OCP\IGroupManager; +use OCP\IUserManager; use Test\AppFramework\Db\MapperTestUtility; /** @@ -33,6 +35,8 @@ class BoardMapperTest extends MapperTestUtility { private $dbConnection; private $aclMapper; private $boardMapper; + private $userManager; + private $groupManager; // Data private $acls; @@ -41,12 +45,17 @@ class BoardMapperTest extends MapperTestUtility { public function setup(){ parent::setUp(); + $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->dbConnection = \OC::$server->getDatabaseConnection(); $this->boardMapper = new BoardMapper( $this->dbConnection, \OC::$server->query(LabelMapper::class), \OC::$server->query(AclMapper::class), - \OC::$server->query(StackMapper::class) + \OC::$server->query(StackMapper::class), + $this->userManager, + $this->groupManager ); $this->aclMapper = \OC::$server->query(AclMapper::class); $this->labelMapper = \OC::$server->query(LabelMapper::class); diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 27f984878..a60955123 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -47,10 +47,6 @@ class BoardServiceTest extends \Test\TestCase { private $aclMapper; /** @var BoardMapper */ private $boardMapper; - /** @var IUserManager */ - private $userManager; - /** @var IGroupManager */ - private $groupManager; /** @var PermissionService */ private $permissionService; @@ -62,22 +58,17 @@ class BoardServiceTest extends \Test\TestCase { $this->boardMapper = $this->createMock(BoardMapper::class); $this->labelMapper = $this->createMock(LabelMapper::class); $this->permissionService = $this->createMock(PermissionService::class); - $this->userManager = $this->createMock(IUserManager::class); - $this->groupManager = $this->createMock(IGroupManager::class); $this->service = new BoardService( $this->boardMapper, $this->l10n, $this->labelMapper, $this->aclMapper, - $this->permissionService, - $this->userManager, - $this->groupManager + $this->permissionService ); $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('admin'); - $this->userManager->expects($this->any())->method('get')->with('admin')->willReturn($user); } public function testFindAll() { @@ -173,7 +164,7 @@ class BoardServiceTest extends \Test\TestCase { $acl->setPermissionShare(true); $acl->setPermissionManage(true); $acl->resolveRelation('participant', function($participant) use (&$user) { - return new User($user); + return null; }); $this->aclMapper->expects($this->once()) ->method('insert')