diff --git a/app/src/api.js b/app/src/api.js index 884e9b38..a91248c0 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -247,19 +247,19 @@ export async function getVideoCaptions(fileGroupId) { } } -export async function deleteVideos(fileGroupIds, force = false) { +export async function deleteFileGroups(fileGroupIds, force = false) { if (!fileGroupIds || fileGroupIds.length === 0) { toast({ type: 'error', title: 'Empty request', - description: 'Unable to delete Videos because no IDs were passed.', + description: 'Unable to delete FileGroups because no IDs were passed.', time: 5000, }); + return; } - console.info(`Deleting Videos: ${fileGroupIds}`); - const i = fileGroupIds.join(','); - const url = force ? `${VIDEOS_API}/${i}?force=true` : `${VIDEOS_API}/${i}`; - const response = await apiDelete(url); + console.info(`Deleting FileGroups: ${fileGroupIds}`); + const body = {file_group_ids: fileGroupIds, force}; + const response = await apiPost(`${API_URI}/files/delete_groups`, body); if (response.ok) { return response; } @@ -271,10 +271,10 @@ export async function deleteVideos(fileGroupIds, force = false) { } catch (e) { // Not JSON, fall through to generic error. } - const message = await getErrorMessage(response, 'Failed to delete videos.'); + const message = await getErrorMessage(response, 'Failed to delete files.'); toast({ type: 'error', - title: 'Deleting Videos failed', + title: 'Delete failed', description: message, time: 5000, }); @@ -751,36 +751,6 @@ export async function deleteItems(itemIds) { return response } -export async function deleteArchives(archiveIds, force = false) { - if (!archiveIds || archiveIds.length === 0) { - toast({ - type: 'error', - title: 'Empty request', - description: 'Unable to delete Archives because no IDs were passed.', - time: 5000, - }); - } - console.log(`Deleting Archives: ${archiveIds}`); - let i = archiveIds.join(','); - const url = force ? `${ARCHIVES_API}/${i}?force=true` : `${ARCHIVES_API}/${i}`; - const response = await apiDelete(url); - if (response.ok) { - return response; - } - try { - const json = await response.clone().json(); - if (json.code === 'FILE_GROUP_IS_TAGGED' && json.file_groups) { - return {tagged: true, file_groups: json.file_groups}; - } - } catch (e) { - // Not JSON, fall through to generic error. - } - const message = await getErrorMessage(response, 'Failed to delete archives'); - toast({ - type: 'error', title: 'Unexpected server response', description: message, time: 5000, - }); - throw Error('Failed to delete archives'); -} export async function searchArchives(offset, limit, domain, searchStr, order, tagNames, headline) { // Build a search query to retrieve a list of videos from the API @@ -2253,30 +2223,6 @@ export async function getDoc(fileGroupId) { return await apiGet(`${DOCS_API}/${fileGroupId}`); } -export async function deleteDocs(docIds, force = false) { - const ids = Array.isArray(docIds) ? docIds.join(',') : docIds; - const url = force ? `${DOCS_API}/${ids}?force=true` : `${DOCS_API}/${ids}`; - const response = await apiDelete(url); - if (response.ok) { - return response; - } - try { - const json = await response.clone().json(); - if (json.code === 'FILE_GROUP_IS_TAGGED' && json.file_groups) { - return {tagged: true, file_groups: json.file_groups}; - } - } catch (e) { - // Not JSON, fall through to generic error. - } - const message = await getErrorMessage(response, 'Failed to delete documents.'); - toast({ - type: 'error', - title: 'Deleting Documents failed', - description: message, - time: 5000, - }); - throw Error(message); -} export async function searchDocs(offset, limit, searchStr, order, tagNames, author, subject, mimetype) { offset = parseInt(offset || 0); diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index 90d450c7..0e726d7f 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -44,8 +44,8 @@ import { useTitle } from "./Common"; import { - deleteArchives, deleteDomain, + deleteFileGroups, fetchArchiveBrowsers, fetchArchiveDownloaderConfig, generateArchiveScreenshot, @@ -135,7 +135,7 @@ function ArchivePage() { null; const localDeleteArchive = async (force = false) => { - const result = await deleteArchives([archiveFile.id], force); + const result = await deleteFileGroups([archiveFile.id], force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; @@ -1149,13 +1149,13 @@ function ArchivesPage() { } } - const archiveIdsForSelection = () => archives + const fileGroupIdsForSelection = () => archives .filter(i => selectedArchives.indexOf(i['primary_path']) >= 0) .map(i => i['id']); const onDelete = async (force = false) => { - const archiveIds = archiveIdsForSelection(); - const result = await deleteArchives(archiveIds, force); + const fileGroupIds = fileGroupIdsForSelection(); + const result = await deleteFileGroups(fileGroupIds, force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; diff --git a/app/src/components/Docs.js b/app/src/components/Docs.js index 21b90dd1..578c6e3c 100644 --- a/app/src/components/Docs.js +++ b/app/src/components/Docs.js @@ -1,7 +1,7 @@ import React, {useContext, useEffect, useState} from 'react'; import {Link, Route, Routes, useLocation, useNavigate, useParams, useSearchParams} from "react-router"; import {Grid, Icon as SIcon, Loader, StatisticLabel, StatisticValue} from "semantic-ui-react"; -import {deleteDocs, getDocStatistics, tagFileGroup, untagFileGroup} from "../api"; +import {deleteFileGroups, getDocStatistics, tagFileGroup, untagFileGroup} from "../api"; import {Media, ThemeContext} from "../contexts/contexts"; import { APIButton, @@ -59,8 +59,8 @@ function DocsPage() { } const onDelete = async (force = false) => { - const docIds = docs.filter(i => selectedDocs.indexOf(i['primary_path']) >= 0).map(i => i['id']); - const result = await deleteDocs(docIds, force); + const fileGroupIds = docs.filter(i => selectedDocs.indexOf(i['primary_path']) >= 0).map(i => i['id']); + const result = await deleteFileGroups(fileGroupIds, force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; @@ -262,7 +262,7 @@ function DocPage() { const handleDelete = async (force = false) => { if (doc && doc.id) { - const result = await deleteDocs([docFile.id], force); + const result = await deleteFileGroups([docFile.id], force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; diff --git a/app/src/components/VideoPlayer.js b/app/src/components/VideoPlayer.js index cd98591f..734d2909 100644 --- a/app/src/components/VideoPlayer.js +++ b/app/src/components/VideoPlayer.js @@ -1,6 +1,6 @@ import React, {useContext, useEffect, useState} from 'react'; import {useHotkeys} from 'react-hotkeys-hook'; -import {deleteVideos, downloadVideoMetadata, tagFileGroup, untagFileGroup} from "../api"; +import {deleteFileGroups, downloadVideoMetadata, tagFileGroup, untagFileGroup} from "../api"; import {Link, useNavigate, useParams} from "react-router"; import _ from "lodash"; import { @@ -278,8 +278,8 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) { return
Could not find video
; } - const handleDeleteVideo = async (video_id, force = false) => { - const result = await deleteVideos([video_id], force); + const handleDeleteVideo = async (fileGroupId, force = false) => { + const result = await deleteFileGroups([fileGroupId], force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; diff --git a/app/src/components/Videos.js b/app/src/components/Videos.js index 8283dc44..26ce1285 100644 --- a/app/src/components/Videos.js +++ b/app/src/components/Videos.js @@ -54,7 +54,7 @@ import {Button, Card, Header, Loader, Modal, Placeholder, Popup, Segment, Statis import {BulkTagModal} from "./BulkTagModal"; import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; import { - deleteVideos, + deleteFileGroups, deleteCookies, fetchSuggestedUserAgent, fetchVideoDownloaderConfig, @@ -160,7 +160,7 @@ export function VideosPage() { const onDelete = async (force = false) => { const fileGroupIds = videos.filter(i => selectedVideos.indexOf(i['primary_path']) >= 0).map(i => i['id']); - const result = await deleteVideos(fileGroupIds, force); + const result = await deleteFileGroups(fileGroupIds, force); if (result && result.tagged) { setTaggedFileGroups(result.file_groups); return; diff --git a/app/src/components/collections/ConflictResolutionModal.js b/app/src/components/collections/ConflictResolutionModal.js index f55c875c..ee8fac74 100644 --- a/app/src/components/collections/ConflictResolutionModal.js +++ b/app/src/components/collections/ConflictResolutionModal.js @@ -2,7 +2,7 @@ import React, {useEffect, useState} from 'react'; import {Card, Grid, Icon, Image, Label, Segment, Button as SButton} from 'semantic-ui-react'; import Message from 'semantic-ui-react/dist/commonjs/collections/Message'; import {Button, Modal} from '../Theme'; -import {deleteArchives, deleteVideos} from '../../api'; +import {deleteFileGroups} from '../../api'; import {humanFileSize, isoDatetimeToString} from '../Common'; import {TaggedDeleteConfirmModal} from '../TaggedDeleteConfirmModal'; @@ -32,10 +32,8 @@ function ConflictFileCard({file, onDelete, isRecommended}) { const [taggedFileGroups, setTaggedFileGroups] = useState(null); const performDelete = async (force) => { - if (file.model_type === 'video' && file.file_group_id) { - return await deleteVideos([file.file_group_id], force); - } else if (file.model_type === 'archive' && file.archive_id) { - return await deleteArchives([file.archive_id], force); + if (file.file_group_id) { + return await deleteFileGroups([file.file_group_id], force); } return null; }; diff --git a/app/src/components/collections/ConflictResolutionModal.test.js b/app/src/components/collections/ConflictResolutionModal.test.js index 1c68afea..f24dfaa0 100644 --- a/app/src/components/collections/ConflictResolutionModal.test.js +++ b/app/src/components/collections/ConflictResolutionModal.test.js @@ -6,8 +6,7 @@ import * as api from '../../api'; // Mock the API functions jest.mock('../../api', () => ({ - deleteVideos: jest.fn(), - deleteArchives: jest.fn(), + deleteFileGroups: jest.fn(), })); // Mock react-semantic-toasts-2 @@ -97,8 +96,8 @@ describe('ConflictResolutionModal', () => { expect(screen.getByText('2.0 MB')).toBeInTheDocument(); }); - it('calls deleteVideos when delete button is clicked for video', async () => { - api.deleteVideos.mockResolvedValue({}); + it('calls deleteFileGroups when delete button is clicked for video', async () => { + api.deleteFileGroups.mockResolvedValue({}); renderWithProviders(); @@ -107,7 +106,7 @@ describe('ConflictResolutionModal', () => { fireEvent.click(deleteButtons[0]); await waitFor(() => { - expect(api.deleteVideos).toHaveBeenCalledWith([1], false); + expect(api.deleteFileGroups).toHaveBeenCalledWith([1], false); }); // File should be removed from UI (conflict resolved since only 1 file remains) @@ -123,7 +122,7 @@ describe('ConflictResolutionModal', () => { expect(defaultProps.onResolved).toHaveBeenCalled(); }); - it('calls deleteArchives when delete button is clicked for archive', async () => { + it('calls deleteFileGroups when delete button is clicked for archive', async () => { const archiveConflicts = [ { destination_path: 'archives/example.com/page.html', @@ -143,7 +142,7 @@ describe('ConflictResolutionModal', () => { }, ]; - api.deleteArchives.mockResolvedValue({}); + api.deleteFileGroups.mockResolvedValue({}); renderWithProviders( { fireEvent.click(deleteButton); await waitFor(() => { - expect(api.deleteArchives).toHaveBeenCalledWith([201], false); + // Unified endpoint always uses FileGroup IDs. + expect(api.deleteFileGroups).toHaveBeenCalledWith([10], false); }); }); @@ -319,7 +319,7 @@ describe('ConflictResolutionModal', () => { }); it('shows tagged-delete confirmation modal when API returns FILE_GROUP_IS_TAGGED', async () => { - api.deleteVideos.mockResolvedValue({ + api.deleteFileGroups.mockResolvedValue({ tagged: true, file_groups: [ {id: 1, primary_path: 'unique-tagged-path-12345.mp4', tags: ['unique-tag-zxcv']}, @@ -338,8 +338,8 @@ describe('ConflictResolutionModal', () => { }); expect(screen.getByText('unique-tag-zxcv')).toBeInTheDocument(); - // Confirming should re-call deleteVideos with force=true. - api.deleteVideos.mockResolvedValueOnce({}); + // Confirming should re-call deleteFileGroups with force=true. + api.deleteFileGroups.mockResolvedValueOnce({}); // The TaggedDeleteConfirmModal's Delete button has the trash icon. const confirmButton = screen.getAllByRole('button') .find(b => b.classList.contains('red') && b.textContent.includes('Delete') @@ -347,7 +347,7 @@ describe('ConflictResolutionModal', () => { fireEvent.click(confirmButton); await waitFor(() => { - expect(api.deleteVideos).toHaveBeenLastCalledWith([1], true); + expect(api.deleteFileGroups).toHaveBeenLastCalledWith([1], true); }); }); diff --git a/modules/archive/api.py b/modules/archive/api.py index 706d01cb..506ecaea 100644 --- a/modules/archive/api.py +++ b/modules/archive/api.py @@ -11,7 +11,6 @@ from wrolpi.api_utils import json_response, api_app from wrolpi.cmd import get_installed_browsers from wrolpi.common import logger, wrol_mode_check, api_param_limiter, TRACE_LEVEL from wrolpi.db import get_db_session -from wrolpi.errors import ValidationError from wrolpi.events import Events from wrolpi.schema import JSONErrorResponse from wrolpi.switches import register_switch_handler, ActivateSwitchMethod @@ -44,26 +43,6 @@ async def get_archive(request: Request, file_group_id: int): return json_response({'file_group': archive_file_group, 'history': history}) -@archive_bp.delete('/', name='archive_delete_one') -@archive_bp.delete('/', name='archive_delete_many') -@openapi.description('Delete Archives by FileGroup IDs. Pass ?force=true to delete even tagged FileGroups.') -@openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) -@wrol_mode_check -async def delete_archive(request: Request, file_group_ids: str): - try: - file_group_ids = [int(i) for i in str(file_group_ids).split(',')] - except ValueError: - raise ValidationError('Could not parse file group ids') - force = request.args.get('force', '').lower() == 'true' - tagged = lib.delete_archives_by_file_group_ids(*file_group_ids, force=force) - if tagged: - return json_response( - {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, - HTTPStatus.CONFLICT, - ) - return response.empty() - - archive_limit_limiter = api_param_limiter(100) diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 91c0ec55..b07f4cd6 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -27,10 +27,10 @@ from wrolpi.dates import now, Seconds from wrolpi.db import get_db_session, get_db_curs from wrolpi.errors import UnknownArchive, InvalidOrderBy, InvalidDatetime from wrolpi.events import Events -from wrolpi.files.lib import handle_file_group_search_results, get_tagged_file_groups_by_ids +from wrolpi.files.lib import handle_file_group_search_results from wrolpi.files.models import FileGroup from wrolpi.switches import register_switch_handler, ActivateSwitchMethod -from wrolpi.tags import tag_append_sub_select_where, save_tags_config +from wrolpi.tags import tag_append_sub_select_where from wrolpi.vars import PYTEST, DOCKERIZED logger = logger.getChild(__name__) @@ -1125,29 +1125,16 @@ def get_archive_by_file_group_id(session: Session, file_group_id: int, skip_view return archive -def delete_archives_by_file_group_ids(*file_group_ids: int, force: bool = False) -> Optional[List[dict]]: - """Delete Archives by their FileGroup IDs and all of their files. - - If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups - without deleting anything. If force is True, deletes even tagged files and re-saves the tags - config so `tags.yaml` reflects the removed TagFiles. - """ +def delete_archives_by_file_group_ids(*file_group_ids: int, force: bool = False) -> None: + """Delete Archives by their FileGroup IDs and all of their files. Pass force=True to delete + even tagged FileGroups (raises FileGroupIsTagged otherwise via FileGroup.delete).""" with get_db_session(commit=True) as session: archives: List[Archive] = list(session.query(Archive).filter(Archive.file_group_id.in_(file_group_ids))) if not archives: raise UnknownArchive(f'Unknown Archives with FileGroup IDs: {", ".join(map(str, file_group_ids))}') - - tagged = get_tagged_file_groups_by_ids(session, [a.file_group_id for a in archives]) - if tagged and not force: - return tagged - for archive in archives: archive.delete(force=force) - if tagged: - save_tags_config.activate_switch() - return None - def archive_strftime(dt: datetime) -> str: return dt.strftime('%Y-%m-%d-%H-%M-%S') diff --git a/modules/archive/test/test_api.py b/modules/archive/test/test_api.py index f6f0a710..05f434c5 100644 --- a/modules/archive/test/test_api.py +++ b/modules/archive/test/test_api.py @@ -514,13 +514,14 @@ async def test_archive_browsers_endpoint_dockerized(async_client): @pytest.mark.asyncio async def test_delete_archive_untagged(async_client, test_session, archive_factory): - """An untagged Archive can be deleted via DELETE /api/archive/ without force.""" + """An untagged Archive can be deleted via the unified delete endpoint without force.""" archive = archive_factory('example.com', 'https://example.com/1') test_session.commit() paths = list(archive.my_paths()) assert paths and all(p.is_file() for p in paths) - request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}') + body = json.dumps({'file_group_ids': [archive.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT assert all(not p.exists() for p in paths) assert test_session.query(Archive).count() == 0 @@ -535,7 +536,8 @@ async def test_delete_archive_tagged_requires_force(async_client, test_session, test_session.commit() paths = list(archive.my_paths()) - request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}') + body = json.dumps({'file_group_ids': [archive.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' assert len(response.json['file_groups']) == 1 @@ -547,7 +549,8 @@ async def test_delete_archive_tagged_requires_force(async_client, test_session, assert test_session.query(Archive).count() == 1 # With force=true, the Archive is deleted. - request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}?force=true') + body = json.dumps({'file_group_ids': [archive.file_group_id], 'force': True}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT assert all(not p.exists() for p in paths) assert test_session.query(Archive).count() == 0 @@ -564,8 +567,8 @@ async def test_delete_archive_atomic_when_some_tagged(async_client, test_session untagged_paths = list(untagged.my_paths()) tagged_paths = list(tagged.my_paths()) - ids = f'{untagged.file_group_id},{tagged.file_group_id}' - request, response = await async_client.delete(f'/api/archive/{ids}') + body = json.dumps({'file_group_ids': [untagged.file_group_id, tagged.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' # Only the tagged FileGroup is in the response. diff --git a/modules/docs/api.py b/modules/docs/api.py index 646a55aa..c69c1cb4 100644 --- a/modules/docs/api.py +++ b/modules/docs/api.py @@ -1,15 +1,14 @@ from http import HTTPStatus -from sanic import response, Request, Blueprint +from sanic import Request, Blueprint from sanic_ext import validate from sanic_ext.extensions.openapi import openapi from wrolpi.api_utils import json_response -from wrolpi.common import logger, wrol_mode_check, api_param_limiter -from wrolpi.errors import ValidationError +from wrolpi.common import logger, api_param_limiter from wrolpi.schema import JSONErrorResponse from . import schema -from .lib import get_statistics, _doc_response, _get_doc, _delete_docs, _search_docs +from .lib import get_statistics, _doc_response, _get_doc, _search_docs NAME = 'docs' @@ -34,26 +33,6 @@ async def get_doc(request: Request, file_group_id: int): return json_response(_doc_response(doc)) -@docs_bp.delete('/', name='doc_delete_one') -@docs_bp.delete('/', name='doc_delete_many') -@openapi.description('Delete Docs by FileGroup IDs. Pass ?force=true to delete even tagged FileGroups.') -@openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) -@wrol_mode_check -async def delete_docs(request: Request, file_group_ids: str): - try: - file_group_ids = [int(i) for i in str(file_group_ids).split(',')] - except ValueError: - raise ValidationError('Could not parse file group ids') - force = request.args.get('force', '').lower() == 'true' - tagged = _delete_docs(*file_group_ids, force=force) - if tagged: - return json_response( - {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, - HTTPStatus.CONFLICT, - ) - return response.empty() - - doc_limit_limiter = api_param_limiter(100) diff --git a/modules/docs/lib.py b/modules/docs/lib.py index 384306dc..708b3d70 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -11,8 +11,7 @@ from wrolpi.collections.models import Collection from wrolpi.collections.types import collection_type_registry from wrolpi.db import get_db_session from wrolpi.errors import ValidationError -from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype, get_tagged_file_groups_by_ids -from wrolpi.tags import save_tags_config +from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype logger = logging.getLogger(__name__) @@ -347,35 +346,6 @@ def _get_doc(session, file_group_id: int): return doc -def _delete_docs(*file_group_ids, force: bool = False) -> Optional[List[dict]]: - """Delete Docs by their FileGroup IDs and all of their files. - - If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups - without deleting anything. If force is True, deletes even tagged files and re-saves the tags - config so `tags.yaml` reflects the removed TagFiles. - """ - from .models import Doc - with get_db_session(commit=True) as session: - docs = list(session.query(Doc).filter(Doc.file_group_id.in_(file_group_ids))) - if not docs: - raise ValidationError( - f'Could not find Docs to delete with file_group_ids: {", ".join(map(str, file_group_ids))}' - ) - - tagged = get_tagged_file_groups_by_ids(session, [d.file_group_id for d in docs]) - if tagged and not force: - return tagged - - # Delete the FileGroup, which removes the files on disk and cascades the Doc row via - # the ON DELETE CASCADE foreign key on Doc.file_group_id. - for doc in docs: - doc.file_group.delete(force=force) - - if tagged: - save_tags_config.activate_switch() - return None - - def _search_docs(search_str=None, author=None, subject=None, language=None, mimetype=None, limit=20, offset=0, order_by='published_datetime', tag_names=None): from .models import Doc diff --git a/modules/docs/models.py b/modules/docs/models.py index da629063..73432573 100644 --- a/modules/docs/models.py +++ b/modules/docs/models.py @@ -76,6 +76,10 @@ class Doc(ModelHelper, Base): d['data']['ebook_path'] = self.file_group.resolve_path(d['data']['ebook_path']) return d + @staticmethod + def get_by_path(session: Session, path) -> Optional['Doc']: + return session.query(Doc).join(FileGroup).filter(FileGroup.primary_path == str(path)).one_or_none() + @staticmethod def can_model(file_group: FileGroup) -> bool: if file_group.mimetype and mimetype_is_doc(file_group.mimetype): diff --git a/modules/docs/test/test_api.py b/modules/docs/test/test_api.py index 34a1aa88..0ffc7204 100644 --- a/modules/docs/test/test_api.py +++ b/modules/docs/test/test_api.py @@ -87,7 +87,8 @@ async def test_delete_doc(async_client, test_session, test_directory, example_ep paths = list(doc.file_group.my_paths()) assert paths and all(p.is_file() for p in paths) - request, response = await async_client.delete(f'/api/docs/{file_group_id}') + body = json.dumps({'file_group_ids': [file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT # Doc, FileGroup, and on-disk files all removed. @@ -99,7 +100,7 @@ async def test_delete_doc(async_client, test_session, test_directory, example_ep @pytest.mark.asyncio async def test_delete_doc_tagged_requires_force(async_client, test_session, doc_factory, tag_factory, await_switches): - """DELETE /api/docs/ returns 409 for tagged docs until ?force=true is passed.""" + """The unified delete endpoint returns 409 for tagged docs until force=true is passed.""" from wrolpi.files.models import FileGroup tag = await tag_factory() doc = doc_factory() @@ -108,7 +109,8 @@ async def test_delete_doc_tagged_requires_force(async_client, test_session, doc_ file_group_id = doc.file_group_id paths = list(doc.file_group.my_paths()) - request, response = await async_client.delete(f'/api/docs/{file_group_id}') + body = json.dumps({'file_group_ids': [file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' assert len(response.json['file_groups']) == 1 @@ -119,7 +121,8 @@ async def test_delete_doc_tagged_requires_force(async_client, test_session, doc_ assert test_session.query(Doc).count() == 1 # With force=true, the Doc is deleted. - request, response = await async_client.delete(f'/api/docs/{file_group_id}?force=true') + body = json.dumps({'file_group_ids': [file_group_id], 'force': True}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT assert all(not p.exists() for p in paths) assert test_session.query(Doc).count() == 0 @@ -129,7 +132,7 @@ async def test_delete_doc_tagged_requires_force(async_client, test_session, doc_ @pytest.mark.asyncio async def test_delete_doc_atomic_when_some_tagged(async_client, test_session, doc_factory, tag_factory): - """When deleting multiple Docs and any are tagged, NONE are deleted until ?force=true.""" + """When deleting multiple Docs and any are tagged, NONE are deleted until force=true.""" untagged = doc_factory() tagged = doc_factory() tag = await tag_factory() @@ -138,8 +141,8 @@ async def test_delete_doc_atomic_when_some_tagged(async_client, test_session, do untagged_paths = list(untagged.file_group.my_paths()) tagged_paths = list(tagged.file_group.my_paths()) - ids = f'{untagged.file_group_id},{tagged.file_group_id}' - request, response = await async_client.delete(f'/api/docs/{ids}') + body = json.dumps({'file_group_ids': [untagged.file_group_id, tagged.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' returned_ids = {fg['id'] for fg in response.json['file_groups']} diff --git a/modules/videos/test/test_api.py b/modules/videos/test/test_api.py index e88ca711..5f6c2b07 100644 --- a/modules/videos/test/test_api.py +++ b/modules/videos/test/test_api.py @@ -307,7 +307,8 @@ async def test_delete_video(async_client, test_session, video_factory, test_down files = video.file_group.my_paths() test_session.commit() - request, response = await async_client.delete(f'/api/videos/{video.file_group_id}') + body = json.dumps({'file_group_ids': [video.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT assert test_session.query(Video).count() == 0, 'Video was not deleted.' diff --git a/modules/videos/test/test_video.py b/modules/videos/test/test_video.py index ff41e5ac..426389dc 100644 --- a/modules/videos/test/test_video.py +++ b/modules/videos/test/test_video.py @@ -1,3 +1,4 @@ +import json from datetime import datetime, timezone from http import HTTPStatus @@ -49,7 +50,7 @@ async def test_delete_video_with_tag(async_client, test_session, video_factory, @pytest.mark.asyncio async def test_delete_video_api_tagged_requires_force(async_client, test_session, video_factory, tag_factory, await_switches): - """DELETE /api/videos/ returns 409 for tagged videos until ?force=true is passed.""" + """The unified delete endpoint returns 409 for tagged videos until force=true is passed.""" video = video_factory(with_video_file=True, with_poster_ext='png') tag = await tag_factory() video.add_tag(test_session, tag.name) @@ -57,7 +58,8 @@ async def test_delete_video_api_tagged_requires_force(async_client, test_session file_group_id = video.file_group_id paths = list(video.file_group.my_paths()) - request, response = await async_client.delete(f'/api/videos/{file_group_id}') + body = json.dumps({'file_group_ids': [file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' assert len(response.json['file_groups']) == 1 @@ -68,7 +70,8 @@ async def test_delete_video_api_tagged_requires_force(async_client, test_session assert test_session.query(Video).count() == 1 # With force=true, the Video is deleted. - request, response = await async_client.delete(f'/api/videos/{file_group_id}?force=true') + body = json.dumps({'file_group_ids': [file_group_id], 'force': True}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT assert all(not p.exists() for p in paths) assert test_session.query(Video).count() == 0 @@ -78,7 +81,7 @@ async def test_delete_video_api_tagged_requires_force(async_client, test_session @pytest.mark.asyncio async def test_delete_video_api_atomic_when_some_tagged(async_client, test_session, video_factory, tag_factory): - """When deleting multiple Videos and any are tagged, NONE are deleted until ?force=true.""" + """When deleting multiple Videos and any are tagged, NONE are deleted until force=true.""" untagged = video_factory(with_video_file=True) tagged = video_factory(with_video_file=True) tag = await tag_factory() @@ -87,8 +90,8 @@ async def test_delete_video_api_atomic_when_some_tagged(async_client, test_sessi untagged_paths = list(untagged.file_group.my_paths()) tagged_paths = list(tagged.file_group.my_paths()) - ids = f'{untagged.file_group_id},{tagged.file_group_id}' - request, response = await async_client.delete(f'/api/videos/{ids}') + body = json.dumps({'file_group_ids': [untagged.file_group_id, tagged.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.CONFLICT assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' returned_ids = {fg['id'] for fg in response.json['file_groups']} diff --git a/modules/videos/video/api.py b/modules/videos/video/api.py index 3aa9278d..ede8939e 100644 --- a/modules/videos/video/api.py +++ b/modules/videos/video/api.py @@ -1,18 +1,16 @@ from http import HTTPStatus -from sanic import response, Blueprint +from sanic import Blueprint from sanic.request import Request from sanic_ext import validate from sanic_ext.extensions.openapi import openapi from wrolpi.api_utils import json_response -from wrolpi.common import logger, wrol_mode_check -from wrolpi.errors import InvalidOrderBy, ValidationError -from wrolpi.events import Events +from wrolpi.common import logger +from wrolpi.errors import InvalidOrderBy from wrolpi.schema import JSONErrorResponse from . import lib from .. import schema -from ..lib import save_channels_config video_bp = Blueprint('Video', '/api/videos') @@ -73,27 +71,3 @@ async def search_videos(_: Request, body: schema.VideoSearchRequest): return json_response(ret) -@video_bp.delete('/', name='Video Delete Many') -@video_bp.delete('/', name='Video Delete One') -@openapi.description('Delete videos. Pass ?force=true to delete even tagged FileGroups.') -@openapi.response(HTTPStatus.NO_CONTENT) -@openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) -@wrol_mode_check -async def video_delete(request: Request, file_group_ids: str): - try: - file_group_ids = [int(i) for i in str(file_group_ids).split(',')] - except Exception: - raise ValidationError('Unable to parse file group ids') - - session = request.ctx.session - force = request.args.get('force', '').lower() == 'true' - tagged = lib.delete_videos(session, *file_group_ids, force=force) - if tagged: - return json_response( - {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, - HTTPStatus.CONFLICT, - ) - - save_channels_config.activate_switch() - Events.send_deleted(f'Deleted {len(file_group_ids)} videos') - return response.empty() diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index e82cb29d..1e7c02e8 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -12,12 +12,11 @@ from wrolpi.common import logger, limit_concurrent, wrol_mode_check from wrolpi.dates import now from wrolpi.db import get_db_session from wrolpi.downloader import download_manager -from wrolpi.files.lib import handle_file_group_search_results, get_tagged_file_groups_by_ids +from wrolpi.files.lib import handle_file_group_search_results from wrolpi.files.models import FileGroup -from wrolpi.tags import tag_append_sub_select_where, save_tags_config +from wrolpi.tags import tag_append_sub_select_where from wrolpi.vars import VIDEO_COMMENTS_FETCH_COUNT, YTDLP_CACHE_DIR from ..cookies import cookies_unlocked, cookies_for_download -from ..errors import UnknownVideo from ..lib import get_yt_dlp_http_headers, get_yt_dlp_sleep_opts logger.getChild(__name__) @@ -162,34 +161,6 @@ def search_videos( return results, total -def delete_videos(session: Session, *file_group_ids: int, force: bool = False) -> Optional[List[dict]]: - """Delete Videos by their FileGroup IDs and all of their files. - - If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups - without deleting anything. If force is True, deletes even tagged files and re-saves the tags - config so `tags.yaml` reflects the removed TagFiles. - """ - videos = list(session.query(Video).filter(Video.file_group_id.in_(file_group_ids))) - if not videos: - raise UnknownVideo('Could not find videos to delete') - - tagged = get_tagged_file_groups_by_ids(session, [v.file_group_id for v in videos]) - if tagged and not force: - return tagged - - logger.warning(f'Deleting {len(videos)} videos') - # Get all URLs, skip them once (so config isn't saved multiple times). - urls = list(filter(None, [i.file_group.url for i in videos])) - download_manager.add_to_skip_list(*urls) - for video in videos: - video.delete(force=force) - session.commit() - - if tagged: - save_tags_config.activate_switch() - return None - - def download_video_info_json(url: str) -> dict: """Download video info JSON, using encrypted cookies for authentication if available.""" ydl_opts = dict( diff --git a/modules/videos/video/test/test_api.py b/modules/videos/video/test/test_api.py index 5c0f6a62..2b2bccf2 100644 --- a/modules/videos/video/test/test_api.py +++ b/modules/videos/video/test/test_api.py @@ -120,31 +120,36 @@ async def test_delete_video_api(async_client, test_session, channel_factory, vid vid1_video_path, vid1_caption_path = vid1.video_path, vid1.caption_paths[0] vid2_video_path, vid2_info_json_path = vid2.video_path, vid2.info_json_path + # Capture URLs before deletion since cascade removes the Video rows from the session. + vid1_url, vid2_url = vid1.file_group.url, vid2.file_group.url # No videos have been deleted yet. assert vid1_video_path.is_file() and vid1_caption_path.is_file() assert vid2_video_path.is_file() and vid2_info_json_path.is_file() - request, response = await async_client.delete(f'/api/videos/{vid1.file_group_id}') + body = json.dumps({'file_group_ids': [vid1.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT # Video was added to skip list. - assert test_download_manager.is_skipped(vid1.file_group.url) + assert test_download_manager.is_skipped(vid1_url) # Video was deleted. assert test_session.query(Video).count() == 1 assert vid1_video_path.is_file() is False and vid1_caption_path.is_file() is False assert vid2_video_path.is_file() and vid2_info_json_path.is_file() - request, response = await async_client.delete(f'/api/videos/{vid2.file_group_id}') + body = json.dumps({'file_group_ids': [vid2.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NO_CONTENT - assert test_download_manager.is_skipped(vid1.file_group.url, vid2.file_group.url) + assert test_download_manager.is_skipped(vid1_url, vid2_url) assert test_session.query(Video).count() == 0 assert vid1_video_path.is_file() is False and vid1_caption_path.is_file() is False assert vid2_video_path.is_file() is False and vid2_info_json_path.is_file() is False # 3 does not exist. - request, response = await async_client.delete(f'/api/videos/3') + body = json.dumps({'file_group_ids': [3]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) assert response.status_code == HTTPStatus.NOT_FOUND @@ -169,7 +174,8 @@ async def test_wrol_mode(async_client, simple_channel, simple_video, wrol_mode_f assert resp.json['code'] == 'WROL_MODE_ENABLED' # Can't delete a video - _, resp = await async_client.delete(f'/api/videos/{simple_video.file_group_id}') + body = dumps({'file_group_ids': [simple_video.file_group_id]}) + _, resp = await async_client.post('/api/files/delete_groups', content=body) assert resp.status_code == HTTPStatus.FORBIDDEN assert resp.json['code'] == 'WROL_MODE_ENABLED' diff --git a/wrolpi/files/api.py b/wrolpi/files/api.py index 1539af13..e70c8592 100644 --- a/wrolpi/files/api.py +++ b/wrolpi/files/api.py @@ -12,6 +12,7 @@ from sanic_ext.extensions.openapi import openapi from wrolpi.common import get_media_directory, wrol_mode_check, get_relative_to_media_directory, logger, \ background_task, walk, timer, TRACE_LEVEL, unique_by_predicate from wrolpi.errors import InvalidFile, UnknownDirectory, FileUploadFailed, FileConflict +from wrolpi.events import Events from . import lib, schema from .worker import file_worker from ..api_utils import json_response, api_app @@ -75,6 +76,38 @@ async def delete_file(_: Request, body: schema.DeleteRequest): return response.empty() +@files_bp.post('/delete_groups') +@openapi.definition( + summary='Delete FileGroups (and their model records) by ID. Pass force=true to delete' + ' even tagged FileGroups.', + body=schema.DeleteFileGroupsRequest, +) +@validate(schema.DeleteFileGroupsRequest) +@wrol_mode_check +async def delete_file_groups(request: Request, body: schema.DeleteFileGroupsRequest): + if not body.file_group_ids: + raise InvalidFile('file_group_ids cannot be empty') + session = request.ctx.session + + if not body.force: + tagged = lib.get_tagged_file_groups_by_ids(session, body.file_group_ids) + if tagged: + return json_response( + {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, + HTTPStatus.CONFLICT, + ) + + affected_models = lib.delete_file_groups(session, body.file_group_ids, force=body.force) + + if 'video' in affected_models: + # A Channel's tracked downloads may have changed; rewrite channels.yaml. + from modules.videos.lib import save_channels_config + save_channels_config.activate_switch() + + Events.send_deleted(f'Deleted {len(body.file_group_ids)} files') + return response.empty() + + @files_bp.post('/refresh') @openapi.definition( summary='Refresh and index all paths (files/directories) in the provided list. Refresh all files if not provided.', diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index c7498b26..afad6c99 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -56,7 +56,7 @@ __all__ = ['list_directories_contents', 'delete', 'split_path_stem_and_suffix', 'get_mimetype', 'split_file_name_words', 'get_primary_file', 'get_file_statistics', 'search_file_suggestion_count', 'glob_shared_stem', 'upsert_file', 'get_unique_files_by_stem', 'rename', 'delete_directory', 'handle_file_group_search_results', 'get_file_location_href', - 'get_tagged_file_groups_by_ids'] + 'get_tagged_file_groups_by_ids', 'delete_file_groups'] def get_file_tag_names(session: Session, file: pathlib.Path) -> List[str]: @@ -219,7 +219,7 @@ def list_directories_contents(session: Session, directories_: List[str]) -> Dict def get_tagged_file_groups_by_ids(session: Session, file_group_ids: List[int]) -> List[dict]: """Return serialized FileGroups (subset of `file_group_ids`) that have at least one TagFile. - Used by Archive/Video/Doc deletion endpoints to require force-confirmation before deleting tagged files.""" + Used by the unified delete endpoint to require force-confirmation before deleting tagged files.""" if not file_group_ids: return [] rows = session.query(FileGroup) \ @@ -230,6 +230,92 @@ def get_tagged_file_groups_by_ids(session: Session, file_group_ids: List[int]) - return [fg.__json__() for fg in rows] +def get_tagged_file_groups_by_paths(session: Session, paths: Iterable[pathlib.Path]) -> List[FileGroup]: + """Return FileGroup objects under any of the given paths that have at least one TagFile. + + For a file path, matches FileGroups whose primary_path shares the file's stem in the same + directory. For a directory path, matches FileGroups whose `directory` is the path or a + descendant. Used by the path-based delete endpoint to require force-confirmation before + deleting tagged files.""" + paths = list(paths) + if not paths: + return [] + tagged: List[FileGroup] = [] + seen_ids = set() + for path in paths: + query = session.query(FileGroup) \ + .join(TagFile, TagFile.file_group_id == FileGroup.id) + if path.is_file(): + # Could be deleting a file in a FileGroup that has been tagged. + stem, _ = split_path_stem_and_suffix(path) + query = query.filter(FileGroup.primary_path.like(f'{path.parent}/{stem}%')) + else: + # Search for any FileGroups that have been tagged under this directory. + # Use indexed directory column for efficient lookup. + path_str = str(path) + query = query.filter(or_( + FileGroup.directory == path_str, + FileGroup.directory.like(f'{path_str}/%'), + )) + for file_group in query: + if path.is_dir() or any(p for p in paths if p in file_group.my_paths()): + if file_group.id not in seen_ids: + seen_ids.add(file_group.id) + tagged.append(file_group) + return tagged + + +def delete_file_groups(session: Session, file_group_ids: List[int], force: bool = False) -> Set[str]: + """Delete FileGroups (and their model rows via FK CASCADE) by ID. + + Returns the set of `FileGroup.model` values that were deleted, so the caller can fire any + model-specific post-delete switches (e.g., `save_channels_config` for videos). + + The caller is expected to have already gated this on tag confirmation via + `get_tagged_file_groups_by_ids`. `force` propagates to FileGroup.delete so the safety net + at FileGroup.delete (raises FileGroupIsTagged if force is False and tags exist) still + protects callers that skipped the pre-check.""" + files = FileGroup.get_by_ids(session, file_group_ids) + if not files: + raise UnknownFile(f'No FileGroups found for ids: {file_group_ids}') + + affected_models: Set[str] = {fg.model for fg in files if fg.model} + had_tagged = any(fg.tag_names for fg in files) + + # Capture domain Collection IDs for any Archive FileGroups so we can prune empty ones after + # the cascade removes the Archive rows. (Archive.delete used to do this per-archive; with + # cascade-based deletion we do it as a single post-pass.) + archive_collection_ids: Set[int] = set() + if 'archive' in affected_models: + from modules.archive.models import Archive + archive_collection_ids = { + collection_id for (collection_id,) in session.query(Archive.collection_id) + .filter(Archive.file_group_id.in_([fg.id for fg in files])) + if collection_id is not None + } + + for fg in files: + fg.delete(force=force) + + if archive_collection_ids: + # Flush so the FK CASCADE has removed the Archive rows before we count remaining. + session.flush() + from modules.archive.models import Archive + from wrolpi.collections.models import Collection + for collection_id in archive_collection_ids: + collection = session.query(Collection).filter_by(id=collection_id, kind='domain').one_or_none() + if collection is None: + continue + remaining = session.query(Archive).filter_by(collection_id=collection_id).count() + if remaining == 0: + session.delete(collection) + + if had_tagged: + save_tags_config.activate_switch() + + return affected_models + + @wrol_mode_check async def delete(*paths: Union[str, pathlib.Path], force: bool = False): """ @@ -251,34 +337,16 @@ async def delete(*paths: Union[str, pathlib.Path], force: bool = False): continue if p1 != p2 and p1 in p2.parents: raise InvalidFile(f'Cannot deleted nested paths') - if not force: - tagged_file_groups = [] - seen_ids = set() - with get_db_session() as session: - # Search for any files that have been tagged. - for path in paths: - query = session.query(FileGroup, TagFile) \ - .join(TagFile, TagFile.file_group_id == FileGroup.id) - if path.is_file(): - # Could be deleting a file in a FileGroup that has been tagged. - stem, _ = split_path_stem_and_suffix(path) - query = query.filter(FileGroup.primary_path.like(f'{path.parent}/{stem}%')) - else: - # Search for any FileGroups that have been tagged under this directory. - # Use indexed directory column for efficient lookup - path_str = str(path) - query = query.filter(or_( - FileGroup.directory == path_str, - FileGroup.directory.like(f'{path_str}/%') - )) - for (file_group, tag_file) in query: - if path.is_dir() or any(i for i in paths if i in file_group.my_paths()): - if file_group.id not in seen_ids: - seen_ids.add(file_group.id) - tagged_file_groups.append(file_group) - if tagged_file_groups: - # Serialize while session is still active. - return [i.__json__() for i in tagged_file_groups] + + # Discover any tagged FileGroups under these paths so we can either gate the deletion (when + # !force) or fire `save_tags_config` afterwards (when force=True and any were tagged). + with get_db_session() as session: + tagged_file_groups = get_tagged_file_groups_by_paths(session, paths) + if tagged_file_groups and not force: + # Serialize while session is still active. + return [i.__json__() for i in tagged_file_groups] + had_tagged = bool(tagged_file_groups) + for path in paths: ignored_directories = get_wrolpi_config().ignored_directories if ignored_directories and str(path) in ignored_directories: @@ -294,6 +362,10 @@ async def delete(*paths: Union[str, pathlib.Path], force: bool = False): from wrolpi.files.worker import file_worker await file_worker.refresh_sync(list(paths)) + if had_tagged: + # `tags.yaml` references TagFiles that the refresh just removed; rewrite it. + save_tags_config.activate_switch() + def _mimetype_suffix_map(path: Path, mimetype: str): """Special handling for mimetypes. Python Magic may not return the correct mimetype.""" @@ -1529,17 +1601,9 @@ def delete_directory(directory: pathlib.Path, recursive: bool = False, force: bo if recursive: if not force: with get_db_session() as session: - # Use indexed directory column for efficient lookup - directory_str = str(directory) - tagged = session.query(FileGroup) \ - .filter(or_( - FileGroup.directory == directory_str, - FileGroup.directory.like(f'{directory_str}/%') - )) \ - .join(TagFile, TagFile.file_group_id == FileGroup.id) \ - .limit(1).one_or_none() + tagged = get_tagged_file_groups_by_paths(session, [directory]) if tagged: - raise FileGroupIsTagged(f'Cannot delete {tagged} because it is tagged') + raise FileGroupIsTagged(f'Cannot delete {tagged[0]} because it is tagged') shutil.rmtree(directory) else: directory.rmdir() @@ -1733,7 +1797,7 @@ async def upsert_file(file: pathlib.Path | str, tag_names: List[str] = None) -> # Mark download as completed download.complete() # Link the download to the location of the uploaded file. - model = file_group.get_model_record() + model = file_group.get_model_record(session) download.location = model.location if model else file_group.location # Add tags if provided diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index 9cb93686..1e939057 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -544,6 +544,13 @@ class FileGroup(ModelHelper, Base): raise UnknownFile(f'Unable to find FileGroup with id {id_}') return fg + @classmethod + def get_by_ids(cls, session: Session, file_group_ids: List[int]) -> List['FileGroup']: + """Return FileGroups matching the given IDs (no error if some are missing).""" + if not file_group_ids: + return [] + return session.query(cls).filter(cls.id.in_(file_group_ids)).all() + @staticmethod def get_by_path(session: Session, path) -> Optional['FileGroup']: file_group = session.query(FileGroup).filter(FileGroup.primary_path == str(path)).one_or_none() @@ -741,12 +748,10 @@ class FileGroup(ModelHelper, Base): elif Zim.can_model(self): return Zim - def get_model_record(self) -> Optional[ModelHelper]: + def get_model_record(self, session: Session) -> Optional[ModelHelper]: + """Return this FileGroup's model record (Video/Archive/Doc/Zim) using the given session, or None.""" klass = self.get_model_class() - if klass: - with get_db_session() as session: - return klass.get_by_path(session, self.primary_path) - return None + return klass.get_by_path(session, self.primary_path) if klass else None def do_model(self, session: Session) -> Optional[ModelHelper]: """Get/Create the Model record (Video/Archive/etc.) for this FileGroup, if any.""" diff --git a/wrolpi/files/schema.py b/wrolpi/files/schema.py index 84625f18..1cfd720b 100644 --- a/wrolpi/files/schema.py +++ b/wrolpi/files/schema.py @@ -21,6 +21,12 @@ class DeleteRequest: force: bool = False +@dataclass +class DeleteFileGroupsRequest: + file_group_ids: List[int] = field(default_factory=list) + force: bool = False + + @dataclass class FilesSearchRequest: search_str: Optional[str] = None diff --git a/wrolpi/files/test/test_api.py b/wrolpi/files/test/test_api.py index 971ce748..d60ed961 100644 --- a/wrolpi/files/test/test_api.py +++ b/wrolpi/files/test/test_api.py @@ -163,6 +163,63 @@ async def test_delete_wrol_mode(async_client, wrol_mode_fixture): assert response.json['code'] == 'WROL_MODE_ENABLED' +@pytest.mark.asyncio +async def test_delete_file_groups_mixed_batch(async_client, test_session, video_factory, archive_factory): + """A single POST /api/files/delete_groups can delete a mixture of model types in one call.""" + from modules.archive.models import Archive + video = video_factory(with_video_file=True) + archive = archive_factory('example.com', 'https://example.com/1') + test_session.commit() + video_paths = list(video.file_group.my_paths()) + archive_paths = list(archive.my_paths()) + + body = json.dumps({'file_group_ids': [video.file_group_id, archive.file_group_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) + assert response.status_code == HTTPStatus.NO_CONTENT + + assert all(not p.exists() for p in video_paths) + assert all(not p.exists() for p in archive_paths) + assert test_session.query(Video).count() == 0 + assert test_session.query(Archive).count() == 0 + assert test_session.query(FileGroup).count() == 0 + + +@pytest.mark.asyncio +async def test_delete_file_groups_default_deleter(async_client, test_session, make_files_structure, + refresh_files): + """A FileGroup with no specialized model (e.g., a plain text file) is removed via FileGroup.delete.""" + foo, = make_files_structure({'foo.txt': 'hello'}) + await refresh_files() + + fg = test_session.query(FileGroup).one() + assert fg.model is None # plain text isn't modeled by Video/Archive/Doc/Zim + fg_id = fg.id + + body = json.dumps({'file_group_ids': [fg_id]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) + assert response.status_code == HTTPStatus.NO_CONTENT + + assert not foo.exists() + assert test_session.query(FileGroup).filter_by(id=fg_id).one_or_none() is None + + +@pytest.mark.asyncio +async def test_delete_file_groups_unknown_ids(async_client, test_session): + """Deleting non-existent IDs returns 404 from UnknownFile.""" + body = json.dumps({'file_group_ids': [99999]}) + request, response = await async_client.post('/api/files/delete_groups', content=body) + assert response.status_code == HTTPStatus.NOT_FOUND + assert response.json['code'] == 'UNKNOWN_FILE' + + +@pytest.mark.asyncio +async def test_delete_file_groups_empty_ids(async_client): + """An empty file_group_ids list returns 400 (InvalidFile).""" + body = json.dumps({'file_group_ids': []}) + request, response = await async_client.post('/api/files/delete_groups', content=body) + assert response.status_code == HTTPStatus.BAD_REQUEST + + @pytest.mark.asyncio async def test_files_search_recent(test_session, test_directory, async_client, video_file_factory, refresh_files): """File search can return the most recently viewed files.""" @@ -712,7 +769,7 @@ async def test_post_upload(test_session, async_client, test_directory, make_file assert not file_group.tag_names, 'No tags this time.' assert file_group.indexed, 'File should be indexed after upload.' assert file_group.model, 'File should be modeled' - assert isinstance(file_group.get_model_record(), Video) + assert isinstance(file_group.get_model_record(test_session), Video) assert file_group.location == f'/videos/{file_group.id}' # Video was modeled, but has no Channel. video: Video = test_session.query(Video).one()