Added:

* handling connection timeout
* refactoring
* test connection timeout

src/main/java/edu/allegro/exercise/ResultCodes.java

@@ -5,7 +5,8 @@ import edu.allegro.exercise.model.github.ErrorResponse;
 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");
+    FORBIDDEN(403, 403, "Github client: API rate limit exceeded"),
+    TIMEOUT(504, 504, "Githoub client: connection timeout");
 
 
     private final int statusCode;

src/main/java/edu/allegro/exercise/Server.java

@@ -2,6 +2,7 @@ package edu.allegro.exercise;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
+import edu.allegro.exercise.handlers.ErrorHandler;
 import edu.allegro.exercise.handlers.GetUserRepoInfoService;
 import ratpack.error.ClientErrorHandler;
 import ratpack.error.ServerErrorHandler;
@@ -18,7 +19,7 @@ public class Server {
                         .serverConfig(config)
                         .registry(Registry.builder()
                                 .add(ClientErrorHandler.class, new DefaultProductionErrorHandler())
-                                .add(ServerErrorHandler.class, new DefaultProductionErrorHandler())
+                                .add(ServerErrorHandler.class, new ErrorHandler())
                                 .with(registrySpec ->
                                         registrySpec.add(new ObjectMapper().registerModule(new Jdk8Module())))
                                 .build())

src/main/java/edu/allegro/exercise/handlers/ErrorHandler.java

@@ -0,0 +1,31 @@
+package edu.allegro.exercise.handlers;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import ratpack.error.ServerErrorHandler;
+import ratpack.handling.Context;
+import ratpack.http.client.HttpClientReadTimeoutException;
+import ratpack.path.InvalidPathEncodingException;
+
+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;
+
+public class ErrorHandler implements ServerErrorHandler {
+    private static final Logger log = LoggerFactory.getLogger(ErrorHandler.class);
+
+    @Override
+    public void error(Context context, Throwable throwable) throws Exception {
+        if (throwable instanceof HttpClientReadTimeoutException) {
+            handleError(context, GATEWAY_TIMEOUT.code());
+        }
+        handleError(context, INTERNAL_SERVER_ERROR.code());
+        log.error("Internal error occur: ", throwable);
+    }
+
+    @Override
+    public void error(Context context, InvalidPathEncodingException exception) throws Exception {
+        handleError(context, INTERNAL_SERVER_ERROR.code());
+        log.error("Internal error occur: ", exception);
+    }
+}

src/main/java/edu/allegro/exercise/handlers/GetUserRepoInfoService.java

@@ -2,8 +2,6 @@ package edu.allegro.exercise.handlers;
 
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import edu.allegro.exercise.ResultCodes;
-import edu.allegro.exercise.model.github.ErrorResponse;
 import edu.allegro.exercise.model.github.UserRepoInfo;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import org.slf4j.Logger;
@@ -18,9 +16,11 @@ import ratpack.jackson.Jackson;
 import ratpack.jackson.JsonRender;
 
 import java.io.IOException;
+import java.net.URISyntaxException;
 import java.util.Optional;
 
-import static ratpack.jackson.Jackson.json;
+import static edu.allegro.exercise.handlers.HandlersUtils.handleError;
+import static edu.allegro.exercise.handlers.HandlersUtils.logErrorResponse;
 
 public class GetUserRepoInfoService {
     private static final Logger log = LoggerFactory.getLogger(GetUserRepoInfoService.class);
@@ -40,16 +40,15 @@ public class GetUserRepoInfoService {
     public Action<Chain> create() {
         return chain ->
                 chain.prefix(REPOSITORIES, action ->
-                        action.get(PATH, ctx -> {
-                            Promise<Optional<ReceivedResponse>> optionalPromise =
-                                    repositoryInfo.getInfo(getToken(ctx, OWNER), getToken(ctx, REPO))
-                                            .mapIf(this::ifSuccess, handleOK(), handleNOK(ctx));
+                        action.get(PATH, this::handle));
+    }
 
-                            Promise<JsonRender> response = optionalPromise
-                                    .map(responseOp -> responseOp.map(this::getUserRepoInfo))
-                                    .map(Jackson::json);
-                            ctx.render(response);
-                        }));
+    private void handle(Context ctx) throws URISyntaxException {
+        Promise<JsonRender> response = repositoryInfo.getInfo(getToken(ctx, OWNER), getToken(ctx, REPO))
+                .mapIf(this::ifSuccess, handleOK(), handleNOK(ctx))
+                .map(responseOp -> responseOp.map(this::getUserRepoInfo))
+                .map(Jackson::json);
+        ctx.render(response);
     }
 
     private boolean ifSuccess(ReceivedResponse response) {
@@ -62,20 +61,12 @@ public class GetUserRepoInfoService {
 
     private Function<ReceivedResponse, Optional<ReceivedResponse>> handleNOK(Context ctx) {
         return response -> {
-            logErrorResponse(response);
-            ErrorResponse errorResponse = ResultCodes.fromCode(response.getStatusCode());
-            ctx.getResponse().status(errorResponse.getStatusCode());
-            ctx.render(json(errorResponse));
+            logErrorResponse(response, log);
+            handleError(ctx, response.getStatusCode());
             return Optional.empty();
         };
     }
 
-    private void logErrorResponse(ReceivedResponse response) {
-        log.info(response.getHeaders().asMultiValueMap().toString());
-        log.info(response.getStatus().toString());
-        log.info(response.getBody().getText());
-    }
-
     private String getToken(Context ctx, String token) {
         return ctx.getPathTokens().get(token);
     }

src/main/java/edu/allegro/exercise/handlers/HandlersUtils.java

@@ -0,0 +1,23 @@
+package edu.allegro.exercise.handlers;
+
+import edu.allegro.exercise.ResultCodes;
+import edu.allegro.exercise.model.github.ErrorResponse;
+import org.slf4j.Logger;
+import ratpack.handling.Context;
+import ratpack.http.client.ReceivedResponse;
+
+import static ratpack.jackson.Jackson.json;
+
+class HandlersUtils {
+    static void handleError(Context ctx, int code) {
+        ErrorResponse errorResponse = ResultCodes.fromCode(code);
+        ctx.getResponse().status(errorResponse.getStatusCode());
+        ctx.render(json(errorResponse));
+    }
+
+    static void logErrorResponse(ReceivedResponse response, Logger log) {
+        log.info(response.getHeaders().asMultiValueMap().toString());
+        log.info(response.getStatus().toString());
+        log.info(response.getBody().getText());
+    }
+}

src/main/java/edu/allegro/exercise/handlers/RepositoryInfo.java

@@ -5,6 +5,7 @@ import io.netty.util.internal.StringUtil;
 import ratpack.exec.Promise;
 import ratpack.http.Headers;
 import ratpack.http.client.HttpClient;
+import ratpack.http.client.HttpClientReadTimeoutException;
 import ratpack.http.client.ReceivedResponse;
 import ratpack.http.client.RequestSpec;
 import ratpack.http.internal.NettyHeadersBackedHeaders;
@@ -45,7 +46,7 @@ public class RepositoryInfo {
 
     }
 
-    public Promise<ReceivedResponse> getInfo(String owner, String repo) throws URISyntaxException {
+    public Promise<ReceivedResponse> getInfo(String owner, String repo) throws URISyntaxException, HttpClientReadTimeoutException {
         String url = resourceLink + REPOS + owner + "/" + repo;
 
         return client.get(new URI(url),
@@ -56,7 +57,6 @@ public class RepositoryInfo {
         requestSpec.getHeaders().copy(headers);
     }
 
-
     private Headers setHeaders(Map<String, Object> configuration) {
         DefaultHttpHeaders defaultHttpHeaders = new DefaultHttpHeaders();
         Headers headers = new NettyHeadersBackedHeaders(defaultHttpHeaders);

src/test/java/edu/allegro/exercise/handlers/GetUserRepoInfoTest.java

@@ -11,7 +11,7 @@ public class GetUserRepoInfoTest {
         put("githubAccept", "application/vnd.github.v3+json");
         put("githubUserBasic", "YWxsZWdyb0V4ZXJjaXNlOkNpSHVuQmFIaWR1andpNg==");
         put("clientPoolSize", 10);
-        put("clientTimeout", 10);
+        put("clientTimeout", 1);
     }};
 
 

src/test/java/edu/allegro/exercise/handlers/IntegrationTest.java

@@ -9,11 +9,14 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import ratpack.error.ClientErrorHandler;
+import ratpack.error.ServerErrorHandler;
+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;
 
@@ -23,6 +26,7 @@ public class IntegrationTest {
     private static final String GIHTUB_REPO = "SiSPD";
     private static final String GIHTUB_REPO_NOK = "SiSPDs";
     private static final String GITHUB_REPO_FORBIDDEN = "Forbidden";
+    private static final String GITHUB_REPO_TIMEOUT = "timeout";
     private final EmbeddedApp app;
 
     @Rule
@@ -34,6 +38,10 @@ public class IntegrationTest {
         app = EmbeddedApp.fromServer(RatpackServer.of(ratpackServerSpec ->
                 ratpackServerSpec
                         .serverConfig(ServerConfigBuilder::build)
+                        .registry(Registry.builder()
+                                .add(ClientErrorHandler.class, new DefaultProductionErrorHandler())
+                                .add(ServerErrorHandler.class, new ErrorHandler())
+                                .build())
                         .handlers(getUserRepoInfoService.create())));
     }
 
@@ -46,40 +54,46 @@ public class IntegrationTest {
 
     @Test
     public void shouldReturnForbiddenTest() throws Exception {
-        app.test(this::githubShouldReturnForbiddenAppShouldReturn403);
+        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());
+        });
     }
 
 
     @Test
     public void shouldReturnOKInfoAboutOwnerRepoTest() throws Exception {
-        app.test(this::shouldReturnOKInfoAboutOwnerRepo);
+        app.test(httpClient -> {
+            ReceivedResponse receivedResponse = httpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GIHTUB_REPO));
+            logResponse(receivedResponse);
+            Assert.assertTrue("Response should be ok for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO, receivedResponse.getStatusCode() == HttpResponseStatus.OK.code());
+        });
     }
 
     @Test
     public void forNotExistingRepoShouldReturn404Test() throws Exception {
-        app.test(this::forNotExistingReposhouldReturn404);
+        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());
+        });
     }
 
     @Test
-    public void WhenGithubShutdownShouldReturn500OnGithubUnavailabilityTest() throws Exception {
+    public void whenGithubShutdownShouldReturn500OnGithubUnavailabilityTest() throws Exception {
         rule.stop();
-        app.test(this::forNotExistingReposhouldReturn404);
+        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());
+        });
     }
 
-    private void githubShouldReturnForbiddenAppShouldReturn403(TestHttpClient testHttpClient) {
-        ReceivedResponse receivedResponse = testHttpClient.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());
-    }
-
-    private void shouldReturnOKInfoAboutOwnerRepo(TestHttpClient testHttpClient) {
-        ReceivedResponse receivedResponse = testHttpClient.get(String.format("/repositories/%s/%s", GITHUB_OWNER, GIHTUB_REPO));
-        logResponse(receivedResponse);
-        Assert.assertTrue("Response should be ok for owner: " + GITHUB_OWNER + " and for repo: " + GIHTUB_REPO, receivedResponse.getStatusCode() == HttpResponseStatus.OK.code());
-    }
-
-    private void forNotExistingReposhouldReturn404(TestHttpClient testHttpClient) {
-        ReceivedResponse receivedResponse = testHttpClient.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.OK.code());
+    @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());
+        });
     }
 
     private void logResponse(ReceivedResponse response) {

src/test/resources/mappings/repos_sinu0_sispd-570cfa1e-8aaa-4cd6-8037-d919654649d4.json

@@ -0,0 +1,40 @@
+{
+  "id": "570cfa1e-8aaa-4cd6-8037-d919654649d4",
+  "name": "repos_sinu0_sispd",
+  "request": {
+    "url": "/repos/sinu0/timeout",
+    "method": "GET"
+  },
+  "response": {
+    "status": 200,
+    "fixedDelayMilliseconds": 2000,
+    "bodyFileName": "repos_sinu0_sispd-570cfa1e-8aaa-4cd6-8037-d919654649d5.json",
+    "headers": {
+      "Server": "GitHub.com",
+      "Date": "Fri, 16 Mar 2018 22:04:16 GMT",
+      "Content-Type": "application/json; charset=utf-8",
+      "Status": "200 OK",
+      "X-RateLimit-Limit": "60",
+      "X-RateLimit-Remaining": "47",
+      "X-RateLimit-Reset": "1521240469",
+      "Cache-Control": "public, max-age=60, s-maxage=60",
+      "Vary": "Accept",
+      "ETag": "W/\"7a441b97c24d52980b539d9d4ae38cd3\"",
+      "Last-Modified": "Sat, 12 Jan 2013 19:04:16 GMT",
+      "X-GitHub-Media-Type": "unknown, github.v3",
+      "Access-Control-Expose-Headers": "ETag, Link, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval",
+      "Access-Control-Allow-Origin": "*",
+      "Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload",
+      "X-Frame-Options": "deny",
+      "X-Content-Type-Options": "nosniff",
+      "X-XSS-Protection": "1; mode=block",
+      "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
+      "Content-Security-Policy": "default-src 'none'",
+      "X-Runtime-rack": "0.054554",
+      "X-GitHub-Request-Id": "8956:23E8:D5D24:1D7E85:5AAC3F5F"
+    }
+  },
+  "uuid": "570cfa1e-8aaa-4cd6-8037-d919654649d5",
+  "persistent": true,
+  "insertionIndex": 3
+}