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.
This commit is contained in:
parent
048434d49c
commit
8b5b06ca9a
@ -226,11 +226,11 @@ conda run -n AniWorld python -m pytest tests/unit/test_anime_models.py -v
|
|||||||
|
|
||||||
**Success Criteria:**
|
**Success Criteria:**
|
||||||
|
|
||||||
- [ ] Internal dictionary keyed by `serie.key`
|
- [x] Internal dictionary keyed by `serie.key`
|
||||||
- [ ] `add()`, `contains()`, `GetList()` work correctly
|
- [x] `add()`, `contains()`, `GetList()` work correctly
|
||||||
- [ ] Helper methods `get_by_key()` and `get_by_folder()` implemented
|
- [x] Helper methods `get_by_key()` and `get_by_folder()` implemented
|
||||||
- [ ] All existing tests pass
|
- [x] All existing tests pass
|
||||||
- [ ] No breaking changes to public API
|
- [x] No breaking changes to public API
|
||||||
|
|
||||||
**Test Command:**
|
**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
|
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
|
#### Task 1.3: Update SerieScanner to Use Key Consistently
|
||||||
|
|||||||
@ -3,22 +3,35 @@
|
|||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
from json import JSONDecodeError
|
from json import JSONDecodeError
|
||||||
from typing import Dict, Iterable, List
|
from typing import Dict, Iterable, List, Optional
|
||||||
|
|
||||||
from src.core.entities.series import Serie
|
from src.core.entities.series import Serie
|
||||||
|
|
||||||
|
|
||||||
class SerieList:
|
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:
|
def __init__(self, base_path: str) -> None:
|
||||||
self.directory: str = base_path
|
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()
|
self.load_series()
|
||||||
|
|
||||||
def add(self, serie: Serie) -> None:
|
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):
|
if self.contains(serie.key):
|
||||||
return
|
return
|
||||||
|
|
||||||
@ -27,12 +40,20 @@ class SerieList:
|
|||||||
os.makedirs(anime_path, exist_ok=True)
|
os.makedirs(anime_path, exist_ok=True)
|
||||||
if not os.path.isfile(data_path):
|
if not os.path.isfile(data_path):
|
||||||
serie.save_to_file(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:
|
def contains(self, key: str) -> bool:
|
||||||
"""Return True when a series identified by ``key`` already exists."""
|
"""
|
||||||
|
Return True when a series identified by ``key`` already exists.
|
||||||
return any(value.key == key for value in self.folderDict.values())
|
|
||||||
|
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:
|
def load_series(self) -> None:
|
||||||
"""Populate the in-memory map with metadata discovered on disk."""
|
"""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:
|
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:
|
try:
|
||||||
self.folderDict[anime_folder] = Serie.load_from_file(data_path)
|
serie = Serie.load_from_file(data_path)
|
||||||
logging.debug("Successfully loaded metadata for %s", anime_folder)
|
# 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:
|
except (OSError, JSONDecodeError, KeyError, ValueError) as error:
|
||||||
logging.error(
|
logging.error(
|
||||||
"Failed to load metadata for folder %s from %s: %s",
|
"Failed to load metadata for folder %s from %s: %s",
|
||||||
@ -76,24 +108,52 @@ class SerieList:
|
|||||||
|
|
||||||
def GetMissingEpisode(self) -> List[Serie]:
|
def GetMissingEpisode(self) -> List[Serie]:
|
||||||
"""Return all series that still contain missing episodes."""
|
"""Return all series that still contain missing episodes."""
|
||||||
|
|
||||||
return [
|
return [
|
||||||
serie
|
serie
|
||||||
for serie in self.folderDict.values()
|
for serie in self.keyDict.values()
|
||||||
if serie.episodeDict
|
if serie.episodeDict
|
||||||
]
|
]
|
||||||
|
|
||||||
def get_missing_episodes(self) -> List[Serie]:
|
def get_missing_episodes(self) -> List[Serie]:
|
||||||
"""PEP8-friendly alias for :meth:`GetMissingEpisode`."""
|
"""PEP8-friendly alias for :meth:`GetMissingEpisode`."""
|
||||||
|
|
||||||
return self.GetMissingEpisode()
|
return self.GetMissingEpisode()
|
||||||
|
|
||||||
def GetList(self) -> List[Serie]:
|
def GetList(self) -> List[Serie]:
|
||||||
"""Return all series instances stored in the list."""
|
"""Return all series instances stored in the list."""
|
||||||
|
return list(self.keyDict.values())
|
||||||
return list(self.folderDict.values())
|
|
||||||
|
|
||||||
def get_all(self) -> List[Serie]:
|
def get_all(self) -> List[Serie]:
|
||||||
"""PEP8-friendly alias for :meth:`GetList`."""
|
"""PEP8-friendly alias for :meth:`GetList`."""
|
||||||
|
|
||||||
return self.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
|
||||||
|
|||||||
203
tests/unit/test_serie_list.py
Normal file
203
tests/unit/test_serie_list.py
Normal file
@ -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
|
||||||
Loading…
x
Reference in New Issue
Block a user