Skip to content
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

switch to Files Node API for zip generation #17822

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Nov 5, 2019

since it is disencouraged to use the static Filesystem methods

@blizzz
Copy link
Member Author

blizzz commented Nov 5, 2019

/backport to stable17

$fileTime = \OC\Files\Filesystem::filemtime($file);
$fh = \OC\Files\Filesystem::fopen($file, 'r');
$streamer->addFileFromStream($fh, basename($file), $fileSize, $fileTime);
$owner = \OC\Files\Filesystem::getOwner($file);
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't use the owner but the current user

Copy link
Member Author

Choose a reason for hiding this comment

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

@icewind1991 what about unauthenticated users, on public shares?

Copy link
Member

Choose a reason for hiding this comment

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

They have the "current user" set correctly afaik, since it's also needed for the old Filesystem stuff to work (worth double testing though)

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, using $userFolder = \OC::$server->getUserFolder(); returns null in that case. Of cours we can try that first and fall back to the owner? Or does it open any loophole? Previously it also only just read it. IIRC it's best to avoid using root folder directly for performance.

Copy link
Member

Choose a reason for hiding this comment

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

\OC::$server->getRootFolder()->get(Filesystem::getRoot()) would always have the same behavior as using the old api

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz force-pushed the fix/noid/tip-download-to-node-api branch from 4852da5 to f9bfd48 Compare November 7, 2019 08:45
@blizzz blizzz requested a review from icewind1991 November 7, 2019 08:45
@blizzz
Copy link
Member Author

blizzz commented Nov 7, 2019

/backport to stable16

@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 7, 2019
@blizzz blizzz merged commit 9fe4b95 into master Nov 7, 2019
@blizzz blizzz deleted the fix/noid/tip-download-to-node-api branch November 7, 2019 10:03
@backportbot-nextcloud
Copy link

backport to stable17 in #17840

@backportbot-nextcloud
Copy link

backport to stable16 in #17841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: files technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants