Skip to content

fix: Also cleanup version metadata if expiring - #39786

Merged
juliusknorr merged 1 commit into
masterfrom
bugfix/version-expire-cleanup
Aug 14, 2023
Merged

fix: Also cleanup version metadata if expiring#39786
juliusknorr merged 1 commit into
masterfrom
bugfix/version-expire-cleanup

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Aug 10, 2023

Copy link
Copy Markdown
Member

Found while writing e2e tests for #39140

Steps to reproduce:

  • Upload some file multiple times with close timestamps
  • Run cron which expires versions (ref
    privatestatic$max_versions_per_interval = [
    //first 10sec, one version every 2sec
    1 => ['intervalEndsAfter' => 10, 'step' => 2],
    //next minute, one version every 10sec
    2 => ['intervalEndsAfter' => 60, 'step' => 10],
    //next hour, one version every minute
    3 => ['intervalEndsAfter' => 3600, 'step' => 60],
    //next 24h, one version every hour
    4 => ['intervalEndsAfter' => 86400, 'step' => 3600],
    //next 30days, one version per day
    5 => ['intervalEndsAfter' => 2592000, 'step' => 86400],
    //until the end one version per week
    6 => ['intervalEndsAfter' => -1, 'step' => 604800],
    ];
    )

This leads to the version files being deleted while the oc_files_versions table still contains references, meaning that they show up as broken files in the versions sidebar.

Ideally expiration should also be handled by the version manager, but this requires some larger efforts and is not backportable.

Quick script to reproduce:

#!/bin/bash
NC_URL=https://admin:admin@nextcloud.local/remote.php/webdav/
filename=$(date | md5sum | cut -d "" -f 1)echo$filenameecho a1 > 1/$filename.md
sleep 2
echo a2 > 2/$filename.md
sleep 2
echo a3 > 3/$filename.md
sleep 2
echo a4 > 4/$filename.md
sleep 2
echo a5 > 5/$filename.md
curl -T "1/$filename.md""$NC_URL/$filename.md"
curl -T "2/$filename.md""$NC_URL/$filename.md"
curl -T "3/$filename.md""$NC_URL/$filename.md"
curl -T "4/$filename.md""$NC_URL/$filename.md"
curl -T "5/$filename.md""$NC_URL/$filename.md"

Afterwards you can compare the following sql queries before and after cron for validation:

# with the echoed filenameselect*from oc_filecache where name like'3882259fffd1ef36e300511488e561be%';
# with the file idselect*from oc_files_versions where file_id =8740;

Checklist

@juliusknorr
juliusknorr requested review from a team, ArtificialOwl, artonge, icewind1991 and nfebe and removed request for a teamAugust 10, 2023 07:29
Comment threadapps/files_versions/lib/Storage.php Fixed
@juliusknorr
juliusknorrforce-pushed the bugfix/version-expire-cleanup branch from 26c7cda to e3bdb72CompareAugust 10, 2023 07:31
@juliusknorrjuliusknorr added bug 3. to review Waiting for reviews labels Aug 10, 2023
@juliusknorrjuliusknorr added this to the Nextcloud 28 milestone Aug 10, 2023
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable27

@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable26

Comment threadapps/files_versions/lib/Storage.php Outdated
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/version-expire-cleanup branch from e3bdb72 to bbb7172CompareAugust 14, 2023 17:31
@solracsf

Copy link
Copy Markdown
Member

@juliushaertl no 25 ?

@juliusknorr

Copy link
Copy Markdown
MemberAuthor

Only 26+ is affected where the new table for metadata was introduced

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

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@juliusknorr@solracsf@icewind1991@artonge@github-advanced-security