diff options
33 files changed, 135 insertions, 117 deletions
diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index b232cd06f4..6390426030 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -18,7 +18,7 @@ public: int64_t modified = 0; int64_t expires = 0; std::string etag; - std::string data; + std::shared_ptr<const std::string> data; }; } diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm index f1f6e3fbb9..f5a4d8ef97 100644 --- a/platform/darwin/http_request_nsurl.mm +++ b/platform/darwin/http_request_nsurl.mm @@ -221,6 +221,9 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e // TODO: Use different codes for host not found, timeout, invalid URL etc. // These can be categorized in temporary and permanent errors. response = std::make_unique<Response>(); + if (data) { + response->data = std::make_shared<std::string>((const char *)[data bytes], [data length]); + } response->status = Response::Error; response->message = [[error localizedDescription] UTF8String]; @@ -253,7 +256,7 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e const long responseCode = [(NSHTTPURLResponse *)res statusCode]; response = std::make_unique<Response>(); - response->data = {(const char *)[data bytes], [data length]}; + 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"]; diff --git a/platform/default/asset_request_fs.cpp b/platform/default/asset_request_fs.cpp index 9fc4e55de4..ac54cda0a1 100644 --- a/platform/default/asset_request_fs.cpp +++ b/platform/default/asset_request_fs.cpp @@ -138,8 +138,10 @@ void AssetRequest::fileStated(uv_fs_t *req) { #endif self->response->etag = util::toString(stat->st_ino); const auto size = (unsigned int)(stat->st_size); - self->response->data.resize(size); - self->buffer = uv_buf_init(const_cast<char *>(self->response->data.data()), size); + auto data = std::make_shared<std::string>(); + self->response->data = data; + data->resize(size); + self->buffer = uv_buf_init(const_cast<char *>(data->data()), size); uv_fs_req_cleanup(req); #if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 uv_fs_read(req->loop, req, self->fd, self->buffer.base, self->buffer.len, -1, fileRead); diff --git a/platform/default/asset_request_zip.cpp b/platform/default/asset_request_zip.cpp index 13c7df4cc8..2947bb3c6a 100644 --- a/platform/default/asset_request_zip.cpp +++ b/platform/default/asset_request_zip.cpp @@ -180,8 +180,10 @@ void AssetRequest::fileStated(uv_zip_t *zip) { response = std::make_unique<Response>(); // Allocate the space for reading the data. - response->data.resize(zip->stat->size); - buffer = uv_buf_init(const_cast<char *>(response->data.data()), zip->stat->size); + auto data = std::make_shared<std::string>(); + data->resize(zip->stat->size); + buffer = uv_buf_init(const_cast<char *>(data->data()), zip->stat->size); + response->data = data; // Get the modification time in case we have one. if (zip->stat->valid & ZIP_STAT_MTIME) { diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp index 307993876a..2d80f0b60a 100644 --- a/platform/default/http_request_curl.cpp +++ b/platform/default/http_request_curl.cpp @@ -111,6 +111,7 @@ private: HTTPCURLContext *context = nullptr; // Will store the current response. + std::shared_ptr<std::string> data; std::unique_ptr<Response> response; // In case of revalidation requests, this will store the old response. @@ -516,11 +517,11 @@ size_t HTTPCURLRequest::writeCallback(void *const contents, const size_t size, c auto impl = reinterpret_cast<HTTPCURLRequest *>(userp); MBGL_VERIFY_THREAD(impl->tid); - if (!impl->response) { - impl->response = std::make_unique<Response>(); + if (!impl->data) { + impl->data = std::make_shared<std::string>(); } - impl->response->data.append((char *)contents, size * nmemb); + impl->data->append((char *)contents, size * nmemb); return size * nmemb; } @@ -688,22 +689,27 @@ void HTTPCURLRequest::handleResult(CURLcode code) { // This is an unsolicited 304 response and should only happen on malfunctioning // HTTP servers. It likely doesn't include any data, but we don't have much options. response->status = Response::Successful; + response->data = std::move(data); return finish(ResponseStatus::Successful); } } else if (responseCode == 200) { response->status = Response::Successful; + response->data = std::move(data); return finish(ResponseStatus::Successful); } else if (responseCode == 404) { response->status = Response::NotFound; + response->data = std::move(data); return finish(ResponseStatus::Successful); } else if (responseCode >= 500 && responseCode < 600) { // Server errors may be temporary, so back off exponentially. response->status = Response::Error; + response->data = std::move(data); response->message = "HTTP status code " + util::toString(responseCode); return finish(ResponseStatus::TemporaryError); } else { // We don't know how to handle any other errors, so declare them as permanently failing. response->status = Response::Error; + response->data = std::move(data); response->message = "HTTP status code " + util::toString(responseCode); return finish(ResponseStatus::PermanentError); } diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp index 46df7ed2af..e618c960c4 100644 --- a/platform/default/sqlite_cache.cpp +++ b/platform/default/sqlite_cache.cpp @@ -117,9 +117,9 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) { response->modified = getStmt->get<int64_t>(1); response->etag = getStmt->get<std::string>(2); response->expires = getStmt->get<int64_t>(3); - response->data = getStmt->get<std::string>(4); + response->data = std::make_shared<std::string>(std::move(getStmt->get<std::string>(4))); if (getStmt->get<int>(5)) { // == compressed - response->data = util::decompress(response->data); + response->data = std::make_shared<std::string>(std::move(util::decompress(*response->data))); } callback(std::move(response)); } else { @@ -171,18 +171,21 @@ void SQLiteCache::Impl::put(const Resource& resource, std::shared_ptr<const Resp putStmt->bind(6 /* expires */, response->expires); std::string data; - if (resource.kind != Resource::SpriteImage) { + if (resource.kind != Resource::SpriteImage && response->data) { // Do not compress images, since they are typically compressed already. - data = util::compress(response->data); + data = util::compress(*response->data); } - if (!data.empty() && data.size() < response->data.size()) { + if (!data.empty() && data.size() < response->data->size()) { // Store the compressed data when it is smaller than the original // uncompressed data. putStmt->bind(7 /* data */, data, false); // do not retain the string internally. putStmt->bind(8 /* compressed */, true); + } else if (response->data) { + putStmt->bind(7 /* data */, *response->data, false); // do not retain the string internally. + putStmt->bind(8 /* compressed */, false); } else { - putStmt->bind(7 /* data */, response->data, false); // do not retain the string internally. + putStmt->bind(7 /* data */, "", false); putStmt->bind(8 /* compressed */, false); } diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp index fb4d47045b..211a696cd7 100644 --- a/platform/node/src/node_request.cpp +++ b/platform/node/src/node_request.cpp @@ -110,10 +110,10 @@ NAN_METHOD(NodeRequest::Respond) { if (Nan::Has(res, Nan::New("data").ToLocalChecked()).FromJust()) { auto dataHandle = Nan::Get(res, Nan::New("data").ToLocalChecked()).ToLocalChecked(); if (node::Buffer::HasInstance(dataHandle)) { - response->data = std::string { + response->data = std::make_shared<std::string>( node::Buffer::Data(dataHandle), node::Buffer::Length(dataHandle) - }; + ); } else { return Nan::ThrowTypeError("Response data must be a Buffer"); } diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp index b197ea12f3..11d631a57a 100644 --- a/src/mbgl/map/map_context.cpp +++ b/src/mbgl/map/map_context.cpp @@ -113,7 +113,7 @@ void MapContext::setStyleURL(const std::string& url) { styleRequest = nullptr; if (res.status == Response::Successful) { - loadStyleJSON(res.data, base); + loadStyleJSON(*res.data, base); } else if (res.status == Response::NotFound && styleURL.find("mapbox://") == 0) { Log::Error(Event::Setup, "style %s could not be found or is an incompatible legacy map or style", styleURL.c_str()); } else { diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 7e30682895..551a9eb190 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -163,7 +163,7 @@ void Source::load() { } rapidjson::Document d; - d.Parse<0>(res.data.c_str()); + d.Parse<0>(res.data->c_str()); if (d.HasParseError()) { std::stringstream message; diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index a54d96f005..cbd8c44338 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -20,10 +20,8 @@ namespace mbgl { struct Sprite::Loader { - bool loadedJSON = false; - bool loadedImage = false; - std::unique_ptr<Data> data = std::make_unique<Data>(); - + std::shared_ptr<const std::string> image; + std::shared_ptr<const std::string> json; RequestHolder jsonRequest; RequestHolder spriteRequest; }; @@ -50,8 +48,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) } loader->jsonRequest = nullptr; if (res.status == Response::Successful) { - loader->data->json = res.data; - loader->loadedJSON = true; + loader->json = res.data; } else { std::stringstream message; message << "Failed to load [" << jsonURL << "]: " << res.message; @@ -70,8 +67,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) } loader->spriteRequest = nullptr; if (res.status == Response::Successful) { - loader->data->image = res.data; - loader->loadedImage = true; + loader->image = res.data; } else { std::stringstream message; message << "Failed to load [" << spriteURL << "]: " << res.message; @@ -88,14 +84,12 @@ Sprite::~Sprite() { void Sprite::emitSpriteLoadedIfComplete() { assert(loader); - if (!loader->loadedImage || !loader->loadedJSON || !observer) { + if (!loader->image || !loader->json || !observer) { return; } - std::unique_ptr<Data> data(std::move(loader->data)); - loader.reset(); - - auto result = parseSprite(data->image, data->json); + auto local = std::move(loader); + auto result = parseSprite(*local->image, *local->json); if (result.is<Sprites>()) { loaded = true; observer->onSpriteLoaded(result.get<Sprites>()); diff --git a/src/mbgl/map/sprite.hpp b/src/mbgl/map/sprite.hpp index 32fa16de12..47ce1dbc8b 100644 --- a/src/mbgl/map/sprite.hpp +++ b/src/mbgl/map/sprite.hpp @@ -19,11 +19,6 @@ class Request; class Sprite : private util::noncopyable { public: - struct Data { - std::string image; - std::string json; - }; - class Observer { public: virtual ~Observer() = default; diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp index 1a1ff84061..9b4fa8622b 100644 --- a/src/mbgl/map/vector_tile_data.hpp +++ b/src/mbgl/map/vector_tile_data.hpp @@ -42,7 +42,7 @@ private: bool parsing = false; const SourceInfo& source; RequestHolder req; - std::string data; + std::shared_ptr<const std::string> data; float lastAngle = 0; float currentAngle; float lastPitch = 0; diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index 5d698fc0f0..3e4c94ce40 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -127,7 +127,6 @@ void DefaultFileSource::Impl::update(DefaultFileRequest* request) { if (!request->response->stale && request->response->isExpired()) { // Create a new Response object with `stale = true`, but the same data, and // replace the current request object we have. - // TODO: Make content shared_ptrs so we won't make copies of the content. auto response = std::make_shared<Response>(*request->response); response->stale = true; request->response = response; diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp index f351e66c2a..66008adb96 100644 --- a/src/mbgl/text/glyph_pbf.cpp +++ b/src/mbgl/text/glyph_pbf.cpp @@ -98,14 +98,15 @@ GlyphPBF::GlyphPBF(GlyphStore* store, GlyphPBF::~GlyphPBF() = default; void GlyphPBF::parse(GlyphStore* store, const std::string& fontStack, const std::string& url) { - if (data.empty()) { + assert(data); + if (data->empty()) { // If there is no data, this means we either haven't // received any data. return; } try { - parseGlyphPBF(**store->getFontStack(fontStack), std::move(data)); + parseGlyphPBF(**store->getFontStack(fontStack), *data); } catch (const std::exception& ex) { std::stringstream message; message << "Failed to parse [" << url << "]: " << ex.what(); diff --git a/src/mbgl/text/glyph_pbf.hpp b/src/mbgl/text/glyph_pbf.hpp index 205824bfe5..bf8567dbec 100644 --- a/src/mbgl/text/glyph_pbf.hpp +++ b/src/mbgl/text/glyph_pbf.hpp @@ -42,7 +42,7 @@ private: void parse(GlyphStore* store, const std::string& fontStack, const std::string& url); - std::string data; + std::shared_ptr<const std::string> data; std::atomic<bool> parsed; RequestHolder req; diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 71c930b4ef..fc599713b3 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -16,8 +16,8 @@ class Worker::Impl { public: Impl() = default; - void parseRasterTile(RasterBucket* bucket, std::string data, std::function<void (TileParseResult)> callback) { - std::unique_ptr<util::Image> image(new util::Image(data)); + void parseRasterTile(RasterBucket* bucket, const std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback) { + std::unique_ptr<util::Image> image(new util::Image(*data)); if (!(*image)) { callback(TileParseResult("error parsing raster image")); } @@ -29,9 +29,9 @@ public: callback(TileParseResult(TileData::State::parsed)); } - void parseVectorTile(TileWorker* worker, std::string data, std::function<void (TileParseResult)> callback) { + void parseVectorTile(TileWorker* worker, const std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback) { try { - pbf tilePBF(reinterpret_cast<const unsigned char *>(data.data()), data.size()); + pbf tilePBF(reinterpret_cast<const unsigned char*>(data->data()), data->size()); callback(worker->parse(VectorTile(tilePBF))); } catch (const std::exception& ex) { callback(TileParseResult(ex.what())); @@ -61,12 +61,12 @@ Worker::Worker(std::size_t count) { Worker::~Worker() = default; -std::unique_ptr<WorkRequest> Worker::parseRasterTile(RasterBucket& bucket, std::string data, std::function<void (TileParseResult)> callback) { +std::unique_ptr<WorkRequest> Worker::parseRasterTile(RasterBucket& bucket, const std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback) { current = (current + 1) % threads.size(); return threads[current]->invokeWithCallback(&Worker::Impl::parseRasterTile, callback, &bucket, data); } -std::unique_ptr<WorkRequest> Worker::parseVectorTile(TileWorker& worker, std::string data, std::function<void (TileParseResult)> callback) { +std::unique_ptr<WorkRequest> Worker::parseVectorTile(TileWorker& worker, const std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback) { current = (current + 1) % threads.size(); return threads[current]->invokeWithCallback(&Worker::Impl::parseVectorTile, callback, &worker, data); } diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp index 4e63b45abf..18b1bc92b1 100644 --- a/src/mbgl/util/worker.hpp +++ b/src/mbgl/util/worker.hpp @@ -33,12 +33,12 @@ public: Request parseRasterTile( RasterBucket&, - std::string data, + std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback); Request parseVectorTile( TileWorker&, - std::string data, + std::shared_ptr<const std::string> data, std::function<void (TileParseResult)> callback); Request parseLiveTile( diff --git a/test/fixtures/mock_file_source.cpp b/test/fixtures/mock_file_source.cpp index b420ba7298..407f54408f 100644 --- a/test/fixtures/mock_file_source.cpp +++ b/test/fixtures/mock_file_source.cpp @@ -55,7 +55,7 @@ void MockFileSource::Impl::replyWithSuccess(Request* req) const { res->status = Response::Status::Successful; try { - res->data = util::read_file(req->resource.url); + res->data = std::make_shared<const std::string>(std::move(util::read_file(req->resource.url))); } catch (const std::exception& err) { res->status = Response::Status::Error; res->message = err.what(); @@ -95,8 +95,9 @@ void MockFileSource::Impl::replyWithCorruptedData(Request* req) const { std::shared_ptr<Response> res = std::make_shared<Response>(); res->status = Response::Status::Successful; - res->data = util::read_file(req->resource.url); - res->data.insert(0, "CORRUPTED"); + auto data = std::make_shared<std::string>(std::move(util::read_file(req->resource.url))); + data->insert(0, "CORRUPTED"); + res->data = std::move(data); req->notify(res); } diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp index 1294513858..5b0923900e 100644 --- a/test/storage/cache_response.cpp +++ b/test/storage/cache_response.cpp @@ -20,7 +20,8 @@ TEST_F(Storage, CacheResponse) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response 1", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Response 1", *res.data); EXPECT_LT(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -36,7 +37,8 @@ TEST_F(Storage, CacheResponse) { fs.cancel(req); EXPECT_EQ(response.status, res.status); EXPECT_EQ(response.stale, res.stale); - EXPECT_EQ(response.data, res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ(*response.data, *res.data); EXPECT_EQ(response.expires, res.expires); EXPECT_EQ(response.modified, res.modified); EXPECT_EQ(response.etag, res.etag); diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp index 90633b83ac..d7dcbe71cd 100644 --- a/test/storage/cache_revalidate.cpp +++ b/test/storage/cache_revalidate.cpp @@ -13,8 +13,6 @@ TEST_F(Storage, CacheRevalidateSame) { SQLiteCache cache(":memory:"); DefaultFileSource fs(&cache); - const Response *reference = nullptr; - const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; Request* req1 = nullptr; Request* req2 = nullptr; @@ -27,21 +25,16 @@ TEST_F(Storage, CacheRevalidateSame) { } first = false; - EXPECT_EQ(nullptr, reference); - reference = &res; - EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Response", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("snowfall", res.etag); EXPECT_EQ("", res.message); req2 = fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { - // Make sure we get a different object than before, since this request should've been revalidated. - EXPECT_TRUE(reference != &res2); - if (res2.stale) { // Discard stale responses, if any. return; @@ -57,7 +50,9 @@ TEST_F(Storage, CacheRevalidateSame) { EXPECT_EQ(Response::Successful, res2.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response", res2.data); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ(res.data, res2.data); + EXPECT_EQ("Response", *res2.data); // We use this to indicate that a 304 reply came back. EXPECT_LT(0, res2.expires); EXPECT_EQ(0, res2.modified); @@ -80,8 +75,6 @@ TEST_F(Storage, CacheRevalidateModified) { SQLiteCache cache(":memory:"); DefaultFileSource fs(&cache); - const Response *reference = nullptr; - const Resource revalidateModified{ Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; Request* req1 = nullptr; @@ -95,21 +88,16 @@ TEST_F(Storage, CacheRevalidateModified) { } first = false; - EXPECT_EQ(nullptr, reference); - reference = &res; - EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Response", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(1420070400, res.modified); EXPECT_EQ("", res.etag); EXPECT_EQ("", res.message); req2 = fs.request(revalidateModified, uv_default_loop(), [&, res](const Response &res2) { - // Make sure we get a different object than before, since this request should've been revalidated. - EXPECT_TRUE(reference != &res2); - if (res2.stale) { // Discard stale responses, if any. return; @@ -124,8 +112,10 @@ TEST_F(Storage, CacheRevalidateModified) { req2 = nullptr; EXPECT_EQ(Response::Successful, res2.status); - EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response", res2.data); + EXPECT_EQ(false, res2.stale); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ("Response", *res2.data); + EXPECT_EQ(res.data, res2.data); // We use this to indicate that a 304 reply came back. EXPECT_LT(0, res2.expires); EXPECT_EQ(1420070400, res2.modified); @@ -147,8 +137,6 @@ TEST_F(Storage, CacheRevalidateEtag) { SQLiteCache cache(":memory:"); DefaultFileSource fs(&cache); - const Response *reference = nullptr; - const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" }; Request* req1 = nullptr; Request* req2 = nullptr; @@ -161,21 +149,16 @@ TEST_F(Storage, CacheRevalidateEtag) { } first = false; - EXPECT_EQ(nullptr, reference); - reference = &res; - EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response 1", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Response 1", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("response-1", res.etag); EXPECT_EQ("", res.message); req2 = fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { - // Make sure we get a different object than before, since this request should've been revalidated. - EXPECT_TRUE(reference != &res2); - if (res2.stale) { // Discard stale responses, if any. return; @@ -191,7 +174,9 @@ TEST_F(Storage, CacheRevalidateEtag) { EXPECT_EQ(Response::Successful, res2.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Response 2", res2.data); + ASSERT_TRUE(res2.data.get()); + EXPECT_NE(res.data, res2.data); + EXPECT_EQ("Response 2", *res2.data); EXPECT_EQ(0, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("response-2", res2.etag); diff --git a/test/storage/database.cpp b/test/storage/database.cpp index 20f96305b1..dd186f2a3f 100644 --- a/test/storage/database.cpp +++ b/test/storage/database.cpp @@ -178,11 +178,12 @@ TEST_F(Storage, DatabaseLockedWrite) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { ASSERT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + ASSERT_TRUE(res->data.get()); + EXPECT_EQ("Demo", *res->data); }); // Make sure that we got a no errors @@ -210,7 +211,7 @@ TEST_F(Storage, DatabaseLockedRefresh) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { EXPECT_EQ(nullptr, res.get()); @@ -226,7 +227,7 @@ TEST_F(Storage, DatabaseLockedRefresh) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.refresh({ Resource::Unknown, "mapbox://test" }, response->expires); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { EXPECT_EQ(nullptr, res.get()); @@ -255,11 +256,12 @@ TEST_F(Storage, DatabaseDeleted) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { ASSERT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + ASSERT_TRUE(res->data.get()); + EXPECT_EQ("Demo", *res->data); }); Log::removeObserver(); @@ -272,11 +274,12 @@ TEST_F(Storage, DatabaseDeleted) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { ASSERT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + ASSERT_TRUE(res->data.get()); + EXPECT_EQ("Demo", *res->data); }); auto observer = Log::removeObserver(); @@ -302,11 +305,12 @@ TEST_F(Storage, DatabaseInvalid) { Log::setObserver(std::make_unique<FixtureLogObserver>()); auto response = std::make_shared<Response>(); - response->data = "Demo"; + response->data = std::make_shared<std::string>("Demo"); cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) { ASSERT_NE(nullptr, res.get()); - EXPECT_EQ("Demo", res->data); + ASSERT_TRUE(res->data.get()); + EXPECT_EQ("Demo", *res->data); }); auto observer = Log::removeObserver(); diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index f0bc7ea6d2..f101252761 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -20,7 +20,7 @@ TEST_F(Storage, AssetReadDirectory) { fs.cancel(req); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ(0ul, res.data.size()); + ASSERT_FALSE(res.data.get()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index 434af703f2..ab7ba326ad 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -21,7 +21,8 @@ TEST_F(Storage, AssetEmptyFile) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ(0ul, res.data.size()); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("", *res.data); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); EXPECT_NE("", res.etag); @@ -48,12 +49,14 @@ TEST_F(Storage, AssetNonEmptyFile) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ(16ul, res.data.size()); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("content is here\n", *res.data); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); EXPECT_NE("", res.etag); EXPECT_EQ("", res.message); - EXPECT_EQ("content is here\n", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("content is here\n", *res.data); NonEmptyFile.finish(); }); @@ -76,7 +79,7 @@ TEST_F(Storage, AssetNonExistentFile) { fs.cancel(req); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ(0ul, res.data.size()); + ASSERT_FALSE(res.data.get()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_cancel.cpp b/test/storage/http_cancel.cpp index 5743ccb8f7..7dfe9c58d5 100644 --- a/test/storage/http_cancel.cpp +++ b/test/storage/http_cancel.cpp @@ -40,7 +40,8 @@ TEST_F(Storage, HTTPCancelMultiple) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_coalescing.cpp b/test/storage/http_coalescing.cpp index 8558362034..2748fb8757 100644 --- a/test/storage/http_coalescing.cpp +++ b/test/storage/http_coalescing.cpp @@ -27,7 +27,8 @@ TEST_F(Storage, HTTPCoalescing) { } EXPECT_EQ(Response::Successful, res.status); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -69,7 +70,8 @@ TEST_F(Storage, HTTPMultiple) { // Do not cancel the request right away. EXPECT_EQ(Response::Successful, res.status); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(2147483647, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -85,7 +87,8 @@ TEST_F(Storage, HTTPMultiple) { fs.cancel(req2); EXPECT_EQ(Response::Successful, res2.status); - EXPECT_EQ("Hello World!", res2.data); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ("Hello World!", *res2.data); EXPECT_EQ(2147483647, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("", res2.etag); @@ -115,7 +118,8 @@ TEST_F(Storage, HTTPStale) { req1 = fs.request(resource, uv_default_loop(), [&] (const Response &res) { // Do not cancel the request right away. EXPECT_EQ(Response::Successful, res.status); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(false, res.stale); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); @@ -132,7 +136,8 @@ TEST_F(Storage, HTTPStale) { // Start a second request for the same resource after the first one has been completed. req2 = fs.request(resource, uv_default_loop(), [&] (const Response &res2) { EXPECT_EQ(Response::Successful, res2.status); - EXPECT_EQ("Hello World!", res2.data); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ("Hello World!", *res2.data); EXPECT_EQ(0, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("", res2.etag); diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 50b46a41b8..d0eefb408c 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -38,7 +38,8 @@ TEST_F(Storage, HTTPError) { EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -67,7 +68,7 @@ TEST_F(Storage, HTTPError) { #else FAIL(); #endif - EXPECT_EQ("", res.data); + ASSERT_FALSE(res.data.get()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_header_parsing.cpp b/test/storage/http_header_parsing.cpp index 93fdcb6231..17c3ce16cf 100644 --- a/test/storage/http_header_parsing.cpp +++ b/test/storage/http_header_parsing.cpp @@ -21,7 +21,8 @@ TEST_F(Storage, HTTPHeaderParsing) { fs.cancel(req1); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(1420797926, res.expires); EXPECT_EQ(1420794326, res.modified); EXPECT_EQ("foo", res.etag); @@ -37,7 +38,8 @@ TEST_F(Storage, HTTPHeaderParsing) { fs.cancel(req2); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_GT(2, std::abs(res.expires - now - 120)) << "Expiration date isn't about 120 seconds in the future"; EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_issue_1369.cpp b/test/storage/http_issue_1369.cpp index ba4e3d3885..c5135b3ab9 100644 --- a/test/storage/http_issue_1369.cpp +++ b/test/storage/http_issue_1369.cpp @@ -34,7 +34,8 @@ TEST_F(Storage, HTTPIssue1369) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_load.cpp b/test/storage/http_load.cpp index 092bf2db5a..987e463507 100644 --- a/test/storage/http_load.cpp +++ b/test/storage/http_load.cpp @@ -26,7 +26,8 @@ TEST_F(Storage, HTTPLoad) { reqs[i] = nullptr; EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ(std::string("Request ") + std::to_string(current), res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ(std::string("Request ") + std::to_string(current), *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_other_loop.cpp b/test/storage/http_other_loop.cpp index cd9fad95be..f6e805285f 100644 --- a/test/storage/http_other_loop.cpp +++ b/test/storage/http_other_loop.cpp @@ -17,7 +17,8 @@ TEST_F(Storage, HTTPOtherLoop) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index c5e89b88e8..468ba33248 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -24,7 +24,8 @@ TEST_F(Storage, HTTPReading) { EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -38,6 +39,8 @@ TEST_F(Storage, HTTPReading) { EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::NotFound, res.status); EXPECT_EQ(false, res.stale); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Cannot GET /doesnotexist\n", *res.data); EXPECT_EQ("", res.message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); @@ -51,6 +54,8 @@ TEST_F(Storage, HTTPReading) { EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ(false, res.stale); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Server Error!", *res.data); EXPECT_EQ("HTTP status code 500", res.message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_timeout.cpp b/test/storage/http_timeout.cpp index 71553677bd..b5cd877e76 100644 --- a/test/storage/http_timeout.cpp +++ b/test/storage/http_timeout.cpp @@ -20,7 +20,8 @@ TEST_F(Storage, HTTPTimeout) { counter++; EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(false, res.stale); - EXPECT_EQ("Hello World!", res.data); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); EXPECT_LT(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/server.js b/test/storage/server.js index 342a61cdc1..efc1e1d6c0 100755 --- a/test/storage/server.js +++ b/test/storage/server.js @@ -74,7 +74,7 @@ app.get('/revalidate-etag', function(req, res) { }); app.get('/permanent-error', function(req, res) { - res.status(500).end(); + res.status(500).send('Server Error!'); }); var temporaryErrorCounter = 0; |