Use mapper classes for relational data

Signed-off-by: Julius Härtl <jus@bitgrid.net>

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;
 	}
 

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));
+		});
+	}
+
 
 }

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');
 	}
 
 }

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));
+		});
+	}
+
 
 }

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,8 +75,14 @@ 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
 	 *
@@ -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);
 	}

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);
+		$newAcl = $this->aclMapper->insert($acl);
-		return $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);
 	}
 

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;
 	}
 
 	/**

templates/part.boardlist.php

@@ -17,11 +17,8 @@
 			<td><a href="#/board/{{b.id}}">{{ b.title }}</a></td>
 			<td>
 				<div id="assigned-users">
-					<div class="avatardiv" avatar
+					<div class="avatardiv" avatar displayname="{{ b.owner.uid }}" title="{{ b.owner.displayname }}"></div>
-						 displayname="{{ b.owner.uid }}" title="{{ b.owner.displayname }}"></div>
+					<div class="avatardiv" avatar displayname="{{ acl.participant.uid }}" title="{{ acl.participant.uid }}" ng-repeat="acl in b.acl | limitTo: 7"></div>
-					<div class="avatardiv" avatar
-						 displayname="{{ acl.participant.uid }}" title="{{ acl.participant.uid }}"
-						 ng-repeat="acl in b.acl | limitTo: 7"></div>
 				</div>
 			</td>
 		</tr>

templates/part.card.php

@@ -26,7 +26,7 @@
 		<?php p($l->t('Modified:')); ?> <span class="live-relative-timestamp" data-timestamp="{{cardservice.getCurrent().lastModified*1000}}">{{ cardservice.getCurrent().lastModified|relativeDateFilter }}</span>
 		<?php p($l->t('Created:')); ?> <span class="live-relative-timestamp" data-timestamp="{{cardservice.getCurrent().createdAt*1000}}">{{ cardservice.getCurrent().createdAt|relativeDateFilter }}</span>
 		<?php p($l->t('by')); ?>
-		<span>{{ cardservice.getCurrent().owner }}</span>
+		<span>{{ cardservice.getCurrent().owner.displayname }}</span>
 	</div>
 
 	<div id="labels">

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')),

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);

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->permissionService
-			$this->userManager,
-			$this->groupManager
 		);
 
 		$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')