Minor cleanup/optimisations to downloading, should help a little bit more with asset downloading.

Closes #359

5 files changed, 48 insertions, 38 deletions

diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java
index e25b8ca5..508e8c3e 100644
--- a/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java
+++ b/src/main/java/net/fabricmc/loom/configuration/providers/MinecraftProvider.java
@@ -102,8 +102,8 @@ public class MinecraftProvider extends DependencyProvider {
 			try {
 				mergeJars(getProject().getLogger());
 			} catch (ZipError e) {
-				DownloadUtil.delete(minecraftClientJar);
-				DownloadUtil.delete(minecraftServerJar);
+				HashedDownloadUtil.delete(minecraftClientJar);
+				HashedDownloadUtil.delete(minecraftServerJar);
 
 				getProject().getLogger().error("Could not merge JARs! Deleting source JARs - please re-run the command and move on.", e);
 				throw new RuntimeException();
diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java
index 96c4c307..62879458 100644
--- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java
+++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MojangMappingsDependency.java
@@ -56,7 +56,7 @@ import org.zeroturnaround.zip.ZipUtil;
 import net.fabricmc.loom.LoomGradleExtension;
 import net.fabricmc.loom.LoomGradlePlugin;
 import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta;
-import net.fabricmc.loom.util.DownloadUtil;
+import net.fabricmc.loom.util.HashedDownloadUtil;
 import net.fabricmc.lorenztiny.TinyMappingsReader;
 import net.fabricmc.mapping.tree.TinyMappingFactory;
 
@@ -127,11 +127,11 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 			throw new RuntimeException("Failed to find official mojang mappings for " + getVersion());
 		}
 
-		String clientMappingsUrl = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS).getUrl();
-		String serverMappingsUrl = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS).getUrl();
+		MinecraftVersionMeta.Download clientMappingsDownload = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS);
+		MinecraftVersionMeta.Download serverMappingsDownload = versionInfo.getDownload(MANIFEST_CLIENT_MAPPINGS);
 
-		DownloadUtil.downloadIfChanged(new URL(clientMappingsUrl), clientMappings.toFile(), project.getLogger());
-		DownloadUtil.downloadIfChanged(new URL(serverMappingsUrl), serverMappings.toFile(), project.getLogger());
+		HashedDownloadUtil.downloadIfInvalid(new URL(clientMappingsDownload.getUrl()), clientMappings.toFile(), clientMappingsDownload.getSha1(), project.getLogger(), false);
+		HashedDownloadUtil.downloadIfInvalid(new URL(serverMappingsDownload.getUrl()), serverMappings.toFile(), clientMappingsDownload.getSha1(), project.getLogger(), false);
 
 		MappingSet mappings = MappingSet.create();
 
diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java
index 64722c60..97e8503c 100644
--- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java
+++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/MinecraftAssetsProvider.java
@@ -105,35 +105,39 @@ public class MinecraftAssetsProvider {
 				}
 			} else {
 				executor.execute(() -> {
-					ProgressLogger progressLogger;
-
-					if (loggers.isEmpty()) {
-						//Create a new logger if we need one
-						progressLogger = ProgressLogger.getProgressFactory(project, MinecraftAssetsProvider.class.getName());
-						progressLogger.start("Downloading assets...", "assets");
-					} else {
-						// use a free logger if we can
-						progressLogger = loggers.pop();
-					}
-
-					String assetName = entry.getKey();
-					int end = assetName.lastIndexOf("/") + 1;
+					final String[] assetName = {entry.getKey()};
+					int end = assetName[0].lastIndexOf("/") + 1;
 
 					if (end > 0) {
-						assetName = assetName.substring(end);
+						assetName[0] = assetName[0].substring(end);
 					}
 
-					project.getLogger().debug(":downloading asset " + assetName);
-					progressLogger.progress(String.format("%-30.30s", assetName) + " - " + sha1);
+					project.getLogger().debug("validating asset " + assetName[0]);
+
+					final ProgressLogger[] progressLogger = new ProgressLogger[1];
 
 					try {
-						HashedDownloadUtil.downloadIfInvalid(new URL(Constants.RESOURCES_BASE + sha1.substring(0, 2) + "/" + sha1), file, sha1, project.getLogger(), true);
+						HashedDownloadUtil.downloadIfInvalid(new URL(Constants.RESOURCES_BASE + sha1.substring(0, 2) + "/" + sha1), file, sha1, project.getLogger(), true, () -> {
+							if (loggers.isEmpty()) {
+								//Create a new logger if we need one
+								progressLogger[0] = ProgressLogger.getProgressFactory(project, MinecraftAssetsProvider.class.getName());
+								progressLogger[0].start("Downloading assets...", "assets");
+							} else {
+								// use a free logger if we can
+								progressLogger[0] = loggers.pop();
+							}
+
+							project.getLogger().debug("downloading asset " + assetName[0]);
+							progressLogger[0].progress(String.format("%-30.30s", assetName[0]) + " - " + sha1);
+						});
 					} catch (IOException e) {
-						throw new RuntimeException("Failed to download: " + assetName, e);
+						throw new RuntimeException("Failed to download: " + assetName[0], e);
 					}
 
-					//Give this logger back
-					loggers.add(progressLogger);
+					if (progressLogger[0] != null) {
+						//Give this logger back if we used it
+						loggers.add(progressLogger[0]);
+					}
 				});
 			}
 		}
diff --git a/src/main/java/net/fabricmc/loom/util/DownloadUtil.java b/src/main/java/net/fabricmc/loom/util/DownloadUtil.java
index 9bb2b7e2..90131b4c 100644
--- a/src/main/java/net/fabricmc/loom/util/DownloadUtil.java
+++ b/src/main/java/net/fabricmc/loom/util/DownloadUtil.java
@@ -89,6 +89,7 @@ public class DownloadUtil {
 
 		if ((code < 200 || code > 299) && code != HttpURLConnection.HTTP_NOT_MODIFIED) {
 			//Didn't get what we expected
+			delete(to);
 			throw new IOException(connection.getResponseMessage() + " for " + from);
 		}
 
@@ -111,7 +112,7 @@ public class DownloadUtil {
 		try { // Try download to the output
 			FileUtils.copyInputStreamToFile(connection.getInputStream(), to);
 		} catch (IOException e) {
-			to.delete(); // Probably isn't good if it fails to copy/save
+			delete(to); // Probably isn't good if it fails to copy/save
 			throw e;
 		}
 
diff --git a/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java b/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java
index da294b8e..86cf80a3 100644
--- a/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java
+++ b/src/main/java/net/fabricmc/loom/util/HashedDownloadUtil.java
@@ -25,6 +25,7 @@
 package net.fabricmc.loom.util;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.URL;
@@ -41,19 +42,23 @@ import net.fabricmc.loom.LoomGradlePlugin;
 
 public class HashedDownloadUtil {
 	public static void downloadIfInvalid(URL from, File to, String expectedHash, Logger logger, boolean quiet) throws IOException {
+		downloadIfInvalid(from, to, expectedHash, logger, quiet, () -> { });
+	}
+
+	public static void downloadIfInvalid(URL from, File to, String expectedHash, Logger logger, boolean quiet, Runnable startDownload) throws IOException {
 		if (LoomGradlePlugin.refreshDeps) {
 			delete(to);
 		}
 
-		if (to.exists()) {
-			String sha1 = getSha1(to, logger);
+		String sha1 = getSha1(to, logger);
 
-			if (expectedHash.equals(sha1)) {
-				// The hash in the sha1 file matches
-				return;
-			}
+		if (expectedHash.equals(sha1)) {
+			// The hash in the sha1 file matches
+			return;
 		}
 
+		startDownload.run();
+
 		HttpURLConnection connection = (HttpURLConnection) from.openConnection();
 		connection.setRequestProperty("Accept-Encoding", "gzip");
 		connection.connect();
@@ -62,6 +67,7 @@ public class HashedDownloadUtil {
 
 		if ((code < 200 || code > 299) && code != HttpURLConnection.HTTP_NOT_MODIFIED) {
 			//Didn't get what we expected
+			delete(to);
 			throw new IOException(connection.getResponseMessage() + " for " + from);
 		}
 
@@ -96,12 +102,11 @@ public class HashedDownloadUtil {
 	private static String getSha1(File to, Logger logger) {
 		File sha1File = getSha1File(to);
 
-		if (!sha1File.exists()) {
-			return null;
-		}
-
 		try {
 			return Files.asCharSource(sha1File, StandardCharsets.UTF_8).read();
+		} catch (FileNotFoundException ignored) {
+			// Quicker to catch this than do an exists check before.
+			return null;
 		} catch (IOException e) {
 			logger.warn("Error reading sha1 file '{}'.", sha1File);
 			return null;