From 0a7527b03e5ea9d56ea39862217294fd7db81f97 Mon Sep 17 00:00:00 2001 From: Marc Schlegel Date: Thu, 1 Aug 2019 06:39:47 +0200 Subject: [PATCH] [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. --- .../okhttp-gson/ApiClientTest.java | 25 ++++++++----------- .../libraries/okhttp-gson/ApiClient.mustache | 16 +++--------- .../org/openapitools/client/ApiClient.java | 16 +++--------- .../org/openapitools/client/ApiClient.java | 16 +++--------- .../openapitools/client/ApiClientTest.java | 25 ++++++++----------- 5 files changed, 29 insertions(+), 69 deletions(-) diff --git a/CI/samples.ci/client/petstore/java/test-manual/okhttp-gson/ApiClientTest.java b/CI/samples.ci/client/petstore/java/test-manual/okhttp-gson/ApiClientTest.java index 6181e8afa2d..626901593bd 100644 --- a/CI/samples.ci/client/petstore/java/test-manual/okhttp-gson/ApiClientTest.java +++ b/CI/samples.ci/client/petstore/java/test-manual/okhttp-gson/ApiClientTest.java @@ -10,6 +10,7 @@ import java.util.TimeZone; import org.junit.*; import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.*; public class ApiClientTest { @@ -329,24 +330,18 @@ public class ApiClientTest { assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif")); } - @Test - public void testInterceptorCleanupWithNewClient() { + public void testNewHttpClient() { OkHttpClient oldClient = apiClient.getHttpClient(); - assertEquals(1, oldClient.networkInterceptors().size()); - - OkHttpClient newClient = new OkHttpClient(); - apiClient.setHttpClient(newClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); - apiClient.setHttpClient(newClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); + apiClient.setHttpClient(oldClient.newBuilder().build()); + assertThat(apiClient.getHttpClient(), is(not(oldClient))); } - @Test - public void testInterceptorCleanupWithSameClient() { - OkHttpClient oldClient = apiClient.getHttpClient(); - assertEquals(1, oldClient.networkInterceptors().size()); - apiClient.setHttpClient(oldClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); + /** + * Tests the invariant that the HttpClient for the ApiClient must never be null + */ + @Test(expected = NullPointerException.class) + public void testNullHttpClient() { + apiClient.setHttpClient(null); } } diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiClient.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiClient.mustache index cd173326c10..682bf945069 100644 --- a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiClient.mustache +++ b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/ApiClient.mustache @@ -187,24 +187,14 @@ public class ApiClient { } /** - * Set HTTP client + * Set HTTP client, which must never be null. * * @param newHttpClient An instance of OkHttpClient * @return Api Client + * @throws NullPointerException when newHttpClient is null */ public ApiClient setHttpClient(OkHttpClient newHttpClient) { - if(!httpClient.equals(newHttpClient)) { - OkHttpClient.Builder builder = newHttpClient.newBuilder(); - Iterator networkInterceptorIterator = httpClient.networkInterceptors().iterator(); - while(networkInterceptorIterator.hasNext()) { - builder.addNetworkInterceptor(networkInterceptorIterator.next()); - } - Iterator interceptorIterator = httpClient.interceptors().iterator(); - while(interceptorIterator.hasNext()) { - builder.addInterceptor(interceptorIterator.next()); - } - this.httpClient = builder.build(); - } + this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!"); return this; } diff --git a/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/ApiClient.java b/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/ApiClient.java index 0934b6f1aa9..42fc1fa212a 100644 --- a/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/ApiClient.java +++ b/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/ApiClient.java @@ -170,24 +170,14 @@ public class ApiClient { } /** - * Set HTTP client + * Set HTTP client, which must never be null. * * @param newHttpClient An instance of OkHttpClient * @return Api Client + * @throws NullPointerException when newHttpClient is null */ public ApiClient setHttpClient(OkHttpClient newHttpClient) { - if(!httpClient.equals(newHttpClient)) { - OkHttpClient.Builder builder = newHttpClient.newBuilder(); - Iterator networkInterceptorIterator = httpClient.networkInterceptors().iterator(); - while(networkInterceptorIterator.hasNext()) { - builder.addNetworkInterceptor(networkInterceptorIterator.next()); - } - Iterator interceptorIterator = httpClient.interceptors().iterator(); - while(interceptorIterator.hasNext()) { - builder.addInterceptor(interceptorIterator.next()); - } - this.httpClient = builder.build(); - } + this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!"); return this; } diff --git a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/ApiClient.java b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/ApiClient.java index 0934b6f1aa9..42fc1fa212a 100644 --- a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/ApiClient.java +++ b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/ApiClient.java @@ -170,24 +170,14 @@ public class ApiClient { } /** - * Set HTTP client + * Set HTTP client, which must never be null. * * @param newHttpClient An instance of OkHttpClient * @return Api Client + * @throws NullPointerException when newHttpClient is null */ public ApiClient setHttpClient(OkHttpClient newHttpClient) { - if(!httpClient.equals(newHttpClient)) { - OkHttpClient.Builder builder = newHttpClient.newBuilder(); - Iterator networkInterceptorIterator = httpClient.networkInterceptors().iterator(); - while(networkInterceptorIterator.hasNext()) { - builder.addNetworkInterceptor(networkInterceptorIterator.next()); - } - Iterator interceptorIterator = httpClient.interceptors().iterator(); - while(interceptorIterator.hasNext()) { - builder.addInterceptor(interceptorIterator.next()); - } - this.httpClient = builder.build(); - } + this.httpClient = Objects.requireNonNull(newHttpClient, "HttpClient must not be null!"); return this; } diff --git a/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ApiClientTest.java b/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ApiClientTest.java index 6181e8afa2d..626901593bd 100644 --- a/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ApiClientTest.java +++ b/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ApiClientTest.java @@ -10,6 +10,7 @@ import java.util.TimeZone; import org.junit.*; import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.*; public class ApiClientTest { @@ -329,24 +330,18 @@ public class ApiClientTest { assertEquals("sun.gif", apiClient.sanitizeFilename(".\\sun.gif")); } - @Test - public void testInterceptorCleanupWithNewClient() { + public void testNewHttpClient() { OkHttpClient oldClient = apiClient.getHttpClient(); - assertEquals(1, oldClient.networkInterceptors().size()); - - OkHttpClient newClient = new OkHttpClient(); - apiClient.setHttpClient(newClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); - apiClient.setHttpClient(newClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); + apiClient.setHttpClient(oldClient.newBuilder().build()); + assertThat(apiClient.getHttpClient(), is(not(oldClient))); } - @Test - public void testInterceptorCleanupWithSameClient() { - OkHttpClient oldClient = apiClient.getHttpClient(); - assertEquals(1, oldClient.networkInterceptors().size()); - apiClient.setHttpClient(oldClient); - assertEquals(1, apiClient.getHttpClient().networkInterceptors().size()); + /** + * Tests the invariant that the HttpClient for the ApiClient must never be null + */ + @Test(expected = NullPointerException.class) + public void testNullHttpClient() { + apiClient.setHttpClient(null); } }