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.

Sign up for GitHub

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

Jump to bottom

Fix cross-storage move info when moving between two received shares #28022

Merged
merged 3 commits into from
Jun 26, 2017
Merged

Fix cross-storage move info when moving between two received shares #28022

merged 3 commits into from
Jun 26, 2017

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented May 26, 2017 •
edited
Loading

Description

Need to properly resolve the given path to a source path in
getMoveInfo. This was missing when adding this new method.

Related Issue

Fixes #28018

Motivation and Context

How Has This Been Tested?

  • Manual test with the steps from the ticket.
  • Pending integration tests from @SergioBertolinSG, feel free to push them onto this branch.
  • Will need regression testing of other cross-storage "move" scenarios

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@PVince81 PVince81 added this to the 10.0.3 milestone May 26, 2017
@PVince81 PVince81 mentioned this pull request May 26, 2017
Closed
@PVince81
Copy link
Contributor Author

Cherry-picked integration tests from #28020

@PVince81
Copy link
Contributor Author

@VicDeo @tomneedham @jvillafanez @butonic please review, it's deep FS stuff...

@PVince81
Copy link
Contributor Author

Problem also exists on stable9 (9.1.5). Confirmed by backporting the integration tests quickly and running them.

@mrow4a
Copy link
Contributor

mrow4a commented May 26, 2017

Testing just as in #28018 ?

@PVince81
Copy link
Contributor Author

Jenkins 🍹 🏖

@PVince81
Copy link
Contributor Author

@mrow4a yes, but better come up with broader cases

@SamuAlfageme SamuAlfageme mentioned this pull request May 30, 2017
Closed
Vincent Petry and others added 3 commits May 31, 2017 11:44
Fix cross-storage move info when moving between two received shares
8f35f3e 
Need to properly resolve the given path to a source path in
`getMoveInfo`. This was missing when adding this new method.
@SergioBertolinSG
Trying to reproduce core#28018
2b29079
Improve tests
cb94e0b
@PVince81 PVince81 force-pushed the cross-storage-moveinfo branch from 421a58b to cb94e0b Compare May 31, 2017 09:44
@SergioBertolinSG SergioBertolinSG mentioned this pull request Jun 20, 2017
Closed
@PVince81 PVince81 added the p2-high Escalation, on top of current planning, release blocker label Jun 21, 2017
@guruz
Copy link
Contributor

guruz commented Jun 22, 2017

Maybe we can work with the people in owncloud/client#5712 (comment) to see if this fixes their issue? (after it was reviewed here..)

@PVince81
Copy link
Contributor Author

There is a high chance that it fixes the issue. However this doesn't automatically repair already broken filecache. It only prevents the error situation to occur in the first place.

@PVince81
Copy link
Contributor Author

@VicDeo @tomneedham @jvillafanez @butonic review please, we must get rid of this filecache destroyer. Thanks.

@guruz guruz requested a review from butonic June 26, 2017 13:00
@jvillafanez
Copy link
Member

👍

@PVince81 PVince81 merged commit f539ea5 into master Jun 26, 2017
@PVince81 PVince81 deleted the cross-storage-moveinfo branch June 26, 2017 14:49
@PVince81 PVince81 mentioned this pull request Jun 26, 2017
Merged
@PVince81
Copy link
Contributor Author

stable10: #28219

@PVince81 PVince81 modified the milestones: 10.1, 10.0.3 Jun 26, 2017
@PVince81
Copy link
Contributor Author

similar issue was reported internally on 9.1, I'll backport and test this onto other stable branches

@PVince81
Copy link
Contributor Author

Oh noes... the fix is not enough for stable9.1 😢

@PVince81 PVince81 mentioned this pull request Jun 27, 2017
Merged
@PVince81
Copy link
Contributor Author

PVince81 commented Jun 27, 2017 •
edited
Loading

stable9.1 PR: #28238

edit: works now

@PVince81
Copy link
Contributor Author

keeping backport label for stable9 later

@PVince81
Copy link
Contributor Author

stable9 looks more tricky. I think I'd rather focus on the repair routines first.

@PVince81 PVince81 mentioned this pull request Jul 3, 2017
Closed
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 3, 2017

stable8.2 is NOT affected

@PVince81 PVince81 mentioned this pull request Jul 3, 2017
Merged
@PVince81
Copy link
Contributor Author

PVince81 commented Jul 3, 2017

stable9: #28284

@PVince81 PVince81 mentioned this pull request Jul 3, 2017
Merged
11 tasks
@DeepDiver1975 DeepDiver1975 modified the milestones: 10.1, development Oct 10, 2017
@PVince81 PVince81 mentioned this pull request Mar 15, 2018
Closed
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@butonic butonic Awaiting requested review from butonic

Assignees
No one assigned
Labels
3 - To Review p2-high Escalation, on top of current planning, release blocker
Projects
None yet
Milestone
10.0.5
Development

Successfully merging this pull request may close these issues.

Creation of subfolders using desktop client after a move across shares can lead to duplicated folders.
6 participants
@PVince81 @mrow4a @guruz @jvillafanez @DeepDiver1975 @SergioBertolinSG