diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 4831b6484..bc0cd7c9e 100644 --- a/sources/diagram.cpp +++ b/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; } diff --git a/sources/qetgraphicsitem/element.h b/sources/qetgraphicsitem/element.h index 9bb02202d..33d8b752e 100644 --- a/sources/qetgraphicsitem/element.h +++ b/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 *); diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index df45c2c08..e66714577 100644 --- a/sources/qetproject.cpp +++ b/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 */ diff --git a/sources/qetproject.h b/sources/qetproject.h index e05f3b216..991b9f1a9 100644 --- a/sources/qetproject.h +++ b/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 diagrams() const; int folioIndex(const Diagram *) const; @@ -365,6 +366,7 @@ class QETProject : public QObject QFuture m_backup_future; KAutoSaveFile m_backup_file; QUuid m_uuid = QUuid::createUuid(); + QHash m_derived_uuid_keys; projectDataBase m_data_base; QVector m_terminal_strip_vector; diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 94283cc09..52d156547 100644 --- a/tests/qttest/CMakeLists.txt +++ b/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=\"$\"") + +# 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=\"$\"") diff --git a/tests/qttest/tst_derivedsymboluuid.cpp b/tests/qttest/tst_derivedsymboluuid.cpp new file mode 100644 index 000000000..d3fa5861a --- /dev/null +++ b/tests/qttest/tst_derivedsymboluuid.cpp @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// 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 symbols(const QDomElement &diagram) +{ + QList 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 diagrams(const QDomDocument &doc) +{ + QList 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 symbolUuids(const QDomDocument &doc) +{ + QMultiHash 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 a = symbolUuids(resave(fixture())); + const QMultiHash 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(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 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"