Optimize put - dont fetch metadata for part file in checksuming#27531
Conversation
|
Needs additional unit test or integration? As in all other PRs https://github.com/owncloud/core/pulls/mrow4a, this pops up occasionaly with reordered users , @butonic |
|
Did we merge your sorting fix ? If yes then you can rebase this. |
a00f8b8 to
5481629
Compare
| */ | ||
| public function getMetaData($path) { | ||
| $parentMetaData = $this->getWrapperStorage()->getMetaData($path); | ||
| // Check if it is partial file. Partial file metadata are only checksums |
There was a problem hiding this comment.
add $parentMetaData = [] in the case we don't fetch it. I know PHP is "clever" and will make an array out thin air, but having it explicit is more readable
|
@mrow4a any update ? After making the change requested above please rebase to get more integration tests from master |
|
@PVince81 @DeepDiver1975 @jvillafanez I think this is ready :> |
|
@mrow4a seems you missed my comment ? |
|
Ohh, true, thanks! Will change |
| * | ||
| * @param string $file | ||
| * @return boolean | ||
| */ |
There was a problem hiding this comment.
We should have an utility method for common path checks. This isn't the first time I've seen (nor written 😞 ) this function.
I'd leave it as technical debt for now, but it should be tackled as soon as time allows it.
There was a problem hiding this comment.
@DeepDiver1975 where will you put this utility?
| public static function isPartialFile($file) { | ||
| if (pathinfo($file, PATHINFO_EXTENSION) === 'part') { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Any case for this? a/b.part/c will be consider as a part file, which I guess it's ok because a/b.part shouldn't exists in the first place, or the file is inside a "part" directory which is fine to ignore.
There was a problem hiding this comment.
Hmm, why is it important for Scanner? https://github.com/owncloud/core/blob/master/lib/private/Files/Cache/Scanner.php#L468 @PVince81 @jvillafanez
There was a problem hiding this comment.
I think .part files are ignored by the scanner
64539f8 to
36675cd
Compare
|
@jvillafanez @PVince81 can we merge, I checked with DiagnosticApp and it works as desired. |
|
@mrow4a please backport to stable10 |
|
Note: this causes an issue when uploading ".part" files directly through Webdav, see #28929 (comment) However we're not supposed to allow uploading ".part" files anyway through Webdav. The latter will be fixed in #7496 (comment). This is also for information purposes. No fix is needed for the code from this PR here. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
-1 query as well here