Refactor plugin syncing in `library::`.

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<std::string, int*> m;
-	int v = getWithDefault(m, "a", 3);
+	std::map<std::string, int> 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>
-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;

include/library.hpp

@@ -1,7 +1,7 @@
 #pragma once
 #include <common.hpp>
 
-#include <vector>
+#include <map>
 
 
 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<Update> updates;
+// plugin slug -> Update
+extern std::map<std::string, Update> updates;
 extern std::string updateStatus;
+extern std::string updatingSlug;
+extern bool updatingPlugins;
 extern bool restartRequested;
 
 

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 */

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);
 	}
 };
 

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);
 			// });
 		}
 

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<std::string, std::set<std::string>> 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<Update> updates;
+std::map<std::string, Update> updates;
 std::string updateStatus;
+std::string updatingSlug;
+bool updatingPlugins = false;
 bool restartRequested = false;
 
-std::string appVersion;
-std::string appDownloadUrl;
-std::string appChangelogUrl;
 
 
 } // namespace library

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;

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);