From 46118ab6feb3291c2065c1ad7d290347b2a92b7d Mon Sep 17 00:00:00 2001 From: casey langen Date: Mon, 25 Jan 2021 23:45:08 -0800 Subject: [PATCH] Ensure CategoryTrackListQuery is implemented successfully, and added correct exception handling for query result parse failures in the remote library WebSocketClient. --- .../library/query/CategoryTrackListQuery.cpp | 5 ++-- .../library/query/DirectoryTrackListQuery.cpp | 2 +- .../library/query/GetPlaylistQuery.cpp | 2 +- .../library/query/SearchTrackListQuery.cpp | 2 +- .../library/query/TrackListQueryBase.h | 11 ++++---- .../library/query/util/Serialization.cpp | 14 ++++++++++ .../library/query/util/Serialization.h | 6 ++++ src/musikcore/net/WebSocketClient.cpp | 10 +++++-- src/musikcore/net/WebSocketClient.h | 1 + src/musikcore/support/Duration.cpp | 20 +++++++++---- src/musikcore/support/Duration.h | 1 + src/musikcube/app/window/TrackListView.cpp | 28 +++++++++++++++---- src/musikcube/app/window/TrackListView.h | 26 ++++++++--------- 13 files changed, 91 insertions(+), 37 deletions(-) diff --git a/src/musikcore/library/query/CategoryTrackListQuery.cpp b/src/musikcore/library/query/CategoryTrackListQuery.cpp index 78c3dfa75..f45138a3a 100755 --- a/src/musikcore/library/query/CategoryTrackListQuery.cpp +++ b/src/musikcore/library/query/CategoryTrackListQuery.cpp @@ -185,14 +185,13 @@ void CategoryTrackListQuery::ProcessResult(musik::core::db::Statement& trackQuer runningDuration += trackQuery.ColumnInt32(1); if (this->parseHeaders && album != lastAlbum) { - headers->insert(index); - if (!headers->empty()) { (*durations)[lastHeaderIndex] = runningDuration; lastHeaderIndex = index; runningDuration = 0; } + headers->insert(index); lastAlbum = album; } @@ -241,7 +240,7 @@ std::string CategoryTrackListQuery::SerializeResult() { void CategoryTrackListQuery::DeserializeResult(const std::string& data) { this->SetStatus(IQuery::Failed); nlohmann::json result = nlohmann::json::parse(data)["result"]; - this->DeserializeTrackListAndHeaders(result, this->library, this->result, this->headers); + this->DeserializeTrackListAndHeaders(result, this->library, this); this->SetStatus(IQuery::Finished); } diff --git a/src/musikcore/library/query/DirectoryTrackListQuery.cpp b/src/musikcore/library/query/DirectoryTrackListQuery.cpp index 6f3a97c73..17bec12b2 100644 --- a/src/musikcore/library/query/DirectoryTrackListQuery.cpp +++ b/src/musikcore/library/query/DirectoryTrackListQuery.cpp @@ -122,7 +122,7 @@ std::string DirectoryTrackListQuery::SerializeResult() { void DirectoryTrackListQuery::DeserializeResult(const std::string& data) { this->SetStatus(IQuery::Failed); nlohmann::json result = nlohmann::json::parse(data)["result"]; - this->DeserializeTrackListAndHeaders(result, this->library, this->result, this->headers); + this->DeserializeTrackListAndHeaders(result, this->library, this); this->SetStatus(IQuery::Finished); } diff --git a/src/musikcore/library/query/GetPlaylistQuery.cpp b/src/musikcore/library/query/GetPlaylistQuery.cpp index 3b934d4ea..483708da7 100644 --- a/src/musikcore/library/query/GetPlaylistQuery.cpp +++ b/src/musikcore/library/query/GetPlaylistQuery.cpp @@ -119,7 +119,7 @@ std::string GetPlaylistQuery::SerializeResult() { void GetPlaylistQuery::DeserializeResult(const std::string& data) { this->SetStatus(IQuery::Failed); nlohmann::json result = nlohmann::json::parse(data)["result"]; - this->DeserializeTrackListAndHeaders(result, this->library, this->result, this->headers); + this->DeserializeTrackListAndHeaders(result, this->library, this); this->SetStatus(IQuery::Finished); } diff --git a/src/musikcore/library/query/SearchTrackListQuery.cpp b/src/musikcore/library/query/SearchTrackListQuery.cpp index 9144b7640..dfcf27a31 100755 --- a/src/musikcore/library/query/SearchTrackListQuery.cpp +++ b/src/musikcore/library/query/SearchTrackListQuery.cpp @@ -193,7 +193,7 @@ std::string SearchTrackListQuery::SerializeResult() { void SearchTrackListQuery::DeserializeResult(const std::string& data) { this->SetStatus(IQuery::Failed); nlohmann::json result = nlohmann::json::parse(data)["result"]; - this->DeserializeTrackListAndHeaders(result, this->library, this->result, this->headers); + this->DeserializeTrackListAndHeaders(result, this->library, this); this->SetStatus(IQuery::Finished); } diff --git a/src/musikcore/library/query/TrackListQueryBase.h b/src/musikcore/library/query/TrackListQueryBase.h index b0669d375..d460ae6c5 100755 --- a/src/musikcore/library/query/TrackListQueryBase.h +++ b/src/musikcore/library/query/TrackListQueryBase.h @@ -103,6 +103,7 @@ namespace musik { namespace core { namespace library { namespace query { nlohmann::json output = { { "result", { { "headers", *this->GetHeaders() }, + { "durations", serialization::DurationMapToJsonMap(*this->GetDurations()) }, { "trackList", serialization::TrackListToJson(*this->GetResult(), true) } }} }; @@ -110,13 +111,13 @@ namespace musik { namespace core { namespace library { namespace query { } void DeserializeTrackListAndHeaders( - nlohmann::json &result, + nlohmann::json& result, ILibraryPtr library, - Result tracks, - Headers headers) + TrackListQueryBase* query) { - serialization::TrackListFromJson(result["trackList"], *tracks, library, true); - serialization::JsonArrayToSet, size_t>(result["headers"], *headers); + serialization::JsonArrayToSet, size_t>(result["headers"], *query->GetHeaders()); + serialization::JsonMapToDuration(result["durations"], *query->GetDurations()); + serialization::TrackListFromJson(result["trackList"], *query->GetResult(), library, true); } private: diff --git a/src/musikcore/library/query/util/Serialization.cpp b/src/musikcore/library/query/util/Serialization.cpp index bed702089..52cdcd916 100644 --- a/src/musikcore/library/query/util/Serialization.cpp +++ b/src/musikcore/library/query/util/Serialization.cpp @@ -237,6 +237,20 @@ namespace musik { namespace core { namespace library { namespace query { return output; } + nlohmann::json DurationMapToJsonMap(const std::map& input) { + nlohmann::json output; + for (auto kv : input) { + output[std::to_string(kv.first)] = kv.second; + } + return output; + } + + void JsonMapToDuration(const nlohmann::json& input, std::map& output) { + for (const auto& item : input.items()) { + output[std::stoi(item.key())] = item.value().get(); + } + } + } } } } } diff --git a/src/musikcore/library/query/util/Serialization.h b/src/musikcore/library/query/util/Serialization.h index e48c40305..7965dc3e8 100644 --- a/src/musikcore/library/query/util/Serialization.h +++ b/src/musikcore/library/query/util/Serialization.h @@ -99,6 +99,12 @@ namespace musik { namespace core { namespace library { namespace query { } } + nlohmann::json DurationMapToJsonMap( + const std::map& input); + + void JsonMapToDuration( + const nlohmann::json& input, + std::map& output); } } } } } diff --git a/src/musikcore/net/WebSocketClient.cpp b/src/musikcore/net/WebSocketClient.cpp index 71be631f0..ddf61b05d 100644 --- a/src/musikcore/net/WebSocketClient.cpp +++ b/src/musikcore/net/WebSocketClient.cpp @@ -171,8 +171,14 @@ WebSocketClient::WebSocketClient(IMessageQueue* messageQueue, Listener* listener std::string rawResult; if (extractRawQueryResult(responseJson, rawResult)) { if (query) { - query->DeserializeResult(rawResult); - this->listener->OnClientQuerySucceeded(this, messageId, query); + try { + query->DeserializeResult(rawResult); + this->listener->OnClientQuerySucceeded(this, messageId, query); + } + catch (...) { + this->listener->OnClientQueryFailed( + this, messageId, query, QueryError::ParseFailed); + } } else { this->listener->OnClientQueryFailed( diff --git a/src/musikcore/net/WebSocketClient.h b/src/musikcore/net/WebSocketClient.h index fa7bb95af..bd0c3f9be 100644 --- a/src/musikcore/net/WebSocketClient.h +++ b/src/musikcore/net/WebSocketClient.h @@ -65,6 +65,7 @@ namespace musik { namespace core { namespace net { Disconnected = 2, AuthFailed = 3, QueryNotFound = 4, + ParseFailed = 5, }; enum class ConnectionError : int { diff --git a/src/musikcore/support/Duration.cpp b/src/musikcore/support/Duration.cpp index f6e9aada2..089a4b6e0 100755 --- a/src/musikcore/support/Duration.cpp +++ b/src/musikcore/support/Duration.cpp @@ -34,16 +34,26 @@ #include "pch.hpp" #include "Duration.h" +#include "NarrowCast.h" #include +template +static std::string formatDuration(N seconds) { + N mins = (seconds / 60); + N secs = seconds - (mins * 60); + char buffer[128]; + snprintf(buffer, sizeof(buffer), "%d:%02d", narrow_cast(mins), narrow_cast(secs)); + return std::string(buffer); +} + namespace musik { namespace core { namespace duration { std::string Duration(int seconds) { - int mins = (seconds / 60); - int secs = seconds - (mins * 60); - char buffer[128]; - snprintf(buffer, sizeof(buffer), "%d:%02d", mins, secs); - return std::string(buffer); + return formatDuration(seconds); + } + + std::string Duration(size_t seconds) { + return formatDuration(seconds); } std::string Duration(double seconds) { diff --git a/src/musikcore/support/Duration.h b/src/musikcore/support/Duration.h index ab156a6fe..555d01ffd 100755 --- a/src/musikcore/support/Duration.h +++ b/src/musikcore/support/Duration.h @@ -40,6 +40,7 @@ namespace musik { namespace core { namespace duration { std::string Duration(const std::string& str); std::string Duration(int seconds); + std::string Duration(size_t seconds); std::string Duration(double seconds); } } } diff --git a/src/musikcube/app/window/TrackListView.cpp b/src/musikcube/app/window/TrackListView.cpp index 0f9b2a213..cc4480e9d 100755 --- a/src/musikcube/app/window/TrackListView.cpp +++ b/src/musikcube/app/window/TrackListView.cpp @@ -131,12 +131,12 @@ void TrackListView::OnTrackListWindowCached(const musik::core::TrackList* track, void TrackListView::OnQueryCompleted(IQuery* query) { if (this->query && query == this->query.get()) { if (this->query->GetStatus() == IQuery::Finished) { - bool hadTracks = this->tracks && this->tracks->Count() > 0; - bool prevQuerySame = this->lastQueryHash == this->query->GetQueryHash(); + const bool hadTracks = this->tracks && this->tracks->Count() > 0; + const bool prevQuerySame = this->lastQueryHash == this->query->GetQueryHash(); this->SetTrackListAndUpateEventHandlers(this->query->GetResult()); this->AdjustTrackListCacheWindowSize(); - this->headers.Set(this->query->GetHeaders()); + this->headers.Set(this->query->GetHeaders(), this->query->GetDurations()); this->lastQueryHash = this->query->GetQueryHash(); /* update our internal state */ @@ -389,11 +389,12 @@ TrackListView::Adapter::Adapter(TrackListView &parent) /* * * * TrackListView::HeaderCalculator * * * */ -void TrackListView::HeaderCalculator::Set(Headers rawOffsets) { +void TrackListView::HeaderCalculator::Set(Headers rawOffsets, Durations durations) { this->rawOffsets = rawOffsets; + this->durations = durations; this->absoluteOffsets.reset(); if (rawOffsets) { - this->absoluteOffsets.reset(new std::set()); + this->absoluteOffsets = std::make_shared>(); size_t i = 0; for (auto val : (*this->rawOffsets)) { this->absoluteOffsets->insert(val + i); @@ -411,6 +412,16 @@ size_t TrackListView::HeaderCalculator::AdapterToTrackListIndex(size_t index) { return this->ApplyHeaderOffset(index, this->absoluteOffsets, -1); } +size_t TrackListView::HeaderCalculator::DurationFromAdapterIndex(size_t index) { + /* ugh, HeaderCalculator should probably be re-thought, but this is especially ugly */ + const auto adjustedIndex = this->ApplyHeaderOffset(index + 1, this->absoluteOffsets, -1); + auto it = this->durations->find(adjustedIndex); + if (it != this->durations->end()) { + return it->second; + } + return 0; +} + size_t TrackListView::HeaderCalculator::TrackListToAdapterIndex(size_t index) { return this->ApplyHeaderOffset(index, this->rawOffsets, 1); } @@ -490,6 +501,11 @@ IScrollAdapter::EntryPtr TrackListView::Adapter::GetEntry(cursespp::ScrollableWi album = _TSTR("tracklist_unknown_album"); } + auto duration = this->parent.headers.DurationFromAdapterIndex(rawIndex); + if (duration > 0) { + album += " - " + core::duration::Duration(duration); + } + album = text::Ellipsize(album, this->GetWidth()); auto entry = std::make_shared( @@ -503,7 +519,7 @@ IScrollAdapter::EntryPtr TrackListView::Adapter::GetEntry(cursespp::ScrollableWi } } - size_t trackIndex = this->parent.headers.AdapterToTrackListIndex(rawIndex); + const size_t trackIndex = this->parent.headers.AdapterToTrackListIndex(rawIndex); TrackPtr track = parent.tracks->Get(trackIndex, sGetAsync); Color attrs = Color::Default; diff --git a/src/musikcube/app/window/TrackListView.h b/src/musikcube/app/window/TrackListView.h index 86b52b326..7774ef133 100755 --- a/src/musikcube/app/window/TrackListView.h +++ b/src/musikcube/app/window/TrackListView.h @@ -63,8 +63,9 @@ namespace musik { sigslot::signal1 Requeried; /* types */ - typedef std::function RowDecorator; - typedef std::shared_ptr > Headers; + using RowDecorator = std::function; + using Headers = TrackListQueryBase::Headers; + using Durations = TrackListQueryBase::Durations; /* ctor, dtor */ TrackListView( @@ -118,10 +119,8 @@ namespace musik { TrackListEntry(const std::string& str, int index, RowType type) : cursespp::SingleLineEntry(str), index(index), type(type) { } - virtual ~TrackListEntry() { } - - RowType GetType() { return type; } - int GetIndex() { return index; } + RowType GetType() noexcept { return type; } + int GetIndex() noexcept { return index; } private: RowType type; @@ -132,10 +131,8 @@ namespace musik { class Adapter : public cursespp::ScrollAdapterBase { public: Adapter(TrackListView &parent); - virtual ~Adapter() { } - - virtual size_t GetEntryCount(); - virtual EntryPtr GetEntry(cursespp::ScrollableWindow* window, size_t index); + size_t GetEntryCount() override; + EntryPtr GetEntry(cursespp::ScrollableWindow* window, size_t index) override; private: TrackListView &parent; @@ -143,16 +140,18 @@ namespace musik { }; private: - /* class to help with header offset calculation */ + /* class to help with header offset calculation. this thing is really gross and + should probably be refactored at some point. */ class HeaderCalculator { public: static const size_t NO_INDEX = (size_t) -1; - void Set(Headers rawOffsets); + void Set(Headers rawOffsets, Durations durations); void Reset(); + bool HeaderAt(size_t index); size_t AdapterToTrackListIndex(size_t index); size_t TrackListToAdapterIndex(size_t index); - bool HeaderAt(size_t index); + size_t DurationFromAdapterIndex(size_t index); size_t NextHeaderIndex(size_t selectedIndex); size_t PrevHeaderIndex(size_t selectedIndex); size_t Count(); @@ -162,6 +161,7 @@ namespace musik { Headers absoluteOffsets; Headers rawOffsets; + Durations durations; }; void OnTrackChanged(size_t index, musik::core::TrackPtr track);