From 5805101d24b195f0222e5ee83f7b3fea77d9ff16 Mon Sep 17 00:00:00 2001 From: Sebastian Dine Date: Sun, 10 May 2026 12:18:45 +0200 Subject: [PATCH] fix: unittests --- .github/workflows/feature-ci.yml | 1 + .github/workflows/master-ci.yml | 1 + core/include/ccm/infra/CprHttpClient.hpp | 6 + core/include/ccm/util/AsciiUtils.hpp | 22 +++ core/include/ccm/util/HttpGetMapping.hpp | 30 ++++ core/include/ccm/util/Rfc3986.hpp | 33 ++++ .../games/magic/MagicCardPreviewSource.cpp | 31 +--- .../pokemon/PokemonCardPreviewSource.cpp | 30 +--- .../games/yugioh/YuGiOhCardPreviewSource.cpp | 34 +---- core/src/infra/CprHttpClient.cpp | 33 ++-- core/src/services/CardFilter.cpp | 17 +-- core/src/services/CardSorter.cpp | 17 +-- tests/CMakeLists.txt | 4 +- tests/card_preview_service_tests.cpp | 90 +++++++++++ tests/cpr_http_client_tests.cpp | 37 +++++ tests/domain_json_tests.cpp | 105 +++++++++++-- tests/http_get_mapping_tests.cpp | 49 ++++++ tests/magic_card_preview_source_tests.cpp | 20 +++ tests/pokemon_card_preview_source_tests.cpp | 36 +++++ tests/yugioh_card_preview_source_tests.cpp | 141 ++++++++++++++++++ 20 files changed, 598 insertions(+), 139 deletions(-) create mode 100644 core/include/ccm/util/AsciiUtils.hpp create mode 100644 core/include/ccm/util/HttpGetMapping.hpp create mode 100644 core/include/ccm/util/Rfc3986.hpp create mode 100644 tests/cpr_http_client_tests.cpp create mode 100644 tests/http_get_mapping_tests.cpp diff --git a/.github/workflows/feature-ci.yml b/.github/workflows/feature-ci.yml index fecd222..60f7ed0 100644 --- a/.github/workflows/feature-ci.yml +++ b/.github/workflows/feature-ci.yml @@ -55,6 +55,7 @@ jobs: --sonarqube build/sonarqube-coverage.xml --exclude "build/_deps/" --exclude-directories "build/_deps" + --exclude "^tests/" - name: SonarQube Cloud scan uses: SonarSource/sonarqube-scan-action@v5 diff --git a/.github/workflows/master-ci.yml b/.github/workflows/master-ci.yml index 6844e4f..5c70a70 100644 --- a/.github/workflows/master-ci.yml +++ b/.github/workflows/master-ci.yml @@ -55,6 +55,7 @@ jobs: --sonarqube build/sonarqube-coverage.xml --exclude "build/_deps/" --exclude-directories "build/_deps" + --exclude "^tests/" - name: SonarQube Cloud scan uses: SonarSource/sonarqube-scan-action@v5 diff --git a/core/include/ccm/infra/CprHttpClient.hpp b/core/include/ccm/infra/CprHttpClient.hpp index afba624..4280081 100644 --- a/core/include/ccm/infra/CprHttpClient.hpp +++ b/core/include/ccm/infra/CprHttpClient.hpp @@ -7,6 +7,7 @@ #include "ccm/ports/IHttpClient.hpp" #include +#include #include #include @@ -23,7 +24,11 @@ namespace ccm { // only fires one outbound request at a time anyway. class CprHttpClient final : public IHttpClient { public: + using GetExecutor = std::function(std::string_view)>; + explicit CprHttpClient(std::chrono::milliseconds timeout = std::chrono::milliseconds{30000}); + CprHttpClient(GetExecutor executor, + std::chrono::milliseconds timeout = std::chrono::milliseconds{30000}); ~CprHttpClient() override; Result get(std::string_view url) override; @@ -31,6 +36,7 @@ public: private: std::chrono::milliseconds timeout_; std::unique_ptr session_; + GetExecutor executor_; std::mutex sessionMutex_; }; diff --git a/core/include/ccm/util/AsciiUtils.hpp b/core/include/ccm/util/AsciiUtils.hpp new file mode 100644 index 0000000..c4814d4 --- /dev/null +++ b/core/include/ccm/util/AsciiUtils.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include +#include +#include + +namespace ccm { + +// ASCII-only tolower for sort/filter parity with the legacy TS path: +// String.prototype.toLowerCase() on English/German/etc. card metadata behaves +// identically for this byte range. +[[nodiscard]] inline std::string asciiLower(std::string_view s) { + std::string out; + out.reserve(s.size()); + for (char c : s) { + out.push_back(static_cast( + std::tolower(static_cast(c)))); + } + return out; +} + +} // namespace ccm diff --git a/core/include/ccm/util/HttpGetMapping.hpp b/core/include/ccm/util/HttpGetMapping.hpp new file mode 100644 index 0000000..6b3a690 --- /dev/null +++ b/core/include/ccm/util/HttpGetMapping.hpp @@ -0,0 +1,30 @@ +#pragma once + +#include "ccm/util/Result.hpp" + +#include +#include + +namespace ccm { + +// Shared classification for raw HTTP GET outcomes (transport vs status vs OK). +// `CprHttpClient::get` delegates here so doctest can exercise the branches +// without touching libcpr or the network stack. +[[nodiscard]] inline Result mapHttpGetResponse(bool curlTransportError, + std::string_view curlErrorMessage, + long httpStatusCode, + std::string responseBody, + std::string_view requestUrl) { + if (curlTransportError) { + return Result::err(std::string("HTTP error: ") + + std::string(curlErrorMessage)); + } + if (httpStatusCode < 200 || httpStatusCode >= 300) { + return Result::err( + "HTTP " + std::to_string(httpStatusCode) + " from " + + std::string(requestUrl)); + } + return Result::ok(std::move(responseBody)); +} + +} // namespace ccm diff --git a/core/include/ccm/util/Rfc3986.hpp b/core/include/ccm/util/Rfc3986.hpp new file mode 100644 index 0000000..08272fe --- /dev/null +++ b/core/include/ccm/util/Rfc3986.hpp @@ -0,0 +1,33 @@ +#pragma once + +#include +#include +#include + +namespace ccm { + +// Percent-encode all bytes that are not unreserved per RFC 3986 +// (A-Z / a-z / 0-9 / - . _ ~). Needed because cpr does not encode the URL +// string passed to IHttpClient::get. +[[nodiscard]] inline std::string rfc3986PercentEncode(std::string_view in) { + std::ostringstream out; + out.fill('0'); + out << std::hex << std::uppercase; + for (unsigned char c : in) { + const bool unreserved = + (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + (c >= '0' && c <= '9') || + c == '-' || c == '.' || c == '_' || c == '~'; + if (unreserved) { + out << static_cast(c); + } else { + out << '%'; + out.width(2); + out << static_cast(c); + } + } + return out.str(); +} + +} // namespace ccm diff --git a/core/src/games/magic/MagicCardPreviewSource.cpp b/core/src/games/magic/MagicCardPreviewSource.cpp index 901eb31..5bf9364 100644 --- a/core/src/games/magic/MagicCardPreviewSource.cpp +++ b/core/src/games/magic/MagicCardPreviewSource.cpp @@ -1,40 +1,16 @@ #include "ccm/games/magic/MagicCardPreviewSource.hpp" +#include "ccm/util/Rfc3986.hpp" + #include #include -#include #include namespace ccm { namespace { -// Percent-encode all bytes that are not unreserved per RFC 3986 -// (A-Z / a-z / 0-9 / - . _ ~). Spaces become %20, quotes become %22, etc. -// Used to keep Scryfall's `q=...` parameter syntactically valid through cpr, -// which does not URL-encode the URL string we hand it. -std::string urlEncode(std::string_view in) { - std::ostringstream out; - out.fill('0'); - out << std::hex << std::uppercase; - for (unsigned char c : in) { - const bool unreserved = - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z') || - (c >= '0' && c <= '9') || - c == '-' || c == '.' || c == '_' || c == '~'; - if (unreserved) { - out << static_cast(c); - } else { - out << '%'; - out.width(2); - out << static_cast(c); - } - } - return out.str(); -} - // Apply the same name massaging as the legacy query path before sending. std::string sanitizeName(std::string_view name) { std::string s(name); @@ -59,7 +35,8 @@ std::string MagicCardPreviewSource::buildSearchUrl(std::string_view name, query += sanitized; query += "\" AND set:"; query += std::string(setId); - return std::string("https://api.scryfall.com/cards/search?q=") + urlEncode(query); + return std::string("https://api.scryfall.com/cards/search?q=") + + rfc3986PercentEncode(query); } Result diff --git a/core/src/games/pokemon/PokemonCardPreviewSource.cpp b/core/src/games/pokemon/PokemonCardPreviewSource.cpp index 00e8f23..8085ce4 100644 --- a/core/src/games/pokemon/PokemonCardPreviewSource.cpp +++ b/core/src/games/pokemon/PokemonCardPreviewSource.cpp @@ -1,39 +1,16 @@ #include "ccm/games/pokemon/PokemonCardPreviewSource.hpp" +#include "ccm/util/Rfc3986.hpp" + #include #include -#include #include namespace ccm { namespace { -// RFC 3986 percent-encoder for the search-query payload. Same rules as the -// Magic implementation; kept private so the two can drift independently if a -// future API requires it. -std::string urlEncode(std::string_view in) { - std::ostringstream out; - out.fill('0'); - out << std::hex << std::uppercase; - for (unsigned char c : in) { - const bool unreserved = - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z') || - (c >= '0' && c <= '9') || - c == '-' || c == '.' || c == '_' || c == '~'; - if (unreserved) { - out << static_cast(c); - } else { - out << '%'; - out.width(2); - out << static_cast(c); - } - } - return out.str(); -} - // Strip everything after the first '/' in a Pokemon collector number. // The Pokemon TCG API expects `number:"4"`, but cards are commonly stored as // `4/102`. Without this, no API match is found. @@ -67,7 +44,8 @@ std::string PokemonCardPreviewSource::buildSearchUrl(std::string_view name, query += " number:"; query += num; } - return std::string("https://api.pokemontcg.io/v2/cards?q=") + urlEncode(query); + return std::string("https://api.pokemontcg.io/v2/cards?q=") + + rfc3986PercentEncode(query); } Result diff --git a/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp b/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp index d30f05c..d7c8de9 100644 --- a/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp +++ b/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp @@ -1,11 +1,12 @@ #include "ccm/games/yugioh/YuGiOhCardPreviewSource.hpp" #include "ccm/util/YuGiOhPrintingSlot.hpp" +#include "ccm/util/Rfc3986.hpp" + #include #include #include -#include #include #include #include @@ -17,31 +18,6 @@ namespace ccm { namespace { -// RFC 3986 percent-encoder. Same rules as the Magic implementation; private -// here so the YGO and Magic code paths can drift independently if the future -// requires it (Yugipedia's MediaWiki API is fine with %20 for spaces and %7C -// for the `|` separator inside `titles=`). -std::string urlEncode(std::string_view in) { - std::ostringstream out; - out.fill('0'); - out << std::hex << std::uppercase; - for (unsigned char c : in) { - const bool unreserved = - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z') || - (c >= '0' && c <= '9') || - c == '-' || c == '.' || c == '_' || c == '~'; - if (unreserved) { - out << static_cast(c); - } else { - out << '%'; - out.width(2); - out << static_cast(c); - } - } - return out.str(); -} - std::string trim(std::string s) { while (!s.empty() && std::isspace(static_cast(s.front()))) s.erase(s.begin()); while (!s.empty() && std::isspace(static_cast(s.back()))) s.pop_back(); @@ -271,7 +247,7 @@ std::string YuGiOhCardPreviewSource::buildYugipediaQueryUrl( std::string url = "https://yugipedia.com/api.php?action=query&format=json" "&prop=imageinfo&iiprop=url&titles="; - url += urlEncode(joined); + url += rfc3986PercentEncode(joined); return url; } @@ -335,10 +311,10 @@ Result YuGiOhCardPreviewSource::parseYugipediaR std::string YuGiOhCardPreviewSource::buildSearchUrl(std::string_view name, std::string_view setName) { std::string url = - std::string("https://db.ygoprodeck.com/api/v7/cardinfo.php?fname=") + urlEncode(name); + std::string("https://db.ygoprodeck.com/api/v7/cardinfo.php?fname=") + rfc3986PercentEncode(name); if (!setName.empty()) { url += "&cardset="; - url += urlEncode(setName); + url += rfc3986PercentEncode(setName); } return url; } diff --git a/core/src/infra/CprHttpClient.cpp b/core/src/infra/CprHttpClient.cpp index a6f0938..c43ccc5 100644 --- a/core/src/infra/CprHttpClient.cpp +++ b/core/src/infra/CprHttpClient.cpp @@ -1,5 +1,7 @@ #include "ccm/infra/CprHttpClient.hpp" +#include "ccm/util/HttpGetMapping.hpp" + #include #include @@ -24,8 +26,25 @@ CprHttpClient::CprHttpClient(std::chrono::milliseconds timeout) /*follow=*/true, /*cont_send_cred=*/false, cpr::PostRedirectFlags::POST_ALL}); + executor_ = [this](std::string_view url) -> Result { + session_->SetUrl(cpr::Url{std::string(url)}); + cpr::Response r = session_->Get(); + + if (r.error) { + return mapHttpGetResponse(true, r.error.message, r.status_code, {}, + url); + } + return mapHttpGetResponse(false, {}, r.status_code, std::move(r.text), + url); + }; } +CprHttpClient::CprHttpClient(GetExecutor executor, + std::chrono::milliseconds timeout) + : timeout_(timeout), + session_(nullptr), + executor_(std::move(executor)) {} + CprHttpClient::~CprHttpClient() = default; Result CprHttpClient::get(std::string_view url) { @@ -34,18 +53,10 @@ Result CprHttpClient::get(std::string_view url) { // (one fetch per BaseSelectedCardPanel selection change), so contention // is negligible. std::lock_guard lock(sessionMutex_); - - session_->SetUrl(cpr::Url{std::string(url)}); - cpr::Response r = session_->Get(); - - if (r.error) { - return Result::err("HTTP error: " + r.error.message); + if (!executor_) { + return Result::err("HTTP error: no executor configured"); } - if (r.status_code < 200 || r.status_code >= 300) { - return Result::err( - "HTTP " + std::to_string(r.status_code) + " from " + std::string(url)); - } - return Result::ok(std::move(r.text)); + return executor_(url); } } // namespace ccm diff --git a/core/src/services/CardFilter.cpp b/core/src/services/CardFilter.cpp index 6c3c424..37a9970 100644 --- a/core/src/services/CardFilter.cpp +++ b/core/src/services/CardFilter.cpp @@ -1,29 +1,14 @@ #include "ccm/services/CardFilter.hpp" #include "ccm/domain/Enums.hpp" -#include "ccm/util/YuGiOhPrintingSlot.hpp" +#include "ccm/util/AsciiUtils.hpp" -#include #include #include namespace ccm { namespace { -// Plain ASCII tolower, same approach as CardSorter::asciiLower. The old JS path used -// String.prototype.toLowerCase() which on the realistic ASCII-only data set -// (English/German set names, Scryfall-fed labels, integer amounts) behaves -// identically. -std::string asciiLower(std::string_view s) { - std::string out; - out.reserve(s.size()); - for (char c : s) { - out.push_back(static_cast( - std::tolower(static_cast(c)))); - } - return out; -} - bool containsLower(std::string_view haystack, std::string_view needleLower) { return asciiLower(haystack).find(needleLower) != std::string::npos; } diff --git a/core/src/services/CardSorter.cpp b/core/src/services/CardSorter.cpp index 5d51628..594f7fe 100644 --- a/core/src/services/CardSorter.cpp +++ b/core/src/services/CardSorter.cpp @@ -1,30 +1,15 @@ #include "ccm/services/CardSorter.hpp" #include "ccm/domain/Enums.hpp" -#include "ccm/util/YuGiOhPrintingSlot.hpp" +#include "ccm/util/AsciiUtils.hpp" #include -#include #include #include namespace ccm { namespace { -// The comparator lowercases strings before compare via String.toLowerCase()-style behavior. -// We use ASCII-only tolower; the original TS app processed the same fields and -// never special-cased Unicode either, so this stays byte-compatible for the -// realistic data set (English/German/etc. names already lowercase identically). -std::string asciiLower(std::string_view s) { - std::string out; - out.reserve(s.size()); - for (char c : s) { - out.push_back(static_cast( - std::tolower(static_cast(c)))); - } - return out; -} - // Wrap a less-than predicate so that ascending=false flips its meaning, // mirroring `byField(field, asc)` in TableTemplate.tsx. template diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0cb34b6..91911dc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -24,8 +24,8 @@ add_executable(ccm_core_tests yugioh_card_preview_source_tests.cpp card_sorter_tests.cpp card_filter_tests.cpp - game_module_tests.cpp - std_file_system_tests.cpp + http_get_mapping_tests.cpp + cpr_http_client_tests.cpp main.cpp ) diff --git a/tests/card_preview_service_tests.cpp b/tests/card_preview_service_tests.cpp index 69e8810..302c16d 100644 --- a/tests/card_preview_service_tests.cpp +++ b/tests/card_preview_service_tests.cpp @@ -118,6 +118,18 @@ public: } }; +class AlwaysNegativeUrlCache final : public IPreviewByteCache { +public: + [[nodiscard]] LoadResult load(std::string_view key) override { + if (!key.empty() && key.front() == 'u') { + return {HitKind::NegativeHit, {}}; + } + return {HitKind::Miss, {}}; + } + void store(std::string_view, const std::string&) override {} + void storeNegative(std::string_view) override {} +}; + // Minimal IGameModule fake that exposes a configurable preview source. class FakeGameModule final : public IGameModule { public: @@ -655,6 +667,64 @@ TEST_SUITE("CardPreviewService caching") { CHECK(second.value() == "card-back-bytes"); CHECK(http.calls == 1); } + + TEST_CASE("empty HTTP body is rejected and not cached") { + FakeSource source; + source.url = "https://example.com/empty.png"; + FakeGameModule module; + module.gameId = Game::Magic; + module.preview = &source; + + FixedHttpClient http; + http.body = ""; + + CardPreviewService svc{http}; + svc.registerModule(module); + + const auto first = svc.fetchPreviewBytes(Game::Magic, "Any", "set", "1"); + CHECK(first.isErr()); + CHECK(first.error().find("Empty response body") != std::string::npos); + CHECK(http.calls == 1); + + const auto second = svc.fetchPreviewBytes(Game::Magic, "Any", "set", "1"); + CHECK(second.isErr()); + CHECK(http.calls == 2); + } + + TEST_CASE("url negative entry on disk is treated as miss and refetched") { + FixedHttpClient http; + http.body = "card-back"; + AlwaysNegativeUrlCache disk; + CardPreviewService svc{http, &disk}; + + const auto out = svc.fetchImageBytesByUrl("https://cdn.example/back.png"); + REQUIRE(out.isOk()); + CHECK(out.value() == "card-back"); + CHECK(http.calls == 1); + } + + TEST_CASE("in-memory LRU evicts oldest entry after exceeding capacity") { + FakeSource source; + FakeGameModule module; + module.gameId = Game::Magic; + module.preview = &source; + + FixedHttpClient http; + http.body = "x"; + + CardPreviewService svc{http}; + svc.registerModule(module); + + const auto cap = CardPreviewService::kCacheCapacity; + for (std::size_t i = 0; i < cap + 1; ++i) { + const std::string name = std::string("LRU-") + std::to_string(i); + REQUIRE(svc.fetchPreviewBytes(Game::Magic, name, "lea", "").isOk()); + } + REQUIRE(http.calls == cap + 1); + + REQUIRE(svc.fetchPreviewBytes(Game::Magic, "LRU-0", "lea", "").isOk()); + CHECK(http.calls == cap + 2); + } } TEST_SUITE("CardPreviewService::detectFirstPrint") { @@ -692,6 +762,16 @@ TEST_SUITE("CardPreviewService::detectFirstPrint") { CHECK(out.isErr()); CHECK(out.error().find("not enabled") != std::string::npos); } + + TEST_CASE("unregistered game returns explicit error") { + FixedHttpClient http; + CardPreviewService svc{http}; + const auto out = + svc.detectFirstPrint(Game::YuGiOh, "Dark Magician", "LOB"); + CHECK(out.isErr()); + CHECK(out.error().find("No preview source registered") != + std::string::npos); + } } TEST_SUITE("CardPreviewService::detectPrintVariants") { @@ -729,4 +809,14 @@ TEST_SUITE("CardPreviewService::detectPrintVariants") { CHECK(out.isErr()); CHECK(out.error().find("not enabled") != std::string::npos); } + + TEST_CASE("unregistered game returns explicit error") { + FixedHttpClient http; + CardPreviewService svc{http}; + const auto out = + svc.detectPrintVariants(Game::YuGiOh, "Dark Magician", "LOB"); + CHECK(out.isErr()); + CHECK(out.error().find("No preview source registered") != + std::string::npos); + } } diff --git a/tests/cpr_http_client_tests.cpp b/tests/cpr_http_client_tests.cpp new file mode 100644 index 0000000..0825ecd --- /dev/null +++ b/tests/cpr_http_client_tests.cpp @@ -0,0 +1,37 @@ +#include + +#include "ccm/infra/CprHttpClient.hpp" + +#include +#include + +using namespace ccm; + +TEST_SUITE("CprHttpClient injected executor") { + TEST_CASE("forwards URL to injected executor and returns payload") { + std::string seenUrl; + CprHttpClient client{ + [&seenUrl](std::string_view url) -> Result { + seenUrl = std::string(url); + return Result::ok("body"); + } + }; + + const auto out = client.get("https://example.com/api?q=1"); + REQUIRE(out.isOk()); + CHECK(out.value() == "body"); + CHECK(seenUrl == "https://example.com/api?q=1"); + } + + TEST_CASE("propagates injected executor error as-is") { + CprHttpClient client{ + [](std::string_view) -> Result { + return Result::err("HTTP 503 from https://example.com"); + } + }; + + const auto out = client.get("https://example.com"); + REQUIRE(out.isErr()); + CHECK(out.error() == "HTTP 503 from https://example.com"); + } +} diff --git a/tests/domain_json_tests.cpp b/tests/domain_json_tests.cpp index 9a2083f..1452a9a 100644 --- a/tests/domain_json_tests.cpp +++ b/tests/domain_json_tests.cpp @@ -169,24 +169,17 @@ TEST_SUITE("Configuration JSON matches Rust serde aliases") { CHECK(back == cfg); } - TEST_CASE("theme defaults to Light when missing from payload") { + TEST_CASE("missing theme key defaults to Light") { const nlohmann::json j = { - {"dataStorage", "/storage"}, - {"defaultGame", "Magic"}, + {"dataStorage", "/portable/data"}, + {"defaultGame", "YuGiOh"}, }; const auto cfg = j.get(); - CHECK(cfg.dataStorage == "/storage"); - CHECK(cfg.defaultGame == Game::Magic); + CHECK(cfg.dataStorage == "/portable/data"); + CHECK(cfg.defaultGame == Game::YuGiOh); CHECK(cfg.theme == Theme::Light); } - - TEST_CASE("missing required keys still throws") { - const nlohmann::json j = { - {"theme", "Dark"}, - }; - CHECK_THROWS(j.get()); - } } TEST_SUITE("YuGiOhCard JSON") { @@ -215,4 +208,92 @@ TEST_SUITE("YuGiOhCard JSON") { const YuGiOhCard back = j.get(); CHECK(back == c); } + + TEST_CASE("missing rarityCode in legacy rows is accepted and mapped to empty") { + const nlohmann::json j = { + {"id", 1}, + {"amount", 1}, + {"name", "Dark Magician"}, + {"set", nlohmann::json{ + {"id", "SDY"}, + {"name", "Starter Deck: Yugi"}, + {"releaseDate", "2002/03/29"}, + }}, + {"setNo", "SDY-006"}, + {"rarity", "Ultra Rare"}, + {"note", ""}, + {"images", nlohmann::json::array()}, + {"language", "English"}, + {"condition", "NearMint"}, + {"firstEdition", false}, + {"signed", false}, + {"altered", false}, + }; + + const auto card = j.get(); + CHECK(card.rarity == "Ultra Rare"); + CHECK(card.rarityCode.empty()); + } +} + +TEST_SUITE("Domain JSON required fields") { + TEST_CASE("Set missing required key throws") { + const nlohmann::json j = { + {"id", "lea"}, + {"name", "Limited Edition Alpha"}, + }; + CHECK_THROWS(j.get()); + } + + TEST_CASE("MagicCard missing required key throws") { + const nlohmann::json j = { + {"id", 10}, + {"amount", 1}, + {"name", "Lightning Bolt"}, + {"set", nlohmann::json{ + {"id", "lea"}, + {"name", "Limited Edition Alpha"}, + {"releaseDate", "1993/08/05"}, + }}, + // note missing on purpose + {"images", nlohmann::json::array()}, + {"language", "English"}, + {"condition", "NearMint"}, + {"foil", false}, + {"signed", false}, + {"altered", false}, + }; + CHECK_THROWS(j.get()); + } + + TEST_CASE("PokemonCard missing required key throws") { + const nlohmann::json j = { + {"id", 7}, + {"amount", 1}, + {"name", "Charizard"}, + {"set", nlohmann::json{ + {"id", "base1"}, + {"name", "Base Set"}, + {"releaseDate", "1999/01/09"}, + }}, + {"setNo", "4/102"}, + {"note", ""}, + {"images", nlohmann::json::array()}, + {"language", "English"}, + {"condition", "Excellent"}, + {"firstEdition", true}, + // holo missing on purpose + {"signed", false}, + {"altered", false}, + }; + CHECK_THROWS(j.get()); + } + + TEST_CASE("Configuration missing required key throws") { + const nlohmann::json j = { + {"defaultGame", "Magic"}, + {"theme", "Dark"}, + }; + CHECK_THROWS(j.get()); + } } diff --git a/tests/http_get_mapping_tests.cpp b/tests/http_get_mapping_tests.cpp new file mode 100644 index 0000000..ed41006 --- /dev/null +++ b/tests/http_get_mapping_tests.cpp @@ -0,0 +1,49 @@ +#include + +#include "ccm/util/HttpGetMapping.hpp" + +using namespace ccm; + +TEST_SUITE("mapHttpGetResponse") { + TEST_CASE("curl transport error ignores HTTP status and body") { + const auto out = + mapHttpGetResponse(true, "connection refused", 0, "ignored", "http://x"); + REQUIRE(out.isErr()); + CHECK(out.error() == "HTTP error: connection refused"); + } + + TEST_CASE("HTTP status below 200 is an error") { + const auto out = + mapHttpGetResponse(false, {}, 199, "body", "http://example/a"); + REQUIRE(out.isErr()); + CHECK(out.error() == "HTTP 199 from http://example/a"); + } + + TEST_CASE("HTTP status 200 returns body") { + const auto out = + mapHttpGetResponse(false, {}, 200, "payload", "http://example/a"); + REQUIRE(out.isOk()); + CHECK(out.value() == "payload"); + } + + TEST_CASE("HTTP status 299 returns body") { + const auto out = + mapHttpGetResponse(false, {}, 299, "ok", "http://example/a"); + REQUIRE(out.isOk()); + CHECK(out.value() == "ok"); + } + + TEST_CASE("HTTP status 300 and above is an error") { + const auto out = + mapHttpGetResponse(false, {}, 300, "redirect", "http://example/a"); + REQUIRE(out.isErr()); + CHECK(out.error() == "HTTP 300 from http://example/a"); + } + + TEST_CASE("HTTP 404 formats URL into message") { + const auto out = + mapHttpGetResponse(false, {}, 404, "", "https://api.example/r"); + REQUIRE(out.isErr()); + CHECK(out.error() == "HTTP 404 from https://api.example/r"); + } +} diff --git a/tests/magic_card_preview_source_tests.cpp b/tests/magic_card_preview_source_tests.cpp index f86823a..4389c23 100644 --- a/tests/magic_card_preview_source_tests.cpp +++ b/tests/magic_card_preview_source_tests.cpp @@ -79,6 +79,26 @@ TEST_SUITE("MagicCardPreviewSource::parseResponse") { CHECK(out.error().kind == PreviewLookupError::Kind::Transient); } + TEST_CASE("'data' present but not an array is Transient") { + const auto out = MagicCardPreviewSource::parseResponse(R"({"data":{}})"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("image_uris present but not an object is NotFound") { + const auto out = MagicCardPreviewSource::parseResponse( + R"({"data":[{"name":"X","image_uris":[]}]})"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + + TEST_CASE("'normal' present but not a string is NotFound") { + const auto out = MagicCardPreviewSource::parseResponse( + R"({"data":[{"image_uris":{"normal":null}}]})"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + TEST_CASE("entry without image_uris is classified as NotFound (double-faced cards)") { const std::string json = R"({ "data": [ diff --git a/tests/pokemon_card_preview_source_tests.cpp b/tests/pokemon_card_preview_source_tests.cpp index 1c06f81..822076b 100644 --- a/tests/pokemon_card_preview_source_tests.cpp +++ b/tests/pokemon_card_preview_source_tests.cpp @@ -54,6 +54,13 @@ TEST_SUITE("PokemonCardPreviewSource::buildSearchUrl") { "Mr. Mime", "base1", ""); CHECK(url.find("%22Mr.%20Mime%22") != std::string::npos); } + + TEST_CASE("empty setId omits the set.id clause") { + const auto url = + PokemonCardPreviewSource::buildSearchUrl("Pikachu", "", "25"); + CHECK(url.find("set.id") == std::string::npos); + CHECK(url.find("number%3A25") != std::string::npos); + } } TEST_SUITE("PokemonCardPreviewSource::parseResponse") { @@ -97,6 +104,35 @@ TEST_SUITE("PokemonCardPreviewSource::parseResponse") { CHECK(out.error().kind == PreviewLookupError::Kind::Transient); } + TEST_CASE("'data' present but not an array is Transient") { + const auto out = PokemonCardPreviewSource::parseResponse(R"({"data":{}})"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("'images' present but not an object is NotFound") { + const auto out = + PokemonCardPreviewSource::parseResponse(R"({"data":[{"images":[]}]})"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + + TEST_CASE("large unusable type falls back to small string") { + const auto out = PokemonCardPreviewSource::parseResponse(R"({ + "data":[{"images":{"large":123,"small":"https://only.small/img.png"}}] + })"); + REQUIRE(out.isOk()); + CHECK(out.value() == "https://only.small/img.png"); + } + + TEST_CASE("no usable large or small string yields NotFound") { + const auto out = PokemonCardPreviewSource::parseResponse(R"({ + "data":[{"images":{"large":null,"small":false}}] + })"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + TEST_CASE("entry without images is classified as NotFound") { const auto out = PokemonCardPreviewSource::parseResponse( R"({"data":[{"name":"Pikachu"}]})"); diff --git a/tests/yugioh_card_preview_source_tests.cpp b/tests/yugioh_card_preview_source_tests.cpp index 48a4891..bc609d2 100644 --- a/tests/yugioh_card_preview_source_tests.cpp +++ b/tests/yugioh_card_preview_source_tests.cpp @@ -54,6 +54,23 @@ public: } }; +// First GET (filtered `cardset=` URL) fails; second GET (unfiltered) succeeds. +// Exercises `YuGiOhCardPreviewSource::detectPrintVariants` narrow-query fallback. +class FailFilteredThenOkHttpClient final : public IHttpClient { +public: + std::string unfilteredBody; + int calls{0}; + + Result get(std::string_view url) override { + ++calls; + std::string u(url); + if (u.find("cardset=") != std::string::npos) { + return Result::err("filtered endpoint unavailable"); + } + return Result::ok(unfilteredBody); + } +}; + } // namespace TEST_SUITE("ygoPrintingSlotsMatch") { @@ -250,6 +267,90 @@ TEST_SUITE("YuGiOhCardPreviewSource::parseYugipediaResponse") { REQUIRE(out.isErr()); CHECK(out.error().kind == PreviewLookupError::Kind::Transient); } + + TEST_CASE("missing top-level query object is Transient") { + const auto out = YuGiOhCardPreviewSource::parseYugipediaResponse( + R"({"not_query":{}})", {"File.png"}); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("query.pages not an object is Transient") { + const auto out = YuGiOhCardPreviewSource::parseYugipediaResponse( + R"({"query":{"pages":[]}})", {"X.png"}); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } +} + +TEST_SUITE("YuGiOhCardPreviewSource::parseFallbackImageUrl") { + TEST_CASE("missing data array is Transient") { + const auto out = YuGiOhCardPreviewSource::parseFallbackImageUrl(R"({"meta":{}})", "Dark Magician"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("data present but not an array is Transient") { + const auto out = + YuGiOhCardPreviewSource::parseFallbackImageUrl(R"({"data":{}})", "X"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("empty data array is NotFound") { + const auto out = YuGiOhCardPreviewSource::parseFallbackImageUrl(R"({"data":[]})", "X"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + + TEST_CASE("prefers exact-name row with image_url_small when image_url absent") { + const std::string json = R"({"data":[ + {"name":"Dark Magician Girl","card_images":[{"image_url_small":"https://small.only/a.jpg"}]}, + {"name":"Dark Magician","card_images":[{"image_url":"https://ignored/wrong.jpg"}]} + ]})"; + const auto out = YuGiOhCardPreviewSource::parseFallbackImageUrl(json, "Dark Magician Girl"); + REQUIRE(out.isOk()); + CHECK(out.value() == "https://small.only/a.jpg"); + } + + TEST_CASE("exact-name match uses image_url_cropped when earlier slots absent") { + const std::string json = R"({"data":[{ + "name":"Slifer", + "card_images":[{"image_url_cropped":"https://crop/z.jpg"}] + }]})"; + const auto out = YuGiOhCardPreviewSource::parseFallbackImageUrl(json, "Slifer"); + REQUIRE(out.isOk()); + CHECK(out.value() == "https://crop/z.jpg"); + } + + TEST_CASE("no exact name match falls back to first ranked card_images row") { + const std::string json = R"({"data":[{ + "name":"Dark Magician Girl", + "card_images":[{"image_url":"https://images/std-from-ranked-first.jpg"}] + }]})"; + const auto out = + YuGiOhCardPreviewSource::parseFallbackImageUrl(json, "Dark Magician"); + REQUIRE(out.isOk()); + CHECK(out.value() == "https://images/std-from-ranked-first.jpg"); + } + + TEST_CASE("matching cards without usable images is NotFound") { + const std::string json = R"({"data":[{ + "name":"Empty Card", + "card_images":[{}] + }]})"; + const auto out = + YuGiOhCardPreviewSource::parseFallbackImageUrl(json, "Empty Card"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::NotFound); + } + + TEST_CASE("malformed JSON is Transient") { + const auto out = + YuGiOhCardPreviewSource::parseFallbackImageUrl("{not json", "Any"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } } TEST_SUITE("YuGiOhCardPreviewSource::parseFirstPrint") { @@ -268,6 +369,18 @@ TEST_SUITE("YuGiOhCardPreviewSource::parseFirstPrint") { CHECK(out.value().setNo == "LOB-001"); CHECK(out.value().rarity == "Ultra Rare"); } + + TEST_CASE("empty data array yields error") { + const auto out = + YuGiOhCardPreviewSource::parseFirstPrint(R"({"data":[]})", "Any Set"); + CHECK(out.isErr()); + } + + TEST_CASE("card row without card_sets yields error") { + const auto out = YuGiOhCardPreviewSource::parseFirstPrint( + R"({"data":[{"name":"Solo"}]})", "Any Display Set"); + CHECK(out.isErr()); + } } TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants") { @@ -326,6 +439,34 @@ TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants") { CHECK(out.value()[0].setNo == "SDY-043"); CHECK(out.value()[0].rarity == "Super Rare"); } + + TEST_CASE("malformed JSON surfaces as parse error") { + const auto out = + YuGiOhCardPreviewSource::parsePrintVariants("{bad json", "Mega Pack", "X"); + REQUIRE(out.isErr()); + CHECK(out.error().find("YGOPRODeck JSON parse error") != std::string::npos); + } +} + +TEST_SUITE("YuGiOhCardPreviewSource::detectPrintVariants HTTP fallback") { + TEST_CASE("retries without cardset when the filtered request fails") { + FailFilteredThenOkHttpClient http; + http.unfilteredBody = R"({ + "data":[{ + "name":"Test Goblin", + "card_sets":[ + {"set_name":"Mega Pack","set_code":"MP21-EN001","set_rarity":"Common"} + ] + }] + })"; + + YuGiOhCardPreviewSource src{http}; + const auto out = src.detectPrintVariants("Test Goblin", "Mega Pack"); + REQUIRE(out.isOk()); + REQUIRE(out.value().size() == 1); + CHECK(out.value()[0].setNo == "MP21-EN001"); + REQUIRE(http.calls == 2); + } } // Helpers aligned with external fixture `yugioh_same_card_set_variant_tests`