Merge pull request #1445 from ispyisail/fix/db-stale-after-edits

Fix the project database going stale after folio, symbol and wire edits

sources/dataBase/projectdatabase.cpp

@@ -75,18 +75,7 @@ projectDataBase::projectDataBase(QETProject *project, QObject *parent) :
 	connect(m_project, &QETProject::projectDiagramsOrderChanged, [this]()
 	{
 		m_content_changed = true;
-		for (auto diagram : m_project->diagrams())
-		{
-			m_diagram_order_changed.bindValue(":pos", m_project->folioIndex(diagram)+1);
-			m_diagram_order_changed.bindValue(":uuid", diagram->uuid());
-			m_diagram_order_changed.exec();
-
-
-			m_diagram_info_order_changed.bindValue(":folio", diagram->border_and_titleblock.titleblockInformation().value("folio"));
-			m_diagram_info_order_changed.bindValue(":uuid", diagram->uuid());
-			m_diagram_info_order_changed.exec();
-
-		}
+		updateFolioPositions();
 		emit dataBaseUpdated();
 	});
 }
@@ -122,6 +111,7 @@ void projectDataBase::updateDB()
 	{
 		flushDrawingItems();
 		flushLinks();
+		flushElementPositions();
 		emit dataBaseUpdated();
 		return;
 	}
@@ -649,6 +639,7 @@ bool projectDataBase::populateFromDocument(const QDomDocument &document, QString
 		query.exec(QStringLiteral("DELETE FROM ") + table);
 	}
 	m_dirty_link_elements.clear();
+	m_moved_elements.clear();
 
 	for (int i = 0 ; i < diagram_uuids.size() ; ++i)
 	{
@@ -824,9 +815,11 @@ QSqlQuery projectDataBase::newQuery(const QString &query, QString *error) {
 		//Drawing-item rows are rewritten lazily, see drawingItemChanged().
 		//Every read from outside comes through here, so this is the one
 		//place the queue has to be emptied for a reader to see current rows.
-		//The same goes for link rows, see linksChanged().
+		//The same goes for link rows and elements' folio cells, see
+		//linksChanged() and elementMoved().
 	flushDrawingItems();
 	flushLinks();
+	flushElementPositions();
 
 	// First gate: which kind of statement is acceptable here at all. A
 	// textual check is the right tool for that and the wrong tool for
@@ -920,6 +913,8 @@ void projectDataBase::addElement(Element *element)
 	}
 	connect(element, &Element::linkedElementChanged,
 			this, &projectDataBase::linksChanged, Qt::UniqueConnection);
+	connect(element, &QGraphicsObject::xChanged, this, &projectDataBase::elementMoved, Qt::UniqueConnection);
+	connect(element, &QGraphicsObject::yChanged, this, &projectDataBase::elementMoved, Qt::UniqueConnection);
 
 	bindElementInfoValues(m_insert_element_info_query, element);
 	if (!m_insert_element_info_query.exec()) {
@@ -1022,18 +1017,78 @@ void projectDataBase::addDiagram(Diagram *diagram)
 		addDrawingItem(item);
 	}
 
-		//The information "folio" of other existing diagram can have the variable %total,
-		//so when a new diagram is added this variable change.
-		//We need to update this information in the database.
-	for (auto diagram : project()->diagrams())
+		//The folios after the new one moved down, and a folio number made
+		//from %id or %total changed on every folio.
+	updateFolioPositions();
+	emit dataBaseUpdated();
+}
+
+/**
+	@brief projectDataBase::elementMoved
+	The sender() element moved: its folio cell is written again by the
+	next flushElementPositions().
+*/
+void projectDataBase::elementMoved()
+{
+	auto *element = qobject_cast<Element *>(sender());
+	if (element && !m_moved_elements.contains(element)) {
+		m_moved_elements << element;
+	}
+}
+
+/**
+	@brief projectDataBase::flushElementPositions
+	Write the folio cell of every element queued by elementMoved()
+*/
+void projectDataBase::flushElementPositions()
+{
+	if (m_moved_elements.isEmpty()) {
+		return;
+	}
+
+	const auto moved = m_moved_elements;
+	m_moved_elements.clear();
+	const bool own_transaction = m_data_base.transaction();
+	QSqlQuery update(m_data_base);
+	update.prepare(QStringLiteral("UPDATE element SET pos = :pos WHERE uuid = :uuid"));
+	for (const QPointer<Element> &element : moved)
 	{
-		m_diagram_info_order_changed.bindValue(":folio", diagram->border_and_titleblock.titleblockInformation().value("folio"));
-		m_diagram_info_order_changed.bindValue(":uuid", diagram->uuid());
-		if (!m_diagram_info_order_changed.exec()) {
-			qDebug() << "projectDataBase::addDiagram update diagram infp order error : " << m_diagram_info_order_changed.lastError();
+		if (!element || !element->diagram()) {
+			continue;
+		}
+		update.bindValue(QStringLiteral(":pos"),
+						 element->diagram()->convertPosition(element->scenePos()).toString());
+		update.bindValue(QStringLiteral(":uuid"), element->uuid().toString());
+		if (!update.exec()) {
+			qDebug() << "projectDataBase::flushElementPositions error : " << update.lastError();
+		}
+	}
+	if (own_transaction) {
+		m_data_base.commit();
+	}
+}
+
+/**
+	@brief projectDataBase::updateFolioPositions
+	Write every folio's position and folio number again, after a folio was
+	added, removed or moved.
+*/
+void projectDataBase::updateFolioPositions()
+{
+	for (auto diagram : m_project->diagrams())
+	{
+		m_diagram_order_changed.bindValue(":pos", m_project->folioIndex(diagram)+1);
+		m_diagram_order_changed.bindValue(":uuid", diagram->uuid().toString());
+		if (!m_diagram_order_changed.exec()) {
+			qDebug() << "projectDataBase::updateFolioPositions position error : " << m_diagram_order_changed.lastError();
+		}
+
+		m_diagram_info_order_changed.bindValue(":folio", diagram->border_and_titleblock.titleblockInformation().value("folio"));
+		m_diagram_info_order_changed.bindValue(":uuid", diagram->uuid().toString());
+		if (!m_diagram_info_order_changed.exec()) {
+			qDebug() << "projectDataBase::updateFolioPositions folio error : " << m_diagram_info_order_changed.lastError();
 		}
 	}
-	emit dataBaseUpdated();
 }
 
 void projectDataBase::removeDiagram(Diagram *diagram)
@@ -1118,6 +1173,15 @@ void projectDataBase::removeDiagram(Diagram *diagram)
 		}
 	}
 
+	QSqlQuery remove_info(m_data_base);
+	remove_info.prepare(QStringLiteral("DELETE FROM diagram_info WHERE diagram_uuid = :uuid"));
+	remove_info.bindValue(QStringLiteral(":uuid"), uuid_str);
+	if (!remove_info.exec()) {
+		qDebug() << "projectDataBase::removeDiagram diagram_info delete error : " << remove_info.lastError();
+		m_data_base.rollback();
+		return;
+	}
+
 	m_remove_diagram_query.bindValue(":uuid", uuid_str);
 	if (!m_remove_diagram_query.exec()) {
 		qDebug() << "projectDataBase::removeDiagram delete error : " << m_remove_diagram_query.lastError();
@@ -1126,6 +1190,8 @@ void projectDataBase::removeDiagram(Diagram *diagram)
 	}
 
 	m_data_base.commit();
+		//The folios after it moved up, and %id / %total changed.
+	updateFolioPositions();
 	emit dataBaseUpdated();
 }
 
@@ -1187,12 +1253,39 @@ void projectDataBase::addConductor(Conductor *conductor)
 void projectDataBase::removeConductor(Conductor *conductor)
 {
 	m_content_changed = true;
+		//The terminal table lists the terminals a conductor ends on: the
+		//conductor's two ends go with it unless another conductor ends there.
+	QList<QPair<QString, QString>> ends;
+	QSqlQuery read_ends(m_data_base);
+	read_ends.prepare(QStringLiteral("SELECT terminal1_uuid, terminal1_element_uuid, "
+									 "terminal2_uuid, terminal2_element_uuid "
+									 "FROM conductor WHERE uuid = :uuid"));
+	read_ends.bindValue(QStringLiteral(":uuid"), conductor->uuid().toString());
+	if (read_ends.exec() && read_ends.next()) {
+		ends << qMakePair(read_ends.value(0).toString(), read_ends.value(1).toString())
+			 << qMakePair(read_ends.value(2).toString(), read_ends.value(3).toString());
+	}
+
 	m_remove_conductor_query.bindValue(":uuid", conductor->uuid().toString());
 	if (!m_remove_conductor_query.exec()) {
 		qDebug() << "projectDataBase::removeConductor delete error : " << m_remove_conductor_query.lastError();
-	} else {
-		emit dataBaseUpdated();
+		return;
 	}
+
+	QSqlQuery remove_end(m_data_base);
+	remove_end.prepare(QStringLiteral(
+			"DELETE FROM terminal WHERE uuid = :uuid AND element_uuid = :element_uuid "
+			"AND NOT EXISTS (SELECT 1 FROM conductor WHERE "
+			"(terminal1_uuid = :uuid AND terminal1_element_uuid = :element_uuid) OR "
+			"(terminal2_uuid = :uuid AND terminal2_element_uuid = :element_uuid))"));
+	for (const auto &end : std::as_const(ends)) {
+		remove_end.bindValue(QStringLiteral(":uuid"), end.first);
+		remove_end.bindValue(QStringLiteral(":element_uuid"), end.second);
+		if (!remove_end.exec()) {
+			qDebug() << "projectDataBase::removeConductor terminal delete error : " << remove_end.lastError();
+		}
+	}
+	emit dataBaseUpdated();
 }
 
 /**
@@ -2061,6 +2154,7 @@ static ElementData::Types allElementTypes()
 */
 void projectDataBase::populateElementTable()
 {
+	m_moved_elements.clear();
 	QSqlQuery query_(m_data_base);
 	query_.exec("DELETE FROM element");
 
@@ -2597,6 +2691,7 @@ void projectDataBase::exportDb(projectDataBase *db,
 	const auto escaped_path = path_.replace("'", "''");
 	db->flushDrawingItems();
 	db->flushLinks();
+	db->flushElementPositions();
 	QSqlQuery query(db->m_data_base);
 	if (!query.exec("VACUUM INTO '" % escaped_path % "'")) {
 		qWarning() << "Unable to export project database:" << query.lastError().text();

sources/dataBase/projectdatabase.h

@@ -126,6 +126,8 @@ class projectDataBase : public QObject
 		void drawingItemDestroyed(QObject *object);
 			//Queue the sender()'s link rows for rewriting.
 		void linksChanged();
+			//Queue the sender() element's folio cell for rewriting.
+		void elementMoved();
 
 	public:
 
@@ -139,6 +141,8 @@ class projectDataBase : public QObject
 		void createWiringListView();
 		void createDrawingItemView();
 		void populateDiagramTable();
+		void updateFolioPositions();
+		void flushElementPositions();
 		void populateElementTable();
 		void populateElementInfoTable();
 		void populateDiagramInfoTable();
@@ -214,6 +218,10 @@ class projectDataBase : public QObject
 			//written: a link is made first and its contact group set after,
 			//so the rows are written when next read, as drawing items are.
 		QList<QPointer<Element>> m_dirty_link_elements;
+			//Elements moved since their folio cell was last written: a move
+			//sends xChanged and yChanged for every step of a drag, so the
+			//cell is written when next read, as drawing items are.
+		QList<QPointer<Element>> m_moved_elements;
 
 #ifdef QET_EXPORT_PROJECT_DB
 	public:

tests/qttest/tst_databasefromdocument.cpp

@@ -56,6 +56,12 @@ class tst_databasefromdocument : public QObject
 	{
 		const QString out = run({QStringLiteral("--run"), m_dir.filePath(QStringLiteral("dump.js")),
 								 project}, from_folios);
+		return tablesIn(out, how);
+	}
+
+	// The DUMP line of a --run output, each table as a sorted list of rows
+	static QJsonObject tablesIn(const QString &out, QString *how)
+	{
 		QJsonObject tables;
 		for (const QString &line : out.split(QLatin1Char('\n'))) {
 			if (line.contains(QStringLiteral("Project database filled")))
@@ -345,6 +351,105 @@ private slots:
 									 "a link is not one the folios would make as saved"));
 	}
 
+	// After a folio is added and another removed, the folio tables hold
+	// what a fresh open of the saved project gives: every folio's position
+	// and folio number, and no row for the removed folio. Positions only
+	// changed when folios were reordered, and the removed folio's
+	// diagram_info row stayed.
+	void folioAddedAndRemoved()
+	{
+		const QString saved = m_dir.filePath(QStringLiteral("folios.qet"));
+		const QString edited = m_dir.filePath(QStringLiteral("folios-edited.qet"));
+		run({QStringLiteral("--resave"), QStringLiteral(QET_EXAMPLES_DIR "/tremie_vibrante.qet"), saved});
+		QVERIFY(QFile::exists(saved));
+		QFile js(m_dir.filePath(QStringLiteral("folios.js")));
+		QVERIFY(js.open(QIODevice::WriteOnly));
+		js.write(QStringLiteral(
+				 "qet.log('ADDED ' + qet.insertFolio(0));\n"
+				 "qet.log('REMOVED ' + qet.removeFolio(2));\n"
+				 "var out = {};\n"
+				 "['diagram', 'diagram_info'].forEach(function (t) { out[t] = qet.query('SELECT * FROM ' + t); });\n"
+				 "qet.log('DUMP ' + JSON.stringify(out));\n"
+				 "qet.log('SAVED ' + qet.save('%1'));\n").arg(edited).toUtf8());
+		js.close();
+		const QString log = run({QStringLiteral("--run"), js.fileName(), saved});
+		QVERIFY2(log.contains(QStringLiteral("ADDED 0")) && log.contains(QStringLiteral("REMOVED true"))
+				 && log.contains(QStringLiteral("SAVED true")), qPrintable(log.right(400)));
+		QString unused;
+		const QJsonObject after_edits = tablesIn(log, &unused);
+		const QJsonObject reopened = dump(edited, false, &unused);
+		for (const QString &table : {QStringLiteral("diagram"), QStringLiteral("diagram_info")}) {
+			const QJsonArray a = after_edits.value(table).toArray(), b = reopened.value(table).toArray();
+			QVERIFY2(!a.isEmpty(), qPrintable(table));
+			QVERIFY2(a == b, qPrintable(QStringLiteral("%1: %2 rows after the edits, %3 on reopening, or different")
+										.arg(table).arg(a.size()).arg(b.size())));
+		}
+	}
+
+	// After an element is moved several columns, its folio cell in the
+	// element table is the one a fresh open of the saved project gives.
+	// It used to stay where the element was added.
+	void movedElementCell()
+	{
+		const QString saved = m_dir.filePath(QStringLiteral("moved.qet"));
+		const QString edited = m_dir.filePath(QStringLiteral("moved-edited.qet"));
+		run({QStringLiteral("--resave"), QStringLiteral(QET_EXAMPLES_DIR "/tremie_vibrante.qet"), saved});
+		QVERIFY(QFile::exists(saved));
+		QFile js(m_dir.filePath(QStringLiteral("moved.js")));
+		QVERIFY(js.open(QIODevice::WriteOnly));
+		js.write(QStringLiteral(
+				 "var e = qet.elementUuids(0)[0];\n"
+				 "var before = qet.query(\"SELECT pos FROM element WHERE uuid = '\" + e + \"'\")[0].pos;\n"
+				 "qet.log('MOVED ' + qet.moveElement(0, e, 300, 0));\n"
+				 "var after = qet.query(\"SELECT pos FROM element WHERE uuid = '\" + e + \"'\")[0].pos;\n"
+				 "qet.log('CELL ' + before + ' ' + after);\n"
+				 "qet.log('DUMP ' + JSON.stringify({element: qet.query('SELECT * FROM element')}));\n"
+				 "qet.log('SAVED ' + qet.save('%1'));\n").arg(edited).toUtf8());
+		js.close();
+		const QString log = run({QStringLiteral("--run"), js.fileName(), saved});
+		QVERIFY2(log.contains(QStringLiteral("MOVED true")) && log.contains(QStringLiteral("SAVED true")),
+				 qPrintable(log.right(400)));
+		QString unused;
+		const QJsonArray a = tablesIn(log, &unused).value(QStringLiteral("element")).toArray();
+		const QJsonArray b = dump(edited, false, &unused).value(QStringLiteral("element")).toArray();
+		QVERIFY(!a.isEmpty());
+		QVERIFY2(a == b, qPrintable(log.mid(log.indexOf(QStringLiteral("CELL")), 40)));
+	}
+
+	// After every conductor on a folio is deleted, the terminal table
+	// holds what a fresh open of the saved project gives: a terminal is
+	// listed while a conductor ends on it. The ends of a deleted conductor
+	// used to stay.
+	void deletedWireTerminals()
+	{
+		const QString saved = m_dir.filePath(QStringLiteral("unwired.qet"));
+		const QString edited = m_dir.filePath(QStringLiteral("unwired-edited.qet"));
+		run({QStringLiteral("--resave"), QStringLiteral(QET_EXAMPLES_DIR "/tremie_vibrante.qet"), saved});
+		QVERIFY(QFile::exists(saved));
+		QFile js(m_dir.filePath(QStringLiteral("unwired.js")));
+		QVERIFY(js.open(QIODevice::WriteOnly));
+		js.write(QStringLiteral(
+				 "var n = 0;\n"
+				 "qet.conductorUuids(0).forEach(function (c) {\n"
+				 "  var end = qet.conductorEnds(0, c)[0];\n"
+				 "  if (!end) return;\n"
+				 "  var m = end.match(/^(\\{[^}]+\\}) terminal (\\d+)$/);\n"
+				 "  if (m && qet.deleteConductor(0, m[1], parseInt(m[2]))) ++n;\n"
+				 "});\n"
+				 "qet.log('DELETED ' + n);\n"
+				 "qet.log('DUMP ' + JSON.stringify({terminal: qet.query('SELECT * FROM terminal')}));\n"
+				 "qet.log('SAVED ' + qet.save('%1'));\n").arg(edited).toUtf8());
+		js.close();
+		const QString log = run({QStringLiteral("--run"), js.fileName(), saved});
+		QVERIFY2(!log.contains(QStringLiteral("DELETED 0")) && log.contains(QStringLiteral("SAVED true")),
+				 qPrintable(log.right(400)));
+		QString unused;
+		const QJsonArray a = tablesIn(log, &unused).value(QStringLiteral("terminal")).toArray();
+		const QJsonArray b = dump(edited, false, &unused).value(QStringLiteral("terminal")).toArray();
+		QVERIFY2(a == b, qPrintable(QStringLiteral("%1 terminal rows after the edits, %2 on reopening")
+									.arg(a.size()).arg(b.size())));
+	}
+
 	// A file whose items carry no saved uuid is filled from the folios,
 	// which work those uuids out as they are built, and says why.
 	void olderFileFallsBack()