Refactor: Defer folder creation to download time
- Remove folder creation from add_series endpoint - Add folder creation to download() method in SeriesApp - Maintain database persistence and targeted scanning - Update tests to use tmp_path fixtures - All add_series and download tests passing (13/13)
This commit is contained in:
parent
3d2ef53463
commit
5c0a019e72
@ -108,34 +108,47 @@ For each task completed:
|
||||
|
||||
## TODO List:
|
||||
|
||||
### Completed Tasks:
|
||||
### Task: Refactor Series Addition and Folder Creation Logic
|
||||
|
||||
1. **✅ Fixed copy issue to folder /mnt/server/serien/Serien/** (Completed: 2026-01-09)
|
||||
**Priority:** High
|
||||
**Status:** ✅ Complete
|
||||
|
||||
**Issue**: PermissionError when copying downloaded files to target directory
|
||||
#### Overview
|
||||
|
||||
```
|
||||
PermissionError: [Errno 13] Permission denied: '/mnt/server/serien/Serien/Gachiakuta (2025)/Season 1/Gachiakuta - S01E023 - (German Dub).mp4'
|
||||
```
|
||||
Refactored the series addition workflow to defer folder creation until download time and ensure proper series scanning on addition. This improves the separation of concerns and ensures a cleaner workflow.
|
||||
|
||||
**Root Cause**:
|
||||
#### Completed Changes
|
||||
|
||||
- `shutil.copy2()` and `shutil.copy()` attempt to preserve file metadata (permissions, timestamps, ownership)
|
||||
- Preserving metadata requires special permissions on the target directory
|
||||
- The mounted network directory `/mnt/server/serien/Serien/` has restricted metadata permissions
|
||||
1. **Folder Creation Removed from Series Addition**
|
||||
- Modified [src/server/api/anime.py](../src/server/api/anime.py) to remove folder creation on series add
|
||||
- Series are now only added to the database and in-memory structures
|
||||
- Folder creation is deferred to download time
|
||||
|
||||
**Solution**:
|
||||
2. **Folder Creation Added to Download Start**
|
||||
- Updated [src/core/SeriesApp.py](../src/core/SeriesApp.py) `download()` method
|
||||
- Added folder existence check before download
|
||||
- Creates folder if it doesn't exist using the series folder name
|
||||
- Includes proper error handling and logging
|
||||
|
||||
- Replaced `shutil.copy2()` with `shutil.copyfile()` in [enhanced_provider.py](../src/core/providers/enhanced_provider.py#L558)
|
||||
- Replaced `shutil.copy()` with `shutil.copyfile()` in [aniworld_provider.py](../src/core/providers/aniworld_provider.py#L329)
|
||||
- `shutil.copyfile()` only copies file content without attempting to preserve metadata
|
||||
3. **Database Persistence Maintained**
|
||||
- Series are still properly saved to the database on addition
|
||||
- No regression in database entry creation
|
||||
|
||||
**Verification**:
|
||||
4. **Targeted Scanning Works**
|
||||
- Scan logic continues to work correctly
|
||||
- Only the added series is scanned (not full library rescan)
|
||||
- Works correctly even when folder doesn't exist yet
|
||||
|
||||
- Created comprehensive tests confirming the fix works
|
||||
- Download process can now successfully copy files to `/mnt/server/serien/Serien/`
|
||||
- Both providers (aniworld and enhanced) updated
|
||||
#### Test Results
|
||||
|
||||
### Active Tasks:
|
||||
- All add_series endpoint tests passing (9/9)
|
||||
- All SeriesApp download tests passing (4/4)
|
||||
- Total: 1132 tests passing (up from 1123 before changes)
|
||||
- Remaining failures are unrelated to these changes (scan_service, download_service issues)
|
||||
|
||||
#### Note on Year Attribute
|
||||
|
||||
Year information is not currently available in the Serie class or search results. The folder naming currently uses just the series name without the year suffix. This can be enhanced in a future task when year metadata is added to the system.
|
||||
|
||||
---
|
||||
|
||||
_No active tasks at the moment._
|
||||
|
||||
@ -12,6 +12,7 @@ Note:
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import os
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
@ -317,6 +318,36 @@ class SeriesApp:
|
||||
)
|
||||
)
|
||||
|
||||
# Create series folder if it doesn't exist
|
||||
folder_path = os.path.join(self.directory_to_search, serie_folder)
|
||||
if not os.path.exists(folder_path):
|
||||
try:
|
||||
os.makedirs(folder_path, exist_ok=True)
|
||||
logger.info(
|
||||
"Created series folder: %s (key: %s)",
|
||||
folder_path,
|
||||
key
|
||||
)
|
||||
except OSError as e:
|
||||
logger.error(
|
||||
"Failed to create series folder %s: %s",
|
||||
folder_path,
|
||||
str(e)
|
||||
)
|
||||
# Fire download failed event
|
||||
self._events.download_status(
|
||||
DownloadStatusEventArgs(
|
||||
serie_folder=serie_folder,
|
||||
key=key,
|
||||
season=season,
|
||||
episode=episode,
|
||||
status="failed",
|
||||
message=f"Failed to create folder: {str(e)}",
|
||||
item_id=item_id,
|
||||
)
|
||||
)
|
||||
return False
|
||||
|
||||
try:
|
||||
def download_progress_handler(progress_info):
|
||||
"""Handle download progress events from loader."""
|
||||
|
||||
@ -739,8 +739,7 @@ async def add_series(
|
||||
db_id
|
||||
)
|
||||
|
||||
# Step D: Create folder on disk and add to SerieList
|
||||
folder_path = None
|
||||
# Step D: Add to SerieList (in-memory only, no folder creation)
|
||||
if series_app and hasattr(series_app, "list"):
|
||||
serie = Serie(
|
||||
key=key,
|
||||
@ -750,25 +749,15 @@ async def add_series(
|
||||
episodeDict={}
|
||||
)
|
||||
|
||||
# Add to SerieList - this creates the folder with sanitized name
|
||||
if hasattr(series_app.list, 'add'):
|
||||
with warnings.catch_warnings():
|
||||
warnings.simplefilter("ignore", DeprecationWarning)
|
||||
folder_path = series_app.list.add(serie, use_sanitized_folder=True)
|
||||
# Update folder to reflect what was actually created
|
||||
folder = serie.folder
|
||||
elif hasattr(series_app.list, 'keyDict'):
|
||||
# Manual folder creation and cache update
|
||||
if hasattr(series_app.list, 'directory'):
|
||||
folder_path = os.path.join(series_app.list.directory, folder)
|
||||
os.makedirs(folder_path, exist_ok=True)
|
||||
# Add to in-memory cache without creating folder on disk
|
||||
if hasattr(series_app.list, 'keyDict'):
|
||||
series_app.list.keyDict[key] = serie
|
||||
|
||||
logger.info(
|
||||
"Created folder for series: %s at %s",
|
||||
name,
|
||||
folder_path or folder
|
||||
)
|
||||
logger.info(
|
||||
"Added series to in-memory cache: %s (key=%s, folder=%s)",
|
||||
name,
|
||||
key,
|
||||
folder
|
||||
)
|
||||
|
||||
# Step E: Trigger targeted scan for missing episodes
|
||||
try:
|
||||
@ -818,7 +807,7 @@ async def add_series(
|
||||
"status": "success",
|
||||
"message": f"Successfully added series: {name}",
|
||||
"key": key,
|
||||
"folder": folder_path or folder,
|
||||
"folder": folder,
|
||||
"db_id": db_id,
|
||||
"missing_episodes": missing_episodes_serializable,
|
||||
"total_missing": total_missing
|
||||
|
||||
@ -107,10 +107,14 @@ class TestSeriesAppDownload:
|
||||
@patch('src.core.SeriesApp.SerieScanner')
|
||||
@patch('src.core.SeriesApp.SerieList')
|
||||
async def test_download_success(
|
||||
self, mock_serie_list, mock_scanner, mock_loaders
|
||||
self, mock_serie_list, mock_scanner, mock_loaders, tmp_path
|
||||
):
|
||||
"""Test successful download."""
|
||||
test_dir = "/test/anime"
|
||||
test_dir = str(tmp_path / "anime")
|
||||
# Create the test directory
|
||||
import os
|
||||
os.makedirs(test_dir, exist_ok=True)
|
||||
|
||||
app = SeriesApp(test_dir)
|
||||
|
||||
# Mock the events to prevent NoneType errors
|
||||
@ -130,16 +134,24 @@ class TestSeriesAppDownload:
|
||||
# Verify result
|
||||
assert result is True
|
||||
app.loader.download.assert_called_once()
|
||||
|
||||
# Verify folder was created
|
||||
folder_path = os.path.join(test_dir, "anime_folder")
|
||||
assert os.path.exists(folder_path)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch('src.core.SeriesApp.Loaders')
|
||||
@patch('src.core.SeriesApp.SerieScanner')
|
||||
@patch('src.core.SeriesApp.SerieList')
|
||||
async def test_download_with_progress_callback(
|
||||
self, mock_serie_list, mock_scanner, mock_loaders
|
||||
self, mock_serie_list, mock_scanner, mock_loaders, tmp_path
|
||||
):
|
||||
"""Test download with progress callback."""
|
||||
test_dir = "/test/anime"
|
||||
test_dir = str(tmp_path / "anime")
|
||||
# Create the test directory
|
||||
import os
|
||||
os.makedirs(test_dir, exist_ok=True)
|
||||
|
||||
app = SeriesApp(test_dir)
|
||||
|
||||
# Mock the events
|
||||
@ -172,10 +184,14 @@ class TestSeriesAppDownload:
|
||||
@patch('src.core.SeriesApp.SerieScanner')
|
||||
@patch('src.core.SeriesApp.SerieList')
|
||||
async def test_download_cancellation(
|
||||
self, mock_serie_list, mock_scanner, mock_loaders
|
||||
self, mock_serie_list, mock_scanner, mock_loaders, tmp_path
|
||||
):
|
||||
"""Test download cancellation during operation."""
|
||||
test_dir = "/test/anime"
|
||||
test_dir = str(tmp_path / "anime")
|
||||
# Create the test directory
|
||||
import os
|
||||
os.makedirs(test_dir, exist_ok=True)
|
||||
|
||||
app = SeriesApp(test_dir)
|
||||
|
||||
# Mock the events
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user