[client] Clean up update checker, handle NPEs, close streams

1 files changed, 76 insertions, 84 deletions

diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/util/ClientVersion.java b/dozentenmodul/src/main/java/org/openslx/dozmod/util/ClientVersion.java
index 9a124b17..4582e4e3 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/util/ClientVersion.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/util/ClientVersion.java
@@ -1,117 +1,114 @@
 package org.openslx.dozmod.util;
 
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStreamReader;
+import java.io.InputStream;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.jar.Attributes;
 import java.util.jar.Manifest;
 
+import org.apache.commons.io.output.ByteArrayOutputStream;
 import org.apache.log4j.Logger;
-
-import com.google.gson.Gson;
+import org.openslx.util.Json;
 
 // ClientVersion is a bad name, TODO find better suited one :)
 public class ClientVersion {
 
 	private static final Logger LOGGER = Logger.getLogger(ClientVersion.class);
-	
-	private static Long localRevisionTime = null;
-	private static Long remoteRevisionTime = null;
+
+	private static long localRevisionTime = 0;
+	private static long remoteRevisionTime = 0;
 	private static String localRevision = null;
 	private static String remoteRevision = null;
-	
+
 	private static void init() {
-		loadLocalVersion();
-		loadRemoteVersion("http://132.230.4.25/dozmod.version");
+		if (localRevision == null) {
+			loadLocalVersion();
+		}
+		if (localRevision == null) {
+			localRevision = "???";
+		}
+		if (remoteRevision == null) {
+			loadRemoteVersion("http://132.230.4.25/dozmod.version");
+		}
+		if (remoteRevision == null) {
+			remoteRevision = "???";
+		}
 	}
 
-
 	/**
-	 * Gets the local revision id if loading it worked, "???" otherwise. 
+	 * Gets the local revision id if loading it worked, "???" otherwise.
 	 * 
 	 * @return id as String
 	 */
 	public static String getLocalRevision() {
-		if (localRevision == null) {
-			init();
-			if (localRevision == null)
-				// fallback for dev purposes...
-				localRevision = "???";
-		}
+		init();
 		return localRevision;
 	}
+
 	/**
 	 * @return
 	 */
 	public static long getLocalRevTimestamp() {
-		if (localRevisionTime == null) {
-			init();
-			if (localRevisionTime == null)
-				// fallback for dev purposes...
-				localRevisionTime = 0L;
-		}
+		init();
 		return localRevisionTime;
 	}
+
 	/**
 	 * Gets the revision id of the latest remote version
 	 * 
-	 * @return id as String if loading worked, "-" otherwise
+	 * @return id as String if loading worked, "???" otherwise
 	 */
 	public static String getRemoteRevision() {
-		if (remoteRevision == null) {
-			init();
-			if (remoteRevision == null)
-				remoteRevision = "-";
-		}
+		init();
 		return remoteRevision;
 	}
+
 	/**
 	 * Gets the timestamp of the latest remote version
 	 * 
 	 * @return timestamp as Long if loading it worked, 0L otherwise
 	 */
 	public static long getRemoteRevTimestamp() {
-		if (remoteRevisionTime == null) {
-			init();
-			if (remoteRevisionTime == null)
-				return 0L;
-		}
+		init();
 		return remoteRevisionTime;
 	}
+
 	/**
-	 * Checks if we are running latest bwSuite version 
+	 * Checks if we are running latest bwSuite version
 	 * 
 	 * @return true if there is no newer version, false otherwise
 	 */
 	public static boolean isNewest() {
-		if (localRevisionTime == null || remoteRevisionTime == null)
-			init();
-		// fallback if the above did not work: fail
-		if (localRevisionTime == null || remoteRevisionTime == null)
+		init();
+		// if either local or remote version is unknown, just pretend there's no update
+		// as there most likely isn't and we'd just annoy the user
+		// TODO: Report "fail" state so at least on manual update check we can tell that it failed
+		if (localRevisionTime == 0 || remoteRevisionTime == 0)
 			return true;
 		return localRevisionTime >= remoteRevisionTime;
 	}
+
 	/**
 	 * Loads the local version information from the jar's MANIFEST.MF
 	 * into the fields 'localRevision' and 'localRevisionTime'
 	 */
 	private static void loadLocalVersion() {
-		Class clazz = ClientVersion.class;
+		Class<ClientVersion> clazz = ClientVersion.class;
 		String className = clazz.getSimpleName() + ".class";
 		String classPath = clazz.getResource(className).toString();
 		if (!classPath.startsWith("jar")) {
 			// Class not from JAR
 			return;
 		}
-		String manifestPath = classPath.substring(0, classPath.lastIndexOf("!") + 1) + 
-		    "/META-INF/MANIFEST.MF";
+		String manifestPath = classPath.replaceAll("![^!]*$", "") + "/META-INF/MANIFEST.MF";
 		Manifest manifest = null;
-		try {
-			manifest = new Manifest(new URL(manifestPath).openStream());
+		try (InputStream stream = new URL(manifestPath).openStream()) {
+			manifest = new Manifest(stream);
 		} catch (Exception e) {
-			LOGGER.error("Could not open MANIFEST", e);
-			return;
+			if (manifest == null) {
+				LOGGER.error("Could not open MANIFEST", e);
+				return;
+			}
 		}
 		Attributes attributes = manifest.getMainAttributes();
 		// if attr are null, then we couldn't open the jar's MANIFEST
@@ -122,64 +119,59 @@ public class ClientVersion {
 		String manifestRevTime = null;
 		try {
 			manifestRev = attributes.getValue("Build-Revision");
+		} catch (Exception e) {
+			LOGGER.warn("Error while reading revision: ", e);
+		}
+		try {
 			manifestRevTime = attributes.getValue("Build-Revision-Timestamp");
-		} catch (IllegalArgumentException e) {
-			LOGGER.warn("Error while reading version: ", e);
-			return;
+		} catch (Exception e) {
+			LOGGER.warn("Error while reading timestamp: ", e);
 		}
-		if (manifestRev != null)
+		if (manifestRev != null) {
 			localRevision = manifestRev;
+		}
 		if (manifestRevTime != null) {
-			localRevisionTime = Long.valueOf(manifestRevTime);
-			// hax since we get milliseconds not seconds
-			localRevisionTime = localRevisionTime / 1000L; 
+			try {
+				// hax since we get milliseconds not seconds
+				localRevisionTime = Long.valueOf(manifestRevTime) / 1000L;
+			} catch (NumberFormatException e) {
+				LOGGER.warn("Build timestamp is not a number!", e);
+			}
 		}
 	}
+
 	/**
-	 * Loads the given UrlString as JSON and saves the remote information 
+	 * Loads the given UrlString as JSON and saves the remote information
 	 * into fields 'remoteRevision' and 'remoteRevisionTime'
 	 * 
 	 * The remote JSON should have 'timestamp' and 'revision', like:
-	 * 		{ "timestamp": 1, "revision": 2 }
+	 * { "timestamp": 1, "revision": 2 }
 	 */
 	private static void loadRemoteVersion(final String urlString) {
-		Gson gson = new Gson();
 		String json = null;
-		BufferedReader reader = null;
-		try {
-			URL url = new URL(urlString);
-			reader = new BufferedReader(new InputStreamReader(url.openStream()));
-			StringBuffer buffer = new StringBuffer();
-			int read;
-			char[] chars = new char[1024];
-			while ((read = reader.read(chars)) != -1)
-				buffer.append(chars, 0, read); 
-			json = buffer.toString();
+		try (InputStream reader = new URL(urlString).openStream()) {
+			ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+			buffer.write(reader);
+			buffer.close();
+			json = new String(buffer.toByteArray(), StandardCharsets.UTF_8);
 		} catch (Exception e) {
-			LOGGER.error("Could not fetch remote version", e);
-			return;
-		} finally {
-			if (reader != null)
-				try {
-					reader.close();
-				} catch (IOException e) {
-				}
+			if (json == null) {
+				LOGGER.error("Could not fetch remote version", e);
+				return;
+			}
 		}
 
-		VersionQuery query = gson.fromJson(json, VersionQuery.class);
-		if (query.revision != null)
-			remoteRevision = query.revision;
-		if (query.timestamp != null) {
-			// seconds timestamp here...
-			remoteRevisionTime = query.timestamp;
-		}
+		VersionQuery query = Json.deserialize(json, VersionQuery.class);
+		remoteRevision = query.revision;
+		// seconds timestamp here...
+		remoteRevisionTime = query.timestamp;
 	}
 
 	/**
 	 * Class for GSON json parsing
 	 */
 	static class VersionQuery {
-		Long timestamp;
+		long timestamp;
 		String revision;
 	}
 }