Add support for fixed ContentLength

[gbonnefille]

Some servers do not support chunked message. Sending whole part in a single message would help to interact with such servers.

[Acconut]

Can you elaborate more about what the problem is and how this PR fixes it?

[gbonnefille]

For what I understood, the previous implementation of tus-java-client is based on chunked transfer encoding and the server I tried to use seems to not support this encoding.

Effect: the server considers the client sent an empty message.

The fix consists in adding an upload method. This method disable the chunked transfer, and send a message of size requestPayloadSize in a single run.

I tested it yesterday and it gives full satisfaction.

[foxware00]

Is there any plans to merge this, we could benefit from this change. It'll also close off #30

@Acconut this would be really useful for us as it's currently preventing us from using TUS in our Android setting

[EverydayPineapple]

I'd also like to know on plans for merging this @Acconut

[Acconut]

@EverydayPineapple @foxware00 Thanks for reminding me about this. There are no plans for merging it but I got some more feedback we need to look into.

[foxware00]

@EverydayPineapple @foxware00 Thanks for reminding me about this. There are no plans for merging it but I got some more feedback we need to look into.

@Acconut thank you. This is critical for me, for the usage of Tus. A present we're running this branch internally until it's upstreamed. Many thanks

[Acconut]

Let's see if @gbonnefille is still interested in this PR. Otherwise someone else would have to pick it up since I don't have time to work on it directly. If you are interested in doing so, let me know :)

[foxware00]

Let's see if @gbonnefille is still interested in this PR. Otherwise someone else would have to pick it up since I don't have time to work on it directly. If you are interested in doing so, let me know :)

Sure if @gbonnefille isn't happy to do it. I would be happy to apply your changes

[gbonnefille]

I'm still interested on it, but spare time is lacking now. I hope to do something on this during the week.

[Acconut]

Perfect, let me know if you need help.

[dmytrohridin]

Hey guys Any progress on this, need some help maybe? cc @Acconut

[Acconut]

I think there are still some open comments from me that would need to be addressed before we can merge. If there is still some interest in getting this merged, I am happy to review the work! In the future I hope to replace the HTTPUrlConnection with a more flexible HTTP Client (maybe OkHttp or something), which would make this code not necessary anymore.

[foxware00]

@Acconut I was picking this back up as I've noticed an issue in this logic around chunking/payload size. If I understand your open question correctly. Is it that payload size == chunk size when chunked transfer is disabled?

This seems to make sense to me, I'm interested in a way to solve it, but one way is removing the extra code to close the connection and force chunk/payload to be the smaller of the two when chunkedTransferEncoding is disabled.

In doing so, you negate the need for the additional code closing the connection and the logic within uploadChunk remains identical for both paths.

My question to you is how this fits in the the library, I'm happy to make the change. I feel we just need to document logic around chunk/payload being only valid in chunkedEncodingMode.

Such as below

if (!client.chunkedTransferEncoding()) {
    if (getChunkSize() < getRequestPayloadSize()) {
        setRequestPayloadSize(getChunkSize());
    } else {
        setChunkSize(getRequestPayloadSize());
    }
}

Another option is to ignore the chunkSize when chunkedEncoding is disabled and fallback to payload size. That feels a bit more obvious to the user of the library.

Then, getChunkSize becomes the following.

/**
 * Returns the current chunk size set using {@link #setChunkSize(int)}. 
 * If chunked transfer encoding is disabled, the payload size is returned instead
 *
 * @return Current chunk size
 */
public int getChunkSize() {
    if (client.chunkedTransferEncoding()) {
        return buffer.length;    
    }
    return requestPayloadSize;
}

[gbonnefille]

@Acconut I was picking this back up as I've noticed an issue in this logic around chunking/payload size. If I understand your open question correctly. Is it that payload size == chunk size when chunked transfer is disabled?

This seems to make sense to me, I'm interested in a way to solve it, but one way is removing the extra code to close the connection and force chunk/payload to be the smaller of the two when chunkedTransferEncoding is disabled.

In doing so, you negate the need for the additional code closing the connection and the logic within uploadChunk remains identical for both paths.

My question to you is how this fits in the the library, I'm happy to make the change. I feel we just need to document logic around chunk/payload being only valid in chunkedEncodingMode.

Such as below

if (!client.chunkedTransferEncoding()) {
    if (getChunkSize() < getRequestPayloadSize()) {
        setRequestPayloadSize(getChunkSize());
    } else {
        setChunkSize(getRequestPayloadSize());
    }
}

Another option is to ignore the chunkSize when chunkedEncoding is disabled and fallback to payload size. That feels a bit more obvious to the user of the library.

Then, getChunkSize becomes the following.

/**
 * Returns the current chunk size set using {@link #setChunkSize(int)}. 
 * If chunked transfer encoding is disabled, the payload size is returned instead
 *
 * @return Current chunk size
 */
public int getChunkSize() {
    if (client.chunkedTransferEncoding()) {
        return buffer.length;    
    }
    return requestPayloadSize;
}

See https://github.com/tus/tus-java-client/pull/32#pullrequestreview-404176924 IMHO, chunkSize < payloadSize and @Acconut stated this is because chunkSize is the max size supported by client due to memory management.