Fix unique document detection on startup

Due to the delayed document registration, specifying the same document multiple times on the command line would actually open the file multiple times. This delayed registration was actually a hack in the first place. Fixed by wrapping the Document constructor.
rgriebl · May 8, 2024 · eb30535 · eb30535
1 parent e3eb99b
commit eb30535
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 53 deletions.
diff --git a/src/common/application.cpp b/src/common/application.cpp
@@ -262,7 +262,7 @@ void Application::afterInit()
     }
 
     ActionManager::ActionTable applicationActionTable = {
-        { "document_new", [](bool) { new Document(); } },
+        { "document_new", [](bool) { Document::create(); } },
         { "document_open", [](bool) { Document::load(); } },
         { "document_import_bl_xml", [](bool) { DocumentIO::importBrickLinkXML(); } },
         { "document_import_ldraw_model", [](bool) { DocumentIO::importLDrawModel(); } },

diff --git a/src/common/document.cpp b/src/common/document.cpp
@@ -142,26 +142,36 @@ void ColumnLayoutCmd::undo()
 ///////////////////////////////////////////////////////////////////////
 
 
-Document::Document(QObject *parent)
-    : Document(new DocumentModel(), parent)
-{ }
+Document *Document::create(QObject *parent)
+{
+    return create(new DocumentModel(), parent);
+}
 
-Document::Document(DocumentModel *model, QObject *parent)
-    : Document(model, QByteArray { }, parent)
-{ }
+Document *Document::create(DocumentModel *model, QObject *parent)
+{
+    return create(model, QByteArray { }, parent);
+}
 
-Document::Document(DocumentModel *model, const QByteArray &columnsState, bool restoredFromAutosave,
-                   QObject *parent)
-    : Document(model, columnsState, parent)
+Document *Document::create(DocumentModel *model, const QByteArray &columnsState, QObject *parent)
+{
+    return create(model, columnsState, false, parent);
+}
+
+Document *Document::create(DocumentModel *model, const QByteArray &columnsState,
+                           bool restoredFromAutosave, QObject *parent)
 {
-    m_restoredFromAutosave = restoredFromAutosave;
+    auto doc = new Document(model, columnsState, restoredFromAutosave, parent);
+    DocumentList::inst()->add(doc);
+    return doc;
 }
 
-Document::Document(DocumentModel *model, const QByteArray &columnsState, QObject *parent)
+Document::Document(DocumentModel *model, const QByteArray &columnsState, bool restoredFromAutosave,
+                   QObject *parent)
     : QObject(parent)
     , m_model(model)
     , m_selectionModel(new QItemSelectionModel(m_model, this))
     , m_uuid(QUuid::createUuid())
+    , m_restoredFromAutosave(restoredFromAutosave)
 {
     setTitle(tr("Untitled"));
 
@@ -466,8 +476,6 @@ Document::Document(DocumentModel *model, const QByteArray &columnsState, QObject
                   Application::openUrl(BrickLink::Core::urlForStoreItemSearch(lot->item(), lot->color()));
           } },
     };
-
-    DocumentList::inst()->add(this);
 }
 
 
@@ -1763,7 +1771,7 @@ Document *Document::fromPartInventory(const BrickLink::Item *item,
     auto pr = BrickLink::IO::fromPartInventory(item, color, multiply, condition, extraParts,
                                                partOutTraits, status);
 
-    auto *document = new Document(new DocumentModel(std::move(pr))); // Document own the items now
+    auto *document = create(new DocumentModel(std::move(pr))); // Document own the items now
     document->setTitle(tr("Inventory for %1").arg(QString::fromLatin1(item->id())));
 
     auto thumbnail = BrickLink::core()->pictureCache()->picture(item, color, true);
@@ -2563,7 +2571,7 @@ int Document::processAutosaves(AutosaveAction action)
                     DocumentIO::BsxContents bsx;
                     auto model = new DocumentModel(std::move(pr), true /*mark as modified*/);
                     model->restoreSortFilterState(savedSortFilterState);
-                    auto *doc = new Document(model, columnState, true /* is autosave restore*/);
+                    auto *doc = create(model, columnState, true /* is autosave restore*/);
 
                     if (!savedFileName.isEmpty()) {
                         QFileInfo fi(savedFileName);

diff --git a/src/common/document.h b/src/common/document.h
@@ -88,10 +88,12 @@ class Document : public QObject
     Q_OBJECT
 
 public:
-    Document(QObject *parent = nullptr);
-    Document(DocumentModel *model, QObject *parent = nullptr);
-    Document(DocumentModel *model, const QByteArray &columnsState, QObject *parent = nullptr);
-    Document(DocumentModel *model, const QByteArray &columnsState, bool restoredFromAutosave, QObject *parent = nullptr);
+    static Document *create(QObject *parent = nullptr);
+    static Document *create(DocumentModel *model, QObject *parent = nullptr);
+    static Document *create(DocumentModel *model, const QByteArray &columnsState,
+                            QObject *parent = nullptr);
+    static Document *create(DocumentModel *model, const QByteArray &columnsState,
+                            bool restoredFromAutosave, QObject *parent = nullptr);
     ~Document() override;
 
     void setActive(bool active);
@@ -266,6 +268,9 @@ class Document : public QObject
     void orderChanged(BrickLink::Order *order);
 
 private:
+    explicit Document(DocumentModel *model, const QByteArray &columnsState,
+                      bool restoredFromAutosave, QObject *parent = nullptr);
+
     void applyTo(const LotList &lots,
                  const char *actionName, const std::function<DocumentModel::ApplyToResult (const Lot &, Lot &)> &callback);
     bool updatePriceToGuide(BrickLink::Lot *lot, const BrickLink::PriceGuide *pg);

diff --git a/src/common/documentio.cpp b/src/common/documentio.cpp
@@ -59,7 +59,7 @@ Document *DocumentIO::importBrickLinkStore(BrickLink::Store *store)
         pr.addLot(new Lot(*lot));
     pr.setCurrencyCode(store->currencyCode());
 
-    auto *document = new Document(new DocumentModel(std::move(pr)));
+    auto *document = Document::create(new DocumentModel(std::move(pr)));
     document->setTitle(tr("Store %1").arg(QLocale().toString(store->lastUpdated(), QLocale::ShortFormat)));
     document->setThumbnail(u"bricklink-store"_qs);
     return document;
@@ -75,7 +75,7 @@ Document *DocumentIO::importBrickLinkOrder(BrickLink::Order *order)
         pr.addLot(std::move(lot));
     pr.setCurrencyCode(order->currencyCode());
 
-    auto *document = new Document(new DocumentModel(std::move(pr)));
+    auto *document = Document::create(new DocumentModel(std::move(pr)));
     document->setOrder(order);
     document->setTitle(tr("Order %1 (%2)").arg(order->id(), order->otherParty()));
     document->setThumbnail(u"view-financial-list"_qs);
@@ -92,7 +92,7 @@ Document *DocumentIO::importBrickLinkCart(BrickLink::Cart *cart)
         pr.addLot(new Lot(*lot));
     pr.setCurrencyCode(cart->currencyCode());
 
-    auto *document = new Document(new DocumentModel(std::move(pr)));
+    auto *document = Document::create(new DocumentModel(std::move(pr)));
     document->setTitle(tr("Cart in store %1").arg(cart->storeName()));
     document->setThumbnail(u"bricklink-cart"_qs);
     return document;
@@ -107,7 +107,7 @@ Document *DocumentIO::importBrickLinkWantedList(BrickLink::WantedList *wantedLis
     for (const auto *lot : lots)
         pr.addLot(new Lot(*lot));
 
-    auto *document = new Document(new DocumentModel(std::move(pr)));
+    auto *document = Document::create(new DocumentModel(std::move(pr)));
     QString name = wantedList->name().isEmpty() ? QString::number(wantedList->id())
                                                 : wantedList->name();
     document->setTitle(tr("Wanted List %1").arg(name));
@@ -133,7 +133,7 @@ QCoro::Task<Document *> DocumentIO::importBrickLinkXML(QString fileName)
             auto result = BrickLink::IO::fromBrickLinkXML(f.readAll(),
                                                           BrickLink::IO::Hint::PlainOrWanted,
                                                           f.fileTime(QFile::FileModificationTime));
-            auto *document = new Document(new DocumentModel(std::move(result))); // Document owns the items now
+            auto *document = Document::create(new DocumentModel(std::move(result))); // Document owns the items now
             document->setTitle(tr("Import of %1").arg(QFileInfo(fn).fileName()));
             co_return document;
 
@@ -196,7 +196,7 @@ QCoro::Task<Document *> DocumentIO::importLDrawModel(QString fileName)
         if (!b || !pr.hasLots())
             throw Exception(tr("Could not parse the LDraw data"));
 
-        document = new Document(new DocumentModel(std::move(pr))); // Document owns the items now
+        document = Document::create(new DocumentModel(std::move(pr))); // Document owns the items now
         document->setTitle(tr("Import of %1").arg(QFileInfo(fn).fileName()));
         co_return document;
 
@@ -634,7 +634,7 @@ Document *DocumentIO::parseBsxInventory(QFile *in)
                 auto model = std::make_unique<DocumentModel>(std::move(bsx), (bsx.fixedLotCount() != 0) /*forceModified*/);
                 if (!bsx.guiSortFilterState.isEmpty())
                     model->restoreSortFilterState(bsx.guiSortFilterState);
-                return new Document(model.release(), bsx.guiColumnLayout);
+                return Document::create(model.release(), bsx.guiColumnLayout);
             }
             default:
                 break;

diff --git a/src/common/documentlist.cpp b/src/common/documentlist.cpp
@@ -147,30 +147,25 @@ QVariant DocumentList::data(const QModelIndex &index, int role) const
 
 void DocumentList::add(Document *document)
 {
-    // Please note: Document might not be 100% initialized at this point, because we got called
-    // from the base constructor!
-
-    QMetaObject::invokeMethod(this, [this, document]() {
-        emit documentCreated(document);
-
-        beginInsertRows({ }, rowCount(), rowCount());
-        m_documents.append(document);
-        endInsertRows();
-
-        auto updateDisplay = [this, document]() {
-            int row = int(m_documents.indexOf(document));
-            emit dataChanged(index(row), index(row), { Qt::DisplayRole, Qt::ToolTipRole });
-        };
-        connect(document, &Document::filePathChanged,
-                this, updateDisplay);
-        connect(document, &Document::titleChanged,
-                this, updateDisplay);
-        connect(document->model(), &DocumentModel::modificationChanged,
-                this, updateDisplay);
-
-        emit documentAdded(document);
-        emit countChanged(count());
-    }, Qt::QueuedConnection);
+    emit documentCreated(document);
+
+    beginInsertRows({ }, rowCount(), rowCount());
+    m_documents.append(document);
+    endInsertRows();
+
+    auto updateDisplay = [this, document]() {
+        int row = int(m_documents.indexOf(document));
+        emit dataChanged(index(row), index(row), { Qt::DisplayRole, Qt::ToolTipRole });
+    };
+    connect(document, &Document::filePathChanged,
+            this, updateDisplay);
+    connect(document, &Document::titleChanged,
+            this, updateDisplay);
+    connect(document->model(), &DocumentModel::modificationChanged,
+            this, updateDisplay);
+
+    emit documentAdded(document);
+    emit countChanged(count());
 }
 
 void DocumentList::remove(Document *document)

diff --git a/src/common/documentlist.h b/src/common/documentlist.h
@@ -40,6 +40,6 @@ class DocumentList : public QAbstractListModel
     QVector<Document *> m_documents;
     static DocumentList *s_inst;
 
-    friend Document::Document(DocumentModel *, const QByteArray &, QObject *);
+    friend Document *Document::create(DocumentModel *, const QByteArray &, bool, QObject *);
     friend Document::~Document();
 };
diff --git a/src/desktop/importorderdialog.cpp b/src/desktop/importorderdialog.cpp
@@ -297,7 +297,7 @@ void ImportOrderDialog::importOrders(const QModelIndexList &rows, bool combined)
         ++orderCount;
     }
     if (combined) {
-        auto doc = new Document(new DocumentModel(std::move(combinedPr))); // Document owns the items now
+        auto doc = Document::create(new DocumentModel(std::move(combinedPr))); // Document owns the items now
         doc->setTitle(tr("Multiple Orders"));
         doc->setThumbnail(u"view-financial-list"_qs);
     }