From 880d37cda567114de29d076a3b7648dad830140a Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Tue, 21 Aug 2018 13:09:26 -0700 Subject: Add unit tests and fixtures for OfflineDatabase::mergeDatabase --- platform/default/mbgl/storage/offline_database.cpp | 10 +- src/mbgl/util/io.cpp | 2 +- test/fixtures/offline_database/sideload_ambient.db | Bin 0 -> 90112 bytes test/fixtures/offline_database/sideload_sat.db | Bin 0 -> 73728 bytes .../offline_database/sideload_sat_multiple.db | Bin 0 -> 90112 bytes test/storage/offline_database.test.cpp | 203 +++++++++++++++++++++ 6 files changed, 209 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/offline_database/sideload_ambient.db create mode 100644 test/fixtures/offline_database/sideload_sat.db create mode 100644 test/fixtures/offline_database/sideload_sat_multiple.db diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index 30b76d1666..8256912d39 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -648,7 +648,7 @@ OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { query.bind(1, sideDatabasePath); query.run(); } catch (const mapbox::sqlite::Exception& ex) { - handleError(ex, "merge databse attach"); + Log::Error(Event::Database, static_cast(ex.code), "Can't attach database (%s) for merge: %s", sideDatabasePath.c_str(), ex.what()); return unexpected(std::current_exception()); } try { @@ -656,8 +656,7 @@ OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { //database and attaches it. Check for matching schema version. auto sideUserVersion = static_cast(getPragma("PRAGMA side.user_version")); if (sideUserVersion != 6) { - Log::Warning(Event::Database, "Merge database does not match user_version of main database"); - throw std::runtime_error("merge database does not match schema or has incorrect user_version"); + throw std::runtime_error("Merge database does not match schema or has incorrect user_version"); } mapbox::sqlite::Transaction transaction(*db); @@ -682,9 +681,10 @@ OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { db->exec("DETACH DATABASE side"); // Explicit move to avoid triggering the copy constructor. return { std::move(result) }; - } catch (const mapbox::sqlite::Exception& ex) { + } catch (const std::runtime_error& ex) { db->exec("DETACH DATABASE side"); - handleError(ex, "merge databse post merge sql"); + Log::Error(Event::Database, "%s", ex.what()); + return unexpected(std::current_exception()); } return {}; diff --git a/src/mbgl/util/io.cpp b/src/mbgl/util/io.cpp index c84634ac88..84a9ce7eac 100644 --- a/src/mbgl/util/io.cpp +++ b/src/mbgl/util/io.cpp @@ -55,7 +55,7 @@ void deleteFile(const std::string& filename) { void copyFile(const std::string& destination, const std::string& source) { std::ifstream src(source, std::ios::binary); if (!src.good()) { - throw IOException(errno, "Cannot read file " + destination); + throw IOException(errno, "Cannot read file " + source); } std::ofstream dst(destination, std::ios::binary); if (!dst.good()) { diff --git a/test/fixtures/offline_database/sideload_ambient.db b/test/fixtures/offline_database/sideload_ambient.db new file mode 100644 index 0000000000..5f10a23c61 Binary files /dev/null and b/test/fixtures/offline_database/sideload_ambient.db differ diff --git a/test/fixtures/offline_database/sideload_sat.db b/test/fixtures/offline_database/sideload_sat.db new file mode 100644 index 0000000000..6146e30872 Binary files /dev/null and b/test/fixtures/offline_database/sideload_sat.db differ diff --git a/test/fixtures/offline_database/sideload_sat_multiple.db b/test/fixtures/offline_database/sideload_sat_multiple.db new file mode 100644 index 0000000000..0a74be5ff9 Binary files /dev/null and b/test/fixtures/offline_database/sideload_sat_multiple.db differ diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 2ed72e0d8c..c9e3203abe 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -17,6 +17,7 @@ using namespace mbgl; using mapbox::sqlite::ResultCode; static constexpr const char* filename = "test/fixtures/offline_database/offline.db"; +static constexpr const char* filename_sideload = "test/fixtures/offline_database/offline_sideload.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 @@ -1115,3 +1116,205 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { fs.reset(); } #endif // __QT__ + +TEST(OfflineDatabase, MergeDatabaseWithSingleRegion_New) { + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat.db"); + + OfflineDatabase db(":memory:"); + EXPECT_EQ(0u, db.listRegions()->size()); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_EQ(1u, result->size()); + EXPECT_EQ(1u, db.listRegions()->size()); + + auto regionId = result->front().getID(); + auto status = db.getRegionCompletedStatus(regionId); + EXPECT_EQ(2u, status->completedResourceCount); + EXPECT_EQ(1u, status->completedTileCount); +} + +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(MergeDatabaseWithSingleRegion_Update)) { + deleteDatabaseFiles(); + util::deleteFile(filename_sideload); + util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db"); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat.db"); + int64_t regionId; + + { + OfflineDatabase db(filename); + auto regions = db.listRegions(); + EXPECT_EQ(1u, db.listRegions()->size()); + regionId = regions->front().getID(); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_EQ(1u, result->size()); + // When updating an identical region, the region id remains unchanged. + EXPECT_EQ(regionId, result->front().getID()); + + auto status = db.getRegionCompletedStatus(regionId); + EXPECT_EQ(5u, status->completedResourceCount); + EXPECT_EQ(1u, status->completedTileCount); + + //Verify the modified timestamp matches the tile in the sideloaded db. + auto updatedTile = db.getRegionResource(regionId, + Resource::tile("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", + 1, 0, 0, 1, Tileset::Scheme::XYZ)); + EXPECT_EQ(Timestamp{ Seconds(1520409600) }, *(updatedTile->first.modified)); + } +} + +TEST(OfflineDatabase, MergeDatabaseWithSingleRegion_NoUpdate) { + deleteDatabaseFiles(); + util::deleteFile(filename_sideload); + + //Swap sideload/main database from update test and ensure that an older tile is not copied over + util::copyFile(filename_sideload, "test/fixtures/offline_database/satellite_test.db"); + util::copyFile(filename, "test/fixtures/offline_database/sideload_sat.db"); + + OfflineDatabase db(filename); + auto result = db.mergeDatabase(filename_sideload); + EXPECT_EQ(1u, result->size()); + EXPECT_EQ(1u, db.listRegions()->size()); + + auto regionId = result->front().getID(); + auto updatedTile = db.getRegionResource(regionId, + Resource::tile("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", + 1, 0, 0, 1, Tileset::Scheme::XYZ)); + + //Verify the modified timestamp matches the tile in the main db. + EXPECT_EQ(Timestamp{ Seconds(1520409600) }, *(updatedTile->first.modified)); +} + +TEST(OfflineDatabase, MergeDatabaseWithSingleRegion_AmbientTiles) { + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_ambient.db"); + + OfflineDatabase db(":memory:"); + auto result = db.mergeDatabase(filename_sideload); + auto regionId = result->front().getID(); + + EXPECT_TRUE(bool(db.hasRegionResource(regionId, Resource::tile("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", 1, 0, 0, 1, Tileset::Scheme::XYZ)))); + + //Ambient resources should not be copied + EXPECT_FALSE(bool(db.hasRegionResource(regionId, Resource::style("mapbox://styles/mapbox/streets-v9")))); + EXPECT_FALSE(bool(db.hasRegionResource(regionId, Resource::tile("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", 1, 0, 1, 2, Tileset::Scheme::XYZ)))); + EXPECT_FALSE(bool(db.hasRegionResource(regionId, Resource::tile("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", 1, 1, 1, 2, Tileset::Scheme::XYZ)))); +} + +TEST(OfflineDatabase, MergeDatabaseWithMultipleRegions_New) { + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat_multiple.db"); + + OfflineDatabase db(":memory:"); + EXPECT_EQ(0u, db.listRegions()->size()); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_EQ(2u, result->size()); + EXPECT_EQ(2u, db.listRegions()->size()); + + auto region1Id = result->front().getID(); + auto status = db.getRegionCompletedStatus(region1Id); + EXPECT_EQ(4u, status->completedResourceCount); + EXPECT_EQ(3u, status->completedTileCount); + + auto region2Id = result->back().getID(); + status = db.getRegionCompletedStatus(region2Id); + EXPECT_EQ(1u, status->completedTileCount); + EXPECT_EQ(2u, status->completedResourceCount); +} + +TEST(OfflineDatabase, MergeDatabaseWithMultipleRegionsWithOverlap) { + deleteDatabaseFiles(); + util::deleteFile(filename_sideload); + util::copyFile(filename, "test/fixtures/offline_database/sideload_sat.db"); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat_multiple.db"); + + { + OfflineDatabase db(filename); + EXPECT_EQ(1u, db.listRegions()->size()); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_EQ(2u, result->size()); + EXPECT_EQ(3u, db.listRegions()->size()); + } + + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(filename, mapbox::sqlite::ReadOnly); + { + // clang-format off + mapbox::sqlite::Statement stmt { db, "SELECT COUNT(*) " + "FROM region_tiles " + "JOIN tiles WHERE tile_id = id" }; + // clang-format on + mapbox::sqlite::Query query { stmt }; + query.run(); + + // Ensure multiple entries for tiles shared between regions + EXPECT_EQ(1 + 3 + 1, query.get(0)); + } + { + // clang-format off + mapbox::sqlite::Statement stmt { db, "SELECT COUNT(*) " + "FROM region_resources " + "JOIN resources WHERE resource_id = id" }; + // clang-format on + mapbox::sqlite::Query query { stmt }; + query.run(); + + // Ensure multiple entries for resources shared between regions + EXPECT_EQ(1 + 1 + 1, query.get(0)); + } +} + +TEST(OfflineDatabase, MergeDatabaseWithInvalidPath) { + FixtureLog log; + + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database"); + + OfflineDatabase db(":memory:"); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_FALSE(result); + + EXPECT_EQ(1u, log.count({ EventSeverity::Error, Event::Database, -1, "Merge database does not match schema or has incorrect user_version" })); + EXPECT_EQ(0u, log.uncheckedCount()); +} + +TEST(OfflineDatabase, MergeDatabaseWithInvalidDb) { + FixtureLog log; + + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database/corrupt-immediate.db"); + + OfflineDatabase db(":memory:"); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_FALSE(result); + + EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't attach database (test/fixtures/offline_database/offline_sideload.db) for merge: database disk image is malformed"))); + EXPECT_EQ(0u, log.uncheckedCount()); +} + +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(MergeDatabaseWithDiskFull)) { + FixtureLog log; + test::SQLite3TestFS fs; + + deleteDatabaseFiles(); + util::deleteFile(filename_sideload); + util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db"); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat.db"); + + OfflineDatabase db(filename_test_fs); + + fs.setWriteLimit(0); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_FALSE(result.has_value()); + + EXPECT_EQ(1u, log.count({ EventSeverity::Error, Event::Database, -1, "database or disk is full"})); + EXPECT_EQ(0u, log.uncheckedCount()); +} +#endif // __QT__ + -- cgit v1.2.1