From c04551a4e3e1fd2d04b884d3a9b78caef93d1a9d Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 6 Feb 2020 23:27:33 +0200 Subject: Expose READ_ONLY_MODE_KEY property for DatabaseFileSource --- include/mbgl/storage/database_file_source.hpp | 6 +++ .../include/mbgl/storage/offline_database.hpp | 4 +- .../src/mbgl/storage/database_file_source.cpp | 10 +--- .../default/src/mbgl/storage/offline_database.cpp | 56 ++++++++++++---------- 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/include/mbgl/storage/database_file_source.hpp b/include/mbgl/storage/database_file_source.hpp index 81e5315fd8..9fb9ccdb3e 100644 --- a/include/mbgl/storage/database_file_source.hpp +++ b/include/mbgl/storage/database_file_source.hpp @@ -7,6 +7,12 @@ namespace mbgl { +// Properties that may be supported by database file sources. + +// Property to set database mode. When set, database opens in read-only mode; database opens in read-write-create mode +// otherwise. type: bool +constexpr const char* READ_ONLY_MODE_KEY = "read-only-mode"; + class ResourceOptions; // TODO: Split DatabaseFileSource into Ambient cache and Database interfaces. diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp index 96c012b307..62a82f3a25 100644 --- a/platform/default/include/mbgl/storage/offline_database.hpp +++ b/platform/default/include/mbgl/storage/offline_database.hpp @@ -94,8 +94,7 @@ public: std::exception_ptr pack(); void runPackDatabaseAutomatically(bool autopack_) { autopack = autopack_; } - // For testing only - void reopenDatabaseReadOnlyForTesting(); + void reopenDatabaseReadOnly(bool readOnly); private: void initialize(); @@ -113,6 +112,7 @@ private: void cleanup(); bool disabled(); void vacuum(); + void checkFlags(); mapbox::sqlite::Statement& getStatement(const char *); diff --git a/platform/default/src/mbgl/storage/database_file_source.cpp b/platform/default/src/mbgl/storage/database_file_source.cpp index 778e416696..276482faa0 100644 --- a/platform/default/src/mbgl/storage/database_file_source.cpp +++ b/platform/default/src/mbgl/storage/database_file_source.cpp @@ -14,10 +14,6 @@ #include namespace mbgl { - -// For testing use only -constexpr const char* READ_ONLY_MODE_KEY = "read-only-mode"; - class DatabaseFileSourceThread { public: DatabaseFileSourceThread(std::shared_ptr onlineFileSource_, std::string cachePath) @@ -122,7 +118,7 @@ public: void setOfflineMapboxTileCountLimit(uint64_t limit) { db->setOfflineMapboxTileCountLimit(limit); } - void reopenDatabaseReadOnlyForTesting() { db->reopenDatabaseReadOnlyForTesting(); } + void reopenDatabaseReadOnly(bool readOnly) { db->reopenDatabaseReadOnly(readOnly); } private: expected getDownload(int64_t regionID) { @@ -278,9 +274,7 @@ void DatabaseFileSource::setOfflineMapboxTileCountLimit(uint64_t limit) const { void DatabaseFileSource::setProperty(const std::string& key, const mapbox::base::Value& value) { if (key == READ_ONLY_MODE_KEY && value.getBool()) { - if (*value.getBool()) { - impl->actor().invoke(&DatabaseFileSourceThread::reopenDatabaseReadOnlyForTesting); - } + impl->actor().invoke(&DatabaseFileSourceThread::reopenDatabaseReadOnly, *value.getBool()); } else { std::string message = "Resource provider does not support property " + key; Log::Error(Event::General, message.c_str()); diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp index 695bbd82f5..643818fc99 100644 --- a/platform/default/src/mbgl/storage/offline_database.cpp +++ b/platform/default/src/mbgl/storage/offline_database.cpp @@ -162,7 +162,7 @@ void OfflineDatabase::removeExisting() { void OfflineDatabase::removeOldCacheTable() { assert(db); - assert(!readOnly); + checkFlags(); db->exec("DROP TABLE IF EXISTS http_cache"); if (autopack) vacuum(); @@ -170,7 +170,7 @@ void OfflineDatabase::removeOldCacheTable() { void OfflineDatabase::createSchema() { assert(db); - assert(!readOnly); + checkFlags(); vacuum(); db->exec("PRAGMA journal_mode = DELETE"); @@ -183,7 +183,7 @@ void OfflineDatabase::createSchema() { void OfflineDatabase::migrateToVersion3() { assert(db); - assert(!readOnly); + checkFlags(); vacuum(); db->exec("PRAGMA user_version = 3"); @@ -197,7 +197,7 @@ void OfflineDatabase::migrateToVersion3() { void OfflineDatabase::migrateToVersion5() { assert(db); - assert(!readOnly); + checkFlags(); db->exec("PRAGMA journal_mode = DELETE"); db->exec("PRAGMA synchronous = FULL"); @@ -206,7 +206,7 @@ void OfflineDatabase::migrateToVersion5() { void OfflineDatabase::migrateToVersion6() { assert(db); - assert(!readOnly); + checkFlags(); mapbox::sqlite::Transaction transaction(*db); db->exec("ALTER TABLE resources ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0"); @@ -217,7 +217,7 @@ void OfflineDatabase::migrateToVersion6() { void OfflineDatabase::vacuum() { assert(db); - assert(!readOnly); + checkFlags(); if (getPragma("PRAGMA auto_vacuum") != 2 /*INCREMENTAL*/) { db->exec("PRAGMA auto_vacuum = INCREMENTAL"); @@ -227,6 +227,12 @@ void OfflineDatabase::vacuum() { } } +void OfflineDatabase::checkFlags() { + if (readOnly) { + throw std::runtime_error("Cannot modify database in read-only mode"); + } +} + mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { if (!db) { initialize(); @@ -269,7 +275,7 @@ optional OfflineDatabase::hasInternal(const Resource& resource) { } std::pair OfflineDatabase::put(const Resource& resource, const Response& response) try { - assert(!readOnly); + if (readOnly) return {false, 0}; if (!db) { initialize(); @@ -289,7 +295,7 @@ std::pair OfflineDatabase::put(const Resource& resource, const R } std::pair OfflineDatabase::putInternal(const Resource& resource, const Response& response, bool evict_) { - assert(!readOnly); + checkFlags(); if (response.error) { return { false, 0 }; @@ -394,7 +400,7 @@ bool OfflineDatabase::putResource(const Resource& resource, const Response& response, const std::string& data, bool compressed) { - assert(!readOnly); + checkFlags(); if (response.notModified) { // clang-format off @@ -582,7 +588,7 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, const Response& response, const std::string& data, bool compressed) { - assert(!readOnly); + checkFlags(); if (response.notModified) { // clang-format off @@ -684,7 +690,7 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, } std::exception_ptr OfflineDatabase::invalidateAmbientCache() try { - assert(!readOnly); + checkFlags(); // clang-format off mapbox::sqlite::Query tileQuery{ getStatement( @@ -717,7 +723,7 @@ std::exception_ptr OfflineDatabase::invalidateAmbientCache() try { } std::exception_ptr OfflineDatabase::clearAmbientCache() try { - assert(!readOnly); + checkFlags(); // clang-format off mapbox::sqlite::Query tileQuery{ getStatement( @@ -750,7 +756,7 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try { } std::exception_ptr OfflineDatabase::invalidateRegion(int64_t regionID) try { - assert(!readOnly); + checkFlags(); { // clang-format off @@ -814,7 +820,7 @@ expected OfflineDatabase::listRegions() try expected OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata) try { - assert(!readOnly); + checkFlags(); // clang-format off mapbox::sqlite::Query query{ getStatement( @@ -833,7 +839,7 @@ OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, expected OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { - assert(!readOnly); + checkFlags(); try { // clang-format off @@ -911,7 +917,7 @@ OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { expected OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { - assert(!readOnly); + checkFlags(); // clang-format off mapbox::sqlite::Query query{ getStatement( @@ -929,7 +935,7 @@ OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetad } std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try { - assert(!readOnly); + checkFlags(); { mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") }; @@ -966,7 +972,7 @@ optional OfflineDatabase::hasRegionResource(const Resource& resource) t uint64_t OfflineDatabase::putRegionResource(int64_t regionID, const Resource& resource, const Response& response) try { - assert(!readOnly); + checkFlags(); if (!db) { initialize(); @@ -983,7 +989,7 @@ uint64_t OfflineDatabase::putRegionResource(int64_t regionID, void OfflineDatabase::putRegionResources(int64_t regionID, const std::list>& resources, OfflineRegionStatus& status) try { - assert(!readOnly); + checkFlags(); if (!db) { initialize(); @@ -1028,7 +1034,7 @@ void OfflineDatabase::putRegionResources(int64_t regionID, } uint64_t OfflineDatabase::putRegionResourceInternal(int64_t regionID, const Resource& resource, const Response& response) { - assert(!readOnly); + checkFlags(); uint64_t size = putInternal(resource, response, false).second; bool previouslyUnused = markUsed(regionID, resource); @@ -1048,7 +1054,7 @@ uint64_t OfflineDatabase::putRegionResourceInternal(int64_t regionID, const Reso } bool OfflineDatabase::markUsed(int64_t regionID, const Resource& resource) { - assert(!readOnly); + checkFlags(); if (resource.kind == Resource::Kind::Tile) { // clang-format off @@ -1202,7 +1208,7 @@ T OfflineDatabase::getPragma(const char* sql) { // delete an arbitrary number of old cache entries. The free pages approach saves // us from calling VACUUM or keeping a running total, which can be costly. bool OfflineDatabase::evict(uint64_t neededFreeSize) { - assert(!readOnly); + checkFlags(); uint64_t pageSize = getPragma("PRAGMA page_size"); uint64_t pageCount = getPragma("PRAGMA page_count"); @@ -1378,11 +1384,11 @@ std::exception_ptr OfflineDatabase::resetDatabase() try { return std::current_exception(); } -void OfflineDatabase::reopenDatabaseReadOnlyForTesting() { - readOnly = true; - +void OfflineDatabase::reopenDatabaseReadOnly(bool readOnly_) { + if (readOnly == readOnly_) return; try { cleanup(); + readOnly = readOnly_; initialize(); } catch (...) { handleError("reopen database read-only"); -- cgit v1.2.1