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

Versions on external storage never expire #24161

Closed
PVince81 opened this Issue Apr 21, 2016 · 3 comments

Comments

Projects
None yet
1 participant
@PVince81
Member

PVince81 commented Apr 21, 2016

Steps

  1. Mount an external storage (ex: SMB) as "/smb" system-wide, apply to all users
  2. Login as any user
  3. Create a file "/smb/test.txt"
  4. Edit the file and save many times to create 10-20 versions
  5. Check the version files for that user: find data/user1 -iname \*.v\*
  6. update oc_jobs set last_run=0
  7. Run cron.php

Expected result

Some version expired

Actual result

No versions expired.

Versions

ownCloud 9.0.1

The reason is because the expire() method calls getUidAndFilename and the external storage has no owner, so it bails out with NoUserException without doing any expiration.

According to @schiesbn, versions on system-wide ext storage are always specific to the current user viewing the folder. So the correct logic would be to make it fallback to the user that was specified in the expire job.

CC @nickvergessen @VicDeo

@PVince81 PVince81 added this to the 9.0.3-next-maintenance milestone Apr 21, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Apr 21, 2016

Member

Looks like the code in getUidAndFilename is very old, it assumes that the owner can be another user. This is obsolete because since the introduction of expire background jobs, we only ever expire for the user specified in the expire job. For shared folders, the expire job is only created for owners even when a recipient edited a file.

This means that as part of this fix we can get rid of getOwner() and always use the expire job user.
It would also fix the case with external storage where we need to use that user too.

Member

PVince81 commented Apr 21, 2016

Looks like the code in getUidAndFilename is very old, it assumes that the owner can be another user. This is obsolete because since the introduction of expire background jobs, we only ever expire for the user specified in the expire job. For shared folders, the expire job is only created for owners even when a recipient edited a file.

This means that as part of this fix we can get rid of getOwner() and always use the expire job user.
It would also fix the case with external storage where we need to use that user too.

@PVince81 PVince81 added the sev2-high label Apr 25, 2016

@PVince81 PVince81 modified the milestones: 9.0.3-current-maintenance, 9.0.4-next-maintenance Jun 17, 2016

@PVince81 PVince81 modified the milestones: 9.0.5, 9.0.4 Jul 18, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Aug 15, 2016

Member

@VicDeo can you take care of this ?

Member

PVince81 commented Aug 15, 2016

@VicDeo can you take care of this ?

@PVince81 PVince81 modified the milestones: 9.0.6, 9.0.5 Aug 18, 2016

@PVince81 PVince81 modified the milestones: 9.0.7, 9.0.6 Oct 20, 2016

@PVince81 PVince81 referenced this issue Nov 10, 2016

Merged

Properly expire ext storage versions #26601

8 of 14 tasks complete
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Nov 10, 2016

Member

Fix is here #26601

Member

PVince81 commented Nov 10, 2016

Fix is here #26601

@PVince81 PVince81 closed this in #26601 Nov 22, 2016

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