From 23bdc1d9866654edf6b4b2c2efb9489ba5d7c71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 30 Aug 2026 13:39:00 +0200 Subject: [PATCH 1/6] [Oracle] Replace vendored QtJson with native QJson for set import - Drop the vendored oracle/src/qt-json/json.{h,cpp} implementation - Switch SetToDownload and importCardsFromSet from QList to native QJsonArray/QJsonObject - Release set JSON data after import in the save sets page Took 20 minutes --- format.sh | 1 - oracle/CMakeLists.txt | 1 - oracle/src/oracleimporter.cpp | 143 ++++----- oracle/src/oracleimporter.h | 11 +- oracle/src/pages.cpp | 3 + oracle/src/qt-json/AUTHORS | 3 - oracle/src/qt-json/LICENSE | 27 -- oracle/src/qt-json/README | 96 ------ oracle/src/qt-json/json.cpp | 545 ---------------------------------- oracle/src/qt-json/json.h | 204 ------------- 10 files changed, 83 insertions(+), 951 deletions(-) delete mode 100644 oracle/src/qt-json/AUTHORS delete mode 100644 oracle/src/qt-json/LICENSE delete mode 100644 oracle/src/qt-json/README delete mode 100644 oracle/src/qt-json/json.cpp delete mode 100644 oracle/src/qt-json/json.h diff --git a/format.sh b/format.sh index 3fa435be1..ca3557ea7 100755 --- a/format.sh +++ b/format.sh @@ -22,7 +22,6 @@ libcockatrice_* \ exclude=("libcockatrice_rng/libcockatrice/rng/sfmt/" \ "libcockatrice_utility/libcockatrice/utility/peglib.h" \ "oracle/src/lzma/" \ -"oracle/src/qt-json/" \ "oracle/src/zip/" \ "servatrice/src/smtp/") exts=("cpp" "h" "proto") diff --git a/oracle/CMakeLists.txt b/oracle/CMakeLists.txt index 0736db7f5..6a29b6935 100644 --- a/oracle/CMakeLists.txt +++ b/oracle/CMakeLists.txt @@ -23,7 +23,6 @@ set(oracle_SOURCES src/pages.cpp src/pagetemplates.cpp src/parsehelpers.cpp - src/qt-json/json.cpp ../cockatrice/src/client/settings/cache_settings.cpp ../cockatrice/src/client/settings/card_counter_settings.cpp ../cockatrice/src/client/settings/shortcuts_settings.cpp diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index fdb32bb8d..a531b82d1 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -3,9 +3,10 @@ #include "libcockatrice/interfaces/noop_card_preference_provider.h" #include "libcockatrice/interfaces/noop_card_set_priority_controller.h" #include "parsehelpers.h" -#include "qt-json/json.h" #include +#include +#include #include #include #include @@ -44,24 +45,24 @@ static CardSet::Priority getSetPriority(const QString &setType, const QString &s bool OracleImporter::readSetsFromByteArray(const QByteArray &data) { - bool ok; - auto setsMap = QtJson::Json::parse(QString(data), ok).toMap().value("data").toMap(); - if (!ok) { - qDebug() << "error: QtJson::Json::parse()"; + QJsonParseError error; + auto doc = QJsonDocument::fromJson(data, &error); + if (error.error != QJsonParseError::NoError) { + qDebug() << "error: QJsonDocument::fromJson():" << error.errorString(); return false; } + auto setsObj = doc.object().value("data").toObject(); + QList newSetList; - QListIterator it(setsMap.values()); - - while (it.hasNext()) { - QVariantMap map = it.next().toMap(); - QString shortName = map.value("code").toString().toUpper(); - QString longName = map.value("name").toString(); - QList setCards = map.value("cards").toList(); - QString setType = map.value("type").toString(); - QDate releaseDate = map.value("releaseDate").toDate(); + for (auto it = setsObj.constBegin(); it != setsObj.constEnd(); ++it) { + QJsonObject setObj = it.value().toObject(); + QString shortName = setObj.value("code").toString().toUpper(); + QString longName = setObj.value("name").toString(); + QJsonArray setCards = setObj.value("cards").toArray(); + QString setType = setObj.value("type").toString(); + QDate releaseDate = QDate::fromString(setObj.value("releaseDate").toString(), Qt::ISODate); CardSet::Priority priority = getSetPriority(setType, shortName); // capitalize set type if (setType.length() > 0) { @@ -142,12 +143,11 @@ CardInfoPtr OracleImporter::addCard(QString name, // Workaround for card name weirdness name = name.replace("Æ", "AE"); name = name.replace("’", "'"); - if (cards.contains(name)) { - CardInfoPtr card = cards.value(name); + auto existingIt = cards.constFind(name); + if (existingIt != cards.constEnd()) { + CardInfoPtr card = existingIt.value(); card->addToSet(printingInfo.getSet(), printingInfo); - if (card->getProperties().filter(formatRegex).empty()) { - card->combineLegalities(properties); - } + card->combineLegalities(properties); return card; } @@ -222,12 +222,12 @@ CardInfoPtr OracleImporter::addCard(QString name, return newCard; } -static QString getStringPropertyFromMap(const QVariantMap &card, const QString &propertyName) +static QString getJsonString(const QJsonObject &obj, const QString &key) { - return card.contains(propertyName) ? card.value(propertyName).toString() : QString(""); + return obj.value(key).toString(); } -int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList &cardsList) +int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList) { // mtgjson name => xml name static const QMap cardProperties{ @@ -248,7 +248,7 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList static const QString ptSeparator = "/"; static constexpr bool isToken = false; - static const QList setsWithCardsWithSameNameButDifferentText = {"UST"}; + static const QSet setsWithCardsWithSameNameButDifferentText = {"UST"}; int numCards = 0; @@ -256,16 +256,16 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList QMap, QString>> splitCards; // Keeps track of all names encountered so far - QList allNameProps; + QSet allNameProps; - for (const QVariant &cardVar : cardsList) { - QVariantMap card = cardVar.toMap(); + for (const QJsonValue &cardVal : cardsList) { + QJsonObject card = cardVal.toObject(); /* Currently used layouts are: * augment, double_faced_token, flip, host, leveler, meld, normal, planar, * saga, scheme, split, token, transform, vanguard */ - QString layout = getStringPropertyFromMap(card, "layout"); + QString layout = getJsonString(card, "layout"); // don't import tokens from the json file if (layout == "token") { @@ -273,9 +273,9 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList } // normal cards handling - QString name = getStringPropertyFromMap(card, "name"); - QString text = getStringPropertyFromMap(card, "text"); - QString faceName = getStringPropertyFromMap(card, "faceName"); + QString name = getJsonString(card, "name"); + QString text = getJsonString(card, "text"); + QString faceName = getJsonString(card, "faceName"); if (faceName.isEmpty()) { faceName = name; } @@ -283,39 +283,34 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList // card properties QHash properties; for (auto i = cardProperties.cbegin(), end = cardProperties.cend(); i != end; ++i) { - QString mtgjsonProperty = i.key(); - QString xmlPropertyName = i.value(); - QString propertyValue = getStringPropertyFromMap(card, mtgjsonProperty); + QString propertyValue = getJsonString(card, i.key()); if (!propertyValue.isEmpty()) { - properties.insert(xmlPropertyName, propertyValue); + properties.insert(i.value(), propertyValue); } } // per-set properties QHash printingProps; for (auto i = setInfoProperties.cbegin(), end = setInfoProperties.cend(); i != end; ++i) { - QString mtgjsonProperty = i.key(); - QString xmlPropertyName = i.value(); - QString propertyValue = getStringPropertyFromMap(card, mtgjsonProperty); + QString propertyValue = getJsonString(card, i.key()); if (!propertyValue.isEmpty()) { - printingProps.insert(xmlPropertyName, propertyValue); + printingProps.insert(i.value(), propertyValue); } } // handle flavorNames specially due to double-faced cards - QString faceFlavorName = getStringPropertyFromMap(card, "faceFlavorName"); - QString flavorName = !faceFlavorName.isEmpty() ? faceFlavorName : getStringPropertyFromMap(card, "flavorName"); + QString faceFlavorName = getJsonString(card, "faceFlavorName"); + QString flavorName = !faceFlavorName.isEmpty() ? faceFlavorName : getJsonString(card, "flavorName"); if (!flavorName.isEmpty()) { printingProps.insert("flavorName", flavorName); } // Identifiers + QJsonObject identifiers = card.value("identifiers").toObject(); for (auto i = identifierProperties.cbegin(), end = identifierProperties.cend(); i != end; ++i) { - QString mtgjsonProperty = i.key(); - QString xmlPropertyName = i.value(); - QString propertyValue = getStringPropertyFromMap(card.value("identifiers").toMap(), mtgjsonProperty); + QString propertyValue = getJsonString(identifiers, i.key()); if (!propertyValue.isEmpty()) { - printingProps.insert(xmlPropertyName, propertyValue); + printingProps.insert(i.value(), propertyValue); } } @@ -331,21 +326,20 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList allNameProps.contains(faceName) && layout == "normal" && lastChar.isLetter()) { numComponent = " (" + QString(lastChar).toLower() + ")"; } - allNameProps.append(faceName); + allNameProps.insert(faceName); // special handling properties - QString colors = card.value("colors").toStringList().join(""); + QString colors = card.value("colors").toVariant().toStringList().join(""); if (!colors.isEmpty()) { properties.insert("colors", colors); } - // special handling properties - QString colorIdentity = card.value("colorIdentity").toStringList().join(""); + QString colorIdentity = card.value("colorIdentity").toVariant().toStringList().join(""); if (!colorIdentity.isEmpty()) { properties.insert("coloridentity", colorIdentity); } - const auto &mainCardType = getMainCardType(card.value("types").toStringList()); + const auto &mainCardType = getMainCardType(card.value("types").toVariant().toStringList()); if (mainCardType.isEmpty()) { qDebug() << "warning: no mainCardType for card:" << name; } else { @@ -354,22 +348,22 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList // Depending on whether power and/or toughness are present, the format // is either P/T (most common), P (no toughness), or /T (no power). - QString power = getStringPropertyFromMap(card, "power"); - QString toughness = getStringPropertyFromMap(card, "toughness"); + QString power = getJsonString(card, "power"); + QString toughness = getJsonString(card, "toughness"); if (toughness.isEmpty() && !power.isEmpty()) { properties.insert("pt", power); } else if (!toughness.isEmpty()) { properties.insert("pt", power + ptSeparator + toughness); } - auto legalities = card.value("legalities").toMap(); - for (auto i = legalities.cbegin(), end = legalities.cend(); i != end; ++i) { + auto legalities = card.value("legalities").toObject(); + for (auto i = legalities.constBegin(), end = legalities.constEnd(); i != end; ++i) { properties.insert(QString("format-%1").arg(i.key()), i.value().toString().toLower()); } // split cards are considered a single card, enqueue for later merging if (layout == "split" || layout == "aftermath" || layout == "adventure" || layout == "prepare") { - auto _faceName = getStringPropertyFromMap(card, "faceName"); + auto _faceName = getJsonString(card, "faceName"); SplitCardPart split(_faceName, text, properties, printingInfo); auto found_iter = splitCards.find(name + numProperty); if (found_iter == splitCards.end()) { @@ -382,11 +376,11 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList QList relatedCards; // add other face for split cards as card relation - if (!getStringPropertyFromMap(card, "side").isEmpty()) { - auto faceManaValue = getStringPropertyFromMap(card, "faceManaValue"); + if (!getJsonString(card, "side").isEmpty()) { + auto faceManaValue = getJsonString(card, "faceManaValue"); if (faceManaValue.isEmpty()) { // check the old name for the property, for backwards compatibility purposes - faceManaValue = getStringPropertyFromMap(card, "faceConvertedManaCost"); + faceManaValue = getJsonString(card, "faceConvertedManaCost"); } properties["cmc"] = faceManaValue; @@ -406,15 +400,15 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList name = faceName; } - // mtgjon related cards - if (card.contains("relatedCards")) { - QVariantMap givenRelated = card.value("relatedCards").toMap(); + // mtgjson related cards + QJsonObject givenRelated = card.value("relatedCards").toObject(); + if (!givenRelated.isEmpty()) { // conjured cards from a spellbook - if (givenRelated.contains("spellbook")) { - auto spbk = givenRelated.value("spellbook").toStringList(); - for (const QString &spbkName : spbk) { - relatedCards.append( - new CardRelation(spbkName, CardRelationType::DoesNotAttach, false, false, 1, true)); + QJsonArray spellbook = givenRelated.value("spellbook").toArray(); + if (!spellbook.isEmpty()) { + for (const QJsonValue &spbkVal : spellbook) { + relatedCards.append(new CardRelation(spbkVal.toString(), CardRelationType::DoesNotAttach, false, + false, 1, true)); } } } @@ -427,7 +421,6 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList // split cards handling static const QString splitCardPropSeparator = QString(" // "); static const QString splitCardTextSeparator = QString("\n\n---\n\n"); - static const QList noRelatedCards = {}; QList, QString>> partsAndNames = splitCards.values(); for (auto [splitCardParts, name] : partsAndNames) { @@ -465,20 +458,20 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QList } } } - CardInfoPtr newCard = addCard(name, text, isToken, properties, noRelatedCards, printingInfo); + CardInfoPtr newCard = addCard(name, text, isToken, properties, {}, printingInfo); numCards++; } return numCards; } -FormatRulesNameMap OracleImporter::createDefaultMagicFormats() +static FormatRulesNameMap buildDefaultMagicFormats() { // Predefined common exceptions CardCondition superTypeIsBasic; superTypeIsBasic.field = "type"; superTypeIsBasic.matchType = "regex"; - superTypeIsBasic.value = "\bBasic\b[^—]+\bLand\b"; + superTypeIsBasic.value = R"(\bBasic\b[^—]+\bLand\b)"; ExceptionRule basicLands; basicLands.conditions.append(superTypeIsBasic); @@ -491,7 +484,6 @@ FormatRulesNameMap OracleImporter::createDefaultMagicFormats() ExceptionRule mayContainAnyNumber; mayContainAnyNumber.conditions.append(anyNumberAllowed); - // Map to store default rules FormatRulesNameMap defaultFormatRulesNameMap; // ----------------- Helper lambda to create format ----------------- @@ -537,6 +529,12 @@ FormatRulesNameMap OracleImporter::createDefaultMagicFormats() return defaultFormatRulesNameMap; } +FormatRulesNameMap OracleImporter::createDefaultMagicFormats() +{ + static const FormatRulesNameMap cached = buildDefaultMagicFormats(); + return cached; +} + int OracleImporter::startImport() { static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController(); @@ -576,6 +574,11 @@ bool OracleImporter::saveToFile(const QString &fileName, const QString &sourceUr return parser.saveToFile(createDefaultMagicFormats(), sets, cards, fileName, sourceUrl, sourceVersion); } +void OracleImporter::releaseSetData() +{ + allSets.clear(); +} + void OracleImporter::clear() { sets.clear(); diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index 99644f9ce..696e35cab 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -1,6 +1,8 @@ #ifndef ORACLEIMPORTER_H #define ORACLEIMPORTER_H +#include +#include #include #include #include @@ -44,7 +46,7 @@ class SetToDownload { private: QString shortName, longName; - QList cards; + QJsonArray cards; QDate releaseDate; QString setType; CardSet::Priority priority; @@ -58,7 +60,7 @@ public: { return longName; } - const QList &getCards() const + const QJsonArray &getCards() const { return cards; } @@ -76,7 +78,7 @@ public: } SetToDownload(QString _shortName, QString _longName, - QList _cards, + QJsonArray _cards, CardSet::Priority _priority, QString _setType = QString(), const QDate &_releaseDate = QDate()) @@ -154,7 +156,7 @@ public: bool readSetsFromByteArray(const QByteArray &data); int startImport(); bool saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion); - int importCardsFromSet(const CardSetPtr ¤tSet, const QList &cardsList); + int importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList); FormatRulesNameMap createDefaultMagicFormats(); const CardNameMap &getCardList() const { @@ -164,6 +166,7 @@ public: { return allSets; } + void releaseSetData(); void clear(); }; diff --git a/oracle/src/pages.cpp b/oracle/src/pages.cpp index df4d1a98c..94e662ffe 100644 --- a/oracle/src/pages.cpp +++ b/oracle/src/pages.cpp @@ -560,6 +560,9 @@ void SaveSetsPage::initializePage() int setsImported = wizard()->importer->startImport(); + // JSON data no longer needed after CardInfo objects are built + wizard()->importer->releaseSetData(); + if (setsImported == 0) { QMessageBox::critical(this, tr("Error"), tr("No set has been imported.")); } diff --git a/oracle/src/qt-json/AUTHORS b/oracle/src/qt-json/AUTHORS deleted file mode 100644 index 29a85929f..000000000 --- a/oracle/src/qt-json/AUTHORS +++ /dev/null @@ -1,3 +0,0 @@ -Eeli Reilin -Luis Gustavo S. Barreto -Stephen Kockentiedt diff --git a/oracle/src/qt-json/LICENSE b/oracle/src/qt-json/LICENSE deleted file mode 100644 index 3c42b515a..000000000 --- a/oracle/src/qt-json/LICENSE +++ /dev/null @@ -1,27 +0,0 @@ -Copyright 2011 Eeli Reilin. All rights reserved. - -Redistribution and use in source and binary forms, with or without modification, -are permitted provided that the following conditions are met: - - 1. Redistributions of source code must retain the above copyright notice, - this list of conditions and the following disclaimer. - - 2. Redistributions in binary form must reproduce the above copyright notice, - this list of conditions and the following disclaimer in the documentation - and/or other materials provided with the distribution. - -THIS SOFTWARE IS PROVIDED BY ''AS IS'' AND ANY EXPRESS OR -IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF -MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO -EVENT SHALL EELI REILIN OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, -INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, -OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF -LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE -OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF -ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -The views and conclusions contained in the software and documentation -are those of the authors and should not be interpreted as representing -official policies, either expressed or implied, of Eeli Reilin. - diff --git a/oracle/src/qt-json/README b/oracle/src/qt-json/README deleted file mode 100644 index b60c1599b..000000000 --- a/oracle/src/qt-json/README +++ /dev/null @@ -1,96 +0,0 @@ -######################################################################## -1. INTRODUCTION - -The Json class is a simple class for parsing JSON data into a QVariant -hierarchies. Now, we can also reverse the process and serialize -QVariant hierarchies into valid JSON data. - - -######################################################################## -2. HOW TO USE - -The parser is really easy to use. Let's say we have the following -QString of JSON data: - ------------------------------------------------------------------------- -{ - "encoding" : "UTF-8", - "plug-ins" : [ - "python", - "c++", - "ruby" - ], - "indent" : { - "length" : 3, - "use_space" : true - } -} ------------------------------------------------------------------------- - -We would first call the parse-method: - ------------------------------------------------------------------------- -//Say that we're using the QtJson namespace -using namespace QtJson; -bool ok; -//json is a QString containing the JSON data -QVariantMap result = Json::parse(json, ok).toMap(); - -if(!ok) { - qFatal("An error occurred during parsing"); - exit(1); -} ------------------------------------------------------------------------- - -Assuming the parsing process completed without errors, we would then -go through the hierarchy: - ------------------------------------------------------------------------- -qDebug() << "encoding:" << result["encoding"].toString(); -qDebug() << "plugins:"; - -foreach(QVariant plugin, result["plug-ins"].toList()) { - qDebug() << "\t-" << plugin.toString(); -} - -QVariantMap nestedMap = result["indent"].toMap(); -qDebug() << "length:" << nestedMap["length"].toInt(); -qDebug() << "use_space:" << nestedMap["use_space"].toBool(); ------------------------------------------------------------------------- - -The previous code would print out the following: - ------------------------------------------------------------------------- -encoding: "UTF-8" -plugins: - - "python" - - "c++" - - "ruby" -length: 3 -use_space: true ------------------------------------------------------------------------- - -To write JSON data from Qt object is as simple as parsing: - ------------------------------------------------------------------------- -QVariantMap map; -map["name"] = "Name"; -map["age"] = 22; - -QByteArray data = Json::serialize(map); ------------------------------------------------------------------------- - -The byte array 'data' contains valid JSON data: - ------------------------------------------------------------------------- -{ - name: "Luis Gustavo", - age: 22, -} ------------------------------------------------------------------------- - - -######################################################################## -4. CONTRIBUTING - -The code is available to download at GitHub. Contribute if you dare! diff --git a/oracle/src/qt-json/json.cpp b/oracle/src/qt-json/json.cpp deleted file mode 100644 index ff739b49d..000000000 --- a/oracle/src/qt-json/json.cpp +++ /dev/null @@ -1,545 +0,0 @@ -/* Copyright 2011 Eeli Reilin. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY ''AS IS'' AND ANY EXPRESS OR - * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO - * EVENT SHALL EELI REILIN OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, - * OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, - * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * The views and conclusions contained in the software and documentation - * are those of the authors and should not be interpreted as representing - * official policies, either expressed or implied, of Eeli Reilin. - */ - -/** - * \file json.cpp - */ - -#include "json.h" - -#include -#include - -namespace QtJson -{ - -static QString sanitizeString(QString str) -{ - str.replace(QLatin1String("\\"), QLatin1String("\\\\")); - str.replace(QLatin1String("\""), QLatin1String("\\\"")); - str.replace(QLatin1String("\b"), QLatin1String("\\b")); - str.replace(QLatin1String("\f"), QLatin1String("\\f")); - str.replace(QLatin1String("\n"), QLatin1String("\\n")); - str.replace(QLatin1String("\r"), QLatin1String("\\r")); - str.replace(QLatin1String("\t"), QLatin1String("\\t")); - return QString(QLatin1String("\"%1\"")).arg(str); -} - -static QByteArray join(const QList &list, const QByteArray &sep) -{ - QByteArray res; - for (const QByteArray &i : list) { - if (!res.isEmpty()) { - res += sep; - } - res += i; - } - return res; -} - -/** - * parse - */ -QVariant Json::parse(const QString &json) -{ - bool success = true; - return Json::parse(json, success); -} - -/** - * parse - */ -QVariant Json::parse(const QString &json, bool &success) -{ - success = true; - - // Return an empty QVariant if the JSON data is either null or empty - if (!json.isNull() || !json.isEmpty()) { - // We'll start from index 0 - int index = 0; - - // Parse the first value - QVariant value = Json::parseValue(json, index, success); - - // Return the parsed value - return value; - } else { - // Return the empty QVariant - return QVariant(); - } -} - -QByteArray Json::serialize(const QVariant &data) -{ - bool success = true; - return Json::serialize(data, success); -} - -QByteArray Json::serialize(const QVariant &data, bool &success) -{ - QByteArray str; - success = true; - - if (!data.isValid()) // invalid or null? - { - str = "null"; - } - else if ((data.typeId() == QMetaType::Type::QVariantList) || - (data.typeId() == QMetaType::Type::QStringList)) // variant is a list? - { - QList values; - const QVariantList list = data.toList(); - for (const QVariant &v : list) { - QByteArray serializedValue = serialize(v); - if (serializedValue.isNull()) { - success = false; - break; - } - values << serializedValue; - } - - str = "[ " + join(values, ", ") + " ]"; - } - else if ((data.typeId() == QMetaType::Type::QVariantHash)) // variant is a hash? - { - const QVariantHash vhash = data.toHash(); - QHashIterator it(vhash); - str = "{ "; - QList pairs; - - while (it.hasNext()) { - it.next(); - QByteArray serializedValue = serialize(it.value()); - - if (serializedValue.isNull()) { - success = false; - break; - } - - pairs << sanitizeString(it.key()).toUtf8() + " : " + serializedValue; - } - - str += join(pairs, ", "); - str += " }"; - } - else if ((data.typeId() == QMetaType::Type::QVariantMap)) // variant is a map? - { - const QVariantMap vmap = data.toMap(); - QMapIterator it(vmap); - str = "{ "; - QList pairs; - while (it.hasNext()) { - it.next(); - QByteArray serializedValue = serialize(it.value()); - if (serializedValue.isNull()) { - success = false; - break; - } - pairs << sanitizeString(it.key()).toUtf8() + " : " + serializedValue; - } - str += join(pairs, ", "); - str += " }"; - } - else if ((data.typeId() == QMetaType::Type::QString) || - (data.typeId() == QMetaType::Type::QByteArray)) // a string or a byte array? - { - str = sanitizeString(data.toString()).toUtf8(); - } - else if (data.typeId() == QMetaType::Type::Double) // double? - { - str = QByteArray::number(data.toDouble(), 'g', 20); - if (!str.contains(".") && !str.contains("e")) { - str += ".0"; - } - } - else if (data.typeId() == QMetaType::Type::Bool) // boolean value? - { - str = data.toBool() ? "true" : "false"; - } - else if (data.typeId() == QMetaType::Type::ULongLong) // large unsigned number? - { - str = QByteArray::number(data.value()); - } else if (data.canConvert()) // any signed number? - { - str = QByteArray::number(data.value()); - } else if (data.canConvert()) { - str = QString::number(data.value()).toUtf8(); - } else if (data.canConvert()) // can value be converted to string? - { - // this will catch QDate, QDateTime, QUrl, ... - str = sanitizeString(data.toString()).toUtf8(); - } else { - success = false; - } - if (success) { - return str; - } else { - return QByteArray(); - } -} - -/** - * parseValue - */ -QVariant Json::parseValue(const QString &json, int &index, bool &success) -{ - // Determine what kind of data we should parse by - // checking out the upcoming token - switch (Json::lookAhead(json, index)) { - case JsonTokenString: - return Json::parseString(json, index, success); - case JsonTokenNumber: - return Json::parseNumber(json, index); - case JsonTokenCurlyOpen: - return Json::parseObject(json, index, success); - case JsonTokenSquaredOpen: - return Json::parseArray(json, index, success); - case JsonTokenTrue: - Json::nextToken(json, index); - return QVariant(true); - case JsonTokenFalse: - Json::nextToken(json, index); - return QVariant(false); - case JsonTokenNull: - Json::nextToken(json, index); - return QVariant(); - case JsonTokenNone: - break; - } - - // If there were no tokens, flag the failure and return an empty QVariant - success = false; - return QVariant(); -} - -/** - * parseObject - */ -QVariant Json::parseObject(const QString &json, int &index, bool &success) -{ - QVariantMap map; - int token; - - // Get rid of the whitespace and increment index - Json::nextToken(json, index); - - // Loop through all of the key/value pairs of the object - bool done = false; - while (!done) { - // Get the upcoming token - token = Json::lookAhead(json, index); - - if (token == JsonTokenNone) { - success = false; - return QVariantMap(); - } else if (token == JsonTokenComma) { - Json::nextToken(json, index); - } else if (token == JsonTokenCurlyClose) { - Json::nextToken(json, index); - return map; - } else { - // Parse the key/value pair's name - QString name = Json::parseString(json, index, success).toString(); - - if (!success) { - return QVariantMap(); - } - - // Get the next token - token = Json::nextToken(json, index); - - // If the next token is not a colon, flag the failure - // return an empty QVariant - if (token != JsonTokenColon) { - success = false; - return QVariant(QVariantMap()); - } - - // Parse the key/value pair's value - QVariant value = Json::parseValue(json, index, success); - - if (!success) { - return QVariantMap(); - } - - // Assign the value to the key in the map - map[name] = value; - } - } - - // Return the map successfully - return QVariant(map); -} - -/** - * parseArray - */ -QVariant Json::parseArray(const QString &json, int &index, bool &success) -{ - QVariantList list; - - Json::nextToken(json, index); - - bool done = false; - while (!done) { - int token = Json::lookAhead(json, index); - - if (token == JsonTokenNone) { - success = false; - return QVariantList(); - } else if (token == JsonTokenComma) { - Json::nextToken(json, index); - } else if (token == JsonTokenSquaredClose) { - Json::nextToken(json, index); - break; - } else { - QVariant value = Json::parseValue(json, index, success); - - if (!success) { - return QVariantList(); - } - - list.push_back(value); - } - } - - return QVariant(list); -} - -/** - * parseString - */ -QVariant Json::parseString(const QString &json, int &index, bool &success) -{ - QString s; - QChar c; - - Json::eatWhitespace(json, index); - - c = json[index++]; - - bool complete = false; - while (!complete) { - if (index == json.size()) { - break; - } - - c = json[index++]; - - if (c == '\"') { - complete = true; - break; - } else if (c == '\\') { - if (index == json.size()) { - break; - } - - c = json[index++]; - - if (c == '\"') { - s.append('\"'); - } else if (c == '\\') { - s.append('\\'); - } else if (c == '/') { - s.append('/'); - } else if (c == 'b') { - s.append('\b'); - } else if (c == 'f') { - s.append('\f'); - } else if (c == 'n') { - s.append('\n'); - } else if (c == 'r') { - s.append('\r'); - } else if (c == 't') { - s.append('\t'); - } else if (c == 'u') { - int remainingLength = json.size() - index; - - if (remainingLength >= 4) { - QString unicodeStr = json.mid(index, 4); - - int symbol = unicodeStr.toInt(0, 16); - - s.append(QChar(symbol)); - - index += 4; - } else { - break; - } - } - } else { - s.append(c); - } - } - - if (!complete) { - success = false; - return QVariant(); - } - - return QVariant(s); -} - -/** - * parseNumber - */ -QVariant Json::parseNumber(const QString &json, int &index) -{ - Json::eatWhitespace(json, index); - - int lastIndex = Json::lastIndexOfNumber(json, index); - int charLength = (lastIndex - index) + 1; - QString numberStr; - - numberStr = json.mid(index, charLength); - - index = lastIndex + 1; - - if (numberStr.contains('.')) { - return QVariant(numberStr.toDouble(NULL)); - } else if (numberStr.startsWith('-')) { - return QVariant(numberStr.toLongLong(NULL)); - } else { - return QVariant(numberStr.toULongLong(NULL)); - } -} - -/** - * lastIndexOfNumber - */ -int Json::lastIndexOfNumber(const QString &json, int index) -{ - static const QString numericCharacters("0123456789+-.eE"); - int lastIndex; - - for (lastIndex = index; lastIndex < json.size(); lastIndex++) { - if (numericCharacters.indexOf(json[lastIndex]) == -1) { - break; - } - } - - return lastIndex - 1; -} - -/** - * eatWhitespace - */ -void Json::eatWhitespace(const QString &json, int &index) -{ - static const QString whitespaceChars(" \t\n\r"); - for (; index < json.size(); index++) { - if (whitespaceChars.indexOf(json[index]) == -1) { - break; - } - } -} - -/** - * lookAhead - */ -int Json::lookAhead(const QString &json, int index) -{ - int saveIndex = index; - return Json::nextToken(json, saveIndex); -} - -/** - * nextToken - */ -int Json::nextToken(const QString &json, int &index) -{ - Json::eatWhitespace(json, index); - - if (index == json.size()) { - return JsonTokenNone; - } - - QChar c = json[index]; - index++; - switch (c.toLatin1()) { - case '{': - return JsonTokenCurlyOpen; - case '}': - return JsonTokenCurlyClose; - case '[': - return JsonTokenSquaredOpen; - case ']': - return JsonTokenSquaredClose; - case ',': - return JsonTokenComma; - case '"': - return JsonTokenString; - case '0': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - case '-': - return JsonTokenNumber; - case ':': - return JsonTokenColon; - } - - index--; - - int remainingLength = json.size() - index; - - // True - if (remainingLength >= 4) { - if (json[index] == 't' && json[index + 1] == 'r' && json[index + 2] == 'u' && json[index + 3] == 'e') { - index += 4; - return JsonTokenTrue; - } - } - - // False - if (remainingLength >= 5) { - if (json[index] == 'f' && json[index + 1] == 'a' && json[index + 2] == 'l' && json[index + 3] == 's' && - json[index + 4] == 'e') { - index += 5; - return JsonTokenFalse; - } - } - - // Null - if (remainingLength >= 4) { - if (json[index] == 'n' && json[index + 1] == 'u' && json[index + 2] == 'l' && json[index + 3] == 'l') { - index += 4; - return JsonTokenNull; - } - } - - return JsonTokenNone; -} - -} // namespace QtJson diff --git a/oracle/src/qt-json/json.h b/oracle/src/qt-json/json.h deleted file mode 100644 index cf0499d4e..000000000 --- a/oracle/src/qt-json/json.h +++ /dev/null @@ -1,204 +0,0 @@ -/* Copyright 2011 Eeli Reilin. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY ''AS IS'' AND ANY EXPRESS OR - * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO - * EVENT SHALL EELI REILIN OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, - * OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, - * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * The views and conclusions contained in the software and documentation - * are those of the authors and should not be interpreted as representing - * official policies, either expressed or implied, of Eeli Reilin. - */ - -/** - * \file json.h - */ - -#ifndef JSON_H -#define JSON_H - -#include -#include - -namespace QtJson -{ - -/** - * \enum JsonToken - */ -enum JsonToken -{ - JsonTokenNone = 0, - JsonTokenCurlyOpen = 1, - JsonTokenCurlyClose = 2, - JsonTokenSquaredOpen = 3, - JsonTokenSquaredClose = 4, - JsonTokenColon = 5, - JsonTokenComma = 6, - JsonTokenString = 7, - JsonTokenNumber = 8, - JsonTokenTrue = 9, - JsonTokenFalse = 10, - JsonTokenNull = 11 -}; - -/** - * \class Json - * \brief A JSON data parser - * - * Json parses a JSON data into a QVariant hierarchy. - */ -class Json -{ - public: - /** - * Parse a JSON string - * - * \param json The JSON data - */ - static QVariant parse(const QString &json); - - /** - * Parse a JSON string - * - * \param json The JSON data - * \param success The success of the parsing - */ - static QVariant parse(const QString &json, bool &success); - - /** - * This method generates a textual JSON representation - * - * \param data The JSON data generated by the parser. - * \param success The success of the serialization - */ - static QByteArray serialize(const QVariant &data); - - /** - * This method generates a textual JSON representation - * - * \param data The JSON data generated by the parser. - * \param success The success of the serialization - * - * \return QByteArray Textual JSON representation - */ - static QByteArray serialize(const QVariant &data, bool &success); - - private: - /** - * Parses a value starting from index - * - * \param json The JSON data - * \param index The start index - * \param success The success of the parse process - * - * \return QVariant The parsed value - */ - static QVariant parseValue(const QString &json, int &index, - bool &success); - - /** - * Parses an object starting from index - * - * \param json The JSON data - * \param index The start index - * \param success The success of the object parse - * - * \return QVariant The parsed object map - */ - static QVariant parseObject(const QString &json, int &index, - bool &success); - - /** - * Parses an array starting from index - * - * \param json The JSON data - * \param index The starting index - * \param success The success of the array parse - * - * \return QVariant The parsed variant array - */ - static QVariant parseArray(const QString &json, int &index, - bool &success); - - /** - * Parses a string starting from index - * - * \param json The JSON data - * \param index The starting index - * \param success The success of the string parse - * - * \return QVariant The parsed string - */ - static QVariant parseString(const QString &json, int &index, - bool &success); - - /** - * Parses a number starting from index - * - * \param json The JSON data - * \param index The starting index - * - * \return QVariant The parsed number - */ - static QVariant parseNumber(const QString &json, int &index); - - /** - * Get the last index of a number starting from index - * - * \param json The JSON data - * \param index The starting index - * - * \return The last index of the number - */ - static int lastIndexOfNumber(const QString &json, int index); - - /** - * Skip unwanted whitespace symbols starting from index - * - * \param json The JSON data - * \param index The start index - */ - static void eatWhitespace(const QString &json, int &index); - - /** - * Check what token lies ahead - * - * \param json The JSON data - * \param index The starting index - * - * \return int The upcoming token - */ - static int lookAhead(const QString &json, int index); - - /** - * Get the next JSON token - * - * \param json The JSON data - * \param index The starting index - * - * \return int The next JSON token - */ - static int nextToken(const QString &json, int &index); -}; - - -} //end namespace - -#endif //JSON_H From b16497327fffbcbbe83b6b4026ec27d58b4f7dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 30 Aug 2026 22:54:12 +0200 Subject: [PATCH 2/6] [Oracle] Restore property coercion and legality merge in native JSON import --- oracle/src/oracleimporter.cpp | 23 ++++++++++++++++++----- oracle/src/oracleimporter.h | 5 ++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index a531b82d1..752317512 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -147,7 +147,11 @@ CardInfoPtr OracleImporter::addCard(QString name, if (existingIt != cards.constEnd()) { CardInfoPtr card = existingIt.value(); card->addToSet(printingInfo.getSet(), printingInfo); - card->combineLegalities(properties); + // Only merge legalities when the card has none yet, so multi-format + // printings don't overwrite each other's legality lists. + if (card->getProperties().filter(formatRegex).empty()) { + card->combineLegalities(properties); + } return card; } @@ -224,7 +228,10 @@ CardInfoPtr OracleImporter::addCard(QString name, static QString getJsonString(const QJsonObject &obj, const QString &key) { - return obj.value(key).toString(); + // QVariant coerces numbers and booleans to text, while QJsonValue::toString() + // returns a null string for them — some MTGJSON fields (manaValue, + // convertedManaCost, isOnlineOnly, isRebalanced) carry those types. + return obj.value(key).toVariant().toString(); } int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList) @@ -329,12 +336,18 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson allNameProps.insert(faceName); // special handling properties - QString colors = card.value("colors").toVariant().toStringList().join(""); + QString colors; + for (const QJsonValue &color : card.value("colors").toArray()) { + colors += color.toString(); + } if (!colors.isEmpty()) { properties.insert("colors", colors); } - QString colorIdentity = card.value("colorIdentity").toVariant().toStringList().join(""); + QString colorIdentity; + for (const QJsonValue &color : card.value("colorIdentity").toArray()) { + colorIdentity += color.toString(); + } if (!colorIdentity.isEmpty()) { properties.insert("coloridentity", colorIdentity); } @@ -529,7 +542,7 @@ static FormatRulesNameMap buildDefaultMagicFormats() return defaultFormatRulesNameMap; } -FormatRulesNameMap OracleImporter::createDefaultMagicFormats() +const FormatRulesNameMap &OracleImporter::createDefaultMagicFormats() { static const FormatRulesNameMap cached = buildDefaultMagicFormats(); return cached; diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index 696e35cab..52a7cd349 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -157,7 +157,10 @@ public: int startImport(); bool saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion); int importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList); - FormatRulesNameMap createDefaultMagicFormats(); + /** + * @brief Returns the default format rules. The result is memoized on first use and must be treated as immutable. + */ + const FormatRulesNameMap &createDefaultMagicFormats(); const CardNameMap &getCardList() const { return cards; From 770282c5b5d8544117ad4753165573e9f947b437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 30 Aug 2026 13:39:09 +0200 Subject: [PATCH 3/6] [Oracle] Add oracle importer tests and fix set parsing details - Add oracle_importer_test and oracle_importer_benchmark_test targets - Preserve the first printing's legalities when an existing card is reused - Concatenate split-card coloridentity and sort/dedupe card colors - Use a raw string for the Basic Land format regex - Pre-allocate the card hash and micro-optimize string handling Took 2 minutes --- oracle/src/oracleimporter.cpp | 41 +- tests/oracle/CMakeLists.txt | 32 ++ .../oracle/oracle_importer_benchmark_test.cpp | 264 +++++++++ tests/oracle/oracle_importer_test.cpp | 525 ++++++++++++++++++ 4 files changed, 847 insertions(+), 15 deletions(-) create mode 100644 tests/oracle/oracle_importer_benchmark_test.cpp create mode 100644 tests/oracle/oracle_importer_test.cpp diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 752317512..f0dc3b1ae 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -67,7 +68,8 @@ bool OracleImporter::readSetsFromByteArray(const QByteArray &data) // capitalize set type if (setType.length() > 0) { // basic grammar for words that aren't capitalized, like in "From the Vault" - const QStringList noCapitalize = {"the", "a", "an", "on", "to", "for", "of", "in", "and", "with", "or"}; + static const QStringList noCapitalize = {"the", "a", "an", "on", "to", "for", + "of", "in", "and", "with", "or"}; QStringList words = setType.split("_"); setType.clear(); bool first = false; @@ -75,7 +77,7 @@ bool OracleImporter::readSetsFromByteArray(const QByteArray &data) if (first && noCapitalize.contains(item)) { setType += item + QString(" "); } else { - setType += item[0].toUpper() + item.mid(1, -1) + QString(" "); + setType += item[0].toUpper() + item.mid(1) + QString(" "); first = true; } } @@ -123,14 +125,8 @@ static void sortAndReduceColors(QString &colors) std::sort(colors.begin(), colors.end(), [](const QChar a, const QChar b) { return colorOrder.value(a, INT_MAX) < colorOrder.value(b, INT_MAX); }); // reduce - QChar lastChar = '\0'; - for (int i = 0; i < colors.size(); ++i) { - if (colors.at(i) == lastChar) { - colors.remove(i, 1); - } else { - lastChar = colors.at(i); - } - } + auto last = std::unique(colors.begin(), colors.end()); + colors.erase(last, colors.end()); } CardInfoPtr OracleImporter::addCard(QString name, @@ -186,8 +182,9 @@ CardInfoPtr OracleImporter::addCard(QString name, // DETECT CARD POSITIONING INFO - bool landscapeOrientation = properties.value("maintype") == "Battle" || properties.value("layout") == "split" || - properties.value("layout") == "planar"; + QString layoutVal = properties.value("layout"); + bool landscapeOrientation = + properties.value("maintype") == "Battle" || layoutVal == "split" || layoutVal == "planar"; // cards that enter the field tapped bool cipt = parseCipt(name, text) || landscapeOrientation; @@ -426,7 +423,8 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson } } - CardInfoPtr newCard = addCard(name + numComponent, text, isToken, properties, relatedCards, printingInfo); + CardInfoPtr newCard = + addCard(name + numComponent, text, isToken, std::move(properties), relatedCards, printingInfo); numCards++; } } @@ -459,7 +457,7 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson if (!thisCardPropertyValue.isEmpty() && originalPropertyValue != thisCardPropertyValue) { if (originalPropertyValue.isEmpty()) { // don't create //es if one field is empty properties.insert(prop, thisCardPropertyValue); - } else if (prop == "colors") { // the card is both colors + } else if (prop == "colors" || prop == "coloridentity") { // the card is both colors properties.insert(prop, originalPropertyValue + thisCardPropertyValue); } else if (prop == "maintype") { // don't create maintypes with //es in them continue; @@ -471,7 +469,7 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson } } } - CardInfoPtr newCard = addCard(name, text, isToken, properties, {}, printingInfo); + CardInfoPtr newCard = addCard(name, text, isToken, std::move(properties), {}, printingInfo); numCards++; } @@ -552,6 +550,19 @@ int OracleImporter::startImport() { static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController(); + // Pre-allocate the cards hash to avoid rehashing during import. The hash + // is keyed by distinct card name rather than by printings: AllPrintings + // ships ~100k printings for ~35k names, so reserving the printing count + // would overallocate ~3x (against this stack's RAM goal). Collecting + // distinct names is cheap — one pass over the already-parsed name fields. + QSet distinctNames; + for (const SetToDownload &curSetToParse : allSets) { + for (const QJsonValue &cardValue : curSetToParse.getCards()) { + distinctNames.insert(cardValue.toObject().value("name").toString()); + } + } + cards.reserve(distinctNames.size()); + // add an empty set for tokens CardSetPtr tokenSet = CardSet::newInstance(noOpController, CardSet::TOKENS_SETNAME, tr("Dummy set containing tokens"), "Tokens"); diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index c5c1e9097..d126390c7 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -7,3 +7,35 @@ endif() target_link_libraries(parse_cipt_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) add_test(NAME parse_cipt_test COMMAND parse_cipt_test) + +# Oracle importer unit tests +add_executable( + oracle_importer_test ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp + oracle_importer_test.cpp +) + +if(NOT GTEST_FOUND) + add_dependencies(oracle_importer_test gtest) +endif() + +target_link_libraries( + oracle_importer_test libcockatrice_card libcockatrice_interfaces Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) + +add_test(NAME oracle_importer_test COMMAND oracle_importer_test) + +# Oracle importer benchmark tests (manual, not run in CI) +add_executable( + oracle_importer_benchmark_test ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp + ../../oracle/src/parsehelpers.cpp oracle_importer_benchmark_test.cpp +) + +if(NOT GTEST_FOUND) + add_dependencies(oracle_importer_benchmark_test gtest) +endif() + +target_link_libraries( + oracle_importer_benchmark_test libcockatrice_card libcockatrice_interfaces Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) diff --git a/tests/oracle/oracle_importer_benchmark_test.cpp b/tests/oracle/oracle_importer_benchmark_test.cpp new file mode 100644 index 000000000..b3cc55319 --- /dev/null +++ b/tests/oracle/oracle_importer_benchmark_test.cpp @@ -0,0 +1,264 @@ +#include "../../oracle/src/oracleimporter.h" + +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include + +// Helper: build a synthetic MTGJSON-style JSON with the given number of sets and cards per set +static QByteArray buildSyntheticData(int numSets, int cardsPerSet) +{ + QJsonObject dataObj; + for (int s = 0; s < numSets; ++s) { + QJsonArray cardsArray; + for (int c = 0; c < cardsPerSet; ++c) { + QJsonObject card; + card["name"] = QString("Card %1").arg(s * cardsPerSet + c); + card["text"] = "This is a test card with some rules text."; + card["layout"] = "normal"; + card["manaCost"] = "{W}"; + card["type"] = "Creature — Human"; + card["power"] = "2"; + card["toughness"] = "2"; + card["colors"] = QJsonArray{"W"}; + card["colorIdentity"] = QJsonArray{"W"}; + card["types"] = QJsonArray{"Creature"}; + // Real MTGJSON types: floats and booleans, not strings. This + // exercises the QVariant coercion in the property reader. + card["convertedManaCost"] = 1.0; + card["manaValue"] = 1.0; + card["isOnlineOnly"] = false; + card["isRebalanced"] = false; + + QJsonObject legalities; + legalities["standard"] = "legal"; + legalities["modern"] = "legal"; + legalities["legacy"] = "legal"; + legalities["vintage"] = "legal"; + legalities["commander"] = "legal"; + card["legalities"] = legalities; + + QJsonObject identifiers; + identifiers["scryfallId"] = QString("id-%1-%2").arg(s).arg(c); + card["identifiers"] = identifiers; + + // In AllPrintings, number and rarity are flat fields on the card + // object, exactly as set below. + card["number"] = QString::number(c + 1); + card["rarity"] = "common"; + + cardsArray.append(card); + } + + QJsonObject setObj; + setObj["code"] = QString("T%1").arg(s, 2, 10, QChar('0')); + setObj["name"] = QString("Test Set %1").arg(s); + setObj["type"] = "expansion"; + setObj["releaseDate"] = "2024-01-01"; + setObj["cards"] = cardsArray; + + dataObj[QString("T%1").arg(s, 2, 10, QChar('0'))] = setObj; + } + + QJsonObject root; + root["data"] = dataObj; + return QJsonDocument(root).toJson(QJsonDocument::Compact); +} + +// ============================================================================ +// Import throughput benchmark +// ============================================================================ + +TEST(OracleBenchmark, ImportThroughput) +{ + static constexpr int numSets = 10; + static constexpr int cardsPerSet = 500; + + QByteArray data = buildSyntheticData(numSets, cardsPerSet); + + OracleImporter importer; + + // Phase 1: Parse JSON + QElapsedTimer timer; + timer.start(); + bool ok = importer.readSetsFromByteArray(data); + ASSERT_TRUE(ok); + qint64 parseMs = timer.elapsed(); + + // Phase 2: Import cards + timer.restart(); + int importedSets = importer.startImport(); + qint64 importMs = timer.elapsed(); + + int totalImported = 0; + for (const auto &card : importer.getCardList()) { + Q_UNUSED(card); + totalImported++; + } + + // The fixture generates globally unique card names, so the expected + // counts are exact: a regression here means cards were dropped. + ASSERT_EQ(importedSets, numSets); + ASSERT_EQ(totalImported, numSets * cardsPerSet); + // Real-data probe: numeric convertedManaCost must be coerced to text + // (regression for the QJsonValue::toString() reader in #7214). + auto probeCard = importer.getCardList().value("Card 0"); + ASSERT_FALSE(probeCard.isNull()); + ASSERT_EQ(probeCard->getProperty("cmc"), "1"); + + qDebug().noquote() + << QString("Oracle Import Benchmark: %1 sets, %2 unique cards").arg(importedSets).arg(totalImported); + qDebug().noquote() << QString(" JSON parse: %1 ms").arg(parseMs); + qDebug().noquote() << QString(" Card import: %1 ms").arg(importMs); + qDebug().noquote() << QString(" Total: %1 ms").arg(parseMs + importMs); + if (importMs > 0) { + qDebug().noquote() << QString(" Throughput: %1 cards/sec") + .arg(static_cast(totalImported) / importMs * 1000.0, 0, 'f', 0); + } +} + +// ============================================================================ +// readSetsFromByteArray benchmark +// ============================================================================ + +TEST(OracleBenchmark, ParseJsonThroughput) +{ + static constexpr int numSets = 20; + static constexpr int cardsPerSet = 1000; + + QByteArray data = buildSyntheticData(numSets, cardsPerSet); + + // Run 5 iterations and report average + static constexpr int iterations = 5; + qint64 totalMs = 0; + + for (int i = 0; i < iterations; ++i) { + OracleImporter importer; + QElapsedTimer timer; + timer.start(); + bool ok = importer.readSetsFromByteArray(data); + ASSERT_TRUE(ok); + totalMs += timer.elapsed(); + } + + qint64 avgMs = totalMs / iterations; + qDebug().noquote() << QString("Parse Benchmark (%1 iterations): avg %2 ms for %3 sets x %4 cards") + .arg(iterations) + .arg(avgMs) + .arg(numSets) + .arg(cardsPerSet); +} + +// ============================================================================ +// Split card merging benchmark +// ============================================================================ + +TEST(OracleBenchmark, SplitCardMerging) +{ + static constexpr int numSplitCards = 1000; + + QJsonArray cardsList; + for (int i = 0; i < numSplitCards; ++i) { + QJsonObject face1; + face1["name"] = QString("Fire %1 // Ice %1").arg(i); + face1["text"] = "Fire side text."; + face1["layout"] = "split"; + face1["side"] = "a"; + face1["faceName"] = QString("Fire %1").arg(i); + face1["colors"] = QJsonArray{"R"}; + face1["colorIdentity"] = QJsonArray{"R"}; + face1["types"] = QJsonArray{"Instant"}; + face1["manaCost"] = "{R}"; + face1["legalities"] = QJsonObject{{"standard", "not_legal"}}; + face1["identifiers"] = QJsonObject{{"scryfallId", QString("f-%1").arg(i)}}; + face1["number"] = QString::number(i + 1); + face1["rarity"] = "uncommon"; + + QJsonObject face2; + face2["name"] = QString("Fire %1 // Ice %1").arg(i); + face2["text"] = "Ice side text."; + face2["layout"] = "split"; + face2["side"] = "b"; + face2["faceName"] = QString("Ice %1").arg(i); + face2["colors"] = QJsonArray{"U"}; + face2["colorIdentity"] = QJsonArray{"U"}; + face2["types"] = QJsonArray{"Instant"}; + face2["manaCost"] = "{U}"; + face2["legalities"] = QJsonObject{{"standard", "not_legal"}}; + face2["identifiers"] = QJsonObject{{"scryfallId", QString("i-%1").arg(i)}}; + face2["number"] = QString::number(i + 1); + face2["rarity"] = "uncommon"; + + cardsList.append(face1); + cardsList.append(face2); + } + + NoopCardSetPriorityController controller; + OracleImporter importer; + CardSetPtr set = CardSet::newInstance(&controller, "TST", "Split Test"); + + QElapsedTimer timer; + timer.start(); + int count = importer.importCardsFromSet(set, cardsList); + qint64 ms = timer.elapsed(); + + ASSERT_EQ(count, numSplitCards); + qDebug().noquote() << QString("Split Card Merge Benchmark: %1 cards in %2 ms (%3 cards/sec)") + .arg(count) + .arg(ms) + .arg(ms > 0 ? static_cast(count) / ms * 1000.0 : 0.0, 0, 'f', 0); +} + +// ============================================================================ +// sortAndReduceColors microbenchmark +// ============================================================================ + +// We can't call sortAndReduceColors directly (it's static), so we benchmark +// through importCardsFromSet with color properties. + +TEST(OracleBenchmark, ImportCardsWithColors) +{ + static constexpr int numCards = 10000; + + NoopCardSetPriorityController controller; + OracleImporter importer; + CardSetPtr set = CardSet::newInstance(&controller, "TST", "Color Test"); + + QJsonArray cardsList; + for (int i = 0; i < numCards; ++i) { + QJsonObject card; + card["name"] = QString("Color Card %1").arg(i); + card["text"] = "Rules text."; + card["layout"] = "normal"; + card["manaCost"] = "{W}"; + card["type"] = "Creature — Human"; + card["types"] = QJsonArray{"Creature"}; + card["colors"] = QJsonArray{"B", "R", "G", "W", "U"}; + card["colorIdentity"] = QJsonArray{"B", "R", "G", "W", "U"}; + card["number"] = QString::number(i + 1); + card["rarity"] = "common"; + card["legalities"] = QJsonObject{{"standard", "legal"}}; + card["identifiers"] = QJsonObject{{"scryfallId", QString("c-%1").arg(i)}}; + cardsList.append(card); + } + + QElapsedTimer timer; + timer.start(); + int count = importer.importCardsFromSet(set, cardsList); + qint64 ms = timer.elapsed(); + + ASSERT_EQ(count, numCards); + qDebug().noquote() << QString("Import with Colors Benchmark: %1 cards in %2 ms (%3 cards/sec)") + .arg(count) + .arg(ms) + .arg(ms > 0 ? static_cast(count) / ms * 1000.0 : 0.0, 0, 'f', 0); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp new file mode 100644 index 000000000..19f66e5e6 --- /dev/null +++ b/tests/oracle/oracle_importer_test.cpp @@ -0,0 +1,525 @@ +#include "../../oracle/src/oracleimporter.h" + +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include +#include + +class OracleImporterTest : public ::testing::Test +{ +protected: + void SetUp() override + { + controller = new NoopCardSetPriorityController(); + importer = new OracleImporter(); + set = CardSet::newInstance(controller, "TST", "Test Set"); + } + + void TearDown() override + { + delete importer; + delete controller; + } + + // Helper: build a minimal card JSON object + QJsonObject makeCard(const QString &name, + const QString &colors = "", + const QString &colorIdentity = "", + const QVariantMap &legalities = {}) + { + QJsonObject card; + card["name"] = name; + card["text"] = "Rules text."; + card["layout"] = "normal"; + card["manaCost"] = "{W}"; + card["type"] = "Creature — Human"; + card["types"] = QJsonArray{"Creature"}; + card["number"] = "1"; + card["rarity"] = "common"; + + if (!colors.isEmpty()) { + QJsonArray arr; + for (const QChar &c : colors) { + arr.append(QString(c)); + } + card["colors"] = arr; + } + if (!colorIdentity.isEmpty()) { + QJsonArray arr; + for (const QChar &c : colorIdentity) { + arr.append(QString(c)); + } + card["colorIdentity"] = arr; + } + if (!legalities.isEmpty()) { + QJsonObject legalObj; + for (auto it = legalities.constBegin(); it != legalities.constEnd(); ++it) { + legalObj[it.key()] = it.value().toString(); + } + card["legalities"] = legalObj; + } + + QJsonObject identifiers; + identifiers["scryfallId"] = QUuid::createUuid().toString(QUuid::WithoutBraces); + card["identifiers"] = identifiers; + + return card; + } + + NoopCardSetPriorityController *controller; + OracleImporter *importer; + CardSetPtr set; +}; + +// ============================================================================ +// sortAndReduceColors tests (tested via importCardsFromSet) +// ============================================================================ + +TEST_F(OracleImporterTest, SortAndReduceColorsSingleColor) +{ + QJsonArray cards{makeCard("Red Card", "R", "R")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Red Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("colors"), "R"); +} + +TEST_F(OracleImporterTest, SortAndReduceColorsDeduplicates) +{ + QJsonArray cards{makeCard("Dedup Card", "WWUUB", "WU")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Dedup Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("colors"), "WUB"); +} + +TEST_F(OracleImporterTest, SortAndReduceColorsSortsWUBRG) +{ + QJsonArray cards{makeCard("Sort Card", "RGW", "RGW")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Sort Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("colors"), "WRG"); +} + +TEST_F(OracleImporterTest, SortAndReduceColorsAllFive) +{ + QJsonArray cards{makeCard("Five Color", "BRGWU", "BRGWU")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Five Color"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("colors"), "WUBRG"); +} + +TEST_F(OracleImporterTest, SortAndReduceColorIdentity) +{ + QJsonArray cards{makeCard("Color Id Card", "W", "GWR")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Color Id Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("coloridentity"), "WRG"); +} + +TEST_F(OracleImporterTest, SingleColorNotSorted) +{ + QJsonArray cards{makeCard("Single Card", "B", "B")}; + importer->importCardsFromSet(set, cards); + + auto card = importer->getCardList().value("Single Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("colors"), "B"); +} + +// ============================================================================ +// Legality guard tests +// ============================================================================ + +TEST_F(OracleImporterTest, NewCardKeepsLegalityProperties) +{ + // Verifies that format-* properties survive addCard on a fresh card + // (not the combineLegalities guard, which only runs on existing printings). + QVariantMap leg; + leg["standard"] = "legal"; + leg["modern"] = "legal"; + QJsonArray cards{makeCard("Legal Card", "", "", leg)}; + + importer->importCardsFromSet(set, cards); + auto card = importer->getCardList().value("Legal Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("format-standard"), "legal"); + ASSERT_EQ(card->getProperty("format-modern"), "legal"); +} + +TEST_F(OracleImporterTest, LegalityMergeAllowedWhenCardHasNoLegalities) +{ + // First printing carries no legalities at all, so the guard's + // `properties.filter(formatRegex).empty()` predicate is true and the + // second printing's legalities must be merged in. + QJsonArray cards1{makeCard("Unmerged Card")}; + importer->importCardsFromSet(set, cards1); + + CardSetPtr set2 = CardSet::newInstance(controller, "TS2", "Second Set"); + QVariantMap leg; + leg["standard"] = "legal"; + QJsonArray cards2{makeCard("Unmerged Card", "", "", leg)}; + importer->importCardsFromSet(set2, cards2); + + auto card = importer->getCardList().value("Unmerged Card"); + ASSERT_FALSE(card.isNull()); + ASSERT_EQ(card->getProperty("format-standard"), "legal"); +} + +TEST_F(OracleImporterTest, LegalityGuardPreservesFirstPrinting) +{ + // First printing: standard=legal, modern=legal + QVariantMap leg1; + leg1["standard"] = "legal"; + leg1["modern"] = "legal"; + QJsonArray cards1{makeCard("Guarded Card", "", "", leg1)}; + importer->importCardsFromSet(set, cards1); + + // Second printing: standard=banned, modern=not_legal + CardSetPtr set2 = CardSet::newInstance(controller, "TS2", "Second Set"); + QVariantMap leg2; + leg2["standard"] = "banned"; + leg2["modern"] = "not_legal"; + QJsonArray cards2{makeCard("Guarded Card", "", "", leg2)}; + importer->importCardsFromSet(set2, cards2); + + auto card = importer->getCardList().value("Guarded Card"); + ASSERT_FALSE(card.isNull()); + // Guard should preserve first printing's legalities + ASSERT_EQ(card->getProperty("format-standard"), "legal"); + ASSERT_EQ(card->getProperty("format-modern"), "legal"); +} + +// ============================================================================ +// createDefaultMagicFormats tests +// ============================================================================ + +TEST_F(OracleImporterTest, CreateDefaultMagicFormatsContainsExpectedFormats) +{ + auto formats = importer->createDefaultMagicFormats(); + ASSERT_TRUE(formats.contains("standard")); + ASSERT_TRUE(formats.contains("modern")); + ASSERT_TRUE(formats.contains("legacy")); + ASSERT_TRUE(formats.contains("vintage")); + ASSERT_TRUE(formats.contains("commander")); + ASSERT_TRUE(formats.contains("pauper")); + ASSERT_TRUE(formats.contains("pioneer")); + ASSERT_TRUE(formats.contains("brawl")); + ASSERT_TRUE(formats.contains("historic")); + ASSERT_TRUE(formats.contains("timeless")); + ASSERT_TRUE(formats.contains("duel")); + ASSERT_TRUE(formats.contains("oathbreaker")); +} + +TEST_F(OracleImporterTest, CreateDefaultMagicFormatsSingletonDeckSizes) +{ + auto formats = importer->createDefaultMagicFormats(); + auto commander = formats.value("commander"); + ASSERT_FALSE(commander.isNull()); + ASSERT_EQ(commander->minDeckSize, 100); + ASSERT_EQ(commander->maxDeckSize, 100); + ASSERT_EQ(commander->maxSideboardSize, 15); + + auto brawl = formats.value("brawl"); + ASSERT_FALSE(brawl.isNull()); + ASSERT_EQ(brawl->minDeckSize, 60); + ASSERT_EQ(brawl->maxDeckSize, 60); +} + +TEST_F(OracleImporterTest, CreateDefaultMagicFormatsVintageHasRestricted) +{ + auto formats = importer->createDefaultMagicFormats(); + auto vintage = formats.value("vintage"); + ASSERT_FALSE(vintage.isNull()); + bool hasRestricted = false; + for (const auto &ac : vintage->allowedCounts) { + if (ac.label == "restricted") { + hasRestricted = true; + ASSERT_EQ(ac.max, 1); + } + } + ASSERT_TRUE(hasRestricted); +} + +TEST_F(OracleImporterTest, CreateDefaultMagicFormatsRegexMatchesBasicLands) +{ + auto formats = importer->createDefaultMagicFormats(); + auto standard = formats.value("standard"); + ASSERT_FALSE(standard.isNull()); + ASSERT_FALSE(standard->exceptions.isEmpty()); + + auto &basicLandsException = standard->exceptions.first(); + ASSERT_FALSE(basicLandsException.conditions.isEmpty()); + + auto &condition = basicLandsException.conditions.first(); + ASSERT_EQ(condition.field, "type"); + ASSERT_EQ(condition.matchType, "regex"); + + // Verify the regex actually works (was broken before: \b = backspace, not word boundary) + QRegularExpression regex(condition.value); + ASSERT_TRUE(regex.isValid()); + ASSERT_TRUE(regex.match("Basic Land — Forest").hasMatch()); + ASSERT_TRUE(regex.match("Basic Snow Land — Mountain").hasMatch()); + ASSERT_FALSE(regex.match("Creature — Elf Warrior").hasMatch()); +} + +TEST_F(OracleImporterTest, CreateDefaultMagicFormatsCaching) +{ + // The memoized map returns the same FormatRulesPtr instances, so the + // shared pointers must be identical across calls. This is the only + // observable effect of the cache: contents would match either way. + auto first = importer->createDefaultMagicFormats(); + auto second = importer->createDefaultMagicFormats(); + ASSERT_EQ(first.value("standard").data(), second.value("standard").data()); +} + +// ============================================================================ +// readSetsFromByteArray tests +// ============================================================================ + +TEST_F(OracleImporterTest, ReadSetsFromByteArrayValidJson) +{ + QJsonObject setObj; + setObj["code"] = "tst"; + setObj["name"] = "Test Set"; + setObj["type"] = "expansion"; + setObj["releaseDate"] = "2024-01-01"; + setObj["cards"] = QJsonArray(); + + QJsonObject root; + root["data"] = QJsonObject{{"TST", setObj}}; + + QByteArray data = QJsonDocument(root).toJson(); + ASSERT_TRUE(importer->readSetsFromByteArray(data)); + ASSERT_EQ(importer->getSets().size(), 1); + ASSERT_EQ(importer->getSets().first().getShortName(), "TST"); +} + +TEST_F(OracleImporterTest, ReadSetsFromByteArrayInvalidJson) +{ + QByteArray data = "not valid json"; + ASSERT_FALSE(importer->readSetsFromByteArray(data)); +} + +TEST_F(OracleImporterTest, ReadSetsFromByteArrayEmptyData) +{ + QJsonObject root; + root["data"] = QJsonObject(); + + QByteArray data = QJsonDocument(root).toJson(); + ASSERT_FALSE(importer->readSetsFromByteArray(data)); +} + +TEST_F(OracleImporterTest, ReadSetsFromByteArrayCapitalizesSetType) +{ + QJsonObject setObj; + setObj["code"] = "ftv"; + setObj["name"] = "From The Vault"; + setObj["type"] = "from_the_vault"; + setObj["releaseDate"] = "2024-01-01"; + setObj["cards"] = QJsonArray(); + + QJsonObject root; + root["data"] = QJsonObject{{"FTV", setObj}}; + + QByteArray data = QJsonDocument(root).toJson(); + ASSERT_TRUE(importer->readSetsFromByteArray(data)); + ASSERT_EQ(importer->getSets().first().getSetType(), "From the Vault"); +} + +TEST_F(OracleImporterTest, ReadSetsFromByteArraySortsSetsByName) +{ + // QJsonObject iterates keys in lexicographic order ("AAA" before "ZZZ"), + // so leaving the natural order matching the alphabetical sort makes the + // assertion pass trivially. Inverting it keeps the sort meaningful: + // iteration yields "AAA" (Zeta Set) first, then the sort by name must + // promote "ZZZ" (Alpha Set) to the front. + QJsonObject setA; + setA["code"] = "aaa"; + setA["name"] = "Zeta Set"; + setA["type"] = "expansion"; + setA["releaseDate"] = "2024-01-01"; + setA["cards"] = QJsonArray(); + + QJsonObject setB; + setB["code"] = "zzz"; + setB["name"] = "Alpha Set"; + setB["type"] = "expansion"; + setB["releaseDate"] = "2024-01-01"; + setB["cards"] = QJsonArray(); + + QJsonObject root; + root["data"] = QJsonObject{{"AAA", setA}, {"ZZZ", setB}}; + + QByteArray data = QJsonDocument(root).toJson(); + ASSERT_TRUE(importer->readSetsFromByteArray(data)); + auto sets = importer->getSets(); + ASSERT_GE(sets.size(), 2); + ASSERT_EQ(sets.first().getShortName(), "ZZZ"); +} + +// ============================================================================ +// Split card coloridentity tests +// ============================================================================ + +TEST_F(OracleImporterTest, SplitCardColorIdentityConcatenated) +{ + QJsonObject leg{{"standard", "not_legal"}}; + + QJsonObject face1; + face1["name"] = "Fire // Ice"; + face1["text"] = "Fire deals 2 damage."; + face1["layout"] = "split"; + face1["side"] = "a"; + face1["faceName"] = "Fire"; + face1["colors"] = QJsonArray{"R"}; + face1["colorIdentity"] = QJsonArray{"R"}; + face1["types"] = QJsonArray{"Instant"}; + face1["manaCost"] = "{R}"; + face1["legalities"] = leg; + face1["identifiers"] = QJsonObject{{"scryfallId", "aaa"}}; + face1["number"] = "1"; + face1["rarity"] = "uncommon"; + + QJsonObject face2; + face2["name"] = "Fire // Ice"; + face2["text"] = "Ice taps target artifact."; + face2["layout"] = "split"; + face2["side"] = "b"; + face2["faceName"] = "Ice"; + face2["colors"] = QJsonArray{"U"}; + face2["colorIdentity"] = QJsonArray{"U"}; + face2["types"] = QJsonArray{"Instant"}; + face2["manaCost"] = "{U}"; + face2["legalities"] = leg; + face2["identifiers"] = QJsonObject{{"scryfallId", "bbb"}}; + face2["number"] = "1"; + face2["rarity"] = "uncommon"; + + QJsonArray cardsList{face1, face2}; + int count = importer->importCardsFromSet(set, cardsList); + ASSERT_EQ(count, 1); + + auto card = importer->getCardList().value("Fire // Ice"); + ASSERT_FALSE(card.isNull()); + + // coloridentity should be "RU" (concatenated), then sorted to "UR" + // by sortAndReduceColors when it reaches addCard + ASSERT_EQ(card->getProperty("coloridentity"), "UR"); +} + +TEST_F(OracleImporterTest, SplitCardColorsConcatenated) +{ + QJsonObject leg{{"standard", "not_legal"}}; + + QJsonObject face1; + face1["name"] = "Fire // Ice"; + face1["text"] = "Fire deals 2 damage."; + face1["layout"] = "split"; + face1["side"] = "a"; + face1["faceName"] = "Fire"; + face1["colors"] = QJsonArray{"R"}; + face1["colorIdentity"] = QJsonArray{"R"}; + face1["types"] = QJsonArray{"Instant"}; + face1["manaCost"] = "{R}"; + face1["legalities"] = leg; + face1["identifiers"] = QJsonObject{{"scryfallId", "aaa"}}; + face1["number"] = "1"; + face1["rarity"] = "uncommon"; + + QJsonObject face2; + face2["name"] = "Fire // Ice"; + face2["text"] = "Ice taps target artifact."; + face2["layout"] = "split"; + face2["side"] = "b"; + face2["faceName"] = "Ice"; + face2["colors"] = QJsonArray{"U"}; + face2["colorIdentity"] = QJsonArray{"U"}; + face2["types"] = QJsonArray{"Instant"}; + face2["manaCost"] = "{U}"; + face2["legalities"] = leg; + face2["identifiers"] = QJsonObject{{"scryfallId", "bbb"}}; + face2["number"] = "1"; + face2["rarity"] = "uncommon"; + + QJsonArray cardsList{face1, face2}; + importer->importCardsFromSet(set, cardsList); + + auto card = importer->getCardList().value("Fire // Ice"); + ASSERT_FALSE(card.isNull()); + + QString colors = card->getProperty("colors"); + ASSERT_FALSE(colors.contains("//")) << "colors should not contain '//', got: " << colors.toStdString(); + ASSERT_TRUE(colors.contains("R")); + ASSERT_TRUE(colors.contains("U")); +} + +// ============================================================================ +// Mana cost formatting tests +// ============================================================================ + +TEST_F(OracleImporterTest, ManaCostStripsBraces) +{ + QJsonObject card = makeCard("Mana Card"); + card["manaCost"] = "{2}{W}{B}"; + QJsonArray cards{card}; + + importer->importCardsFromSet(set, cards); + auto result = importer->getCardList().value("Mana Card"); + ASSERT_FALSE(result.isNull()); + ASSERT_EQ(result->getProperty("manacost"), "2WB"); +} + +// ============================================================================ +// Card deduplication tests +// ============================================================================ + +TEST_F(OracleImporterTest, DuplicateCardNameReturnsExisting) +{ + QJsonArray cards{makeCard("Dupe Card")}; + importer->importCardsFromSet(set, cards); + + CardSetPtr set2 = CardSet::newInstance(controller, "TS2", "Second Set"); + QJsonArray cards2{makeCard("Dupe Card")}; + importer->importCardsFromSet(set2, cards2); + + ASSERT_EQ(importer->getCardList().size(), 1); +} + +TEST_F(OracleImporterTest, AELigatureReplaced) +{ + QJsonObject card = makeCard(QString::fromUtf8("\xC3\x86ther Vial")); // Æther Vial + QJsonArray cards{card}; + + importer->importCardsFromSet(set, cards); + // Æ is replaced with AE, resulting in "AEther Vial" + ASSERT_FALSE(importer->getCardList().contains(QString::fromUtf8("\xC3\x86ther Vial"))); + ASSERT_TRUE(importer->getCardList().contains("AEther Vial")); +} + +TEST_F(OracleImporterTest, ApostropheNormalized) +{ + QJsonObject card = makeCard(QString::fromUtf8("Jace\u2019s Ingenuity")); + QJsonArray cards{card}; + + importer->importCardsFromSet(set, cards); + ASSERT_TRUE(importer->getCardList().contains("Jace's Ingenuity")); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From bd1fb69800e9780bdf470d5ec763bec4aaf1bd5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 14:50:58 +0200 Subject: [PATCH 4/6] [Oracle/Tests] Pin cmc coercion in CI run; scope the reserve pass The #7214 coercion assertion lived only in oracle_importer_benchmark_test, which gets no add_test and so never runs under ctest. Add NumericManaValueCoercedToCmc and LegacyConvertedManaCostCoercedToCmc to oracle_importer_test (a CI-ran binary): manaValue/convertedManaCost are JSON numbers in AllPrintings, and QJsonValue::toString() would drop them to an empty cmc without the #7214 coercion fix. Wrap the distinct-name reserve pass in a bare block so the ~35k name QStrings are handed back before the memory-heavy import loop starts. --- oracle/src/oracleimporter.cpp | 14 +++++++++----- tests/oracle/oracle_importer_test.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index f0dc3b1ae..49577baf0 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -555,13 +555,17 @@ int OracleImporter::startImport() // ships ~100k printings for ~35k names, so reserving the printing count // would overallocate ~3x (against this stack's RAM goal). Collecting // distinct names is cheap — one pass over the already-parsed name fields. - QSet distinctNames; - for (const SetToDownload &curSetToParse : allSets) { - for (const QJsonValue &cardValue : curSetToParse.getCards()) { - distinctNames.insert(cardValue.toObject().value("name").toString()); + { + QSet distinctNames; + for (const SetToDownload &curSetToParse : allSets) { + for (const QJsonValue &cardValue : curSetToParse.getCards()) { + distinctNames.insert(cardValue.toObject().value("name").toString()); + } } + cards.reserve(distinctNames.size()); + // The set goes out of scope here, handing the ~35k name QStrings back + // to the allocator before the (memory-heavy) import loop starts. } - cards.reserve(distinctNames.size()); // add an empty set for tokens CardSetPtr tokenSet = diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp index 19f66e5e6..145a2ca0f 100644 --- a/tests/oracle/oracle_importer_test.cpp +++ b/tests/oracle/oracle_importer_test.cpp @@ -482,6 +482,33 @@ TEST_F(OracleImporterTest, ManaCostStripsBraces) ASSERT_EQ(result->getProperty("manacost"), "2WB"); } +// cmc comes through as a JSON number ("convertedManaCost"/"manaValue" are +// floats in AllPrintings), so this pins the number-to-text coercion that +// QJsonValue::toString() dropped in #7214. +TEST_F(OracleImporterTest, NumericManaValueCoercedToCmc) +{ + QJsonObject card = makeCard("Cmc Card"); + card["manaValue"] = 3; + QJsonArray cards{card}; + + importer->importCardsFromSet(set, cards); + auto result = importer->getCardList().value("Cmc Card"); + ASSERT_FALSE(result.isNull()); + ASSERT_EQ(result->getProperty("cmc"), "3"); +} + +TEST_F(OracleImporterTest, LegacyConvertedManaCostCoercedToCmc) +{ + QJsonObject card = makeCard("Legacy Cmc Card"); + card["convertedManaCost"] = 3.0; + QJsonArray cards{card}; + + importer->importCardsFromSet(set, cards); + auto result = importer->getCardList().value("Legacy Cmc Card"); + ASSERT_FALSE(result.isNull()); + ASSERT_EQ(result->getProperty("cmc"), "3"); +} + // ============================================================================ // Card deduplication tests // ============================================================================ From 81c9edbf898118fc11ef32e44854212432015085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sat, 29 Aug 2026 21:08:56 +0200 Subject: [PATCH 5/6] [Oracle] Add RAM usage benchmarks for the oracle importer - Measure process peak/current RSS via procfs (Linux) or getrusage (macOS) - Add a synthetic-scale RAM benchmark and an opt-in real AllPrintings run gated by COCKATRICE_ORACLE_RAM_BENCHMARK=1 - Mirror the wizard's magic-byte handling to decompress .xz/.zip payloads - Wire optional ZLIB/LibLZMA into the benchmark target and raise its timeout Took 2 minutes --- tests/oracle/CMakeLists.txt | 36 ++- .../oracle/oracle_importer_benchmark_test.cpp | 300 ++++++++++++++++++ 2 files changed, 332 insertions(+), 4 deletions(-) diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index d126390c7..cbff4f19c 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -25,10 +25,33 @@ target_link_libraries( add_test(NAME oracle_importer_test COMMAND oracle_importer_test) -# Oracle importer benchmark tests (manual, not run in CI) +# Oracle importer benchmark tests (manual, not run in CI, incl. RAM benchmark) +# Optional compression libs, mirrored from oracle/CMakeLists.txt, so the benchmark +# can download and decompress whatever AllPrintings format the default URL selects. +find_package(ZLIB) +if(ZLIB_FOUND) + add_definitions("-DHAS_ZLIB") + set(_ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/zip/unzip.cpp ../../oracle/src/zip/zipglobal.cpp) + set(_ORACLE_BENCH_EXTRA_LIBRARIES ${ZLIB_LIBRARIES}) + include_directories(${ZLIB_INCLUDE_DIRS}) +else() + message(STATUS "Oracle tests: zlib not found; zip download benchmark disabled") +endif() + +find_package(LibLZMA) +if(LIBLZMA_FOUND) + add_definitions("-DHAS_LZMA") + list(APPEND _ORACLE_BENCH_EXTRA_SOURCES ../../oracle/src/lzma/decompress.cpp) + list(APPEND _ORACLE_BENCH_EXTRA_LIBRARIES ${LIBLZMA_LIBRARIES}) + include_directories(${LIBLZMA_INCLUDE_DIRS}) +else() + message(STATUS "Oracle tests: LibLZMA not found; xz download benchmark disabled") +endif() + add_executable( - oracle_importer_benchmark_test ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp - ../../oracle/src/parsehelpers.cpp oracle_importer_benchmark_test.cpp + oracle_importer_benchmark_test + ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp + oracle_importer_benchmark_test.cpp ${_ORACLE_BENCH_EXTRA_SOURCES} ) if(NOT GTEST_FOUND) @@ -36,6 +59,11 @@ if(NOT GTEST_FOUND) endif() target_link_libraries( - oracle_importer_benchmark_test libcockatrice_card libcockatrice_interfaces Threads::Threads ${GTEST_BOTH_LIBRARIES} + oracle_importer_benchmark_test + libcockatrice_card + libcockatrice_interfaces + Threads::Threads + ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} + ${_ORACLE_BENCH_EXTRA_LIBRARIES} ) diff --git a/tests/oracle/oracle_importer_benchmark_test.cpp b/tests/oracle/oracle_importer_benchmark_test.cpp index b3cc55319..fe2d0f107 100644 --- a/tests/oracle/oracle_importer_benchmark_test.cpp +++ b/tests/oracle/oracle_importer_benchmark_test.cpp @@ -1,13 +1,33 @@ #include "../../oracle/src/oracleimporter.h" #include "gtest/gtest.h" +#include +#include #include #include +#include +#include #include #include #include +#include +#include +#include +#include +#include #include +#if defined(HAS_LZMA) +#include "../../oracle/src/lzma/decompress.h" +#endif +#if defined(HAS_ZLIB) +#include "../../oracle/src/zip/unzip.h" +#endif +#if defined(Q_OS_MACOS) +#include +#include +#endif + // Helper: build a synthetic MTGJSON-style JSON with the given number of sets and cards per set static QByteArray buildSyntheticData(int numSets, int cardsPerSet) { @@ -257,8 +277,288 @@ TEST(OracleBenchmark, ImportCardsWithColors) .arg(ms > 0 ? static_cast(count) / ms * 1000.0 : 0.0, 0, 'f', 0); } +// ============================================================================ +// RAM usage measurement +// ============================================================================ + +// Mirrors the default AllPrintings URL selection in oracle/src/pages.cpp. +#if defined(HAS_LZMA) +static const QUrl kDefaultAllPrintingsUrl("https://www.mtgjson.com/api/v5/AllPrintings.json.xz"); +#elif defined(HAS_ZLIB) +static const QUrl kDefaultAllPrintingsUrl("https://www.mtgjson.com/api/v5/AllPrintings.json.zip"); +#else +static const QUrl kDefaultAllPrintingsUrl("https://www.mtgjson.com/api/v5/AllPrintings.json"); +#endif + +// Magic bytes also from oracle/src/pages.cpp +static const QByteArray kXzSignature("\xFD\x37\x7A\x58\x5A", 6); +static const QByteArray kZipSignature("PK"); + +struct MemorySnapshot +{ + qint64 peakRssKb = -1; // process high-water mark (VmHWM on Linux, ru_maxrss on macOS) + qint64 rssKb = -1; // current resident set size + bool available = false; + + static MemorySnapshot current() + { + MemorySnapshot snap; +#if defined(Q_OS_LINUX) + QFile statusFile("/proc/self/status"); + if (statusFile.open(QIODevice::ReadOnly | QIODevice::Text)) { + // /proc files report size() == 0, so atEnd() is immediately true: read everything first. + const QList lines = statusFile.readAll().split('\n'); + for (const QByteArray &line : lines) { + if (line.startsWith("VmHWM:")) { + snap.peakRssKb = line.mid(6).trimmed().split(' ').value(0).toLongLong(); + } else if (line.startsWith("VmRSS:")) { + snap.rssKb = line.mid(6).trimmed().split(' ').value(0).toLongLong(); + } + } + snap.available = snap.peakRssKb >= 0; + } +#elif defined(Q_OS_MACOS) + struct rusage usage; + if (getrusage(RUSAGE_SELF, &usage) == 0) { + snap.peakRssKb = usage.ru_maxrss / 1024; // bytes -> kB + snap.available = snap.peakRssKb >= 0; + } + // getrusage has no current-RSS equivalent; task_info's resident_size + // is the closest macOS analog to Linux VmRSS. + mach_task_basic_info info = {}; + mach_msg_type_number_t count = MACH_TASK_BASIC_INFO_COUNT; + if (task_info(mach_task_self(), MACH_TASK_BASIC_INFO, reinterpret_cast(&info), &count) == + KERN_SUCCESS) { + snap.rssKb = info.resident_size / 1024; + } +#endif + return snap; + } +}; + +static QString formatKb(qint64 kb) +{ + if (kb < 0) { + return "N/A"; + } + return QString("%1 MB").arg(kb / 1024.0, 0, 'f', 1); +} + +static void logRamPhase(const QString &phase, const MemorySnapshot &baseline, const MemorySnapshot ¤t) +{ + if (!baseline.available || !current.available) { + qDebug().noquote() << QString(" %1: memory stats unavailable on this platform").arg(phase); + return; + } + // VmHWM / ru_maxrss are monotonically non-decreasing high-water marks, so a + // peak-based delta between phases is ~0.0 MB by construction once the + // fixture build has set the process peak. The live signals are current RSS + // and the process peak; the delta is only meaningful where current RSS is + // a per-phase value (see "after releaseSetData()"). + QString rssDelta = "N/A"; + if (current.rssKb >= 0 && baseline.rssKb >= 0) { + rssDelta = formatKb(current.rssKb - baseline.rssKb); + } + qDebug().noquote() << QString(" %1: current RSS %2 | delta vs baseline %3 | process peak %4") + .arg(phase) + .arg(formatKb(current.rssKb)) + .arg(rssDelta) + .arg(formatKb(current.peakRssKb)); +} + +// Decompresses the download payload when the default URL is a compressed build, +// mirroring the wizard's magic-byte handling in oracle/src/pages.cpp. +static QByteArray decompressSetsData(const QByteArray &payload) +{ + if (payload.startsWith(kXzSignature)) { +#if defined(HAS_LZMA) + QBuffer inBuffer(const_cast(&payload)); + QByteArray out; + QBuffer outBuffer(&out); + inBuffer.open(QIODevice::ReadOnly); + outBuffer.open(QIODevice::WriteOnly); + XzDecompressor xz; + if (!xz.decompress(&inBuffer, &outBuffer)) { + qDebug() << "RAM benchmark: xz decompression failed"; + return {}; + } + return out; +#else + qDebug() << "RAM benchmark: download is xz-compressed but this build has no LZMA support"; + return {}; +#endif + } + if (payload.startsWith(kZipSignature)) { +#if defined(HAS_ZLIB) + QBuffer inBuffer(const_cast(&payload)); + inBuffer.open(QIODevice::ReadOnly); + UnZip unzip; + if (unzip.openArchive(&inBuffer) != UnZip::Ok) { + qDebug() << "RAM benchmark: zip archive open failed"; + return {}; + } + if (unzip.fileList().size() != 1) { + qDebug() << "RAM benchmark: zip archive doesn't contain exactly one file"; + return {}; + } + QByteArray out; + QBuffer outBuffer(&out); + outBuffer.open(QIODevice::WriteOnly); + const auto errorCode = unzip.extractFile(unzip.fileList().value(0), &outBuffer); + unzip.closeArchive(); + if (errorCode != UnZip::Ok) { + qDebug() << "RAM benchmark: zip extraction failed"; + return {}; + } + return out; +#else + qDebug() << "RAM benchmark: download is zip-compressed but this build has no zlib support"; + return {}; +#endif + } + return payload; +} + +TEST(OracleBenchmark, ImportRamUsage) +{ + static constexpr int numSets = 30; + static constexpr int cardsPerSet = 2000; // ~60k cards, roughly AllPrintings scale + + // Baseline must precede the fixture build: a high-water mark set while + // generating the synthetic JSON would otherwise mask the importer phases. + // Where memory stats are unavailable (Windows), skip before doing the + // 60k-card fixture build, which would otherwise be pure wasted work. + const MemorySnapshot baseline = MemorySnapshot::current(); + if (!baseline.available) { + GTEST_SKIP() << "Memory stats unavailable on this platform"; + } + + const QByteArray data = buildSyntheticData(numSets, cardsPerSet); + + NoopCardSetPriorityController controller; + OracleImporter importer; + + QElapsedTimer timer; + timer.start(); + ASSERT_TRUE(importer.readSetsFromByteArray(data)); + const qint64 parseMs = timer.elapsed(); + const MemorySnapshot afterParse = MemorySnapshot::current(); + + timer.restart(); + const int importedSets = importer.startImport(); + const qint64 importMs = timer.elapsed(); + const MemorySnapshot afterImport = MemorySnapshot::current(); + + importer.releaseSetData(); + const MemorySnapshot afterRelease = MemorySnapshot::current(); + + const int totalCards = importer.getCardList().size(); + qDebug().noquote() << QString("Oracle RAM Benchmark (synthetic): %1 sets, %2 cards, %3 MB JSON") + .arg(importedSets) + .arg(totalCards) + .arg(data.size() / (1024.0 * 1024.0), 0, 'f', 1); + qDebug().noquote() << QString(" JSON parse: %1 ms").arg(parseMs); + qDebug().noquote() << QString(" Card import: %1 ms").arg(importMs); + logRamPhase("parse", baseline, afterParse); + logRamPhase("import", afterParse, afterImport); + logRamPhase("after releaseSetData()", afterImport, afterRelease); +} + +TEST(OracleBenchmark, ImportRamUsageAllPrintings) +{ + // Only "1" enables the download: unset (the default and the CI setup) and + // an explicit "0" both disable it. + bool envOk = false; + const int enabled = qEnvironmentVariableIntValue("COCKATRICE_ORACLE_RAM_BENCHMARK", &envOk); + if (!envOk || enabled == 0) { + GTEST_SKIP() << "Set COCKATRICE_ORACLE_RAM_BENCHMARK=1 to download the real AllPrintings dataset for this " + "RAM benchmark. Default URL: " + << kDefaultAllPrintingsUrl.toDisplayString().toStdString(); + } + + // Baseline must precede the request so the phase covers the download + + // decompress step, including the payload materialized by readAll(). + const MemorySnapshot baseline = MemorySnapshot::current(); + if (!baseline.available) { + GTEST_SKIP() << "Memory stats unavailable on this platform"; + } + + QNetworkAccessManager nam; + QNetworkRequest request(kDefaultAllPrintingsUrl); + request.setHeader(QNetworkRequest::UserAgentHeader, "Cockatrice Oracle RAM benchmark"); + QNetworkReply *reply = nam.get(request); + + QEventLoop loop; + QTimer timeoutTimer; + timeoutTimer.setSingleShot(true); + bool timedOut = false; + QObject::connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit); + QObject::connect(&timeoutTimer, &QTimer::timeout, &loop, [&] { + timedOut = true; + reply->abort(); + }); + timeoutTimer.start(10 * 60 * 1000); + loop.exec(); + timeoutTimer.stop(); + + // abort() leaves reply->error() as OperationCanceledError, so a timed-out + // download takes the same GTEST_SKIP path as any other network error + // instead of reading a truncated body and failing the parse below. + if (timedOut || reply->error() != QNetworkReply::NoError) { + GTEST_SKIP() << "Download failed: " << reply->errorString().toStdString(); + } + const QByteArray payload = reply->readAll(); + reply->deleteLater(); + + // mtgjson can answer 200 with an HTML page (mirrors the wizard's '<' check + // in pages.cpp); reject it before trying to decompress/parse. + if (payload.startsWith("<")) { + GTEST_SKIP() << "Download returned a non-JSON body (HTML page instead of data), skipping"; + } + + const QByteArray setsData = decompressSetsData(payload); + const MemorySnapshot afterDownload = MemorySnapshot::current(); + if (setsData.isEmpty()) { + GTEST_SKIP() << "No data to import (download or decompression failed)"; + } + + NoopCardSetPriorityController controller; + OracleImporter importer; + + QElapsedTimer timer; + timer.start(); + ASSERT_TRUE(importer.readSetsFromByteArray(setsData)); + const qint64 parseMs = timer.elapsed(); + const MemorySnapshot afterParse = MemorySnapshot::current(); + + timer.restart(); + const int importedSets = importer.startImport(); + const qint64 importMs = timer.elapsed(); + const MemorySnapshot afterImport = MemorySnapshot::current(); + + importer.releaseSetData(); + const MemorySnapshot afterRelease = MemorySnapshot::current(); + + const int totalCards = importer.getCardList().size(); + qDebug().noquote() << QString("Oracle RAM Benchmark (real AllPrintings): %1 sets, %2 unique cards") + .arg(importedSets) + .arg(totalCards); + qDebug().noquote() << QString(" URL: %1").arg(kDefaultAllPrintingsUrl.toDisplayString()); + qDebug().noquote() << QString(" Downloaded: %1 MB, decompressed: %2 MB") + .arg(payload.size() / (1024.0 * 1024.0), 0, 'f', 1) + .arg(setsData.size() / (1024.0 * 1024.0), 0, 'f', 1); + qDebug().noquote() << QString(" JSON parse: %1 ms").arg(parseMs); + qDebug().noquote() << QString(" Card import: %1 ms").arg(importMs); + logRamPhase("download+decompress", baseline, afterDownload); + logRamPhase("parse", afterDownload, afterParse); + logRamPhase("import", afterParse, afterImport); + logRamPhase("after releaseSetData()", afterImport, afterRelease); +} + int main(int argc, char **argv) { + // Required for the event loop used by the real-AllPrintings download benchmark + QCoreApplication app(argc, argv); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } From 64e5c0de193a8182ccd18f92c7d367981f61c27a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 14:55:52 +0200 Subject: [PATCH 6/6] [Oracle/Tests] Measure parse against post-fixture baseline; assert release empties sets --- .../oracle/oracle_importer_benchmark_test.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/oracle/oracle_importer_benchmark_test.cpp b/tests/oracle/oracle_importer_benchmark_test.cpp index fe2d0f107..567519a48 100644 --- a/tests/oracle/oracle_importer_benchmark_test.cpp +++ b/tests/oracle/oracle_importer_benchmark_test.cpp @@ -353,8 +353,9 @@ static void logRamPhase(const QString &phase, const MemorySnapshot &baseline, co // VmHWM / ru_maxrss are monotonically non-decreasing high-water marks, so a // peak-based delta between phases is ~0.0 MB by construction once the // fixture build has set the process peak. The live signals are current RSS - // and the process peak; the delta is only meaningful where current RSS is - // a per-phase value (see "after releaseSetData()"). + // and the process peak; the delta is meaningful only where the baseline was + // taken immediately before the phase it measures (e.g. the import phase, + // which compares afterParse against afterImport). QString rssDelta = "N/A"; if (current.rssKb >= 0 && baseline.rssKb >= 0) { rssDelta = formatKb(current.rssKb - baseline.rssKb); @@ -435,6 +436,13 @@ TEST(OracleBenchmark, ImportRamUsage) const QByteArray data = buildSyntheticData(numSets, cardsPerSet); + // The fixture build leaves freed-but-unreturned arenas behind (current RSS + // rarely falls once glibc allocates). Baseline immediately after it so the + // parse phase measures only the importer's own growth (~40 MB) rather than + // swallowing the fixture builder's spike. + const MemorySnapshot afterFixture = MemorySnapshot::current(); + logRamPhase("fixture build", baseline, afterFixture); + NoopCardSetPriorityController controller; OracleImporter importer; @@ -459,9 +467,14 @@ TEST(OracleBenchmark, ImportRamUsage) .arg(data.size() / (1024.0 * 1024.0), 0, 'f', 1); qDebug().noquote() << QString(" JSON parse: %1 ms").arg(parseMs); qDebug().noquote() << QString(" Card import: %1 ms").arg(importMs); - logRamPhase("parse", baseline, afterParse); + logRamPhase("parse", afterFixture, afterParse); logRamPhase("import", afterParse, afterImport); logRamPhase("after releaseSetData()", afterImport, afterRelease); + + // Freeing the parsed tree rarely moves current RSS (allocator reuse), so the + // meaningful signal that release actually dropped the buffers is emptiness, + // not an RSS delta. + ASSERT_TRUE(importer.getSets().isEmpty()); } TEST(OracleBenchmark, ImportRamUsageAllPrintings)