[java-client][okhttp-gson] fixes for interceptors in ApiClient.java (#3502)

ApiClient for Okhttp3 must not copy the interceptors when setting the HttpClient.
Enforce invariant that the HttpClient must never be null.
This commit is contained in:
Marc Schlegel 2019-08-01 06:39:47 +02:00 committed by Jérémie Bresson
parent bcc3a9ecf7
commit 0a7527b03e
5 changed files with 29 additions and 69 deletions

View File

@ -10,6 +10,7 @@ import java.util.TimeZone;
import org.junit.*; import org.junit.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.*;
public class ApiClientTest { public class ApiClientTest {
@ -329,24 +330,18 @@ public class ApiClientTest {
assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif")); assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif"));
} }
@Test @Test
public void testInterceptorCleanupWithNewClient() { public void testNewHttpClient() {
OkHttpClient oldClient = apiClient.getHttpClient(); OkHttpClient oldClient = apiClient.getHttpClient();
assertEquals(1, oldClient.networkInterceptors().size()); apiClient.setHttpClient(oldClient.newBuilder().build());
assertThat(apiClient.getHttpClient(), is(not(oldClient)));
OkHttpClient newClient = new OkHttpClient();
apiClient.setHttpClient(newClient);
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size());
apiClient.setHttpClient(newClient);
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size());
} }
@Test /**
public void testInterceptorCleanupWithSameClient() { * Tests the invariant that the HttpClient for the ApiClient must never be null
OkHttpClient oldClient = apiClient.getHttpClient(); */
assertEquals(1, oldClient.networkInterceptors().size()); @Test(expected = NullPointerException.class)
apiClient.setHttpClient(oldClient); public void testNullHttpClient() {
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); apiClient.setHttpClient(null);
} }
} }

View File

@ -187,24 +187,14 @@ public class ApiClient {
} }
/** /**
* Set HTTP client * Set HTTP client, which must never be null.
* *
* @param newHttpClient An instance of OkHttpClient * @param newHttpClient An instance of OkHttpClient
* @return Api Client * @return Api Client
* @throws NullPointerException when newHttpClient is null
*/ */
public ApiClient setHttpClient(OkHttpClient newHttpClient) { public ApiClient setHttpClient(OkHttpClient newHttpClient) {
if(!httpClient.equals(newHttpClient)) { this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!");
OkHttpClient.Builder builder = newHttpClient.newBuilder();
Iterator<Interceptor> networkInterceptorIterator = httpClient.networkInterceptors().iterator();
while(networkInterceptorIterator.hasNext()) {
builder.addNetworkInterceptor(networkInterceptorIterator.next());
}
Iterator<Interceptor> interceptorIterator = httpClient.interceptors().iterator();
while(interceptorIterator.hasNext()) {
builder.addInterceptor(interceptorIterator.next());
}
this.httpClient = builder.build();
}
return this; return this;
} }

View File

@ -170,24 +170,14 @@ public class ApiClient {
} }
/** /**
* Set HTTP client * Set HTTP client, which must never be null.
* *
* @param newHttpClient An instance of OkHttpClient * @param newHttpClient An instance of OkHttpClient
* @return Api Client * @return Api Client
* @throws NullPointerException when newHttpClient is null
*/ */
public ApiClient setHttpClient(OkHttpClient newHttpClient) { public ApiClient setHttpClient(OkHttpClient newHttpClient) {
if(!httpClient.equals(newHttpClient)) { this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!");
OkHttpClient.Builder builder = newHttpClient.newBuilder();
Iterator<Interceptor> networkInterceptorIterator = httpClient.networkInterceptors().iterator();
while(networkInterceptorIterator.hasNext()) {
builder.addNetworkInterceptor(networkInterceptorIterator.next());
}
Iterator<Interceptor> interceptorIterator = httpClient.interceptors().iterator();
while(interceptorIterator.hasNext()) {
builder.addInterceptor(interceptorIterator.next());
}
this.httpClient = builder.build();
}
return this; return this;
} }

View File

@ -170,24 +170,14 @@ public class ApiClient {
} }
/** /**
* Set HTTP client * Set HTTP client, which must never be null.
* *
* @param newHttpClient An instance of OkHttpClient * @param newHttpClient An instance of OkHttpClient
* @return Api Client * @return Api Client
* @throws NullPointerException when newHttpClient is null
*/ */
public ApiClient setHttpClient(OkHttpClient newHttpClient) { public ApiClient setHttpClient(OkHttpClient newHttpClient) {
if(!httpClient.equals(newHttpClient)) { this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!");
OkHttpClient.Builder builder = newHttpClient.newBuilder();
Iterator<Interceptor> networkInterceptorIterator = httpClient.networkInterceptors().iterator();
while(networkInterceptorIterator.hasNext()) {
builder.addNetworkInterceptor(networkInterceptorIterator.next());
}
Iterator<Interceptor> interceptorIterator = httpClient.interceptors().iterator();
while(interceptorIterator.hasNext()) {
builder.addInterceptor(interceptorIterator.next());
}
this.httpClient = builder.build();
}
return this; return this;
} }

View File

@ -10,6 +10,7 @@ import java.util.TimeZone;
import org.junit.*; import org.junit.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.*;
public class ApiClientTest { public class ApiClientTest {
@ -329,24 +330,18 @@ public class ApiClientTest {
assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif")); assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif"));
} }
@Test @Test
public void testInterceptorCleanupWithNewClient() { public void testNewHttpClient() {
OkHttpClient oldClient = apiClient.getHttpClient(); OkHttpClient oldClient = apiClient.getHttpClient();
assertEquals(1, oldClient.networkInterceptors().size()); apiClient.setHttpClient(oldClient.newBuilder().build());
assertThat(apiClient.getHttpClient(), is(not(oldClient)));
OkHttpClient newClient = new OkHttpClient();
apiClient.setHttpClient(newClient);
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size());
apiClient.setHttpClient(newClient);
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size());
} }
@Test /**
public void testInterceptorCleanupWithSameClient() { * Tests the invariant that the HttpClient for the ApiClient must never be null
OkHttpClient oldClient = apiClient.getHttpClient(); */
assertEquals(1, oldClient.networkInterceptors().size()); @Test(expected = NullPointerException.class)
apiClient.setHttpClient(oldClient); public void testNullHttpClient() {
assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); apiClient.setHttpClient(null);
} }
} }