Skip to content

Prevent getting a version expiry list when no versions available - #38390

Merged
JammingBen merged 2 commits into
masterfrom
issues/38373
Feb 11, 2021
Merged

Prevent getting a version expiry list when no versions available#38390
JammingBen merged 2 commits into
masterfrom
issues/38373

Conversation

@JammingBen

Copy link
Copy Markdown
Contributor

Description

Previous to this little fix, when getting a version expiry list with an empty $version array, OC ran into an error. Not a critical one, but still not nice and spams the owncloud.log file.

How does this happen?

Versions of a file expire at a certain point of time. When a version is added to a file, an expiry command is scheduled via cronjob. Eventually the cronjob runs and will expire (=delete) the version of this file. But it is also possible to clean up all versions in between those two actions, for instance with the command occ version:cleanup. When the cronjob runs afterwards, no versions will be available to expire -> error.

Steps to reproduce:

  • Create a new file, then edit that file -> a version will be created.
  • Clean up all versions: occ version:cleanup.
  • Run cronjob to expire versions: occ system:cron.
  • Check your owncloud.log file, it should've logged the error: Trying to access array offset on value of type bool at /path/to/your/owncloud.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE
@JammingBen JammingBen self-assigned this Feb 10, 2021

@jvillafanez jvillafanez left a comment

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.

I don't expect a really big list of versions (maybe 100 at most), so the performance impact should be minimal. No real need to change anything.

$expiration = self::getExpiration();

if ($expiration->shouldAutoExpire()) {
if ($expiration->shouldAutoExpire() && \count($versions) > 0) {

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.

!empty($versions) might be faster

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@JammingBen
JammingBen merged commit a6a4d52 into master Feb 11, 2021
@delete-merged-branch
delete-merged-branch Bot deleted the issues/38373 branch February 11, 2021 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants