From 821b6bdb9ab87ad120e7cdcbd9391e78768dfee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 13 Jun 2018 15:50:23 +0200 Subject: [core] harden OfflineDatabase --- cmake/test-files.cmake | 2 + platform/default/default_file_source.cpp | 28 +- platform/default/mbgl/storage/offline_database.cpp | 295 ++++++---- platform/default/mbgl/storage/offline_database.hpp | 20 +- platform/default/mbgl/storage/offline_download.cpp | 31 +- platform/default/sqlite3.cpp | 48 +- platform/qt/src/sqlite3.cpp | 12 +- test/fixtures/offline_database/corrupt-delayed.db | Bin 0 -> 19456 bytes .../fixtures/offline_database/corrupt-immediate.db | Bin 0 -> 4096 bytes test/src/mbgl/test/fixture_log_observer.cpp | 6 +- test/src/mbgl/test/sqlite3_test_fs.cpp | 317 +++++++++++ test/src/mbgl/test/sqlite3_test_fs.hpp | 39 ++ test/storage/offline_database.test.cpp | 595 +++++++++++++++------ test/storage/offline_download.test.cpp | 81 +-- test/storage/sqlite.test.cpp | 6 - 15 files changed, 1126 insertions(+), 354 deletions(-) create mode 100644 test/fixtures/offline_database/corrupt-delayed.db create mode 100644 test/fixtures/offline_database/corrupt-immediate.db create mode 100644 test/src/mbgl/test/sqlite3_test_fs.cpp create mode 100644 test/src/mbgl/test/sqlite3_test_fs.hpp diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index ecd7700e97..80fe85701d 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -101,6 +101,8 @@ set(MBGL_TEST_FILES test/src/mbgl/test/fixture_log_observer.hpp test/src/mbgl/test/getrss.cpp test/src/mbgl/test/getrss.hpp + test/src/mbgl/test/sqlite3_test_fs.cpp + test/src/mbgl/test/sqlite3_test_fs.hpp test/src/mbgl/test/stub_file_source.cpp test/src/mbgl/test/stub_file_source.hpp test/src/mbgl/test/stub_geometry_tile_feature.hpp diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp index 89aabeb8d3..00fe26e8a2 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -79,9 +79,9 @@ public: } void getRegionStatus(int64_t regionID, std::function)> callback) { - try { - callback({}, getDownload(regionID).getStatus()); - } catch (...) { + if (auto download = getDownload(regionID)) { + callback({}, download->getStatus()); + } else { callback(std::current_exception(), {}); } } @@ -97,11 +97,15 @@ public: } void setRegionObserver(int64_t regionID, std::unique_ptr observer) { - getDownload(regionID).setObserver(std::move(observer)); + if (auto download = getDownload(regionID)) { + download->setObserver(std::move(observer)); + } } void setRegionDownloadState(int64_t regionID, OfflineRegionDownloadState state) { - getDownload(regionID).setState(state); + if (auto download = getDownload(regionID)) { + download->setState(state); + } } void request(AsyncRequest* req, Resource resource, ActorRef ref) { @@ -186,13 +190,19 @@ public: } private: - OfflineDownload& getDownload(int64_t regionID) { + OfflineDownload* getDownload(int64_t regionID) { auto it = downloads.find(regionID); if (it != downloads.end()) { - return *it->second; + return it->second.get(); + } + auto definition = offlineDatabase->getRegionDefinition(regionID); + if (!definition) { + return nullptr; } - return *downloads.emplace(regionID, - std::make_unique(regionID, offlineDatabase->getRegionDefinition(regionID), *offlineDatabase, onlineFileSource)).first->second; + + auto download = std::make_unique(regionID, std::move(*definition), + *offlineDatabase, onlineFileSource); + return downloads.emplace(regionID, std::move(download)).first->second.get(); } // shared so that destruction is done on the creating thread diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index 92e534634d..d9a06fbc92 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -13,7 +13,12 @@ namespace mbgl { OfflineDatabase::OfflineDatabase(std::string path_, uint64_t maximumCacheSize_) : path(std::move(path_)), maximumCacheSize(maximumCacheSize_) { - ensureSchema(); + try { + initialize(); + } catch (const std::exception& ex) { + handleError(ex, "open database"); + // Assume that we can't open the database right now and work with an empty database object. + } } OfflineDatabase::~OfflineDatabase() { @@ -22,110 +27,102 @@ OfflineDatabase::~OfflineDatabase() { try { statements.clear(); db.reset(); - } catch (mapbox::sqlite::Exception& ex) { - Log::Error(Event::Database, (int)ex.code, ex.what()); + } catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "close database"); } } -void OfflineDatabase::ensureSchema() { - auto result = mapbox::sqlite::Database::tryOpen(path, mapbox::sqlite::ReadWriteCreate); - if (result.is()) { - const auto& ex = result.get(); - if (ex.code == mapbox::sqlite::ResultCode::NotADB) { - // Corrupted; blow it away. - removeExisting(); - result = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate); - } else { - Log::Error(Event::Database, "Unexpected error connecting to database: %s", ex.what()); - throw ex; - } +void OfflineDatabase::initialize() { + assert(!db); + assert(statements.empty()); + + db = std::make_unique( + mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate)); + db->setBusyTimeout(Milliseconds::max()); + db->exec("PRAGMA foreign_keys = ON"); + + const auto userVersion = getPragma("PRAGMA user_version"); + switch (userVersion) { + case 0: + case 1: + // Newly created database, or old cache-only database; remove old table if it exists. + removeOldCacheTable(); + createSchema(); + return; + case 2: + migrateToVersion3(); + // fall through + case 3: + // Removed migration, see below. + // fall through + case 4: + migrateToVersion5(); + // fall through + case 5: + migrateToVersion6(); + // fall through + case 6: + // Happy path; we're done + return; + default: + // Downgrade: delete the database and try to reinitialize. + removeExisting(); + initialize(); } +} - try { - assert(result.is()); - db = std::make_unique(std::move(result.get())); - db->setBusyTimeout(Milliseconds::max()); - db->exec("PRAGMA foreign_keys = ON"); - - switch (userVersion()) { - case 0: - case 1: - // Newly created database, or old cache-only database; remove old table if it exists. - removeOldCacheTable(); - break; - case 2: - migrateToVersion3(); - // fall through - case 3: - case 4: - migrateToVersion5(); - // fall through - case 5: - migrateToVersion6(); - // fall through - case 6: - // happy path; we're done - return; - default: - // downgrade, delete the database - removeExisting(); - break; - } - } catch (const mapbox::sqlite::Exception& ex) { - // Unfortunately, SQLITE_NOTADB is not always reported upon opening the database. - // Apparently sometimes it is delayed until the first read operation. - if (ex.code == mapbox::sqlite::ResultCode::NotADB) { - removeExisting(); +void OfflineDatabase::handleError(const std::exception& ex, const char* action) { + if (auto sqliteEx = dynamic_cast(&ex)) { + if (sqliteEx->code == mapbox::sqlite::ResultCode::NotADB || + sqliteEx->code == mapbox::sqlite::ResultCode::Corrupt) { + // Corrupted; delete database so that we have a clean slate for the next operation. + Log::Error(Event::Database, static_cast(sqliteEx->code), "Can't %s: %s", action, sqliteEx->what()); + try { + removeExisting(); + } catch (const util::IOException& ioEx) { + Log::Error(Event::Database, ioEx.code, "Can't %s: %s", action, ioEx.what()); + } } else { - throw; - } - } - - try { - #include "offline_schema.cpp.include" - - // When downgrading the database, or when the database is corrupt, we've deleted the old database handle, - // so we need to reopen it. - if (!db) { - db = std::make_unique(mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate)); - db->setBusyTimeout(Milliseconds::max()); - db->exec("PRAGMA foreign_keys = ON"); + // We treat the error as temporary, and pretend we have an inaccessible DB. + Log::Warning(Event::Database, static_cast(sqliteEx->code), "Can't %s: %s", action, sqliteEx->what()); } - - db->exec("PRAGMA auto_vacuum = INCREMENTAL"); - db->exec("PRAGMA journal_mode = DELETE"); - db->exec("PRAGMA synchronous = FULL"); - db->exec(schema); - db->exec("PRAGMA user_version = 6"); - } catch (...) { - Log::Error(Event::Database, "Unexpected error creating database schema: %s", util::toString(std::current_exception()).c_str()); - throw; + } else if (auto ioEx = dynamic_cast(&ex)) { + // We failed to delete the database file. + Log::Error(Event::Database, ioEx->code, "Can't %s: %s", action, ioEx->what()); + } else { + Log::Error(Event::Database, -1, "Can't %s: unknown error", action); } } -int OfflineDatabase::userVersion() { - return static_cast(getPragma("PRAGMA user_version")); -} - void OfflineDatabase::removeExisting() { Log::Warning(Event::Database, "Removing existing incompatible offline database"); statements.clear(); db.reset(); - try { - util::deleteFile(path); - } catch (util::IOException& ex) { - Log::Error(Event::Database, ex.code, ex.what()); - } + util::deleteFile(path); } void OfflineDatabase::removeOldCacheTable() { + assert(db); db->exec("DROP TABLE IF EXISTS http_cache"); db->exec("VACUUM"); } +void OfflineDatabase::createSchema() { + #include "offline_schema.cpp.include" + assert(db); + db->exec("PRAGMA auto_vacuum = INCREMENTAL"); + db->exec("PRAGMA journal_mode = DELETE"); + db->exec("PRAGMA synchronous = FULL"); + mapbox::sqlite::Transaction transaction(*db); + db->exec(schema); + db->exec("PRAGMA user_version = 6"); + transaction.commit(); +} + void OfflineDatabase::migrateToVersion3() { + assert(db); db->exec("PRAGMA auto_vacuum = INCREMENTAL"); db->exec("VACUUM"); db->exec("PRAGMA user_version = 3"); @@ -138,12 +135,14 @@ void OfflineDatabase::migrateToVersion3() { // See: https://github.com/mapbox/mapbox-gl-native/pull/6320 void OfflineDatabase::migrateToVersion5() { + assert(db); db->exec("PRAGMA journal_mode = DELETE"); db->exec("PRAGMA synchronous = FULL"); db->exec("PRAGMA user_version = 5"); } void OfflineDatabase::migrateToVersion6() { + assert(db); mapbox::sqlite::Transaction transaction(*db); db->exec("ALTER TABLE resources ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0"); db->exec("ALTER TABLE tiles ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0"); @@ -152,6 +151,9 @@ void OfflineDatabase::migrateToVersion6() { } mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { + if (!db) { + initialize(); + } auto it = statements.find(sql); if (it == statements.end()) { it = statements.emplace(sql, std::make_unique(*db, sql)).first; @@ -159,9 +161,12 @@ mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { return *it->second; } -optional OfflineDatabase::get(const Resource& resource) { +optional OfflineDatabase::get(const Resource& resource) try { auto result = getInternal(resource); - return result ? result->first : optional(); + return result ? optional{ result->first } : nullopt; +} catch (const std::exception& ex) { + handleError(ex, "read resource"); + return nullopt; } optional> OfflineDatabase::getInternal(const Resource& resource) { @@ -182,11 +187,17 @@ optional OfflineDatabase::hasInternal(const Resource& resource) { } } -std::pair OfflineDatabase::put(const Resource& resource, const Response& response) { +std::pair OfflineDatabase::put(const Resource& resource, const Response& response) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate); auto result = putInternal(resource, response, true); transaction.commit(); return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "write resource"); + return { false, 0 }; } std::pair OfflineDatabase::putInternal(const Resource& resource, const Response& response, bool evict_) { @@ -227,11 +238,19 @@ std::pair OfflineDatabase::putInternal(const Resource& resource, optional> OfflineDatabase::getResource(const Resource& resource) { // Update accessed timestamp used for LRU eviction. - { + try { mapbox::sqlite::Query accessedQuery{ getStatement("UPDATE resources SET accessed = ?1 WHERE url = ?2") }; accessedQuery.bind(1, util::now()); accessedQuery.bind(2, resource.url); accessedQuery.run(); + } catch (const mapbox::sqlite::Exception& ex) { + if (ex.code == mapbox::sqlite::ResultCode::NotADB || + ex.code == mapbox::sqlite::ResultCode::Corrupt) { + throw; + } + + // If we don't have any indication that the database is corrupt, continue as usual. + Log::Warning(Event::Database, static_cast(ex.code), "Can't update timestamp: %s", ex.what()); } // clang-format off @@ -245,7 +264,7 @@ optional> OfflineDatabase::getResource(const Resou query.bind(1, resource.url); if (!query.run()) { - return {}; + return nullopt; } Response response; @@ -274,7 +293,7 @@ optional OfflineDatabase::hasResource(const Resource& resource) { mapbox::sqlite::Query query{ getStatement("SELECT length(data) FROM resources WHERE url = ?") }; query.bind(1, resource.url); if (!query.run()) { - return {}; + return nullopt; } return query.get>(0); @@ -366,7 +385,8 @@ bool OfflineDatabase::putResource(const Resource& resource, } optional> OfflineDatabase::getTile(const Resource::TileData& tile) { - { + // Update accessed timestamp used for LRU eviction. + try { // clang-format off mapbox::sqlite::Query accessedQuery{ getStatement( "UPDATE tiles " @@ -385,6 +405,13 @@ optional> OfflineDatabase::getTile(const Resource: accessedQuery.bind(5, tile.y); accessedQuery.bind(6, tile.z); accessedQuery.run(); + } catch (const mapbox::sqlite::Exception& ex) { + if (ex.code == mapbox::sqlite::ResultCode::NotADB || ex.code == mapbox::sqlite::ResultCode::Corrupt) { + throw; + } + + // If we don't have any indication that the database is corrupt, continue as usual. + Log::Warning(Event::Database, static_cast(ex.code), "Can't update timestamp: %s", ex.what()); } // clang-format off @@ -406,7 +433,7 @@ optional> OfflineDatabase::getTile(const Resource: query.bind(5, tile.z); if (!query.run()) { - return {}; + return nullopt; } Response response; @@ -450,7 +477,7 @@ optional OfflineDatabase::hasTile(const Resource::TileData& tile) { size.bind(5, tile.z); if (!size.run()) { - return {}; + return nullopt; } return size.get>(0); @@ -559,23 +586,30 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, return true; } -std::vector OfflineDatabase::listRegions() { +std::vector OfflineDatabase::listRegions() try { mapbox::sqlite::Query query{ getStatement("SELECT id, definition, description FROM regions") }; - std::vector result; - while (query.run()) { - result.push_back(OfflineRegion( - query.get(0), - decodeOfflineRegionDefinition(query.get(1)), - query.get>(2))); + const auto id = query.get(0); + const auto definition = query.get(1); + const auto description = query.get>(2); + try { + OfflineRegion region(id, decodeOfflineRegionDefinition(definition), description); + result.emplace_back(std::move(region)); + } catch (const std::exception& ex) { + // Catch errors from malformed offline region definitions + // and skip them. + Log::Error(Event::General, "%s", ex.what()); + } } - return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "list regions"); + return {}; } -OfflineRegion OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, - const OfflineRegionMetadata& metadata) { +optional OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, + const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "INSERT INTO regions (definition, description) " @@ -585,11 +619,13 @@ OfflineRegion OfflineDatabase::createRegion(const OfflineRegionDefinition& defin query.bind(1, encodeOfflineRegionDefinition(definition)); query.bindBlob(2, metadata); query.run(); - return OfflineRegion(query.lastInsertRowId(), definition, metadata); +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "create region"); + return nullopt; } -OfflineRegionMetadata OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) { +optional OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "UPDATE regions SET description = ?1 " @@ -600,9 +636,12 @@ OfflineRegionMetadata OfflineDatabase::updateMetadata(const int64_t regionID, co query.run(); return metadata; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "update region metadata"); + return nullopt; } -void OfflineDatabase::deleteRegion(OfflineRegion&& region) { +void OfflineDatabase::deleteRegion(OfflineRegion&& region) try { { mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") }; query.bind(1, region.getID()); @@ -610,13 +649,17 @@ void OfflineDatabase::deleteRegion(OfflineRegion&& region) { } evict(0); + assert(db); db->exec("PRAGMA incremental_vacuum"); // Ensure that the cached offlineTileCount value is recalculated. offlineMapboxTileCount = {}; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "delete region"); + return; } -optional> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) { +optional> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) try { auto response = getInternal(resource); if (response) { @@ -624,9 +667,12 @@ optional> OfflineDatabase::getRegionResource(int64 } return response; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "read region resource"); + return nullopt; } -optional OfflineDatabase::hasRegionResource(int64_t regionID, const Resource& resource) { +optional OfflineDatabase::hasRegionResource(int64_t regionID, const Resource& resource) try { auto response = hasInternal(resource); if (response) { @@ -634,16 +680,32 @@ optional OfflineDatabase::hasRegionResource(int64_t regionID, const Res } return response; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "query region resource"); + return nullopt; } -uint64_t OfflineDatabase::putRegionResource(int64_t regionID, const Resource& resource, const Response& response) { +uint64_t OfflineDatabase::putRegionResource(int64_t regionID, + const Resource& resource, + const Response& response) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db); auto size = putRegionResourceInternal(regionID, resource, response); transaction.commit(); return size; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "write region resource"); + return 0; } -void OfflineDatabase::putRegionResources(int64_t regionID, const std::list>& resources, OfflineRegionStatus& status) { +void OfflineDatabase::putRegionResources(int64_t regionID, + const std::list>& resources, + OfflineRegionStatus& status) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db); for (const auto& elem : resources) { @@ -667,6 +729,8 @@ void OfflineDatabase::putRegionResources(int64_t regionID, const std::list OfflineDatabase::getRegionDefinition(int64_t regionID) try { mapbox::sqlite::Query query{ getStatement("SELECT definition FROM regions WHERE id = ?1") }; query.bind(1, regionID); query.run(); return decodeOfflineRegionDefinition(query.get(0)); +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "load region"); + return nullopt; } -OfflineRegionStatus OfflineDatabase::getRegionCompletedStatus(int64_t regionID) { +optional OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { OfflineRegionStatus result; std::tie(result.completedResourceCount, result.completedResourceSize) @@ -786,6 +853,9 @@ OfflineRegionStatus OfflineDatabase::getRegionCompletedStatus(int64_t regionID) result.completedResourceSize += result.completedTileSize; return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "get region status"); + return nullopt; } std::pair OfflineDatabase::getCompletedResourceCountAndSize(int64_t regionID) { @@ -920,7 +990,7 @@ bool OfflineDatabase::offlineMapboxTileCountLimitExceeded() { return getOfflineMapboxTileCount() >= offlineMapboxTileCountLimit; } -uint64_t OfflineDatabase::getOfflineMapboxTileCount() { +uint64_t OfflineDatabase::getOfflineMapboxTileCount() try { // Calculating this on every call would be much simpler than caching and // manually updating the value, but it would make offline downloads an O(n²) // operation, because the database query below involves an index scan of @@ -942,6 +1012,9 @@ uint64_t OfflineDatabase::getOfflineMapboxTileCount() { offlineMapboxTileCount = query.get(0); return *offlineMapboxTileCount; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "get offline Mapbox tile count"); + return std::numeric_limits::max(); } bool OfflineDatabase::exceedsOfflineMapboxTileCountLimit(const Resource& resource) { diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp index 38eb3783ba..ee8bc6a3b0 100644 --- a/platform/default/mbgl/storage/offline_database.hpp +++ b/platform/default/mbgl/storage/offline_database.hpp @@ -44,10 +44,10 @@ public: std::vector listRegions(); - OfflineRegion createRegion(const OfflineRegionDefinition&, - const OfflineRegionMetadata&); + optional createRegion(const OfflineRegionDefinition&, + const OfflineRegionMetadata&); - OfflineRegionMetadata updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); + optional updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); void deleteRegion(OfflineRegion&&); @@ -57,8 +57,8 @@ public: uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&); void putRegionResources(int64_t regionID, const std::list>&, OfflineRegionStatus&); - OfflineRegionDefinition getRegionDefinition(int64_t regionID); - OfflineRegionStatus getRegionCompletedStatus(int64_t regionID); + optional getRegionDefinition(int64_t regionID); + optional getRegionCompletedStatus(int64_t regionID); void setOfflineMapboxTileCountLimit(uint64_t); uint64_t getOfflineMapboxTileCountLimit(); @@ -67,13 +67,15 @@ public: bool exceedsOfflineMapboxTileCountLimit(const Resource&); private: - int userVersion(); - void ensureSchema(); + void initialize(); + void handleError(const std::exception&, const char* action); + void removeExisting(); void removeOldCacheTable(); - void migrateToVersion3(); + void createSchema(); void migrateToVersion5(); - void migrateToVersion6(); + void migrateToVersion3(); + void migrateToVersion6(); mapbox::sqlite::Statement& getStatement(const char *); diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp index 4da51db655..179d2d5f57 100644 --- a/platform/default/mbgl/storage/offline_download.cpp +++ b/platform/default/mbgl/storage/offline_download.cpp @@ -62,39 +62,44 @@ OfflineRegionStatus OfflineDownload::getStatus() const { return status; } - OfflineRegionStatus result = offlineDatabase.getRegionCompletedStatus(id); + auto result = offlineDatabase.getRegionCompletedStatus(id); + if (!result) { + // We can't find this offline region because the database is unavailable, or the download + // does not exist. + return {}; + } - result.requiredResourceCount++; + result->requiredResourceCount++; optional styleResponse = offlineDatabase.get(Resource::style(definition.styleURL)); if (!styleResponse) { - return result; + return *result; } style::Parser parser; parser.parse(*styleResponse->data); - result.requiredResourceCountIsPrecise = true; + result->requiredResourceCountIsPrecise = true; for (const auto& source : parser.sources) { SourceType type = source->getType(); auto handleTiledSource = [&] (const variant& urlOrTileset, const uint16_t tileSize) { if (urlOrTileset.is()) { - result.requiredResourceCount += + result->requiredResourceCount += definition.tileCount(type, tileSize, urlOrTileset.get().zoomRange); } else { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; const auto& url = urlOrTileset.get(); optional sourceResponse = offlineDatabase.get(Resource::source(url)); if (sourceResponse) { style::conversion::Error error; optional tileset = style::conversion::convertJSON(*sourceResponse->data, error); if (tileset) { - result.requiredResourceCount += + result->requiredResourceCount += definition.tileCount(type, tileSize, (*tileset).zoomRange); } } else { - result.requiredResourceCountIsPrecise = false; + result->requiredResourceCountIsPrecise = false; } } }; @@ -121,7 +126,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { case SourceType::GeoJSON: { const auto& geojsonSource = *source->as(); if (geojsonSource.getURL()) { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; } break; } @@ -129,7 +134,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { case SourceType::Image: { const auto& imageSource = *source->as(); if (imageSource.getURL()) { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; } break; } @@ -142,14 +147,14 @@ OfflineRegionStatus OfflineDownload::getStatus() const { } if (!parser.glyphURL.empty()) { - result.requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; + result->requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; } if (!parser.spriteURL.empty()) { - result.requiredResourceCount += 2; + result->requiredResourceCount += 2; } - return result; + return *result; } void OfflineDownload::activateDownload() { diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp index 2e76f6eec4..e3012a532e 100644 --- a/platform/default/sqlite3.cpp +++ b/platform/default/sqlite3.cpp @@ -7,11 +7,38 @@ #include #include +#include #include namespace mapbox { namespace sqlite { +static_assert(mbgl::underlying_type(ResultCode::OK) == SQLITE_OK, "error"); +static_assert(mbgl::underlying_type(ResultCode::Error) == SQLITE_ERROR, "error"); +static_assert(mbgl::underlying_type(ResultCode::Internal) == SQLITE_INTERNAL, "error"); +static_assert(mbgl::underlying_type(ResultCode::Perm) == SQLITE_PERM, "error"); +static_assert(mbgl::underlying_type(ResultCode::Abort) == SQLITE_ABORT, "error"); +static_assert(mbgl::underlying_type(ResultCode::Busy) == SQLITE_BUSY, "error"); +static_assert(mbgl::underlying_type(ResultCode::Locked) == SQLITE_LOCKED, "error"); +static_assert(mbgl::underlying_type(ResultCode::NoMem) == SQLITE_NOMEM, "error"); +static_assert(mbgl::underlying_type(ResultCode::ReadOnly) == SQLITE_READONLY, "error"); +static_assert(mbgl::underlying_type(ResultCode::Interrupt) == SQLITE_INTERRUPT, "error"); +static_assert(mbgl::underlying_type(ResultCode::IOErr) == SQLITE_IOERR, "error"); +static_assert(mbgl::underlying_type(ResultCode::Corrupt) == SQLITE_CORRUPT, "error"); +static_assert(mbgl::underlying_type(ResultCode::NotFound) == SQLITE_NOTFOUND, "error"); +static_assert(mbgl::underlying_type(ResultCode::Full) == SQLITE_FULL, "error"); +static_assert(mbgl::underlying_type(ResultCode::CantOpen) == SQLITE_CANTOPEN, "error"); +static_assert(mbgl::underlying_type(ResultCode::Protocol) == SQLITE_PROTOCOL, "error"); +static_assert(mbgl::underlying_type(ResultCode::Schema) == SQLITE_SCHEMA, "error"); +static_assert(mbgl::underlying_type(ResultCode::TooBig) == SQLITE_TOOBIG, "error"); +static_assert(mbgl::underlying_type(ResultCode::Constraint) == SQLITE_CONSTRAINT, "error"); +static_assert(mbgl::underlying_type(ResultCode::Mismatch) == SQLITE_MISMATCH, "error"); +static_assert(mbgl::underlying_type(ResultCode::Misuse) == SQLITE_MISUSE, "error"); +static_assert(mbgl::underlying_type(ResultCode::NoLFS) == SQLITE_NOLFS, "error"); +static_assert(mbgl::underlying_type(ResultCode::Auth) == SQLITE_AUTH, "error"); +static_assert(mbgl::underlying_type(ResultCode::Range) == SQLITE_RANGE, "error"); +static_assert(mbgl::underlying_type(ResultCode::NotADB) == SQLITE_NOTADB, "error"); + class DatabaseImpl { public: DatabaseImpl(sqlite3* db_) @@ -23,7 +50,7 @@ public: { const int error = sqlite3_close(db); if (error != SQLITE_OK) { - mbgl::Log::Error(mbgl::Event::Database, "%s (Code %i)", sqlite3_errmsg(db), error); + mbgl::Log::Error(mbgl::Event::Database, error, "Failed to close database: %s", sqlite3_errmsg(db)); } } @@ -65,11 +92,8 @@ public: template using optional = std::experimental::optional; -static void errorLogCallback(void *, const int err, const char *msg) { - mbgl::Log::Record(mbgl::EventSeverity::Info, mbgl::Event::Database, err, "%s", msg); -} - -const static bool sqliteVersionCheck __attribute__((unused)) = []() { +__attribute__((constructor)) +static void initalize() { if (sqlite3_libversion_number() / 1000000 != SQLITE_VERSION_NUMBER / 1000000) { char message[96]; snprintf(message, 96, @@ -78,15 +102,17 @@ const static bool sqliteVersionCheck __attribute__((unused)) = []() { throw std::runtime_error(message); } +#ifndef NDEBUG // Enable SQLite logging before initializing the database. - sqlite3_config(SQLITE_CONFIG_LOG, errorLogCallback, nullptr); - - return true; -}(); + sqlite3_config(SQLITE_CONFIG_LOG, [](void *, const int err, const char *msg) { + mbgl::Log::Record(mbgl::EventSeverity::Debug, mbgl::Event::Database, err, "%s", msg); + }, nullptr); +#endif +} mapbox::util::variant Database::tryOpen(const std::string &filename, int flags) { sqlite3* db = nullptr; - const int error = sqlite3_open_v2(filename.c_str(), &db, flags, nullptr); + const int error = sqlite3_open_v2(filename.c_str(), &db, flags | SQLITE_OPEN_URI, nullptr); if (error != SQLITE_OK) { const auto message = sqlite3_errmsg(db); return Exception { error, message }; diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 2ca09fd3ad..96cee5fff8 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -23,13 +23,6 @@ namespace mapbox { namespace sqlite { -// https://www.sqlite.org/rescode.html#ok -static_assert(mbgl::underlying_type(ResultCode::OK) == 0, "error"); -// https://www.sqlite.org/rescode.html#cantopen -static_assert(mbgl::underlying_type(ResultCode::CantOpen) == 14, "error"); -// https://www.sqlite.org/rescode.html#notadb -static_assert(mbgl::underlying_type(ResultCode::NotADB) == 26, "error"); - void checkQueryError(const QSqlQuery& query) { QSqlError lastError = query.lastError(); if (lastError.type() != QSqlError::NoError) { @@ -114,6 +107,11 @@ mapbox::util::variant Database::tryOpen(const std::string & connectOptions.append("QSQLITE_OPEN_READONLY"); } + if (filename.compare(0, 5, "file:") == 0) { + if (!connectOptions.isEmpty()) connectOptions.append(';'); + connectOptions.append("QSQLITE_OPEN_URI"); + } + db.setConnectOptions(connectOptions); db.setDatabaseName(QString(filename.c_str())); diff --git a/test/fixtures/offline_database/corrupt-delayed.db b/test/fixtures/offline_database/corrupt-delayed.db new file mode 100644 index 0000000000..04989dbf36 Binary files /dev/null and b/test/fixtures/offline_database/corrupt-delayed.db differ diff --git a/test/fixtures/offline_database/corrupt-immediate.db b/test/fixtures/offline_database/corrupt-immediate.db new file mode 100644 index 0000000000..8909c402b2 Binary files /dev/null and b/test/fixtures/offline_database/corrupt-immediate.db differ diff --git a/test/src/mbgl/test/fixture_log_observer.cpp b/test/src/mbgl/test/fixture_log_observer.cpp index d8a4b9edce..d768c0284a 100644 --- a/test/src/mbgl/test/fixture_log_observer.cpp +++ b/test/src/mbgl/test/fixture_log_observer.cpp @@ -32,7 +32,9 @@ bool FixtureLog::Observer::onRecord(EventSeverity severity, const std::string& msg) { std::lock_guard lock(messagesMutex); - messages.emplace_back(severity, event, code, msg); + if (severity != EventSeverity::Debug) { + messages.emplace_back(severity, event, code, msg); + } return true; } @@ -48,7 +50,7 @@ size_t FixtureLog::Observer::count(const Message& message, bool substring) const size_t message_count = 0; for (const auto& msg : messages) { - if (msg.matches(message, substring)) { + if (!msg.checked && msg.matches(message, substring)) { message_count++; msg.checked = true; } diff --git a/test/src/mbgl/test/sqlite3_test_fs.cpp b/test/src/mbgl/test/sqlite3_test_fs.cpp new file mode 100644 index 0000000000..543071679b --- /dev/null +++ b/test/src/mbgl/test/sqlite3_test_fs.cpp @@ -0,0 +1,317 @@ +#ifndef __QT__ // Qt doesn't expose SQLite VFS + +#include + +#include + +#include +#include +#include +#include + +static bool sqlite3_test_fs_debug = false; +static bool sqlite3_test_fs_io = true; +static bool sqlite3_test_fs_file_open = true; +static bool sqlite3_test_fs_file_create = true; +static int64_t sqlite3_test_fs_read_limit = -1; +static int64_t sqlite3_test_fs_write_limit = -1; + +static sqlite3_vfs* unix_fs; + +static const sqlite3_io_methods* unix_fs_methods; + +static int sqlite3_test_fs_close(sqlite3_file* file) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: close(%p)\n", file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xClose(file); +} + +static int sqlite3_test_fs_read(sqlite3_file* file, void* ptr, int iAmt, sqlite3_int64 iOfst) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: read(%p, amount=%d, offset=%lld)\n", file, iAmt, iOfst); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + if (sqlite3_test_fs_read_limit >= 0) { + if (iAmt > sqlite3_test_fs_read_limit) { + iAmt = 0; + return SQLITE_IOERR; + } + sqlite3_test_fs_read_limit -= iAmt; + } + return unix_fs_methods->xRead(file, ptr, iAmt, iOfst); +} + +static int sqlite3_test_fs_write(sqlite3_file* file, const void* ptr, int iAmt, sqlite3_int64 iOfst) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: write(%p, amount=%d, offset=%lld)\n", file, iAmt, iOfst); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + if (sqlite3_test_fs_write_limit >= 0) { + if (iAmt > sqlite3_test_fs_write_limit) { + iAmt = 0; + return SQLITE_FULL; + } + sqlite3_test_fs_write_limit -= iAmt; + } + return unix_fs_methods->xWrite(file, ptr, iAmt, iOfst); +} + +static int sqlite3_test_fs_truncate(sqlite3_file* file, sqlite3_int64 size) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: truncate(%p, size=%lld)\n", file, size); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xTruncate(file, size); +} + +static int sqlite3_test_fs_sync(sqlite3_file* file, int flags) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: sync(%p, flags=%d)\n", file, flags); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xSync(file, flags); +} + +static int sqlite3_test_fs_file_size(sqlite3_file* file, sqlite3_int64* pSize) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: file_size(%p)\n", file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xFileSize(file, pSize); +} + +static int sqlite3_test_fs_lock(sqlite3_file* file, int lockType) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: lock(%p, type=%d)\n", file, lockType); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xLock(file, lockType); +} + +static int sqlite3_test_fs_unlock(sqlite3_file* file, int lockType) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: unlock(%p, type=%d)\n", file, lockType); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xUnlock(file, lockType); +} + +static int sqlite3_test_fs_check_reserved_lock(sqlite3_file* file, int* pResOut) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: check_reserved_lock(%p)\n", file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xCheckReservedLock(file, pResOut); +} + +static int sqlite3_test_fs_file_control(sqlite3_file* file, int op, void* pArg) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: file_control(%p, op=%d)\n", file, op); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xFileControl(file, op, pArg); +} + +static int sqlite3_test_fs_sector_size(sqlite3_file* file) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: sector_size(%p)\n", file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xSectorSize(file); +} + +static int sqlite3_test_fs_device_characteristics(sqlite3_file* file) { + if (sqlite3_test_fs_debug) { + // fprintf(stderr, "SQLite3: device_characteristics(%p)\n", file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs_methods->xDeviceCharacteristics(file); +} + +static sqlite3_vfs test_fs; + +static sqlite3_io_methods sqlite3_test_fs_methods = { + 1, + sqlite3_test_fs_close, + sqlite3_test_fs_read, + sqlite3_test_fs_write, + sqlite3_test_fs_truncate, + sqlite3_test_fs_sync, + sqlite3_test_fs_file_size, + sqlite3_test_fs_lock, + sqlite3_test_fs_unlock, + sqlite3_test_fs_check_reserved_lock, + sqlite3_test_fs_file_control, + sqlite3_test_fs_sector_size, + sqlite3_test_fs_device_characteristics, + 0, + 0, + 0, + 0, + 0, + 0 +}; + +static int sqlite3_test_fs_open(sqlite3_vfs* vfs, const char* zName, sqlite3_file* file, int flags, int* pOutFlags) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: open(name=%s, flags=%d) -> %p\n", zName, flags, file); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + if (!sqlite3_test_fs_file_open) { + return SQLITE_CANTOPEN; + } + if (!sqlite3_test_fs_file_create) { + int res; + const int result = unix_fs->xAccess(vfs, zName, SQLITE_ACCESS_EXISTS, &res); + if (result != SQLITE_OK) { + return result; + } + if (res != 1) { + return SQLITE_CANTOPEN; + } + } + const int status = unix_fs->xOpen(vfs, zName, file, flags, pOutFlags); + if (status == SQLITE_OK) { + unix_fs_methods = file->pMethods; + file->pMethods = &sqlite3_test_fs_methods; + } + return status; +} + +#include + +static int sqlite3_test_fs_delete(sqlite3_vfs* vfs, const char *zName, int syncDir) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: delete(name=%s, sync=%d)\n", zName, syncDir); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs->xDelete(vfs, zName, syncDir); +} + +static int sqlite3_test_fs_access(sqlite3_vfs* vfs, const char *zName, int flags, int *pResOut) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: access(name=%s, flags=%d)\n", zName, flags); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + return unix_fs->xAccess(vfs, zName, flags, pResOut); +} + +static bool sqlite3_test_fs_initialized = false; + +namespace mbgl { +namespace test { + +SQLite3TestFS::SQLite3TestFS() { + if (sqlite3_test_fs_initialized) { + throw std::runtime_error("SQLite3 Test FS is already initialized"); + } + + unix_fs = sqlite3_vfs_find("unix"); + if (!unix_fs) { + abort(); + } + + test_fs.iVersion = unix_fs->iVersion; + test_fs.szOsFile = unix_fs->szOsFile; + test_fs.mxPathname = unix_fs->mxPathname; + test_fs.zName = "test_fs"; + test_fs.pAppData = unix_fs->pAppData; + + test_fs.xOpen = sqlite3_test_fs_open; + test_fs.xDelete = sqlite3_test_fs_delete; + test_fs.xAccess = sqlite3_test_fs_access; + test_fs.xFullPathname = unix_fs->xFullPathname; + test_fs.xDlOpen = unix_fs->xDlOpen; + test_fs.xDlError = unix_fs->xDlError; + test_fs.xDlSym = unix_fs->xDlSym; + test_fs.xDlClose = unix_fs->xDlClose; + test_fs.xRandomness = unix_fs->xRandomness; + test_fs.xSleep = unix_fs->xSleep; + test_fs.xCurrentTime = unix_fs->xCurrentTime; + test_fs.xGetLastError = unix_fs->xGetLastError; + test_fs.xCurrentTimeInt64 = unix_fs->xCurrentTimeInt64; + test_fs.xSetSystemCall = unix_fs->xSetSystemCall; + test_fs.xGetSystemCall = unix_fs->xGetSystemCall; + test_fs.xNextSystemCall = unix_fs->xNextSystemCall; + + sqlite3_vfs_register(&test_fs, 0); + + sqlite3_test_fs_initialized = true; +} + +SQLite3TestFS::~SQLite3TestFS() { + assert(sqlite3_test_fs_initialized); + reset(); + sqlite3_vfs_unregister(&test_fs); + sqlite3_test_fs_initialized = false; +} + +void SQLite3TestFS::setDebug(bool value) { + sqlite3_test_fs_debug = value; +} + +void SQLite3TestFS::allowIO(bool value) { + sqlite3_test_fs_io = value; +} + +void SQLite3TestFS::allowFileOpen(bool value) { + sqlite3_test_fs_file_open = value; +} + +void SQLite3TestFS::allowFileCreate(bool value) { + sqlite3_test_fs_file_create = value; +} + +void SQLite3TestFS::setReadLimit(int64_t value) { + sqlite3_test_fs_read_limit = value; +} + +void SQLite3TestFS::setWriteLimit(int64_t value) { + sqlite3_test_fs_write_limit = value; +} + +void SQLite3TestFS::reset() { + setDebug(false); + allowIO(true); + allowFileOpen(true); + allowFileCreate(true); + setReadLimit(-1); + setWriteLimit(-1); +} + +} // namespace test +} // namespace mbgl + +#endif // __QT__ diff --git a/test/src/mbgl/test/sqlite3_test_fs.hpp b/test/src/mbgl/test/sqlite3_test_fs.hpp new file mode 100644 index 0000000000..00351f49ac --- /dev/null +++ b/test/src/mbgl/test/sqlite3_test_fs.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include + +namespace mbgl { +namespace test { + +class SQLite3TestFS { +public: + SQLite3TestFS(); + ~SQLite3TestFS(); + + // When enabled, the VFS will log all I/O operations to stdout. + void setDebug(bool); + + // Allow any type of I/O. Will fail with SQLITE_AUTH if set to false. This is useful to simulate + // scenarios where the OS blocks an entire app's I/O, e.g. when it's in the background. + void allowIO(bool); + + // Allow files to be opened. Will fail with SQLITE_CANTOPEN if set to false. + void allowFileOpen(bool); + + // Allow files to be created. Will fail with SQLITE_CANTOPEN if set to false. + void allowFileCreate(bool); + + // Allow N bytes to be read, then fail reads with SQLITE_IOERR. -1 == unlimited + // This limit is global, not per file. + void setReadLimit(int64_t); + + // Allow N bytes to be written, then fail writes with SQLITE_FULL. -1 == unlimited + // This limit is global, not per file. + void setWriteLimit(int64_t); + + // Reset all restrictions. + void reset(); +}; + +} // namespace test +} // namespace mbgl diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 10343ec305..5ca9d31a30 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -13,47 +14,232 @@ using namespace std::literals::string_literals; using namespace mbgl; +using mapbox::sqlite::ResultCode; static constexpr const char* filename = "test/fixtures/offline_database/offline.db"; +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +static constexpr const char* filename_test_fs = "file:test/fixtures/offline_database/offline.db?vfs=test_fs"; +#endif -TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Create)) { - FixtureLog log; +static void deleteDatabaseFiles() { + // Delete leftover journaling files as well. util::deleteFile(filename); + util::deleteFile(filename + "-wal"s); + util::deleteFile(filename + "-journal"s); +} + +static FixtureLog::Message error(ResultCode code, const char* message) { + return { EventSeverity::Error, Event::Database, static_cast(code), message }; +} + +static FixtureLog::Message warning(ResultCode code, const char* message) { + return { EventSeverity::Warning, Event::Database, static_cast(code), message }; +} + +static int databasePageCount(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma page_count" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} +static int databaseUserVersion(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma user_version" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static std::string databaseJournalMode(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma journal_mode" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static int databaseSyncMode(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma synchronous" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static std::vector databaseTableColumns(const std::string& path, const std::string& name) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + const auto sql = std::string("pragma table_info(") + name + ")"; + mapbox::sqlite::Statement stmt{ db, sql.c_str() }; + mapbox::sqlite::Query query{ stmt }; + std::vector columns; + while (query.run()) { + columns.push_back(query.get(1)); + } + return columns; +} + +namespace fixture { + +const Resource resource{ Resource::Style, "mapbox://test" }; +const Resource tile = Resource::tile("mapbox://test", 1, 0, 0, 0, Tileset::Scheme::XYZ); +const Response response = [] { + Response res; + res.data = std::make_shared("first"); + return res; +}(); + +} // namespace + +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Create)) { + FixtureLog log; + deleteDatabaseFiles(); OfflineDatabase db(filename); EXPECT_FALSE(bool(db.get({ Resource::Unknown, "mapbox://test" }))); EXPECT_EQ(0u, log.uncheckedCount()); } +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(CreateFail)) { + FixtureLog log; + deleteDatabaseFiles(); + test::SQLite3TestFS fs; + + // Opening the database will fail because our mock VFS returns a SQLITE_CANTOPEN error because + // it is not allowed to create the file. The OfflineDatabase object should handle this gracefully + // and treat it like an empty cache that can't be written to. + fs.allowFileCreate(false); + OfflineDatabase db(filename_test_fs); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't open database: unable to open database file"))); + + EXPECT_EQ(0u, log.uncheckedCount()); + + // We can try to insert things into the cache, but since the cache database isn't open, it won't be stored. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(res, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't write resource: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // We can also still "query" the database even though it is not open, and we will always get an empty result. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_FALSE(bool(db.get(res))); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't update timestamp: unable to open database file"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't read resource: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // Now, we're "freeing up" some space on the disk, and try to insert and query again. This time, we should + // be opening the datbase, creating the schema, and writing the data so that we can retrieve it again. + fs.allowFileCreate(true); + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + + // Next, set the file system to read only mode and try to read the data again. While we can't + // write anymore, we should still be able to read, and the query that tries to update the last + // accessed timestamp may fail without crashing. + fs.allowFileCreate(false); + fs.setWriteLimit(0); + for (const auto& res : { fixture::resource, fixture::tile }) { + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't update timestamp: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + fs.setDebug(false); + + // We're allowing SQLite to create a journal file, but restrict the number of bytes it + // can write so that it can start writing the journal file, but eventually fails during the + // timestamp update. + fs.allowFileCreate(true); + fs.setWriteLimit(8192); + for (const auto& res : { fixture::resource, fixture::tile }) { + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::Full, "Can't update timestamp: database or disk is full"))); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + + // Lastly, we're disabling all I/O to simulate a backgrounded app that is restricted from doing + // any disk I/O at all. + fs.setWriteLimit(-1); + fs.allowIO(false); + for (const auto& res : { fixture::resource, fixture::tile }) { + // First, try reading. + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + EXPECT_FALSE(result); + + // Now try inserting. + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(res, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // Allow deleting the database. + fs.reset(); +} +#endif // __QT__ + TEST(OfflineDatabase, TEST_REQUIRES_WRITE(SchemaVersion)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); { mapbox::sqlite::Database db = mapbox::sqlite::Database::open(filename, mapbox::sqlite::ReadWriteCreate); db.exec("PRAGMA user_version = 1"); } + { + OfflineDatabase db(filename); + } + + EXPECT_EQ(6, databaseUserVersion(filename)); + OfflineDatabase db(filename); + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } EXPECT_EQ(0u, log.uncheckedCount()); } TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Invalid)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::write_file(filename, "this is an invalid file"); OfflineDatabase db(filename); - -#ifndef __QT__ - // Only non-Qt platforms are setting a logger on the SQLite object. - EXPECT_EQ(1u, log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::NotADB), - "statement aborts at 1: [PRAGMA user_version] file is encrypted or is not a database" }, true)); -#endif - EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); - EXPECT_EQ(0u, log.uncheckedCount()); + EXPECT_EQ(1u, log.count(error(ResultCode::NotADB, "Can't open database: file is encrypted or is not a database"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } } TEST(OfflineDatabase, PutDoesNotStoreConnectionErrors) { @@ -114,7 +300,7 @@ TEST(OfflineDatabase, PutResource) { TEST(OfflineDatabase, TEST_REQUIRES_WRITE(GetResourceFromOfflineRegion)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db"); OfflineDatabase db(filename, mapbox::sqlite::ReadOnly); @@ -224,14 +410,15 @@ TEST(OfflineDatabase, CreateRegion) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - EXPECT_EQ(definition.styleURL, region.getDefinition().styleURL); - EXPECT_EQ(definition.bounds, region.getDefinition().bounds); - EXPECT_EQ(definition.minZoom, region.getDefinition().minZoom); - EXPECT_EQ(definition.maxZoom, region.getDefinition().maxZoom); - EXPECT_EQ(definition.pixelRatio, region.getDefinition().pixelRatio); - EXPECT_EQ(metadata, region.getMetadata()); + EXPECT_EQ(definition.styleURL, region->getDefinition().styleURL); + EXPECT_EQ(definition.bounds, region->getDefinition().bounds); + EXPECT_EQ(definition.minZoom, region->getDefinition().minZoom); + EXPECT_EQ(definition.maxZoom, region->getDefinition().maxZoom); + EXPECT_EQ(definition.pixelRatio, region->getDefinition().pixelRatio); + EXPECT_EQ(metadata, region->getMetadata()); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -241,10 +428,11 @@ TEST(OfflineDatabase, UpdateMetadata) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); OfflineRegionMetadata newmetadata {{ 4, 5, 6 }}; - db.updateMetadata(region.getID(), newmetadata); + db.updateMetadata(region->getID(), newmetadata); EXPECT_EQ(db.listRegions().at(0).getMetadata(), newmetadata); EXPECT_EQ(0u, log.uncheckedCount()); @@ -256,11 +444,12 @@ TEST(OfflineDatabase, ListRegions) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); std::vector regions = db.listRegions(); ASSERT_EQ(1u, regions.size()); - EXPECT_EQ(region.getID(), regions.at(0).getID()); + EXPECT_EQ(region->getID(), regions.at(0).getID()); EXPECT_EQ(definition.styleURL, regions.at(0).getDefinition().styleURL); EXPECT_EQ(definition.bounds, regions.at(0).getDefinition().bounds); EXPECT_EQ(definition.minZoom, regions.at(0).getDefinition().minZoom); @@ -277,14 +466,16 @@ TEST(OfflineDatabase, GetRegionDefinition) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); - OfflineRegionDefinition result = db.getRegionDefinition(region.getID()); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); + auto result = db.getRegionDefinition(region->getID()); + ASSERT_TRUE(result); - EXPECT_EQ(definition.styleURL, result.styleURL); - EXPECT_EQ(definition.bounds, result.bounds); - EXPECT_EQ(definition.minZoom, result.minZoom); - EXPECT_EQ(definition.maxZoom, result.maxZoom); - EXPECT_EQ(definition.pixelRatio, result.pixelRatio); + EXPECT_EQ(definition.styleURL, result->styleURL); + EXPECT_EQ(definition.bounds, result->bounds); + EXPECT_EQ(definition.minZoom, result->minZoom); + EXPECT_EQ(definition.maxZoom, result->maxZoom); + EXPECT_EQ(definition.pixelRatio, result->pixelRatio); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -294,15 +485,16 @@ TEST(OfflineDatabase, DeleteRegion) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); Response response; response.noContent = true; - db.putRegionResource(region.getID(), Resource::style("http://example.com/"), response); - db.putRegionResource(region.getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"), response); + db.putRegionResource(region->getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); - db.deleteRegion(std::move(region)); + db.deleteRegion(std::move(*region)); ASSERT_EQ(0u, db.listRegions().size()); @@ -314,38 +506,35 @@ TEST(OfflineDatabase, CreateRegionInfiniteMaxZoom) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; OfflineRegionMetadata metadata; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - EXPECT_EQ(0, region.getDefinition().minZoom); - EXPECT_EQ(INFINITY, region.getDefinition().maxZoom); + EXPECT_EQ(0, region->getDefinition().minZoom); + EXPECT_EQ(INFINITY, region->getDefinition().maxZoom); EXPECT_EQ(0u, log.uncheckedCount()); } TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ConcurrentUse)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); OfflineDatabase db1(filename); EXPECT_EQ(0u, log.uncheckedCount()); OfflineDatabase db2(filename); - Resource resource { Resource::Style, "http://example.com/" }; - Response response; - response.noContent = true; - std::thread thread1([&] { for (auto i = 0; i < 100; i++) { - db1.put(resource, response); - EXPECT_TRUE(bool(db1.get(resource))); + db1.put(fixture::resource, fixture::response); + EXPECT_TRUE(bool(db1.get(fixture::resource))); } }); std::thread thread2([&] { for (auto i = 0; i < 100; i++) { - db2.put(resource, response); - EXPECT_TRUE(bool(db2.get(resource))); + db2.put(fixture::resource, fixture::response); + EXPECT_TRUE(bool(db2.get(fixture::resource))); } }); @@ -407,13 +596,14 @@ TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Response response; response.data = randomString(1024); for (uint32_t i = 1; i <= 100; i++) { - db.putRegionResource(region.getID(), Resource::style("http://example.com/"s + util::toString(i)), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"s + util::toString(i)), response); } EXPECT_TRUE(bool(db.get(Resource::style("http://example.com/1")))); @@ -444,32 +634,36 @@ TEST(OfflineDatabase, GetRegionCompletedStatus) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - OfflineRegionStatus status1 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(0u, status1.completedResourceCount); - EXPECT_EQ(0u, status1.completedResourceSize); - EXPECT_EQ(0u, status1.completedTileCount); - EXPECT_EQ(0u, status1.completedTileSize); + auto status1 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status1); + EXPECT_EQ(0u, status1->completedResourceCount); + EXPECT_EQ(0u, status1->completedResourceSize); + EXPECT_EQ(0u, status1->completedTileCount); + EXPECT_EQ(0u, status1->completedTileSize); Response response; response.data = std::make_shared("data"); - uint64_t styleSize = db.putRegionResource(region.getID(), Resource::style("http://example.com/"), response); + uint64_t styleSize = db.putRegionResource(region->getID(), Resource::style("http://example.com/"), response); - OfflineRegionStatus status2 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(1u, status2.completedResourceCount); - EXPECT_EQ(styleSize, status2.completedResourceSize); - EXPECT_EQ(0u, status2.completedTileCount); - EXPECT_EQ(0u, status2.completedTileSize); + auto status2 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status2); + EXPECT_EQ(1u, status2->completedResourceCount); + EXPECT_EQ(styleSize, status2->completedResourceSize); + EXPECT_EQ(0u, status2->completedTileCount); + EXPECT_EQ(0u, status2->completedTileSize); - uint64_t tileSize = db.putRegionResource(region.getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); + uint64_t tileSize = db.putRegionResource(region->getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); - OfflineRegionStatus status3 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(2u, status3.completedResourceCount); - EXPECT_EQ(styleSize + tileSize, status3.completedResourceSize); - EXPECT_EQ(1u, status3.completedTileCount); - EXPECT_EQ(tileSize, status3.completedTileSize); + auto status3 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status3); + EXPECT_EQ(2u, status3->completedResourceCount); + EXPECT_EQ(styleSize + tileSize, status3->completedResourceSize); + EXPECT_EQ(1u, status3->completedTileCount); + EXPECT_EQ(tileSize, status3->completedTileSize); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -478,21 +672,22 @@ TEST(OfflineDatabase, HasRegionResource) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/1")))); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/1")))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); Response response; response.data = randomString(1024); for (uint32_t i = 1; i <= 100; i++) { - db.putRegionResource(region.getID(), Resource::style("http://example.com/"s + util::toString(i)), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"s + util::toString(i)), response); } - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/1")))); - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); - EXPECT_EQ(1024, *(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/1")))); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); + EXPECT_EQ(1024, *(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -501,7 +696,8 @@ TEST(OfflineDatabase, HasRegionResourceTile) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Resource resource { Resource::Tile, "http://example.com/" }; resource.tileData = Resource::TileData { @@ -515,15 +711,16 @@ TEST(OfflineDatabase, HasRegionResourceTile) { response.data = std::make_shared("first"); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), resource))); - db.putRegionResource(region.getID(), resource, response); - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), resource))); - EXPECT_EQ(5, *(db.hasRegionResource(region.getID(), resource))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), resource))); + db.putRegionResource(region->getID(), resource, response); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), resource))); + EXPECT_EQ(5, *(db.hasRegionResource(region->getID(), resource))); - OfflineRegion anotherRegion = db.createRegion(definition, OfflineRegionMetadata()); - EXPECT_LT(region.getID(), anotherRegion.getID()); - EXPECT_TRUE(bool(db.hasRegionResource(anotherRegion.getID(), resource))); - EXPECT_EQ(5, *(db.hasRegionResource(anotherRegion.getID(), resource))); + auto anotherRegion = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(anotherRegion); + EXPECT_LT(region->getID(), anotherRegion->getID()); + EXPECT_TRUE(bool(db.hasRegionResource(anotherRegion->getID(), resource))); + EXPECT_EQ(5, *(db.hasRegionResource(anotherRegion->getID(), resource))); EXPECT_EQ(0u, log.uncheckedCount()); @@ -535,8 +732,10 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata; - OfflineRegion region1 = db.createRegion(definition, metadata); - OfflineRegion region2 = db.createRegion(definition, metadata); + auto region1 = db.createRegion(definition, metadata); + ASSERT_TRUE(region1); + auto region2 = db.createRegion(definition, metadata); + ASSERT_TRUE(region2); Resource nonMapboxTile = Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); Resource mapboxTile1 = Resource::tile("mapbox://tiles/1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); @@ -549,27 +748,27 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a non-tile resource. - db.putRegionResource(region1.getID(), Resource::style("http://example.com/"), response); + db.putRegionResource(region1->getID(), Resource::style("http://example.com/"), response); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a non-Mapbox tile. - db.putRegionResource(region1.getID(), nonMapboxTile, response); + db.putRegionResource(region1->getID(), nonMapboxTile, response); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count increases after putting a Mapbox tile not used by another region. - db.putRegionResource(region1.getID(), mapboxTile1, response); + db.putRegionResource(region1->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a Mapbox tile used by another region. - db.putRegionResource(region2.getID(), mapboxTile1, response); + db.putRegionResource(region2->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a Mapbox tile used by the same region. - db.putRegionResource(region2.getID(), mapboxTile1, response); + db.putRegionResource(region2->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after deleting a region when the tile is still used by another region. - db.deleteRegion(std::move(region2)); + db.deleteRegion(std::move(*region2)); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after the putting a non-offline Mapbox tile. @@ -577,11 +776,11 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count increases after putting a pre-existing, but non-offline Mapbox tile. - db.putRegionResource(region1.getID(), mapboxTile2, response); + db.putRegionResource(region1->getID(), mapboxTile2, response); EXPECT_EQ(2u, db.getOfflineMapboxTileCount()); // Count decreases after deleting a region when the tiles are not used by other regions. - db.deleteRegion(std::move(region1)); + db.deleteRegion(std::move(*region1)); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); EXPECT_EQ(0u, log.uncheckedCount()); @@ -592,7 +791,8 @@ TEST(OfflineDatabase, BatchInsertion) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Response response; response.data = randomString(1024); @@ -603,7 +803,7 @@ TEST(OfflineDatabase, BatchInsertion) { } OfflineRegionStatus status; - db.putRegionResources(region.getID(), resources, status); + db.putRegionResources(region->getID(), resources, status); for (uint32_t i = 1; i <= 100; i++) { EXPECT_TRUE(bool(db.get(Resource::style("http://example.com/"s + util::toString(i))))); @@ -617,80 +817,39 @@ TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { OfflineDatabase db(":memory:", 1024 * 100); db.setOfflineMapboxTileCountLimit(1); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); - + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); + Response response; response.data = randomString(1024); std::list> resources; - + resources.emplace_back(Resource::style("http://example.com/"), response); resources.emplace_back(Resource::tile("mapbox://tiles/1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); resources.emplace_back(Resource::tile("mapbox://tiles/2", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); - + OfflineRegionStatus status; try { - db.putRegionResources(region.getID(), resources, status); + db.putRegionResources(region->getID(), resources, status); EXPECT_FALSE(true); } catch (const MapboxTileLimitExceededException&) { // Expected } - - EXPECT_EQ(status.completedTileCount, 1u); - EXPECT_EQ(status.completedResourceCount, 2u); - EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedTileCount, 1u); - EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedResourceCount, 2u); - - EXPECT_EQ(0u, log.uncheckedCount()); -} - -static int databasePageCount(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma page_count" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static int databaseUserVersion(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma user_version" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} -static std::string databaseJournalMode(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma journal_mode" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} + EXPECT_EQ(1u, status.completedTileCount); + EXPECT_EQ(2u, status.completedResourceCount); + const auto completedStatus = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(completedStatus); + EXPECT_EQ(1u, completedStatus->completedTileCount); + EXPECT_EQ(2u, completedStatus->completedResourceCount); -static int databaseSyncMode(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma synchronous" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static std::vector databaseTableColumns(const std::string& path, const std::string& name) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - const auto sql = std::string("pragma table_info(") + name + ")"; - mapbox::sqlite::Statement stmt{ db, sql.c_str() }; - mapbox::sqlite::Query query{ stmt }; - std::vector columns; - while (query.run()) { - columns.push_back(query.get(1)); - } - return columns; + EXPECT_EQ(0u, log.uncheckedCount()); } TEST(OfflineDatabase, MigrateFromV2Schema) { // v2.db is a v2 database containing a single offline region with a small number of resources. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v2.db"); { @@ -711,7 +870,7 @@ TEST(OfflineDatabase, MigrateFromV2Schema) { TEST(OfflineDatabase, MigrateFromV3Schema) { // v3.db is a v3 database, migrated from v2. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v3.db"); { @@ -730,7 +889,7 @@ TEST(OfflineDatabase, MigrateFromV3Schema) { TEST(OfflineDatabase, MigrateFromV4Schema) { // v4.db is a v4 database, migrated from v2 & v3. This database used `journal_mode = WAL` and `synchronous = NORMAL`. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v4.db"); { @@ -756,7 +915,7 @@ TEST(OfflineDatabase, MigrateFromV4Schema) { TEST(OfflineDatabase, MigrateFromV5Schema) { // v5.db is a v5 database, migrated from v2, v3 & v4. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v5.db"); { @@ -804,3 +963,133 @@ TEST(OfflineDatabase, DowngradeSchema) { EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); EXPECT_EQ(0u, log.uncheckedCount()); } + +TEST(OfflineDatabase, CorruptDatabaseOnOpen) { + FixtureLog log; + util::deleteFile(filename); + util::copyFile(filename, "test/fixtures/offline_database/corrupt-immediate.db"); + + // This database is corrupt in a way that will prevent opening the database. + OfflineDatabase db(filename); + EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't open database: database disk image is malformed"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } +} + +TEST(OfflineDatabase, CorruptDatabaseOnQuery) { + FixtureLog log; + util::deleteFile(filename); + util::copyFile(filename, "test/fixtures/offline_database/corrupt-delayed.db"); + + // This database is corrupt in a way that won't manifest itself until we start querying it, + // so just opening it will not cause an error. + OfflineDatabase db(filename); + + // Just opening this corrupt database should not have produced an error yet, since + // PRAGMA user_version still succeeds with this database. + EXPECT_EQ(0u, log.uncheckedCount()); + + // The first request fails because the database is corrupt and has to be recreated. + EXPECT_EQ(nullopt, db.get(fixture::tile)); + EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't read resource: database disk image is malformed"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } +} + +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { + FixtureLog log; + deleteDatabaseFiles(); + test::SQLite3TestFS fs; + + OfflineDatabase db(filename_test_fs); + EXPECT_EQ(0u, log.uncheckedCount()); + + // First, create a region object so that we can try deleting it later. + OfflineTilePyramidRegionDefinition definition( + "mapbox://style", LatLngBounds::hull({ 37.66, -122.57 }, { 37.83, -122.32 }), 0, 8, 2); + auto region = db.createRegion(definition, {}); + ASSERT_TRUE(region); + + // Now forbid any type of IO on the database and test that none of the calls crashes. + fs.allowIO(false); + + EXPECT_EQ(nullopt, db.get(fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(fixture::resource, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + const auto regions = db.listRegions(); + EXPECT_TRUE(regions.empty()); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't list regions: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.createRegion(definition, {})); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't create region: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.updateMetadata(region->getID(), {})); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update region metadata: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.getRegionResource(region->getID(), fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.hasRegionResource(region->getID(), fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't query region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(0u, db.putRegionResource(region->getID(), fixture::resource, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + OfflineRegionStatus status; + db.putRegionResources(region->getID(), { std::make_tuple(fixture::resource, fixture::response) }, status); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write region resources: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.getRegionDefinition(region->getID())); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't load region: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.getRegionCompletedStatus(region->getID())); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't get region status: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + db.deleteRegion(std::move(*region)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't delete region: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(std::numeric_limits::max(), db.getOfflineMapboxTileCount()); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't get offline Mapbox tile count: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + fs.reset(); +} +#endif // __QT__ diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index 57780eba40..d54b6b2131 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -42,10 +42,10 @@ public: OfflineDatabase db { ":memory:" }; std::size_t size = 0; - OfflineRegion createRegion() { + optional createRegion() { OfflineRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0 }; OfflineRegionMetadata metadata; - return db.createRegion(definition, metadata); + return db.createRegion(definition, metadata); } Response response(const std::string& path) { @@ -60,9 +60,10 @@ public: TEST(OfflineDownload, NoSubresources) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -100,9 +101,10 @@ TEST(OfflineDownload, NoSubresources) { TEST(OfflineDownload, InlineSource) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -140,9 +142,10 @@ TEST(OfflineDownload, InlineSource) { TEST(OfflineDownload, GeoJSONSource) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -175,9 +178,10 @@ TEST(OfflineDownload, GeoJSONSource) { TEST(OfflineDownload, Activate) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -250,9 +254,10 @@ TEST(OfflineDownload, Activate) { TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { FakeFileSource fileSource; OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, fileSource); @@ -272,9 +277,10 @@ TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { TEST(OfflineDownload, GetStatusNoResources) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); OfflineRegionStatus status = download.getStatus(); @@ -289,9 +295,10 @@ TEST(OfflineDownload, GetStatusNoResources) { TEST(OfflineDownload, GetStatusStyleComplete) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -311,9 +318,10 @@ TEST(OfflineDownload, GetStatusStyleComplete) { TEST(OfflineDownload, GetStatusStyleAndSourceComplete) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -337,9 +345,10 @@ TEST(OfflineDownload, GetStatusStyleAndSourceComplete) { TEST(OfflineDownload, RequestError) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -365,9 +374,10 @@ TEST(OfflineDownload, RequestError) { TEST(OfflineDownload, RequestErrorsAreRetried) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -398,9 +408,10 @@ TEST(OfflineDownload, RequestErrorsAreRetried) { TEST(OfflineDownload, TileCountLimitExceededNoTileResponse) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -440,9 +451,10 @@ TEST(OfflineDownload, TileCountLimitExceededNoTileResponse) { TEST(OfflineDownload, TileCountLimitExceededWithTileResponse) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -494,9 +506,10 @@ TEST(OfflineDownload, TileCountLimitExceededWithTileResponse) { TEST(OfflineDownload, WithPreviouslyExistingTile) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -528,9 +541,10 @@ TEST(OfflineDownload, WithPreviouslyExistingTile) { TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -556,7 +570,7 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { test.loop.run(); OfflineDownload redownload( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -595,9 +609,10 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { TEST(OfflineDownload, Deactivate) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); diff --git a/test/storage/sqlite.test.cpp b/test/storage/sqlite.test.cpp index 22958c8bed..cdbb7f26d7 100644 --- a/test/storage/sqlite.test.cpp +++ b/test/storage/sqlite.test.cpp @@ -38,12 +38,6 @@ TEST(SQLite, TEST_REQUIRES_WRITE(TryOpen)) { auto result = mapbox::sqlite::Database::tryOpen("test/fixtures/offline_database/foobar123.db", mapbox::sqlite::ReadOnly); ASSERT_TRUE(result.is()); ASSERT_EQ(result.get().code, mapbox::sqlite::ResultCode::CantOpen); - -#ifndef __QT__ - // Only non-Qt platforms are setting a logger on the SQLite object. - EXPECT_EQ(1u, log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::CantOpen), "cannot open file" }, true)); - EXPECT_EQ(1u, log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::CantOpen), "No such file or directory" }, true)); -#endif EXPECT_EQ(0u, log.uncheckedCount()); } -- cgit v1.2.1