From 8b5b06ca9aef173c0d9ae3c48831ca0949ee7a22 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 23 Nov 2025 12:25:08 +0100 Subject: [PATCH] feat: Standardize SerieList to use key as primary identifier (Task 1.2) - Renamed folderDict to keyDict for clarity - Updated internal storage to use serie.key instead of serie.folder - Optimized contains() from O(n) to O(1) with direct key lookup - Added get_by_key() as primary lookup method - Added get_by_folder() for backward compatibility - Enhanced docstrings to clarify key vs folder usage - Created comprehensive test suite (12 tests, all passing) - Verified no breaking changes (16 SeriesApp tests pass) This establishes key as the single source of truth for series identification while maintaining folder as metadata for filesystem operations only. --- instructions.md | 33 +++++- src/core/entities/SerieList.py | 98 +++++++++++++--- tests/unit/test_serie_list.py | 203 +++++++++++++++++++++++++++++++++ 3 files changed, 310 insertions(+), 24 deletions(-) create mode 100644 tests/unit/test_serie_list.py diff --git a/instructions.md b/instructions.md index cbb54f4..93abb21 100644 --- a/instructions.md +++ b/instructions.md @@ -226,11 +226,11 @@ conda run -n AniWorld python -m pytest tests/unit/test_anime_models.py -v **Success Criteria:** -- [ ] Internal dictionary keyed by `serie.key` -- [ ] `add()`, `contains()`, `GetList()` work correctly -- [ ] Helper methods `get_by_key()` and `get_by_folder()` implemented -- [ ] All existing tests pass -- [ ] No breaking changes to public API +- [x] Internal dictionary keyed by `serie.key` +- [x] `add()`, `contains()`, `GetList()` work correctly +- [x] Helper methods `get_by_key()` and `get_by_folder()` implemented +- [x] All existing tests pass +- [x] No breaking changes to public API **Test Command:** @@ -238,6 +238,29 @@ conda run -n AniWorld python -m pytest tests/unit/test_anime_models.py -v conda run -n AniWorld python -m pytest tests/unit/ -k "SerieList" -v ``` +**Status:** ✅ COMPLETED + +**Implementation Details:** + +- Renamed `self.folderDict` to `self.keyDict` for clarity +- Updated internal storage to use `serie.key` as dictionary key +- Modified `add()` method to store series by key: `self.keyDict[serie.key] = serie` +- Optimized `contains()` method to use direct dictionary lookup: `return key in self.keyDict` +- Updated `_load_data()` to store loaded series by key with enhanced logging +- All methods (`GetMissingEpisode()`, `GetList()`, etc.) now iterate over `self.keyDict.values()` +- Added `get_by_key(key: str) -> Optional[Serie]` as primary lookup method +- Added `get_by_folder(folder: str) -> Optional[Serie]` for backward compatibility +- Created comprehensive test suite in `tests/unit/test_serie_list.py` with 12 tests covering: + - Key-based storage initialization + - Add, contains, and retrieval operations + - Duplicate prevention by key + - Helper method functionality (get_by_key, get_by_folder) + - Public API compatibility + - File loading and persistence +- All 12 new tests pass successfully +- All 16 SeriesApp tests pass, confirming no breaking changes +- Enhanced docstrings to clarify `key` as identifier and `folder` as metadata + --- #### Task 1.3: Update SerieScanner to Use Key Consistently diff --git a/src/core/entities/SerieList.py b/src/core/entities/SerieList.py index f20111b..288c46f 100644 --- a/src/core/entities/SerieList.py +++ b/src/core/entities/SerieList.py @@ -3,22 +3,35 @@ import logging import os from json import JSONDecodeError -from typing import Dict, Iterable, List +from typing import Dict, Iterable, List, Optional from src.core.entities.series import Serie class SerieList: - """Represents the collection of cached series stored on disk.""" + """ + Represents the collection of cached series stored on disk. + + Series are identified by their unique 'key' (provider identifier). + The 'folder' is metadata only and not used for lookups. + """ def __init__(self, base_path: str) -> None: self.directory: str = base_path - self.folderDict: Dict[str, Serie] = {} + # Internal storage using serie.key as the dictionary key + self.keyDict: Dict[str, Serie] = {} self.load_series() def add(self, serie: Serie) -> None: - """Persist a new series if it is not already present.""" - + """ + Persist a new series if it is not already present. + + Uses serie.key for identification. The serie.folder is used for + filesystem operations only. + + Args: + serie: The Serie instance to add + """ if self.contains(serie.key): return @@ -27,12 +40,20 @@ class SerieList: os.makedirs(anime_path, exist_ok=True) if not os.path.isfile(data_path): serie.save_to_file(data_path) - self.folderDict[serie.folder] = serie + # Store by key, not folder + self.keyDict[serie.key] = serie def contains(self, key: str) -> bool: - """Return True when a series identified by ``key`` already exists.""" - - return any(value.key == key for value in self.folderDict.values()) + """ + Return True when a series identified by ``key`` already exists. + + Args: + key: The unique provider identifier for the series + + Returns: + True if the series exists in the collection + """ + return key in self.keyDict def load_series(self) -> None: """Populate the in-memory map with metadata discovered on disk.""" @@ -61,11 +82,22 @@ class SerieList: ) def _load_data(self, anime_folder: str, data_path: str) -> None: - """Load a single series metadata file into the in-memory collection.""" - + """ + Load a single series metadata file into the in-memory collection. + + Args: + anime_folder: The folder name (for logging only) + data_path: Path to the metadata file + """ try: - self.folderDict[anime_folder] = Serie.load_from_file(data_path) - logging.debug("Successfully loaded metadata for %s", anime_folder) + serie = Serie.load_from_file(data_path) + # Store by key, not folder + self.keyDict[serie.key] = serie + logging.debug( + "Successfully loaded metadata for %s (key: %s)", + anime_folder, + serie.key + ) except (OSError, JSONDecodeError, KeyError, ValueError) as error: logging.error( "Failed to load metadata for folder %s from %s: %s", @@ -76,24 +108,52 @@ class SerieList: def GetMissingEpisode(self) -> List[Serie]: """Return all series that still contain missing episodes.""" - return [ serie - for serie in self.folderDict.values() + for serie in self.keyDict.values() if serie.episodeDict ] def get_missing_episodes(self) -> List[Serie]: """PEP8-friendly alias for :meth:`GetMissingEpisode`.""" - return self.GetMissingEpisode() def GetList(self) -> List[Serie]: """Return all series instances stored in the list.""" - - return list(self.folderDict.values()) + return list(self.keyDict.values()) def get_all(self) -> List[Serie]: """PEP8-friendly alias for :meth:`GetList`.""" - return self.GetList() + + def get_by_key(self, key: str) -> Optional[Serie]: + """ + Get a series by its unique provider key. + + This is the primary method for series lookup. + + Args: + key: The unique provider identifier (e.g., "attack-on-titan") + + Returns: + The Serie instance if found, None otherwise + """ + return self.keyDict.get(key) + + def get_by_folder(self, folder: str) -> Optional[Serie]: + """ + Get a series by its folder name. + + This method is provided for backward compatibility only. + Prefer using get_by_key() for new code. + + Args: + folder: The filesystem folder name (e.g., "Attack on Titan (2013)") + + Returns: + The Serie instance if found, None otherwise + """ + for serie in self.keyDict.values(): + if serie.folder == folder: + return serie + return None diff --git a/tests/unit/test_serie_list.py b/tests/unit/test_serie_list.py new file mode 100644 index 0000000..30e0f07 --- /dev/null +++ b/tests/unit/test_serie_list.py @@ -0,0 +1,203 @@ +"""Tests for SerieList class - identifier standardization.""" + +import os +import tempfile + +import pytest + +from src.core.entities.SerieList import SerieList +from src.core.entities.series import Serie + + +@pytest.fixture +def temp_directory(): + """Create a temporary directory for testing.""" + with tempfile.TemporaryDirectory() as tmpdir: + yield tmpdir + + +@pytest.fixture +def sample_serie(): + """Create a sample Serie for testing.""" + return Serie( + key="attack-on-titan", + name="Attack on Titan", + site="https://aniworld.to/anime/stream/attack-on-titan", + folder="Attack on Titan (2013)", + episodeDict={1: [1, 2, 3]} + ) + + +class TestSerieListKeyBasedStorage: + """Test SerieList uses key for internal storage.""" + + def test_init_creates_empty_keydict(self, temp_directory): + """Test initialization creates keyDict.""" + serie_list = SerieList(temp_directory) + assert hasattr(serie_list, 'keyDict') + assert isinstance(serie_list.keyDict, dict) + + def test_add_stores_by_key(self, temp_directory, sample_serie): + """Test add() stores series by key.""" + serie_list = SerieList(temp_directory) + serie_list.add(sample_serie) + + # Verify stored by key, not folder + assert sample_serie.key in serie_list.keyDict + assert serie_list.keyDict[sample_serie.key] == sample_serie + + def test_contains_checks_by_key(self, temp_directory, sample_serie): + """Test contains() checks by key.""" + serie_list = SerieList(temp_directory) + serie_list.add(sample_serie) + + assert serie_list.contains(sample_serie.key) + assert not serie_list.contains("nonexistent-key") + + def test_add_prevents_duplicates_by_key( + self, temp_directory, sample_serie + ): + """Test add() prevents duplicates based on key.""" + serie_list = SerieList(temp_directory) + + # Add same serie twice + serie_list.add(sample_serie) + initial_count = len(serie_list.keyDict) + + serie_list.add(sample_serie) + + # Should still have only one entry + assert len(serie_list.keyDict) == initial_count + assert len(serie_list.keyDict) == 1 + + def test_get_by_key_returns_correct_serie( + self, temp_directory, sample_serie + ): + """Test get_by_key() retrieves series correctly.""" + serie_list = SerieList(temp_directory) + serie_list.add(sample_serie) + + result = serie_list.get_by_key(sample_serie.key) + assert result is not None + assert result.key == sample_serie.key + assert result.name == sample_serie.name + + def test_get_by_key_returns_none_for_missing(self, temp_directory): + """Test get_by_key() returns None for nonexistent key.""" + serie_list = SerieList(temp_directory) + + result = serie_list.get_by_key("nonexistent-key") + assert result is None + + def test_get_by_folder_backward_compatibility( + self, temp_directory, sample_serie + ): + """Test get_by_folder() provides backward compatibility.""" + serie_list = SerieList(temp_directory) + serie_list.add(sample_serie) + + result = serie_list.get_by_folder(sample_serie.folder) + assert result is not None + assert result.key == sample_serie.key + assert result.folder == sample_serie.folder + + def test_get_by_folder_returns_none_for_missing(self, temp_directory): + """Test get_by_folder() returns None for nonexistent folder.""" + serie_list = SerieList(temp_directory) + + result = serie_list.get_by_folder("Nonexistent Folder") + assert result is None + + def test_get_all_returns_all_series(self, temp_directory, sample_serie): + """Test get_all() returns all series from keyDict.""" + serie_list = SerieList(temp_directory) + serie_list.add(sample_serie) + + serie2 = Serie( + key="naruto", + name="Naruto", + site="https://aniworld.to/anime/stream/naruto", + folder="Naruto (2002)", + episodeDict={1: [1, 2]} + ) + serie_list.add(serie2) + + all_series = serie_list.get_all() + assert len(all_series) == 2 + assert sample_serie in all_series + assert serie2 in all_series + + def test_get_missing_episodes_filters_by_episode_dict( + self, temp_directory + ): + """Test get_missing_episodes() returns only series with episodes.""" + serie_list = SerieList(temp_directory) + + # Serie with missing episodes + serie_with_episodes = Serie( + key="serie-with-episodes", + name="Serie With Episodes", + site="https://aniworld.to/anime/stream/serie-with-episodes", + folder="Serie With Episodes (2020)", + episodeDict={1: [1, 2, 3]} + ) + + # Serie without missing episodes + serie_without_episodes = Serie( + key="serie-without-episodes", + name="Serie Without Episodes", + site="https://aniworld.to/anime/stream/serie-without-episodes", + folder="Serie Without Episodes (2021)", + episodeDict={} + ) + + serie_list.add(serie_with_episodes) + serie_list.add(serie_without_episodes) + + missing = serie_list.get_missing_episodes() + assert len(missing) == 1 + assert serie_with_episodes in missing + assert serie_without_episodes not in missing + + def test_load_series_stores_by_key(self, temp_directory, sample_serie): + """Test load_series() stores series by key when loading from disk.""" + # Create directory structure and save serie + folder_path = os.path.join(temp_directory, sample_serie.folder) + os.makedirs(folder_path, exist_ok=True) + data_path = os.path.join(folder_path, "data") + sample_serie.save_to_file(data_path) + + # Create new SerieList (triggers load_series in __init__) + serie_list = SerieList(temp_directory) + + # Verify loaded by key + assert sample_serie.key in serie_list.keyDict + loaded_serie = serie_list.keyDict[sample_serie.key] + assert loaded_serie.key == sample_serie.key + assert loaded_serie.name == sample_serie.name + + +class TestSerieListPublicAPI: + """Test that public API still works correctly.""" + + def test_public_methods_work(self, temp_directory, sample_serie): + """Test that all public methods work correctly after refactoring.""" + serie_list = SerieList(temp_directory) + + # Test add + serie_list.add(sample_serie) + + # Test contains + assert serie_list.contains(sample_serie.key) + + # Test GetList/get_all + assert len(serie_list.GetList()) == 1 + assert len(serie_list.get_all()) == 1 + + # Test GetMissingEpisode/get_missing_episodes + assert len(serie_list.GetMissingEpisode()) == 1 + assert len(serie_list.get_missing_episodes()) == 1 + + # Test new helper methods + assert serie_list.get_by_key(sample_serie.key) is not None + assert serie_list.get_by_folder(sample_serie.folder) is not None