fix: move shares count cache logic to the DeckShareProvider

Signed-off-by: Luka Trovic <luka@nextcloud.com>

fix: conflicts

Signed-off-by: Luka Trovic <luka@nextcloud.com>

fix: conflicts and test issues

Signed-off-by: Luka Trovic <luka@nextcloud.com>

lib/Service/AttachmentService.php

@@ -137,7 +137,7 @@ class AttachmentService {
 
 	/**
 	 * @param $cardId
-	 * @param bool $update | Force the update of the cached values 
+	 * @param bool $update | Force the update of the cached values
 	 * @return int|mixed
 	 * @throws BadRequestException
 	 */
@@ -147,7 +147,7 @@ class AttachmentService {
 		}
 
 		$count = $this->cache->get('card-' . $cardId);
-		if ($update OR !$count) {
+		if ($update || !$count) {
 			$count = count($this->attachmentMapper->findAll($cardId));
 
 			foreach (array_keys($this->services) as $attachmentType) {

lib/Sharing/DeckShareFileCountHelper.php

@@ -0,0 +1,145 @@
+<?php
+
+/*
+ * @copyright Copyright (c) 2020 Julius Härtl <jus@bitgrid.net>
+ *
+ * @author Julius Härtl <jus@bitgrid.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+declare(strict_types=1);
+
+
+namespace OCA\Deck\Sharing;
+
+use OCA\Deck\Db\AttachmentMapper;
+use OCA\Deck\BadRequestException;
+use OCP\Share\IShare;
+use OCP\IDBConnection;
+
+class DeckShareFileCountHelper {
+
+	/**
+	 * @var array
+	 */
+	private $processors = [];
+	private AttachmentMapper $attachmentMapper;
+
+	public function __construct(AttachmentMapper $attachmentMapper) {
+		$this->attachmentMapper = $attachmentMapper;
+		$this->registerProcessors();
+	}
+
+	/**
+	 * @param $cardId
+	 * @return int
+	 * @throws BadRequestException
+	 */
+	public function getAttachmentCount($cardId) {
+		$count = $this->countDeckFile($cardId);
+		foreach (array_keys($this->processors) as $type) {
+			if ($processor = $this->processor($type)) {
+				$count += $this->{$processor}((int)$cardId);
+			}
+		}
+
+		return $count;
+	}
+
+	/**
+	 * @param string $type
+	 * @return string|null
+	 * @throws BadRequestException
+	 */
+	private function processor(string $type) {
+		if (!array_key_exists($type, $this->processors)) {
+			throw new BadRequestException('This type of file is not found');
+		}
+
+		return $this->processors[$type];
+	}
+
+	private function registerProcessors() {
+		return $this->processors = [
+			'deck_file' => null,
+			'file' => 'countFile'
+		];
+	}
+
+	/**
+	 * Count deck files
+	 *
+	 * @param $cardId
+	 * @return int
+	 */
+	private function countDeckFile($cardId) {
+		return count($this->attachmentMapper->findAll($cardId));
+	}
+
+	/**
+	 * Count files other than the deck ones
+	 *
+	 * @param int $cardId
+	 * @return int
+	 */
+	private function countFile(int $cardId) {
+		/** @var IDBConnection $qb */
+		$db = \OC::$server->getDatabaseConnection();
+		$qb = $db->getQueryBuilder();
+		$qb->select('s.id', 'f.fileid', 'f.path')
+			->selectAlias('st.id', 'storage_string_id')
+			->from('share', 's')
+			->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
+			->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'))
+			->andWhere($qb->expr()->eq('s.share_type', $qb->createNamedParameter(IShare::TYPE_DECK)))
+			->andWhere($qb->expr()->eq('s.share_with', $qb->createNamedParameter($cardId)))
+			->andWhere($qb->expr()->isNull('s.parent'))
+			->andWhere($qb->expr()->orX(
+				$qb->expr()->eq('s.item_type', $qb->createNamedParameter('file')),
+				$qb->expr()->eq('s.item_type', $qb->createNamedParameter('folder'))
+			));
+
+		$count = 0;
+		$cursor = $qb->execute();
+		while ($data = $cursor->fetch()) {
+			if ($this->isAccessibleResult($data)) {
+				$count++;
+			}
+		}
+		$cursor->closeCursor();
+		return $count;
+	}
+
+	private function isAccessibleResult(array $data): bool {
+		// exclude shares leading to deleted file entries
+		if ($data['fileid'] === null || $data['path'] === null) {
+			return false;
+		}
+
+		// exclude shares leading to trashbin on home storages
+		$pathSections = explode('/', $data['path'], 2);
+		// FIXME: would not detect rare md5'd home storage case properly
+		if (
+			$pathSections[0] !== 'files'
+			&& in_array(explode(':', $data['storage_string_id'], 2)[0], ['home', 'object'])
+		) {
+			return false;
+		}
+		return true;
+	}
+}

lib/Sharing/DeckShareProvider.php

@@ -32,9 +32,11 @@ use OCA\Deck\Db\Board;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Db\User;
+use OCA\Deck\BadRequestException;
 use OCA\Deck\NoPermissionException;
+use OCA\Deck\InvalidAttachmentType;
 use OCA\Deck\Service\PermissionService;
-use OCA\Deck\Service\AttachmentService;
+use OCA\Deck\Service\IAttachmentService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -44,9 +46,10 @@ use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Folder;
 use OCP\Files\IMimeTypeLoader;
 use OCP\Files\Node;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IDBConnection;
 use OCP\IL10N;
-use OCP\Security\ISecureRandom;
 use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
@@ -66,41 +69,46 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 
 	public const SHARE_TYPE_DECK_USER = IShare::TYPE_DECK_USER;
 
+	/** @var IAttachmentService[] */
+	private $services = [];
 	/** @var IDBConnection */
 	private $dbConnection;
 	/** @var IManager */
 	private $shareManager;
+	/** @var DeckShareFileCountHelper */
+	private $deckShareFileCountHelper;
 	/** @var BoardMapper */
 	private $boardMapper;
 	/** @var CardMapper */
 	private $cardMapper;
 	/** @var PermissionService */
 	private $permissionService;
-	/** @var AttachmentService */
-	private $attachmentService;
 	/** @var ITimeFactory */
+	/** @var ICache */
+	private $cache;
 	private $timeFactory;
 	private $l;
 
 	public function __construct(
-		IDBConnection $connection, 
-		IManager $shareManager, 
-		ISecureRandom $secureRandom, 
-		BoardMapper $boardMapper, 
-		CardMapper $cardMapper, 
-		AttachmentService $attachmentService,
-		PermissionService $permissionService, 
+		IDBConnection $connection,
+		IManager $shareManager,
+		BoardMapper $boardMapper,
+		CardMapper $cardMapper,
+		PermissionService $permissionService,
+		ICacheFactory $cacheFactory,
+		DeckShareFileCountHelper $deckShareFileCountHelper,
 		IL10N $l
 	) {
 		$this->dbConnection = $connection;
 		$this->shareManager = $shareManager;
 		$this->boardMapper = $boardMapper;
 		$this->cardMapper = $cardMapper;
-		$this->attachmentService = $attachmentService;
+		$this->deckShareFileCountHelper = $deckShareFileCountHelper;
 		$this->permissionService = $permissionService;
 
 		$this->l = $l;
 		$this->timeFactory = \OC::$server->get(ITimeFactory::class);
+		$this->cache = $cacheFactory->createDistributed('deck-card-attachments-');
 	}
 
 	public static function register(IEventDispatcher $dispatcher): void {
@@ -165,12 +173,39 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 			$share->getExpirationDate()
 		);
 		$data = $this->getRawShare($shareId);
-
-		$this->attachmentService->count($cardId, true);
+		$this->updateCardAttachmentsCountCache($cardId);
 
 		return $this->createShareObject($data);
 	}
 
+	/**
+	 * @param $cardId
+	 * @return int|mixed
+	 * @throws BadRequestException
+	 */
+	private function updateCardAttachmentsCountCache($cardId) {
+		if (is_numeric($cardId) === false) {
+			throw new BadRequestException('card id must be a number');
+		}
+
+		$count = $this->deckShareFileCountHelper->getAttachmentCount($cardId);
+		$this->cache->set('card-' . $cardId, $count);
+
+		return $count;
+	}
+
+	/**
+	 * @param string $type
+	 * @return IAttachmentService
+	 * @throws InvalidAttachmentType
+	 */
+	public function getService($type) {
+		if (isset($this->services[$type])) {
+			return $this->services[$type];
+		}
+		throw new InvalidAttachmentType($type);
+	}
+
 	/**
 	 * Add share to the database and return the ID
 	 *

src/components/card/AttachmentList.vue

@@ -211,7 +211,7 @@ export default {
 	},
 	methods: {
 		...mapActions([
-			"fetchAttachments",
+			'fetchAttachments',
 		]),
 		handleUploadFile(event) {
 			const files = event.target.files ?? []