[all] Do not set Response data for 404s

For AssetFileSource and the node FileSource this was already the case; this makes the other implementations consistent.
This commit is contained in:
John Firebaugh
2016-02-03 17:35:06 -08:00
parent cdedb66387
commit 7eb1a91c4e
6 changed files with 15 additions and 29 deletions

View File

@@ -23,7 +23,7 @@ public:
// This is set to true for 304 Not Modified responses.
bool notModified = false;
// The actual data of the response. This is null if notModified is true.
// The actual data of the response. Present only for non-error, non-notModified responses.
std::shared_ptr<const std::string> data;
optional<SystemTimePoint> modified;

View File

@@ -199,17 +199,14 @@ void HTTPAndroidRequest::onResponse(JNIEnv* env, int code, jstring /* message */
response->expires = util::parseTimePoint(mbgl::android::std_string_from_jstring(env, expires).c_str());
}
if (body != nullptr) {
jbyte* bodyData = env->GetByteArrayElements(body, nullptr);
response->data = std::make_shared<std::string>(reinterpret_cast<char*>(bodyData), env->GetArrayLength(body));
env->ReleaseByteArrayElements(body, bodyData, JNI_ABORT);
}
if (code == 200) {
// Nothing to do; this is what we want
if (body != nullptr) {
jbyte* bodyData = env->GetByteArrayElements(body, nullptr);
response->data = std::make_shared<std::string>(reinterpret_cast<char*>(bodyData), env->GetArrayLength(body));
env->ReleaseByteArrayElements(body, bodyData, JNI_ABORT);
}
} else if (code == 304) {
response->notModified = true;
response->data.reset();
} else if (code == 404) {
response->error = std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");
} else if (code >= 500 && code < 600) {

View File

@@ -194,8 +194,6 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
} else if ([res isKindOfClass:[NSHTTPURLResponse class]]) {
const long responseCode = [(NSHTTPURLResponse *)res statusCode];
response->data = std::make_shared<std::string>((const char *)[data bytes], [data length]);
NSDictionary *headers = [(NSHTTPURLResponse *)res allHeaderFields];
NSString *cache_control = [headers objectForKey:@"Cache-Control"];
if (cache_control) {
@@ -218,10 +216,9 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
}
if (responseCode == 200) {
// Nothing to do; this is what we want.
response->data = std::make_shared<std::string>((const char *)[data bytes], [data length]);
} else if (responseCode == 304) {
response->notModified = true;
response->data.reset();
} else if (responseCode == 404) {
response->error =
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");

View File

@@ -509,19 +509,14 @@ void HTTPCURLRequest::handleResult(CURLcode code) {
long responseCode = 0;
curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &responseCode);
// Move over any data we got. We're storing this in a separate object because the Response
// object defines it as const.
if (data) {
response->data = std::move(data);
} else {
response->data = std::make_shared<std::string>();
}
if (responseCode == 200) {
// Nothing to do; this is what we want.
if (data) {
response->data = std::move(data);
} else {
response->data = std::make_shared<std::string>();
}
} else if (responseCode == 304) {
response->notModified = true;
response->data.reset();
} else if (responseCode == 404) {
response->error =
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");

View File

@@ -26,8 +26,7 @@ TEST_F(Storage, HTTPTemporaryError) {
ASSERT_NE(nullptr, res.error);
EXPECT_EQ(Response::Error::Reason::Server, res.error->reason);
EXPECT_EQ("HTTP status code 500", res.error->message);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("", *res.data);
ASSERT_FALSE(bool(res.data));
EXPECT_FALSE(bool(res.expires));
EXPECT_FALSE(bool(res.modified));
EXPECT_FALSE(bool(res.etag));

View File

@@ -47,9 +47,8 @@ TEST_F(Storage, HTTP404) {
EXPECT_TRUE(util::ThreadContext::currentlyOn(util::ThreadType::Main));
ASSERT_NE(nullptr, res.error);
EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("Cannot GET /doesnotexist\n", *res.data);
EXPECT_EQ("HTTP status code 404", res.error->message);
EXPECT_FALSE(bool(res.data));
EXPECT_FALSE(bool(res.expires));
EXPECT_FALSE(bool(res.modified));
EXPECT_FALSE(bool(res.etag));
@@ -74,9 +73,8 @@ TEST_F(Storage, HTTP500) {
EXPECT_TRUE(util::ThreadContext::currentlyOn(util::ThreadType::Main));
ASSERT_NE(nullptr, res.error);
EXPECT_EQ(Response::Error::Reason::Server, res.error->reason);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("Server Error!", *res.data);
EXPECT_EQ("HTTP status code 500", res.error->message);
EXPECT_FALSE(bool(res.data));
EXPECT_FALSE(bool(res.expires));
EXPECT_FALSE(bool(res.modified));
EXPECT_FALSE(bool(res.etag));