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

Add listener and interfaces to allow versions migration across storage #44187

Merged

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Mar 14, 2024

Add new API to allow migrating version across backends. For example when moving a file to a groupfolder.

  • Add test to ensure moving versions from and to a share is not broken
  • Add IVersionsImporterBackend and implement it in LegacyVersionsBackend

The equivalent Groupfolder PR is here: nextcloud/groupfolders#2860

@artonge artonge self-assigned this Mar 14, 2024
@artonge artonge added enhancement 2. developing Work in progress feature: versions php Pull requests that update Php code labels Mar 14, 2024
@artonge artonge added this to the Nextcloud 29 milestone Mar 14, 2024
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch from 8273c37 to ebd56af Compare March 14, 2024 09:48
@Altahrim Altahrim mentioned this pull request Mar 14, 2024
@artonge artonge changed the title Add listener and interfaces to allow versions migration accros storages Add listener and interfaces to allow versions migration across storages Mar 18, 2024
@artonge artonge changed the title Add listener and interfaces to allow versions migration across storages Add listener and interfaces to allow versions migration across storage Mar 18, 2024
@Altahrim Altahrim mentioned this pull request Mar 18, 2024
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch from ebd56af to 01a40ea Compare March 18, 2024 14:30
@Altahrim Altahrim mentioned this pull request Mar 20, 2024
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 3bed871 to 975ef17 Compare March 20, 2024 15:23
@artonge artonge marked this pull request as ready for review March 20, 2024 15:24
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 7 times, most recently from 404bae4 to 65bbca7 Compare March 21, 2024 11:30
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 4 times, most recently from a9bbdea to 452a63e Compare March 21, 2024 14:03
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 4825f46 to d3b22fd Compare March 25, 2024 15:32
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from b3224be to 5cacb07 Compare March 25, 2024 17:26
@artonge artonge requested a review from come-nc March 25, 2024 17:54
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch from 5cacb07 to 363dd98 Compare March 25, 2024 17:59
@artonge artonge added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 26, 2024
@artonge artonge requested a review from emoral435 March 26, 2024 09:30
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch from 363dd98 to 61aac1f Compare March 26, 2024 11:15
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 5 times, most recently from 14ef689 to b2cf644 Compare March 26, 2024 11:44
/** @var Folder $source */
foreach ($target->getDirectoryListing() as $targetChild) {
if ($event instanceof NodeCopiedEvent) {
$sourceChild = $source->get($targetChild->getName());
Copy link
Member

Choose a reason for hiding this comment

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

is there any reason for not using the same method as used for renames.

The "rename logic" gets the nodes from a getDirectoryListing per folder while this has to do a get for every child. So the "rename logic" should be significantly more efficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it failed in some way, let me finish testing on the groupfolder side, and I'll try it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, in case of copy, we need to do the tree walking to have the id of the source in any case, so it won't improve performances to cache the nodes.

Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

small nitpicks, but overall good PR :)

@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 0150a64 to 6430699 Compare March 26, 2024 14:42
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 56d9a89 to ba93fbb Compare March 26, 2024 16:36
…igration across storages

Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the artonge/feat/support_migrating_versions_across_storages branch from ba93fbb to 369274c Compare March 26, 2024 16:40
@artonge artonge merged commit 72fbbc7 into master Mar 26, 2024
167 checks passed
@artonge artonge deleted the artonge/feat/support_migrating_versions_across_storages branch March 26, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement feature: versions php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants