From 3f86f4e1ff44e30d0b9cfb71a8f1514418a6f3b3 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Wed, 14 Feb 2024 13:30:11 -0100 Subject: [PATCH] feat(metadata): migrate to lazy appconfig Signed-off-by: Maxence Lange --- lib/private/Files/Cache/Cache.php | 4 +- lib/private/Files/Cache/CacheQueryBuilder.php | 6 +-- lib/private/Files/Cache/QuerySearchHelper.php | 7 +-- .../FilesMetadata/FilesMetadataManager.php | 44 ++++--------------- lib/private/FilesMetadata/MetadataQuery.php | 30 ++++++++++++- .../FilesMetadata/IFilesMetadataManager.php | 16 ++++--- lib/public/FilesMetadata/IMetadataQuery.php | 2 + 7 files changed, 55 insertions(+), 54 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 8f0f962a3b771..62e1e57a17174 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -171,7 +171,7 @@ public function get($file) { } elseif (!$data) { return $data; } else { - $data['metadata'] = $metadataQuery?->extractMetadata($data)->asArray() ?? []; + $data['metadata'] = $metadataQuery->extractMetadata($data)->asArray(); return self::cacheEntryFromData($data, $this->mimetypeLoader); } } @@ -243,7 +243,7 @@ public function getFolderContentsById($fileId) { $result->closeCursor(); return array_map(function (array $data) use ($metadataQuery) { - $data['metadata'] = $metadataQuery?->extractMetadata($data)->asArray() ?? []; + $data['metadata'] = $metadataQuery->extractMetadata($data)->asArray(); return self::cacheEntryFromData($data, $this->mimetypeLoader); }, $files); } diff --git a/lib/private/Files/Cache/CacheQueryBuilder.php b/lib/private/Files/Cache/CacheQueryBuilder.php index d80375e94ee25..a237dd915d735 100644 --- a/lib/private/Files/Cache/CacheQueryBuilder.php +++ b/lib/private/Files/Cache/CacheQueryBuilder.php @@ -138,11 +138,11 @@ public function whereParentInParameter(string $parameter) { /** * join metadata to current query builder and returns an helper * - * @return IMetadataQuery|null NULL if no metadata have never been generated + * @return IMetadataQuery */ - public function selectMetadata(): ?IMetadataQuery { + public function selectMetadata(): IMetadataQuery { $metadataQuery = $this->filesMetadataManager->getMetadataQuery($this, $this->alias, 'fileid'); - $metadataQuery?->retrieveMetadata(); + $metadataQuery->retrieveMetadata(); return $metadataQuery; } } diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index d8c5e66e12951..849135d44f00e 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -195,12 +195,7 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array $files = $result->fetchAll(); $rawEntries = array_map(function (array $data) use ($metadataQuery) { - // migrate to null safe ... - if ($metadataQuery === null) { - $data['metadata'] = []; - } else { - $data['metadata'] = $metadataQuery->extractMetadata($data)->asArray(); - } + $data['metadata'] = $metadataQuery->extractMetadata($data)->asArray(); return Cache::cacheEntryFromData($data, $this->mimetypeLoader); }, $files); diff --git a/lib/private/FilesMetadata/FilesMetadataManager.php b/lib/private/FilesMetadata/FilesMetadataManager.php index a684b870637b6..08c1b4f459c6e 100644 --- a/lib/private/FilesMetadata/FilesMetadataManager.php +++ b/lib/private/FilesMetadata/FilesMetadataManager.php @@ -51,8 +51,7 @@ use OCP\FilesMetadata\IMetadataQuery; use OCP\FilesMetadata\Model\IFilesMetadata; use OCP\FilesMetadata\Model\IMetadataValueWrapper; -use OCP\IConfig; -use OCP\IDBConnection; +use OCP\IAppConfig; use Psr\Log\LoggerInterface; /** @@ -69,7 +68,7 @@ class FilesMetadataManager implements IFilesMetadataManager { public function __construct( private IEventDispatcher $eventDispatcher, private IJobList $jobList, - private IConfig $config, + private IAppConfig $appConfig, private LoggerInterface $logger, private MetadataRequestService $metadataRequestService, private IndexRequestService $indexRequestService, @@ -206,7 +205,7 @@ public function saveMetadata(IFilesMetadata $filesMetadata): void { // update metadata types list $current = $this->getKnownMetadata(); $current->import($filesMetadata->jsonSerialize(true)); - $this->config->setAppValue('core', self::CONFIG_KEY, json_encode($current)); + $this->appConfig->setValueArray('core', self::CONFIG_KEY, $current->jsonSerialize(), lazy: true); } /** @@ -235,7 +234,7 @@ public function deleteMetadata(int $fileId): void { * @param string $fileIdField alias of the field that contains file ids * * @inheritDoc - * @return IMetadataQuery|null + * @return IMetadataQuery * @see IMetadataQuery * @since 28.0.0 */ @@ -243,12 +242,8 @@ public function getMetadataQuery( IQueryBuilder $qb, string $fileTableAlias, string $fileIdField - ): ?IMetadataQuery { - if (!$this->metadataInitiated()) { - return null; - } - - return new MetadataQuery($qb, $this->getKnownMetadata(), $fileTableAlias, $fileIdField); + ): IMetadataQuery { + return new MetadataQuery($qb, $this, $fileTableAlias, $fileIdField); } /** @@ -263,8 +258,7 @@ public function getKnownMetadata(): IFilesMetadata { $this->all = new FilesMetadata(); try { - $data = json_decode($this->config->getAppValue('core', self::CONFIG_KEY, '[]'), true, 127, JSON_THROW_ON_ERROR); - $this->all->import($data); + $this->all->import($this->appConfig->getValueArray('core', self::CONFIG_KEY, lazy: true)); } catch (JsonException) { $this->logger->warning('issue while reading stored list of metadata. Advised to run ./occ files:scan --all --generate-metadata'); } @@ -310,7 +304,7 @@ public function initMetadata( } $current->import([$key => ['type' => $type, 'indexed' => $indexed, 'editPermission' => $editPermission]]); - $this->config->setAppValue('core', self::CONFIG_KEY, json_encode($current)); + $this->appConfig->setValueArray('core', self::CONFIG_KEY, $current->jsonSerialize(), lazy: true); $this->all = $current; } @@ -323,26 +317,4 @@ public static function loadListeners(IEventDispatcher $eventDispatcher): void { $eventDispatcher->addServiceListener(NodeWrittenEvent::class, MetadataUpdate::class); $eventDispatcher->addServiceListener(CacheEntryRemovedEvent::class, MetadataDelete::class); } - - /** - * Will confirm that tables were created and store an app value to cache the result. - * Can be removed in 29 as this is to avoid strange situation when Nextcloud files were - * replaced but the upgrade was not triggered yet. - * - * @return bool - */ - private function metadataInitiated(): bool { - if ($this->config->getAppValue('core', self::MIGRATION_DONE, '0') === '1') { - return true; - } - - $dbConnection = \OCP\Server::get(IDBConnection::class); - if ($dbConnection->tableExists(MetadataRequestService::TABLE_METADATA)) { - $this->config->setAppValue('core', self::MIGRATION_DONE, '1'); - - return true; - } - - return false; - } } diff --git a/lib/private/FilesMetadata/MetadataQuery.php b/lib/private/FilesMetadata/MetadataQuery.php index aa079c678d77a..e7bb0225174a7 100644 --- a/lib/private/FilesMetadata/MetadataQuery.php +++ b/lib/private/FilesMetadata/MetadataQuery.php @@ -31,9 +31,11 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\FilesMetadata\Exceptions\FilesMetadataNotFoundException; use OCP\FilesMetadata\Exceptions\FilesMetadataTypeException; +use OCP\FilesMetadata\IFilesMetadataManager; use OCP\FilesMetadata\IMetadataQuery; use OCP\FilesMetadata\Model\IFilesMetadata; use OCP\FilesMetadata\Model\IMetadataValueWrapper; +use Psr\Log\LoggerInterface; /** * @inheritDoc @@ -43,12 +45,23 @@ class MetadataQuery implements IMetadataQuery { private array $knownJoinedIndex = []; public function __construct( private IQueryBuilder $queryBuilder, - private IFilesMetadata $knownMetadata, + private IFilesMetadata|IFilesMetadataManager $manager, private string $fileTableAlias = 'fc', private string $fileIdField = 'fileid', private string $alias = 'meta', private string $aliasIndexPrefix = 'meta_index' ) { + if ($manager instanceof IFilesMetadata) { + /** + * Since 29, because knownMetadata is stored in lazy appconfig, it seems smarter + * to not call getKnownMetadata() at the load of this class as it is only needed + * in {@see getMetadataValueField}. + * + * FIXME: remove support for IFilesMetadata + */ + $logger = \OCP\Server::get(LoggerInterface::class); + $logger->debug('It is deprecated to use IFilesMetadata as second parameter when calling MetadataQuery::__construct()'); + } } /** @@ -158,7 +171,20 @@ public function getMetadataKeyField(string $metadataKey): string { * @since 28.0.0 */ public function getMetadataValueField(string $metadataKey): string { - return match ($this->knownMetadata->getType($metadataKey)) { + if ($this->manager instanceof IFilesMetadataManager) { + /** + * Since 29, because knownMetadata is stored in lazy appconfig, it seems smarter + * to not call getKnownMetadata() at the load of this class as it is only needed + * in this method. + * + * FIXME: keep only this line and remove support for previous IFilesMetadata in constructor + */ + $knownMetadata = $this->manager->getKnownMetadata(); + } else { + $knownMetadata = $this->manager; + } + + return match ($knownMetadata->getType($metadataKey)) { IMetadataValueWrapper::TYPE_STRING => $this->joinedTableAlias($metadataKey) . '.meta_value_string', IMetadataValueWrapper::TYPE_INT, IMetadataValueWrapper::TYPE_BOOL => $this->joinedTableAlias($metadataKey) . '.meta_value_int', default => throw new FilesMetadataTypeException('metadata is not set as indexed'), diff --git a/lib/public/FilesMetadata/IFilesMetadataManager.php b/lib/public/FilesMetadata/IFilesMetadataManager.php index 55feefc4f12b8..6dee48c264be2 100644 --- a/lib/public/FilesMetadata/IFilesMetadataManager.php +++ b/lib/public/FilesMetadata/IFilesMetadataManager.php @@ -122,7 +122,7 @@ public function deleteMetadata(int $fileId): void; * @param string $fileTableAlias alias of the table that contains data about files * @param string $fileIdField alias of the field that contains file ids * - * @return IMetadataQuery|null NULL if table are not set yet or never used + * @return IMetadataQuery * @see IMetadataQuery * @since 28.0.0 */ @@ -130,22 +130,27 @@ public function getMetadataQuery( IQueryBuilder $qb, string $fileTableAlias, string $fileIdField - ): ?IMetadataQuery; + ): IMetadataQuery; /** * returns all type of metadata currently available. * The list is stored in a IFilesMetadata with null values but correct type. * + * Note: this method loads lazy appconfig values. + * * @return IFilesMetadata * @since 28.0.0 */ public function getKnownMetadata(): IFilesMetadata; /** - * initiate a metadata key with its type. + * Initiate a metadata key with its type. + * * The call is mandatory before using the metadata property in a webdav request. - * It is not needed to only use this method when the app is enabled: the method can be - * called each time during the app loading as the metadata will only be initiated if not known + * The call should be part of a migration/repair step and not be called on app's boot + * process as it is using lazy-appconfig value + * + * Note: this method loads lazy appconfig values. * * @param string $key metadata key * @param string $type metadata type @@ -164,6 +169,7 @@ public function getKnownMetadata(): IFilesMetadata; * @see IMetadataValueWrapper::EDIT_REQ_WRITE_PERMISSION * @see IMetadataValueWrapper::EDIT_REQ_READ_PERMISSION * @since 28.0.0 + * @since 29.0.0 uses lazy config value - do not use this method out of repair steps */ public function initMetadata(string $key, string $type, bool $indexed, int $editPermission): void; } diff --git a/lib/public/FilesMetadata/IMetadataQuery.php b/lib/public/FilesMetadata/IMetadataQuery.php index c1c649ac24310..c3ea8c82d9dbb 100644 --- a/lib/public/FilesMetadata/IMetadataQuery.php +++ b/lib/public/FilesMetadata/IMetadataQuery.php @@ -83,6 +83,8 @@ public function getMetadataKeyField(string $metadataKey): string; /** * returns the name of the field for metadata string value to be used in query expressions * + * Note: this method loads lazy appconfig values. + * * @param string $metadataKey metadata key * * @return string table field