From 3a656c9c4afaf6a231529ca902499ab7a5592d74 Mon Sep 17 00:00:00 2001 From: Bryce! Date: Wed, 4 Nov 2026 11:33:49 -0500 Subject: [PATCH] Preserve installed card databases when Oracle refresh fails (#7739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Preserve installed card databases when Oracle refresh fails * Fix the Oracle refresh verdict and cover the parser write path A background refresh that already installed cards.xml exited 1 when the token save afterwards failed, so the client kept its old in-memory database and reported the update as failed. Return the same verdict main() already computes: a reject after the database was written is a successful update. Cover the writer Oracle actually uses: add a short-write death test for CockatriceXml4Parser::saveToFile, which reverted to QFile would truncate an installed database without any test noticing. Route a failed background token download through reject() instead of a modal box on a hidden wizard, which left the client waiting with no timeout and blocked later updates. Test the first non-space byte in place rather than copying the payload, and report an import refusal with the importer's own message. --------- Co-authored-by: Lukas Brübach --- .../card/database/parser/cockatrice_xml_3.cpp | 5 +- .../card/database/parser/cockatrice_xml_4.cpp | 5 +- oracle/CMakeLists.txt | 1 + oracle/src/database_writer.cpp | 52 +++++ oracle/src/database_writer.h | 17 ++ oracle/src/oracleimporter.cpp | 69 ++++-- oracle/src/oracleimporter.h | 22 ++ oracle/src/oraclewizard.cpp | 21 +- oracle/src/pages.cpp | 76 +++++-- oracle/src/pages.h | 1 + oracle/src/pagetemplates.cpp | 40 ++-- tests/carddatabase/carddatabase_test.cpp | 64 ++++++ tests/oracle/CMakeLists.txt | 1 + tests/oracle/oracle_importer_test.cpp | 215 ++++++++++++++++++ 14 files changed, 517 insertions(+), 72 deletions(-) create mode 100644 oracle/src/database_writer.cpp create mode 100644 oracle/src/database_writer.h diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp index d40c1c063..8d5735db7 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_3.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -499,7 +500,7 @@ bool CockatriceXml3Parser::saveToFile(FormatRulesNameMap _formats, { Q_UNUSED(_formats); - QFile file(fileName); + QSaveFile file(fileName); if (!file.open(QIODevice::WriteOnly)) { return false; } @@ -539,5 +540,5 @@ bool CockatriceXml3Parser::saveToFile(FormatRulesNameMap _formats, xml.writeEndElement(); // cockatrice_carddatabase xml.writeEndDocument(); - return true; + return !xml.hasError() && file.commit(); } diff --git a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp index 3ee1e808e..3965b80f0 100644 --- a/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp +++ b/libcockatrice_card/libcockatrice/card/database/parser/cockatrice_xml_4.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -682,7 +683,7 @@ bool CockatriceXml4Parser::saveToFile(FormatRulesNameMap _formats, const QString &sourceUrl, const QString &sourceVersion) { - QFile file(fileName); + QSaveFile file(fileName); if (!file.open(QIODevice::WriteOnly)) { return false; } @@ -730,5 +731,5 @@ bool CockatriceXml4Parser::saveToFile(FormatRulesNameMap _formats, xml.writeEndElement(); // cockatrice_carddatabase xml.writeEndDocument(); - return true; + return !xml.hasError() && file.commit(); } diff --git a/oracle/CMakeLists.txt b/oracle/CMakeLists.txt index 587463572..f68f51f23 100644 --- a/oracle/CMakeLists.txt +++ b/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 diff --git a/oracle/src/database_writer.cpp b/oracle/src/database_writer.cpp new file mode 100644 index 000000000..e80aa853a --- /dev/null +++ b/oracle/src/database_writer.cpp @@ -0,0 +1,52 @@ +#include "database_writer.h" + +#include +#include +#include +#include + +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 diff --git a/oracle/src/database_writer.h b/oracle/src/database_writer.h new file mode 100644 index 000000000..173fd23ab --- /dev/null +++ b/oracle/src/database_writer.h @@ -0,0 +1,17 @@ +#ifndef ORACLE_DATABASE_WRITER_H +#define ORACLE_DATABASE_WRITER_H + +#include +#include + +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 diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 477c54d21..c83c962e3 100644 --- a/oracle/src/oracleimporter.cpp +++ b/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(); diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index d872e184e..5ebe9cf36 100644 --- a/oracle/src/oracleimporter.h +++ b/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. * diff --git a/oracle/src/oraclewizard.cpp b/oracle/src/oraclewizard.cpp index c33d821a5..dc8c7ecb3 100644 --- a/oracle/src/oraclewizard.cpp +++ b/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,13 @@ 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 a verdict, + // using the same rule as main(): a reject after the database was written is still + // a successful update, only a reject that wrote nothing counts as a failure. + if (backgroundMode) { + QCoreApplication::exit(hasSavedDatabase() ? 0 : 1); + } } void OracleWizard::runInBackground() @@ -212,17 +220,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; } diff --git a/oracle/src/pages.cpp b/oracle/src/pages.cpp index 4c664a38c..f5c38be1c 100644 --- a/oracle/src/pages.cpp +++ b/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" @@ -43,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -169,7 +171,16 @@ LoadSetsResult loadSetsData(const QPointer &importer, QByteArray break; } - if (data.startsWith("<")) { + // Test the first non-space byte in place: trimmed() would duplicate the whole + // payload, which on the plain-XML path is the largest download we handle. + const auto firstNonSpace = std::find_if_not(data.cbegin(), data.cend(), + [](char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r'; }); + const bool looksLikeXml = firstNonSpace != data.cend() && *firstNonSpace == '<'; + if (looksLikeXml) { + result.errorMessage = OracleDatabase::validateXml(data); + if (!result.errorMessage.isEmpty()) { + return result; + } result.ok = true; result.plainXml = true; result.xmlData = std::move(data); @@ -651,6 +662,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 +732,7 @@ SaveSetsPage::SaveSetsPage(QWidget *parent) : OracleWizardPage(parent) bool SaveSetsPage::isComplete() const { - return !importActive; + return !importActive && importReady; } void SaveSetsPage::cleanupPage() @@ -730,8 +745,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 +775,9 @@ void SaveSetsPage::initializePage() emit completeChanged(); const QPointer importer = wizard()->importer; + if (auto *activeImporter = importer.data()) { + activeImporter->prepareImport(); + } importFuture = QtConcurrent::run([importer] { return importer ? importer->startImport() : 0; }); importWatcher.setFuture(importFuture); } @@ -788,22 +809,28 @@ void SaveSetsPage::importFinished() const int setsImported = importWatcher.result(); const QPointer 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 +881,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 +911,21 @@ 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; + // The importer refuses to write a database whose import did not finish, and + // reports the reason there; do not dress an import failure up as a disk failure. + const QString importError = wizard()->importer->getImportError(); + return failSave(importError.isEmpty() ? tr("The file could not be saved to %1").arg(fileName) : importError); } wizard()->markDatabaseSaved(); diff --git a/oracle/src/pages.h b/oracle/src/pages.h index 0530010ba..e29a8e84a 100644 --- a/oracle/src/pages.h +++ b/oracle/src/pages.h @@ -128,6 +128,7 @@ private: QFuture importFuture; int totalSets = 0; bool importActive = false; + bool importReady = false; protected: void initializePage() override; diff --git a/oracle/src/pagetemplates.cpp b/oracle/src/pagetemplates.cpp index dcc8e070c..7e5f30bb7 100644 --- a/oracle/src/pagetemplates.cpp +++ b/oracle/src/pagetemplates.cpp @@ -1,5 +1,6 @@ #include "pagetemplates.h" +#include "database_writer.h" #include "oraclewizard.h" #include @@ -184,9 +185,17 @@ void SimpleDownloadFilePage::actDownloadFinished() } QNetworkReply::NetworkError errorCode = reply->error(); if (errorCode != QNetworkReply::NoError) { - QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString())); - wizard()->enableButtons(); + const QString message = tr("Network error: %1.").arg(reply->errorString()); reply->deleteLater(); + // A hidden wizard cannot show a modal box: without an explicit stop it would sit + // here forever and the hosting client would wait with no timeout. + if (wizard()->backgroundMode) { + qWarning() << message; + wizard()->reject(); + return; + } + QMessageBox::critical(this, tr("Error"), message); + wizard()->enableButtons(); return; } @@ -218,6 +227,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 +254,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 +269,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; } diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index 22cbe9074..b54205f30 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -36,6 +36,12 @@ #include #include +#ifdef Q_OS_UNIX +#include +#include +#include +#endif + namespace { @@ -204,6 +210,64 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) ASSERT_EQ(printings.first().getProperty("manacost"), "1R"); } +#ifdef Q_OS_UNIX +/** + * @brief A failed write keeps the previously installed card database intact. + * + * This is the writer Oracle uses to install a refreshed database, so the truncation + * bug has to be covered here and not only through OracleDatabase::saveXml. + */ +TEST(CardDatabaseTest, Xml4ShortWriteKeepsInstalledFile) +{ + NoopCardSetPriorityController controller; + CardSetPtr set = + CardSet::newInstance(&controller, "TST", "Test Set", "expansion", QDate(), CardSet::PriorityPrimary); + + QHash props; + props["manacost"] = "1R"; + PrintingInfo printing(set, LazyPropertiesHash(props)); + SetToPrintingsMap setsInfo; + setsInfo["TST"].append(printing); + + CardInfo::UiAttributes attributes = {.tableRow = 1}; + CardInfoPtr card = + CardInfo::newInstance("Lightning Bolt", QString(1024, 'x'), false, {}, {}, {}, setsInfo, attributes); + + SetNameMap sets; + sets.insert("TST", set); + CardNameMap cards; + cards.insert("Lightning Bolt", card); + + QTemporaryDir tempDir; + ASSERT_TRUE(tempDir.isValid()); + const QString fileName = tempDir.filePath("cards.xml"); + + QFile installed(fileName); + ASSERT_TRUE(installed.open(QIODevice::WriteOnly)); + ASSERT_EQ(installed.write("installed database"), 18); + installed.close(); + + // A real filesystem write failure, isolated in the death-test child so the + // process-wide file limit never affects other cases or the parent runner. + ASSERT_EXIT( + { + std::signal(SIGXFSZ, SIG_IGN); + struct rlimit limit; + limit.rlim_cur = 128; + limit.rlim_max = 128; + if (setrlimit(RLIMIT_FSIZE, &limit) != 0) { + _exit(2); + } + CockatriceXml4Parser writer(&controller); + _exit(writer.saveToFile({}, sets, cards, fileName) ? 1 : 0); + }, + ::testing::ExitedWithCode(0), ""); + + ASSERT_TRUE(installed.open(QIODevice::ReadOnly)); + EXPECT_EQ(installed.readAll(), "installed database"); +} +#endif + /** * @brief Path provider serving a sandboxed copy of the test card database. */ diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index 6958673f7..1cc1ad952 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -9,6 +9,7 @@ cockatrice_add_test( ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp ../../oracle/src/raw_json_scanner.cpp + ../../oracle/src/database_writer.cpp oracle_importer_test.cpp LIBS libcockatrice_card diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp index 83d77d465..259bae087 100644 --- a/tests/oracle/oracle_importer_test.cpp +++ b/tests/oracle/oracle_importer_test.cpp @@ -1,3 +1,4 @@ +#include "../../oracle/src/database_writer.h" #include "../../oracle/src/oracleimporter.h" #include "../../oracle/src/raw_json_scanner.h" #include "libcockatrice/card/card_info.h" @@ -6,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -20,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +33,12 @@ #include #include +#ifdef Q_OS_UNIX +#include +#include +#include +#endif + class OracleImporterTest : public ::testing::Test { protected: @@ -1140,6 +1149,212 @@ TEST_F(OracleImporterTest, StartImportAppliesLocalizedData) ASSERT_EQ(result->getLocalizedText("pt"), "Raio causa 3 de dano a qualquer alvo."); } +namespace +{ +QByteArray refreshSource(const QString &name = "New Card") +{ + QJsonObject card{{"name", name}, {"text", "Rules text."}, {"layout", "normal"}, + {"type", "Artifact"}, {"number", "1"}, {"rarity", "common"}}; + QJsonObject set{{"code", "TST"}, {"name", "Test Set"}, {"type", "expansion"}, {"cards", QJsonArray{card}}}; + return QJsonDocument(QJsonObject{{"data", QJsonObject{{"TST", set}}}}).toJson(QJsonDocument::Compact); +} +} // namespace + +TEST_F(OracleImporterTest, FailedLazyParseKeepsInstalledDatabase) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = dir.filePath("cards.xml"); + QFile original(path); + ASSERT_TRUE(original.open(QIODevice::WriteOnly)); + ASSERT_EQ(original.write("existing database"), 17); + original.close(); + + // The metadata scanner skips numeric values without converting them. Qt's + // per-set parser rejects an overflowing number after selection succeeds. + auto badSet = QJsonDocument::fromJson(refreshSource()).object().value("data").toObject().value("TST").toObject(); + auto goodSet = badSet; + goodSet["code"] = "AAA"; + goodSet["name"] = "A successful first set"; + auto badCard = badSet.value("cards").toArray().first().toObject(); + badCard["number"] = "overflow"; + badSet["cards"] = QJsonArray{badCard}; + QByteArray data = QJsonDocument(QJsonObject{{"data", QJsonObject{{"AAA", goodSet}, {"TST", badSet}}}}) + .toJson(QJsonDocument::Compact); + data.replace("\"number\":\"overflow\"", "\"number\":1e9999"); + ASSERT_TRUE(importer->readSetsFromByteArray(data)); + EXPECT_EQ(importer->startImport(), 0); + EXPECT_FALSE(importer->hasSuccessfulImport()); + EXPECT_TRUE(importer->getImportError().contains("TST")); + EXPECT_TRUE(importer->getCardList().isEmpty()); + EXPECT_FALSE(importer->saveToFile(path, {}, {})); + ASSERT_TRUE(original.open(QIODevice::ReadOnly)); + EXPECT_EQ(original.readAll(), "existing database"); +} + +TEST_F(OracleImporterTest, EmptyImportCannotReplaceInstalledCards) +{ + const QByteArray source = R"({"data":{"TST":{"code":"TST","name":"Test","type":"expansion","cards":[]}}})"; + ASSERT_TRUE(importer->readSetsFromByteArray(source)); + EXPECT_EQ(importer->startImport(), 0); + EXPECT_FALSE(importer->hasSuccessfulImport()); + EXPECT_FALSE(importer->getImportError().isEmpty()); +} + +TEST_F(OracleImporterTest, MissingSourceBytesCannotProduceSuccessfulImport) +{ + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); + const auto selected = importer->getSets(); + importer->releaseSetData(); + importer->getSets() = selected; + EXPECT_EQ(importer->startImport(), 0); + EXPECT_FALSE(importer->hasSuccessfulImport()); + EXPECT_TRUE(importer->getImportError().contains("TST")); +} + +TEST_F(OracleImporterTest, CancellationDuringFinalSetCannotSavePartialDatabase) +{ + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); + QObject::connect( + importer, &OracleImporter::setIndexChanged, importer, + [this](int, int, const QString &name) { + if (!name.isEmpty()) { + importer->cancelImport(); + } + }, + Qt::DirectConnection); + EXPECT_EQ(importer->startImport(), 0); + EXPECT_FALSE(importer->hasSuccessfulImport()); + EXPECT_TRUE(importer->getCardList().isEmpty()); +} + +TEST_F(OracleImporterTest, CancellationBeforeWorkerStartsIsNotLost) +{ + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); + importer->prepareImport(); + importer->cancelImport(); + EXPECT_EQ(importer->startImport(), 0); + EXPECT_FALSE(importer->hasSuccessfulImport()); + importer->prepareImport(); + EXPECT_EQ(importer->startImport(), 1); + EXPECT_TRUE(importer->hasSuccessfulImport()); +} + +TEST_F(OracleImporterTest, NewImportReplacesPreviousInMemoryCards) +{ + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource("Old Card"))); + ASSERT_EQ(importer->startImport(), 1); + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource("New Card"))); + EXPECT_FALSE(importer->hasSuccessfulImport()); + ASSERT_EQ(importer->startImport(), 1); + EXPECT_EQ(importer->getCardList().size(), 1); + EXPECT_FALSE(importer->getCardList().contains("Old Card")); + EXPECT_TRUE(importer->getCardList().contains("New Card")); +} + +TEST_F(OracleImporterTest, FailedSourceLoadCannotReusePreviousSuccessfulImport) +{ + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); + ASSERT_EQ(importer->startImport(), 1); + ASSERT_FALSE(importer->readSetsFromByteArray("invalid JSON")); + EXPECT_FALSE(importer->hasSuccessfulImport()); + EXPECT_EQ(importer->startImport(), 0); +} + +TEST_F(OracleImporterTest, SuccessfulRefreshCommitsCompleteXml) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = dir.filePath("cards.xml"); + ASSERT_TRUE(importer->readSetsFromByteArray(refreshSource())); + ASSERT_EQ(importer->startImport(), 1); + importer->releaseSetData(); + ASSERT_TRUE(importer->saveToFile(path, "test-source", "v1")); + QFile file(path); + ASSERT_TRUE(file.open(QIODevice::ReadOnly)); + const QByteArray xml = file.readAll(); + EXPECT_TRUE(xml.contains("New Card")); + EXPECT_TRUE(xml.contains("test-source")); + EXPECT_TRUE(xml.contains("")); +} + +TEST(OracleDatabaseWriter, AcceptsCompleteSupportedVersions) +{ + EXPECT_TRUE(OracleDatabase::validateXml( + "") + .isEmpty()); + EXPECT_TRUE(OracleDatabase::validateXml("") + .isEmpty()); + EXPECT_FALSE( + OracleDatabase::validateXml("") + .isEmpty()); +} + +TEST(OracleDatabaseWriter, InvalidDownloadPreservesInstalledBytes) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = dir.filePath("tokens.xml"); + QFile original(path); + ASSERT_TRUE(original.open(QIODevice::WriteOnly)); + ASSERT_EQ(original.write("installed tokens"), 16); + original.close(); + for (const QByteArray &payload : {QByteArray("server error"), + QByteArray(""), QByteArray()}) { + QString error; + EXPECT_FALSE(OracleDatabase::saveXml(path, payload, &error)); + EXPECT_FALSE(error.isEmpty()); + ASSERT_TRUE(original.open(QIODevice::ReadOnly)); + EXPECT_EQ(original.readAll(), "installed tokens"); + original.close(); + } +} + +TEST(OracleDatabaseWriter, CompleteDownloadReplacesInstalledBytes) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = dir.filePath("cards.xml"); + const QByteArray data(""); + QString error = "previous failure"; + ASSERT_TRUE(OracleDatabase::saveXml(path, data, &error)); + EXPECT_TRUE(error.isEmpty()); + QFile file(path); + ASSERT_TRUE(file.open(QIODevice::ReadOnly)); + EXPECT_EQ(file.readAll(), data); +} + +#ifdef Q_OS_UNIX +TEST(OracleDatabaseWriter, ShortWriteKeepsPreviouslyInstalledDatabase) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString path = dir.filePath("cards.xml"); + QFile original(path); + ASSERT_TRUE(original.open(QIODevice::WriteOnly)); + ASSERT_EQ(original.write("installed database"), 18); + original.close(); + const QByteArray data = ""; + // A real filesystem write failure, isolated in the death-test child so the + // process-wide file limit never affects other cases or the parent runner. + ASSERT_EXIT( + { + std::signal(SIGXFSZ, SIG_IGN); + struct rlimit limit; + limit.rlim_cur = 128; + limit.rlim_max = 128; + if (setrlimit(RLIMIT_FSIZE, &limit) != 0) { + _exit(2); + } + _exit(OracleDatabase::saveXml(path, data) ? 1 : 0); + }, + ::testing::ExitedWithCode(0), ""); + ASSERT_TRUE(original.open(QIODevice::ReadOnly)); + EXPECT_EQ(original.readAll(), "installed database"); +} +#endif + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv);