From 95f081a58eccbaaec50b8a17ff73f16f03aa0dfd Mon Sep 17 00:00:00 2001 From: ergin-a1 Date: Thu, 23 Mar 2017 13:38:26 -0700 Subject: [PATCH] AGO-8289 | Fixes infinite loop when session is expired https://github.com/Silverpop/engage-api-client/issues/20 --- pom.xml | 2 +- .../com/silverpop/api/client/ApiClient.java | 130 +++++++++--------- 2 files changed, 67 insertions(+), 65 deletions(-) diff --git a/pom.xml b/pom.xml index d269d4a..3afc813 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ com.silverpop spapi-client jar - 1.2 + 1.2.1 com.thoughtworks.xstream diff --git a/src/main/java/com/silverpop/api/client/ApiClient.java b/src/main/java/com/silverpop/api/client/ApiClient.java index 565b046..44a7e52 100644 --- a/src/main/java/com/silverpop/api/client/ApiClient.java +++ b/src/main/java/com/silverpop/api/client/ApiClient.java @@ -3,6 +3,8 @@ import java.io.IOException; import java.util.Map; +import com.silverpop.api.client.authentication.LogoutCommand; +import com.silverpop.api.client.xmlapi.XmlApiSession; import org.apache.commons.httpclient.Header; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpMethodBase; @@ -14,35 +16,35 @@ public abstract class ApiClient { private Log log = LogFactory.getLog(this.getClass()); - private ApiCommandProcessor commandProcessor; - protected HttpClient httpClient; + private ApiCommandProcessor commandProcessor; + protected HttpClient httpClient; protected ApiSession session; public ApiSession getSession() { - return session; - } + return session; + } - protected ApiClient(ApiCommandProcessor commandProcessor, ApiSession session) { - this(commandProcessor, new HttpClient(), session); - } + protected ApiClient(ApiCommandProcessor commandProcessor, ApiSession session) { + this(commandProcessor, new HttpClient(), session); + } - protected ApiClient(ApiCommandProcessor commandProcessor, HttpClient httpClient, ApiSession session) { - this.commandProcessor = commandProcessor; - this.httpClient = httpClient; - this.session = session; - } + protected ApiClient(ApiCommandProcessor commandProcessor, HttpClient httpClient, ApiSession session) { + this.commandProcessor = commandProcessor; + this.httpClient = httpClient; + this.session = session; + } - public ApiResult executeCommand(ApiCommand command) throws ApiResultException { - return executeCommand(command, null); - } + public ApiResult executeCommand(ApiCommand command) throws ApiResultException { + return executeCommand(command, null); + } - public ApiResult executeCommand(ApiCommand command, Map requestHeaders) throws ApiResultException { + public ApiResult executeCommand(ApiCommand command, Map requestHeaders) throws ApiResultException { try { return validateSessionAndExecuteCommand(command, requestHeaders); - } catch(ApiResultException e) { - if(retryCommand(e.getErrorResult())) { + } catch (ApiResultException e) { + if (retryCommand(e.getErrorResult(),command)) { getSession().close(); return validateSessionAndExecuteCommand(command, requestHeaders); } else { @@ -51,26 +53,26 @@ public ApiResult executeCommand(ApiCommand command, Map requestHe } } - private boolean retryCommand(ApiErrorResult errorResult) { - return errorResult.isSessionLost() && getSession().isReAuthenticate(); - } + private boolean retryCommand(ApiErrorResult errorResult, ApiCommand apiCommand) { + return errorResult.isSessionLost() && getSession().isReAuthenticate() && !(apiCommand instanceof LogoutCommand); + } - private ApiResult validateSessionAndExecuteCommand(ApiCommand command, Map requestHeaders) throws ApiResultException { - ensureSessionIsOpen(); + private ApiResult validateSessionAndExecuteCommand(ApiCommand command, Map requestHeaders) throws ApiResultException { + ensureSessionIsOpen(); - REQUEST request = commandProcessor.prepareRequest(command); + REQUEST request = commandProcessor.prepareRequest(command); addAdditionalHeadersToRequest(request, requestHeaders); addAdditionalHeadersToRequest(request, getSession().getDefaultHeaders()); HttpMethodBase method = commandProcessor.prepareMethod(getSession().getUrl(), request); - String in = executeMethod(method); + String in = executeMethod(method); - ApiResponse response = commandProcessor.processResponse(in, request.getResultType()); - - return extractResult(command.getClass().getName(), response); - } + ApiResponse response = commandProcessor.processResponse(in, request.getResultType()); - private void addAdditionalHeadersToRequest(REQUEST request, Map requestHeaders) { + return extractResult(command.getClass().getName(), response); + } + + private void addAdditionalHeadersToRequest(REQUEST request, Map requestHeaders) { if (requestHeaders != null) { for (String key : requestHeaders.keySet()) { request.addHeader(key, requestHeaders.get(key)); @@ -78,44 +80,44 @@ private void addAdditionalHeadersToRequest(REQUEST request, Map r } } - private void ensureSessionIsOpen() { - if(!getSession().isOpen()) { - getSession().open(); - } - } + private void ensureSessionIsOpen() { + if (!getSession().isOpen()) { + getSession().open(); + } + } - protected String executeMethod(HttpMethodBase method) throws ApiResultException { - try { + protected String executeMethod(HttpMethodBase method) throws ApiResultException { + try { log.info("executing method:" + method); - int responseCode = httpClient.executeMethod(method); - - String responseBody = method.getResponseBodyAsString(); - if (responseBody != null && !responseBody.isEmpty()) { - return responseBody; - } else { - StringBuilder buffer = new StringBuilder(); - buffer.append("No response body was returned!\nResponse Headers-\n"); - Header[] headers = method.getResponseHeaders(); - for (Header header : headers) { - buffer.append(header.getName()).append(": ").append(header.getValue()).append("\n"); - } + int responseCode = httpClient.executeMethod(method); + + String responseBody = method.getResponseBodyAsString(); + if (responseBody != null && !responseBody.isEmpty()) { + return responseBody; + } else { + StringBuilder buffer = new StringBuilder(); + buffer.append("No response body was returned!\nResponse Headers-\n"); + Header[] headers = method.getResponseHeaders(); + for (Header header : headers) { + buffer.append(header.getName()).append(": ").append(header.getValue()).append("\n"); + } buffer.append("HTTP-Response-Code: ").append(responseCode).append("\n"); - buffer.append("Content length reported as: ").append(method.getResponseContentLength()); + buffer.append("Content length reported as: ").append(method.getResponseContentLength()); log.info(buffer.toString()); throw new ApiResultException("Error executing API: " + buffer.toString(), new NoResponseApiErrorResult()); - } - } catch(IOException e) { - throw new ApiException("Error executing API: ", e); - } - } - - private ApiResult extractResult(String requestName, ApiResponse response) throws ApiResultException { - if(response.isSuccessful()) { - return response.buildResult(); - } else { + } + } catch (IOException e) { + throw new ApiException("Error executing API: ", e); + } + } + + private ApiResult extractResult(String requestName, ApiResponse response) throws ApiResultException { + if (response.isSuccessful()) { + return response.buildResult(); + } else { log.debug("Got Error Response"); - String msg = String.format("API call '%s' unsuccessful.", requestName); - throw new ApiResultException(msg, response.buildErrorResult()); - } - } + String msg = String.format("API call '%s' unsuccessful.", requestName); + throw new ApiResultException(msg, response.buildErrorResult()); + } + } }