With objectstore, scan from sourcescanner for shared files #32377
Conversation
| if ($sourceScanner instanceof NoopScanner) { | ||
| // No Operation Scanner will not update share permission (scanFile won't call getData) | ||
| list(, $internalPath) = $this->storage->resolvePath($file); | ||
| return $sourceScanner->scanFile($internalPath, $reuseExisting, $parentId, $cacheData, $lock); |
There was a problem hiding this comment.
Due to the fact that we use "source scanner" directly, and not use inheritance, function getData is not being called for objectstorage (NoopScanner).
I tried to implement the wrapper, but it is nearly impossible due to how Storage interfaces are implemented (https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Storage.php#L51) - storage has function getScanner, which in turn can accept another storage as argument.
Codecov Report
@@ Coverage Diff @@
## master #32377 +/- ##
============================================
- Coverage 64.03% 64.03% -0.01%
+ Complexity 18579 18577 -2
============================================
Files 1171 1171
Lines 69876 69870 -6
Branches 1267 1267
============================================
- Hits 44747 44743 -4
+ Misses 24759 24757 -2
Partials 370 370
Continue to review full report at Codecov.
|
| * | ||
| * @inheritdoc | ||
| */ | ||
| public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) { |
There was a problem hiding this comment.
Removing "scan" as might be better if parent method will be executed (It does some locking before running scanFile). CC @DeepDiver1975
|
@DeepDiver1975 I just tested it on smashbox and all good. In reviewing, please use |
|
@DeepDiver1975 the referenced PR #32337 was backported. |
|
yes - baskport is required - but I tend to postpone this once we have smashbox green @mrow4a please keep a list of necessary backports - bet in a ticker in here in core to track this all - THX |
|
@DeepDiver1975 exactly, I will backport all scanner related stuff together if needed. Or objectstore related in general |
|
@mrow4a ping - is this still waiting for a "major backport of scanner related stuff"? |
|
@phil-davis we dont have smashbox green yet. There is some missing files_primary_s3 functionality. CC @DeepDiver1975 |
|
I noticed this while backporting "Introduce PHPstan" in PR #35774 Backporting this PR would/should bypass having to massage this in |
|
@phil-davis this is just a performance optimization to avoid unnecessary calls to "locks" with objectstorage. Other storages are not affected |
In #32337 (scan function has been added, but is not needed). Similarly scanFile gets correction (typo).
@DeepDiver1975