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

File upload transmission #92

Merged
merged 3 commits into from
Sep 30, 2022
Merged

File upload transmission #92

merged 3 commits into from
Sep 30, 2022

Conversation

thekid
Copy link
Member

@thekid thekid commented Aug 24, 2022

This pull request introduces a transmit() method for file uploads to replace the transfer() method, making uploads consistent with downloads.

Example

Given the following file upload code:

$uploads= new Folder(...);
$handler= function($req, $res) use($uploads) {
  if ($multipart= $req->multipart()) {
    if ('100-continue' === $req->header('Expect')) $res->hint(100, 'Continue');

    $files= [];
    $bytes= 0;
    foreach ($multipart->files() as $name => $file) {
      $files[]= $name;
      $bytes+= $file->transfer($uploads);
    }

    // Do something with files and bytes...
  }
};

The problem is that all other requests are blocked until all uploads have been processed. By adding a yield statement after $file->transfer($uploads);, we could hand back control to the server once a file has been transferred (which is what README suggested):

 foreach ($multipart->files() as $name => $file) {
   $files[]= $name;
   $bytes+= $file->transfer($uploads);
+  yield;
 }

However, with big file uploads (or clients with slow connection speeds), this might still be blocking other requests for a significant amount of time. With the new API, the example can be changed as follows:

 foreach ($multipart->files() as $name => $file) {
   $files[]= $name;
-  $bytes+= $file->transfer($uploads);
+  $bytes+= yield from $file->transmit($uploads);
 }

Further notes

This pull request makes use of the new features added in xp-framework/networking#22. However, the code also runs without this featureset being available!

@thekid thekid mentioned this pull request Sep 25, 2022
@thekid thekid merged commit 3ca6ce5 into master Sep 30, 2022
@thekid thekid deleted the refactor/async-io branch September 30, 2022 11:56
@thekid
Copy link
Member Author

thekid commented Sep 30, 2022

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

Successfully merging this pull request may close these issues.

1 participant