Prepare for more DefaultFileSource changes

.gitignore

@@ -17,12 +17,6 @@ xcuserdata
 /test/fixtures/api/2.png
 /test/fixtures/offline_database/offline.db
 /test/fixtures/offline_database/offline.db-*
-/test/fixtures/offline_database/satellite.db
-/test/fixtures/offline_database/satellite.db-*
-/test/fixtures/offline_database/invalid.db
-/test/fixtures/offline_database/invalid.db-*
-/test/fixtures/offline_database/migrated.db
-/test/fixtures/offline_database/migrated.db-*
 /test/fixtures/**/actual.png
 /test/fixtures/**/diff.png
 /test/output

include/mbgl/util/logging.hpp

@@ -67,8 +67,8 @@ class Log {
 
 private:
     static void record(EventSeverity severity, Event event, const std::string &msg);
-    static void record(EventSeverity severity, Event event, const char* format, ...);
-    static void record(EventSeverity severity, Event event, int64_t code);
+    static void record(EventSeverity severity, Event event, const char* format = "", ...);
+    static void record(EventSeverity severity, Event event, int64_t code, const char* format = "", ...);
     static void record(EventSeverity severity, Event event, int64_t code, const std::string &msg);
 
     // This method is the data sink that must be implemented by each platform we

platform/default/mbgl/storage/offline_database.cpp

@@ -28,67 +28,70 @@ OfflineDatabase::~OfflineDatabase() {
 }
 
 void OfflineDatabase::ensureSchema() {
-    if (path != ":memory:") {
-        auto result = mapbox::sqlite::Database::tryOpen(path, mapbox::sqlite::ReadWrite);
-        if (result.is<mapbox::sqlite::Exception>()) {
-            const auto& ex = result.get<mapbox::sqlite::Exception>();
-            if (ex.code == mapbox::sqlite::ResultCode::NotADB) {
-                // Corrupted; blow it away.
-                removeExisting();
-            } else if (ex.code == mapbox::sqlite::ResultCode::CantOpen) {
-                // Doesn't exist yet.
-            } else {
-                Log::Error(Event::Database, "Unexpected error connecting to database: %s", ex.what());
-                throw ex;
-            }
+    auto result = mapbox::sqlite::Database::tryOpen(path, mapbox::sqlite::ReadWriteCreate);
+    if (result.is<mapbox::sqlite::Exception>()) {
+        const auto& ex = result.get<mapbox::sqlite::Exception>();
+        if (ex.code == mapbox::sqlite::ResultCode::NotADB) {
+            // Corrupted; blow it away.
+            removeExisting();
+            result = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate);
         } else {
-            try {
-                db = std::make_unique<mapbox::sqlite::Database>(std::move(result.get<mapbox::sqlite::Database>()));
-                db->setBusyTimeout(Milliseconds::max());
-                db->exec("PRAGMA foreign_keys = ON");
-
-                switch (userVersion()) {
-                case 0:
-                case 1:
-                    // cache-only database; ok to delete
-                    removeExisting();
-                    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();
-                } else {
-                    throw;
-                }
-            }
+            Log::Error(Event::Database, "Unexpected error connecting to database: %s", ex.what());
+            throw ex;
         }
     }
 
     try {
-        #include "offline_schema.cpp.include"
-
-        db = std::make_unique<mapbox::sqlite::Database>(mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWrite | mapbox::sqlite::Create));
+        assert(result.is<mapbox::sqlite::Database>());
+        db = std::make_unique<mapbox::sqlite::Database>(std::move(result.get<mapbox::sqlite::Database>()));
         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();
+        } 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>(mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate));
+            db->setBusyTimeout(Milliseconds::max());
+            db->exec("PRAGMA foreign_keys = ON");
+        }
+
         db->exec("PRAGMA auto_vacuum = INCREMENTAL");
         db->exec("PRAGMA journal_mode = DELETE");
         db->exec("PRAGMA synchronous = FULL");
@@ -117,6 +120,11 @@ void OfflineDatabase::removeExisting() {
     }
 }
 
+void OfflineDatabase::removeOldCacheTable() {
+    db->exec("DROP TABLE IF EXISTS http_cache");
+    db->exec("VACUUM");
+}
+
 void OfflineDatabase::migrateToVersion3() {
     db->exec("PRAGMA auto_vacuum = INCREMENTAL");
     db->exec("VACUUM");
@@ -197,7 +205,7 @@ std::pair<bool, uint64_t> OfflineDatabase::putInternal(const Resource& resource,
     }
 
     if (evict_ && !evict(size)) {
-        Log::Debug(Event::Database, "Unable to make space for entry");
+        Log::Info(Event::Database, "Unable to make space for entry");
         return { false, 0 };
     }
 

platform/default/mbgl/storage/offline_database.hpp

@@ -70,6 +70,7 @@ class OfflineDatabase : private util::noncopyable {
     int userVersion();
     void ensureSchema();
     void removeExisting();
+    void removeOldCacheTable();
     void migrateToVersion3();
     void migrateToVersion5();
     void migrateToVersion6();

platform/default/sqlite3.cpp

@@ -27,6 +27,9 @@ class DatabaseImpl {
         }
     }
 
+    void setBusyTimeout(std::chrono::milliseconds timeout);
+    void exec(const std::string& sql);
+
     sqlite3* db;
 };
 
@@ -62,84 +65,8 @@ class StatementImpl {
 template <typename T>
 using optional = std::experimental::optional<T>;
 
-static const char* codeToString(const int err) {
-    switch (err) {
-    case SQLITE_OK: return "SQLITE_OK";
-    case SQLITE_ERROR: return "SQLITE_ERROR";
-    case SQLITE_INTERNAL: return "SQLITE_INTERNAL";
-    case SQLITE_PERM: return "SQLITE_PERM";
-    case SQLITE_ABORT: return "SQLITE_ABORT";
-    case SQLITE_BUSY: return "SQLITE_BUSY";
-    case SQLITE_LOCKED: return "SQLITE_LOCKED";
-    case SQLITE_NOMEM: return "SQLITE_NOMEM";
-    case SQLITE_READONLY: return "SQLITE_READONLY";
-    case SQLITE_INTERRUPT: return "SQLITE_INTERRUPT";
-    case SQLITE_IOERR: return "SQLITE_IOERR";
-    case SQLITE_CORRUPT: return "SQLITE_CORRUPT";
-    case SQLITE_NOTFOUND: return "SQLITE_NOTFOUND";
-    case SQLITE_FULL: return "SQLITE_FULL";
-    case SQLITE_CANTOPEN: return "SQLITE_CANTOPEN";
-    case SQLITE_PROTOCOL: return "SQLITE_PROTOCOL";
-    case SQLITE_EMPTY: return "SQLITE_EMPTY";
-    case SQLITE_SCHEMA: return "SQLITE_SCHEMA";
-    case SQLITE_TOOBIG: return "SQLITE_TOOBIG";
-    case SQLITE_CONSTRAINT: return "SQLITE_CONSTRAINT";
-    case SQLITE_MISMATCH: return "SQLITE_MISMATCH";
-    case SQLITE_MISUSE: return "SQLITE_MISUSE";
-    case SQLITE_NOLFS: return "SQLITE_NOLFS";
-    case SQLITE_AUTH: return "SQLITE_AUTH";
-    case SQLITE_FORMAT: return "SQLITE_FORMAT";
-    case SQLITE_RANGE: return "SQLITE_RANGE";
-    case SQLITE_NOTADB: return "SQLITE_NOTADB";
-    case SQLITE_NOTICE: return "SQLITE_NOTICE";
-    case SQLITE_WARNING: return "SQLITE_WARNING";
-    case SQLITE_ROW: return "SQLITE_ROW";
-    case SQLITE_DONE: return "SQLITE_DONE";
-    default: return "<unknown>";
-    }
-}
-
 static void errorLogCallback(void *, const int err, const char *msg) {
-    auto severity = mbgl::EventSeverity::Info;
-
-    switch (err) {
-        case SQLITE_ERROR:      // Generic error
-        case SQLITE_INTERNAL:   // Internal logic error in SQLite
-        case SQLITE_PERM:       // Access permission denied
-        case SQLITE_ABORT:      // Callback routine requested an abort
-        case SQLITE_BUSY:       // The database file is locked
-        case SQLITE_LOCKED:     // A table in the database is locked
-        case SQLITE_NOMEM:      // A malloc() failed
-        case SQLITE_READONLY:   // Attempt to write a readonly database
-        case SQLITE_INTERRUPT:  // Operation terminated by sqlite3_interrupt(
-        case SQLITE_IOERR:      // Some kind of disk I/O error occurred
-        case SQLITE_CORRUPT:    // The database disk image is malformed
-        case SQLITE_NOTFOUND:   // Unknown opcode in sqlite3_file_control()
-        case SQLITE_FULL:       // Insertion failed because database is full
-        case SQLITE_CANTOPEN:   // Unable to open the database file
-        case SQLITE_PROTOCOL:   // Database lock protocol error
-        case SQLITE_EMPTY:      // Internal use only
-        case SQLITE_SCHEMA:     // The database schema changed
-        case SQLITE_TOOBIG:     // String or BLOB exceeds size limit
-        case SQLITE_CONSTRAINT: // Abort due to constraint violation
-        case SQLITE_MISMATCH:   // Data type mismatch
-        case SQLITE_MISUSE:     // Library used incorrectly
-        case SQLITE_NOLFS:      // Uses OS features not supported on host
-        case SQLITE_AUTH:       // Authorization denied
-        case SQLITE_FORMAT:     // Not used
-        case SQLITE_RANGE:      // 2nd parameter to sqlite3_bind out of range
-        case SQLITE_NOTADB:     // File opened that is not a database file
-            severity = mbgl::EventSeverity::Error;
-            break;
-        case SQLITE_WARNING:    // Warnings from sqlite3_log()
-            severity = mbgl::EventSeverity::Warning;
-            break;
-        case SQLITE_NOTICE:     // Notifications from sqlite3_log()
-        default:
-            break;
-    }
-
-    mbgl::Log::Record(severity, mbgl::Event::Database, "%s (%s)", msg, codeToString(err));
+    mbgl::Log::Record(mbgl::EventSeverity::Info, mbgl::Event::Database, err, "%s", msg);
 }
 
 const static bool sqliteVersionCheck __attribute__((unused)) = []() {
@@ -192,23 +119,31 @@ Database::~Database() = default;
 
 void Database::setBusyTimeout(std::chrono::milliseconds timeout) {
     assert(impl);
-    const int err = sqlite3_busy_timeout(impl->db,
+    impl->setBusyTimeout(timeout);
+}
+
+void DatabaseImpl::setBusyTimeout(std::chrono::milliseconds timeout) {
+    const int err = sqlite3_busy_timeout(db,
         int(std::min<std::chrono::milliseconds::rep>(timeout.count(), std::numeric_limits<int>::max())));
     if (err != SQLITE_OK) {
-        throw Exception { err, sqlite3_errmsg(impl->db) };
+        throw Exception { err, sqlite3_errmsg(db) };
     }
 }
 
 void Database::exec(const std::string &sql) {
     assert(impl);
+    impl->exec(sql);
+}
+
+void DatabaseImpl::exec(const std::string& sql) {
     char *msg = nullptr;
-    const int err = sqlite3_exec(impl->db, sql.c_str(), nullptr, nullptr, &msg);
+    const int err = sqlite3_exec(db, sql.c_str(), nullptr, nullptr, &msg);
     if (msg) {
         const std::string message = msg;
         sqlite3_free(msg);
         throw Exception { err, message };
     } else if (err != SQLITE_OK) {
-        throw Exception { err, sqlite3_errmsg(impl->db) };
+        throw Exception { err, sqlite3_errmsg(db) };
     }
 }
 
@@ -481,16 +416,16 @@ uint64_t Query::changes() const {
 }
 
 Transaction::Transaction(Database& db_, Mode mode)
-    : db(db_) {
+    : dbImpl(*db_.impl) {
     switch (mode) {
     case Deferred:
-        db.exec("BEGIN DEFERRED TRANSACTION");
+        dbImpl.exec("BEGIN DEFERRED TRANSACTION");
         break;
     case Immediate:
-        db.exec("BEGIN IMMEDIATE TRANSACTION");
+        dbImpl.exec("BEGIN IMMEDIATE TRANSACTION");
         break;
     case Exclusive:
-        db.exec("BEGIN EXCLUSIVE TRANSACTION");
+        dbImpl.exec("BEGIN EXCLUSIVE TRANSACTION");
         break;
     }
 }
@@ -507,12 +442,12 @@ Transaction::~Transaction() {
 
 void Transaction::commit() {
     needRollback = false;
-    db.exec("COMMIT TRANSACTION");
+    dbImpl.exec("COMMIT TRANSACTION");
 }
 
 void Transaction::rollback() {
     needRollback = false;
-    db.exec("ROLLBACK TRANSACTION");
+    dbImpl.exec("ROLLBACK TRANSACTION");
 }
 
 } // namespace sqlite

platform/default/sqlite3.hpp

@@ -11,13 +11,8 @@ namespace mapbox {
 namespace sqlite {
 
 enum OpenFlag : int {
-    ReadOnly = 0x00000001,
-    ReadWrite = 0x00000002,
-    Create = 0x00000004,
-    NoMutex = 0x00008000,
-    FullMutex = 0x00010000,
-    SharedCache = 0x00020000,
-    PrivateCache = 0x00040000,
+    ReadOnly        = 0b001,
+    ReadWriteCreate = 0b110,
 };
 
 enum class ResultCode : int {
@@ -69,6 +64,7 @@ class DatabaseImpl;
 class Statement;
 class StatementImpl;
 class Query;
+class Transaction;
 
 class Database {
 private:
@@ -91,6 +87,7 @@ class Database {
     std::unique_ptr<DatabaseImpl> impl;
 
     friend class Statement;
+    friend class Transaction;
 };
 
 // A Statement object represents a prepared statement that can be run repeatedly run with a Query object.
@@ -173,7 +170,7 @@ class Transaction {
     void rollback();
 
 private:
-    Database& db;
+    DatabaseImpl& dbImpl;
     bool needRollback = true;
 };