Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Jan 26, 2025

Summary

  • Harden some checks in the files scanner (strong equality)
  • Some refactoring to early return
  • Fix type issues

Checklist

@susnux susnux added bug 3. to review Waiting for reviews php Pull requests that update Php code labels Jan 26, 2025
@susnux susnux added this to the Nextcloud 32 milestone Jan 26, 2025
@susnux
Copy link
Contributor Author

susnux commented Jan 26, 2025

/backport to stable31

@susnux
Copy link
Contributor Author

susnux commented Jan 26, 2025

Stable30 and stable29 differ a bit, so their PR is here: #50437

@susnux susnux force-pushed the fix/encoding-wrapper-scanner branch from c53892b to 12d0806 Compare January 26, 2025 15:14
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +181 to +182
$cacheData = $cacheData ?? $this->cache->get($file);
if ($reuseExisting && $cacheData !== false && isset($cacheData['fileid'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory $cacheData can never be null based on the logic and the return type of Cache::get(), but looking at the backing code I wouldn't be so sure that it really never returns null, so maybe add a check for null as well.

$data['fileid'] = $this->addToCache($file, $newData, $fileId);
}

if ($cacheData !== false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here again, maybe check for null as well.

@susnux susnux force-pushed the fix/encoding-wrapper-scanner branch from 484b509 to 53815b3 Compare January 27, 2025 00:25
@susnux susnux force-pushed the fix/encoding-wrapper-scanner branch 2 times, most recently from 3ed5a56 to 75fb094 Compare January 27, 2025 21:52
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/encoding-wrapper-scanner branch from 578af46 to b48ee2e Compare January 28, 2025 19:08
@susnux susnux merged commit fb131c1 into master Jan 28, 2025
189 checks passed
@susnux susnux deleted the fix/encoding-wrapper-scanner branch January 28, 2025 19:47
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants