Fixing archive deleting.

modules/archive/api.py

@@ -32,7 +32,7 @@ async def post_archive(_: Request, data: dict):
     return response.empty()
 
 
-@bp.delete('/<url_id:int>')
+@bp.delete('/<archive_id:int>')
 @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:

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():

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
         '''

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'<Domain id={self.id} domain={self.domain} directory={self.directory}>'
+
+    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()

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

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