Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dynamic metadata request on PROPFIND requests #40964

Merged
merged 1 commit into from
Nov 8, 2023
Merged
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: 0 additions & 4 deletions apps/dav/lib/Connector/Sabre/Directory.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

use OC\Files\Mount\MoveableMount;
use OC\Files\View;
use OC\Metadata\FileMetadata;
use OCA\DAV\AppInfo\Application;
use OCA\DAV\Connector\Sabre\Exception\FileLocked;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
Expand Down Expand Up @@ -70,9 +69,6 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol
private ?array $quotaInfo = null;
private ?CachingTree $tree = null;

/** @var array<string, array<int, FileMetadata>> */
private array $metadata = [];

/**
* Sets up the node, expects a full path name
*/
Expand Down
16 changes: 0 additions & 16 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
use OC\Files\Filesystem;
use OC\Files\Stream\HashWrapper;
use OC\Files\View;
use OC\Metadata\FileMetadata;
use OCA\DAV\AppInfo\Application;
use OCA\DAV\Connector\Sabre\Exception\BadGateway;
use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge;
Expand Down Expand Up @@ -81,9 +80,6 @@ class File extends Node implements IFile {
protected IRequest $request;
protected IL10N $l10n;

/** @var array<string, FileMetadata> */
private array $metadata = [];

/**
* Sets up the node, expects a full path name
*
Expand Down Expand Up @@ -796,16 +792,4 @@ public function hash(string $type) {
public function getNode(): \OCP\Files\File {
return $this->node;
}

public function getMetadata(string $group): FileMetadata {
return $this->metadata[$group];
}

public function setMetadata(string $group, FileMetadata $metadata): void {
$this->metadata[$group] = $metadata;
}

public function hasMetadata(string $group) {
return array_key_exists($group, $this->metadata);
}
}
127 changes: 55 additions & 72 deletions apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,20 @@
namespace OCA\DAV\Connector\Sabre;

use OC\AppFramework\Http\Request;
use OC\Metadata\IMetadataManager;
use OC\FilesMetadata\Model\MetadataValueWrapper;
use OCP\Constants;
use OCP\Files\ForbiddenException;
use OCP\Files\StorageNotAvailableException;
use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException;
use OCP\FilesMetadata\IFilesMetadataManager;
use OCP\FilesMetadata\Model\IMetadataValueWrapper;
use OCP\IConfig;
use OCP\IPreview;
use OCP\IRequest;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\IFile;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\PropPatch;
use Sabre\DAV\Server;
Expand Down Expand Up @@ -86,17 +87,6 @@ class FilesPlugin extends ServerPlugin {
public const SUBFOLDER_COUNT_PROPERTYNAME = '{http://nextcloud.org/ns}contained-folder-count';
public const SUBFILE_COUNT_PROPERTYNAME = '{http://nextcloud.org/ns}contained-file-count';
public const FILE_METADATA_PREFIX = '{http://nextcloud.org/ns}metadata-';
public const FILE_METADATA_SIZE = '{http://nextcloud.org/ns}file-metadata-size';
public const FILE_METADATA_GPS = '{http://nextcloud.org/ns}file-metadata-gps';

public const ALL_METADATA_PROPS = [
self::FILE_METADATA_SIZE => 'size',
self::FILE_METADATA_GPS => 'gps',
];
public const METADATA_MIMETYPES = [
'size' => 'image',
'gps' => 'image',
];

/** Reference to main server object */
private ?Server $server = null;
Expand Down Expand Up @@ -434,31 +424,6 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
$propFind->handle(self::UPLOAD_TIME_PROPERTYNAME, function () use ($node) {
return $node->getFileInfo()->getUploadTime();
});

if ($this->config->getSystemValueBool('enable_file_metadata', true)) {
foreach (self::ALL_METADATA_PROPS as $prop => $meta) {
$propFind->handle($prop, function () use ($node, $meta) {
if ($node->getFileInfo()->getMimePart() !== self::METADATA_MIMETYPES[$meta]) {
return [];
}

if ($node->hasMetadata($meta)) {
$metadata = $node->getMetadata($meta);
} else {
// This code path should not be called since we try to preload
// the metadata when loading the folder or the search results
// in one go
$metadataManager = \OC::$server->get(IMetadataManager::class);
$metadata = $metadataManager->fetchMetadataFor($meta, [$node->getId()])[$node->getId()];

// TODO would be nice to display this in the profiler...
\OC::$server->get(LoggerInterface::class)->debug('Inefficient fetching of metadata');
}

return $metadata->getValue();
});
}
}
}

if ($node instanceof Directory) {
Expand All @@ -472,39 +437,6 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)

$requestProperties = $propFind->getRequestedProperties();

$requestedMetaData = [];
foreach ($requestProperties as $requestProperty) {
if (isset(self::ALL_METADATA_PROPS[$requestProperty])) {
$requestedMetaData[] = self::ALL_METADATA_PROPS[$requestProperty];
}
}
if (
$this->config->getSystemValueBool('enable_file_metadata', true) &&
$propFind->getDepth() === 1 &&
$requestedMetaData
) {
$children = $node->getChildren();
// Preloading of the metadata

/** @var IMetaDataManager $metadataManager */
$metadataManager = \OC::$server->get(IMetadataManager::class);

foreach ($requestedMetaData as $requestedMeta) {
$relevantMimeType = self::METADATA_MIMETYPES[$requestedMeta];
$childrenForMeta = array_filter($children, function (INode $child) use ($relevantMimeType) {
return $child instanceof File && $child->getFileInfo()->getMimePart() === $relevantMimeType;
});
$fileIds = array_map(function (File $child) {
return $child->getFileInfo()->getId();
}, $childrenForMeta);
$preloadedMetadata = $metadataManager->fetchMetadataFor($requestedMeta, $fileIds);

foreach ($childrenForMeta as $child) {
$child->setMetadata($requestedMeta, $preloadedMetadata[$child->getFileInfo()->getId()]);
}
}
}

if (in_array(self::SUBFILE_COUNT_PROPERTYNAME, $requestProperties, true)
|| in_array(self::SUBFOLDER_COUNT_PROPERTYNAME, $requestProperties, true)) {
$nbFiles = 0;
Expand Down Expand Up @@ -590,6 +522,57 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
$node->setCreationTime((int) $time);
return true;
});


/** @var IFilesMetadataManager */
$filesMetadataManager = \OCP\Server::get(IFilesMetadataManager::class);
$knownMetadata = $filesMetadataManager->getKnownMetadata();

foreach ($propPatch->getRemainingMutations() as $mutation) {
if (!str_starts_with($mutation, self::FILE_METADATA_PREFIX)) {
continue;
}

$propPatch->handle($mutation, function (mixed $value) use ($knownMetadata, $node, $mutation, $filesMetadataManager): bool {
$metadata = $filesMetadataManager->getMetadata((int)$node->getFileId(), true);
$metadataKey = substr($mutation, strlen(self::FILE_METADATA_PREFIX));

// If the metadata is unknown, it defaults to string.
try {
$type = $knownMetadata->getType($metadataKey);
} catch (FilesMetadataNotFoundException) {
$type = IMetadataValueWrapper::TYPE_STRING;
}

switch ($type) {
case IMetadataValueWrapper::TYPE_STRING:
$metadata->setString($metadataKey, $value, $knownMetadata->isIndex($metadataKey));
break;
case IMetadataValueWrapper::TYPE_INT:
$metadata->setInt($metadataKey, $value, $knownMetadata->isIndex($metadataKey));
break;
case IMetadataValueWrapper::TYPE_FLOAT:
$metadata->setFloat($metadataKey, $value);
break;
case IMetadataValueWrapper::TYPE_BOOL:
$metadata->setBool($metadataKey, $value, $knownMetadata->isIndex($metadataKey));
break;
case IMetadataValueWrapper::TYPE_ARRAY:
$metadata->setArray($metadataKey, $value);
break;
case IMetadataValueWrapper::TYPE_STRING_LIST:
$metadata->setStringList($metadataKey, $value, $knownMetadata->isIndex($metadataKey));
break;
case IMetadataValueWrapper::TYPE_INT_LIST:
$metadata->setIntList($metadataKey, $value, $knownMetadata->isIndex($metadataKey));
break;
}

$filesMetadataManager->saveMetadata($metadata);
return true;
});
}

/**
* Disable modification of the displayname property for files and
* folders via PROPPATCH. See PROPFIND for more information.
Expand Down
23 changes: 0 additions & 23 deletions apps/dav/lib/Files/FileSearchBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
use OC\Files\Search\SearchOrder;
use OC\Files\Search\SearchQuery;
use OC\Files\View;
use OC\Metadata\IMetadataManager;
use OCA\DAV\Connector\Sabre\CachingTree;
use OCA\DAV\Connector\Sabre\Directory;
use OCA\DAV\Connector\Sabre\FilesPlugin;
Expand Down Expand Up @@ -115,7 +114,6 @@ public function getPropertyDefinitionsForScope(string $href, ?string $path): arr
new SearchPropertyDefinition(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME, true, false, false),
new SearchPropertyDefinition(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME, true, false, false),
new SearchPropertyDefinition(FilesPlugin::HAS_PREVIEW_PROPERTYNAME, true, false, false, SearchPropertyDefinition::DATATYPE_BOOLEAN),
new SearchPropertyDefinition(FilesPlugin::FILE_METADATA_SIZE, true, false, false, SearchPropertyDefinition::DATATYPE_STRING),
new SearchPropertyDefinition(FilesPlugin::FILEID_PROPERTYNAME, true, false, false, SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER),
];

Expand Down Expand Up @@ -152,27 +150,6 @@ private function getPropertyDefinitionsForMetadata(): array {
* @param string[] $requestProperties
*/
public function preloadPropertyFor(array $nodes, array $requestProperties): void {
if (in_array(FilesPlugin::FILE_METADATA_SIZE, $requestProperties, true)) {
// Preloading of the metadata
$fileIds = [];
foreach ($nodes as $node) {
/** @var \OCP\Files\Node|\OCA\DAV\Connector\Sabre\Node $node */
if (str_starts_with($node->getFileInfo()->getMimeType(), 'image/')) {
/** @var \OCA\DAV\Connector\Sabre\File $node */
$fileIds[] = $node->getFileInfo()->getId();
}
}
/** @var IMetaDataManager $metadataManager */
$metadataManager = \OC::$server->get(IMetadataManager::class);
$preloadedMetadata = $metadataManager->fetchMetadataFor('size', $fileIds);
foreach ($nodes as $node) {
/** @var \OCP\Files\Node|\OCA\DAV\Connector\Sabre\Node $node */
if (str_starts_with($node->getFileInfo()->getMimeType(), 'image/')) {
/** @var \OCA\DAV\Connector\Sabre\File $node */
$node->setMetadata('size', $preloadedMetadata[$node->getFileInfo()->getId()]);
}
}
}
}

/**
Expand Down
3 changes: 3 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
* This file is licensed under the Affero General Public License version 3 or
* later.
* See the COPYING-README file.
*
* @group DB
*/
class FilesPluginTest extends TestCase {
public const GETETAG_PROPERTYNAME = FilesPlugin::GETETAG_PROPERTYNAME;
Expand Down Expand Up @@ -424,6 +426,7 @@ public function testUpdateProps(): void {
self::CREATIONDATE_PROPERTYNAME => $testCreationDate,
]);


$this->plugin->handleUpdateProperties(
'/dummypath',
$propPatch
Expand Down
7 changes: 0 additions & 7 deletions apps/files/lib/Command/Scan.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@
use OC\DB\ConnectionAdapter;
use OC\FilesMetadata\FilesMetadataManager;
use OC\ForbiddenException;
use OC\Metadata\MetadataManager;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Events\FileCacheUpdated;
use OCP\Files\Events\NodeAddedToCache;
use OCP\Files\Events\NodeRemovedFromCache;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\NotFoundException;
Expand All @@ -71,7 +69,6 @@ class Scan extends Base {
public function __construct(
private IUserManager $userManager,
private IRootFolder $rootFolder,
private MetadataManager $metadataManager,
private FilesMetadataManager $filesMetadataManager,
private IEventDispatcher $eventDispatcher,
private LoggerInterface $logger,
Expand Down Expand Up @@ -141,10 +138,6 @@ protected function scanFiles(string $user, string $path, bool $scanMetadata, Out
$this->abortIfInterrupted();
if ($scanMetadata) {
$node = $this->rootFolder->get($path);
if ($node instanceof File) {
$this->metadataManager->generateMetadata($node, false);
}

$this->filesMetadataManager->refreshMetadata(
$node,
IFilesMetadataManager::PROCESS_LIVE | IFilesMetadataManager::PROCESS_BACKGROUND
Expand Down
4 changes: 3 additions & 1 deletion apps/files_trashbin/lib/Trashbin.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
use OCP\Files\Folder;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\FilesMetadata\IFilesMetadataManager;
use OCP\IConfig;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
Expand Down Expand Up @@ -993,7 +994,8 @@ private static function getVersionsFromTrash($filename, $timestamp, string $user
$query = new CacheQueryBuilder(
\OC::$server->getDatabaseConnection(),
\OC::$server->getSystemConfig(),
\OC::$server->get(LoggerInterface::class)
\OC::$server->get(LoggerInterface::class),
\OC::$server->get(IFilesMetadataManager::class),
);
$normalizedParentPath = ltrim(Filesystem::normalizePath(dirname('files_trashbin/versions/'. $filename)), '/');
$parentId = $cache->getId($normalizedParentPath);
Expand Down
17 changes: 0 additions & 17 deletions core/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
use OC\Authentication\Notifications\Notifier as AuthenticationNotifier;
use OC\Core\Listener\BeforeTemplateRenderedListener;
use OC\Core\Notification\CoreNotifier;
use OC\Metadata\FileEventListener;
use OC\TagManager;
use OCP\AppFramework\App;
use OCP\AppFramework\Http\Events\BeforeLoginTemplateRenderedEvent;
Expand All @@ -54,13 +53,9 @@
use OCP\DB\Events\AddMissingPrimaryKeyEvent;
use OCP\DB\Types;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Events\Node\NodeDeletedEvent;
use OCP\Files\Events\Node\NodeWrittenEvent;
use OCP\Files\Events\NodeRemovedFromCache;
use OCP\User\Events\BeforeUserDeletedEvent;
use OCP\User\Events\UserDeletedEvent;
use OCP\Util;
use OCP\IConfig;

/**
* Class Application
Expand Down Expand Up @@ -331,18 +326,6 @@ public function __construct() {
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedFilesCleanupListener::class);
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedWebAuthnCleanupListener::class);

// Metadata
/** @var IConfig $config */
$config = $container->get(IConfig::class);
if ($config->getSystemValueBool('enable_file_metadata', true)) {
/** @psalm-suppress InvalidArgument */
$eventDispatcher->addServiceListener(NodeDeletedEvent::class, FileEventListener::class);
/** @psalm-suppress InvalidArgument */
$eventDispatcher->addServiceListener(NodeRemovedFromCache::class, FileEventListener::class);
/** @psalm-suppress InvalidArgument */
$eventDispatcher->addServiceListener(NodeWrittenEvent::class, FileEventListener::class);
}

// Tags
$eventDispatcher->addServiceListener(UserDeletedEvent::class, TagManager::class);
}
Expand Down
8 changes: 0 additions & 8 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1496,14 +1496,6 @@
'OC\\Memcache\\Redis' => $baseDir . '/lib/private/Memcache/Redis.php',
'OC\\Memcache\\WithLocalCache' => $baseDir . '/lib/private/Memcache/WithLocalCache.php',
'OC\\MemoryInfo' => $baseDir . '/lib/private/MemoryInfo.php',
'OC\\Metadata\\Capabilities' => $baseDir . '/lib/private/Metadata/Capabilities.php',
'OC\\Metadata\\FileEventListener' => $baseDir . '/lib/private/Metadata/FileEventListener.php',
'OC\\Metadata\\FileMetadata' => $baseDir . '/lib/private/Metadata/FileMetadata.php',
'OC\\Metadata\\FileMetadataMapper' => $baseDir . '/lib/private/Metadata/FileMetadataMapper.php',
'OC\\Metadata\\IMetadataManager' => $baseDir . '/lib/private/Metadata/IMetadataManager.php',
'OC\\Metadata\\IMetadataProvider' => $baseDir . '/lib/private/Metadata/IMetadataProvider.php',
'OC\\Metadata\\MetadataManager' => $baseDir . '/lib/private/Metadata/MetadataManager.php',
'OC\\Metadata\\Provider\\ExifProvider' => $baseDir . '/lib/private/Metadata/Provider/ExifProvider.php',
'OC\\Migration\\BackgroundRepair' => $baseDir . '/lib/private/Migration/BackgroundRepair.php',
'OC\\Migration\\ConsoleOutput' => $baseDir . '/lib/private/Migration/ConsoleOutput.php',
'OC\\Migration\\SimpleOutput' => $baseDir . '/lib/private/Migration/SimpleOutput.php',
Expand Down
Loading
Loading