From 9725d0032944ebe5a73d59781a66742c5d1ae404 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 25 Nov 2022 11:19:47 +0100 Subject: [PATCH 1/3] Cleanup comments code - Fix various psalm issues - Add as much typing as possible while preserving stable API Signed-off-by: Carl Schwan --- apps/comments/lib/Activity/Filter.php | 38 ++--- apps/comments/lib/Activity/Listener.php | 5 +- apps/comments/lib/Activity/Provider.php | 2 +- apps/comments/lib/Activity/Setting.php | 51 ++----- .../lib/Collaboration/CommentersSorter.php | 10 +- apps/comments/lib/EventHandler.php | 22 +-- .../Listener/CommentsEntityEventListener.php | 13 +- .../MaxAutoCompleteResultsInitialState.php | 4 +- apps/comments/lib/Notification/Listener.php | 23 +-- .../lib/Search/CommentsSearchProvider.php | 13 -- apps/comments/lib/Search/Result.php | 11 +- apps/dav/lib/Comments/CommentNode.php | 4 +- apps/dav/lib/Comments/CommentsPlugin.php | 4 +- .../dav/lib/Comments/EntityTypeCollection.php | 17 +-- apps/dav/lib/Comments/RootCollection.php | 28 ++-- lib/private/Comments/Comment.php | 133 +++++++----------- lib/private/Comments/Manager.php | 102 ++++---------- lib/public/Comments/ICommentsManager.php | 12 +- 18 files changed, 147 insertions(+), 345 deletions(-) diff --git a/apps/comments/lib/Activity/Filter.php b/apps/comments/lib/Activity/Filter.php index efd8d5140aeec..2b6e898f59955 100644 --- a/apps/comments/lib/Activity/Filter.php +++ b/apps/comments/lib/Activity/Filter.php @@ -27,64 +27,42 @@ use OCP\IURLGenerator; class Filter implements IFilter { - - /** @var IL10N */ - protected $l; - - /** @var IURLGenerator */ - protected $url; + protected IL10N $l; + protected IURLGenerator $url; public function __construct(IL10N $l, IURLGenerator $url) { $this->l = $l; $this->url = $url; } - /** - * @return string Lowercase a-z only identifier - * @since 11.0.0 - */ - public function getIdentifier() { + public function getIdentifier(): string { return 'comments'; } - /** - * @return string A translated string - * @since 11.0.0 - */ - public function getName() { + public function getName(): string { return $this->l->t('Comments'); } - /** - * @return int - * @since 11.0.0 - */ - public function getPriority() { + public function getPriority(): int { return 40; } - /** - * @return string Full URL to an icon, empty string when none is given - * @since 11.0.0 - */ - public function getIcon() { + public function getIcon(): string { return $this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.svg')); } /** * @param string[] $types * @return string[] An array of allowed apps from which activities should be displayed - * @since 11.0.0 */ - public function filterTypes(array $types) { + public function filterTypes(array $types): array { return $types; } /** * @return string[] An array of allowed apps from which activities should be displayed - * @since 11.0.0 */ - public function allowedApps() { + public function allowedApps(): array { return ['comments']; } } diff --git a/apps/comments/lib/Activity/Listener.php b/apps/comments/lib/Activity/Listener.php index 69315b6ac004a..a066ccdefc11c 100644 --- a/apps/comments/lib/Activity/Listener.php +++ b/apps/comments/lib/Activity/Listener.php @@ -59,10 +59,7 @@ public function __construct(IManager $activityManager, $this->shareHelper = $shareHelper; } - /** - * @param CommentsEvent $event - */ - public function commentEvent(CommentsEvent $event) { + public function commentEvent(CommentsEvent $event): void { if ($event->getComment()->getObjectType() !== 'files' || $event->getEvent() !== CommentsEvent::EVENT_ADD || !$this->appManager->isInstalled('activity')) { diff --git a/apps/comments/lib/Activity/Provider.php b/apps/comments/lib/Activity/Provider.php index f9a5971c7f359..727f6d66b507c 100644 --- a/apps/comments/lib/Activity/Provider.php +++ b/apps/comments/lib/Activity/Provider.php @@ -58,7 +58,7 @@ public function __construct(IFactory $languageFactory, IURLGenerator $url, IComm * @throws \InvalidArgumentException * @since 11.0.0 */ - public function parse($language, IEvent $event, IEvent $previousEvent = null) { + public function parse($language, IEvent $event, IEvent $previousEvent = null): IEvent { if ($event->getApp() !== 'comments') { throw new \InvalidArgumentException(); } diff --git a/apps/comments/lib/Activity/Setting.php b/apps/comments/lib/Activity/Setting.php index c0d91c244a699..3352bf2a59bfe 100644 --- a/apps/comments/lib/Activity/Setting.php +++ b/apps/comments/lib/Activity/Setting.php @@ -26,72 +26,37 @@ use OCP\IL10N; class Setting implements ISetting { + protected IL10N $l; - /** @var IL10N */ - protected $l; - - /** - * @param IL10N $l - */ public function __construct(IL10N $l) { $this->l = $l; } - /** - * @return string Lowercase a-z and underscore only identifier - * @since 11.0.0 - */ - public function getIdentifier() { + public function getIdentifier(): string { return 'comments'; } - /** - * @return string A translated string - * @since 11.0.0 - */ - public function getName() { + public function getName(): string { return $this->l->t('Comments for files'); } - /** - * @return int whether the filter should be rather on the top or bottom of - * the admin section. The filters are arranged in ascending order of the - * priority values. It is required to return a value between 0 and 100. - * @since 11.0.0 - */ - public function getPriority() { + public function getPriority(): int { return 50; } - /** - * @return bool True when the option can be changed for the stream - * @since 11.0.0 - */ - public function canChangeStream() { + public function canChangeStream(): bool { return true; } - /** - * @return bool True when the option can be changed for the stream - * @since 11.0.0 - */ - public function isDefaultEnabledStream() { + public function isDefaultEnabledStream(): bool { return true; } - /** - * @return bool True when the option can be changed for the mail - * @since 11.0.0 - */ - public function canChangeMail() { + public function canChangeMail(): bool { return true; } - /** - * @return bool True when the option can be changed for the stream - * @since 11.0.0 - */ - public function isDefaultEnabledMail() { + public function isDefaultEnabledMail(): bool { return false; } } diff --git a/apps/comments/lib/Collaboration/CommentersSorter.php b/apps/comments/lib/Collaboration/CommentersSorter.php index 8723b132e0384..b7c993b4f2017 100644 --- a/apps/comments/lib/Collaboration/CommentersSorter.php +++ b/apps/comments/lib/Collaboration/CommentersSorter.php @@ -27,14 +27,13 @@ use OCP\Comments\ICommentsManager; class CommentersSorter implements ISorter { - private ICommentsManager $commentsManager; public function __construct(ICommentsManager $commentsManager) { $this->commentsManager = $commentsManager; } - public function getId() { + public function getId(): string { return 'commenters'; } @@ -42,10 +41,10 @@ public function getId() { * Sorts people who commented on the given item atop (descelating) of the * others * - * @param array $sortArray + * @param array &$sortArray * @param array $context */ - public function sort(array &$sortArray, array $context) { + public function sort(array &$sortArray, array $context): void { $commenters = $this->retrieveCommentsInformation($context['itemType'], $context['itemId']); if (count($commenters) === 0) { return; @@ -76,6 +75,9 @@ public function sort(array &$sortArray, array $context) { } } + /** + * @return array> + */ protected function retrieveCommentsInformation(string $type, string $id): array { $comments = $this->commentsManager->getForObject($type, $id); if (count($comments) === 0) { diff --git a/apps/comments/lib/EventHandler.php b/apps/comments/lib/EventHandler.php index 6027a24b0269f..722e3b8cc7231 100644 --- a/apps/comments/lib/EventHandler.php +++ b/apps/comments/lib/EventHandler.php @@ -34,21 +34,15 @@ * @package OCA\Comments */ class EventHandler implements ICommentsEventHandler { - /** @var ActivityListener */ - private $activityListener; - - /** @var NotificationListener */ - private $notificationListener; + private ActivityListener $activityListener; + private NotificationListener $notificationListener; public function __construct(ActivityListener $activityListener, NotificationListener $notificationListener) { $this->activityListener = $activityListener; $this->notificationListener = $notificationListener; } - /** - * @param CommentsEvent $event - */ - public function handle(CommentsEvent $event) { + public function handle(CommentsEvent $event): void { if ($event->getComment()->getObjectType() !== 'files') { // this is a 'files'-specific Handler return; @@ -73,17 +67,11 @@ public function handle(CommentsEvent $event) { } } - /** - * @param CommentsEvent $event - */ - private function activityHandler(CommentsEvent $event) { + private function activityHandler(CommentsEvent $event): void { $this->activityListener->commentEvent($event); } - /** - * @param CommentsEvent $event - */ - private function notificationHandler(CommentsEvent $event) { + private function notificationHandler(CommentsEvent $event): void { $this->notificationListener->evaluate($event); } } diff --git a/apps/comments/lib/Listener/CommentsEntityEventListener.php b/apps/comments/lib/Listener/CommentsEntityEventListener.php index 5675e1904cc64..b49304c409b22 100644 --- a/apps/comments/lib/Listener/CommentsEntityEventListener.php +++ b/apps/comments/lib/Listener/CommentsEntityEventListener.php @@ -28,16 +28,25 @@ use OCP\Comments\CommentsEntityEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Files\IRootFolder; class CommentsEntityEventListener implements IEventListener { + private IRootFolder $rootFolder; + private ?string $userId; + + public function __construct(IRootFolder $rootFolder, ?string $userId = null) { + $this->rootFolder = $rootFolder; + $this->userId = $userId; + } + public function handle(Event $event): void { if (!($event instanceof CommentsEntityEvent)) { // Unrelated return; } - $event->addEntityCollection('files', function ($name) { - $nodes = \OC::$server->getUserFolder()->getById((int)$name); + $event->addEntityCollection('files', function ($name): bool { + $nodes = $this->rootFolder->getUserFolder($this->userId)->getById((int)$name); return !empty($nodes); }); } diff --git a/apps/comments/lib/MaxAutoCompleteResultsInitialState.php b/apps/comments/lib/MaxAutoCompleteResultsInitialState.php index 015eaf19d84eb..cf7335e84c8b6 100644 --- a/apps/comments/lib/MaxAutoCompleteResultsInitialState.php +++ b/apps/comments/lib/MaxAutoCompleteResultsInitialState.php @@ -31,9 +31,7 @@ use OCP\IConfig; class MaxAutoCompleteResultsInitialState extends InitialStateProvider { - - /** @var IConfig */ - private $config; + private IConfig $config; public function __construct(IConfig $config) { $this->config = $config; diff --git a/apps/comments/lib/Notification/Listener.php b/apps/comments/lib/Notification/Listener.php index d1662f842662b..bbea310ef7e90 100644 --- a/apps/comments/lib/Notification/Listener.php +++ b/apps/comments/lib/Notification/Listener.php @@ -27,15 +27,12 @@ use OCP\Comments\IComment; use OCP\IUserManager; use OCP\Notification\IManager; +use OCP\Notification\INotification; class Listener { - protected IManager $notificationManager; protected IUserManager $userManager; - /** - * Listener constructor. - */ public function __construct( IManager $notificationManager, IUserManager $userManager @@ -44,10 +41,7 @@ public function __construct( $this->userManager = $userManager; } - /** - * @param CommentsEvent $event - */ - public function evaluate(CommentsEvent $event) { + public function evaluate(CommentsEvent $event): void { $comment = $event->getComment(); $mentions = $this->extractMentions($comment->getMentions()); @@ -77,12 +71,9 @@ public function evaluate(CommentsEvent $event) { } /** - * creates a notification instance and fills it with comment data - * - * @param IComment $comment - * @return \OCP\Notification\INotification + * Creates a notification instance and fills it with comment data */ - public function instantiateNotification(IComment $comment) { + public function instantiateNotification(IComment $comment): INotification { $notification = $this->notificationManager->createNotification(); $notification ->setApp('comments') @@ -94,12 +85,12 @@ public function instantiateNotification(IComment $comment) { } /** - * flattens the mention array returned from comments to a list of user ids. + * Flattens the mention array returned from comments to a list of user ids. * * @param array $mentions - * @return string[] containing the mentions, e.g. ['alice', 'bob'] + * @return list containing the mentions, e.g. ['alice', 'bob'] */ - public function extractMentions(array $mentions) { + public function extractMentions(array $mentions): array { if (empty($mentions)) { return []; } diff --git a/apps/comments/lib/Search/CommentsSearchProvider.php b/apps/comments/lib/Search/CommentsSearchProvider.php index b36f82f84012c..cca01c8ac44ed 100644 --- a/apps/comments/lib/Search/CommentsSearchProvider.php +++ b/apps/comments/lib/Search/CommentsSearchProvider.php @@ -39,7 +39,6 @@ use function pathinfo; class CommentsSearchProvider implements IProvider { - private IUserManager $userManager; private IL10N $l10n; private IURLGenerator $urlGenerator; @@ -55,23 +54,14 @@ public function __construct(IUserManager $userManager, $this->legacyProvider = $legacyProvider; } - /** - * @inheritDoc - */ public function getId(): string { return 'comments'; } - /** - * @inheritDoc - */ public function getName(): string { return $this->l10n->t('Comments'); } - /** - * @inheritDoc - */ public function getOrder(string $route, array $routeParameters): int { if ($route === 'files.View.index') { // Files first @@ -80,9 +70,6 @@ public function getOrder(string $route, array $routeParameters): int { return 10; } - /** - * @inheritDoc - */ public function search(IUser $user, ISearchQuery $query): SearchResult { return SearchResult::complete( $this->l10n->t('Comments'), diff --git a/apps/comments/lib/Search/Result.php b/apps/comments/lib/Search/Result.php index ec799b7e30a57..06016d2b46d13 100644 --- a/apps/comments/lib/Search/Result.php +++ b/apps/comments/lib/Search/Result.php @@ -58,10 +58,6 @@ class Result extends BaseResult { public $fileName; /** - * @param string $search - * @param IComment $comment - * @param string $authorName - * @param string $path * @throws NotFoundException * @deprecated 20.0.0 */ @@ -70,7 +66,7 @@ public function __construct(string $search, string $authorName, string $path) { parent::__construct( - (int) $comment->getId(), + $comment->getId(), $comment->getMessage() /* @todo , [link to file] */ ); @@ -83,8 +79,6 @@ public function __construct(string $search, } /** - * @param string $path - * @return string * @throws NotFoundException */ protected function getVisiblePath(string $path): string { @@ -98,9 +92,6 @@ protected function getVisiblePath(string $path): string { } /** - * @param string $message - * @param string $search - * @return string * @throws NotFoundException */ protected function getRelevantMessagePart(string $message, string $search): string { diff --git a/apps/dav/lib/Comments/CommentNode.php b/apps/dav/lib/Comments/CommentNode.php index b41dbc276e895..690d5d6c70960 100644 --- a/apps/dav/lib/Comments/CommentNode.php +++ b/apps/dav/lib/Comments/CommentNode.php @@ -165,10 +165,8 @@ public function setName($name) { /** * Returns the last modification time, as a unix timestamp - * - * @return int */ - public function getLastModified() { + public function getLastModified(): ?int { return null; } diff --git a/apps/dav/lib/Comments/CommentsPlugin.php b/apps/dav/lib/Comments/CommentsPlugin.php index f31e479c2120a..96e0285018d10 100644 --- a/apps/dav/lib/Comments/CommentsPlugin.php +++ b/apps/dav/lib/Comments/CommentsPlugin.php @@ -176,7 +176,7 @@ public function onReport($reportName, $report, $uri) { } if (!is_null($args['datetime'])) { - $args['datetime'] = new \DateTime($args['datetime']); + $args['datetime'] = new \DateTime((string)$args['datetime']); } $results = $node->findChildren($args['limit'], $args['offset'], $args['datetime']); @@ -189,7 +189,7 @@ public function onReport($reportName, $report, $uri) { $responses[] = new Response( $this->server->getBaseUri() . $nodePath, [200 => $resultSet[0][200]], - 200 + '200' ); } } diff --git a/apps/dav/lib/Comments/EntityTypeCollection.php b/apps/dav/lib/Comments/EntityTypeCollection.php index d140a33ec4c01..32adcee54f092 100644 --- a/apps/dav/lib/Comments/EntityTypeCollection.php +++ b/apps/dav/lib/Comments/EntityTypeCollection.php @@ -42,25 +42,14 @@ * @package OCA\DAV\Comments */ class EntityTypeCollection extends RootCollection { - protected LoggerInterface $logger; - - /** @var IUserManager */ - protected $userManager; + protected IUserManager $userManager; /** @var \Closure */ protected $childExistsFunction; - /** - * @param string $name - * @param ICommentsManager $commentsManager - * @param IUserManager $userManager - * @param IUserSession $userSession - * @param LoggerInterface $logger - * @param \Closure $childExistsFunction - */ public function __construct( - $name, + string $name, ICommentsManager $commentsManager, IUserManager $userManager, IUserSession $userSession, @@ -68,7 +57,7 @@ public function __construct( \Closure $childExistsFunction ) { $name = trim($name); - if (empty($name) || !is_string($name)) { + if (empty($name)) { throw new \InvalidArgumentException('"name" parameter must be non-empty string'); } $this->name = $name; diff --git a/apps/dav/lib/Comments/RootCollection.php b/apps/dav/lib/Comments/RootCollection.php index 9832030291e86..39d644b476690 100644 --- a/apps/dav/lib/Comments/RootCollection.php +++ b/apps/dav/lib/Comments/RootCollection.php @@ -36,26 +36,14 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; class RootCollection implements ICollection { - /** @var EntityTypeCollection[]|null */ - private $entityTypeCollections; - - /** @var ICommentsManager */ - protected $commentsManager; - - /** @var string */ - protected $name = 'comments'; - + private ?array $entityTypeCollections = null; + protected ICommentsManager $commentsManager; + protected string $name = 'comments'; protected LoggerInterface $logger; - - /** @var IUserManager */ - protected $userManager; - - /** @var IUserSession */ - protected $userSession; - - /** @var EventDispatcherInterface */ - protected $dispatcher; + protected IUserManager $userManager; + protected IUserSession $userSession; + protected EventDispatcherInterface $dispatcher; public function __construct( ICommentsManager $commentsManager, @@ -149,6 +137,7 @@ public function getChild($name) { */ public function getChildren() { $this->initCollections(); + assert(!is_null($this->entityTypeCollections)); return $this->entityTypeCollections; } @@ -160,6 +149,7 @@ public function getChildren() { */ public function childExists($name) { $this->initCollections(); + assert(!is_null($this->entityTypeCollections)); return isset($this->entityTypeCollections[$name]); } @@ -196,7 +186,7 @@ public function setName($name) { /** * Returns the last modification time, as a unix timestamp * - * @return int + * @return ?int */ public function getLastModified() { return null; diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index 4284beb3fcc3e..af2c73c770bf5 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -30,7 +30,7 @@ use OCP\Comments\MessageTooLongException; class Comment implements IComment { - protected $data = [ + protected array $data = [ 'id' => '', 'parentId' => '0', 'topmostParentId' => '0', @@ -61,21 +61,20 @@ public function __construct(array $data = null) { } /** - * returns the ID of the comment + * 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() { + public function getId(): string { return $this->data['id']; } /** - * sets the ID of the comment and returns itself + * 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 @@ -87,7 +86,7 @@ public function getId() { * @throws IllegalIDChangeException * @since 9.0.0 */ - public function setId($id) { + public function setId($id): IComment { if (!is_string($id)) { throw new \InvalidArgumentException('String expected.'); } @@ -102,23 +101,21 @@ public function setId($id) { } /** - * returns the parent ID of the comment + * Returns the parent ID of the comment * - * @return string * @since 9.0.0 */ - public function getParentId() { + public function getParentId(): string { return $this->data['parentId']; } /** - * sets the parent ID and returns itself + * Sets the parent ID and returns itself * * @param string $parentId - * @return IComment * @since 9.0.0 */ - public function setParentId($parentId) { + public function setParentId($parentId): IComment { if (!is_string($parentId)) { throw new \InvalidArgumentException('String expected.'); } @@ -127,24 +124,22 @@ public function setParentId($parentId) { } /** - * returns the topmost parent ID of the comment + * Returns the topmost parent ID of the comment * - * @return string * @since 9.0.0 */ - public function getTopmostParentId() { + public function getTopmostParentId(): string { return $this->data['topmostParentId']; } /** - * sets the topmost parent ID and returns itself + * Sets the topmost parent ID and returns itself * * @param string $id - * @return IComment * @since 9.0.0 */ - public function setTopmostParentId($id) { + public function setTopmostParentId($id): IComment { if (!is_string($id)) { throw new \InvalidArgumentException('String expected.'); } @@ -153,23 +148,21 @@ public function setTopmostParentId($id) { } /** - * returns the number of children + * Returns the number of children * - * @return int * @since 9.0.0 */ - public function getChildrenCount() { + public function getChildrenCount(): int { return $this->data['childrenCount']; } /** - * sets the number of children + * Sets the number of children * * @param int $count - * @return IComment * @since 9.0.0 */ - public function setChildrenCount($count) { + public function setChildrenCount($count): IComment { if (!is_int($count)) { throw new \InvalidArgumentException('Integer expected.'); } @@ -178,12 +171,10 @@ public function setChildrenCount($count) { } /** - * returns the message of the comment - * - * @return string + * Returns the message of the comment * @since 9.0.0 */ - public function getMessage() { + public function getMessage(): string { return $this->data['message']; } @@ -192,11 +183,10 @@ public function getMessage() { * * @param string $message * @param int $maxLength - * @return IComment * @throws MessageTooLongException * @since 9.0.0 */ - public function setMessage($message, $maxLength = self::MAX_MESSAGE_LENGTH) { + public function setMessage($message, $maxLength = self::MAX_MESSAGE_LENGTH): IComment { if (!is_string($message)) { throw new \InvalidArgumentException('String expected.'); } @@ -228,9 +218,9 @@ public function setMessage($message, $maxLength = self::MAX_MESSAGE_LENGTH) { * ] * */ - public function getMentions() { + public function getMentions(): array { $ok = preg_match_all("/\B(?getMessage(), $mentions); - if (!$ok || !isset($mentions[0]) || !is_array($mentions[0])) { + if (!$ok || !isset($mentions[0])) { return []; } $mentionIds = array_unique($mentions[0]); @@ -252,23 +242,21 @@ public function getMentions() { } /** - * returns the verb of the comment + * Returns the verb of the comment * - * @return string * @since 9.0.0 */ - public function getVerb() { + public function getVerb(): string { return $this->data['verb']; } /** - * sets the verb of the comment, e.g. 'comment' or 'like' + * Sets the verb of the comment, e.g. 'comment' or 'like' * * @param string $verb - * @return IComment * @since 9.0.0 */ - public function setVerb($verb) { + public function setVerb($verb): IComment { if (!is_string($verb) || !trim($verb)) { throw new \InvalidArgumentException('Non-empty String expected.'); } @@ -277,34 +265,29 @@ public function setVerb($verb) { } /** - * returns the actor type - * - * @return string + * Returns the actor type * @since 9.0.0 */ - public function getActorType() { + public function getActorType(): string { return $this->data['actorType']; } /** - * returns the actor ID - * - * @return string + * Returns the actor ID * @since 9.0.0 */ - public function getActorId() { + public function getActorId(): string { return $this->data['actorId']; } /** - * sets (overwrites) the actor type and id + * Sets (overwrites) the actor type and id * * @param string $actorType e.g. 'users' * @param string $actorId e.g. 'zombie234' - * @return IComment * @since 9.0.0 */ - public function setActor($actorType, $actorId) { + public function setActor($actorType, $actorId): IComment { if ( !is_string($actorType) || !trim($actorType) || !is_string($actorId) || $actorId === '' @@ -317,76 +300,64 @@ public function setActor($actorType, $actorId) { } /** - * returns the creation date of the comment. + * 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']; + public function getCreationDateTime(): \DateTime { + return $this->data['creationDT'] ?? new \DateTime(); } /** - * sets the creation date of the comment and returns itself - * - * @param \DateTime $timestamp - * @return IComment + * Sets the creation date of the comment and returns itself * @since 9.0.0 */ - public function setCreationDateTime(\DateTime $timestamp) { - $this->data['creationDT'] = $timestamp; + public function setCreationDateTime(\DateTime $dateTime): IComment { + $this->data['creationDT'] = $dateTime; return $this; } /** - * returns the DateTime of the most recent child, if set, otherwise null - * - * @return \DateTime|null + * Returns the DateTime of the most recent child, if set, otherwise null * @since 9.0.0 */ - public function getLatestChildDateTime() { + public function getLatestChildDateTime(): ?\DateTime { return $this->data['latestChildDT']; } /** * @inheritDoc */ - public function setLatestChildDateTime(?\DateTime $dateTime = null) { + public function setLatestChildDateTime(?\DateTime $dateTime = null): IComment { $this->data['latestChildDT'] = $dateTime; return $this; } /** - * returns the object type the comment is attached to - * - * @return string + * Returns the object type the comment is attached to * @since 9.0.0 */ - public function getObjectType() { + public function getObjectType(): string { return $this->data['objectType']; } /** - * returns the object id the comment is attached to - * - * @return string + * Returns the object id the comment is attached to * @since 9.0.0 */ - public function getObjectId() { + public function getObjectId(): string { return $this->data['objectId']; } /** - * sets (overwrites) the object of the comment + * Sets (overwrites) the object of the comment * * @param string $objectType e.g. 'files' * @param string $objectId e.g. '16435' - * @return IComment * @since 9.0.0 */ - public function setObject($objectType, $objectId) { + public function setObject($objectType, $objectId): IComment { if ( !is_string($objectType) || !trim($objectType) || !is_string($objectId) || trim($objectId) === '' @@ -399,9 +370,7 @@ public function setObject($objectType, $objectId) { } /** - * returns the reference id of the comment - * - * @return string|null + * Returns the reference id of the comment * @since 19.0.0 */ public function getReferenceId(): ?string { @@ -409,10 +378,9 @@ public function getReferenceId(): ?string { } /** - * sets (overwrites) the reference id of the comment + * Sets (overwrites) the reference id of the comment * * @param string $referenceId e.g. sha256 hash sum - * @return IComment * @since 19.0.0 */ public function setReferenceId(?string $referenceId): IComment { @@ -463,9 +431,8 @@ public function getExpireDate(): ?\DateTime { * database. * * @param array $data - * @return IComment */ - protected function fromArray($data) { + protected function fromArray($data): IComment { foreach (array_keys($data) as $key) { // translate DB keys to internal setter names $setter = 'set' . implode('', array_map('ucfirst', explode('_', $key))); diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index 117318fb2782f..ee5b59c98ec68 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -47,35 +47,23 @@ use Psr\Log\LoggerInterface; class Manager implements ICommentsManager { - /** @var IDBConnection */ - protected $dbConn; - - /** @var LoggerInterface */ - protected $logger; - - /** @var IConfig */ - protected $config; - - /** @var ITimeFactory */ - protected $timeFactory; - - /** @var IEmojiHelper */ - protected $emojiHelper; - - /** @var IInitialStateService */ - protected $initialStateService; - + protected IDBConnection $dbConn; + protected LoggerInterface $logger; + protected IConfig $config; + protected ITimeFactory $timeFactory; + protected IEmojiHelper $emojiHelper; + protected IInitialStateService $initialStateService; /** @var IComment[] */ - protected $commentsCache = []; + protected array $commentsCache = []; /** @var \Closure[] */ - protected $eventHandlerClosures = []; + protected array $eventHandlerClosures = []; /** @var ICommentsEventHandler[] */ - protected $eventHandlers = []; + protected array $eventHandlers = []; /** @var \Closure[] */ - protected $displayNameResolvers = []; + protected array $displayNameResolvers = []; public function __construct(IDBConnection $dbConn, LoggerInterface $logger, @@ -96,9 +84,8 @@ public function __construct(IDBConnection $dbConn, * IComment interface. * * @param array $data - * @return array */ - protected function normalizeDatabaseData(array $data) { + protected function normalizeDatabaseData(array $data): array { $data['id'] = (string)$data['id']; $data['parent_id'] = (string)$data['parent_id']; $data['topmost_parent_id'] = (string)$data['topmost_parent_id']; @@ -134,11 +121,6 @@ protected function normalizeDatabaseData(array $data) { return $data; } - - /** - * @param array $data - * @return IComment - */ public function getCommentFromData(array $data): IComment { return new Comment($this->normalizeDatabaseData($data)); } @@ -152,7 +134,7 @@ public function getCommentFromData(array $data): IComment { * by parameter for convenience * @throws \UnexpectedValueException */ - protected function prepareCommentForDatabaseWrite(IComment $comment) { + protected function prepareCommentForDatabaseWrite(IComment $comment): IComment { if (!$comment->getActorType() || $comment->getActorId() === '' || !$comment->getObjectType() @@ -191,10 +173,9 @@ protected function prepareCommentForDatabaseWrite(IComment $comment) { * returns the topmost parent id of a given comment identified by ID * * @param string $id - * @return string * @throws NotFoundException */ - protected function determineTopmostParentId($id) { + protected function determineTopmostParentId($id): string { $comment = $this->get($id); if ($comment->getParentId() === '0') { return $comment->getId(); @@ -210,7 +191,7 @@ protected function determineTopmostParentId($id) { * @param \DateTime $cDateTime the date time of the most recent child * @throws NotFoundException */ - protected function updateChildrenInformation($id, \DateTime $cDateTime) { + protected function updateChildrenInformation($id, \DateTime $cDateTime): void { $qb = $this->dbConn->getQueryBuilder(); $query = $qb->select($qb->func()->count('id')) ->from('comments') @@ -237,7 +218,7 @@ protected function updateChildrenInformation($id, \DateTime $cDateTime) { * @param string $id * @throws \InvalidArgumentException */ - protected function checkRoleParameters($role, $type, $id) { + protected function checkRoleParameters($role, $type, $id): void { if ( !is_string($type) || empty($type) || !is_string($id) || empty($id) @@ -248,10 +229,8 @@ protected function checkRoleParameters($role, $type, $id) { /** * run-time caches a comment - * - * @param IComment $comment */ - protected function cache(IComment $comment) { + protected function cache(IComment $comment): void { $id = $comment->getId(); if (empty($id)) { return; @@ -264,7 +243,7 @@ protected function cache(IComment $comment) { * * @param mixed $id the comment's id */ - protected function uncache($id) { + protected function uncache($id): void { $id = (string)$id; if (isset($this->commentsCache[$id])) { unset($this->commentsCache[$id]); @@ -275,12 +254,11 @@ protected function uncache($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) { + public function get($id): IComment { if ((int)$id === 0) { throw new \InvalidArgumentException('IDs must be translatable to a number in this implementation.'); } @@ -309,35 +287,9 @@ public function get($id) { } /** - * 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'=> [] - * ], - * … - * ] - * ] + * @inheritDoc */ - public function getTree($id, $limit = 0, $offset = 0) { + public function getTree($id, $limit = 0, $offset = 0): array { $tree = []; $tree['comment'] = $this->get($id); $tree['replies'] = []; @@ -382,7 +334,7 @@ public function getTree($id, $limit = 0, $offset = 0) { * @param int $offset optional, starting point * @param \DateTime $notOlderThan optional, timestamp of the oldest comments * that may be returned - * @return IComment[] + * @return list * @since 9.0.0 */ public function getForObject( @@ -434,8 +386,7 @@ public function getForObject( * @param int $limit optional, number of maximum comments to be returned. if * set to 0, all comments are returned. * @param bool $includeLastKnown - * @return IComment[] - * @return array + * @return list */ public function getForObjectSince( string $objectType, @@ -465,7 +416,7 @@ public function getForObjectSince( * @param int $limit optional, number of maximum comments to be returned. if * set to 0, all comments are returned. * @param bool $includeLastKnown - * @return IComment[] + * @return list */ public function getCommentsWithVerbForObjectSinceComment( string $objectType, @@ -565,11 +516,10 @@ public function getCommentsWithVerbForObjectSinceComment( * @param string $objectType the object type, e.g. 'files' * @param string $objectId the id of the object * @param int $id the comment to look for - * @return Comment|null */ protected function getLastKnownComment(string $objectType, string $objectId, - int $id) { + int $id): ?IComment { $query = $this->dbConn->getQueryBuilder(); $query->select('*') ->from('comments') @@ -599,7 +549,7 @@ protected function getLastKnownComment(string $objectType, * @param string $verb Limit the verb of the comment * @param int $offset * @param int $limit - * @return IComment[] + * @return list */ public function search(string $search, string $objectType, string $objectId, string $verb, int $offset, int $limit = 50): array { $objectIds = []; @@ -618,7 +568,7 @@ public function search(string $search, string $objectType, string $objectId, str * @param string $verb Limit the verb of the comment * @param int $offset * @param int $limit - * @return IComment[] + * @return list */ public function searchForObjects(string $search, string $objectType, array $objectIds, string $verb, int $offset, int $limit = 50): array { $query = $this->dbConn->getQueryBuilder(); diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index a245e5d2f2006..d3f78b4f2e370 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -36,6 +36,9 @@ * * This class manages the access to comments * + * @psalm-type CommentNode = array{comment: IComment, replies: list} + * @psalm-type CommentTree = list + * * @since 9.0.0 */ interface ICommentsManager { @@ -66,7 +69,6 @@ public function get($id); * @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 @@ -110,7 +112,7 @@ public function getTree($id, $limit = 0, $offset = 0); * @param int $offset optional, starting point * @param \DateTime|null $notOlderThan optional, timestamp of the oldest comments * that may be returned - * @return IComment[] + * @return list * @since 9.0.0 */ public function getForObject( @@ -129,7 +131,7 @@ public function getForObject( * @param int $limit optional, number of maximum comments to be returned. if * set to 0, all comments are returned. * @param bool $includeLastKnown - * @return IComment[] + * @return list * @since 14.0.0 * @deprecated 24.0.0 - Use getCommentsWithVerbForObjectSinceComment instead */ @@ -151,7 +153,7 @@ public function getForObjectSince( * @param int $limit optional, number of maximum comments to be returned. if * set to 0, all comments are returned. * @param bool $includeLastKnown - * @return IComment[] + * @return list * @since 24.0.0 */ public function getCommentsWithVerbForObjectSinceComment( @@ -173,7 +175,7 @@ public function getCommentsWithVerbForObjectSinceComment( * @param string $verb Limit the verb of the comment * @param int $offset * @param int $limit - * @return IComment[] + * @return list * @since 14.0.0 */ public function search(string $search, string $objectType, string $objectId, string $verb, int $offset, int $limit = 50): array; From 77355a8a79898a03d4f45e853e3d99c9a633509b Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 28 Nov 2022 10:54:47 +0100 Subject: [PATCH 2/3] Improve typing in ICommentsManager Signed-off-by: Carl Schwan --- lib/public/Comments/ICommentsManager.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index d3f78b4f2e370..8d7ffd164b3af 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -36,9 +36,6 @@ * * This class manages the access to comments * - * @psalm-type CommentNode = array{comment: IComment, replies: list} - * @psalm-type CommentTree = list - * * @since 9.0.0 */ interface ICommentsManager { @@ -64,11 +61,12 @@ interface ICommentsManager { public function get($id); /** - * returns the comment specified by the id and all it's child comments + * Returns the comment specified by the id and all it's child comments * * @param string $id * @param int $limit max number of entries to return, 0 returns all * @param int $offset the start entry + * @return array{comment: IComment, replies: list}>} * @since 9.0.0 * * The return array looks like this From 7aa97dcc2369c421b6d32ad32c33d357e582af4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 20 Jun 2023 12:14:57 +0200 Subject: [PATCH 3/3] Throw if creation date is read before inserting into database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Comments/Comment.php | 6 +++++- lib/private/Comments/Manager.php | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index af2c73c770bf5..35e88c74438a3 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -304,9 +304,13 @@ public function setActor($actorType, $actorId): IComment { * * If not explicitly set, it shall default to the time of initialization. * @since 9.0.0 + * @throw \LogicException if creation date time is not set yet */ public function getCreationDateTime(): \DateTime { - return $this->data['creationDT'] ?? new \DateTime(); + if (!isset($this->data['creationDT'])) { + throw new \LogicException('Cannot get creation date before setting one or writting to database'); + } + return $this->data['creationDT']; } /** diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index ee5b59c98ec68..af4fda277d6cf 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -154,7 +154,9 @@ protected function prepareCommentForDatabaseWrite(IComment $comment): IComment { $comment->setLatestChildDateTime(null); } - if (is_null($comment->getCreationDateTime())) { + try { + $comment->getCreationDateTime(); + } catch(\LogicException $e) { $comment->setCreationDateTime(new \DateTime()); }