From 0b2c911ba37208271d56bddea44f667bb129f230 Mon Sep 17 00:00:00 2001
From: Andrew Belt <andrewpbelt@gmail.com>
Date: Wed, 5 Jun 2019 05:56:16 -0400
Subject: [PATCH] Most most of plugin/model validation to fromJson.

---
 plugin.mk             |  2 +-
 src/plugin.cpp        | 34 ----------------------------------
 src/plugin/Model.cpp  | 22 +++++++++++++++++++++-
 src/plugin/Plugin.cpp | 24 ++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/plugin.mk b/plugin.mk
index 574c90c0..e7812aab 100644
--- a/plugin.mk
+++ b/plugin.mk
@@ -65,7 +65,7 @@ endif
 ifdef ARCH_MAC
 	rsync -rR $(DISTRIBUTABLES) dist/$(SLUG)/
 else
-	cp -R --parents $(DISTRIBUTABLES) dist/$(SLUG)/
+	cp -r --parents $(DISTRIBUTABLES) dist/$(SLUG)/
 endif
 	@# Create ZIP package
 	cd dist && zip -q -9 -r $(SLUG)-$(VERSION)-$(ARCH).zip $(SLUG)
diff --git a/src/plugin.cpp b/src/plugin.cpp
index 77eea06f..9c3be741 100644
--- a/src/plugin.cpp
+++ b/src/plugin.cpp
@@ -132,46 +132,12 @@ static Plugin *loadPlugin(std::string path) {
 		// Load manifest
 		plugin->fromJson(rootJ);
 
-		// Check slug
-		if (!isSlugValid(plugin->slug)) {
-			throw UserException(string::f("Plugin slug \"%s\" is invalid", plugin->slug.c_str()));
-		}
-
 		// Reject plugin if slug already exists
 		Plugin *oldPlugin = getPlugin(plugin->slug);
 		if (oldPlugin) {
 			throw UserException(string::f("Plugin %s is already loaded, not attempting to load it again", plugin->slug.c_str()));
 		}
 
-		// Remove models without names
-		for (auto it = plugin->models.begin(); it != plugin->models.end();) {
-			Model *model = *it;
-			if (model->name == "") {
-				it = plugin->models.erase(it);
-				continue;
-			}
-			it++;
-		}
-
-		// Normalize tags
-		for (Model *model : plugin->models) {
-			std::vector<std::string> normalizedTags;
-			for (const std::string &tag : model->tags) {
-				std::string normalizedTag = normalizeTag(tag);
-				if (!normalizedTag.empty())
-					normalizedTags.push_back(normalizedTag);
-			}
-			model->tags = normalizedTags;
-		}
-
-		// Search for presets
-		for (Model *model : plugin->models) {
-			std::string presetDir = asset::plugin(plugin, "presets/" + model->slug);
-			for (const std::string &presetPath : system::getEntries(presetDir)) {
-				model->presetPaths.push_back(presetPath);
-			}
-		}
-
 		INFO("Loaded plugin %s v%s from %s", plugin->slug.c_str(), plugin->version.c_str(), path.c_str());
 		plugins.push_back(plugin);
 
diff --git a/src/plugin/Model.cpp b/src/plugin/Model.cpp
index 1026c87b..9bacd8f6 100644
--- a/src/plugin/Model.cpp
+++ b/src/plugin/Model.cpp
@@ -1,4 +1,8 @@
 #include <plugin/Model.hpp>
+#include <plugin.hpp>
+#include <asset.hpp>
+#include <system.hpp>
+#include <string.hpp>
 
 
 namespace rack {
@@ -6,23 +10,39 @@ namespace plugin {
 
 
 void Model::fromJson(json_t *rootJ) {
+	assert(plugin);
+
 	json_t *nameJ = json_object_get(rootJ, "name");
 	if (nameJ)
 		name = json_string_value(nameJ);
+	if (name == "")
+		throw UserException(string::f("No module name for slug %s", slug.c_str()));
 
 	json_t *descriptionJ = json_object_get(rootJ, "description");
 	if (descriptionJ)
 		description = json_string_value(descriptionJ);
 
+	// Tags
+	tags.clear();
 	json_t *tagsJ = json_object_get(rootJ, "tags");
 	if (tagsJ) {
 		size_t i;
 		json_t *tagJ;
 		json_array_foreach(tagsJ, i, tagJ) {
 			std::string tag = json_string_value(tagJ);
-			tags.push_back(tag);
+			// Normalize tag
+			tag = normalizeTag(tag);
+			if (tag != "")
+				tags.push_back(tag);
 		}
 	}
+
+	// Preset paths
+	presetPaths.clear();
+	std::string presetDir = asset::plugin(plugin, "presets/" + slug);
+	for (const std::string &presetPath : system::getEntries(presetDir)) {
+		presetPaths.push_back(presetPath);
+	}
 }
 
 
diff --git a/src/plugin/Plugin.cpp b/src/plugin/Plugin.cpp
index 8c768b0c..8f370077 100644
--- a/src/plugin/Plugin.cpp
+++ b/src/plugin/Plugin.cpp
@@ -32,18 +32,30 @@ Model *Plugin::getModel(std::string slug) {
 }
 
 void Plugin::fromJson(json_t *rootJ) {
+	// Slug
 	json_t *slugJ = json_object_get(rootJ, "slug");
 	if (slugJ)
 		slug = json_string_value(slugJ);
+	if (slug == "")
+		throw UserException("No plugin slug");
+	if (!isSlugValid(slug))
+		throw UserException(string::f("Plugin slug \"%s\" is invalid", slug.c_str()));
 
+	// Version
 	json_t *versionJ = json_object_get(rootJ, "version");
 	if (versionJ)
 		version = json_string_value(versionJ);
+	if (version == "")
+		throw UserException("No plugin version");
 
+	// Name
 	json_t *nameJ = json_object_get(rootJ, "name");
 	if (nameJ)
 		name = json_string_value(nameJ);
+	if (name == "")
+		throw UserException("No plugin name");
 
+	// Brand
 	json_t *brandJ = json_object_get(rootJ, "brand");
 	if (brandJ)
 		brand = json_string_value(brandJ);
@@ -116,6 +128,18 @@ void Plugin::fromJson(json_t *rootJ) {
 			model->fromJson(moduleJ);
 		}
 	}
+
+	// Remove models without names
+	// This is a hacky way of matching JSON models with C++ models.
+	for (auto it = models.begin(); it != models.end();) {
+		Model *model = *it;
+		if (model->name == "") {
+			it = models.erase(it);
+			delete model;
+			continue;
+		}
+		it++;
+	}
 }