diff --git a/app/src/api.js b/app/src/api.js index 02e214cf..d2517e08 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -312,11 +312,11 @@ export async function postArchive(url) { return await apiPost(`${API_URI}/archive`, i); } -export async function deleteArchive(url_id) { - return await apiDelete(`${API_URI}/archive/${url_id}`); +export async function deleteArchive(archive_id) { + return await apiDelete(`${API_URI}/archive/${archive_id}`); } -export async function searchURLs(offset, limit, domain = null) { +export async function searchArchives(offset, limit, domain = null) { // Build a search query to retrieve a list of videos from the API offset = offset || 0; limit = limit || DEFAULT_LIMIT; @@ -328,7 +328,7 @@ export async function searchURLs(offset, limit, domain = null) { let response = await apiPost(`${ARCHIVES_API}/search`, body); if (response.status === 200) { let data = await response.json(); - return [data['urls'], data['totals']['urls']]; + return [data['archives'], data['totals']['archives']]; } else { throw Error(`Unable to search archives`); } diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index 666e562c..88dd52b6 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -1,14 +1,14 @@ import React from "react"; import {Card, Confirm, Container, Form, Header, Icon, Image, Placeholder, Tab, Table} from "semantic-ui-react"; import Paginator, {APIForm, objectToQuery, scrollToTop, uploadDate} from "./Common"; -import {deleteArchive, fetchDomains, postArchive, refreshArchives, searchURLs} from "../api"; +import {deleteArchive, fetchDomains, postArchive, refreshArchives, searchArchives} from "../api"; import Button from "semantic-ui-react/dist/commonjs/elements/Button"; import {Link, NavLink} from "react-router-dom"; import * as QueryString from "query-string"; import {ArchivePlaceholder} from "./Placeholder"; -function FailedUrlCard({url, syncURL, deleteURL}) { +function FailedArchiveCard({url, syncURL, deleteURL}) { let syncIcon = ( ; + let readabilityUrl = archive.readability_path ? `/media/${archive.readability_path}` : null; + let readabilityIcon = ; if (readabilityUrl) { readabilityIcon = ( ); @@ -129,12 +127,12 @@ class URLCard extends React.Component { -

{url.latest.title || url.url}

+

{archive.title || archive.url}

- {uploadDate(latest.archive_datetime)} + {uploadDate(archive.archive_datetime)} @@ -151,12 +149,12 @@ class URLCard extends React.Component { } -class URLCards extends React.Component { +class ArchiveCards extends React.Component { render() { return ( - {this.props.urls.map((i) => { - return + {this.props.archives.map((i) => { + return })} ) @@ -174,7 +172,7 @@ class Archives extends React.Component { this.state = { activePage: activePage, limit: 20, - urls: null, + archives: null, totalPages: null, domain: domain, }; @@ -194,11 +192,11 @@ class Archives extends React.Component { } async fetchURLs() { - this.setState({urls: null}); + this.setState({archives: null}); let offset = this.state.limit * this.state.activePage - this.state.limit; - let [urls, total] = await searchURLs(offset, this.state.limit, this.state.domain); + let [archives, total] = await searchArchives(offset, this.state.limit, this.state.domain); let totalPages = Math.round(total / this.state.limit) || 1; - this.setState({urls, totalPages: totalPages}); + this.setState({archives: archives, totalPages: totalPages}); } clearSearch() { @@ -224,9 +222,9 @@ class Archives extends React.Component { } render() { - let {urls, activePage, totalPages} = this.state; + let {archives, activePage, totalPages} = this.state; - if (urls === null) { + if (archives === null) { return (<>
Latest Archives
@@ -260,7 +258,7 @@ class Archives extends React.Component { <>
Latest Archives
{domainButton} - + {pagination} ) diff --git a/modules/archive/api.py b/modules/archive/api.py index 6a861a91..30309d00 100644 --- a/modules/archive/api.py +++ b/modules/archive/api.py @@ -32,7 +32,7 @@ async def post_archive(_: Request, data: dict): return response.empty() -@bp.delete('/') +@bp.delete('/') @validate_doc( 'Delete a website record', responses=[ @@ -40,8 +40,8 @@ async def post_archive(_: Request, data: dict): ] ) @wrol_mode_check -async def delete_url(_: Request, url_id: int): - lib.delete_url(url_id) +async def delete_archive(_: Request, archive_id: int): + lib.delete_archive(archive_id) return response.empty() @@ -84,6 +84,7 @@ async def fetch_domains(_: Request): 'Search archive contents and titles', consumes=ArchiveSearchRequest, produces=ArchiveSearchResponse, + optional_body=True, ) async def search_archives(_: Request, data: dict): try: diff --git a/modules/archive/conftest.py b/modules/archive/conftest.py index 2f18abd1..ebd1f51c 100644 --- a/modules/archive/conftest.py +++ b/modules/archive/conftest.py @@ -27,6 +27,9 @@ def archive_factory(test_session, archive_directory): now = time_generator() def _(domain: str = None, url: str = None, title: str = None, contents: str = None) -> Archive: + if domain: + assert '/' not in domain + title = title or str(uuid4()) url = url or f'https://example.com/{title}' domain_dir = domain or title @@ -53,7 +56,7 @@ def archive_factory(test_session, archive_directory): directory=domain_dir, ) test_session.add(domain) - test_session.commit() + test_session.flush([domain]) url_ = test_session.query(URL).filter_by(url=url).one_or_none() if not url_: @@ -62,7 +65,7 @@ def archive_factory(test_session, archive_directory): domain_id=domain.id, ) test_session.add(url_) - test_session.commit() + test_session.flush([url_]) archive = Archive( readability_json_path=readability_json_path, @@ -75,8 +78,12 @@ def archive_factory(test_session, archive_directory): url_id=url_.id, domain_id=domain.id, contents=contents, + archive_datetime=timestamp, ) test_session.add(archive) + test_session.flush([archive]) + + url_.latest_id = archive.id test_session.commit() for path in archive.my_paths(): diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 276c4250..39618236 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -296,21 +296,25 @@ def get_url_count(domain: str = '') -> int: return count -def delete_url(url_id: int): +def delete_archive(archive_id: int): """ - Delete a URL record, all it's Archives and files. + Delete an Archive and all of it's files. """ - with get_db_session() as session: - url: URL = session.query(URL).filter_by(id=url_id).one_or_none() - if not url: - raise UnknownURL(f'Unknown url with id: {url_id}') + with get_db_session(commit=True) as session: + archive: Archive = session.query(Archive).filter_by(id=archive_id).one_or_none() + if not archive: + raise UnknownURL(f'Unknown Archive with id: {archive_id}') # Delete any files associated with this URL. - for archive in url.archives: - archive.unlink() - - with get_db_session(commit=True) as session: - session.query(URL).filter_by(id=url_id).delete() + archive.delete() + # URL must have the latest + url = archive.url + url.update_latest() + # Delete this archive. + session.query(Archive).filter_by(id=archive_id).delete() + # Remove the URL if necessary. + if not url.archives: + session.query(URL).filter_by(id=url.id).delete() def group_archive_files(files: Iterator[pathlib.Path]) -> groupby: @@ -490,23 +494,33 @@ def get_domains(): def search(search_str: str, domain: str, limit: int, offset: int): with get_db_curs() as curs: + columns = 'id, COUNT(*) OVER() AS total' + params = dict(offset=offset, limit=limit) wheres = '' - params = dict(search_str=search_str, offset=offset, limit=limit) + + if search_str: + columns = 'id, ts_rank_cd(textsearch, websearch_to_tsquery(%(search_str)s)), COUNT(*) OVER() AS total' + params['search_str'] = search_str + wheres += '\nAND textsearch @@ websearch_to_tsquery(%(search_str)s)' if domain: curs.execute('SELECT id FROM domains WHERE domain=%s', (domain,)) - domain_id = curs.fetchone()[0] + try: + domain_id = curs.fetchone()[0] + except TypeError: + # No domains match the provided domain. + return [], 0 params['domain_id'] = domain_id - wheres += 'AND domain_id = %(domain_id)s' + wheres += '\nAND domain_id = %(domain_id)s' # TODO handle different Archive search orders. stmt = f''' - SELECT id, ts_rank_cd(textsearch, websearch_to_tsquery(%(search_str)s)), COUNT(*) OVER() AS total + SELECT {columns} FROM archive WHERE - textsearch @@ websearch_to_tsquery(%(search_str)s) + singlefile_path IS NOT NULL AND singlefile_path != '' {wheres} - ORDER BY 2 DESC, 1 ASC + ORDER BY 2 DESC, archive_datetime DESC -- highest rank, then most recent OFFSET %(offset)s LIMIT %(limit)s ''' diff --git a/modules/archive/models.py b/modules/archive/models.py index 188ae6ce..970a9329 100644 --- a/modules/archive/models.py +++ b/modules/archive/models.py @@ -1,7 +1,7 @@ from typing import Generator from sqlalchemy import Column, Integer, String, ForeignKey, Computed -from sqlalchemy.orm import relationship +from sqlalchemy.orm import relationship, Session from sqlalchemy.orm.collections import InstrumentedList from wrolpi.common import ModelHelper, Base, tsvector @@ -76,11 +76,11 @@ class Archive(Base, ModelHelper): domain=self.domain.dict() if self.domain else None, domain_id=self.domain_id, id=self.id, - readability_json_path=self.readability_json_path.path if self.readability_json_path else None, - readability_path=self.readability_path.path if self.readability_path else None, - readability_txt_path=self.readability_txt_path.path if self.readability_txt_path else None, - screenshot_path=self.screenshot_path.path if self.screenshot_path else None, - singlefile_path=self.singlefile_path.path if self.singlefile_path else None, + readability_json_path=self.readability_json_path, + readability_path=self.readability_path, + readability_txt_path=self.readability_txt_path, + screenshot_path=self.screenshot_path, + singlefile_path=self.singlefile_path, status=self.status, title=self.title, url=url, @@ -88,6 +88,18 @@ class Archive(Base, ModelHelper): ) return d + def delete(self): + self.unlink() + + session = Session.object_session(self) + + if self.url: + self.url.update_latest() + if not self.url.latest_id: + self.url.delete() + + session.query(Archive).filter_by(id=self.id).delete() + class URL(Base, ModelHelper): __tablename__ = 'url' @@ -126,6 +138,19 @@ class URL(Base, ModelHelper): # No archives exist! pass + def delete(self): + session = Session.object_session(self) + archive_ids = [int(i.id) for i in self.archives] + if archive_ids: + for id_ in archive_ids: + session.query(Archive).filter_by(id=id_).delete() + + domain = self.domain + session.query(URL).filter_by(id=self.id).delete() + + if len(domain.urls) == 0: + domain.delete() + class Domain(Base, ModelHelper): __tablename__ = 'domains' # plural to avoid conflict @@ -138,3 +163,9 @@ class Domain(Base, ModelHelper): def __repr__(self): return f'' + + def delete(self): + session = Session.object_session(self) + session.execute('DELETE FROM archive WHERE domain_id=:id', dict(id=self.id)) + session.execute('DELETE FROM url WHERE domain_id=:id', dict(id=self.id)) + session.query(Domain).filter_by(id=self.id).delete() diff --git a/modules/archive/test/test_api.py b/modules/archive/test/test_api.py index 835c09ee..0b4b8537 100644 --- a/modules/archive/test/test_api.py +++ b/modules/archive/test/test_api.py @@ -18,11 +18,11 @@ def test_archives_search(test_session, archive_directory, archive_factory, test_ # 1 and 2 contain "foo". data = {'search_str': 'foo'} - check_results(data, [1, 2]) + check_results(data, [2, 1]) # 2 and 3 contain "baz". data = {'search_str': 'baz'} - check_results(data, [2, 3]) + check_results(data, [3, 2]) # 1 contains "bar". data = {'search_str': 'bar'} @@ -47,3 +47,32 @@ def test_archives_search(test_session, archive_directory, archive_factory, test_ # All titles contain "qux", but they contain different amounts. They are ordered by the amount. data = {'search_str': 'qux'} check_results(data, [3, 2, 1]) + + +def test_archives_search_no_query(archive_factory, test_client): + """ + Archive Search API endpoint does not require data in the body. + """ + # Add 100 random archives. + for _ in range(100): + archive_factory('example.com') + + # All archives are returned when no `search_str` is passed. + request, response = test_client.post('/api/archive/search') + assert response.status_code == HTTPStatus.OK, response.json + assert [i['id'] for i in response.json['archives']] == list(range(100, 80, -1)) + assert response.json['totals']['archives'] == 100 + + # All archives are from "example.com". + data = dict(domain='example.com') + request, response = test_client.post('/api/archive/search', content=json.dumps(data)) + assert response.status_code == HTTPStatus.OK, response.json + assert [i['id'] for i in response.json['archives']] == list(range(100, 80, -1)) + assert response.json['totals']['archives'] == 100 + + # No archives are from "example.org". + data = dict(domain='example.org') + request, response = test_client.post('/api/archive/search', content=json.dumps(data)) + assert response.status_code == HTTPStatus.OK, response.json + assert [i['id'] for i in response.json['archives']] == [] + assert response.json['totals']['archives'] == 0 diff --git a/modules/archive/test/test_lib.py b/modules/archive/test/test_lib.py index 9c22aed4..82f0c10e 100644 --- a/modules/archive/test/test_lib.py +++ b/modules/archive/test/test_lib.py @@ -1,16 +1,15 @@ import json import pathlib from datetime import datetime -from http import HTTPStatus import mock -from modules.archive.lib import new_archive, get_or_create_domain_and_url, get_urls, get_url_count, delete_url, \ - _refresh_archives, get_new_archive_files +from modules.archive.lib import new_archive, get_or_create_domain_and_url, get_urls, get_url_count, _refresh_archives, \ + get_new_archive_files, delete_archive from modules.archive.models import Archive, URL, Domain from wrolpi.common import get_media_directory from wrolpi.db import get_db_session -from wrolpi.errors import InvalidDomain, UnknownURL +from wrolpi.errors import InvalidDomain from wrolpi.media_path import MediaPath from wrolpi.root_api import CustomJSONEncoder from wrolpi.test.common import TestAPI, wrap_test_db, wrap_media_directory @@ -209,37 +208,6 @@ class TestArchive(TestAPI): new_archive('https://example.org', sync=True) self.assertEqual(get_url_count('example.org'), 1) - @wrap_test_db - def test_delete_url(self): - with mock.patch('modules.archive.lib.request_archive', make_fake_request_archive()): - archive = new_archive('https://example.com', sync=True) - - with get_db_session() as session: - urls = session.query(URL).all() - self.assertEqual(len(urls), 1) - archives = session.query(Archive).all() - self.assertEqual(len(archives), 1) - - self.assertIsNotNone(archive.singlefile_path) - singlefile_path = archive.singlefile_path.path - self.assertTrue(singlefile_path.is_file()) - - url_id = archive.url.id - - # Delete the URL, all archives and all files. - delete_url(url_id) - self.assertFalse(singlefile_path.exists()) - - # Can't delete the same URL twice. - self.assertRaises(UnknownURL, delete_url, url_id) - - with get_db_session() as session: - urls = session.query(URL).all() - self.assertEqual(len(urls), 0) - - # Bad ID - self.assertRaises(UnknownURL, delete_url, 123) - @wrap_test_db def test_get_title_from_html(self): with wrap_media_directory(): @@ -373,7 +341,7 @@ def test_archive_refresh_deleted_archive(test_session, archive_directory, archiv check_counts(archive_count=0, url_count=0, domain_count=0) -def test_refresh_archives_fills_contents(test_session, archive_directory, archive_factory, test_client): +def test_refresh_archives_fills_contents(test_session, archive_factory, test_client): """ Refreshing archives fills in any missing contents. """ @@ -405,3 +373,25 @@ def test_refresh_archives_fills_contents(test_session, archive_directory, archiv assert archive2.contents assert archive3.contents assert not archive4.contents + + +def test_delete_archive(test_session, archive_factory): + archive1 = archive_factory('example.com', 'https://example.com/1') + archive2 = archive_factory('example.com', 'https://example.com/1') + archive3 = archive_factory('example.com', 'https://example.com/1') + + url = test_session.query(URL).one() + assert url.latest == archive3 + + # Delete the oldest. + delete_archive(archive1.id) + + # Delete the latest. + delete_archive(archive3.id) + + # Delete the last archive. The URL and Domain should also be deleted. + delete_archive(archive2.id) + url = test_session.query(URL).one_or_none() + assert url is None + domain = test_session.query(Domain).one_or_none() + assert domain is None diff --git a/wrolpi/db.py b/wrolpi/db.py index 4f23bf1e..428bcda6 100644 --- a/wrolpi/db.py +++ b/wrolpi/db.py @@ -1,4 +1,5 @@ from contextlib import contextmanager +from functools import wraps from typing import ContextManager, Tuple, List import psycopg2 @@ -113,18 +114,31 @@ def get_db_curs(commit: bool = False): connection.rollback() +def optional_session(commit: bool = False): + """ + Wraps a function, if a Session is passed as keyword `session`, it will be used. Otherwise, a new session will be + created and passed to the function. + """ + + def wrapper(func: callable): + @wraps(func) + def wrapped(*a, session: Session = None, **kw): + if session: + return func(*a, **kw, session=session) + else: + with get_db_session(commit=commit) as session: + return func(*a, **kw, session=session) + + return wrapped + + return wrapper + + +@optional_session() def get_ranked_models(ranked_ids: List[int], model: Base, session: Session = None) -> List[Base]: """ Get all objects whose ids are in the `ranked_ids`, order them by their position in `ranked_ids`. """ - - def _(s): - r = s.query(model).filter(model.id.in_(ranked_ids)).all() - r = sorted(r, key=lambda i: ranked_ids.index(i.id)) - return r - - if session: - return _(session) - - with get_db_session() as session: - return _(session) + results = session.query(model).filter(model.id.in_(ranked_ids)).all() + results = sorted(results, key=lambda i: ranked_ids.index(i.id)) + return results diff --git a/wrolpi/schema.py b/wrolpi/schema.py index d95a0e93..d532a618 100644 --- a/wrolpi/schema.py +++ b/wrolpi/schema.py @@ -13,12 +13,18 @@ from wrolpi.errors import NoBodyContents, ValidationError, MissingRequiredField, from wrolpi.vars import DATE_FORMAT -def validate_data(model: type, data: dict): +def validate_data(model: type, data: dict, optional_body: bool = False): """ Convert a JSON object to the model's specification. If the JSON object matches the model's specification, this function will return a python dict of that data. If it doesn't match, this will return a Sanic response object containing an error. + + If `optional_body` is True, then the request body may be empty. Otherwise, raise NoBodyContents. """ + if optional_body and not data: + # Body was empty in the request, but body is not required. + return {} + if not data: raise NoBodyContents() @@ -76,7 +82,8 @@ def validate_data(model: type, data: dict): return new_data -def validate_doc(summary: str = None, consumes=None, produces=None, responses=(), tag: str = None): +def validate_doc(summary: str = None, consumes=None, produces=None, responses=(), tag: str = None, + optional_body: bool = False): """ Apply Sanic OpenAPI docs to the wrapped route. Perform simple validation on requests. """ @@ -89,7 +96,7 @@ def validate_doc(summary: str = None, consumes=None, produces=None, responses=() if 'data' in kw: raise OverflowError(f'data kwarg already being passed to {func}') - data = validate_data(consumes, request.json) + data = validate_data(consumes, request.json, optional_body) if isinstance(data, sanic.response.HTTPResponse): # Error in validation return data