From 2d648e36b93622d050335b84e1d92f5c24e6e585 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 26 Jan 2026 14:08:50 +0100 Subject: [PATCH] fix(systemtags): Correct the return type of system tag object mapper Currently, it is documented in some places as returning a string but returns a int or a string depending on the database used. This then breaks then using strict comparaison in https://github.com/nextcloud/approval/pull/362 Signed-off-by: Carl Schwan --- apps/dav/lib/SystemTag/SystemTagPlugin.php | 25 +++++++-------- .../lib/Command/Files/DeleteAll.php | 7 ++-- .../lib/Search/TagSearchProvider.php | 2 +- .../SystemTag/SystemTagObjectMapper.php | 22 ++++++------- .../SystemTag/ISystemTagObjectMapper.php | 32 +++++++++++-------- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index eaf3a4d81438c..34f9b365389e0 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -62,7 +62,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { */ private $server; - /** @var array */ + /** @var array> */ private array $cachedTagMappings = []; /** @var array */ private array $cachedTags = []; @@ -210,17 +210,17 @@ private function preloadCollection( } if ($collection instanceof Directory - && !isset($this->cachedTagMappings[$collection->getId()]) + && !isset($this->cachedTagMappings[(string)$collection->getId()]) && $propFind->getStatus( self::SYSTEM_TAGS_PROPERTYNAME ) !== null) { - $fileIds = [$collection->getId()]; + $fileIds = [(string)$collection->getId()]; // note: pre-fetching only supported for depth <= 1 $folderContent = $collection->getChildren(); foreach ($folderContent as $info) { if ($info instanceof Node) { - $fileIds[] = $info->getId(); + $fileIds[] = (string)$info->getId(); } } @@ -231,7 +231,7 @@ private function preloadCollection( // also cache the ones that were not found foreach ($emptyFileIds as $fileId) { - $this->cachedTagMappings[$fileId] = []; + $this->cachedTagMappings[(string)$fileId] = []; } } } @@ -350,10 +350,10 @@ private function propfindForFile(PropFind $propFind, Node $node): void { * @return ISystemTag[] */ private function getTagsForFile(int $fileId, ?IUser $user): array { - if (isset($this->cachedTagMappings[$fileId])) { - $tagIds = $this->cachedTagMappings[$fileId]; + if (isset($this->cachedTagMappings[(string)$fileId])) { + $tagIds = $this->cachedTagMappings[(string)$fileId]; } else { - $tags = $this->tagMapper->getTagIdsForObjects([$fileId], 'files'); + $tags = $this->tagMapper->getTagIdsForObjects([(string)$fileId], 'files'); $fileTags = current($tags); if ($fileTags) { $tagIds = $fileTags; @@ -362,13 +362,10 @@ private function getTagsForFile(int $fileId, ?IUser $user): array { } } - $tags = array_filter(array_map(function (string $tagId) { - return $this->cachedTags[$tagId] ?? null; - }, $tagIds)); + $tags = array_filter(array_map( + fn (string $tagId): ?ISystemTag => $this->cachedTags[$tagId] ?? null, $tagIds)); - $uncachedTagIds = array_filter($tagIds, function (string $tagId): bool { - return !isset($this->cachedTags[$tagId]); - }); + $uncachedTagIds = array_filter($tagIds, fn (string $tagId): bool => !isset($this->cachedTags[$tagId])); if (count($uncachedTagIds)) { $retrievedTags = $this->tagManager->getTagsByIds($uncachedTagIds); diff --git a/apps/systemtags/lib/Command/Files/DeleteAll.php b/apps/systemtags/lib/Command/Files/DeleteAll.php index 59ebb741b8820..f60be49cbe3b5 100644 --- a/apps/systemtags/lib/Command/Files/DeleteAll.php +++ b/apps/systemtags/lib/Command/Files/DeleteAll.php @@ -35,13 +35,14 @@ public function execute(InputInterface $input, OutputInterface $output): int { $targetInput = $input->getArgument('target'); $targetNode = $this->fileUtils->getNode($targetInput); - if (! $targetNode) { + if (!$targetNode) { $output->writeln("file $targetInput not found"); return 1; } - $tags = $this->systemTagObjectMapper->getTagIdsForObjects([$targetNode->getId()], 'files'); - $this->systemTagObjectMapper->unassignTags((string)$targetNode->getId(), 'files', $tags[$targetNode->getId()]); + $targetNodeId = (string)$targetNode->getId(); + $tags = $this->systemTagObjectMapper->getTagIdsForObjects([$targetNodeId], 'files'); + $this->systemTagObjectMapper->unassignTags($targetNodeId, 'files', $tags[$targetNodeId]); $output->writeln('all tags removed.'); return 0; diff --git a/apps/systemtags/lib/Search/TagSearchProvider.php b/apps/systemtags/lib/Search/TagSearchProvider.php index 265ff78395d97..0dc1f13013194 100644 --- a/apps/systemtags/lib/Search/TagSearchProvider.php +++ b/apps/systemtags/lib/Search/TagSearchProvider.php @@ -130,7 +130,7 @@ public function search(IUser $user, ISearchQuery $query): SearchResult { $searchResultEntry = new SearchResultEntry( $thumbnailUrl, $result->getName(), - $this->formatSubline($query, $matchedTags[$nodeId]), + $this->formatSubline($query, $matchedTags[(string)$nodeId]), $this->urlGenerator->getAbsoluteURL($link), $result->getMimetype() === FileInfo::MIMETYPE_FOLDER ? 'icon-folder' diff --git a/lib/private/SystemTag/SystemTagObjectMapper.php b/lib/private/SystemTag/SystemTagObjectMapper.php index e0eb68cfa1791..ce399d977df60 100644 --- a/lib/private/SystemTag/SystemTagObjectMapper.php +++ b/lib/private/SystemTag/SystemTagObjectMapper.php @@ -20,6 +20,7 @@ use OCP\SystemTag\TagAssignedEvent; use OCP\SystemTag\TagNotFoundException; use OCP\SystemTag\TagUnassignedEvent; +use Override; class SystemTagObjectMapper implements ISystemTagObjectMapper { public const RELATION_TABLE = 'systemtag_object_mapping'; @@ -31,9 +32,7 @@ public function __construct( ) { } - /** - * {@inheritdoc} - */ + #[Override] public function getTagIdsForObjects($objIds, string $objectType): array { if (!\is_array($objIds)) { $objIds = [$objIds]; @@ -59,7 +58,7 @@ public function getTagIdsForObjects($objIds, string $objectType): array { $result = $query->executeQuery(); while ($row = $result->fetch()) { $objectId = $row['objectid']; - $mapping[$objectId][] = $row['systemtagid']; + $mapping[$objectId][] = (string)$row['systemtagid']; } $result->closeCursor(); @@ -108,9 +107,7 @@ public function getObjectIdsForTags($tagIds, string $objectType, int $limit = 0, return $objectIds; } - /** - * {@inheritdoc} - */ + #[Override] public function assignTags(string $objId, string $objectType, $tagIds): void { if (!\is_array($tagIds)) { $tagIds = [$tagIds]; @@ -168,18 +165,18 @@ public function assignTags(string $objId, string $objectType, $tagIds): void { return; } + $tagsAssigned = array_map(static fn (string $tagId): int => (int)$tagId, $tagsAssigned); + $this->dispatcher->dispatch(MapperEvent::EVENT_ASSIGN, new MapperEvent( MapperEvent::EVENT_ASSIGN, $objectType, $objId, - $tagsAssigned + $tagsAssigned, )); $this->dispatcher->dispatchTyped(new TagAssignedEvent($objectType, [$objId], $tagsAssigned)); } - /** - * {@inheritdoc} - */ + #[Override] public function unassignTags(string $objId, string $objectType, $tagIds): void { if (!\is_array($tagIds)) { $tagIds = [$tagIds]; @@ -199,6 +196,9 @@ public function unassignTags(string $objId, string $objectType, $tagIds): void { $this->updateEtagForTags($tagIds); + // convert ids to int because the event uses ints + $tagIds = array_map(static fn (string $tagId): int => (int)$tagId, $tagIds); + $this->dispatcher->dispatch(MapperEvent::EVENT_UNASSIGN, new MapperEvent( MapperEvent::EVENT_UNASSIGN, $objectType, diff --git a/lib/public/SystemTag/ISystemTagObjectMapper.php b/lib/public/SystemTag/ISystemTagObjectMapper.php index c604fa93c586a..41760c1c4ff94 100644 --- a/lib/public/SystemTag/ISystemTagObjectMapper.php +++ b/lib/public/SystemTag/ISystemTagObjectMapper.php @@ -8,30 +8,36 @@ */ namespace OCP\SystemTag; +use OCP\AppFramework\Attribute\Consumable; + /** * Public interface to access and manage system-wide tags. * * @since 9.0.0 */ +#[Consumable(since: '9.0.0')] interface ISystemTagObjectMapper { /** * Get a list of tag ids for the given object ids. * * This returns an array that maps object id to tag ids + * + * ``` * [ - * 1 => array('id1', 'id2'), - * 2 => array('id3', 'id2'), - * 3 => array('id5'), - * 4 => array() + * 1 => ['id1', 'id2'], + * 2 => ['id3', 'id2'], + * 3 => ['id5'], + * 4 => [] * ] + * ``` * * Untagged objects will have an empty array associated. * - * @param string|array $objIds object ids + * @param string|list $objIds object ids * @param string $objectType object type * - * @return array with object id as key and an array - * of tag ids as value + * @return array> with object id as key and an array + * of tag ids as value * * @since 9.0.0 */ @@ -40,12 +46,12 @@ public function getTagIdsForObjects($objIds, string $objectType): array; /** * Get a list of objects tagged with $tagIds. * - * @param string|array $tagIds Tag id or array of tag ids. + * @param string|list $tagIds Tag id or array of tag ids. * @param string $objectType object type * @param int $limit Count of object ids you want to get * @param string $offset The last object id you already received * - * @return string[] array of object ids or empty array if none found + * @return list array of object ids or empty array if none found * * @throws TagNotFoundException if at least one of the * given tags does not exist @@ -66,7 +72,7 @@ public function getObjectIdsForTags($tagIds, string $objectType, int $limit = 0, * * @param string $objId object id * @param string $objectType object type - * @param string|array $tagIds tag id or array of tag ids to assign + * @param string|list $tagIds tag id or array of tag ids to assign * * @throws TagNotFoundException if at least one of the * given tags does not exist @@ -85,7 +91,7 @@ public function assignTags(string $objId, string $objectType, $tagIds); * * @param string $objId object id * @param string $objectType object type - * @param string|array $tagIds tag id or array of tag ids to unassign + * @param string|list $tagIds tag id or array of tag ids to unassign * * @throws TagNotFoundException if at least one of the * given tags does not exist @@ -97,7 +103,7 @@ public function unassignTags(string $objId, string $objectType, $tagIds); /** * Checks whether the given objects have the given tag. * - * @param string|array $objIds object ids + * @param string|list $objIds object ids * @param string $objectType object type * @param string $tagId tag id to check * @param bool $all true to check that ALL objects have the tag assigned, @@ -128,7 +134,7 @@ public function getAvailableObjectTypes(): array; * * @param string $tagId tag id * @param string $objectType object type - * @param string[] $objectIds list of object ids + * @param list $objectIds list of object ids * * @throws TagNotFoundException if the tag does not exist * @since 31.0.0