Code cleaup

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

lib/Db/Group.php

@@ -24,7 +24,6 @@
 namespace OCA\Deck\Db;
 
 use OCP\IGroup;
-use OCP\IUser;
 
 class Group extends RelationalObject {
 

lib/Db/RelationalEntity.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
+ * @copyright Copyright (c) 2017 Julius Härtl <jus@bitgrid.net>
  *
  * @author Julius Härtl <jus@bitgrid.net>
  *
@@ -23,16 +23,14 @@
 
 namespace OCA\Deck\Db;
 
-
 class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSerializable {
 
-	private $primaryKey;
 	private $_relations = array();
 	private $_resolvedProperties = [];
 
 	/**
 	 * Mark a property as relation so it will not get updated using Mapper::update
-	 * @param string $property Name of the property
+	 * @param $property string Name of the property
 	 */
 	public function addRelation($property) {
 		if (!in_array($property, $this->_relations)) {
@@ -40,6 +38,10 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria
 		}
 	}
 
+	/**
+	 * Mark a property as resolvable via resolveRelation()
+	 * @param $property string Name of the property
+	 */
 	public function addResolvable($property) {
 		$this->_resolvedProperties[$property] = null;
 	}
@@ -63,21 +65,43 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria
 		$properties = get_object_vars($this);
 		$reflection = new \ReflectionClass($this);
 		$json = [];
-		foreach($properties as $property=>$value) {
+		foreach ($properties as $property => $value) {
-			if(substr($property, 0, 1) !== '_' && $reflection->hasProperty($property)) {
+			if (substr($property, 0, 1) !== '_' && $reflection->hasProperty($property)) {
 				$propertyReflection = $reflection->getProperty($property);
-				if(!$propertyReflection->isPrivate()) {
+				if (!$propertyReflection->isPrivate()) {
 					$json[$property] = $this->getter($property);
 				}
 			}
 		}
 		return $json;
-
+	}
+		/*
+	 * Resolve relational data from external methods
+	 *
+	 * example usage:
+	 *
+	 * in Board::__construct()
+	 * 		$this->addResolvable('owner')
+	 *
+	 * in BoardService
+	 * 		$board->resolveRelation('owner', function($owner) use (&$userManager) {
+	 * 			return new \OCA\Deck\Db\User($userManager->get($owner));
+	 * 		});
+	 *
+	 * 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
+	 * @throws \Exception
+	 */
 	public function resolveRelation($property, $resolver) {
+		$result = null;
 		if($property !== null && $this->$property !== null) {
 			$result = $resolver($this->$property);
-		} else {
-			$result = null;
 		}
 
 		if($result instanceof RelationalObject || $result === null) {
@@ -93,7 +117,7 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria
 			if($this->_resolvedProperties[$attr] !== null) {
 				return $this->_resolvedProperties[$attr];
 			} else {
-				return $this->getter($attr, $args);
+				return $this->getter($attr);
 			}
 		}
 
@@ -101,9 +125,8 @@ class RelationalEntity extends \OCP\AppFramework\Db\Entity implements \JsonSeria
 		if(strpos($methodName, 'set') === 0 && array_key_exists($attr, $this->_resolvedProperties)) {
 			if(!is_scalar($args[0])) {
 				$args[0] = $args[0]['primaryKey'];
-				parent::setter($attr, $args);
 			}
-			parent::setter($attr, $args);
+			return parent::setter($attr, $args);
 		}
 		return parent::__call($methodName, $args);
 	}

lib/Db/RelationalObject.php

@@ -21,17 +21,11 @@
  *
  */
 
-/**
- * Created by PhpStorm.
- * User: jus
- * Date: 27.02.17
- * Time: 14:05
- */
-
 namespace OCA\Deck\Db;
 
+class RelationalObject implements \JsonSerializable {
 
-abstract class RelationalObject implements \JsonSerializable {
+	private $primaryKey;
 
 	/**
 	 * RelationalObject constructor.
@@ -51,11 +45,15 @@ abstract class RelationalObject implements \JsonSerializable {
 		);
 	}
 
+	/**
+	 * This method should be overwritten if object doesn't implement \JsonSerializable
+	 */
 	public function getObjectSerialization() {
+		if($this->object instanceof \JsonSerializable) {
 			$this->object->jsonSerialize();
+		} else {
+			throw new \Exception('jsonSerialize is not implemented on ' . get_class($this));
+		}
 	}
 
-	public function getPrimaryKey() {
-		return $this->getPrimaryKey();
-	}
 }

lib/Db/User.php

@@ -23,12 +23,11 @@
 
 namespace OCA\Deck\Db;
 
-use OCP\IGroup;
 use OCP\IUser;
 
 class User extends RelationalObject {
 
-	public function __construct(IUser $user = null) {
+	public function __construct(IUser $user) {
 		$primaryKey = $user->getUID();
 		parent::__construct($primaryKey, $user);
 	}

lib/Service/BoardService.php

@@ -91,12 +91,12 @@ class BoardService {
 	private function mapAcl(Acl &$acl) {
 		$userManager = $this->userManager;
 		$groupManager = $this->groupManager;
-		$acl->resolveRelation('participant', function($value) use (&$acl, &$userManager, &$groupManager) {
+		$acl->resolveRelation('participant', function($participant) use (&$acl, &$userManager, &$groupManager) {
 			if($acl->getType() === Acl::PERMISSION_TYPE_USER) {
-				return new User($userManager->get($acl->getParticipant($value)));
+				return new User($userManager->get($acl->getParticipant($participant)));
 			}
 			if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
-				return new Group($groupManager->get($acl->getParticipant($value)));
+				return new Group($groupManager->get($acl->getParticipant($participant)));
 			}
 		});
 		return $acl;
@@ -104,8 +104,8 @@ class BoardService {
 
 	private function mapOwner(Board $board) {
 		$userManager = $this->userManager;
-		$board->resolveRelation('owner', function($value) use (&$userManager) {
+		$board->resolveRelation('owner', function($owner) use (&$userManager) {
-			return new User($userManager->get($value));
+			return new User($userManager->get($owner));
 		});
 		return $board;
 	}

tests/unit/Db/BoardTest.php

@@ -20,8 +20,8 @@ class BoardTest extends \PHPUnit_Framework_TestCase {
 			'title' => "My Board",
 			'owner' => "admin",
 			'color' => "000000",
-			'labels' => null,
+			'labels' => array(),
-			'acl' => null,
+			'acl' => array(),
 			'archived' => false
 		], $board->jsonSerialize());
 	}
@@ -35,7 +35,7 @@ class BoardTest extends \PHPUnit_Framework_TestCase {
 			'owner' => "admin",
 			'color' => "000000",
 			'labels' => array("foo", "bar"),
-			'acl' => null,
+			'acl' => array(),
 			'archived' => false
 		], $board->jsonSerialize());
 	}
@@ -55,8 +55,8 @@ class BoardTest extends \PHPUnit_Framework_TestCase {
 			'title' => "My Board",
 			'owner' => "admin",
 			'color' => "000000",
-			'labels' => null,
+			'labels' => array(),
-			'acl' => null,
+			'acl' => array(),
 			'archived' => false,
 			'shared' => 1,
 		], $board->jsonSerialize());

tests/unit/Service/BoardServiceTest.php

@@ -172,7 +172,7 @@ class BoardServiceTest extends \Test\TestCase {
 		$acl->setPermissionEdit(true);
 		$acl->setPermissionShare(true);
 		$acl->setPermissionManage(true);
-		$acl->resolveRelation('participant', function($value) use (&$user) {
+		$acl->resolveRelation('participant', function($participant) use (&$user) {
 			return new User($user);
 		});
 		$this->aclMapper->expects($this->once())