[TIMOB-20483] Fix HTTPClient.onsendstream progress

[garymathews]

• Use fixed streaming mode to disable buffer
• Remove redundant setUseCaches(true)

TEST CASE #1

var win = Ti.UI.createWindow({backgroundColor: 'grey'}),
    progress = Ti.UI.createProgressBar({
        width: '90%',
        min: 0,
        max: 1,
        value: 0,
    }),
    timestamp = Date.now(),
    data = Array(1024*1024*2).join('0'), // 2,097,152 bytes
    http = Ti.Network.createHTTPClient({
        onload: function() {
            var seconds =  Math.floor((Date.now()-timestamp)/1000);
            Ti.API.info(seconds + 's : success');
            Ti.API.info('response: ' + this.responseText);
        },
        onsendstream: function(e) {
            var seconds =  Math.floor((Date.now()-timestamp)/1000);
            Ti.API.info(seconds + 's : ' + e.progress);
            progress.setValue(e.progress);
        }
    });

win.addEventListener('open', function() {
    http.open('POST', 'http://requestb.in/sfwuxksf'); // may need a new requestb to test
    Ti.API.info('data.length():' + data.length);
    http.send({data: data})
});

win.add(progress);
win.open();

TEST CASE #2

var http = Ti.Network.createHTTPClient({
        onload: function() {
            Ti.API.info('response: ' + this.responseText);
            if (this.responseText.indexOf('Redirect test page') > -1) {
                Ti.API.info('success');
            }
        }
    });
http.open('POST', 'https://jigsaw.w3.org/HTTP/300/301.html');
http.send();

TEST CASE #3

var http = Ti.Network.createHTTPClient({
        onload: function (e) {
            Ti.API.info('success: ' + this.responseText);
        }
    });

http.open('POST', 'http://requestb.in/1lz63bq1');
http.send({
    name: 'ID_' + new Date().getTime(),
    attachment: Ti.Filesystem.getFile(Ti.Filesystem.resourcesDirectory, 'Logo.png').read()
});

NOTE: Also needs testing with TIMOB-23852

JIRA Ticket

[sgtcoolguy]

I think we need to try and use setFixedLengthStreamingMode, but call it later in the lifecycle, like in the ConnectionRunnable. We also need some tests to verify whether this breaks POST/PUT against URLs requiring basic auth or having at least one redirect.

[sgtcoolguy]

Some notes:

• This method is explicitly noted in the docs as to be used when we don't know the content length. But I think we always do know the content length (stored as totalLength in the connectionRunnable class?). When the length is known, they urge you to use #setFixedLengthStreamingMode(long) https://developer.android.com/reference/java/net/HttpURLConnection.html#setFixedLengthStreamingMode(long)
• Secondly, when either of these are used, we avoid buffering which improves performance, but both have an important note in the docs:

When output streaming is enabled, authentication and redirection cannot be handled automatically. A HttpRetryException will be thrown when reading the response if authentication or redirection are required. This exception can be queried for the details of the error.

As such, I think this change will actually break requests done against URLs that have either redirects or ask for basic auth. It'd be very useful to add some unit tests that cover these scenarios to verify.

[garymathews]

I did initially want to use setFixedLengthStreamingMode but totalLength isn't the true length of the content that gets sent. The data gets manipulated into a form, which causes length issues when using setFixedLengthStreamingMode.

I did test against HTTPS and here's one with a redirect that works too:

var http = Ti.Network.createHTTPClient({
        onload: function() {
            Ti.API.info('response: ' + this.responseText);
        }
    });
http.open('POST', 'https://jigsaw.w3.org/HTTP/300/301.html');
http.send();

However, I'll look into correcting totalLength and use setFixedLengthStreamingMode

[garymathews]

@sgtcoolguy Updated PR

[sgtcoolguy]

@garymathews Looks like two httpclient unit tests failed.

[sgtcoolguy]

Looks to me like you're missing a few of the parts to add up the total length. The original code is nasty, so it's hard to point out. But in ConnectionRunnable.run() you set the contentLength to the size of the form. I don't think that's actually right, because below that you'll see with multipart there could be several parts and a form.

if (parts.size() > 0 && needMultipart) {
  contentLength = 0;
  for(ContentBody body : parts.values()) {
    contentLength += body.getContentLength();
  }
  if (form != null) {
    contentLength += form.getContentLength();
  }                 
} else {
  // This code may get ugly. But basically you have to check if raw data was String, Entity, or fall back to form, see #handleURLEncodedData
  if (data instanceof String) {
    contentLength = ((String) data).length(); // This is probably wrong. We turn it into a StringEntity later and can get content length from that.
  } else if (data instanceof FileEntity) { // As far as I can tell, this is the only other valid type that raw data gets converted to
    contentLength = ((FileEntity) data).getContentLength();
  } else {
    contentLength = form.getContentLength();
  }
}

[garymathews]

@sgtcoolguy Another problem is, when calls such as addFilePart happen, the connection has already been made. Therefor I can't set the fixed length of the client to the correct length. addFilePart already starts to write to the stream. So although the content length is known, it's undetermined before the connection is made.

[sgtcoolguy]

@garymathews that's true, but that happens later in #run(). The chunk of code I gave above at the top of #run() should work OK. I know it's not the most pleasant to iterate over the parts/form at top just to get length and then again to actually set up the connection contents, but it should work.

[garymathews]

@sgtcoolguy Updated PR, let's see if the tests pass!

[garymathews]

test this please