From: Louis Chemineau Date: Thu, 30 Mar 2023 13:58:42 +0000 (+0200) Subject: Migrate metadata as JSON to value as STRING X-Git-Tag: v26.0.1rc1~15^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=138702c60ca33e7f51caca6c3782c45ac4bda70d;p=nextcloud-server.git Migrate metadata as JSON to value as STRING Signed-off-by: Louis Chemineau --- diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index a6c9b8b4ebe..6b6f622a5a7 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -436,7 +436,7 @@ class FilesPlugin extends ServerPlugin { \OC::$server->get(LoggerInterface::class)->debug('Inefficient fetching of metadata'); } - return json_encode((object)$sizeMetadata->getMetadata(), JSON_THROW_ON_ERROR); + return $sizeMetadata->getValue(); }); } } diff --git a/core/Migrations/Version24000Date20220404230027.php b/core/Migrations/Version24000Date20220404230027.php index d53a43af959..26af9a4c6cc 100644 --- a/core/Migrations/Version24000Date20220404230027.php +++ b/core/Migrations/Version24000Date20220404230027.php @@ -52,11 +52,15 @@ class Version24000Date20220404230027 extends SimpleMigrationStep { 'notnull' => true, 'length' => 50, ]); - $table->addColumn('metadata', Types::JSON, [ - 'notnull' => true, + $table->addColumn('value', Types::TEXT, [ + 'notnull' => false, + 'default' => '', ]); $table->setPrimaryKey(['id', 'group_name'], 'file_metadata_idx'); + + return $schema; } - return $schema; + + return null; } } diff --git a/core/Migrations/Version27000Date20230309104325.php b/core/Migrations/Version27000Date20230309104325.php new file mode 100644 index 00000000000..e11b37b4b29 --- /dev/null +++ b/core/Migrations/Version27000Date20230309104325.php @@ -0,0 +1,90 @@ + + * + * @author Louis Chmn + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Migrate oc_file_metadata.metadata as JSON type to oc_file_metadata.value a STRING type + * @see \OC\Metadata\FileMetadata + */ +class Version27000Date20230309104325 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $connection + ) { + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if ($metadataTable->hasColumn('value')) { + return null; + } + + $metadataTable->addColumn('value', Types::TEXT, [ + 'notnull' => false, + 'default' => '', + ]); + return $schema; + } + + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return void + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if (!$metadataTable->hasColumn('metadata')) { + return; + } + + $this->connection + ->getQueryBuilder() + ->update('file_metadata') + ->set('value', 'metadata') + ->executeStatement(); + } +} diff --git a/core/Migrations/Version27000Date20230309104802.php b/core/Migrations/Version27000Date20230309104802.php new file mode 100644 index 00000000000..4bd50fe0396 --- /dev/null +++ b/core/Migrations/Version27000Date20230309104802.php @@ -0,0 +1,57 @@ + + * + * @author Louis Chmn + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Migrate oc_file_metadata.metadata as JSON type to oc_file_metadata.value a STRING type + * @see \OC\Metadata\FileMetadata + */ +class Version27000Date20230309104802 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if ($metadataTable->hasColumn('metadata')) { + $metadataTable->dropColumn('metadata'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 7fa420c8ca0..4ee381cc0bc 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1086,6 +1086,8 @@ return array( 'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => $baseDir . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => $baseDir . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104325' => $baseDir . '/core/Migrations/Version27000Date20230309104325.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104802' => $baseDir . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 9f3fc0bf709..a9729c33317 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1119,6 +1119,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104325' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104325.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104802' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', diff --git a/lib/private/Metadata/FileMetadata.php b/lib/private/Metadata/FileMetadata.php index 9ad0f9d35c6..a9808a86998 100644 --- a/lib/private/Metadata/FileMetadata.php +++ b/lib/private/Metadata/FileMetadata.php @@ -28,16 +28,24 @@ use OCP\DB\Types; /** * @method string getGroupName() * @method void setGroupName(string $groupName) - * @method array getMetadata() - * @method void setMetadata(array $metadata) + * @method string getValue() + * @method void setValue(string $value) * @see \OC\Core\Migrations\Version240000Date20220404230027 */ class FileMetadata extends Entity { protected ?string $groupName = null; - protected ?array $metadata = null; + protected ?string $value = null; public function __construct() { $this->addType('groupName', 'string'); - $this->addType('metadata', Types::JSON); + $this->addType('value', Types::STRING); + } + + public function getDecodedValue(): array { + return json_decode($this->getValue(), true) ?? []; + } + + public function setArrayAsValue(array $value): void { + $this->setValue(json_encode($value, JSON_THROW_ON_ERROR)); } } diff --git a/lib/private/Metadata/FileMetadataMapper.php b/lib/private/Metadata/FileMetadataMapper.php index f8f8df4bf3b..f3120e5e515 100644 --- a/lib/private/Metadata/FileMetadataMapper.php +++ b/lib/private/Metadata/FileMetadataMapper.php @@ -89,7 +89,7 @@ class FileMetadataMapper extends QBMapper { continue; } $empty = new FileMetadata(); - $empty->setMetadata([]); + $empty->setValue(''); $empty->setGroupName($groupName); $empty->setId($id); $metadata[$id] = $empty; @@ -132,13 +132,13 @@ class FileMetadataMapper extends QBMapper { $idType = $this->getParameterTypeForProperty($entity, 'id'); $groupNameType = $this->getParameterTypeForProperty($entity, 'groupName'); - $metadataValue = $entity->getMetadata(); - $metadataType = $this->getParameterTypeForProperty($entity, 'metadata'); + $value = $entity->getValue(); + $valueType = $this->getParameterTypeForProperty($entity, 'value'); $qb = $this->db->getQueryBuilder(); $qb->update($this->tableName) - ->set('metadata', $qb->createNamedParameter($metadataValue, $metadataType)) + ->set('value', $qb->createNamedParameter($value, $valueType)) ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, $idType))) ->andWhere($qb->expr()->eq('group_name', $qb->createNamedParameter($groupName, $groupNameType))) ->executeStatement(); diff --git a/lib/private/Metadata/MetadataManager.php b/lib/private/Metadata/MetadataManager.php index 77407a2f529..6d96ff1ab68 100644 --- a/lib/private/Metadata/MetadataManager.php +++ b/lib/private/Metadata/MetadataManager.php @@ -78,6 +78,9 @@ class MetadataManager implements IMetadataManager { $this->fileMetadataMapper->clear($fileId); } + /** + * @return array + */ public function fetchMetadataFor(string $group, array $fileIds): array { return $this->fileMetadataMapper->findForGroupForFiles($fileIds, $group); } diff --git a/lib/private/Metadata/Provider/ExifProvider.php b/lib/private/Metadata/Provider/ExifProvider.php index ae2c57ba7e5..4e211e7b6c4 100644 --- a/lib/private/Metadata/Provider/ExifProvider.php +++ b/lib/private/Metadata/Provider/ExifProvider.php @@ -65,12 +65,12 @@ class ExifProvider implements IMetadataProvider { $size = new FileMetadata(); $size->setGroupName('size'); $size->setId($file->getId()); - $size->setMetadata([]); + $size->setArrayAsValue([]); if (!$data) { $sizeResult = getimagesizefromstring($file->getContent()); if ($sizeResult !== false) { - $size->setMetadata([ + $size->setArrayAsValue([ 'width' => $sizeResult[0], 'height' => $sizeResult[1], ]); @@ -79,7 +79,7 @@ class ExifProvider implements IMetadataProvider { } } elseif (array_key_exists('COMPUTED', $data)) { if (array_key_exists('Width', $data['COMPUTED']) && array_key_exists('Height', $data['COMPUTED'])) { - $size->setMetadata([ + $size->setArrayAsValue([ 'width' => $data['COMPUTED']['Width'], 'height' => $data['COMPUTED']['Height'], ]); @@ -95,7 +95,7 @@ class ExifProvider implements IMetadataProvider { $gps = new FileMetadata(); $gps->setGroupName('gps'); $gps->setId($file->getId()); - $gps->setMetadata([ + $gps->setArrayAsValue([ 'latitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLatitude'], $data['GPS']['GPSLatitudeRef']), 'longitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLongitude'], $data['GPS']['GPSLongitudeRef']), ]); diff --git a/tests/lib/Metadata/FileMetadataMapperTest.php b/tests/lib/Metadata/FileMetadataMapperTest.php index 1a005f24b8a..4f7708ab9a9 100644 --- a/tests/lib/Metadata/FileMetadataMapperTest.php +++ b/tests/lib/Metadata/FileMetadataMapperTest.php @@ -51,23 +51,23 @@ class FileMetadataMapperTest extends \Test\TestCase { $file1 = new FileMetadata(); $file1->setId(1); $file1->setGroupName('size'); - $file1->setMetadata([]); + $file1->setArrayAsValue([]); $file2 = new FileMetadata(); $file2->setId(2); $file2->setGroupName('size'); - $file2->setMetadata(['width' => 293, 'height' => 23]); + $file2->setArrayAsValue(['width' => 293, 'height' => 23]); // not added, it's the default $file3 = new FileMetadata(); $file3->setId(3); $file3->setGroupName('size'); - $file3->setMetadata([]); + $file3->setArrayAsValue([]); $file4 = new FileMetadata(); $file4->setId(4); $file4->setGroupName('size'); - $file4->setMetadata(['complex' => ["yes", "maybe" => 34.0]]); + $file4->setArrayAsValue(['complex' => ["yes", "maybe" => 34.0]]); $this->mapper->insert($file1); $this->mapper->insert($file2); @@ -75,10 +75,10 @@ class FileMetadataMapperTest extends \Test\TestCase { $files = $this->mapper->findForGroupForFiles([1, 2, 3, 4], 'size'); - $this->assertEquals($files[1]->getMetadata(), $file1->getMetadata()); - $this->assertEquals($files[2]->getMetadata(), $file2->getMetadata()); - $this->assertEquals($files[3]->getMetadata(), $file3->getMetadata()); - $this->assertEquals($files[4]->getMetadata(), $file4->getMetadata()); + $this->assertEquals($files[1]->getValue(), $file1->getValue()); + $this->assertEquals($files[2]->getValue(), $file2->getValue()); + $this->assertEquals($files[3]->getDecodedValue(), $file3->getDecodedValue()); + $this->assertEquals($files[4]->getValue(), $file4->getValue()); $this->mapper->clear(1); $this->mapper->clear(2);