patch: Patch/ygo set 25th (#13)

tests/CMakeLists.txt

@@ -21,6 +21,7 @@ add_executable(ccm_core_tests
     std_file_system_tests.cpp
     pokemon_set_source_tests.cpp
     pokemon_card_preview_source_tests.cpp
+    icard_preview_source_tests.cpp
     yugioh_set_source_tests.cpp
     yugioh_card_preview_source_tests.cpp
     game_module_tests.cpp

tests/card_preview_service_tests.cpp

@@ -703,6 +703,29 @@ TEST_SUITE("CardPreviewService caching") {
         CHECK(http.calls == 1);
     }
 
+    TEST_CASE("fetchImageBytesByUrl serves from persistent cache hit without HTTP") {
+        FixedHttpClient http;
+        http.body = "warm-card-back";
+        InMemoryByteCache disk;
+
+        // Seed persistent cache via first service instance.
+        {
+            CardPreviewService seed{http, &disk};
+            const auto seeded = seed.fetchImageBytesByUrl("https://cdn.example/back.png");
+            REQUIRE(seeded.isOk());
+            CHECK(seeded.value() == "warm-card-back");
+        }
+        REQUIRE(http.calls == 1);
+
+        // Fresh service instance: force HTTP failure and ensure disk hit is used.
+        CardPreviewService warm{http, &disk};
+        http.ok = false;
+        const auto warmHit = warm.fetchImageBytesByUrl("https://cdn.example/back.png");
+        REQUIRE(warmHit.isOk());
+        CHECK(warmHit.value() == "warm-card-back");
+        CHECK(http.calls == 1);
+    }
+
     TEST_CASE("in-memory LRU evicts oldest entry after exceeding capacity") {
         FakeSource source;
         FakeGameModule module;

tests/collection_service_tests.cpp

@@ -16,9 +16,15 @@ namespace {
 class InMemoryRepo final : public ICollectionRepository<MagicCard> {
 public:
     Map storage;
+    bool failLoad{false};
+    bool failSave{false};
 
-    Result<Map> load(Game) override { return Result<Map>::ok(storage); }
+    Result<Map> load(Game) override {
+        if (failLoad) return Result<Map>::err("load failed");
+        return Result<Map>::ok(storage);
+    }
     Result<void> save(Game, const Map& m) override {
+        if (failSave) return Result<void>::err("save failed");
         storage = m;
         return Result<void>::ok();
     }
@@ -27,12 +33,14 @@ public:
 class StubImageStore final : public IImageStore {
 public:
     std::vector<std::pair<Game, std::string>> removed;
+    bool failRemove{false};
 
     Result<std::string> copyIn(Game, const std::filesystem::path&, const std::string& n) override {
         return Result<std::string>::ok(n);
     }
     Result<void> remove(Game g, const std::string& n) override {
         removed.emplace_back(g, n);
+        if (failRemove) return Result<void>::err("remove failed for " + n);
         return Result<void>::ok();
     }
     std::filesystem::path resolvePath(Game, const std::string& n) const override {
@@ -117,4 +125,72 @@ TEST_SUITE("CollectionService<MagicCard>") {
 
         CHECK(svc.remove(Game::Magic, 12345).isErr());
     }
+
+    TEST_CASE("load errors are propagated by list and findById") {
+        InMemoryRepo repo;
+        repo.failLoad = true;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto listed = svc.list(Game::Magic);
+        REQUIRE(listed.isErr());
+        CHECK(listed.error() == "load failed");
+
+        const auto found = svc.findById(Game::Magic, 1);
+        REQUIRE(found.isErr());
+        CHECK(found.error() == "load failed");
+    }
+
+    TEST_CASE("save errors are propagated by add and update") {
+        InMemoryRepo repo;
+        repo.failSave = true;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto addRes = svc.add(Game::Magic, makeCard("A"));
+        REQUIRE(addRes.isErr());
+        CHECK(addRes.error() == "save failed");
+
+        repo.failSave = false;
+        const auto id = svc.add(Game::Magic, makeCard("B"));
+        REQUIRE(id.isOk());
+
+        repo.failSave = true;
+        MagicCard updated = makeCard("Renamed");
+        updated.id = id.value();
+        const auto updateRes = svc.update(Game::Magic, updated);
+        REQUIRE(updateRes.isErr());
+        CHECK(updateRes.error() == "save failed");
+    }
+
+    TEST_CASE("findById returns nullopt for missing id") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto out = svc.findById(Game::Magic, 99);
+        REQUIRE(out.isOk());
+        CHECK_FALSE(out.value().has_value());
+    }
+
+    TEST_CASE("remove reports image cleanup issues but still removes card") {
+        InMemoryRepo repo;
+        StubImageStore store;
+        store.failRemove = true;
+        CollectionService<MagicCard> svc{repo, store};
+
+        const auto id = svc.add(
+            Game::Magic, makeCard("With Images", {"a.png", "b.png"}));
+        REQUIRE(id.isOk());
+
+        const auto removed = svc.remove(Game::Magic, id.value());
+        REQUIRE(removed.isErr());
+        CHECK(removed.error().find("Card removed but image cleanup had issues:") != std::string::npos);
+        CHECK(removed.error().find("remove failed for a.png") != std::string::npos);
+        CHECK(removed.error().find("remove failed for b.png") != std::string::npos);
+
+        const auto listed = svc.list(Game::Magic);
+        REQUIRE(listed.isOk());
+        CHECK(listed.value().empty());
+    }
 }

tests/cpr_http_client_tests.cpp

@@ -34,4 +34,60 @@ TEST_SUITE("CprHttpClient injected executor") {
         REQUIRE(out.isErr());
         CHECK(out.error() == "HTTP 503 from https://example.com");
     }
+
+    TEST_CASE("returns an explicit error when executor is empty") {
+        CprHttpClient::GetExecutor empty;
+        CprHttpClient client{empty};
+
+        const auto out = client.get("https://example.com");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "HTTP error: no executor configured");
+    }
+}
+
+TEST_SUITE("CprHttpClient injected raw executor") {
+    TEST_CASE("maps 2xx raw response to success body") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = false,
+                    .transportMessage = "",
+                    .statusCode = 200,
+                    .body = "ok-body",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/success");
+        REQUIRE(out.isOk());
+        CHECK(out.value() == "ok-body");
+    }
+
+    TEST_CASE("maps transport error via shared http mapping") {
+        CprHttpClient client{
+            [](std::string_view) -> CprHttpClient::RawResponse {
+                return CprHttpClient::RawResponse{
+                    .transportError = true,
+                    .transportMessage = "timeout",
+                    .statusCode = 0,
+                    .body = "",
+                };
+            }
+        };
+
+        const auto out = client.get("https://example.com/timeout");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("timeout") != std::string::npos);
+    }
+}
+
+TEST_SUITE("CprHttpClient real session") {
+    TEST_CASE("default constructor handles malformed URL without crashing") {
+        // Exercise the real cpr::Session-backed constructor/lambda path
+        // without depending on external network availability.
+        CprHttpClient client{};
+        const auto out = client.get("://not-a-valid-url");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("HTTP") != std::string::npos);
+    }
 }

tests/icard_preview_source_tests.cpp

@@ -0,0 +1,41 @@
+#include <doctest/doctest.h>
+
+#include "ccm/ports/ICardPreviewSource.hpp"
+
+using namespace ccm;
+
+namespace {
+
+class MinimalPreviewSource final : public ICardPreviewSource {
+public:
+    Result<std::string, PreviewLookupError>
+        fetchImageUrl(std::string_view,
+                      std::string_view,
+                      std::string_view) override {
+        return Result<std::string, PreviewLookupError>::err(
+            PreviewLookupError{PreviewLookupError::Kind::NotFound, "not found"});
+    }
+};
+
+}  // namespace
+
+TEST_SUITE("ICardPreviewSource defaults") {
+    TEST_CASE("auto-detect is disabled by default") {
+        MinimalPreviewSource src;
+        CHECK_FALSE(src.supportsAutoDetectPrint());
+    }
+
+    TEST_CASE("default detectFirstPrint returns explicit unsupported error") {
+        MinimalPreviewSource src;
+        const auto out = src.detectFirstPrint("Card", "Set");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "Auto-detect not supported by this game.");
+    }
+
+    TEST_CASE("default detectPrintVariants returns explicit unsupported error") {
+        MinimalPreviewSource src;
+        const auto out = src.detectPrintVariants("Card", "Set");
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "Print variant listing not supported by this game.");
+    }
+}

tests/image_service_tests.cpp

@@ -19,16 +19,24 @@ public:
     std::vector<Call> copies;
     std::vector<std::pair<Game, std::string>> removes;
     std::string returnedExt = ".png";
+    int failCopyAt = -1;
+    int failRemoveAt = -1;
 
     Result<std::string> copyIn(Game game,
                                const std::filesystem::path& srcPath,
                                const std::string& targetName) override {
         copies.push_back({game, srcPath, targetName});
+        if (failCopyAt >= 0 && static_cast<int>(copies.size()) == failCopyAt) {
+            return Result<std::string>::err("copy failed at " + std::to_string(failCopyAt));
+        }
         return Result<std::string>::ok(targetName + returnedExt);
     }
 
     Result<void> remove(Game game, const std::string& imageName) override {
         removes.emplace_back(game, imageName);
+        if (failRemoveAt >= 0 && static_cast<int>(removes.size()) == failRemoveAt) {
+            return Result<void>::err("remove failed at " + std::to_string(failRemoveAt));
+        }
         return Result<void>::ok();
     }
 
@@ -55,6 +63,11 @@ TEST_SUITE("ImageService::nextImageIndex") {
         std::vector<std::string> imgs2 = {"otherIMG_BACK.png"};
         CHECK(ImageService::nextImageIndex(imgs2) == 0);
     }
+
+    TEST_CASE("index increments from two-digit legacy cap") {
+        std::vector<std::string> imgs = {"set+name+99.png"};
+        CHECK(ImageService::nextImageIndex(imgs) == 100);
+    }
 }
 
 TEST_SUITE("ImageService::buildTargetName") {
@@ -128,4 +141,38 @@ TEST_SUITE("ImageService::normalizeNamesForPersistedCard") {
         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;
+        ImageService svc{store};
+
+        const std::vector<std::string> images{
+            "Beta+BlackLotus+0.png",
+            "Beta+BlackLotus+1.jpg"
+        };
+        const auto out = svc.normalizeNamesForPersistedCard(
+            Game::Magic, 42, "Beta", "Black Lotus", images);
+
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("copy failed at 2") != std::string::npos);
+        // Second copy failed, so first created file should be rolled back.
+        REQUIRE(store.removes.size() == 1);
+        CHECK(store.removes[0].second == "42+Beta+BlackLotus+0.png");
+    }
+
+    TEST_CASE("remove failure after rename returns error") {
+        RecordingImageStore store;
+        store.failRemoveAt = 1;
+        ImageService svc{store};
+
+        const std::vector<std::string> images{
+            "Beta+BlackLotus+0.png"
+        };
+        const auto out = svc.normalizeNamesForPersistedCard(
+            Game::Magic, 42, "Beta", "Black Lotus", images);
+
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("remove failed at 1") != std::string::npos);
+    }
 }

tests/json_set_repository_tests.cpp

@@ -22,6 +22,41 @@ ConfigService makeConfig(InMemoryFileSystem& fs, const std::string& dataDir) {
     cfg.initialize();
     return cfg;
 }
+
+class FailingSetFs final : public IFileSystem {
+public:
+    bool ensureOk{true};
+    bool writeOk{true};
+    bool readOk{true};
+    std::string readPayload{"[]"};
+    std::filesystem::path lastWritePath;
+    std::string lastWriteBody;
+
+    [[nodiscard]] bool exists(const std::filesystem::path&) const override { return true; }
+    [[nodiscard]] bool isDirectory(const std::filesystem::path&) const override { return true; }
+
+    Result<void> ensureDirectory(const std::filesystem::path&) override {
+        if (!ensureOk) return Result<void>::err("ensure failed");
+        return Result<void>::ok();
+    }
+    Result<std::string> readText(const std::filesystem::path&) override {
+        if (!readOk) return Result<std::string>::err("read failed");
+        return Result<std::string>::ok(readPayload);
+    }
+    Result<void> writeText(const std::filesystem::path& p, std::string_view contents) override {
+        if (!writeOk) return Result<void>::err("write failed");
+        lastWritePath = p;
+        lastWriteBody = std::string(contents);
+        return Result<void>::ok();
+    }
+    Result<void> copyFile(const std::filesystem::path&, const std::filesystem::path&, bool) override {
+        return Result<void>::ok();
+    }
+    Result<void> remove(const std::filesystem::path&) override { return Result<void>::ok(); }
+    Result<std::vector<std::filesystem::path>> listDirectory(const std::filesystem::path&) override {
+        return Result<std::vector<std::filesystem::path>>::ok({});
+    }
+};
 }  // namespace
 
 TEST_SUITE("JsonSetRepository") {
@@ -49,4 +84,48 @@ TEST_SUITE("JsonSetRepository") {
         const auto loaded = repo.load(Game::Pokemon);
         CHECK(loaded.isErr());
     }
+
+    TEST_CASE("load propagates read errors from filesystem") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        fs.readOk = false;
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const auto loaded = repo.load(Game::Magic);
+        REQUIRE(loaded.isErr());
+        CHECK(loaded.error() == "read failed");
+    }
+
+    TEST_CASE("load reports parse error for malformed sets.json") {
+        InMemoryFileSystem configFs;
+        auto cfg = makeConfig(configFs, "/data");
+        FailingSetFs fs;
+        fs.readPayload = "{bad json";
+        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");
+        FailingSetFs fs;
+        JsonSetRepository repo{fs, cfg, dirNameFn};
+
+        const std::vector<Set> sets = {{"lea", "Limited Edition Alpha", "1993/08/05"}};
+
+        fs.ensureOk = false;
+        const auto ensureFail = repo.save(Game::Magic, sets);
+        REQUIRE(ensureFail.isErr());
+        CHECK(ensureFail.error() == "ensure failed");
+
+        fs.ensureOk = true;
+        fs.writeOk = false;
+        const auto writeFail = repo.save(Game::Magic, sets);
+        REQUIRE(writeFail.isErr());
+        CHECK(writeFail.error() == "write failed");
+    }
 }

tests/yugioh_card_preview_source_tests.cpp

@@ -642,6 +642,12 @@ TEST_SUITE("YuGiOhCardPreviewSource::parsePrintVariants yugioh_same_card_set_var
 }
 
 TEST_SUITE("YuGiOhCardPreviewSource::fetchImageUrl") {
+    TEST_CASE("supports auto-detect print metadata") {
+        FixedHttpClient http;
+        YuGiOhCardPreviewSource src{http};
+        CHECK(src.supportsAutoDetectPrint());
+    }
+
     TEST_CASE("queries Yugipedia first and uses the per-printing scan when found") {
         // Two same-passcode reprints with genuinely different art (LOB vs
         // SDK Blue-Eyes). Yugipedia hosts both, so we should always pick the

tests/yugioh_set_source_tests.cpp

@@ -29,9 +29,18 @@ TEST_SUITE("YuGiOhSetSource::parseResponse") {
         ])";
         const auto out = YuGiOhSetSource::parseResponse(json);
         REQUIRE(out.isOk());
-        REQUIRE(out.value().size() == 2);
-        CHECK(out.value()[0].id == "AAA");
-        CHECK(out.value()[0].releaseDate == "2020/01/01");
+        bool foundA = false;
+        bool foundB = false;
+        for (const auto& set : out.value()) {
+            if (set.id == "AAA" && set.name == "Set A" && set.releaseDate == "2020/01/01") {
+                foundA = true;
+            }
+            if (set.id == "BBB" && set.name == "Set B" && set.releaseDate == "2021/02/03") {
+                foundB = true;
+            }
+        }
+        CHECK(foundA);
+        CHECK(foundB);
     }
 
     TEST_CASE("sorts by release date ascending") {
@@ -47,6 +56,48 @@ TEST_SUITE("YuGiOhSetSource::parseResponse") {
     TEST_CASE("missing array returns error") {
         CHECK(YuGiOhSetSource::parseResponse(R"({"data":[]})").isErr());
     }
+
+    TEST_CASE("adds 25th Anniversary aliases when upstream list misses them") {
+        const auto out = YuGiOhSetSource::parseResponse(R"([
+            {"set_name":"Legend of Blue Eyes White Dragon","set_code":"LOB","tcg_date":"2002-03-08"}
+        ])");
+        REQUIRE(out.isOk());
+
+        bool foundLob25th = false;
+        bool foundIoc25th = false;
+        for (const auto& set : out.value()) {
+            if (set.name == "Legend of Blue Eyes White Dragon (25th Anniversary Edition)"
+                && set.id == "LOB-25TH") {
+                foundLob25th = true;
+            }
+            if (set.name == "Invasion of Chaos (25th Anniversary Edition)" && set.id == "IOC-25TH") {
+                foundIoc25th = true;
+            }
+        }
+        CHECK(foundLob25th);
+        CHECK(foundIoc25th);
+    }
+
+    TEST_CASE("does not duplicate aliases that already exist by name") {
+        const auto out = YuGiOhSetSource::parseResponse(R"json([
+            {"set_name":"Legend of Blue Eyes White Dragon (25th Anniversary Edition)","set_code":"LOB-25TH","tcg_date":"2023-04-20"}
+        ])json");
+        REQUIRE(out.isOk());
+
+        int aliasCount = 0;
+        for (const auto& set : out.value()) {
+            if (set.name == "Legend of Blue Eyes White Dragon (25th Anniversary Edition)") {
+                ++aliasCount;
+            }
+        }
+        CHECK(aliasCount == 1);
+    }
+
+    TEST_CASE("malformed json returns parse error") {
+        const auto out = YuGiOhSetSource::parseResponse("{bad json");
+        REQUIRE(out.isErr());
+        CHECK(out.error().find("YGOPRODeck set parse error:") != std::string::npos);
+    }
 }
 
 TEST_SUITE("YuGiOhSetSource::fetchAll") {
@@ -59,4 +110,13 @@ TEST_SUITE("YuGiOhSetSource::fetchAll") {
         CHECK(out.value().front().id == "X");
         CHECK(http.lastUrl == "https://db.ygoprodeck.com/api/v7/cardsets.php");
     }
+
+    TEST_CASE("network error is propagated") {
+        FixedHttpClient http;
+        http.ok = false;
+        YuGiOhSetSource src{http};
+        const auto out = src.fetchAll();
+        REQUIRE(out.isErr());
+        CHECK(out.error() == "offline");
+    }
 }