From 284eb620dd413fd2327d8d90778f556c1c3da503 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Thu, 20 Jan 2022 22:37:03 -0800 Subject: [PATCH 01/16] Add Source Han font for preview generation to Docker image Include the license and a README explaining things in the repo itself. Depending on an external source for this is intended to be temporary, the goal is to have a Bookywrm-managed source for these, but this should be stable enough for now. We build it into the Dockerfile to make it available without adding it to the git repo itself, because git history is forever and we don't want to bake large files into the history. Theoretically it would make sense to gate this download on the ENABLE_PREVIEW_IMAGES environment variable, but ENV variables aren't available at Docker image build time (for sensible reasons), so we just unconditonally download it. This does mean users will ultimately download it anyway, but the benefit to doing this over adding it to the git history is that if we switch fonts, or update this one, or change strategies altogether, this version of the font will no longer have to be downloaded. Additionally, the font won't be downloaded until the application is actually built, which involves a bunch of other downloading (of Docker images and the like), so it's a reasonable time to do it. --- Dockerfile | 7 ++ .../static/fonts/source_han_sans/LICENSE.txt | 96 +++++++++++++++++++ .../static/fonts/source_han_sans/README.txt | 9 ++ 3 files changed, 112 insertions(+) create mode 100644 bookwyrm/static/fonts/source_han_sans/LICENSE.txt create mode 100644 bookwyrm/static/fonts/source_han_sans/README.txt diff --git a/Dockerfile b/Dockerfile index 349dd82b1..8a24e8883 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,6 +6,13 @@ RUN mkdir /app /app/static /app/images WORKDIR /app +# Use RUN curl because ADD will re-download the file every time to make sure it +# hasn't changed, which is exactly what we don't want +RUN mkdir -p /app/static/fonts/source_han_sans +RUN curl \ + https://github.com/adobe-fonts/source-han-sans/raw/release/Variable/OTC/SourceHanSans-VF.ttf.ttc \ + -o /app/static/fonts/source_han_sans/SourceHanSans-VF.ttf.ttc + COPY requirements.txt /app/ RUN pip install -r requirements.txt --no-cache-dir RUN apt-get update && apt-get install -y gettext libgettextpo-dev tidy && apt-get clean diff --git a/bookwyrm/static/fonts/source_han_sans/LICENSE.txt b/bookwyrm/static/fonts/source_han_sans/LICENSE.txt new file mode 100644 index 000000000..ddf7b7e91 --- /dev/null +++ b/bookwyrm/static/fonts/source_han_sans/LICENSE.txt @@ -0,0 +1,96 @@ +Copyright 2014-2021 Adobe (http://www.adobe.com/), with Reserved Font +Name 'Source'. Source is a trademark of Adobe in the United States +and/or other countries. + +This Font Software is licensed under the SIL Open Font License, +Version 1.1. + +This license is copied below, and is also available with a FAQ at: +http://scripts.sil.org/OFL + +----------------------------------------------------------- +SIL OPEN FONT LICENSE Version 1.1 - 26 February 2007 +----------------------------------------------------------- + +PREAMBLE +The goals of the Open Font License (OFL) are to stimulate worldwide +development of collaborative font projects, to support the font +creation efforts of academic and linguistic communities, and to +provide a free and open framework in which fonts may be shared and +improved in partnership with others. + +The OFL allows the licensed fonts to be used, studied, modified and +redistributed freely as long as they are not sold by themselves. The +fonts, including any derivative works, can be bundled, embedded, +redistributed and/or sold with any software provided that any reserved +names are not used by derivative works. The fonts and derivatives, +however, cannot be released under any other type of license. The +requirement for fonts to remain under this license does not apply to +any document created using the fonts or their derivatives. + +DEFINITIONS +"Font Software" refers to the set of files released by the Copyright +Holder(s) under this license and clearly marked as such. This may +include source files, build scripts and documentation. + +"Reserved Font Name" refers to any names specified as such after the +copyright statement(s). + +"Original Version" refers to the collection of Font Software +components as distributed by the Copyright Holder(s). + +"Modified Version" refers to any derivative made by adding to, +deleting, or substituting -- in part or in whole -- any of the +components of the Original Version, by changing formats or by porting +the Font Software to a new environment. + +"Author" refers to any designer, engineer, programmer, technical +writer or other person who contributed to the Font Software. + +PERMISSION & CONDITIONS +Permission is hereby granted, free of charge, to any person obtaining +a copy of the Font Software, to use, study, copy, merge, embed, +modify, redistribute, and sell modified and unmodified copies of the +Font Software, subject to the following conditions: + +1) Neither the Font Software nor any of its individual components, in +Original or Modified Versions, may be sold by itself. + +2) Original or Modified Versions of the Font Software may be bundled, +redistributed and/or sold with any software, provided that each copy +contains the above copyright notice and this license. These can be +included either as stand-alone text files, human-readable headers or +in the appropriate machine-readable metadata fields within text or +binary files as long as those fields can be easily viewed by the user. + +3) No Modified Version of the Font Software may use the Reserved Font +Name(s) unless explicit written permission is granted by the +corresponding Copyright Holder. This restriction only applies to the +primary font name as presented to the users. + +4) The name(s) of the Copyright Holder(s) or the Author(s) of the Font +Software shall not be used to promote, endorse or advertise any +Modified Version, except to acknowledge the contribution(s) of the +Copyright Holder(s) and the Author(s) or with their explicit written +permission. + +5) The Font Software, modified or unmodified, in part or in whole, +must be distributed entirely under this license, and must not be +distributed under any other license. The requirement for fonts to +remain under this license does not apply to any document created using +the Font Software. + +TERMINATION +This license becomes null and void if any of the above conditions are +not met. + +DISCLAIMER +THE FONT SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO ANY WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT +OF COPYRIGHT, PATENT, TRADEMARK, OR OTHER RIGHT. IN NO EVENT SHALL THE +COPYRIGHT HOLDER BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +INCLUDING ANY GENERAL, SPECIAL, INDIRECT, INCIDENTAL, OR CONSEQUENTIAL +DAMAGES, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF THE USE OR INABILITY TO USE THE FONT SOFTWARE OR FROM +OTHER DEALINGS IN THE FONT SOFTWARE. diff --git a/bookwyrm/static/fonts/source_han_sans/README.txt b/bookwyrm/static/fonts/source_han_sans/README.txt new file mode 100644 index 000000000..53cfa9b8f --- /dev/null +++ b/bookwyrm/static/fonts/source_han_sans/README.txt @@ -0,0 +1,9 @@ +The font file itself is not included in the Git repository to avoid putting +large files in the repo history. The Docker image should download the correct +font into this folder automatically. + +In case something goes wrong, the font used is the Variable OTC TTF, available +as of this writing from the Adobe Fonts GitHub repository: +https://github.com/adobe-fonts/source-han-sans/tree/release#user-content-variable-otcs + +BookWyrm expects the file to be in this folder, named SourceHanSans-VF.ttf.ttc From 6f5115c716a4443726e7b0dd31ae6e16d412d1ce Mon Sep 17 00:00:00 2001 From: Joachim Date: Wed, 19 Jan 2022 16:52:57 +0100 Subject: [PATCH 02/16] Use Source Han Sans for preview images generation --- bookwyrm/preview_images.py | 18 ++++++++++-------- requirements.txt | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/bookwyrm/preview_images.py b/bookwyrm/preview_images.py index a97ae2d5c..917d985a2 100644 --- a/bookwyrm/preview_images.py +++ b/bookwyrm/preview_images.py @@ -29,23 +29,25 @@ margin = math.floor(IMG_HEIGHT / 10) gutter = math.floor(margin / 2) inner_img_height = math.floor(IMG_HEIGHT * 0.8) inner_img_width = math.floor(inner_img_height * 0.7) -font_dir = os.path.join(settings.STATIC_ROOT, "fonts/public_sans") +font_dir = os.path.join(settings.STATIC_ROOT, "fonts/source_han_sans") -def get_font(font_name, size=28): +def get_font(weight, size=28): """Loads custom font""" - if font_name == "light": - font_path = os.path.join(font_dir, "PublicSans-Light.ttf") - if font_name == "regular": - font_path = os.path.join(font_dir, "PublicSans-Regular.ttf") - elif font_name == "bold": - font_path = os.path.join(font_dir, "PublicSans-Bold.ttf") + font_path = os.path.join(font_dir, "SourceHanSans-VF.ttf.ttc") try: font = ImageFont.truetype(font_path, size) except OSError: font = ImageFont.load_default() + if (weight == 'light'): + font.set_variation_by_name('Light') + if (weight == 'bold'): + font.set_variation_by_name('Bold') + if (weight == 'regular'): + font.set_variation_by_name('Regular') + return font diff --git a/requirements.txt b/requirements.txt index 534a0593d..1f25895c7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ django-model-utils==4.0.0 environs==9.3.4 flower==1.0.0 Markdown==3.3.3 -Pillow>=8.2.0 +Pillow>=9.0.0 psycopg2==2.8.4 pycryptodome==3.9.4 python-dateutil==2.8.1 From 766a0cc652c693786e6d3f25c624a1f4c8bac405 Mon Sep 17 00:00:00 2001 From: Joachim Date: Thu, 20 Jan 2022 22:19:49 +0100 Subject: [PATCH 03/16] Fix tests --- bookwyrm/preview_images.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/bookwyrm/preview_images.py b/bookwyrm/preview_images.py index 917d985a2..f48ca15e2 100644 --- a/bookwyrm/preview_images.py +++ b/bookwyrm/preview_images.py @@ -41,12 +41,15 @@ def get_font(weight, size=28): except OSError: font = ImageFont.load_default() - if (weight == 'light'): - font.set_variation_by_name('Light') - if (weight == 'bold'): - font.set_variation_by_name('Bold') - if (weight == 'regular'): - font.set_variation_by_name('Regular') + try: + if weight == "light": + font.set_variation_by_name("Light") + if weight == "bold": + font.set_variation_by_name("Bold") + if weight == "regular": + font.set_variation_by_name("Regular") + except AttributeError: + pass return font From 9e6390662b0dd9877c9151002b28468b5d4849e9 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Tue, 25 Jan 2022 00:22:13 -0800 Subject: [PATCH 04/16] Download fonts at app startup instead We can't bake the font into the Docker image as such, because we mount the volumes which blows away anything we have in the app tree beforehand. We could stash it somewhere in the image and then copy it from there on app startup or something, but at that point we might as well just download it as part of the app startup. --- Dockerfile | 7 ------- bookwyrm/apps.py | 35 +++++++++++++++++++++++++++++++++++ bookwyrm/preview_images.py | 26 ++++++++++++++++++-------- bookwyrm/settings.py | 18 +++++++++++++----- 4 files changed, 66 insertions(+), 20 deletions(-) create mode 100644 bookwyrm/apps.py diff --git a/Dockerfile b/Dockerfile index 8a24e8883..349dd82b1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,13 +6,6 @@ RUN mkdir /app /app/static /app/images WORKDIR /app -# Use RUN curl because ADD will re-download the file every time to make sure it -# hasn't changed, which is exactly what we don't want -RUN mkdir -p /app/static/fonts/source_han_sans -RUN curl \ - https://github.com/adobe-fonts/source-han-sans/raw/release/Variable/OTC/SourceHanSans-VF.ttf.ttc \ - -o /app/static/fonts/source_han_sans/SourceHanSans-VF.ttf.ttc - COPY requirements.txt /app/ RUN pip install -r requirements.txt --no-cache-dir RUN apt-get update && apt-get install -y gettext libgettextpo-dev tidy && apt-get clean diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py new file mode 100644 index 000000000..a3fe1c2cf --- /dev/null +++ b/bookwyrm/apps.py @@ -0,0 +1,35 @@ +import os +import urllib +import logging + +from django.apps import AppConfig + +from bookwyrm import settings + +logger = logging.getLogger(__name__) + +def download_file(url, destination): + try: + stream = urllib.request.urlopen(url) + with open(destination, "b+w") as f: + f.write(stream.read()) + except (urllib.error.HTTPError, urllib.error.URLError): + logger.error("Failed to download file %s", url) + + +class BookwyrmConfig(AppConfig): + name = "bookwyrm" + verbose_name = "BookWyrm" + + def ready(self): + if settings.ENABLE_PREVIEW_IMAGES and settings.FONTS: + # Download any fonts that we don't have yet + logger.debug("Downloading fonts..") + for name, config in settings.FONTS.items(): + font_path = os.path.join( + settings.FONT_DIR, config["directory"], config["filename"] + ) + + if "url" in config and not os.path.exists(font_path): + logger.info("Just a sec, downloading %s", name) + download_file(config["url"], font_path) diff --git a/bookwyrm/preview_images.py b/bookwyrm/preview_images.py index f48ca15e2..325742d72 100644 --- a/bookwyrm/preview_images.py +++ b/bookwyrm/preview_images.py @@ -4,6 +4,8 @@ import os import textwrap from io import BytesIO from uuid import uuid4 +import urllib +import logging import colorsys from colorthief import ColorThief @@ -17,29 +19,37 @@ from django.db.models import Avg from bookwyrm import models, settings from bookwyrm.tasks import app +logger = logging.getLogger(__name__) IMG_WIDTH = settings.PREVIEW_IMG_WIDTH IMG_HEIGHT = settings.PREVIEW_IMG_HEIGHT BG_COLOR = settings.PREVIEW_BG_COLOR TEXT_COLOR = settings.PREVIEW_TEXT_COLOR DEFAULT_COVER_COLOR = settings.PREVIEW_DEFAULT_COVER_COLOR +DEFAULT_FONT = settings.PREVIEW_DEFAULT_FONT TRANSPARENT_COLOR = (0, 0, 0, 0) margin = math.floor(IMG_HEIGHT / 10) gutter = math.floor(margin / 2) inner_img_height = math.floor(IMG_HEIGHT * 0.8) inner_img_width = math.floor(inner_img_height * 0.7) -font_dir = os.path.join(settings.STATIC_ROOT, "fonts/source_han_sans") + + +def get_imagefont(name, size): + try: + config = settings.FONTS[name] + path = os.path.join(settings.FONT_DIR, config["directory"], config["filename"]) + return ImageFont.truetype(path, size) + except KeyError: + logger.error("Font %s not found in config", name) + except OSError: + logger.error("Could not load font %s from file", name) + + return ImageFont.load_default() def get_font(weight, size=28): - """Loads custom font""" - font_path = os.path.join(font_dir, "SourceHanSans-VF.ttf.ttc") - - try: - font = ImageFont.truetype(font_path, size) - except OSError: - font = ImageFont.load_default() + font = get_imagefont(DEFAULT_FONT, size) try: if weight == "light": diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 197e672c1..b85925b77 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -35,6 +35,9 @@ LOCALE_PATHS = [ ] LANGUAGE_COOKIE_NAME = env.str("LANGUAGE_COOKIE_NAME", "django_language") +STATIC_ROOT = os.path.join(BASE_DIR, env("STATIC_ROOT", "static")) +MEDIA_ROOT = os.path.join(BASE_DIR, env("MEDIA_ROOT", "images")) + DEFAULT_AUTO_FIELD = "django.db.models.AutoField" # Preview image @@ -44,6 +47,16 @@ PREVIEW_TEXT_COLOR = env.str("PREVIEW_TEXT_COLOR", "#363636") PREVIEW_IMG_WIDTH = env.int("PREVIEW_IMG_WIDTH", 1200) PREVIEW_IMG_HEIGHT = env.int("PREVIEW_IMG_HEIGHT", 630) PREVIEW_DEFAULT_COVER_COLOR = env.str("PREVIEW_DEFAULT_COVER_COLOR", "#002549") +PREVIEW_DEFAULT_FONT = env.str("PREVIEW_DEFAULT_FONT", "Source Han Sans") + +FONTS = { + "Source Han Sans": { + "directory": "source_han_sans", + "filename": "SourceHanSans-VF.ttf.ttc", + "url": "https://github.com/adobe-fonts/source-han-sans/raw/release/Variable/OTC/SourceHanSans-VF.ttf.ttc", + } +} +FONT_DIR = os.path.join(STATIC_ROOT, "fonts") # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/3.2/howto/deployment/checklist/ @@ -310,13 +323,8 @@ if USE_S3: MEDIA_FULL_URL = MEDIA_URL STATIC_FULL_URL = STATIC_URL DEFAULT_FILE_STORAGE = "bookwyrm.storage_backends.ImagesStorage" - # I don't know if it's used, but the site crashes without it - STATIC_ROOT = os.path.join(BASE_DIR, env("STATIC_ROOT", "static")) - MEDIA_ROOT = os.path.join(BASE_DIR, env("MEDIA_ROOT", "images")) else: STATIC_URL = "/static/" - STATIC_ROOT = os.path.join(BASE_DIR, env("STATIC_ROOT", "static")) MEDIA_URL = "/images/" MEDIA_FULL_URL = f"{PROTOCOL}://{DOMAIN}{MEDIA_URL}" STATIC_FULL_URL = f"{PROTOCOL}://{DOMAIN}{STATIC_URL}" - MEDIA_ROOT = os.path.join(BASE_DIR, env("MEDIA_ROOT", "images")) From a1a3aa45f4860b6270e924c4a6dfad545c1cadb7 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Tue, 25 Jan 2022 00:37:05 -0800 Subject: [PATCH 05/16] Don't log autoload debug This is just too much --- bookwyrm/apps.py | 1 + bookwyrm/settings.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index a3fe1c2cf..c30b7c8f7 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -8,6 +8,7 @@ from bookwyrm import settings logger = logging.getLogger(__name__) + def download_file(url, destination): try: stream = urllib.request.urlopen(url) diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index b85925b77..75987ae9b 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -163,6 +163,9 @@ LOGGING = { "handlers": ["console", "mail_admins"], "level": LOG_LEVEL, }, + "django.utils.autoreload": { + "level": "INFO", + }, # Add a bookwyrm-specific logger "bookwyrm": { "handlers": ["console"], From 0c53f4e003205f6ba027bc696b2f81a6b1537235 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Tue, 25 Jan 2022 01:04:46 -0800 Subject: [PATCH 06/16] Fix linting and formatting --- bookwyrm/apps.py | 10 +++++++--- bookwyrm/preview_images.py | 3 ++- bookwyrm/settings.py | 1 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index c30b7c8f7..c2dfd4b81 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -1,3 +1,4 @@ +"""Do further startup configuration and initialization""" import os import urllib import logging @@ -10,15 +11,18 @@ logger = logging.getLogger(__name__) def download_file(url, destination): + """Downloads a file to the given path""" try: - stream = urllib.request.urlopen(url) - with open(destination, "b+w") as f: - f.write(stream.read()) + with urllib.request.urlopen(url) as stream: + with open(destination, "b+w") as outfile: + outfile.write(stream.read()) except (urllib.error.HTTPError, urllib.error.URLError): logger.error("Failed to download file %s", url) class BookwyrmConfig(AppConfig): + """Handles additional configuration""" + name = "bookwyrm" verbose_name = "BookWyrm" diff --git a/bookwyrm/preview_images.py b/bookwyrm/preview_images.py index 325742d72..891c8b6da 100644 --- a/bookwyrm/preview_images.py +++ b/bookwyrm/preview_images.py @@ -4,7 +4,6 @@ import os import textwrap from io import BytesIO from uuid import uuid4 -import urllib import logging import colorsys @@ -36,6 +35,7 @@ inner_img_width = math.floor(inner_img_height * 0.7) def get_imagefont(name, size): + """Loads an ImageFont based on config""" try: config = settings.FONTS[name] path = os.path.join(settings.FONT_DIR, config["directory"], config["filename"]) @@ -49,6 +49,7 @@ def get_imagefont(name, size): def get_font(weight, size=28): + """Gets a custom font with the given weight and size""" font = get_imagefont(DEFAULT_FONT, size) try: diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 75987ae9b..591a3b3b3 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -50,6 +50,7 @@ PREVIEW_DEFAULT_COVER_COLOR = env.str("PREVIEW_DEFAULT_COVER_COLOR", "#002549") PREVIEW_DEFAULT_FONT = env.str("PREVIEW_DEFAULT_FONT", "Source Han Sans") FONTS = { + # pylint: disable=line-too-long "Source Han Sans": { "directory": "source_han_sans", "filename": "SourceHanSans-VF.ttf.ttc", From 224dc4100ac5487f23106ef279710ab461481934 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 28 Jan 2022 17:32:41 -0800 Subject: [PATCH 07/16] Activitstreams tests --- .../activitystreams/test_abstractstream.py | 57 ++++++++++++++++++- .../tests/activitystreams/test_booksstream.py | 26 +++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/activitystreams/test_abstractstream.py b/bookwyrm/tests/activitystreams/test_abstractstream.py index 2c5cf6102..8d278041a 100644 --- a/bookwyrm/tests/activitystreams/test_abstractstream.py +++ b/bookwyrm/tests/activitystreams/test_abstractstream.py @@ -1,6 +1,9 @@ """ testing activitystreams """ +from datetime import datetime from unittest.mock import patch from django.test import TestCase +from django.utils import timezone + from bookwyrm import activitystreams, models @@ -51,13 +54,63 @@ class Activitystreams(TestCase): """the abstract base class for stream objects""" self.assertEqual( self.test_stream.stream_id(self.local_user), - "{}-test".format(self.local_user.id), + f"{self.local_user.id}-test", ) self.assertEqual( self.test_stream.unread_id(self.local_user), - "{}-test-unread".format(self.local_user.id), + f"{self.local_user.id}-test-unread", ) + def test_unread_by_status_type_id(self, *_): + """stream for status type""" + self.assertEqual( + self.test_stream.unread_by_status_type_id(self.local_user), + f"{self.local_user.id}-test-unread-by-type", + ) + + def test_get_rank(self, *_): + """sort order""" + date = datetime(2022, 1, 28, 0, 0, tzinfo=timezone.utc) + status = models.Status.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + published_date=date, + ) + self.assertEqual( + str(self.test_stream.get_rank(status)), + "1643328000.0", + ) + + def test_get_activity_stream(self, *_): + """load statuses""" + status = models.Status.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + ) + status2 = models.Comment.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + book=self.book, + ) + models.Comment.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + book=self.book, + ) + with patch("bookwyrm.activitystreams.r.set"), patch( + "bookwyrm.activitystreams.r.delete" + ), patch("bookwyrm.activitystreams.ActivityStream.get_store") as redis_mock: + redis_mock.return_value = [status.id, status2.id] + result = self.test_stream.get_activity_stream(self.local_user) + self.assertEqual(result.count(), 2) + self.assertEqual(result.first(), status) + self.assertEqual(result.last(), status2) + self.assertIsInstance(result.last(), models.Comment) + def test_abstractstream_get_audience(self, *_): """get a list of users that should see a status""" status = models.Status.objects.create( diff --git a/bookwyrm/tests/activitystreams/test_booksstream.py b/bookwyrm/tests/activitystreams/test_booksstream.py index c001d6dd8..dedf488ae 100644 --- a/bookwyrm/tests/activitystreams/test_booksstream.py +++ b/bookwyrm/tests/activitystreams/test_booksstream.py @@ -52,3 +52,29 @@ class Activitystreams(TestCase): # yes book, yes audience result = activitystreams.BooksStream().get_statuses_for_user(self.local_user) self.assertEqual(list(result), [status]) + + def test_book_statuses(self, *_): + """statuses about a book""" + alt_book = models.Edition.objects.create( + title="hi", parent_work=self.book.parent_work + ) + status = models.Status.objects.create( + user=self.local_user, content="hi", privacy="public" + ) + status = models.Comment.objects.create( + user=self.remote_user, content="hi", privacy="public", book=alt_book + ) + models.ShelfBook.objects.create( + user=self.local_user, + shelf=self.local_user.shelf_set.first(), + book=self.book, + ) + with patch( + "bookwyrm.activitystreams.BooksStream.bulk_add_objects_to_store" + ) as redis_mock: + activitystreams.BooksStream().add_book_statuses(self.local_user, self.book) + args = redis_mock.call_args[0] + queryset = args[0] + self.assertEqual(queryset.count(), 1) + self.assertTrue(status in queryset) + self.assertEqual(args[1], f"{self.local_user.id}-books") From d6abd9b32d0a1c2a25cef56c9405e607eef461cc Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Tue, 1 Feb 2022 21:45:13 -0800 Subject: [PATCH 08/16] Ensure directory exists, don't crash if we fail to write We should be creating the directory because the static tree from the repo isn't actually copied into the container, so we can't rely on it existing. And if we can't write it, we should catch that error instead of crashing the whole thing, oops! --- bookwyrm/apps.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index c2dfd4b81..10ccc89ff 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -13,11 +13,15 @@ logger = logging.getLogger(__name__) def download_file(url, destination): """Downloads a file to the given path""" try: + # Ensure our destination directory exists + os.makedirs(os.path.dirname(destination)) with urllib.request.urlopen(url) as stream: with open(destination, "b+w") as outfile: outfile.write(stream.read()) except (urllib.error.HTTPError, urllib.error.URLError): logger.error("Failed to download file %s", url) + except OSError: + logger.error("Couldn't open font file %s for writing", destination) class BookwyrmConfig(AppConfig): From 060f515aea5afa255e3441a1b357de5280886a69 Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Tue, 1 Feb 2022 21:54:51 -0800 Subject: [PATCH 09/16] Be even more conservative on errors This runs at startup of anything, so we should be extra sure to not break anything, and lots of things can go wrong downloading files from the internet --- bookwyrm/apps.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index 10ccc89ff..8940edccc 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -22,6 +22,8 @@ def download_file(url, destination): logger.error("Failed to download file %s", url) except OSError: logger.error("Couldn't open font file %s for writing", destination) + except: # pylint: disable=bare-except + logger.exception("Unknown error in file download") class BookwyrmConfig(AppConfig): From 5a3ce5e328c46f45d730a1aed1606c1e7e355264 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 11:48:56 -0800 Subject: [PATCH 10/16] Fixes rating in about page superlatives --- bookwyrm/templates/about/about.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templates/about/about.html b/bookwyrm/templates/about/about.html index 4e533b11a..6f16aa675 100644 --- a/bookwyrm/templates/about/about.html +++ b/bookwyrm/templates/about/about.html @@ -28,7 +28,7 @@
{% if superlatives.top_rated %} - {% with book=superlatives.top_rated.default_edition rating=top_rated.rating %} + {% with book=superlatives.top_rated.default_edition rating=superlatives.top_rated.rating %}
From 1f6ecc39ac32b5867c970549e72a1a8c8a3b025b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:15:06 -0800 Subject: [PATCH 11/16] Adds allowlist for html attrs --- bookwyrm/sanitize_html.py | 12 +++++++++++- bookwyrm/tests/test_sanitize_html.py | 13 ++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/bookwyrm/sanitize_html.py b/bookwyrm/sanitize_html.py index 8b0e3c4cb..538264fc1 100644 --- a/bookwyrm/sanitize_html.py +++ b/bookwyrm/sanitize_html.py @@ -22,6 +22,9 @@ class InputHtmlParser(HTMLParser): # pylint: disable=abstract-method "ol", "li", ] + self.allowed_attrs = [ + "href", "rel", "src", "alt" + ] self.tag_stack = [] self.output = [] # if the html appears invalid, we just won't allow any at all @@ -30,7 +33,14 @@ class InputHtmlParser(HTMLParser): # pylint: disable=abstract-method def handle_starttag(self, tag, attrs): """check if the tag is valid""" if self.allow_html and tag in self.allowed_tags: - self.output.append(("tag", self.get_starttag_text())) + allowed_attrs = " ".join( + f'{a}="{v}"' for a, v in attrs if a in self.allowed_attrs + ) + reconstructed = f'<{tag}' + if allowed_attrs: + reconstructed += " " + allowed_attrs + reconstructed += ">" + self.output.append(("tag", reconstructed)) self.tag_stack.append(tag) else: self.output.append(("data", "")) diff --git a/bookwyrm/tests/test_sanitize_html.py b/bookwyrm/tests/test_sanitize_html.py index 6c4053483..572f83325 100644 --- a/bookwyrm/tests/test_sanitize_html.py +++ b/bookwyrm/tests/test_sanitize_html.py @@ -24,13 +24,24 @@ class Sanitizer(TestCase): self.assertEqual(input_text, output) def test_valid_html_attrs(self): - """and don't remove attributes""" + """and don't remove useful attributes""" input_text = 'yes html' parser = InputHtmlParser() parser.feed(input_text) output = parser.get_output() self.assertEqual(input_text, output) + def test_valid_html_invalid_attrs(self): + """do remove un-approved attributes""" + input_text = 'yes html' + parser = InputHtmlParser() + parser.feed(input_text) + output = parser.get_output() + self.assertEqual( + output, + 'yes html' + ) + def test_invalid_html(self): """remove all html when the html is malformed""" input_text = "yes html" From 2c7a6e85186255c123708f4936af6465fd9b193f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:17:16 -0800 Subject: [PATCH 12/16] Correct status order --- bookwyrm/tests/activitystreams/test_abstractstream.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/activitystreams/test_abstractstream.py b/bookwyrm/tests/activitystreams/test_abstractstream.py index 8d278041a..f81c6fd6e 100644 --- a/bookwyrm/tests/activitystreams/test_abstractstream.py +++ b/bookwyrm/tests/activitystreams/test_abstractstream.py @@ -107,8 +107,8 @@ class Activitystreams(TestCase): redis_mock.return_value = [status.id, status2.id] result = self.test_stream.get_activity_stream(self.local_user) self.assertEqual(result.count(), 2) - self.assertEqual(result.first(), status) - self.assertEqual(result.last(), status2) + self.assertEqual(result.first(), status2) + self.assertEqual(result.last(), status) self.assertIsInstance(result.last(), models.Comment) def test_abstractstream_get_audience(self, *_): From cae7191a2b65f82c70054bcb73efeb7518f77dee Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:19:56 -0800 Subject: [PATCH 13/16] Python formatting --- bookwyrm/sanitize_html.py | 6 ++---- bookwyrm/tests/test_sanitize_html.py | 5 +---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/bookwyrm/sanitize_html.py b/bookwyrm/sanitize_html.py index 538264fc1..4edd2818e 100644 --- a/bookwyrm/sanitize_html.py +++ b/bookwyrm/sanitize_html.py @@ -22,9 +22,7 @@ class InputHtmlParser(HTMLParser): # pylint: disable=abstract-method "ol", "li", ] - self.allowed_attrs = [ - "href", "rel", "src", "alt" - ] + self.allowed_attrs = ["href", "rel", "src", "alt"] self.tag_stack = [] self.output = [] # if the html appears invalid, we just won't allow any at all @@ -36,7 +34,7 @@ class InputHtmlParser(HTMLParser): # pylint: disable=abstract-method allowed_attrs = " ".join( f'{a}="{v}"' for a, v in attrs if a in self.allowed_attrs ) - reconstructed = f'<{tag}' + reconstructed = f"<{tag}" if allowed_attrs: reconstructed += " " + allowed_attrs reconstructed += ">" diff --git a/bookwyrm/tests/test_sanitize_html.py b/bookwyrm/tests/test_sanitize_html.py index 572f83325..5814f2207 100644 --- a/bookwyrm/tests/test_sanitize_html.py +++ b/bookwyrm/tests/test_sanitize_html.py @@ -37,10 +37,7 @@ class Sanitizer(TestCase): parser = InputHtmlParser() parser.feed(input_text) output = parser.get_output() - self.assertEqual( - output, - 'yes html' - ) + self.assertEqual(output, 'yes html') def test_invalid_html(self): """remove all html when the html is malformed""" From 85aad7c219fbedbe8307c1e7b386f911740ca729 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:25:44 -0800 Subject: [PATCH 14/16] Another sorting order error --- bookwyrm/tests/activitystreams/test_abstractstream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/activitystreams/test_abstractstream.py b/bookwyrm/tests/activitystreams/test_abstractstream.py index f81c6fd6e..af94233f0 100644 --- a/bookwyrm/tests/activitystreams/test_abstractstream.py +++ b/bookwyrm/tests/activitystreams/test_abstractstream.py @@ -109,7 +109,7 @@ class Activitystreams(TestCase): self.assertEqual(result.count(), 2) self.assertEqual(result.first(), status2) self.assertEqual(result.last(), status) - self.assertIsInstance(result.last(), models.Comment) + self.assertIsInstance(result.first(), models.Comment) def test_abstractstream_get_audience(self, *_): """get a list of users that should see a status""" From 3b12af63b60752f655834059c95456a4416a8836 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:49:33 -0800 Subject: [PATCH 15/16] Fixes links on import page --- bookwyrm/templates/import/import_status.html | 4 ++-- bookwyrm/views/imports/troubleshoot.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/import/import_status.html b/bookwyrm/templates/import/import_status.html index 374ea22c7..3a063954a 100644 --- a/bookwyrm/templates/import/import_status.html +++ b/bookwyrm/templates/import/import_status.html @@ -47,7 +47,7 @@ {% trans "In progress" %} - {% trans "Refresh" %} + {% trans "Refresh" %}
@@ -230,7 +230,7 @@ {% if not legacy %}
- {% include 'snippets/pagination.html' with page=items %} + {% include 'snippets/pagination.html' with page=items path=page_path %}
{% endif %} {% endspaceless %}{% endblock %} diff --git a/bookwyrm/views/imports/troubleshoot.py b/bookwyrm/views/imports/troubleshoot.py index f637b966b..e9ac275f8 100644 --- a/bookwyrm/views/imports/troubleshoot.py +++ b/bookwyrm/views/imports/troubleshoot.py @@ -5,6 +5,7 @@ from django.core.paginator import Paginator from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator +from django.urls import reverse from django.views import View from bookwyrm import models @@ -35,6 +36,7 @@ class ImportTroubleshoot(View): page.number, on_each_side=2, on_ends=1 ), "complete": True, + "page_path": reverse("import-troubleshoot", args=[job.id]), } return TemplateResponse(request, "import/troubleshoot.html", data) From 9013b1417ae552681120858de1eea9b212a129a7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 3 Feb 2022 13:59:53 -0800 Subject: [PATCH 16/16] Show cancel buttons on modals in static mode --- bookwyrm/templates/book/file_links/add_link_modal.html | 4 +--- bookwyrm/templates/readthrough/readthrough_modal.html | 2 -- bookwyrm/templates/snippets/report_modal.html | 4 +--- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/bookwyrm/templates/book/file_links/add_link_modal.html b/bookwyrm/templates/book/file_links/add_link_modal.html index 0002b82b3..d5b3fcd0d 100644 --- a/bookwyrm/templates/book/file_links/add_link_modal.html +++ b/bookwyrm/templates/book/file_links/add_link_modal.html @@ -56,9 +56,7 @@ {% block modal-footer %} -{% if not static %} - -{% endif %} + {% endblock %} {% block modal-form-close %}{% endblock %} diff --git a/bookwyrm/templates/readthrough/readthrough_modal.html b/bookwyrm/templates/readthrough/readthrough_modal.html index 07d27b664..73f445f61 100644 --- a/bookwyrm/templates/readthrough/readthrough_modal.html +++ b/bookwyrm/templates/readthrough/readthrough_modal.html @@ -70,9 +70,7 @@ {% block modal-footer %} -{% if not static %} -{% endif %} {% endblock %} {% block modal-form-close %} diff --git a/bookwyrm/templates/snippets/report_modal.html b/bookwyrm/templates/snippets/report_modal.html index 0cf786ec4..f65cab590 100644 --- a/bookwyrm/templates/snippets/report_modal.html +++ b/bookwyrm/templates/snippets/report_modal.html @@ -50,9 +50,7 @@ {% block modal-footer %} -{% if not static %} - -{% endif %} + {% endblock %}