diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 29f866164..897264987 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -82,7 +81,7 @@ bool OracleImporter::readSetsFromByteArray(QByteArray data) setType = setType.trimmed(); } SetToDownload set(shortName, longName, priority, setType, releaseDate); - set.setRawRange(range.dataRange); + set.setRawRange(range); newSetList.append(set); } @@ -92,7 +91,7 @@ bool OracleImporter::readSetsFromByteArray(QByteArray data) return false; } allSets = newSetList; - rawSetsData = std::move(data); + rawSetsData = data; return true; } @@ -144,8 +143,6 @@ CardInfoPtr OracleImporter::addCard(QString name, if (existingIt != cards.constEnd()) { CardInfoPtr card = existingIt.value(); card->addToSet(printingInfo.getSet(), printingInfo); - // 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); } @@ -226,10 +223,7 @@ CardInfoPtr OracleImporter::addCard(QString name, static QString getJsonString(const QJsonObject &obj, const QString &key) { - // 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(); + return obj.value(key).toString(); } int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList) @@ -334,18 +328,12 @@ int OracleImporter::importCardsFromSet(const CardSetPtr ¤tSet, const QJson allNameProps.insert(faceName); // special handling properties - QString colors; - for (const QJsonValue &color : card.value("colors").toArray()) { - colors += color.toString(); - } + QString colors = card.value("colors").toVariant().toStringList().join(""); if (!colors.isEmpty()) { properties.insert("colors", colors); } - QString colorIdentity; - for (const QJsonValue &color : card.value("colorIdentity").toArray()) { - colorIdentity += color.toString(); - } + QString colorIdentity = card.value("colorIdentity").toVariant().toStringList().join(""); if (!colorIdentity.isEmpty()) { properties.insert("coloridentity", colorIdentity); } @@ -541,7 +529,7 @@ static FormatRulesNameMap buildDefaultMagicFormats() return defaultFormatRulesNameMap; } -const FormatRulesNameMap &OracleImporter::createDefaultMagicFormats() +FormatRulesNameMap OracleImporter::createDefaultMagicFormats() { static const FormatRulesNameMap cached = buildDefaultMagicFormats(); return cached; @@ -551,11 +539,7 @@ int OracleImporter::startImport() { static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController(); - // Pre-allocate the cards hash to avoid rehashing during import. Keys are - // distinct card names while raw ranges only count printings (AllPrintings - // ~100k printings vs ~35k names), so this over-reserves somewhat; an exact - // distinct-name count would require eagerly parsing, which the lazy reader - // deliberately avoids. It's a capacity hint, so the overshoot is harmless. + // Pre-allocate cards hash to avoid rehashing during import int estimatedCards = 0; for (const SetToDownload &curSetToParse : allSets) { estimatedCards += curSetToParse.getRawRange().cardCount; @@ -579,17 +563,7 @@ int OracleImporter::startImport() // parse only this set's slice of the raw document so the whole JSON tree is // never kept in memory at once - const RawJson::SetDataRange &rawRange = curSetToParse.getRawRange(); - const qsizetype rangeEnd = rawRange.start + rawRange.length; - if (rawRange.start < 0 || rawRange.length <= 0 || rangeEnd > rawSetsData.size()) { - // rawSetsData is cleared by releaseSetData() while SetToDownload copies - // taken from getSets() keep their ranges, and nothing else enforces the - // pairing — so never index past the buffer on stale/mismatched ranges. - qWarning() << "error: out-of-bounds raw range for set" << curSetToParse.getShortName() << "skipping"; - ++setIndex; - emit setIndexChanged(0, setIndex, curSetToParse.getLongName()); - continue; - } + const RawJson::SetRange &rawRange = curSetToParse.getRawRange(); const QByteArray setBytes(rawSetsData.constData() + rawRange.start, rawRange.length); QJsonParseError parseError; const QJsonDocument setDoc = QJsonDocument::fromJson(setBytes, &parseError); @@ -597,10 +571,6 @@ int OracleImporter::startImport() qWarning() << "error: parsing card data for set" << curSetToParse.getShortName() << ":" << parseError.errorString(); ++setIndex; - // Keep the progress accounting honest: a set that failed to parse - // still advanced the index, so report it (with zero imported cards) - // rather than letting SaveSetsPage's bar stall per failed set. - emit setIndexChanged(0, setIndex, curSetToParse.getLongName()); continue; } diff --git a/oracle/src/oracleimporter.h b/oracle/src/oracleimporter.h index 8cb30ca40..1450e2d99 100644 --- a/oracle/src/oracleimporter.h +++ b/oracle/src/oracleimporter.h @@ -54,7 +54,7 @@ private: CardSet::Priority priority; // Byte range of this set's object within the importer's raw JSON text. Parsing // one set at a time keeps peak memory low instead of holding the whole document. - RawJson::SetDataRange rawRange; + RawJson::SetRange rawRange; public: const QString &getShortName() const @@ -77,7 +77,7 @@ public: { return priority; } - const RawJson::SetDataRange &getRawRange() const + const RawJson::SetRange &getRawRange() const { return rawRange; } @@ -90,7 +90,7 @@ public: setType(std::move(_setType)), priority(_priority) { } - void setRawRange(const RawJson::SetDataRange &_rawRange) + void setRawRange(const RawJson::SetRange &_rawRange) { rawRange = _rawRange; } @@ -175,10 +175,7 @@ public: int startImport(); bool saveToFile(const QString &fileName, const QString &sourceUrl, const QString &sourceVersion); int importCardsFromSet(const CardSetPtr ¤tSet, const QJsonArray &cardsList); - /** - * @brief Returns the default format rules. The result is memoized on first use and must be treated as immutable. - */ - const FormatRulesNameMap &createDefaultMagicFormats(); + FormatRulesNameMap createDefaultMagicFormats(); const CardNameMap &getCardList() const { return cards; diff --git a/oracle/src/raw_json_scanner.cpp b/oracle/src/raw_json_scanner.cpp index 01ea3fab5..0311605f3 100644 --- a/oracle/src/raw_json_scanner.cpp +++ b/oracle/src/raw_json_scanner.cpp @@ -5,12 +5,6 @@ namespace { -// Nesting cap matching QJsonDocument's limit, so a pathologically deep document -// fails shallowly instead of overflowing the stack through the recursive -// skipValue/skipArray/skipObject walk (Qt's parser caps at 1024 for the same -// reason and reports DeepNesting). -constexpr int kMaxNestingDepth = 1024; - inline bool isWhitespace(char c) { return c == ' ' || c == '\t' || c == '\r' || c == '\n'; @@ -222,26 +216,6 @@ bool decodeString(const char *&p, const char *end, QString &out) return false; } -/** - * @brief Reads a set-metadata field, tolerating null and non-string values. - * - * A set's metadata may carry null or non-string values in otherwise-valid - * payloads ("releaseDate": null, "type": 7). The token itself was already - * structurally validated by skipValue, so a non-string value is accepted and - * leaves @p out at its default (empty) — one bad set must not abort the - * import of every other set in the document. - */ -bool decodeStringMember(const char *&fs, const char *&fe, QString &out) -{ - if (fs >= fe) { - return false; - } - if (*fs != '"') { - return true; - } - return decodeString(fs, fe, out); -} - bool matchLiteral(const char *&p, const char *end, const char *literal, int length) { if (end - p < length || memcmp(p, literal, static_cast(length)) != 0) { @@ -295,9 +269,9 @@ bool skipNumber(const char *&p, const char *end) return true; } -bool skipValue(const char *&p, const char *end, int depth); -bool skipObject(const char *&p, const char *end, int depth); -bool skipArray(const char *&p, const char *end, int depth); +bool skipValue(const char *&p, const char *end); +bool skipObject(const char *&p, const char *end); +bool skipArray(const char *&p, const char *end); bool skipPrimitive(const char *&p, const char *end) { @@ -323,11 +297,8 @@ bool skipPrimitive(const char *&p, const char *end) return false; } -bool skipObject(const char *&p, const char *end, int depth) +bool skipObject(const char *&p, const char *end) { - if (depth <= 0) { - return false; // nest deeper than the cap - } ++p; // '{' p = skipWhitespace(p, end); if (p < end && *p == '}') { @@ -347,7 +318,7 @@ bool skipObject(const char *&p, const char *end, int depth) return false; } ++p; - if (!skipValue(p, end, depth - 1)) { + if (!skipValue(p, end)) { return false; } p = skipWhitespace(p, end); @@ -366,11 +337,8 @@ bool skipObject(const char *&p, const char *end, int depth) } } -bool skipArray(const char *&p, const char *end, int depth) +bool skipArray(const char *&p, const char *end) { - if (depth <= 0) { - return false; // nest deeper than the cap - } ++p; // '[' p = skipWhitespace(p, end); if (p < end && *p == ']') { @@ -378,7 +346,7 @@ bool skipArray(const char *&p, const char *end, int depth) return true; } for (;;) { - if (!skipValue(p, end, depth - 1)) { + if (!skipValue(p, end)) { return false; } p = skipWhitespace(p, end); @@ -397,21 +365,18 @@ bool skipArray(const char *&p, const char *end, int depth) } } -bool skipValue(const char *&p, const char *end, int depth) +bool skipValue(const char *&p, const char *end) { - if (depth <= 0) { - return false; // nest deeper than the cap - } p = skipWhitespace(p, end); if (p >= end) { return false; } const char c = *p; if (c == '{') { - return skipObject(p, end, depth - 1); + return skipObject(p, end); } if (c == '[') { - return skipArray(p, end, depth - 1); + return skipArray(p, end); } return skipPrimitive(p, end); } @@ -422,11 +387,8 @@ bool skipValue(const char *&p, const char *end, int depth) * For each member invokes @p memberCallback with the key and the byte range of * its value. Advancing @p p is unaffected by the callback. */ -template bool forEachObjectMember(const char *&p, const char *end, int depth, F &&memberCallback) +template bool forEachObjectMember(const char *&p, const char *end, F &&memberCallback) { - if (depth <= 0) { - return false; // nest deeper than the cap - } ++p; // '{' p = skipWhitespace(p, end); if (p < end && *p == '}') { @@ -449,7 +411,7 @@ template bool forEachObjectMember(const char *&p, const char *end, ++p; const char *valueStart = skipWhitespace(p, end); const char *valueEnd = valueStart; - if (!skipValue(valueEnd, end, depth - 1)) { + if (!skipValue(valueEnd, end)) { return false; } if (!memberCallback(key, valueStart, valueEnd)) { @@ -473,11 +435,8 @@ template bool forEachObjectMember(const char *&p, const char *end, } // Counts the direct elements of an array value; returns -1 if the array is malformed. -int countArrayElements(const char *p, const char *end, int depth) +int countArrayElements(const char *p, const char *end) { - if (depth <= 0) { - return -1; // nest deeper than the cap - } ++p; // '[' p = skipWhitespace(p, end); int count = 0; @@ -485,7 +444,7 @@ int countArrayElements(const char *p, const char *end, int depth) return 0; } for (;;) { - if (!skipValue(p, end, depth - 1)) { + if (!skipValue(p, end)) { return -1; } ++count; @@ -545,55 +504,48 @@ QList scanSetRanges(const QByteArray &json, ScanError *error) return false; } const char *setP = valueStart; - const bool ok = forEachObjectMember(setP, valueEnd, kMaxNestingDepth - 1, - [&](const QString &setCode, const char *setStart, const char *setEnd) { - if (setStart >= setEnd || *setStart != '{') { - malformedSetData = true; - return false; - } - SetRange range; - range.dataRange.start = setStart - begin; - range.dataRange.length = setEnd - setStart; - range.code = setCode; + const bool ok = forEachObjectMember( + setP, valueEnd, [&](const QString &setCode, const char *setStart, const char *setEnd) { + if (setStart >= setEnd || *setStart != '{') { + malformedSetData = true; + return false; + } + SetRange range; + range.start = setStart - begin; + range.length = setEnd - setStart; + range.code = setCode; - const char *memberP = setStart; - const bool metaOk = forEachObjectMember( - memberP, setEnd, kMaxNestingDepth - 2, - [&](const QString &field, const char *fs, const char *fe) { - if (field == QStringLiteral("code")) { - return decodeStringMember(fs, fe, range.code); - } - if (field == QStringLiteral("name")) { - return decodeStringMember(fs, fe, range.name); - } - if (field == QStringLiteral("type")) { - return decodeStringMember(fs, fe, range.type); - } - if (field == QStringLiteral("releaseDate")) { - return decodeStringMember(fs, fe, range.releaseDate); - } - if (field == QStringLiteral("cards")) { - if (fs >= fe) { - return false; - } - if (*fs != '[') { - // e.g. "cards": null — treat as an empty array, - // matching Qt's tolerance. - return true; - } - range.dataRange.cardCount = - countArrayElements(fs, fe, kMaxNestingDepth - 2); - return range.dataRange.cardCount >= 0; - } - return true; - }); - if (!metaOk) { - malformedSetData = true; - return false; - } - ranges.append(range); - return true; - }); + const char *memberP = setStart; + const bool metaOk = + forEachObjectMember(memberP, setEnd, [&](const QString &field, const char *fs, const char *fe) { + if (field == QStringLiteral("code")) { + return fs < fe && *fs == '"' && decodeString(fs, fe, range.code); + } + if (field == QStringLiteral("name")) { + return fs < fe && *fs == '"' && decodeString(fs, fe, range.name); + } + if (field == QStringLiteral("type")) { + return fs < fe && *fs == '"' && decodeString(fs, fe, range.type); + } + if (field == QStringLiteral("releaseDate")) { + return fs < fe && *fs == '"' && decodeString(fs, fe, range.releaseDate); + } + if (field == QStringLiteral("cards")) { + if (fs >= fe || *fs != '[') { + return false; + } + range.cardCount = countArrayElements(fs, fe); + return range.cardCount >= 0; + } + return true; + }); + if (!metaOk) { + malformedSetData = true; + return false; + } + ranges.append(range); + return true; + }); if (!ok) { malformedSetData = true; return false; @@ -602,7 +554,7 @@ QList scanSetRanges(const QByteArray &json, ScanError *error) return true; }; - if (!forEachObjectMember(p, end, kMaxNestingDepth, topLevelCallback)) { + if (!forEachObjectMember(p, end, topLevelCallback)) { return fail(malformedSetData ? QStringLiteral("malformed set data") : QStringLiteral("malformed JSON")); } p = skipWhitespace(p, end); diff --git a/oracle/src/raw_json_scanner.h b/oracle/src/raw_json_scanner.h index f6e3a4647..f4c1ba0ab 100644 --- a/oracle/src/raw_json_scanner.h +++ b/oracle/src/raw_json_scanner.h @@ -8,12 +8,7 @@ namespace RawJson { -/** - * @brief The byte extent of a set's object inside the scanned document, plus - * the size of its cards array. This is the slice SetToDownload needs for lazy - * per-set parsing; the metadata strings live in SetRange alongside it. - */ -struct SetDataRange +struct SetRange { /** @brief Byte offset of the set's object within the scanned buffer. */ qsizetype start = -1; @@ -21,12 +16,6 @@ struct SetDataRange qsizetype length = 0; /** @brief Number of entries in the set's "cards" array. */ int cardCount = 0; -}; - -struct SetRange -{ - /** @brief The byte slice of this set within the document. */ - SetDataRange dataRange; QString code; QString name; QString type; @@ -51,13 +40,8 @@ struct ScanError * QJsonDocument::fromJson() over the whole file. * * The whole document is structurally validated while scanning (strings, - * escapes, braces, and a trailing-content check) and nesting depth is capped at - * 1024 to match QJsonDocument, so pathologically deep documents fail shallowly - * instead of exhausting the stack. Verdicts agree with QJsonDocument::fromJson - * on structurally malformed input; unlike Qt, string metadata fields - * ("name", "type", "releaseDate", "code") tolerate null / non-string values by - * defaulting to empty rather than rejecting the whole document, so one broken - * set cannot abort the import of the rest. + * escapes, braces, and a trailing-content check), so malformed input is + * rejected just like QJsonDocument::fromJson would. * * Following QJsonDocument::fromJson's convention, the parsed ranges are * returned by value and any failure is reported through the @p error out diff --git a/tests/oracle/CMakeLists.txt b/tests/oracle/CMakeLists.txt index 9bc5ee5be..f25a76417 100644 --- a/tests/oracle/CMakeLists.txt +++ b/tests/oracle/CMakeLists.txt @@ -25,7 +25,6 @@ target_link_libraries( add_test(NAME oracle_importer_test COMMAND oracle_importer_test) -# 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) @@ -48,6 +47,7 @@ else() message(STATUS "Oracle tests: LibLZMA not found; xz download benchmark disabled") endif() +# Oracle importer benchmark tests add_executable( oracle_importer_benchmark_test ${VERSION_STRING_CPP} ../../oracle/src/oracleimporter.cpp ../../oracle/src/parsehelpers.cpp @@ -67,3 +67,6 @@ target_link_libraries( ${TEST_QT_MODULES} ${_ORACLE_BENCH_EXTRA_LIBRARIES} ) + +add_test(NAME oracle_importer_benchmark_test COMMAND oracle_importer_benchmark_test) +set_tests_properties(oracle_importer_benchmark_test PROPERTIES TIMEOUT 120) diff --git a/tests/oracle/oracle_importer_benchmark_test.cpp b/tests/oracle/oracle_importer_benchmark_test.cpp index e3890bba7..cee00e1d4 100644 --- a/tests/oracle/oracle_importer_benchmark_test.cpp +++ b/tests/oracle/oracle_importer_benchmark_test.cpp @@ -24,7 +24,6 @@ #include "../../oracle/src/zip/unzip.h" #endif #if defined(Q_OS_MACOS) -#include #include #endif @@ -46,12 +45,7 @@ static QByteArray buildSyntheticData(int numSets, int cardsPerSet) 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; + card["convertedManaCost"] = "1"; QJsonObject legalities; legalities["standard"] = "legal"; @@ -65,8 +59,10 @@ static QByteArray buildSyntheticData(int numSets, int cardsPerSet) 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. + QJsonObject numObj; + numObj["number"] = QString::number(c + 1); + numObj["rarity"] = "common"; + // Add per-set properties via nested object (mtgjson format for AllPrintings) card["number"] = QString::number(c + 1); card["rarity"] = "common"; @@ -99,6 +95,7 @@ TEST(OracleBenchmark, ImportThroughput) QByteArray data = buildSyntheticData(numSets, cardsPerSet); + NoopCardSetPriorityController controller; OracleImporter importer; // Phase 1: Parse JSON @@ -119,16 +116,6 @@ TEST(OracleBenchmark, ImportThroughput) 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); @@ -151,6 +138,8 @@ TEST(OracleBenchmark, ParseJsonThroughput) QByteArray data = buildSyntheticData(numSets, cardsPerSet); + NoopCardSetPriorityController controller; + // Run 5 iterations and report average static constexpr int iterations = 5; qint64 totalMs = 0; @@ -324,14 +313,6 @@ struct MemorySnapshot 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; } @@ -351,19 +332,10 @@ static void logRamPhase(const QString &phase, const MemorySnapshot &baseline, co 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") + qDebug().noquote() << QString(" %1: current RSS %2 | peak added %3 | process peak %4") .arg(phase) .arg(formatKb(current.rssKb)) - .arg(rssDelta) + .arg(formatKb(current.peakRssKb - baseline.peakRssKb)) .arg(formatKb(current.peakRssKb)); } @@ -425,17 +397,9 @@ 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); + const MemorySnapshot baseline = MemorySnapshot::current(); NoopCardSetPriorityController controller; OracleImporter importer; @@ -467,23 +431,12 @@ TEST(OracleBenchmark, ImportRamUsage) 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) { + if (qEnvironmentVariableIsEmpty("COCKATRICE_ORACLE_RAM_BENCHMARK")) { 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"); @@ -492,31 +445,19 @@ TEST(OracleBenchmark, ImportRamUsageAllPrintings) 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(); - }); + QObject::connect(&timeoutTimer, &QTimer::timeout, &loop, &QEventLoop::quit); 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) { + if (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 MemorySnapshot baseline = MemorySnapshot::current(); const QByteArray setsData = decompressSetsData(payload); const MemorySnapshot afterDownload = MemorySnapshot::current(); if (setsData.isEmpty()) { diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp index 30f4ed92a..9f4b0ab7b 100644 --- a/tests/oracle/oracle_importer_test.cpp +++ b/tests/oracle/oracle_importer_test.cpp @@ -143,10 +143,8 @@ TEST_F(OracleImporterTest, SingleColorNotSorted) // Legality guard tests // ============================================================================ -TEST_F(OracleImporterTest, NewCardKeepsLegalityProperties) +TEST_F(OracleImporterTest, LegalityGuardCombinesForNewCard) { - // 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"; @@ -159,25 +157,6 @@ TEST_F(OracleImporterTest, NewCardKeepsLegalityProperties) 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 @@ -227,13 +206,11 @@ 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); } @@ -242,7 +219,6 @@ 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") { @@ -257,7 +233,6 @@ 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(); @@ -277,12 +252,10 @@ TEST_F(OracleImporterTest, CreateDefaultMagicFormatsRegexMatchesBasicLands) 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()); + ASSERT_EQ(first.size(), second.size()); + ASSERT_EQ(first.keys(), second.keys()); } // ============================================================================ @@ -341,33 +314,28 @@ TEST_F(OracleImporterTest, ReadSetsFromByteArrayCapitalizesSetType) 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["code"] = "zzz"; setA["name"] = "Zeta Set"; setA["type"] = "expansion"; setA["releaseDate"] = "2024-01-01"; setA["cards"] = QJsonArray(); QJsonObject setB; - setB["code"] = "zzz"; + setB["code"] = "aaa"; setB["name"] = "Alpha Set"; setB["type"] = "expansion"; setB["releaseDate"] = "2024-01-01"; setB["cards"] = QJsonArray(); QJsonObject root; - root["data"] = QJsonObject{{"AAA", setA}, {"ZZZ", setB}}; + root["data"] = QJsonObject{{"ZZZ", setA}, {"AAA", 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"); + ASSERT_EQ(sets.first().getShortName(), "AAA"); } // ============================================================================ @@ -415,9 +383,11 @@ TEST_F(OracleImporterTest, SplitCardColorIdentityConcatenated) 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"); + // coloridentity should be "RU" (concatenated), not "R // U" + QString ci = card->getProperty("coloridentity"); + ASSERT_FALSE(ci.contains("//")) << "coloridentity should not contain '//', got: " << ci.toStdString(); + ASSERT_TRUE(ci.contains("R")); + ASSERT_TRUE(ci.contains("U")); } TEST_F(OracleImporterTest, SplitCardColorsConcatenated) @@ -540,8 +510,8 @@ TEST_F(OracleImporterTest, ScanSetRangesMatchFullJsonParse) const QJsonObject wholeData = QJsonDocument::fromJson(bytes).object().value("data").toObject(); for (const RawJson::SetRange &range : ranges) { QJsonParseError parseError; - const QJsonDocument sliceDoc = QJsonDocument::fromJson( - QByteArray(bytes.constData() + range.dataRange.start, range.dataRange.length), &parseError); + const QJsonDocument sliceDoc = + QJsonDocument::fromJson(QByteArray(bytes.constData() + range.start, range.length), &parseError); ASSERT_EQ(parseError.error, QJsonParseError::NoError) << range.code.toStdString() << ": " << parseError.errorString().toStdString(); ASSERT_EQ(sliceDoc.object(), wholeData.value(range.code).toObject()) << "set " << range.code.toStdString(); @@ -565,11 +535,11 @@ TEST_F(OracleImporterTest, ScanSetRangesDecodesEscapesAndCountsCards) ASSERT_EQ(range.name, expectedName); ASSERT_EQ(range.type, "expansion"); ASSERT_EQ(range.releaseDate, "2024-01-05"); - ASSERT_EQ(range.dataRange.cardCount, 3); + ASSERT_EQ(range.cardCount, 3); QJsonParseError parseError; - const QJsonDocument sliceDoc = QJsonDocument::fromJson( - QByteArray(json.constData() + range.dataRange.start, range.dataRange.length), &parseError); + const QJsonDocument sliceDoc = + QJsonDocument::fromJson(QByteArray(json.constData() + range.start, range.length), &parseError); ASSERT_EQ(parseError.error, QJsonParseError::NoError); ASSERT_EQ(sliceDoc.object().value("name").toString(), expectedName); ASSERT_EQ(sliceDoc.object().value("cards").toArray().size(), 3); @@ -595,67 +565,6 @@ TEST_F(OracleImporterTest, ScanSetRangesRejectsInvalidJson) } } -TEST_F(OracleImporterTest, ScanSetRangesMatchesFullJsonParseVerdicts) -{ - // Verdicts must agree with QJsonDocument::fromJson for the inputs below — - // including the metadata quirks ("name": null, "type": 7, "releaseDate": null, - // "cards": null) that used to make the scanner reject sets Qt accepts. - const QList inputs = { - "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"type\":\"x\",\"releaseDate\":\"2024-01-01\",\"cards\":[{" - "\"n\":1}]}}}", - "{\"data\":{\"A\":{\"code\":\"a\",\"name\":null,\"type\":\"x\",\"releaseDate\":\"2024-01-01\",\"cards\":[]}}}", - "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"type\":null,\"releaseDate\":\"2024-01-01\",\"cards\":[]}}}", - "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"type\":7,\"releaseDate\":\"2024-01-01\",\"cards\":null}}}", - "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"releaseDate\":\"2024-01-01\",\"cards\":[1,2,3]}}}", - // structurally invalid JSON (both parsers must reject) - "not json", - "{\"data\":{\"A\":{\"name\":\"unterminated}}", - }; - - for (const QByteArray &input : inputs) { - QJsonParseError qtError; - QJsonDocument::fromJson(input, &qtError); - const bool qtOk = qtError.error == QJsonParseError::NoError; - - RawJson::ScanError scanError; - const QList ranges = RawJson::scanSetRanges(input, &scanError); - EXPECT_EQ(qtOk, !scanError.isError()) << "verdict mismatch for: " << input.constData(); - if (scanError.isError()) { - continue; - } - for (const RawJson::SetRange &range : ranges) { - QJsonParseError sliceError; - QJsonDocument::fromJson(QByteArray(input.constData() + range.dataRange.start, range.dataRange.length), - &sliceError); - EXPECT_EQ(sliceError.error, QJsonParseError::NoError) << "bad range slice for: " << input.constData(); - } - } -} - -TEST_F(OracleImporterTest, ScanSetRangesRejectsDeepNesting) -{ - // Far beyond the shared 1024 container cap: Qt reports DeepNesting and the - // scanner must reject too, without overflowing the stack through its - // recursive skipValue walk. - QString nesting; - nesting.reserve(10000); - for (int i = 0; i < 5000; ++i) { - nesting += '['; - } - for (int i = 0; i < 5000; ++i) { - nesting += ']'; - } - const QByteArray json = ("{\"data\":{\"A\":{\"code\":\"a\",\"cards\":" + nesting + "}}}").toUtf8(); - - QJsonParseError qtError; - QJsonDocument::fromJson(json, &qtError); - ASSERT_NE(qtError.error, QJsonParseError::NoError) << "expected Qt to reject deep nesting"; - - RawJson::ScanError scanError; - RawJson::scanSetRanges(json, &scanError); - ASSERT_TRUE(scanError.isError()) << "scanner accepted a document Qt rejects as too deeply nested"; -} - // ============================================================================ // Lazy per-set parsing tests // ============================================================================