is RequestManager.updateRequestOptions() need add autoClone() ?

[summer-zhoujie]

trafficstars

First of all,I'm very sorry, my English is very poor, so I may need you to spend some effort to understand me. Source Code From: github the branch master ClassName: RequestManager Method: updateRequestOptions() the code block

@GuardedBy("this")
private RequestOptions requestOptions;

synchronized RequestOptions getDefaultRequestOptions() {
    return requestOptions;
}

protected synchronized void setRequestOptions(@NonNull RequestOptions toSet) {
    requestOptions = toSet.clone().autoClone();
}

private synchronized void updateRequestOptions(@NonNull RequestOptions toUpdate) {
  requestOptions = requestOptions.apply(toUpdate);
}

why toSet.clone().autoClone() need autoClone() I guess it's to prevent the requestOptions returned by getDefaultRequestOptions() from being tampered with by external parties. I am not sure whether my guess is correct or not. Assuming that it is correct.

then, when the user call updateRequestOptions() once, now requestOptions 's isAutoCloneEnabled is false,

At this point, when a user obtains a requestOptions instance through the getDefaultRequestOptions() function, they will directly modify the RequestManager 's requestOptions property. As a result, the protection formed by the setRequestOptions() function on the requestOptions is no longer present.

Final, Should we do this? Add an autoClone() function at the end of updateRequestOptions().

private synchronized void updateRequestOptions(@NonNull RequestOptions toUpdate) {
  requestOptions = requestOptions.apply(toUpdate).autoClone();
}

Please help me answer my confusion, thank you very much!

[summer-zhoujie]

@sjudd

[sjudd]

I believe this is safe because we're not replacing the request options, we're applying a new set of options. So if autoClone() is already set on this.requestOptions, then we don't need to append autoClone() here.