Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Remove dependence on Resource from FileCache #3431

Closed
wants to merge 1 commit into from
Closed

[core] Remove dependence on Resource from FileCache #3431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions include/mbgl/storage/file_cache.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

namespace mbgl {

struct Resource;
class Response;
class WorkRequest;

Expand All @@ -19,8 +18,8 @@ class FileCache : private util::noncopyable {
enum class Hint : bool { Full, Refresh };
using Callback = std::function<void(std::unique_ptr<Response>)>;

virtual std::unique_ptr<WorkRequest> get(const Resource &resource, Callback callback) = 0;
virtual void put(const Resource &resource, std::shared_ptr<const Response> response, Hint hint) = 0;
virtual std::unique_ptr<WorkRequest> get(const std::string& url, Callback callback) = 0;
virtual void put(const std::string& url, std::shared_ptr<const Response> response, Hint hint) = 0;
};

} // namespace mbgl
Expand Down
4 changes: 2 additions & 2 deletions include/mbgl/storage/sqlite_cache.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ class SQLiteCache : public FileCache {
void setMaximumCacheEntrySize(uint64_t size);

// FileCache API
std::unique_ptr<WorkRequest> get(const Resource &resource, Callback callback) override;
void put(const Resource &resource, std::shared_ptr<const Response> response, Hint hint) override;
std::unique_ptr<WorkRequest> get(const std::string& url, Callback callback) override;
void put(const std::string& url, std::shared_ptr<const Response> response, Hint hint) override;

class Impl;

Expand Down
4 changes: 2 additions & 2 deletions platform/default/online_file_source.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ void OnlineFileSource::Impl::startCacheRequest(OnlineFileRequestImpl& request) {
// Check the cache for existing data so that we can potentially
// revalidate the information without having to redownload everything.
request.cacheRequest =
cache->get(request.resource, [this, &request](std::shared_ptr<Response> response) {
cache->get(request.resource.url, [this, &request](std::shared_ptr<Response> response) {
request.cacheRequest = nullptr;
if (response) {
response->stale = response->isExpired();
Expand Down Expand Up @@ -291,7 +291,7 @@ void OnlineFileSource::Impl::startRealRequest(OnlineFileRequestImpl& request) {
if (request.getResponse() && response->data == request.getResponse()->data) {
hint = FileCache::Hint::Refresh;
}
cache->put(request.resource, response, hint);
cache->put(request.resource.url, response, hint);
}

request.setResponse(response);
Expand Down
32 changes: 14 additions & 18 deletions platform/default/sqlite_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void SQLiteCache::Impl::createSchema() {
"CREATE TABLE IF NOT EXISTS `http_cache` ("
" `url` TEXT PRIMARY KEY NOT NULL,"
" `status` INTEGER NOT NULL," // The response status (Successful or Error).
" `kind` INTEGER NOT NULL," // The kind of file.
" `kind` INTEGER NOT NULL," // The kind of file. (No longer used; column remains only for backward compatibility.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably remove this, as the cache will get recreated anyway after #3429. Don't forget to bump the cache schema version.

" `modified` INTEGER," // Timestamp when the file was last modified.
" `etag` TEXT,"
" `expires` INTEGER," // Timestamp when the server says the file expires.
Expand Down Expand Up @@ -264,15 +264,15 @@ void SQLiteCache::Impl::pruneEntries() {
}
}

std::unique_ptr<WorkRequest> SQLiteCache::get(const Resource &resource, Callback callback) {
std::unique_ptr<WorkRequest> SQLiteCache::get(const std::string& url, Callback callback) {
// Can be called from any thread, but most likely from the file source thread.
// Will try to load the URL from the SQLite database and call the callback when done.
// Note that the callback is probably going to invoked from another thread, so the caller
// must make sure that it can run in that thread.
return thread->invokeWithCallback(&Impl::get, callback, resource);
return thread->invokeWithCallback(&Impl::get, callback, url);
}

void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
void SQLiteCache::Impl::get(const std::string& url, Callback callback) {
try {
initializeDatabase();

Expand All @@ -285,7 +285,7 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
getStmt->reset();
}

const auto canonicalURL = util::mapbox::canonicalURL(resource.url);
const auto canonicalURL = util::mapbox::canonicalURL(url);
getStmt->bind(1, canonicalURL.c_str());
if (getStmt->run()) {
// There is data.
Expand Down Expand Up @@ -333,18 +333,18 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
}
}

void SQLiteCache::put(const Resource &resource, std::shared_ptr<const Response> response, Hint hint) {
void SQLiteCache::put(const std::string& url, std::shared_ptr<const Response> response, Hint hint) {
// Can be called from any thread, but most likely from the file source thread. We are either
// storing a new response or updating the currently stored response, potentially setting a new
// expiry date.
if (hint == Hint::Full) {
thread->invoke(&Impl::put, resource, response);
thread->invoke(&Impl::put, url, response);
} else if (hint == Hint::Refresh) {
thread->invoke(&Impl::refresh, resource, response->expires);
thread->invoke(&Impl::refresh, url, response->expires);
}
}

void SQLiteCache::Impl::put(const Resource& resource, std::shared_ptr<const Response> response) {
void SQLiteCache::Impl::put(const std::string& url, std::shared_ptr<const Response> response) {
try {
initializeDatabase();
pruneEntries();
Expand Down Expand Up @@ -372,24 +372,20 @@ void SQLiteCache::Impl::put(const Resource& resource, std::shared_ptr<const Resp
putStmt->reset();
}

const auto canonicalURL = util::mapbox::canonicalURL(resource.url);
const auto canonicalURL = util::mapbox::canonicalURL(url);
putStmt->bind(1 /* url */, canonicalURL.c_str());
if (response->error) {
putStmt->bind(2 /* status */, int(response->error->reason));
} else {
putStmt->bind(2 /* status */, 1 /* success */);
}
putStmt->bind(3 /* kind */, int(resource.kind));
putStmt->bind(3 /* kind */, 0);
putStmt->bind(4 /* modified */, response->modified.count());
putStmt->bind(5 /* etag */, response->etag.c_str());
putStmt->bind(6 /* expires */, response->expires.count());
putStmt->bind(7 /* accessed */, toSeconds(SystemClock::now()).count());

std::string data;
if (resource.kind != Resource::SpriteImage && response->data) {
// Do not compress images, since they are typically compressed already.
data = util::compress(*response->data);
}
std::string data = util::compress(*response->data);

if (!data.empty() && data.size() < response->data->size()) {
// Store the compressed data when it is smaller than the original
Expand All @@ -413,7 +409,7 @@ void SQLiteCache::Impl::put(const Resource& resource, std::shared_ptr<const Resp
}
}

void SQLiteCache::Impl::refresh(const Resource& resource, Seconds expires) {
void SQLiteCache::Impl::refresh(const std::string& url, Seconds expires) {
try {
initializeDatabase();

Expand All @@ -426,7 +422,7 @@ void SQLiteCache::Impl::refresh(const Resource& resource, Seconds expires) {
refreshStmt->reset();
}

const auto canonicalURL = util::mapbox::canonicalURL(resource.url);
const auto canonicalURL = util::mapbox::canonicalURL(url);
refreshStmt->bind(1, toSeconds(SystemClock::now()).count());
refreshStmt->bind(2, expires.count());
refreshStmt->bind(3, canonicalURL.c_str());
Expand Down
6 changes: 3 additions & 3 deletions platform/default/sqlite_cache_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ class SQLiteCache::Impl {
void setMaximumCacheSize(uint64_t size);
void setMaximumCacheEntrySize(uint64_t size);

void get(const Resource&, Callback);
void put(const Resource& resource, std::shared_ptr<const Response> response);
void refresh(const Resource& resource, Seconds expires);
void get(const std::string& url, Callback);
void put(const std::string& url, std::shared_ptr<const Response> response);
void refresh(const std::string& url, Seconds expires);

private:
void initializeDatabase();
Expand Down
9 changes: 3 additions & 6 deletions test/storage/cache_size.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ bool tileIsCached(mbgl::SQLiteCache* cache, unsigned id) {
response = std::move(res);
};

Resource resource{ Resource::Kind::Tile, url };
auto req = cache->get(resource, callback);
auto req = cache->get(url, callback);

while (!replied) {
util::RunLoop::Get()->runOnce();
Expand Down Expand Up @@ -53,8 +52,7 @@ void insertTile(mbgl::SQLiteCache* cache, unsigned id, uint64_t size) {

response->data = data;

Resource resource{ Resource::Kind::Tile, url };
cache->put(resource, response, SQLiteCache::Hint::Full);
cache->put(url, response, SQLiteCache::Hint::Full);
}

void refreshTile(mbgl::SQLiteCache* cache, unsigned id) {
Expand All @@ -66,8 +64,7 @@ void refreshTile(mbgl::SQLiteCache* cache, unsigned id) {
response->modified = toSeconds(SystemClock::now());
response->expires = toSeconds(SystemClock::now()) + Seconds(5000);

Resource resource{ Resource::Kind::Tile, url };
cache->put(resource, response, SQLiteCache::Hint::Refresh);
cache->put(url, response, SQLiteCache::Hint::Refresh);
}

uint64_t cacheSize(mbgl::SQLiteCache* cache, unsigned entryCount, uint64_t entrySize) {
Expand Down
36 changes: 18 additions & 18 deletions test/storage/database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ TEST_F(Storage, DatabaseDoesNotExist) {

SQLiteCache::Impl cache("test/fixtures/404/cache.db");

cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand Down Expand Up @@ -57,7 +57,7 @@ TEST_F(Storage, DatabaseCreate) {

SQLiteCache::Impl cache("test/fixtures/database/cache.db");

cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand Down Expand Up @@ -117,7 +117,7 @@ TEST_F(Storage, DatabaseLockedRead) {
// First request should fail.
Log::setObserver(std::make_unique<FixtureLogObserver>());

cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand All @@ -134,7 +134,7 @@ TEST_F(Storage, DatabaseLockedRead) {
// First, try getting a file (the cache value should not exist).
Log::setObserver(std::make_unique<FixtureLogObserver>());

cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand All @@ -160,8 +160,8 @@ TEST_F(Storage, DatabaseLockedWrite) {
Log::setObserver(std::make_unique<FixtureLogObserver>());

auto response = std::make_shared<Response>();
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand All @@ -179,8 +179,8 @@ TEST_F(Storage, DatabaseLockedWrite) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
ASSERT_NE(nullptr, res.get());
ASSERT_TRUE(res->data.get());
EXPECT_EQ("Demo", *res->data);
Expand Down Expand Up @@ -212,8 +212,8 @@ TEST_F(Storage, DatabaseLockedRefresh) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand All @@ -228,8 +228,8 @@ TEST_F(Storage, DatabaseLockedRefresh) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.refresh({ Resource::Unknown, "mapbox://test" }, response->expires);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.refresh("mapbox://test", response->expires);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
EXPECT_EQ(nullptr, res.get());
});

Expand Down Expand Up @@ -257,8 +257,8 @@ TEST_F(Storage, DatabaseDeleted) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
ASSERT_NE(nullptr, res.get());
ASSERT_TRUE(res->data.get());
EXPECT_EQ("Demo", *res->data);
Expand All @@ -275,8 +275,8 @@ TEST_F(Storage, DatabaseDeleted) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
ASSERT_NE(nullptr, res.get());
ASSERT_TRUE(res->data.get());
EXPECT_EQ("Demo", *res->data);
Expand Down Expand Up @@ -306,8 +306,8 @@ TEST_F(Storage, DatabaseInvalid) {

auto response = std::make_shared<Response>();
response->data = std::make_shared<std::string>("Demo");
cache.put({ Resource::Unknown, "mapbox://test" }, response);
cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr<Response> res) {
cache.put("mapbox://test", response);
cache.get("mapbox://test", [] (std::unique_ptr<Response> res) {
ASSERT_NE(nullptr, res.get());
ASSERT_TRUE(res->data.get());
EXPECT_EQ("Demo", *res->data);
Expand Down