refactor(domain): strip live filesystem I/O from VOs and entities
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)))
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user