Give symbols saved without a uuid the same one on every load

A symbol saved without a uuid got a random one from Element::fromXml()
on every load, and the next save wrote it out: two loads of the same file
gave the same symbol two identities, and anything pointing at it by uuid
(a script, a comparison of two versions, a wire's identity) could not
follow it from one session to the next.

When a folio is loaded, such a symbol now gets a UUID v5 derived from
what it is and where it sits: its type, its position on the folio and its
orientation. Never the folio's index, so inserting or moving a folio does
not change it. Identical symbols stacked on one spot, or a copied folio,
are told apart by a counter kept per project (QETProject::derivedUuid()),
in load order among those symbols alone. A paste still renews uuids.

Symbols that have a uuid in the file keep it. All 24 example projects
already have one for every symbol, so they are unchanged; with the
symbols' uuids stripped, each saves byte-for-byte the same twice (master:
different every time).

tst_derivedsymboluuid runs --resave on a fixture with its uuids stripped:
same uuids on every load, same after a folio is inserted in front, saved
uuids kept, stacked copies differ. The first two fail without this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sources/diagram.cpp

@@ -1676,6 +1676,23 @@ bool Diagram::fromXml(QDomElement &document,
 			delete nvel_elmt;
 			qDebug() << QStringLiteral("Diagram::fromXml() : Le chargement des parametres d'un element a echoue");
 		} else {
+				//A symbol saved without a uuid got a random one from
+				//Element::fromXml(): a different identity on every load,
+				//written out on the next save. Derive it instead from what
+				//the symbol is and where it sits on its folio -- never from
+				//the folio's index, so inserting or moving a folio does not
+				//change it. Only for a folio being loaded: a paste renews
+				//uuids anyway.
+			if (consider_informations && m_project
+				&& QUuid(element_xml.attribute(QStringLiteral("uuid"))).isNull()) {
+				nvel_elmt->setUuid(m_project->derivedUuid(
+									   QStringLiteral("element"),
+									   QStringList{type_id,
+												   element_xml.attribute(QStringLiteral("x")),
+												   element_xml.attribute(QStringLiteral("y")),
+												   element_xml.attribute(QStringLiteral("orientation"))}
+									   .join(QLatin1Char('\n'))));
+			}
 			ItemGroups::setGroup(nvel_elmt, ItemGroups::read(element_xml));
 			added_elements << nvel_elmt;
 		}

sources/qetgraphicsitem/element.h

@@ -245,6 +245,7 @@ class Element : public QetGraphicsItem
 		QString linkTypeToString() const;
 
 		void newUuid() {m_uuid = QUuid::createUuid();} 	//create new uuid for this element
+		void setUuid(const QUuid &uuid) {m_uuid = uuid;}
 
 	protected:
 		void drawAxes(QPainter *, const QStyleOptionGraphicsItem *);

sources/qetproject.cpp

@@ -275,6 +275,27 @@ QUuid QETProject::uuid() const
 	return m_uuid;
 }
 
+/**
+	@brief QETProject::derivedUuid
+	A uuid for an item of this project that was saved without one, the same
+	on every load of the same file.
+	@p key describes the item by what it is, never by its place in the file
+	or its folio's index: inserting or moving a folio must not change it.
+	Items with the same @p kind and @p key anywhere in the project (a copied
+	folio, two identical symbols stacked on one spot) are told apart by a
+	counter, in load order among those items alone.
+	@return a UUID v5, which cannot collide with the v4 uuids given to new
+	items
+*/
+QUuid QETProject::derivedUuid(const QString &kind, const QString &key)
+{
+	static const QUuid derived_ns(QStringLiteral("{7d1e9c3a-5b2f-4e8a-9c61-2f4b8d0e6a17}"));
+	const QString full = kind + QLatin1Char('\n') + key;
+	const int n = m_derived_uuid_keys[full]++;
+	return QUuid::createUuidV5(derived_ns,
+							   n ? full + QLatin1Char('\n') + QString::number(n) : full);
+}
+
 /**
 	@brief QETProject::init
 */

sources/qetproject.h

@@ -107,6 +107,7 @@ class QETProject : public QObject
 		ProjectPropertiesHandler& projectPropertiesHandler();
 		projectDataBase *dataBase();
 		QUuid uuid() const;
+		QUuid derivedUuid(const QString &kind, const QString &key);
 		ProjectState state() const;
 		QList<Diagram *> diagrams() const;
 		int folioIndex(const Diagram *) const;
@@ -365,6 +366,7 @@ class QETProject : public QObject
 		QFuture<bool> m_backup_future;
 		KAutoSaveFile m_backup_file;
 		QUuid m_uuid = QUuid::createUuid();
+		QHash<QString, int> m_derived_uuid_keys;
 		projectDataBase m_data_base;
 		QVector<TerminalStrip *> m_terminal_strip_vector;
 

tests/qttest/CMakeLists.txt

@@ -328,3 +328,15 @@ target_include_directories(tst_conductorselfretrace PRIVATE ${QET_DIR}/sources)
 target_link_libraries(tst_conductorselfretrace PRIVATE Qt::Test)
 target_compile_definitions(tst_conductorselfretrace PRIVATE
   "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\"")
+
+# A symbol saved without a uuid gets the same one on every load, and
+# inserting a folio does not change it. Runs the real binary's --resave on
+# fixtures/qet_bug_repro_resaved.qet with the symbols' uuids stripped.
+add_executable(
+  tst_derivedsymboluuid
+  tst_derivedsymboluuid.cpp)
+add_test(NAME tst_derivedsymboluuid COMMAND tst_derivedsymboluuid)
+add_dependencies(tst_derivedsymboluuid qelectrotech)
+target_link_libraries(tst_derivedsymboluuid PRIVATE Qt::Test Qt::Xml)
+target_compile_definitions(tst_derivedsymboluuid PRIVATE
+  "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\"")

tests/qttest/tst_derivedsymboluuid.cpp

@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <QtTest>
+
+#include <QDomDocument>
+#include <QFile>
+#include <QHash>
+#include <QProcess>
+#include <QProcessEnvironment>
+#include <QSet>
+#include <QTemporaryDir>
+#include <QUuid>
+
+// A symbol saved without a uuid must get the same one on every load of the
+// same file, and inserting a folio in front of it must not change it. Until
+// this was fixed it got a random uuid each time (Element::fromXml), which
+// the next save wrote out.
+//
+// Runs the real binary (--resave) on the fixture with its symbols' uuids
+// stripped, and reads the uuids back from the saved file.
+namespace {
+
+QDomDocument load(const QString &path)
+{
+	QDomDocument doc;
+	QFile file(path);
+	if (file.open(QIODevice::ReadOnly))
+		doc.setContent(&file);
+	return doc;
+}
+
+QList<QDomElement> symbols(const QDomElement &diagram)
+{
+	QList<QDomElement> out;
+	const QDomNodeList nodes = diagram.firstChildElement(QStringLiteral("elements"))
+			.elementsByTagName(QStringLiteral("element"));
+	for (int i = 0; i < nodes.size(); ++i) {
+		const QDomElement e = nodes.at(i).toElement();
+		if (e.parentNode().parentNode() == diagram)
+			out << e;
+	}
+	return out;
+}
+
+QList<QDomElement> diagrams(const QDomDocument &doc)
+{
+	QList<QDomElement> out;
+	for (QDomElement d = doc.documentElement().firstChildElement(QStringLiteral("diagram"));
+		 !d.isNull(); d = d.nextSiblingElement(QStringLiteral("diagram")))
+		out << d;
+	return out;
+}
+
+// Symbols without uuids, and no wires (they refer to symbols by uuid).
+void stripUuids(QDomDocument &doc)
+{
+	for (QDomElement d : diagrams(doc)) {
+		for (QDomElement e : symbols(d))
+			e.removeAttribute(QStringLiteral("uuid"));
+		d.removeChild(d.firstChildElement(QStringLiteral("conductors")));
+	}
+}
+
+// type|x|y|orientation -> uuid, for every symbol in the file
+QMultiHash<QString, QString> symbolUuids(const QDomDocument &doc)
+{
+	QMultiHash<QString, QString> out;
+	for (const QDomElement &d : diagrams(doc))
+		for (const QDomElement &e : symbols(d))
+			out.insert(QStringList{e.attribute(QStringLiteral("type")),
+								   e.attribute(QStringLiteral("x")),
+								   e.attribute(QStringLiteral("y")),
+								   e.attribute(QStringLiteral("orientation"))}
+					   .join(QLatin1Char('|')),
+					   e.attribute(QStringLiteral("uuid")));
+	return out;
+}
+
+} // namespace
+
+class tst_derivedsymboluuid : public QObject
+{
+	Q_OBJECT
+
+	QTemporaryDir m_dir;
+	int m_run = 0;
+
+	// Save @p doc, run --resave on it in a sandbox of its own (so a running
+	// QElectroTech cannot answer instead), and return what was saved.
+	QDomDocument resave(const QDomDocument &doc)
+	{
+		const QString in = m_dir.filePath(QStringLiteral("in%1.qet").arg(m_run));
+		const QString out = m_dir.filePath(QStringLiteral("out%1.qet").arg(m_run));
+		const QString home = m_dir.filePath(QStringLiteral("home%1").arg(m_run++));
+		QDir().mkpath(home);
+		QFile f(in);
+		if (!f.open(QIODevice::WriteOnly)) return {};
+		f.write(doc.toByteArray());
+		f.close();
+
+		QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+		env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen"));
+		env.insert(QStringLiteral("HOME"), home);
+		env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config"));
+		env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data"));
+		QProcess proc;
+		proc.setProcessEnvironment(env);
+		proc.start(QStringLiteral(QET_TEST_BINARY_PATH),
+				   {QStringLiteral("--resave"), in, out});
+		if (!proc.waitForFinished(60000) || proc.exitCode() != 0) return {};
+		return load(out);
+	}
+
+	QDomDocument fixture()
+	{
+		QDomDocument doc = load(QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet"));
+		stripUuids(doc);
+		return doc;
+	}
+
+private slots:
+	void initTestCase()
+	{
+		QVERIFY(m_dir.isValid());
+		QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH)));
+		QVERIFY(!fixture().isNull());
+	}
+
+	void sameUuidsOnEveryLoad()
+	{
+		const QMultiHash<QString, QString> a = symbolUuids(resave(fixture()));
+		const QMultiHash<QString, QString> b = symbolUuids(resave(fixture()));
+		QCOMPARE(a.size(), 9);      // the fixture's placed symbols
+		QCOMPARE(a, b);
+		for (const QString &u : a)
+			QVERIFY2(!QUuid(u).isNull(), qPrintable(u));
+		QCOMPARE(QSet<QString>(a.begin(), a.end()).size(), a.size());
+	}
+
+	void insertingAFolioChangesNothing()
+	{
+		QDomDocument moved = fixture();
+		QDomElement first = diagrams(moved).first();
+		QDomElement blank = first.cloneNode(false).toElement();
+		blank.setAttribute(QStringLiteral("title"), QStringLiteral("new"));
+		blank.appendChild(moved.createElement(QStringLiteral("elements")));
+		moved.documentElement().insertBefore(blank, first);
+
+		const QMultiHash<QString, QString> after = symbolUuids(resave(moved));
+		QCOMPARE(after.size(), 9);
+		QCOMPARE(after, symbolUuids(resave(fixture())));
+	}
+
+	void savedUuidsAreKept()
+	{
+		QDomDocument doc = load(QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet"));
+		QCOMPARE(symbolUuids(resave(doc)), symbolUuids(doc));
+	}
+
+	void stackedIdenticalSymbolsDiffer()
+	{
+		QDomDocument doc = fixture();
+		QDomElement one = symbols(diagrams(doc).first()).first();
+		QDomElement copy = one.cloneNode(true).toElement();
+			//A copy's terminals carry their own file ids; a symbol whose
+			//terminal ids are already taken is not loaded at all.
+		const QDomNodeList terminals = copy.elementsByTagName(QStringLiteral("terminal"));
+		for (int i = 0; i < terminals.size(); ++i)
+			terminals.at(i).toElement().setAttribute(QStringLiteral("id"), 90000 + i);
+		one.parentNode().appendChild(copy);
+
+		const QStringList both = symbolUuids(resave(doc)).values(
+					QStringList{one.attribute(QStringLiteral("type")),
+								one.attribute(QStringLiteral("x")),
+								one.attribute(QStringLiteral("y")),
+								one.attribute(QStringLiteral("orientation"))}
+					.join(QLatin1Char('|')));
+		QCOMPARE(both.size(), 2);
+		QVERIFY(both.at(0) != both.at(1));
+	}
+};
+
+QTEST_APPLESS_MAIN(tst_derivedsymboluuid)
+
+#include "tst_derivedsymboluuid.moc"