refactor: R12 S3 — invert scheduler->routes, publish cross-module privates
Layering cleanup, no behaviour change (ruff clean backend+scripts, pytest 193): - Invert the scheduler->routes dependency: reset_demo_state (+ its demo-seed helpers) move from routes/admin.py to a new service module app/demo.py. Both routes/admin.py and scheduler.py now import it from app.demo, so the scheduler no longer reaches into the routes layer — its lazy import (the routes<->scheduler cycle workaround) is gone, hoisted to a top-level import. - Publish the private symbols that were imported across module boundaries (the underscore convention was lying): feed._serialize -> serialize, feed._to_tsquery_str -> to_tsquery_str, videos._insert_stubs -> insert_stubs, videos._stub_from_playlist_item -> stub_from_playlist_item, worker._uploader_url -> uploader_url, plex._shared._item_or_404 -> item_or_404. Scoped renames — the 5 other route modules keep their own truly private _serialize helpers. - _release_asset moves from routes/downloads.py to app/downloads/service.py as the public release_asset (asset bookkeeping is a service concern); the routes call it via service.release_asset.
This commit is contained in:
@@ -511,9 +511,9 @@ def redownload_download(
|
||||
asset = service.get_or_create_asset(db, job.source_kind, job.source_ref, sig)
|
||||
service.populate_from_catalog(db, asset)
|
||||
|
||||
# Now drop the old hold — `_release_asset` deletes the old file + row only if this job was its
|
||||
# Now drop the old hold — `service.release_asset` deletes the old file + row only if this job was its
|
||||
# last holder (a co-held asset is left intact for its other holders).
|
||||
_release_asset(db, job)
|
||||
service.release_asset(db, job)
|
||||
|
||||
asset.ref_count = (asset.ref_count or 0) + 1
|
||||
job.asset_id = asset.id
|
||||
@@ -538,7 +538,7 @@ def cancel_download(
|
||||
) -> dict:
|
||||
job = _own_job(db, user, job_id)
|
||||
if job.status not in ("done", "canceled"):
|
||||
_release_asset(db, job) # release while the job still counts as holding
|
||||
service.release_asset(db, job) # release while the job still counts as holding
|
||||
job.status = "canceled"
|
||||
db.commit()
|
||||
return _serialize_job(db, job)
|
||||
@@ -549,39 +549,12 @@ def delete_download(
|
||||
job_id: int, user: User = Depends(require_human), db: Session = Depends(get_db)
|
||||
) -> dict:
|
||||
job = _own_job(db, user, job_id)
|
||||
_release_asset(db, job)
|
||||
service.release_asset(db, job)
|
||||
db.delete(job)
|
||||
db.commit()
|
||||
return {"deleted": job_id}
|
||||
|
||||
|
||||
def _release_asset(db: Session, job: DownloadJob) -> None:
|
||||
"""Drop this job's hold on its asset when it leaves a holding state. Once NO job holds the
|
||||
asset anymore, delete the file + row immediately — a deleted download should free its disk,
|
||||
and the shared cache only needs to span *overlapping* holders (a later re-add just downloads
|
||||
again). `error` counts as holding: enqueue always +1'd ref_count and the worker never
|
||||
decrements on failure (ref_count is the route's job), so an errored job releases here on
|
||||
delete — else its +1 leaks and a later resume→ready keeps the file pinned past its last holder.
|
||||
Idempotent for the cancel path: cancel already released while the job was holding, then set it
|
||||
`canceled` (∉ the set below), so delete-after-cancel is a no-op and never double-releases.
|
||||
|
||||
We DON'T delete an errored asset row here even at ref==0: another request may be concurrently
|
||||
re-enqueuing the same (source,format) — get_or_create_asset would reset that row to `pending` and
|
||||
+1 it, and deleting it under that would FK-null the new job's asset (a lost update on ref_count).
|
||||
A ready asset is still freed at ref==0 (its file is the point); an orphaned errored row is cheap
|
||||
(no file) and gets reused+reset by the next enqueue, or reclaimed by a later GC pass."""
|
||||
if not job.asset_id or job.status not in ("queued", "running", "paused", "done", "error"):
|
||||
return
|
||||
asset = db.get(MediaAsset, job.asset_id)
|
||||
if asset is None:
|
||||
return
|
||||
asset.ref_count = max(0, (asset.ref_count or 0) - 1)
|
||||
if asset.ref_count == 0 and asset.status == "ready":
|
||||
if asset.rel_path:
|
||||
storage.delete_asset_files(settings.download_root, asset.rel_path)
|
||||
db.delete(asset)
|
||||
|
||||
|
||||
# --- file download (range-aware, custom display name) --------------------------------------
|
||||
|
||||
def _accessible_job(db: Session, user: User, job_id: int) -> DownloadJob:
|
||||
|
||||
Reference in New Issue
Block a user