Merge pull request #49 from gbranchaudrubenovitch/test-gab

Addressed review comments for #48

src/main/java/com/appdirect/sdk/appmarket/events/APIResult.java

@@ -47,6 +47,15 @@ public static APIResult asyncEventResult(String message) {
 		return result;
 	}
 
+	/**
+	 * Creates a failed result with the given error code and message.
+	 * Note: HTTP OK (200) will be returned to the appmarket; all responses
+	 * going to the appmarket need to have a 200 status, whether they are successful or not.
+	 *
+	 * @param errorCode the code of the error
+	 * @param message   human-readable error message that explains the issue.
+	 * @return the failed result object
+	 */
 	public static APIResult failure(ErrorCode errorCode, String message) {
 		APIResult result = new APIResult(errorCode, message);
 		result.setStatusCodeReturnedToAppmarket(200);

src/test/java/com/appdirect/sdk/feature/ErrorHandlingWorks.java

@@ -38,24 +38,23 @@ public void stop() throws Exception {
 	public void whenEventIsNotFoundOnAppmarket_weReturnAPayloadMatchingMarketplaceFormat() throws Exception {
 		HttpResponse response = fakeAppmarket.sendEventTo(connectorEventEndpoint(), "/nonExistant/v1/events/order");
 
-		assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
+		assertStatusCodeIs200_soAppmarketShowsProperMessageToUser(response);
 		assertThat(EntityUtils.toString(response.getEntity())).isEqualTo("{\"success\":false,\"errorCode\":\"NOT_FOUND\",\"message\":\"Failed to fetch event: Not Found\"}");
 	}
 
 	@Test
 	public void whenHostOfEventIsUnknown_weReturnTheRightPayloadToAppmarket() throws Exception {
 		HttpResponse response = fakeAppmarket.sendSignedRequestTo(connectorEventEndpoint(), asList("eventUrl", "http://does-not.exists"));
 
-		assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
+		assertStatusCodeIs200_soAppmarketShowsProperMessageToUser(response);
 		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);
+		assertStatusCodeIs200_soAppmarketShowsProperMessageToUser(response);
 		assertThat(EntityUtils.toString(response.getEntity())).isEqualTo("{\"success\":false,\"errorCode\":\"OPERATION_CANCELLED\",\"message\":\"You made this call fail\"}");
 	}
 
@@ -66,4 +65,12 @@ private String connectorEventEndpoint() {
 	private String baseConnectorUrl() {
 		return "http://localhost:" + localConnectorPort;
 	}
+
+	/**
+	 * Returning a failure with a non-200 code results in the appmarket showing a "communication error" and
+	 * logging the first few chars of the raw response, leading to truncated and messy logs. So let's return 200.
+	 */
+	private void assertStatusCodeIs200_soAppmarketShowsProperMessageToUser(HttpResponse response) {
+		assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
+	}
 }