Package Kit: re-use downloads from unfinished transactions

There are three parts to this change:
- In FetchFileJob, if the request fails with a timeout or IO error
  (probably because of unstable connection) attempt to resume the
  download with a range request. No limit on number of retries
  currently, maybe we should add one.
- In PackageManager, before downloading a file, look around in other
  transaction directories in case it's already there. Partial and
  complete downloads are differentiated by an attribute which the
  fetch file job maintains. For complete downloads, no fetch job is
  scheduled, for partial downloads, the fetch job will request the
  remainder of the file.
- In BHttpRequest, the implementation of SetRangeStart() and
  SetRangeEnd() have been added, along with some refactoring to
  handle listener notifications consistently. This also fixed a
  bug where the final notification for download progress was not
  emitted for compressed data.

Fixes #12414.

Change-Id: I3e285741ed0e5651594a7c2e1c7170644a9d297d
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3404
Reviewed-by: Stephan Aßmus <[email protected]>
Reviewed-by: Alex von Gluck IV <[email protected]>

headers/os/net/HttpRequest.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2013 Haiku Inc. All rights reserved.
+ * Copyright 2010-2021 Haiku Inc. All rights reserved.
  * Distributed under the terms of the MIT License.
  */
 #ifndef _B_URL_PROTOCOL_HTTP_H_
@@ -95,6 +95,11 @@ private:
 	// Utility methods
 			bool				_IsDefaultPort();
 
+	// Listener notification
+			void				_NotifyDataReceived(const char* data,
+									off_t pos, ssize_t length,
+									off_t bytesReceived, ssize_t bytesTotal);
+
 private:
 			bool				fSSL;
 

src/kits/network/libnetapi/HttpRequest.cpp

@@ -1,11 +1,12 @@
 /*
- * Copyright 2010-2015 Haiku Inc. All rights reserved.
+ * Copyright 2010-2021 Haiku Inc. All rights reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
  *		Christophe Huriaux, [email protected]
  *		Niels Sascha Reedijk, [email protected]
  *		Adrien Destugues, [email protected]
+ *		Stephan Aßmus, [email protected]
  */
 
 
@@ -209,17 +210,36 @@ BHttpRequest::SetAutoReferrer(bool enable)
 
 
 void
-BHttpRequest::SetHeaders(const BHttpHeaders& headers)
+BHttpRequest::SetUserName(const BString& name)
 {
-	AdoptHeaders(new(std::nothrow) BHttpHeaders(headers));
+	fOptUsername = name;
 }
 
 
 void
-BHttpRequest::AdoptHeaders(BHttpHeaders* const headers)
+BHttpRequest::SetPassword(const BString& password)
 {
-	delete fOptHeaders;
+	fOptPassword = password;
-	fOptHeaders = headers;
+}
+
+
+void
+BHttpRequest::SetRangeStart(off_t position)
+{
+	// This field is used within the transfer loop, so only
+	// allow setting it before sending the request.
+	if (fRequestStatus == kRequestInitialState)
+		fOptRangeStart = position;
+}
+
+
+void
+BHttpRequest::SetRangeEnd(off_t position)
+{
+	// This field could be used in the transfer loop, so only
+	// allow setting it before sending the request.
+	if (fRequestStatus == kRequestInitialState)
+		fOptRangeEnd = position;
 }
 
 
@@ -230,6 +250,13 @@ BHttpRequest::SetPostFields(const BHttpForm& fields)
 }
 
 
+void
+BHttpRequest::SetHeaders(const BHttpHeaders& headers)
+{
+	AdoptHeaders(new(std::nothrow) BHttpHeaders(headers));
+}
+
+
 void
 BHttpRequest::AdoptPostFields(BHttpForm* const fields)
 {
@@ -251,16 +278,10 @@ BHttpRequest::AdoptInputData(BDataIO* const data, const ssize_t size)
 
 
 void
-BHttpRequest::SetUserName(const BString& name)
+BHttpRequest::AdoptHeaders(BHttpHeaders* const headers)
 {
-	fOptUsername = name;
+	delete fOptHeaders;
-}
+	fOptHeaders = headers;
-
-
-void
-BHttpRequest::SetPassword(const BString& password)
-{
-	fOptPassword = password;
 }
 
 
@@ -740,17 +761,14 @@ BHttpRequest::_MakeRequest()
 						ssize_t size = decompressorStorage.Size();
 						BStackOrHeapArray<char, 4096> buffer(size);
 						size = decompressorStorage.Read(buffer, size);
-						if (size > 0) {
+						_NotifyDataReceived(buffer, bytesUnpacked, size,
-							fListener->DataReceived(this, buffer, bytesUnpacked,
+							bytesReceived, bytesTotal);
-								size);
 						bytesUnpacked += size;
-						}
 					} else if (bytesRead > 0) {
-						fListener->DataReceived(this, inputTempBuffer,
+						_NotifyDataReceived(inputTempBuffer,
-							bytesReceived - bytesRead, bytesRead);
+							bytesReceived - bytesRead, bytesRead,
+							bytesReceived, bytesTotal);
 					}
-					fListener->DownloadProgress(this, bytesReceived,
-						std::max((off_t)0, bytesTotal));
 				}
 
 				if (bytesTotal >= 0 && bytesReceived >= bytesTotal)
@@ -768,14 +786,12 @@ BHttpRequest::_MakeRequest()
 					ssize_t size = decompressorStorage.Size();
 					BStackOrHeapArray<char, 4096> buffer(size);
 					size = decompressorStorage.Read(buffer, size);
-					if (fListener != NULL && size > 0) {
+					_NotifyDataReceived(buffer, bytesUnpacked, size,
-						fListener->DataReceived(this, buffer,
+						bytesReceived, bytesTotal);
-							bytesUnpacked, size);
 					bytesUnpacked += size;
 				}
 			}
 		}
-		}
 
 		parseEnd = (fInputBuffer.Size() == 0);
 	}
@@ -908,6 +924,20 @@ BHttpRequest::_SerializeHeaders()
 	if (fOptReferer.CountChars() > 0)
 		outputHeaders.AddHeader("Referer", fOptReferer.String());
 
+	// Optional range requests headers
+	if (fOptRangeStart != -1 || fOptRangeEnd != -1) {
+		if (fOptRangeStart == -1)
+			fOptRangeStart = 0;
+		BString range;
+		if (fOptRangeEnd != -1) {
+			range.SetToFormat("bytes=%" B_PRIdOFF "-%" B_PRIdOFF,
+				fOptRangeStart, fOptRangeEnd);
+		} else {
+			range.SetToFormat("bytes=%" B_PRIdOFF "-", fOptRangeStart);
+		}
+		outputHeaders.AddHeader("Range", range.String());
+	}
+
 	// Authentication
 	if (fContext != NULL) {
 		BHttpAuthentication& authentication = fContext->GetAuthentication(fUrl);
@@ -1158,3 +1188,29 @@ BHttpRequest::_IsDefaultPort()
 }
 
 
+void
+BHttpRequest::_NotifyDataReceived(const char* data, off_t pos, ssize_t size,
+	off_t bytesReceived, ssize_t bytesTotal)
+{
+	if (fListener == NULL || size <= 0)
+		return;
+	if (fOptRangeStart > 0) {
+		pos += fOptRangeStart;
+		// bytesReceived and bytesTotal refer to the requested range,
+		// so that should technically not be adjusted for the range start.
+		// For displaying progress to the user, this is not ideal, though.
+		// But only for the case where we request the remainder of a file.
+		// Range requests can also be used to request any portion of a
+		// resource, so not modifying them is technically more correct.
+		// We can use a little trick, though: We know when the remainder
+		// is requested, because then fOptRangeEnd is -1.
+		if (fOptRangeEnd == -1) {
+			bytesReceived += fOptRangeStart;
+			if (bytesTotal > 0)
+				bytesTotal += fOptRangeStart;
+		}
+	}
+	fListener->DataReceived(this, data, pos, size);
+	fListener->DownloadProgress(this, bytesReceived,
+		std::max((off_t)0, bytesTotal));
+}

src/kits/package/FetchFileJob.cpp

@@ -1,11 +1,12 @@
 /*
- * Copyright 2011-2015, Haiku, Inc. All Rights Reserved.
+ * Copyright 2011-2021, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
  *		Axel Dörfler <[email protected]>
  *		Rene Gollent <[email protected]>
  *		Oliver Tappe <[email protected]>
+ *		Stephan Aßmus <[email protected]>
  */
 
 
@@ -22,6 +23,8 @@
 #	include <UrlProtocolRoster.h>
 #endif
 
+#include "FetchUtils.h"
+
 
 namespace BPackageKit {
 
@@ -36,7 +39,7 @@ FetchFileJob::FetchFileJob(const BContext& context, const BString& title,
 	inherited(context, title),
 	fFileURL(fileURL),
 	fTargetEntry(targetEntry),
-	fTargetFile(&targetEntry, B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY),
+	fTargetFile(&targetEntry, B_CREATE_FILE | B_WRITE_ONLY),
 	fError(B_ERROR),
 	fDownloadProgress(0.0)
 {
@@ -90,13 +93,40 @@ FetchFileJob::Execute()
 	if (result != B_OK)
 		return result;
 
-	BUrlRequest* request = BUrlProtocolRoster::MakeRequest(fFileURL.String(),
+	result = FetchUtils::SetFileType(fTargetFile,
+		"application/x-vnd.haiku-package");
+	if (result != B_OK) {
+		fprintf(stderr, "failed to set file type for '%s': %s\n",
+			DownloadFileName(), strerror(result));
+	}
+
+	do {
+		printf("downloading %s\n", DownloadURL());
+		BUrlRequest* request = BUrlProtocolRoster::MakeRequest(DownloadURL(),
 			this);
 		if (request == NULL)
 			return B_BAD_VALUE;
 
+		// Try to resume the download where we left off
+		off_t currentPosition;
+		BHttpRequest* http= dynamic_cast<BHttpRequest*>(request);
+		if (http != NULL && fTargetFile.GetSize(&currentPosition) == B_OK
+			&& currentPosition > 0) {
+			printf("requesting range start %" B_PRIdOFF "\n", currentPosition);
+			http->SetRangeStart(currentPosition);
+		}
+
 		thread_id thread = request->Run();
 		wait_for_thread(thread, NULL);
+	} while (fError == B_IO_ERROR || fError == B_DEV_TIMEOUT);
+
+	if (fError == B_OK) {
+		result = FetchUtils::MarkDownloadComplete(fTargetFile);
+		if (result != B_OK) {
+			fprintf(stderr, "failed to mark download '%s' as complete: %s\n",
+				DownloadFileName(), strerror(result));
+		}
+	}
 
 	return fError;
 }
@@ -146,10 +176,8 @@ FetchFileJob::RequestCompleted(BUrlRequest* request, bool success)
 			}
 			switch (code) {
 				case B_HTTP_STATUS_OK:
-					fError = B_OK;
-					break;
 				case B_HTTP_STATUS_PARTIAL_CONTENT:
-					fError = B_PARTIAL_READ;
+					fError = B_OK;
 					break;
 				case B_HTTP_STATUS_REQUEST_TIMEOUT:
 				case B_HTTP_STATUS_GATEWAY_TIMEOUT:
@@ -198,7 +226,7 @@ FetchFileJob::FetchFileJob(const BContext& context, const BString& title,
 	inherited(context, title),
 	fFileURL(fileURL),
 	fTargetEntry(targetEntry),
-	fTargetFile(&targetEntry, B_CREATE_FILE | B_ERASE_FILE | B_WRITE_ONLY),
+	fTargetFile(&targetEntry, B_CREATE_FILE | B_WRITE_ONLY),
 	fDownloadProgress(0.0)
 {
 }

src/kits/package/FetchUtils.cpp

@@ -0,0 +1,107 @@
+/*
+ * Copyright 2020, Haiku, Inc. All Rights Reserved.
+ * Distributed under the terms of the MIT License.
+ *
+ * Authors:
+ *		Stephan Aßmus <[email protected]>
+ */
+
+
+#include "FetchUtils.h"
+#include "string.h"
+
+#include <Entry.h>
+#include <Node.h>
+#include <TypeConstants.h>
+
+namespace BPackageKit {
+
+namespace BPrivate {
+
+
+#ifdef HAIKU_TARGET_PLATFORM_HAIKU
+
+#define DL_COMPLETE_ATTR "Meta:DownloadCompleted"
+
+
+/*static*/ bool
+FetchUtils::IsDownloadCompleted(const char* path)
+{
+	BEntry entry(path, true);
+	BNode node(&entry);
+	return IsDownloadCompleted(node);
+}
+
+
+/*static*/ bool
+FetchUtils::IsDownloadCompleted(BNode& node)
+{
+    bool isComplete;
+    status_t status = _GetAttribute(node, DL_COMPLETE_ATTR,
+        B_BOOL_TYPE, &isComplete, sizeof(isComplete));
+    if (status != B_OK) {
+        // Most likely cause is that the attribute was not written,
+        // for example by previous versions of the Package Kit.
+        // Worst outcome of assuming a partial download should be
+        // a no-op range request.
+        isComplete = false;
+    }
+    return isComplete;
+}
+
+
+/*static*/ status_t
+FetchUtils::MarkDownloadComplete(BNode& node)
+{
+    bool isComplete = true;
+    return _SetAttribute(node, DL_COMPLETE_ATTR,
+        B_BOOL_TYPE, &isComplete, sizeof(isComplete));
+}
+
+
+/*static*/ status_t
+FetchUtils::SetFileType(BNode& node, const char* type)
+{
+	return _SetAttribute(node, "BEOS:TYPE",
+        B_MIME_STRING_TYPE, type, strlen(type) + 1);
+}
+
+status_t
+FetchUtils::_SetAttribute(BNode& node, const char* attrName,
+    type_code type, const void* data, size_t size)
+{
+	if (node.InitCheck() != B_OK)
+		return node.InitCheck();
+
+	ssize_t written = node.WriteAttr(attrName, type, 0, data, size);
+	if (written != (ssize_t)size) {
+		if (written < 0)
+			return (status_t)written;
+		return B_IO_ERROR;
+	}
+	return B_OK;
+}
+
+
+status_t
+FetchUtils::_GetAttribute(BNode& node, const char* attrName,
+    type_code type, void* data, size_t size)
+{
+	if (node.InitCheck() != B_OK)
+		return node.InitCheck();
+
+	ssize_t read = node.ReadAttr(attrName, type, 0, data, size);
+	if (read != (ssize_t)size) {
+		if (read < 0)
+			return (status_t)read;
+		return B_IO_ERROR;
+	}
+	return B_OK;
+}
+
+
+#endif // HAIKU_TARGET_PLATFORM_HAIKU
+
+}	// namespace BPrivate
+
+}	// namespace BPackageKit

src/kits/package/FetchUtils.h

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2020, Stephan Aßmus <[email protected]>
+ * Distributed under the terms of the MIT License.
+ */
+#ifndef _PACKAGE__PRIVATE__FETCH_UTILS_H_
+#define _PACKAGE__PRIVATE__FETCH_UTILS_H_
+
+
+#include "SupportDefs.h"
+#include <Node.h>
+
+namespace BPackageKit {
+
+namespace BPrivate {
+
+
+class FetchUtils {
+public:
+	static	bool				IsDownloadCompleted(const char* path);
+	static	bool				IsDownloadCompleted(BNode& node);
+
+	static	status_t			MarkDownloadComplete(const char* path);
+	static	status_t			MarkDownloadComplete(BNode& node);
+
+	static	status_t			SetFileType(BNode& node, const char* type);
+
+private:
+	static	status_t			_SetAttribute(BNode& node,
+									const char* attrName,
+									type_code type, const void* data,
+									size_t size);
+	static	status_t			_GetAttribute(BNode& node,
+									const char* attrName,
+									type_code type, void* data,
+									size_t size);
+};
+
+
+}	// namespace BPrivate
+
+}	// namespace BPackageKit
+
+
+#endif // _PACKAGE__PRIVATE__FETCH_UTILS_H_

src/kits/package/Jamfile

@@ -84,6 +84,7 @@ for architectureObject in [ MultiArchSubDirSetup ] {
 			DownloadFileRequest.cpp
 			DropRepositoryRequest.cpp
 			FetchFileJob.cpp
+			FetchUtils.cpp
 			InstallationLocationInfo.cpp
 			Job.cpp
 			PackageInfo.cpp

src/kits/package/manager/PackageManager.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015, Haiku, Inc. All Rights Reserved.
+ * Copyright 2013-2020, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -10,6 +10,8 @@
 
 #include <package/manager/PackageManager.h>
 
+#include <glob.h>
+
 #include <Catalog.h>
 #include <Directory.h>
 #include <package/CommitTransactionResult.h>
@@ -31,6 +33,7 @@
 #include <package/ValidateChecksumJob.h>
 
 #include "FetchFileJob.h"
+#include "FetchUtils.h"
 #include "PackageManagerUtils.h"
 
 #undef B_TRANSLATION_CONTEXT
@@ -38,6 +41,7 @@
 
 
 using BPackageKit::BPrivate::FetchFileJob;
+using BPackageKit::BPrivate::FetchUtils;
 using BPackageKit::BPrivate::ValidateChecksumJob;
 
 
@@ -560,15 +564,40 @@ BPackageManager::_PreparePackageChanges(
 		RemoteRepository* remoteRepository
 			= dynamic_cast<RemoteRepository*>(package->Repository());
 		if (remoteRepository != NULL) {
+			// first check if the package already exists in a previous
+			// transaction
+			bool alreadyDownloaded = false;
+			BPath path(&transaction->TransactionDirectory());
+			BPath parent;
+			if (path.GetParent(&parent) == B_OK) {
+				BString globPath = parent.Path();
+				globPath << "/*/" << fileName;
+				glob_t globbuf;
+				if (glob(globPath.String(), 0, NULL, &globbuf) == 0) {
+					path.Append(fileName);
+					if (BCopyEngine().CopyEntry(globbuf.gl_pathv[0],
+							path.Path()) == B_OK) {
+						alreadyDownloaded = FetchUtils::IsDownloadCompleted(
+							path.Path());
+						printf("Re-using download '%s' from previous "
+							"transaction%s\n", globbuf.gl_pathv[0],
+							alreadyDownloaded ? "" : " (partial)");
+					}
+				}
+			}
+
+			if (!alreadyDownloaded) {
 				// download the package
 				BString url = remoteRepository->Config().PackagesURL();
 				url << '/' << fileName;
 
 				status_t error = DownloadPackage(url, entry,
 					package->Info().Checksum());
-			if (error != B_OK)
+				if (error != B_OK) {
 					DIE(error, "Failed to download package %s",
 						package->Info().Name().String());
+				}
+			}
 		} else if (package->Repository() != &installationRepository) {
 			// clone the existing package
 			LocalRepository* localRepository