Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl 2022-05-06 11:32:59 +02:00
Родитель ed3be361b5
Коммит 03f400620e
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4C614C6ED2CDE6DF
7 изменённых файлов: 111 добавлений и 223 удалений

Просмотреть файл

@ -0,0 +1,52 @@
<?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\Cache;
use OCP\ICache;
use OCP\ICacheFactory;
class AttachmentCacheHelper {
/** @var ICache */
private $cache;
public function __construct(ICacheFactory $cacheFactory) {
$this->cache = $cacheFactory->createDistributed('deck-attachments');
}
public function getAttachmentCount(int $cardId): ?int {
return $this->cache->get('count-' . $cardId);
}
public function setAttachmentCount(int $cardId, int $count): void {
$this->cache->set('count-' . $cardId, $count);
}
public function clearAttachmentCount(int $cardId): void {
$this->cache->remove('count-' . $cardId);
}
}

Просмотреть файл

@ -34,11 +34,10 @@ use OCA\Deck\Db\ChangeHelper;
use OCA\Deck\InvalidAttachmentType;
use OCA\Deck\NoPermissionException;
use OCA\Deck\NotFoundException;
use OCA\Deck\Cache\AttachmentCacheHelper;
use OCA\Deck\StatusException;
use OCP\AppFramework\Db\IMapperException;
use OCP\AppFramework\Http\Response;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IL10N;
class AttachmentService {
@ -49,9 +48,10 @@ class AttachmentService {
/** @var IAttachmentService[] */
private $services = [];
/** @var Application */
private $application;
/** @var ICache */
private $cache;
/** @var AttachmentCacheHelper */
private $attachmentCacheHelper;
/** @var IL10N */
private $l10n;
/** @var ActivityManager */
@ -59,13 +59,13 @@ class AttachmentService {
/** @var ChangeHelper */
private $changeHelper;
public function __construct(AttachmentMapper $attachmentMapper, CardMapper $cardMapper, ChangeHelper $changeHelper, PermissionService $permissionService, Application $application, ICacheFactory $cacheFactory, $userId, IL10N $l10n, ActivityManager $activityManager) {
public function __construct(AttachmentMapper $attachmentMapper, CardMapper $cardMapper, ChangeHelper $changeHelper, PermissionService $permissionService, Application $application, AttachmentCacheHelper $attachmentCacheHelper, $userId, IL10N $l10n, ActivityManager $activityManager) {
$this->attachmentMapper = $attachmentMapper;
$this->cardMapper = $cardMapper;
$this->permissionService = $permissionService;
$this->userId = $userId;
$this->application = $application;
$this->cache = $cacheFactory->createDistributed('deck-card-attachments-');
$this->attachmentCacheHelper = $attachmentCacheHelper;
$this->l10n = $l10n;
$this->activityManager = $activityManager;
$this->changeHelper = $changeHelper;
@ -137,17 +137,18 @@ class AttachmentService {
/**
* @param $cardId
* @param bool $update | Force the update of the cached values
* @return int|mixed
* @throws BadRequestException
* @throws InvalidAttachmentType
* @throws \OCP\DB\Exception
*/
public function count($cardId, $update = false) {
public function count($cardId) {
if (is_numeric($cardId) === false) {
throw new BadRequestException('card id must be a number');
}
$count = $this->cache->get('card-' . $cardId);
if ($update || !$count) {
$count = $this->attachmentCacheHelper->getAttachmentCount((int)$cardId);
if ($count === null) {
$count = count($this->attachmentMapper->findAll($cardId));
foreach (array_keys($this->services) as $attachmentType) {
@ -157,7 +158,7 @@ class AttachmentService {
}
}
$this->cache->set('card-' . $cardId, $count);
$this->attachmentCacheHelper->setAttachmentCount((int)$cardId, $count);
}
return $count;
@ -187,7 +188,7 @@ class AttachmentService {
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
$this->cache->clear('card-' . $cardId);
$this->attachmentCacheHelper->clearAttachmentCount((int)$cardId);
$attachment = new Attachment();
$attachment->setCardId($cardId);
$attachment->setType($type);
@ -299,7 +300,7 @@ class AttachmentService {
}
$this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT);
$this->cache->clear('card-' . $attachment->getCardId());
$this->attachmentCacheHelper->clearAttachmentCount($cardId);
$attachment->setData($data);
try {
@ -357,7 +358,7 @@ class AttachmentService {
}
}
$this->cache->clear('card-' . $attachment->getCardId());
$this->attachmentCacheHelper->clearAttachmentCount($cardId);
$this->changeHelper->cardChanged($attachment->getCardId());
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE);
return $attachment;
@ -371,7 +372,7 @@ class AttachmentService {
}
$this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT);
$this->cache->clear('card-' . $attachment->getCardId());
$this->attachmentCacheHelper->clearAttachmentCount($cardId);
try {
$service = $this->getService($attachment->getType());

Просмотреть файл

@ -1,145 +0,0 @@
<?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;
}
}

Просмотреть файл

@ -27,16 +27,14 @@ declare(strict_types=1);
namespace OCA\Deck\Sharing;
use OC\Files\Cache\Cache;
use OCA\Deck\Cache\AttachmentCacheHelper;
use OCA\Deck\Db\Acl;
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\IAttachmentService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
use OCP\AppFramework\Utility\ITimeFactory;
@ -46,8 +44,6 @@ 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\Share\Exceptions\GenericShareException;
@ -69,14 +65,12 @@ 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 AttachmentCacheHelper */
private $attachmentCacheHelper;
/** @var BoardMapper */
private $boardMapper;
/** @var CardMapper */
@ -84,9 +78,8 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
/** @var PermissionService */
private $permissionService;
/** @var ITimeFactory */
/** @var ICache */
private $cache;
private $timeFactory;
/** @var IL10N */
private $l;
public function __construct(
@ -95,20 +88,18 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
BoardMapper $boardMapper,
CardMapper $cardMapper,
PermissionService $permissionService,
ICacheFactory $cacheFactory,
DeckShareFileCountHelper $deckShareFileCountHelper,
AttachmentCacheHelper $attachmentCacheHelper,
IL10N $l
) {
$this->dbConnection = $connection;
$this->shareManager = $shareManager;
$this->boardMapper = $boardMapper;
$this->cardMapper = $cardMapper;
$this->deckShareFileCountHelper = $deckShareFileCountHelper;
$this->attachmentCacheHelper = $attachmentCacheHelper;
$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 {
@ -173,39 +164,12 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
$share->getExpirationDate()
);
$data = $this->getRawShare($shareId);
$this->updateCardAttachmentsCountCache($cardId);
$this->attachmentCacheHelper->clearAttachmentCount((int)$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
*
@ -390,6 +354,8 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
$qb->orWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId())));
$qb->execute();
$this->attachmentCacheHelper->clearAttachmentCount((int)$share->getSharedWith());
}
/**

Просмотреть файл

@ -84,6 +84,7 @@ export default {
async fetchAttachments({ commit }, cardId) {
const attachments = await apiClient.fetchAttachments(cardId)
commit('createAttachments', { cardId, attachments })
commit('cardSetAttachmentCount', { cardId, count: attachments.length })
},
async createAttachment({ commit }, { cardId, formData, onUploadProgress }) {

Просмотреть файл

@ -252,6 +252,12 @@ export default {
}
Vue.set(state.cards[existingIndex], 'lastModified', Date.now() / 1000)
},
cardSetAttachmentCount(state, { cardId, count }) {
const existingIndex = state.cards.findIndex(_card => _card.id === cardId)
if (existingIndex !== -1) {
Vue.set(state.cards[existingIndex], 'attachmentCount', count)
}
},
cardIncreaseAttachmentCount(state, cardId) {
const existingIndex = state.cards.findIndex(_card => _card.id === cardId)
if (existingIndex !== -1) {

Просмотреть файл

@ -25,6 +25,7 @@ namespace OCA\Deck\Service;
use OCA\Deck\Activity\ActivityManager;
use OCA\Deck\AppInfo\Application;
use OCA\Deck\Cache\AttachmentCacheHelper;
use OCA\Deck\Db\Acl;
use OCA\Deck\Db\Attachment;
use OCA\Deck\Db\AttachmentMapper;
@ -67,10 +68,11 @@ class AttachmentServiceTest extends TestCase {
private $cardMapper;
/** @var PermissionService|MockObject */
private $permissionService;
/** @var string */
private $userId = 'admin';
/** @var Application|MockObject */
private $application;
private $cacheFactory;
private $attachmentCacheHelper;
/** @var AttachmentService */
private $attachmentService;
/** @var MockObject */
@ -78,8 +80,6 @@ class AttachmentServiceTest extends TestCase {
/** @var ActivityManager */
private $activityManager;
private $appContainer;
/** ICache */
private $cache;
/** @var IL10N */
private $l10n;
/** @var ChangeHelper */
@ -104,12 +104,9 @@ class AttachmentServiceTest extends TestCase {
$this->cardMapper = $this->createMock(CardMapper::class);
$this->permissionService = $this->createMock(PermissionService::class);
$this->application = $this->createMock(Application::class);
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->attachmentCacheHelper = $this->createMock(AttachmentCacheHelper::class);
$this->activityManager = $this->createMock(ActivityManager::class);
$this->cache = $this->createMock(ICache::class);
$this->cacheFactory->expects($this->any())->method('createDistributed')->willReturn($this->cache);
$this->appContainer->expects($this->exactly(2))
->method('query')
->withConsecutive(
@ -128,7 +125,17 @@ class AttachmentServiceTest extends TestCase {
$this->l10n = $this->createMock(IL10N::class);
$this->changeHelper = $this->createMock(ChangeHelper::class);
$this->attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $this->application, $this->cacheFactory, $this->userId, $this->l10n, $this->activityManager);
$this->attachmentService = new AttachmentService(
$this->attachmentMapper,
$this->cardMapper,
$this->changeHelper,
$this->permissionService,
$this->application,
$this->attachmentCacheHelper,
$this->userId,
$this->l10n,
$this->activityManager
);
}
public function testRegisterAttachmentService() {
@ -153,7 +160,7 @@ class AttachmentServiceTest extends TestCase {
$application->expects($this->any())
->method('getContainer')
->willReturn($appContainer);
$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->cacheFactory, $this->userId, $this->l10n, $this->activityManager);
$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager);
$attachmentService->registerAttachmentService('custom', MyAttachmentService::class);
$this->assertEquals($fileServiceMock, $attachmentService->getService('deck_file'));
$this->assertEquals(MyAttachmentService::class, get_class($attachmentService->getService('custom')));
@ -183,7 +190,7 @@ class AttachmentServiceTest extends TestCase {
->method('getContainer')
->willReturn($appContainer);
$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->cacheFactory, $this->userId, $this->l10n, $this->activityManager);
$attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager);
$attachmentService->registerAttachmentService('custom', MyAttachmentService::class);
$attachmentService->getService('deck_file_invalid');
}
@ -262,14 +269,14 @@ class AttachmentServiceTest extends TestCase {
}
public function testCount() {
$this->cache->expects($this->once())->method('get')->with('card-123')->willReturn(null);
$this->attachmentCacheHelper->expects($this->once())->method('getAttachmentCount')->with(123)->willReturn(null);
$this->attachmentMapper->expects($this->once())->method('findAll')->willReturn([1,2,3,4]);
$this->cache->expects($this->once())->method('set')->with('card-123', 4)->willReturn(null);
$this->attachmentCacheHelper->expects($this->once())->method('setAttachmentCount')->with(123, 4);
$this->assertEquals(4, $this->attachmentService->count(123));
}
public function testCountCacheHit() {
$this->cache->expects($this->once())->method('get')->with('card-123')->willReturn(4);
$this->attachmentCacheHelper->expects($this->once())->method('getAttachmentCount')->with(123)->willReturn(4);
$this->assertEquals(4, $this->attachmentService->count(123));
}
@ -277,7 +284,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(123);
$this->attachmentServiceImpl->expects($this->once())
->method('create');
$this->attachmentMapper->expects($this->once())
@ -330,7 +337,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(1);
$this->attachmentMapper->expects($this->once())
->method('find')
->with(1)
@ -357,7 +364,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(1);
$this->attachmentMapper->expects($this->once())
->method('find')
->with(1)
@ -378,7 +385,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(1);
$this->attachmentMapper->expects($this->once())
->method('find')
->with(1)
@ -403,7 +410,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(1);
$this->attachmentMapper->expects($this->once())
->method('find')
->with(1)
@ -424,7 +431,7 @@ class AttachmentServiceTest extends TestCase {
$attachment = $this->createAttachment('deck_file', 'file_name.jpg');
$expected = $this->createAttachment('deck_file', 'file_name.jpg');
$this->mockPermission(Acl::PERMISSION_EDIT);
$this->cache->expects($this->once())->method('clear')->with('card-123');
$this->attachmentCacheHelper->expects($this->once())->method('clearAttachmentCount')->with(1);
$this->attachmentMapper->expects($this->once())
->method('find')
->with(1)