Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/files/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
use OCP\AppFramework\Bootstrap\IRegistrationContext;
use OCP\Collaboration\Reference\RenderReferenceEvent;
use OCP\Collaboration\Resources\IProviderManager;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Cache\CacheEntriesRemovedEvent;
use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
Expand Down Expand Up @@ -114,7 +114,7 @@ public function register(IRegistrationContext $context): void {
$context->registerEventListener(RenderReferenceEvent::class, RenderReferenceEventListener::class);
$context->registerEventListener(BeforeNodeRenamedEvent::class, SyncLivePhotosListener::class);
$context->registerEventListener(BeforeNodeDeletedEvent::class, SyncLivePhotosListener::class);
$context->registerEventListener(CacheEntryRemovedEvent::class, SyncLivePhotosListener::class, 1); // Ensure this happen before the metadata are deleted.
$context->registerEventListener(CacheEntriesRemovedEvent::class, SyncLivePhotosListener::class, 1); // Ensure this happen before the metadata are deleted.
$context->registerEventListener(BeforeNodeCopiedEvent::class, SyncLivePhotosListener::class);
$context->registerEventListener(NodeCopiedEvent::class, SyncLivePhotosListener::class);
$context->registerEventListener(LoadSearchPlugins::class, LoadSearchPluginsListener::class);
Expand Down
28 changes: 23 additions & 5 deletions apps/files/lib/Listener/SyncLivePhotosListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Exceptions\AbortedEventException;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Cache\CacheEntriesRemovedEvent;
use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
Expand Down Expand Up @@ -63,8 +63,8 @@ public function handle(Event $event): void {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId());
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getNode()->getId());
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId());
} elseif ($event instanceof CacheEntriesRemovedEvent) {
$this->handleCacheEntriesRemovedEvent($event);
}

if ($peerFileId === null) {
Expand All @@ -83,12 +83,30 @@ public function handle(Event $event): void {
$this->handleMove($event->getSource(), $event->getTarget(), $peerFile);
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$this->handleDeletion($event, $peerFile);
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFile->delete();
}
}
}

public function handleCacheEntriesRemovedEvent(CacheEntriesRemovedEvent $cacheEntriesRemovedEvent): void {
$entries = $cacheEntriesRemovedEvent->getCacheEntryRemovedEvents();
$fileIds = [];
foreach ($entries as $entry) {
$fileIds[] = $entry->getFileId();
}

$peerFileIds = $this->livePhotosService->getLivePhotoPeerIds($fileIds);

foreach ($peerFileIds as $peerFileId) {
// Check the user's folder.
$peerFile = $this->userFolder->getFirstNodeById($peerFileId);

if ($peerFile === null) {
return; // Peer file not found.
}
$peerFile->delete();
}
}

private function runMoveOrCopyChecks(Node $sourceFile, Node $targetFile, Node $peerFile): void {
$targetParent = $targetFile->getParent();
$sourceExtension = $sourceFile->getExtension();
Expand Down
18 changes: 18 additions & 0 deletions apps/files/lib/Service/LivePhotosService.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,22 @@ public function getLivePhotoPeerId(int $fileId): ?int {

return (int)$metadata->getString('files-live-photo');
}

/**
* Get the associated live photo for multiple file ids
* @param int[] $fileIds
* @return int[]
*/
public function getLivePhotoPeerIds(array $fileIds): array {
$metadata = $this->filesMetadataManager->getMetadataForFiles($fileIds);
$peersIds = [];
foreach ($metadata as $item) {
if (!$item->hasKey('files-live-photo')) {
continue;
}

$peersIds[] = (int)$item->getString('files-live-photo');
}
return $peersIds;
}
}
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@
'OCP\\Files\\AlreadyExistsException' => $baseDir . '/lib/public/Files/AlreadyExistsException.php',
'OCP\\Files\\AppData\\IAppDataFactory' => $baseDir . '/lib/public/Files/AppData/IAppDataFactory.php',
'OCP\\Files\\Cache\\AbstractCacheEvent' => $baseDir . '/lib/public/Files/Cache/AbstractCacheEvent.php',
'OCP\\Files\\Cache\\CacheEntriesRemovedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntriesRemovedEvent.php',
'OCP\\Files\\Cache\\CacheEntryInsertedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryInsertedEvent.php',
'OCP\\Files\\Cache\\CacheEntryRemovedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryRemovedEvent.php',
'OCP\\Files\\Cache\\CacheEntryUpdatedEvent' => $baseDir . '/lib/public/Files/Cache/CacheEntryUpdatedEvent.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Files\\AlreadyExistsException' => __DIR__ . '/../../..' . '/lib/public/Files/AlreadyExistsException.php',
'OCP\\Files\\AppData\\IAppDataFactory' => __DIR__ . '/../../..' . '/lib/public/Files/AppData/IAppDataFactory.php',
'OCP\\Files\\Cache\\AbstractCacheEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/AbstractCacheEvent.php',
'OCP\\Files\\Cache\\CacheEntriesRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntriesRemovedEvent.php',
'OCP\\Files\\Cache\\CacheEntryInsertedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryInsertedEvent.php',
'OCP\\Files\\Cache\\CacheEntryRemovedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryRemovedEvent.php',
'OCP\\Files\\Cache\\CacheEntryUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/Files/Cache/CacheEntryUpdatedEvent.php',
Expand Down
12 changes: 10 additions & 2 deletions lib/private/Files/Cache/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Cache\CacheEntriesRemovedEvent;
use OCP\Files\Cache\CacheEntryInsertedEvent;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Cache\CacheEntryUpdatedEvent;
Expand Down Expand Up @@ -620,13 +621,17 @@ private function removeChildren(ICacheEntry $entry) {
$query->executeStatement();
}

$cacheEntryRemovedEvents = [];
foreach (array_combine($deletedIds, $deletedPaths) as $fileId => $filePath) {
$cacheEntryRemovedEvent = new CacheEntryRemovedEvent(
$cacheEntryRemovedEvents[] = new CacheEntryRemovedEvent(
$this->storage,
$filePath,
$fileId,
$this->getNumericStorageId()
);
}
$this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent($cacheEntryRemovedEvents));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: if an error happens in any app handling the previous per-item events, the core will never get a chance to handle this event leaving the data around. This is more related to handling events in code, rather than this PR itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to send the per-item events after the grouped-item event

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is worse, it means no events gets emitted if there is an issue halway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CarlSchwan if you want to keep the per-item event handling after the grouped one, to avoid what Côme mentions, maybe you can wrap the first with a try-catch(Exception), so at least we can try emitting the single ones? The same could be done in reverse, I guess.

Anyways the topic of "what happens when a request crashes" during/before event handling is something that cannot be solved in this PR specifically, I just wanted to raise the point that this may lead to issues and unfortunately there is no easy way around it.

foreach ($cacheEntryRemovedEvents as $cacheEntryRemovedEvent) {
$this->eventDispatcher->dispatchTyped($cacheEntryRemovedEvent);
}
}
Expand Down Expand Up @@ -784,7 +789,10 @@ public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) {
$this->connection->commit();

if ($sourceCache->getNumericStorageId() !== $this->getNumericStorageId()) {
$this->eventDispatcher->dispatchTyped(new CacheEntryRemovedEvent($this->storage, $sourcePath, $sourceId, $sourceCache->getNumericStorageId()));
$event = new CacheEntryRemovedEvent($this->storage, $sourcePath, $sourceId, $sourceCache->getNumericStorageId());
$this->eventDispatcher->dispatchTyped($event);
$this->eventDispatcher->dispatchTyped(new CacheEntriesRemovedEvent([$event]));

$event = new CacheEntryInsertedEvent($this->storage, $targetPath, $sourceId, $this->getNumericStorageId());
$this->eventDispatcher->dispatch(CacheInsertEvent::class, $event);
$this->eventDispatcher->dispatchTyped($event);
Expand Down
18 changes: 16 additions & 2 deletions lib/private/FilesMetadata/FilesMetadataManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use OCP\DB\Exception as DBException;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Cache\CacheEntriesRemovedEvent;
use OCP\Files\Events\Node\NodeWrittenEvent;
use OCP\Files\InvalidPathException;
use OCP\Files\Node;
Expand Down Expand Up @@ -214,6 +214,20 @@ public function deleteMetadata(int $fileId): void {
}
}

public function deleteMetadataForFiles(int $storage, array $fileIds): void {
try {
$this->metadataRequestService->dropMetadataForFiles($storage, $fileIds);
} catch (Exception $e) {
$this->logger->warning('issue while deleteMetadata', ['exception' => $e, 'fileIds' => $fileIds]);
}

try {
$this->indexRequestService->dropIndexForFiles($fileIds);
} catch (Exception $e) {
$this->logger->warning('issue while deleteMetadata', ['exception' => $e, 'fileIds' => $fileIds]);
}
}

/**
* @param IQueryBuilder $qb
* @param string $fileTableAlias alias of the table that contains data about files
Expand Down Expand Up @@ -301,6 +315,6 @@ public function initMetadata(
*/
public static function loadListeners(IEventDispatcher $eventDispatcher): void {
$eventDispatcher->addServiceListener(NodeWrittenEvent::class, MetadataUpdate::class);
$eventDispatcher->addServiceListener(CacheEntryRemovedEvent::class, MetadataDelete::class);
$eventDispatcher->addServiceListener(CacheEntriesRemovedEvent::class, MetadataDelete::class);
}
}
23 changes: 17 additions & 6 deletions lib/private/FilesMetadata/Listener/MetadataDelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
use Exception;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Cache\CacheEntriesRemovedEvent;
use OCP\FilesMetadata\IFilesMetadataManager;
use Psr\Log\LoggerInterface;

/**
* Handle file deletion event and remove stored metadata related to the deleted file
*
* @template-implements IEventListener<CacheEntryRemovedEvent>
* @template-implements IEventListener<CacheEntriesRemovedEvent>
*/
class MetadataDelete implements IEventListener {
public function __construct(
Expand All @@ -28,14 +28,25 @@ public function __construct(
}

public function handle(Event $event): void {
if (!($event instanceof CacheEntryRemovedEvent)) {
if (!($event instanceof CacheEntriesRemovedEvent)) {
return;
}

$entries = $event->getCacheEntryRemovedEvents();
$storageToFileIds = [];

foreach ($entries as $entry) {
try {
$storageToFileIds[$entry->getStorageId()] ??= [];
$storageToFileIds[$entry->getStorageId()][] = $entry->getFileId();
} catch (Exception $e) {
$this->logger->warning('issue while running MetadataDelete', ['exception' => $e]);
}
}

try {
$nodeId = $event->getFileId();
if ($nodeId > 0) {
$this->filesMetadataManager->deleteMetadata($nodeId);
foreach ($storageToFileIds as $storageId => $fileIds) {
$this->filesMetadataManager->deleteMetadataForFiles($storageId, $fileIds);
}
} catch (Exception $e) {
$this->logger->warning('issue while running MetadataDelete', ['exception' => $e]);
Expand Down
26 changes: 26 additions & 0 deletions lib/private/FilesMetadata/Service/IndexRequestService.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,30 @@ public function dropIndex(int $fileId, string $key = ''): void {

$qb->executeStatement();
}

/**
* Drop indexes related to multiple file ids
* if a key is specified, only drop entries related to it
*
* @param int[] $fileIds file ids
* @param string $key metadata key
*
* @throws DbException
*/
public function dropIndexForFiles(array $fileIds, string $key = ''): void {
$chunks = array_chunk($fileIds, 1000);

foreach ($chunks as $chunk) {
$qb = $this->dbConnection->getQueryBuilder();
$expr = $qb->expr();
$qb->delete(self::TABLE_METADATA_INDEX)
->where($expr->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));

if ($key !== '') {
$qb->andWhere($expr->eq('meta_key', $qb->createNamedParameter($key)));
}

$qb->executeStatement();
}
}
}
22 changes: 20 additions & 2 deletions lib/private/FilesMetadata/Service/MetadataRequestService.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,10 @@ public function getMetadataFromFileId(int $fileId): IFilesMetadata {
*/
public function getMetadataFromFileIds(array $fileIds): array {
$qb = $this->dbConnection->getQueryBuilder();
$qb->select('file_id', 'json', 'sync_token')->from(self::TABLE_METADATA);
$qb->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)));
$qb->select('file_id', 'json', 'sync_token')
->from(self::TABLE_METADATA)
->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
->runAcrossAllShards();

$list = [];
$result = $qb->executeQuery();
Expand Down Expand Up @@ -143,6 +145,22 @@ public function dropMetadata(int $fileId): void {
$qb->executeStatement();
}

/**
* @param int[] $fileIds
* @throws Exception
*/
public function dropMetadataForFiles(int $storage, array $fileIds): void {
$chunks = array_chunk($fileIds, 1000);

foreach ($chunks as $chunk) {
$qb = $this->dbConnection->getQueryBuilder();
$qb->delete(self::TABLE_METADATA)
->where($qb->expr()->in('file_id', $qb->createNamedParameter($fileIds, IQueryBuilder::PARAM_INT_ARRAY)))
->hintShardKey('storage', $storage);
$qb->executeStatement();
}
}

/**
* update metadata in the database
*
Expand Down
35 changes: 35 additions & 0 deletions lib/public/Files/Cache/CacheEntriesRemovedEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\Files\Cache;

use OCP\EventDispatcher\Event;

/**
* Meta-event wrapping multiple CacheEntryRemovedEvent for when an existing
* entry in the cache gets removed.
*
* @since 32.0.0
*/
#[\OCP\AppFramework\Attribute\Listenable(since: '32.0.0')]
class CacheEntriesRemovedEvent extends Event {
/**
* @param CacheEntryRemovedEvent[] $cacheEntryRemovedEvents
*/
public function __construct(
private readonly array $cacheEntryRemovedEvents,
) {
}

/**
* @return CacheEntryRemovedEvent[]
*/
public function getCacheEntryRemovedEvents(): array {
return $this->cacheEntryRemovedEvents;
}
}
4 changes: 4 additions & 0 deletions lib/public/Files/Cache/CacheEntryRemovedEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
/**
* Event for when an existing entry in the cache gets removed
*
* Prefer using CacheEntriesRemovedEvent as it is more efficient when deleting
* multiple files at the same time.
*
* @since 21.0.0
* @see CacheEntriesRemovedEvent
*/
class CacheEntryRemovedEvent extends AbstractCacheEvent implements ICacheEvent {
}
12 changes: 12 additions & 0 deletions lib/public/FilesMetadata/IFilesMetadataManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace OCP\FilesMetadata;

use OCP\AppFramework\Attribute\Consumable;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Node;
use OCP\FilesMetadata\Exceptions\FilesMetadataException;
Expand All @@ -20,6 +21,7 @@
*
* @since 28.0.0
*/
#[Consumable(since: '28.0.0')]
interface IFilesMetadataManager {
/** @since 28.0.0 */
public const PROCESS_LIVE = 1;
Expand Down Expand Up @@ -98,6 +100,16 @@ public function saveMetadata(IFilesMetadata $filesMetadata): void;
*/
public function deleteMetadata(int $fileId): void;

/**
* Delete metadata and its indexes of multiple file ids
*
* @param int $storage The storage id coresponding to the $fileIds
* @param array<int> $fileIds file ids
* @return void
* @since 32.0.0
*/
public function deleteMetadataForFiles(int $storage, array $fileIds): void;

/**
* generate and return a MetadataQuery to help building sql queries
*
Expand Down
Loading