Folder move added
This commit is contained in:
kodi
@@ -0,0 +1,190 @@
|
||||
# Batch Directory Move v1
|
||||
|
||||
## 1. Scope
|
||||
|
||||
Doel van batch directory move v1 is het gecontroleerd ondersteunen van verplaatsingen voor meerdere geselecteerde items via de bestaande move/task-flow, zonder de huidige file-flow te destabiliseren.
|
||||
|
||||
Aanbevolen v1-scope:
|
||||
- alleen same-root batch move
|
||||
- meerdere directories ondersteunen
|
||||
- gemengde selectie van files + directories ondersteunen, maar alleen binnen dezelfde root
|
||||
- geen cross-root batch directory move
|
||||
- geen rollback
|
||||
- geen recursive copy-delete move voor directories
|
||||
- geen partial rename-semantiek in batchmodus
|
||||
|
||||
Niet in scope:
|
||||
- cross-root batch directory move
|
||||
- batch move met meerdere roots tegelijk
|
||||
- batch move met conflictoplossing UI
|
||||
- cancel/retry
|
||||
- rollback bij gedeeltelijk succes
|
||||
- hernoemen per item binnen batch-popup
|
||||
|
||||
## 2. Selectiesituaties
|
||||
|
||||
### Meerdere directories
|
||||
- Toegestaan in v1, mits alle geselecteerde directories binnen dezelfde root vallen.
|
||||
- Bestemming is de current path van het inactieve paneel.
|
||||
- Elk item krijgt als doelpad: `destination_base + / + item.name`.
|
||||
|
||||
### Meerdere files + directories gemengd
|
||||
- Toegestaan in v1, mits alle geselecteerde items binnen dezelfde root vallen.
|
||||
- Files en directories worden in dezelfde batch verwerkt.
|
||||
- Elk item gebruikt dezelfde destination-map semantiek.
|
||||
|
||||
### 1 directory + meerdere files
|
||||
- Valt onder gemengde selectie en is dus toegestaan onder dezelfde same-root-regels.
|
||||
|
||||
### Niet toegestaan in v1
|
||||
- selectie die items uit verschillende roots combineert
|
||||
- selectie waarbij voor een directory het doelpad in de eigen subtree valt
|
||||
- selectie met symlink-source-items
|
||||
- selectie waarbij een doelpad al bestaat
|
||||
|
||||
## 3. Same-root versus cross-root
|
||||
|
||||
Aanbevolen v1-keuze:
|
||||
- batch directory move blijft beperkt tot same-root
|
||||
- cross-root batch directory move wordt expliciet geblokkeerd
|
||||
|
||||
Motivatie:
|
||||
- same-root kan native rename/move gebruiken en sluit aan op de huidige directory move v1
|
||||
- cross-root voor directories vereist recursieve copy + delete en introduceert veel meer partial-failure-risico
|
||||
- gemengde batch over roots maakt progress, foutafhandeling en rollback aanzienlijk complexer
|
||||
|
||||
Geblokkeerde melding in v1:
|
||||
- `Cross-root batch directory move is not supported in v1`
|
||||
|
||||
## 4. Taskmodel
|
||||
|
||||
Aanbevolen richting:
|
||||
- één task voor de hele batch
|
||||
|
||||
Motivatie:
|
||||
- sluit beter aan op de gebruikersactie: één batch-confirmatie, één batchresultaat
|
||||
- voorkomt een explosie van losse tasks bij grote selecties
|
||||
- maakt batchprogress en foutsamenvatting eenvoudiger zichtbaar
|
||||
|
||||
Progress:
|
||||
- `done_items` / `total_items` zijn leidend
|
||||
- `total_items` = aantal geselecteerde top-level items in de batch
|
||||
- `done_items` telt per succesvol verplaatst top-level item op
|
||||
- `done_bytes` / `total_bytes` blijven `null` in v1 voor batch directory move
|
||||
- ook bij gemengde selectie is het verstandig in v1 item-gebaseerde progress leidend te houden
|
||||
|
||||
Failure-semantiek:
|
||||
- fail-fast binnen de batchtask
|
||||
- bij eerste runtime-fout stopt verdere verwerking
|
||||
- task eindigt als `failed`
|
||||
- reeds verplaatste items blijven verplaatst
|
||||
- geen rollback in v1
|
||||
|
||||
Partial completion model:
|
||||
- partial completion wordt impliciet zichtbaar via `done_items < total_items`
|
||||
- `failed_item` bevat het item waarop de batch stopte
|
||||
- geen aparte `partial_completed` status in v1
|
||||
|
||||
## 5. Backend-impact
|
||||
|
||||
Hergebruik:
|
||||
- bestaande move-validatie voor individuele file/directory move
|
||||
- bestaande task persistence en read-endpoints
|
||||
- bestaande same-root native move in filesystem adapter
|
||||
- bestaande path_guard containment en symlink-afwijzing
|
||||
|
||||
Waarschijnlijke wijzigingen:
|
||||
- move service: batchvalidatie en task-creatie voor meerdere items
|
||||
- task runner: batch move worker
|
||||
- filesystem adapter: hergebruik van bestaande file move en directory move primitives
|
||||
- task repository: waarschijnlijk geen schemawijziging nodig als `done_items`, `total_items`, `current_item`, `failed_item` al volstaan
|
||||
|
||||
Belangrijk semantisch behoud:
|
||||
- rename voor exact 1 item blijft apart via bestaande rename-flow/F6-popupbeslislogica
|
||||
- batchmodus is altijd move, nooit rename
|
||||
|
||||
## 6. UI-impact
|
||||
|
||||
Gedrag voor Move-knop en F6:
|
||||
- bij meerdere geselecteerde items mag de bestaande batch move-confirmatie worden hergebruikt
|
||||
- die confirmatie moet duidelijk tonen:
|
||||
- aantal geselecteerde items
|
||||
- destination-map
|
||||
- dat batch same-root vereist voor directories
|
||||
- voor gemengde selectie moet dezelfde batch-confirmatie bruikbaar blijven
|
||||
|
||||
Benodigde meldingen:
|
||||
- `Cross-root batch directory move is not supported in v1`
|
||||
- `Batch move requires all selected items to be in the same root`
|
||||
- `Destination already exists for one or more items`
|
||||
- `Destination cannot be inside source`
|
||||
- `Source must not be a symlink`
|
||||
|
||||
Aanbeveling:
|
||||
- hergebruik bestaande batch confirm-popup
|
||||
- geen extra popup-type toevoegen in v1
|
||||
- alleen de validatietekst en submit-logica uitbreiden
|
||||
|
||||
## 7. Security en validatie
|
||||
|
||||
Per item moeten minimaal deze checks gelden:
|
||||
- source ligt binnen whitelist
|
||||
- destination-parent ligt binnen whitelist
|
||||
- source is geen symlink
|
||||
- destination bestaat nog niet
|
||||
- directory destination ligt niet in subtree van source
|
||||
- same-root verplicht voor directories in batch v1
|
||||
|
||||
Gemengde foutgevallen:
|
||||
- als selectie meerdere roots bevat: directe validatiefout vóór task-creatie
|
||||
- als één item ongeldig is: hele batchcreatie afwijzen vóór task-creatie
|
||||
- geen "best effort" pre-validatie waarbij geldige items toch alvast starten
|
||||
|
||||
Containment:
|
||||
- path_guard blijft leidend voor alle bron- en doelpaden
|
||||
- geen vrije pathconstructie buiten bestaande root mapping
|
||||
|
||||
## 8. Teststrategie
|
||||
|
||||
Golden tests:
|
||||
- batch same-root directories success
|
||||
- batch same-root mixed files + directories success
|
||||
- batch cross-root directories blocked
|
||||
- batch mixed-root selection blocked
|
||||
- batch destination exists blocked
|
||||
- batch destination inside source blocked
|
||||
- batch symlink source blocked
|
||||
- batch task failed shape bij runtime io_error
|
||||
|
||||
Regressietests:
|
||||
- exacte 1 file move-flow blijft ongewijzigd
|
||||
- exacte 1 directory same-root move blijft werken
|
||||
- batch file-only move blijft werken
|
||||
- F6 rename/move voor exact 1 item blijft semantisch gelijk
|
||||
|
||||
Securitytests:
|
||||
- traversal in één van de source paths
|
||||
- traversal in destination base
|
||||
- symlink directory source
|
||||
- symlink file source binnen gemengde selectie
|
||||
|
||||
Runtime edge cases:
|
||||
- eerste item succeeds, tweede faalt -> task failed met `done_items == 1`
|
||||
- destination conflict ontdekt vóór task-creatie
|
||||
- empty selection blijft no-op in UI, geen backend-aanroep
|
||||
|
||||
## 9. Aanbeveling
|
||||
|
||||
Aanbevolen v1-richting met laag regressierisico:
|
||||
- ondersteun alleen same-root batch move
|
||||
- ondersteun gemengde selectie van files + directories alleen als alle items in dezelfde root zitten
|
||||
- gebruik één task per batch
|
||||
- gebruik item-based progress (`done_items` / `total_items`)
|
||||
- fail-fast zonder rollback
|
||||
- hergebruik bestaande batch move-confirmatie in de UI
|
||||
|
||||
Waarom deze richting:
|
||||
- bouwt rechtstreeks voort op de huidige same-root directory move v1 en batch file move
|
||||
- beperkt backendcomplexiteit tot validatie + batchrunner, zonder cross-root recursieve directorylogica
|
||||
- houdt de UI voorspelbaar: één batchactie, één task, één resultaat
|
||||
- minimaliseert regressierisico voor de bestaande single-item file-flow en F6-semantiek
|
||||
@@ -0,0 +1,308 @@
|
||||
# DIRECTORY_MOVE_V1_DESIGN.md
|
||||
|
||||
## 1. Scope
|
||||
|
||||
Doel van Directory Move v1 is om directory-verplaatsing toe te voegen binnen het bestaande move/task/securitymodel, zonder het huidige veiligheidsniveau of de voorspelbaarheid van de backend te verzwakken.
|
||||
|
||||
In scope voor v1:
|
||||
- directory move als expliciete uitbreiding van de bestaande move-operatie
|
||||
- task-based uitvoering
|
||||
- conflictcontrole op destination
|
||||
- beveiligde padvalidatie via bestaand `path_guard`
|
||||
- duidelijke foutstatussen bij ongeldige of gevaarlijke moves
|
||||
|
||||
Out of scope voor v1:
|
||||
- geen rollbackmechanisme
|
||||
- geen cancel/retry
|
||||
- geen gedeeltelijke recovery van half-voltooide cross-root moves
|
||||
- geen symlink-following buiten whitelist
|
||||
- geen speciale merge- of overwrite-semantiek
|
||||
- geen multi-select directory move als aparte batch-semantiek buiten bestaande sequentiële task-aanmaak
|
||||
|
||||
---
|
||||
|
||||
## 2. Same-root versus cross-root
|
||||
|
||||
### Directory move binnen dezelfde root
|
||||
|
||||
Dit is de veiligste en simpelste stap.
|
||||
|
||||
Kenmerken:
|
||||
- kan meestal native via `rename()`/`move` op directoryniveau
|
||||
- snel en atomair binnen hetzelfde filesystem/root
|
||||
- geen inhoudelijke recursieve file copy nodig
|
||||
- veel minder kans op partial failure
|
||||
|
||||
### Directory move tussen verschillende roots
|
||||
|
||||
Dit is aanzienlijk complexer.
|
||||
|
||||
Kenmerken:
|
||||
- vereist recursieve copy van directory tree
|
||||
- daarna delete van source tree
|
||||
- progressmeting en foutafhandeling worden lastiger
|
||||
- partial failure is realistischer
|
||||
- rollback is ingewikkeld en in v1 ongewenst
|
||||
|
||||
### Aanbevolen scopekeuze
|
||||
|
||||
Aanbevolen v1:
|
||||
- **alleen same-root directory move ondersteunen**
|
||||
- **cross-root directory move nog niet ondersteunen**
|
||||
|
||||
Reden:
|
||||
- sluit goed aan op bestaand taskmodel zonder grote complexiteitsexplosie
|
||||
- beperkt regressierisico
|
||||
- voorkomt een half-robuste recursieve copy+delete-implementatie zonder rollback
|
||||
|
||||
---
|
||||
|
||||
## 3. Semantiek
|
||||
|
||||
### Wanneer is het een echte move
|
||||
|
||||
Directory move is van toepassing als:
|
||||
- source een directory is
|
||||
- destination een volledig doelpad is
|
||||
- destination nog niet bestaat
|
||||
|
||||
Binnen same-root:
|
||||
- de operatie gebruikt native rename/move op directoryniveau
|
||||
- dit geldt zowel voor verplaatsen naar andere parent als voor directory move binnen dezelfde root met andere naam
|
||||
|
||||
### Destination bestaat al
|
||||
|
||||
Gedrag v1:
|
||||
- destination mag niet bestaan
|
||||
- response: `already_exists`
|
||||
|
||||
Geen merge in v1.
|
||||
|
||||
### Nested destinations
|
||||
|
||||
Verboden gevallen:
|
||||
- source naar een child van zichzelf
|
||||
- source naar een pad in eigen subtree
|
||||
|
||||
Voorbeeld:
|
||||
- source: `/Volumes/8TB/Shows`
|
||||
- destination: `/Volumes/8TB/Shows/Archive/Shows`
|
||||
|
||||
Dit moet direct geblokkeerd worden.
|
||||
|
||||
Aanbevolen fout:
|
||||
- `invalid_request` met duidelijke boodschap zoals `Destination cannot be inside source`
|
||||
|
||||
### Verplaatsen van map in zichzelf
|
||||
|
||||
Ook verboden:
|
||||
- destination exact gelijk aan source
|
||||
- destination onder source subtree
|
||||
|
||||
Dit moet vóór task-creatie worden afgewezen.
|
||||
|
||||
---
|
||||
|
||||
## 4. Taskmodel
|
||||
|
||||
Directory move blijft task-based, ook voor same-root moves.
|
||||
|
||||
### Progressmeting
|
||||
|
||||
Voor same-root native directory move is echte byte-progress meestal niet beschikbaar.
|
||||
|
||||
Aanbevolen v1:
|
||||
- gebruik vooral `done_items` / `total_items`
|
||||
- voor same-root kan dat minimaal zijn:
|
||||
- `total_items = 1`
|
||||
- `done_items = 0 -> 1`
|
||||
- `done_bytes` / `total_bytes` mogen `null` blijven in v1 voor same-root directory move
|
||||
|
||||
### Fail-fast gedrag
|
||||
|
||||
Aanbevolen v1:
|
||||
- fail-fast
|
||||
- zodra de move-operatie faalt, task -> `failed`
|
||||
|
||||
### Partial failure
|
||||
|
||||
Voor same-root native directory move is partial failure veel minder waarschijnlijk.
|
||||
- of de rename slaagt
|
||||
- of de rename faalt
|
||||
|
||||
Dus v1-model:
|
||||
- geen expliciete partial-successstatus
|
||||
- eindstatus blijft:
|
||||
- `completed`
|
||||
- `failed`
|
||||
|
||||
### Rollback
|
||||
|
||||
Geen rollback in v1.
|
||||
|
||||
Voor same-root native rename is dat meestal niet nodig.
|
||||
Voor cross-root zou rollback wel relevant worden, maar die scope wordt niet aanbevolen voor v1.
|
||||
|
||||
---
|
||||
|
||||
## 5. Security
|
||||
|
||||
### Symlinkgedrag in directory trees
|
||||
|
||||
Aanbevolen v1-beleid:
|
||||
- source directory zelf mag geen symlink zijn
|
||||
- bij same-root native rename wordt de tree niet recursief gevolgd of gekopieerd, dus child-symlinks hoeven niet actief gevolgd te worden voor uitvoering
|
||||
- toch blijft containmentcontrole op source en destination verplicht
|
||||
|
||||
### Traversal en containment
|
||||
|
||||
- bron- en destination-pad blijven via bestaand `path_guard`
|
||||
- destination binnen whitelist verplicht
|
||||
- destination mag niet buiten root escapen
|
||||
- nested destination-in-source moet expliciet extra gevalideerd worden
|
||||
|
||||
### PathGuard-impact
|
||||
|
||||
Bestaand `path_guard` blijft basis voor:
|
||||
- whitelistvalidatie
|
||||
- traversalblokkade
|
||||
- symlink containment check op bron/destination-resolutie
|
||||
|
||||
Aanvullend nodig voor directory move:
|
||||
- helper of servicecheck om te bepalen of destination in subtree van source ligt
|
||||
|
||||
### Escapes buiten whitelist voorkomen
|
||||
|
||||
Bij same-root-only v1:
|
||||
- containment blijft relatief eenvoudig
|
||||
- geen recursieve copy over child nodes nodig
|
||||
- dus ook minder attack surface dan cross-root recursieve tree copy
|
||||
|
||||
---
|
||||
|
||||
## 6. Backend-impact
|
||||
|
||||
Waarschijnlijk te wijzigen delen:
|
||||
- `webui/backend/app/services/move_task_service.py`
|
||||
- `webui/backend/app/tasks_runner.py`
|
||||
- `webui/backend/app/fs/filesystem_adapter.py`
|
||||
- mogelijk `webui/backend/app/api/schemas.py` alleen als taskdetail-documentatie wordt aangescherpt
|
||||
- mogelijk kleine aanvulling in `path_guard.py` of move-service validatiehelpers
|
||||
|
||||
### Bestaande move-logica die hergebruikt kan worden
|
||||
|
||||
Herbruikbaar:
|
||||
- task-creatie
|
||||
- repository-persistency
|
||||
- statusflow `queued -> running -> completed/failed`
|
||||
- destination-exists checks
|
||||
- parent directory validation
|
||||
- algemene error mapping
|
||||
|
||||
Nieuw voor directory move:
|
||||
- source typecheck moet directory toestaan in same-root-case
|
||||
- same-root task runner moet directory rename kunnen uitvoeren
|
||||
- nested-destination-validatie
|
||||
|
||||
### Rename binnen zelfde parent blijft apart
|
||||
|
||||
Ja.
|
||||
|
||||
Aanbevolen scheiding:
|
||||
- `rename` endpoint blijft aparte, directe single-path operatie
|
||||
- `move` blijft task-based voor echte moves
|
||||
- ook als een same-root directory move technisch op rename lijkt, blijft het semantisch onderdeel van `move`
|
||||
|
||||
Dat houdt UI- en API-rollen duidelijk.
|
||||
|
||||
---
|
||||
|
||||
## 7. UI-impact
|
||||
|
||||
### Move-knop en F6 bij exact 1 directory
|
||||
|
||||
Aanbevolen gedrag:
|
||||
- F6 / Move-knop blijven dezelfde popupflow gebruiken
|
||||
- exact 1 directory geselecteerd:
|
||||
- zelfde parent + andere naam -> huidige `rename` route blijft toegestaan
|
||||
- andere parent binnen dezelfde root -> task-based directory move toegestaan in v1
|
||||
- cross-root destination -> blokkeren met duidelijke melding, bijvoorbeeld:
|
||||
- `Cross-root directory move is not supported in v1`
|
||||
|
||||
### Multi-select met directories
|
||||
|
||||
Aanbevolen v1:
|
||||
- geen gemengde halfslimme batchflow
|
||||
- als multi-select directories bevat:
|
||||
- alleen toestaan als alle geselecteerde directories voldoen aan same-root move-semantiek, of
|
||||
- eenvoudiger en veiliger voor v1: nog blokkeren met duidelijke melding
|
||||
|
||||
Aanbevolen eerste stap:
|
||||
- **multi-select met directories nog blokkeren**
|
||||
- melding:
|
||||
- `Batch directory move is not supported in v1`
|
||||
|
||||
Reden:
|
||||
- beperkt scope en regressierisico
|
||||
- houdt UI-flow voorspelbaar
|
||||
|
||||
---
|
||||
|
||||
## 8. Teststrategie
|
||||
|
||||
### Golden tests
|
||||
|
||||
Toe te voegen voor move-API:
|
||||
- same-root directory move success
|
||||
- directory destination exists -> `already_exists`
|
||||
- directory source not found
|
||||
- directory source is symlink -> blokkade
|
||||
- nested destination blocked
|
||||
- exact same source/destination blocked
|
||||
- cross-root directory move blocked
|
||||
|
||||
### Regressietests
|
||||
|
||||
- file move success same-root blijft werken
|
||||
- file move success cross-root blijft werken als huidige scope dat al ondersteunt
|
||||
- rename endpoint blijft ongewijzigd
|
||||
- browse en delete blijven ongewijzigd
|
||||
|
||||
### Securitytests
|
||||
|
||||
- traversal source
|
||||
- traversal destination
|
||||
- symlink source rejection
|
||||
- destination inside source rejection
|
||||
- destination outside whitelist rejection
|
||||
|
||||
### Runtime edge cases
|
||||
|
||||
- rename/move van lege directory
|
||||
- rename/move van directory met inhoud binnen same-root
|
||||
- permission failure -> `io_error`
|
||||
- destination parent ontbreekt -> bestaande foutmapping
|
||||
|
||||
---
|
||||
|
||||
## 9. Aanbeveling
|
||||
|
||||
Aanbevolen v1-richting:
|
||||
- **alleen same-root directory move ondersteunen**
|
||||
- **cross-root directory move nog niet ondersteunen**
|
||||
|
||||
Korte motivatie:
|
||||
- laagste complexiteit
|
||||
- beste aansluiting op bestaand taskmodel
|
||||
- minimale partial-failure kans
|
||||
- veel kleiner regressie- en securityrisico dan recursieve cross-root tree copy+delete
|
||||
|
||||
Praktische v1-scope:
|
||||
- exact 1 directory
|
||||
- same-root move task-based
|
||||
- nested destination geblokkeerd
|
||||
- destination exists geblokkeerd
|
||||
- cross-root directory move expliciet geweigerd
|
||||
- batch directory move nog niet
|
||||
|
||||
Dat is de veiligste eerste stap met duidelijke semantiek en goede testbaarheid.
|
||||
Binary file not shown.
Binary file not shown.
@@ -125,39 +125,62 @@ class TaskRepository:
|
||||
).fetchall()
|
||||
return [self._to_dict(row) for row in rows]
|
||||
|
||||
def mark_running(self, task_id: str, done_bytes: int, total_bytes: int | None, current_item: str | None) -> None:
|
||||
def mark_running(
|
||||
self,
|
||||
task_id: str,
|
||||
done_bytes: int | None = None,
|
||||
total_bytes: int | None = None,
|
||||
done_items: int | None = None,
|
||||
total_items: int | None = None,
|
||||
current_item: str | None = None,
|
||||
) -> None:
|
||||
started_at = self._now_iso()
|
||||
with self._connection() as conn:
|
||||
conn.execute(
|
||||
"""
|
||||
UPDATE tasks
|
||||
SET status = ?, started_at = ?, done_bytes = ?, total_bytes = ?, current_item = ?
|
||||
SET status = ?, started_at = ?, done_bytes = ?, total_bytes = ?, done_items = ?, total_items = ?, current_item = ?
|
||||
WHERE id = ?
|
||||
""",
|
||||
("running", started_at, done_bytes, total_bytes, current_item, task_id),
|
||||
("running", started_at, done_bytes, total_bytes, done_items, total_items, current_item, task_id),
|
||||
)
|
||||
|
||||
def update_progress(self, task_id: str, done_bytes: int, total_bytes: int | None, current_item: str | None) -> None:
|
||||
def update_progress(
|
||||
self,
|
||||
task_id: str,
|
||||
done_bytes: int | None = None,
|
||||
total_bytes: int | None = None,
|
||||
done_items: int | None = None,
|
||||
total_items: int | None = None,
|
||||
current_item: str | None = None,
|
||||
) -> None:
|
||||
with self._connection() as conn:
|
||||
conn.execute(
|
||||
"""
|
||||
UPDATE tasks
|
||||
SET done_bytes = ?, total_bytes = ?, current_item = ?
|
||||
SET done_bytes = ?, total_bytes = ?, done_items = ?, total_items = ?, current_item = ?
|
||||
WHERE id = ?
|
||||
""",
|
||||
(done_bytes, total_bytes, current_item, task_id),
|
||||
(done_bytes, total_bytes, done_items, total_items, current_item, task_id),
|
||||
)
|
||||
|
||||
def mark_completed(self, task_id: str, done_bytes: int | None, total_bytes: int | None) -> None:
|
||||
def mark_completed(
|
||||
self,
|
||||
task_id: str,
|
||||
done_bytes: int | None = None,
|
||||
total_bytes: int | None = None,
|
||||
done_items: int | None = None,
|
||||
total_items: int | None = None,
|
||||
) -> None:
|
||||
finished_at = self._now_iso()
|
||||
with self._connection() as conn:
|
||||
conn.execute(
|
||||
"""
|
||||
UPDATE tasks
|
||||
SET status = ?, finished_at = ?, done_bytes = ?, total_bytes = ?
|
||||
SET status = ?, finished_at = ?, done_bytes = ?, total_bytes = ?, done_items = ?, total_items = ?
|
||||
WHERE id = ?
|
||||
""",
|
||||
("completed", finished_at, done_bytes, total_bytes, task_id),
|
||||
("completed", finished_at, done_bytes, total_bytes, done_items, total_items, task_id),
|
||||
)
|
||||
|
||||
def mark_failed(
|
||||
@@ -168,16 +191,29 @@ class TaskRepository:
|
||||
failed_item: str | None,
|
||||
done_bytes: int | None,
|
||||
total_bytes: int | None,
|
||||
done_items: int | None = None,
|
||||
total_items: int | None = None,
|
||||
) -> None:
|
||||
finished_at = self._now_iso()
|
||||
with self._connection() as conn:
|
||||
conn.execute(
|
||||
"""
|
||||
UPDATE tasks
|
||||
SET status = ?, finished_at = ?, error_code = ?, error_message = ?, failed_item = ?, done_bytes = ?, total_bytes = ?
|
||||
SET status = ?, finished_at = ?, error_code = ?, error_message = ?, failed_item = ?, done_bytes = ?, total_bytes = ?, done_items = ?, total_items = ?
|
||||
WHERE id = ?
|
||||
""",
|
||||
("failed", finished_at, error_code, error_message, failed_item, done_bytes, total_bytes, task_id),
|
||||
(
|
||||
"failed",
|
||||
finished_at,
|
||||
error_code,
|
||||
error_message,
|
||||
failed_item,
|
||||
done_bytes,
|
||||
total_bytes,
|
||||
done_items,
|
||||
total_items,
|
||||
task_id,
|
||||
),
|
||||
)
|
||||
|
||||
def _ensure_schema(self) -> None:
|
||||
|
||||
Binary file not shown.
@@ -38,6 +38,9 @@ class FilesystemAdapter:
|
||||
def move_file(self, source: str, destination: str) -> None:
|
||||
Path(source).rename(Path(destination))
|
||||
|
||||
def move_directory(self, source: str, destination: str) -> None:
|
||||
Path(source).rename(Path(destination))
|
||||
|
||||
def is_directory_empty(self, path: Path) -> bool:
|
||||
return not any(path.iterdir())
|
||||
|
||||
|
||||
Binary file not shown.
@@ -1,5 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from backend.app.api.errors import AppError
|
||||
from backend.app.api.schemas import TaskCreateResponse
|
||||
from backend.app.db.task_repository import TaskRepository
|
||||
@@ -20,14 +22,16 @@ class MoveTaskService:
|
||||
if lexical_source.is_symlink():
|
||||
raise AppError(
|
||||
code="type_conflict",
|
||||
message="Source must be a regular file",
|
||||
message="Source must not be a symlink",
|
||||
status_code=409,
|
||||
details={"path": source},
|
||||
)
|
||||
if not resolved_source.absolute.is_file():
|
||||
source_is_file = resolved_source.absolute.is_file()
|
||||
source_is_directory = resolved_source.absolute.is_dir()
|
||||
if not source_is_file and not source_is_directory:
|
||||
raise AppError(
|
||||
code="type_conflict",
|
||||
message="Source must be a file",
|
||||
message="Unsupported source path type",
|
||||
status_code=409,
|
||||
details={"path": source},
|
||||
)
|
||||
@@ -41,6 +45,14 @@ class MoveTaskService:
|
||||
)
|
||||
self._map_directory_validation(parent_relative)
|
||||
|
||||
if source_is_directory and resolved_destination.absolute == resolved_source.absolute:
|
||||
raise AppError(
|
||||
code="invalid_request",
|
||||
message="Destination must differ from source",
|
||||
status_code=400,
|
||||
details={"path": source, "destination": destination},
|
||||
)
|
||||
|
||||
if resolved_destination.absolute.exists():
|
||||
raise AppError(
|
||||
code="already_exists",
|
||||
@@ -49,6 +61,36 @@ class MoveTaskService:
|
||||
details={"path": resolved_destination.relative},
|
||||
)
|
||||
|
||||
same_root = resolved_source.alias == resolved_destination.alias
|
||||
|
||||
if source_is_directory:
|
||||
if not same_root:
|
||||
raise AppError(
|
||||
code="invalid_request",
|
||||
message="Cross-root directory move is not supported in v1",
|
||||
status_code=400,
|
||||
details={"path": source, "destination": destination},
|
||||
)
|
||||
if self._is_nested_destination(resolved_source.absolute, resolved_destination.absolute):
|
||||
raise AppError(
|
||||
code="invalid_request",
|
||||
message="Destination cannot be inside source",
|
||||
status_code=400,
|
||||
details={"path": source, "destination": destination},
|
||||
)
|
||||
|
||||
task = self._repository.create_task(
|
||||
operation="move",
|
||||
source=resolved_source.relative,
|
||||
destination=resolved_destination.relative,
|
||||
)
|
||||
self._runner.enqueue_move_directory(
|
||||
task_id=task["id"],
|
||||
source=str(resolved_source.absolute),
|
||||
destination=str(resolved_destination.absolute),
|
||||
)
|
||||
return TaskCreateResponse(task_id=task["id"], status=task["status"])
|
||||
|
||||
total_bytes = int(resolved_source.absolute.stat().st_size)
|
||||
task = self._repository.create_task(
|
||||
operation="move",
|
||||
@@ -56,7 +98,6 @@ class MoveTaskService:
|
||||
destination=resolved_destination.relative,
|
||||
)
|
||||
|
||||
same_root = resolved_source.alias == resolved_destination.alias
|
||||
self._runner.enqueue_move_file(
|
||||
task_id=task["id"],
|
||||
source=str(resolved_source.absolute),
|
||||
@@ -79,3 +120,11 @@ class MoveTaskService:
|
||||
details=exc.details,
|
||||
)
|
||||
raise
|
||||
|
||||
@staticmethod
|
||||
def _is_nested_destination(source: Path, destination: Path) -> bool:
|
||||
try:
|
||||
destination.relative_to(source)
|
||||
return True
|
||||
except ValueError:
|
||||
return False
|
||||
|
||||
@@ -35,6 +35,14 @@ class TaskRunner:
|
||||
)
|
||||
thread.start()
|
||||
|
||||
def enqueue_move_directory(self, task_id: str, source: str, destination: str) -> None:
|
||||
thread = threading.Thread(
|
||||
target=self._run_move_directory,
|
||||
args=(task_id, source, destination),
|
||||
daemon=True,
|
||||
)
|
||||
thread.start()
|
||||
|
||||
def _run_copy_file(self, task_id: str, source: str, destination: str, total_bytes: int) -> None:
|
||||
self._repository.mark_running(
|
||||
task_id=task_id,
|
||||
@@ -123,3 +131,28 @@ class TaskRunner:
|
||||
done_bytes=progress["done"],
|
||||
total_bytes=total_bytes,
|
||||
)
|
||||
|
||||
def _run_move_directory(self, task_id: str, source: str, destination: str) -> None:
|
||||
self._repository.mark_running(
|
||||
task_id=task_id,
|
||||
done_items=0,
|
||||
total_items=1,
|
||||
current_item=source,
|
||||
)
|
||||
|
||||
try:
|
||||
self._filesystem.move_directory(source=source, destination=destination)
|
||||
self._repository.mark_completed(
|
||||
task_id=task_id,
|
||||
done_items=1,
|
||||
total_items=1,
|
||||
)
|
||||
except OSError as exc:
|
||||
self._repository.mark_failed(
|
||||
task_id=task_id,
|
||||
error_code="io_error",
|
||||
error_message=str(exc),
|
||||
failed_item=source,
|
||||
done_items=0,
|
||||
total_items=1,
|
||||
)
|
||||
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
@@ -96,6 +96,33 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertTrue((self.root1 / "moved.txt").exists())
|
||||
self.assertFalse(src.exists())
|
||||
|
||||
def test_move_directory_success_same_root_and_completed(self) -> None:
|
||||
src_dir = self.root1 / "source-dir"
|
||||
src_dir.mkdir()
|
||||
(src_dir / "nested.txt").write_text("hello", encoding="utf-8")
|
||||
target_parent = self.root1 / "target-parent"
|
||||
target_parent.mkdir()
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/source-dir", "destination": "storage1/target-parent/moved-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 202)
|
||||
body = response.json()
|
||||
self.assertEqual(body["status"], "queued")
|
||||
|
||||
detail = self._wait_task(body["task_id"])
|
||||
self.assertEqual(detail["status"], "completed")
|
||||
self.assertEqual(detail["done_items"], 1)
|
||||
self.assertEqual(detail["total_items"], 1)
|
||||
self.assertIsNone(detail["done_bytes"])
|
||||
self.assertIsNone(detail["total_bytes"])
|
||||
self.assertTrue((self.root1 / "target-parent" / "moved-dir").is_dir())
|
||||
self.assertTrue((self.root1 / "target-parent" / "moved-dir" / "nested.txt").exists())
|
||||
self.assertFalse(src_dir.exists())
|
||||
|
||||
def test_move_success_cross_root_create_task_shape_and_completed(self) -> None:
|
||||
src = self.root1 / "source.txt"
|
||||
src.write_text("hello", encoding="utf-8")
|
||||
@@ -116,6 +143,19 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertTrue((self.root2 / "moved.txt").exists())
|
||||
self.assertFalse(src.exists())
|
||||
|
||||
def test_move_directory_cross_root_blocked(self) -> None:
|
||||
src_dir = self.root1 / "source-dir"
|
||||
src_dir.mkdir()
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/source-dir", "destination": "storage2/source-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(response.json()["error"]["code"], "invalid_request")
|
||||
|
||||
def test_move_source_not_found(self) -> None:
|
||||
response = self._request(
|
||||
"POST",
|
||||
@@ -126,13 +166,24 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.json()["error"]["code"], "path_not_found")
|
||||
|
||||
def test_move_source_is_directory_type_conflict(self) -> None:
|
||||
def test_move_directory_source_not_found(self) -> None:
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/missing-dir", "destination": "storage1/out-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.json()["error"]["code"], "path_not_found")
|
||||
|
||||
def test_move_source_is_directory_type_conflict_for_file_destination_parent(self) -> None:
|
||||
(self.root1 / "dir").mkdir()
|
||||
(self.root1 / "out.txt").write_text("x", encoding="utf-8")
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/dir", "destination": "storage1/out.txt"},
|
||||
{"source": "storage1/dir", "destination": "storage1/out.txt/child"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 409)
|
||||
@@ -151,6 +202,19 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "already_exists")
|
||||
|
||||
def test_move_directory_destination_exists_already_exists(self) -> None:
|
||||
(self.root1 / "source-dir").mkdir()
|
||||
(self.root1 / "target-dir").mkdir()
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/source-dir", "destination": "storage1/target-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "already_exists")
|
||||
|
||||
def test_move_traversal_source(self) -> None:
|
||||
response = self._request(
|
||||
"POST",
|
||||
@@ -173,6 +237,33 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(response.status_code, 403)
|
||||
self.assertEqual(response.json()["error"]["code"], "path_traversal_detected")
|
||||
|
||||
def test_move_directory_destination_inside_source_blocked(self) -> None:
|
||||
src_dir = self.root1 / "source-dir"
|
||||
src_dir.mkdir()
|
||||
(src_dir / "child").mkdir()
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/source-dir", "destination": "storage1/source-dir/child/moved-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(response.json()["error"]["code"], "invalid_request")
|
||||
|
||||
def test_move_directory_same_source_destination_blocked(self) -> None:
|
||||
src_dir = self.root1 / "source-dir"
|
||||
src_dir.mkdir()
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/source-dir", "destination": "storage1/source-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertEqual(response.json()["error"]["code"], "invalid_request")
|
||||
|
||||
def test_move_source_symlink_rejected(self) -> None:
|
||||
target = self.root1 / "real.txt"
|
||||
target.write_text("x", encoding="utf-8")
|
||||
@@ -188,6 +279,22 @@ class MoveApiGoldenTest(unittest.TestCase):
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "type_conflict")
|
||||
|
||||
def test_move_directory_source_symlink_rejected(self) -> None:
|
||||
target = self.root1 / "real-dir"
|
||||
target.mkdir()
|
||||
(target / "nested.txt").write_text("x", encoding="utf-8")
|
||||
link = self.root1 / "dir-link"
|
||||
link.symlink_to(target, target_is_directory=True)
|
||||
|
||||
response = self._request(
|
||||
"POST",
|
||||
"/api/files/move",
|
||||
{"source": "storage1/dir-link", "destination": "storage1/out-dir"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 409)
|
||||
self.assertEqual(response.json()["error"]["code"], "type_conflict")
|
||||
|
||||
def test_move_runtime_io_error_failed_task_shape(self) -> None:
|
||||
src = self.root1 / "source.txt"
|
||||
src.write_text("hello", encoding="utf-8")
|
||||
|
||||
@@ -82,6 +82,10 @@ class UiSmokeGoldenTest(unittest.TestCase):
|
||||
self.assertTrue((static_root / "style.css").exists())
|
||||
app_js = (static_root / "app.js").read_text(encoding="utf-8")
|
||||
self.assertIn('currentPath: "/Volumes"', app_js)
|
||||
self.assertIn('Cross-root directory move is not supported in v1', app_js)
|
||||
self.assertIn('Batch directory move is not supported in v1', app_js)
|
||||
self.assertIn("function rootKeyFromPath(path)", app_js)
|
||||
self.assertIn("function isNestedPath(sourcePath, destinationPath)", app_js)
|
||||
|
||||
app_js_url = app.url_path_for("ui", path="/app.js")
|
||||
style_css_url = app.url_path_for("ui", path="/style.css")
|
||||
|
||||
Binary file not shown.
@@ -0,0 +1,27 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[3]))
|
||||
|
||||
from backend.app.services.move_task_service import MoveTaskService
|
||||
|
||||
|
||||
class MoveTaskServiceTest(unittest.TestCase):
|
||||
def test_is_nested_destination_true_for_child_path(self) -> None:
|
||||
source = Path("/tmp/source")
|
||||
destination = source / "child" / "target"
|
||||
|
||||
self.assertTrue(MoveTaskService._is_nested_destination(source, destination))
|
||||
|
||||
def test_is_nested_destination_false_for_sibling_path(self) -> None:
|
||||
source = Path("/tmp/source")
|
||||
destination = Path("/tmp/other/target")
|
||||
|
||||
self.assertFalse(MoveTaskService._is_nested_destination(source, destination))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
+41
-3
@@ -259,6 +259,30 @@ function baseName(path) {
|
||||
return index >= 0 ? path.slice(index + 1) : path;
|
||||
}
|
||||
|
||||
function rootKeyFromPath(path) {
|
||||
const normalized = (path || "").trim();
|
||||
if (!normalized) {
|
||||
return null;
|
||||
}
|
||||
if (normalized.startsWith("/")) {
|
||||
const segments = normalized.split("/").filter(Boolean);
|
||||
if (segments.length < 2) {
|
||||
return normalized;
|
||||
}
|
||||
return `/${segments[0]}/${segments[1]}`;
|
||||
}
|
||||
return normalized.split("/")[0];
|
||||
}
|
||||
|
||||
function isNestedPath(sourcePath, destinationPath) {
|
||||
const source = (sourcePath || "").replace(/\/+$/, "");
|
||||
const destination = (destinationPath || "").replace(/\/+$/, "");
|
||||
if (!source || !destination) {
|
||||
return false;
|
||||
}
|
||||
return destination.startsWith(`${source}/`);
|
||||
}
|
||||
|
||||
function renderBreadcrumbs(pane, path) {
|
||||
const nav = document.getElementById(`${pane}-breadcrumbs`);
|
||||
nav.innerHTML = "";
|
||||
@@ -892,6 +916,12 @@ function showDirectoryMoveNotSupported() {
|
||||
setStatus(message);
|
||||
}
|
||||
|
||||
function showBatchDirectoryMoveNotSupported() {
|
||||
const message = "Batch directory move is not supported in v1";
|
||||
setError("actions-error", message);
|
||||
setStatus(message);
|
||||
}
|
||||
|
||||
function resetRenameMoveState() {
|
||||
renameMoveState = {
|
||||
source: null,
|
||||
@@ -965,7 +995,7 @@ function openF6Flow() {
|
||||
return openRenameMovePopup();
|
||||
}
|
||||
if (selectedItems.some((item) => item.kind !== "file")) {
|
||||
showDirectoryMoveNotSupported();
|
||||
showBatchDirectoryMoveNotSupported();
|
||||
return true;
|
||||
}
|
||||
return openBatchMovePopup(selectedItems);
|
||||
@@ -991,10 +1021,18 @@ async function submitRenameMovePopup() {
|
||||
elements.error.textContent = "Destination must differ from source";
|
||||
return;
|
||||
}
|
||||
if (source.kind === "directory" && destinationParent !== sourceParent) {
|
||||
elements.error.textContent = "Directory move is not supported in v1";
|
||||
if (source.kind === "directory") {
|
||||
if (isNestedPath(source.path, destination)) {
|
||||
elements.error.textContent = "Destination cannot be inside source";
|
||||
return;
|
||||
}
|
||||
if (destinationParent !== sourceParent) {
|
||||
if (rootKeyFromPath(destination) !== rootKeyFromPath(source.path)) {
|
||||
elements.error.textContent = "Cross-root directory move is not supported in v1";
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
if (destinationParent === sourceParent) {
|
||||
|
||||
Reference in New Issue
Block a user