fix(android): fix onprogress payload value for data other than HashMap

[ypbnv]

JIRA: https://jira.appcelerator.org/browse/TIMOB-27293

Description:
Request's length was not accounted for data of type different from HashMap, thus not reporting the
progress properly in the callback. Also there was a duplication in calculating a request's data size, so I removed the redundant variable. I believe all the cases are covered from the code that is left.
Working on a dedicated unit test for this.

Edit: Added the unit test.

Test case:
The JIRA ticket has a good code sample for testing.

[build]

	Messages
📖	💾 Here's the generated SDK zipfile.
📖	✅ All tests are passing Nice one! All 4405 tests are passing. (There are 479 skipped tests not included in that total)
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.

Generated by 🚫 dangerJS against b5ef204

[garymathews]

CR: PASS

[jquick-axway]

@ypbnv, we should check if content length is zero. I can see this potentially happening with an HTTP "PUT" without a body.

public void progress(int progress)
{
	if (contentLength <= 0) {
		return;
	}

	// The rest of the code goes here...
}

[ypbnv]

Done.

[ssekhri]

FR Passed.
Verified on:
Mac OS 10.14.6
Ti SDK: 8.3.0.v20190916012915
Appc CLI: 7.1.1
Node: 10.5.0
JDK: 10.0.2
Studio: 5.1.4.201909061933
Device: Nexus4 (v5.1.1), Pixel2 emulator (8.0.0)

[jquick-axway]

@ypbnv, can you look into the unit test failure please? That's the only thing blocking this PR. Thanks!

[garymathews]

Looks like this has broke callbackTestForPOSTMethod test

[ypbnv]

After some more research - callbackTestForPOSTMethod should serve the same purpose of the unit test added by this PR. It should check the progress callback for sending data through HTTPRequest.

It fails because the test is successful when the progress event callback returns 1. This happened before, because it was calculated using the removed totalLength variable. It contained the length only of the data in the passed dictionary. In the case of the unit test that was a buffer with 10*1024 which has a 512 factor (this is how we track progress - with chunks of 512 bytes), so the last 512 bytes triggered the event with a progress 1. I think this would not be a reliable test because any content size different from X times of 512 bytes will never return progress value of 1. I am not really sure if this is the case for iOS, though.

Apparently on iOS you can get the same progress value multiple times, so this is why the latest unit test fails there. It is expecting to get gradually increasing values from the event between 0 and 1. I am not really sure if this is a good way to report progress though.

This ends up with the question - should we expect the progress event to always end up with 1? It makes sense that a 100% is part of the progress change, but for now it is not always happening in Android. One way would be to adjust the native code to always end up with 1 or we should change the unit test to not expect 1 for a successful run. I suppose it is a matter of parity with iOS.
@jquick-axway @garymathews @vijaysingh-axway

[jquick-axway]

Yes, the progress should be 1.0 (aka: 100%) when the upload finishes successfully. That is a reasonable expectation. This problem needs to be solved.

[jquick-axway]

@ypbnv, add the following code to the ProgressOutputStream class. This will solve the problem. I tested it out and it does the job.

private class ProgressOutputStream extends FilterOutputStream
{
	// Existing code is here...

	@Override
	public void close() throws IOException
	{
		super.close();

		if (!aborted && (transferred > 0)) {
			lastTransferred = transferred;
			listener.progress(transferred);
		}
	}
}

Ideally, the progress handling should be redesigned, but I can live with this.

[ypbnv]

@jquick-axway Added the snippet and also moved the closing of PrintWriter to happen for request that do not need multipart too.

[jquick-axway]

CR: Pass