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

Commit

Permalink
[core] Remove dependence on Resource from FileCache
Browse files Browse the repository at this point in the history 
Resource is going away as part of #3374.

The only place that Resource#kind was used was to bypass an attempt
at compression for sprite images. But it's fine for that path to just do
the compression and use it if it works -- even if not optimal, sprite
images account for a relatively small portion of cache rows.
  • Loading branch information
@jfirebaugh
jfirebaugh committed Jan 8, 2016
1 parent 656eb87 commit 625bf84
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 52 deletions.
5 changes: 2 additions & 3 deletions include/mbgl/storage/file_cache.hpp
View file
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,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 @@ -288,7 +288,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
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,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.)
" `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 @@ -298,15 +298,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 @@ -319,7 +319,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 @@ -367,18 +367,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 @@ -406,24 +406,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 @@ -447,7 +443,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 @@ -460,7 +456,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,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 @@ -52,8 +51,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 @@ -65,8 +63,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
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 @@ -151,7 +151,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 @@ -168,7 +168,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 @@ -194,8 +194,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 @@ -213,8 +213,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 @@ -246,8 +246,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 @@ -262,8 +262,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 @@ -291,8 +291,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 @@ -309,8 +309,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 @@ -340,8 +340,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

0 comments on commit 625bf84

Please sign in to comment.