Preserve installed card databases when Oracle refresh fails

oracle/CMakeLists.txt

@@ -17,6 +17,7 @@ set(ORACLE_WIN32_QM_INSTALL_DIR "translations")
 # Sources
 # ------------------------
 set(oracle_SOURCES
+    src/database_writer.cpp
     src/main.cpp
     src/oraclewizard.cpp
     src/oracleimporter.cpp

oracle/src/database_writer.cpp

@@ -0,0 +1,52 @@
+#include "database_writer.h"
+
+#include <QCoreApplication>
+#include <QIODevice>
+#include <QSaveFile>
+#include <QXmlStreamReader>
+
+namespace OracleDatabase
+{
+QString validateXml(const QByteArray &data)
+{
+    QXmlStreamReader xml(data);
+    if (!xml.readNextStartElement() || xml.name() != QLatin1String("cockatrice_carddatabase")) {
+        return QCoreApplication::translate("OracleDatabase", "The download is not a Cockatrice card database.");
+    }
+    const auto version = xml.attributes().value("version");
+    if (version != QLatin1String("3") && version != QLatin1String("4")) {
+        return QCoreApplication::translate("OracleDatabase", "The downloaded card database version is not supported.");
+    }
+    while (!xml.atEnd()) {
+        xml.readNext();
+    }
+    if (xml.hasError()) {
+        return QCoreApplication::translate("OracleDatabase",
+                                           "The downloaded card database is incomplete or invalid: %1")
+            .arg(xml.errorString());
+    }
+    return {};
+}
+
+bool saveXml(const QString &fileName, const QByteArray &data, QString *error)
+{
+    const QString validationError = validateXml(data);
+    if (!validationError.isEmpty()) {
+        if (error) {
+            *error = validationError;
+        }
+        return false;
+    }
+    QSaveFile file(fileName);
+    if (!file.open(QIODevice::WriteOnly) || file.write(data) != data.size() || !file.commit()) {
+        if (error) {
+            *error = file.errorString();
+        }
+        return false;
+    }
+    if (error) {
+        error->clear();
+    }
+    return true;
+}
+} // namespace OracleDatabase

oracle/src/database_writer.h

@@ -0,0 +1,17 @@
+#ifndef ORACLE_DATABASE_WRITER_H
+#define ORACLE_DATABASE_WRITER_H
+
+#include <QByteArray>
+#include <QString>
+
+namespace OracleDatabase
+{
+// Validate the complete downloaded document without allocating a card database.
+// Empty string means valid; otherwise returns a diagnostic suitable for the wizard.
+QString validateXml(const QByteArray &data);
+// Never truncate the installed database: validate, write a sibling temporary file,
+// check the complete write and atomically commit it.
+bool saveXml(const QString &fileName, const QByteArray &data, QString *error = nullptr);
+} // namespace OracleDatabase
+
+#endif

oracle/src/oracleimporter.cpp

@@ -100,6 +100,10 @@ static CardSet::Priority getSetPriority(const QString &setType, const QString &s
 
 bool OracleImporter::readSetsFromByteArray(QByteArray data)
 {
+    importSuccessful = false;
+    importError.clear();
+    allSets.clear();
+    rawSetsData.clear();
     const RawJson::ScanProgressCallback progress =
         progressReporting
             ? [this](
@@ -764,7 +768,15 @@ int OracleImporter::startImport()
 {
     static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController();
 
-    importCancelled.storeRelease(0);
+    importSuccessful = false;
+    importError.clear();
+    sets.clear();
+    cards.clear();
+    localizedEntries.clear();
+    splitLocalizedTexts.clear();
+    if (allSets.isEmpty()) {
+        return failImport(tr("No set has been selected for import."));
+    }
 
     // Pre-allocate the cards hash to avoid rehashing during import. Keys are
     // distinct card names while raw ranges only count printings (AllPrintings
@@ -786,9 +798,7 @@ int OracleImporter::startImport()
 
     for (const SetToDownload &curSetToParse : allSets) {
         if (importCancelled.loadAcquire()) {
-            // The wizard was closed mid-import: stop at the next set boundary so
-            // the caller can wait for this future without processing every set.
-            break;
+            return failImport(tr("The card database import was cancelled."));
         }
 
         CardSetPtr newSet = CardSet::newInstance(noOpController, curSetToParse.getShortName(),
@@ -798,30 +808,20 @@ int OracleImporter::startImport()
         // parse only this set's slice of the raw document so the whole JSON tree is
         // never kept in memory at once
         const RawJson::SetDataRange &rawRange = curSetToParse.getRawRange();
-        const qsizetype rangeEnd = rawRange.start + rawRange.length;
-        if (rawRange.start < 0 || rawRange.length <= 0 || rangeEnd > rawSetsData.size()) {
-            // rawSetsData is cleared by releaseSetData() while SetToDownload copies
-            // taken from getSets() keep their ranges, and nothing else enforces the
-            // pairing — so never index past the buffer on stale/mismatched ranges.
-            qWarning() << "error: out-of-bounds raw range for set" << curSetToParse.getShortName() << "skipping";
-            ++setIndex;
-            emit setIndexChanged(0, setIndex, curSetToParse.getLongName());
-            continue;
+        if (rawRange.start < 0 || rawRange.length <= 0 || rawRange.start > rawSetsData.size() ||
+            rawRange.length > rawSetsData.size() - rawRange.start) {
+            return failImport(tr("The source data for set %1 is unavailable. Load the sets file again.")
+                                  .arg(curSetToParse.getShortName()));
         }
-        // sliced() shares the buffer instead of deep-copying the slice; the largest
-        // sets in AllPrintings are tens of MB, so the copy is worth avoiding here.
         const QByteArray setBytes = rawSetsData.sliced(rawRange.start, rawRange.length);
         QJsonParseError parseError;
         const QJsonDocument setDoc = QJsonDocument::fromJson(setBytes, &parseError);
         if (parseError.error != QJsonParseError::NoError) {
-            qWarning() << "error: parsing card data for set" << curSetToParse.getShortName() << ":"
-                       << parseError.errorString();
-            ++setIndex;
-            // Keep the progress accounting honest: a set that failed to parse
-            // still advanced the index, so report it (with zero imported cards)
-            // rather than letting SaveSetsPage's bar stall per failed set.
-            emit setIndexChanged(0, setIndex, curSetToParse.getLongName());
-            continue;
+            return failImport(
+                tr("Could not import set %1: %2").arg(curSetToParse.getShortName(), parseError.errorString()));
+        }
+        if (!setDoc.isObject() || !setDoc.object().value("cards").isArray()) {
+            return failImport(tr("Set %1 does not contain a card list.").arg(curSetToParse.getShortName()));
         }
 
         // Only add the set to the database once its slice parsed cleanly;
@@ -838,7 +838,14 @@ int OracleImporter::startImport()
         emit setIndexChanged(numCardsInSet, setIndex, curSetToParse.getLongName());
     }
 
+    if (importCancelled.loadAcquire()) {
+        return failImport(tr("The card database import was cancelled."));
+    }
+    if (cards.isEmpty()) {
+        return failImport(tr("No cards were imported. The existing card database has been kept."));
+    }
     applyLocalizedData();
+    importSuccessful = true;
 
     emit setIndexChanged(0, setIndex, QString());
 
@@ -846,8 +853,23 @@ int OracleImporter::startImport()
     return setIndex;
 }
 
+int OracleImporter::failImport(const QString &message)
+{
+    importSuccessful = false;
+    importError = message;
+    sets.clear();
+    cards.clear();
+    localizedEntries.clear();
+    splitLocalizedTexts.clear();
+    qWarning() << message;
+    return 0;
+}
+
 bool OracleImporter::saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion)
 {
+    if (!importSuccessful) {
+        return false;
+    }
     CockatriceXml4Parser parser(new NoopCardSetPriorityController());
 
     return parser.saveToFile(createDefaultMagicFormats(), sets, cards, fileName, sourceUrl, sourceVersion);
@@ -861,6 +883,7 @@ void OracleImporter::releaseSetData()
 
 void OracleImporter::clear()
 {
+    prepareImport();
     sets.clear();
     cards.clear();
     allSets.clear();

oracle/src/oracleimporter.h

@@ -198,6 +198,11 @@ private:
      * the whole import (or racing it).
      */
     QAtomicInt importCancelled;
+    bool importSuccessful = false;
+    QString importError;
+
+    // Discard the incomplete in-memory database; the installed file is untouched.
+    int failImport(const QString &message);
 
     /**
      * The ISO-639 language code whose foreignData is imported; "en" by default.
@@ -278,7 +283,24 @@ public:
      * the wizard can hand over its decompressed buffer without copying it.
      */
     bool readSetsFromByteArray(QByteArray data);
+    // Returns the complete selected-set count on success, zero on failure or cancellation.
     int startImport();
+    bool hasSuccessfulImport() const
+    {
+        return importSuccessful;
+    }
+    const QString &getImportError() const
+    {
+        return importError;
+    }
+    // Call on the owning thread before scheduling a worker, so a cancellation
+    // arriving before that worker starts is not lost.
+    void prepareImport()
+    {
+        importCancelled.storeRelease(0);
+        importSuccessful = false;
+        importError.clear();
+    }
     /**
      * @brief Requests an in-flight startImport() to stop at the next set boundary.
      *

oracle/src/oraclewizard.cpp

@@ -1,6 +1,7 @@
 #include "oraclewizard.h"
 
 #include "client/settings/cache_settings.h"
+#include "database_writer.h"
 #include "main.h"
 #include "oracleimporter.h"
 #include "pages.h"
@@ -141,6 +142,11 @@ void OracleWizard::reject()
     }
 
     QWizard::reject();
+    // A hidden wizard does not close the application's last visible window.
+    // Stop background refreshes explicitly so the hosting client receives failure.
+    if (backgroundMode) {
+        QCoreApplication::exit(1);
+    }
 }
 
 void OracleWizard::runInBackground()
@@ -212,17 +218,10 @@ void OracleWizard::disableButtons()
 
 bool OracleWizard::saveTokensToFile(const QString &fileName)
 {
-    QFile file(fileName);
-    if (!file.open(QIODevice::WriteOnly)) {
-        qDebug() << "File open (w) failed for" << fileName;
+    QString error;
+    if (!OracleDatabase::saveXml(fileName, tokensData, &error)) {
+        qWarning() << error;
         return false;
     }
-
-    if (file.write(tokensData) == -1) {
-        qDebug() << "File write (w) failed for" << fileName;
-        return false;
-    }
-
-    file.close();
     return true;
 }

oracle/src/pages.cpp

@@ -1,6 +1,7 @@
 #include "pages.h"
 
 #include "client/settings/cache_settings.h"
+#include "database_writer.h"
 #include "main.h"
 #include "oracleimporter.h"
 #include "oraclewizard.h"
@@ -169,7 +170,11 @@ LoadSetsResult loadSetsData(const QPointer<OracleImporter> &importer, QByteArray
         break;
     }
 
-    if (data.startsWith("<")) {
+    if (data.trimmed().startsWith("<")) {
+        result.errorMessage = OracleDatabase::validateXml(data);
+        if (!result.errorMessage.isEmpty()) {
+            return result;
+        }
         result.ok = true;
         result.plainXml = true;
         result.xmlData = std::move(data);
@@ -651,6 +656,10 @@ void LoadSetsPage::importFinished()
         } else if (!result.ok && !result.plainXml) {
             qWarning() << tr("The file was retrieved successfully, but it does not contain any sets data.");
         }
+        if (!result.ok) {
+            wizard()->reject();
+            return;
+        }
         emit readyToContinue();
         return;
     }
@@ -717,7 +726,7 @@ SaveSetsPage::SaveSetsPage(QWidget *parent) : OracleWizardPage(parent)
 
 bool SaveSetsPage::isComplete() const
 {
-    return !importActive;
+    return !importActive && importReady;
 }
 
 void SaveSetsPage::cleanupPage()
@@ -730,8 +739,11 @@ void SaveSetsPage::cleanupPage()
 
 void SaveSetsPage::initializePage()
 {
+    importReady = false;
     retranslateUi();
     if (wizard()->downloadedPlainXml) {
+        importReady = true;
+        emit completeChanged();
         messageLog->hide();
         progressBar->hide();
         if (wizard()->backgroundMode) {
@@ -757,6 +769,9 @@ void SaveSetsPage::initializePage()
     emit completeChanged();
 
     const QPointer<OracleImporter> importer = wizard()->importer;
+    if (auto *activeImporter = importer.data()) {
+        activeImporter->prepareImport();
+    }
     importFuture = QtConcurrent::run([importer] { return importer ? importer->startImport() : 0; });
     importWatcher.setFuture(importFuture);
 }
@@ -788,22 +803,28 @@ void SaveSetsPage::importFinished()
 
     const int setsImported = importWatcher.result();
     const QPointer<OracleImporter> importer = wizard()->importer;
+    importReady = importer && importer->hasSuccessfulImport() && setsImported == totalSets;
+    const QString error = importer ? importer->getImportError() : tr("The card database importer is unavailable.");
     if (importer) {
         importer->releaseSetData();
     }
+    wizard()->button(QWizard::NextButton)->setEnabled(importReady);
+    emit completeChanged();
 
-    if (wizard()->backgroundMode) {
-        if (setsImported == 0) {
-            qWarning() << tr("No set has been imported.");
+    if (!importReady) {
+        const QString message = error.isEmpty() ? tr("No set has been imported.") : error;
+        if (wizard()->backgroundMode) {
+            qWarning() << message;
+            wizard()->reject();
+        } else {
+            QMessageBox::critical(this, tr("Error"), message);
         }
-        emit readyToContinue();
         return;
     }
 
     progressBar->setValue(progressBar->maximum());
-
-    if (setsImported == 0) {
-        QMessageBox::critical(this, tr("Error"), tr("No set has been imported."));
+    if (wizard()->backgroundMode) {
+        emit readyToContinue();
     }
 }
 
@@ -854,6 +875,18 @@ void SaveSetsPage::updateTotalProgress(int cardsImported, int setIndex, const QS
 
 bool SaveSetsPage::validatePage()
 {
+    if (!isComplete()) {
+        return false;
+    }
+    const auto failSave = [this](const QString &message) {
+        qWarning() << message;
+        if (wizard()->backgroundMode) {
+            wizard()->reject();
+        } else {
+            QMessageBox::critical(this, tr("Error"), message);
+        }
+        return false;
+    };
     QString defaultPath = SettingsCache::instance().getCardDatabasePath();
     QString windowName = tr("Save card database");
     QString fileType = tr("XML; card database (*.xml)");
@@ -872,24 +905,18 @@ bool SaveSetsPage::validatePage()
     QFileInfo fi(fileName);
     QDir fileDir(fi.path());
     if (!fileDir.exists() && !fileDir.mkpath(fileDir.absolutePath())) {
-        return false;
+        return failSave(tr("The database directory could not be created: %1").arg(fileDir.absolutePath()));
     }
 
     if (wizard()->downloadedPlainXml) {
-        QFile file(fileName);
-        if (!file.open(QIODevice::WriteOnly)) {
-            qDebug() << "File write (w) failed for" << fileName;
-            return false;
-        }
-        if (file.write(wizard()->xmlData) < 1) {
-            qDebug() << "File write (w) failed for" << fileName;
-            return false;
+        QString error;
+        if (!OracleDatabase::saveXml(fileName, wizard()->xmlData, &error)) {
+            return failSave(tr("The file could not be saved to %1: %2").arg(fileName, error));
         }
         wizard()->xmlData.clear();
     } else if (!wizard()->importer->saveToFile(fileName, wizard()->getCardSourceUrl(),
                                                wizard()->getCardSourceVersion())) {
-        QMessageBox::critical(this, tr("Error"), tr("The file could not be saved to %1").arg(fileName));
-        return false;
+        return failSave(tr("The file could not be saved to %1").arg(fileName));
     }
 
     wizard()->markDatabaseSaved();

oracle/src/pages.h

@@ -128,6 +128,7 @@ private:
     QFuture<int> importFuture;
     int totalSets = 0;
     bool importActive = false;
+    bool importReady = false;
 
 protected:
     void initializePage() override;

oracle/src/pagetemplates.cpp

@@ -1,5 +1,6 @@
 #include "pagetemplates.h"
 
+#include "database_writer.h"
 #include "oraclewizard.h"
 
 #include <QCheckBox>
@@ -218,6 +219,15 @@ void SimpleDownloadFilePage::actDownloadFinished()
 
 bool SimpleDownloadFilePage::saveToFile()
 {
+    const auto failSave = [this](const QString &message) {
+        qWarning() << message;
+        if (wizard()->backgroundMode) {
+            wizard()->reject();
+        } else {
+            QMessageBox::critical(this, tr("Error"), message);
+        }
+        return false;
+    };
     QString defaultPath = getDefaultSavePath();
     QString windowName = getWindowTitle();
     QString fileType = getFileType();
@@ -236,12 +246,11 @@ bool SimpleDownloadFilePage::saveToFile()
     QFileInfo fi(fileName);
     QDir fileDir(fi.path());
     if (!fileDir.exists() && !fileDir.mkpath(fileDir.absolutePath())) {
-        return false;
+        return failSave(tr("The database directory could not be created: %1").arg(fileDir.absolutePath()));
     }
 
     if (!internalSaveToFile(fileName)) {
-        QMessageBox::critical(this, tr("Error"), tr("The file could not be saved to %1").arg(fileName));
-        return false;
+        return failSave(tr("The file could not be saved to %1").arg(fileName));
     }
 
     // clean saved downloadData
@@ -252,17 +261,10 @@ bool SimpleDownloadFilePage::saveToFile()
 
 bool SimpleDownloadFilePage::internalSaveToFile(const QString &fileName)
 {
-    QFile file(fileName);
-    if (!file.open(QIODevice::WriteOnly)) {
-        qDebug() << "File open (w) failed for" << fileName;
+    QString error;
+    if (!OracleDatabase::saveXml(fileName, downloadData, &error)) {
+        qWarning() << error;
         return false;
     }
-
-    if (file.write(downloadData) == -1) {
-        qDebug() << "File write (w) failed for" << fileName;
-        return false;
-    }
-
-    file.close();
     return true;
 }