Skip to content

Use File Node API for more download cases, skip unreadable files - #18433

Merged
blizzz merged 3 commits into
masterfrom
fix/noid/fsusage
Dec 19, 2019
Merged

Use File Node API for more download cases, skip unreadable files#18433
blizzz merged 3 commits into
masterfrom
fix/noid/fsusage

Conversation

@blizzz

@blizzzblizzz commented Dec 16, 2019

Copy link
Copy Markdown
Member

Follow up to #17822

@blizzzblizzz added this to the Nextcloud 18 milestone Dec 16, 2019
@blizzzblizzz changed the title User File Node API for more download cases, skip unreadable filesUse File Node API for more download cases, skip unreadable filesDec 16, 2019
rullzer
rullzer requested changes Dec 17, 2019
View reviewed changes
Comment threadlib/private/Streamer.php
Comment threadlib/private/legacy/files.php
@rullzerrullzer mentioned this pull request Dec 17, 2019
Merged
34 tasks
@blizzz

blizzz commented Dec 17, 2019

Copy link
Copy Markdown
MemberAuthor

mh, some integration tests for download throw 404s and i can reproduce locally :/

@blizzzblizzz added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 17, 2019
@rullzer

rullzer commented Dec 17, 2019

Copy link
Copy Markdown
Member

Lets restart them first

@rullzer

rullzer commented Dec 17, 2019

Copy link
Copy Markdown
Member

Nope we'll have to dive into that..

@blizzz

blizzz commented Dec 17, 2019

Copy link
Copy Markdown
MemberAuthor

yeah, no surprise it fails for me, too… I'll dive, either a bit later, or tomorrow morning.

@blizzzblizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 17, 2019
@blizzz

blizzz commented Dec 17, 2019

Copy link
Copy Markdown
MemberAuthor

first arg has to be full path, not just a name. works one folder down, but not deeper structures.

@blizzz

blizzz commented Dec 18, 2019

Copy link
Copy Markdown
MemberAuthor

Passes now (apart from those MySQL timeouts)

icewind1991
icewind1991 requested changes Dec 18, 2019
View reviewed changes
Comment threadlib/private/Streamer.php
* @throws InvalidPathException
*/
public function addDirRecursive($dir, $internalDir='') {
public function addDirRecursive(string $dir, string $internalDir = ''): void {

@icewind1991icewind1991Dec 18, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

passing the Node/FileInfo instead of the path here would be nice as it would cut the number of filecache queries in half

@blizzzblizzzDec 18, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Passing the Node would be more helpful, I agree and had the same thought. I discarded it to have a simpler and smaller changeset to backport.

@blizzz

blizzz commented Dec 18, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable17

@blizzz

blizzz commented Dec 18, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable16

@blizzz

blizzz commented Dec 18, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable15

@rullzerrullzer mentioned this pull request Dec 19, 2019
Merged
18 tasks
juliusknorrand others added 3 commits December 19, 2019 13:05
@juliusknorr@blizzz
Use files node API for single file downloads
9e5d611
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@blizzz
use Nodes API for zip streaming
79eae96
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
when downloading from web, skip files that are not accessible
af91efd
* avoids a 403, but enables download of resources that are not restricted
* single file downloads still cause 403
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzzblizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 19, 2019
@blizzz
blizzz merged commit 59402bf into masterDec 19, 2019
@blizzz
blizzz deleted the fix/noid/fsusage branch December 19, 2019 16:55
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Dec 19, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Dec 19, 2019

Copy link
Copy Markdown

backport to stable17 in #18488

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Dec 19, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Dec 19, 2019

Copy link
Copy Markdown

backport to stable16 in #18489

@backportbot-nextcloud

backportbot-nextcloudBot commented Dec 19, 2019

Copy link
Copy Markdown

The backport to stable15 failed. Please do this backport manually.

@blizzz

blizzz commented Dec 20, 2019

Copy link
Copy Markdown
MemberAuthor

15 is EOL now

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kesselbkesselbkesselb left review comments
@rullzerrullzerrullzer approved these changes
@icewind1991icewind1991icewind1991 approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: filesfeature: filesystem

Projects

None yet

Milestone

Nextcloud 18.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@blizzz@rullzer@icewind1991@kesselb@juliusknorr