[core] Include prior values of caching headers in Resource

This allows the FileSource interface itself to support revalidation. We could (and probably should) now rewrite HTTPContextBase implementations as FileSource implementations.
This commit is contained in:
John Firebaugh
2016-01-13 14:32:21 -08:00
parent 27134df2e4
commit a877b9b192
10 changed files with 134 additions and 192 deletions

View File

@@ -1,12 +1,15 @@
#ifndef MBGL_STORAGE_RESOURCE
#define MBGL_STORAGE_RESOURCE
#include <mbgl/storage/response.hpp>
#include <mbgl/util/optional.hpp>
#include <string>
#include <functional>
namespace mbgl {
struct Resource {
class Resource {
public:
enum Kind : uint8_t {
Unknown = 0,
Style,
@@ -17,19 +20,17 @@ struct Resource {
SpriteJSON
};
Resource(Kind kind_, const std::string& url_)
: kind(kind_),
url(url_) {
}
const Kind kind;
const std::string url;
inline bool operator==(const Resource &res) const {
return kind == res.kind && url == res.url;
}
struct Hash {
std::size_t operator()(Resource const& r) const {
return std::hash<std::string>()(r.url) ^ (std::hash<uint8_t>()(r.kind) << 1);
}
};
optional<SystemTimePoint> priorModified;
optional<SystemTimePoint> priorExpires;
optional<std::string> priorEtag;
};
} // namespace mbgl

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 guaranteed to never be empty.
// The actual data of the response. This is null if notModified is true.
std::shared_ptr<const std::string> data;
optional<SystemTimePoint> modified;
@@ -34,7 +34,7 @@ public:
class Response::Error {
public:
enum class Reason : uint8_t {
// Success = 1, // Reserve 1 for Success.
Success = 1,
NotFound = 2,
Server = 3,
Connection = 4,

View File

@@ -1,5 +1,6 @@
#include <mbgl/storage/http_context_base.hpp>
#include <mbgl/storage/http_request_base.hpp>
#include <mbgl/storage/resource.hpp>
#include <mbgl/storage/response.hpp>
#include <mbgl/platform/log.hpp>
#include "jni.hpp"
@@ -22,9 +23,7 @@ public:
explicit HTTPAndroidContext();
~HTTPAndroidContext();
HTTPRequestBase* createRequest(const std::string& url,
HTTPRequestBase::Callback,
std::shared_ptr<const Response>) final;
HTTPRequestBase* createRequest(const Resource&, HTTPRequestBase::Callback) final;
JavaVM *vm = nullptr;
jobject obj = nullptr;
@@ -32,10 +31,7 @@ public:
class HTTPAndroidRequest : public HTTPRequestBase {
public:
HTTPAndroidRequest(HTTPAndroidContext*,
const std::string& url,
Callback,
std::shared_ptr<const Response>);
HTTPAndroidRequest(HTTPAndroidContext*, const Resource&, Callback);
~HTTPAndroidRequest();
void cancel() final;
@@ -107,32 +103,28 @@ HTTPAndroidContext::~HTTPAndroidContext() {
vm = nullptr;
}
HTTPRequestBase* HTTPAndroidContext::createRequest(const std::string& url,
HTTPRequestBase::Callback callback,
std::shared_ptr<const Response> response) {
return new HTTPAndroidRequest(this, url, callback, response);
HTTPRequestBase* HTTPAndroidContext::createRequest(const Resource& resource, HTTPRequestBase::Callback callback) {
return new HTTPAndroidRequest(this, resource, callback);
}
HTTPAndroidRequest::HTTPAndroidRequest(HTTPAndroidContext* context_, const std::string& url_, Callback callback_, std::shared_ptr<const Response> response_)
: HTTPRequestBase(url_, callback_),
HTTPAndroidRequest::HTTPAndroidRequest(HTTPAndroidContext* context_, const Resource& resource_, Callback callback_)
: HTTPRequestBase(resource_, callback_),
context(context_),
existingResponse(response_),
async([this] { finish(); }) {
std::string etagStr;
std::string modifiedStr;
if (existingResponse) {
if (existingResponse->etag) {
etagStr = *existingResponse->etag;
} else if (existingResponse->modified) {
modifiedStr = util::rfc1123(*existingResponse->modified);
}
if (resource.priorEtag) {
etagStr = *resource.priorEtag;
} else if (resource.priorModified) {
modifiedStr = util::rfc1123(*resource.priorModified);
}
JNIEnv *env = nullptr;
bool detach = mbgl::android::attach_jni_thread(context->vm, &env, "HTTPAndroidContext::HTTPAndroidRequest()");
jstring resourceUrl = mbgl::android::std_string_to_jstring(env, url);
jstring resourceUrl = mbgl::android::std_string_to_jstring(env, resource.url);
jstring userAgent = mbgl::android::std_string_to_jstring(env, "MapboxGL/1.0");
jstring etag = mbgl::android::std_string_to_jstring(env, etagStr);
jstring modified = mbgl::android::std_string_to_jstring(env, modifiedStr);
@@ -180,7 +172,7 @@ void HTTPAndroidRequest::cancel() {
void HTTPAndroidRequest::finish() {
if (!cancelled) {
notify(std::move(response));
notify(*response);
}
delete this;
@@ -216,22 +208,7 @@ void HTTPAndroidRequest::onResponse(JNIEnv* env, int code, jstring /* message */
// Nothing to do; this is what we want
} else if (code == 304) {
response->notModified = true;
if (existingResponse) {
response->data = existingResponse->data;
if (!response->expires) {
response->expires = existingResponse->expires;
}
if (!response->modified) {
response->modified = existingResponse->modified;
}
if (!response->etag) {
response->etag = existingResponse->etag;
}
}
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

@@ -1,5 +1,6 @@
#include <mbgl/storage/http_context_base.hpp>
#include <mbgl/storage/http_request_base.hpp>
#include <mbgl/storage/resource.hpp>
#include <mbgl/storage/response.hpp>
#include <mbgl/util/async_task.hpp>
@@ -16,10 +17,7 @@ class HTTPNSURLContext;
class HTTPNSURLRequest : public HTTPRequestBase {
public:
HTTPNSURLRequest(HTTPNSURLContext*,
const std::string& url,
Callback,
std::shared_ptr<const Response>);
HTTPNSURLRequest(HTTPNSURLContext*, const Resource&, Callback);
~HTTPNSURLRequest();
void cancel() final;
@@ -32,7 +30,6 @@ private:
bool cancelled = false;
NSURLSessionDataTask *task = nullptr;
std::unique_ptr<Response> response;
const std::shared_ptr<const Response> existingResponse;
util::AsyncTask async;
};
@@ -43,9 +40,7 @@ public:
HTTPNSURLContext();
~HTTPNSURLContext();
HTTPRequestBase* createRequest(const std::string& url,
HTTPRequestBase::Callback,
std::shared_ptr<const Response>) final;
HTTPRequestBase* createRequest(const Resource&, HTTPRequestBase::Callback) final;
NSURLSession *session = nil;
NSString *userAgent = nil;
@@ -79,21 +74,17 @@ HTTPNSURLContext::~HTTPNSURLContext() {
userAgent = nullptr;
}
HTTPRequestBase* HTTPNSURLContext::createRequest(const std::string& url,
HTTPRequestBase::Callback callback,
std::shared_ptr<const Response> response) {
return new HTTPNSURLRequest(this, url, callback, response);
HTTPRequestBase* HTTPNSURLContext::createRequest(const Resource& resource, HTTPRequestBase::Callback callback) {
return new HTTPNSURLRequest(this, resource, callback);
}
// -------------------------------------------------------------------------------------------------
HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_,
const std::string& url_,
Callback callback_,
std::shared_ptr<const Response> existingResponse_)
: HTTPRequestBase(url_, callback_),
const Resource& resource_,
Callback callback_)
: HTTPRequestBase(resource_, callback_),
context(context_),
existingResponse(existingResponse_),
async([this] { handleResponse(); }) {
// Hold the main loop alive until the request returns. This
// is needed because completion handler runs in another
@@ -101,7 +92,7 @@ HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_,
util::RunLoop::Get()->ref();
@autoreleasepool {
NSURL* url = [NSURL URLWithString:@(url_.c_str())];
NSURL* url = [NSURL URLWithString:@(resource.url.c_str())];
if (context->accountType == 0 &&
([url.host isEqualToString:@"mapbox.com"] || [url.host hasSuffix:@".mapbox.com"])) {
NSString* absoluteString = [url.absoluteString
@@ -110,14 +101,12 @@ HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_,
}
NSMutableURLRequest* req = [NSMutableURLRequest requestWithURL:url];
if (existingResponse) {
if (existingResponse->etag) {
[req addValue:@((*existingResponse->etag).c_str())
forHTTPHeaderField:@"If-None-Match"];
} else if (existingResponse->modified) {
[req addValue:@(util::rfc1123(*existingResponse->modified).c_str())
forHTTPHeaderField:@"If-Modified-Since"];
}
if (resource.priorEtag) {
[req addValue:@(resource.priorEtag->c_str())
forHTTPHeaderField:@"If-None-Match"];
} else if (resource.priorModified) {
[req addValue:@(util::rfc1123(*resource.priorModified).c_str())
forHTTPHeaderField:@"If-Modified-Since"];
}
[req addValue:context->userAgent forHTTPHeaderField:@"User-Agent"];
@@ -145,7 +134,7 @@ void HTTPNSURLRequest::handleResponse() {
if (!cancelled) {
// Actually return the response.
notify(std::move(response));
notify(*response);
}
delete this;
@@ -232,22 +221,7 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
// Nothing to do; this is what we want.
} else if (responseCode == 304) {
response->notModified = true;
if (existingResponse) {
response->data = existingResponse->data;
if (!response->expires) {
response->expires = existingResponse->expires;
}
if (!response->modified) {
response->modified = existingResponse->modified;
}
if (!response->etag) {
response->etag = existingResponse->etag;
}
}
response->data.reset();
} else if (responseCode == 404) {
response->error =
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");

View File

@@ -1,5 +1,6 @@
#include <mbgl/storage/http_context_base.hpp>
#include <mbgl/storage/http_request_base.hpp>
#include <mbgl/storage/resource.hpp>
#include <mbgl/storage/response.hpp>
#include <mbgl/platform/log.hpp>
@@ -45,9 +46,7 @@ public:
HTTPCURLContext();
~HTTPCURLContext();
HTTPRequestBase* createRequest(const std::string& url,
HTTPRequestBase::Callback,
std::shared_ptr<const Response>) final;
HTTPRequestBase* createRequest(const Resource&, HTTPRequestBase::Callback) final;
static int handleSocket(CURL *handle, curl_socket_t s, int action, void *userp, void *socketp);
static int startTimeout(CURLM *multi, long timeout_ms, void *userp);
@@ -77,10 +76,7 @@ class HTTPCURLRequest : public HTTPRequestBase {
MBGL_STORE_THREAD(tid)
public:
HTTPCURLRequest(HTTPCURLContext*,
const std::string& url,
Callback,
std::shared_ptr<const Response>);
HTTPCURLRequest(HTTPCURLContext*, const Resource&, Callback);
~HTTPCURLRequest();
void cancel() final;
@@ -137,10 +133,8 @@ HTTPCURLContext::~HTTPCURLContext() {
timeout.stop();
}
HTTPRequestBase* HTTPCURLContext::createRequest(const std::string& url,
HTTPRequestBase::Callback callback,
std::shared_ptr<const Response> response) {
return new HTTPCURLRequest(this, url, callback, response);
HTTPRequestBase* HTTPCURLContext::createRequest(const Resource& resource, HTTPRequestBase::Callback callback) {
return new HTTPCURLRequest(this, resource, callback);
}
CURL *HTTPCURLContext::getHandle() {
@@ -351,25 +345,22 @@ static CURLcode sslctx_function(CURL * /* curl */, void *sslctx, void * /* parm
}
#endif
HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const std::string& url_, Callback callback_, std::shared_ptr<const Response> response_)
: HTTPRequestBase(url_, callback_),
HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const Resource& resource_, Callback callback_)
: HTTPRequestBase(resource_, callback_),
context(context_),
existingResponse(response_),
handle(context->getHandle()) {
// Zero out the error buffer.
memset(error, 0, sizeof(error));
// If there's already a response, set the correct etags/modified headers to make sure we are
// getting a 304 response if possible. This avoids redownloading unchanged data.
if (existingResponse) {
if (existingResponse->etag) {
const std::string header = std::string("If-None-Match: ") + *existingResponse->etag;
headers = curl_slist_append(headers, header.c_str());
} else if (existingResponse->modified) {
const std::string time =
std::string("If-Modified-Since: ") + util::rfc1123(*existingResponse->modified);
headers = curl_slist_append(headers, time.c_str());
}
if (resource.priorEtag) {
const std::string header = std::string("If-None-Match: ") + *resource.priorEtag;
headers = curl_slist_append(headers, header.c_str());
} else if (resource.priorModified) {
const std::string time =
std::string("If-Modified-Since: ") + util::rfc1123(*resource.priorModified);
headers = curl_slist_append(headers, time.c_str());
}
if (headers) {
@@ -385,7 +376,7 @@ HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const std::string& u
handleError(curl_easy_setopt(handle, CURLOPT_CAINFO, "ca-bundle.crt"));
#endif
handleError(curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 1));
handleError(curl_easy_setopt(handle, CURLOPT_URL, url.c_str()));
handleError(curl_easy_setopt(handle, CURLOPT_URL, resource.url.c_str()));
handleError(curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, writeCallback));
handleError(curl_easy_setopt(handle, CURLOPT_WRITEDATA, this));
handleError(curl_easy_setopt(handle, CURLOPT_HEADERFUNCTION, headerCallback));
@@ -529,22 +520,7 @@ void HTTPCURLRequest::handleResult(CURLcode code) {
// Nothing to do; this is what we want.
} else if (responseCode == 304) {
response->notModified = true;
if (existingResponse) {
response->data = existingResponse->data;
if (!response->expires) {
response->expires = existingResponse->expires;
}
if (!response->modified) {
response->modified = existingResponse->modified;
}
if (!response->etag) {
response->etag = existingResponse->etag;
}
}
response->data.reset();
} else if (responseCode == 404) {
response->error =
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");
@@ -560,7 +536,7 @@ void HTTPCURLRequest::handleResult(CURLcode code) {
}
// Actually return the response.
notify(std::move(response));
notify(*response);
delete this;
}

View File

@@ -48,19 +48,16 @@ private:
void scheduleCacheRequest(OnlineFileSource::Impl&);
void scheduleRealRequest(OnlineFileSource::Impl&, bool forceImmediate = false);
const Resource resource;
Resource resource;
std::unique_ptr<WorkRequest> cacheRequest;
HTTPRequestBase* realRequest = nullptr;
util::Timer realRequestTimer;
Callback callback;
// The current response data. Used to create conditional HTTP requests, and to check whether
// new responses we got changed any data.
std::shared_ptr<const Response> response;
// Counts the number of subsequent failed requests. We're using this value for exponential
// backoff when retrying requests.
uint32_t failedRequests = 0;
Response::Error::Reason failedRequestReason = Response::Error::Reason::Success;
};
class OnlineFileSource::Impl {
@@ -169,7 +166,7 @@ OnlineFileRequestImpl::OnlineFileRequestImpl(const Resource& resource_, Callback
if (impl.cache) {
scheduleCacheRequest(impl);
} else {
scheduleRealRequest(impl);
scheduleRealRequest(impl, true);
}
}
@@ -184,25 +181,28 @@ OnlineFileRequestImpl::~OnlineFileRequestImpl() {
void OnlineFileRequestImpl::scheduleCacheRequest(OnlineFileSource::Impl& impl) {
// Check the cache for existing data so that we can potentially
// revalidate the information without having to redownload everything.
cacheRequest = impl.cache->get(resource, [this, &impl](std::shared_ptr<Response> response_) {
cacheRequest = impl.cache->get(resource, [this, &impl](std::shared_ptr<Response> response) {
cacheRequest = nullptr;
if (response_) {
response = response_;
if (response) {
resource.priorModified = response->modified;
resource.priorExpires = response->expires;
resource.priorEtag = response->etag;
callback(*response);
}
// Force immediate revalidation if the cached response didn't indicate an expiration. If
// it did indicate an expiration, revalidation will happen in the normal scheduling flow.
scheduleRealRequest(impl, response && !response->expires);
// Force immediate revalidation if we don't have a cached response, or the cached
// response does not have an expiration time. Otherwise revalidation will happen in
// the normal scheduling flow.
scheduleRealRequest(impl, !response || !response->expires);
});
}
static Duration errorRetryTimeout(const Response& response, uint32_t failedRequests) {
if (response.error && response.error->reason == Response::Error::Reason::Server) {
static Duration errorRetryTimeout(Response::Error::Reason failedRequestReason, uint32_t failedRequests) {
if (failedRequestReason == Response::Error::Reason::Server) {
// Retry after one second three times, then start exponential backoff.
return Seconds(failedRequests <= 3 ? 1 : 1 << std::min(failedRequests - 3, 31u));
} else if (response.error && response.error->reason == Response::Error::Reason::Connection) {
} else if (failedRequestReason == Response::Error::Reason::Connection) {
// Immediate exponential backoff.
assert(failedRequests > 0);
return Seconds(1 << std::min(failedRequests - 1, 31u));
@@ -212,9 +212,9 @@ static Duration errorRetryTimeout(const Response& response, uint32_t failedReque
}
}
static Duration expirationTimeout(const Response& response) {
if (response.expires) {
return std::max(SystemDuration::zero(), *response.expires - SystemClock::now());
static Duration expirationTimeout(optional<SystemTimePoint> expires) {
if (expires) {
return std::max(SystemDuration::zero(), *expires - SystemClock::now());
} else {
return Duration::max();
}
@@ -226,47 +226,65 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
return;
}
Duration timeout = Duration::zero();
// If we're not being asked for a forced refresh, calculate a timeout that depends on how many
// consecutive errors we've encountered, and on the expiration time, if present.
Duration timeout = forceImmediate
? Duration::zero()
: std::min(errorRetryTimeout(failedRequestReason, failedRequests),
expirationTimeout(resource.priorExpires));
// If there was a prior response and we're not being asked for a forced refresh, calculate a timeout
// that depends on how many consecutive errors we've encountered, and on the expiration time.
if (response && !forceImmediate) {
timeout = std::min(errorRetryTimeout(*response, failedRequests),
expirationTimeout(*response));
if (timeout == Duration::max()) {
return;
}
if (timeout == Duration::max()) {
return;
}
realRequestTimer.start(timeout, Duration::zero(), [this, &impl] {
assert(!realRequest);
realRequest = impl.httpContext->createRequest(resource.url, [this, &impl](std::shared_ptr<const Response> response_) {
realRequest = impl.httpContext->createRequest(resource, [this, &impl](Response response) {
realRequest = nullptr;
if (impl.cache) {
impl.cache->put(resource, *response_);
}
// If we didn't get various caching headers in the response, continue using the
// previous values. Otherwise, update the previous values to the new values.
response = response_;
if (response->error) {
failedRequests++;
if (!response.modified) {
response.modified = resource.priorModified;
} else {
// Reset the number of subsequent failed requests after we got a successful one.
failedRequests = 0;
resource.priorModified = response.modified;
}
callback(*response);
if (!response.expires) {
response.expires = resource.priorExpires;
} else {
resource.priorExpires = response.expires;
}
if (!response.etag) {
response.etag = resource.priorEtag;
} else {
resource.priorEtag = response.etag;
}
if (impl.cache) {
impl.cache->put(resource, response);
}
if (response.error) {
failedRequests++;
failedRequestReason = response.error->reason;
} else {
failedRequests = 0;
failedRequestReason = Response::Error::Reason::Success;
}
callback(response);
scheduleRealRequest(impl);
}, response);
});
});
}
void OnlineFileRequestImpl::networkIsReachableAgain(OnlineFileSource::Impl& impl) {
// We need all requests to fail at least once before we are going to start retrying
// them, and we only immediately restart request that failed due to connection issues.
if (response && response->error && response->error->reason == Response::Error::Reason::Connection) {
if (failedRequestReason == Response::Error::Reason::Connection) {
scheduleRealRequest(impl, true);
}
}

View File

@@ -13,9 +13,7 @@ public:
static std::unique_ptr<HTTPContextBase> createContext();
virtual ~HTTPContextBase() = default;
virtual HTTPRequestBase* createRequest(const std::string& url,
HTTPRequestBase::Callback,
std::shared_ptr<const Response>) = 0;
virtual HTTPRequestBase* createRequest(const Resource&, HTTPRequestBase::Callback) = 0;
};
} // namespace mbgl

View File

@@ -1,14 +1,14 @@
#ifndef MBGL_STORAGE_HTTP_REQUEST_BASE
#define MBGL_STORAGE_HTTP_REQUEST_BASE
#include <mbgl/storage/response.hpp>
#include <mbgl/storage/resource.hpp>
#include <mbgl/util/noncopyable.hpp>
#include <mbgl/util/chrono.hpp>
#include <mbgl/util/optional.hpp>
#include <memory>
#include <functional>
#include <utility>
#include <string>
namespace mbgl {
@@ -16,10 +16,10 @@ class Response;
class HTTPRequestBase : private util::noncopyable {
public:
using Callback = std::function<void (std::shared_ptr<const Response> response)>;
using Callback = std::function<void (Response)>;
HTTPRequestBase(const std::string& url_, Callback notify_)
: url(url_)
HTTPRequestBase(const Resource& resource_, Callback notify_)
: resource(resource_)
, notify(std::move(notify_))
, cancelled(false) {
}
@@ -30,7 +30,7 @@ public:
protected:
static optional<SystemTimePoint> parseCacheControl(const char *value);
std::string url;
Resource resource;
Callback notify;
bool cancelled;
};

View File

@@ -10,7 +10,7 @@
namespace mbgl {
struct Resource;
class Resource;
class Response;
class WorkRequest;

View File

@@ -40,8 +40,7 @@ TEST_F(Storage, CacheRevalidateSame) {
EXPECT_EQ(nullptr, res2.error);
EXPECT_TRUE(res2.notModified);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Response", *res2.data);
ASSERT_FALSE(res2.data.get());
EXPECT_TRUE(bool(res2.expires));
EXPECT_FALSE(bool(res2.modified));
// We're not sending the ETag in the 304 reply, but it should still be there.
@@ -92,8 +91,7 @@ TEST_F(Storage, CacheRevalidateModified) {
EXPECT_EQ(nullptr, res2.error);
EXPECT_TRUE(res2.notModified);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Response", *res2.data);
ASSERT_FALSE(res2.data.get());
EXPECT_TRUE(bool(res2.expires));
EXPECT_EQ(SystemClock::from_time_t(1420070400), *res2.modified);
EXPECT_FALSE(res2.etag);