From b146f766b9a5c0be2f0e07d5409249c760898e06 Mon Sep 17 00:00:00 2001 From: Andrew Belt Date: Wed, 16 Sep 2020 03:18:47 -0400 Subject: [PATCH] Refactor plugin syncing in `library::`. --- include/common.hpp | 8 ++-- include/library.hpp | 12 ++--- include/network.hpp | 2 +- src/app/MenuBar.cpp | 86 ++++++++++++++++++++--------------- src/app/ModuleBrowser.cpp | 2 +- src/library.cpp | 94 +++++++++++++++++++-------------------- src/network.cpp | 32 ++++++------- src/rtmidi.cpp | 4 +- 8 files changed, 128 insertions(+), 112 deletions(-) diff --git a/include/common.hpp b/include/common.hpp index 43747ca3..4890e6fe 100644 --- a/include/common.hpp +++ b/include/common.hpp @@ -169,15 +169,15 @@ Does *not* add the default value to the map. Posted to https://stackoverflow.com/a/63683271/272642. Example: - std::map m; - int v = getWithDefault(m, "a", 3); + std::map m; + int v = get(m, "a", 3); // v is 3 because the key "a" does not exist - int w = getWithDefault(m, "a"); + int w = get(m, "a"); // w is 0 because no default value is given, so it assumes the default int. */ template -typename C::mapped_type getWithDefault(const C& m, const typename C::key_type& key, const typename C::mapped_type& def = typename C::mapped_type()) { +typename C::mapped_type get(const C& m, const typename C::key_type& key, const typename C::mapped_type& def = typename C::mapped_type()) { typename C::const_iterator it = m.find(key); if (it == m.end()) return def; diff --git a/include/library.hpp b/include/library.hpp index aa54d036..eac5a721 100644 --- a/include/library.hpp +++ b/include/library.hpp @@ -1,7 +1,7 @@ #pragma once #include -#include +#include namespace rack { @@ -13,8 +13,7 @@ namespace library { struct Update { - std::string pluginSlug; - std::string pluginName; + std::string name; std::string version; std::string changelogUrl; float progress = 0.f; @@ -31,7 +30,7 @@ void logIn(const std::string& email, const std::string& password); void logOut(); void queryUpdates(); bool hasUpdates(); -void syncUpdate(Update* update); +void syncUpdate(const std::string& slug); void syncUpdates(); bool isSyncing(); @@ -41,8 +40,11 @@ extern std::string appDownloadUrl; extern std::string appChangelogUrl; extern std::string loginStatus; -extern std::vector updates; +// plugin slug -> Update +extern std::map updates; extern std::string updateStatus; +extern std::string updatingSlug; +extern bool updatingPlugins; extern bool restartRequested; diff --git a/include/network.hpp b/include/network.hpp index 9acb1271..4603f315 100644 --- a/include/network.hpp +++ b/include/network.hpp @@ -25,7 +25,7 @@ enum Method { void init(); /** Requests a JSON API URL over HTTP(S), using the data as the query (GET) or the body (POST, etc) -Caller must json_decref(). +Caller must json_decref() if return value is non-NULL. */ json_t* requestJson(Method method, const std::string& url, json_t* dataJ, const CookieMap& cookies = {}); /** Returns true if downloaded successfully */ diff --git a/src/app/MenuBar.cpp b/src/app/MenuBar.cpp index 0e11f7cb..c08fe3f4 100644 --- a/src/app/MenuBar.cpp +++ b/src/app/MenuBar.cpp @@ -575,6 +575,7 @@ static bool isLoggingIn = false; struct AccountEmailField : ui::TextField { ui::TextField* passwordField; + void onSelectKey(const event::SelectKey& e) override { if (e.action == GLFW_PRESS && e.key == GLFW_KEY_TAB) { APP->event->setSelected(passwordField); @@ -603,6 +604,7 @@ struct AccountPasswordField : ui::PasswordField { struct LogInItem : ui::MenuItem { ui::TextField* emailField; ui::TextField* passwordField; + void onAction(const event::Action& e) override { isLoggingIn = true; std::string email = emailField->text; @@ -612,7 +614,7 @@ struct LogInItem : ui::MenuItem { isLoggingIn = false; }); t.detach(); - e.consume(NULL); + e.unconsume(); } void step() override { @@ -623,13 +625,13 @@ struct LogInItem : ui::MenuItem { } }; -struct SyncItem : ui::MenuItem { +struct SyncUpdatesItem : ui::MenuItem { void step() override { disabled = true; if (library::updateStatus != "") { text = library::updateStatus; } - else if (library::isSyncing()) { + else if (library::updatingPlugins || library::updatingSlug != "") { text = "Updating..."; } else if (!library::hasUpdates()) { @@ -643,59 +645,72 @@ struct SyncItem : ui::MenuItem { } void onAction(const event::Action& e) override { - std::thread t([ = ] { + std::thread t([=] { library::syncUpdates(); }); t.detach(); - e.consume(NULL); + e.unconsume(); } }; -struct PluginSyncItem : ui::MenuItem { - library::Update* update; +struct SyncUpdateItem : ui::MenuItem { + std::string slug; + + void setUpdate(const std::string& slug) { + this->slug = slug; + auto it = library::updates.find(slug); + if (it == library::updates.end()) + return; - void setUpdate(library::Update* update) { - this->update = update; - text = update->pluginName; - plugin::Plugin* p = plugin::getPlugin(update->pluginSlug); + text = it->second.name; + plugin::Plugin* p = plugin::getPlugin(slug); if (p) { rightText += "v" + p->version + " → "; } - rightText += "v" + update->version; + rightText += "v" + it->second.version; } ui::Menu* createChildMenu() override { - if (update->changelogUrl != "") { - ui::Menu* menu = new ui::Menu; + auto it = library::updates.find(slug); + if (it == library::updates.end()) + return NULL; - UrlItem* changelogUrl = new UrlItem; - changelogUrl->text = "Changelog"; - changelogUrl->url = update->changelogUrl; - menu->addChild(changelogUrl); + if (it->second.changelogUrl == "") + return NULL; - return menu; - } - return NULL; + ui::Menu* menu = new ui::Menu; + + UrlItem* changelogUrl = new UrlItem; + changelogUrl->text = "Changelog"; + changelogUrl->url = it->second.changelogUrl; + menu->addChild(changelogUrl); + + return menu; } void step() override { disabled = library::isSyncing(); - if (update->progress >= 1) { - rightText = CHECKMARK_STRING; - disabled = true; - } - else if (update->progress > 0) { - rightText = string::f("%.0f%%", update->progress * 100.f); + + auto it = library::updates.find(slug); + if (it != library::updates.end()) { + if (it->second.progress >= 1) { + rightText = CHECKMARK_STRING; + disabled = true; + } + else if (it->second.progress > 0) { + rightText = string::f("%.0f%%", it->second.progress * 100.f); + } } + MenuItem::step(); } void onAction(const event::Action& e) override { - std::thread t([ = ] { - library::syncUpdate(update); + std::thread t([=] { + library::syncUpdate(slug); }); t.detach(); - e.consume(NULL); + e.unconsume(); } }; @@ -761,20 +776,20 @@ struct LibraryMenu : ui::Menu { logOutItem->text = "Log out"; addChild(logOutItem); - SyncItem* syncItem = new SyncItem; + SyncUpdatesItem* syncItem = new SyncUpdatesItem; syncItem->text = "Update all"; addChild(syncItem); - if (library::hasUpdates()) { + if (!library::updates.empty()) { addChild(new ui::MenuSeparator); ui::MenuLabel* updatesLabel = new ui::MenuLabel; updatesLabel->text = "Updates"; addChild(updatesLabel); - for (library::Update& update : library::updates) { - PluginSyncItem* updateItem = new PluginSyncItem; - updateItem->setUpdate(&update); + for (auto& pair : library::updates) { + SyncUpdateItem* updateItem = new SyncUpdateItem; + updateItem->setUpdate(pair.first); addChild(updateItem); } } @@ -832,7 +847,6 @@ struct AppUpdateItem : ui::MenuItem { void onAction(const event::Action& e) override { system::openBrowser(library::appDownloadUrl); APP->window->close(); - e.consume(NULL); } }; diff --git a/src/app/ModuleBrowser.cpp b/src/app/ModuleBrowser.cpp index aa324d72..c3b00a41 100644 --- a/src/app/ModuleBrowser.cpp +++ b/src/app/ModuleBrowser.cpp @@ -512,7 +512,7 @@ struct ModuleBrowser : widget::OpaqueWidget { // // Sort by score // modelContainer->children.sort([&](Widget *w1, Widget *w2) { // // If score was not computed, scores[w] returns 0, but this doesn't matter because those widgets aren't visible. - // return getWithDefault(scores, w1, 0.f) > getWithDefault(scores, w2, 0.f); + // return get(scores, w1, 0.f) > get(scores, w2, 0.f); // }); } diff --git a/src/library.cpp b/src/library.cpp index 1ec79561..09ad864b 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -90,9 +90,7 @@ void logIn(const std::string& email, const std::string& password) { loginStatus = "No response from server"; return; } - DEFER({ - json_decref(resJ); - }); + DEFER({json_decref(resJ);}); json_t* errorJ = json_object_get(resJ, "error"); if (errorJ) { @@ -137,9 +135,7 @@ void queryUpdates() { updateStatus = "Could not query updates"; return; } - DEFER({ - json_decref(pluginsResJ); - }); + DEFER({json_decref(pluginsResJ);}); json_t* errorJ = json_object_get(pluginsResJ, "error"); if (errorJ) { @@ -159,9 +155,7 @@ void queryUpdates() { updateStatus = "Could not query updates"; return; } - DEFER({ - json_decref(manifestsResJ); - }); + DEFER({json_decref(manifestsResJ);}); json_t* manifestsJ = json_object_get(manifestsResJ, "manifests"); json_t* pluginsJ = json_object_get(pluginsResJ, "plugins"); @@ -171,31 +165,38 @@ void queryUpdates() { json_array_foreach(pluginsJ, pluginIndex, pluginJ) { Update update; // Get plugin manifest - update.pluginSlug = json_string_value(pluginJ); - json_t* manifestJ = json_object_get(manifestsJ, update.pluginSlug.c_str()); + std::string slug = json_string_value(pluginJ); + json_t* manifestJ = json_object_get(manifestsJ, slug.c_str()); if (!manifestJ) { - WARN("VCV account has plugin %s but no manifest was found", update.pluginSlug.c_str()); + WARN("VCV account has plugin %s but no manifest was found", slug.c_str()); continue; } // Get plugin name json_t* nameJ = json_object_get(manifestJ, "name"); if (nameJ) - update.pluginName = json_string_value(nameJ); + update.name = json_string_value(nameJ); // Get version json_t* versionJ = json_object_get(manifestJ, "version"); if (!versionJ) { - WARN("Plugin %s has no version in manifest", update.pluginSlug.c_str()); + WARN("Plugin %s has no version in manifest", slug.c_str()); continue; } update.version = json_string_value(versionJ); // Check if update is needed - plugin::Plugin* p = plugin::getPlugin(update.pluginSlug); + plugin::Plugin* p = plugin::getPlugin(slug); if (p && p->version == update.version) continue; + // Don't add update if it exists already + auto it = updates.find(slug); + if (it != updates.end()) { + if (it->second.version == update.version) + continue; + } + // Check status json_t* statusJ = json_object_get(manifestJ, "status"); if (!statusJ) @@ -210,7 +211,8 @@ void queryUpdates() { update.changelogUrl = json_string_value(changelogUrlJ); } - updates.push_back(update); + // Add update to updates map + updates[slug] = update; } @@ -223,9 +225,7 @@ void queryUpdates() { updateStatus = "Could not query updates"; return; } - DEFER({ - json_decref(whitelistResJ); - }); + DEFER({json_decref(whitelistResJ);}); std::map> moduleWhitelist; json_t* pluginsJ = json_object_get(whitelistResJ, "plugins"); @@ -252,73 +252,73 @@ void queryUpdates() { bool hasUpdates() { - for (Update& update : updates) { - if (update.progress < 1.f) + for (auto& pair : updates) { + if (pair.second.progress < 1.f) return true; } return false; } -static bool isSyncingUpdate = false; -static bool isSyncingUpdates = false; +void syncUpdate(const std::string& slug) { + if (settings::token.empty()) + return; + auto it = updates.find(slug); + if (it == updates.end()) + return; + Update& update = it->second; -void syncUpdate(Update* update) { - isSyncingUpdate = true; - DEFER({ - isSyncingUpdate = false; - }); + updatingSlug = slug; + DEFER({updatingSlug = "";}); std::string downloadUrl = API_URL + "/download"; - downloadUrl += "?slug=" + network::encodeUrl(update->pluginSlug); - downloadUrl += "&version=" + network::encodeUrl(update->version); + downloadUrl += "?slug=" + network::encodeUrl(slug); + downloadUrl += "&version=" + network::encodeUrl(update.version); downloadUrl += "&arch=" + network::encodeUrl(APP_ARCH); network::CookieMap cookies; cookies["token"] = settings::token; - INFO("Downloading plugin %s %s %s", update->pluginSlug.c_str(), update->version.c_str(), APP_ARCH.c_str()); + INFO("Downloading plugin %s v%s for %s", slug.c_str(), update.version.c_str(), APP_ARCH.c_str()); // Download zip - std::string pluginDest = system::join(asset::pluginsPath, update->pluginSlug + ".zip"); - if (!network::requestDownload(downloadUrl, pluginDest, &update->progress, cookies)) { - WARN("Plugin %s download was unsuccessful", update->pluginSlug.c_str()); + std::string pluginDest = system::join(asset::pluginsPath, slug + ".zip"); + if (!network::requestDownload(downloadUrl, pluginDest, &update.progress, cookies)) { + WARN("Plugin %s download was unsuccessful", slug.c_str()); return; } } void syncUpdates() { - isSyncingUpdates = true; - DEFER({ - isSyncingUpdates = false; - }); - if (settings::token.empty()) return; - for (Update& update : updates) { - if (update.progress < 1.f) - syncUpdate(&update); + // Iterate by value because the map might change + for (auto pair : updates) { + syncUpdate(pair.first); } restartRequested = true; } bool isSyncing() { - return isSyncingUpdate || isSyncingUpdates; + return updatingPlugins || (updatingSlug != ""); } +std::string appVersion; +std::string appDownloadUrl; +std::string appChangelogUrl; + std::string loginStatus; -std::vector updates; +std::map updates; std::string updateStatus; +std::string updatingSlug; +bool updatingPlugins = false; bool restartRequested = false; -std::string appVersion; -std::string appDownloadUrl; -std::string appChangelogUrl; } // namespace library diff --git a/src/network.cpp b/src/network.cpp index 2896430f..610a9586 100644 --- a/src/network.cpp +++ b/src/network.cpp @@ -65,7 +65,7 @@ json_t* requestJson(Method method, const std::string& url, json_t* dataJ, const // Process data if (dataJ) { if (method == METHOD_GET) { - // Append ?key=value&... to url + // Append ?key1=value1&key2=value2&... to url urlS += "?"; bool isFirst = true; const char* key; @@ -131,17 +131,17 @@ json_t* requestJson(Method method, const std::string& url, json_t* dataJ, const // Cleanup if (reqStr) - free(reqStr); + std::free(reqStr); curl_easy_cleanup(curl); curl_slist_free_all(headers); - if (res == CURLE_OK) { - // Parse JSON response - json_error_t error; - json_t* rootJ = json_loads(resText.c_str(), 0, &error); - return rootJ; - } - return NULL; + if (res != CURLE_OK) + return NULL; + + // Parse JSON response + json_error_t error; + json_t* rootJ = json_loads(resText.c_str(), 0, &error); + return rootJ; } @@ -156,6 +156,7 @@ static int xferInfoCallback(void* clientp, curl_off_t dltotal, curl_off_t dlnow, return 0; } + bool requestDownload(const std::string& url, const std::string& filename, float* progress, const CookieMap& cookies) { CURL* curl = createCurl(); @@ -189,21 +190,20 @@ bool requestDownload(const std::string& url, const std::string& filename, float* return res == CURLE_OK; } + std::string encodeUrl(const std::string& s) { CURL* curl = curl_easy_init(); + DEFER({curl_easy_cleanup(curl);}); assert(curl); char* escaped = curl_easy_escape(curl, s.c_str(), s.size()); - std::string ret = escaped; - curl_free(escaped); - curl_easy_cleanup(curl); - return ret; + DEFER({curl_free(escaped);}); + return std::string(escaped); } + std::string urlPath(const std::string& url) { CURLU* curl = curl_url(); - DEFER({ - curl_url_cleanup(curl); - }); + DEFER({curl_url_cleanup(curl);}); if (curl_url_set(curl, CURLUPART_URL, url.c_str(), 0)) return ""; char* buf; diff --git a/src/rtmidi.cpp b/src/rtmidi.cpp index b8e7c6bb..6a7368e3 100644 --- a/src/rtmidi.cpp +++ b/src/rtmidi.cpp @@ -251,7 +251,7 @@ struct RtMidiDriver : midi::Driver { midi::InputDevice* subscribeInput(int deviceId, midi::Input* input) override { if (!(0 <= deviceId && deviceId < (int) rtMidiIn->getPortCount())) return NULL; - RtMidiInputDevice* device = getWithDefault(inputDevices, deviceId, NULL); + RtMidiInputDevice* device = get(inputDevices, deviceId, NULL); if (!device) { try { inputDevices[deviceId] = device = new RtMidiInputDevice(driverId, deviceId); @@ -314,7 +314,7 @@ struct RtMidiDriver : midi::Driver { midi::OutputDevice* subscribeOutput(int deviceId, midi::Output* output) override { if (!(0 <= deviceId && deviceId < (int) rtMidiOut->getPortCount())) return NULL; - RtMidiOutputDevice* device = getWithDefault(outputDevices, deviceId, NULL); + RtMidiOutputDevice* device = get(outputDevices, deviceId, NULL); if (!device) { try { outputDevices[deviceId] = device = new RtMidiOutputDevice(driverId, deviceId);