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

[5.6] Add stream download response #22777

Merged
merged 1 commit into from
Jan 17, 2018
Merged

[5.6] Add stream download response #22777

merged 1 commit into from
Jan 17, 2018

Conversation

TheoKouzelis
Copy link
Contributor

Adds the ability to respond with remote files as downloads.

<?php

    Route::get('/', function () {
        return response()->streamDownload(function () {
            echo file_get_contents('https://my.remote.com/file/store-12345.jpg');
        }, 'nice-name.jpg');
    });

@m1guelpf
Copy link
Contributor

For reference, this is a resubmission of #22764 to 5.6

@taylorotwell
Copy link
Member

Is anything actually being streamed here? Doesn't file_get_contents load the content of the entire remote URL into memory?

@m1guelpf
Copy link
Contributor

m1guelpf commented Jan 14, 2018

@taylorotwell The file_get_contents above is an example, the important thing is that this PR allows developers to return anything the callable returns as an attachament.

Imagine I'm using the GitHub API to get the readme of this repo. Now I can "stream" the file from memory instead of saving it to a file and returning the file:

<?php

    Route::get('/', function () {
        return response()->streamDownload(function () {
            echo GitHub::api('repo')->contents()->readme('laravel', 'laravel')['contents']
        }, 'laravel-readme.md');
    });

@TheoKouzelis
Copy link
Contributor Author

TheoKouzelis commented Jan 14, 2018

Hi @taylorotwell

Thanks for the PR review.

Yeah @m1guelpf has explained the PR perfectly. Just to add a little more context, this PR was born out of this Stackoverflow question where people were looking to use the "download" response with remote files and I thought the "stream" response would be the best way to achieve that.

@m1guelpf You could be right about the return instead of echo. But I can only find examples of the StreamResponse closures having files echoed in them. Example 1 Example 2

@m1guelpf
Copy link
Contributor

@TheoKouzelis Yeah, you're rigth. It makes sense, you have to use echo.

@joecohens
Copy link
Contributor

joecohens commented Jan 16, 2018

Not sure if this helps, but we can also see this case on the League Glide Symfony response which is also used by the Glide Laravel integration:

https://github.com/thephpleague/glide-symfony/blob/2dc271c959aa86c060261e2a93c493a54f98efbb/src/Responses/SymfonyResponseFactory.php#L26-L57

Note: If I'm not mistaken, also the dev should know how to write the streamed response closure, like for example when using file_get_contents:

https://github.com/twistor/flysystem-http/blob/acba993b790777067d7aa405458b4e06d329a9f3/src/HttpAdapter.php#L174-L187

Hope it helps 😄

@taylorotwell taylorotwell merged commit 41a585d into laravel:5.6 Jan 17, 2018
@TheoKouzelis
Copy link
Contributor Author

Thanks for merging the PR @taylorotwell

If we don't include the contract change. Would it be worth me back porting this to the other 5.x branches?

@CharifMakaoui
Copy link

Is this method works for large files, like hd movies (1080p)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants