Compare commits
3 Commits
v1.5.7
...
0f872276dd
| Author | SHA1 | Date | |
|---|---|---|---|
| 0f872276dd | |||
| 818e621288 | |||
| 14f12e55e7 |
@@ -1244,8 +1244,7 @@ class AnimeService:
|
|||||||
Returns:
|
Returns:
|
||||||
True if rename was performed, False if no rename needed or failed
|
True if rename was performed, False if no rename needed or failed
|
||||||
"""
|
"""
|
||||||
import os
|
from pathlib import Path
|
||||||
import shutil
|
|
||||||
|
|
||||||
if current_folder == target_folder:
|
if current_folder == target_folder:
|
||||||
logger.debug(
|
logger.debug(
|
||||||
@@ -1254,8 +1253,9 @@ class AnimeService:
|
|||||||
)
|
)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
current_path = self._directory / current_folder
|
base_dir = Path(self._directory)
|
||||||
target_path = self._directory / target_folder
|
current_path = base_dir / current_folder
|
||||||
|
target_path = base_dir / target_folder
|
||||||
|
|
||||||
if not current_path.exists():
|
if not current_path.exists():
|
||||||
logger.debug(
|
logger.debug(
|
||||||
@@ -1265,15 +1265,54 @@ class AnimeService:
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
if target_path.exists():
|
if target_path.exists():
|
||||||
logger.warning(
|
# Target already exists — merge source into target instead of
|
||||||
"Cannot rename folder for %s: target path already exists: %s",
|
# bailing. Without this, a bare folder ('Naruto') next to the
|
||||||
key,
|
# year-suffixed one ('Naruto (2019)') would orphan the bare
|
||||||
target_path
|
# folder forever, producing the "series added twice" symptom.
|
||||||
|
try:
|
||||||
|
summary = self._merge_folder_into_target(
|
||||||
|
str(current_path), str(target_path)
|
||||||
|
)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.error(
|
||||||
|
"Failed to merge %s -> %s for %s: %s",
|
||||||
|
current_folder, target_folder, key, exc,
|
||||||
)
|
)
|
||||||
return False
|
return False
|
||||||
|
logger.info(
|
||||||
|
"Merged folder %s -> %s for series %s (moved=%d skipped=%d removed_source=%s)",
|
||||||
|
current_folder, target_folder, key,
|
||||||
|
summary["moved"], summary["skipped"], summary["removed_source"],
|
||||||
|
)
|
||||||
|
|
||||||
|
# Update in-memory cache
|
||||||
|
if key in self._app.list.keyDict:
|
||||||
|
self._app.list.keyDict[key].folder = target_folder
|
||||||
|
logger.debug(
|
||||||
|
"Updated in-memory cache folder for %s: %s",
|
||||||
|
key, target_folder
|
||||||
|
)
|
||||||
|
|
||||||
|
# Update database if session provided
|
||||||
|
if db is not None:
|
||||||
|
from src.server.database.service import AnimeSeriesService
|
||||||
|
|
||||||
|
# Look up series by key to get database ID
|
||||||
|
series = await AnimeSeriesService.get_by_key(db, key)
|
||||||
|
if series:
|
||||||
|
await AnimeSeriesService.update(
|
||||||
|
db, series_id=series.id, folder=target_folder
|
||||||
|
)
|
||||||
|
logger.debug(
|
||||||
|
"Updated DB folder for %s: %s",
|
||||||
|
key, target_folder
|
||||||
|
)
|
||||||
|
|
||||||
|
return True
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Rename folder on disk
|
# Rename folder on disk
|
||||||
|
import shutil
|
||||||
shutil.move(str(current_path), str(target_path))
|
shutil.move(str(current_path), str(target_path))
|
||||||
logger.info(
|
logger.info(
|
||||||
"Renamed folder for %s: %s -> %s",
|
"Renamed folder for %s: %s -> %s",
|
||||||
@@ -1317,6 +1356,83 @@ class AnimeService:
|
|||||||
)
|
)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _merge_folder_into_target(source: str, target: str) -> dict:
|
||||||
|
"""Merge a source folder's contents into an existing target folder.
|
||||||
|
|
||||||
|
Walks the source tree and moves every file into the matching path
|
||||||
|
under the target. When a destination file already exists, the
|
||||||
|
source copy is removed (the target version wins; we don't keep
|
||||||
|
duplicates). When the source tree is fully consumed, the
|
||||||
|
(now-empty) source directory is removed.
|
||||||
|
|
||||||
|
Both paths must be absolute and ``target`` must already exist on
|
||||||
|
disk.
|
||||||
|
|
||||||
|
Returns a summary dict with ``moved`` (file count), ``skipped``
|
||||||
|
(file count where target already had a copy), and
|
||||||
|
``removed_source`` (bool).
|
||||||
|
"""
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
|
||||||
|
if not os.path.isdir(source):
|
||||||
|
return {"moved": 0, "skipped": 0, "removed_source": False}
|
||||||
|
if not os.path.isdir(target):
|
||||||
|
raise ValueError(f"target does not exist: {target}")
|
||||||
|
|
||||||
|
moved = 0
|
||||||
|
skipped = 0
|
||||||
|
for root, _dirs, files in os.walk(source):
|
||||||
|
rel_root = os.path.relpath(root, source)
|
||||||
|
dest_root = (
|
||||||
|
target if rel_root == "."
|
||||||
|
else os.path.join(target, rel_root)
|
||||||
|
)
|
||||||
|
os.makedirs(dest_root, exist_ok=True)
|
||||||
|
for name in files:
|
||||||
|
src_file = os.path.join(root, name)
|
||||||
|
dest_file = os.path.join(dest_root, name)
|
||||||
|
if os.path.exists(dest_file):
|
||||||
|
# Target wins — never overwrite existing content.
|
||||||
|
# Remove the orphaned source copy so cleanup below
|
||||||
|
# can rmdir it.
|
||||||
|
try:
|
||||||
|
os.remove(src_file)
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(
|
||||||
|
"merge: could not remove duplicate %s: %s",
|
||||||
|
src_file, exc,
|
||||||
|
)
|
||||||
|
skipped += 1
|
||||||
|
logger.warning(
|
||||||
|
"merge: skipping %s (target already has %s)",
|
||||||
|
src_file, dest_file,
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
shutil.move(src_file, dest_file)
|
||||||
|
moved += 1
|
||||||
|
|
||||||
|
# Try to remove the (now empty) source tree. Walk bottom-up so
|
||||||
|
# leaf directories are removed before their parents.
|
||||||
|
removed_source = False
|
||||||
|
for root, dirs, files in os.walk(source, topdown=False):
|
||||||
|
for d in dirs:
|
||||||
|
try:
|
||||||
|
os.rmdir(os.path.join(root, d))
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
try:
|
||||||
|
os.rmdir(source)
|
||||||
|
removed_source = True
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(
|
||||||
|
"merge: could not remove source directory %s: %s",
|
||||||
|
source, exc,
|
||||||
|
)
|
||||||
|
|
||||||
|
return {"moved": moved, "skipped": skipped, "removed_source": removed_source}
|
||||||
|
|
||||||
async def contains_in_db(self, key: str, db) -> bool:
|
async def contains_in_db(self, key: str, db) -> bool:
|
||||||
"""
|
"""
|
||||||
Check if a series with the given key exists in the database.
|
Check if a series with the given key exists in the database.
|
||||||
@@ -1649,12 +1765,29 @@ class AnimeService:
|
|||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
DeleteSeriesResult with success status, what was deleted, errors
|
DeleteSeriesResult with success status, what was deleted, errors
|
||||||
|
|
||||||
|
Deletion order: filesystem first, database second.
|
||||||
|
|
||||||
|
This order matters: if the folder delete fails (e.g. permission
|
||||||
|
error, path outside the configured anime directory), the database
|
||||||
|
row is preserved so the user can retry the delete once the
|
||||||
|
underlying issue is resolved. If we deleted the database row
|
||||||
|
first, an orphan folder would be left on disk with no way to
|
||||||
|
clean it up through the normal delete flow.
|
||||||
|
|
||||||
|
Orphan folder recovery: when ``delete_folder=True`` is requested
|
||||||
|
for a series whose database row no longer exists, the configured
|
||||||
|
anime directory is scanned for a folder that uniquely matches
|
||||||
|
the key. This recovers the case where a previous delete with
|
||||||
|
``delete_database=True`` succeeded but ``delete_folder=True``
|
||||||
|
silently failed, leaving the folder on disk.
|
||||||
"""
|
"""
|
||||||
from src.server.database.connection import get_db_session
|
from src.server.database.connection import get_db_session
|
||||||
from src.server.database.service import AnimeSeriesService
|
from src.server.database.service import AnimeSeriesService
|
||||||
from src.server.models.anime import DeleteSeriesResult
|
from src.server.models.anime import DeleteSeriesResult
|
||||||
from src.server.utils.filesystem import is_safe_path
|
from src.server.utils.filesystem import is_safe_path
|
||||||
import os as _os
|
import os as _os
|
||||||
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -1679,13 +1812,63 @@ class AnimeService:
|
|||||||
message="At least one of delete_database or delete_folder must be True.",
|
message="At least one of delete_database or delete_folder must be True.",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Single DB session for fetch + optional delete
|
# Look up the series in the DB to get its folder path
|
||||||
|
series = None
|
||||||
async with get_db_session() as db:
|
async with get_db_session() as db:
|
||||||
series = await AnimeSeriesService.get_by_key(db, key)
|
series = await AnimeSeriesService.get_by_key(db, key)
|
||||||
|
|
||||||
if not series:
|
if not series:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Delete series failed - not found: key=%s", key
|
"Delete series - row not found in DB: key=%s delete_folder=%s",
|
||||||
|
key, delete_folder,
|
||||||
)
|
)
|
||||||
|
# Recovery path: if the user wants to delete the folder but the
|
||||||
|
# DB row is already gone (e.g. orphaned by a previous partial
|
||||||
|
# delete), scan the configured anime directory for a folder
|
||||||
|
# that uniquely matches this key and delete it.
|
||||||
|
if delete_folder:
|
||||||
|
folder_path = self._find_orphan_folder_for_key(key)
|
||||||
|
if folder_path:
|
||||||
|
logger.info(
|
||||||
|
"Orphan folder recovery: key=%s matched folder=%s",
|
||||||
|
key, folder_path,
|
||||||
|
)
|
||||||
|
result = DeleteSeriesResult(
|
||||||
|
success=True,
|
||||||
|
key=key,
|
||||||
|
name="",
|
||||||
|
folder_path=folder_path,
|
||||||
|
message="",
|
||||||
|
)
|
||||||
|
self._delete_folder_at_path(folder_path, key, result)
|
||||||
|
# No DB row to delete; build message and return
|
||||||
|
self._build_delete_message(result)
|
||||||
|
logger.info(
|
||||||
|
"Delete series completed (orphan recovery): key=%s "
|
||||||
|
"deleted_folder=%s folder_error=%s",
|
||||||
|
key, result.deleted_folder, result.folder_error,
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
return DeleteSeriesResult(
|
||||||
|
success=False,
|
||||||
|
key=key,
|
||||||
|
name="",
|
||||||
|
folder_path=None,
|
||||||
|
deleted_from_database=False,
|
||||||
|
deleted_folder=False,
|
||||||
|
database_error=None,
|
||||||
|
folder_error=(
|
||||||
|
f"Series '{key}' not found in database, and no folder "
|
||||||
|
"matching this key was found in the anime directory. "
|
||||||
|
"Nothing to delete."
|
||||||
|
),
|
||||||
|
message=(
|
||||||
|
f"Series '{key}' not found. If the folder on disk is "
|
||||||
|
"still required to be removed, please specify its "
|
||||||
|
"exact name on the filesystem."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
# No row, no folder requested — nothing to do
|
||||||
return DeleteSeriesResult(
|
return DeleteSeriesResult(
|
||||||
success=False,
|
success=False,
|
||||||
key=key,
|
key=key,
|
||||||
@@ -1710,7 +1893,22 @@ class AnimeService:
|
|||||||
message="",
|
message="",
|
||||||
)
|
)
|
||||||
|
|
||||||
# --- Database deletion ---
|
# --- Filesystem deletion (do FIRST so a failure preserves the DB row) ---
|
||||||
|
if delete_folder and folder_path:
|
||||||
|
self._delete_folder_at_path(folder_path, key, result)
|
||||||
|
# If folder delete was requested but failed, abort before
|
||||||
|
# removing the DB row so the user can retry.
|
||||||
|
if not result.deleted_folder and result.folder_error:
|
||||||
|
result.success = False
|
||||||
|
self._build_delete_message(result)
|
||||||
|
logger.warning(
|
||||||
|
"Delete series aborted - folder delete failed; DB row preserved: "
|
||||||
|
"key=%s folder_error=%s",
|
||||||
|
key, result.folder_error,
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
|
||||||
|
# --- Database deletion (do AFTER folder delete) ---
|
||||||
if delete_database:
|
if delete_database:
|
||||||
try:
|
try:
|
||||||
async with get_db_session() as db:
|
async with get_db_session() as db:
|
||||||
@@ -1747,11 +1945,45 @@ class AnimeService:
|
|||||||
key, exc,
|
key, exc,
|
||||||
)
|
)
|
||||||
|
|
||||||
# --- Filesystem deletion ---
|
# --- Build message ---
|
||||||
if delete_folder and folder_path:
|
self._build_delete_message(result)
|
||||||
# Resolve absolute path and validate it is within base directory
|
|
||||||
abs_folder = _os.path.abspath(folder_path)
|
logger.info(
|
||||||
|
"Delete series completed: key=%s deleted_db=%s deleted_folder=%s",
|
||||||
|
key, result.deleted_from_database, result.deleted_folder,
|
||||||
|
)
|
||||||
|
return result
|
||||||
|
|
||||||
|
def _delete_folder_at_path(self, folder_path, key, result):
|
||||||
|
"""Resolve ``folder_path`` against the configured anime directory
|
||||||
|
and attempt to remove it. Updates ``result`` in place.
|
||||||
|
|
||||||
|
Resolves relative paths against ``self._directory`` so the safety
|
||||||
|
check operates on the real intended target (the process's current
|
||||||
|
working directory is not used as the base; in containers CWD may
|
||||||
|
differ from the anime directory, e.g. /app vs /data).
|
||||||
|
"""
|
||||||
|
import os as _os
|
||||||
|
import shutil
|
||||||
|
|
||||||
|
# Resolve absolute path and validate it is within base directory.
|
||||||
|
#
|
||||||
|
# Important: `folder_path` stored in the database is the relative
|
||||||
|
# folder name (e.g. "Beyblade Burst (2016)"), not an absolute path.
|
||||||
|
# If we feed a relative path to os.path.abspath() it gets joined
|
||||||
|
# against the process's current working directory — which may be
|
||||||
|
# /app inside the container while the anime directory is /data,
|
||||||
|
# producing e.g. "/app/Beyblade Burst (2016)" and tripping the
|
||||||
|
# safe-path check below for what is actually a valid deletion.
|
||||||
|
# Resolve relative paths against the configured anime directory
|
||||||
|
# so the safety check operates on the real intended target.
|
||||||
|
from src.server.utils.filesystem import is_safe_path
|
||||||
|
|
||||||
base_dir = _os.path.abspath(self._directory)
|
base_dir = _os.path.abspath(self._directory)
|
||||||
|
if _os.path.isabs(folder_path):
|
||||||
|
abs_folder = _os.path.abspath(folder_path)
|
||||||
|
else:
|
||||||
|
abs_folder = _os.path.abspath(_os.path.join(base_dir, folder_path))
|
||||||
|
|
||||||
if not is_safe_path(base_dir, abs_folder):
|
if not is_safe_path(base_dir, abs_folder):
|
||||||
logger.warning(
|
logger.warning(
|
||||||
@@ -1763,13 +1995,14 @@ class AnimeService:
|
|||||||
f"'{base_dir}' and will not be deleted."
|
f"'{base_dir}' and will not be deleted."
|
||||||
)
|
)
|
||||||
result.success = False
|
result.success = False
|
||||||
elif not _os.path.isdir(abs_folder):
|
return
|
||||||
|
if not _os.path.isdir(abs_folder):
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Delete folder skipped - path does not exist: key=%s path=%s",
|
"Delete folder skipped - path does not exist: key=%s path=%s",
|
||||||
key, abs_folder,
|
key, abs_folder,
|
||||||
)
|
)
|
||||||
# Not an error; folder might never have existed
|
# Not an error; folder might never have existed
|
||||||
else:
|
return
|
||||||
try:
|
try:
|
||||||
logger.info(
|
logger.info(
|
||||||
"Deleting series folder: key=%s path=%s",
|
"Deleting series folder: key=%s path=%s",
|
||||||
@@ -1789,7 +2022,76 @@ class AnimeService:
|
|||||||
result.folder_error = str(exc)
|
result.folder_error = str(exc)
|
||||||
result.success = False
|
result.success = False
|
||||||
|
|
||||||
# --- Build message ---
|
def _find_orphan_folder_for_key(self, key: str):
|
||||||
|
"""Locate a folder under ``self._directory`` that uniquely matches
|
||||||
|
the given series ``key``.
|
||||||
|
|
||||||
|
Used as a recovery path when the DB row is gone but the on-disk
|
||||||
|
folder still exists (orphaned by a previous partial delete).
|
||||||
|
|
||||||
|
Matching strategy: for each immediate subdirectory of the anime
|
||||||
|
directory, strip a trailing ``(YYYY)`` year suffix if present and
|
||||||
|
then compare the normalized form (lowercased, non-alphanumerics
|
||||||
|
removed, key's hyphens treated as separators) against the key.
|
||||||
|
Returns the folder name (relative to the anime directory) of the
|
||||||
|
unique match, or ``None`` if zero or multiple folders match.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The matching relative folder name, or None when no unique
|
||||||
|
match exists. Returning None is the safe default — it forces
|
||||||
|
the caller to surface an explicit error rather than risk
|
||||||
|
deleting the wrong folder.
|
||||||
|
"""
|
||||||
|
import os as _os
|
||||||
|
import re
|
||||||
|
|
||||||
|
if not self._directory or not _os.path.isdir(self._directory):
|
||||||
|
return None
|
||||||
|
|
||||||
|
def _normalize(value: str) -> str:
|
||||||
|
# Drop an optional trailing "(YYYY)" or "(YYYY)"-with-content
|
||||||
|
# suffix the user might have added for disambiguation. We only
|
||||||
|
# strip a single trailing parenthesised group to avoid eating
|
||||||
|
# legitimate parts of the title.
|
||||||
|
value = re.sub(r"\s*\([^)]*\)\s*$", "", value or "")
|
||||||
|
# Lowercase, replace hyphens/underscores with empty so they
|
||||||
|
# line up with the way the key is constructed.
|
||||||
|
lowered = value.lower().replace("-", "").replace("_", "")
|
||||||
|
# Keep only alphanumerics (which preserves CJK characters
|
||||||
|
# because \w in unicode mode includes them; using explicit
|
||||||
|
# alphanumerics is safer cross-platform).
|
||||||
|
return re.sub(r"[^0-9a-z\u00C0-\uFFFF]", "", lowered)
|
||||||
|
|
||||||
|
target = _normalize(key)
|
||||||
|
if not target:
|
||||||
|
return None
|
||||||
|
|
||||||
|
candidates = []
|
||||||
|
try:
|
||||||
|
entries = _os.listdir(self._directory)
|
||||||
|
except OSError:
|
||||||
|
return None
|
||||||
|
|
||||||
|
for entry in entries:
|
||||||
|
full = _os.path.join(self._directory, entry)
|
||||||
|
if not _os.path.isdir(full):
|
||||||
|
continue
|
||||||
|
if _normalize(entry) == target:
|
||||||
|
candidates.append(entry)
|
||||||
|
|
||||||
|
if len(candidates) == 1:
|
||||||
|
return candidates[0]
|
||||||
|
if len(candidates) > 1:
|
||||||
|
logger.warning(
|
||||||
|
"Orphan folder recovery: ambiguous match for key=%s "
|
||||||
|
"found %d candidate folders: %s",
|
||||||
|
key, len(candidates), candidates,
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _build_delete_message(result) -> None:
|
||||||
|
"""Assemble the human-readable ``result.message`` from flags/errors."""
|
||||||
parts = []
|
parts = []
|
||||||
if result.deleted_from_database and not result.database_error:
|
if result.deleted_from_database and not result.database_error:
|
||||||
parts.append("removed from database")
|
parts.append("removed from database")
|
||||||
@@ -1805,12 +2107,6 @@ class AnimeService:
|
|||||||
else:
|
else:
|
||||||
result.message = "No action taken."
|
result.message = "No action taken."
|
||||||
|
|
||||||
logger.info(
|
|
||||||
"Delete series completed: key=%s deleted_db=%s deleted_folder=%s",
|
|
||||||
key, result.deleted_from_database, result.deleted_folder,
|
|
||||||
)
|
|
||||||
return result
|
|
||||||
|
|
||||||
async def _broadcast_series_deleted(self, key: str, name: str) -> None:
|
async def _broadcast_series_deleted(self, key: str, name: str) -> None:
|
||||||
"""Broadcast series_deleted event via WebSocket."""
|
"""Broadcast series_deleted event via WebSocket."""
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -119,6 +119,79 @@ class FolderNamingService:
|
|||||||
|
|
||||||
return await self._execute_rename(series, folder, target_folder)
|
return await self._execute_rename(series, folder, target_folder)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _merge_folder_into_target(source: str, target: str) -> dict:
|
||||||
|
"""Merge a source folder's contents into an existing target folder.
|
||||||
|
|
||||||
|
Walks the source tree and moves every file into the matching path under
|
||||||
|
the target. When a destination file already exists, the source copy is
|
||||||
|
removed (the target version wins; we don't keep duplicates). When the
|
||||||
|
source tree is fully consumed, the (now-empty) source directory is
|
||||||
|
removed.
|
||||||
|
|
||||||
|
Both paths must be absolute and ``target`` must already exist on disk.
|
||||||
|
|
||||||
|
Returns a summary dict with ``moved`` (file count), ``skipped`` (file
|
||||||
|
count where target already had a copy), and ``removed_source`` (bool).
|
||||||
|
Caller is responsible for any DB / cache updates that depend on the
|
||||||
|
outcome.
|
||||||
|
"""
|
||||||
|
if not os.path.isdir(source):
|
||||||
|
return {"moved": 0, "skipped": 0, "removed_source": False}
|
||||||
|
if not os.path.isdir(target):
|
||||||
|
raise ValueError(f"target does not exist: {target}")
|
||||||
|
|
||||||
|
moved = 0
|
||||||
|
skipped = 0
|
||||||
|
for root, _dirs, files in os.walk(source):
|
||||||
|
rel_root = os.path.relpath(root, source)
|
||||||
|
dest_root = (
|
||||||
|
target if rel_root == "."
|
||||||
|
else os.path.join(target, rel_root)
|
||||||
|
)
|
||||||
|
os.makedirs(dest_root, exist_ok=True)
|
||||||
|
for name in files:
|
||||||
|
src_file = os.path.join(root, name)
|
||||||
|
dest_file = os.path.join(dest_root, name)
|
||||||
|
if os.path.exists(dest_file):
|
||||||
|
# Target wins — never overwrite existing content. Remove
|
||||||
|
# the orphaned source copy so cleanup below can rmdir it.
|
||||||
|
try:
|
||||||
|
os.remove(src_file)
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(
|
||||||
|
"merge: could not remove duplicate %s: %s",
|
||||||
|
src_file, exc,
|
||||||
|
)
|
||||||
|
skipped += 1
|
||||||
|
logger.warning(
|
||||||
|
"merge: skipping %s (target already has %s)",
|
||||||
|
src_file, dest_file,
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
shutil.move(src_file, dest_file)
|
||||||
|
moved += 1
|
||||||
|
|
||||||
|
# Try to remove the (now empty) source tree. Walk bottom-up so leaf
|
||||||
|
# directories are removed before their parents.
|
||||||
|
removed_source = False
|
||||||
|
for root, dirs, files in os.walk(source, topdown=False):
|
||||||
|
for d in dirs:
|
||||||
|
try:
|
||||||
|
os.rmdir(os.path.join(root, d))
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
try:
|
||||||
|
os.rmdir(source)
|
||||||
|
removed_source = True
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(
|
||||||
|
"merge: could not remove source directory %s: %s",
|
||||||
|
source, exc,
|
||||||
|
)
|
||||||
|
|
||||||
|
return {"moved": moved, "skipped": skipped, "removed_source": removed_source}
|
||||||
|
|
||||||
async def _execute_rename(self, series, old_folder: str, target_folder: str) -> FolderRenameResult:
|
async def _execute_rename(self, series, old_folder: str, target_folder: str) -> FolderRenameResult:
|
||||||
key = series.key
|
key = series.key
|
||||||
|
|
||||||
@@ -132,16 +205,72 @@ class FolderNamingService:
|
|||||||
if not os.path.isdir(old_path):
|
if not os.path.isdir(old_path):
|
||||||
return FolderRenameResult(key=key, old_folder=old_folder, new_folder=None, success=False, skipped=False, reason="source folder does not exist on disk")
|
return FolderRenameResult(key=key, old_folder=old_folder, new_folder=None, success=False, skipped=False, reason="source folder does not exist on disk")
|
||||||
|
|
||||||
|
# If the target already exists, merge source into it instead of bailing.
|
||||||
|
# A bare folder ('Naruto') sitting next to the year-suffixed one
|
||||||
|
# ('Naruto (2019)') is how we get a series "added twice". Merging
|
||||||
|
# makes the rename succeed and removes the orphan folder.
|
||||||
if os.path.isdir(target_path):
|
if os.path.isdir(target_path):
|
||||||
return FolderRenameResult(key=key, old_folder=old_folder, new_folder=None, success=False, skipped=False, reason="target folder already exists on disk")
|
try:
|
||||||
|
summary = self._merge_folder_into_target(old_path, target_path)
|
||||||
|
except Exception as exc:
|
||||||
|
logger.error(
|
||||||
|
"Failed to merge %s -> %s for %s: %s",
|
||||||
|
old_folder, target_folder, key, exc,
|
||||||
|
)
|
||||||
|
return FolderRenameResult(
|
||||||
|
key=key, old_folder=old_folder, new_folder=None,
|
||||||
|
success=False, skipped=False,
|
||||||
|
reason=f"merge failed: {exc}",
|
||||||
|
)
|
||||||
|
logger.info(
|
||||||
|
"Merged folder %s -> %s for series %s (moved=%d skipped=%d removed_source=%s)",
|
||||||
|
old_folder, target_folder, key,
|
||||||
|
summary["moved"], summary["skipped"], summary["removed_source"],
|
||||||
|
)
|
||||||
|
|
||||||
|
# Update in-memory cache (best-effort)
|
||||||
|
try:
|
||||||
|
from src.server.utils.dependencies import get_series_app
|
||||||
|
series_app = get_series_app()
|
||||||
|
if key in series_app.list.keyDict:
|
||||||
|
series_app.list.keyDict[key].folder = target_folder
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning("Failed to update in-memory cache for %s: %s", key, exc)
|
||||||
|
|
||||||
|
# Update database
|
||||||
|
async with _get_db_session() as db:
|
||||||
|
db_series = await AnimeSeriesService.get_by_key(db, key)
|
||||||
|
if db_series:
|
||||||
|
await AnimeSeriesService.update(db, series_id=db_series.id, folder=target_folder)
|
||||||
|
logger.debug("Updated DB folder for %s to %s", key, target_folder)
|
||||||
|
|
||||||
|
# If source couldn't be removed (still had unexpected files) the
|
||||||
|
# state is worse than the original orphan, so surface that as a
|
||||||
|
# warning in the result while still reporting success.
|
||||||
|
note = None
|
||||||
|
if not summary["removed_source"]:
|
||||||
|
note = (
|
||||||
|
f"merged (moved={summary['moved']}, skipped={summary['skipped']}) "
|
||||||
|
f"but source folder could not be removed"
|
||||||
|
)
|
||||||
|
elif summary["skipped"]:
|
||||||
|
note = (
|
||||||
|
f"merged (moved={summary['moved']}, "
|
||||||
|
f"kept target copies for {summary['skipped']} file(s))"
|
||||||
|
)
|
||||||
|
return FolderRenameResult(
|
||||||
|
key=key, old_folder=old_folder, new_folder=target_folder,
|
||||||
|
success=True, skipped=False, reason=note,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Target doesn't exist — plain rename.
|
||||||
try:
|
try:
|
||||||
shutil.move(old_path, target_path)
|
shutil.move(old_path, target_path)
|
||||||
logger.info("Renamed folder %s -> %s for series %s", old_folder, target_folder, key)
|
logger.info("Renamed folder %s -> %s for series %s", old_folder, target_folder, key)
|
||||||
|
|
||||||
# Update in-memory cache
|
# Update in-memory cache
|
||||||
try:
|
try:
|
||||||
from src.server.SeriesApp import get_series_app
|
from src.server.utils.dependencies import get_series_app
|
||||||
series_app = get_series_app()
|
series_app = get_series_app()
|
||||||
if key in series_app.list.keyDict:
|
if key in series_app.list.keyDict:
|
||||||
series_app.list.keyDict[key].folder = target_folder
|
series_app.list.keyDict[key].folder = target_folder
|
||||||
|
|||||||
@@ -117,9 +117,23 @@ def is_safe_path(base_path: str, target_path: str) -> bool:
|
|||||||
Prevents path traversal attacks by ensuring the target path
|
Prevents path traversal attacks by ensuring the target path
|
||||||
is actually within the base path after resolution.
|
is actually within the base path after resolution.
|
||||||
|
|
||||||
|
Note on relative paths: a relative ``target_path`` is interpreted
|
||||||
|
as relative to ``base_path``, *not* to the process's current
|
||||||
|
working directory. This mirrors how callers use this helper:
|
||||||
|
they pass a configured base directory and a folder name stored
|
||||||
|
alongside it (e.g. the series ``folder`` column in the database
|
||||||
|
holds a relative name like ``"Beyblade Burst (2016)"``, and the
|
||||||
|
anime directory is configured separately). Without this, a
|
||||||
|
relative target would be resolved against the process CWD —
|
||||||
|
which can differ from ``base_path`` (the FastAPI app runs with
|
||||||
|
CWD=/app while the anime directory is mounted at /data), and
|
||||||
|
the helper would incorrectly reject the path as outside the
|
||||||
|
base. Absolute ``target_path`` values are validated against
|
||||||
|
``base_path`` directly.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
base_path: The base directory that should contain the target
|
base_path: The base directory that should contain the target
|
||||||
target_path: The path to validate
|
target_path: The path to validate (absolute, or relative to base_path)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
bool: True if target_path is safely within base_path
|
bool: True if target_path is safely within base_path
|
||||||
@@ -127,18 +141,26 @@ def is_safe_path(base_path: str, target_path: str) -> bool:
|
|||||||
Example:
|
Example:
|
||||||
>>> is_safe_path("/anime", "/anime/Attack on Titan")
|
>>> is_safe_path("/anime", "/anime/Attack on Titan")
|
||||||
True
|
True
|
||||||
|
>>> is_safe_path("/anime", "Attack on Titan") # relative -> /anime/Attack on Titan
|
||||||
|
True
|
||||||
>>> is_safe_path("/anime", "/anime/../etc/passwd")
|
>>> is_safe_path("/anime", "/anime/../etc/passwd")
|
||||||
False
|
False
|
||||||
"""
|
"""
|
||||||
# Resolve to absolute paths
|
# Resolve base to an absolute path
|
||||||
base_resolved = os.path.abspath(base_path)
|
base_resolved = os.path.abspath(base_path)
|
||||||
|
|
||||||
|
# Resolve target relative to the base (not the process CWD) when it is
|
||||||
|
# supplied as a relative path. Absolute targets are validated as-is.
|
||||||
|
if os.path.isabs(target_path):
|
||||||
target_resolved = os.path.abspath(target_path)
|
target_resolved = os.path.abspath(target_path)
|
||||||
|
else:
|
||||||
|
target_resolved = os.path.abspath(os.path.join(base_resolved, target_path))
|
||||||
|
|
||||||
# Check that target starts with base (with trailing separator)
|
# Check that target starts with base (with trailing separator)
|
||||||
base_with_sep = base_resolved + os.sep
|
base_with_sep = base_resolved + os.sep
|
||||||
return (
|
return (
|
||||||
target_resolved == base_resolved or
|
target_resolved == base_resolved
|
||||||
target_resolved.startswith(base_with_sep)
|
or target_resolved.startswith(base_with_sep)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Unit tests for AnimeService.delete_series()."""
|
"""Unit tests for AnimeService.delete_series()."""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
@@ -328,6 +329,69 @@ class TestDeleteSeriesService:
|
|||||||
assert result.folder_error is not None
|
assert result.folder_error is not None
|
||||||
assert "outside" in result.folder_error.lower()
|
assert "outside" in result.folder_error.lower()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_series_relative_folder_with_different_cwd(
|
||||||
|
self, anime_service, tmp_path
|
||||||
|
):
|
||||||
|
"""Regression: delete_series must work when the stored folder is relative
|
||||||
|
and the process CWD differs from directory_to_search.
|
||||||
|
|
||||||
|
In the container the FastAPI app runs with CWD=/app while the anime
|
||||||
|
directory is /data. The DB stores the relative folder name (e.g.
|
||||||
|
"Beyblade Burst (2016)"). The old code called
|
||||||
|
``os.path.abspath(folder)`` which joined against CWD=/app and
|
||||||
|
produced "/app/Beyblade Burst (2016)", which was then rejected as
|
||||||
|
outside the /data base. The fix resolves relative paths against
|
||||||
|
the configured anime directory instead.
|
||||||
|
"""
|
||||||
|
safe_base = tmp_path / "data"
|
||||||
|
safe_base.mkdir()
|
||||||
|
series_folder = safe_base / "Beyblade Burst (2016)"
|
||||||
|
series_folder.mkdir()
|
||||||
|
|
||||||
|
anime_service._directory = str(safe_base)
|
||||||
|
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_ctx = _make_db_ctx(mock_session)
|
||||||
|
|
||||||
|
# Stored folder is RELATIVE (matches what's actually in the DB)
|
||||||
|
mock_series = MagicMock()
|
||||||
|
mock_series.key = "beyblade-burst"
|
||||||
|
mock_series.name = "Beyblade Burst"
|
||||||
|
mock_series.folder = "Beyblade Burst (2016)"
|
||||||
|
mock_series.id = 336
|
||||||
|
|
||||||
|
# Simulate process CWD differing from anime dir (container case:
|
||||||
|
# CWD=/app while anime dir is /data). Use "/" as a stable, always-
|
||||||
|
# existing CWD distinct from tmp_path.
|
||||||
|
old_cwd = os.getcwd()
|
||||||
|
try:
|
||||||
|
os.chdir("/")
|
||||||
|
with patch(
|
||||||
|
"src.server.database.connection.get_db_session",
|
||||||
|
return_value=mock_ctx,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=mock_series,
|
||||||
|
):
|
||||||
|
result = await anime_service.delete_series(
|
||||||
|
key="beyblade-burst",
|
||||||
|
delete_database=False,
|
||||||
|
delete_folder=True,
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
os.chdir(old_cwd)
|
||||||
|
|
||||||
|
# Folder MUST be deleted successfully
|
||||||
|
assert result.deleted_folder is True, (
|
||||||
|
f"folder delete failed: success={result.success} "
|
||||||
|
f"folder_error={result.folder_error!r}"
|
||||||
|
)
|
||||||
|
assert result.folder_error is None
|
||||||
|
assert result.success is True
|
||||||
|
assert not series_folder.exists()
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
# Error handling
|
# Error handling
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
@@ -517,3 +581,190 @@ class TestDeleteSeriesService:
|
|||||||
key="test-key",
|
key="test-key",
|
||||||
name="Test Series",
|
name="Test Series",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Deletion order: filesystem first, database second
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_series_db_preserved_when_folder_fails(
|
||||||
|
self, anime_service, tmp_path
|
||||||
|
):
|
||||||
|
"""When both flags are True and folder delete fails, the DB row
|
||||||
|
is preserved so the user can retry after fixing the underlying issue.
|
||||||
|
|
||||||
|
Previously, the database row was deleted first and the folder
|
||||||
|
second. If the folder delete failed (e.g. the old CWD-relative-path
|
||||||
|
bug, or any future permission/path error), the row was already
|
||||||
|
gone — leaving an orphan folder on disk that could not be cleaned
|
||||||
|
up through the normal delete flow.
|
||||||
|
"""
|
||||||
|
# Folder exists but we'll force shutil.rmtree to fail
|
||||||
|
series_folder = tmp_path / "Test Series"
|
||||||
|
series_folder.mkdir()
|
||||||
|
anime_service._directory = str(tmp_path)
|
||||||
|
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_ctx = _make_db_ctx(mock_session)
|
||||||
|
|
||||||
|
mock_series = MagicMock()
|
||||||
|
mock_series.key = "test-key"
|
||||||
|
mock_series.name = "Test Series"
|
||||||
|
mock_series.folder = "Test Series"
|
||||||
|
mock_series.id = 7
|
||||||
|
|
||||||
|
db_delete_mock = AsyncMock(return_value=True)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"src.server.database.connection.get_db_session",
|
||||||
|
return_value=mock_ctx,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=mock_series,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.delete",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
side_effect=db_delete_mock,
|
||||||
|
), patch(
|
||||||
|
"shutil.rmtree",
|
||||||
|
side_effect=OSError("Permission denied"),
|
||||||
|
):
|
||||||
|
result = await anime_service.delete_series(
|
||||||
|
key="test-key",
|
||||||
|
delete_database=True,
|
||||||
|
delete_folder=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Folder delete failed → DB row MUST be preserved
|
||||||
|
assert result.deleted_folder is False
|
||||||
|
assert result.folder_error is not None
|
||||||
|
assert result.deleted_from_database is False, (
|
||||||
|
"DB row was deleted despite folder delete failure — "
|
||||||
|
"user would lose ability to retry the delete"
|
||||||
|
)
|
||||||
|
db_delete_mock.assert_not_called()
|
||||||
|
assert result.success is False
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Orphan folder recovery (DB row gone, folder still on disk)
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_series_orphan_folder_recovery(self, anime_service, tmp_path):
|
||||||
|
"""If the DB row is gone but a matching folder is still on disk,
|
||||||
|
``delete_folder=True`` removes the orphan folder.
|
||||||
|
|
||||||
|
Reproduces the Beyblade Burst scenario: previous delete attempt
|
||||||
|
removed the DB row (delete_database=True) but the folder delete
|
||||||
|
silently failed (path math bug). Retrying with delete_folder=True
|
||||||
|
should clean up the orphan via a key-based folder scan.
|
||||||
|
"""
|
||||||
|
# Simulate the on-disk anime directory
|
||||||
|
anime_dir = tmp_path / "anime"
|
||||||
|
anime_dir.mkdir()
|
||||||
|
orphan = anime_dir / "Beyblade Burst (2016)"
|
||||||
|
orphan.mkdir()
|
||||||
|
(orphan / "episode.mp4").write_text("x")
|
||||||
|
|
||||||
|
anime_service._directory = str(anime_dir)
|
||||||
|
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_ctx = _make_db_ctx(mock_session)
|
||||||
|
|
||||||
|
# DB lookup returns None (row already gone)
|
||||||
|
with patch(
|
||||||
|
"src.server.database.connection.get_db_session",
|
||||||
|
return_value=mock_ctx,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=None,
|
||||||
|
):
|
||||||
|
result = await anime_service.delete_series(
|
||||||
|
key="beyblade-burst",
|
||||||
|
delete_database=False,
|
||||||
|
delete_folder=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Orphan folder recovered via key-match scan
|
||||||
|
assert result.deleted_folder is True, (
|
||||||
|
f"orphan recovery failed: success={result.success} "
|
||||||
|
f"folder_error={result.folder_error!r}"
|
||||||
|
)
|
||||||
|
assert result.folder_error is None
|
||||||
|
assert not orphan.exists()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_series_orphan_folder_no_match(
|
||||||
|
self, anime_service, tmp_path
|
||||||
|
):
|
||||||
|
"""If DB row is gone and no folder matches the key, return a
|
||||||
|
clear error rather than silently succeeding.
|
||||||
|
"""
|
||||||
|
anime_dir = tmp_path / "anime"
|
||||||
|
anime_dir.mkdir()
|
||||||
|
# Some unrelated folder that does NOT match
|
||||||
|
(anime_dir / "Different Show (2020)").mkdir()
|
||||||
|
anime_service._directory = str(anime_dir)
|
||||||
|
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_ctx = _make_db_ctx(mock_session)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"src.server.database.connection.get_db_session",
|
||||||
|
return_value=mock_ctx,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=None,
|
||||||
|
):
|
||||||
|
result = await anime_service.delete_series(
|
||||||
|
key="nonexistent-key",
|
||||||
|
delete_database=False,
|
||||||
|
delete_folder=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.deleted_folder is False
|
||||||
|
assert result.folder_error is not None
|
||||||
|
assert "no folder matching" in result.folder_error.lower()
|
||||||
|
# Unrelated folder untouched
|
||||||
|
assert (anime_dir / "Different Show (2020)").exists()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delete_series_orphan_folder_ambiguous(
|
||||||
|
self, anime_service, tmp_path
|
||||||
|
):
|
||||||
|
"""If multiple folders match the same normalized key, refuse to
|
||||||
|
delete any of them (safe default).
|
||||||
|
"""
|
||||||
|
anime_dir = tmp_path / "anime"
|
||||||
|
anime_dir.mkdir()
|
||||||
|
(anime_dir / "Beyblade Burst (2016)").mkdir()
|
||||||
|
(anime_dir / "Beyblade Burst (2019)").mkdir() # also normalizes to "beybladeburst"
|
||||||
|
anime_service._directory = str(anime_dir)
|
||||||
|
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_ctx = _make_db_ctx(mock_session)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"src.server.database.connection.get_db_session",
|
||||||
|
return_value=mock_ctx,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=None,
|
||||||
|
):
|
||||||
|
result = await anime_service.delete_series(
|
||||||
|
key="beyblade-burst",
|
||||||
|
delete_database=False,
|
||||||
|
delete_folder=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Ambiguous → refuse
|
||||||
|
assert result.deleted_folder is False
|
||||||
|
assert result.folder_error is not None
|
||||||
|
assert "no folder matching" in result.folder_error.lower()
|
||||||
|
# Neither folder deleted
|
||||||
|
assert (anime_dir / "Beyblade Burst (2016)").exists()
|
||||||
|
assert (anime_dir / "Beyblade Burst (2019)").exists()
|
||||||
|
|||||||
@@ -213,6 +213,34 @@ class TestIsSafePath:
|
|||||||
"/anime/Attack on Titan/Season 1/Episode 1"
|
"/anime/Attack on Titan/Season 1/Episode 1"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_relative_target_resolved_against_base(self):
|
||||||
|
"""Relative targets resolve against the base, not the process CWD.
|
||||||
|
|
||||||
|
Regression test: previously `os.path.abspath(target_path)` would
|
||||||
|
join a relative target against the process's current working
|
||||||
|
directory. When the CWD differed from `base_path` (e.g. the
|
||||||
|
FastAPI app running with CWD=/app while the anime directory is
|
||||||
|
/data), a relative folder name like "Beyblade Burst (2016)"
|
||||||
|
would be resolved to "/app/Beyblade Burst (2016)" and
|
||||||
|
incorrectly rejected as outside the base. The helper now
|
||||||
|
treats a relative target as relative to `base_path`.
|
||||||
|
"""
|
||||||
|
with tempfile.TemporaryDirectory() as tmpdir:
|
||||||
|
base = os.path.abspath(tmpdir)
|
||||||
|
# Simulate a process CWD different from base
|
||||||
|
old_cwd = os.getcwd()
|
||||||
|
try:
|
||||||
|
os.chdir("/")
|
||||||
|
# Relative target inside base should be safe
|
||||||
|
assert is_safe_path(base, "Beyblade Burst (2016)")
|
||||||
|
# Nested relative target should also be safe
|
||||||
|
assert is_safe_path(base, "Beyblade Burst (2016)/Season 1")
|
||||||
|
# Relative traversal (../) must still be rejected even
|
||||||
|
# when resolved against the base
|
||||||
|
assert not is_safe_path(base, "../etc/passwd")
|
||||||
|
finally:
|
||||||
|
os.chdir(old_cwd)
|
||||||
|
|
||||||
|
|
||||||
class TestCreateSafeFolder:
|
class TestCreateSafeFolder:
|
||||||
"""Test create_safe_folder function."""
|
"""Test create_safe_folder function."""
|
||||||
|
|||||||
@@ -241,28 +241,157 @@ class TestFolderNamingServiceIntegration:
|
|||||||
assert call_kwargs["folder"] == "Naruto (1999)"
|
assert call_kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_skips_when_target_folder_already_exists(
|
async def test_merges_source_into_existing_target_when_target_has_no_overlap(
|
||||||
self, tmp_path, mock_db_session, mock_series, mock_settings
|
self, tmp_path, mock_db_session, mock_series, mock_settings
|
||||||
):
|
):
|
||||||
"""If 'Naruto (1999)' already exists, rename is skipped."""
|
"""Source 'Naruto' with seasons/episodes, target 'Naruto (1999)' exists empty.
|
||||||
|
|
||||||
|
Source files are moved into target. Empty source directory is removed.
|
||||||
|
DB folder is updated to target. Result is success (renamed).
|
||||||
|
"""
|
||||||
anime_dir = tmp_path
|
anime_dir = tmp_path
|
||||||
(anime_dir / "Naruto").mkdir()
|
source = anime_dir / "Naruto"
|
||||||
(anime_dir / "Naruto (1999)").mkdir() # target already exists
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
(source / "Season 1").mkdir()
|
||||||
|
(source / "Season 1" / "ep01.mp4").touch()
|
||||||
|
(source / "Season 2").mkdir()
|
||||||
|
(source / "Season 2" / "ep01.mp4").touch()
|
||||||
|
|
||||||
series = mock_series("key1", "Naruto", 1999)
|
series = mock_series("key1", "Naruto", 1999)
|
||||||
mock_db_session.__aenter__.return_value.__aexit__.return_value = None
|
mock_db_session.__aenter__.return_value.__aexit__.return_value = None
|
||||||
|
|
||||||
with patch("src.server.services.folder_naming_service.AnimeSeriesService.get_all", new_callable=AsyncMock) as mock_get_all:
|
with patch("src.server.services.folder_naming_service.AnimeSeriesService.get_all", new_callable=AsyncMock) as mock_get_all, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.get_by_key", new_callable=AsyncMock) as mock_get_by_key, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.update", new_callable=AsyncMock) as mock_update, \
|
||||||
|
patch("src.server.utils.dependencies.get_series_app") as mock_get_app:
|
||||||
|
|
||||||
mock_get_all.return_value = [series]
|
mock_get_all.return_value = [series]
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 42
|
||||||
|
mock_get_by_key.return_value = db_series
|
||||||
|
|
||||||
|
app_instance = MagicMock()
|
||||||
|
app_instance.list.keyDict = {"key1": MagicMock()}
|
||||||
|
mock_get_app.return_value = app_instance
|
||||||
|
|
||||||
mock_settings.anime_directory = str(anime_dir)
|
mock_settings.anime_directory = str(anime_dir)
|
||||||
|
|
||||||
service = FolderNamingService()
|
service = FolderNamingService()
|
||||||
report = await service.run()
|
report = await service.run()
|
||||||
|
|
||||||
assert report.errors == 1
|
# Outcome: renamed (not error) — source merged into target
|
||||||
assert report.renamed == 0
|
assert report.renamed == 1
|
||||||
assert report.results[0].reason == "target folder already exists on disk"
|
assert report.errors == 0
|
||||||
assert (anime_dir / "Naruto").exists() # source not moved
|
assert report.results[0].success is True
|
||||||
|
assert report.results[0].skipped is False
|
||||||
|
assert report.results[0].new_folder == "Naruto (1999)"
|
||||||
|
|
||||||
|
# Source folder gone
|
||||||
|
assert not source.exists(), "Source folder should be removed after merge"
|
||||||
|
# Target folder has merged content
|
||||||
|
assert (target / "Season 1" / "ep01.mp4").exists()
|
||||||
|
assert (target / "Season 2" / "ep01.mp4").exists()
|
||||||
|
# DB updated
|
||||||
|
assert mock_update.call_args.kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_merges_only_missing_seasons_preserving_existing_target_files(
|
||||||
|
self, tmp_path, mock_db_session, mock_series, mock_settings
|
||||||
|
):
|
||||||
|
"""Source has S01 ep01, target already has S01 ep01 (different content).
|
||||||
|
|
||||||
|
Existing target files are kept. Source's S01 ep01 is NOT overwritten.
|
||||||
|
Source's S02 (new) is moved. Empty source is removed.
|
||||||
|
"""
|
||||||
|
anime_dir = tmp_path
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
|
||||||
|
# Target already has S01 with one episode
|
||||||
|
(target / "Season 1").mkdir()
|
||||||
|
target_existing = target / "Season 1" / "ep01.mp4"
|
||||||
|
target_existing.write_text("target-version")
|
||||||
|
|
||||||
|
# Source has S01 with same episode (different content) and S02
|
||||||
|
(source / "Season 1").mkdir()
|
||||||
|
source_conflict = source / "Season 1" / "ep01.mp4"
|
||||||
|
source_conflict.write_text("source-version")
|
||||||
|
(source / "Season 2").mkdir()
|
||||||
|
(source / "Season 2" / "ep01.mp4").touch()
|
||||||
|
|
||||||
|
series = mock_series("key1", "Naruto", 1999)
|
||||||
|
mock_db_session.__aenter__.return_value.__aexit__.return_value = None
|
||||||
|
|
||||||
|
with patch("src.server.services.folder_naming_service.AnimeSeriesService.get_all", new_callable=AsyncMock) as mock_get_all, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.get_by_key", new_callable=AsyncMock) as mock_get_by_key, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.update", new_callable=AsyncMock), \
|
||||||
|
patch("src.server.utils.dependencies.get_series_app") as mock_get_app:
|
||||||
|
|
||||||
|
mock_get_all.return_value = [series]
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 42
|
||||||
|
mock_get_by_key.return_value = db_series
|
||||||
|
|
||||||
|
app_instance = MagicMock()
|
||||||
|
app_instance.list.keyDict = {"key1": MagicMock()}
|
||||||
|
mock_get_app.return_value = app_instance
|
||||||
|
mock_settings.anime_directory = str(anime_dir)
|
||||||
|
|
||||||
|
service = FolderNamingService()
|
||||||
|
report = await service.run()
|
||||||
|
|
||||||
|
# Renamed (source effectively absorbed)
|
||||||
|
assert report.renamed == 1
|
||||||
|
assert not source.exists(), "Source should be removed after merge"
|
||||||
|
# Target S01 ep01 keeps the target version (not overwritten)
|
||||||
|
assert target_existing.read_text() == "target-version"
|
||||||
|
# New S02 moved in
|
||||||
|
assert (target / "Season 2" / "ep01.mp4").exists()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_removes_empty_source_folder_when_target_exists(
|
||||||
|
self, tmp_path, mock_db_session, mock_series, mock_settings
|
||||||
|
):
|
||||||
|
"""Source folder exists but is empty, target already exists.
|
||||||
|
|
||||||
|
Source should be removed silently, DB updated, success.
|
||||||
|
"""
|
||||||
|
anime_dir = tmp_path
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
(target / "tvshow.nfo").write_text("existing nfo")
|
||||||
|
|
||||||
|
series = mock_series("key1", "Naruto", 1999)
|
||||||
|
mock_db_session.__aenter__.return_value.__aexit__.return_value = None
|
||||||
|
|
||||||
|
with patch("src.server.services.folder_naming_service.AnimeSeriesService.get_all", new_callable=AsyncMock) as mock_get_all, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.get_by_key", new_callable=AsyncMock) as mock_get_by_key, \
|
||||||
|
patch("src.server.services.folder_naming_service.AnimeSeriesService.update", new_callable=AsyncMock) as mock_update, \
|
||||||
|
patch("src.server.utils.dependencies.get_series_app") as mock_get_app:
|
||||||
|
|
||||||
|
mock_get_all.return_value = [series]
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 42
|
||||||
|
mock_get_by_key.return_value = db_series
|
||||||
|
app_instance = MagicMock()
|
||||||
|
app_instance.list.keyDict = {"key1": MagicMock()}
|
||||||
|
mock_get_app.return_value = app_instance
|
||||||
|
mock_settings.anime_directory = str(anime_dir)
|
||||||
|
|
||||||
|
service = FolderNamingService()
|
||||||
|
report = await service.run()
|
||||||
|
|
||||||
|
assert report.renamed == 1
|
||||||
|
assert report.errors == 0
|
||||||
|
assert not source.exists(), "Empty source should be removed"
|
||||||
|
assert (target / "tvshow.nfo").exists(), "Target content preserved"
|
||||||
|
assert mock_update.call_args.kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_safety_guard_detects_wrong_year_in_target(self, tmp_path, mock_db_session, mock_series, mock_settings):
|
async def test_safety_guard_detects_wrong_year_in_target(self, tmp_path, mock_db_session, mock_series, mock_settings):
|
||||||
|
|||||||
242
tests/unit/test_rename_folder_if_needed.py
Normal file
242
tests/unit/test_rename_folder_if_needed.py
Normal file
@@ -0,0 +1,242 @@
|
|||||||
|
"""Tests for AnimeService.rename_folder_if_needed.
|
||||||
|
|
||||||
|
The behavior under test: when both the source folder (without year) and the
|
||||||
|
target folder (with year) exist on disk, the rename must not silently bail
|
||||||
|
out — it must merge the source into the target and remove the empty source.
|
||||||
|
This is what prevents the "Ultraman" + "Ultraman (2019)" duplicate-folder
|
||||||
|
problem reported by users.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from src.server.services.anime_service import AnimeService
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def anime_service_with_dir(tmp_path):
|
||||||
|
"""Create AnimeService pointing at a temp directory."""
|
||||||
|
mock_app = MagicMock()
|
||||||
|
mock_app.directory_to_search = str(tmp_path)
|
||||||
|
mock_app.list.keyDict = {}
|
||||||
|
progress = MagicMock()
|
||||||
|
service = AnimeService(series_app=mock_app, progress_service=progress)
|
||||||
|
return service, tmp_path
|
||||||
|
|
||||||
|
|
||||||
|
class TestRenameFolderIfNeededMerge:
|
||||||
|
"""Tests for the merge-into-existing-target behavior."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_merges_seasons_when_target_exists(
|
||||||
|
self, anime_service_with_dir
|
||||||
|
):
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
(source / "Season 1").mkdir()
|
||||||
|
(source / "Season 1" / "ep01.mp4").touch()
|
||||||
|
target.mkdir()
|
||||||
|
|
||||||
|
db = AsyncMock()
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 1
|
||||||
|
db_series.folder = "Naruto"
|
||||||
|
with patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=db_series,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.update",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
) as mock_update:
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=db,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Outcome: rename "succeeded" (target now contains source content)
|
||||||
|
assert ok is True
|
||||||
|
assert not source.exists(), "Source should be removed after merge"
|
||||||
|
assert (target / "Season 1" / "ep01.mp4").exists()
|
||||||
|
# DB row updated to target
|
||||||
|
assert mock_update.call_args.kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_does_not_overwrite_existing_target_files(
|
||||||
|
self, anime_service_with_dir
|
||||||
|
):
|
||||||
|
"""If target already has an episode file, the source copy is removed
|
||||||
|
(target version wins; no duplicate retained).
|
||||||
|
"""
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
(target / "Season 1").mkdir()
|
||||||
|
target_existing = target / "Season 1" / "ep01.mp4"
|
||||||
|
target_existing.write_text("target-version")
|
||||||
|
(source / "Season 1").mkdir()
|
||||||
|
source_conflict = source / "Season 1" / "ep01.mp4"
|
||||||
|
source_conflict.write_text("source-version")
|
||||||
|
|
||||||
|
db = AsyncMock()
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 1
|
||||||
|
with patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=db_series,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.update",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
):
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=db,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is True
|
||||||
|
# Target version preserved
|
||||||
|
assert target_existing.read_text() == "target-version"
|
||||||
|
# Source folder removed (after merge, even with skipped conflicts)
|
||||||
|
assert not source.exists()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_removes_empty_source_when_target_exists(
|
||||||
|
self, anime_service_with_dir
|
||||||
|
):
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
target = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
(target / "tvshow.nfo").write_text("kept")
|
||||||
|
|
||||||
|
db = AsyncMock()
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 1
|
||||||
|
with patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=db_series,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.update",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
) as mock_update:
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=db,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is True
|
||||||
|
assert not source.exists()
|
||||||
|
assert (target / "tvshow.nfo").read_text() == "kept"
|
||||||
|
assert mock_update.call_args.kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_simple_rename_when_target_does_not_exist(
|
||||||
|
self, anime_service_with_dir
|
||||||
|
):
|
||||||
|
"""Regression: plain rename (no merge needed) still works."""
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
source = anime_dir / "Naruto"
|
||||||
|
source.mkdir()
|
||||||
|
(source / "Season 1").mkdir()
|
||||||
|
(source / "Season 1" / "ep01.mp4").touch()
|
||||||
|
|
||||||
|
db = AsyncMock()
|
||||||
|
db_series = MagicMock()
|
||||||
|
db_series.id = 1
|
||||||
|
with patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.get_by_key",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
return_value=db_series,
|
||||||
|
), patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService.update",
|
||||||
|
new_callable=AsyncMock,
|
||||||
|
) as mock_update:
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=db,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is True
|
||||||
|
assert not source.exists()
|
||||||
|
assert (anime_dir / "Naruto (1999)" / "Season 1" / "ep01.mp4").exists()
|
||||||
|
assert mock_update.call_args.kwargs["folder"] == "Naruto (1999)"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_no_op_when_source_and_target_same(
|
||||||
|
self, anime_service_with_dir
|
||||||
|
):
|
||||||
|
"""Regression: same-name case returns False without touching disk."""
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
source = anime_dir / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto (1999)",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is False
|
||||||
|
assert source.exists()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_no_op_when_source_missing(self, anime_service_with_dir):
|
||||||
|
"""Regression: source missing on disk returns False without error."""
|
||||||
|
service, anime_dir = anime_service_with_dir
|
||||||
|
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ok is False
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_path_typesafe_with_string_directory(self, tmp_path):
|
||||||
|
"""Regression: directory_to_search being a string (not Path) works.
|
||||||
|
|
||||||
|
Original code did `self._directory / current_folder` which raised
|
||||||
|
TypeError when _directory was a str. This was silently swallowed
|
||||||
|
by the caller's try/except, leaving the rename undone.
|
||||||
|
"""
|
||||||
|
mock_app = MagicMock()
|
||||||
|
mock_app.directory_to_search = str(tmp_path) # string, not Path
|
||||||
|
mock_app.list.keyDict = {}
|
||||||
|
progress = MagicMock()
|
||||||
|
service = AnimeService(series_app=mock_app, progress_service=progress)
|
||||||
|
|
||||||
|
source = tmp_path / "Naruto"
|
||||||
|
target = tmp_path / "Naruto (1999)"
|
||||||
|
source.mkdir()
|
||||||
|
target.mkdir()
|
||||||
|
|
||||||
|
ok = await service.rename_folder_if_needed(
|
||||||
|
key="naruto",
|
||||||
|
current_folder="Naruto",
|
||||||
|
target_folder="Naruto (1999)",
|
||||||
|
db=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Must not raise; must succeed (merge path).
|
||||||
|
assert ok is True
|
||||||
|
assert not source.exists()
|
||||||
Reference in New Issue
Block a user