Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Remove redundant use of HttpClient #378

Closed
theScrabi opened this issue Mar 10, 2021 · 1 comment
Closed

Remove redundant use of HttpClient #378

theScrabi opened this issue Mar 10, 2021 · 1 comment
Assignees

Comments

@theScrabi
Copy link
Contributor

theScrabi commented Mar 10, 2021

With the changes done by removing the ok http singleton I also introduced a redundend use of the HttpClient wrapper.

The HttpClient gets introduced to the HttpMethod through the constructor:

abstract class HttpBaseMethod constructor(clientWrapper: HttpClient, url: URL) {
var okHttpClient: OkHttpClient

However the execute() method of the HttpMethod is later called from the OwnCloudClient which extends the HttpClient wrapper:

public int executeHttpMethod(HttpBaseMethod method) throws Exception {
boolean repeatWithFreshCredentials;
int repeatCounter = 0;
int status;
do {
String requestId = RandomUtils.generateRandomUUID();
// Header to allow tracing requests in apache and ownCloud logs
Timber.d("Executing in request with id %s", requestId);
method.setRequestHeader(HttpConstants.OC_X_REQUEST_ID, requestId);
method.setRequestHeader(HttpConstants.USER_AGENT_HEADER, SingleSessionManager.getUserAgent());
method.setRequestHeader(HttpConstants.ACCEPT_ENCODING_HEADER, HttpConstants.ACCEPT_ENCODING_IDENTITY);
if (mCredentials.getHeaderAuth() != null && method.getRequestHeader(AUTHORIZATION_HEADER) == null) {
method.setRequestHeader(AUTHORIZATION_HEADER, mCredentials.getHeaderAuth());
}
status = method.execute();

Getting "in touch" twice with the client is redundant and should be removed. Here are two possible solutions:

1. Try to Inject the client from the constructor:

The HttpClient modifying methods in the `HttpBaseMethod` do not need to be changed: https://github.com/owncloud/android-library/blob/9d80a95a884715f9d71aa16f86bd36d0d50bce14/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/methods/HttpBaseMethod.kt#L110

But the code calling the method.execute() in the OwnCloudClient has to be moved to the HttpMethod:

public int executeHttpMethod(HttpBaseMethod method) throws Exception {

2. Try to inject the client from the execute method:

The HttpClient modifying methods in the `HttpBaseMethod` need to be modified: https://github.com/owncloud/android-library/blob/9d80a95a884715f9d71aa16f86bd36d0d50bce14/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/methods/HttpBaseMethod.kt#L110

Instead of modifying the OkHttp client directly we could save the modifications and apply them when calling the execute(client) method.
The only change that need to be applied to OwnCloudClient would be that this is suplied the the execution() method.


Issue: #378
Library: #427
App: owncloud/android#3353

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 8, 2022

Done

@jesmrec jesmrec closed this as completed Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants