Skip to content

Commit

Permalink
Migrate metadata as JSON to value as STRING
Browse files Browse the repository at this point in the history
Signed-off-by: Louis Chemineau <[email protected]>
  • Loading branch information
artonge committed Mar 9, 2023
1 parent 691aa8d commit 12753e3
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 24 deletions.
2 changes: 1 addition & 1 deletion apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
\OC::$server->get(LoggerInterface::class)->debug('Inefficient fetching of metadata');
}

return json_encode((object)$sizeMetadata->getMetadata(), JSON_THROW_ON_ERROR);
return $sizeMetadata->getValue();
});
}
}
Expand Down
10 changes: 7 additions & 3 deletions core/Migrations/Version24000Date20220404230027.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,15 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
'notnull' => true,
'length' => 50,
]);
$table->addColumn('metadata', Types::JSON, [
'notnull' => true,
$table->addColumn('value', Types::STRING, [
'notnull' => false,
'default' => '',
]);
$table->setPrimaryKey(['id', 'group_name'], 'file_metadata_idx');

return $schema;
}
return $schema;

return null;
}
}
125 changes: 125 additions & 0 deletions core/Migrations/Version27000Date20230309104325.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Louis Chmn <[email protected]>
*
* @author Louis Chmn <[email protected]>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Core\Migrations;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\QueryBuilder\IQueryBuilder;
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::STRING, [
'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;
}

$updateQuery = $this->connection->getQueryBuilder();
$updateQuery->update('file_metadata')
->set('value', $updateQuery->createParameter('value'))
->where($updateQuery->expr()->eq('id', $updateQuery->createParameter('id')))
->andWhere($updateQuery->expr()->eq('group_name', $updateQuery->createParameter('group_name')));

$selectQuery = $this->connection->getQueryBuilder();
$selectQuery->select('id', 'group_name', 'metadata')
->from('file_metadata')
->orderBy('id', 'ASC')
->setMaxResults(1000);

$offset = 0;
$movedRows = 0;
do {
$movedRows = $this->chunkedCopying($updateQuery, $selectQuery, $offset);
$offset += $movedRows;
} while ($movedRows !== 0);
}

protected function chunkedCopying(IQueryBuilder $updateQuery, IQueryBuilder $selectQuery, int $offset): int {
$this->connection->beginTransaction();

$results = $selectQuery
->setFirstResult($offset)
->executeQuery();

while ($row = $results->fetch()) {
$updateQuery
->setParameter('id', (int)$row['id'])
->setParameter('group_name', $row['group_name'])
->setParameter('value', $row['metadata'])
->executeStatement();
}

$results->closeCursor();
$this->connection->commit();

return $results->rowCount();
}
}
57 changes: 57 additions & 0 deletions core/Migrations/Version27000Date20230309104802.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Louis Chmn <[email protected]>
*
* @author Louis Chmn <[email protected]>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/

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;
}
}
16 changes: 12 additions & 4 deletions lib/private/Metadata/FileMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,24 @@
/**
* @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 setObjectAsValue(array $value): void {
$this->setValue(json_encode($value, JSON_THROW_ON_ERROR));
}
}
8 changes: 4 additions & 4 deletions lib/private/Metadata/FileMetadataMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public function findForGroupForFiles(array $fileIds, string $groupName): array {
continue;
}
$empty = new FileMetadata();
$empty->setMetadata([]);
$empty->setValue('');
$empty->setGroupName($groupName);
$empty->setId($id);
$metadata[$id] = $empty;
Expand Down Expand Up @@ -132,13 +132,13 @@ public function update(Entity $entity): Entity {

$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();
Expand Down
3 changes: 3 additions & 0 deletions lib/private/Metadata/MetadataManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ public function clearMetadata(int $fileId): void {
$this->fileMetadataMapper->clear($fileId);
}

/**
* @return array<int, FileMetadata>
*/
public function fetchMetadataFor(string $group, array $fileIds): array {
return $this->fileMetadataMapper->findForGroupForFiles($fileIds, $group);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/private/Metadata/Provider/ExifProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ public function execute(File $file): array {
$size = new FileMetadata();
$size->setGroupName('size');
$size->setId($file->getId());
$size->setMetadata([]);
$size->setObjectAsValue([]);

if (!$data) {
$sizeResult = getimagesizefromstring($file->getContent());
if ($sizeResult !== false) {
$size->setMetadata([
$size->setObjectAsValue([
'width' => $sizeResult[0],
'height' => $sizeResult[1],
]);
Expand All @@ -79,7 +79,7 @@ public function execute(File $file): array {
}
} elseif (array_key_exists('COMPUTED', $data)) {
if (array_key_exists('Width', $data['COMPUTED']) && array_key_exists('Height', $data['COMPUTED'])) {
$size->setMetadata([
$size->setObjectAsValue([
'width' => $data['COMPUTED']['Width'],
'height' => $data['COMPUTED']['Height'],
]);
Expand All @@ -95,7 +95,7 @@ public function execute(File $file): array {
$gps = new FileMetadata();
$gps->setGroupName('gps');
$gps->setId($file->getId());
$gps->setMetadata([
$gps->setObjectAsValue([
'latitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLatitude'], $data['GPS']['GPSLatitudeRef']),
'longitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLongitude'], $data['GPS']['GPSLongitudeRef']),
]);
Expand Down
16 changes: 8 additions & 8 deletions tests/lib/Metadata/FileMetadataMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,34 +51,34 @@ public function testFindForGroupForFiles() {
$file1 = new FileMetadata();
$file1->setId(1);
$file1->setGroupName('size');
$file1->setMetadata([]);
$file1->setObjectAsValue([]);

$file2 = new FileMetadata();
$file2->setId(2);
$file2->setGroupName('size');
$file2->setMetadata(['width' => 293, 'height' => 23]);
$file2->setObjectAsValue(['width' => 293, 'height' => 23]);

// not added, it's the default
$file3 = new FileMetadata();
$file3->setId(3);
$file3->setGroupName('size');
$file3->setMetadata([]);
$file3->setObjectAsValue([]);

$file4 = new FileMetadata();
$file4->setId(4);
$file4->setGroupName('size');
$file4->setMetadata(['complex' => ["yes", "maybe" => 34.0]]);
$file4->setObjectAsValue(['complex' => ["yes", "maybe" => 34.0]]);

$this->mapper->insert($file1);
$this->mapper->insert($file2);
$this->mapper->insert($file4);

$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]->getValue(), $file3->getValue());
$this->assertEquals($files[4]->getValue(), $file4->getValue());

$this->mapper->clear(1);
$this->mapper->clear(2);
Expand Down

0 comments on commit 12753e3

Please sign in to comment.