Merge pull request #1471 from ispyisail/fix/lazy-refused-opens-eager

Fix lazy folios opening older projects slower than without them

sources/bordertitleblock.cpp

@@ -321,8 +321,8 @@ TitleBlockProperties BorderTitleBlock::localTitleBlock() const
 /**
 	@brief BorderTitleBlock::current
 	@return the properties of the titleblock: for a folio placed in a
-	project the project's store holds them (DB-ACCESSORS-PLAN.md stage
-	4.4), for any other this object's own copy.
+	project the project's store holds them, for any other this object's
+	own copy.
 */
 TitleBlockProperties BorderTitleBlock::current() const
 {

sources/bordertitleblock.h

@@ -110,8 +110,8 @@ class BorderTitleBlock : public QObject
 		QRectF insideBorderRect() const;
 	
 		// methods to get title block basic data. A folio placed in a
-		// project reads them from the project's store
-		// (DB-ACCESSORS-PLAN.md stage 4.4), any other from its own copy.
+		// project reads them from the project's store, any other from its
+		// own copy.
 		/// @return the value of the title block "Author" field
 		QString author() const { const TitleBlockProperties *s = stored(); return(s ? s->author : btb_author_); }
 		/// @return the value of the title block "Date" field

sources/dataBase/projectdatabase.cpp

@@ -253,6 +253,58 @@ void projectDataBase::updateDB(const QDomDocument &document)
 	updateDB();
 }
 
+/**
+	@brief projectDataBase::documentRefusal
+	@return why populateFromDocument() will refuse @p document, for the
+	reasons it gives that the document alone can tell -- a folio, symbol
+	or wire with no saved uuid, two symbols sharing one, a folio number
+	using %autonum -- or an empty string. The other reasons need the
+	project's definitions or every folio's links; a document refused for
+	one of those is still filled from the folios, only later.
+*/
+QString projectDataBase::documentRefusal(const QDomDocument &document)
+{
+	if (qEnvironmentVariableIntValue("QET_DATABASE_FROM_FOLIOS") == 1) {
+		return QStringLiteral("QET_DATABASE_FROM_FOLIOS is set");
+	}
+	QSet<QUuid> element_uuids;
+	const QDomNodeList diagram_nodes = document.elementsByTagName(QStringLiteral("diagram"));
+	for (int i = 0 ; i < diagram_nodes.size() ; ++i)
+	{
+		QDomElement diagram_xml = diagram_nodes.at(i).toElement();
+		if (QUuid(diagram_xml.attribute(QStringLiteral("uuid"))).isNull()) {
+			return QStringLiteral("a folio has no saved uuid");
+		}
+		if (diagram_xml.attribute(QStringLiteral("folio")).contains(QStringLiteral("%autonum"))) {
+			return QStringLiteral("a folio number uses %autonum");
+		}
+		for (QDomElement element_xml : QET::findInDomElement(
+				 diagram_xml, QStringLiteral("elements"), QStringLiteral("element")))
+		{
+				//Skipped by Diagram::fromXml() as well
+			if (!Element::valideXml(element_xml)) {
+				continue;
+			}
+			const QUuid uuid(element_xml.attribute(QStringLiteral("uuid")));
+			if (uuid.isNull() || element_uuids.contains(uuid)) {
+				return QStringLiteral("an element has no saved uuid, or shares one");
+			}
+			element_uuids.insert(uuid);
+		}
+		for (QDomElement conductor_xml : QET::findInDomElement(
+				 diagram_xml, QStringLiteral("conductors"), QStringLiteral("conductor")))
+		{
+			if (Conductor::valideXml(conductor_xml)
+				&& (QUuid(conductor_xml.attribute(QStringLiteral("uuid"))).isNull()
+					|| !conductor_xml.hasAttribute(QStringLiteral("element1"))
+					|| !conductor_xml.hasAttribute(QStringLiteral("element2")))) {
+				return QStringLiteral("a conductor has no saved uuid, or names its ends the older way");
+			}
+		}
+	}
+	return QString();
+}
+
 namespace {
 struct DocumentTerminal
 {
@@ -2445,7 +2497,7 @@ void projectDataBase::populateElementTable()
 /**
 	@brief projectDataBase::populateElementInfoTable
 	Populate the element info table from the symbol information store
-	(DB-ACCESSORS-PLAN.md stage 4.3), in the order of the element table.
+	in the order of the element table.
 	The label column is the label a symbol shows, which its folio's
 	numbering decides: the symbol holding the uuid gives it.
 

sources/dataBase/projectdatabase.h

@@ -67,6 +67,14 @@ class projectDataBase : public QObject
 				populateFromDocument()), from the built folios otherwise.
 			*/
 		void updateDB(const QDomDocument &document);
+			/**
+				Why updateDB(document) will fill the tables from the folios
+				rather than from @p document, for the reasons the document
+				alone can tell; empty when it knows of none. Read before the
+				folios are: a project whose tables will come from the folios
+				builds every folio as it opens, even with QET_LAZY_FOLIOS.
+			*/
+		static QString documentRefusal(const QDomDocument &document);
 			/**
 				Suppress the full rebuild performed by updateDB().
 
@@ -106,7 +114,7 @@ class projectDataBase : public QObject
 
 			//The information of every symbol placed in the project, by
 			//symbol uuid: the store each symbol's information is written
-			//through (DB-ACCESSORS-PLAN.md stage 4.1). A symbol not placed
+			//through. A symbol not placed
 			//(a preview, the symbol editor) keeps its own.
 		DiagramContext elementInformation(const QUuid &element) const;
 		bool hasElementInformation(const QUuid &element) const;
@@ -124,7 +132,7 @@ class projectDataBase : public QObject
 
 			//The title block properties of every folio of the project, by
 			//folio uuid: the store each folio's title block is written
-			//through (DB-ACCESSORS-PLAN.md stage 4.4). A folio not in a
+			//through. A folio not in a
 			//project keeps its own.
 		TitleBlockProperties folioTitleBlock(const QUuid &folio) const;
 		const TitleBlockProperties *storedFolioTitleBlock(const QUuid &folio) const;
@@ -136,7 +144,7 @@ class projectDataBase : public QObject
 
 			//The properties of every wire placed in the project, by wire
 			//uuid: the store each wire's properties are written through
-			//(DB-ACCESSORS-PLAN.md stage 4.5). A wire not placed keeps its own.
+			//A wire not placed keeps its own.
 		ConductorProperties conductorProperties(const QUuid &conductor) const;
 		bool hasConductorProperties(const QUuid &conductor) const;
 		int placedConductorCount(const QUuid &conductor) const;
@@ -145,7 +153,7 @@ class projectDataBase : public QObject
 		QStringList conductorPropertiesMismatches() const;
 
 			//Fill the three stores from a project's document before its
-			//folios are built (DB-ACCESSORS-PLAN.md stage 4.6), then, once
+			//folios are built, then, once
 			//they are, say how many of those values building agreed with.
 		void prefillFromDocument(const QDomDocument &document);
 		void endPrefill(bool keep_unplaced = false);

sources/diagram.h

@@ -211,7 +211,7 @@ class Diagram : public QGraphicsScene
 				 bool = true,
 				 DiagramContent * = nullptr);
 			/// false while this folio's items wait in its XML to be
-			/// built (QET_LAZY_FOLIOS, LAZY-FOLIO-PLAN.md)
+			/// built (QET_LAZY_FOLIOS)
 		bool isLoaded() const {return m_deferred.isNull();}
 		QDomElement deferredXml() const {return m_deferred;}
 		QString headerXml();

sources/qetgraphicsitem/conductor.cpp

@@ -1857,7 +1857,7 @@ ConductorProperties Conductor::properties() const
 /**
 	@brief Conductor::placedStore
 	@return the project database holding this wire's properties
-	(DB-ACCESSORS-PLAN.md stage 4.5), or nullptr if it is not placed in a
+	or nullptr if it is not placed in a
 	project, or shares its uuid with another wire for the moment a copy
 	takes to get its own
 */

sources/qetgraphicsitem/element.cpp

@@ -1859,7 +1859,7 @@ void Element::setUuid(const QUuid &uuid)
 /**
 	@brief Element::storeInformation
 	Write this symbol's information to its project's store, once it is
-	placed (DB-ACCESSORS-PLAN.md stage 4.1): every change of a placed
+	placed: every change of a placed
 	symbol's information comes through here.
 */
 void Element::storeInformation()
@@ -1925,7 +1925,7 @@ ElementData Element::elementData() const
 /**
 	@brief Element::elementInformations
 	@return this symbol's information: for a placed symbol the project's
-	store holds it (DB-ACCESSORS-PLAN.md stage 4.2), for one not placed
+	store holds it, for one not placed
 	(a preview, the symbol editor) its own copy.
 */
 DiagramContext Element::elementInformations() const

sources/qetproject.cpp

@@ -559,7 +559,7 @@ QETProject::ProjectState QETProject::state() const
 QList<Diagram *> QETProject::diagrams() const
 {
 		//Every caller of diagrams() expects every folio built: those that
-		//do not are moved to folios() one by one (LAZY-FOLIO-PLAN.md)
+		//do not are moved to folios() one by one
 	loadFolios();
 	return(m_diagrams_list);
 }
@@ -2306,8 +2306,7 @@ QDomDocument QETProject::toXml(QHash<int, QByteArray> *kept_folios)
 		loadFolios();
 	}
 		//QET_CHECK_ELEMENT_INFO_STORE=1: say whether the stores of symbol
-		//information, folio title blocks and wire properties
-		//(DB-ACCESSORS-PLAN.md) agree with every placed symbol, folio and
+		//information, folio title blocks and wire properties agree with every placed symbol, folio and
 		//wire, at each save. A test and debugging aid; it changes nothing.
 	if (qEnvironmentVariableIntValue("QET_CHECK_ELEMENT_INFO_STORE") == 1) {
 		const QStringList mismatches = m_data_base.elementInformationMismatches();
@@ -3030,9 +3029,22 @@ void QETProject::readProjectXml(QDomDocument &xml_project)
 
 		//The stores of symbol information, folio title blocks and wire
 		//properties are filled from the document before the folios are
-		//built (DB-ACCESSORS-PLAN.md stage 4.6)
+		//built
 	m_data_base.prefillFromDocument(xml_project);
 
+		//QET_LAZY_FOLIOS leaves the folios to be built when needed only
+		//when the database can be filled from the document: otherwise
+		//filling it from the folios builds every one of them anyway, late
+		//and more slowly than reading them now
+	m_lazy_open = qEnvironmentVariableIntValue("QET_LAZY_FOLIOS") == 1;
+	if (m_lazy_open) {
+		const QString why = projectDataBase::documentRefusal(xml_project);
+		if (!why.isEmpty()) {
+			qInfo().noquote() << "Project opened with every folio built:" << why;
+			m_lazy_open = false;
+		}
+	}
+
 		//Load the diagrams
 	readDiagramsXml(xml_project);
 	const qint64 diagrams_ms = phase_timer.restart();
@@ -3072,7 +3084,6 @@ void QETProject::readProjectXml(QDomDocument &xml_project)
 		//number can hold what a save rewrites (tst_freetextnonfinite,
 		//tst_foliofreezeflags): only with QET_LAZY_SAVE_KEPT=1, until a
 		//.qetz, whose manifest names its writer, does it
-		//(QETZ-DB-TRUTH-PLAN.md D4)
 	m_kept_folios_trusted = qEnvironmentVariableIntValue("QET_LAZY_SAVE_KEPT") == 1
 			&& unloadedFolioCount() > 0
 			&& m_project_qet_version == QetVersion::currentVersion()
@@ -3149,7 +3160,7 @@ void QETProject::readDiagramsXml(QDomDocument &xml_project)
 		//Where each line of the file starts, for a folio's text: lines
 		//[its start, the next element's start) (QETProject::toXml())
 	QList<qsizetype> line_starts;
-	if (qEnvironmentVariableIntValue("QET_LAZY_FOLIOS") == 1 && !m_opened_content.isEmpty()) {
+	if (m_lazy_open && !m_opened_content.isEmpty()) {
 		line_starts << 0;
 		for (qsizetype i = 0 ; i < m_opened_content.size() ; ++i)
 			if (m_opened_content.at(i) == '\n') line_starts << i + 1;
@@ -3196,7 +3207,7 @@ void QETProject::readDiagramsXml(QDomDocument &xml_project)
 			connect(diagram, &Diagram::usedTitleBlockTemplateChanged,
 					this, &QETProject::usedTitleBlockTemplateChanged);
 
-			m_deferring_folios = qEnvironmentVariableIntValue("QET_LAZY_FOLIOS") == 1;
+			m_deferring_folios = m_lazy_open;
 			diagram->initFromXml(diagram_xml_element);
 			m_deferring_folios = false;
 			if (!diagram->isLoaded() && !line_starts.isEmpty())

sources/qetproject.h

@@ -123,7 +123,7 @@ class QETProject : public QObject
 		ProjectState state() const;
 		QList<Diagram *> diagrams() const;
 			//The folios whether their items are built or not
-			//(QET_LAZY_FOLIOS, LAZY-FOLIO-PLAN.md); diagrams() builds them
+			//(QET_LAZY_FOLIOS); diagrams() builds them
 		QList<Diagram *> folios() const;
 		bool deferringFolios() const {return m_deferring_folios;}
 		int unloadedFolioCount() const;
@@ -516,6 +516,9 @@ class QETProject : public QObject
 				const DiagramContext &information,
 				const QHash<QString, QStringList> &titles_by_formula) const;
 		bool m_deferring_folios = false;
+			/// Opening with folios built only when needed: QET_LAZY_FOLIOS
+			/// is set and the database can be filled from the document
+		bool m_lazy_open = false;
 			/// Saving may write a folio not built as it was read
 			/// (writesKeptFolios()): the file is this version's, its
 			/// database rows came from it, and nothing project-wide that

tests/qttest/lazy_folios_probe.cpp

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-// Lazy folios (QET_LAZY_FOLIOS, LAZY-FOLIO-PLAN.md stages 5.2 and 5.3),
+// Lazy folios (QET_LAZY_FOLIOS),
 // linked against the application's objects: a folio shown in a project
 // opened without building its folios draws and links as in the project
 // opened as usual, building only it and the folios its links and tables
@@ -37,6 +37,8 @@
 #include "../../sources/undocommand/deleteqgraphicsitemcommand.h"
 #include "../../sources/factory/elementfactory.h"
 #include "../../sources/undocommand/addgraphicsobjectcommand.h"
+#include "../../sources/dataBase/projectdatabase.h"
+#include <QDomDocument>
 
 static void check(bool value, const char *message) { if (!value) throw std::runtime_error(message); }
 
@@ -567,6 +569,44 @@ int main(int argc, char **argv)
 			check(potential(wire) == potential(usual_wire), "a potential followed as when built");
 			std::printf("potential through reports: %d wires, %d folios not built\n",
 						int(potential(wire).size()), followed.unloadedFolioCount());
+		}
+			//Two symbols sharing a uuid: the database is filled from the
+			//folios, which builds every one of them, so the project opens
+			//with every folio built rather than building them all later
+		{
+			QFile file(path);
+			check(file.open(QIODevice::ReadOnly), "fixture read");
+			QDomDocument document;
+			check(bool(document.setContent(&file)), "fixture parsed");
+			file.close();
+			check(projectDataBase::documentRefusal(document).isEmpty(), "the fixture fills the database itself");
+			QDomNodeList elements = document.elementsByTagName(QStringLiteral("element"));
+			QList<QDomElement> placed;
+			for (int i = 0; i < elements.size(); ++i) {
+				QDomElement e = elements.at(i).toElement();
+				if (Element::valideXml(e)) placed << e;
+			}
+			check(placed.size() > 1, "two placed symbols");
+			placed.last().setAttribute(QStringLiteral("uuid"), placed.first().attribute(QStringLiteral("uuid")));
+			check(!projectDataBase::documentRefusal(document).isEmpty(), "a shared uuid is seen before the folios are read");
+			const QString shared = output + "/lazy_folios_shared_uuid.qet";
+			QFile out(shared);
+			check(out.open(QIODevice::WriteOnly), "copy written");
+			out.write(document.toByteArray(4));
+			out.close();
+			qputenv("QET_LAZY_FOLIOS", "1");
+			qputenv("QET_LAZY_FOLIOS_TRACE", "1");
+			static int built_late; built_late = 0;
+			const QtMessageHandler previous = qInstallMessageHandler(
+					[](QtMsgType, const QMessageLogContext &, const QString &message) {
+				if (message.contains(QStringLiteral("built for a caller of diagrams()"))) ++built_late;
+			});
+			QETProject opened(shared);
+			qInstallMessageHandler(previous);
+			qunsetenv("QET_LAZY_FOLIOS_TRACE");
+			check(opened.state() == QETProject::Ok, "a project with a shared uuid opens");
+			check(opened.unloadedFolioCount() == 0, "a project with a shared uuid opens with every folio built");
+			check(built_late == 0, "a project with a shared uuid builds its folios as it reads them, not later");
 		}
 		std::printf("PASS: %d folios\n", int(folios.size()));
 	} catch (const std::exception &error) {

tests/qttest/tst_resaveunchanged.cpp

@@ -340,8 +340,8 @@ private slots:
 		return QString::fromUtf8(proc.readAllStandardOutput() + proc.readAllStandardError());
 	}
 
-	// The stores of symbol information and folio title blocks
-	// (DB-ACCESSORS-PLAN.md stages 4.1, 4.4) agree with every placed symbol
+	// The stores of symbol information and folio title blocks agree with
+	// every placed symbol
 	// and folio once a project is open: every example.
 	void elementInfoStoreAgrees_data()
 	{
@@ -361,7 +361,7 @@ private slots:
 	}
 
 	// The stores filled from the document before the folios are built
-	// (DB-ACCESSORS-PLAN.md stage 4.6) hold what building them gives, for a
+	// hold what building them gives, for a
 	// project saved by this version (every symbol, folio and wire carries
 	// a uuid): nothing differs, nothing is left unplaced.
 	void storesFilledBeforeBuilding()