Fix resuming and caching of incomplete downloads (#28)

- don't cache an incomplete download - verify that the incomplete download hasn't changed on the server since we last tried the download. If it did we do a full download. - improve status messages
ermshiperete · Oct 20, 2017 · c04e8a3 · c04e8a3
1 parent 4cf8eea
commit c04e8a3
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 33 deletions.
diff --git a/BuildDependencyLib/Artifacts/DownloadFileJob.cs b/BuildDependencyLib/Artifacts/DownloadFileJob.cs
@@ -49,18 +49,9 @@ public async Task<bool> Execute(ILog log, string workDir)
 			if (!Conditions.AreTrue())
 				return true;
 
-//			string httpUsername;
-//			string httpPassword;
-
-			// Assign values to these objects here so that they can
-			// be referenced in the finally block
-			Stream remoteStream = null;
-			Stream localStream = null;
-			HttpWebResponse response = null;
 			var lastModified = DateTime.Now;
 
 			var targetFile = Path.Combine(workDir, TargetFile);
-			var tmpTargetFile = targetFile + ".~tmp";
 
 			bool gotCachedFile = false;
 			if (FileCache.Enabled)
@@ -69,7 +60,7 @@ public async Task<bool> Execute(ILog log, string workDir)
 				if (File.Exists(cachedFile))
 				{
 					gotCachedFile = true;
-					log.LogMessage($"Found cached file {targetFile}");
+					log.LogMessage($"Found cached file {TargetFile}");
 					if (File.Exists(targetFile))
 					{
 						var targetFileInfo = new FileInfo(targetFile);
@@ -88,12 +79,34 @@ public async Task<bool> Execute(ILog log, string workDir)
 				return gotCachedFile;
 			}
 
-			log.LogMessage("Checking {0}, downloading if newer...", TargetFile);
+			var success = await DownloadFile(log, targetFile);
+
+			if (File.Exists(targetFile))
+				new FileInfo(targetFile) {LastWriteTime = lastModified};
+
+			if (success)
+				log.LogMessage("Download of {0} finished after {1}.", TargetFile, DateTime.Now - lastModified);
+
+			return success;
+		}
+
+		private async Task<bool> DownloadFile(ILog log, string targetFile)
+		{
+//			string httpUsername;
+//			string httpPassword;
+
+			// Assign values to these objects here so that they can
+			// be referenced in the finally block
+			Stream remoteStream = null;
+			Stream localStream = null;
+			HttpWebResponse response = null;
 
 			// Use a try/catch/finally block as both the WebRequest and Stream
 			// classes throw exceptions upon error
 			try
 			{
+				var tmpTargetFile = targetFile + ".~tmp";
+
 				// Create a request for the specified remote file name
 				var request = WebRequest.Create(Url) as HttpWebRequest;
 //				// If a username or password have been given, use them
@@ -104,27 +117,48 @@ public async Task<bool> Execute(ILog log, string workDir)
 //					request.Credentials = new NetworkCredential(username, password);
 //				}
 
+				// REVIEW: would it be better to use ETag in the HTTP header instead of relying
+				// on the timestamp for caching and continuing incomplete downloads?
+
 				bool appendFile = false;
 				long tmpFileLength = 0;
-				if (File.Exists(targetFile))
-				{
-					var fi = new FileInfo(targetFile);
-					request.IfModifiedSince = fi.LastWriteTimeUtc;
-				}
 				if (File.Exists(tmpTargetFile))
 				{
 					// Interrupted download
+					log.LogMessage("Found incomplete download file, continuing download of {0}...", TargetFile);
+
 					var fi = new FileInfo(tmpTargetFile);
+					request.Headers.Add("If-Unmodified-Since", fi.LastWriteTimeUtc.ToString("r"));
 					tmpFileLength = fi.Length;
 					request.AddRange(tmpFileLength);
 					appendFile = true;
 				}
+				else if (File.Exists(targetFile))
+				{
+					log.LogMessage("Checking {0}, downloading if newer...", TargetFile);
+
+					var fi = new FileInfo(targetFile);
+					request.IfModifiedSince = fi.LastWriteTimeUtc;
+				}
+				else
+					log.LogMessage("Downloading {0}...", TargetFile);
 
 				// Send the request to the server and retrieve the
 				// WebResponse object
 				response = (HttpWebResponse) await Task.Factory.FromAsync(
 					request.BeginGetResponse, request.EndGetResponse, null);
 
+				if (File.Exists(tmpTargetFile) && (response.StatusCode == HttpStatusCode.PreconditionFailed ||
+					response.LastModified > new FileInfo(tmpTargetFile).LastWriteTimeUtc))
+				{
+					// file got changed on the server since we downloaded the incomplete file
+					log.LogMessage("File {0} changed on server since start of incomplete download; initiating complete download",
+						TargetFile);
+					File.Delete(tmpTargetFile);
+					response.Close();
+					return await DownloadFile(log, targetFile);
+				}
+
 				// Once the WebResponse object has been retrieved,
 				// get the stream object associated with the response's data
 				remoteStream = response.GetResponseStream();
@@ -150,11 +184,24 @@ public async Task<bool> Execute(ILog log, string workDir)
 					localStream.Write(buffer, 0, bytesRead);
 				} while (bytesRead > 0);
 
+				var localStreamLength = localStream.Length;
 				localStream.Close();
+
+				if (localStreamLength != response.ContentLength + tmpFileLength)
+				{
+					log.LogMessage(LogMessageImportance.High,
+						"WARNING: couldn't download complete file {0}, continuing next time", TargetFile);
+					log.LogMessage(LogMessageImportance.Low,
+						"{2}: Expected file length: {0}, but received {1} bytes",
+						response.ContentLength + tmpFileLength, localStreamLength, TargetFile);
+					return false;
+				}
+
 				File.Delete(targetFile);
 				File.Move(tmpTargetFile, targetFile);
 				if (FileCache.Enabled)
 					FileCache.CacheFile(Url, targetFile);
+				return true;
 			}
 			catch (WebException wex)
 			{
@@ -207,16 +254,8 @@ public async Task<bool> Execute(ILog log, string workDir)
 				// is thrown at some point
 				response?.Close();
 				remoteStream?.Close();
-				if (localStream != null)
-				{
-					localStream.Close();
-					var fi = new FileInfo(targetFile) { LastWriteTime = lastModified };
-				}
+				localStream?.Close();
 			}
-
-			log.LogMessage("Download of {0} finished after {1}.", TargetFile, DateTime.Now - lastModified);
-
-			return true;
 		}
 
 		private static void CopyCachedFile(ILog log, string cachedFile, string targetFile)

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,19 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-[comment]: <> (Available types of changes:
-### Added
-### Changed
+## [Unreleased]
+
 ### Fixed
-### Deprecated
-### Removed
-### Security
-)
 
-## [Unreleased]
+## [0.4.1] - 2017-10-20
 
 ### Fixed
 
+- Resuming and caching of incomplete downloads (#28)
+
 ## [0.4.0] - 2017-10-19
 
 ### Added
@@ -56,3 +53,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 - Improve error message if TC project name changed (#19)
+
+[comment]: <> (Available types of changes:
+### Added
+### Changed
+### Fixed
+### Deprecated
+### Removed
+### Security
+)
diff --git a/Samples/BuildDependencyTestApp/Program.cs b/Samples/BuildDependencyTestApp/Program.cs
@@ -23,6 +23,7 @@ public static void Main(string[] args)
 				UseDependencyFile = true,
 				KeepJobsFile = true,
 				RunAsync = true,
+				UseCache = true,
 				// Platform = "x64",
 			};
 			Directory.CreateDirectory(task.WorkingDir);