Skip to content

Commit

Permalink
fix: move shares count cache logic to the DeckShareProvider
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
luka-nextcloud authored and juliusknorr committed May 6, 2022
1 parent 0b6990f commit ed3be36
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 17 deletions.
4 changes: 2 additions & 2 deletions lib/Service/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public function findAll($cardId, $withDeleted = false) {

/**
* @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
*/
Expand All @@ -147,7 +147,7 @@ public function count($cardId, $update = false) {
}

$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) {
Expand Down
145 changes: 145 additions & 0 deletions lib/Sharing/DeckShareFileCountHelper.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
63 changes: 49 additions & 14 deletions lib/Sharing/DeckShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
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;
Expand All @@ -44,9 +46,10 @@
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;
Expand All @@ -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 {
Expand Down Expand Up @@ -165,12 +173,39 @@ public function create(IShare $share) {
$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
*
Expand Down
2 changes: 1 addition & 1 deletion src/components/card/AttachmentList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export default {
},
methods: {
...mapActions([
"fetchAttachments",
'fetchAttachments',
]),
handleUploadFile(event) {
const files = event.target.files ?? []
Expand Down

0 comments on commit ed3be36

Please sign in to comment.