First draft for refactoring

src/main/java/io/kamax/matrix/client/AMatrixHttpClient.java

@@ -23,11 +23,14 @@
 import com.google.gson.Gson;
 import com.google.gson.JsonParser;
 
+import io.kamax.matrix.MatrixErrorInfo;
 import io.kamax.matrix._MatrixID;
 import io.kamax.matrix.hs._MatrixHomeserver;
 
+import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.client.entity.EntityBuilder;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.entity.ContentType;
@@ -36,8 +39,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.charset.Charset;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -47,9 +52,10 @@ public abstract class AMatrixHttpClient implements _MatrixClientRaw {
 
     protected MatrixClientContext context;
 
-    protected CloseableHttpClient client = HttpClients.createDefault();
     protected Gson gson = new Gson();
     protected JsonParser jsonParser = new JsonParser();
+    // TODO refactoring: make private
+    protected CloseableHttpClient client = HttpClients.createDefault();
     private Pattern accessTokenUrlPattern = Pattern.compile("\\?access_token=(?<token>[^&]*)");
 
     public AMatrixHttpClient(MatrixClientContext context) {
@@ -76,6 +82,68 @@ public _MatrixID getUser() {
         return context.getUser();
     }
 
+    protected String execute(HttpRequestBase request) {
+        log(request);
+        try (CloseableHttpResponse response = client.execute(request)) {
+
+            HttpEntity entity = response.getEntity();
+            int responseStatus = response.getStatusLine().getStatusCode();
+
+            Charset charset = ContentType.getOrDefault(entity).getCharset();
+            String body = IOUtils.toString(entity.getContent(), charset);
+
+            if (responseStatus == 200) {
+                log.debug("Request successfully executed.");
+            } else if (responseStatus == 404) {
+                log.debug("Requested resource not found.");
+            } else {
+                MatrixErrorInfo info = gson.fromJson(body, MatrixErrorInfo.class);
+                log.debug("Request returned with an error. Status code: {}, errcode: {}, error: {}", responseStatus,
+                        info.getErrcode(), info.getError());
+
+                body = handleError(request, responseStatus, info);
+            }
+            return body;
+
+        } catch (IOException e) {
+            throw new MatrixClientRequestException(e);
+        }
+    }
+
+    /**
+     * Default handling of errors. Can be overwritten by a custom implementation in inherited classes.
+     * 
+     * @param request
+     * @param responseStatus
+     * @param info
+     * @return body of the response of a repeated call of the request, else this methods throws a
+     *         MatrixClientRequestException
+     */
+    protected String handleError(HttpRequestBase request, int responseStatus, MatrixErrorInfo info) {
+        String message = String.format("Request failed with status code: %s", responseStatus);
+
+        if (responseStatus == 429) {
+            return handleRateLimited(request, info);
+        }
+
+        throw new MatrixClientRequestException(info, message);
+    }
+
+    /**
+     * Default handling of rate limited calls. Can be overwritten by a custom implementation in inherited classes.
+     * 
+     * @param request
+     * @param info
+     * @return body of the response of a repeated call of the request, else this methods throws a
+     *         MatrixClientRequestException
+     */
+    protected String handleRateLimited(HttpRequestBase request, MatrixErrorInfo info) {
+        throw new MatrixClientRequestException(info, "Request was rate limited.");
+        // TODO Add default handling of rate limited call, i.e. repeated call after given time interval.
+        // 1. Wait for timeout
+        // 2. return execute(request)
+    }
+
     protected HttpRequestBase log(HttpRequestBase req) {
         String reqUrl = req.getURI().toASCIIString();
         Matcher m = accessTokenUrlPattern.matcher(reqUrl);
@@ -87,8 +155,8 @@ protected HttpRequestBase log(HttpRequestBase req) {
             reqUrl = b.toString();
         }
 
-        log.info("Doing {} {}", req.getMethod(), reqUrl);
-
+        log.debug("Doing {} {}", req.getMethod(), reqUrl);
+        // TODO refactoring: make private and remove return
         return req;
     }
 
@@ -131,5 +199,4 @@ protected URI getMediaPath(String action) {
     protected HttpEntity getJsonEntity(Object o) {
         return EntityBuilder.create().setText(gson.toJson(o)).setContentType(ContentType.APPLICATION_JSON).build();
     }
-
 }

src/main/java/io/kamax/matrix/client/MatrixHttpRoom.java

@@ -31,6 +31,7 @@
 import io.kamax.matrix.json.RoomMessageTextPutBody;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
@@ -43,10 +44,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 
 public class MatrixHttpRoom extends AMatrixHttpClient implements _MatrixRoom {
 
@@ -105,32 +103,12 @@ public Optional<String> getName() {
 
     @Override
     public Optional<String> getTopic() {
-        try {
-            URI path = getClientPath("/rooms/{roomId}/state/m.room.topic");
-
-            try (CloseableHttpResponse res = client.execute(log(new HttpGet(path)))) {
-                Charset charset = ContentType.getOrDefault(res.getEntity()).getCharset();
-                String body = IOUtils.toString(res.getEntity().getContent(), charset);
-
-                if (res.getStatusLine().getStatusCode() != 200) {
-                    if (res.getStatusLine().getStatusCode() == 404) {
-                        // No topic has been set
-                        return Optional.empty();
-                    }
-
-                    // TODO handle rate limited
-                    if (res.getStatusLine().getStatusCode() == 429) {
-                        log.warn("Request was rate limited", new Exception());
-                    }
-                    MatrixErrorInfo info = gson.fromJson(body, MatrixErrorInfo.class);
-                    throw new MatrixClientRequestException(info, "Couldn't get topic for room " + roomId);
-                }
-
-                return Optional.of(jsonParser.parse(body).getAsJsonObject().get("topic").getAsString());
-            }
-        } catch (IOException e) {
-            throw new MatrixClientRequestException(e);
+        URI path = getClientPath("/rooms/{roomId}/state/m.room.topic");
+        String body = execute(new HttpGet(path));
+        if (StringUtils.isNotBlank(body)) {
+            return Optional.of(jsonParser.parse(body).getAsJsonObject().get("topic").getAsString());
         }
+        return Optional.empty();
     }
 
     @Override