X Tutup
Skip to content

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

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

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

Conversation

@JammingBen
Copy link
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
Copy link
Member

@jvillafanez jvillafanez left a comment

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trying to access array offset on value of type bool at

4 participants

X Tutup