diff --git a/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp b/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp index d7c8de9..b2efc9b 100644 --- a/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp +++ b/core/src/games/yugioh/YuGiOhCardPreviewSource.cpp @@ -31,6 +31,17 @@ std::string toLower(std::string s) { return s; } +std::string canonicalizeSetNameForAutoDetect(std::string_view setName) { + std::string canonical = trim(std::string(setName)); + constexpr std::string_view k25thSuffix = " (25th Anniversary Edition)"; + if (canonical.size() > k25thSuffix.size() + && canonical.ends_with(k25thSuffix)) { + canonical.erase(canonical.size() - k25thSuffix.size()); + canonical = trim(std::move(canonical)); + } + return canonical; +} + // Pull the standard art URL out of a YGOPRODeck card object. We deliberately // always return card_images[0]: when no `cardset=` filter is applied, that // slot is the original/standard artwork (alt-art passcodes follow), which is @@ -366,7 +377,7 @@ Result> YuGiOhCardPreviewSource::parsePrintVarian if (!j.contains("data") || !j.at("data").is_array() || j.at("data").empty()) { return R::err("YGOPRODeck returned no matching cards."); } - const std::string wantedSet = trim(std::string(preferredSetName)); + const std::string wantedSet = canonicalizeSetNameForAutoDetect(preferredSetName); const std::string wantedNameLower = toLower(trim(std::string(wantedCardName))); std::vector collected; @@ -532,15 +543,16 @@ Result> YuGiOhCardPreviewSource::detectPrintVaria std::string_view name, std::string_view setId) { using R = Result>; - const std::string url = buildSearchUrl(name, setId); + const std::string canonicalSetName = canonicalizeSetNameForAutoDetect(setId); + const std::string url = buildSearchUrl(name, canonicalSetName); auto resp = http_.get(url); if (resp) { - return parsePrintVariants(resp.value(), setId, name); + return parsePrintVariants(resp.value(), canonicalSetName, name); } const std::string fallbackUrl = buildSearchUrl(name, ""); auto fallback = http_.get(fallbackUrl); if (!fallback) return R::err(fallback.error()); - return parsePrintVariants(fallback.value(), setId, name); + return parsePrintVariants(fallback.value(), canonicalSetName, name); } } // namespace ccm diff --git a/core/src/games/yugioh/YuGiOhSetSource.cpp b/core/src/games/yugioh/YuGiOhSetSource.cpp index 6d3ebff..0c5aa02 100644 --- a/core/src/games/yugioh/YuGiOhSetSource.cpp +++ b/core/src/games/yugioh/YuGiOhSetSource.cpp @@ -16,6 +16,7 @@ struct YuGiOhSetAlias { }; constexpr std::array kMissing25thAnniversaryReprints{{ + // Keep this list in sync with docs/assets-and-info-apis.md (Info API section). {"LOB-25TH", "Legend of Blue Eyes White Dragon (25th Anniversary Edition)", "2023/04/20"}, {"MRD-25TH", "Metal Raiders (25th Anniversary Edition)", "2023/04/20"}, {"SRL-25TH", "Spell Ruler (25th Anniversary Edition)", "2023/04/20"}, diff --git a/docs/assets-and-info-apis.md b/docs/assets-and-info-apis.md index 772b9f9..7e1c78d 100644 --- a/docs/assets-and-info-apis.md +++ b/docs/assets-and-info-apis.md @@ -39,6 +39,8 @@ Upstream documentation: `https://db.ygoprodeck.com/api/v7/cardsets.php` Used by `YuGiOhSetSource`. The response is a top-level JSON array. Each object maps `set_code` → internal `Set.id`, `set_name` → `Set.name`, and `tcg_date` → `Set.releaseDate` with `-` rewritten to `/` for consistency with other games’ date strings. Results are sorted ascending by `releaseDate`. +CCM3 also applies a deterministic local patch step in `YuGiOhSetSource::appendMissingSetAliases(...)` after parsing: if upstream omits known 25th Anniversary TCG reprints, the app injects missing aliases for `LOB-25TH`, `MRD-25TH`, `SRL-25TH`, `PSV-25TH`, `DCR-25TH`, and `IOC-25TH` (with fixed release dates) so users can still select those products in the set picker. + ### Asset API: Yugipedia `api.php` (primary) `https://yugipedia.com/api.php?action=query&prop=imageinfo&iiprop=url&titles=...` diff --git a/tests/card_preview_service_tests.cpp b/tests/card_preview_service_tests.cpp index a1061f9..d0bca9c 100644 --- a/tests/card_preview_service_tests.cpp +++ b/tests/card_preview_service_tests.cpp @@ -703,6 +703,17 @@ TEST_SUITE("CardPreviewService caching") { CHECK(http.calls == 1); } + TEST_CASE("fetchImageBytesByUrl propagates HTTP errors when uncached") { + FixedHttpClient http; + http.ok = false; + http.err = "url fetch failed"; + CardPreviewService svc{http}; + + const auto out = svc.fetchImageBytesByUrl("https://cdn.example/back.png"); + REQUIRE(out.isErr()); + CHECK(out.error() == "url fetch failed"); + } + TEST_CASE("fetchImageBytesByUrl serves from persistent cache hit without HTTP") { FixedHttpClient http; http.body = "warm-card-back"; diff --git a/tests/collection_service_tests.cpp b/tests/collection_service_tests.cpp index 837c3b1..0d587c0 100644 --- a/tests/collection_service_tests.cpp +++ b/tests/collection_service_tests.cpp @@ -59,6 +59,16 @@ MagicCard makeCard(const std::string& name, std::vector imgs = {}) } // namespace TEST_SUITE("CollectionService") { + TEST_CASE("nextId uses highest existing id plus one") { + InMemoryRepo repo; + StubImageStore store; + CollectionService svc{repo, store}; + + repo.storage.emplace(2, makeCard("A")); + repo.storage.emplace(9, makeCard("B")); + CHECK(CollectionService::nextId(repo.storage) == 10); + } + TEST_CASE("nextId on empty map is 0, then strictly increments") { InMemoryRepo repo; StubImageStore store; @@ -163,6 +173,21 @@ TEST_SUITE("CollectionService") { CHECK(updateRes.error() == "save failed"); } + TEST_CASE("add overwrites input card id with generated id") { + InMemoryRepo repo; + StubImageStore store; + CollectionService svc{repo, store}; + + MagicCard card = makeCard("Has User Id"); + card.id = 777; + const auto out = svc.add(Game::Magic, card); + REQUIRE(out.isOk()); + CHECK(out.value() == 0); + REQUIRE(repo.storage.count(0) == 1); + CHECK(repo.storage.at(0).name == "Has User Id"); + CHECK(repo.storage.count(777) == 0); + } + TEST_CASE("findById returns nullopt for missing id") { InMemoryRepo repo; StubImageStore store; @@ -193,4 +218,22 @@ TEST_SUITE("CollectionService") { REQUIRE(listed.isOk()); CHECK(listed.value().empty()); } + + TEST_CASE("remove propagates save failure after image cleanup") { + InMemoryRepo repo; + StubImageStore store; + CollectionService svc{repo, store}; + + const auto id = svc.add( + Game::Magic, makeCard("With Images", {"a.png", "b.png"})); + REQUIRE(id.isOk()); + + repo.failSave = true; + const auto removed = svc.remove(Game::Magic, id.value()); + REQUIRE(removed.isErr()); + CHECK(removed.error() == "save failed"); + REQUIRE(store.removed.size() == 2); + CHECK(store.removed[0].second == "a.png"); + CHECK(store.removed[1].second == "b.png"); + } } diff --git a/tests/cpr_http_client_tests.cpp b/tests/cpr_http_client_tests.cpp index b6c308b..a3cec84 100644 --- a/tests/cpr_http_client_tests.cpp +++ b/tests/cpr_http_client_tests.cpp @@ -79,6 +79,59 @@ TEST_SUITE("CprHttpClient injected raw executor") { REQUIRE(out.isErr()); CHECK(out.error().find("timeout") != std::string::npos); } + + TEST_CASE("passes URL through raw executor unchanged") { + std::string seenUrl; + CprHttpClient client{ + [&seenUrl](std::string_view url) -> CprHttpClient::RawResponse { + seenUrl = std::string(url); + return CprHttpClient::RawResponse{ + .transportError = false, + .transportMessage = "", + .statusCode = 200, + .body = "ok", + }; + } + }; + + const auto out = client.get("https://example.com/raw?q=a%20b"); + REQUIRE(out.isOk()); + CHECK(seenUrl == "https://example.com/raw?q=a%20b"); + } + + TEST_CASE("maps non-2xx status to error") { + CprHttpClient client{ + [](std::string_view) -> CprHttpClient::RawResponse { + return CprHttpClient::RawResponse{ + .transportError = false, + .transportMessage = "", + .statusCode = 503, + .body = "service unavailable", + }; + } + }; + + const auto out = client.get("https://example.com/fail"); + REQUIRE(out.isErr()); + CHECK(out.error().find("HTTP 503") != std::string::npos); + } + + TEST_CASE("transport error takes precedence over status code") { + CprHttpClient client{ + [](std::string_view) -> CprHttpClient::RawResponse { + return CprHttpClient::RawResponse{ + .transportError = true, + .transportMessage = "socket closed", + .statusCode = 200, + .body = "ignored", + }; + } + }; + + const auto out = client.get("https://example.com/transport"); + REQUIRE(out.isErr()); + CHECK(out.error().find("socket closed") != std::string::npos); + } } TEST_SUITE("CprHttpClient real session") { diff --git a/tests/domain_json_tests.cpp b/tests/domain_json_tests.cpp index 115c85a..fe608d8 100644 --- a/tests/domain_json_tests.cpp +++ b/tests/domain_json_tests.cpp @@ -290,6 +290,29 @@ TEST_SUITE("Domain JSON required fields") { CHECK_THROWS(j.get()); } + TEST_CASE("YuGiOhCard missing required key throws") { + const nlohmann::json j = { + {"id", 7}, + {"amount", 1}, + {"name", "Blue-Eyes White Dragon"}, + {"set", nlohmann::json{ + {"id", "sdk"}, + {"name", "Starter Deck Kaiba"}, + {"releaseDate", "2002/03/29"}, + }}, + {"setNo", "SDK-001"}, + {"note", ""}, + {"images", nlohmann::json::array()}, + {"language", "English"}, + {"condition", "NearMint"}, + {"firstEdition", true}, + // rarity missing on purpose + {"signed", false}, + {"altered", false}, + }; + CHECK_THROWS(j.get()); + } + TEST_CASE("Configuration missing required key throws") { const nlohmann::json j = { {"defaultGame", "Magic"}, @@ -297,4 +320,13 @@ TEST_SUITE("Domain JSON required fields") { }; CHECK_THROWS(j.get()); } + + TEST_CASE("Configuration invalid theme value throws when present") { + const nlohmann::json j = { + {"dataStorage", "/portable/data"}, + {"defaultGame", "Magic"}, + {"theme", "Neon"}, + }; + CHECK_THROWS(j.get()); + } } diff --git a/tests/image_service_tests.cpp b/tests/image_service_tests.cpp index 921814d..dca7c8a 100644 --- a/tests/image_service_tests.cpp +++ b/tests/image_service_tests.cpp @@ -68,6 +68,11 @@ TEST_SUITE("ImageService::nextImageIndex") { std::vector imgs = {"set+name+99.png"}; CHECK(ImageService::nextImageIndex(imgs) == 100); } + + TEST_CASE("three-digit filename index follows two-digit compatibility parser") { + std::vector imgs = {"set+name+255.png"}; + CHECK(ImageService::nextImageIndex(imgs) == 56); + } } TEST_SUITE("ImageService::buildTargetName") { @@ -98,6 +103,40 @@ TEST_SUITE("ImageService::addImage") { CHECK(store.copies[0].game == Game::Magic); CHECK(store.copies[0].target == "Beta+BlackLotus+0"); } + + TEST_CASE("propagates copyIn failures") { + RecordingImageStore store; + store.failCopyAt = 1; + ImageService svc{store}; + + std::vector existing; + const auto out = svc.addImage(Game::Magic, "/tmp/source.png", + /*newEntry=*/true, /*cardId=*/0, + "Beta", "Black Lotus", existing); + REQUIRE(out.isErr()); + CHECK(out.error().find("copy failed at 1") != std::string::npos); + } +} + +TEST_SUITE("ImageService::removeImage and resolveImagePath") { + TEST_CASE("removeImage delegates to store remove") { + RecordingImageStore store; + ImageService svc{store}; + + const auto out = svc.removeImage(Game::Magic, "x.png"); + REQUIRE(out.isOk()); + REQUIRE(store.removes.size() == 1); + CHECK(store.removes[0].first == Game::Magic); + CHECK(store.removes[0].second == "x.png"); + } + + TEST_CASE("resolveImagePath delegates to store resolvePath") { + RecordingImageStore store; + ImageService svc{store}; + + const auto p = svc.resolveImagePath(Game::Pokemon, "pikachu.jpg"); + CHECK(p == std::filesystem::path("/fake/pikachu.jpg")); + } } TEST_SUITE("ImageService::normalizeNamesForPersistedCard") { @@ -142,6 +181,20 @@ TEST_SUITE("ImageService::normalizeNamesForPersistedCard") { CHECK(store.removes.empty()); } + TEST_CASE("skips rename when computed output name equals input") { + RecordingImageStore store; + ImageService svc{store}; + + const std::vector images{"42+Beta+BlackLotus+0.png"}; + auto normalized = svc.normalizeNamesForPersistedCard( + Game::Magic, 42, "Beta", "Black Lotus", images); + + REQUIRE(normalized.isOk()); + CHECK(normalized.value() == images); + CHECK(store.copies.empty()); + CHECK(store.removes.empty()); + } + TEST_CASE("copy failure rolls back already-created names and returns error") { RecordingImageStore store; store.failCopyAt = 2; diff --git a/tests/json_collection_repository_tests.cpp b/tests/json_collection_repository_tests.cpp index 580401c..538cd28 100644 --- a/tests/json_collection_repository_tests.cpp +++ b/tests/json_collection_repository_tests.cpp @@ -8,6 +8,8 @@ #include +#include + using namespace ccm; using ccm::testing::InMemoryFileSystem; @@ -27,6 +29,37 @@ ConfigService makeConfig(InMemoryFileSystem& fs, const std::string& dataDir) { std::string magicDir(Game g) { return g == Game::Magic ? "magic" : "pokemon"; } +class FailingCollectionFs final : public IFileSystem { +public: + bool existsValue{true}; + bool ensureOk{true}; + bool writeOk{true}; + bool readOk{true}; + std::string readPayload{"{}"}; + + [[nodiscard]] bool exists(const std::filesystem::path&) const override { return existsValue; } + [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; } + Result ensureDirectory(const std::filesystem::path&) override { + if (!ensureOk) return Result::err("ensure failed"); + return Result::ok(); + } + Result readText(const std::filesystem::path&) override { + if (!readOk) return Result::err("read failed"); + return Result::ok(readPayload); + } + Result writeText(const std::filesystem::path&, std::string_view) override { + if (!writeOk) return Result::err("write failed"); + return Result::ok(); + } + Result copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override { + return Result::ok(); + } + Result remove(const std::filesystem::path&) override { return Result::ok(); } + Result> listDirectory(const std::filesystem::path&) override { + return Result>::ok({}); + } +}; + } // namespace TEST_SUITE("JsonCollectionRepository") { @@ -85,4 +118,49 @@ TEST_SUITE("JsonCollectionRepository") { REQUIRE(j.contains("17")); CHECK(j.at("17").at("id") == 17); } + + TEST_CASE("load returns parse error for non-object root") { + InMemoryFileSystem fs; + auto cfg = makeConfig(fs, "/data"); + JsonCollectionRepository repo{fs, cfg, magicDir}; + fs.writeText("/data/magic/collection.json", R"(["not","an","object"])"); + + const auto loaded = repo.load(Game::Magic); + REQUIRE(loaded.isErr()); + CHECK(loaded.error().find("JSON parse error:") != std::string::npos); + } + + TEST_CASE("load returns parse error for non-numeric object keys") { + InMemoryFileSystem fs; + auto cfg = makeConfig(fs, "/data"); + JsonCollectionRepository repo{fs, cfg, magicDir}; + fs.writeText("/data/magic/collection.json", R"({"abc":{"id":1}})"); + + const auto loaded = repo.load(Game::Magic); + REQUIRE(loaded.isErr()); + CHECK(loaded.error().find("JSON parse error:") != std::string::npos); + } + + TEST_CASE("save and initialize-on-load propagate ensureDirectory/write errors") { + InMemoryFileSystem configFs; + auto cfg = makeConfig(configFs, "/data"); + FailingCollectionFs fs; + JsonCollectionRepository repo{fs, cfg, magicDir}; + + fs.ensureOk = false; + const auto saveEnsureFail = repo.save(Game::Magic, {}); + REQUIRE(saveEnsureFail.isErr()); + CHECK(saveEnsureFail.error() == "ensure failed"); + + fs.ensureOk = true; + fs.writeOk = false; + const auto saveWriteFail = repo.save(Game::Magic, {}); + REQUIRE(saveWriteFail.isErr()); + CHECK(saveWriteFail.error() == "write failed"); + + fs.existsValue = false; + const auto loadCreateFail = repo.load(Game::Magic); + REQUIRE(loadCreateFail.isErr()); + CHECK(loadCreateFail.error() == "write failed"); + } } diff --git a/tests/json_set_repository_tests.cpp b/tests/json_set_repository_tests.cpp index 0f47e4c..e90c40b 100644 --- a/tests/json_set_repository_tests.cpp +++ b/tests/json_set_repository_tests.cpp @@ -31,6 +31,7 @@ public: std::string readPayload{"[]"}; std::filesystem::path lastWritePath; std::string lastWriteBody; + std::filesystem::path lastReadPath; [[nodiscard]] bool exists(const std::filesystem::path&) const override { return true; } [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; } @@ -40,6 +41,7 @@ public: return Result::ok(); } Result readText(const std::filesystem::path&) override { + lastReadPath = std::filesystem::path("/tracked/read/path"); if (!readOk) return Result::err("read failed"); return Result::ok(readPayload); } @@ -109,6 +111,18 @@ TEST_SUITE("JsonSetRepository") { CHECK(loaded.error().find("sets.json parse error:") != std::string::npos); } + TEST_CASE("load reports parse error for wrong JSON shape") { + InMemoryFileSystem configFs; + auto cfg = makeConfig(configFs, "/data"); + FailingSetFs fs; + fs.readPayload = R"({"not":"an array"})"; + JsonSetRepository repo{fs, cfg, dirNameFn}; + + const auto loaded = repo.load(Game::Magic); + REQUIRE(loaded.isErr()); + CHECK(loaded.error().find("sets.json parse error:") != std::string::npos); + } + TEST_CASE("save propagates ensureDirectory and writeText failures") { InMemoryFileSystem configFs; auto cfg = makeConfig(configFs, "/data"); @@ -128,4 +142,14 @@ TEST_SUITE("JsonSetRepository") { REQUIRE(writeFail.isErr()); CHECK(writeFail.error() == "write failed"); } + + TEST_CASE("paths are composed from dataStorage and game dir") { + InMemoryFileSystem fs; + auto cfg = makeConfig(fs, "/data"); + JsonSetRepository repo{fs, cfg, dirNameFn}; + const std::vector sets = {{"base1", "Base Set", "1999/01/09"}}; + + REQUIRE(repo.save(Game::Pokemon, sets).isOk()); + CHECK(fs.files().count("/data/pokemon/sets.json") == 1); + } } diff --git a/tests/local_image_store_tests.cpp b/tests/local_image_store_tests.cpp index 22ad8f2..98b93ae 100644 --- a/tests/local_image_store_tests.cpp +++ b/tests/local_image_store_tests.cpp @@ -21,6 +21,37 @@ std::string dirNameForGame(Game g) { return "magic"; } +class FailingImageFs final : public IFileSystem { +public: + bool ensureOk{true}; + bool copyOk{true}; + bool removeOk{true}; + + [[nodiscard]] bool exists(const std::filesystem::path&) const override { return true; } + [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; } + Result ensureDirectory(const std::filesystem::path&) override { + if (!ensureOk) return Result::err("ensure failed"); + return Result::ok(); + } + Result readText(const std::filesystem::path&) override { + return Result::ok({}); + } + Result writeText(const std::filesystem::path&, std::string_view) override { + return Result::ok(); + } + Result copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override { + if (!copyOk) return Result::err("copy failed"); + return Result::ok(); + } + Result remove(const std::filesystem::path&) override { + if (!removeOk) return Result::err("remove failed"); + return Result::ok(); + } + Result> listDirectory(const std::filesystem::path&) override { + return Result>::ok({}); + } +}; + } // namespace TEST_SUITE("LocalImageStore") { @@ -87,4 +118,30 @@ TEST_SUITE("LocalImageStore") { const std::filesystem::path got = store.resolvePath(Game::Pokemon, "pic.jpg"); CHECK(got.generic_string() == "/coll/pokemon/images/pic.jpg"); } + + TEST_CASE("copyIn propagates ensureDirectory failure") { + InMemoryFileSystem configFs; + ConfigService cfg{configFs, "/app/config.json", "/coll"}; + REQUIRE(cfg.initialize().isOk()); + FailingImageFs fs; + fs.ensureOk = false; + LocalImageStore store(fs, cfg, dirNameForGame); + + const auto out = store.copyIn(Game::Magic, "/incoming/a.png", "id001"); + REQUIRE(out.isErr()); + CHECK(out.error() == "ensure failed"); + } + + TEST_CASE("remove propagates filesystem remove failure when file exists") { + InMemoryFileSystem configFs; + ConfigService cfg{configFs, "/app/config.json", "/coll"}; + REQUIRE(cfg.initialize().isOk()); + FailingImageFs fs; + fs.removeOk = false; + LocalImageStore store(fs, cfg, dirNameForGame); + + const auto out = store.remove(Game::Magic, "a.png"); + REQUIRE(out.isErr()); + CHECK(out.error() == "remove failed"); + } } diff --git a/tests/local_preview_byte_cache_tests.cpp b/tests/local_preview_byte_cache_tests.cpp index 874199c..301e679 100644 --- a/tests/local_preview_byte_cache_tests.cpp +++ b/tests/local_preview_byte_cache_tests.cpp @@ -234,6 +234,55 @@ TEST_SUITE("LocalPreviewByteCache") { CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss); } + TEST_CASE("entry with payload but missing sidecar is treated as miss") { + TempDir td; + StdFileSystem fs; + LocalPreviewByteCache cache(fs, td.path); + cache.store("real-key", "REAL"); + + for (const auto& entry : fs::directory_iterator(td.path)) { + if (entry.path().extension() == ".idx") { + std::error_code ec; + fs::remove(entry.path(), ec); + } + } + + CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss); + } + + TEST_CASE("entry with negative marker but missing sidecar is treated as miss") { + TempDir td; + StdFileSystem fs; + LocalPreviewByteCache cache(fs, td.path); + cache.storeNegative("real-key"); + + for (const auto& entry : fs::directory_iterator(td.path)) { + if (entry.path().extension() == ".idx") { + std::error_code ec; + fs::remove(entry.path(), ec); + } + } + + CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss); + } + + TEST_CASE("entry with unreadable payload file is treated as miss") { + TempDir td; + StdFileSystem fs; + LocalPreviewByteCache cache(fs, td.path); + cache.store("real-key", "REAL"); + + for (const auto& entry : fs::directory_iterator(td.path)) { + if (entry.path().extension() == ".bin") { + std::error_code ec; + fs::remove(entry.path(), ec); + break; + } + } + + CHECK(cache.load("real-key").kind == IPreviewByteCache::HitKind::Miss); + } + TEST_CASE("evicts oldest entry when the size cap would be exceeded") { TempDir td; StdFileSystem fs; diff --git a/tests/set_service_tests.cpp b/tests/set_service_tests.cpp index 0576cfb..26656af 100644 --- a/tests/set_service_tests.cpp +++ b/tests/set_service_tests.cpp @@ -39,11 +39,13 @@ class InMemSetRepo final : public ISetRepository { public: std::vector stored; bool hasStored = false; + bool failSave = false; Result> load(Game) override { if (!hasStored) return Result>::err("no cache"); return Result>::ok(stored); } Result save(Game, const std::vector& s) override { + if (failSave) return Result::err("save failed"); stored = s; hasStored = true; return Result::ok(); @@ -145,4 +147,36 @@ TEST_SUITE("SetService") { CHECK(pokemon.source.calls == 1); CHECK(yugioh.source.calls == 1); } + + TEST_CASE("updateSets propagates repository save failures") { + InMemSetRepo repo; + repo.failSave = true; + SetService svc{repo}; + FakeGameModule magic{Game::Magic}; + magic.source.result = Result>::ok({{"lea", "Alpha", "1993/08/05"}}); + svc.registerModule(&magic); + + const auto out = svc.updateSets(Game::Magic); + REQUIRE(out.isErr()); + CHECK(out.error() == "save failed"); + } + + TEST_CASE("registering a second module for same game id overwrites previous one") { + InMemSetRepo repo; + SetService svc{repo}; + FakeGameModule firstMagic{Game::Magic}; + firstMagic.source.result = Result>::ok({{"a", "First", "2000/01/01"}}); + FakeGameModule secondMagic{Game::Magic}; + secondMagic.source.result = Result>::ok({{"b", "Second", "2001/01/01"}}); + + svc.registerModule(&firstMagic); + svc.registerModule(&secondMagic); + + const auto out = svc.updateSets(Game::Magic); + REQUIRE(out.isOk()); + REQUIRE(out.value().size() == 1); + CHECK(out.value().front().id == "b"); + CHECK(firstMagic.source.calls == 0); + CHECK(secondMagic.source.calls == 1); + } } diff --git a/tests/std_file_system_tests.cpp b/tests/std_file_system_tests.cpp index 1afcf9a..a75d122 100644 --- a/tests/std_file_system_tests.cpp +++ b/tests/std_file_system_tests.cpp @@ -112,6 +112,22 @@ TEST_SUITE("StdFileSystem") { CHECK(r.value() == "hi"); } + TEST_CASE("writeText/readText work for top-level relative files") { + TempDir td; + StdFileSystem fs; + const auto oldCwd = fs::current_path(); + fs::current_path(td.path); + + const fs::path topLevel = "top-level.txt"; + REQUIRE(fs.writeText(topLevel, "hello").isOk()); + const auto r = fs.readText(topLevel); + REQUIRE(r.isOk()); + CHECK(r.value() == "hello"); + + std::error_code ec; + fs::current_path(oldCwd, ec); + } + TEST_CASE("copyFile copies bytes and respects overwrite flag") { TempDir td; StdFileSystem fs; diff --git a/tests/yugioh_card_preview_source_tests.cpp b/tests/yugioh_card_preview_source_tests.cpp index f043265..d12728c 100644 --- a/tests/yugioh_card_preview_source_tests.cpp +++ b/tests/yugioh_card_preview_source_tests.cpp @@ -446,6 +446,22 @@ TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants") { REQUIRE(out.isErr()); CHECK(out.error().find("YGOPRODeck JSON parse error") != std::string::npos); } + + TEST_CASE("maps 25th Anniversary display-set alias to original set name") { + const std::string json = R"({ + "data":[ + {"name":"Dark Magician", + "card_sets":[ + {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"} + ]} + ] + })"; + const auto out = YuGiOhCardPreviewSource::parsePrintVariants( + json, "Legend of Blue Eyes White Dragon (25th Anniversary Edition)", "Dark Magician"); + REQUIRE(out.isOk()); + REQUIRE(out.value().size() == 1); + CHECK(out.value()[0].setNo == "LOB-005"); + } } TEST_SUITE("YuGiOhCardPreviewSource::detectPrintVariants HTTP fallback") { @@ -467,6 +483,26 @@ TEST_SUITE("YuGiOhCardPreviewSource::detectPrintVariants HTTP fallback") { CHECK(out.value()[0].setNo == "MP21-EN001"); REQUIRE(http.calls == 2); } + + TEST_CASE("uses original set name in cardset query for 25th alias") { + FixedHttpClient http; + http.body = R"({ + "data":[{ + "name":"Dark Magician", + "card_sets":[ + {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"} + ] + }] + })"; + + YuGiOhCardPreviewSource src{http}; + const auto out = src.detectPrintVariants( + "Dark Magician", "Legend of Blue Eyes White Dragon (25th Anniversary Edition)"); + REQUIRE(out.isOk()); + CHECK(http.lastUrl.find("cardset=Legend%20of%20Blue%20Eyes%20White%20Dragon") + != std::string::npos); + CHECK(http.lastUrl.find("25th") == std::string::npos); + } } // Helpers aligned with external fixture `yugioh_same_card_set_variant_tests` @@ -760,6 +796,34 @@ TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") { CHECK(out.error().kind == PreviewLookupError::Kind::Transient); } + TEST_CASE("Yugipedia clean-miss + YGOPRODeck transient is overall Transient") { + RoutingHttpClient http; + http.yugipediaBody = R"({"query":{"pages":{ + "-1":{"title":"File:Whatever-LOB-EN-UR-UE.png","missing":""} + }}})"; + http.ygoprodeckOk = false; + + YuGiOhCardPreviewSource src{http}; + const auto out = src.fetchImageUrl( + "No Such Card", "Legend of Blue Eyes White Dragon", "LOB-999||Ultra Rare||UE"); + REQUIRE(out.isErr()); + CHECK(out.error().kind == PreviewLookupError::Kind::Transient); + } + + TEST_CASE("tuple-style setNo parsing trims fields and supports empty set code") { + FixedHttpClient http; + http.ok = true; + http.body = R"({"data":[{"name":"Dark Magician", + "card_images":[{"image_url":"https://images.ygoprodeck.com/std-dm.jpg"}]}]})"; + + YuGiOhCardPreviewSource src{http}; + const auto out = src.fetchImageUrl( + "Dark Magician", "Legend of Blue Eyes White Dragon", " || Ultra Rare || 1E "); + REQUIRE(out.isOk()); + CHECK(out.value() == "https://images.ygoprodeck.com/std-dm.jpg"); + CHECK(http.lastUrl.find("ygoprodeck.com") != std::string::npos); + } + TEST_CASE("skips Yugipedia entirely when the set code is missing") { // Without a set code we can't construct any candidate filename - go // straight to the YGOPRODeck fallback to avoid wasting an HTTP call. @@ -777,3 +841,36 @@ TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") { CHECK(http.lastUrl.find("yugipedia.com") == std::string::npos); } } + +TEST_SUITE("YuGiOhCardPreviewSource::detectFirstPrint") { + TEST_CASE("returns first variant from filtered request") { + FixedHttpClient http; + http.body = R"({ + "data":[ + {"name":"Dark Magician", + "card_sets":[ + {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB-005","set_rarity":"Ultra Rare"} + ]} + ] + })"; + YuGiOhCardPreviewSource src{http}; + + const auto out = src.detectFirstPrint( + "Dark Magician", "Legend of Blue Eyes White Dragon"); + REQUIRE(out.isOk()); + CHECK(out.value().setNo == "LOB-005"); + CHECK(out.value().rarity == "Ultra Rare"); + CHECK(http.lastUrl.find("cardset=Legend%20of%20Blue%20Eyes%20White%20Dragon") + != std::string::npos); + } + + TEST_CASE("propagates unfiltered fallback errors when both requests fail") { + FixedHttpClient http; + http.ok = false; + YuGiOhCardPreviewSource src{http}; + + const auto out = src.detectFirstPrint("Any", "Any Set"); + REQUIRE(out.isErr()); + CHECK(out.error() == "offline"); + } +} diff --git a/tests/yugioh_set_source_tests.cpp b/tests/yugioh_set_source_tests.cpp index 2543f59..6272c89 100644 --- a/tests/yugioh_set_source_tests.cpp +++ b/tests/yugioh_set_source_tests.cpp @@ -98,6 +98,44 @@ TEST_SUITE("YuGiOhSetSource::parseResponse") { REQUIRE(out.isErr()); CHECK(out.error().find("YGOPRODeck set parse error:") != std::string::npos); } + + TEST_CASE("missing fields fall back to empty strings and keep parsing") { + const std::string json = R"([ + {"set_name":"Set A"}, + {"set_code":"BBB","tcg_date":"2021-02-03"} + ])"; + const auto out = YuGiOhSetSource::parseResponse(json); + REQUIRE(out.isOk()); + bool foundMissingCode = false; + bool foundMissingName = false; + for (const auto& set : out.value()) { + if (set.name == "Set A" && set.id.empty() && set.releaseDate.empty()) { + foundMissingCode = true; + } + if (set.id == "BBB" && set.name.empty() && set.releaseDate == "2021/02/03") { + foundMissingName = true; + } + } + CHECK(foundMissingCode); + CHECK(foundMissingName); + } + + TEST_CASE("preserves slash-formatted dates and normalizes hyphen dates") { + const std::string json = R"([ + {"set_name":"Slash Date","set_code":"S","tcg_date":"2024/01/01"}, + {"set_name":"Hyphen Date","set_code":"H","tcg_date":"2024-01-02"} + ])"; + const auto out = YuGiOhSetSource::parseResponse(json); + REQUIRE(out.isOk()); + bool sawSlash = false; + bool sawHyphenNormalized = false; + for (const auto& set : out.value()) { + if (set.id == "S" && set.releaseDate == "2024/01/01") sawSlash = true; + if (set.id == "H" && set.releaseDate == "2024/01/02") sawHyphenNormalized = true; + } + CHECK(sawSlash); + CHECK(sawHyphenNormalized); + } } TEST_SUITE("YuGiOhSetSource::fetchAll") { diff --git a/ui_wx/include/ccm/ui/BaseCardEditDialog.hpp b/ui_wx/include/ccm/ui/BaseCardEditDialog.hpp index eef99a1..a88822b 100644 --- a/ui_wx/include/ccm/ui/BaseCardEditDialog.hpp +++ b/ui_wx/include/ccm/ui/BaseCardEditDialog.hpp @@ -352,11 +352,14 @@ private: failed.reserve(static_cast(paths.size())); for (const auto& path : paths) { + const std::string setNameForImage = (game_ == Game::YuGiOh && !card_.set.id.empty()) + ? card_.set.id + : card_.set.name; auto added = imageService_.addImage(game_, std::filesystem::path(path.ToStdString()), mode_ == EditMode::Create, card_.id, - card_.set.name, + setNameForImage, card_.name, card_.images); if (!added) { diff --git a/ui_wx/src/YuGiOhGameView.cpp b/ui_wx/src/YuGiOhGameView.cpp index 601a8f1..7b9e9ac 100644 --- a/ui_wx/src/YuGiOhGameView.cpp +++ b/ui_wx/src/YuGiOhGameView.cpp @@ -113,8 +113,11 @@ void YuGiOhGameView::onAddCard(wxWindow* parentWindow) { YuGiOhCard persisted = dlg.card(); persisted.id = added.value(); + const std::string setNameForImage = persisted.set.id.empty() + ? persisted.set.name + : persisted.set.id; auto normalized = images_.normalizeNamesForPersistedCard( - Game::YuGiOh, persisted.id, persisted.set.name, persisted.name, persisted.images); + Game::YuGiOh, persisted.id, setNameForImage, persisted.name, persisted.images); if (normalized) { if (normalized.value() != persisted.images) { persisted.images = std::move(normalized).value();