Add auto-updating of plugins with a 60-second interval. Refactor updating and syncing of plugins.

include/library.hpp

@@ -12,11 +12,11 @@ namespace rack {
 namespace library {
 
 
-struct Update {
+struct UpdateInfo {
 	std::string name;
 	std::string version;
 	std::string changelogUrl;
-	float progress = 0.f;
+	bool downloaded = false;
 };
 
 
@@ -33,7 +33,6 @@ void checkUpdates();
 bool hasUpdates();
 void syncUpdate(const std::string& slug);
 void syncUpdates();
-bool isSyncing();
 
 
 extern std::string appVersion;
@@ -41,11 +40,14 @@ extern std::string appDownloadUrl;
 extern std::string appChangelogUrl;
 
 extern std::string loginStatus;
-// plugin slug -> Update
-extern std::map<std::string, Update> updates;
+// plugin slug -> UpdateInfo
+extern std::map<std::string, UpdateInfo> updateInfos;
 extern std::string updateStatus;
-extern std::string updatingSlug;
-extern bool updatingPlugins;
+extern std::string updateSlug;
+extern float updateProgress;
+/** Whether plugins are currently downloading. */
+extern bool isSyncing;
+/** Whether the UI should ask the user to restart after updating plugins. */
 extern bool restartRequested;
 
 

src/app/MenuBar.cpp

@@ -628,11 +628,10 @@ struct LogInItem : ui::MenuItem {
 
 struct SyncUpdatesItem : ui::MenuItem {
 	void step() override {
-		disabled = true;
 		if (library::updateStatus != "") {
 			text = library::updateStatus;
 		}
-		else if (library::updatingPlugins || library::updatingSlug != "") {
+		else if (library::isSyncing) {
 			text = "Updating...";
 		}
 		else if (!library::hasUpdates()) {
@@ -640,8 +639,9 @@ struct SyncUpdatesItem : ui::MenuItem {
 		}
 		else {
 			text = "Update all";
-			disabled = false;
 		}
+
+		disabled = library::isSyncing || !library::hasUpdates();
 		MenuItem::step();
 	}
 
@@ -659,47 +659,55 @@ struct SyncUpdateItem : ui::MenuItem {
 
 	void setUpdate(const std::string& slug) {
 		this->slug = slug;
-		auto it = library::updates.find(slug);
-		if (it == library::updates.end())
+
+		auto it = library::updateInfos.find(slug);
+		if (it == library::updateInfos.end())
 			return;
+		library::UpdateInfo update = it->second;
 
-		text = it->second.name;
-		plugin::Plugin* p = plugin::getPlugin(slug);
-		if (p) {
-			rightText += "v" + p->version + " → ";
-		}
-		rightText += "v" + it->second.version;
+		text = update.name;
 	}
 
 	ui::Menu* createChildMenu() override {
-		auto it = library::updates.find(slug);
-		if (it == library::updates.end())
+		auto it = library::updateInfos.find(slug);
+		if (it == library::updateInfos.end())
 			return NULL;
+		library::UpdateInfo update = it->second;
 
-		if (it->second.changelogUrl == "")
+		if (update.changelogUrl == "")
 			return NULL;
 
 		ui::Menu* menu = new ui::Menu;
 
 		UrlItem* changelogUrl = new UrlItem;
 		changelogUrl->text = "Changelog";
-		changelogUrl->url = it->second.changelogUrl;
+		changelogUrl->url = update.changelogUrl;
 		menu->addChild(changelogUrl);
 
 		return menu;
 	}
 
 	void step() override {
-		disabled = library::isSyncing();
+		disabled = library::isSyncing;
 
-		auto it = library::updates.find(slug);
-		if (it != library::updates.end()) {
-			if (it->second.progress >= 1) {
+		auto it = library::updateInfos.find(slug);
+		if (it != library::updateInfos.end()) {
+			library::UpdateInfo update = it->second;
+
+			if (update.downloaded) {
 				rightText = CHECKMARK_STRING;
 				disabled = true;
 			}
-			else if (it->second.progress > 0) {
-				rightText = string::f("%.0f%%", it->second.progress * 100.f);
+			else if (slug == library::updateSlug) {
+				rightText = string::f("%.0f%%", library::updateProgress * 100.f);
+			}
+			else {
+				rightText = "";
+				plugin::Plugin* p = plugin::getPlugin(slug);
+				if (p) {
+					rightText += "v" + p->version + " → ";
+				}
+				rightText += "v" + update.version;
 			}
 		}
 
@@ -793,14 +801,14 @@ struct LibraryMenu : ui::Menu {
 			syncItem->text = "Update all";
 			addChild(syncItem);
 
-			if (!library::updates.empty()) {
+			if (!library::updateInfos.empty()) {
 				addChild(new ui::MenuSeparator);
 
 				ui::MenuLabel* updatesLabel = new ui::MenuLabel;
 				updatesLabel->text = "Updates";
 				addChild(updatesLabel);
 
-				for (auto& pair : library::updates) {
+				for (auto& pair : library::updateInfos) {
 					SyncUpdateItem* updateItem = new SyncUpdateItem;
 					updateItem->setUpdate(pair.first);
 					addChild(updateItem);

src/library.cpp

@@ -1,4 +1,6 @@
 #include <thread>
+#include <mutex>
+#include <condition_variable>
 
 #include <library.hpp>
 #include <settings.hpp>
@@ -16,6 +18,26 @@ namespace rack {
 namespace library {
 
 
+static std::mutex updatesLoopMutex;
+static std::condition_variable updatesLoopCv;
+static bool updatesLoopRunning = false;
+
+
+static void checkUpdatesLoop() {
+	updatesLoopRunning = true;
+	while (updatesLoopRunning) {
+		checkUpdates();
+
+		// Sleep a few seconds, or wake up when destroy() is called
+		std::unique_lock<std::mutex> lock(updatesLoopMutex);
+		auto duration = std::chrono::seconds(60);
+		if (!updatesLoopRunning)
+			break;
+		updatesLoopCv.wait_for(lock, duration, []() {return !updatesLoopRunning;});
+	}
+}
+
+
 void init() {
 	if (settings::autoCheckUpdates && !settings::devMode) {
 		std::thread t([&]() {
@@ -24,7 +46,7 @@ void init() {
 		t.detach();
 
 		std::thread t2([&] {
-			checkUpdates();
+			checkUpdatesLoop();
 		});
 		t2.detach();
 	}
@@ -32,6 +54,12 @@ void init() {
 
 
 void destroy() {
+	// Stop checkUpdatesLoop thread if it's running
+	{
+		std::lock_guard<std::mutex> lock(updatesLoopMutex);
+		updatesLoopRunning = false;
+		updatesLoopCv.notify_all();
+	}
 }
 
 
@@ -108,25 +136,32 @@ void logIn(const std::string& email, const std::string& password) {
 
 void logOut() {
 	settings::token = "";
-	updates.clear();
+	updateInfos.clear();
 }
 
 
+static network::CookieMap getTokenCookies() {
+	network::CookieMap cookies;
+	cookies["token"] = settings::token;
+	return cookies;
+}
+
 void checkUpdates() {
 	if (settings::token.empty())
 		return;
 
-	updates.clear();
+	// Refuse to check for updates while updating plugins
+	if (isSyncing)
+		return;
+
 	updateStatus = "Querying for updates...";
 
 	// Get user's plugins list
 	std::string pluginsUrl = API_URL + "/plugins";
-	network::CookieMap cookies;
-	cookies["token"] = settings::token;
-	json_t* pluginsResJ = network::requestJson(network::METHOD_GET, pluginsUrl, NULL, cookies);
+	json_t* pluginsResJ = network::requestJson(network::METHOD_GET, pluginsUrl, NULL, getTokenCookies());
 	if (!pluginsResJ) {
 		WARN("Request for user's plugins failed");
-		updateStatus = "Could not query updates";
+		updateStatus = "Could not query plugins";
 		return;
 	}
 	DEFER({json_decref(pluginsResJ);});
@@ -134,7 +169,7 @@ void checkUpdates() {
 	json_t* errorJ = json_object_get(pluginsResJ, "error");
 	if (errorJ) {
 		WARN("Request for user's plugins returned an error: %s", json_string_value(errorJ));
-		updateStatus = "Could not query updates";
+		updateStatus = "Could not query plugins";
 		return;
 	}
 
@@ -157,7 +192,6 @@ void checkUpdates() {
 	size_t pluginIndex;
 	json_t* pluginJ;
 	json_array_foreach(pluginsJ, pluginIndex, pluginJ) {
-		Update update;
 		// Get plugin manifest
 		std::string slug = json_string_value(pluginJ);
 		json_t* manifestJ = json_object_get(manifestsJ, slug.c_str());
@@ -166,6 +200,14 @@ void checkUpdates() {
 			continue;
 		}
 
+		// Don't replace existing UpdateInfo, even if version is newer.
+		// This keeps things sane and ensures that only one version of each plugin is downloaded to `plugins/` at a time.
+		auto it = updateInfos.find(slug);
+		if (it != updateInfos.end()) {
+			continue;
+		}
+		UpdateInfo update;
+
 		// Get plugin name
 		json_t* nameJ = json_object_get(manifestJ, "name");
 		if (nameJ)
@@ -184,14 +226,7 @@ void checkUpdates() {
 		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
+		// Require that "status" is "available"
 		json_t* statusJ = json_object_get(manifestJ, "status");
 		if (!statusJ)
 			continue;
@@ -201,19 +236,17 @@ void checkUpdates() {
 
 		// Get changelog URL
 		json_t* changelogUrlJ = json_object_get(manifestJ, "changelogUrl");
-		if (changelogUrlJ) {
+		if (changelogUrlJ)
 			update.changelogUrl = json_string_value(changelogUrlJ);
-		}
 
 		// Add update to updates map
-		updates[slug] = update;
+		updateInfos[slug] = update;
 	}
 
-
 	// Get module whitelist
 	{
-		std::string whitelistUrl = API_URL + "/moduleWhitelist";
-		json_t* whitelistResJ = network::requestJson(network::METHOD_GET, whitelistUrl, NULL, cookies);
+		std::string whitelistUrl = API_URL + "/modules";
+		json_t* whitelistResJ = network::requestJson(network::METHOD_GET, whitelistUrl, NULL, getTokenCookies());
 		if (!whitelistResJ) {
 			WARN("Request for module whitelist failed");
 			updateStatus = "Could not query updates";
@@ -246,8 +279,8 @@ void checkUpdates() {
 
 
 bool hasUpdates() {
-	for (auto& pair : updates) {
-		if (pair.second.progress < 1.f)
+	for (auto& pair : updateInfos) {
+		if (!pair.second.downloaded)
 			return true;
 	}
 	return false;
@@ -258,31 +291,45 @@ void syncUpdate(const std::string& slug) {
 	if (settings::token.empty())
 		return;
 
-	auto it = updates.find(slug);
-	if (it == updates.end())
+	isSyncing = true;
+	DEFER({isSyncing = false;});
+
+	// Get the UpdateInfo object
+	auto it = updateInfos.find(slug);
+	if (it == updateInfos.end())
 		return;
-	Update& update = it->second;
+	UpdateInfo update = it->second;
+
+	updateSlug = slug;
+	DEFER({updateSlug = "";});
 
-	updatingSlug = slug;
-	DEFER({updatingSlug = "";});
+	// Set progress to 0%
+	updateProgress = 0.f;
+	DEFER({updateProgress = 0.f;});
 
+	INFO("Downloading plugin %s v%s for %s", slug.c_str(), update.version.c_str(), APP_ARCH.c_str());
+
+	// Get download URL
 	std::string downloadUrl = API_URL + "/download";
 	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 v%s for %s", slug.c_str(), update.version.c_str(), APP_ARCH.c_str());
-
-	// Download plugin package
+	// Get file path
 	std::string packageFilename = slug + "-" + update.version + "-" + APP_ARCH + ".vcvplugin";
 	std::string packagePath = system::join(asset::pluginsPath, packageFilename);
-	if (!network::requestDownload(downloadUrl, packagePath, &update.progress, cookies)) {
+
+	// Download plugin package
+	if (!network::requestDownload(downloadUrl, packagePath, &updateProgress, getTokenCookies())) {
 		WARN("Plugin %s download was unsuccessful", slug.c_str());
 		return;
 	}
+
+	// updateInfos could possibly change in the checkUpdates() thread, so re-get the UpdateInfo to modify it.
+	it = updateInfos.find(slug);
+	if (it == updateInfos.end())
+		return;
+	it->second.downloaded = true;
 }
 
 
@@ -290,31 +337,27 @@ void syncUpdates() {
 	if (settings::token.empty())
 		return;
 
-	// Iterate by value because the map might change
-	for (auto pair : updates) {
+	// updateInfos could possibly change in the checkUpdates() thread, but checkUpdates() will not execute if syncUpdate() is running, so the chance of the updateInfos map being modified while iterating is rare.
+	auto updateInfosClone = updateInfos;
+	for (auto& pair : updateInfosClone) {
 		syncUpdate(pair.first);
 	}
 	restartRequested = true;
 }
 
 
-bool isSyncing() {
-	return updatingPlugins || (updatingSlug != "");
-}
-
-
 std::string appVersion;
 std::string appDownloadUrl;
 std::string appChangelogUrl;
 
 std::string loginStatus;
-std::map<std::string, Update> updates;
+std::map<std::string, UpdateInfo> updateInfos;
 std::string updateStatus;
-std::string updatingSlug;
-bool updatingPlugins = false;
+std::string updateSlug;
+float updateProgress = 0.f;
+bool isSyncing = false;
 bool restartRequested = false;
 
 
-
 } // namespace library
 } // namespace rack