From f4ac2df116ad9deb34753ddfb669e2e3499f66ee Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 15 Feb 2021 18:13:25 +0900 Subject: [PATCH 1/5] Migrate HTTP client to okhttp. --- build.gradle | 2 +- .../MockedResponseErrorHandlingSpec.groovy | 6 +- .../functional/e2e/ExploratoryE2ESpec.groovy | 4 +- .../gradle/nexus/BaseStagingTask.groovy | 11 ++- .../infra/NexusHttpResponseException.groovy | 6 +- .../infra/SimplifiedHttpJsonRestClient.groovy | 96 +++++++++---------- .../logic/RepositoryStateFetcherSpec.groovy | 2 +- 7 files changed, 65 insertions(+), 62 deletions(-) diff --git a/build.gradle b/build.gradle index 76efa1f..d2cffc9 100644 --- a/build.gradle +++ b/build.gradle @@ -36,7 +36,7 @@ sourceSets { dependencies { compile gradleApi() compile localGroovy() - compile 'org.codehaus.groovy.modules.http-builder:http-builder:0.7.1' + compile 'com.squareup.okhttp3:okhttp:4.9.1' testCompile('org.spockframework:spock-core:1.3-groovy-2.5') { //groovy 2.3.x or 2.4.x is already provided by Gradle itself diff --git a/src/funcTest/groovy/io/codearte/gradle/nexus/functional/MockedResponseErrorHandlingSpec.groovy b/src/funcTest/groovy/io/codearte/gradle/nexus/functional/MockedResponseErrorHandlingSpec.groovy index 584a82a..5c2a868 100644 --- a/src/funcTest/groovy/io/codearte/gradle/nexus/functional/MockedResponseErrorHandlingSpec.groovy +++ b/src/funcTest/groovy/io/codearte/gradle/nexus/functional/MockedResponseErrorHandlingSpec.groovy @@ -2,11 +2,10 @@ package io.codearte.gradle.nexus.functional import com.github.tomakehurst.wiremock.junit.WireMockRule import groovy.transform.NotYetImplemented -import groovyx.net.http.HttpResponseException -import groovyx.net.http.RESTClient import io.codearte.gradle.nexus.infra.NexusHttpResponseException import io.codearte.gradle.nexus.infra.SimplifiedHttpJsonRestClient import io.codearte.gradle.nexus.logic.RepositoryCloser +import okhttp3.OkHttpClient import org.junit.Rule import spock.lang.Specification @@ -40,7 +39,7 @@ class MockedResponseErrorHandlingSpec extends Specification { def "should present response body on 500 server error"() { given: - SimplifiedHttpJsonRestClient client = new SimplifiedHttpJsonRestClient(new RESTClient(), TEST_MOCKED_USERNAME, TEST_MOCKED_PASSWORD) + SimplifiedHttpJsonRestClient client = new SimplifiedHttpJsonRestClient(new OkHttpClient(), TEST_MOCKED_USERNAME, TEST_MOCKED_PASSWORD) RepositoryCloser closer = new RepositoryCloser(client, getMockedUrl(), TEST_MOCKED_REPOSITORY_DESCRIPTION) and: stubFor(post(urlEqualTo("/staging/bulk/close")) @@ -56,7 +55,6 @@ class MockedResponseErrorHandlingSpec extends Specification { NexusHttpResponseException e = thrown() e.statusCode == 500 e.message.contains("Missing staging repository: $TEST_MOCKED_NOT_EXISTING_REPOSITORY_ID") - e.cause instanceof HttpResponseException } private String getMockedUrl() { diff --git a/src/funcTest/groovy/io/codearte/gradle/nexus/functional/e2e/ExploratoryE2ESpec.groovy b/src/funcTest/groovy/io/codearte/gradle/nexus/functional/e2e/ExploratoryE2ESpec.groovy index 4344774..4015027 100644 --- a/src/funcTest/groovy/io/codearte/gradle/nexus/functional/e2e/ExploratoryE2ESpec.groovy +++ b/src/funcTest/groovy/io/codearte/gradle/nexus/functional/e2e/ExploratoryE2ESpec.groovy @@ -1,7 +1,6 @@ package io.codearte.gradle.nexus.functional.e2e import groovy.transform.NotYetImplemented -import groovyx.net.http.RESTClient import io.codearte.gradle.nexus.functional.BaseNexusStagingFunctionalSpec import io.codearte.gradle.nexus.infra.SimplifiedHttpJsonRestClient import io.codearte.gradle.nexus.logic.OperationRetrier @@ -15,6 +14,7 @@ import io.codearte.gradle.nexus.logic.RepositoryStateFetcher import io.codearte.gradle.nexus.logic.RetryingRepositoryTransitioner import io.codearte.gradle.nexus.logic.StagingProfileFetcher import nebula.test.functional.ExecutionResult +import okhttp3.OkHttpClient import spock.lang.Ignore import spock.lang.Stepwise @@ -29,7 +29,7 @@ class ExploratoryE2ESpec extends BaseNexusStagingFunctionalSpec implements E2ESp private static String resolvedStagingRepositoryId def setup() { - client = new SimplifiedHttpJsonRestClient(new RESTClient(), nexusUsernameAT, nexusPasswordAT) + client = new SimplifiedHttpJsonRestClient(new OkHttpClient(), nexusUsernameAT, nexusPasswordAT) repoStateFetcher = new RepositoryStateFetcher(client, E2E_SERVER_BASE_PATH) retrier = new OperationRetrier<>() } diff --git a/src/main/groovy/io/codearte/gradle/nexus/BaseStagingTask.groovy b/src/main/groovy/io/codearte/gradle/nexus/BaseStagingTask.groovy index 575a3aa..f8b42f8 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/BaseStagingTask.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/BaseStagingTask.groovy @@ -2,7 +2,6 @@ package io.codearte.gradle.nexus import groovy.transform.CompileStatic import groovy.transform.PackageScope -import groovyx.net.http.RESTClient import io.codearte.gradle.nexus.infra.SimplifiedHttpJsonRestClient import io.codearte.gradle.nexus.logic.OperationRetrier import io.codearte.gradle.nexus.logic.RepositoryCloser @@ -11,6 +10,7 @@ import io.codearte.gradle.nexus.logic.RepositoryReleaser import io.codearte.gradle.nexus.logic.RepositoryState import io.codearte.gradle.nexus.logic.RepositoryStateFetcher import io.codearte.gradle.nexus.logic.StagingProfileFetcher +import okhttp3.OkHttpClient import org.gradle.api.DefaultTask import org.gradle.api.Incubating import org.gradle.api.Project @@ -57,6 +57,8 @@ abstract class BaseStagingTask extends DefaultTask { @Incubating final Property stagingRepositoryId + private SimplifiedHttpJsonRestClient restClient + @Inject BaseStagingTask(Project project, NexusStagingExtension extension) { ObjectFactory objectFactory = project.getObjects(); @@ -65,8 +67,11 @@ abstract class BaseStagingTask extends DefaultTask { } @PackageScope - SimplifiedHttpJsonRestClient createClient() { - return new SimplifiedHttpJsonRestClient(new RESTClient(), getUsername(), getPassword()) + synchronized SimplifiedHttpJsonRestClient createClient() { + if (restClient == null) { + restClient = new SimplifiedHttpJsonRestClient(new OkHttpClient(), getUsername(), getPassword()) + } + return restClient } protected StagingProfileFetcher createProfileFetcherWithGivenClient(SimplifiedHttpJsonRestClient client) { diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/NexusHttpResponseException.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/NexusHttpResponseException.groovy index 02e6f88..0fc22e4 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/NexusHttpResponseException.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/NexusHttpResponseException.groovy @@ -5,7 +5,7 @@ import groovy.transform.CompileStatic /** * Custom exception to propagate server errors. * - * Created as groovyx.net.http.HttpResponseException contains in a message only a reason phrase (e.g. Server Error) without response body + * Created as OkHttp does not throw exceptions for HTTP errors (e.g. Server Error) * which in many cases is crucial to determine the resons why error was returned. * * It may be made redundant once migrated to other HTTP library. @@ -15,8 +15,8 @@ class NexusHttpResponseException extends NexusStagingException { final int statusCode - NexusHttpResponseException(int statusCode, String message, Throwable cause) { - super(message, cause) + NexusHttpResponseException(int statusCode, String message) { + super(message) this.statusCode = statusCode } } diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy index 55ad18b..5c1e9f9 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy @@ -1,77 +1,77 @@ package io.codearte.gradle.nexus.infra +import groovy.json.JsonOutput +import groovy.json.JsonSlurper import groovy.transform.CompileStatic import groovy.util.logging.Slf4j -import groovyx.net.http.ContentType -import groovyx.net.http.HttpResponseDecorator -import groovyx.net.http.HttpResponseException -import groovyx.net.http.RESTClient +import okhttp3.Credentials +import okhttp3.Interceptor +import okhttp3.MediaType +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody +import okhttp3.Response +import org.jetbrains.annotations.NotNull /** * Specialized REST client to communicate with Nexus. - * - * Note: The same as RESTClient from HTTP Builder this class is not thread-safe. */ @CompileStatic @Slf4j class SimplifiedHttpJsonRestClient { + private static final MediaType JSON = MediaType.parse("application/json") + private enum RequestType { GET, POST } - private final RESTClient restClient - private final String username - private final String password + private final OkHttpClient restClient + + SimplifiedHttpJsonRestClient(OkHttpClient restClient, String username, String password) { + OkHttpClient.Builder clientBuilder = restClient.newBuilder() + if (username != null) { + clientBuilder.addInterceptor(new Interceptor() { + @Override + Response intercept(@NotNull Interceptor.Chain chain) throws IOException { + Request.Builder request = chain.request().newBuilder() + .addHeader("Authorization", Credentials.basic(username, password)) - SimplifiedHttpJsonRestClient(RESTClient restClient, String username, String password) { - this.restClient = restClient - this.username = username - this.password = password -// params.requestContentType = ContentType.JSON //Does not set Content-Type header as required by WireMock - restClient.headers["Content-Type"] = "application/json" //reported as error in Idea (effectively works fine) -// restClient.headers.put("Content-Type", "application/json") + return chain.proceed(request.build()) + } + }) + } + this.restClient = clientBuilder.build() } Map get(String uri) { - return (Map) sendRequestHandlingErrors(uri, null, restClient.&get, RequestType.GET).data + Request request = new Request.Builder() + .url(uri) + .build() + return sendRequestHandlingErrors(request, RequestType.GET) } Map post(String uri, Map content) { - return (Map) sendRequestHandlingErrors(uri, content, restClient.&post, RequestType.POST).data + Request request = new Request.Builder() + .method("POST", RequestBody.create(JsonOutput.toJson(content), JSON)) + .url(uri) + .build() + return sendRequestHandlingErrors(request, RequestType.POST) } - private HttpResponseDecorator sendRequestHandlingErrors(String uri, Map content, Closure clientMethodHandler, RequestType requestTypeName) { + private Map sendRequestHandlingErrors(Request request, RequestType requestTypeName) { try { - return prepareAndSendRequest(uri, content, clientMethodHandler, requestTypeName) - } catch (HttpResponseException e) { - HttpResponseDecorator resp = e.getResponse(); - String message = "${resp.statusLine.statusCode}: ${resp.statusLine.reasonPhrase}, body: ${resp.data ?: ''}" - //TODO: Suppress error message on 404 if waiting for drop? - log.warn("$requestTypeName request failed. ${message}") - throw new NexusHttpResponseException(e.getStatusCode(), message, e) - } - } - - private HttpResponseDecorator prepareAndSendRequest(String uri, Map content, Closure clientMethodHandler, RequestType requestType) { - setUriAndAuthentication(uri) - Map params = createAndInitializeCallParametersMap() - if (content != null) { - params.body = content - log.debug("$requestType request content: $content") + return restClient.newCall(request).execute().withCloseable { + if (!it.successful) { + String message = "${it.code()}: ${it.message()}, body: ${it.body().string() ?: ''}" + //TODO: Suppress error message on 404 if waiting for drop? + log.warn("$requestTypeName request failed. ${message}") + throw new NexusHttpResponseException(it.code(), message) + } + (Map) new JsonSlurper().parse(it.body().byteStream()) + } + } catch(IOException e) { + throw new NexusStagingException("Could not connect to Nexus.", e) } - log.debug("$requestType request URL: $uri") - return (HttpResponseDecorator) clientMethodHandler(params) - } - - private void setUriAndAuthentication(String uri) { - restClient.uri = uri - if (username != null) { - restClient.auth.basic(username, password) //has to be after URI is set - } - } - - private Map createAndInitializeCallParametersMap() { //New for every call - it is cleared up after call by RESTClient - return [contentType: ContentType.JSON] } } diff --git a/src/test/groovy/io/codearte/gradle/nexus/logic/RepositoryStateFetcherSpec.groovy b/src/test/groovy/io/codearte/gradle/nexus/logic/RepositoryStateFetcherSpec.groovy index f1cb1ce..3260b99 100644 --- a/src/test/groovy/io/codearte/gradle/nexus/logic/RepositoryStateFetcherSpec.groovy +++ b/src/test/groovy/io/codearte/gradle/nexus/logic/RepositoryStateFetcherSpec.groovy @@ -42,7 +42,7 @@ class RepositoryStateFetcherSpec extends BaseOperationExecutorSpec implements Fe def "should map 404 error from server to NOT_FOUND state"() { given: client.get(getGetRepositoryStateFullUrlForRepoId(TEST_REPOSITORY_ID)) >> { - throw new NexusHttpResponseException(404, "404: Not Found, body: ${aNotFoundRepo(TEST_REPOSITORY_ID)}", null) + throw new NexusHttpResponseException(404, "404: Not Found, body: ${aNotFoundRepo(TEST_REPOSITORY_ID)}") } when: RepositoryState repoState = repoStateFetcher.getNonTransitioningRepositoryStateById(TEST_REPOSITORY_ID) From 1b59d6a87b9a097079968aa951f501c04864db54 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 16 Feb 2021 09:47:49 +0900 Subject: [PATCH 2/5] Password null check --- .../gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy index 5c1e9f9..465a71e 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy @@ -30,7 +30,7 @@ class SimplifiedHttpJsonRestClient { SimplifiedHttpJsonRestClient(OkHttpClient restClient, String username, String password) { OkHttpClient.Builder clientBuilder = restClient.newBuilder() - if (username != null) { + if (username != null && password != null) { clientBuilder.addInterceptor(new Interceptor() { @Override Response intercept(@NotNull Interceptor.Chain chain) throws IOException { From bfa84f25c29fecd491fe3739bfecd8a4b45624bc Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 16 Feb 2021 17:51:22 +0900 Subject: [PATCH 3/5] Fix headers and empty response parsing --- .../infra/SimplifiedHttpJsonRestClient.groovy | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy index 465a71e..81cb23b 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy @@ -6,7 +6,6 @@ import groovy.transform.CompileStatic import groovy.util.logging.Slf4j import okhttp3.Credentials import okhttp3.Interceptor -import okhttp3.MediaType import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody @@ -20,8 +19,6 @@ import org.jetbrains.annotations.NotNull @Slf4j class SimplifiedHttpJsonRestClient { - private static final MediaType JSON = MediaType.parse("application/json") - private enum RequestType { GET, POST } @@ -30,17 +27,20 @@ class SimplifiedHttpJsonRestClient { SimplifiedHttpJsonRestClient(OkHttpClient restClient, String username, String password) { OkHttpClient.Builder clientBuilder = restClient.newBuilder() - if (username != null && password != null) { - clientBuilder.addInterceptor(new Interceptor() { - @Override - Response intercept(@NotNull Interceptor.Chain chain) throws IOException { - Request.Builder request = chain.request().newBuilder() - .addHeader("Authorization", Credentials.basic(username, password)) + clientBuilder.addInterceptor(new Interceptor() { + @Override + Response intercept(@NotNull Interceptor.Chain chain) throws IOException { + Request.Builder request = chain.request().newBuilder() + .header("Content-Type", "application/json") + .header("Accept", "application/json") - return chain.proceed(request.build()) + if (username != null && password != null) { + request.header("Authorization", Credentials.basic(username, password)) } - }) - } + + return chain.proceed(request.build()) + } + }) this.restClient = clientBuilder.build() } @@ -53,7 +53,7 @@ class SimplifiedHttpJsonRestClient { Map post(String uri, Map content) { Request request = new Request.Builder() - .method("POST", RequestBody.create(JsonOutput.toJson(content), JSON)) + .method("POST", RequestBody.create(JsonOutput.toJson(content), null)) .url(uri) .build() return sendRequestHandlingErrors(request, RequestType.POST) @@ -68,7 +68,11 @@ class SimplifiedHttpJsonRestClient { log.warn("$requestTypeName request failed. ${message}") throw new NexusHttpResponseException(it.code(), message) } - (Map) new JsonSlurper().parse(it.body().byteStream()) + if (it.body().contentLength() == 0) { + return [:] + } else { + (Map) new JsonSlurper().parse(it.body().byteStream()) + } } } catch(IOException e) { throw new NexusStagingException("Could not connect to Nexus.", e) From 5d101a68024b8f6ff2c545ee954d1514cf68c230 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Wed, 17 Feb 2021 13:37:25 +0900 Subject: [PATCH 4/5] Cleanups and try raising timeouts --- .../infra/SimplifiedHttpJsonRestClient.groovy | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy index 81cb23b..c5b1455 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy @@ -4,6 +4,8 @@ import groovy.json.JsonOutput import groovy.json.JsonSlurper import groovy.transform.CompileStatic import groovy.util.logging.Slf4j +import java.util.concurrent.TimeUnit +import javax.annotation.concurrent.ThreadSafe import okhttp3.Credentials import okhttp3.Interceptor import okhttp3.OkHttpClient @@ -17,16 +19,17 @@ import org.jetbrains.annotations.NotNull */ @CompileStatic @Slf4j +@ThreadSafe class SimplifiedHttpJsonRestClient { - private enum RequestType { - GET, POST - } - private final OkHttpClient restClient SimplifiedHttpJsonRestClient(OkHttpClient restClient, String username, String password) { OkHttpClient.Builder clientBuilder = restClient.newBuilder() + .connectTimeout(60, TimeUnit.SECONDS) + .readTimeout(60, TimeUnit.SECONDS) + .callTimeout(60, TimeUnit.SECONDS) + .writeTimeout(60, TimeUnit.SECONDS) clientBuilder.addInterceptor(new Interceptor() { @Override Response intercept(@NotNull Interceptor.Chain chain) throws IOException { @@ -48,24 +51,24 @@ class SimplifiedHttpJsonRestClient { Request request = new Request.Builder() .url(uri) .build() - return sendRequestHandlingErrors(request, RequestType.GET) + return sendRequestHandlingErrors(request) } Map post(String uri, Map content) { Request request = new Request.Builder() - .method("POST", RequestBody.create(JsonOutput.toJson(content), null)) + .post(RequestBody.create(JsonOutput.toJson(content), null)) .url(uri) .build() - return sendRequestHandlingErrors(request, RequestType.POST) + return sendRequestHandlingErrors(request) } - private Map sendRequestHandlingErrors(Request request, RequestType requestTypeName) { + private Map sendRequestHandlingErrors(Request request) { try { return restClient.newCall(request).execute().withCloseable { if (!it.successful) { String message = "${it.code()}: ${it.message()}, body: ${it.body().string() ?: ''}" //TODO: Suppress error message on 404 if waiting for drop? - log.warn("$requestTypeName request failed. ${message}") + log.warn("${request.method()} request failed. ${message}") throw new NexusHttpResponseException(it.code(), message) } if (it.body().contentLength() == 0) { From acd0e552a22804cf04e1b4da16c5e45ddeb6bd17 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 18 Feb 2021 09:02:16 +0900 Subject: [PATCH 5/5] Extract constant --- .../gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy index c5b1455..1691486 100644 --- a/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy +++ b/src/main/groovy/io/codearte/gradle/nexus/infra/SimplifiedHttpJsonRestClient.groovy @@ -22,6 +22,8 @@ import org.jetbrains.annotations.NotNull @ThreadSafe class SimplifiedHttpJsonRestClient { + private static final String CONTENT_TYPE_JSON = "application/json" + private final OkHttpClient restClient SimplifiedHttpJsonRestClient(OkHttpClient restClient, String username, String password) { @@ -34,8 +36,8 @@ class SimplifiedHttpJsonRestClient { @Override Response intercept(@NotNull Interceptor.Chain chain) throws IOException { Request.Builder request = chain.request().newBuilder() - .header("Content-Type", "application/json") - .header("Accept", "application/json") + .header("Content-Type", CONTENT_TYPE_JSON) + .header("Accept", CONTENT_TYPE_JSON) if (username != null && password != null) { request.header("Authorization", Credentials.basic(username, password))