Skip to content

Don't die with LockedException when removing/restoring multiple files from trash - #28438

Merged
skjnldsv merged 1 commit into
nextcloud:masterfrom
csware:issue-1649
Dec 21, 2021
Merged

Don't die with LockedException when removing/restoring multiple files from trash#28438
skjnldsv merged 1 commit into
nextcloud:masterfrom
csware:issue-1649

Conversation

@csware

@cswarecsware commented Aug 15, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

fixes issue #16491

TODO: Maybe in medium term the whole restore/delete from trash activity should be guarded by a lock.

@csware
cswareforce-pushed the issue-1649 branch 2 times, most recently from ea801ce to ee41c31CompareAugust 15, 2021 18:28
@szaimenszaimen added the 3. to review Waiting for reviews label Aug 17, 2021
@szaimenszaimen added this to the Nextcloud 23 milestone Aug 17, 2021
@skjnldsv
skjnldsv requested a review from a teamAugust 18, 2021 12:58
juliusknorr
juliusknorr reviewed Aug 18, 2021
View reviewed changes
Comment threadapps/files_trashbin/lib/Trashbin.php
@@ -968,9 +968,21 @@ private static function getVersionsFromTrash($filename, $timestamp, $user) {
[$storage,] = $view->resolvePath('/');

//force rescan of versions, local storage may not have updated the cache

@juliusknorrjuliusknorrAug 18, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not fully sure about that logic at all, i cannot see why the rescan might be needed these days unless there is an issue with moving the versions there initially. But might be a legacy leftover, maybe @icewind1991 or @PVince81 have any further insight?

Intial scanning introduced in 0254a3c#diff-25ac00f6c51dc1ab8783ce98d51cec3d11a46f104c141a9a0df1896d2bfd31efR923-R924

@PVince81PVince81Aug 24, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure. and even stranger that this was introduced as part of primary object store, which would usually not be affected by this

@csware
Don't die with LockedException when removing/restoring multiple files…
1c736ab
… from trash
fixes issue nextcloud#16491
Signed-off-by: Sven Strickroth <email@cs-ware.de>
@csware

csware commented Aug 19, 2021

Copy link
Copy Markdown
ContributorAuthor

One test is failing, however, I can't see why. A hint might be really helpful.

@solracsfsolracsf linked an issue Aug 20, 2021 that may be closed by this pull request
Permanently deleting/restoring multiple files from trash results in OCP\Lock\LockedException#16491
Closed
@blizzz

blizzz commented Aug 24, 2021

Copy link
Copy Markdown
Member

One test is failing, however, I can't see why. A hint might be really helpful.

likely a false positive

@csware

csware commented Sep 13, 2021

Copy link
Copy Markdown
ContributorAuthor

Any news?

@skjnldsvskjnldsv mentioned this pull request Oct 13, 2021
Merged
@skjnldsv
skjnldsv requested review from a team, juliusknorr and nickvergessenOctober 22, 2021 09:55
@skjnldsvskjnldsv mentioned this pull request Oct 25, 2021
Merged
25 tasks
@csware

csware commented Oct 25, 2021

Copy link
Copy Markdown
ContributorAuthor

I', using this for several weeks and it works.

@skjnldsvskjnldsv mentioned this pull request Nov 1, 2021
Merged
19 tasks
@blizzzblizzz mentioned this pull request Nov 3, 2021
Closed
18 tasks
@skjnldsvskjnldsv mentioned this pull request Nov 8, 2021
Merged
23 tasks
@blizzzblizzz modified the milestones: Nextcloud 23, Nextcloud 24Nov 30, 2021
@skjnldsv
skjnldsv merged commit 0b01103 into nextcloud:masterDec 21, 2021
@skjnldsvskjnldsv removed the 3. to review Waiting for reviews label Dec 21, 2021
@skjnldsvskjnldsv added the 4. to release Ready to be released and/or waiting for tests to finish label Dec 21, 2021
@Octopus2

Octopus2 commented Dec 23, 2021

Copy link
Copy Markdown

Hello and thank you for solving this annoying problem.
Is there a possibility of a backport to version 22 and 23?
Best regards

@csware

csware commented Dec 23, 2021

Copy link
Copy Markdown
ContributorAuthor

/backport to stable23
/backport to stable22

@csware
csware deleted the issue-1649 branch December 23, 2021 10:23
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Dec 23, 2021
Merged
@csware

csware commented Dec 23, 2021

Copy link
Copy Markdown
ContributorAuthor

/backport to stable22

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Dec 23, 2021
Merged
@solracsfsolracsf mentioned this pull request Dec 26, 2021
Closed
@khlschrnkkhlschrnk mentioned this pull request Nov 17, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsvskjnldsvskjnldsv approved these changes
@come-nccome-nccome-nc approved these changes
@PVince81PVince81Awaiting requested review from PVince81
@icewind1991icewind1991Awaiting requested review from icewind1991
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr
@nickvergessennickvergessenAwaiting requested review from nickvergessen

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

Permanently deleting/restoring multiple files from trash results in OCP\Lock\LockedException

8 participants

@csware@blizzz@Octopus2@PVince81@juliusknorr@skjnldsv@come-nc@szaimen