From 794e14cf643f1bffbb43d34ff6e52e8b9fbf989e Mon Sep 17 00:00:00 2001 From: Mariusz Pacek Date: Sat, 17 Mar 2018 17:40:26 +0100 Subject: [PATCH] Added: * Checking result of service * Added additional error code --- pom.xml | 6 ++ .../edu/allegro/exercise/ResultCodes.java | 3 +- .../exercise/handlers/ErrorHandler.java | 7 ++- src/main/resources/config.yaml | 2 +- .../edu/allegro/exercise/ResultCodesTest.java | 5 ++ ...RepoInfoTest.java => HandlerUtilTest.java} | 11 +++- .../exercise/handlers/IntegrationTest.java | 61 +++++++++++++------ src/test/resources/jsonExpected.json | 7 +++ src/test/resources/jsonNotExpected.json | 7 +++ 9 files changed, 86 insertions(+), 23 deletions(-) rename src/test/java/edu/allegro/exercise/handlers/{GetUserRepoInfoTest.java => HandlerUtilTest.java} (63%) create mode 100644 src/test/resources/jsonExpected.json create mode 100644 src/test/resources/jsonNotExpected.json diff --git a/pom.xml b/pom.xml index 29522d0..13a6043 100644 --- a/pom.xml +++ b/pom.xml @@ -17,6 +17,7 @@ 2.15.0 2.9.4 1.20 + 1.5.0 @@ -59,6 +60,11 @@ ${wiremock.version} test + + org.skyscreamer + jsonassert + ${jsonassert.version} + diff --git a/src/main/java/edu/allegro/exercise/ResultCodes.java b/src/main/java/edu/allegro/exercise/ResultCodes.java index 125806e..fd59284 100644 --- a/src/main/java/edu/allegro/exercise/ResultCodes.java +++ b/src/main/java/edu/allegro/exercise/ResultCodes.java @@ -6,7 +6,8 @@ public enum ResultCodes { BAD_REQUEST(400, 500, "Github client: Bad request"), NOT_FOUND(404, 404, "Github client: Given repository or owner doesn't exist"), FORBIDDEN(403, 403, "Github client: API rate limit exceeded"), - TIMEOUT(504, 504, "Githoub client: connection timeout"); + TIMEOUT(504, 504, "Githoub client: connection timeout"), + BAD_GATEWAY(503, 503, "Githoub client: connection timeout"); private final int statusCode; diff --git a/src/main/java/edu/allegro/exercise/handlers/ErrorHandler.java b/src/main/java/edu/allegro/exercise/handlers/ErrorHandler.java index c303954..6ed736c 100644 --- a/src/main/java/edu/allegro/exercise/handlers/ErrorHandler.java +++ b/src/main/java/edu/allegro/exercise/handlers/ErrorHandler.java @@ -7,9 +7,10 @@ import ratpack.handling.Context; import ratpack.http.client.HttpClientReadTimeoutException; import ratpack.path.InvalidPathEncodingException; +import java.net.ConnectException; + import static edu.allegro.exercise.handlers.HandlersUtils.handleError; -import static io.netty.handler.codec.http.HttpResponseStatus.GATEWAY_TIMEOUT; -import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR; +import static io.netty.handler.codec.http.HttpResponseStatus.*; public class ErrorHandler implements ServerErrorHandler { private static final Logger log = LoggerFactory.getLogger(ErrorHandler.class); @@ -18,6 +19,8 @@ public class ErrorHandler implements ServerErrorHandler { public void error(Context context, Throwable throwable) throws Exception { if (throwable instanceof HttpClientReadTimeoutException) { handleError(context, GATEWAY_TIMEOUT.code()); + } else if (throwable instanceof ConnectException) { + handleError(context, SERVICE_UNAVAILABLE.code()); } handleError(context, INTERNAL_SERVER_ERROR.code()); log.error("Internal error occur: ", throwable); diff --git a/src/main/resources/config.yaml b/src/main/resources/config.yaml index afb5ac9..3402e48 100644 --- a/src/main/resources/config.yaml +++ b/src/main/resources/config.yaml @@ -6,7 +6,7 @@ server: app: clientPoolSize: 20 clientTimeout: 10 - githubResourceV3: https://api.github.com/repos/ + githubResourceV3: https://api.github.com githubUserAgent: Allegro-exercise githubAccept: application/vnd.github.v3+json githubUserBasic: YWxsZWdyb0V4ZXJjaXNlOkNpSHVuQmFIaWR1andpNg== \ No newline at end of file diff --git a/src/test/java/edu/allegro/exercise/ResultCodesTest.java b/src/test/java/edu/allegro/exercise/ResultCodesTest.java index 4191503..7da6331 100644 --- a/src/test/java/edu/allegro/exercise/ResultCodesTest.java +++ b/src/test/java/edu/allegro/exercise/ResultCodesTest.java @@ -1,6 +1,7 @@ package edu.allegro.exercise; import edu.allegro.exercise.model.github.ErrorResponse; +import org.apache.commons.lang3.StringUtils; import org.junit.Test; import java.util.HashSet; @@ -14,6 +15,7 @@ public class ResultCodesTest { public void errorResponseShouldBe404Test() { ErrorResponse errorResponse = ResultCodes.fromCode(404); assertTrue("Github error server 404 should be mapped on 404 error response", errorResponse.getStatusCode() == 404); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(errorResponse.getStatusDescription())); } @@ -21,6 +23,7 @@ public class ResultCodesTest { public void errorResponseShouldBe400Test() { ErrorResponse errorResponse = ResultCodes.fromCode(403); assertTrue("Github error server 403 should be mapped on 400 error response", errorResponse.getStatusCode() == 403); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(errorResponse.getStatusDescription())); } @@ -28,6 +31,7 @@ public class ResultCodesTest { public void errorResponseShouldBe500Test() { ErrorResponse errorResponse = ResultCodes.fromCode(500); assertTrue("Github error server 500 should be mapped on 500 error response", errorResponse.getStatusCode() == 500); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(errorResponse.getStatusDescription())); } @Test @@ -40,6 +44,7 @@ public class ResultCodesTest { if (!notUsed.contains(i)) { ErrorResponse errorResponse = ResultCodes.fromCode(i); assertTrue("For error code: " + i + " ErrorResponse should be 500", errorResponse.getStatusCode() == 500); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(errorResponse.getStatusDescription())); } } } diff --git a/src/test/java/edu/allegro/exercise/handlers/GetUserRepoInfoTest.java b/src/test/java/edu/allegro/exercise/handlers/HandlerUtilTest.java similarity index 63% rename from src/test/java/edu/allegro/exercise/handlers/GetUserRepoInfoTest.java rename to src/test/java/edu/allegro/exercise/handlers/HandlerUtilTest.java index c46800d..3bef245 100644 --- a/src/test/java/edu/allegro/exercise/handlers/GetUserRepoInfoTest.java +++ b/src/test/java/edu/allegro/exercise/handlers/HandlerUtilTest.java @@ -1,9 +1,13 @@ package edu.allegro.exercise.handlers; +import org.apache.logging.log4j.core.util.IOUtils; + +import java.io.IOException; +import java.io.InputStreamReader; import java.util.HashMap; import java.util.Map; -public class GetUserRepoInfoTest { +public class HandlerUtilTest { private static final Map CONFIGURATION = new HashMap() {{ put("githubResourceV3", "http://localhost:1111"); @@ -21,4 +25,9 @@ public class GetUserRepoInfoTest { } + public static String getFileFromResourceAsString(String name) throws IOException { + InputStreamReader reader = new InputStreamReader(Thread.currentThread().getContextClassLoader().getResourceAsStream(name)); + return IOUtils.toString(reader); + } + } \ No newline at end of file diff --git a/src/test/java/edu/allegro/exercise/handlers/IntegrationTest.java b/src/test/java/edu/allegro/exercise/handlers/IntegrationTest.java index ee0e286..146076d 100644 --- a/src/test/java/edu/allegro/exercise/handlers/IntegrationTest.java +++ b/src/test/java/edu/allegro/exercise/handlers/IntegrationTest.java @@ -1,10 +1,12 @@ package edu.allegro.exercise.handlers; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.github.tomakehurst.wiremock.common.Slf4jNotifier; import com.github.tomakehurst.wiremock.junit.WireMockRule; import io.netty.handler.codec.http.HttpResponseStatus; +import org.apache.commons.lang3.StringUtils; import org.junit.After; -import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.slf4j.Logger; @@ -15,16 +17,19 @@ import ratpack.error.internal.DefaultProductionErrorHandler; import ratpack.http.client.ReceivedResponse; import ratpack.registry.Registry; import ratpack.server.RatpackServer; -import ratpack.server.ServerConfigBuilder; import ratpack.test.embed.EmbeddedApp; +import ratpack.test.http.TestHttpClient; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.junit.Assert.assertTrue; +import static org.skyscreamer.jsonassert.JSONAssert.assertEquals; + public class IntegrationTest { private static final Logger log = LoggerFactory.getLogger(IntegrationTest.class); private static final String GITHUB_OWNER = "sinu0"; private static final String GIHTUB_REPO = "SiSPD"; - private static final String GIHTUB_REPO_NOK = "SiSPDs"; + private static final String GIHTUB_REPO_NOT_FOUND = "SiSPDs"; private static final String GITHUB_REPO_FORBIDDEN = "Forbidden"; private static final String GITHUB_REPO_TIMEOUT = "timeout"; private final EmbeddedApp app; @@ -34,13 +39,17 @@ public class IntegrationTest { public IntegrationTest() throws Exception { rule.start(); - GetUserRepoInfoService getUserRepoInfoService = GetUserRepoInfoTest.userRepoTest(); + GetUserRepoInfoService getUserRepoInfoService = HandlerUtilTest.userRepoTest(); app = EmbeddedApp.fromServer(RatpackServer.of(ratpackServerSpec -> ratpackServerSpec - .serverConfig(ServerConfigBuilder::build) + .serverConfig(conf -> conf + .threads(1) + ) .registry(Registry.builder() .add(ClientErrorHandler.class, new DefaultProductionErrorHandler()) .add(ServerErrorHandler.class, new ErrorHandler()) + .with(registrySpec -> + registrySpec.add(new ObjectMapper().registerModule(new Jdk8Module()))) .build()) .handlers(getUserRepoInfoService.create()))); } @@ -55,47 +64,63 @@ public class IntegrationTest { @Test public void shouldReturnForbiddenTest() throws Exception { app.test(httpClient -> { - ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GITHUB_REPO_FORBIDDEN)); - Assert.assertTrue("Response should be nok for owner: " + GITHUB_OWNER + " and for repo: " + GITHUB_REPO_FORBIDDEN, receivedResponse.getStatusCode() == HttpResponseStatus.FORBIDDEN.code()); + ReceivedResponse receivedResponse = whenClientCall(httpClient, GITHUB_REPO_FORBIDDEN); + + assertTrue("Response should be FORBIDDEN for owner: " + GITHUB_OWNER + " and for repo: " + GITHUB_REPO_FORBIDDEN, receivedResponse.getStatusCode() == HttpResponseStatus.FORBIDDEN.code()); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(receivedResponse.getBody().getText())); + }); } - @Test public void shouldReturnOKInfoAboutOwnerRepoTest() throws Exception { app.test(httpClient -> { - ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GIHTUB_REPO)); + String jsonExpected = HandlerUtilTest.getFileFromResourceAsString("jsonExpected.json"); + ReceivedResponse receivedResponse = whenClientCall(httpClient, GIHTUB_REPO); + String response = receivedResponse.getBody().getText(); logResponse(receivedResponse); - Assert.assertTrue("Response should be ok for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO, receivedResponse.getStatusCode() == HttpResponseStatus.OK.code()); + + assertTrue("Response should be OK for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO, receivedResponse.getStatusCode() == HttpResponseStatus.OK.code()); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(receivedResponse.getBody().getText())); + assertEquals(jsonExpected, response, true); }); } @Test public void forNotExistingRepoShouldReturn404Test() throws Exception { app.test(httpClient -> { - ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GIHTUB_REPO_NOK)); - Assert.assertTrue("Response should be nok for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO_NOK, receivedResponse.getStatusCode() == HttpResponseStatus.NOT_FOUND.code()); + ReceivedResponse receivedResponse = whenClientCall(httpClient, GIHTUB_REPO_NOT_FOUND); + + assertTrue("Response should be NOT_FOUND for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO_NOT_FOUND, receivedResponse.getStatusCode() == HttpResponseStatus.NOT_FOUND.code()); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(receivedResponse.getBody().getText())); }); } @Test - public void whenGithubShutdownShouldReturn500OnGithubUnavailabilityTest() throws Exception { + public void whenGithubShutdownShouldReturn503OnGithubUnavailabilityTest() throws Exception { rule.stop(); app.test(httpClient -> { - ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GIHTUB_REPO_NOK)); - Assert.assertTrue("Response should be nok for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO_NOK, receivedResponse.getStatusCode() == HttpResponseStatus.INTERNAL_SERVER_ERROR.code()); + ReceivedResponse receivedResponse = whenClientCall(httpClient, GIHTUB_REPO); + + assertTrue("Response should be for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO, receivedResponse.getStatusCode() == HttpResponseStatus.SERVICE_UNAVAILABLE.code()); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(receivedResponse.getBody().getText())); }); } @Test public void whenGithubIsVerySlowShouldReturnTimeout() throws Exception { app.test(httpClient -> { - ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GITHUB_REPO_TIMEOUT)); - logResponse(receivedResponse); - Assert.assertTrue("Response should be 504 for owner: " + GITHUB_OWNER + " and for repo: " + GITHUB_REPO_TIMEOUT, receivedResponse.getStatusCode() == HttpResponseStatus.GATEWAY_TIMEOUT.code()); + ReceivedResponse receivedResponse = whenClientCall(httpClient, GITHUB_REPO_TIMEOUT); + + assertTrue("Response should be 504 for owner: " + GITHUB_OWNER + " and for repo: " + GITHUB_REPO_TIMEOUT, receivedResponse.getStatusCode() == HttpResponseStatus.GATEWAY_TIMEOUT.code()); + assertTrue("And description should not be empty", StringUtils.isNotEmpty(receivedResponse.getBody().getText())); }); } + private ReceivedResponse whenClientCall(TestHttpClient httpClient, String gihtubRepo) { + return httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, gihtubRepo)); + } + private void logResponse(ReceivedResponse response) { log.info(response.getHeaders().asMultiValueMap().toString()); log.info(response.getStatus().toString()); diff --git a/src/test/resources/jsonExpected.json b/src/test/resources/jsonExpected.json new file mode 100644 index 0000000..c8f013a --- /dev/null +++ b/src/test/resources/jsonExpected.json @@ -0,0 +1,7 @@ +{ + "full_name": "sinu0/SiSPD", + "description": "Symulacja i sterowanie procesów dyskretnych - projekt.", + "created_at": "2012-10-22T16:19:46Z", + "clone_url": "https://github.com/sinu0/SiSPD.git", + "stargazers_count": 0 +} \ No newline at end of file diff --git a/src/test/resources/jsonNotExpected.json b/src/test/resources/jsonNotExpected.json new file mode 100644 index 0000000..68cbd37 --- /dev/null +++ b/src/test/resources/jsonNotExpected.json @@ -0,0 +1,7 @@ +{ + "full_name": "sinu0/scribe-java", + "description": "Simple OAuth library for Java", + "created_at": "2012-08-18T13:17:18Z", + "clone_url": "https://github.com/sinu0/scribe-java.git", + "stargazers_count": 0 +} \ No newline at end of file