Skip to content

Commit 06b064e

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #57514 from nextcloud/carl/optimize-cacheentry
perf(files): Optimize CacheEntry creation
2 parents 902d8b0 + 0682797 commit 06b064e

File tree

4 files changed

+62
-55
lines changed

4 files changed

+62
-55
lines changed

‎apps/files_sharing/tests/CacheTest.php‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@
99

1010
use OC\Files\Cache\Cache;
1111
use OC\Files\Filesystem;
12+
use OC\Files\SetupManager;
1213
use OC\Files\Storage\Storage;
1314
use OC\Files\Storage\Temporary;
1415
use OC\Files\Storage\Wrapper\Jail;
1516
use OC\Files\View;
1617
use OCA\Files_Sharing\SharedStorage;
1718
use OCP\Constants;
1819
use OCP\Files\Cache\IWatcher;
20+
use OCP\Files\IRootFolder;
1921
use OCP\IUserManager;
2022
use OCP\Server;
2123
use OCP\Share\IShare;
@@ -424,8 +426,7 @@ public function testGetPathByIdDirectShare(): void {
424426
$share = $this->shareManager->createShare($share);
425427
$share->setStatus(IShare::STATUS_ACCEPTED);
426428
$this->shareManager->updateShare($share);
427-
428-
\OC_Util::tearDownFS();
429+
Server::get(SetupManager::class)->tearDown();
429430

430431
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
431432
$this->assertTrue(Filesystem::file_exists('/test.txt'));
@@ -456,7 +457,7 @@ public function testGetPathByIdShareSubFolder(): void {
456457
$share = $this->shareManager->createShare($share);
457458
$share->setStatus(IShare::STATUS_ACCEPTED);
458459
$this->shareManager->updateShare($share);
459-
\OC_Util::tearDownFS();
460+
Server::get(SetupManager::class)->tearDown();
460461

461462
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
462463
$this->assertTrue(Filesystem::file_exists('/foo'));
@@ -484,7 +485,7 @@ public function testNumericStorageId(): void {
484485
$share = $this->shareManager->createShare($share);
485486
$share->setStatus(IShare::STATUS_ACCEPTED);
486487
$this->shareManager->updateShare($share);
487-
\OC_Util::tearDownFS();
488+
Server::get(SetupManager::class)->tearDown();
488489

489490
[$sourceStorage] = Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER1 . '/files/foo');
490491

@@ -521,7 +522,7 @@ public function testShareJailedStorage(): void {
521522
$share = $this->shareManager->createShare($share);
522523
$share->setStatus(IShare::STATUS_ACCEPTED);
523524
$this->shareManager->updateShare($share);
524-
\OC_Util::tearDownFS();
525+
Server::get(SetupManager::class)->tearDown();
525526

526527
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
527528
$this->assertEquals('foo', Filesystem::file_get_contents('/sub/foo.txt'));
@@ -560,7 +561,7 @@ public function testSearchShareJailedStorage(): void {
560561
$share = $this->shareManager->createShare($share);
561562
$share->setStatus(IShare::STATUS_ACCEPTED);
562563
$this->shareManager->updateShare($share);
563-
\OC_Util::tearDownFS();
564+
Server::get(SetupManager::class)->tearDown();
564565

565566
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
566567

@@ -571,7 +572,7 @@ public function testSearchShareJailedStorage(): void {
571572
$this->assertCount(1, $results);
572573
}
573574

574-
public function testWatcherRootChange() {
575+
public function testWatcherRootChange(): void {
575576
$sourceStorage = new Temporary();
576577
$sourceStorage->mkdir('shared');
577578
$sourceStorage->file_put_contents('shared/foo.txt', 'foo');
@@ -581,7 +582,8 @@ public function testWatcherRootChange() {
581582

582583
self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
583584

584-
$rootFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1);
585+
$rootFolder = Server::get(IRootFolder::class)
586+
->getUserFolder(self::TEST_FILES_SHARING_API_USER1);
585587
$node = $rootFolder->get('foo/shared');
586588
$this->assertEquals(3, $node->getSize());
587589

@@ -594,14 +596,14 @@ public function testWatcherRootChange() {
594596
$share = $this->shareManager->createShare($share);
595597
$share->setStatus(IShare::STATUS_ACCEPTED);
596598
$this->shareManager->updateShare($share);
597-
\OC_Util::tearDownFS();
599+
Server::get(SetupManager::class)->tearDown();
598600

599601
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
600602

601-
$view = Filesystem::getView();
602-
603-
$sourceStorage->rmdir('shared');
603+
$this->assertTrue($sourceStorage->rmdir('shared'));
604604

605-
$this->assertFalse($view->getFileInfo('shared'));
605+
$this->assertFalse(Server::get(IRootFolder::class)
606+
->getUserFolder(self::TEST_FILES_SHARING_API_USER2)
607+
->nodeExists('shared'));
606608
}
607609
}

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -148,41 +148,50 @@ public function get($file) {
148148

149149
/**
150150
* Create a CacheEntry from database row
151-
*
152-
* @param array $data
153-
* @param IMimeTypeLoader $mimetypeLoader
154-
* @return CacheEntry
155151
*/
156-
public static function cacheEntryFromData($data, IMimeTypeLoader $mimetypeLoader) {
157-
//fix types
158-
$data['name'] = (string)$data['name'];
159-
$data['path'] = (string)$data['path'];
160-
$data['fileid'] = (int)$data['fileid'];
161-
$data['parent'] = (int)$data['parent'];
162-
$data['size'] = Util::numericToNumber($data['size']);
163-
$data['unencrypted_size'] = Util::numericToNumber($data['unencrypted_size'] ?? 0);
164-
$data['mtime'] = (int)$data['mtime'];
165-
$data['storage_mtime'] = (int)$data['storage_mtime'];
166-
$data['encryptedVersion'] = (int)$data['encrypted'];
167-
$data['encrypted'] = (bool)$data['encrypted'];
168-
$data['storage_id'] = $data['storage'];
169-
$data['storage'] = (int)$data['storage'];
170-
$data['mimetype'] = $mimetypeLoader->getMimetypeById($data['mimetype']);
171-
$data['mimepart'] = $mimetypeLoader->getMimetypeById($data['mimepart']);
172-
if ($data['storage_mtime'] == 0) {
173-
$data['storage_mtime'] = $data['mtime'];
174-
}
175-
if (isset($data['f_permissions'])) {
176-
$data['scan_permissions'] ??= $data['f_permissions'];
177-
}
178-
$data['permissions'] = (int)$data['permissions'];
179-
if (isset($data['creation_time'])) {
180-
$data['creation_time'] = (int)$data['creation_time'];
181-
}
182-
if (isset($data['upload_time'])) {
183-
$data['upload_time'] = (int)$data['upload_time'];
152+
public static function cacheEntryFromData(array $data, IMimeTypeLoader $mimetypeLoader): CacheEntry {
153+
$normalized = [
154+
'name' => (string)$data['name'],
155+
'etag' => (string)$data['etag'],
156+
'path' => (string)$data['path'],
157+
'path_hash' => (string)$data['path_hash'],
158+
'checksum' => (string)$data['checksum'],
159+
'fileid' => (int)$data['fileid'],
160+
'parent' => (int)$data['parent'],
161+
'size' => Util::numericToNumber($data['size']),
162+
'unencrypted_size' => Util::numericToNumber($data['unencrypted_size'] ?? 0),
163+
'mtime' => (int)$data['mtime'],
164+
'storage_mtime' => (int)($data['storage_mtime'] ?: $data['mtime']),
165+
'encryptedVersion' => (int)$data['encrypted'],
166+
'encrypted' => (bool)$data['encrypted'],
167+
'storage_id' => $data['storage'],
168+
'storage_string_id' => isset($data['storage_string_id']) ? (string)$data['storage_string_id'] : null,
169+
'storage' => (int)$data['storage'],
170+
'mimetype' => $mimetypeLoader->getMimetypeById($data['mimetype']),
171+
'mimepart' => $mimetypeLoader->getMimetypeById($data['mimepart']),
172+
'permissions' => (int)$data['permissions'],
173+
'creation_time' => isset($data['creation_time']) ? (int)$data['creation_time'] : null,
174+
'upload_time' => isset($data['upload_time']) ? (int)$data['upload_time'] : null,
175+
'metadata_etag' => isset($data['metadata_etag']) ? (string)$data['metadata_etag'] : null,
176+
'scan_permissions' => $data['scan_permissions'] ?? ($data['f_permissions'] ?? null),
177+
'metadata' => $data['metadata'] ?? null,
178+
'meta_json' => $data['meta_json'] ?? null,
179+
'meta_sync_token' => $data['meta_sync_token'] ?? null,
180+
];
181+
182+
if (isset($data['folder_id'])) {
183+
// groupfolders specific fields
184+
$normalized['folder_id'] = (int)$data['folder_id'];
185+
$normalized['mount_point'] = (string)$data['mount_point'];
186+
$normalized['quota'] = $data['quota'];
187+
$normalized['acl'] = (int)$data['acl'];
188+
$normalized['acl_default_no_permission'] = (int)$data['acl_default_no_permission'];
189+
$normalized['storage_id'] = (int)$data['storage_id'];
190+
$normalized['root_id'] = (int)$data['root_id'];
191+
$normalized['options'] = (string)$data['options'];
184192
}
185-
return new CacheEntry($data);
193+
194+
return new CacheEntry($normalized);
186195
}
187196

188197
/**

‎lib/private/Files/Cache/CacheEntry.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,9 @@
1313
* meta data for a file or folder
1414
*/
1515
class CacheEntry implements ICacheEntry {
16-
/**
17-
* @var array
18-
*/
19-
private $data;
20-
21-
public function __construct(array $data) {
22-
$this->data = $data;
16+
public function __construct(
17+
private array $data,
18+
) {
2319
}
2420

2521
public function offsetSet($offset, $value): void {

‎lib/private/Files/ObjectStore/ObjectStoreStorage.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,9 @@ public function copyFromStorage(
587587
$sourceEntry = $sourceStorage->getCache()->get($sourceInternalPath);
588588
$sourceEntryData = $sourceEntry->getData();
589589
// $sourceEntry['permissions'] here is the permissions from the jailed storage for the current
590-
// user. Instead we use $sourceEntryData['scan_permissions'] that are the permissions from the
590+
// user. Instead, we use $sourceEntryData['scan_permissions'] that are the permissions from the
591591
// unjailed storage.
592-
if (is_array($sourceEntryData) && array_key_exists('scan_permissions', $sourceEntryData)) {
592+
if (is_array($sourceEntryData) && $sourceEntryData['scan_permissions'] !== null) {
593593
$sourceEntry['permissions'] = $sourceEntryData['scan_permissions'];
594594
}
595595
$this->copyInner($sourceStorage->getCache(), $sourceEntry, $targetInternalPath);

0 commit comments

Comments
 (0)