You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2016/06/02 23:02:03 UTC

[GitHub] incubator-guacamole-client pull request #14: GUACAMOLE-44: Slice uploaded fi...

GitHub user mike-jumper opened a pull request:

    https://github.com/apache/incubator-guacamole-client/pull/14

    GUACAMOLE-44: Slice uploaded files into chunks (DO NOT slurp entire file into memory)

    The old file upload code was inherently flawed as it required reading the entire file into memory. Besides being a poor user experience (long delay before upload begins), this could even crash the browser if the file was large enough.
    
    This change adds a new writer-style object, `Guacamole.FileWriter`, whose `sendFile()` efficiently streams a provided `File` over an underlying `Guacamole.OutputStream`. The implementation streams the `File` properly via `slice()`; it is not read in its entirety into memory.
    
    As this required wrapping a `Guacamole.ArrayBufferWriter` and being aware of the Guacamole protocol's instruction size restrictions, this change replaces the old magic 6048 with a well-documented constant, `Guacamole.ArrayBufferWriter.DEFAULT_BLOB_LENGTH`, and exposes the blob length used by any particular `Guacamole.ArrayBufferWriter` instance with a mutable `blobLength` property.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mike-jumper/incubator-guacamole-client slice-uploads

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-guacamole-client/pull/14.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #14
    
----
commit 2c977a31349848826596ef510993a45217145ccc
Author: Michael Jumper <mj...@apache.org>
Date:   2016-06-02T22:40:09Z

    GUACAMOLE-44: Explicitly define and document the magic 6048-byte blob within ArrayBufferWriter. Allow the blob size to be overridden.

commit 998eff9ca3878fe419690bf7ba7d582606ce6821
Author: Michael Jumper <mj...@apache.org>
Date:   2016-06-02T22:41:08Z

    GUACAMOLE-44: Implement Guacamole.FileWriter which provides for streaming local files over a Guacamole.OutputStream.

commit 2934f4a9be67952d2be03124fc4ff34a3fd9c33b
Author: Michael Jumper <mj...@apache.org>
Date:   2016-06-02T22:51:15Z

    GUACAMOLE-44: Use Guacamole.FileWriter within ManagedFileUpload (rather than load entire file into memory).

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #14: GUACAMOLE-44: Slice uploaded fi...

Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/14#discussion_r65634557
  
    --- Diff: guacamole-common-js/src/main/webapp/modules/ArrayBufferWriter.js ---
    @@ -62,6 +62,20 @@ Guacamole.ArrayBufferWriter = function(stream) {
         }
     
         /**
    +     * The maximum length of any blob sent by this Guacamole.ArrayBufferWriter,
    +     * in bytes. Data sent via
    +     * {@link Guacamole.ArrayBufferWriter#sendData|sendData()} which exceeds
    --- End diff --
    
    It's the way you provide text for JSDoc3's `@link`: http://usejsdoc.org/tags-inline-link.html
    
    Re-reading the above ... it looks like there's a more readable, more markdown-y syntax available. Perhaps I should switch over to that before precedent is set?
    
    I believe this is our first use of `@link` in the docs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #14: GUACAMOLE-44: Slice uploaded fi...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-guacamole-client/pull/14


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #14: GUACAMOLE-44: Slice uploaded fi...

Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/14#discussion_r65634256
  
    --- Diff: guacamole-common-js/src/main/webapp/modules/ArrayBufferWriter.js ---
    @@ -62,6 +62,20 @@ Guacamole.ArrayBufferWriter = function(stream) {
         }
     
         /**
    +     * The maximum length of any blob sent by this Guacamole.ArrayBufferWriter,
    +     * in bytes. Data sent via
    +     * {@link Guacamole.ArrayBufferWriter#sendData|sendData()} which exceeds
    --- End diff --
    
    What's this syntax? "sendData|sendData()"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #14: GUACAMOLE-44: Slice uploaded fi...

Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/14#discussion_r65634966
  
    --- Diff: guacamole-common-js/src/main/webapp/modules/ArrayBufferWriter.js ---
    @@ -62,6 +62,20 @@ Guacamole.ArrayBufferWriter = function(stream) {
         }
     
         /**
    +     * The maximum length of any blob sent by this Guacamole.ArrayBufferWriter,
    +     * in bytes. Data sent via
    +     * {@link Guacamole.ArrayBufferWriter#sendData|sendData()} which exceeds
    --- End diff --
    
    If there's a more readable syntax available, we should probably use that. :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---