From Adrien Destugues <pulkomandy@xxxxxxxxx>:
Adrien Destugues has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/3517 ;)
Change subject: Package Kit: re-use downloads from unfinished transactions
......................................................................
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 <superstippi@xxxxxx>
Reviewed-by: Alex von Gluck IV <kallisti5@xxxxxxxxxxx>
(cherry picked from commit f15516ff926d89c0853b1ce3262e5b70eecdb003)
---
M headers/os/net/HttpRequest.h
M src/kits/network/libnetapi/HttpRequest.cpp
M src/kits/package/FetchFileJob.cpp
A src/kits/package/FetchUtils.cpp
A src/kits/package/FetchUtils.h
M src/kits/package/Jamfile
M src/kits/package/manager/PackageManager.cpp
7 files changed, 321 insertions(+), 51 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/17/3517/1
diff --git a/headers/os/net/HttpRequest.h b/headers/os/net/HttpRequest.h
index 7704bae..37845c5 100644
--- a/headers/os/net/HttpRequest.h
+++ b/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_
@@ -91,6 +91,11 @@
// 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;
diff --git a/src/kits/network/libnetapi/HttpRequest.cpp
b/src/kits/network/libnetapi/HttpRequest.cpp
index 2638e66..db9eb38 100644
--- a/src/kits/network/libnetapi/HttpRequest.cpp
+++ b/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, c.huriaux@xxxxxxxxx
* Niels Sascha Reedijk, niels.reedijk@xxxxxxxxx
* Adrien Destugues, pulkomandy@xxxxxxxxxxxxx
+ * Stephan Aßmus, superstippi@xxxxxx
*/
@@ -209,17 +210,36 @@
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;
- fOptHeaders = headers;
+ fOptPassword = password;
+}
+
+
+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;
}
@@ -231,6 +251,13 @@
void
+BHttpRequest::SetHeaders(const BHttpHeaders& headers)
+{
+ AdoptHeaders(new(std::nothrow) BHttpHeaders(headers));
+}
+
+
+void
BHttpRequest::AdoptPostFields(BHttpForm* const fields)
{
delete fOptPostFields;
@@ -251,16 +278,10 @@
void
-BHttpRequest::SetUserName(const BString& name)
+BHttpRequest::AdoptHeaders(BHttpHeaders* const headers)
{
- fOptUsername = name;
-}
-
-
-void
-BHttpRequest::SetPassword(const BString& password)
-{
- fOptPassword = password;
+ delete fOptHeaders;
+ fOptHeaders = headers;
}
@@ -740,17 +761,14 @@
ssize_t size =
decompressorStorage.Size();
BStackOrHeapArray<char, 4096>
buffer(size);
size =
decompressorStorage.Read(buffer, size);
- if (size > 0) {
-
fListener->DataReceived(this, buffer, bytesUnpacked,
- size);
- bytesUnpacked += size;
- }
+ _NotifyDataReceived(buffer,
bytesUnpacked, size,
+ bytesReceived,
bytesTotal);
+ bytesUnpacked += size;
} else if (bytesRead > 0) {
- fListener->DataReceived(this,
inputTempBuffer,
- bytesReceived -
bytesRead, bytesRead);
+
_NotifyDataReceived(inputTempBuffer,
+ bytesReceived -
bytesRead, bytesRead,
+ bytesReceived,
bytesTotal);
}
- fListener->DownloadProgress(this,
bytesReceived,
- std::max((off_t)0, bytesTotal));
}
if (bytesTotal >= 0 && bytesReceived >=
bytesTotal)
@@ -768,11 +786,9 @@
ssize_t size =
decompressorStorage.Size();
BStackOrHeapArray<char, 4096>
buffer(size);
size = decompressorStorage.Read(buffer,
size);
- if (fListener != NULL && size > 0) {
- fListener->DataReceived(this,
buffer,
- bytesUnpacked, size);
- bytesUnpacked += size;
- }
+ _NotifyDataReceived(buffer,
bytesUnpacked, size,
+ bytesReceived, bytesTotal);
+ bytesUnpacked += size;
}
}
}
@@ -909,6 +925,20 @@
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);
@@ -1153,3 +1183,29 @@
}
+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));
+}
diff --git a/src/kits/package/FetchFileJob.cpp
b/src/kits/package/FetchFileJob.cpp
index 07853d7..50de805 100644
--- a/src/kits/package/FetchFileJob.cpp
+++ b/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 <axeld@xxxxxxxxxxxxxxxx>
* Rene Gollent <rene@xxxxxxxxxxx>
* Oliver Tappe <zooey@xxxxxxxxxxxxxxx>
+ * Stephan Aßmus <superstippi@xxxxxx>
*/
@@ -22,6 +23,8 @@
# include <UrlProtocolRoster.h>
#endif
+#include "FetchUtils.h"
+
namespace BPackageKit {
@@ -36,7 +39,7 @@
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 @@
if (result != B_OK)
return result;
- BUrlRequest* request =
BUrlProtocolRoster::MakeRequest(fFileURL.String(),
- this);
- if (request == NULL)
- return B_BAD_VALUE;
+ 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));
+ }
- thread_id thread = request->Run();
- wait_for_thread(thread, NULL);
+ 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(¤tPosition) ==
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 @@
}
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 @@
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)
{
}
diff --git a/src/kits/package/FetchUtils.cpp b/src/kits/package/FetchUtils.cpp
new file mode 100644
index 0000000..046d7f8
--- /dev/null
+++ b/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 <superstippi@xxxxxx>
+ */
+
+
+#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
diff --git a/src/kits/package/FetchUtils.h b/src/kits/package/FetchUtils.h
new file mode 100644
index 0000000..9abb168
--- /dev/null
+++ b/src/kits/package/FetchUtils.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright 2020, Stephan Aßmus <superstippi@xxxxxx>
+ * 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_
diff --git a/src/kits/package/Jamfile b/src/kits/package/Jamfile
index 9eb761e..1566846 100644
--- a/src/kits/package/Jamfile
+++ b/src/kits/package/Jamfile
@@ -88,6 +88,7 @@
DownloadFileRequest.cpp
DropRepositoryRequest.cpp
FetchFileJob.cpp
+ FetchUtils.cpp
InstallationLocationInfo.cpp
Job.cpp
PackageInfo.cpp
diff --git a/src/kits/package/manager/PackageManager.cpp
b/src/kits/package/manager/PackageManager.cpp
index 6c9908d..f3607b0 100644
--- a/src/kits/package/manager/PackageManager.cpp
+++ b/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 @@
RemoteRepository* remoteRepository
=
dynamic_cast<RemoteRepository*>(package->Repository());
if (remoteRepository != NULL) {
- // download the package
- BString url = remoteRepository->Config().PackagesURL();
- url << '/' << fileName;
+ // 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)");
+ }
+ }
+ }
- status_t error = DownloadPackage(url, entry,
- package->Info().Checksum());
- if (error != B_OK)
- DIE(error, "Failed to download package %s",
- package->Info().Name().String());
+ 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) {
+ DIE(error, "Failed to download package
%s",
+
package->Info().Name().String());
+ }
+ }
} else if (package->Repository() != &installationRepository) {
// clone the existing package
LocalRepository* localRepository
--
To view, visit https://review.haiku-os.org/c/haiku/+/3517
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: r1beta2
Gerrit-Change-Id: I3e285741ed0e5651594a7c2e1c7170644a9d297d
Gerrit-Change-Number: 3517
Gerrit-PatchSet: 1
Gerrit-Owner: Adrien Destugues <pulkomandy@xxxxxxxxx>
Gerrit-MessageType: newchange
