diff options
| author | jacqueline <me@jacqueline.id.au> | 2023-05-16 14:11:38 +1000 |
|---|---|---|
| committer | jacqueline <me@jacqueline.id.au> | 2023-05-16 14:11:38 +1000 |
| commit | d71f726c42963d55809605b4dc4144970ca0f230 (patch) | |
| tree | 5dc8e6355f5e870e767c3d23ec4009bc39155821 | |
| parent | 785349eb5b3255d0a51ca7459531f75cb47c90ac (diff) | |
| download | tangara-fw-d71f726c42963d55809605b4dc4144970ca0f230.tar.gz | |
Add pagination to database queries
| -rw-r--r-- | src/database/database.cpp | 219 | ||||
| -rw-r--r-- | src/database/include/database.hpp | 81 | ||||
| -rw-r--r-- | src/database/include/song.hpp | 11 | ||||
| -rw-r--r-- | src/database/song.cpp | 6 | ||||
| -rw-r--r-- | src/database/test/test_database.cpp | 129 | ||||
| -rw-r--r-- | src/main/app_console.cpp | 29 |
6 files changed, 325 insertions, 150 deletions
diff --git a/src/database/database.cpp b/src/database/database.cpp index df10ebb9..f5fe5240 100644 --- a/src/database/database.cpp +++ b/src/database/database.cpp @@ -2,9 +2,11 @@ #include <stdint.h> +#include <algorithm> #include <cstdint> #include <functional> #include <iomanip> +#include <memory> #include <sstream> #include "esp_log.h" @@ -216,6 +218,43 @@ auto Database::Update() -> std::future<void> { }); } +auto Database::GetSongs(std::size_t page_size) -> std::future<Result<Song>*> { + return RunOnDbTask<Result<Song>*>([=, this]() -> Result<Song>* { + Continuation<Song> c{.iterator = nullptr, + .prefix = CreateDataPrefix().data, + .start_key = CreateDataPrefix().data, + .forward = true, + .was_prev_forward = true, + .page_size = page_size}; + return dbGetPage(c); + }); +} + +auto Database::GetDump(std::size_t page_size) + -> std::future<Result<std::string>*> { + return RunOnDbTask<Result<std::string>*>([=, this]() -> Result<std::string>* { + Continuation<std::string> c{.iterator = nullptr, + .prefix = "", + .start_key = "", + .forward = true, + .was_prev_forward = true, + .page_size = page_size}; + return dbGetPage(c); + }); +} + +template <typename T> +auto Database::GetPage(Continuation<T>* c) -> std::future<Result<T>*> { + Continuation<T> copy = *c; + return RunOnDbTask<Result<T>*>( + [=, this]() -> Result<T>* { return dbGetPage(copy); }); +} + +template auto Database::GetPage<Song>(Continuation<Song>* c) + -> std::future<Result<Song>*>; +template auto Database::GetPage<std::string>(Continuation<std::string>* c) + -> std::future<Result<std::string>*>; + auto Database::dbMintNewSongId() -> SongId { SongId next_id = 1; std::string val; @@ -287,37 +326,148 @@ auto Database::dbPutSong(SongId id, dbPutHash(hash, id); } -auto parse_song(ITagParser* parser, - const leveldb::Slice& key, - const leveldb::Slice& value) -> std::optional<Song> { - std::optional<SongData> data = ParseDataValue(value); - if (!data) { +template <typename T> +auto Database::dbGetPage(const Continuation<T>& c) -> Result<T>* { + // Work out our starting point. Sometimes this will already done. + leveldb::Iterator* it = nullptr; + if (c.iterator != nullptr) { + it = c.iterator->release(); + } + if (it == nullptr) { + it = db_->NewIterator(leveldb::ReadOptions()); + it->Seek(c.start_key); + } + + // Fix off-by-one if we just changed direction. + if (c.forward != c.was_prev_forward) { + if (c.forward) { + it->Next(); + } else { + it->Prev(); + } + } + + // Grab results. + std::optional<std::string> first_key; + std::vector<T> records; + while (records.size() < c.page_size && it->Valid()) { + if (!it->key().starts_with(c.prefix)) { + break; + } + if (!first_key) { + first_key = it->key().ToString(); + } + std::optional<T> parsed = ParseRecord<T>(it->key(), it->value()); + if (parsed) { + records.push_back(*parsed); + } + if (c.forward) { + it->Next(); + } else { + it->Prev(); + } + } + + std::unique_ptr<leveldb::Iterator> iterator(it); + if (iterator != nullptr) { + if (!iterator->Valid() || !it->key().starts_with(c.prefix)) { + iterator.reset(); + } + } + + // Put results into canonical order if we were iterating backwards. + if (!c.forward) { + std::reverse(records.begin(), records.end()); + } + + // Work out the new continuations. + std::optional<Continuation<T>> next_page; + if (c.forward) { + if (iterator != nullptr) { + // We were going forward, and now we want the next page. Re-use the + // existing iterator, and point the start key at it. + std::string key = iterator->key().ToString(); + next_page = Continuation<T>{ + .iterator = std::make_shared<std::unique_ptr<leveldb::Iterator>>( + std::move(iterator)), + .prefix = c.prefix, + .start_key = key, + .forward = true, + .was_prev_forward = true, + .page_size = c.page_size, + }; + } + // No iterator means we ran out of results in this direction. + } else { + // We were going backwards, and now we want the next page. This is a + // reversal, to set the start key to the first record we saw and mark that + // it's off by one. + next_page = Continuation<T>{ + .iterator = nullptr, + .prefix = c.prefix, + .start_key = *first_key, + .forward = true, + .was_prev_forward = false, + .page_size = c.page_size, + }; + } + + std::optional<Continuation<T>> prev_page; + if (c.forward) { + // We were going forwards, and now we want the previous page. Set the search + // key to the first result we saw, and mark that it's off by one. + prev_page = Continuation<T>{ + .iterator = nullptr, + .prefix = c.prefix, + .start_key = *first_key, + .forward = false, + .was_prev_forward = true, + .page_size = c.page_size, + }; + } else { + if (iterator != nullptr) { + // We were going backwards, and we still want to go backwards. The + // iterator is still valid. + std::string key = iterator->key().ToString(); + prev_page = Continuation<T>{ + .iterator = std::make_shared<std::unique_ptr<leveldb::Iterator>>( + std::move(iterator)), + .prefix = c.prefix, + .start_key = key, + .forward = false, + .was_prev_forward = false, + .page_size = c.page_size, + }; + } + // No iterator means we ran out of results in this direction. + } + + return new Result<T>(std::move(records), next_page, prev_page); +} + +template auto Database::dbGetPage<Song>(const Continuation<Song>& c) + -> Result<Song>*; +template auto Database::dbGetPage<std::string>( + const Continuation<std::string>& c) -> Result<std::string>*; + +template <> +auto Database::ParseRecord<Song>(const leveldb::Slice& key, + const leveldb::Slice& val) + -> std::optional<Song> { + std::optional<SongData> data = ParseDataValue(val); + if (!data || data->is_tombstoned()) { return {}; } SongTags tags; - if (!parser->ReadAndParseTags(data->filepath(), &tags)) { + if (!tag_parser_->ReadAndParseTags(data->filepath(), &tags)) { return {}; } return Song(*data, tags); } -auto Database::GetSongs(std::size_t page_size) -> std::future<Result<Song>> { - return RunOnDbTask<Result<Song>>([=, this]() -> Result<Song> { - return Query<Song>(CreateDataPrefix().slice, page_size, - std::bind_front(&parse_song, tag_parser_)); - }); -} - -auto Database::GetMoreSongs(std::size_t page_size, Continuation c) - -> std::future<Result<Song>> { - leveldb::Iterator* it = c.release(); - return RunOnDbTask<Result<Song>>([=, this]() -> Result<Song> { - return Query<Song>(it, page_size, - std::bind_front(&parse_song, tag_parser_)); - }); -} - -auto parse_dump(const leveldb::Slice& key, const leveldb::Slice& value) +template <> +auto Database::ParseRecord<std::string>(const leveldb::Slice& key, + const leveldb::Slice& val) -> std::optional<std::string> { std::ostringstream stream; stream << "key: "; @@ -335,33 +485,14 @@ auto parse_dump(const leveldb::Slice& key, const leveldb::Slice& value) << static_cast<int>(str[i]); } } - for (std::size_t i = 2; i < str.size(); i++) { - } } stream << "\tval: 0x"; - std::string str = value.ToString(); - for (int i = 0; i < value.size(); i++) { + std::string str = val.ToString(); + for (int i = 0; i < val.size(); i++) { stream << std::hex << std::setfill('0') << std::setw(2) << static_cast<int>(str[i]); } return stream.str(); } -auto Database::GetDump(std::size_t page_size) - -> std::future<Result<std::string>> { - leveldb::Iterator* it = db_->NewIterator(leveldb::ReadOptions()); - it->SeekToFirst(); - return RunOnDbTask<Result<std::string>>([=, this]() -> Result<std::string> { - return Query<std::string>(it, page_size, &parse_dump); - }); -} - -auto Database::GetMoreDump(std::size_t page_size, Continuation c) - -> std::future<Result<std::string>> { - leveldb::Iterator* it = c.release(); - return RunOnDbTask<Result<std::string>>([=, this]() -> Result<std::string> { - return Query<std::string>(it, page_size, &parse_dump); - }); -} - } // namespace database diff --git a/src/database/include/database.hpp b/src/database/include/database.hpp index 29872e8d..da0ed083 100644 --- a/src/database/include/database.hpp +++ b/src/database/include/database.hpp @@ -22,7 +22,15 @@ namespace database { -typedef std::unique_ptr<leveldb::Iterator> Continuation; +template <typename T> +struct Continuation { + std::shared_ptr<std::unique_ptr<leveldb::Iterator>> iterator; + std::string prefix; + std::string start_key; + bool forward; + bool was_prev_forward; + size_t page_size; +}; /* * Wrapper for a set of results from the database. Owns the list of results, as @@ -32,29 +40,23 @@ typedef std::unique_ptr<leveldb::Iterator> Continuation; template <typename T> class Result { public: - auto values() -> std::vector<T>* { return values_.release(); } - auto continuation() -> Continuation { return std::move(c_); } - auto HasMore() -> bool { return c_->Valid(); } - - Result(std::vector<T>* values, Continuation c) - : values_(values), c_(std::move(c)) {} - - Result(std::unique_ptr<std::vector<T>> values, Continuation c) - : values_(std::move(values)), c_(std::move(c)) {} + auto values() const -> const std::vector<T>& { return values_; } - Result(Result&& other) - : values_(move(other.values_)), c_(std::move(other.c_)) {} + auto next_page() -> std::optional<Continuation<T>>& { return next_page_; } + auto prev_page() -> std::optional<Continuation<T>>& { return prev_page_; } - Result operator=(Result&& other) { - return Result(other.values(), std::move(other.continuation())); - } + Result(const std::vector<T>&& values, + std::optional<Continuation<T>> next, + std::optional<Continuation<T>> prev) + : values_(values), next_page_(next), prev_page_(prev) {} Result(const Result&) = delete; Result& operator=(const Result&) = delete; private: - std::unique_ptr<std::vector<T>> values_; - Continuation c_; + std::vector<T> values_; + std::optional<Continuation<T>> next_page_; + std::optional<Continuation<T>> prev_page_; }; class Database { @@ -73,13 +75,11 @@ class Database { auto Update() -> std::future<void>; - auto GetSongs(std::size_t page_size) -> std::future<Result<Song>>; - auto GetMoreSongs(std::size_t page_size, Continuation c) - -> std::future<Result<Song>>; + auto GetSongs(std::size_t page_size) -> std::future<Result<Song>*>; + auto GetDump(std::size_t page_size) -> std::future<Result<std::string>*>; - auto GetDump(std::size_t page_size) -> std::future<Result<std::string>>; - auto GetMoreDump(std::size_t page_size, Continuation c) - -> std::future<Result<std::string>>; + template <typename T> + auto GetPage(Continuation<T>* c) -> std::future<Result<T>*>; Database(const Database&) = delete; Database& operator=(const Database&) = delete; @@ -110,31 +110,20 @@ class Database { -> void; template <typename T> - using Parser = std::function<std::optional<T>(const leveldb::Slice& key, - const leveldb::Slice& value)>; - - template <typename T> - auto Query(const leveldb::Slice& prefix, - std::size_t max_results, - Parser<T> parser) -> Result<T> { - leveldb::Iterator* it = db_->NewIterator(leveldb::ReadOptions()); - it->Seek(prefix); - return Query(it, max_results, parser); - } + auto dbGetPage(const Continuation<T>& c) -> Result<T>*; template <typename T> - auto Query(leveldb::Iterator* it, std::size_t max_results, Parser<T> parser) - -> Result<T> { - auto results = std::make_unique<std::vector<T>>(); - for (std::size_t i = 0; i < max_results && it->Valid(); i++) { - std::optional<T> r = std::invoke(parser, it->key(), it->value()); - if (r) { - results->push_back(*r); - } - it->Next(); - } - return {std::move(results), std::unique_ptr<leveldb::Iterator>(it)}; - } + auto ParseRecord(const leveldb::Slice& key, const leveldb::Slice& val) + -> std::optional<T>; }; +template <> +auto Database::ParseRecord<Song>(const leveldb::Slice& key, + const leveldb::Slice& val) + -> std::optional<Song>; +template <> +auto Database::ParseRecord<std::string>(const leveldb::Slice& key, + const leveldb::Slice& val) + -> std::optional<std::string>; + } // namespace database diff --git a/src/database/include/song.hpp b/src/database/include/song.hpp index e51e5587..9a84e124 100644 --- a/src/database/include/song.hpp +++ b/src/database/include/song.hpp @@ -4,6 +4,7 @@ #include <optional> #include <string> +#include <utility> #include "leveldb/db.h" #include "span.hpp" @@ -133,16 +134,20 @@ class SongData { */ class Song { public: - Song(SongData data, SongTags tags) : data_(data), tags_(tags) {} + Song(const SongData& data, const SongTags& tags) : data_(data), tags_(tags) {} + Song(const Song& other) = default; - auto data() -> const SongData& { return data_; } - auto tags() -> const SongTags& { return tags_; } + auto data() const -> const SongData& { return data_; } + auto tags() const -> const SongTags& { return tags_; } bool operator==(const Song&) const = default; + Song operator=(const Song& other) const { return Song(other); } private: const SongData data_; const SongTags tags_; }; +void swap(Song& first, Song& second); + } // namespace database diff --git a/src/database/song.cpp b/src/database/song.cpp index a0e6ce4b..b6a1baac 100644 --- a/src/database/song.cpp +++ b/src/database/song.cpp @@ -36,4 +36,10 @@ auto SongData::Exhume(const std::string& new_path) const -> SongData { return SongData(id_, new_path, tags_hash_, play_count_, false); } +void swap(Song& first, Song& second) { + Song temp = first; + first = second; + second = temp; +} + } // namespace database diff --git a/src/database/test/test_database.cpp b/src/database/test/test_database.cpp index d61421ee..a88c4bb3 100644 --- a/src/database/test/test_database.cpp +++ b/src/database/test/test_database.cpp @@ -60,8 +60,8 @@ TEST_CASE("song database", "[integration]") { std::unique_ptr<Database> db(open_res.value()); SECTION("empty database") { - std::unique_ptr<std::vector<Song>> res(db->GetSongs(10).get().values()); - REQUIRE(res->size() == 0); + std::unique_ptr<Result<Song>> res(db->GetSongs(10).get()); + REQUIRE(res->values().size() == 0); } SECTION("add new songs") { @@ -71,24 +71,23 @@ TEST_CASE("song database", "[integration]") { db->Update(); - std::unique_ptr<std::vector<Song>> res(db->GetSongs(10).get().values()); - REQUIRE(res->size() == 3); - CHECK(*res->at(0).tags().title == "Song 1"); - CHECK(res->at(0).data().id() == 1); - CHECK(*res->at(1).tags().title == "Song 2"); - CHECK(res->at(1).data().id() == 2); - CHECK(*res->at(2).tags().title == "Song 3"); - CHECK(res->at(2).data().id() == 3); + std::unique_ptr<Result<Song>> res(db->GetSongs(10).get()); + REQUIRE(res->values().size() == 3); + CHECK(*res->values().at(0).tags().title == "Song 1"); + CHECK(res->values().at(0).data().id() == 1); + CHECK(*res->values().at(1).tags().title == "Song 2"); + CHECK(res->values().at(1).data().id() == 2); + CHECK(*res->values().at(2).tags().title == "Song 3"); + CHECK(res->values().at(2).data().id() == 3); SECTION("update with no filesystem changes") { db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - REQUIRE(new_res->size() == 3); - CHECK(res->at(0) == new_res->at(0)); - CHECK(res->at(1) == new_res->at(1)); - CHECK(res->at(2) == new_res->at(2)); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + REQUIRE(new_res->values().size() == 3); + CHECK(res->values().at(0) == new_res->values().at(0)); + CHECK(res->values().at(1) == new_res->values().at(1)); + CHECK(res->values().at(2) == new_res->values().at(2)); } SECTION("update with all songs gone") { @@ -96,19 +95,17 @@ TEST_CASE("song database", "[integration]") { db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - CHECK(new_res->size() == 0); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + CHECK(new_res->values().size() == 0); SECTION("update with one song returned") { songs.MakeSong("song2.wav", "Song 2"); db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - REQUIRE(new_res->size() == 1); - CHECK(res->at(1) == new_res->at(0)); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + REQUIRE(new_res->values().size() == 1); + CHECK(res->values().at(1) == new_res->values().at(0)); } } @@ -117,11 +114,10 @@ TEST_CASE("song database", "[integration]") { db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - REQUIRE(new_res->size() == 2); - CHECK(res->at(0) == new_res->at(0)); - CHECK(res->at(2) == new_res->at(1)); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + REQUIRE(new_res->values().size() == 2); + CHECK(res->values().at(0) == new_res->values().at(0)); + CHECK(res->values().at(2) == new_res->values().at(1)); } SECTION("update with tags changed") { @@ -129,14 +125,14 @@ TEST_CASE("song database", "[integration]") { db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - REQUIRE(new_res->size() == 3); - CHECK(res->at(0) == new_res->at(0)); - CHECK(res->at(1) == new_res->at(1)); - CHECK(*new_res->at(2).tags().title == "The Song 3"); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + REQUIRE(new_res->values().size() == 3); + CHECK(res->values().at(0) == new_res->values().at(0)); + CHECK(res->values().at(1) == new_res->values().at(1)); + CHECK(*new_res->values().at(2).tags().title == "The Song 3"); // The id should not have changed, since this was just a tag update. - CHECK(res->at(2).data().id() == new_res->at(2).data().id()); + CHECK(res->values().at(2).data().id() == + new_res->values().at(2).data().id()); } SECTION("update with one new song") { @@ -144,14 +140,61 @@ TEST_CASE("song database", "[integration]") { db->Update(); - std::unique_ptr<std::vector<Song>> new_res( - db->GetSongs(10).get().values()); - REQUIRE(new_res->size() == 4); - CHECK(res->at(0) == new_res->at(0)); - CHECK(res->at(1) == new_res->at(1)); - CHECK(res->at(2) == new_res->at(2)); - CHECK(*new_res->at(3).tags().title == "Song 1 (nightcore remix)"); - CHECK(new_res->at(3).data().id() == 4); + std::unique_ptr<Result<Song>> new_res(db->GetSongs(10).get()); + REQUIRE(new_res->values().size() == 4); + CHECK(res->values().at(0) == new_res->values().at(0)); + CHECK(res->values().at(1) == new_res->values().at(1)); + CHECK(res->values().at(2) == new_res->values().at(2)); + CHECK(*new_res->values().at(3).tags().title == + "Song 1 (nightcore remix)"); + CHECK(new_res->values().at(3).data().id() == 4); + } + + SECTION("get songs with pagination") { + std::unique_ptr<Result<Song>> res(db->GetSongs(1).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 1); + REQUIRE(res->next_page()); + + res.reset(db->GetPage(&res->next_page().value()).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 2); + REQUIRE(res->next_page()); + + res.reset(db->GetPage(&res->next_page().value()).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 3); + REQUIRE(!res->next_page()); + + SECTION("page backwards") { + REQUIRE(res->prev_page()); + + res.reset(db->GetPage(&res->prev_page().value()).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 2); + REQUIRE(res->prev_page()); + + res.reset(db->GetPage(&res->prev_page().value()).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 1); + REQUIRE(!res->prev_page()); + + SECTION("page forwards again") { + REQUIRE(res->next_page()); + + res.reset(db->GetPage(&res->next_page().value()).get()); + + REQUIRE(res->values().size() == 1); + CHECK(res->values().at(0).data().id() == 2); + CHECK(res->next_page()); + CHECK(res->prev_page()); + } + } } } } diff --git a/src/main/app_console.cpp b/src/main/app_console.cpp index 759afa91..a85faaf0 100644 --- a/src/main/app_console.cpp +++ b/src/main/app_console.cpp @@ -176,15 +176,15 @@ int CmdDbSongs(int argc, char** argv) { return 1; } - database::Result<database::Song> res = - sInstance->database_->GetSongs(20).get(); + std::unique_ptr<database::Result<database::Song>> res( + sInstance->database_->GetSongs(5).get()); while (true) { - std::unique_ptr<std::vector<database::Song>> r(res.values()); - for (database::Song s : *r) { + for (database::Song s : res->values()) { std::cout << s.tags().title.value_or("[BLANK]") << std::endl; } - if (res.HasMore()) { - res = sInstance->database_->GetMoreSongs(10, res.continuation()).get(); + if (res->next_page()) { + auto continuation = res->next_page().value(); + res.reset(sInstance->database_->GetPage(&continuation).get()); } else { break; } @@ -211,17 +211,18 @@ int CmdDbDump(int argc, char** argv) { std::cout << "=== BEGIN DUMP ===" << std::endl; - database::Result<std::string> res = sInstance->database_->GetDump(20).get(); + std::unique_ptr<database::Result<std::string>> res( + sInstance->database_->GetDump(5).get()); while (true) { - std::unique_ptr<std::vector<std::string>> r(res.values()); - if (r == nullptr) { - break; - } - for (std::string s : *r) { + for (std::string s : res->values()) { std::cout << s << std::endl; } - if (res.HasMore()) { - res = sInstance->database_->GetMoreDump(20, res.continuation()).get(); + if (res->next_page()) { + auto continuation = res->next_page().value(); + res.reset( + sInstance->database_->GetPage<std::string>(&continuation).get()); + } else { + break; } } |
