hrev50921 adds 1 changeset to branch 'master'
old head: 766a9a49b6caa7045f27cdf9d5e7759e6530f156
new head: 0c1bbfe508784e219d8e9f939ddd1bb787e26e4e
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=0c1bbfe50878+%5E766a9a49b6ca
----------------------------------------------------------------------------
0c1bbfe50878: HaikuDepot: suggested code improvements from Adrien
[ Andrew Lindesay <apl@xxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
Revision: hrev50921
Commit: 0c1bbfe508784e219d8e9f939ddd1bb787e26e4e
URL: http://cgit.haiku-os.org/haiku/commit/?id=0c1bbfe50878
Author: Andrew Lindesay <apl@xxxxxxxxxxxxxx>
Date: Tue Jan 31 07:45:36 2017 UTC
----------------------------------------------------------------------------
7 files changed, 96 insertions(+), 88 deletions(-)
.../server/ServerIconExportUpdateProcess.cpp | 80 ++++++++++----------
src/apps/haikudepot/server/ServerSettings.cpp | 49 ++++++------
src/apps/haikudepot/server/ServerSettings.h | 15 ++--
src/apps/haikudepot/server/WebAppInterface.cpp | 15 ++--
src/apps/haikudepot/ui/App.cpp | 2 +-
.../util/ToFileUrlProtocolListener.cpp | 20 ++++-
.../haikudepot/util/ToFileUrlProtocolListener.h | 3 +-
----------------------------------------------------------------------------
diff --git a/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
b/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
index 46ed44f..4e6e54c 100644
--- a/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
+++ b/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
@@ -10,9 +10,11 @@
#include <sys/stat.h>
#include <time.h>
+#include <AutoDeleter.h>
#include <HttpRequest.h>
#include <Json.h>
#include <Url.h>
+#include <UrlProtocolRoster.h>
#include <support/ZlibCompressionAlgorithm.h>
#include "ServerSettings.h"
@@ -130,8 +132,8 @@
ServerIconExportUpdateProcess::_IfModifiedSinceHeaderValue(BString& headerValue)
status_t
ServerIconExportUpdateProcess::_Download(BPath& tarGzFilePath)
{
- BString urlString =
ServerSettings::CreateFullUrl("/__pkgicon/all.tar.gz");
- return _Download(tarGzFilePath, BUrl(urlString), 0, 0);
+ return _Download(tarGzFilePath,
+ ServerSettings::CreateFullUrl("/__pkgicon/all.tar.gz"), 0, 0);
}
@@ -152,10 +154,8 @@ ServerIconExportUpdateProcess::_Download(BPath&
tarGzFilePath, const BUrl& url,
fprintf(stdout, "will stream '%s' to [%s]\n", url.UrlString().String(),
tarGzFilePath.Path());
- bool isSecure = url.Protocol() == BString("https");
ToFileUrlProtocolListener listener(tarGzFilePath, "icon-export",
ServerSettings::UrlConnectionTraceLoggingEnabled());
- BUrlContext context;
BHttpHeaders headers;
ServerSettings::AugmentHeaders(headers);
@@ -169,55 +169,51 @@ ServerIconExportUpdateProcess::_Download(BPath&
tarGzFilePath, const BUrl& url,
headers.AddHeader("If-Modified-Since", ifModifiedSinceHeader);
}
- BHttpRequest request(url, isSecure, "HTTP", &listener, &context);
- request.SetMethod(B_HTTP_GET);
- request.SetHeaders(headers);
- request.SetTimeout(TIMEOUT_MICROSECONDS);
+ BHttpRequest *request = dynamic_cast<BHttpRequest *>(
+ BUrlProtocolRoster::MakeRequest(url, &listener));
+ ObjectDeleter<BHttpRequest> requestDeleter(request);
+ request->SetHeaders(headers);
+ request->SetMaxRedirections(0);
+ request->SetTimeout(TIMEOUT_MICROSECONDS);
- thread_id thread = request.Run();
+ thread_id thread = request->Run();
wait_for_thread(thread, NULL);
const BHttpResult& result = dynamic_cast<const BHttpResult&>(
- request.Result());
+ request->Result());
int32 statusCode = result.StatusCode();
- switch (statusCode) {
- case HTTP_STATUS_OK:
- fprintf(stdout, "did complete streaming data\n");
- return B_OK;
-
- case HTTP_STATUS_NOT_MODIFIED:
- fprintf(stdout, "remote data has not changed since
[%s]\n",
- ifModifiedSinceHeader.String());
- return APP_ERR_NOT_MODIFIED;
-
- case HTTP_STATUS_FOUND: // redirect
- {
- const BHttpHeaders responseHeaders = result.Headers();
- const char *locationValue = responseHeaders["Location"];
-
- if (NULL != locationValue && 0 !=
strlen(locationValue)) {
- BUrl location(locationValue);
- fprintf(stdout, "will redirect to; %s\n",
- location.UrlString().String());
+ if (BHttpRequest::IsSuccessStatusCode(statusCode)) {
+ fprintf(stdout, "did complete streaming data\n");
+ return B_OK;
+ } else if (statusCode == HTTP_STATUS_NOT_MODIFIED) {
+ fprintf(stdout, "remote data has not changed since [%s]\n",
+ ifModifiedSinceHeader.String());
+ return APP_ERR_NOT_MODIFIED;
+ } else if (BHttpRequest::IsRedirectionStatusCode(statusCode)) {
+ const BHttpHeaders responseHeaders = result.Headers();
+ const char *locationValue = responseHeaders["Location"];
+
+ if (NULL != locationValue && 0 != strlen(locationValue)) {
+ BUrl location(locationValue);
+ fprintf(stdout, "will redirect to; %s\n",
+ location.UrlString().String());
return _Download(tarGzFilePath, location,
redirects + 1, 0);
- }
-
- fprintf(stdout, "unable to find 'Location' header for
redirect\n");
- return B_IO_ERROR;
}
- default:
- if (0 == statusCode || 5 == (statusCode / 100)) {
- fprintf(stdout, "error response from server; %"
B_PRId32 " --> "
- "retry...\n", statusCode);
- return _Download(tarGzFilePath, url, redirects,
failures + 1);
- }
+ fprintf(stdout, "unable to find 'Location' header for
redirect\n");
+ return B_IO_ERROR;
+ } else {
+ if (0 == statusCode || 5 == (statusCode / 100)) {
+ fprintf(stdout, "error response from server; %"
B_PRId32 " --> "
+ "retry...\n", statusCode);
+ return _Download(tarGzFilePath, url, redirects,
failures + 1);
+ }
- fprintf(stdout, "unexpected response from server; %"
B_PRId32 "\n",
- statusCode);
- return B_IO_ERROR;
+ fprintf(stdout, "unexpected response from server; %" B_PRId32
"\n",
+ statusCode);
+ return B_IO_ERROR;
}
}
diff --git a/src/apps/haikudepot/server/ServerSettings.cpp
b/src/apps/haikudepot/server/ServerSettings.cpp
index d681120..b145a6a 100644
--- a/src/apps/haikudepot/server/ServerSettings.cpp
+++ b/src/apps/haikudepot/server/ServerSettings.cpp
@@ -13,74 +13,69 @@
#include <Roster.h>
#include <Url.h>
-#include "AutoLocker.h"
-
#define BASEURL_DEFAULT "https://depot.haiku-os.org";
#define USERAGENT_FALLBACK_VERSION "0.0.0"
-BString ServerSettings::fBaseUrl = BString(BASEURL_DEFAULT);
-BString ServerSettings::fUserAgent = BString();
-BLocker ServerSettings::fUserAgentLocker;
-bool ServerSettings::fUrlConnectionTraceLogging = false;
+BUrl ServerSettings::sBaseUrl = BUrl(BASEURL_DEFAULT);
+BString ServerSettings::sUserAgent = BString();
+pthread_once_t ServerSettings::sUserAgentInitOnce = PTHREAD_ONCE_INIT;
+bool ServerSettings::sUrlConnectionTraceLogging = false;
status_t
-ServerSettings::SetBaseUrl(const BString& value)
+ServerSettings::SetBaseUrl(const BUrl& value)
{
- BUrl url(value);
-
- if (!url.IsValid()) {
+ if (!value.IsValid()) {
fprintf(stderr, "the url is not valid\n");
return B_BAD_VALUE;
}
- if (url.Protocol() != "http" && url.Protocol() != "https") {
+ if (value.Protocol() != "http" && value.Protocol() != "https") {
fprintf(stderr, "the url protocol must be 'http' or 'https'\n");
return B_BAD_VALUE;
}
- fBaseUrl.SetTo(value);
-
- if (fBaseUrl.EndsWith("/")) {
- fprintf(stderr, "will remove trailing '/' character in url
base\n");
- fBaseUrl.Remove(fBaseUrl.Length() - 1, 1);
- }
+ sBaseUrl = value;
return B_OK;
}
-BString
+BUrl
ServerSettings::CreateFullUrl(const BString urlPathComponents)
{
- return BString(fBaseUrl) << urlPathComponents;
+ return BUrl(sBaseUrl, urlPathComponents);
}
const BString
ServerSettings::GetUserAgent()
{
- AutoLocker<BLocker> lock(&fUserAgentLocker);
+ if (sUserAgent.IsEmpty())
+ pthread_once(&sUserAgentInitOnce,
&ServerSettings::_InitUserAgent);
- if (fUserAgent.IsEmpty()) {
- fUserAgent.SetTo("HaikuDepot/");
- fUserAgent.Append(_GetUserAgentVersionString());
- }
+ return sUserAgent;
+}
- return fUserAgent;
+
+const void
+ServerSettings::_InitUserAgent()
+{
+ sUserAgent.SetTo("HaikuDepot/");
+ sUserAgent.Append(_GetUserAgentVersionString());
}
void
ServerSettings::EnableUrlConnectionTraceLogging() {
- fUrlConnectionTraceLogging = true;
+ sUrlConnectionTraceLogging = true;
}
bool
ServerSettings::UrlConnectionTraceLoggingEnabled() {
- return fUrlConnectionTraceLogging;
+ return sUrlConnectionTraceLogging;
}
diff --git a/src/apps/haikudepot/server/ServerSettings.h
b/src/apps/haikudepot/server/ServerSettings.h
index cac2b6e..d3085d4 100644
--- a/src/apps/haikudepot/server/ServerSettings.h
+++ b/src/apps/haikudepot/server/ServerSettings.h
@@ -8,27 +8,28 @@
#include <File.h>
#include <HttpHeaders.h>
-#include <Locker.h>
#include <String.h>
+#include <Url.h>
class ServerSettings {
public:
- static status_t
SetBaseUrl(const BString& baseUrl);
+ static status_t
SetBaseUrl(const BUrl& baseUrl);
static const BString GetUserAgent();
static void
AugmentHeaders(BHttpHeaders& headers);
- static BString CreateFullUrl(
+ static BUrl
CreateFullUrl(
const BString urlPathComponents);
static void
EnableUrlConnectionTraceLogging();
static bool
UrlConnectionTraceLoggingEnabled();
private:
+ static const void
_InitUserAgent();
static const BString
_GetUserAgentVersionString();
- static BString fBaseUrl;
- static BString fUserAgent;
- static BLocker
fUserAgentLocker;
- static bool
fUrlConnectionTraceLogging;
+ static BUrl
sBaseUrl;
+ static BString sUserAgent;
+ static pthread_once_t sUserAgentInitOnce;
+ static bool
sUrlConnectionTraceLogging;
};
#endif // SERVER_SETTINGS_H
diff --git a/src/apps/haikudepot/server/WebAppInterface.cpp
b/src/apps/haikudepot/server/WebAppInterface.cpp
index e1b2b7f..4036a64 100644
--- a/src/apps/haikudepot/server/WebAppInterface.cpp
+++ b/src/apps/haikudepot/server/WebAppInterface.cpp
@@ -516,11 +516,11 @@ status_t
WebAppInterface::RetrieveScreenshot(const BString& code,
int32 width, int32 height, BDataIO* stream)
{
- BString urlString =
ServerSettings::CreateFullUrl(BString("/__pkgscreenshot/") << code
- << ".png" << "?tw=" << width << "&th=" << height);
- bool isSecure = 0 == urlString.FindFirst("https://";);
+ BUrl url = ServerSettings::CreateFullUrl(
+ BString("/__pkgscreenshot/") << code << ".png" << "?tw="
+ << width << "&th=" << height);
- BUrl url(urlString);
+ bool isSecure = url.Protocol() == "https";
ProtocolListener listener(
ServerSettings::UrlConnectionTraceLoggingEnabled());
@@ -545,7 +545,7 @@ WebAppInterface::RetrieveScreenshot(const BString& code,
return B_OK;
fprintf(stderr, "failed to get screenshot from '%s': %" B_PRIi32 "\n",
- urlString.String(), statusCode);
+ url.UrlString().String(), statusCode);
return B_ERROR;
}
@@ -629,9 +629,8 @@ WebAppInterface::_SendJsonRequest(const char* domain,
BString jsonString,
if (ServerSettings::UrlConnectionTraceLoggingEnabled())
printf("_SendJsonRequest(%s)\n", jsonString.String());
- BString urlString = ServerSettings::CreateFullUrl(BString("/__api/v1/")
<< domain);
- bool isSecure = 0 == urlString.FindFirst("https://";);
- BUrl url(urlString);
+ BUrl url = ServerSettings::CreateFullUrl(BString("/__api/v1/") <<
domain);
+ bool isSecure = url.Protocol() == "https";
ProtocolListener listener(
ServerSettings::UrlConnectionTraceLoggingEnabled());
diff --git a/src/apps/haikudepot/ui/App.cpp b/src/apps/haikudepot/ui/App.cpp
index 7578977..3a8777a 100644
--- a/src/apps/haikudepot/ui/App.cpp
+++ b/src/apps/haikudepot/ui/App.cpp
@@ -197,7 +197,7 @@ App::ArgvReceived(int32 argc, char* argv[])
Quit();
}
- if (ServerSettings::SetBaseUrl(argv[i + 1]) !=
B_OK) {
+ if (ServerSettings::SetBaseUrl(BUrl(argv[i +
1])) != B_OK) {
fprintf(stdout, "malformed web app base
url; %s\n",
argv[i + 1]);
Quit();
diff --git a/src/apps/haikudepot/util/ToFileUrlProtocolListener.cpp
b/src/apps/haikudepot/util/ToFileUrlProtocolListener.cpp
index 97b57bd..e7e68b6 100644
--- a/src/apps/haikudepot/util/ToFileUrlProtocolListener.cpp
+++ b/src/apps/haikudepot/util/ToFileUrlProtocolListener.cpp
@@ -6,6 +6,7 @@
#include "ToFileUrlProtocolListener.h"
#include <File.h>
+#include <HttpRequest.h>
#include <stdio.h>
@@ -16,6 +17,7 @@ ToFileUrlProtocolListener::ToFileUrlProtocolListener(BPath
path,
fDownloadIO = new BFile(path.Path(), O_WRONLY | O_CREAT);
fTraceLoggingIdentifier = traceLoggingIdentifier;
fTraceLogging = traceLogging;
+ fShouldDownload = true;
}
@@ -48,6 +50,19 @@ void
ToFileUrlProtocolListener::HeadersReceived(BUrlRequest* caller,
const BUrlResult& result)
{
+
+ // check that the status code is success. Only if it is successful
+ // should the payload be streamed to the file.
+
+ const BHttpResult& httpResult = dynamic_cast<const
BHttpResult&>(result);
+ int32 statusCode = httpResult.StatusCode();
+
+ if (!BHttpRequest::IsSuccessStatusCode(statusCode)) {
+ fprintf(stdout, "received %" B_PRId32
+ " --> will not store download to file\n", statusCode);
+ fShouldDownload = false;
+ }
+
}
@@ -55,12 +70,13 @@ void
ToFileUrlProtocolListener::DataReceived(BUrlRequest* caller, const char* data,
off_t position, ssize_t size)
{
- if (fDownloadIO != NULL && size > 0) {
+ if (fShouldDownload && fDownloadIO != NULL && size > 0) {
size_t remaining = size;
size_t written = 0;
do {
- written = fDownloadIO->Write(&data[size - remaining],
remaining);
+ written = fDownloadIO->WriteAt(position, &data[size -
remaining],
+ remaining);
remaining -= written;
} while (remaining > 0 && written > 0);
diff --git a/src/apps/haikudepot/util/ToFileUrlProtocolListener.h
b/src/apps/haikudepot/util/ToFileUrlProtocolListener.h
index 4b49a10..3744a19 100644
--- a/src/apps/haikudepot/util/ToFileUrlProtocolListener.h
+++ b/src/apps/haikudepot/util/ToFileUrlProtocolListener.h
@@ -33,9 +33,10 @@ public:
const
char* text);
private:
+ bool fShouldDownload;
bool fTraceLogging;
BString fTraceLoggingIdentifier;
- BDataIO* fDownloadIO;
+ BPositionIO* fDownloadIO;
};