From 084c28f0bb3969aaa09078c278f08f83e389eda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 14 Aug 2018 13:04:00 -0700 Subject: [core] recreate offline database when it is deleted out from under our feet --- platform/default/mbgl/storage/offline_database.cpp | 7 ++++-- platform/default/sqlite3.cpp | 4 ++++ platform/default/sqlite3.hpp | 25 +++++++++++----------- test/storage/offline_database.test.cpp | 8 +++---- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index 7516185f27..79bc3c8f27 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -79,8 +79,11 @@ void OfflineDatabase::initialize() { void OfflineDatabase::handleError(const mapbox::sqlite::Exception& ex, const char* action) { if (ex.code == mapbox::sqlite::ResultCode::NotADB || - ex.code == mapbox::sqlite::ResultCode::Corrupt) { - // Corrupted; delete database so that we have a clean slate for the next operation. + ex.code == mapbox::sqlite::ResultCode::Corrupt || + (ex.code == mapbox::sqlite::ResultCode::ReadOnly && + ex.extendedCode == mapbox::sqlite::ExtendedResultCode::ReadOnlyDBMoved)) { + // The database was corruped, moved away, or deleted. We're going to start fresh with a + // clean slate for the next operation. Log::Error(Event::Database, static_cast(ex.code), "Can't %s: %s", action, ex.what()); try { removeExisting(); diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp index fed5a3a185..faaa85efd8 100644 --- a/platform/default/sqlite3.cpp +++ b/platform/default/sqlite3.cpp @@ -45,6 +45,10 @@ public: DatabaseImpl(sqlite3* db_) : db(db_) { + const int error = sqlite3_extended_result_codes(db, true); + if (error != SQLITE_OK) { + mbgl::Log::Warning(mbgl::Event::Database, error, "Failed to enable extended result codes: %s", sqlite3_errmsg(db)); + } } ~DatabaseImpl() diff --git a/platform/default/sqlite3.hpp b/platform/default/sqlite3.hpp index 612e92acc6..16f76a0d1a 100644 --- a/platform/default/sqlite3.hpp +++ b/platform/default/sqlite3.hpp @@ -15,7 +15,7 @@ enum OpenFlag : int { ReadWriteCreate = 0b110, }; -enum class ResultCode : int { +enum class ResultCode : uint8_t { OK = 0, Error = 1, Internal = 2, @@ -40,24 +40,25 @@ enum class ResultCode : int { NoLFS = 22, Auth = 23, Range = 25, - NotADB = 26 + NotADB = 26, +}; + +enum class ExtendedResultCode : uint8_t { + Unknown = 0, + ReadOnlyDBMoved = 4, }; class Exception : public std::runtime_error { public: - Exception(int err, const char* msg) - : std::runtime_error(msg), code(static_cast(err)) { - } - Exception(ResultCode err, const char* msg) - : std::runtime_error(msg), code(err) { - } + Exception(ResultCode err, const char* msg) : Exception(static_cast(err), msg) {} + Exception(int err, const char* msg) : Exception(err, std::string{ msg }) {} Exception(int err, const std::string& msg) - : std::runtime_error(msg), code(static_cast(err)) { - } - Exception(ResultCode err, const std::string& msg) - : std::runtime_error(msg), code(err) { + : std::runtime_error(msg), + code(static_cast(err)), + extendedCode(static_cast(err >> 8)) { } const ResultCode code = ResultCode::OK; + const ExtendedResultCode extendedCode = ExtendedResultCode::Unknown; }; class DatabaseImpl; diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index a1fd2ea619..346cd6da80 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -32,7 +32,7 @@ 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) { +static __attribute__((unused)) FixtureLog::Message warning(ResultCode code, const char* message) { return { EventSeverity::Warning, Event::Database, static_cast(code), message }; } @@ -232,7 +232,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Invalid)) { // Checking two possibilities for the error string because it apparently changes between SQLite versions. EXPECT_EQ(1u, log.count(error(ResultCode::NotADB, "Can't open database: file is encrypted or is not a database"), true) + log.count(error(ResultCode::NotADB, "Can't open database: file is not a database"), true)); - EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -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 }) { @@ -977,7 +977,7 @@ TEST(OfflineDatabase, CorruptDatabaseOnOpen) { // 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(1u, log.count({ EventSeverity::Warning, Event::Database, -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. @@ -1007,7 +1007,7 @@ TEST(OfflineDatabase, CorruptDatabaseOnQuery) { // 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(1u, log.count({ EventSeverity::Warning, Event::Database, -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. -- cgit v1.2.1