From 73cb142ae16c32fa54e114d85b8ca7e5cac2b6d4 Mon Sep 17 00:00:00 2001
From: Roland <roland@learningselfreliance.com>
Date: Tue, 31 Oct 2023 10:20:45 -0600
Subject: [PATCH] Reverting to older singlefile container to keep archive
 screenshot ratio.  Using FileGroup.url on ArchivePage.

---
 app/src/components/Archive.js     |  8 +++-----
 app/src/components/VideoPlayer.js |  2 +-
 docker/archive/Dockerfile         |  7 ++++---
 docker/archive/main.py            | 11 ++++++----
 modules/archive/__init__.py       |  2 ++
 modules/archive/lib.py            |  2 +-
 modules/archive/test/test_lib.py  | 34 ++++++++++++++++++++++---------
 modules/videos/downloader.py      |  4 ++++
 modules/zim/downloader.py         |  2 ++
 wrolpi/common.py                  | 10 +++++----
 wrolpi/downloader.py              |  2 ++
 11 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js
index 1100fbf7..a1b4fbdd 100644
--- a/app/src/components/Archive.js
+++ b/app/src/components/Archive.js
@@ -149,9 +149,7 @@ function ArchivePage() {
     if (domain) {
         const domainUrl = `/archive?domain=${domain}`;
         domainHeader = <Header as='h4'>
-            <CardLink to={domainUrl}>
-                {domain}
-            </CardLink>
+            <a href={domainUrl}>{domain}</a>
         </Header>;
     }
 
@@ -181,7 +179,7 @@ function ArchivePage() {
             {humanFileSize(size)}
 
             <Header as={'h3'}>URL</Header>
-            <p>{data.url ? <a href={data.url}>{data.url}</a> : 'N/A'}</p>
+            <p>{archiveFile.url ? <a href={archiveFile.url}>{archiveFile.url}</a> : 'N/A'}</p>
 
             <Header as={'h3'}>Modified Date</Header>
             <p>{modifiedDatetimeString}</p>
@@ -310,7 +308,7 @@ export function ArchiveCard({file}) {
                     <Button icon='file alternate' content='Details'
                             labelPosition='left'/>
                 </Link>
-                <Button icon='external' href={data.url} target='_blank' rel='noopener noreferrer'/>
+                <Button icon='external' href={file.url} target='_blank' rel='noopener noreferrer'/>
             </CardDescription>
         </CardContent>
     </Card>
diff --git a/app/src/components/VideoPlayer.js b/app/src/components/VideoPlayer.js
index b6feaecf..4baabef1 100644
--- a/app/src/components/VideoPlayer.js
+++ b/app/src/components/VideoPlayer.js
@@ -277,7 +277,7 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) {
 
 export default VideoPage;
 
-const chapterRegex = new RegExp('^(\\(?(?:((\\d?\\d):)?(?:(\\d?\\d):(\\d\\d)))\\)?)\\s+(.*)$', 'i');
+const chapterRegex = new RegExp('^(\\(?(?:((\\d?\\d):)?(?:(\\d?\\d):(\\d\\d)))\\)?)\.?\\s+(.*)$', 'i');
 
 function formatVideoDescription(description, setVideoTime) {
     // Convert timestamps to links which change the video's playback location.  Change hashtags to search links.
diff --git a/docker/archive/Dockerfile b/docker/archive/Dockerfile
index e393fc18..09f6e83a 100644
--- a/docker/archive/Dockerfile
+++ b/docker/archive/Dockerfile
@@ -1,4 +1,5 @@
-FROM capsulecode/singlefile
+# Keep older singlefile version so we can keep screenshots at specific ratio.
+FROM capsulecode/singlefile:v0.3.32
 
 USER root
 WORKDIR /app
@@ -14,7 +15,7 @@ COPY docker/archive/requirements.txt /app/requirements.txt
 RUN pip3 install -r /app/requirements.txt
 COPY docker/archive /app
 
-# Install Readability too
-RUN npm install -g 'git+https://github.com/pirate/readability-extractor'
+# Install a newer singlefile and Readability.
+RUN npm install -g single-file-cli@1.1.16 'git+https://github.com/pirate/readability-extractor'
 
 ENTRYPOINT [ "python3", "/app/main.py"]
diff --git a/docker/archive/main.py b/docker/archive/main.py
index 86ad2230..9b078947 100644
--- a/docker/archive/main.py
+++ b/docker/archive/main.py
@@ -84,7 +84,10 @@ async def call_single_file(url) -> bytes:
     logger.debug(f'archive cmd: {cmd}')
     stdout, stderr, return_code = await check_output(cmd, always_log_stderr=True)
     if return_code != 0 or not stdout:
-        raise ValueError(f'Failed to single-file {url}')
+        if stderr:
+            for line in stderr.splitlines():
+                logger.error(line)
+        raise RuntimeError(f'Failed to single-file {url} got the following error: {stderr}')
     logger.debug(f'done archiving for {url}')
     return stdout
 
@@ -103,19 +106,18 @@ async def extract_readability(path: str, url: str) -> dict:
         output = json.loads(stdout)
     except JSONDecodeError as e:
         stderr = stderr.decode() if stderr else None
-        raise ChildProcessError(f'Failed to extract readability.  {stderr}') from e
+        raise RuntimeError(f'Failed to extract readability.  {stderr}') from e
     logger.debug(f'done readability for {url}')
     return output
 
 
 async def take_screenshot(url: str) -> bytes:
-    logger.info(f'Screenshot: {url}')
     cmd = '/usr/bin/chromium-browser' \
           ' --headless' \
           ' --disable-gpu' \
           ' --no-sandbox' \
           ' --screenshot' \
-          ' --window-size=1280,720' \
+          ' --window-size=1280x720' \
           f' "{url}"'
     logger.debug(f'Screenshot cmd: {cmd}')
     with tempfile.TemporaryDirectory() as tmp_dir:
@@ -156,6 +158,7 @@ async def post_archive(request: Request):
         # Use html suffix so chrome screenshot recognizes it as an HTML file.
         with tempfile.NamedTemporaryFile('wb', suffix='.html') as fh:
             fh.write(singlefile)
+            fh.flush()
             readability = None
             try:
                 readability = await extract_readability(fh.name, url)
diff --git a/modules/archive/__init__.py b/modules/archive/__init__.py
index e54cccf4..67993644 100644
--- a/modules/archive/__init__.py
+++ b/modules/archive/__init__.py
@@ -57,6 +57,8 @@ class ArchiveDownloader(Downloader, ABC):
             if session := Session.object_session(archive):
                 session.commit()
 
+        logger.info(f'Successfully downloaded Archive {download.url} {archive}')
+
         return DownloadResult(success=True, location=f'/archive/{archive.id}')
 
     @optional_session
diff --git a/modules/archive/lib.py b/modules/archive/lib.py
index 6fc779f4..d4cec58f 100644
--- a/modules/archive/lib.py
+++ b/modules/archive/lib.py
@@ -320,7 +320,7 @@ def parse_article_html_metadata(html: Union[bytes, str], assume_utc: bool = True
         if isinstance(schema, dict) and schema.get('@context') in ('https://schema.org', 'http://schema.org'):
             # Found https://schema.org/
             if headline := schema.get('headline'):
-                metadata.title = metadata.title or schema.get('headline')
+                metadata.title = metadata.title or headline
             if datePublished := schema.get('datePublished'):
                 try:
                     metadata.published_datetime = metadata.published_datetime or dates.strpdate(datePublished)
diff --git a/modules/archive/test/test_lib.py b/modules/archive/test/test_lib.py
index f1994ac6..e22a80ca 100644
--- a/modules/archive/test/test_lib.py
+++ b/modules/archive/test/test_lib.py
@@ -665,13 +665,13 @@ def test_archive_history(test_session, archive_factory):
     assert not archive5.history, 'archive5 has no history'
 
 
-SINGLEFILE_1 = b'''
+METADATA_EXAMPLE_1 = b'''
 <!DOCTYPE html>
 <html lang="en-US">
 </html>
 '''
 
-SINGLEFILE_2 = b'''
+METADATA_EXAMPLE_2 = b'''
 <!DOCTYPE html>
 <html lang="en-US">
  <head>
@@ -687,7 +687,7 @@ SINGLEFILE_2 = b'''
 </html>
 '''
 
-SINGLEFILE_3 = b'''
+METADATA_EXAMPLE_3 = b'''
 <!DOCTYPE html>
 <html lang="en-US">
 <title>This is not a meta title and should be ignored</title>
@@ -700,7 +700,7 @@ SINGLEFILE_3 = b'''
 </script>
 </html>'''
 
-SINGLEFILE_4 = b'''
+METADATA_EXAMPLE_4 = b'''
 <!DOCTYPE html>
 <html lang="en-US">
 <title>This is not a meta title and should be ignored</title>
@@ -709,7 +709,7 @@ SINGLEFILE_4 = b'''
 </a>
 </html>'''
 
-SINGLEFILE_5 = b'''
+METADATA_EXAMPLE_5 = b'''
 <!DOCTYPE html>
 <html lang="en-US">
 <title>This is not a meta title and should be ignored</title>
@@ -724,7 +724,7 @@ SINGLEFILE_5 = b'''
 def test_parse_article_html_metadata():
     """Singlefiles may contain metadata tags."""
     # Metadata can be empty.
-    metadata = lib.parse_article_html_metadata(SINGLEFILE_1)
+    metadata = lib.parse_article_html_metadata(METADATA_EXAMPLE_1)
     assert metadata.title is None
     assert metadata.published_datetime is None
     assert metadata.modified_datetime is None
@@ -732,7 +732,7 @@ def test_parse_article_html_metadata():
     assert metadata.author is None
 
     # Data from <meta> elements.
-    metadata = lib.parse_article_html_metadata(SINGLEFILE_2)
+    metadata = lib.parse_article_html_metadata(METADATA_EXAMPLE_2)
     assert metadata.title == 'The Title'
     assert metadata.published_datetime == datetime(2023, 10, 18, 4, 52, 23, tzinfo=utc)
     assert metadata.modified_datetime == datetime(2023, 10, 19, 5, 53, 24, tzinfo=utc)
@@ -741,7 +741,7 @@ def test_parse_article_html_metadata():
 
     # Data from Article structured data
     # https://developers.google.com/search/docs/appearance/structured-data/article
-    metadata = lib.parse_article_html_metadata(SINGLEFILE_3)
+    metadata = lib.parse_article_html_metadata(METADATA_EXAMPLE_3)
     assert metadata.title == 'The headline'
     assert metadata.published_datetime == datetime(2022, 9, 27, 0, 40, 19, tzinfo=utc)
     assert metadata.modified_datetime == datetime(2022, 9, 27, 13, 43, 47, 971000, tzinfo=utc)
@@ -749,11 +749,11 @@ def test_parse_article_html_metadata():
     assert metadata.author == 'BOBBY'
 
     # Data from author anchor element.
-    metadata = lib.parse_article_html_metadata(SINGLEFILE_4)
+    metadata = lib.parse_article_html_metadata(METADATA_EXAMPLE_4)
     assert metadata.author == 'Link Author'
 
     # author anchor may be a list.
-    metadata = lib.parse_article_html_metadata(SINGLEFILE_5)
+    metadata = lib.parse_article_html_metadata(METADATA_EXAMPLE_5)
     assert metadata.author == 'The Span Author'
     assert metadata.published_datetime == datetime(2023, 8, 25, tzinfo=pytz.UTC)
 
@@ -803,3 +803,17 @@ def test_modified_datetime(html, expected):
 )
 def test_author(html, expected):
     assert lib.parse_article_html_metadata(html).author == expected
+
+
+SINGLEFILE_EXAMPLE_1 = b'''<!DOCTYPE html>
+<html class="fonts-loaded" lang="en">
+ <!--
+ Page saved with SingleFile 
+ url: https://www.example.com 
+ saved date: Tue Oct 31 2023 15:57:19 GMT+0000 (Coordinated Universal Time)
+-->
+ <head>'''
+
+
+def test_get_url_from_singlefile():
+    assert lib.get_url_from_singlefile(SINGLEFILE_EXAMPLE_1) == 'https://www.example.com'
diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py
index bbd0752f..bf4c8ec3 100755
--- a/modules/videos/downloader.py
+++ b/modules/videos/downloader.py
@@ -399,6 +399,10 @@ class VideoDownloader(Downloader, ABC):
             location = f'/videos/channel/{channel_id}/video/{video_id}'
         else:
             location = f'/videos/video/{video_id}'
+
+        logger.debug(f'Downloaded video {location=}')
+        logger.info(f'Successfully downloaded video {url} {video}')
+
         result = DownloadResult(
             success=True,
             location=location,
diff --git a/modules/zim/downloader.py b/modules/zim/downloader.py
index 86f77839..4953121a 100644
--- a/modules/zim/downloader.py
+++ b/modules/zim/downloader.py
@@ -109,6 +109,8 @@ class KiwixZimDownloader(Downloader):
         if return_code != 0:
             logger.error(f'Failed to restart kiwix')
 
+        logger.info(f'Successfully downloaded Zim {download.url} to {output_path}')
+
         # Location is just the generic Kiwix viewer.
         return DownloadResult(success=True, location='/zim')
 
diff --git a/wrolpi/common.py b/wrolpi/common.py
index 1545bde6..beee0a52 100644
--- a/wrolpi/common.py
+++ b/wrolpi/common.py
@@ -1548,11 +1548,13 @@ def html_screenshot(html: bytes) -> bytes:
 
     with tempfile.NamedTemporaryFile('wb', suffix='.html') as fh:
         fh.write(html)
+        fh.flush()
 
-        driver = webdriver.Chrome(chrome_options=options)
-        driver.get(f'file://{fh.name}')
-        screenshot = driver.get_screenshot_as_png()
-        return screenshot
+        with webdriver.Chrome(chrome_options=options) as driver:
+            driver.get(f'file://{fh.name}')
+            driver.set_window_size(1280, 720)
+            screenshot = driver.get_screenshot_as_png()
+            return screenshot
 
 
 def chain(iterable: Union[List, Tuple], length: int) -> List:
diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py
index cc4b3a11..c163b203 100644
--- a/wrolpi/downloader.py
+++ b/wrolpi/downloader.py
@@ -1277,6 +1277,8 @@ class RSSDownloader(Downloader, ABC):
         # Only download new URLs.
         urls = [i for i in urls if i not in [i.url for i in sub_downloader.already_downloaded(*urls)]]
 
+        logger.info(f'Successfully got {len(urls)} urls from rss {download.url}')
+
         return DownloadResult(success=True, downloads=urls)
 
     @staticmethod