Skip to content

Emit moveToTrash event only for the deleting user - #16664

Merged
blizzz merged 2 commits into
masterfrom
bugfix/emit-moveToTrash-limit
Sep 6, 2019
Merged

Emit moveToTrash event only for the deleting user#16664
blizzz merged 2 commits into
masterfrom
bugfix/emit-moveToTrash-limit

Conversation

@juliusknorr

Copy link
Copy Markdown
Member

This is the same issue as in #16624 which causes a huge loading time caused by iterating over all mounts for a file id for all users if getById is called on the RootFolder:

$mountsContainingFile = $mountCache->getMountsForFileId((int)$id, $user);

Since the moveToTrash event is only used in e2ee it should be fine to just check for the file in the users mount who is trying to delete the file.

@blizzz

Copy link
Copy Markdown
Member

don't we need the user exists check anymore when requesting the user folder instance?

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/emit-moveToTrash-limit branch from 70e0962 to b1832c8CompareAugust 6, 2019 15:46
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

Hm I currently fail to understand why that unit test is failing with that change :/

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzerrullzer added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 10, 2019
@rullzer

Copy link
Copy Markdown
Member

ok so the issue with the failing test is that the users storage gets initisialized thus for the final change of the lock the user mount is there. Causing the lock to be different.

So assume 'testUser'

  • Then the first read lock is on '/' with path 'testUser/files/test.txt'
  • Second change is on '/' with 'testUser/files/test.txt'
  • Then the userfolder gets initialized
  • Thirs lock change is on '/testUser' with path 'files/test.txt'

The last one fails because it wants to release the exclusive lock (which isn't there)

Comment threadapps/files_trashbin/lib/Storage.php
@juliusknorrjuliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 2, 2019
@blizzz
blizzz merged commit 8c77d3b into masterSep 6, 2019
@blizzz
blizzz deleted the bugfix/emit-moveToTrash-limit branch September 6, 2019 15:46
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable16

@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable15

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable16 in #17099

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable15 in #17100

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@juliusknorr@blizzz@rullzer@skjnldsv