From 2ce2de0ae523e91b90c39a91fdcc975f06a7674a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 23 Nov 2015 23:53:55 +0100 Subject: [PATCH 01/17] add icommentsmanger and icomment implementation register CommentsManager service, allow override, document in config.sample.php don't insert autoincrement ids in tests, because of dislikes from oracle and pgsql specify timezone in null date only accepts strings for ID parameter that can be converted to int replace forgotten hardcoded IDs in tests react on deleted users react on file deletion Postgresql compatibility lastInsertId needs *PREFIX* with the table name do not listen for file deletion, because it is not reliable (trashbin, external storages) add runtime cache for comments --- config/config.sample.php | 13 + lib/private/comments/comment.php | 350 +++++++++++ lib/private/comments/manager.php | 566 ++++++++++++++++++ lib/private/comments/managerfactory.php | 24 + lib/private/server.php | 11 + lib/public/comments/icomment.php | 19 +- lib/public/comments/icommentsmanager.php | 22 +- .../comments/icommentsmanagerfactory.php | 22 + tests/lib/comments/comment.php | 102 ++++ tests/lib/comments/fakefactory.php | 22 + tests/lib/comments/manager.php | 560 +++++++++++++++++ tests/lib/server.php | 1 + 12 files changed, 1706 insertions(+), 6 deletions(-) create mode 100644 lib/private/comments/comment.php create mode 100644 lib/private/comments/manager.php create mode 100644 lib/private/comments/managerfactory.php create mode 100644 lib/public/comments/icommentsmanagerfactory.php create mode 100644 tests/lib/comments/comment.php create mode 100644 tests/lib/comments/fakefactory.php create mode 100644 tests/lib/comments/manager.php diff --git a/config/config.sample.php b/config/config.sample.php index c3abe3a2b87..9b10792944f 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -814,6 +814,19 @@ $CONFIG = array( */ 'enforce_home_folder_naming_rule' => true, +/** + * Comments + * + * Global settings for the Comments infrastructure + */ + +/** + * Replaces the default Comments Manager Factory. This can be utilized if an + * own or 3rdParty CommentsManager should be used that – for instance – uses the + * filesystem instead of the database to keep the comments. + */ +'comments.managerFactory' => '\OC\Comments\ManagerFactory', + /** * Maintenance * diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php new file mode 100644 index 00000000000..2e9e4bd3d84 --- /dev/null +++ b/lib/private/comments/comment.php @@ -0,0 +1,350 @@ + '', + 'parentId' => '0', + 'topmostParentId' => '0', + 'childrenCount' => '0', + 'message' => '', + 'verb' => '', + 'actorType' => '', + 'actorId' => '', + 'objectType' => '', + 'objectId' => '', + 'creationDT' => null, + 'latestChildDT' => null, + ]; + + /** + * Comment constructor. + * + * @param [] $data optional, array with keys according to column names from + * the comments database scheme + */ + public function __construct(array $data = null) { + if(is_array($data)) { + $this->fromArray($data); + } + } + + /** + * returns the ID of the comment + * + * It may return an empty string, if the comment was not stored. + * It is expected that the concrete Comment implementation gives an ID + * by itself (e.g. after saving). + * + * @return string + * @since 9.0.0 + */ + public function getId() { + return $this->data['id']; + } + + /** + * sets the ID of the comment and returns itself + * + * It is only allowed to set the ID only, if the current id is an empty + * string (which means it is not stored in a database, storage or whatever + * the concrete implementation does), or vice versa. Changing a given ID is + * not permitted and must result in an IllegalIDChangeException. + * + * @param string $id + * @return IComment + * @throws IllegalIDChangeException + * @since 9.0.0 + */ + public function setId($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('String expected.'); + } + + if($this->data['id'] === '' || ($this->data['id'] !== '' && $id === '')) { + $this->data['id'] = $id; + return $this; + } + + throw new IllegalIDChangeException('Not allowed to assign a new ID to an already saved comment.'); + } + + /** + * returns the parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getParentId() { + return $this->data['parentId']; + } + + /** + * sets the parent ID and returns itself + * + * @param string $parentId + * @return IComment + * @since 9.0.0 + */ + public function setParentId($parentId) { + if(!is_string($parentId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['parentId'] = $parentId; + return $this; + } + + /** + * returns the topmost parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getTopmostParentId() { + return $this->data['topmostParentId']; + } + + + /** + * sets the topmost parent ID and returns itself + * + * @param string $id + * @return IComment + * @since 9.0.0 + */ + public function setTopmostParentId($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['topmostParentId'] = $id; + return $this; + } + + /** + * returns the number of children + * + * @return int + * @since 9.0.0 + */ + public function getChildrenCount() { + return $this->data['childrenCount']; + } + + /** + * sets the number of children + * + * @param int $count + * @return IComment + * @since 9.0.0 + */ + public function setChildrenCount($count) { + if(!is_int($count)) { + throw new \InvalidArgumentException('Integer expected.'); + } + $this->data['childrenCount'] = $count; + return $this; + } + + /** + * returns the message of the comment + * + * @return string + * @since 9.0.0 + */ + public function getMessage() { + return $this->data['message']; + } + + /** + * sets the message of the comment and returns itself + * + * @param string $message + * @return IComment + * @since 9.0.0 + */ + public function setMessage($message) { + if(!is_string($message)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['message'] = $message; + return $this; + } + + /** + * returns the verb of the comment + * + * @return string + * @since 9.0.0 + */ + public function getVerb() { + return $this->data['verb']; + } + + /** + * sets the verb of the comment, e.g. 'comment' or 'like' + * + * @param string $verb + * @return IComment + * @since 9.0.0 + */ + public function setVerb($verb) { + if(!is_string($verb)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['verb'] = $verb; + return $this; + } + + /** + * returns the actor type + * + * @return string + * @since 9.0.0 + */ + public function getActorType() { + return $this->data['actorType']; + } + + /** + * returns the actor ID + * + * @return string + * @since 9.0.0 + */ + public function getActorId() { + return $this->data['actorId']; + } + + /** + * sets (overwrites) the actor type and id + * + * @param string $actorType e.g. 'user' + * @param string $actorId e.g. 'zombie234' + * @return IComment + * @since 9.0.0 + */ + public function setActor($actorType, $actorId) { + if(!is_string($actorType) || !is_string($actorId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['actorType'] = $actorType; + $this->data['actorId'] = $actorId; + return $this; + } + + /** + * returns the creation date of the comment. + * + * If not explicitly set, it shall default to the time of initialization. + * + * @return \DateTime + * @since 9.0.0 + */ + public function getCreationDateTime() { + return $this->data['creationDT']; + } + + /** + * sets the creation date of the comment and returns itself + * + * @param \DateTime $timestamp + * @return IComment + * @since 9.0.0 + */ + public function setCreationDateTime(\DateTime $timestamp) { + $this->data['creationDT'] = $timestamp; + return $this; + } + + /** + * returns the timestamp of the most recent child + * + * @return int + * @since 9.0.0 + */ + public function getLatestChildDateTime() { + return $this->data['latestChildDT']; + } + + /** + * sets the date of the most recent child + * + * @param \DateTime $dateTime + * @return IComment + * @since 9.0.0 + */ + public function setLatestChildDateTime(\DateTime $dateTime = null) { + $this->data['latestChildDT'] = $dateTime; + return $this; + } + + /** + * returns the object type the comment is attached to + * + * @return string + * @since 9.0.0 + */ + public function getObjectType() { + return $this->data['objectType']; + } + + /** + * returns the object id the comment is attached to + * + * @return string + * @since 9.0.0 + */ + public function getObjectId() { + return $this->data['objectId']; + } + + /** + * sets (overwrites) the object of the comment + * + * @param string $objectType e.g. 'file' + * @param string $objectId e.g. '16435' + * @return IComment + * @since 9.0.0 + */ + public function setObject($objectType, $objectId) { + if(!is_string($objectType) || !is_string($objectId)) { + throw new \InvalidArgumentException('String expected.'); + } + $this->data['objectType'] = $objectType; + $this->data['objectId'] = $objectId; + return $this; + } + + /** + * sets the comment data based on an array with keys as taken from the + * database. + * + * @param [] $data + * @return IComment + */ + protected function fromArray($data) { + foreach(array_keys($data) as $key) { + // translate DB keys to internal setter names + $setter = 'set' . str_replace('_', '', ucwords($key,'_')); + $setter = str_replace('Timestamp', 'DateTime', $setter); + + if(method_exists($this, $setter)) { + $this->$setter($data[$key]); + } + } + + foreach(['actor', 'object'] as $role) { + if(isset($data[$role . '_type']) && isset($data[$role . '_id'])) { + $setter = 'set' . ucfirst($role); + $this->$setter($data[$role . '_type'], $data[$role . '_id']); + } + } + + return $this; + } +} diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php new file mode 100644 index 00000000000..78b2b71c8dd --- /dev/null +++ b/lib/private/comments/manager.php @@ -0,0 +1,566 @@ +dbConn = $dbConn; + $this->logger = $logger; + $userManager->listen('\OC\User', 'postDelete', function($user) { + /** @var \OCP\IUser $user */ + $this->deleteReferencesOfActor('user', $user->getUid()); + }); + } + + /** + * converts data base data into PHP native, proper types as defined by + * IComment interface. + * + * @param array $data + * @return array + */ + protected function normalizeDatabaseData(array $data) { + $data['id'] = strval($data['id']); + $data['parent_id'] = strval($data['parent_id']); + $data['topmost_parent_id'] = strval($data['topmost_parent_id']); + $data['creation_timestamp'] = new \DateTime($data['creation_timestamp']); + $data['latest_child_timestamp'] = new \DateTime($data['latest_child_timestamp']); + $data['children_count'] = intval($data['children_count']); + return $data; + } + + /** + * prepares a comment for an insert or update operation after making sure + * all necessary fields have a value assigned. + * + * @param IComment $comment + * @return IComment + * @throws \UnexpectedValueException + */ + protected function prepareCommentForDatabaseWrite(IComment $comment) { + if( empty($comment->getActorType()) + || empty($comment->getActorId()) + || empty($comment->getObjectType()) + || empty($comment->getObjectId()) + || empty($comment->getVerb()) + ) { + throw new \UnexpectedValueException('Actor, Object and Verb information must be provided for saving'); + } + + if($comment->getId() === '') { + $comment->setChildrenCount(0); + $comment->setLatestChildDateTime(new \DateTime('0000-00-00 00:00:00', new \DateTimeZone('UTC'))); + $comment->setLatestChildDateTime(null); + } + + if(is_null($comment->getCreationDateTime())) { + $comment->setCreationDateTime(new \DateTime()); + } + + if($comment->getParentId() !== '0') { + $comment->setTopmostParentId($this->determineTopmostParentId($comment->getParentId())); + } else { + $comment->setTopmostParentId('0'); + } + + $this->cache($comment); + + return $comment; + } + + /** + * returns the topmost parent id of a given comment identified by ID + * + * @param string $id + * @return string + * @throws NotFoundException + */ + protected function determineTopmostParentId($id) { + $comment = $this->get($id); + if($comment->getParentId() === '0') { + return $comment->getId(); + } else { + return $this->determineTopmostParentId($comment->getId()); + } + } + + /** + * updates child information of a comment + * + * @param string $id + * @param \DateTime $cDateTime the date time of the most recent child + * @throws NotFoundException + */ + protected function updateChildrenInformation($id, \DateTime $cDateTime) { + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select($qb->createFunction('COUNT(`id`)')) + ->from('comments') + ->where($qb->expr()->eq('parent_id', $qb->createParameter('id'))) + ->setParameter('id', $id); + + $resultStatement = $query->execute(); + $data = $resultStatement->fetch(\PDO::FETCH_NUM); + $resultStatement->closeCursor(); + $children = intval($data[0]); + + $comment = $this->get($id); + $comment->setChildrenCount($children); + $comment->setLatestChildDateTime($cDateTime); + $this->save($comment); + } + + /** + * Tests whether actor or object type and id parameters are acceptable. + * Throws exception if not. + * + * @param string $role + * @param string $type + * @param string $id + * @throws \InvalidArgumentException + */ + protected function checkRoleParameters($role, $type, $id) { + if( + !is_string($type) || empty($type) + || !is_string($id) || empty($id) + ) { + throw new \InvalidArgumentException($role . ' parameters must be string and not empty'); + } + } + + /** + * run-time caches a comment + * + * @param IComment $comment + */ + protected function cache(IComment $comment) { + $id = $comment->getId(); + if(empty($id)) { + return; + } + $this->commentsCache[strval($id)] = $comment; + } + + /** + * removes an entry from the comments run time cache + * + * @param mixed $id the comment's id + */ + protected function uncache($id) { + $id = strval($id); + if (isset($this->commentsCache[$id])) { + unset($this->commentsCache[$id]); + } + } + + /** + * returns a comment instance + * + * @param string $id the ID of the comment + * @return IComment + * @throws NotFoundException + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function get($id) { + if(intval($id) === 0) { + throw new \InvalidArgumentException('IDs must be translatable to a number in this implementation.'); + } + + if(isset($this->commentsCache[$id])) { + return $this->commentsCache[$id]; + } + + $qb = $this->dbConn->getQueryBuilder(); + $resultStatement = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $id, \PDO::PARAM_INT) + ->execute(); + + $data = $resultStatement->fetch(); + $resultStatement->closeCursor(); + if(!$data) { + throw new NotFoundException(); + } + + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + return $comment; + } + + /** + * returns the comment specified by the id and all it's child comments. + * At this point of time, we do only support one level depth. + * + * @param string $id + * @param int $limit max number of entries to return, 0 returns all + * @param int $offset the start entry + * @return array + * @since 9.0.0 + * + * The return array looks like this + * [ + * 'comment' => IComment, // root comment + * 'replies' => + * [ + * 0 => + * [ + * 'comment' => IComment, + * 'replies' => [] + * ] + * 1 => + * [ + * 'comment' => IComment, + * 'replies'=> [] + * ], + * … + * ] + * ] + */ + public function getTree($id, $limit = 0, $offset = 0) { + $tree = []; + $tree['comment'] = $this->get($id); + $tree['replies'] = []; + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('topmost_parent_id', $qb->createParameter('id'))) + ->orderBy('creation_timestamp', 'DESC') + ->setParameter('id', $id); + + if($limit > 0) { + $query->setMaxResults($limit); + } + if($offset > 0) { + $query->setFirstResult($offset); + } + + $resultStatement = $query->execute(); + while($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $tree['replies'][] = [ + 'comment' => $comment, + 'replies' => [] + ]; + } + $resultStatement->closeCursor(); + + return $tree; + } + + /** + * returns comments for a specific object (e.g. a file). + * + * The sort order is always newest to oldest. + * + * @param string $objectType the object type, e.g. 'files' + * @param string $objectId the id of the object + * @param int $limit optional, number of maximum comments to be returned. if + * not specified, all comments are returned. + * @param int $offset optional, starting point + * @param \DateTime $notOlderThan optional, timestamp of the oldest comments + * that may be returned + * @return IComment[] + * @since 9.0.0 + */ + public function getForObject( + $objectType, + $objectId, + $limit = 0, + $offset = 0, + \DateTime $notOlderThan = null + ) { + $comments = []; + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select('*') + ->from('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->orderBy('creation_timestamp', 'DESC') + ->setParameter('type', $objectType) + ->setParameter('id', $objectId); + + if($limit > 0) { + $query->setMaxResults($limit); + } + if($offset > 0) { + $query->setFirstResult($offset); + } + if(!is_null($notOlderThan)) { + $query + ->andWhere($qb->expr()->gt('creation_timestamp', $qb->createParameter('notOlderThan'))) + ->setParameter('notOlderThan', $notOlderThan, 'datetime'); + } + + $resultStatement = $query->execute(); + while($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $comments[] = $comment; + } + $resultStatement->closeCursor(); + + return $comments; + } + + /** + * @param $objectType string the object type, e.g. 'files' + * @param $objectId string the id of the object + * @return Int + * @since 9.0.0 + */ + public function getNumberOfCommentsForObject($objectType, $objectId) { + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->select($qb->createFunction('COUNT(`id`)')) + ->from('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->setParameter('type', $objectType) + ->setParameter('id', $objectId); + + $resultStatement = $query->execute(); + $data = $resultStatement->fetch(\PDO::FETCH_NUM); + $resultStatement->closeCursor(); + return intval($data[0]); + } + + /** + * creates a new comment and returns it. At this point of time, it is not + * saved in the used data storage. Use save() after setting other fields + * of the comment (e.g. message or verb). + * + * @param string $actorType the actor type (e.g. 'user') + * @param string $actorId a user id + * @param string $objectType the object type the comment is attached to + * @param string $objectId the object id the comment is attached to + * @return IComment + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function create($actorType, $actorId, $objectType, $objectId) { + if( + !is_string($actorType) || empty($actorType) + || !is_string($actorId) || empty($actorId) + || !is_string($objectType) || empty($objectType) + || !is_string($objectId) || empty($objectId) + ) { + // unsure whether it's a good place to enforce it here, since the + // comment instance can be manipulated anyway. + throw new \InvalidArgumentException('All arguments must be non-empty strings'); + } + $comment = new Comment(); + $comment + ->setActor($actorType, $actorId) + ->setObject($objectType, $objectId); + return $comment; + } + + /** + * permanently deletes the comment specified by the ID + * + * When the comment has child comments, their parent ID will be changed to + * the parent ID of the item that is to be deleted. + * + * @param string $id + * @return bool + * @throws \InvalidArgumentException + * @since 9.0.0 + */ + public function delete($id) { + if(!is_string($id)) { + throw new \InvalidArgumentException('Parameter must be string'); + } + + $qb = $this->dbConn->getQueryBuilder(); + $query = $qb->delete('comments') + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $id); + + try { + $affectedRows = $query->execute(); + $this->uncache($id); + } catch (DriverException $e) { + $this->logger->logException($e, ['app' => 'core_comments']); + return false; + } + return ($affectedRows > 0); + } + + /** + * saves the comment permanently and returns it + * + * if the supplied comment has an empty ID, a new entry comment will be + * saved and the instance updated with the new ID. + * + * Otherwise, an existing comment will be updated. + * + * Throws NotFoundException when a comment that is to be updated does not + * exist anymore at this point of time. + * + * @param IComment &$comment + * @return bool + * @throws NotFoundException + * @since 9.0.0 + */ + public function save(IComment &$comment) { + $comment = $this->prepareCommentForDatabaseWrite($comment); + if($comment->getId() === '') { + $result = $this->insert($comment); + } else { + $result = $this->update($comment); + } + + if($result && !empty($comment->getParentId())) { + $this->updateChildrenInformation( + $comment->getParentId(), + $comment->getCreationDateTime() + ); + $this->cache($comment); + } + + return $result; + } + + /** + * inserts the provided comment in the database + * + * @param IComment $comment + * @return bool + */ + protected function insert(IComment &$comment) { + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter($comment->getParentId()), + 'topmost_parent_id' => $qb->createNamedParameter($comment->getTopmostParentId()), + 'children_count' => $qb->createNamedParameter($comment->getChildrenCount()), + 'actor_type' => $qb->createNamedParameter($comment->getActorType()), + 'actor_id' => $qb->createNamedParameter($comment->getActorId()), + 'message' => $qb->createNamedParameter($comment->getMessage()), + 'verb' => $qb->createNamedParameter($comment->getVerb()), + 'creation_timestamp' => $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime'), + 'object_type' => $qb->createNamedParameter($comment->getObjectType()), + 'object_id' => $qb->createNamedParameter($comment->getObjectId()), + ]) + ->execute(); + + if ($affectedRows > 0) { + $comment->setId(strval($this->dbConn->lastInsertId('*PREFIX*comments'))); + } + + return $affectedRows > 0; + } + + /** + * updates a Comment data row + * + * @param IComment $comment + * @return bool + * @throws NotFoundException + */ + protected function update(IComment $comment) { + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->update('comments') + ->set('parent_id', $qb->createNamedParameter($comment->getParentId())) + ->set('topmost_parent_id', $qb->createNamedParameter($comment->getTopmostParentId())) + ->set('children_count', $qb->createNamedParameter($comment->getChildrenCount())) + ->set('actor_type', $qb->createNamedParameter($comment->getActorType())) + ->set('actor_id', $qb->createNamedParameter($comment->getActorId())) + ->set('message', $qb->createNamedParameter($comment->getMessage())) + ->set('verb', $qb->createNamedParameter($comment->getVerb())) + ->set('creation_timestamp', $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime')) + ->set('latest_child_timestamp', $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime')) + ->set('object_type', $qb->createNamedParameter($comment->getObjectType())) + ->set('object_id', $qb->createNamedParameter($comment->getObjectId())) + ->where($qb->expr()->eq('id', $qb->createParameter('id'))) + ->setParameter('id', $comment->getId()) + ->execute(); + + if($affectedRows === 0) { + throw new NotFoundException('Comment to update does ceased to exist'); + } + + return $affectedRows > 0; + } + + /** + * removes references to specific actor (e.g. on user delete) of a comment. + * The comment itself must not get lost/deleted. + * + * @param string $actorType the actor type (e.g. 'user') + * @param string $actorId a user id + * @return boolean + * @since 9.0.0 + */ + public function deleteReferencesOfActor($actorType, $actorId) { + $this->checkRoleParameters('Actor', $actorType, $actorId); + + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->update('comments') + ->set('actor_type', $qb->createNamedParameter(ICommentsManager::DELETED_USER)) + ->set('actor_id', $qb->createNamedParameter(ICommentsManager::DELETED_USER)) + ->where($qb->expr()->eq('actor_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('actor_id', $qb->createParameter('id'))) + ->setParameter('type', $actorType) + ->setParameter('id', $actorId) + ->execute(); + + $this->commentsCache = []; + + return is_int($affectedRows); + } + + /** + * deletes all comments made of a specific object (e.g. on file delete) + * + * @param string $objectType the object type (e.g. 'file') + * @param string $objectId e.g. the file id + * @return boolean + * @since 9.0.0 + */ + public function deleteCommentsAtObject($objectType, $objectId) { + $this->checkRoleParameters('Object', $objectType, $objectId); + + $qb = $this->dbConn->getQueryBuilder(); + $affectedRows = $qb + ->delete('comments') + ->where($qb->expr()->eq('object_type', $qb->createParameter('type'))) + ->andWhere($qb->expr()->eq('object_id', $qb->createParameter('id'))) + ->setParameter('type', $objectType) + ->setParameter('id', $objectId) + ->execute(); + + $this->commentsCache = []; + + return is_int($affectedRows); + } +} diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php new file mode 100644 index 00000000000..71d73571b10 --- /dev/null +++ b/lib/private/comments/managerfactory.php @@ -0,0 +1,24 @@ +getDatabaseConnection(), + \oc::$server->getUserManager(), + \oc::$server->getLogger() + ); + } +} diff --git a/lib/private/server.php b/lib/private/server.php index 6692e6f6bbf..ecac18d6a9b 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -528,6 +528,13 @@ class Server extends SimpleContainer implements IServerContainer { }); return $manager; }); + $this->registerService('CommentsManager', function(Server $c) { + $config = $c->getConfig(); + $factoryClass = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + /** @var \OCP\Comments\ICommentsManagerFactory $factory */ + $factory = new $factoryClass(); + return $factory->getManager(); + }); $this->registerService('EventDispatcher', function() { return new EventDispatcher(); }); @@ -1121,6 +1128,10 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('NotificationManager'); } + public function getCommentsManager() { + return $this->query('CommentsManager'); + } + /** * @return \OC\IntegrityCheck\Checker */ diff --git a/lib/public/comments/icomment.php b/lib/public/comments/icomment.php index c8f407624a0..8cfc504fc3f 100644 --- a/lib/public/comments/icomment.php +++ b/lib/public/comments/icomment.php @@ -49,13 +49,30 @@ interface IComment { /** * sets the parent ID and returns itself - * * @param string $parentId * @return IComment * @since 9.0.0 */ public function setParentId($parentId); + /** + * returns the topmost parent ID of the comment + * + * @return string + * @since 9.0.0 + */ + public function getTopmostParentId(); + + + /** + * sets the topmost parent ID and returns itself + * + * @param string $id + * @return IComment + * @since 9.0.0 + */ + public function setTopmostParentId($id); + /** * returns the number of children * diff --git a/lib/public/comments/icommentsmanager.php b/lib/public/comments/icommentsmanager.php index ebf7a34bf27..f6883224d60 100644 --- a/lib/public/comments/icommentsmanager.php +++ b/lib/public/comments/icommentsmanager.php @@ -12,6 +12,17 @@ namespace OCP\Comments; */ interface ICommentsManager { + /** + * @const DELETED_USER type and id for a user that has been deleted + * @see deleteReferencesOfActor + * @since 9.0.0 + * + * To be used as replacement for user type actors in deleteReferencesOfActor(). + * + * User interfaces shall show "Deleted user" as display name, if needed. + */ + const DELETED_USER = 'deleted_user'; + /** * returns a comment instance * @@ -28,7 +39,7 @@ interface ICommentsManager { * @param string $id * @param int $limit max number of entries to return, 0 returns all * @param int $offset the start entry - * @return [] + * @return array * @since 9.0.0 * * The return array looks like this @@ -73,7 +84,6 @@ interface ICommentsManager { * @param \DateTime $notOlderThan optional, timestamp of the oldest comments * that may be returned * @return IComment[] - * @throws NotFoundException in case the requested type or id is not present * @since 9.0.0 */ public function getForObject( @@ -88,7 +98,6 @@ interface ICommentsManager { * @param $objectType string the object type, e.g. 'files' * @param $objectId string the id of the object * @return Int - * @throws NotFoundException in case the requested type or id is not present * @since 9.0.0 */ public function getNumberOfCommentsForObject($objectType, $objectId); @@ -130,17 +139,20 @@ interface ICommentsManager { * Throws NotFoundException when a comment that is to be updated does not * exist anymore at this point of time. * - * @param IComment + * @param IComment &$comment * @return bool * @throws NotFoundException * @since 9.0.0 */ - public function save(&$comment); + public function save(IComment &$comment); /** * removes references to specific actor (e.g. on user delete) of a comment. * The comment itself must not get lost/deleted. * + * A 'user' type actor (type and id) should get replaced by the + * value of the DELETED_USER constant of this interface. + * * @param string $actorType the actor type (e.g. 'user') * @param string $actorId a user id * @return boolean diff --git a/lib/public/comments/icommentsmanagerfactory.php b/lib/public/comments/icommentsmanagerfactory.php new file mode 100644 index 00000000000..7bfb79aae00 --- /dev/null +++ b/lib/public/comments/icommentsmanagerfactory.php @@ -0,0 +1,22 @@ + 'user', 'id' => 'alice']; + $creationDT = new \DateTime(); + $latestChildDT = new \DateTime('yesterday'); + $object = ['type' => 'file', 'id' => 'file64']; + + $comment + ->setId($id) + ->setParentId($parentId) + ->setChildrenCount($childrenCount) + ->setMessage($message) + ->setVerb($verb) + ->setActor($actor['type'], $actor['id']) + ->setCreationDateTime($creationDT) + ->setLatestChildDateTime($latestChildDT) + ->setObject($object['type'], $object['id']); + + $this->assertSame($id, $comment->getId()); + $this->assertSame($parentId, $comment->getParentId()); + $this->assertSame($childrenCount, $comment->getChildrenCount()); + $this->assertSame($message, $comment->getMessage()); + $this->assertSame($verb, $comment->getVerb()); + $this->assertSame($actor['type'], $comment->getActorType()); + $this->assertSame($actor['id'], $comment->getActorId()); + $this->assertSame($creationDT, $comment->getCreationDateTime()); + $this->assertSame($latestChildDT, $comment->getLatestChildDateTime()); + $this->assertSame($object['type'], $comment->getObjectType()); + $this->assertSame($object['id'], $comment->getObjectId()); + } + + public function testSetIdIllegalInput() { + $comment = new \OC\Comments\Comment(); + + $this->setExpectedException('\OCP\Comments\IllegalIDChangeException'); + $comment->setId('c23'); + $comment->setId('c17'); + } + + public function testResetId() { + $comment = new \OC\Comments\Comment(); + $comment->setId('c23'); + $comment->setId(''); + } + + public function simpleSetterProvider() { + return [ + ['Id'], + ['ParentId'], + ['Message'], + ['Verb'], + ['ChildrenCount'], + ]; + } + + /** + * @dataProvider simpleSetterProvider + */ + public function testSimpleSetterInvalidInput($field) { + $comment = new \OC\Comments\Comment(); + $setter = 'set' . $field; + + $this->setExpectedException('InvalidArgumentException'); + // we have no field that is supposed to accept a Bool + $comment->$setter(true); + } + + public function roleSetterProvider() { + return [ + ['Actor', true, true], + ['Actor', 'user', true], + ['Actor', true, 'alice'], + ['Object', true, true], + ['Object', 'file', true], + ['Object', true, 'file64'], + ]; + } + + /** + * @dataProvider roleSetterProvider + */ + public function testSetRoleInvalidInput($role, $type, $id){ + $comment = new \OC\Comments\Comment(); + $setter = 'set' . $role; + $this->setExpectedException('InvalidArgumentException'); + $comment->$setter($type, $id); + } + + + +} diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php new file mode 100644 index 00000000000..202b02f6411 --- /dev/null +++ b/tests/lib/comments/fakefactory.php @@ -0,0 +1,22 @@ +markTestSkipped(); + } + + public function getManager() { + return $this->getMock('\OCP\Comments\ICommentsManager'); + } +} diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php new file mode 100644 index 00000000000..610dfe51a47 --- /dev/null +++ b/tests/lib/comments/manager.php @@ -0,0 +1,560 @@ +getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); + \oc::$server->getDatabaseConnection()->prepare($sql)->execute(); + } + + protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null) { + if(is_null($creationDT)) { + $creationDT = new \DateTime(); + } + if(is_null($latestChildDT)) { + $latestChildDT = new \DateTime('yesterday'); + } + + $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter($parentId), + 'topmost_parent_id' => $qb->createNamedParameter($topmostParentId), + 'children_count' => $qb->createNamedParameter(2), + 'actor_type' => $qb->createNamedParameter('user'), + 'actor_id' => $qb->createNamedParameter('alice'), + 'message' => $qb->createNamedParameter('nice one'), + 'verb' => $qb->createNamedParameter('comment'), + 'creation_timestamp' => $qb->createNamedParameter($creationDT, 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'), + 'object_type' => $qb->createNamedParameter('file'), + 'object_id' => $qb->createNamedParameter('file64'), + ]) + ->execute(); + + return \oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + } + + protected function getManager() { + $factory = new \OC\Comments\ManagerFactory(); + return $factory->getManager(); + } + + public function testGetCommentNotFound() { + $manager = $this->getManager(); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->get('22'); + } + + public function testGetCommentNotFoundInvalidInput() { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->get('unexisting22'); + } + + public function testGetComment() { + $manager = $this->getManager(); + + $creationDT = new \DateTime(); + $latestChildDT = new \DateTime('yesterday'); + + $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb + ->insert('comments') + ->values([ + 'parent_id' => $qb->createNamedParameter('2'), + 'topmost_parent_id' => $qb->createNamedParameter('1'), + 'children_count' => $qb->createNamedParameter(2), + 'actor_type' => $qb->createNamedParameter('user'), + 'actor_id' => $qb->createNamedParameter('alice'), + 'message' => $qb->createNamedParameter('nice one'), + 'verb' => $qb->createNamedParameter('comment'), + 'creation_timestamp' => $qb->createNamedParameter($creationDT, 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'), + 'object_type' => $qb->createNamedParameter('file'), + 'object_id' => $qb->createNamedParameter('file64'), + ]) + ->execute(); + + $id = strval(\oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + + $comment = $manager->get($id); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getId(), $id); + $this->assertSame($comment->getParentId(), '2'); + $this->assertSame($comment->getTopmostParentId(), '1'); + $this->assertSame($comment->getChildrenCount(), 2); + $this->assertSame($comment->getActorType(), 'user'); + $this->assertSame($comment->getActorId(), 'alice'); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getVerb(), 'comment'); + $this->assertSame($comment->getObjectType(), 'file'); + $this->assertSame($comment->getObjectId(), 'file64'); + $this->assertEquals($comment->getCreationDateTime(), $creationDT); + $this->assertEquals($comment->getLatestChildDateTime(), $latestChildDT); + } + + public function testGetTreeNotFound() { + $manager = $this->getManager(); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->getTree('22'); + } + + public function testGetTreeNotFoundInvalidIpnut() { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->getTree('unexisting22'); + } + + public function testGetTree() { + $headId = $this->addDatabaseEntry(0, 0); + + $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); + $id = $this->addDatabaseEntry($headId, $headId, new \DateTime('-1 hour')); + + $manager = $this->getManager(); + $tree = $manager->getTree($headId); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($headId)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 3); + + // one level deep + foreach($tree['replies'] as $reply) { + $this->assertTrue($reply['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($reply['comment']->getId(), strval($id)); + $this->assertSame(count($reply['replies']), 0); + $id--; + } + } + + public function testGetTreeNoReplies() { + $id = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + $tree = $manager->getTree($id); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($id)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 0); + + // one level deep + foreach($tree['replies'] as $reply) { + throw new \Exception('This ain`t happen'); + } + } + + public function testGetTreeWithLimitAndOffset() { + $headId = $this->addDatabaseEntry(0, 0); + + $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); + $this->addDatabaseEntry($headId, $headId, new \DateTime('-1 hour')); + $idToVerify = $this->addDatabaseEntry($headId, $headId, new \DateTime()); + + $manager = $this->getManager(); + + for ($offset = 0; $offset < 3; $offset += 2) { + $tree = $manager->getTree(strval($headId), 2, $offset); + + // Verifying the root comment + $this->assertTrue(isset($tree['comment'])); + $this->assertTrue($tree['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($tree['comment']->getId(), strval($headId)); + $this->assertTrue(isset($tree['replies'])); + $this->assertSame(count($tree['replies']), 2); + + // one level deep + foreach ($tree['replies'] as $reply) { + $this->assertTrue($reply['comment'] instanceof \OCP\Comments\IComment); + $this->assertSame($reply['comment']->getId(), strval($idToVerify)); + $this->assertSame(count($reply['replies']), 0); + $idToVerify--; + } + } + } + + public function testGetForObject() { + $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + $comments = $manager->getForObject('file', 'file64'); + + $this->assertTrue(is_array($comments)); + $this->assertSame(count($comments), 1); + $this->assertTrue($comments[0] instanceof \OCP\Comments\IComment); + $this->assertSame($comments[0]->getMessage(), 'nice one'); + } + + public function testGetForObjectWithLimitAndOffset() { + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); + $this->addDatabaseEntry(1, 1, new \DateTime('-4 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + + $manager = $this->getManager(); + $offset = 0; + do { + $comments = $manager->getForObject('file', 'file64', 3, $offset); + + $this->assertTrue(is_array($comments)); + foreach($comments as $comment) { + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getId(), strval($idToVerify)); + $idToVerify--; + } + $offset += 3; + } while(count($comments) > 0); + } + + public function testGetForObjectWithDateTimeConstraint() { + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); + $id1 = $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $id2 = $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + + $manager = $this->getManager(); + $comments = $manager->getForObject('file', 'file64', 0, 0, new \DateTime('-4 hours')); + + $this->assertSame(count($comments), 2); + $this->assertSame($comments[0]->getId(), strval($id2)); + $this->assertSame($comments[1]->getId(), strval($id1)); + } + + public function testGetForObjectWithLimitAndOffsetAndDateTimeConstraint() { + $this->addDatabaseEntry(0, 0, new \DateTime('-7 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); + $this->addDatabaseEntry(1, 1, new \DateTime('-5 hours')); + $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + + $manager = $this->getManager(); + $offset = 0; + do { + $comments = $manager->getForObject('file', 'file64', 3, $offset, new \DateTime('-4 hours')); + + $this->assertTrue(is_array($comments)); + foreach($comments as $comment) { + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getMessage(), 'nice one'); + $this->assertSame($comment->getId(), strval($idToVerify)); + $this->assertTrue(intval($comment->getId()) >= 4); + $idToVerify--; + } + $offset += 3; + } while(count($comments) > 0); + } + + public function testGetNumberOfCommentsForObject() { + for($i = 1; $i < 5; $i++) { + $this->addDatabaseEntry(0, 0); + } + + $manager = $this->getManager(); + + $amount = $manager->getNumberOfCommentsForObject('untype', '00'); + $this->assertSame($amount, 0); + + $amount = $manager->getNumberOfCommentsForObject('file', 'file64'); + $this->assertSame($amount, 4); + } + + public function invalidCreateArgsProvider() { + return [ + ['', 'aId-1', 'oType-1', 'oId-1'], + ['aType-1', '', 'oType-1', 'oId-1'], + ['aType-1', 'aId-1', '', 'oId-1'], + ['aType-1', 'aId-1', 'oType-1', ''], + [1, 'aId-1', 'oType-1', 'oId-1'], + ['aType-1', 1, 'oType-1', 'oId-1'], + ['aType-1', 'aId-1', 1, 'oId-1'], + ['aType-1', 'aId-1', 'oType-1', 1], + ]; + } + + /** + * @dataProvider invalidCreateArgsProvider + */ + public function testCreateCommentInvalidArguments($aType, $aId, $oType, $oId) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->create($aType, $aId, $oType, $oId); + } + + public function testCreateComment() { + $actorType = 'bot'; + $actorId = 'bob'; + $objectType = 'weather'; + $objectId = 'bielefeld'; + + $comment = $this->getManager()->create($actorType, $actorId, $objectType, $objectId); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $this->assertSame($comment->getActorType(), $actorType); + $this->assertSame($comment->getActorId(), $actorId); + $this->assertSame($comment->getObjectType(), $objectType); + $this->assertSame($comment->getObjectId(), $objectId); + } + + public function testDelete() { + $manager = $this->getManager(); + + $done = $manager->delete('404'); + $this->assertFalse($done); + + $done = $manager->delete('%'); + $this->assertFalse($done); + + $done = $manager->delete(''); + $this->assertFalse($done); + + $id = strval($this->addDatabaseEntry(0, 0)); + $comment = $manager->get($id); + $this->assertTrue($comment instanceof \OCP\Comments\IComment); + $done = $manager->delete($id); + $this->assertTrue($done); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->get($id); + } + + public function testSaveNew() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $saveSuccessful = $manager->save($comment); + $this->assertTrue($saveSuccessful); + $this->assertTrue($comment->getId() !== ''); + $this->assertTrue($comment->getId() !== '0'); + $this->assertTrue(!is_null($comment->getCreationDateTime())); + + $loadedComment = $manager->get($comment->getId()); + $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); + $this->assertEquals($comment->getCreationDateTime(), $loadedComment->getCreationDateTime()); + } + + public function testSaveUpdate() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $manager->save($comment); + + $comment->setMessage('very beautiful, I am really so much impressed!'); + $manager->save($comment); + + $loadedComment = $manager->get($comment->getId()); + $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); + } + + public function testSaveUpdateException() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setMessage('very beautiful, I am impressed!') + ->setVerb('comment'); + + $manager->save($comment); + + $manager->delete($comment->getId()); + $comment->setMessage('very beautiful, I am really so much impressed!'); + $this->setExpectedException('\OCP\Comments\NotFoundException'); + $manager->save($comment); + } + + public function testSaveIncomplete() { + $manager = $this->getManager(); + $comment = new \OC\Comments\Comment(); + $comment->setMessage('from no one to nothing'); + $this->setExpectedException('\UnexpectedValueException'); + $manager->save($comment); + } + + public function testSaveAsChild() { + $id = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + for($i = 0; $i < 3; $i++) { + $comment = new \OC\Comments\Comment(); + $comment + ->setActor('user', 'alice') + ->setObject('file', 'file64') + ->setParentId(strval($id)) + ->setMessage('full ack') + ->setVerb('comment') + // setting the creation time avoids using sleep() while making sure to test with different timestamps + ->setCreationDateTime(new \DateTime('+' . $i . ' minutes')); + + $manager->save($comment); + + $this->assertSame($comment->getTopmostParentId(), strval($id)); + $parentComment = $manager->get(strval($id)); + $this->assertSame($parentComment->getChildrenCount(), $i + 1); + $this->assertEquals($parentComment->getLatestChildDateTime(), $comment->getCreationDateTime()); + } + } + + public function invalidActorArgsProvider() { + return + [ + ['', ''], + [1, 'alice'], + ['user', 1], + ]; + } + + /** + * @dataProvider invalidActorArgsProvider + */ + public function testDeleteReferencesOfActorInvalidInput($type, $id) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->deleteReferencesOfActor($type, $id); + } + + public function testDeleteReferencesOfActor() { + $ids = []; + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + // just to make sure they are really set, with correct actor data + $comment = $manager->get(strval($ids[1])); + $this->assertSame($comment->getActorType(), 'user'); + $this->assertSame($comment->getActorId(), 'alice'); + + $wasSuccessful = $manager->deleteReferencesOfActor('user', 'alice'); + $this->assertTrue($wasSuccessful); + + foreach($ids as $id) { + $comment = $manager->get(strval($id)); + $this->assertSame($comment->getActorType(), ICommentsManager::DELETED_USER); + $this->assertSame($comment->getActorId(), ICommentsManager::DELETED_USER); + } + + // actor info is gone from DB, but when database interaction is alright, + // we still expect to get true back + $wasSuccessful = $manager->deleteReferencesOfActor('user', 'alice'); + $this->assertTrue($wasSuccessful); + } + + public function testDeleteReferencesOfActorWithUserManagement() { + $user = \oc::$server->getUserManager()->createUser('xenia', '123456'); + $this->assertTrue($user instanceof \OCP\IUser); + + $manager = $this->getManager(); + $comment = $manager->create('user', $user->getUID(), 'file', 'file64'); + $comment + ->setMessage('Most important comment I ever left on the Internet.') + ->setVerb('comment'); + $status = $manager->save($comment); + $this->assertTrue($status); + + $commentID = $comment->getId(); + $user->delete(); + + $comment =$manager->get($commentID); + $this->assertSame($comment->getActorType(), \OCP\Comments\ICommentsManager::DELETED_USER); + $this->assertSame($comment->getActorId(), \OCP\Comments\ICommentsManager::DELETED_USER); + } + + public function invalidObjectArgsProvider() { + return + [ + ['', ''], + [1, 'file64'], + ['file', 1], + ]; + } + + /** + * @dataProvider invalidObjectArgsProvider + */ + public function testDeleteCommentsAtObjectInvalidInput($type, $id) { + $manager = $this->getManager(); + $this->setExpectedException('\InvalidArgumentException'); + $manager->deleteCommentsAtObject($type, $id); + } + + public function testDeleteCommentsAtObject() { + $ids = []; + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + + // just to make sure they are really set, with correct actor data + $comment = $manager->get(strval($ids[1])); + $this->assertSame($comment->getObjectType(), 'file'); + $this->assertSame($comment->getObjectId(), 'file64'); + + $wasSuccessful = $manager->deleteCommentsAtObject('file', 'file64'); + $this->assertTrue($wasSuccessful); + + $verified = 0; + foreach($ids as $id) { + try { + $manager->get(strval($id)); + } catch (\OCP\Comments\NotFoundException $e) { + $verified++; + } + } + $this->assertSame($verified, 3); + + // actor info is gone from DB, but when database interaction is alright, + // we still expect to get true back + $wasSuccessful = $manager->deleteCommentsAtObject('file', 'file64'); + $this->assertTrue($wasSuccessful); + } + + public function testOverwriteDefaultManager() { + $config = \oc::$server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); + + $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $manager = \oc::$server->getCommentsManager(); + $this->assertEquals($managerMock, $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } + +} diff --git a/tests/lib/server.php b/tests/lib/server.php index b72bef82036..0bee19822d2 100644 --- a/tests/lib/server.php +++ b/tests/lib/server.php @@ -61,6 +61,7 @@ class Server extends \Test\TestCase { ['CapabilitiesManager', '\OC\CapabilitiesManager'], ['ContactsManager', '\OC\ContactsManager'], ['ContactsManager', '\OCP\Contacts\IManager'], + ['CommentsManager', '\OCP\Comments\ICommentsManager'], ['Crypto', '\OC\Security\Crypto'], ['Crypto', '\OCP\Security\ICrypto'], ['CryptoWrapper', '\OC\Session\CryptoWrapper'], From e3dbc3d40ccd9874dadb32b59633cb159afddc48 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 16:35:57 +0100 Subject: [PATCH 02/17] different strategy in cleaning up after user was deleted we do not listen to deletion hooks anymore, because there is no guarantee that they will be heard - requires that something fetches the CommentsManager first. Instead, in the user deletion routine the clean up method will be called directly. Same way as it happens for files, group memberships, config values. --- lib/private/comments/manager.php | 5 ----- lib/private/comments/managerfactory.php | 1 - lib/private/server.php | 3 +++ lib/private/user/user.php | 2 ++ tests/lib/comments/fakefactory.php | 19 +++++++++++++------ tests/lib/comments/manager.php | 19 +++---------------- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 78b2b71c8dd..bb0782c77fd 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -23,15 +23,10 @@ class Manager implements ICommentsManager { public function __construct( IDBConnection $dbConn, - Emitter $userManager, ILogger $logger ) { $this->dbConn = $dbConn; $this->logger = $logger; - $userManager->listen('\OC\User', 'postDelete', function($user) { - /** @var \OCP\IUser $user */ - $this->deleteReferencesOfActor('user', $user->getUid()); - }); } /** diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php index 71d73571b10..0c9fce3e640 100644 --- a/lib/private/comments/managerfactory.php +++ b/lib/private/comments/managerfactory.php @@ -17,7 +17,6 @@ class ManagerFactory implements ICommentsManagerFactory { public function getManager() { return new Manager( \oc::$server->getDatabaseConnection(), - \oc::$server->getUserManager(), \oc::$server->getLogger() ); } diff --git a/lib/private/server.php b/lib/private/server.php index ecac18d6a9b..8439500706d 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -1128,6 +1128,9 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('NotificationManager'); } + /** + * @return \OCP\Comments\ICommentsManager + */ public function getCommentsManager() { return $this->query('CommentsManager'); } diff --git a/lib/private/user/user.php b/lib/private/user/user.php index d827097ee39..6c89dd06f77 100644 --- a/lib/private/user/user.php +++ b/lib/private/user/user.php @@ -189,6 +189,8 @@ class User implements IUser { // Delete the users entry in the storage table \OC\Files\Cache\Storage::remove('home::' . $this->uid); + + \OC::$server->getCommentsManager()->deleteReferencesOfActor('user', $this->uid); } if ($this->emitter) { diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index 202b02f6411..cd85a4f34ca 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -10,13 +10,20 @@ */ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\ICommentsManagerFactory { - public function testNothing() { - // If there would not be at least one test, phpunit would scream failure - // So we have one and skip it. - $this->markTestSkipped(); - } - public function getManager() { return $this->getMock('\OCP\Comments\ICommentsManager'); } + + public function testOverwriteDefaultManager() { + $config = \oc::$server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); + + $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $manager = \oc::$server->getCommentsManager(); + $this->assertEquals($managerMock, $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } } diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 610dfe51a47..35a1c8a2af5 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -475,10 +475,10 @@ class Test_Comments_Manager extends Test\TestCase } public function testDeleteReferencesOfActorWithUserManagement() { - $user = \oc::$server->getUserManager()->createUser('xenia', '123456'); + $user = \OC::$server->getUserManager()->createUser('xenia', '123456'); $this->assertTrue($user instanceof \OCP\IUser); - $manager = $this->getManager(); + $manager = \OC::$server->getCommentsManager(); $comment = $manager->create('user', $user->getUID(), 'file', 'file64'); $comment ->setMessage('Most important comment I ever left on the Internet.') @@ -489,7 +489,7 @@ class Test_Comments_Manager extends Test\TestCase $commentID = $comment->getId(); $user->delete(); - $comment =$manager->get($commentID); + $comment = $manager->get($commentID); $this->assertSame($comment->getActorType(), \OCP\Comments\ICommentsManager::DELETED_USER); $this->assertSame($comment->getActorId(), \OCP\Comments\ICommentsManager::DELETED_USER); } @@ -544,17 +544,4 @@ class Test_Comments_Manager extends Test\TestCase $this->assertTrue($wasSuccessful); } - public function testOverwriteDefaultManager() { - $config = \oc::$server->getConfig(); - $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); - - $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - - $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); - $manager = \oc::$server->getCommentsManager(); - $this->assertEquals($managerMock, $manager); - - $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); - } - } From 4273689e9f37adadef2d514714fadcc38582b87c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:13:18 +0100 Subject: [PATCH 03/17] fix usage of empty --- lib/private/comments/manager.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index bb0782c77fd..12b90a063d1 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -55,11 +55,11 @@ class Manager implements ICommentsManager { * @throws \UnexpectedValueException */ protected function prepareCommentForDatabaseWrite(IComment $comment) { - if( empty($comment->getActorType()) - || empty($comment->getActorId()) - || empty($comment->getObjectType()) - || empty($comment->getObjectId()) - || empty($comment->getVerb()) + if( !$comment->getActorType() + || !$comment->getActorId() + || !$comment->getObjectType() + || !$comment->getObjectId() + || !$comment->getVerb() ) { throw new \UnexpectedValueException('Actor, Object and Verb information must be provided for saving'); } @@ -430,7 +430,7 @@ class Manager implements ICommentsManager { $result = $this->update($comment); } - if($result && !empty($comment->getParentId())) { + if($result && !!$comment->getParentId()) { $this->updateChildrenInformation( $comment->getParentId(), $comment->getCreationDateTime() From 9dc417183077752080799db8d5962ba5147fed29 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:16:51 +0100 Subject: [PATCH 04/17] parameter checks for setting actor and object to happen only in one place --- lib/private/comments/comment.php | 10 ++++++++-- lib/private/comments/manager.php | 11 ----------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php index 2e9e4bd3d84..8efd7d5613a 100644 --- a/lib/private/comments/comment.php +++ b/lib/private/comments/comment.php @@ -229,7 +229,10 @@ class Comment implements IComment { * @since 9.0.0 */ public function setActor($actorType, $actorId) { - if(!is_string($actorType) || !is_string($actorId)) { + if( + !is_string($actorType) || empty($actorType) + || !is_string($actorId) || empty($actorId) + ) { throw new \InvalidArgumentException('String expected.'); } $this->data['actorType'] = $actorType; @@ -312,7 +315,10 @@ class Comment implements IComment { * @since 9.0.0 */ public function setObject($objectType, $objectId) { - if(!is_string($objectType) || !is_string($objectId)) { + if( + !is_string($objectType) || empty($objectType) + || !is_string($objectId) || empty($objectId) + ) { throw new \InvalidArgumentException('String expected.'); } $this->data['objectType'] = $objectType; diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 12b90a063d1..184e37eb12a 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -354,20 +354,9 @@ class Manager implements ICommentsManager { * @param string $objectType the object type the comment is attached to * @param string $objectId the object id the comment is attached to * @return IComment - * @throws \InvalidArgumentException * @since 9.0.0 */ public function create($actorType, $actorId, $objectType, $objectId) { - if( - !is_string($actorType) || empty($actorType) - || !is_string($actorId) || empty($actorId) - || !is_string($objectType) || empty($objectType) - || !is_string($objectId) || empty($objectId) - ) { - // unsure whether it's a good place to enforce it here, since the - // comment instance can be manipulated anyway. - throw new \InvalidArgumentException('All arguments must be non-empty strings'); - } $comment = new Comment(); $comment ->setActor($actorType, $actorId) From 9a440c06b0da6d46417bda3fd6c84d42b69bb328 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:19:40 +0100 Subject: [PATCH 05/17] OC not oc --- lib/private/comments/managerfactory.php | 4 ++-- tests/lib/comments/fakefactory.php | 4 ++-- tests/lib/comments/manager.php | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/private/comments/managerfactory.php b/lib/private/comments/managerfactory.php index 0c9fce3e640..41978d0cf4b 100644 --- a/lib/private/comments/managerfactory.php +++ b/lib/private/comments/managerfactory.php @@ -16,8 +16,8 @@ class ManagerFactory implements ICommentsManagerFactory { */ public function getManager() { return new Manager( - \oc::$server->getDatabaseConnection(), - \oc::$server->getLogger() + \OC::$server->getDatabaseConnection(), + \OC::$server->getLogger() ); } } diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index cd85a4f34ca..15b267b721e 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -15,13 +15,13 @@ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\I } public function testOverwriteDefaultManager() { - $config = \oc::$server->getConfig(); + $config = \OC::$server->getConfig(); $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); - $manager = \oc::$server->getCommentsManager(); + $manager = \OC::$server->getCommentsManager(); $this->assertEquals($managerMock, $manager); $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 35a1c8a2af5..3543393308b 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -13,8 +13,8 @@ class Test_Comments_Manager extends Test\TestCase public function setUp() { parent::setUp(); - $sql = \oc::$server->getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); - \oc::$server->getDatabaseConnection()->prepare($sql)->execute(); + $sql = \OC::$server->getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); + \OC::$server->getDatabaseConnection()->prepare($sql)->execute(); } protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null) { @@ -25,7 +25,7 @@ class Test_Comments_Manager extends Test\TestCase $latestChildDT = new \DateTime('yesterday'); } - $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); $qb ->insert('comments') ->values([ @@ -43,7 +43,7 @@ class Test_Comments_Manager extends Test\TestCase ]) ->execute(); - return \oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + return \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); } protected function getManager() { @@ -69,7 +69,7 @@ class Test_Comments_Manager extends Test\TestCase $creationDT = new \DateTime(); $latestChildDT = new \DateTime('yesterday'); - $qb = \oc::$server->getDatabaseConnection()->getQueryBuilder(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); $qb ->insert('comments') ->values([ @@ -87,7 +87,7 @@ class Test_Comments_Manager extends Test\TestCase ]) ->execute(); - $id = strval(\oc::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + $id = strval(\OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); $comment = $manager->get($id); $this->assertTrue($comment instanceof \OCP\Comments\IComment); From b44a33f68ff705752c55a8572ce7d445336b9080 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:20:40 +0100 Subject: [PATCH 06/17] fix php doc --- lib/public/comments/icomment.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/comments/icomment.php b/lib/public/comments/icomment.php index 8cfc504fc3f..7924ec8d5f6 100644 --- a/lib/public/comments/icomment.php +++ b/lib/public/comments/icomment.php @@ -5,7 +5,7 @@ namespace OCP\Comments; /** * Interface IComment * - * This class represents a comment and offers methods for modification. + * This class represents a comment * * @package OCP\Comments * @since 9.0.0 From d3692c32837a54198f8f8a00713d93dd64db8e61 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 3 Dec 2015 17:23:22 +0100 Subject: [PATCH 07/17] namespaces for tests --- tests/lib/comments/comment.php | 6 +++++- tests/lib/comments/fakefactory.php | 13 ++++++------- tests/lib/comments/manager.php | 5 ++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index 790aca42967..f00dfd527f1 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -1,6 +1,10 @@ getMock('\OCP\Comments\ICommentsManager'); @@ -20,7 +19,7 @@ class Test_Comments_FakeFactory extends Test\TestCase implements \OCP\Comments\I $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - $config->setSystemValue('comments.managerFactory', 'Test_Comments_FakeFactory'); + $config->setSystemValue('comments.managerFactory', '\Test\Comments\Test_Comments_FakeFactory'); $manager = \OC::$server->getCommentsManager(); $this->assertEquals($managerMock, $manager); diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index 3543393308b..d5c415c250a 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -1,13 +1,16 @@ Date: Thu, 3 Dec 2015 21:53:58 +0100 Subject: [PATCH 08/17] rework test about overwriting default comments manager --- tests/lib/comments/fakefactory.php | 21 +++---------------- tests/lib/comments/fakemanager.php | 33 ++++++++++++++++++++++++++++++ tests/lib/server.php | 12 +++++++++++ 3 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 tests/lib/comments/fakemanager.php diff --git a/tests/lib/comments/fakefactory.php b/tests/lib/comments/fakefactory.php index 0fa68e4cb0c..837bcb10585 100644 --- a/tests/lib/comments/fakefactory.php +++ b/tests/lib/comments/fakefactory.php @@ -2,27 +2,12 @@ namespace Test\Comments; -use Test\TestCase; - /** - * Class Test_Comments_FakeFactory + * Class FakeFactory */ -class Test_Comments_FakeFactory extends TestCase implements \OCP\Comments\ICommentsManagerFactory { +class FakeFactory implements \OCP\Comments\ICommentsManagerFactory { public function getManager() { - return $this->getMock('\OCP\Comments\ICommentsManager'); - } - - public function testOverwriteDefaultManager() { - $config = \OC::$server->getConfig(); - $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); - - $managerMock = $this->getMock('\OCP\Comments\ICommentsManager'); - - $config->setSystemValue('comments.managerFactory', '\Test\Comments\Test_Comments_FakeFactory'); - $manager = \OC::$server->getCommentsManager(); - $this->assertEquals($managerMock, $manager); - - $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + return new FakeManager(); } } diff --git a/tests/lib/comments/fakemanager.php b/tests/lib/comments/fakemanager.php new file mode 100644 index 00000000000..a3cd9c0c064 --- /dev/null +++ b/tests/lib/comments/fakemanager.php @@ -0,0 +1,33 @@ +assertInstanceOf('\OC_EventSource', $this->server->createEventSource(), 'service returned by "createEventSource" did not return the right class'); $this->assertInstanceOf('\OCP\IEventSource', $this->server->createEventSource(), 'service returned by "createEventSource" did not return the right class'); } + + public function testOverwriteDefaultCommentsManager() { + $config = $this->server->getConfig(); + $defaultManagerFactory = $config->getSystemValue('comments.managerFactory', '\OC\Comments\ManagerFactory'); + + $config->setSystemValue('comments.managerFactory', '\Test\Comments\FakeFactory'); + + $manager = $this->server->getCommentsManager(); + $this->assertInstanceOf('\OCP\Comments\ICommentsManager', $manager); + + $config->setSystemValue('comments.managerFactory', $defaultManagerFactory); + } } From f9081303b1a2b1a255ec4e869b18d118977f324f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Dec 2015 10:57:31 +0100 Subject: [PATCH 09/17] fix phpdoc --- lib/public/comments/icommentsmanagerfactory.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/public/comments/icommentsmanagerfactory.php b/lib/public/comments/icommentsmanagerfactory.php index 7bfb79aae00..6718dd39ba0 100644 --- a/lib/public/comments/icommentsmanagerfactory.php +++ b/lib/public/comments/icommentsmanagerfactory.php @@ -3,9 +3,10 @@ namespace OCP\Comments; /** - * Interface IComment + * Interface ICommentsManagerFactory * - * This class represents a comment and offers methods for modification. + * This class is responsible for instantiating and returning an ICommentsManager + * instance. * * @package OCP\Comments * @since 9.0.0 From 0c1c0295717f0e75aa725d1c6699a68151f2c758 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Dec 2015 11:13:39 +0100 Subject: [PATCH 10/17] hardening, add some checks for whitespace-only strings --- lib/private/comments/comment.php | 29 +++++++++++++++-------------- tests/lib/comments/comment.php | 18 ++++++++++-------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php index 8efd7d5613a..15d721d099a 100644 --- a/lib/private/comments/comment.php +++ b/lib/private/comments/comment.php @@ -66,6 +66,7 @@ class Comment implements IComment { throw new \InvalidArgumentException('String expected.'); } + $id = trim($id); if($this->data['id'] === '' || ($this->data['id'] !== '' && $id === '')) { $this->data['id'] = $id; return $this; @@ -95,7 +96,7 @@ class Comment implements IComment { if(!is_string($parentId)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['parentId'] = $parentId; + $this->data['parentId'] = trim($parentId); return $this; } @@ -121,7 +122,7 @@ class Comment implements IComment { if(!is_string($id)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['topmostParentId'] = $id; + $this->data['topmostParentId'] = trim($id); return $this; } @@ -171,7 +172,7 @@ class Comment implements IComment { if(!is_string($message)) { throw new \InvalidArgumentException('String expected.'); } - $this->data['message'] = $message; + $this->data['message'] = trim($message); return $this; } @@ -193,10 +194,10 @@ class Comment implements IComment { * @since 9.0.0 */ public function setVerb($verb) { - if(!is_string($verb)) { - throw new \InvalidArgumentException('String expected.'); + if(!is_string($verb) || empty(trim($verb))) { + throw new \InvalidArgumentException('Non-empty String expected.'); } - $this->data['verb'] = $verb; + $this->data['verb'] = trim($verb); return $this; } @@ -230,13 +231,13 @@ class Comment implements IComment { */ public function setActor($actorType, $actorId) { if( - !is_string($actorType) || empty($actorType) - || !is_string($actorId) || empty($actorId) + !is_string($actorType) || empty(trim($actorType)) + || !is_string($actorId) || empty(trim($actorId)) ) { throw new \InvalidArgumentException('String expected.'); } - $this->data['actorType'] = $actorType; - $this->data['actorId'] = $actorId; + $this->data['actorType'] = trim($actorType); + $this->data['actorId'] = trim($actorId); return $this; } @@ -316,13 +317,13 @@ class Comment implements IComment { */ public function setObject($objectType, $objectId) { if( - !is_string($objectType) || empty($objectType) - || !is_string($objectId) || empty($objectId) + !is_string($objectType) || empty(trim($objectType)) + || !is_string($objectId) || empty(trim($objectId)) ) { throw new \InvalidArgumentException('String expected.'); } - $this->data['objectType'] = $objectType; - $this->data['objectId'] = $objectId; + $this->data['objectType'] = trim($objectType); + $this->data['objectId'] = trim($objectId); return $this; } diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index f00dfd527f1..c6a8f118dd1 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -60,24 +60,24 @@ class Test_Comments_Comment extends TestCase public function simpleSetterProvider() { return [ - ['Id'], - ['ParentId'], - ['Message'], - ['Verb'], - ['ChildrenCount'], + ['Id', true], + ['ParentId', true], + ['Message', true], + ['Verb', true], + ['Verb', ''], + ['ChildrenCount', true], ]; } /** * @dataProvider simpleSetterProvider */ - public function testSimpleSetterInvalidInput($field) { + public function testSimpleSetterInvalidInput($field, $input) { $comment = new \OC\Comments\Comment(); $setter = 'set' . $field; $this->setExpectedException('InvalidArgumentException'); - // we have no field that is supposed to accept a Bool - $comment->$setter(true); + $comment->$setter($input); } public function roleSetterProvider() { @@ -85,9 +85,11 @@ class Test_Comments_Comment extends TestCase ['Actor', true, true], ['Actor', 'user', true], ['Actor', true, 'alice'], + ['Actor', ' ', ' '], ['Object', true, true], ['Object', 'file', true], ['Object', true, 'file64'], + ['Object', ' ', ' '], ]; } From dec1f1d24a9bcb710ca75b83d90251f1fe53967e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Dec 2015 14:57:55 +0100 Subject: [PATCH 11/17] anounce CommentsManager getter in public server interface --- lib/public/iservercontainer.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/public/iservercontainer.php b/lib/public/iservercontainer.php index 7cb2672254b..267e5dc4d31 100644 --- a/lib/public/iservercontainer.php +++ b/lib/public/iservercontainer.php @@ -471,6 +471,12 @@ interface IServerContainer { */ public function getNotificationManager(); + /** + * @return \OCP\Comments\ICommentsManager + * @since 9.0.0 + */ + public function getCommentsManager(); + /** * Returns the system-tag manager * From 249dc4490f55c0a40ec8af0800bbce146b6e5eac Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 8 Dec 2015 14:58:18 +0100 Subject: [PATCH 12/17] improve PHP doc and remove superflous by reference indicator --- lib/private/comments/manager.php | 10 +++++----- lib/public/comments/icommentsmanager.php | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 184e37eb12a..a2b55c4ffdd 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -51,7 +51,8 @@ class Manager implements ICommentsManager { * all necessary fields have a value assigned. * * @param IComment $comment - * @return IComment + * @return IComment returns the same updated IComment instance as provided + * by parameter for convenience * @throws \UnexpectedValueException */ protected function prepareCommentForDatabaseWrite(IComment $comment) { @@ -406,14 +407,13 @@ class Manager implements ICommentsManager { * Throws NotFoundException when a comment that is to be updated does not * exist anymore at this point of time. * - * @param IComment &$comment + * @param IComment $comment * @return bool * @throws NotFoundException * @since 9.0.0 */ - public function save(IComment &$comment) { - $comment = $this->prepareCommentForDatabaseWrite($comment); - if($comment->getId() === '') { + public function save(IComment $comment) { + if($this->prepareCommentForDatabaseWrite($comment)->getId() === '') { $result = $this->insert($comment); } else { $result = $this->update($comment); diff --git a/lib/public/comments/icommentsmanager.php b/lib/public/comments/icommentsmanager.php index f6883224d60..7626ffd6351 100644 --- a/lib/public/comments/icommentsmanager.php +++ b/lib/public/comments/icommentsmanager.php @@ -139,12 +139,12 @@ interface ICommentsManager { * Throws NotFoundException when a comment that is to be updated does not * exist anymore at this point of time. * - * @param IComment &$comment + * @param IComment $comment * @return bool * @throws NotFoundException * @since 9.0.0 */ - public function save(IComment &$comment); + public function save(IComment $comment); /** * removes references to specific actor (e.g. on user delete) of a comment. From 8e298f51f84f0cca7f3ae51fa095bcabd06e8cdd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 14:38:01 +0100 Subject: [PATCH 13/17] adjust test's fakemanager to interface change --- tests/lib/comments/fakemanager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/comments/fakemanager.php b/tests/lib/comments/fakemanager.php index a3cd9c0c064..e5cf58dda4f 100644 --- a/tests/lib/comments/fakemanager.php +++ b/tests/lib/comments/fakemanager.php @@ -25,7 +25,7 @@ class FakeManager implements \OCP\Comments\ICommentsManager { public function delete($id) {} - public function save(\OCP\Comments\IComment &$comment) {} + public function save(\OCP\Comments\IComment $comment) {} public function deleteReferencesOfActor($actorType, $actorId) {} From 6af6febad05a4619a6482536c1bddd2fe1925148 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:25:31 +0100 Subject: [PATCH 14/17] php < 5.5 compatible --- lib/private/comments/comment.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php index 15d721d099a..219e7ec8e4b 100644 --- a/lib/private/comments/comment.php +++ b/lib/private/comments/comment.php @@ -194,7 +194,7 @@ class Comment implements IComment { * @since 9.0.0 */ public function setVerb($verb) { - if(!is_string($verb) || empty(trim($verb))) { + if(!is_string($verb) || !trim($verb)) { throw new \InvalidArgumentException('Non-empty String expected.'); } $this->data['verb'] = trim($verb); @@ -231,8 +231,8 @@ class Comment implements IComment { */ public function setActor($actorType, $actorId) { if( - !is_string($actorType) || empty(trim($actorType)) - || !is_string($actorId) || empty(trim($actorId)) + !is_string($actorType) || !trim($actorType) + || !is_string($actorId) || !trim($actorId) ) { throw new \InvalidArgumentException('String expected.'); } @@ -317,8 +317,8 @@ class Comment implements IComment { */ public function setObject($objectType, $objectId) { if( - !is_string($objectType) || empty(trim($objectType)) - || !is_string($objectId) || empty(trim($objectId)) + !is_string($objectType) || !trim($objectType) + || !is_string($objectId) || !trim($objectId) ) { throw new \InvalidArgumentException('String expected.'); } From 55a2715eff10e730c8104acc09b122893160cadd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:25:42 +0100 Subject: [PATCH 15/17] remove unused use statement --- lib/private/comments/manager.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index a2b55c4ffdd..05f981f244d 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -3,7 +3,6 @@ namespace OC\Comments; use Doctrine\DBAL\Exception\DriverException; -use OC\Hooks\Emitter; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; use OCP\Comments\NotFoundException; From fdd06ba1f81d07e4597beb0b5b17f44c296b5921 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:33:34 +0100 Subject: [PATCH 16/17] use getLastInsertId from query builder for convenience --- lib/private/comments/manager.php | 2 +- tests/lib/comments/manager.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/comments/manager.php b/lib/private/comments/manager.php index 05f981f244d..09e59f28370 100644 --- a/lib/private/comments/manager.php +++ b/lib/private/comments/manager.php @@ -455,7 +455,7 @@ class Manager implements ICommentsManager { ->execute(); if ($affectedRows > 0) { - $comment->setId(strval($this->dbConn->lastInsertId('*PREFIX*comments'))); + $comment->setId(strval($qb->getLastInsertId())); } return $affectedRows > 0; diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index d5c415c250a..c8d65c951e3 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -46,7 +46,7 @@ class Test_Comments_Manager extends TestCase ]) ->execute(); - return \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments'); + return $qb->getLastInsertId(); } protected function getManager() { @@ -90,7 +90,7 @@ class Test_Comments_Manager extends TestCase ]) ->execute(); - $id = strval(\OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*comments')); + $id = strval($qb->getLastInsertId()); $comment = $manager->get($id); $this->assertTrue($comment instanceof \OCP\Comments\IComment); From 81d763be4224adeb5676c620d76b3e4ec407187f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 9 Dec 2015 16:55:07 +0100 Subject: [PATCH 17/17] use expectedException annotation, not via method call, for consistency --- tests/lib/comments/comment.php | 8 +++++--- tests/lib/comments/manager.php | 34 ++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index c6a8f118dd1..ae7e913eaab 100644 --- a/tests/lib/comments/comment.php +++ b/tests/lib/comments/comment.php @@ -44,10 +44,12 @@ class Test_Comments_Comment extends TestCase $this->assertSame($object['id'], $comment->getObjectId()); } + /** + * @expectedException \OCP\Comments\IllegalIDChangeException + */ public function testSetIdIllegalInput() { $comment = new \OC\Comments\Comment(); - $this->setExpectedException('\OCP\Comments\IllegalIDChangeException'); $comment->setId('c23'); $comment->setId('c17'); } @@ -71,12 +73,12 @@ class Test_Comments_Comment extends TestCase /** * @dataProvider simpleSetterProvider + * @expectedException \InvalidArgumentException */ public function testSimpleSetterInvalidInput($field, $input) { $comment = new \OC\Comments\Comment(); $setter = 'set' . $field; - $this->setExpectedException('InvalidArgumentException'); $comment->$setter($input); } @@ -95,11 +97,11 @@ class Test_Comments_Comment extends TestCase /** * @dataProvider roleSetterProvider + * @expectedException \InvalidArgumentException */ public function testSetRoleInvalidInput($role, $type, $id){ $comment = new \OC\Comments\Comment(); $setter = 'set' . $role; - $this->setExpectedException('InvalidArgumentException'); $comment->$setter($type, $id); } diff --git a/tests/lib/comments/manager.php b/tests/lib/comments/manager.php index c8d65c951e3..248de683253 100644 --- a/tests/lib/comments/manager.php +++ b/tests/lib/comments/manager.php @@ -54,15 +54,19 @@ class Test_Comments_Manager extends TestCase return $factory->getManager(); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testGetCommentNotFound() { $manager = $this->getManager(); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->get('22'); } + /** + * @expectedException \InvalidArgumentException + */ public function testGetCommentNotFoundInvalidInput() { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->get('unexisting22'); } @@ -108,15 +112,19 @@ class Test_Comments_Manager extends TestCase $this->assertEquals($comment->getLatestChildDateTime(), $latestChildDT); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testGetTreeNotFound() { $manager = $this->getManager(); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->getTree('22'); } + /** + * @expectedException \InvalidArgumentException + */ public function testGetTreeNotFoundInvalidIpnut() { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->getTree('unexisting22'); } @@ -301,10 +309,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidCreateArgsProvider + * @expectedException \InvalidArgumentException */ public function testCreateCommentInvalidArguments($aType, $aId, $oType, $oId) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->create($aType, $aId, $oType, $oId); } @@ -322,6 +330,9 @@ class Test_Comments_Manager extends TestCase $this->assertSame($comment->getObjectId(), $objectId); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testDelete() { $manager = $this->getManager(); @@ -339,7 +350,6 @@ class Test_Comments_Manager extends TestCase $this->assertTrue($comment instanceof \OCP\Comments\IComment); $done = $manager->delete($id); $this->assertTrue($done); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->get($id); } @@ -381,6 +391,9 @@ class Test_Comments_Manager extends TestCase $this->assertSame($comment->getMessage(), $loadedComment->getMessage()); } + /** + * @expectedException \OCP\Comments\NotFoundException + */ public function testSaveUpdateException() { $manager = $this->getManager(); $comment = new \OC\Comments\Comment(); @@ -394,15 +407,16 @@ class Test_Comments_Manager extends TestCase $manager->delete($comment->getId()); $comment->setMessage('very beautiful, I am really so much impressed!'); - $this->setExpectedException('\OCP\Comments\NotFoundException'); $manager->save($comment); } + /** + * @expectedException \UnexpectedValueException + */ public function testSaveIncomplete() { $manager = $this->getManager(); $comment = new \OC\Comments\Comment(); $comment->setMessage('from no one to nothing'); - $this->setExpectedException('\UnexpectedValueException'); $manager->save($comment); } @@ -442,10 +456,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidActorArgsProvider + * @expectedException \InvalidArgumentException */ public function testDeleteReferencesOfActorInvalidInput($type, $id) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->deleteReferencesOfActor($type, $id); } @@ -508,10 +522,10 @@ class Test_Comments_Manager extends TestCase /** * @dataProvider invalidObjectArgsProvider + * @expectedException \InvalidArgumentException */ public function testDeleteCommentsAtObjectInvalidInput($type, $id) { $manager = $this->getManager(); - $this->setExpectedException('\InvalidArgumentException'); $manager->deleteCommentsAtObject($type, $id); }