Document and fixup NetClient to be more flexible

src/hdskins/java/com/voxelmodpack/hdskins/ThreadDownloadImageETag.java

@@ -2,6 +2,8 @@ package com.voxelmodpack.hdskins;
 
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
+import com.voxelmodpack.hdskins.skins.NetClient;
+
 import net.minecraft.client.renderer.IImageBuffer;
 import net.minecraft.client.renderer.texture.SimpleTexture;
 import net.minecraft.client.renderer.texture.TextureUtil;
@@ -12,10 +14,7 @@ import org.apache.http.Header;
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
-import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.util.EntityUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -34,48 +33,48 @@ public class ThreadDownloadImageETag extends SimpleTexture {
 
     @Nonnull
     private final File cacheFile;
+
     private final File eTagFile;
     private final String imageUrl;
+
     @Nullable
     private final IImageBuffer imageBuffer;
 
     @Nullable
     private BufferedImage bufferedImage;
+
     @Nullable
     private Thread imageThread;
     private boolean textureUploaded;
 
     public ThreadDownloadImageETag(@Nonnull File cacheFileIn, String imageUrlIn, ResourceLocation defLocation, @Nullable IImageBuffer imageBufferIn) {
         super(defLocation);
-        this.cacheFile = cacheFileIn;
+        cacheFile = cacheFileIn;
-        this.eTagFile = new File(cacheFile.getParentFile(), cacheFile.getName() + ".etag");
+        eTagFile = new File(cacheFile.getParentFile(), cacheFile.getName() + ".etag");
-        this.imageUrl = imageUrlIn;
+        imageUrl = imageUrlIn;
-        this.imageBuffer = imageBufferIn;
+        imageBuffer = imageBufferIn;
-    }
-
-    private void checkTextureUploaded() {
-        if (!this.textureUploaded) {
-            if (this.bufferedImage != null) {
-                if (this.textureLocation != null) {
-                    this.deleteGlTexture();
-                }
-
-                TextureUtil.uploadTextureImage(super.getGlTextureId(), this.bufferedImage);
-                this.textureUploaded = true;
-            }
-        }
     }
 
     public int getGlTextureId() {
-        this.checkTextureUploaded();
+        if (!textureUploaded) {
+            if (bufferedImage != null) {
+                if (textureLocation != null) {
+                    deleteGlTexture();
+                }
+
+                TextureUtil.uploadTextureImage(super.getGlTextureId(), bufferedImage);
+                textureUploaded = true;
+            }
+        }
+
         return super.getGlTextureId();
     }
 
     private void setBufferedImage(@Nonnull BufferedImage bufferedImageIn) {
-        this.bufferedImage = bufferedImageIn;
+        bufferedImage = bufferedImageIn;
 
-        if (this.imageBuffer != null) {
+        if (imageBuffer != null) {
-            this.imageBuffer.skinAvailable();
+            imageBuffer.skinAvailable();
         }
     }
 
@@ -85,24 +84,22 @@ public class ThreadDownloadImageETag extends SimpleTexture {
     }
 
     public void loadTexture(IResourceManager resourceManager) throws IOException {
-        if (this.bufferedImage == null && this.textureLocation != null) {
+        if (bufferedImage == null && textureLocation != null) {
             super.loadTexture(resourceManager);
         }
 
-        if (this.imageThread == null) {
+        if (imageThread == null) {
-            this.imageThread = new Thread(this::loadTexture, "Texture Downloader #" + THREAD_ID.incrementAndGet());
+            imageThread = new Thread(this::loadTexture, "Texture Downloader #" + THREAD_ID.incrementAndGet());
-            this.imageThread.setDaemon(true);
+            imageThread.setDaemon(true);
-            this.imageThread.start();
+            imageThread.start();
         }
     }
 
     private void loadTexture() {
-        HttpResponse response = null;
+        try (NetClient client = new NetClient("GET", imageUrl)) {
-        try {
+            CloseableHttpResponse response = client.getResponse();
-            HttpClient client = HttpClientBuilder.create().build();
+
-            response = client.execute(new HttpGet(imageUrl));
+            if (client.getResponseCode() == HttpStatus.SC_NOT_FOUND) {
-            int status = response.getStatusLine().getStatusCode();
-            if (status == HttpStatus.SC_NOT_FOUND) {
                 // delete the cache files in case we can't connect in the future
                 clearCache();
             } else if (checkETag(response)) {
@@ -132,9 +129,6 @@ public class ThreadDownloadImageETag extends SimpleTexture {
                 }
             }
             LOGGER.error("Couldn't load skin {} ", imageUrl, e);
-        } finally {
-            if (response != null)
-                EntityUtils.consumeQuietly(response.getEntity());
         }
     }
 
@@ -149,8 +143,8 @@ public class ThreadDownloadImageETag extends SimpleTexture {
     }
 
     private void clearCache() {
-        FileUtils.deleteQuietly(this.cacheFile);
+        FileUtils.deleteQuietly(cacheFile);
-        FileUtils.deleteQuietly(this.eTagFile);
+        FileUtils.deleteQuietly(eTagFile);
     }
 
     private boolean checkETag(HttpResponse response) {
@@ -158,14 +152,15 @@ public class ThreadDownloadImageETag extends SimpleTexture {
             if (cacheFile.isFile()) {
                 String localETag = Files.readFirstLine(eTagFile, Charsets.UTF_8);
                 Header remoteETag = response.getFirstHeader(HttpHeaders.ETAG);
+
                 // true if no remote etag or does match
                 return remoteETag == null || localETag.equals(remoteETag.getValue());
             }
-            return false;
         } catch (IOException e) {
-            // it failed, so re-fetch.
+
-            return false;
         }
+
+        return false; // it failed, so re-fetch.
     }
 
     private void loadTextureFromServer(HttpResponse response) {

src/hdskins/java/com/voxelmodpack/hdskins/skins/BethlehemSkinServer.java

@@ -5,6 +5,7 @@ import java.net.URI;
 import java.util.Locale;
 import java.util.Map;
 import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.http.HttpStatus;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMap.Builder;
@@ -32,14 +33,13 @@ public class BethlehemSkinServer extends AbstractSkinServer {
     @Override
     protected MinecraftTexturesPayload getProfileData(GameProfile profile) {
         try (NetClient client = new NetClient("GET", getPath(profile))) {
-            if (!client.send()) {
+            if (client.getResponseCode() == HttpStatus.SC_OK) {
-                return null;
+                return gson.fromJson(client.getResponseText(), MinecraftTexturesPayload.class);
             }
-
-            return gson.fromJson(client.getResponseText(), MinecraftTexturesPayload.class);
         } catch (IOException e) {
-            return null;
+
         }
+        return null;
     }
 
     @Override
@@ -53,11 +53,11 @@ public class BethlehemSkinServer extends AbstractSkinServer {
                 client.putFile(type.toString().toLowerCase(Locale.US), "image/png", image);
             }
 
-            if (!client.send()) {
+            if (client.getResponseCode() == HttpStatus.SC_OK) {
-                throw new IOException(client.getResponseText());
+                return new SkinUploadResponse(client.getResponseText());
             }
 
-            return new SkinUploadResponse(client.getResponseText());
+            throw new IOException(client.getResponseText());
         }
     }
 

src/hdskins/java/com/voxelmodpack/hdskins/skins/LegacySkinServer.java

@@ -13,6 +13,7 @@ import com.mojang.util.UUIDTypeAdapter;
 
 import net.minecraft.util.Session;
 import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.http.HttpStatus;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -66,7 +67,7 @@ public class LegacySkinServer extends AbstractSkinServer {
             String url = getPath(address, type, profile);
 
             try (NetClient client = new NetClient("GET", url)) {
-                if (!client.send()) {
+                if (client.getResponseCode() != HttpStatus.SC_OK) {
                     throw new IOException("Bad response code: " + client.getResponseCode());
                 }
 

src/hdskins/java/com/voxelmodpack/hdskins/skins/NetClient.java

@@ -8,7 +8,6 @@ import java.io.InputStreamReader;
 import java.net.URI;
 import java.util.Map;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpStatus;
 import org.apache.http.client.methods.CloseableHttpResponse;
@@ -18,13 +17,14 @@ import org.apache.http.entity.ContentType;
 import org.apache.http.entity.mime.MultipartEntityBuilder;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
+import org.apache.http.util.EntityUtils;
 
 /**
  * Ew. Why so many builders? >.<
  */
 public class NetClient implements Closeable {
 
-    private CloseableHttpClient client;
+    private static CloseableHttpClient client = null;
 
     private RequestBuilder rqBuilder;
 
@@ -36,18 +36,34 @@ public class NetClient implements Closeable {
         start(method, uri);
     }
 
+    /**
+     * Starts a new network request.
+     *
+     * @param method    The HTTP method verb. GET/PUT/POST/DELETE/OPTIONS
+     * @param uri       Http link to query
+     *
+     * @return Itself for chaining
+     */
     public NetClient start(String method, String uri) {
         rqBuilder = RequestBuilder.create(method).setUri(uri);
         headers = null;
 
         if (response != null) {
-            IOUtils.closeQuietly(response);
+            EntityUtils.consumeQuietly(response.getEntity());
             response = null;
         }
 
         return this;
     }
 
+    /**
+     * Adds a file to the request. Typically used with PUT/POST for uploading.
+     * @param key           Key identifier to index the file in the request.
+     * @param contentType   Type of file being sent. Usually the mime-type.
+     * @param file          The file or a link to the file.
+     *
+     * @return itself for chaining
+     */
     public NetClient putFile(String key, String contentType, URI file) {
         File f = new File(file);
         HttpEntity entity = MultipartEntityBuilder.create().addBinaryBody(key, f, ContentType.create(contentType), f.getName()).build();
@@ -57,13 +73,22 @@ public class NetClient implements Closeable {
         return this;
     }
 
+    /**
+     * Sets the headers to be included with this request.
+     * @param headers   Headers to send
+     *
+     * @return itself for chaining
+     */
     public NetClient putHeaders(Map<String, ?> headers) {
         this.headers = headers;
 
         return this;
     }
 
-    public boolean send() {
+    /**
+     * Commits and sends the request.
+     */
+    private void send() throws IOException {
         HttpUriRequest request = rqBuilder.build();
 
         if (headers != null) {
@@ -76,31 +101,36 @@ public class NetClient implements Closeable {
             client = HttpClients.createSystem();
         }
 
-        try {
+        response = client.execute(request);
-            response = client.execute(request);
-
-            return getResponseCode() == HttpStatus.SC_OK;
-        } catch (IOException e) { }
-
-        return false;
     }
 
-    public int getResponseCode() {
+    /**
+     * Gets or obtains the http response body.
+     */
+    public CloseableHttpResponse getResponse() throws IOException {
         if (response == null) {
             send();
         }
 
-        return response.getStatusLine().getStatusCode();
+        return response;
     }
 
-    public String getResponseText() {
+    /**
-        if (response == null) {
+     * Gets or obtains a response status code.
-            if (!send()) {
+     */
-                return "";
+    public int getResponseCode() throws IOException {
-            }
+        return getResponse().getStatusLine().getStatusCode();
+    }
+
+    /**
+     * Consumes and returns the entire response body.
+     */
+    public String getResponseText() throws IOException {
+        if (getResponse().getEntity() == null) {
+            return "";
         }
 
-        try (BufferedReader reader = new BufferedReader(new InputStreamReader(response.getEntity().getContent()))) {
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(getResponse().getEntity().getContent()))) {
             StringBuilder builder = new StringBuilder();
 
             int ch;
@@ -109,23 +139,17 @@ public class NetClient implements Closeable {
             }
 
             return builder.toString();
-        } catch (IOException e) {
-
         }
-
-        return "";
     }
 
     @Override
     public void close() throws IOException {
-        if (response != null) {
+        try {
-            IOUtils.closeQuietly(response);
+            if (response != null) {
+                response.close();
+            }
+        } finally {
             response = null;
         }
-
-        if (client != null) {
-            IOUtils.closeQuietly(client);
-            client = null;
-        }
     }
 }