From 0299f2e2352773c53155ae815cd0a525d6408fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Fri, 24 Nov 2023 21:18:43 -0300 Subject: [PATCH 01/37] Add functional tests for search_vector triggers As metadata changes, search continues to work. --- bookwyrm/tests/test_book_search.py | 83 ++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index db6ba8353..2baff9bf1 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -134,3 +134,86 @@ class BookSearch(TestCase): # there's really not much to test here, it's just a dataclass self.assertEqual(result.confidence, 1) self.assertEqual(result.title, "Title") + + +class SearchVectorTriggers(TestCase): + """look for books as they change""" + + def setUp(self): + """we need basic test data and mocks""" + self.work = models.Work.objects.create(title="This Work") + self.author = models.Author.objects.create(name="Name") + self.edition = models.Edition.objects.create( + title="First Edition of Work", + subtitle="Some Extra Words Are Good", + series="A Fabulous Sequence of Items", + parent_work=self.work, + isbn_10="0000000000", + ) + self.edition.authors.add(self.author) + self.edition.save(broadcast=False) + + def test_search_after_changed_metadata(self): + """book found after updating metadata""" + self.assertEqual(self.edition, self._search_first("First")) # title + self.assertEqual(self.edition, self._search_first("Good")) # subtitle + self.assertEqual(self.edition, self._search_first("Sequence")) # series + + self.edition.title = "Second Title of Work" + self.edition.subtitle = "Fewer Words Is Better" + self.edition.series = "A Wondrous Bunch" + self.edition.save(broadcast=False) + + self.assertEqual(self.edition, self._search_first("Second")) # title new + self.assertEqual(self.edition, self._search_first("Fewer")) # subtitle new + self.assertEqual(self.edition, self._search_first("Wondrous")) # series new + + self.assertFalse(self._search_first("First")) # title old + self.assertFalse(self._search_first("Good")) # subtitle old + self.assertFalse(self._search_first("Sequence")) # series old + + def test_search_after_author_remove(self): + """book not found via removed author""" + self.assertEqual(self.edition, self._search_first("Name")) + + self.edition.authors.set([]) + self.edition.save(broadcast=False) + + self.assertFalse(self._search("Name")) + self.assertEqual(self.edition, self._search_first("Edition")) + + def test_search_after_author_add(self): + """book found by newly-added author""" + new_author = models.Author.objects.create(name="Mozilla") + + self.assertFalse(self._search("Mozilla")) + + self.edition.authors.add(new_author) + self.edition.save(broadcast=False) + + self.assertEqual(self.edition, self._search_first("Mozilla")) + self.assertEqual(self.edition, self._search_first("Name")) + + def test_search_after_updated_author_name(self): + """book found under new author name""" + self.assertEqual(self.edition, self._search_first("Name")) + self.assertFalse(self._search("Identifier")) + + self.author.name = "Identifier" + self.author.save(broadcast=False) + self.edition.refresh_from_db() + + self.assertFalse(self._search("Name")) + self.assertEqual(self.edition, self._search_first("Identifier")) + self.assertEqual(self.edition, self._search_first("Work")) + + def _search_first(self, query): + """wrapper around search_title_author""" + return self._search(query, return_first=True) + + # pylint: disable-next=no-self-use + def _search(self, query, *, return_first=False): + """wrapper around search_title_author""" + return book_search.search_title_author( + query, min_confidence=0, return_first=return_first + ) From e4d688665c7d54912b73752a270c4d4ead2a63b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Fri, 24 Nov 2023 14:15:00 -0300 Subject: [PATCH 02/37] Remove index for `author.search_vector`, which is never used --- bookwyrm/migrations/0190_book_search_updates.py | 16 ++++++++++++++++ bookwyrm/models/author.py | 6 ------ 2 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 bookwyrm/migrations/0190_book_search_updates.py diff --git a/bookwyrm/migrations/0190_book_search_updates.py b/bookwyrm/migrations/0190_book_search_updates.py new file mode 100644 index 000000000..52d80fcb9 --- /dev/null +++ b/bookwyrm/migrations/0190_book_search_updates.py @@ -0,0 +1,16 @@ +# Generated by Django 3.2.20 on 2023-11-24 17:11 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("bookwyrm", "0188_theme_loads"), + ] + + operations = [ + migrations.RemoveIndex( + model_name="author", + name="bookwyrm_au_search__b050a8_gin", + ), + ] diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 981e3c0cc..b4488d46a 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -2,7 +2,6 @@ import re from typing import Tuple, Any -from django.contrib.postgres.indexes import GinIndex from django.db import models from bookwyrm import activitypub @@ -68,8 +67,3 @@ class Author(BookDataModel): return f"https://{DOMAIN}/author/{self.id}" activity_serializer = activitypub.Author - - class Meta: - """sets up postgres GIN index field""" - - indexes = (GinIndex(fields=["search_vector"]),) From 44ef928c3ceb8ce9b4426113551baa53563630f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 16:08:36 -0300 Subject: [PATCH 03/37] Alter object row IDs to force test failure in original code --- bookwyrm/tests/test_book_search.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index 2baff9bf1..e66ea97be 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -1,5 +1,7 @@ """ test searching for books """ import datetime +from unittest import expectedFailure +from django.db import connection from django.test import TestCase from django.utils import timezone @@ -153,6 +155,17 @@ class SearchVectorTriggers(TestCase): self.edition.authors.add(self.author) self.edition.save(broadcast=False) + @classmethod + def setUpTestData(cls): + """create conditions that trigger known old bugs""" + with connection.cursor() as cursor: + cursor.execute( + """ + ALTER SEQUENCE bookwyrm_author_id_seq RESTART WITH 20; + ALTER SEQUENCE bookwyrm_book_authors_id_seq RESTART WITH 300; + """ + ) + def test_search_after_changed_metadata(self): """book found after updating metadata""" self.assertEqual(self.edition, self._search_first("First")) # title @@ -194,6 +207,7 @@ class SearchVectorTriggers(TestCase): self.assertEqual(self.edition, self._search_first("Mozilla")) self.assertEqual(self.edition, self._search_first("Name")) + @expectedFailure def test_search_after_updated_author_name(self): """book found under new author name""" self.assertEqual(self.edition, self._search_first("Name")) From 416a6caf2d4124b884fd1a04841908e572d92c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Fri, 24 Nov 2023 22:26:13 -0300 Subject: [PATCH 04/37] Define `author_search_vector_trigger` via Author.Meta.triggers Previously, triggers lived only in a particular migration file. With this change, code for the triggers resides in the model, and their lifecycle is managed through normal Django migrations. --- ...grate_search_vec_triggers_to_pgtriggers.py | 50 +++++++++++++++++++ bookwyrm/models/author.py | 29 +++++++++++ bookwyrm/settings.py | 1 + bookwyrm/utils/db.py | 22 ++++++++ requirements.txt | 1 + 5 files changed, 103 insertions(+) create mode 100644 bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py create mode 100644 bookwyrm/utils/db.py diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py new file mode 100644 index 000000000..10354fa67 --- /dev/null +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -0,0 +1,50 @@ +# Generated by Django 3.2.20 on 2023-11-25 00:47 + +from importlib import import_module +import re + +from django.db import migrations +import pgtrigger.compiler +import pgtrigger.migrations + +trigger_migration = import_module("bookwyrm.migrations.0077_auto_20210623_2155") + +# it's _very_ convenient for development that this migration be reversible +search_vector_trigger = trigger_migration.Migration.operations[4] +author_search_vector_trigger = trigger_migration.Migration.operations[5] + + +assert re.search(r"\bCREATE TRIGGER search_vector_trigger\b", search_vector_trigger.sql) +assert re.search( + r"\bCREATE TRIGGER author_search_vector_trigger\b", + author_search_vector_trigger.sql, +) + + +class Migration(migrations.Migration): + dependencies = [ + ("bookwyrm", "0190_book_search_updates"), + ] + + operations = [ + pgtrigger.migrations.AddTrigger( + model_name="author", + trigger=pgtrigger.compiler.Trigger( + name="reset_search_vector_on_author_edit", + sql=pgtrigger.compiler.UpsertTriggerSql( + func="WITH book AS (SELECT bookwyrm_book.id AS row_id FROM bookwyrm_author LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book_authors.id = new.id LEFT OUTER JOIN bookwyrm_book ON bookwyrm_book.id = bookwyrm_book_authors.book_id) UPDATE bookwyrm_book SET search_vector = '' FROM book WHERE id = book.row_id;RETURN NEW;", + hash="9c0a472e2bf60e63d593cce49f47972c7b227a00", + operation='UPDATE OF "name"', + pgid="pgtrigger_reset_search_vector_on_author_edit_a447c", + table="bookwyrm_author", + when="AFTER", + ), + ), + ), + migrations.RunSQL( + sql="""DROP TRIGGER IF EXISTS author_search_vector_trigger ON bookwyrm_author; + DROP FUNCTION IF EXISTS author_trigger; + """, + reverse_sql=author_search_vector_trigger.sql, + ), + ] diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index b4488d46a..9b4f3c1bd 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -3,9 +3,11 @@ import re from typing import Tuple, Any from django.db import models +import pgtrigger from bookwyrm import activitypub from bookwyrm.settings import DOMAIN +from bookwyrm.utils.db import format_trigger from .book import BookDataModel from . import fields @@ -66,4 +68,31 @@ class Author(BookDataModel): """editions and works both use "book" instead of model_name""" return f"https://{DOMAIN}/author/{self.id}" + class Meta: + """sets up indexes and triggers""" + + triggers = [ + pgtrigger.Trigger( + name="reset_search_vector_on_author_edit", + when=pgtrigger.After, + operation=pgtrigger.UpdateOf("name"), + func=format_trigger( + """WITH book AS ( + SELECT bookwyrm_book.id AS row_id + FROM bookwyrm_author + LEFT OUTER JOIN bookwyrm_book_authors + ON bookwyrm_book_authors.id = new.id + LEFT OUTER JOIN bookwyrm_book + ON bookwyrm_book.id = bookwyrm_book_authors.book_id + ) + UPDATE bookwyrm_book + SET search_vector = '' + FROM book + WHERE id = book.row_id; + RETURN new; + """ + ), + ) + ] + activity_serializer = activitypub.Author diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 4cecc4df6..aaa50c56d 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -104,6 +104,7 @@ INSTALLED_APPS = [ "celery", "django_celery_beat", "imagekit", + "pgtrigger", "storages", ] diff --git a/bookwyrm/utils/db.py b/bookwyrm/utils/db.py new file mode 100644 index 000000000..8b74d9bf5 --- /dev/null +++ b/bookwyrm/utils/db.py @@ -0,0 +1,22 @@ +""" Database utilities """ + +from typing import cast +import sqlparse # type: ignore + + +def format_trigger(sql: str) -> str: + """format SQL trigger before storing + + we remove whitespace and use consistent casing so as to avoid migrations + due to formatting changes. + """ + return cast( + str, + sqlparse.format( + sql, + strip_comments=True, + strip_whitespace=True, + keyword_case="upper", + identifier_case="lower", + ), + ) diff --git a/requirements.txt b/requirements.txt index 36192f148..05fd9d2b4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,6 +7,7 @@ django-celery-beat==2.4.0 django-compressor==4.3.1 django-imagekit==4.1.0 django-model-utils==4.3.1 +django-pgtrigger==4.10.0 django-sass-processor==1.2.2 django-csp==3.7 environs==9.5.0 From bcb3a343d4514fd9bc2674a918755aaa832c8886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 01:49:14 -0300 Subject: [PATCH 05/37] Fix JOIN in `author_search_vector_trigger`, add missing WHERE clause --- .../0191_migrate_search_vec_triggers_to_pgtriggers.py | 4 ++-- bookwyrm/models/author.py | 3 ++- bookwyrm/tests/test_book_search.py | 2 -- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py index 10354fa67..1e51e529e 100644 --- a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -32,8 +32,8 @@ class Migration(migrations.Migration): trigger=pgtrigger.compiler.Trigger( name="reset_search_vector_on_author_edit", sql=pgtrigger.compiler.UpsertTriggerSql( - func="WITH book AS (SELECT bookwyrm_book.id AS row_id FROM bookwyrm_author LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book_authors.id = new.id LEFT OUTER JOIN bookwyrm_book ON bookwyrm_book.id = bookwyrm_book_authors.book_id) UPDATE bookwyrm_book SET search_vector = '' FROM book WHERE id = book.row_id;RETURN NEW;", - hash="9c0a472e2bf60e63d593cce49f47972c7b227a00", + func="WITH book AS (SELECT bookwyrm_book.id AS row_id FROM bookwyrm_author LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book_authors.author_id = bookwyrm_author.id LEFT OUTER JOIN bookwyrm_book ON bookwyrm_book.id = bookwyrm_book_authors.book_id WHERE bookwyrm_author.id = new.id ) UPDATE bookwyrm_book SET search_vector = '' FROM book WHERE id = book.row_id;RETURN NEW;", + hash="abc8ea76fa1bf02a0f56aaae390c1b970bef1278", operation='UPDATE OF "name"', pgid="pgtrigger_reset_search_vector_on_author_edit_a447c", table="bookwyrm_author", diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 9b4f3c1bd..6c5beba50 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -81,9 +81,10 @@ class Author(BookDataModel): SELECT bookwyrm_book.id AS row_id FROM bookwyrm_author LEFT OUTER JOIN bookwyrm_book_authors - ON bookwyrm_book_authors.id = new.id + ON bookwyrm_book_authors.author_id = bookwyrm_author.id LEFT OUTER JOIN bookwyrm_book ON bookwyrm_book.id = bookwyrm_book_authors.book_id + WHERE bookwyrm_author.id = new.id ) UPDATE bookwyrm_book SET search_vector = '' diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index e66ea97be..d2d5b692e 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -1,6 +1,5 @@ """ test searching for books """ import datetime -from unittest import expectedFailure from django.db import connection from django.test import TestCase from django.utils import timezone @@ -207,7 +206,6 @@ class SearchVectorTriggers(TestCase): self.assertEqual(self.edition, self._search_first("Mozilla")) self.assertEqual(self.edition, self._search_first("Name")) - @expectedFailure def test_search_after_updated_author_name(self): """book found under new author name""" self.assertEqual(self.edition, self._search_first("Name")) From 8df408e07eccefb934373f5c69b5068c1bee5046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 16:46:54 -0300 Subject: [PATCH 06/37] Define `search_vector_trigger` via Book.Meta.triggers --- ...grate_search_vec_triggers_to_pgtriggers.py | 20 +++++++++++ bookwyrm/models/book.py | 33 ++++++++++++++++++- bookwyrm/tests/test_book_search.py | 1 - bookwyrm/utils/db.py | 1 + 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py index 1e51e529e..929063781 100644 --- a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -27,6 +27,20 @@ class Migration(migrations.Migration): ] operations = [ + pgtrigger.migrations.AddTrigger( + model_name="book", + trigger=pgtrigger.compiler.Trigger( + name="update_search_vector_on_book_edit", + sql=pgtrigger.compiler.UpsertTriggerSql( + func="new.search_vector := coalesce(nullif(setweight(to_tsvector('english', coalesce(new.title, '')), 'A'), ''), setweight(to_tsvector('simple', coalesce(new.title, '')), 'A')) || setweight(to_tsvector('english', coalesce(new.subtitle, '')), 'B') || (SELECT setweight(to_tsvector('simple', coalesce(array_to_string(array_agg(bookwyrm_author.name), ' '), '')), 'C') FROM bookwyrm_book LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book.id = bookwyrm_book_authors.book_id LEFT OUTER JOIN bookwyrm_author ON bookwyrm_book_authors.author_id = bookwyrm_author.id WHERE bookwyrm_book.id = new.id ) || setweight(to_tsvector('english', coalesce(new.series, '')), 'D');RETURN NEW;", + hash="9c898d46dfb7492ecd18f6c692bbecfa548f0e85", + operation='INSERT OR UPDATE OF "title", "subtitle", "series", "search_vector"', + pgid="pgtrigger_update_search_vector_on_book_edit_bec58", + table="bookwyrm_book", + when="BEFORE", + ), + ), + ), pgtrigger.migrations.AddTrigger( model_name="author", trigger=pgtrigger.compiler.Trigger( @@ -41,6 +55,12 @@ class Migration(migrations.Migration): ), ), ), + migrations.RunSQL( + sql="""DROP TRIGGER IF EXISTS search_vector_trigger ON bookwyrm_book; + DROP FUNCTION IF EXISTS book_trigger; + """, + reverse_sql=search_vector_trigger.sql, + ), migrations.RunSQL( sql="""DROP TRIGGER IF EXISTS author_search_vector_trigger ON bookwyrm_author; DROP FUNCTION IF EXISTS author_trigger; diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 6893b9da1..ed26752e3 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -13,6 +13,7 @@ from django.utils.translation import gettext_lazy as _ from model_utils import FieldTracker from model_utils.managers import InheritanceManager from imagekit.models import ImageSpecField +import pgtrigger from bookwyrm import activitypub from bookwyrm.isbn.isbn import hyphenator_singleton as hyphenator @@ -24,6 +25,7 @@ from bookwyrm.settings import ( ENABLE_PREVIEW_IMAGES, ENABLE_THUMBNAIL_GENERATION, ) +from bookwyrm.utils.db import format_trigger from .activitypub_mixin import OrderedCollectionPageMixin, ObjectMixin from .base_model import BookWyrmModel @@ -232,9 +234,38 @@ class Book(BookDataModel): ) class Meta: - """sets up postgres GIN index field""" + """set up indexes and triggers""" + + # pylint: disable=line-too-long indexes = (GinIndex(fields=["search_vector"]),) + triggers = [ + pgtrigger.Trigger( + name="update_search_vector_on_book_edit", + when=pgtrigger.Before, + operation=pgtrigger.Insert + | pgtrigger.UpdateOf("title", "subtitle", "series", "search_vector"), + func=format_trigger( + """new.search_vector := + COALESCE( + NULLIF(setweight(to_tsvector('english', COALESCE(new.title, '')), 'A'), ''), + setweight(to_tsvector('simple', COALESCE(new.title, '')), 'A') + ) || + setweight(to_tsvector('english', COALESCE(new.subtitle, '')), 'B') || + (SELECT setweight(to_tsvector('simple', COALESCE(array_to_string(ARRAY_AGG(bookwyrm_author.name), ' '), '')), 'C') + FROM bookwyrm_book + LEFT OUTER JOIN bookwyrm_book_authors + ON bookwyrm_book.id = bookwyrm_book_authors.book_id + LEFT OUTER JOIN bookwyrm_author + ON bookwyrm_book_authors.author_id = bookwyrm_author.id + WHERE bookwyrm_book.id = new.id + ) || + setweight(to_tsvector('english', COALESCE(new.series, '')), 'D'); + RETURN new; + """ + ), + ) + ] class Work(OrderedCollectionPageMixin, Book): diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index d2d5b692e..f9c75d279 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -213,7 +213,6 @@ class SearchVectorTriggers(TestCase): self.author.name = "Identifier" self.author.save(broadcast=False) - self.edition.refresh_from_db() self.assertFalse(self._search("Name")) self.assertEqual(self.edition, self._search_first("Identifier")) diff --git a/bookwyrm/utils/db.py b/bookwyrm/utils/db.py index 8b74d9bf5..2bb3b9ff6 100644 --- a/bookwyrm/utils/db.py +++ b/bookwyrm/utils/db.py @@ -16,6 +16,7 @@ def format_trigger(sql: str) -> str: sql, strip_comments=True, strip_whitespace=True, + use_space_around_operators=True, keyword_case="upper", identifier_case="lower", ), From 9bcb5b80ea9dd4f11cae26ad1fc752a7cb71dcde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 18:13:05 -0300 Subject: [PATCH 07/37] Further simplify bookwyrm_author trigger --- ..._migrate_search_vec_triggers_to_pgtriggers.py | 4 ++-- bookwyrm/models/author.py | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py index 929063781..ddfe74a8b 100644 --- a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -46,8 +46,8 @@ class Migration(migrations.Migration): trigger=pgtrigger.compiler.Trigger( name="reset_search_vector_on_author_edit", sql=pgtrigger.compiler.UpsertTriggerSql( - func="WITH book AS (SELECT bookwyrm_book.id AS row_id FROM bookwyrm_author LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book_authors.author_id = bookwyrm_author.id LEFT OUTER JOIN bookwyrm_book ON bookwyrm_book.id = bookwyrm_book_authors.book_id WHERE bookwyrm_author.id = new.id ) UPDATE bookwyrm_book SET search_vector = '' FROM book WHERE id = book.row_id;RETURN NEW;", - hash="abc8ea76fa1bf02a0f56aaae390c1b970bef1278", + func="WITH updated_books AS (SELECT book_id FROM bookwyrm_book_authors WHERE author_id = new.id ) UPDATE bookwyrm_book SET search_vector = '' FROM updated_books WHERE id = updated_books.book_id;RETURN NEW;", + hash="e7bbf08711ff3724c58f4d92fb7a082ffb3d7826", operation='UPDATE OF "name"', pgid="pgtrigger_reset_search_vector_on_author_edit_a447c", table="bookwyrm_author", diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 6c5beba50..154b00ccb 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -77,19 +77,15 @@ class Author(BookDataModel): when=pgtrigger.After, operation=pgtrigger.UpdateOf("name"), func=format_trigger( - """WITH book AS ( - SELECT bookwyrm_book.id AS row_id - FROM bookwyrm_author - LEFT OUTER JOIN bookwyrm_book_authors - ON bookwyrm_book_authors.author_id = bookwyrm_author.id - LEFT OUTER JOIN bookwyrm_book - ON bookwyrm_book.id = bookwyrm_book_authors.book_id - WHERE bookwyrm_author.id = new.id + """WITH updated_books AS ( + SELECT book_id + FROM bookwyrm_book_authors + WHERE author_id = new.id ) UPDATE bookwyrm_book SET search_vector = '' - FROM book - WHERE id = book.row_id; + FROM updated_books + WHERE id = updated_books.book_id; RETURN new; """ ), From bbfbd1e97ab201cd87c6414127c603f615923ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 20:54:09 -0300 Subject: [PATCH 08/37] Add tests for trigger code (i.e. how search_vector is computed) --- bookwyrm/tests/test_book_search.py | 67 +++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index f9c75d279..99b62f0a0 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -137,6 +137,69 @@ class BookSearch(TestCase): self.assertEqual(result.title, "Title") +class SearchVectorTest(TestCase): + """check search_vector is computed correctly""" + + def test_search_vector_simple(self): + """simplest search vector""" + book = self._create_book("Book", "Mary") + self.assertEqual(book.search_vector, "'book':1A 'mary':2C") # A > C (priority) + + def test_search_vector_all_parts(self): + """search vector with subtitle and series""" + # for a book like this we call `to_tsvector("Book Long Mary Bunch")`, hence the + # indexes in the search vector. (priority "D" is the default, and never shown.) + book = self._create_book("Book", "Mary", subtitle="Long", series="Bunch") + self.assertEqual(book.search_vector, "'book':1A 'bunch':4 'long':2B 'mary':3C") + + def test_search_vector_parse_book(self): + """book parts are parsed in english""" + # FIXME: at some point this should stop being the default. + book = self._create_book( + "Edition", "Editor", series="Castle", subtitle="Writing" + ) + self.assertEqual( + book.search_vector, "'castl':4 'edit':1A 'editor':3C 'write':2B" + ) + + def test_search_vector_parse_author(self): + """author name is not stem'd or affected by stop words""" + book = self._create_book("Writing", "Writes") + self.assertEqual(book.search_vector, "'write':1A 'writes':2C") + + book = self._create_book("She Is Writing", "She Writes") + self.assertEqual(book.search_vector, "'she':4C 'write':3A 'writes':5C") + + def test_search_vector_parse_title_empty(self): + """empty parse in English retried as simple title""" + book = self._create_book("Here We", "John") + self.assertEqual(book.search_vector, "'here':1A 'john':3C 'we':2A") + + book = self._create_book("Hear We Come", "John") + self.assertEqual(book.search_vector, "'come':3A 'hear':1A 'john':4C") + + @staticmethod + def _create_book( + title, author_name, /, *, subtitle="", series="", author_alias=None + ): + """quickly create a book""" + work = models.Work.objects.create(title="work") + author = models.Author.objects.create( + name=author_name, aliases=author_alias or [] + ) + edition = models.Edition.objects.create( + title=title, + series=series or None, + subtitle=subtitle or None, + isbn_10="0000000000", + parent_work=work, + ) + edition.authors.add(author) + edition.save(broadcast=False) + edition.refresh_from_db() + return edition + + class SearchVectorTriggers(TestCase): """look for books as they change""" @@ -222,8 +285,8 @@ class SearchVectorTriggers(TestCase): """wrapper around search_title_author""" return self._search(query, return_first=True) - # pylint: disable-next=no-self-use - def _search(self, query, *, return_first=False): + @staticmethod + def _search(query, *, return_first=False): """wrapper around search_title_author""" return book_search.search_title_author( query, min_confidence=0, return_first=return_first From b5805accacb497be62cb4e7bb54a52258a1d12e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 25 Nov 2023 21:47:13 -0300 Subject: [PATCH 09/37] Minor improvements to bookwyrm_book trigger code - do not COALESCE columns that cannot be NULL - do not bring bookwyrm_book to author names JOIN - add comments documenting the four steps --- ...grate_search_vec_triggers_to_pgtriggers.py | 4 ++-- bookwyrm/models/book.py | 21 ++++++++++--------- bookwyrm/tests/test_book_search.py | 16 +++++++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py index ddfe74a8b..5e798b654 100644 --- a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -32,8 +32,8 @@ class Migration(migrations.Migration): trigger=pgtrigger.compiler.Trigger( name="update_search_vector_on_book_edit", sql=pgtrigger.compiler.UpsertTriggerSql( - func="new.search_vector := coalesce(nullif(setweight(to_tsvector('english', coalesce(new.title, '')), 'A'), ''), setweight(to_tsvector('simple', coalesce(new.title, '')), 'A')) || setweight(to_tsvector('english', coalesce(new.subtitle, '')), 'B') || (SELECT setweight(to_tsvector('simple', coalesce(array_to_string(array_agg(bookwyrm_author.name), ' '), '')), 'C') FROM bookwyrm_book LEFT OUTER JOIN bookwyrm_book_authors ON bookwyrm_book.id = bookwyrm_book_authors.book_id LEFT OUTER JOIN bookwyrm_author ON bookwyrm_book_authors.author_id = bookwyrm_author.id WHERE bookwyrm_book.id = new.id ) || setweight(to_tsvector('english', coalesce(new.series, '')), 'D');RETURN NEW;", - hash="9c898d46dfb7492ecd18f6c692bbecfa548f0e85", + func="new.search_vector := setweight(coalesce(nullif(to_tsvector('english', new.title), ''), to_tsvector('simple', new.title)), 'A') || setweight(to_tsvector('english', coalesce(new.subtitle, '')), 'B') || (SELECT setweight(to_tsvector('simple', coalesce(array_to_string(array_agg(bookwyrm_author.name), ' '), '')), 'C') FROM bookwyrm_author LEFT JOIN bookwyrm_book_authors ON bookwyrm_author.id = bookwyrm_book_authors.author_id WHERE bookwyrm_book_authors.book_id = new.id ) || setweight(to_tsvector('english', coalesce(new.series, '')), 'D');RETURN NEW;", + hash="77d6399497c0a89b0bf09d296e33c396da63705c", operation='INSERT OR UPDATE OF "title", "subtitle", "series", "search_vector"', pgid="pgtrigger_update_search_vector_on_book_edit_bec58", table="bookwyrm_book", diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index ed26752e3..e167e2138 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -247,19 +247,20 @@ class Book(BookDataModel): | pgtrigger.UpdateOf("title", "subtitle", "series", "search_vector"), func=format_trigger( """new.search_vector := - COALESCE( - NULLIF(setweight(to_tsvector('english', COALESCE(new.title, '')), 'A'), ''), - setweight(to_tsvector('simple', COALESCE(new.title, '')), 'A') - ) || + -- title, with priority A (parse in English, default to simple if empty) + setweight(COALESCE(nullif( + to_tsvector('english', new.title), ''), + to_tsvector('simple', new.title)), 'A') || + -- subtitle, with priority B (always in English?) setweight(to_tsvector('english', COALESCE(new.subtitle, '')), 'B') || + -- list of authors, with priority C (TODO: add aliases?, bookwyrm-social#3063) (SELECT setweight(to_tsvector('simple', COALESCE(array_to_string(ARRAY_AGG(bookwyrm_author.name), ' '), '')), 'C') - FROM bookwyrm_book - LEFT OUTER JOIN bookwyrm_book_authors - ON bookwyrm_book.id = bookwyrm_book_authors.book_id - LEFT OUTER JOIN bookwyrm_author - ON bookwyrm_book_authors.author_id = bookwyrm_author.id - WHERE bookwyrm_book.id = new.id + FROM bookwyrm_author + LEFT JOIN bookwyrm_book_authors + ON bookwyrm_author.id = bookwyrm_book_authors.author_id + WHERE bookwyrm_book_authors.book_id = new.id ) || + --- last: series name, with lowest priority setweight(to_tsvector('english', COALESCE(new.series, '')), 'D'); RETURN new; """ diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index 99b62f0a0..e9f550a88 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -178,15 +178,17 @@ class SearchVectorTest(TestCase): book = self._create_book("Hear We Come", "John") self.assertEqual(book.search_vector, "'come':3A 'hear':1A 'john':4C") + def test_search_vector_no_author(self): + """book with no authors gets processed normally""" + book = self._create_book("Book", None, series="Bunch") + self.assertEqual(book.search_vector, "'book':1A 'bunch':2") + @staticmethod def _create_book( title, author_name, /, *, subtitle="", series="", author_alias=None ): """quickly create a book""" work = models.Work.objects.create(title="work") - author = models.Author.objects.create( - name=author_name, aliases=author_alias or [] - ) edition = models.Edition.objects.create( title=title, series=series or None, @@ -194,8 +196,12 @@ class SearchVectorTest(TestCase): isbn_10="0000000000", parent_work=work, ) - edition.authors.add(author) - edition.save(broadcast=False) + if author_name is not None: + author = models.Author.objects.create( + name=author_name, aliases=author_alias or [] + ) + edition.authors.add(author) + edition.save(broadcast=False) edition.refresh_from_db() return edition From d6eb390ceed8e6f63926aa8b54a4ef9314b93c75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 26 Nov 2023 15:57:51 -0300 Subject: [PATCH 10/37] Add test that forces `book_authors_search_vector_trigger` to execute --- bookwyrm/tests/test_book_search.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index e9f550a88..42e57f773 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -275,6 +275,29 @@ class SearchVectorTriggers(TestCase): self.assertEqual(self.edition, self._search_first("Mozilla")) self.assertEqual(self.edition, self._search_first("Name")) + def test_search_after_author_add_remove_sql(self): + """add/remove author through SQL to ensure execution of book_authors trigger""" + # Tests calling edition.save(), above, pass even if the trigger in + # bookwyrm_book_authors is removed (probably because they trigger the one + # in bookwyrm_book directly). Here we make sure to exercise the former. + new_author = models.Author.objects.create(name="Mozilla") + + with connection.cursor() as cursor: + cursor.execute( + "DELETE FROM bookwyrm_book_authors WHERE book_id = %s", + [self.edition.id], + ) + self.assertFalse(self._search("Name")) + self.assertFalse(self._search("Mozilla")) + + with connection.cursor() as cursor: + cursor.execute( + "INSERT INTO bookwyrm_book_authors (book_id,author_id) VALUES (%s,%s)", + [self.edition.id, new_author.id], + ) + self.assertFalse(self._search("Name")) + self.assertEqual(self.edition, self._search_first("Mozilla")) + def test_search_after_updated_author_name(self): """book found under new author name""" self.assertEqual(self.edition, self._search_first("Name")) From eb13eb98828509b96c34fbbcae272d37050789f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sat, 13 Jan 2024 18:55:13 +0100 Subject: [PATCH 11/37] Invalidate `active_shelf` when switching editions --- bookwyrm/views/books/editions.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bookwyrm/views/books/editions.py b/bookwyrm/views/books/editions.py index 54d1bd84c..0d59d0bcc 100644 --- a/bookwyrm/views/books/editions.py +++ b/bookwyrm/views/books/editions.py @@ -3,6 +3,7 @@ from functools import reduce import operator from django.contrib.auth.decorators import login_required +from django.core.cache import cache as django_cache from django.core.paginator import Paginator from django.db import transaction from django.db.models import Q @@ -103,4 +104,10 @@ def switch_edition(request): readthrough.book = new_edition readthrough.save() + django_cache.delete_many( + [ + f"active_shelf-{request.user.id}-{book_id}" + for book_id in new_edition.parent_work.editions.values_list("id", flat=True) + ] + ) return redirect(f"/book/{new_edition.id}") From 6ac38564e2c47f73df2f949ac6060c06ce167dd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20C=C3=A1mara?= Date: Sat, 3 Feb 2024 22:55:33 +0000 Subject: [PATCH 12/37] Add wikidata field for authors --- bookwyrm/forms/author.py | 2 ++ bookwyrm/models/author.py | 3 +++ bookwyrm/tests/data/user_import.json | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bookwyrm/forms/author.py b/bookwyrm/forms/author.py index 5b54a07b5..a3a759af7 100644 --- a/bookwyrm/forms/author.py +++ b/bookwyrm/forms/author.py @@ -15,6 +15,7 @@ class AuthorForm(CustomForm): "aliases", "bio", "wikipedia_link", + "wikidata", "website", "born", "died", @@ -32,6 +33,7 @@ class AuthorForm(CustomForm): "wikipedia_link": forms.TextInput( attrs={"aria-describedby": "desc_wikipedia_link"} ), + "wikidata": forms.TextInput(attrs={"aria-describedby": "desc_wikidata"}), "website": forms.TextInput(attrs={"aria-describedby": "desc_website"}), "born": forms.SelectDateWidget(attrs={"aria-describedby": "desc_born"}), "died": forms.SelectDateWidget(attrs={"aria-describedby": "desc_died"}), diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 981e3c0cc..b32d49d54 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -18,6 +18,9 @@ class Author(BookDataModel): wikipedia_link = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True ) + wikidata = fields.CharField( + max_length=255, blank=True, null=True, deduplication_field=True + ) isni = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True ) diff --git a/bookwyrm/tests/data/user_import.json b/bookwyrm/tests/data/user_import.json index 0318ddfeb..ffe2e7d9e 100644 --- a/bookwyrm/tests/data/user_import.json +++ b/bookwyrm/tests/data/user_import.json @@ -136,6 +136,7 @@ ], "bio": "

American political scientist and anthropologist

", "wikipediaLink": "https://en.wikipedia.org/wiki/James_C._Scott", + "wikidata": "Q3025403", "website": "", "@context": "https://www.w3.org/ns/activitystreams" } @@ -320,6 +321,7 @@ "aliases": [], "bio": "", "wikipediaLink": "", + "wikidata": "", "website": "", "@context": "https://www.w3.org/ns/activitystreams" } @@ -396,4 +398,4 @@ "https://your.domain.here/user/rat" ], "blocks": ["https://your.domain.here/user/badger"] -} \ No newline at end of file +} From 89d8537e1bd3d2daa3cd1fbc6d989a5d4be56220 Mon Sep 17 00:00:00 2001 From: Carlos Camara Date: Mon, 5 Feb 2024 22:08:34 +0000 Subject: [PATCH 13/37] Add wikidata field to author's template --- bookwyrm/templates/author/edit_author.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/templates/author/edit_author.html b/bookwyrm/templates/author/edit_author.html index 12ddc4d28..f3e908c9b 100644 --- a/bookwyrm/templates/author/edit_author.html +++ b/bookwyrm/templates/author/edit_author.html @@ -55,6 +55,8 @@

{{ form.wikipedia_link }}

+

{{ form.wikidata }}

+ {% include 'snippets/form_errors.html' with errors_list=form.wikipedia_link.errors id="desc_wikipedia_link" %}

{{ form.website }}

From 12b469a0d6a2c554715f53bcf2a333d16dbed8f2 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 13 Mar 2024 12:30:52 +0100 Subject: [PATCH 14/37] CI: use actions/checkout@v4 --- .github/workflows/black.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/curlylint.yaml | 2 +- .github/workflows/django-tests.yml | 2 +- .github/workflows/lint-frontend.yaml | 2 +- .github/workflows/mypy.yml | 2 +- .github/workflows/prettier.yaml | 2 +- .github/workflows/pylint.yml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index 4e7be4af3..7ac208c94 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -10,7 +10,7 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-python@v4 - uses: psf/black@22.12.0 with: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 68bb05d7e..51316ef62 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -36,7 +36,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/curlylint.yaml b/.github/workflows/curlylint.yaml index 8d5c6b4f7..10ad04ce1 100644 --- a/.github/workflows/curlylint.yaml +++ b/.github/workflows/curlylint.yaml @@ -10,7 +10,7 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install curlylint run: pip install curlylint diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml index de71d9bcf..9a2c615a4 100644 --- a/.github/workflows/django-tests.yml +++ b/.github/workflows/django-tests.yml @@ -23,7 +23,7 @@ jobs: ports: - 5432:5432 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python uses: actions/setup-python@v4 with: diff --git a/.github/workflows/lint-frontend.yaml b/.github/workflows/lint-frontend.yaml index 0d0559e40..21f11ebf3 100644 --- a/.github/workflows/lint-frontend.yaml +++ b/.github/workflows/lint-frontend.yaml @@ -19,7 +19,7 @@ jobs: steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it. - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install modules run: npm install stylelint stylelint-config-recommended stylelint-config-standard stylelint-order eslint diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 6df987aa4..a198efc21 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python 3.11 uses: actions/setup-python@v4 with: diff --git a/.github/workflows/prettier.yaml b/.github/workflows/prettier.yaml index 501516ae1..9c05c7476 100644 --- a/.github/workflows/prettier.yaml +++ b/.github/workflows/prettier.yaml @@ -14,7 +14,7 @@ jobs: steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it. - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install modules run: npm install prettier@2.5.1 diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml index ab8633b48..85f275020 100644 --- a/.github/workflows/pylint.yml +++ b/.github/workflows/pylint.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python 3.11 uses: actions/setup-python@v4 with: From 6af0a0883827b65e5776c2d1e2f7264600ae7d9f Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 13 Mar 2024 12:35:23 +0100 Subject: [PATCH 15/37] CI: use actions/setup-python@v5 and cache pip --- .github/workflows/black.yml | 2 +- .github/workflows/django-tests.yml | 3 ++- .github/workflows/mypy.yml | 3 ++- .github/workflows/pylint.yml | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index 7ac208c94..0633dedb7 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 - uses: psf/black@22.12.0 with: version: 22.12.0 diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml index 9a2c615a4..7d9cb3cba 100644 --- a/.github/workflows/django-tests.yml +++ b/.github/workflows/django-tests.yml @@ -25,9 +25,10 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: 3.11 + cache: pip - name: Install Dependencies run: | python -m pip install --upgrade pip diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index a198efc21..d1e3f9fc9 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -14,9 +14,10 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python 3.11 - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: 3.11 + cache: pip - name: Install Dependencies run: | python -m pip install --upgrade pip diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml index 85f275020..915e3154c 100644 --- a/.github/workflows/pylint.yml +++ b/.github/workflows/pylint.yml @@ -14,9 +14,10 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python 3.11 - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: 3.11 + cache: pip - name: Install Dependencies run: | python -m pip install --upgrade pip From 74fdd9a85a8741ea5ac71432ebaffd6b800f8d23 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 13 Mar 2024 10:31:09 +0100 Subject: [PATCH 16/37] CI: simplify pytest setup --- .github/workflows/django-tests.yml | 52 +++++++++--------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml index 7d9cb3cba..da237592f 100644 --- a/.github/workflows/django-tests.yml +++ b/.github/workflows/django-tests.yml @@ -7,12 +7,20 @@ on: jobs: build: - - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest + env: # overrides for .env.example + POSTGRES_HOST: 127.0.0.1 + PGPORT: 5432 + POSTGRES_USER: postgres + POSTGRES_PASSWORD: hunter2 + POSTGRES_DB: github_actions + SECRET_KEY: beepbeep + EMAIL_HOST_USER: "" + EMAIL_HOST_PASSWORD: "" services: postgres: image: postgres:13 - env: + env: # does not inherit from jobs.build.env POSTGRES_USER: postgres POSTGRES_PASSWORD: hunter2 options: >- @@ -33,39 +41,9 @@ jobs: run: | python -m pip install --upgrade pip pip install -r requirements.txt + - name: Set up .env + run: cp .env.example .env - name: Check migrations up-to-date - run: | - python ./manage.py makemigrations --check - env: - SECRET_KEY: beepbeep - DOMAIN: your.domain.here - EMAIL_HOST: "" - EMAIL_HOST_USER: "" - EMAIL_HOST_PASSWORD: "" + run: python ./manage.py makemigrations --check - name: Run Tests - env: - SECRET_KEY: beepbeep - DEBUG: false - USE_HTTPS: true - DOMAIN: your.domain.here - BOOKWYRM_DATABASE_BACKEND: postgres - MEDIA_ROOT: images/ - POSTGRES_PASSWORD: hunter2 - POSTGRES_USER: postgres - POSTGRES_DB: github_actions - POSTGRES_HOST: 127.0.0.1 - CELERY_BROKER: "" - REDIS_BROKER_PORT: 6379 - REDIS_BROKER_PASSWORD: beep - USE_DUMMY_CACHE: true - FLOWER_PORT: 8888 - EMAIL_HOST: "smtp.mailgun.org" - EMAIL_PORT: 587 - EMAIL_HOST_USER: "" - EMAIL_HOST_PASSWORD: "" - EMAIL_USE_TLS: true - ENABLE_PREVIEW_IMAGES: false - ENABLE_THUMBNAIL_GENERATION: true - HTTP_X_FORWARDED_PROTO: false - run: | - pytest -n 3 + run: pytest -n 3 From 383e6533e11da5c88ceebb9cfad89d6980f7d430 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 13 Mar 2024 11:56:16 +0100 Subject: [PATCH 17/37] CI: use pytest-github-actions-annotate-failures --- .github/workflows/django-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml index da237592f..ce9b2b7b1 100644 --- a/.github/workflows/django-tests.yml +++ b/.github/workflows/django-tests.yml @@ -41,6 +41,7 @@ jobs: run: | python -m pip install --upgrade pip pip install -r requirements.txt + pip install pytest-github-actions-annotate-failures - name: Set up .env run: cp .env.example .env - name: Check migrations up-to-date From 4e20e430379a820c1e4f88590bf6afe08cba8249 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 13 Mar 2024 12:48:02 +0100 Subject: [PATCH 18/37] CI: merge all Python actions into one file --- .github/workflows/black.yml | 17 ----- .github/workflows/django-tests.yml | 50 --------------- .github/workflows/mypy.yml | 51 --------------- .github/workflows/pylint.yml | 28 --------- .github/workflows/python.yml | 99 ++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 146 deletions(-) delete mode 100644 .github/workflows/black.yml delete mode 100644 .github/workflows/django-tests.yml delete mode 100644 .github/workflows/mypy.yml delete mode 100644 .github/workflows/pylint.yml create mode 100644 .github/workflows/python.yml diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml deleted file mode 100644 index 0633dedb7..000000000 --- a/.github/workflows/black.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: Python Formatting (run ./bw-dev black to fix) - -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] - -jobs: - lint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - - uses: psf/black@22.12.0 - with: - version: 22.12.0 diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml deleted file mode 100644 index ce9b2b7b1..000000000 --- a/.github/workflows/django-tests.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: Run Python Tests -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] - -jobs: - build: - runs-on: ubuntu-latest - env: # overrides for .env.example - POSTGRES_HOST: 127.0.0.1 - PGPORT: 5432 - POSTGRES_USER: postgres - POSTGRES_PASSWORD: hunter2 - POSTGRES_DB: github_actions - SECRET_KEY: beepbeep - EMAIL_HOST_USER: "" - EMAIL_HOST_PASSWORD: "" - services: - postgres: - image: postgres:13 - env: # does not inherit from jobs.build.env - POSTGRES_USER: postgres - POSTGRES_PASSWORD: hunter2 - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - steps: - - uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: 3.11 - cache: pip - - name: Install Dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - pip install pytest-github-actions-annotate-failures - - name: Set up .env - run: cp .env.example .env - - name: Check migrations up-to-date - run: python ./manage.py makemigrations --check - - name: Run Tests - run: pytest -n 3 diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml deleted file mode 100644 index d1e3f9fc9..000000000 --- a/.github/workflows/mypy.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: Mypy - -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] - -jobs: - build: - - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - name: Set up Python 3.11 - uses: actions/setup-python@v5 - with: - python-version: 3.11 - cache: pip - - name: Install Dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - - name: Analysing the code with mypy - env: - SECRET_KEY: beepbeep - DEBUG: false - USE_HTTPS: true - DOMAIN: your.domain.here - BOOKWYRM_DATABASE_BACKEND: postgres - MEDIA_ROOT: images/ - POSTGRES_PASSWORD: hunter2 - POSTGRES_USER: postgres - POSTGRES_DB: github_actions - POSTGRES_HOST: 127.0.0.1 - CELERY_BROKER: "" - REDIS_BROKER_PORT: 6379 - REDIS_BROKER_PASSWORD: beep - USE_DUMMY_CACHE: true - FLOWER_PORT: 8888 - EMAIL_HOST: "smtp.mailgun.org" - EMAIL_PORT: 587 - EMAIL_HOST_USER: "" - EMAIL_HOST_PASSWORD: "" - EMAIL_USE_TLS: true - ENABLE_PREVIEW_IMAGES: false - ENABLE_THUMBNAIL_GENERATION: true - HTTP_X_FORWARDED_PROTO: false - run: | - mypy bookwyrm celerywyrm diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml deleted file mode 100644 index 915e3154c..000000000 --- a/.github/workflows/pylint.yml +++ /dev/null @@ -1,28 +0,0 @@ -name: Pylint - -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] - -jobs: - build: - - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - name: Set up Python 3.11 - uses: actions/setup-python@v5 - with: - python-version: 3.11 - cache: pip - - name: Install Dependencies - run: | - python -m pip install --upgrade pip - pip install -r requirements.txt - - name: Analysing the code with pylint - run: | - pylint bookwyrm/ - diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml new file mode 100644 index 000000000..dcbe05aee --- /dev/null +++ b/.github/workflows/python.yml @@ -0,0 +1,99 @@ +name: Python +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +# overrides for .env.example +env: + POSTGRES_HOST: 127.0.0.1 + PGPORT: 5432 + POSTGRES_USER: postgres + POSTGRES_PASSWORD: hunter2 + POSTGRES_DB: github_actions + SECRET_KEY: beepbeep + EMAIL_HOST_USER: "" + EMAIL_HOST_PASSWORD: "" + +jobs: + pytest: + name: Tests (pytest) + runs-on: ubuntu-latest + services: + postgres: + image: postgres:13 + env: # does not inherit from jobs.build.env + POSTGRES_USER: postgres + POSTGRES_PASSWORD: hunter2 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - uses: actions/checkout@v4 + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: 3.11 + cache: pip + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + pip install pytest-github-actions-annotate-failures + - name: Set up .env + run: cp .env.example .env + - name: Check migrations up-to-date + run: python ./manage.py makemigrations --check + - name: Run Tests + run: pytest -n 3 + + pylint: + name: Linting (pylint) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: 3.11 + cache: pip + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Analyse code with pylint + run: pylint bookwyrm/ + + mypy: + name: Typing (mypy) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: 3.11 + cache: pip + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Set up .env + run: cp .env.example .env + - name: Analyse code with mypy + run: mypy bookwyrm celerywyrm + + black: + name: Formatting (black; run ./bw-dev black to fix) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + - uses: psf/black@22.12.0 + with: + version: 22.12.0 From beb49af51492e310c433eb4ed23bb947fc52787e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 17 Mar 2024 21:46:34 -0300 Subject: [PATCH 19/37] Upgade django-pgtrigger to 4.11 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 05fd9d2b4..90ff18edb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ django-celery-beat==2.4.0 django-compressor==4.3.1 django-imagekit==4.1.0 django-model-utils==4.3.1 -django-pgtrigger==4.10.0 +django-pgtrigger==4.11.0 django-sass-processor==1.2.2 django-csp==3.7 environs==9.5.0 From 2cf7ed477df2dddefa899a0feaca9e917f2b4738 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Sun, 17 Mar 2024 22:37:28 -0300 Subject: [PATCH 20/37] Consolidate test_posgres.py into test_book_search.py These are tests I missed when first writing trigger tests in test_book_search.py. --- bookwyrm/tests/test_book_search.py | 60 ++++++++++++++++++++++- bookwyrm/tests/test_postgres.py | 77 ------------------------------ 2 files changed, 58 insertions(+), 79 deletions(-) delete mode 100644 bookwyrm/tests/test_postgres.py diff --git a/bookwyrm/tests/test_book_search.py b/bookwyrm/tests/test_book_search.py index 439d2edbf..0721cb142 100644 --- a/bookwyrm/tests/test_book_search.py +++ b/bookwyrm/tests/test_book_search.py @@ -184,11 +184,67 @@ class SearchVectorTest(TestCase): book = self._create_book("Hear We Come", "John") self.assertEqual(book.search_vector, "'come':3A 'hear':1A 'john':4C") + book = self._create_book("there there", "the") + self.assertEqual(book.search_vector, "'the':3C 'there':1A,2A") + def test_search_vector_no_author(self): """book with no authors gets processed normally""" book = self._create_book("Book", None, series="Bunch") self.assertEqual(book.search_vector, "'book':1A 'bunch':2") + book = self._create_book("there there", None) + self.assertEqual(book.search_vector, "'there':1A,2A") + + # n.b.: the following originally from test_posgres.py + + def test_search_vector_on_update(self): + """make sure that search_vector is being set correctly on edit""" + book = self._create_book("The Long Goodbye", None) + self.assertEqual(book.search_vector, "'goodby':3A 'long':2A") + + book.title = "The Even Longer Goodbye" + book.save(broadcast=False) + book.refresh_from_db() + self.assertEqual(book.search_vector, "'even':2A 'goodby':4A 'longer':3A") + + def test_search_vector_on_author_update(self): + """update search when an author name changes""" + book = self._create_book("The Long Goodbye", "The Rays") + self.assertEqual(book.search_vector, "'goodby':3A 'long':2A 'rays':5C 'the':4C") + + author = models.Author.objects.get(name="The Rays") + author.name = "Jeremy" + author.save(broadcast=False) + book.refresh_from_db() + self.assertEqual(book.search_vector, "'goodby':3A 'jeremy':4C 'long':2A") + + def test_search_vector_on_author_delete(self): + """update search when an author is deleted""" + book = self._create_book("The Long Goodbye", "The Rays") + self.assertEqual(book.search_vector, "'goodby':3A 'long':2A 'rays':5C 'the':4C") + + author = models.Author.objects.get(name="The Rays") + book.authors.remove(author) + book.refresh_from_db() + self.assertEqual(book.search_vector, "'goodby':3A 'long':2A") + + def test_search_vector_fields(self): + """language field irrelevant for search_vector""" + author = models.Author.objects.create(name="The Rays") + book = models.Edition.objects.create( + title="The Long Goodbye", + subtitle="wow cool", + series="series name", + languages=["irrelevant"], + ) + book.authors.add(author) + book.refresh_from_db() + self.assertEqual( + book.search_vector, + # pylint: disable-next=line-too-long + "'cool':5B 'goodby':3A 'long':2A 'name':9 'rays':7C 'seri':8 'the':6C 'wow':4B", + ) + @staticmethod def _create_book( title, author_name, /, *, subtitle="", series="", author_alias=None @@ -212,8 +268,8 @@ class SearchVectorTest(TestCase): return edition -class SearchVectorTriggers(TestCase): - """look for books as they change""" +class SearchVectorUpdates(TestCase): + """look for books as they change""" # functional tests of the above def setUp(self): """we need basic test data and mocks""" diff --git a/bookwyrm/tests/test_postgres.py b/bookwyrm/tests/test_postgres.py deleted file mode 100644 index 8fc3c9d59..000000000 --- a/bookwyrm/tests/test_postgres.py +++ /dev/null @@ -1,77 +0,0 @@ -""" django configuration of postgres """ -from unittest.mock import patch -from django.test import TestCase - -from bookwyrm import models - - -@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") -class PostgresTriggers(TestCase): - """special migrations, fancy stuff ya know""" - - def test_search_vector_on_create(self, _): - """make sure that search_vector is being set correctly on create""" - book = models.Edition.objects.create(title="The Long Goodbye") - book.refresh_from_db() - self.assertEqual(book.search_vector, "'goodby':3A 'long':2A") - - def test_search_vector_on_update(self, _): - """make sure that search_vector is being set correctly on edit""" - book = models.Edition.objects.create(title="The Long Goodbye") - book.title = "The Even Longer Goodbye" - book.save(broadcast=False) - book.refresh_from_db() - self.assertEqual(book.search_vector, "'even':2A 'goodby':4A 'longer':3A") - - def test_search_vector_fields(self, _): - """use multiple fields to create search vector""" - author = models.Author.objects.create(name="The Rays") - book = models.Edition.objects.create( - title="The Long Goodbye", - subtitle="wow cool", - series="series name", - languages=["irrelevant"], - ) - book.authors.add(author) - book.refresh_from_db() - # pylint: disable=line-too-long - self.assertEqual( - book.search_vector, - "'cool':5B 'goodby':3A 'long':2A 'name':9 'rays':7C 'seri':8 'the':6C 'wow':4B", - ) - - def test_search_vector_on_author_update(self, _): - """update search when an author name changes""" - author = models.Author.objects.create(name="The Rays") - book = models.Edition.objects.create( - title="The Long Goodbye", - ) - book.authors.add(author) - author.name = "Jeremy" - author.save(broadcast=False) - book.refresh_from_db() - - self.assertEqual(book.search_vector, "'goodby':3A 'jeremy':4C 'long':2A") - - def test_search_vector_on_author_delete(self, _): - """update search when an author name changes""" - author = models.Author.objects.create(name="Jeremy") - book = models.Edition.objects.create( - title="The Long Goodbye", - ) - - book.authors.add(author) - book.refresh_from_db() - self.assertEqual(book.search_vector, "'goodby':3A 'jeremy':4C 'long':2A") - - book.authors.remove(author) - book.refresh_from_db() - self.assertEqual(book.search_vector, "'goodby':3A 'long':2A") - - def test_search_vector_stop_word_fallback(self, _): - """use a fallback when removing stop words leads to an empty vector""" - book = models.Edition.objects.create( - title="there there", - ) - book.refresh_from_db() - self.assertEqual(book.search_vector, "'there':1A,2A") From 3be227fc86c104ccc115bc373b92bf7ef4a8d371 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 22 Jan 2024 22:00:54 +0000 Subject: [PATCH 21/37] Bump pillow from 10.0.1 to 10.2.0 Bumps [pillow](https://github.com/python-pillow/Pillow) from 10.0.1 to 10.2.0. - [Release notes](https://github.com/python-pillow/Pillow/releases) - [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst) - [Commits](https://github.com/python-pillow/Pillow/compare/10.0.1...10.2.0) --- updated-dependencies: - dependency-name: pillow dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c769916a2..d59a62c98 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,7 +25,7 @@ opentelemetry-instrumentation-celery==0.37b0 opentelemetry-instrumentation-django==0.37b0 opentelemetry-instrumentation-psycopg2==0.37b0 opentelemetry-sdk==1.16.0 -Pillow==10.0.1 +Pillow==10.2.0 protobuf==3.20.* psycopg2==2.9.5 pycryptodome==3.19.1 From ccf2b16d735679cb3349e449612b61d1b9141676 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 19:52:40 +0100 Subject: [PATCH 22/37] requirements.txt: make typing-Pillow match Pillow --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index d59a62c98..5e8611076 100644 --- a/requirements.txt +++ b/requirements.txt @@ -53,7 +53,7 @@ pytidylib==0.3.2 types-bleach==6.0.0.4 types-dataclasses==0.6.6 types-Markdown==3.4.2.10 -types-Pillow==10.0.0.3 +types-Pillow==10.2.0.20240311 types-psycopg2==2.9.21.11 types-python-dateutil==2.8.19.14 types-requests==2.31.0.2 From 748418590fa4bbfd20615a2114c00e322ce8b581 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 20:22:14 +0100 Subject: [PATCH 23/37] docker-compose.yml: mount static_volume for flower Because flower also uses BookwyrmConfig, it wants to download fonts, and will download them to an incorrect location if the static_volume is not mounted. --- docker-compose.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/docker-compose.yml b/docker-compose.yml index 4d4037681..2cb9007da 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -91,6 +91,7 @@ services: env_file: .env volumes: - .:/app + - static_volume:/app/static networks: - main depends_on: From 3367b20965d44b5f2724250491e0b54c84fc3769 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 20:23:26 +0100 Subject: [PATCH 24/37] Font download: destination dir is allowed to exist Without this argument, an existing directory (but not the file) causes an error. --- bookwyrm/apps.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index b0c3e3fa4..5a9f45db5 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -1,4 +1,5 @@ """Do further startup configuration and initialization""" + import os import urllib import logging @@ -14,7 +15,7 @@ def download_file(url, destination): """Downloads a file to the given path""" try: # Ensure our destination directory exists - os.makedirs(os.path.dirname(destination)) + os.makedirs(os.path.dirname(destination), exist_ok=True) with urllib.request.urlopen(url) as stream: with open(destination, "b+w") as outfile: outfile.write(stream.read()) From 7690247ab43db55e309a357494802c3ec724c0a6 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 20:24:02 +0100 Subject: [PATCH 25/37] Font download: log the exact error --- bookwyrm/apps.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bookwyrm/apps.py b/bookwyrm/apps.py index 5a9f45db5..41b1a17a2 100644 --- a/bookwyrm/apps.py +++ b/bookwyrm/apps.py @@ -19,12 +19,12 @@ def download_file(url, 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.info("Failed to download file %s", url) - except OSError: - logger.info("Couldn't open font file %s for writing", destination) - except: # pylint: disable=bare-except - logger.info("Unknown error in file download") + except (urllib.error.HTTPError, urllib.error.URLError) as err: + logger.error("Failed to download file %s: %s", url, err) + except OSError as err: + logger.error("Couldn't open font file %s for writing: %s", destination, err) + except Exception as err: # pylint:disable=broad-except + logger.error("Unknown error in file download: %s", err) class BookwyrmConfig(AppConfig): From 864304f128fc897348342ba83506441469e8fe53 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 21:01:20 +0100 Subject: [PATCH 26/37] docker-compose.yml: make all bind mounts read only Except dev-tools, since it needs to be able to change the source. --- docker-compose.yml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 4d4037681..6b68d6826 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,7 +11,7 @@ services: networks: - main volumes: - - ./nginx:/etc/nginx/conf.d + - ./nginx:/etc/nginx/conf.d:ro - static_volume:/app/static - media_volume:/app/images db: @@ -26,7 +26,7 @@ services: env_file: .env command: python manage.py runserver 0.0.0.0:8000 volumes: - - .:/app + - .:/app:ro - static_volume:/app/static - media_volume:/app/images depends_on: @@ -41,7 +41,7 @@ services: image: redis:7.2.1 command: redis-server --requirepass ${REDIS_ACTIVITY_PASSWORD} --appendonly yes --port ${REDIS_ACTIVITY_PORT} volumes: - - ./redis.conf:/etc/redis/redis.conf + - ./redis.conf:/etc/redis/redis.conf:ro - redis_activity_data:/data env_file: .env networks: @@ -51,7 +51,7 @@ services: image: redis:7.2.1 command: redis-server --requirepass ${REDIS_BROKER_PASSWORD} --appendonly yes --port ${REDIS_BROKER_PORT} volumes: - - ./redis.conf:/etc/redis/redis.conf + - ./redis.conf:/etc/redis/redis.conf:ro - redis_broker_data:/data env_file: .env networks: @@ -63,9 +63,8 @@ services: networks: - main command: celery -A celerywyrm worker -l info -Q high_priority,medium_priority,low_priority,streams,images,suggested_users,email,connectors,lists,inbox,imports,import_triggered,broadcast,misc - volumes: - - .:/app + - .:/app:ro - static_volume:/app/static - media_volume:/app/images depends_on: @@ -79,7 +78,7 @@ services: - main command: celery -A celerywyrm beat -l INFO --scheduler django_celery_beat.schedulers:DatabaseScheduler volumes: - - .:/app + - .:/app:ro - static_volume:/app/static - media_volume:/app/images depends_on: @@ -90,7 +89,7 @@ services: command: celery -A celerywyrm flower --basic_auth=${FLOWER_USER}:${FLOWER_PASSWORD} --url_prefix=flower env_file: .env volumes: - - .:/app + - .:/app:ro networks: - main depends_on: @@ -102,7 +101,7 @@ services: env_file: .env volumes: - /app/dev-tools/ - - .:/app + - .:/app:rw volumes: pgdata: static_volume: From 68cb94daf2e81ed755a4b04f73034e4a3f77d1be Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 21:02:06 +0100 Subject: [PATCH 27/37] docker-compose.yml: don't automatically start dev-tools by assigning profile --- docker-compose.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 6b68d6826..9e2cd67ba 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -102,6 +102,8 @@ services: volumes: - /app/dev-tools/ - .:/app:rw + profiles: + - tools volumes: pgdata: static_volume: From 4d23edddca5692d7ef66f8b66b88e6f3bc061008 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 21:04:34 +0100 Subject: [PATCH 28/37] Make sure /images/ and /static/ exist now that the bind mount is read only Otherwise the static_volume and media_volume can't be mounted there. --- .gitignore | 1 + images/.gitkeep | 0 static/.gitkeep | 0 3 files changed, 1 insertion(+) create mode 100644 images/.gitkeep create mode 100644 static/.gitkeep diff --git a/.gitignore b/.gitignore index ec2a08f80..755375b34 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ # BookWyrm .env /images/ +/static/ bookwyrm/static/css/bookwyrm.css bookwyrm/static/css/themes/ !bookwyrm/static/css/themes/bookwyrm-*.scss diff --git a/images/.gitkeep b/images/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/static/.gitkeep b/static/.gitkeep new file mode 100644 index 000000000..e69de29bb From 47afe34d9767ad7895b7eb72bc4cd88f99eeb148 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Mar 2024 21:48:21 +0000 Subject: [PATCH 29/37] Bump django from 3.2.24 to 3.2.25 Bumps [django](https://github.com/django/django) from 3.2.24 to 3.2.25. - [Commits](https://github.com/django/django/compare/3.2.24...3.2.25) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c769916a2..3c0406acd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ boto3==1.26.57 bw-file-resubmit==0.6.0rc2 celery==5.3.1 colorthief==0.2.1 -Django==3.2.24 +Django==3.2.25 django-celery-beat==2.5.0 django-compressor==4.4 django-csp==3.7 From f423834bd0d8bd54afa29f56fd60459ac7209b67 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Mon, 18 Mar 2024 20:06:12 +0100 Subject: [PATCH 30/37] Catch the correct exception type from Pillow --- bookwyrm/preview_images.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bookwyrm/preview_images.py b/bookwyrm/preview_images.py index aba372abc..995f25bfd 100644 --- a/bookwyrm/preview_images.py +++ b/bookwyrm/preview_images.py @@ -1,4 +1,5 @@ """ Generate social media preview images for twitter/mastodon/etc """ + import math import os import textwrap @@ -42,8 +43,8 @@ def get_imagefont(name, size): 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) + except OSError as err: + logger.error("Could not load font %s from file: %s", name, err) return ImageFont.load_default() @@ -59,7 +60,7 @@ def get_font(weight, size=28): font.set_variation_by_name("Bold") if weight == "regular": font.set_variation_by_name("Regular") - except AttributeError: + except OSError: pass return font From 6a87713f9f4f1c6dd9ad169f967b1335692e203a Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 20 Mar 2024 11:45:12 +0100 Subject: [PATCH 31/37] Recalculate all book search vectors after fixing the author trigger --- .../0191_migrate_search_vec_triggers_to_pgtriggers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py index 5e798b654..03442298f 100644 --- a/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py +++ b/bookwyrm/migrations/0191_migrate_search_vec_triggers_to_pgtriggers.py @@ -67,4 +67,10 @@ class Migration(migrations.Migration): """, reverse_sql=author_search_vector_trigger.sql, ), + migrations.RunSQL( + # Recalculate book search vector for any missed author name changes + # due to bug in JOIN in the old trigger. + sql="UPDATE bookwyrm_book SET search_vector = NULL;", + reverse_sql=migrations.RunSQL.noop, + ), ] From e13e4237f49e1079eb0190898c4b45b53399bcf9 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 20 Mar 2024 12:24:54 +0100 Subject: [PATCH 32/37] black: specify required-version This ensures consistent formatting among different contributors / development setups. https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#required-version --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 pyproject.toml diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 000000000..292ca8c41 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,2 @@ +[tool.black] +required-version = "22" From ab430e020824f08ca15fe3e0da824d9c5865d5c4 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 20 Mar 2024 12:43:14 +0100 Subject: [PATCH 33/37] requirements.txt: add black This way, IDEs can be set up to use the black version from the environment instead of a globally available/bundled black version. --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 82b7bc8c8..6b3d838bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -41,6 +41,7 @@ setuptools>=65.5.1 # Not a direct dependency, pinned to get a security fix tornado==6.3.3 # Not a direct dependency, pinned to get a security fix # Dev +black==22.* celery-types==0.18.0 django-stubs[compatible-mypy]==4.2.4 mypy==1.5.1 From b5b9eddaf0a8336cf9766c901811022713fb78a6 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Wed, 20 Mar 2024 12:46:37 +0100 Subject: [PATCH 34/37] CI: relax black version constraints --- .github/workflows/python.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index dcbe05aee..01241b467 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -94,6 +94,6 @@ jobs: steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 - - uses: psf/black@22.12.0 + - uses: psf/black@stable with: - version: 22.12.0 + version: "22.*" From 682bb3b62fb6ed53daf374376c9bd91f30295809 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Thu, 21 Mar 2024 16:25:29 +0100 Subject: [PATCH 35/37] dev-tools: relax black version constraint --- dev-tools/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/requirements.txt b/dev-tools/requirements.txt index a69d319ab..3bb771f5a 100644 --- a/dev-tools/requirements.txt +++ b/dev-tools/requirements.txt @@ -1 +1 @@ -black==22.12.0 +black==22.* From c3d25c59c51431cf7d5a2700b842a07410dd8936 Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Thu, 21 Mar 2024 16:48:28 +0100 Subject: [PATCH 36/37] Escape search query in generated URLs Otherwise, a query containing '&' or other special characters results in a broken URL. --- bookwyrm/templates/search/book.html | 2 +- bookwyrm/templates/search/layout.html | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/templates/search/book.html b/bookwyrm/templates/search/book.html index 262dcf2f9..b93c96754 100644 --- a/bookwyrm/templates/search/book.html +++ b/bookwyrm/templates/search/book.html @@ -109,7 +109,7 @@

{% if request.user.is_authenticated %} {% if not remote %} - + {% trans "Load results from other catalogues" %} {% else %} diff --git a/bookwyrm/templates/search/layout.html b/bookwyrm/templates/search/layout.html index 725a4f43f..59ea0304e 100644 --- a/bookwyrm/templates/search/layout.html +++ b/bookwyrm/templates/search/layout.html @@ -41,18 +41,18 @@

From a914a44fba28defec6fefcbf90deee828176ddf5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 23 Mar 2024 07:54:54 -0700 Subject: [PATCH 37/37] Removes unnecessary redeclaration of wikidata model field in Author --- bookwyrm/models/author.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index b32d49d54..981e3c0cc 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -18,9 +18,6 @@ class Author(BookDataModel): wikipedia_link = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True ) - wikidata = fields.CharField( - max_length=255, blank=True, null=True, deduplication_field=True - ) isni = fields.CharField( max_length=255, blank=True, null=True, deduplication_field=True )