From 6aa5a2c98241c4a3815b72ab0aa857d4e07785a9 Mon Sep 17 00:00:00 2001 From: chodeus Date: Fri, 7 Aug 2026 14:54:15 +0800 Subject: [PATCH 1/2] feat(cl2k): split one Drive folder into the artwork type layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routing art to logos/backgrounds/squareart meant finding and pasting three Drive folder IDs by hand. Add a per-row "Split by type" action: it creates (or reuses) the three subfolders under the parent you already entered and replaces that row with one routed row per type, carrying their real IDs. Opt-in per Drive. A drive that should stay flat never presses it and keeps a single row claiming several types, exactly as before — there is no migration and no default change. Purely additive on Drive: nothing is moved, renamed or deleted, and unrelated sibling folders under the parent are left alone. The subfolder names come from a fixed internal map, never user input, so the rclone argument surface is unchanged; the parent id is still validated by _reject_unsafe_id. Only the types the parent row claimed are carried to children, so splitting a logos-only Drive doesn't silently start uploading backgrounds. --- backend/api/cl2k_maker.py | 45 ++++++ backend/util/cl2k/gdrive_upload.py | 84 ++++++++++ .../extensions/cl2k/SaveLocationsFields.jsx | 70 ++++++++ tests/test_cl2k_type_subfolders.py | 152 ++++++++++++++++++ 4 files changed, 351 insertions(+) create mode 100644 tests/test_cl2k_type_subfolders.py diff --git a/backend/api/cl2k_maker.py b/backend/api/cl2k_maker.py index 4c99ee6a..c4ae940d 100644 --- a/backend/api/cl2k_maker.py +++ b/backend/api/cl2k_maker.py @@ -334,6 +334,51 @@ def test_drive( return ok(detail, {"folder_id": folder_id}) +@router.post( + "/gdrive/type-subfolders", + summary="Create logos/backgrounds/squareart under a parent Drive folder", +) +def gdrive_type_subfolders( + req: TestDriveRequest, + db: ChubDB = Depends(get_database), + logger: Any = Depends(get_cl2k_logger), +) -> JSONResponse: + """Split one parent Drive folder into the community artwork layout. + + Creates (or reuses) the three type subfolders and returns their real ids, so + the caller can store one routed destination per type. Purely additive on + Drive — nothing is moved, renamed or deleted, and a drive that should stay + flat simply never calls this. + """ + from backend.util.cl2k.gdrive_upload import ensure_type_subfolders, has_upload_token + + folder_id = (req.gdrive_folder_id or "").strip() + if not folder_id: + return error("A Google Drive folder ID is required", "GDRIVE_FOLDER_REQUIRED") + cfg = load_config() + if not has_upload_token(cfg.sync_gdrive): + return error( + "No Google Drive OAuth token configured — set one under Sync GDrive " + "(a service account cannot own files in a personal Drive).", + "GDRIVE_NO_TOKEN", + ) + try: + subfolders = ensure_type_subfolders(folder_id, cfg.sync_gdrive, logger) + except ValueError as exc: + return error(f"Invalid folder ID: {exc}", "GDRIVE_FOLDER_INVALID") + except Exception as exc: + return error( + f"Could not create the type subfolders: {exc}", + "GDRIVE_SUBFOLDERS_FAILED", + status_code=502, + ) + created = [s["name"] for s in subfolders if s["created"]] + detail = ( + f"Created {', '.join(created)}" if created else "All three subfolders already existed" + ) + return ok(detail, {"folder_id": folder_id, "subfolders": subfolders}) + + @router.get( "/external-ids", summary="TMDB external ids (tvdb_id + imdb_id) for a title" ) diff --git a/backend/util/cl2k/gdrive_upload.py b/backend/util/cl2k/gdrive_upload.py index caca240a..a393dfc3 100644 --- a/backend/util/cl2k/gdrive_upload.py +++ b/backend/util/cl2k/gdrive_upload.py @@ -256,6 +256,90 @@ def list_files(folder_id: str, sync_cfg: Any, logger) -> List[str]: return [ln.strip() for ln in result.stdout.splitlines() if ln.strip()] +# image_type -> the Drive subfolder the community artwork drives use for it. A +# FIXED map, never user input, so these never widen the rclone argument surface. +TYPE_SUBFOLDERS = { + "logo": "logos", + "background": "backgrounds", + "squareart": "squareart", +} + + +def ensure_type_subfolders(folder_id: str, sync_cfg: Any, logger) -> List[dict]: + """Create (or find) ``logos``/``backgrounds``/``squareart`` under ``folder_id``. + + Returns one ``{image_type, name, folder_id, created}`` per subfolder so the + caller can store real child ids — the upload path still addresses exactly one + Drive folder per destination. Raises on a missing token or rclone failure. + """ + _reject_unsafe_id(folder_id, "gdrive_folder_id") + auth = _upload_auth_args(sync_cfg) + if not auth: + raise RuntimeError( + "no usable Google Drive OAuth token configured — set a token under " + "Sync GDrive (a service account cannot own files in a personal Drive)" + ) + rclone = _rclone_path() + _ensure_remote(rclone) + + def _dir_ids() -> dict: + result = subprocess.run( + [ + rclone, + "lsjson", + "posters:", + "--drive-root-folder-id", + folder_id, + "--dirs-only", + *auth, + ], + check=False, + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError( + f"rclone lsjson failed: {_rclone_error_detail(result.stderr)}" + ) + return {d["Name"]: d["ID"] for d in json.loads(result.stdout or "[]")} + + existing = _dir_ids() + made = [] + for name in TYPE_SUBFOLDERS.values(): + if name in existing: + continue + result = subprocess.run( + [rclone, "mkdir", f"posters:{name}", "--drive-root-folder-id", folder_id, *auth], + check=False, + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError( + f"rclone mkdir '{name}' failed: {_rclone_error_detail(result.stderr)}" + ) + made.append(name) + + # Re-list once so newly created folders come back with their real ids. + ids = _dir_ids() if made else existing + missing = [n for n in TYPE_SUBFOLDERS.values() if n not in ids] + if missing: + raise RuntimeError(f"Drive did not return an id for: {', '.join(missing)}") + logger.info( + f"cl2k: type subfolders under {folder_id} — created {made or 'none'}, " + f"reused {[n for n in TYPE_SUBFOLDERS.values() if n not in made]}" + ) + return [ + { + "image_type": image_type, + "name": name, + "folder_id": ids[name], + "created": name in made, + } + for image_type, name in TYPE_SUBFOLDERS.items() + ] + + def delete_file(name: str, folder_id: str, sync_cfg: Any, logger) -> None: """Delete the single file ``name`` from Drive folder ``folder_id`` via ``rclone deletefile``. ``name`` is relative to ``folder_id`` (the rclone diff --git a/frontend/src/extensions/cl2k/SaveLocationsFields.jsx b/frontend/src/extensions/cl2k/SaveLocationsFields.jsx index ed4fa834..a5ffb854 100644 --- a/frontend/src/extensions/cl2k/SaveLocationsFields.jsx +++ b/frontend/src/extensions/cl2k/SaveLocationsFields.jsx @@ -398,12 +398,56 @@ const TestUploadButton = ({ folderId, disabled }) => { ); }; +// Splits ONE parent Drive into the community artwork layout. Opt-in per row — +// a Drive that should stay flat simply never uses it, and nothing on Drive is +// moved or deleted either way. +const SplitSubfoldersButton = ({ folderId, disabled, onSplit }) => { + const toast = useToast(); + const [busy, setBusy] = useState(false); + const canRun = !disabled && !busy && !!(folderId || '').trim(); + + const run = useCallback(async () => { + setBusy(true); + try { + const res = await apiCore.post('/cl2k-maker/gdrive/type-subfolders', { + gdrive_folder_id: folderId, + }); + const subfolders = res?.data?.subfolders || []; + if (subfolders.length === 0) throw new Error('No subfolders were returned'); + onSplit(subfolders); + toast.success(res?.message || 'Split into type subfolders'); + } catch (e) { + toast.error(e?.message || 'Could not create the type subfolders'); + } finally { + setBusy(false); + } + }, [folderId, toast, onSplit]); + + return ( + + ); +}; + const DriveEntry = ({ entry, disabled, onPatch, onDelete, onToggleType, + onSplit, autoFocus, clearFocus, }) => { @@ -431,6 +475,11 @@ const DriveEntry = ({ aria-label="Drive folder ID" /> + const clearFocus = useCallback(() => setFocusIndex(null), [setFocusIndex]); + // Replace the split row with one routed row per type. The parent row's own + // claimed types are dropped — they now live on the children — and any type + // it didn't claim is left unclaimed rather than silently switched on. + const splitRow = useCallback( + (index, subfolders) => { + const parent = entries[index] || {}; + const claimed = parent.types || []; + const children = subfolders + .filter(s => claimed.length === 0 || claimed.includes(s.image_type)) + .map(s => ({ + name: `${parent.name || 'Drive'} ${s.name}`.trim(), + folder_id: s.folder_id, + types: [s.image_type], + })); + if (children.length === 0) return; + onChange(entries.flatMap((e, i) => (i === index ? children : [e]))); + }, + [entries, onChange] + ); + return (
onPatch={changes => patch(i, changes)} onDelete={() => remove(i)} onToggleType={type => toggleType(i, type)} + onSplit={subfolders => splitRow(i, subfolders)} autoFocus={focusIndex === i} clearFocus={clearFocus} /> diff --git a/tests/test_cl2k_type_subfolders.py b/tests/test_cl2k_type_subfolders.py new file mode 100644 index 00000000..4af38cf8 --- /dev/null +++ b/tests/test_cl2k_type_subfolders.py @@ -0,0 +1,152 @@ +"""ensure_type_subfolders — split a parent Drive folder into the artwork layout. + +Every rclone call is stubbed; these pin the contract (create only what's missing, +return real ids, never touch existing content) without going near Drive. +""" + +import json + +import pytest + +from backend.util.cl2k import gdrive_upload + + +class _Result: + def __init__(self, returncode=0, stdout="", stderr=""): + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + + +class _Sync: + gdrive_sa_location = None + token = '{"access_token":"x","refresh_token":"y"}' + client_id = "" + client_secret = "" + + +class _Log: + def info(self, *a, **kw): + pass + + def warning(self, *a, **kw): + pass + + def debug(self, *a, **kw): + pass + + +def _dirs(*names): + return json.dumps([{"Name": n, "ID": f"ID_{n}"} for n in names]) + + +@pytest.fixture +def rclone(monkeypatch): + """Record every rclone invocation and script its output.""" + calls = [] + scripted = {"lsjson": [], "mkdir": _Result()} + + def fake_run(cmd, **kwargs): + calls.append(cmd) + verb = cmd[1] + if verb == "lsjson": + queue = scripted["lsjson"] + return queue.pop(0) if queue else _Result(stdout="[]") + if verb == "mkdir": + return scripted["mkdir"] + return _Result() + + monkeypatch.setattr(gdrive_upload.subprocess, "run", fake_run) + monkeypatch.setattr(gdrive_upload, "_rclone_path", lambda: "/usr/bin/rclone") + monkeypatch.setattr(gdrive_upload, "_ensure_remote", lambda *a, **kw: None) + return {"calls": calls, "scripted": scripted} + + +def test_creates_all_three_when_the_parent_is_empty(rclone): + rclone["scripted"]["lsjson"] = [ + _Result(stdout="[]"), # first listing: nothing there + _Result(stdout=_dirs("logos", "backgrounds", "squareart")), # after mkdir + ] + out = gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + assert [s["image_type"] for s in out] == ["logo", "background", "squareart"] + assert [s["name"] for s in out] == ["logos", "backgrounds", "squareart"] + assert all(s["created"] for s in out) + assert {s["folder_id"] for s in out} == {"ID_logos", "ID_backgrounds", "ID_squareart"} + mkdirs = [c for c in rclone["calls"] if c[1] == "mkdir"] + assert len(mkdirs) == 3 + + +def test_reuses_existing_subfolders_and_creates_none(rclone): + rclone["scripted"]["lsjson"] = [_Result(stdout=_dirs("logos", "backgrounds", "squareart"))] + out = gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + assert not any(s["created"] for s in out) + assert [c for c in rclone["calls"] if c[1] == "mkdir"] == [] + # Only ONE listing when nothing was created — no pointless second round-trip. + assert len([c for c in rclone["calls"] if c[1] == "lsjson"]) == 1 + + +def test_creates_only_the_missing_one(rclone): + rclone["scripted"]["lsjson"] = [ + _Result(stdout=_dirs("logos", "backgrounds")), + _Result(stdout=_dirs("logos", "backgrounds", "squareart")), + ] + out = gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + created = [s["name"] for s in out if s["created"]] + assert created == ["squareart"] + mkdirs = [c for c in rclone["calls"] if c[1] == "mkdir"] + assert len(mkdirs) == 1 + assert "posters:squareart" in mkdirs[0] + + +def test_unrelated_sibling_folders_are_left_alone(rclone): + """A parent holding other folders must not be reorganised — only add ours.""" + rclone["scripted"]["lsjson"] = [ + _Result(stdout=_dirs("posters", "archive")), + _Result(stdout=_dirs("posters", "archive", "logos", "backgrounds", "squareart")), + ] + out = gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + assert len(out) == 3 + made = {c[2] for c in rclone["calls"] if c[1] == "mkdir"} + assert made == {"posters:logos", "posters:backgrounds", "posters:squareart"} + # Nothing destructive is ever issued. + verbs = {c[1] for c in rclone["calls"]} + assert verbs.isdisjoint({"delete", "deletefile", "purge", "move", "moveto", "sync"}) + + +def test_raises_when_mkdir_fails(rclone): + rclone["scripted"]["lsjson"] = [_Result(stdout="[]")] + rclone["scripted"]["mkdir"] = _Result(returncode=1, stderr="quota exceeded") + with pytest.raises(RuntimeError, match="mkdir"): + gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + +def test_raises_when_listing_fails(rclone): + rclone["scripted"]["lsjson"] = [_Result(returncode=1, stderr="not found")] + with pytest.raises(RuntimeError, match="lsjson"): + gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + +def test_raises_when_drive_returns_no_id_for_a_subfolder(rclone): + """Fail loudly rather than storing a blank id that would upload to the root.""" + rclone["scripted"]["lsjson"] = [ + _Result(stdout="[]"), + _Result(stdout=_dirs("logos", "backgrounds")), # squareart never came back + ] + with pytest.raises(RuntimeError, match="squareart"): + gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + +def test_rejects_an_unsafe_parent_id(rclone): + with pytest.raises(ValueError): + gdrive_upload.ensure_type_subfolders("--drive-config=/etc/passwd", _Sync(), _Log()) + assert rclone["calls"] == [] # refused before any rclone ran + + +def test_raises_without_an_upload_token(monkeypatch, rclone): + monkeypatch.setattr(gdrive_upload, "_upload_auth_args", lambda cfg: []) + with pytest.raises(RuntimeError, match="OAuth token"): + gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) From 6801f480885100d08560c7ab481900914214ea02 Mon Sep 17 00:00:00 2001 From: chodeus Date: Fri, 7 Aug 2026 16:42:40 +0800 Subject: [PATCH 2/2] fix(cl2k): reject subfolders without a usable id, and never route unclaimed types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects this PR introduced. The completeness check only asked whether the NAME was present in the id map, so a record like {"Name": "logos", "ID": ""} passed and produced a routed row with an empty folder_id. _drive_targets skips a blank id, so that art type would have stopped uploading with no error anywhere. Blank, absent and non-string ids are now filtered out before the check, which makes the existing "did not return an id" guard actually catch them. Parametrised over "", " ", None and a non-string — all four verified red against the old check. The split filter treated an empty parent.types as claiming every type, so a row claiming nothing split into all three routed rows — the opposite of what this PR's own description promises. Only claimed types now carry to children; an unclaimed row produces no children and is left untouched. The button also refuses to rewrite the row unless every art type came back with a non-empty folder_id, so a partial answer can't be saved. --- backend/util/cl2k/gdrive_upload.py | 10 ++++++++- .../extensions/cl2k/SaveLocationsFields.jsx | 13 ++++++++++-- tests/test_cl2k_type_subfolders.py | 21 +++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/backend/util/cl2k/gdrive_upload.py b/backend/util/cl2k/gdrive_upload.py index a393dfc3..f309432d 100644 --- a/backend/util/cl2k/gdrive_upload.py +++ b/backend/util/cl2k/gdrive_upload.py @@ -301,7 +301,15 @@ def _dir_ids() -> dict: raise RuntimeError( f"rclone lsjson failed: {_rclone_error_detail(result.stderr)}" ) - return {d["Name"]: d["ID"] for d in json.loads(result.stdout or "[]")} + # Keep only usable ids: a blank/absent/non-string ID must NOT register the + # name, or the completeness check below passes and we hand back a routed + # row whose folder_id is empty — which _drive_targets skips silently, so + # that art type would just stop uploading with no error anywhere. + return { + d["Name"]: d["ID"] + for d in json.loads(result.stdout or "[]") + if isinstance(d.get("ID"), str) and d["ID"].strip() + } existing = _dir_ids() made = [] diff --git a/frontend/src/extensions/cl2k/SaveLocationsFields.jsx b/frontend/src/extensions/cl2k/SaveLocationsFields.jsx index a5ffb854..822514a1 100644 --- a/frontend/src/extensions/cl2k/SaveLocationsFields.jsx +++ b/frontend/src/extensions/cl2k/SaveLocationsFields.jsx @@ -413,7 +413,14 @@ const SplitSubfoldersButton = ({ folderId, disabled, onSplit }) => { gdrive_folder_id: folderId, }); const subfolders = res?.data?.subfolders || []; - if (subfolders.length === 0) throw new Error('No subfolders were returned'); + // Refuse to rewrite the row on a partial answer: a missing type or a + // blank folder_id would save a destination that silently uploads + // nothing (_drive_targets skips a blank id). + const usable = + CL2K_ART_TYPES.filter(t => t.value !== 'poster').every(t => + subfolders.some(s => s.image_type === t.value && (s.folder_id || '').trim()) + ) && subfolders.every(s => (s.folder_id || '').trim()); + if (!usable) throw new Error('Drive returned an incomplete set of subfolders'); onSplit(subfolders); toast.success(res?.message || 'Split into type subfolders'); } catch (e) { @@ -512,7 +519,9 @@ export const Cl2kGdriveUploadsField = ({ value, onChange, disabled = false }) => const parent = entries[index] || {}; const claimed = parent.types || []; const children = subfolders - .filter(s => claimed.length === 0 || claimed.includes(s.image_type)) + // Only types the parent actually claimed. An unclaimed row splits + // into nothing and is left untouched by the empty-children return. + .filter(s => claimed.includes(s.image_type)) .map(s => ({ name: `${parent.name || 'Drive'} ${s.name}`.trim(), folder_id: s.folder_id, diff --git a/tests/test_cl2k_type_subfolders.py b/tests/test_cl2k_type_subfolders.py index 4af38cf8..c564b856 100644 --- a/tests/test_cl2k_type_subfolders.py +++ b/tests/test_cl2k_type_subfolders.py @@ -140,6 +140,27 @@ def test_raises_when_drive_returns_no_id_for_a_subfolder(rclone): gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) +@pytest.mark.parametrize("bad_id", ["", " ", None, 12345]) +def test_raises_when_drive_returns_a_subfolder_without_a_usable_id(rclone, bad_id): + """A present NAME with a blank/absent/non-string ID must fail loudly. + + The earlier guard only checked the name was present, so a record like + {"Name": "logos", "ID": ""} sailed through and produced a routed row with an + empty folder_id — which _drive_targets skips silently, so that art type would + simply stop uploading with no error anywhere. + """ + listing = json.dumps( + [ + {"Name": "logos", "ID": bad_id}, + {"Name": "backgrounds", "ID": "ID_backgrounds"}, + {"Name": "squareart", "ID": "ID_squareart"}, + ] + ) + rclone["scripted"]["lsjson"] = [_Result(stdout=listing), _Result(stdout=listing)] + with pytest.raises(RuntimeError, match="logos"): + gdrive_upload.ensure_type_subfolders("PARENT", _Sync(), _Log()) + + def test_rejects_an_unsafe_parent_id(rclone): with pytest.raises(ValueError): gdrive_upload.ensure_type_subfolders("--drive-config=/etc/passwd", _Sync(), _Log())