Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Use getFirstNodeById as it is cached #3820

Merged
merged 1 commit into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion REUSE.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SPDX-FileCopyrightText = "2016 Nextcloud contributors"
SPDX-License-Identifier = "AGPL-3.0-or-later"

[[annotations]]
path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/features/**.feature"]
path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/features/**.feature", "tests/psalm-baseline.xml"]
precedence = "aggregate"
SPDX-FileCopyrightText = "2016 Nextcloud GmbH and Nextcloud contributors"
SPDX-License-Identifier = "AGPL-3.0-or-later"
Expand Down
5 changes: 2 additions & 3 deletions lib/Controller/AssetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,12 @@ public function get($token) {

$this->userScopeService->setUserScope($asset->getUid());
$userFolder = $this->rootFolder->getUserFolder($asset->getUid());
$nodes = $userFolder->getById($asset->getFileid());
$node = $userFolder->getFirstNodeById($asset->getFileid());

if ($nodes === []) {
if ($node === null) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}

$node = array_pop($nodes);
if (!($node instanceof File)) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}
Expand Down
5 changes: 2 additions & 3 deletions lib/Controller/DirectViewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function show($token) {
$folder = $this->rootFolder->getUserFolder($direct->getUid());

try {
$item = $folder->getById($direct->getFileid())[0];
$item = $folder->getFirstNodeById($direct->getFileid());
if (!($item instanceof Node)) {
throw new \Exception();
}
Expand Down Expand Up @@ -126,8 +126,7 @@ public function showPublicShare(Direct $direct) {

$node = $share->getNode();
if ($node instanceof Folder) {
$nodes = $node->getById($direct->getFileid());
$node = array_shift($nodes);
$node = $node->getFirstNodeById($direct->getFileid());
if ($node === null) {
throw new NotFoundException();
}
Expand Down
7 changes: 5 additions & 2 deletions lib/Controller/DocumentAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,11 @@ public function create(string $mimeType, string $fileName, string $directoryPath
#[Http\Attribute\NoAdminRequired]
public function openLocal(int $fileId): DataResponse {
try {
$files = $this->rootFolder->getUserFolder($this->userId)->getById($fileId);
$file = array_shift($files);
$file = $this->rootFolder->getUserFolder($this->userId)->getFirstNodeById($fileId);
if ($file === null) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
}

$this->lockManager->unlock(new LockContext(
$file,
ILock::TYPE_APP,
Expand Down
6 changes: 2 additions & 4 deletions lib/Controller/DocumentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,7 @@ private function getFileForUser(int $fileId, ?string $path = null): File {
if ($path !== null) {
$node = $folder->get($path);
} else {
$nodes = $folder->getById($fileId);
$node = array_shift($nodes);
$node = $folder->getFirstNodeById($fileId);
}

if ($node instanceof File) {
Expand Down Expand Up @@ -449,8 +448,7 @@ private function getFileForShare(IShare $share, ?int $fileId, ?string $path = nu
if ($path !== null) {
$node = $node->get($path);
} else {
$nodes = $node->getById($fileId);
$node = array_shift($nodes);
$node = $node->getFirstNodeById($fileId);
}

if ($node instanceof File) {
Expand Down
5 changes: 2 additions & 3 deletions lib/Controller/OCSController.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,12 @@ public function __construct(string $appName,
public function createDirect($fileId) {
try {
$userFolder = $this->rootFolder->getUserFolder($this->userId);
$nodes = $userFolder->getById($fileId);
$node = $userFolder->getFirstNodeById($fileId);

if ($nodes === []) {
if ($node === null) {
throw new OCSNotFoundException();
}

$node = $nodes[0];
if ($node instanceof Folder) {
throw new OCSBadRequestException('Cannot view folder');
}
Expand Down
8 changes: 3 additions & 5 deletions lib/Controller/WopiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,10 @@ public function putFile($fileId,
if ($isPutRelative) {
// the new file needs to be installed in the current user dir
$userFolder = $this->rootFolder->getUserFolder($wopi->getEditorUid());
$file = $userFolder->getById($fileId);
if (count($file) === 0) {
$file = $userFolder->getFirstNodeById($fileId);
if ($file === null) {
return new JSONResponse([], Http::STATUS_NOT_FOUND);
}
$file = $file[0];
$suggested = $this->request->getHeader('X-WOPI-SuggestedTarget');
$suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7');

Expand Down Expand Up @@ -782,8 +781,7 @@ private function getFileForWopiToken(Wopi $wopi) {
return $node;
}

$nodes = $node->getById($wopi->getFileid());
return array_shift($nodes);
return $node->getFirstNodeById($wopi->getFileid());
}

// Group folders requires an active user to be set in order to apply the proper acl permissions as for anonymous requests it requires share permissions for read access
Expand Down
3 changes: 1 addition & 2 deletions lib/Reference/OfficeTargetReferenceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public function resolveReference(string $referenceText): ?IReference {

try {
$userFolder = $this->rootFolder->getUserFolder($this->userId);
$files = $userFolder->getById($fileId);
$file = array_shift($files);
$file = $userFolder->getFirstNodeById($fileId);
} catch (Exception $e) {
$this->logger->info('Failed to get file for office target reference: ' . $fileId, ['exception' => $e]);
return null;
Expand Down
8 changes: 4 additions & 4 deletions lib/TemplateManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ public function get(int $fileId) {

$templateDir = $this->getUserTemplateDir();
// finally get the template file
$files = $templateDir->getById($fileId);
if ($files !== []) {
return $files[0];
$file = $templateDir->getFirstNodeById($fileId);
if ($file !== null) {
return $file;
}

throw new NotFoundException();
Expand Down Expand Up @@ -575,7 +575,7 @@ public function getTemplateSource(int $fileId): ?File {
} catch (NotFoundException $e) {
$userFolder = $this->rootFolder->getUserFolder($this->userId);
try {
$template = $userFolder->getById($templateId);
$template = $userFolder->getFirstNodeById($templateId);
} catch (NotFoundException $e) {
$this->logger->warning('Could not retrieve template source file', ['exception' => $e]);
return null;
Expand Down
16 changes: 8 additions & 8 deletions lib/TokenManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,13 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
}
}
/** @var File $file */
$file = $rootFolder->getById($fileId)[0];
$file = $rootFolder->getFirstNodeById($fileId);

// Check node readability (for storage wrapper overwrites like terms of services)
if ($file === null || !$file->isReadable()) {
throw new NotPermittedException();
}

// If its a public share, use the owner from the share, otherwise check the file object
if (is_null($owneruid)) {
$owner = $file->getOwner();
Expand All @@ -166,11 +172,6 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s
}
}

// Check node readability (for storage wrapper overwrites like terms of services)
if (!$file->isReadable()) {
throw new NotPermittedException();
}

// Safeguard that users without required group permissions cannot create a token
if (!$this->permissionManager->isEnabledForUser($owneruid) && !$this->permissionManager->isEnabledForUser($editoruid)) {
throw new NotPermittedException();
Expand Down Expand Up @@ -226,8 +227,7 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId
$owneruid = $userId;
$editoruid = $userId;
$rootFolder = $this->rootFolder->getUserFolder($editoruid);
$targetFile = $rootFolder->getById($targetFileId);
$targetFile = array_shift($targetFile);
$targetFile = $rootFolder->getFirstNodeById($targetFileId);
if (!$targetFile instanceof File) {
throw new NotFoundException();
}
Expand Down
62 changes: 25 additions & 37 deletions tests/psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,35 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
- SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: AGPL-3.0-or-later
-->
<files psalm-version="5.21.1@8c473e2437be8b6a8fd8f630f0f11a16b114c494">
<files psalm-version="5.24.0@462c80e31c34e58cc4f750c656be3927e80e550e">
<file src="lib/AppConfig.php">
<InvalidArgument>
<code>[]</code>
<code><![CDATA[[]]]></code>
</InvalidArgument>
<RedundantCondition>
<code><![CDATA[array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array']]></code>
</RedundantCondition>
</file>
<file src="lib/Command/ActivateConfig.php">
<UndefinedClass>
<code>Command</code>
<code><![CDATA[Command]]></code>
</UndefinedClass>
</file>
<file src="lib/Command/ConvertToBigInt.php">
<UndefinedClass>
<code>Command</code>
<code><![CDATA[Command]]></code>
</UndefinedClass>
</file>
<file src="lib/Command/InstallDefaultFonts.php">
<UndefinedClass>
<code>Command</code>
<code><![CDATA[Command]]></code>
</UndefinedClass>
</file>
<file src="lib/Command/UpdateEmptyTemplates.php">
<UndefinedClass>
<code>Command</code>
<code><![CDATA[Command]]></code>
</UndefinedClass>
</file>
<file src="lib/Controller/DirectViewController.php">
Expand All @@ -39,49 +35,49 @@
</file>
<file src="lib/Controller/DocumentAPIController.php">
<UndefinedClass>
<code>\OCA\Files\Helper</code>
<code><![CDATA[\OCA\Files\Helper]]></code>
</UndefinedClass>
</file>
<file src="lib/Controller/DocumentController.php">
<InvalidScalarArgument>
<code><![CDATA[$node->getId()]]></code>
</InvalidScalarArgument>
<RedundantCondition>
<code>$app !== ''</code>
<code><![CDATA[$app !== '']]></code>
</RedundantCondition>
</file>
<file src="lib/Controller/SettingsController.php">
<UndefinedClass>
<code>NullOutput</code>
<code>NullOutput</code>
<code><![CDATA[NullOutput]]></code>
<code><![CDATA[NullOutput]]></code>
</UndefinedClass>
</file>
<file src="lib/Controller/TemplatesController.php">
<InvalidReturnStatement>
<code><![CDATA[$this->fetchPreview($template, $x, $y, $a, $forceIcon, $mode)]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code>DataResponse</code>
<code><![CDATA[DataResponse]]></code>
</InvalidReturnType>
<InvalidThrow>
<code>NotFoundResponse</code>
<code><![CDATA[NotFoundResponse]]></code>
</InvalidThrow>
</file>
<file src="lib/Controller/WopiController.php">
<NullArgument>
<code>null</code>
<code><![CDATA[null]]></code>
</NullArgument>
<TypeDoesNotContainType>
<code>$path === ''</code>
<code><![CDATA[$path === '']]></code>
</TypeDoesNotContainType>
<UndefinedInterfaceMethod>
<code>putContent</code>
<code>putContent</code>
<code><![CDATA[putContent]]></code>
<code><![CDATA[putContent]]></code>
</UndefinedInterfaceMethod>
</file>
<file src="lib/Helper.php">
<InvalidScalarArgument>
<code>$time</code>
<code><![CDATA[$time]]></code>
</InvalidScalarArgument>
</file>
<file src="lib/PermissionManager.php">
Expand All @@ -91,28 +87,28 @@
</file>
<file src="lib/Preview/Office.php">
<MissingDependency>
<code>Image</code>
<code><![CDATA[Image]]></code>
</MissingDependency>
<UndefinedThisPropertyAssignment>
<code><![CDATA[$this->capabilitites]]></code>
</UndefinedThisPropertyAssignment>
</file>
<file src="lib/Service/ConnectivityService.php">
<UndefinedClass>
<code>OutputInterface</code>
<code>OutputInterface</code>
<code><![CDATA[OutputInterface]]></code>
<code><![CDATA[OutputInterface]]></code>
</UndefinedClass>
</file>
<file src="lib/Service/FederationService.php">
<TypeDoesNotContainType>
<code>is_array($trustedList)</code>
<code><![CDATA[is_array($trustedList)]]></code>
</TypeDoesNotContainType>
<UndefinedClass>
<code>SharingExternalStorage</code>
<code><![CDATA[SharingExternalStorage]]></code>
</UndefinedClass>
<UndefinedInterfaceMethod>
<code>getRemote</code>
<code>getToken</code>
<code><![CDATA[getRemote]]></code>
<code><![CDATA[getToken]]></code>
</UndefinedInterfaceMethod>
</file>
<file src="lib/Template/CollaboraTemplateProvider.php">
Expand All @@ -124,15 +120,7 @@
}, $collaboraTemplates)]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code>array</code>
</InvalidReturnType>
</file>
<file src="lib/TemplateManager.php">
<InvalidReturnStatement>
<code>$template</code>
</InvalidReturnStatement>
<InvalidReturnType>
<code>?File</code>
<code><![CDATA[array]]></code>
</InvalidReturnType>
</file>
<file src="lib/WOPI/Parser.php">
Expand Down
Loading