Merge branch 'master' into master-feature-renum-elements-rebased

sources/qetproject.cpp

@@ -108,6 +108,30 @@ m_project_properties_handler{this}
 		m_default_guides.append(g);
 	}
 	settings.endArray();
+
+		//Load global auto-numbering defaults from QSettings
+	{
+		auto conductorData = NumerotationContext::loadFromSettings(settings, QStringLiteral("autonum/conductor"));
+		for (auto it = conductorData.first.constBegin(); it != conductorData.first.constEnd(); ++it) {
+			addConductorAutoNum(it.key(), it.value());
+		}
+		if (!conductorData.second.isEmpty()) {
+			setCurrentConductorAutoNum(conductorData.second);
+		}
+
+		auto elementData = NumerotationContext::loadFromSettings(settings, QStringLiteral("autonum/element"));
+		for (auto it = elementData.first.constBegin(); it != elementData.first.constEnd(); ++it) {
+			addElementAutoNum(it.key(), it.value());
+		}
+		if (!elementData.second.isEmpty()) {
+			setCurrrentElementAutonum(elementData.second);
+		}
+
+		auto folioData = NumerotationContext::loadFromSettings(settings, QStringLiteral("autonum/folio"));
+		for (auto it = folioData.first.constBegin(); it != folioData.first.constEnd(); ++it) {
+			addFolioAutoNum(it.key(), it.value());
+		}
+	}
 }
 
 ProjectPropertiesHandler &QETProject::projectPropertiesHandler()
@@ -133,6 +157,9 @@ QETProject::QETProject(const QString &path, QObject *parent) :
 		return;
 	}
 
+		//The file just read already holds everything a crash could lose, so
+		//there is nothing to back up until the project is changed.
+	m_backup_needed = false;
 	init();
 }
 
@@ -185,6 +212,11 @@ QETProject::~QETProject()
 		//We block database signal to avoid hundreds of unnecessary emitted signal
 		//due to deletion (diagram, item, etc...) and as much update made in the not yet deleted things.
 	m_data_base.blockSignals(true);
+		//Same reasoning for the rebuild itself : destroying a table relinks
+		//the tables that were chained to it, which re-queries the database,
+		//which rebuilds it completely -- for a project that is on its way out.
+		//Nothing can observe the result : the database is destroyed with it.
+	m_data_base.setUpdateBlocked(true);
 
 		//Each time a diagram is deleted we also remove it from m_diagram_list
 		//because a lot of thing append during the destructor of a diagram class
@@ -199,6 +231,30 @@ QETProject::~QETProject()
 		delete  diagram;
 		m_diagrams_list.removeOne(diagram);
 	}
+
+		//A diagram can be detached from this project (detachDiagram(), used by
+		//both removeDiagram() and RemoveDiagramCommand::redo()) and scheduled
+		//for deferred deletion via deleteLater(), without that deletion having
+		//actually run yet -- deleteLater() only fires on the next event-loop
+		//iteration, and nothing guarantees one runs before this destructor
+		//does. Such a diagram is no longer in m_diagrams_list (so the loop
+		//above never touches it) but is still a QObject child of this project
+		//(Diagram's constructor passes `project` straight to QGraphicsScene's
+		//parent argument). Left alone, it is destroyed later by QObject's own
+		//automatic child cleanup in ~QObject(), which runs AFTER m_data_base
+		//(a plain value member, destroyed by ordinary C++ member teardown)
+		//has already been destroyed -- and Diagram's destructor calls back
+		//into dataBase()->removeElement() for each of its elements, so that
+		//ordering is a use-after-free (confirmed by crash: SIGSEGV in
+		//QSqlResult::exec(), called from Diagram::~Diagram() by way of
+		//Diagram::removeItem(), by way of QObjectPrivate::deleteChildren()).
+		//Delete any such stragglers now, synchronously, while m_data_base is
+		//still alive. The deleteLater() event, if it is ever processed
+		//afterward, is a safe no-op on an already-deleted QObject.
+	const auto orphaned_diagrams = findChildren<Diagram *>(QString(), Qt::FindDirectChildrenOnly);
+	for (Diagram *diagram : orphaned_diagrams) {
+		delete diagram;
+	}
 }
 
 /**
@@ -230,6 +286,19 @@ void QETProject::init()
 	m_undo_stack = new QUndoStack(this);
 	connect(m_undo_stack, &QUndoStack::cleanChanged, this, &QETProject::undoStackChanged);
 
+		//What counts as a change for writeBackup(): the undo stack moving,
+		//setModified(true), and the embedded collections, which can change
+		//without going through either.
+	const auto backup_needed = [this]() { m_backup_needed = true; };
+	connect(m_undo_stack, &QUndoStack::indexChanged, this, backup_needed);
+	connect(&m_titleblocks_collection, &TitleBlockTemplatesCollection::changed, this, backup_needed);
+	connect(&m_titleblocks_collection, &TitleBlockTemplatesCollection::aboutToRemove, this, backup_needed);
+	connect(m_elements_collection, &XmlElementCollection::elementAdded, this, backup_needed);
+	connect(m_elements_collection, &XmlElementCollection::elementChanged, this, backup_needed);
+	connect(m_elements_collection, &XmlElementCollection::elementRemoved, this, backup_needed);
+	connect(m_elements_collection, &XmlElementCollection::directorieAdded, this, backup_needed);
+	connect(m_elements_collection, &XmlElementCollection::directoryRemoved, this, backup_needed);
+
 	m_save_backup_timer.setInterval(BACKUP_INTERVAL);
 	connect(&m_save_backup_timer, &QTimer::timeout, this, &QETProject::writeBackup);
 	m_save_backup_timer.start();
@@ -251,6 +320,38 @@ void QETProject::init()
 
 }
 
+/**
+	@brief QETProject::derivedUuid
+	Name-based (version 5) uuid for a project file that has no uuid yet,
+	because it was written before the uuid was persisted.
+
+	A random uuid would do as an identity, but it would make saving an
+	unmodified legacy project non-reproducible : every load would invent a
+	different one and write it out (see #754). The uuid is therefore derived
+	from the content of the file, so the same file always yields the same
+	uuid, while two different projects practically never share one.
+	Carriage returns are dropped first, so that a checkout with CRLF line
+	endings (Windows, git autocrlf) gives the same uuid as one with LF.
+
+	It is computed once, when the file is loaded, and saved from then on :
+	editing, renaming or moving the project later does not change it.
+	@param content : the raw content of the project file
+	@return the derived uuid
+*/
+QUuid QETProject::derivedUuid(const QByteArray &content)
+{
+		//Fixed namespace for QElectroTech project uuids, never change it :
+		//doing so would change the uuid given to every legacy project.
+	static const QUuid project_namespace(
+				QStringLiteral("{c8c75719-0fea-4b1c-9f4b-2dd179fb2f0c}"));
+
+	QByteArray normalized(content);
+	normalized.replace('\r', QByteArray());
+	return QUuid::createUuidV5(project_namespace,
+							   QByteArrayLiteral("qet-project-legacy\n")
+							   + normalized);
+}
+
 /**
 	@brief QETProject::openFile
 	@param file
@@ -270,9 +371,22 @@ QETProject::ProjectState QETProject::openFile(QFile *file)
 	QFileInfo fi(*file);
 	setFilePath(fi.absoluteFilePath());
 
-		//Extract the content of the xml
+		//Extract the content of the xml. The raw bytes are kept : a project
+		//file without a persisted uuid derives its uuid from them.
+	const QByteArray content = file->readAll();
 	QDomDocument xml_project;
-	if (!xml_project.setContent(file))
+	// PreserveSpacingOnlyNodes: without it, a text node that is entirely
+	// whitespace -- e.g. a title-block custom variable deliberately set to
+	// a single space, the only way to give it a value other than blank
+	// (bugtracker #973) -- is silently dropped by Qt's default parsing,
+	// and QDomElement::text() then returns "" for it exactly as if it had
+	// never been set. Confirmed in isolation: <a> </a> parses to text()=="",
+	// this option makes it text()==" ". Every place in this codebase that
+	// walks a QDomNode's children already filters on isElement() (see
+	// QET::findInDomElement()), so the extra whitespace-only text nodes
+	// this keeps around are inert everywhere but the two elements that
+	// call .text() on themselves -- which is exactly where the bug was.
+	if (!xml_project.setContent(content, QDomDocument::ParseOption::PreserveSpacingOnlyNodes))
 	{
 		if(opened_here) {
 			file->close();
@@ -281,6 +395,17 @@ QETProject::ProjectState QETProject::openFile(QFile *file)
 	}
 	const qint64 xml_parse_ms = load_timer.elapsed();
 
+		//Restore the persisted project uuid before anything else is built
+		//from the file. The project database already got its connection name
+		//from the uuid created at construction, it does not depend on this.
+	const QDomElement root_elmt = xml_project.documentElement();
+	if (root_elmt.tagName() == QLatin1String("project"))
+	{
+		const QUuid persisted_uuid(root_elmt.attribute(QStringLiteral("uuid")));
+		m_uuid = persisted_uuid.isNull() ? derivedUuid(content)
+										 : persisted_uuid;
+	}
+
 		//Build the project from the xml
 	readProjectXml(xml_project);
 
@@ -485,7 +610,9 @@ QString QETProject::pathNameTitle() const
 			)
 		).arg(final_title);
 	}
-	if (m_modified) {
+	// Same condition as projectWasModified(): project-options changeg (m_modified) OR the undo stack sitting away from
+	// its clean index. 
+	if (m_modified || !m_undo_stack->isClean()) {
 		final_title = QString(
 			tr(
 				"%1 [modifié]",
@@ -1137,12 +1264,24 @@ QDomDocument QETProject::toXml()
 		setTitle(QFileInfo(m_file_path).completeBaseName());
 	}
 	project_root.setAttribute("title", project_title_);
+		//Persist the project identity, so that the project keeps the same
+		//uuid across save/load. Without it every load invents a new one, and
+		//nothing outside the running instance (version control, a cloud or
+		//key-value store, a lock...) can tell which project a file belongs to.
+	project_root.setAttribute(QStringLiteral("uuid"), m_uuid.toString());
 	xml_doc.appendChild(project_root);
 
 	// titleblock templates, if any
 	if (m_titleblocks_collection.templates().count()) {
 		QDomElement titleblocktemplates_elmt = xml_doc.createElement("titleblocktemplates");
-		foreach (QString template_name, m_titleblocks_collection.templates()) {
+			//Sorted, because templates() returns QHash::keys() and Qt
+			//randomises hash order per process. Writing them unsorted put the
+			//<titleblocktemplate> children in a different order on every save,
+			//which is what made a project holding more than one template save
+			//irreproducibly.
+		QStringList template_names = m_titleblocks_collection.templates();
+		template_names.sort();
+		for (const QString &template_name : std::as_const(template_names)) {
 			QDomElement e = m_titleblocks_collection.getTemplateXmlDescription(template_name);
 			titleblocktemplates_elmt.appendChild(xml_doc.importNode(e, true));
 		}
@@ -1356,7 +1495,7 @@ ElementsLocation QETProject::importElement(ElementsLocation &location)
 				// Warn if the new element introduces slave contact groups
 				QDomElement new_kind = location.xml().firstChildElement("kindInformations");
 				if (!new_kind.firstChildElement("slaveContactGroups").isNull()) {
-					QMessageBox::StandardButton answer = QMessageBox::warning(nullptr,
+					QMessageBox::StandardButton answer = QET::QetMessageBox::warning(nullptr,
 						tr("Système de contacts modifié"),
 						tr("Le nouvel élément définit des groupes de contacts esclaves.\n"
 						   "Les éléments esclaves existants ne seront pas automatiquement "
@@ -1569,6 +1708,9 @@ void QETProject::diagramOrderChanged(int old_index, int new_index) {
 	Mark this project as modified and emit the projectModified() signal.
 */
 void QETProject::setModified(bool modified) {
+	if (modified) {
+		m_backup_needed = true;
+	}
 	if (m_modified != modified) {
 		m_modified = modified;
 		emit projectModified(this, m_modified);
@@ -1651,6 +1793,11 @@ void QETProject::readProjectXml(QDomDocument &xml_project)
 	}
 
 	m_data_base.blockSignals(true);
+		//Blocking the signals is not enough : every table model built below
+		//re-queries the database, and each of those queries used to trigger a
+		//complete rebuild of it. The content being loaded is the same for all
+		//of them, so a single rebuild once everything is in place is enough.
+	m_data_base.setUpdateBlocked(true);
 
 		//Load the project-wide properties
 	readProjectPropertiesXml(xml_project);
@@ -1690,6 +1837,7 @@ void QETProject::readProjectXml(QDomDocument &xml_project)
 	const qint64 refresh_ms = phase_timer.restart();
 
 	m_data_base.blockSignals(false);
+	m_data_base.setUpdateBlocked(false);
 	m_data_base.updateDB();
 	const qint64 database_ms = phase_timer.elapsed();
 
@@ -2003,7 +2151,14 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 
 		// export default XRef properties
 	QDomElement xrefs_elmt = xml_document.createElement("xrefs");
-	for (QString key : defaultXRefProperties().keys())
+		//Sorted, because defaultXRefProperties() is a QHash and its key order
+		//is randomised per process. Writing it unsorted made two saves of an
+		//unchanged project differ only in the order of these <xref> children,
+		//so a save was not reproducible and diffing two saved files showed
+		//spurious changes.
+	QStringList xref_keys = defaultXRefProperties().keys();
+	xref_keys.sort();
+	for (QString &key : xref_keys)
 	{
 		auto xrp = defaultXRefProperties(key);
 		xrp.setKey(key);
@@ -2017,7 +2172,12 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 	conductor_autonums.setAttribute("current_autonum", m_current_conductor_autonum);
 	conductor_autonums.setAttribute("freeze_new_conductors", m_freeze_new_conductors ? "true" : "false");
 	conductor_autonums.setAttribute("auto_break_conductors", m_auto_break_conductor ? "true" : "false");
-	foreach (QString key, conductorAutoNum().keys()) {
+		//Sorted for the same reason as the xrefs above: these three
+		//collections are QHash, whose key order is randomised per process,
+		//so an unsorted write reorders these children on every save.
+	QStringList conductor_autonum_keys = conductorAutoNum().keys();
+	conductor_autonum_keys.sort();
+	for (const QString &key : std::as_const(conductor_autonum_keys)) {
 	QDomElement conductor_autonum = conductorAutoNum(key).toXml(xml_document, "conductor_autonum");
 		if (key != "" && conductorAutoNumFormula(key) != "") {
 			conductor_autonum.setAttribute("title", key);
@@ -2029,7 +2189,9 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 
 	//Export Folio Autonums
 	QDomElement folio_autonums = xml_document.createElement("folio_autonums");
-	foreach (QString key, folioAutoNum().keys()) {
+	QStringList folio_autonum_keys = folioAutoNum().keys();
+	folio_autonum_keys.sort();
+	for (const QString &key : std::as_const(folio_autonum_keys)) {
 	QDomElement folio_autonum = folioAutoNum(key).toXml(xml_document, "folio_autonum");
 		folio_autonum.setAttribute("title", key);
 		folio_autonums.appendChild(folio_autonum);
@@ -2040,7 +2202,9 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 	QDomElement element_autonums = xml_document.createElement("element_autonums");
 	element_autonums.setAttribute("current_autonum", m_current_element_autonum);
 	element_autonums.setAttribute("freeze_new_elements", m_freeze_new_elements ? "true" : "false");
-	foreach (QString key, elementAutoNum().keys()) {
+	QStringList element_autonum_keys = elementAutoNum().keys();
+	element_autonum_keys.sort();
+	for (const QString &key : std::as_const(element_autonum_keys)) {
 	QDomElement element_autonum = elementAutoNum(key).toXml(xml_document, "element_autonum");
 		if (key != "" && elementAutoNumFormula(key) != "") {
 			element_autonum.setAttribute("title", key);
@@ -2129,6 +2293,12 @@ void QETProject::writeBackup()
 		//both would write through &m_backup_file on different threads.
 	if (m_backup_future.isRunning())
 		return;
+		//toXml() walks the whole project on the GUI thread, which freezes
+		//big projects for seconds (bugtracker #273, #329). A backup of an
+		//unchanged project would be identical to the last one, so skip it.
+	if (!m_backup_needed)
+		return;
+	m_backup_needed = false;
 		//Capture the document by value (implicitly shared, so cheap): the
 		//Qt5-style QtConcurrent::run(function, reference-args) call did not
 		//survive the Qt6 API change, a lambda behaves identically on both.