🎨 Tidy up date parsing logic and make it more robust

Author: pmonks

src/main/java/org/spdx/storage/listedlicense/SpdxListedLicenseWebStore.java

@@ -38,6 +38,7 @@
 import java.time.Instant;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
 import java.time.temporal.ChronoUnit;
 import java.util.Arrays;
 import java.util.Base64;
@@ -152,13 +153,7 @@ private InputStream getUrlInputStreamThroughCache(final URL url) throws IOExcept
 
 		if (cachedFile.exists() && cachedMetadataFile.exists()) {
 			try {
-				final HashMap<String,String> cachedMetadata = readMetadataFile(cachedMetadataFile);
-
-				if (cachedMetadata != null) {
-					checkCache(cachedMetadataFile, cachedMetadata, url);
-				} else {
-					cacheMiss(url);
-				}
+				checkCache(url);
 			} catch (IOException ioe) {
 				// We know we have a locally cached file here, so if we happen to get an exception we can safely ignore
 				// it and fall back on the (possibly stale) cached content file.  This makes the code more robust in the
@@ -234,31 +229,57 @@ private void writeContentFile(final InputStream urlInputStream, final File cache
 		}
 	}
 
-	private void checkCache(final File cachedMetadataFile, final HashMap<String,String> cachedMetadata, final URL url) throws IOException {
-		final Instant lastChecked = Instant.parse(cachedMetadata.get("lastChecked"));
-        final long    difference  = Math.abs(ChronoUnit.SECONDS.between(Instant.now(), lastChecked));
+	/**
+	 * Attempts to parse s as if it were an ISO8601 formatted String.
+	 * @param s The string to attempt to parse.
+	 * @return The Instant for that ISO8601 value if parsing succeeded, or null if it didn't.
+	 */
+	private final Instant parseISO8601String(final String s) {
+		Instant result = null;
+		if (s != null) {
+			try {
+				result = Instant.parse(s);
+			} catch (final DateTimeParseException dtpe) {
+				result = null;
+			}
+		}
+		return result;
+	}
 
-		if (difference > cacheCheckIntervalSecs) {
-			// It's been a while since we checked the cached download of this URL for staleness, so make an ETag request
-			logger.debug("Outside cache check interval; checking for updates to " + String.valueOf(url));
-			final String            eTag       = cachedMetadata.get("eTag");
-			final HttpURLConnection connection = (HttpURLConnection) url.openConnection();
-			connection.setReadTimeout(READ_TIMEOUT);
-			connection.setRequestProperty("If-None-Match", eTag);
-			final int status = connection.getResponseCode();
-			if (status != HttpURLConnection.HTTP_NOT_MODIFIED) {
-				// The content of the URL has changed, which we handle the same as a cache miss (i.e. we re-download
-				// the content, and write a new metadata file from scratch)
-				cacheMiss(url, connection);
+	private void checkCache(final URL url) throws IOException {
+		final String                 cacheKey           = base64Encode(url);
+		final File                   cachedMetadataFile = new File(cacheDir, cacheKey + ".metadata.json");
+		final HashMap<String,String> cachedMetadata     = readMetadataFile(cachedMetadataFile);
+
+		if (cachedMetadata != null) {
+			final Instant lastChecked = parseISO8601String(cachedMetadata.get("lastChecked"));
+			final long    difference  = lastChecked  != null ? Math.abs(ChronoUnit.SECONDS.between(Instant.now(), lastChecked)) : Long.MAX_VALUE;
+
+			if (difference > cacheCheckIntervalSecs) {
+				// It's been a while since we checked the cached download of this URL for staleness, so make an ETag request
+				logger.debug("Cache check interval exceeded; checking for updates to " + String.valueOf(url));
+				final String eTag = cachedMetadata.get("eTag");
+				final HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+				connection.setReadTimeout(READ_TIMEOUT);
+				connection.setRequestProperty("If-None-Match", eTag);
+				final int status = connection.getResponseCode();
+				if (status != HttpURLConnection.HTTP_NOT_MODIFIED) {
+					// The content of the URL has changed, which we handle the same as a cache miss (i.e. we re-download
+					// the content, and write a new metadata file from scratch)
+					cacheMiss(url, connection);
+				} else {
+					// The content hasn't changed, so just update the lastChecked metadata but otherwise do nothing
+					logger.debug("Cache hit for " + String.valueOf(url));
+					cachedMetadata.put("lastChecked", iso8601.format(Instant.now()));
+					writeMetadataFile(cachedMetadataFile, cachedMetadata);
+				}
 			} else {
-				// The content hasn't changed, so just update the lastChecked metadata but otherwise do nothing
-				logger.debug("Cache hit for " + String.valueOf(url));
-				cachedMetadata.put("lastChecked", iso8601.format(Instant.now()));
-				writeMetadataFile(cachedMetadataFile, cachedMetadata);
+				// We checked recently, so don't need to do anything - the cached content will be used
+				logger.debug("Within cache check interval; skipping check of updates to " + String.valueOf(url));
 			}
 		} else {
-			// We checked recently, so don't need to do anything - the cached content will be used
-			logger.debug("Within cache check interval; skipping check of updates to " + String.valueOf(url));
+			// Metadata doesn't exist - treat it as a cache miss
+			cacheMiss(url);
 		}
 	}