[core] Use better types for modified / expires / etag

8 files changed, 112 insertions, 60 deletions

diff --git a/platform/android/src/http_request_android.cpp b/platform/android/src/http_request_android.cpp
index 05d719938c..81fb6fd306 100644
--- a/platform/android/src/http_request_android.cpp
+++ b/platform/android/src/http_request_android.cpp
@@ -124,10 +124,10 @@ HTTPAndroidRequest::HTTPAndroidRequest(HTTPAndroidContext* context_, const std::
     std::string etagStr;
     std::string modifiedStr;
     if (existingResponse) {
-        if (!existingResponse->etag.empty()) {
-            etagStr = existingResponse->etag;
-        } else if (existingResponse->modified != Seconds::zero()) {
-            modifiedStr = util::rfc1123(existingResponse->modified.count());
+        if (existingResponse->etag) {
+            etagStr = *existingResponse->etag;
+        } else if (existingResponse->modified) {
+            modifiedStr = util::rfc1123(SystemClock::to_time_t(*existingResponse->modified));
         }
     }
 
@@ -195,11 +195,11 @@ void HTTPAndroidRequest::onResponse(int code, std::string message, std::string e
     // the message param is unused, this generates a warning at build time
     // this was breaking builds for `make android -j4`
     (void)message;
-    response->modified = Seconds(parse_date(modified.c_str()));
+    response->modified = SystemClock::from_time_t(parse_date(modified.c_str()));
     response->etag = etag;
     response->expires = parseCacheControl(cacheControl.c_str());
     if (!expires.empty()) {
-        response->expires = Seconds(parse_date(expires.c_str()));
+        response->expires = SystemClock::from_time_t(parse_date(expires.c_str()));
     }
     response->data = std::make_shared<std::string>(body);
 
@@ -211,15 +211,15 @@ void HTTPAndroidRequest::onResponse(int code, std::string message, std::string e
         if (existingResponse) {
             response->data = existingResponse->data;
 
-            if (response->expires == Seconds::zero()) {
+            if (!response->expires) {
                 response->expires = existingResponse->expires;
             }
 
-            if (response->modified == Seconds::zero()) {
+            if (!response->modified) {
                 response->modified = existingResponse->modified;
             }
 
-            if (response->etag.empty()) {
+            if (!response->etag) {
                 response->etag = existingResponse->etag;
             }
         }
diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm
index bb8bb57314..68b9863f74 100644
--- a/platform/darwin/http_request_nsurl.mm
+++ b/platform/darwin/http_request_nsurl.mm
@@ -113,12 +113,12 @@ HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_,
 
         NSMutableURLRequest* req = [NSMutableURLRequest requestWithURL:url];
         if (existingResponse) {
-            if (!existingResponse->etag.empty()) {
-                [req addValue:@(existingResponse->etag.c_str())
-                    forHTTPHeaderField:@"If-None-Match"];
-            } else if (existingResponse->modified != Seconds::zero()) {
-                const std::string time = util::rfc1123(existingResponse->modified.count());
-                [req addValue:@(time.c_str()) forHTTPHeaderField:@"If-Modified-Since"];
+            if (existingResponse->etag) {
+                [req addValue:@((*existingResponse->etag).c_str())
+                     forHTTPHeaderField:@"If-None-Match"];
+            } else if (existingResponse->modified) {
+                [req addValue:@(util::rfc1123(SystemClock::to_time_t(*existingResponse->modified)).c_str())
+                     forHTTPHeaderField:@"If-Modified-Since"];
             }
         }
 
@@ -217,17 +217,17 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
 
         NSString *expires = [headers objectForKey:@"Expires"];
         if (expires) {
-            response->expires = Seconds(parse_date([expires UTF8String]));
+            response->expires = SystemClock::from_time_t(parse_date([expires UTF8String]));
         }
 
         NSString *last_modified = [headers objectForKey:@"Last-Modified"];
         if (last_modified) {
-            response->modified = Seconds(parse_date([last_modified UTF8String]));
+            response->modified = SystemClock::from_time_t(parse_date([last_modified UTF8String]));
         }
 
         NSString *etag = [headers objectForKey:@"ETag"];
         if (etag) {
-            response->etag = [etag UTF8String];
+            response->etag = std::string([etag UTF8String]);
         }
 
         if (responseCode == 200) {
@@ -238,15 +238,15 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
             if (existingResponse) {
                 response->data = existingResponse->data;
 
-                if (response->expires == Seconds::zero()) {
+                if (!response->expires) {
                     response->expires = existingResponse->expires;
                 }
 
-                if (response->modified == Seconds::zero()) {
+                if (!response->modified) {
                     response->modified = existingResponse->modified;
                 }
 
-                if (response->etag.empty()) {
+                if (!response->etag) {
                     response->etag = existingResponse->etag;
                 }
             }
diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp
index f5fc395862..078a172c2b 100644
--- a/platform/default/http_request_curl.cpp
+++ b/platform/default/http_request_curl.cpp
@@ -363,12 +363,12 @@ HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const std::string& u
     // 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.empty()) {
-            const std::string header = std::string("If-None-Match: ") + existingResponse->etag;
+        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 != Seconds::zero()) {
+        } else if (existingResponse->modified) {
             const std::string time =
-                std::string("If-Modified-Since: ") + util::rfc1123(existingResponse->modified.count());
+                std::string("If-Modified-Since: ") + util::rfc1123(SystemClock::to_time_t(*existingResponse->modified));
             headers = curl_slist_append(headers, time.c_str());
         }
     }
@@ -466,15 +466,15 @@ size_t HTTPCURLRequest::headerCallback(char *const buffer, const size_t size, co
         // Always overwrite the modification date; We might already have a value here from the
         // Date header, but this one is more accurate.
         const std::string value { buffer + begin, length - begin - 2 }; // remove \r\n
-        baton->response->modified = Seconds(curl_getdate(value.c_str(), nullptr));
+        baton->response->modified = SystemClock::from_time_t(curl_getdate(value.c_str(), nullptr));
     } else if ((begin = headerMatches("etag: ", buffer, length)) != std::string::npos) {
-        baton->response->etag = { buffer + begin, length - begin - 2 }; // remove \r\n
+        baton->response->etag = std::string(buffer + begin, length - begin - 2); // remove \r\n
     } else if ((begin = headerMatches("cache-control: ", buffer, length)) != std::string::npos) {
         const std::string value { buffer + begin, length - begin - 2 }; // remove \r\n
         baton->response->expires = parseCacheControl(value.c_str());
     } else if ((begin = headerMatches("expires: ", buffer, length)) != std::string::npos) {
         const std::string value { buffer + begin, length - begin - 2 }; // remove \r\n
-        baton->response->expires = Seconds(curl_getdate(value.c_str(), nullptr));
+        baton->response->expires = SystemClock::from_time_t(curl_getdate(value.c_str(), nullptr));
     }
 
     return length;
@@ -534,15 +534,15 @@ void HTTPCURLRequest::handleResult(CURLcode code) {
             if (existingResponse) {
                 response->data = existingResponse->data;
 
-                if (response->expires == Seconds::zero()) {
+                if (!response->expires) {
                     response->expires = existingResponse->expires;
                 }
 
-                if (response->modified == Seconds::zero()) {
+                if (!response->modified) {
                     response->modified = existingResponse->modified;
                 }
 
-                if (response->etag.empty()) {
+                if (!response->etag) {
                     response->etag = existingResponse->etag;
                 }
             }
diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp
index 18f166b833..89a9d019f0 100644
--- a/platform/default/online_file_source.cpp
+++ b/platform/default/online_file_source.cpp
@@ -192,14 +192,13 @@ void OnlineFileRequestImpl::scheduleCacheRequest(OnlineFileSource::Impl& impl) {
             callback(*response);
         }
 
-        // Force an immediate request if the cached response is stale. Note that this is not
-        // quite the same as what `expirationTimeout` would calculate, because in `expirationTimeout`
-        // Seconds::zero() is treated as "no expiration", but here we want it to force a revalidation.
-        scheduleRealRequest(impl, response && response->expires <= toSeconds(SystemClock::now()));
+        // 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);
     });
 }
 
-static Seconds errorRetryTimeout(const Response& response, uint32_t failedRequests) {
+static Duration errorRetryTimeout(const Response& response, uint32_t failedRequests) {
     if (response.error && response.error->reason == 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));
@@ -209,16 +208,15 @@ static Seconds errorRetryTimeout(const Response& response, uint32_t failedReques
         return Seconds(1 << std::min(failedRequests - 1, 31u));
     } else {
         // No error, or not an error that triggers retries.
-        return Seconds::max();
+        return Duration::max();
     }
 }
 
-static Seconds expirationTimeout(const Response& response) {
-    // Seconds::zero() is a special value meaning "no expiration".
-    if (response.expires > Seconds::zero()) {
-        return std::max(Seconds::zero(), response.expires - toSeconds(SystemClock::now()));
+static Duration expirationTimeout(const Response& response) {
+    if (response.expires) {
+        return std::max(SystemDuration::zero(), *response.expires - SystemClock::now());
     } else {
-        return Seconds::max();
+        return Duration::max();
     }
 }
 
@@ -228,7 +226,7 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
         return;
     }
 
-    Seconds timeout = Seconds::zero();
+    Duration timeout = Duration::zero();
 
     // 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.
@@ -236,7 +234,7 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
         timeout = std::min(errorRetryTimeout(*response, failedRequests),
                            expirationTimeout(*response));
 
-        if (timeout == Seconds::max()) {
+        if (timeout == Duration::max()) {
             return;
         }
     }
diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp
index 7e8a56f7f9..5122e01015 100644
--- a/platform/default/sqlite3.cpp
+++ b/platform/default/sqlite3.cpp
@@ -4,6 +4,8 @@
 #include <cassert>
 #include <cstring>
 #include <cstdio>
+#include <chrono>
+#include <experimental/optional>
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-variable"
@@ -24,6 +26,9 @@ const static bool sqliteVersionCheck = []() {
 namespace mapbox {
 namespace sqlite {
 
+template <typename T>
+using optional = std::experimental::optional<T>;
+
 Database::Database(const std::string &filename, int flags) {
     const int err = sqlite3_open_v2(filename.c_str(), &db, flags, nullptr);
     if (err != SQLITE_OK) {
@@ -105,6 +110,11 @@ void Statement::check(int err) {
     }
 }
 
+template <> void Statement::bind(int offset, std::nullptr_t) {
+    assert(stmt);
+    check(sqlite3_bind_null(stmt, offset));
+}
+
 template <> void Statement::bind(int offset, int value) {
     assert(stmt);
     check(sqlite3_bind_int(stmt, offset, value));
@@ -130,12 +140,33 @@ template <> void Statement::bind(int offset, const char *value) {
     check(sqlite3_bind_text(stmt, offset, value, -1, SQLITE_STATIC));
 }
 
-void Statement::bind(int offset, const std::string &value, bool retain) {
+void Statement::bind(int offset, const std::string& value, bool retain) {
     assert(stmt);
     check(sqlite3_bind_blob(stmt, offset, value.data(), int(value.size()),
                             retain ? SQLITE_TRANSIENT : SQLITE_STATIC));
 }
 
+template <> void Statement::bind(int offset, std::chrono::system_clock::time_point value) {
+    assert(stmt);
+    check(sqlite3_bind_int64(stmt, offset, std::chrono::system_clock::to_time_t(value)));
+}
+
+template <> void Statement::bind(int offset, optional<std::string> value) {
+    if (!value) {
+        bind(offset, nullptr);
+    } else {
+        bind(offset, *value);
+    }
+}
+
+template <> void Statement::bind(int offset, optional<std::chrono::system_clock::time_point> value) {
+    if (!value) {
+        bind(offset, nullptr);
+    } else {
+        bind(offset, *value);
+    }
+}
+
 bool Statement::run() {
     assert(stmt);
     const int err = sqlite3_step(stmt);
@@ -173,6 +204,29 @@ template <> std::string Statement::get(int offset) {
     };
 }
 
+template <> std::chrono::system_clock::time_point Statement::get(int offset) {
+    assert(stmt);
+    return std::chrono::system_clock::from_time_t(sqlite3_column_int64(stmt, offset));
+}
+
+template <> optional<std::string> Statement::get(int offset) {
+    assert(stmt);
+    if (sqlite3_column_type(stmt, offset) == SQLITE_NULL) {
+        return optional<std::string>();
+    } else {
+        return get<std::string>(offset);
+    }
+}
+
+template <> optional<std::chrono::system_clock::time_point> Statement::get(int offset) {
+    assert(stmt);
+    if (sqlite3_column_type(stmt, offset) == SQLITE_NULL) {
+        return optional<std::chrono::system_clock::time_point>();
+    } else {
+        return get<std::chrono::system_clock::time_point>(offset);
+    }
+}
+
 void Statement::reset() {
     assert(stmt);
     sqlite3_reset(stmt);
diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp
index 5816467aa5..1980b3dead 100644
--- a/platform/default/sqlite_cache.cpp
+++ b/platform/default/sqlite_cache.cpp
@@ -332,9 +332,9 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
                 // Status codes > 1 indicate an error
                 response->error = std::make_unique<Response::Error>(Response::Error::Reason(status));
             }
-            response->modified = Seconds(getStmt->get<int64_t>(1));
-            response->etag = getStmt->get<std::string>(2);
-            response->expires = Seconds(getStmt->get<int64_t>(3));
+            response->modified = getStmt->get<optional<SystemTimePoint>>(1);
+            response->etag = getStmt->get<optional<std::string>>(2);
+            response->expires = getStmt->get<optional<SystemTimePoint>>(3);
             response->data = std::make_shared<std::string>(getStmt->get<std::string>(4));
             if (getStmt->get<int>(5)) { // == compressed
                 response->data = std::make_shared<std::string>(util::decompress(*response->data));
@@ -357,7 +357,7 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
                 accessedStmt->reset();
             }
 
-            accessedStmt->bind(1, int64_t(toSeconds(SystemClock::now()).count()));
+            accessedStmt->bind(1, SystemClock::now());
             accessedStmt->bind(2, canonicalURL.c_str());
             accessedStmt->run();
         }
@@ -419,10 +419,10 @@ void SQLiteCache::Impl::put(const Resource& resource, const Response& response)
             putStmt->bind(2 /* status */, 1 /* success */);
         }
         putStmt->bind(3 /* kind */, int(resource.kind));
-        putStmt->bind(4 /* modified */, int64_t(response.modified.count()));
-        putStmt->bind(5 /* etag */, response.etag.c_str());
-        putStmt->bind(6 /* expires */, int64_t(response.expires.count()));
-        putStmt->bind(7 /* accessed */, int64_t(toSeconds(SystemClock::now()).count()));
+        putStmt->bind(4 /* modified */, response.modified);
+        putStmt->bind(5 /* etag */, response.etag);
+        putStmt->bind(6 /* expires */, response.expires);
+        putStmt->bind(7 /* accessed */, SystemClock::now());
 
         std::string data;
         if (resource.kind != Resource::SpriteImage && response.data) {
@@ -452,7 +452,7 @@ void SQLiteCache::Impl::put(const Resource& resource, const Response& response)
     }
 }
 
-void SQLiteCache::Impl::refresh(const Resource& resource, Seconds expires) {
+void SQLiteCache::Impl::refresh(const Resource& resource, optional<SystemTimePoint> expires) {
     try {
         initializeDatabase();
 
@@ -466,8 +466,8 @@ void SQLiteCache::Impl::refresh(const Resource& resource, Seconds expires) {
         }
 
         const auto canonicalURL = util::mapbox::canonicalURL(resource.url);
-        refreshStmt->bind(1, int64_t(toSeconds(SystemClock::now()).count()));
-        refreshStmt->bind(2, int64_t(expires.count()));
+        refreshStmt->bind(1, SystemClock::now());
+        refreshStmt->bind(2, expires);
         refreshStmt->bind(3, canonicalURL.c_str());
         refreshStmt->run();
     } catch (mapbox::sqlite::Exception& ex) {
diff --git a/platform/default/sqlite_cache_impl.hpp b/platform/default/sqlite_cache_impl.hpp
index aef91efec9..e156532402 100644
--- a/platform/default/sqlite_cache_impl.hpp
+++ b/platform/default/sqlite_cache_impl.hpp
@@ -3,6 +3,7 @@
 
 #include <mbgl/storage/sqlite_cache.hpp>
 #include <mbgl/util/chrono.hpp>
+#include <mbgl/util/optional.hpp>
 
 namespace mapbox {
 namespace sqlite {
@@ -23,7 +24,7 @@ public:
 
     void get(const Resource&, Callback);
     void put(const Resource&, const Response&);
-    void refresh(const Resource&, Seconds expires);
+    void refresh(const Resource&, optional<SystemTimePoint> expires);
 
 private:
     void initializeDatabase();
diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp
index 8e03a3bc1d..1c8b46b838 100644
--- a/platform/node/src/node_request.cpp
+++ b/platform/node/src/node_request.cpp
@@ -44,7 +44,6 @@ v8::Handle<v8::Object> NodeRequest::Create(const mbgl::Resource& resource, mbgl:
 
 NAN_METHOD(NodeRequest::Respond) {
     using Error = mbgl::Response::Error;
-    using Milliseconds = mbgl::Milliseconds;
 
     mbgl::Response response;
 
@@ -80,14 +79,14 @@ NAN_METHOD(NodeRequest::Respond) {
         if (Nan::Has(res, Nan::New("modified").ToLocalChecked()).FromJust()) {
             const double modified = Nan::Get(res, Nan::New("modified").ToLocalChecked()).ToLocalChecked()->ToNumber()->Value();
             if (!std::isnan(modified)) {
-                response.modified = mbgl::asSeconds(Milliseconds(Milliseconds::rep(modified)));
+                response.modified = mbgl::SystemClock::from_time_t(modified / 1000);
             }
         }
 
         if (Nan::Has(res, Nan::New("expires").ToLocalChecked()).FromJust()) {
             const double expires = Nan::Get(res, Nan::New("expires").ToLocalChecked()).ToLocalChecked()->ToNumber()->Value();
             if (!std::isnan(expires)) {
-                response.expires = mbgl::asSeconds(Milliseconds(Milliseconds::rep(expires)));
+                response.expires = mbgl::SystemClock::from_time_t(expires / 1000);
             }
         }