From e7f95ef4c2054385f58f84cc3f93e486ad4e387b Mon Sep 17 00:00:00 2001 From: Bart Schuurmans Date: Thu, 25 Apr 2024 15:53:53 +0200 Subject: [PATCH] Modify update_fields in save() when modifying objects https://docs.djangoproject.com/en/5.0/releases/4.2/#setting-update-fields-in-model-save-may-now-be-required --- bookwyrm/models/author.py | 6 +++--- bookwyrm/models/book.py | 38 ++++++++++++++++++++++++---------- bookwyrm/models/link.py | 14 +++++++++---- bookwyrm/models/list.py | 8 +++++-- bookwyrm/models/readthrough.py | 9 ++++++-- bookwyrm/models/shelf.py | 15 +++++++++++--- bookwyrm/models/site.py | 7 +++++-- bookwyrm/models/status.py | 10 +++++---- bookwyrm/models/user.py | 29 ++++++++++++++++++++------ bookwyrm/utils/db.py | 14 ++++++++++++- 10 files changed, 112 insertions(+), 38 deletions(-) diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 8ea1858fd..20c4e9e00 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -1,7 +1,7 @@ """ database schema for info about authors """ import re -from typing import Tuple, Any +from typing import Any from django.db import models from django.contrib.postgres.indexes import GinIndex @@ -45,9 +45,9 @@ class Author(BookDataModel): ) bio = fields.HtmlField(null=True, blank=True) - def save(self, *args: Tuple[Any, ...], **kwargs: dict[str, Any]) -> None: + def save(self, *args: Any, **kwargs: Any) -> None: """normalize isni format""" - if self.isni: + if self.isni is not None: self.isni = re.sub(r"\s", "", self.isni) super().save(*args, **kwargs) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 6fc447228..2e6377575 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -2,7 +2,7 @@ from itertools import chain import re -from typing import Any, Dict +from typing import Any, Dict, Optional, Iterable from typing_extensions import Self from django.contrib.postgres.search import SearchVectorField @@ -27,7 +27,7 @@ from bookwyrm.settings import ( ENABLE_PREVIEW_IMAGES, ENABLE_THUMBNAIL_GENERATION, ) -from bookwyrm.utils.db import format_trigger +from bookwyrm.utils.db import format_trigger, add_update_fields from .activitypub_mixin import OrderedCollectionPageMixin, ObjectMixin from .base_model import BookWyrmModel @@ -96,14 +96,19 @@ class BookDataModel(ObjectMixin, BookWyrmModel): abstract = True - def save(self, *args: Any, **kwargs: Any) -> None: + def save( + self, *args: Any, update_fields: Optional[Iterable[str]] = None, **kwargs: Any + ) -> None: """ensure that the remote_id is within this instance""" if self.id: self.remote_id = self.get_remote_id() + update_fields = add_update_fields(update_fields, "remote_id") else: self.origin_id = self.remote_id self.remote_id = None - super().save(*args, **kwargs) + update_fields = add_update_fields(update_fields, "origin_id", "remote_id") + + super().save(*args, update_fields=update_fields, **kwargs) # pylint: disable=arguments-differ def broadcast(self, activity, sender, software="bookwyrm", **kwargs): @@ -510,28 +515,39 @@ class Edition(Book): # max rank is 9 return rank - def save(self, *args: Any, **kwargs: Any) -> None: + def save( + self, *args: Any, update_fields: Optional[Iterable[str]] = None, **kwargs: Any + ) -> None: """set some fields on the edition object""" # calculate isbn 10/13 - if self.isbn_13 and self.isbn_13[:3] == "978" and not self.isbn_10: + if ( + self.isbn_10 is None + and self.isbn_13 is not None + and self.isbn_13[:3] == "978" + ): self.isbn_10 = isbn_13_to_10(self.isbn_13) - if self.isbn_10 and not self.isbn_13: + update_fields = add_update_fields(update_fields, "isbn_10") + if self.isbn_13 is None and self.isbn_10 is not None: self.isbn_13 = isbn_10_to_13(self.isbn_10) + update_fields = add_update_fields(update_fields, "isbn_13") # normalize isbn format - if self.isbn_10: + if self.isbn_10 is not None: self.isbn_10 = normalize_isbn(self.isbn_10) - if self.isbn_13: + if self.isbn_13 is not None: self.isbn_13 = normalize_isbn(self.isbn_13) # set rank - self.edition_rank = self.get_rank() + if (new := self.get_rank()) != self.edition_rank: + self.edition_rank = new + update_fields = add_update_fields(update_fields, "edition_rank") # Create sort title by removing articles from title if self.sort_title in [None, ""]: self.sort_title = self.guess_sort_title() + update_fields = add_update_fields(update_fields, "sort_title") - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) # clear author cache if self.id: diff --git a/bookwyrm/models/link.py b/bookwyrm/models/link.py index 67bf9c4d4..4519f0c81 100644 --- a/bookwyrm/models/link.py +++ b/bookwyrm/models/link.py @@ -1,4 +1,5 @@ """ outlink data """ +from typing import Optional, Iterable from urllib.parse import urlparse from django.core.exceptions import PermissionDenied @@ -6,6 +7,7 @@ from django.db import models from django.utils.translation import gettext_lazy as _ from bookwyrm import activitypub +from bookwyrm.utils.db import add_update_fields from .activitypub_mixin import ActivitypubMixin from .base_model import BookWyrmModel from . import fields @@ -34,17 +36,19 @@ class Link(ActivitypubMixin, BookWyrmModel): """link name via the associated domain""" return self.domain.name - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """create a link""" # get or create the associated domain if not self.domain: domain = urlparse(self.url).hostname self.domain, _ = LinkDomain.objects.get_or_create(domain=domain) + update_fields = add_update_fields(update_fields, "domain") # this is never broadcast, the owning model broadcasts an update if "broadcast" in kwargs: del kwargs["broadcast"] - return super().save(*args, **kwargs) + + super().save(*args, update_fields=update_fields, **kwargs) AvailabilityChoices = [ @@ -88,8 +92,10 @@ class LinkDomain(BookWyrmModel): return raise PermissionDenied() - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """set a default name""" if not self.name: self.name = self.domain - super().save(*args, **kwargs) + update_fields = add_update_fields(update_fields, "name") + + super().save(*args, update_fields=update_fields, **kwargs) diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py index d32a8da95..df7e8162c 100644 --- a/bookwyrm/models/list.py +++ b/bookwyrm/models/list.py @@ -1,4 +1,5 @@ """ make a list of books!! """ +from typing import Optional, Iterable import uuid from django.core.exceptions import PermissionDenied @@ -8,6 +9,7 @@ from django.utils import timezone from bookwyrm import activitypub from bookwyrm.settings import BASE_URL +from bookwyrm.utils.db import add_update_fields from .activitypub_mixin import CollectionItemMixin, OrderedCollectionMixin from .base_model import BookWyrmModel @@ -124,11 +126,13 @@ class List(OrderedCollectionMixin, BookWyrmModel): group=None, curation="closed" ) - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """on save, update embed_key and avoid clash with existing code""" if not self.embed_key: self.embed_key = uuid.uuid4() - super().save(*args, **kwargs) + update_fields = add_update_fields(update_fields, "embed_key") + + super().save(*args, update_fields=update_fields, **kwargs) class ListItem(CollectionItemMixin, BookWyrmModel): diff --git a/bookwyrm/models/readthrough.py b/bookwyrm/models/readthrough.py index 910b2a7a9..7700b4a87 100644 --- a/bookwyrm/models/readthrough.py +++ b/bookwyrm/models/readthrough.py @@ -1,9 +1,13 @@ """ progress in a book """ +from typing import Optional, Iterable + from django.core import validators from django.core.cache import cache from django.db import models from django.db.models import F, Q +from bookwyrm.utils.db import add_update_fields + from .base_model import BookWyrmModel @@ -30,13 +34,14 @@ class ReadThrough(BookWyrmModel): stopped_date = models.DateTimeField(blank=True, null=True) is_active = models.BooleanField(default=True) - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """update user active time""" # an active readthrough must have an unset finish date if self.finish_date or self.stopped_date: self.is_active = False + update_fields = add_update_fields(update_fields, "is_active") - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) cache.delete(f"latest_read_through-{self.user_id}-{self.book_id}") self.user.update_active_date() diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 77c2d26d9..0b9ef2b09 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -1,5 +1,6 @@ """ puttin' books on shelves """ import re +from typing import Optional, Iterable from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.db import models @@ -8,6 +9,7 @@ from django.utils import timezone from bookwyrm import activitypub from bookwyrm.settings import BASE_URL from bookwyrm.tasks import BROADCAST +from bookwyrm.utils.db import add_update_fields from .activitypub_mixin import CollectionItemMixin, OrderedCollectionMixin from .base_model import BookWyrmModel from . import fields @@ -46,7 +48,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel): if not self.identifier: # this needs the auto increment ID from the save() above self.identifier = self.get_identifier() - super().save(*args, **kwargs, broadcast=False) + super().save(*args, **kwargs, broadcast=False, update_fields={"identifier"}) def get_identifier(self): """custom-shelf-123 for the url""" @@ -101,12 +103,19 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel): activity_serializer = activitypub.ShelfItem collection_field = "shelf" - def save(self, *args, priority=BROADCAST, **kwargs): + def save( + self, + *args, + priority=BROADCAST, + update_fields: Optional[Iterable[str]] = None, + **kwargs, + ): if not self.user: self.user = self.shelf.user + update_fields = add_update_fields(update_fields, "user") is_update = self.id is not None - super().save(*args, priority=priority, **kwargs) + super().save(*args, priority=priority, update_fields=update_fields, **kwargs) if is_update and self.user.local: # remove all caches related to all editions of this book diff --git a/bookwyrm/models/site.py b/bookwyrm/models/site.py index 89d6ef395..6c2a73422 100644 --- a/bookwyrm/models/site.py +++ b/bookwyrm/models/site.py @@ -1,5 +1,6 @@ """ the particulars for this instance of BookWyrm """ import datetime +from typing import Optional, Iterable from urllib.parse import urljoin import uuid @@ -15,6 +16,7 @@ from bookwyrm.preview_images import generate_site_preview_image_task from bookwyrm.settings import BASE_URL, ENABLE_PREVIEW_IMAGES, STATIC_FULL_URL from bookwyrm.settings import RELEASE_API from bookwyrm.tasks import app, MISC +from bookwyrm.utils.db import add_update_fields from .base_model import BookWyrmModel, new_access_code from .user import User from .fields import get_absolute_url @@ -136,13 +138,14 @@ class SiteSettings(SiteModel): return get_absolute_url(uploaded) return urljoin(STATIC_FULL_URL, default_path) - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """if require_confirm_email is disabled, make sure no users are pending, if enabled, make sure invite_question_text is not empty""" if not self.invite_question_text: self.invite_question_text = "What is your favourite book?" + update_fields = add_update_fields(update_fields, "invite_question_text") - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) if not self.require_confirm_email: User.objects.filter(is_active=False, deactivation_reason="pending").update( diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 5b953d077..9dc60e647 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -1,6 +1,6 @@ """ models for storing different kinds of Activities """ from dataclasses import MISSING -from typing import Optional +from typing import Optional, Iterable import re from django.apps import apps @@ -20,6 +20,7 @@ from model_utils.managers import InheritanceManager from bookwyrm import activitypub from bookwyrm.preview_images import generate_edition_preview_image_task from bookwyrm.settings import ENABLE_PREVIEW_IMAGES +from bookwyrm.utils.db import add_update_fields from .activitypub_mixin import ActivitypubMixin, ActivityMixin from .activitypub_mixin import OrderedCollectionPageMixin from .base_model import BookWyrmModel @@ -85,12 +86,13 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): models.Index(fields=["thread_id"]), ] - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """save and notify""" - if self.reply_parent: + if self.thread_id is None and self.reply_parent: self.thread_id = self.reply_parent.thread_id or self.reply_parent_id + update_fields = add_update_fields(update_fields, "thread_id") - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) if not self.reply_parent: self.thread_id = self.id diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 73f1b28c6..d5120deac 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -2,6 +2,7 @@ import datetime import re import zoneinfo +from typing import Optional, Iterable from urllib.parse import urlparse from uuid import uuid4 @@ -24,6 +25,7 @@ from bookwyrm.settings import BASE_URL, ENABLE_PREVIEW_IMAGES, LANGUAGES from bookwyrm.signatures import create_key_pair from bookwyrm.tasks import app, MISC from bookwyrm.utils import regex +from bookwyrm.utils.db import add_update_fields from .activitypub_mixin import OrderedCollectionPageMixin, ActivitypubMixin from .base_model import BookWyrmModel, DeactivationReason, new_access_code from .federated_server import FederatedServer @@ -338,13 +340,14 @@ class User(OrderedCollectionPageMixin, AbstractUser): ] return activity_object - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """populate fields for new local users""" created = not bool(self.id) if not self.local and not re.match(regex.FULL_USERNAME, self.username): # generate a username that uses the domain (webfinger format) actor_parts = urlparse(self.remote_id) self.username = f"{self.username}@{actor_parts.hostname}" + update_fields = add_update_fields(update_fields, "username") # this user already exists, no need to populate fields if not created: @@ -353,12 +356,12 @@ class User(OrderedCollectionPageMixin, AbstractUser): elif not self.deactivation_date: self.deactivation_date = timezone.now() - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) return # this is a new remote user, we need to set their remote server field if not self.local: - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) transaction.on_commit(lambda: set_remote_server(self.id)) return @@ -370,8 +373,17 @@ class User(OrderedCollectionPageMixin, AbstractUser): self.shared_inbox = f"{BASE_URL}/inbox" self.outbox = f"{self.remote_id}/outbox" + update_fields = add_update_fields( + update_fields, + "remote_id", + "followers_url", + "inbox", + "shared_inbox", + "outbox", + ) + # an id needs to be set before we can proceed with related models - super().save(*args, **kwargs) + super().save(*args, update_fields=update_fields, **kwargs) # make users editors by default try: @@ -522,14 +534,19 @@ class KeyPair(ActivitypubMixin, BookWyrmModel): # self.owner is set by the OneToOneField on User return f"{self.owner.remote_id}/#main-key" - def save(self, *args, **kwargs): + def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs): """create a key pair""" # no broadcasting happening here if "broadcast" in kwargs: del kwargs["broadcast"] + if not self.public_key: self.private_key, self.public_key = create_key_pair() - return super().save(*args, **kwargs) + update_fields = add_update_fields( + update_fields, "private_key", "public_key" + ) + + super().save(*args, update_fields=update_fields, **kwargs) @app.task(queue=MISC) diff --git a/bookwyrm/utils/db.py b/bookwyrm/utils/db.py index 2bb3b9ff6..fd2601deb 100644 --- a/bookwyrm/utils/db.py +++ b/bookwyrm/utils/db.py @@ -1,6 +1,6 @@ """ Database utilities """ -from typing import cast +from typing import Optional, Iterable, Set, cast import sqlparse # type: ignore @@ -21,3 +21,15 @@ def format_trigger(sql: str) -> str: identifier_case="lower", ), ) + + +def add_update_fields( + update_fields: Optional[Iterable[str]], *fields: str +) -> Optional[Set[str]]: + """ + Helper for adding fields to the update_fields kwarg when modifying an object + in a model's save() method. + + https://docs.djangoproject.com/en/5.0/releases/4.2/#setting-update-fields-in-model-save-may-now-be-required + """ + return set(fields).union(update_fields) if update_fields is not None else None