Skip to content

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

Merged
artonge merged 1 commit into
masterfrom
artonge/feat/support_migrating_versions_across_storages
Mar 26, 2024
Merged

Add listener and interfaces to allow versions migration across storage#44187
artonge merged 1 commit into
masterfrom
artonge/feat/support_migrating_versions_across_storages

Conversation

@artonge

@artongeartonge commented Mar 14, 2024•
edited
Loading

Copy link
Copy Markdown
Collaborator

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

@artongeartonge self-assigned this Mar 14, 2024
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 14, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
@artongeartonge mentioned this pull request Mar 14, 2024
Merged
@artongeartonge added enhancement 2. developing Work in progress feature: versions php Pull requests that update Php code labels Mar 14, 2024
@artongeartonge added this to the Nextcloud 29 milestone Mar 14, 2024
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch from 8273c37 to ebd56afCompareMarch 14, 2024 09:48
@AltahrimAltahrim mentioned this pull request Mar 14, 2024
Merged
@artongeartonge changed the title Add listener and interfaces to allow versions migration accros storagesAdd listener and interfaces to allow versions migration across storagesMar 18, 2024
@artongeartonge changed the title Add listener and interfaces to allow versions migration across storagesAdd listener and interfaces to allow versions migration across storageMar 18, 2024
@AltahrimAltahrim mentioned this pull request Mar 18, 2024
Merged
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch from ebd56af to 01a40eaCompareMarch 18, 2024 14:30
@AltahrimAltahrim mentioned this pull request Mar 20, 2024
Merged
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 3bed871 to 975ef17CompareMarch 20, 2024 15:23
@artonge
artonge marked this pull request as ready for review March 20, 2024 15:24
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 7 times, most recently from 404bae4 to 65bbca7CompareMarch 21, 2024 11:30
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 21, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
Comment threadapps/files_versions/lib/Versions/LegacyVersionsBackend.php Fixed
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 4 times, most recently from a9bbdea to 452a63eCompareMarch 21, 2024 14:03
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 4825f46 to d3b22fdCompareMarch 25, 2024 15:32
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 25, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from b3224be to 5cacb07CompareMarch 25, 2024 17:26
@artonge
artonge requested a review from come-ncMarch 25, 2024 17:54
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch from 5cacb07 to 363dd98CompareMarch 25, 2024 17:59
@artongeartonge added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 26, 2024
@artonge
artonge requested a review from emoral435March 26, 2024 09:30
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch from 363dd98 to 61aac1fCompareMarch 26, 2024 11:15
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 26, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 5 times, most recently from 14ef689 to b2cf644CompareMarch 26, 2024 11:44
icewind1991
icewind1991 previously requested changes Mar 26, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php
/** @var Folder $source */
foreach ($target->getDirectoryListing() as $targetChild) {
if ($event instanceof NodeCopiedEvent) {
$sourceChild = $source->get($targetChild->getName());

@icewind1991icewind1991Mar 26, 2024

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.

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.

@artongeartongeMar 26, 2024

Copy link
Copy Markdown
CollaboratorAuthor

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.

@artongeartongeMar 26, 2024

Copy link
Copy Markdown
CollaboratorAuthor

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.

Comment threadapps/files_versions/lib/Versions/IVersionsImporterBackend.php Outdated
emoral435
emoral435 approved these changes Mar 26, 2024
View reviewed changes

@emoral435emoral435 left a comment

Copy link
Copy Markdown
Contributor

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 :)

Comment threadapps/files_versions/lib/Listener/FileEventsListener.php
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 0150a64 to 6430699CompareMarch 26, 2024 14:42
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 26, 2024
View reviewed changes
Comment threadapps/files_versions/lib/Listener/VersionStorageMoveListener.php Fixed
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch 2 times, most recently from 56d9a89 to ba93fbbCompareMarch 26, 2024 16:36
@artonge
feat(files_versions): Add listener and interfaces to allow versions m…
369274c
…igration across storages
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge
artongeforce-pushed the artonge/feat/support_migrating_versions_across_storages branch from ba93fbb to 369274cCompareMarch 26, 2024 16:40
@artonge
artonge merged commit 72fbbc7 into masterMar 26, 2024
@artonge
artonge deleted the artonge/feat/support_migrating_versions_across_storages branch March 26, 2024 17:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@come-nccome-nccome-nc approved these changes
@icewind1991icewind1991icewind1991 left review comments
@ArtificialOwlArtificialOwlAwaiting requested review from ArtificialOwl
+1 more reviewer
@emoral435emoral435emoral435 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@artongeartonge

Labels

3. to reviewWaiting for reviewsenhancementfeature: versionsphpPull requests that update Php code

Projects

None yet

Milestone

Nextcloud 29

Development

Successfully merging this pull request may close these issues.

5 participants

@artonge@icewind1991@github-advanced-security@come-nc@emoral435