-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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