Skip to content

Commit

Permalink
SUBSCRIPTION_NOTICE handlers can now return failure() without crash (#48
Browse files Browse the repository at this point in the history
)

* Return 200 on exceptions & failures

* Added test to confirm the Notice events can return a failure without any exceptions.
  • Loading branch information
gbranchaudrubenovitch authored and jpboudreault committed Jan 16, 2017
1 parent 1558d73 commit 326fed9
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 17 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
<dependency>
<groupId>com.icegreen</groupId>
<artifactId>greenmail</artifactId>
<version>1.5.2</version>
<version>1.5.3</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/com/appdirect/sdk/appmarket/events/APIResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ public class APIResult {

public APIResult(ErrorCode errorCode, String message) {
this(false, message);
setErrorCode(errorCode);
this.errorCode = errorCode;
}

public APIResult(boolean success, String message) {
setSuccess(success);
setMessage(message);
this.success = success;
this.message = message;
}

public static APIResult success(String message) {
Expand All @@ -48,7 +48,8 @@ public static APIResult asyncEventResult(String message) {
}

public static APIResult failure(ErrorCode errorCode, String message) {
return new APIResult(errorCode, message);
APIResult result = new APIResult(errorCode, message);
result.setStatusCodeReturnedToAppmarket(200);
return result;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appdirect.sdk.appmarket.events;

import static com.appdirect.sdk.appmarket.events.ErrorCode.UNKNOWN_ERROR;
import static java.lang.String.format;

import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -35,7 +36,7 @@ APIResult processEvent(String eventUrl, EventHandlingContext eventContext) {
throw e;
} catch (RuntimeException e) {
log.error("Exception while attempting to process an event. eventUrl={}", eventUrl, e);
throw new DeveloperServiceException(ErrorCode.UNKNOWN_ERROR, format("Failed to process event. eventUrl=%s | exception=%s", eventUrl, e.getMessage()));
throw new DeveloperServiceException(UNKNOWN_ERROR, format("Failed to process event. eventUrl=%s | exception=%s", eventUrl, e.getMessage()));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
package com.appdirect.sdk.appmarket.events;

import javax.servlet.http.HttpServletResponse;

import lombok.extern.slf4j.Slf4j;

import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.ResponseStatus;

import com.appdirect.sdk.exception.DeveloperServiceException;

@Slf4j
@ControllerAdvice
public class DeveloperExceptionHandler {
@ResponseBody
@ResponseStatus(value = HttpStatus.OK)
@ExceptionHandler
public APIResult handleDeveloperServiceException(DeveloperServiceException e) {
public APIResult handleDeveloperServiceException(DeveloperServiceException e, HttpServletResponse response) {
APIResult result = e.getResult();
response.setStatus(result.getStatusCodeReturnedToAppmarket());
log.debug("Handling DeveloperServiceException. APIResult={}", result);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.appdirect.sdk.exception;

import static com.appdirect.sdk.appmarket.events.APIResult.failure;

import lombok.Getter;

import com.appdirect.sdk.appmarket.events.APIResult;
Expand All @@ -18,6 +20,6 @@ public DeveloperServiceException(String message) {

public DeveloperServiceException(ErrorCode errorCode, String message) {
super(message);
this.result = new APIResult(errorCode, message);
this.result = failure(errorCode, message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ public void whenHostOfEventIsUnknown_weReturnTheRightPayloadToAppmarket() throws
assertThat(EntityUtils.toString(response.getEntity())).isEqualTo("{\"success\":false,\"errorCode\":\"UNKNOWN_ERROR\",\"message\":\"Failed to process event. eventUrl=http://does-not.exists | exception=I/O error on GET request for \\\"http://does-not.exists\\\": does-not.exists; nested exception is java.net.UnknownHostException: does-not.exists\"}");
}

@Test
public void whenNoticeEventFails_errorIsReportedToAppmarket() throws Exception {
HttpResponse response = fakeAppmarket.sendEventTo(connectorEventEndpoint(), "/v1/events/subscription-closed", "failThisCall", "true");

assertThat(fakeAppmarket.lastRequestPath()).isEqualTo("/v1/events/subscription-closed");
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
assertThat(EntityUtils.toString(response.getEntity())).isEqualTo("{\"success\":false,\"errorCode\":\"OPERATION_CANCELLED\",\"message\":\"You made this call fail\"}");
}

private String connectorEventEndpoint() {
return baseConnectorUrl() + "/api/v1/integration/processEvent";
}
Expand Down
11 changes: 8 additions & 3 deletions src/test/java/com/appdirect/sdk/feature/MinimalConnector.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.appdirect.sdk.feature;

import static com.appdirect.sdk.appmarket.events.APIResult.failure;
import static com.appdirect.sdk.appmarket.events.APIResult.success;
import static com.appdirect.sdk.appmarket.events.ErrorCode.OPERATION_CANCELLED;
import static com.appdirect.sdk.appmarket.events.ErrorCode.USER_NOT_FOUND;
import static java.lang.String.format;

Expand Down Expand Up @@ -62,9 +64,12 @@ public AppmarketEventHandler<SubscriptionChange> subscriptionChangeHandler() {

@Bean
public AppmarketEventHandler<SubscriptionClosed> subscriptionClosedHandler() {
return event -> success(
format("SUB_CLOSED %s has been processed, for real.", event.getAccountInfo().getAccountIdentifier())
);
return event -> {
boolean callShouldFail = event.getQueryParameters().containsKey("failThisCall");
return callShouldFail ?
failure(OPERATION_CANCELLED, "You made this call fail") :
success(format("SUB_CLOSED %s has been processed, for real.", event.getAccountInfo().getAccountIdentifier()));
};
}

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import static com.appdirect.sdk.appmarket.events.ErrorCode.UNAUTHORIZED;
import static org.assertj.core.api.Assertions.assertThat;

import javax.servlet.http.HttpServletResponse;

import org.junit.Test;
import org.springframework.mock.web.MockHttpServletResponse;

import com.appdirect.sdk.appmarket.events.APIResult;
import com.appdirect.sdk.appmarket.events.DeveloperExceptionHandler;
Expand All @@ -12,11 +15,13 @@
public class DeveloperExceptionHandlerTest {

@Test
public void handleIsvServiceException_returnsTheApiResultInIt() throws Exception {
public void handleIsvServiceException_returnsTheApiResultInIt_setsTheStatusTo200() throws Exception {
HttpServletResponse response = new MockHttpServletResponse();
DeveloperServiceException someException = new DeveloperServiceException(UNAUTHORIZED, "no no no");

APIResult result = new DeveloperExceptionHandler().handleDeveloperServiceException(someException);
APIResult result = new DeveloperExceptionHandler().handleDeveloperServiceException(someException, response);

assertThat(response.getStatus()).isEqualTo(200);
assertThat(result).hasFieldOrPropertyWithValue("errorCode", UNAUTHORIZED)
.hasFieldOrPropertyWithValue("message", "no no no");
}
Expand Down

0 comments on commit 326fed9

Please sign in to comment.