Support dynamic metadata request on PROPFIND requests

Signed-off-by: Louis Chemineau <louis@chmn.me>
nextcloud · Oct 18, 2023 · 8579aa3 · 8579aa3
1 parent 6ee9375
commit 8579aa3
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 74 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
@@ -34,19 +34,16 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OC\AppFramework\Http\Request;
-use OC\Metadata\IMetadataManager;
 use OCP\Constants;
 use OCP\Files\ForbiddenException;
 use OCP\Files\StorageNotAvailableException;
 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\ServerPlugin;
@@ -84,17 +81,7 @@ class FilesPlugin extends ServerPlugin {
 	public const SHARE_NOTE = '{http://nextcloud.org/ns}note';
 	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_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',
-	];
+	public const FILE_METADATA_PREFIX = '{http://nextcloud.org/ns}metadata-';
 
 	/** Reference to main server object */
 	private ?Server $server = null;
@@ -398,6 +385,10 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
 			$propFind->handle(self::DISPLAYNAME_PROPERTYNAME, function () use ($node) {
 				return $node->getName();
 			});
+
+			foreach ($node->getFileInfo()->getMetadata() as $metadataKey => $metadataValue) {
+				$propFind->handle(self::FILE_METADATA_PREFIX.$metadataKey, $metadataValue->getValueAny());
+			}
 		}
 
 		if ($node instanceof \OCA\DAV\Connector\Sabre\File) {
@@ -427,31 +418,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) {
@@ -465,39 +431,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;

diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php
@@ -59,6 +59,7 @@
 use OCP\Files\Search\ISearchOperator;
 use OCP\Files\Search\ISearchQuery;
 use OCP\Files\Storage\IStorage;
+use OCP\FilesMetadata\IFilesMetadataManager;
 use OCP\IDBConnection;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
@@ -132,7 +133,8 @@ protected function getQueryBuilder() {
 		return new CacheQueryBuilder(
 			$this->connection,
 			\OC::$server->getSystemConfig(),
-			\OC::$server->get(LoggerInterface::class)
+			\OC::$server->get(LoggerInterface::class),
+			\OC::$server->get(IFilesMetadataManager::class),
 		);
 	}
 
@@ -154,6 +156,7 @@ public function getNumericStorageId() {
 	public function get($file) {
 		$query = $this->getQueryBuilder();
 		$query->selectFileCache();
+		$metadataQuery = $query->selectMetadata();
 
 		if (is_string($file) || $file == '') {
 			// normalize file
@@ -169,6 +172,8 @@ public function get($file) {
 		$data = $result->fetch();
 		$result->closeCursor();
 
+		$data['metadata'] = $metadataQuery?->extractMetadata($data)?->jsonSerialize() ?? [];
+
 		//merge partial data
 		if (!$data && is_string($file) && isset($this->partial[$file])) {
 			return $this->partial[$file];

diff --git a/lib/private/Files/Cache/CacheQueryBuilder.php b/lib/private/Files/Cache/CacheQueryBuilder.php
@@ -28,6 +28,8 @@
 use OC\DB\QueryBuilder\QueryBuilder;
 use OC\SystemConfig;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\FilesMetadata\IFilesMetadataManager;
+use OCP\FilesMetadata\Model\IMetadataQuery;
 use OCP\IDBConnection;
 use Psr\Log\LoggerInterface;
 
@@ -37,7 +39,12 @@
 class CacheQueryBuilder extends QueryBuilder {
 	private $alias = null;
 
-	public function __construct(IDBConnection $connection, SystemConfig $systemConfig, LoggerInterface $logger) {
+	public function __construct(
+		IDBConnection $connection,
+		SystemConfig $systemConfig,
+		LoggerInterface $logger,
+		private IFilesMetadataManager $filesMetadataManager,
+	) {
 		parent::__construct($connection, $systemConfig, $logger);
 	}
 
@@ -126,4 +133,10 @@ public function whereParentInParameter(string $parameter) {
 
 		return $this;
 	}
+
+	public function selectMetadata(): IMetadataQuery {
+		$metadataQuery = $this->filesMetadataManager->getMetadataQuery($this, $this->alias, 'fileid');
+		$metadataQuery->retrieveMetadata();
+		return $metadataQuery;
+	}
 }
diff --git a/lib/private/FilesMetadata/Model/MetadataValueWrapper.php b/lib/private/FilesMetadata/Model/MetadataValueWrapper.php
@@ -252,6 +252,18 @@ public function getValueStringList(): array {
 		return (array) $this->value;
 	}
 
+	/**
+	 * @return string|int|float|bool|array|string[]|int[]
+	 * @throws FilesMetadataNotFoundException
+	 */
+	public function getValueAny(): mixed {
+		if (null === $this->value) {
+			throw new FilesMetadataNotFoundException('value is not set');
+		}
+
+		return $this->value;
+	}
+
 	/**
 	 * @return array
 	 * @throws FilesMetadataTypeException