nodes: fix serialized ID typos with backward compatibility

- ChromaKey tolerance inputs are now spelled "upper/lower_tolerance_in";
  old projects with the misspelled "tolerence" IDs keep loading (values
  and connections) via a new Node::GetInputIDForLegacyID() hook applied
  in LoadInput and connection deserialization
- flip/ripple/swirl/tile/wave node IDs moved to the consistent
  org.olivevideoeditor.Olive.* domain; NodeFactory::CreateFromID maps
  the old org.oliveeditor.* IDs so old projects still resolve
- Regression tests: legacy ChromaKey IDs (values + connections) and
  legacy factory IDs

app/node/distort/flip/flipdistortnode.cpp

@@ -50,7 +50,7 @@ QString FlipDistortNode::Name() const
 
 QString FlipDistortNode::id() const
 {
-	return QStringLiteral("org.oliveeditor.Olive.flip");
+	return QStringLiteral("org.olivevideoeditor.Olive.flip");
 }
 
 QVector<Node::CategoryID> FlipDistortNode::Category() const

app/node/distort/ripple/rippledistortnode.cpp

@@ -67,7 +67,7 @@ QString RippleDistortNode::Name() const
 
 QString RippleDistortNode::id() const
 {
-	return QStringLiteral("org.oliveeditor.Olive.ripple");
+	return QStringLiteral("org.olivevideoeditor.Olive.ripple");
 }
 
 QVector<Node::CategoryID> RippleDistortNode::Category() const

app/node/distort/swirl/swirldistortnode.cpp

@@ -61,7 +61,7 @@ QString SwirlDistortNode::Name() const
 
 QString SwirlDistortNode::id() const
 {
-	return QStringLiteral("org.oliveeditor.Olive.swirl");
+	return QStringLiteral("org.olivevideoeditor.Olive.swirl");
 }
 
 QVector<Node::CategoryID> SwirlDistortNode::Category() const

app/node/distort/tile/tiledistortnode.cpp

@@ -69,7 +69,7 @@ QString TileDistortNode::Name() const
 
 QString TileDistortNode::id() const
 {
-	return QStringLiteral("org.oliveeditor.Olive.tile");
+	return QStringLiteral("org.olivevideoeditor.Olive.tile");
 }
 
 QVector<Node::CategoryID> TileDistortNode::Category() const

app/node/distort/wave/wavedistortnode.cpp

@@ -54,7 +54,7 @@ QString WaveDistortNode::Name() const
 
 QString WaveDistortNode::id() const
 {
-	return QStringLiteral("org.oliveeditor.Olive.wave");
+	return QStringLiteral("org.olivevideoeditor.Olive.wave");
 }
 
 QVector<Node::CategoryID> WaveDistortNode::Category() const

app/node/factory.cpp

@@ -22,6 +22,7 @@
 #include "factory.h"
 
 #include <QCoreApplication>
+#include <QHash>
 
 #include "audio/pan/pan.h"
 #include "audio/volume/volume.h"
@@ -224,8 +225,25 @@ QString NodeFactory::GetNameFromID(const QString &id)
 
 Node *NodeFactory::CreateFromID(const QString &id)
 {
+	QString resolved_id = id;
+
+	// Node IDs renamed after older project files were written
+	static const QHash<QString, QString> kLegacyIDs = {
+		{ QStringLiteral("org.oliveeditor.Olive.flip"),
+		  QStringLiteral("org.olivevideoeditor.Olive.flip") },
+		{ QStringLiteral("org.oliveeditor.Olive.ripple"),
+		  QStringLiteral("org.olivevideoeditor.Olive.ripple") },
+		{ QStringLiteral("org.oliveeditor.Olive.swirl"),
+		  QStringLiteral("org.olivevideoeditor.Olive.swirl") },
+		{ QStringLiteral("org.oliveeditor.Olive.tile"),
+		  QStringLiteral("org.olivevideoeditor.Olive.tile") },
+		{ QStringLiteral("org.oliveeditor.Olive.wave"),
+		  QStringLiteral("org.olivevideoeditor.Olive.wave") },
+	};
+	resolved_id = kLegacyIDs.value(id, id);
+
 	foreach (Node *n, library_) {
-		if (n->id() == id) {
+		if (n->id() == resolved_id) {
 			return n->copy();
 		}
 	}

app/node/keying/chromakey/chromakey.cpp

@@ -28,9 +28,9 @@ const QString ChromaKeyNode::kColorInput = QStringLiteral("color_key");
 const QString ChromaKeyNode::kMaskOnlyInput = QStringLiteral("mask_only_in");
 const QString ChromaKeyNode::kInvertInput = QStringLiteral("invert_in");
 const QString ChromaKeyNode::kUpperToleranceInput =
-	QStringLiteral("upper_tolerence_in");
+	QStringLiteral("upper_tolerance_in");
 const QString ChromaKeyNode::kLowerToleranceInput =
-	QStringLiteral("lower_tolerence_in");
+	QStringLiteral("lower_tolerance_in");
 const QString ChromaKeyNode::kGarbageMatteInput = QStringLiteral("garbage_in");
 const QString ChromaKeyNode::kCoreMatteInput = QStringLiteral("core_in");
 const QString ChromaKeyNode::kShadowsInput = QStringLiteral("shadows_in");
@@ -163,4 +163,17 @@ void ChromaKeyNode::ConfigChanged()
 	GenerateProcessor();
 }
 
+QString ChromaKeyNode::GetInputIDForLegacyID(const QString &id) const
+{
+	// Older project files used the misspelled "tolerence" input IDs
+	if (id == QStringLiteral("upper_tolerence_in")) {
+		return kUpperToleranceInput;
+	}
+	if (id == QStringLiteral("lower_tolerence_in")) {
+		return kLowerToleranceInput;
+	}
+
+	return super::GetInputIDForLegacyID(id);
+}
+
 } // namespace olive

app/node/keying/chromakey/chromakey.h

@@ -46,6 +46,10 @@ public:
 
 	virtual void ConfigChanged() override;
 
+	// Maps the misspelled tolerance input IDs from old project files onto the
+	// corrected ones
+	virtual QString GetInputIDForLegacyID(const QString &id) const override;
+
 	static const QString kColorInput;
 	static const QString kInvertInput;
 	static const QString kMaskOnlyInput;

app/node/node.cpp

@@ -1351,6 +1351,10 @@ bool Node::Load(QXmlStreamReader *reader, SerializedData *data)
 						}
 					}
 
+					// Translate IDs renamed after older project files were
+					// written
+					param_id = GetInputIDForLegacyID(param_id);
+
 					QString output_node_id;
 
 					while (XMLReadNextStartElement(reader)) {
@@ -1573,6 +1577,11 @@ void Node::PostLoadEvent(SerializedData *data)
 	}
 }
 
+QString Node::GetInputIDForLegacyID(const QString &id) const
+{
+	return id;
+}
+
 bool Node::LoadInput(QXmlStreamReader *reader, SerializedData *data)
 {
 	if (dynamic_cast<NodeGroup *>(this)) {
@@ -1598,6 +1607,9 @@ bool Node::LoadInput(QXmlStreamReader *reader, SerializedData *data)
 		return false;
 	}
 
+	// Translate IDs renamed after older project files were written
+	param_id = GetInputIDForLegacyID(param_id);
+
 	if (!this->HasInputWithID(param_id)) {
 		qWarning() << "Failed to load parameter that didn't exist:" << param_id;
 		reader->skipCurrentElement();

app/node/node.h

@@ -1094,6 +1094,14 @@ public:
 	bool LoadInput(QXmlStreamReader *reader, SerializedData *data);
 	void SaveInput(QXmlStreamWriter *writer, const QString &id) const;
 
+	/**
+	 * @brief Maps an input ID read from an old project file to its current ID
+	 *
+	 * Nodes whose input IDs have been renamed override this so old projects
+	 * keep loading. The default implementation returns the ID unchanged.
+	 */
+	virtual QString GetInputIDForLegacyID(const QString &id) const;
+
 	bool LoadImmediate(QXmlStreamReader *reader, const QString &input,
 					   int element, SerializedData *data);
 	void SaveImmediate(QXmlStreamWriter *writer, const QString &input,

tests/gtest/node_distort_test.cpp

@@ -660,9 +660,8 @@ TEST(FlipDistortNode, MetadataIsCorrect)
 {
 	olive::FlipDistortNode node;
 	// NOTE: unlike most Olive nodes ("org.olivevideoeditor.Olive.*"), the
-	// flip/ripple/swirl/tile/wave nodes use the "org.oliveeditor.Olive.*"
 	// domain (inconsistent ID, documented here as a suspected bug)
-	EXPECT_EQ(node.id(), QStringLiteral("org.oliveeditor.Olive.flip"));
+	EXPECT_EQ(node.id(), QStringLiteral("org.olivevideoeditor.Olive.flip"));
 	EXPECT_EQ(node.Name(), QStringLiteral("Flip"));
 	EXPECT_FALSE(node.Description().isEmpty());
 	EXPECT_TRUE(node.Category().contains(olive::Node::kCategoryDistort));
@@ -1226,8 +1225,7 @@ TEST(MaskDistortNode, ValueWithFeatherNestsBlurJob)
 TEST(RippleDistortNode, MetadataIsCorrect)
 {
 	olive::RippleDistortNode node;
-	// NOTE: "org.oliveeditor.*" domain, inconsistent with most Olive nodes
+	EXPECT_EQ(node.id(), QStringLiteral("org.olivevideoeditor.Olive.ripple"));
-	EXPECT_EQ(node.id(), QStringLiteral("org.oliveeditor.Olive.ripple"));
 	EXPECT_EQ(node.Name(), QStringLiteral("Ripple"));
 	EXPECT_FALSE(node.Description().isEmpty());
 	EXPECT_TRUE(node.Category().contains(olive::Node::kCategoryDistort));
@@ -1359,8 +1357,7 @@ TEST(RippleDistortNode, ValueWithIntensityPushesShaderJob)
 TEST(SwirlDistortNode, MetadataIsCorrect)
 {
 	olive::SwirlDistortNode node;
-	// NOTE: "org.oliveeditor.*" domain, inconsistent with most Olive nodes
+	EXPECT_EQ(node.id(), QStringLiteral("org.olivevideoeditor.Olive.swirl"));
-	EXPECT_EQ(node.id(), QStringLiteral("org.oliveeditor.Olive.swirl"));
 	EXPECT_EQ(node.Name(), QStringLiteral("Swirl"));
 	EXPECT_EQ(node.Description(),
 			  QStringLiteral("Distorts an image by swirling it around a center point."));
@@ -1497,8 +1494,7 @@ TEST(SwirlDistortNode, ValueWithAngleAndRadiusPushesShaderJob)
 TEST(TileDistortNode, MetadataIsCorrect)
 {
 	olive::TileDistortNode node;
-	// NOTE: "org.oliveeditor.*" domain, inconsistent with most Olive nodes
+	EXPECT_EQ(node.id(), QStringLiteral("org.olivevideoeditor.Olive.tile"));
-	EXPECT_EQ(node.id(), QStringLiteral("org.oliveeditor.Olive.tile"));
 	EXPECT_EQ(node.Name(), QStringLiteral("Tile"));
 	EXPECT_FALSE(node.Description().isEmpty());
 	EXPECT_TRUE(node.Category().contains(olive::Node::kCategoryDistort));
@@ -1649,8 +1645,7 @@ TEST(TileDistortNode, ValueWithNonUnitScalePushesShaderJob)
 TEST(WaveDistortNode, MetadataIsCorrect)
 {
 	olive::WaveDistortNode node;
-	// NOTE: "org.oliveeditor.*" domain, inconsistent with most Olive nodes
+	EXPECT_EQ(node.id(), QStringLiteral("org.olivevideoeditor.Olive.wave"));
-	EXPECT_EQ(node.id(), QStringLiteral("org.oliveeditor.Olive.wave"));
 	EXPECT_EQ(node.Name(), QStringLiteral("Wave"));
 	EXPECT_FALSE(node.Description().isEmpty());
 	EXPECT_TRUE(node.Category().contains(olive::Node::kCategoryDistort));

tests/gtest/node_save_load_test.cpp

@@ -16,6 +16,7 @@
 #include "node/generator/solid/solid.h"
 #include "node/generator/text/textv3.h"
 #include "node/keyframe.h"
+#include "node/keying/chromakey/chromakey.h"
 #include "node/math/math/math.h"
 #include "node/node.h"
 #include "node/project.h"
@@ -617,3 +618,40 @@ TEST_F(NodeSaveLoadTest, ConnectionsLinksAndPositionsResolveAfterProjectLoad)
 
 	olive::NodeFactory::Destroy();
 }
+
+TEST_F(NodeSaveLoadTest, LegacyMisspelledChromaKeyIDsAreMapped)
+{
+	auto *src = AddNode<olive::ChromaKeyNode>();
+	src->SetStandardValue(olive::ChromaKeyNode::kUpperToleranceInput, 42.0);
+	src->SetStandardValue(olive::ChromaKeyNode::kLowerToleranceInput, 7.0);
+
+	auto *math = AddNode<olive::MathNode>();
+	olive::Node::ConnectEdge(
+		math, olive::NodeInput(src, olive::ChromaKeyNode::kUpperToleranceInput));
+
+	QString xml = SaveNodeXml(src);
+
+	// Simulate an old project file written with the misspelled "tolerence" IDs
+	xml.replace(QStringLiteral("upper_tolerance_in"),
+				QStringLiteral("upper_tolerence_in"));
+	xml.replace(QStringLiteral("lower_tolerance_in"),
+				QStringLiteral("lower_tolerence_in"));
+
+	olive::ChromaKeyNode loaded;
+	olive::SerializedData data;
+	ASSERT_TRUE(LoadNodeXml(&loaded, xml, &data));
+
+	EXPECT_DOUBLE_EQ(
+		loaded.GetStandardValue(olive::ChromaKeyNode::kUpperToleranceInput)
+			.toDouble(),
+		42.0);
+	EXPECT_DOUBLE_EQ(
+		loaded.GetStandardValue(olive::ChromaKeyNode::kLowerToleranceInput)
+			.toDouble(),
+		7.0);
+
+	// Connections to the renamed inputs are remapped too
+	ASSERT_EQ(data.desired_connections.size(), 1);
+	EXPECT_EQ(data.desired_connections.first().input.input(),
+			  olive::ChromaKeyNode::kUpperToleranceInput);
+}

tests/gtest/project_factory_test.cpp

@@ -587,3 +587,36 @@ TEST(NodeFactory, CreateMenuRestrictedToCategory)
 
 	olive::NodeFactory::Destroy();
 }
+
+TEST(NodeFactory, LegacyDistortIdsResolveToRenamedNodes)
+{
+	olive::NodeFactory::Initialize();
+
+	const QList<QPair<QString, QString>> legacy_ids = {
+		{ QStringLiteral("org.oliveeditor.Olive.flip"),
+		  QStringLiteral("org.olivevideoeditor.Olive.flip") },
+		{ QStringLiteral("org.oliveeditor.Olive.ripple"),
+		  QStringLiteral("org.olivevideoeditor.Olive.ripple") },
+		{ QStringLiteral("org.oliveeditor.Olive.swirl"),
+		  QStringLiteral("org.olivevideoeditor.Olive.swirl") },
+		{ QStringLiteral("org.oliveeditor.Olive.tile"),
+		  QStringLiteral("org.olivevideoeditor.Olive.tile") },
+		{ QStringLiteral("org.oliveeditor.Olive.wave"),
+		  QStringLiteral("org.olivevideoeditor.Olive.wave") },
+	};
+
+	for (const auto &pair : legacy_ids) {
+		std::unique_ptr<olive::Node> node(
+			olive::NodeFactory::CreateFromID(pair.first));
+		ASSERT_NE(node, nullptr) << pair.first.toStdString();
+		EXPECT_EQ(node->id(), pair.second);
+
+		// The current id resolves directly as well
+		std::unique_ptr<olive::Node> current(
+			olive::NodeFactory::CreateFromID(pair.second));
+		ASSERT_NE(current, nullptr) << pair.second.toStdString();
+		EXPECT_EQ(current->id(), pair.second);
+	}
+
+	olive::NodeFactory::Destroy();
+}