Merge pull request #1523 from ispyisail/refactor/link-rows-in-place

Fix the project database listing a symbol's links in another order than its file

sources/dataBase/projectdatabase.cpp

@@ -847,13 +847,15 @@ bool projectDataBase::populateFromDocument(const QDomDocument &document, QString
 		}
 	}
 
-	query.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index) "
-								 "VALUES (:element_uuid, :linked_uuid, :group_index)"));
+	query.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index, ord) "
+								 "VALUES (:element_uuid, :linked_uuid, :group_index, :ord)"));
 	for (const DocumentElement &element : std::as_const(elements)) {
+		int ord = 0;
 		for (const auto &link : element.links) {
 			query.bindValue(QStringLiteral(":element_uuid"), element.uuid);
 			query.bindValue(QStringLiteral(":linked_uuid"), link.first);
 			query.bindValue(QStringLiteral(":group_index"), link.second >= 0 ? QVariant(link.second) : QVariant());
+			query.bindValue(QStringLiteral(":ord"), ord++);
 			if (!query.exec()) {
 				qDebug() << "projectDataBase::populateFromDocument link insert error : " << query.lastError();
 			}
@@ -1092,7 +1094,7 @@ QList<QUuid> projectDataBase::linkedSymbols(const QUuid &element) const
 {
 	QList<QUuid> linked;
 	QSqlQuery query(m_data_base);
-	query.prepare(QStringLiteral("SELECT linked_uuid FROM link WHERE element_uuid = :uuid ORDER BY rowid"));
+	query.prepare(QStringLiteral("SELECT linked_uuid FROM link WHERE element_uuid = :uuid ORDER BY ord, rowid"));
 	query.bindValue(QStringLiteral(":uuid"), element.toString());
 	if (query.exec()) {
 		while (query.next())
@@ -2602,12 +2604,14 @@ bool projectDataBase::createDataBase()
 
 		//Create the link table: one row per element and element it is
 		//linked to -- a coil and its contacts, a pair of folio reports --
-		//from each side, with the contact group the element saved for it.
+		//from each side, with the contact group the element saved for it,
+		//and its place (ord) in the order the element saves its links.
 	const QString link_table(QStringLiteral(
 			"CREATE TABLE link ("
 			"element_uuid VARCHAR(50) NOT NULL, "
 			"linked_uuid VARCHAR(50) NOT NULL, "
 			"group_index INTEGER, "
+			"ord INTEGER NOT NULL DEFAULT 0, "
 			"PRIMARY KEY (element_uuid, linked_uuid), "
 			"FOREIGN KEY (element_uuid) REFERENCES element (uuid))"));
 	if (!query_.exec(link_table)) {
@@ -3868,10 +3872,11 @@ QString projectDataBase::prefillReport() const
 void projectDataBase::populateLinkTable()
 {
 	m_dirty_link_elements.clear();
+	const QSet<QUuid> waiting = waitingSymbols();
 	QSqlQuery query(m_data_base);
 	query.exec(QStringLiteral("DELETE FROM link"));
-	query.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index) "
-								 "VALUES (:element_uuid, :linked_uuid, :group_index)"));
+	query.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index, ord) "
+								 "VALUES (:element_uuid, :linked_uuid, :group_index, :ord)"));
 
 	for (const auto &diagram : m_project->folios())
 	{
@@ -3885,23 +3890,7 @@ void projectDataBase::populateLinkTable()
 		}
 		const ElementProvider ep(diagram);
 		for (const auto &elmt : ep.find(allElementTypes()))
-		{
-			QList<std::pair<QString, int>> rows;
-			for (Element *linked : elmt->linkedElements())
-				rows.append({linked->uuid().toString(), elmt->groupIndexForElement(linked)});
-				//Partners on a folio not built, which it will be linked to
-			for (const auto &waiting : elmt->waitingLinks())
-				rows.append({waiting.first.toString(), waiting.second});
-			for (const auto &row : std::as_const(rows))
-			{
-				query.bindValue(QStringLiteral(":element_uuid"), elmt->uuid().toString());
-				query.bindValue(QStringLiteral(":linked_uuid"), row.first);
-				query.bindValue(QStringLiteral(":group_index"), row.second >= 0 ? QVariant(row.second) : QVariant());
-				if (!query.exec()) {
-					qDebug() << "projectDataBase::populateLinkTable insert error : " << query.lastError();
-				}
-			}
-		}
+			insertLinkRows(query, elmt, waiting);
 	}
 }
 
@@ -3927,6 +3916,83 @@ void projectDataBase::queueLinks(Element *element)
 	}
 }
 
+/**
+	@brief projectDataBase::linkRowMismatches
+	A test and debugging aid (QET_CHECK_LINK_ROWS): the placed
+	symbols whose link rows are not the links they save, in order
+	(Element::linksToSave(), insertLinkRows())
+	@return one entry per such symbol: its uuid
+*/
+QStringList projectDataBase::linkRowMismatches()
+{
+	flushLinks();
+	const QSet<QUuid> waiting = waitingSymbols();
+	QStringList mismatches;
+	QSqlQuery query(m_data_base);
+	query.prepare(QStringLiteral("SELECT linked_uuid, group_index FROM link "
+								 "WHERE element_uuid = :uuid ORDER BY ord, rowid"));
+	for (auto it = m_placed_elements.cbegin() ; it != m_placed_elements.cend() ; ++it)
+	{
+		Element *element = it.value();
+		if (!element) {
+			continue;
+		}
+		QList<std::pair<QString, int>> expected;
+		for (const Element::SavedLink &link : element->linksToSave(waiting))
+			if (!link.built || m_placed_elements.contains(link.uuid, link.built))
+				expected.append({link.uuid.toString(), link.group_index});
+		QList<std::pair<QString, int>> rows;
+		query.bindValue(QStringLiteral(":uuid"), element->uuid().toString());
+		if (query.exec())
+			while (query.next())
+				rows.append({query.value(0).toString(),
+							 query.value(1).isNull() ? -1 : query.value(1).toInt()});
+		if (rows != expected) {
+			mismatches << element->uuid().toString();
+		}
+	}
+	return mismatches;
+}
+
+/**
+	@brief projectDataBase::waitingSymbols
+	@return the symbols on folios not built yet (lazy folios): partners a
+	symbol keeps its saved links to until they are built
+*/
+QSet<QUuid> projectDataBase::waitingSymbols() const
+{
+	return m_project && m_project->unloadedFolioCount() ? m_project->symbolsOnUnbuiltFolios()
+														: QSet<QUuid>();
+}
+
+/**
+	@brief projectDataBase::insertLinkRows
+	Write @p element's link rows with @p insert (prepared with :element_uuid,
+	:linked_uuid, :group_index and :ord): its partners as it saves them
+	(Element::linksToSave()), in that order, so the rows and the file say
+	the same: @p waiting is the symbols on folios not built, as a save
+	takes them (waitingSymbols()). A built partner the project no longer has (on a removed
+	folio, linked still for the undo) gets no row.
+*/
+void projectDataBase::insertLinkRows(QSqlQuery &insert, Element *element,
+									 const QSet<QUuid> &waiting)
+{
+	int ord = 0;
+	for (const Element::SavedLink &link : element->linksToSave(waiting))
+	{
+		if (link.built && !m_placed_elements.contains(link.uuid, link.built)) {
+			continue;
+		}
+		insert.bindValue(QStringLiteral(":element_uuid"), element->uuid().toString());
+		insert.bindValue(QStringLiteral(":linked_uuid"), link.uuid.toString());
+		insert.bindValue(QStringLiteral(":group_index"), link.group_index >= 0 ? QVariant(link.group_index) : QVariant());
+		insert.bindValue(QStringLiteral(":ord"), ord++);
+		if (!insert.exec()) {
+			qDebug() << "projectDataBase::insertLinkRows insert error : " << insert.lastError();
+		}
+	}
+}
+
 /**
 	@brief projectDataBase::flushLinks
 	Write the link rows of every element queued by linksChanged()
@@ -3939,12 +4005,13 @@ void projectDataBase::flushLinks()
 
 	const auto dirty = m_dirty_link_elements;
 	m_dirty_link_elements.clear();
+	const QSet<QUuid> waiting = waitingSymbols();
 	const bool own_transaction = m_data_base.transaction();
 	QSqlQuery remove(m_data_base);
 	remove.prepare(QStringLiteral("DELETE FROM link WHERE element_uuid = :uuid"));
 	QSqlQuery insert(m_data_base);
-	insert.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index) "
-								  "VALUES (:element_uuid, :linked_uuid, :group_index)"));
+	insert.prepare(QStringLiteral("INSERT INTO link (element_uuid, linked_uuid, group_index, ord) "
+								  "VALUES (:element_uuid, :linked_uuid, :group_index, :ord)"));
 	for (const QPointer<Element> &element : dirty)
 	{
 		if (!element) {
@@ -3960,32 +4027,7 @@ void projectDataBase::flushLinks()
 		if (!m_placed_elements.contains(element->uuid(), element)) {
 			continue;
 		}
-		for (Element *linked : element->linkedElements())
-		{
-				//A symbol on a removed folio stays linked, for the undo,
-				//but the project no longer has it
-			if (!m_placed_elements.contains(linked->uuid(), linked)) {
-				continue;
-			}
-			const int group = element->groupIndexForElement(linked);
-			insert.bindValue(QStringLiteral(":element_uuid"), element->uuid().toString());
-			insert.bindValue(QStringLiteral(":linked_uuid"), linked->uuid().toString());
-			insert.bindValue(QStringLiteral(":group_index"), group >= 0 ? QVariant(group) : QVariant());
-			if (!insert.exec()) {
-				qDebug() << "projectDataBase::flushLinks insert error : " << insert.lastError();
-			}
-		}
-			//Partners on a folio not built (lazy folios), which it will
-			//be linked to once that folio is
-		for (const auto &waiting : element->waitingLinks())
-		{
-			insert.bindValue(QStringLiteral(":element_uuid"), element->uuid().toString());
-			insert.bindValue(QStringLiteral(":linked_uuid"), waiting.first.toString());
-			insert.bindValue(QStringLiteral(":group_index"), waiting.second >= 0 ? QVariant(waiting.second) : QVariant());
-			if (!insert.exec()) {
-				qDebug() << "projectDataBase::flushLinks insert error : " << insert.lastError();
-			}
-		}
+		insertLinkRows(insert, element, waiting);
 	}
 	if (own_transaction) {
 		m_data_base.commit();

sources/dataBase/projectdatabase.h

@@ -154,6 +154,7 @@ class projectDataBase : public QObject
 		void conductorPropertiesStored(Conductor *conductor, const ConductorProperties &properties);
 		void conductorUuidChanged(Conductor *conductor, const QUuid &old_uuid);
 		QStringList conductorPropertiesMismatches() const;
+		QStringList linkRowMismatches();
 
 			//Fill the three stores from a project's document before its
 			//folios are built, then, once
@@ -253,6 +254,8 @@ class projectDataBase : public QObject
 		void populateConductorTable();
 		void populateLinkTable();
 		void flushLinks();
+		void insertLinkRows(QSqlQuery &insert, Element *element, const QSet<QUuid> &waiting);
+		QSet<QUuid> waitingSymbols() const;
 		void populateDrawingItemTables();
 		bool populateFromDocument(const QDomDocument &document, QString *why = nullptr);
 		bool writeDrawingItem(QObject *object);

sources/qetgraphicsitem/element.cpp

@@ -1437,32 +1437,11 @@ QDomElement Element::toXml(
 	element.appendChild(inputs);
 
 	//if this element is linked to other elements,
-	//save the uuid of each other elements
-		//A partner on a folio not built yet (lazy folios), which this
-		//symbol will be linked to, is saved too, in the order the symbol was
-		//saved with: saving needs no partner built (stage D5a)
+	//save the uuid of each other elements (linksToSave())
 	QList<std::pair<QUuid, int>> links;
-	if (keepsSavedLinks() && diagram() && diagram()->project()) {
-		const QSet<QUuid> waiting = diagram()->project()->waitingLinkTargets();
-		QSet<Element *> written;
-		for (const auto &link_info : std::as_const(tmp_uuids_link)) {
-			Element *linked = nullptr;
-			for (Element *elmt : std::as_const(connected_elements))
-				if (elmt->uuid() == link_info.uuid && !written.contains(elmt)) { linked = elmt; break; }
-			if (linked) {
-				written.insert(linked);
-				links.append({linked->uuid(), m_group_index_map.value(linked, -1)});
-			} else if (waiting.contains(link_info.uuid)) {
-				links.append({link_info.uuid, link_info.group_index});
-			}
-		}
-		for (Element *elmt : std::as_const(connected_elements))
-			if (!written.contains(elmt))
-				links.append({elmt->uuid(), m_group_index_map.value(elmt, -1)});
-	} else {
-		for (Element *elmt : std::as_const(connected_elements))
-			links.append({elmt->uuid(), m_group_index_map.value(elmt, -1)});
-	}
+	for (const SavedLink &link : linksToSave(diagram() && diagram()->project()
+											 ? diagram()->project()->waitingLinkTargets() : QSet<QUuid>()))
+		links.append({link.uuid, link.group_index});
 	if (!links.isEmpty()) {
 		QDomElement links_uuids = document.createElement(QStringLiteral("links_uuids"));
 		for (const auto &link : std::as_const(links)) {
@@ -1869,6 +1848,45 @@ void Element::initLink(QETProject *prj)
 	}
 }
 
+/**
+	@brief Element::linksToSave
+	The partners this symbol saves, in the order it saves them: the order
+	it was saved with (tmp_uuids_link), then partners linked since. A
+	partner on a folio not built yet (lazy folios), which this symbol will
+	be linked to, is in the list too: saving needs no partner built (stage
+	D5a). toXml() writes this list, and the project database's link rows
+	are written from it, so the two are the same (QETZ-DB-FIRST stage 2a).
+	@param waiting the symbols on folios not built: a saved partner among
+	them is kept (QETProject::symbolsOnUnbuiltFolios())
+	@return each partner's uuid, its contact group (-1 if none), and the
+	partner itself when it is built (nullptr for one waiting)
+*/
+QList<Element::SavedLink> Element::linksToSave(const QSet<QUuid> &waiting) const
+{
+	QList<SavedLink> links;
+	if (keepsSavedLinks() && diagram() && diagram()->project()) {
+		QSet<Element *> written;
+		for (const auto &link_info : std::as_const(tmp_uuids_link)) {
+			Element *linked = nullptr;
+			for (Element *elmt : std::as_const(connected_elements))
+				if (elmt->uuid() == link_info.uuid && !written.contains(elmt)) { linked = elmt; break; }
+			if (linked) {
+				written.insert(linked);
+				links.append({linked->uuid(), m_group_index_map.value(linked, -1), linked});
+			} else if (waiting.contains(link_info.uuid)) {
+				links.append({link_info.uuid, link_info.group_index, nullptr});
+			}
+		}
+		for (Element *elmt : std::as_const(connected_elements))
+			if (!written.contains(elmt))
+				links.append({elmt->uuid(), m_group_index_map.value(elmt, -1), elmt});
+	} else {
+		for (Element *elmt : std::as_const(connected_elements))
+			links.append({elmt->uuid(), m_group_index_map.value(elmt, -1), elmt});
+	}
+	return links;
+}
+
 /**
 	@brief Element::pendingLinks
 	@return the uuids of the partners this symbol was saved linked to and

sources/qetgraphicsitem/element.h

@@ -227,6 +227,13 @@ class Element : public QetGraphicsItem
 		virtual void unlinkElement(Element *) {}
 		virtual void initLink(QETProject *);
 		QSet<QUuid> pendingLinks() const;
+			/// A partner as the symbol saves it (linksToSave())
+		struct SavedLink {
+			QUuid uuid;
+			int group_index = -1;
+			Element *built = nullptr;   ///< nullptr: on a folio not built yet
+		};
+		QList<SavedLink> linksToSave(const QSet<QUuid> &waiting) const;
 		bool keepsSavedLinks() const {return !tmp_uuids_link.isEmpty();}
 		QList<std::pair<QUuid, int>> waitingLinks() const;
 			/**

sources/qetproject.cpp

@@ -2651,6 +2651,16 @@ QDomDocument QETProject::toXml(QHash<int, QByteArray> *kept_folios, QStringList
 	// Write the elements collection.
 	project_root.appendChild(m_elements_collection->root().cloneNode(true));
 
+		//QET_CHECK_LINK_ROWS=1: say whether the project database's link rows
+		//are the links each placed symbol saves, in its order, once the
+		//folios are written. Builds nothing (lazy folios stay as they are):
+		//a test and debugging aid; it changes nothing.
+	if (qEnvironmentVariableIntValue("QET_CHECK_LINK_ROWS") == 1) {
+		const QStringList links = m_data_base.linkRowMismatches();
+		qWarning().noquote() << "link rows:" << links.size() << "differ"
+							 << links.mid(0, 5).join(QStringLiteral(", "));
+	}
+
 	m_waiting_link_targets.clear();
 	return(xml_doc);
 }