From 9556bf9e081d1abe1913a4f268920cef3c51c96a Mon Sep 17 00:00:00 2001 From: Francwa Date: Tue, 19 May 2026 14:58:59 +0200 Subject: [PATCH] refactor(domain): strip live filesystem I/O from VOs and entities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DDD-pure cleanup — entities and value objects no longer query the world at read time. FilePath: drop .exists() / .is_file() / .is_dir(). The VO is now a pure address; ask the injected FilesystemScanner for live state. Movie: drop .has_file() / .is_downloaded(). Invariant: when the application sets file_path, it has already constated the file exists; downstream readers trust the snapshot. Episode: same — drop .has_file() / .is_downloaded(). SubtitlePlacer: drop the pre-check .exists() calls. The placer now attempts os.link() and reports FileNotFoundError / FileExistsError as skip reasons. Removes a TOCTOU race as a bonus. Tests adjusted: the FilePath VO method tests are gone (the methods are gone), test_has_file_false_when_no_path replaced by a plain assertion on file_path is None. Placer tests are unchanged — the skip-reason strings ('not found', 'already exists') match the new try/except paths. The 'snapshot value objects' pattern (ProbedMediaInfo, TmdbMovieInfo) that this cleanup enables is documented in refactor_domain_io.md, to be applied when a future use case actually needs richer metadata — not now, no speculative VOs. --- alfred/domain/movies/entities.py | 8 -------- alfred/domain/shared/value_objects.py | 12 ------------ alfred/domain/subtitles/services/placer.py | 14 +++++++------- alfred/domain/tv_shows/entities.py | 10 ---------- tests/domain/test_shared_value_objects.py | 17 ----------------- tests/domain/test_tv_shows.py | 5 ++--- 6 files changed, 9 insertions(+), 57 deletions(-) diff --git a/alfred/domain/movies/entities.py b/alfred/domain/movies/entities.py index df171e3..bdac712 100644 --- a/alfred/domain/movies/entities.py +++ b/alfred/domain/movies/entities.py @@ -65,14 +65,6 @@ class Movie(MediaWithTracks): def __hash__(self) -> int: return hash(self.imdb_id) - def has_file(self) -> bool: - """Check if the movie has an associated file.""" - return self.file_path is not None and self.file_path.exists() - - def is_downloaded(self) -> bool: - """Check if the movie is downloaded (has a file).""" - return self.has_file() - # Track helpers (has_audio_in / audio_languages / has_subtitles_in / # has_forced_subs / subtitle_languages) come from MediaWithTracks. diff --git a/alfred/domain/shared/value_objects.py b/alfred/domain/shared/value_objects.py index 9aafdcf..74f8d11 100644 --- a/alfred/domain/shared/value_objects.py +++ b/alfred/domain/shared/value_objects.py @@ -67,18 +67,6 @@ class FilePath: # Use object.__setattr__ because dataclass is frozen object.__setattr__(self, "value", path_obj) - def exists(self) -> bool: - """Check if the path exists.""" - return self.value.exists() - - def is_file(self) -> bool: - """Check if the path is a file.""" - return self.value.is_file() - - def is_dir(self) -> bool: - """Check if the path is a directory.""" - return self.value.is_dir() - def __str__(self) -> str: return str(self.value) diff --git a/alfred/domain/subtitles/services/placer.py b/alfred/domain/subtitles/services/placer.py index 81b6a56..584fc59 100644 --- a/alfred/domain/subtitles/services/placer.py +++ b/alfred/domain/subtitles/services/placer.py @@ -78,8 +78,8 @@ class SubtitlePlacer: skipped.append((track, "embedded — no file to place")) continue - if not track.file_path or not track.file_path.exists(): - skipped.append((track, "source file not found")) + if not track.file_path: + skipped.append((track, "source file not set")) continue try: @@ -90,11 +90,6 @@ class SubtitlePlacer: dest_path = dest_dir / dest_name - if dest_path.exists(): - logger.debug(f"SubtitlePlacer: skip {dest_name} — already exists") - skipped.append((track, "destination already exists")) - continue - try: os.link(track.file_path, dest_path) placed.append( @@ -105,6 +100,11 @@ class SubtitlePlacer: ) ) logger.info(f"SubtitlePlacer: placed {dest_name}") + except FileNotFoundError: + skipped.append((track, "source file not found")) + except FileExistsError: + logger.debug(f"SubtitlePlacer: skip {dest_name} — already exists") + skipped.append((track, "destination already exists")) except OSError as e: logger.warning(f"SubtitlePlacer: failed to place {dest_name}: {e}") skipped.append((track, str(e))) diff --git a/alfred/domain/tv_shows/entities.py b/alfred/domain/tv_shows/entities.py index 1e6533a..0ad87ec 100644 --- a/alfred/domain/tv_shows/entities.py +++ b/alfred/domain/tv_shows/entities.py @@ -91,16 +91,6 @@ class Episode(MediaWithTracks): def __hash__(self) -> int: return hash((self.season_number, self.episode_number)) - # ── File presence ────────────────────────────────────────────────────── - - def has_file(self) -> bool: - """True if a file path is set and the file actually exists on disk.""" - return self.file_path is not None and self.file_path.exists() - - def is_downloaded(self) -> bool: - """Alias of ``has_file()`` — reads better in collection-status contexts.""" - return self.has_file() - # Track helpers (has_audio_in / audio_languages / has_subtitles_in / # has_forced_subs / subtitle_languages) come from MediaWithTracks. diff --git a/tests/domain/test_shared_value_objects.py b/tests/domain/test_shared_value_objects.py index 9ba42f0..63335a2 100644 --- a/tests/domain/test_shared_value_objects.py +++ b/tests/domain/test_shared_value_objects.py @@ -72,23 +72,6 @@ class TestFilePath: with pytest.raises(ValidationError): FilePath(123) # type: ignore - def test_exists_true(self, tmp_path): - p = FilePath(tmp_path) - assert p.exists() - - def test_exists_false(self, tmp_path): - p = FilePath(tmp_path / "nonexistent") - assert not p.exists() - - def test_is_file(self, tmp_path): - f = tmp_path / "file.txt" - f.write_text("x") - assert FilePath(f).is_file() - assert not FilePath(tmp_path).is_file() - - def test_is_dir(self, tmp_path): - assert FilePath(tmp_path).is_dir() - def test_str(self, tmp_path): p = FilePath(tmp_path) assert str(p) == str(tmp_path) diff --git a/tests/domain/test_tv_shows.py b/tests/domain/test_tv_shows.py index e5fb35f..0625a73 100644 --- a/tests/domain/test_tv_shows.py +++ b/tests/domain/test_tv_shows.py @@ -157,10 +157,9 @@ class TestEpisode: assert filename.startswith("S01E05") assert "Gray.Matter" in filename - def test_has_file_false_when_no_path(self): + def test_file_path_unset_by_default(self): e = self._ep() - assert not e.has_file() - assert not e.is_downloaded() + assert e.file_path is None def test_str_format(self): e = self._ep(season=2, episode=3, title="Bit by a Dead Bee")