- fixes retry/redirect options customization

[Asky-GH]

Fixes #181

Changes proposed in this pull request

• Change conditions used to evaluate Request level middleware options

[ghost]

All CLA requirements met.

[baywet]

thank you for you contribution @Asky-GH ! Some minor comments from my side. Also, can you make sure you sign the CLA please?

[baywet]

@MIchaelMainer or @zengin can you take a look as well please?

[zengin]

Is this required for giving the flexibility to custom handler implementations?

In the default implementation of RetryHandler and RedirectHandler, the constructors end up calling the default option constructors if they are null. Default constructors of options are already using these default values. This piece of code seems equivalent to:

Suggested change

	final RedirectOptions redirectOptions =
	request.getMaxRedirects() == DEFAULT_MAX_REDIRECTS && request.getShouldRedirect().equals(DEFAULT_SHOULD_REDIRECT)
	? null
	: new RedirectOptions(request.getMaxRedirects(), request.getShouldRedirect());
	final RetryOptions retryOptions =
	request.getMaxRetries() == DEFAULT_MAX_RETRIES && request.getDelay() == DEFAULT_DELAY && request.getShouldRetry().equals(DEFAULT_SHOULD_RETRY)
	? null
	: new RetryOptions(request.getShouldRetry(), request.getMaxRetries(), request.getDelay());
	final RedirectOptions redirectOptions = new RedirectOptions(request.getMaxRedirects(), request.getShouldRedirect());
	final RetryOptions retryOptions = new RetryOptions(request.getShouldRetry(), request.getMaxRetries(), request.getDelay());

[Asky-GH]

Hi @zengin
The changes you are suggesting work effectively the same as the current code of the library. If you take a look at CoreHttpProvider's line 245:

if(redirectOptions != null) {
	corehttpRequestBuilder = corehttpRequestBuilder.tag(RedirectOptions.class, redirectOptions);
}
if(retryOptions != null) {
	corehttpRequestBuilder = corehttpRequestBuilder.tag(RetryOptions.class, retryOptions);
}

RedirectHandler's line 133:

RedirectOptions redirectOptions = request.tag(RedirectOptions.class);
redirectOptions = redirectOptions != null ? redirectOptions : this.mRedirectOptions;

and RetryHandler's line 179:

RetryOptions retryOption = request.tag(RetryOptions.class);
retryOption = retryOption != null ? retryOption : mRetryOption;

you can see that mRedirectOptions and mRetryOption will never be used. Thus it is impossible to customize them so to speak globaly.

[zengin]

Thanks for clarifying.

[baywet]

@Asky-GH Thanks for your contribution, I just added more unit testing to your PR to make sure we know if we break things again in the future.
@zengin thanks for the review!