X Tutup
Skip to content

Commit be5906f

Browse files
phil-davisphil-davis
authored
Merge pull request #38055 from owncloud/bugfix/sharefilelist-perf-problem
avoid retrieving user root iteratively in share controller
2 parents 81b1f25 + d42900f commit be5906f

File tree

3 files changed

+51
-23
lines changed

3 files changed

+51
-23
lines changed

‎apps/files_sharing/lib/Controller/Share20OcsController.php‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ class Share20OcsController extends OCSController {
8686
*/
8787
private $additionalInfoField;
8888

89+
/** @var Folder[] */
90+
private $currentUserFolder;
91+
8992
public function __construct(
9093
$appName,
9194
IRequest $request,
@@ -132,6 +135,21 @@ private function getAdditionalUserInfo(IUser $user) {
132135
return null;
133136
}
134137

138+
/**
139+
* Returns root folder of the current user
140+
*
141+
* @return Folder
142+
*/
143+
private function getCurrentUserFolder() {
144+
// cache only one key, but be sure to check current user session id in case
145+
// current user folder changes
146+
$userSessionId = $this->userSession->getUser()->getUID();
147+
if (!isset($this->currentUserFolder[$userSessionId])) {
148+
$this->currentUserFolder = [$userSessionId => $this->rootFolder->getUserFolder($userSessionId)];
149+
}
150+
return $this->currentUserFolder[$userSessionId];
151+
}
152+
135153
/**
136154
* Convert an IShare to an array for OCS output
137155
*
@@ -172,34 +190,33 @@ protected function formatShare(IShare $share, $received = false) {
172190

173191
// can only fetch path info if mounted already or if owner
174192
if ($share->getState() === Share::STATE_ACCEPTED || $share->getShareOwner() === $this->userSession->getUser()->getUID()) {
175-
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
193+
$userFolder = $this->getCurrentUserFolder();
176194
} else {
177195
// need to go through owner user for pending shares
178196
$userFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
179197
}
180198
} else {
181-
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
199+
$userFolder = $this->getCurrentUserFolder();
182200
}
183201

184-
$nodes = $userFolder->getById($share->getNodeId(), true);
185-
$node = $nodes[0] ?? null;
186-
if ($node === null) {
202+
// we need to retrieve a share mountpoint node for the userfolder,
203+
// we cannot use share->getNode() as it retrieves the original node and
204+
// not a user folder mount. Share node will be needed to retrieve e.g.
205+
// shared storage details
206+
$shareNodes = $userFolder->getById($share->getNodeId(), true);
207+
$shareNode = $shareNodes[0] ?? null;
208+
if ($shareNode === null) {
187209
throw new NotFoundException();
188210
}
189-
$node = $nodes[0];
190211

191-
$result['path'] = $userFolder->getRelativePath($node->getPath());
192-
if ($node instanceof Folder) {
193-
$result['item_type'] = 'folder';
194-
} else {
195-
$result['item_type'] = 'file';
196-
}
197-
$result['mimetype'] = $node->getMimeType();
198-
$result['storage_id'] = $node->getStorage()->getId();
199-
$result['storage'] = $node->getStorage()->getCache()->getNumericStorageId();
200-
$result['item_source'] = $node->getId();
201-
$result['file_source'] = $node->getId();
202-
$result['file_parent'] = $node->getParent()->getId();
212+
$result['path'] = $userFolder->getRelativePath($shareNode->getPath());
213+
$result['mimetype'] = $shareNode->getMimeType();
214+
$result['storage_id'] = $shareNode->getStorage()->getId();
215+
$result['storage'] = $shareNode->getStorage()->getCache()->getNumericStorageId();
216+
$result['item_type'] = $share->getNodeType();
217+
$result['item_source'] = $shareNode->getId();
218+
$result['file_source'] = $shareNode->getId();
219+
$result['file_parent'] = $shareNode->getParent()->getId();
203220
$result['file_target'] = $share->getTarget();
204221

205222
$expiration = $share->getExpirationDate();
@@ -341,7 +358,7 @@ public function createShare() {
341358
return new Result(null, 404, $this->l->t('Please specify a file or folder path'));
342359
}
343360

344-
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
361+
$userFolder = $this->getCurrentUserFolder();
345362

346363
try {
347364
$path = $userFolder->get($path);
@@ -668,7 +685,7 @@ public function getShares() {
668685
// (with "true" value), without duplicate elements, and only valid share types
669686

670687
if ($path !== null) {
671-
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
688+
$userFolder = $this->getCurrentUserFolder();
672689
try {
673690
$path = $userFolder->get($path);
674691
$path->lock(ILockingProvider::LOCK_SHARED);
@@ -999,6 +1016,8 @@ private function updateShareState($id, $state) {
9991016

10001017
$node->unlock(ILockingProvider::LOCK_SHARED);
10011018

1019+
// refresh the mounts by teardown of existing user mounts and remounting
1020+
// by retrieving current user folder
10021021
// FIXME: needs public API!
10031022
Filesystem::tearDown();
10041023
// FIXME: trigger mount for user to make sure the new node is mounted already
@@ -1021,7 +1040,7 @@ private function updateShareState($id, $state) {
10211040
* @return IShare same share with target updated if necessary
10221041
*/
10231042
private function deduplicateShareTarget(IShare $share) {
1024-
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
1043+
$userFolder = $this->getCurrentUserFolder();
10251044
$parentDir = \dirname($share->getTarget());
10261045
if (!$userFolder->nodeExists($parentDir)) {
10271046
// assume the parent folder matches the configured shared folder

‎apps/files_sharing/tests/Controller/Share20OcsControllerTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public function testGetGetShareNotExists() {
308308
}
309309
*/
310310

311-
public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner, $path, $permissions,
311+
public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner, $file, $permissions,
312312
$shareTime, $expiration, $parent, $target, $mail_send, $token=null,
313313
$password=null, $name=null, $attributes=null) {
314314
$share = $this->createMock(IShare::class);
@@ -317,7 +317,8 @@ public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner
317317
$share->method('getSharedWith')->willReturn($sharedWith);
318318
$share->method('getSharedBy')->willReturn($sharedBy);
319319
$share->method('getShareOwner')->willReturn($shareOwner);
320-
$share->method('getNode')->willReturn($path);
320+
$share->method('getNode')->willReturn($file);
321+
$share->method('getNodeType')->willReturn($file instanceof Folder ? 'folder' : 'file');
321322
$share->method('getPermissions')->willReturn($permissions);
322323
$share->method('getAttributes')->willReturn($attributes);
323324
$time = new \DateTime();

‎changelog/unreleased/38055‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Bugfix: Avoid retrieving user root iteratively in share controller
2+
3+
There was a performance problem that with many shares, the "share tab" was slow
4+
to display entries. Now the performance of displaying that tab should be better
5+
as we avoid retrieving the same information for all the shares
6+
7+
https://github.com/owncloud/enterprise/issues/4107
8+
https://github.com/owncloud/core/pull/38055

0 commit comments

Comments
 (0)
X Tutup