Reorder operations in save() overrides

Accessing many-to-many relations before saving is no longer allowed.

Reorder all operations consistently:
1. Validations
2. Modify own fields
3. Perform save by calling super().save()
4. Modify related objects and clear caches

Especially clearing caches should be done after actually saving, otherwise the old data can be
re-added immediately by another request before the new data is written.
This commit is contained in:
Bart Schuurmans 2024-03-29 21:12:59 +01:00
parent 47fdad9c87
commit 0d621b68e0
7 changed files with 32 additions and 20 deletions

View file

@ -50,7 +50,7 @@ class Author(BookDataModel):
if self.isni: if self.isni:
self.isni = re.sub(r"\s", "", self.isni) self.isni = re.sub(r"\s", "", self.isni)
return super().save(*args, **kwargs) super().save(*args, **kwargs)
@property @property
def isni_link(self): def isni_link(self):

View file

@ -103,7 +103,7 @@ class BookDataModel(ObjectMixin, BookWyrmModel):
else: else:
self.origin_id = self.remote_id self.origin_id = self.remote_id
self.remote_id = None self.remote_id = None
return super().save(*args, **kwargs) super().save(*args, **kwargs)
# pylint: disable=arguments-differ # pylint: disable=arguments-differ
def broadcast(self, activity, sender, software="bookwyrm", **kwargs): def broadcast(self, activity, sender, software="bookwyrm", **kwargs):
@ -323,7 +323,7 @@ class Book(BookDataModel):
if not isinstance(self, (Edition, Work)): if not isinstance(self, (Edition, Work)):
raise ValueError("Books should be added as Editions or Works") raise ValueError("Books should be added as Editions or Works")
return super().save(*args, **kwargs) super().save(*args, **kwargs)
def get_remote_id(self): def get_remote_id(self):
"""editions and works both use "book" instead of model_name""" """editions and works both use "book" instead of model_name"""
@ -400,10 +400,11 @@ class Work(OrderedCollectionPageMixin, Book):
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
"""set some fields on the edition object""" """set some fields on the edition object"""
super().save(*args, **kwargs)
# set rank # set rank
for edition in self.editions.all(): for edition in self.editions.all():
edition.save() edition.save()
return super().save(*args, **kwargs)
@property @property
def default_edition(self): def default_edition(self):
@ -526,16 +527,16 @@ class Edition(Book):
# set rank # set rank
self.edition_rank = self.get_rank() self.edition_rank = self.get_rank()
# clear author cache
if self.id:
for author_id in self.authors.values_list("id", flat=True):
cache.delete(f"author-books-{author_id}")
# Create sort title by removing articles from title # Create sort title by removing articles from title
if self.sort_title in [None, ""]: if self.sort_title in [None, ""]:
self.sort_title = self.guess_sort_title() self.sort_title = self.guess_sort_title()
return super().save(*args, **kwargs) super().save(*args, **kwargs)
# clear author cache
if self.id:
for author_id in self.authors.values_list("id", flat=True):
cache.delete(f"author-books-{author_id}")
@transaction.atomic @transaction.atomic
def repair(self): def repair(self):

View file

@ -32,13 +32,15 @@ class ReadThrough(BookWyrmModel):
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
"""update user active time""" """update user active time"""
cache.delete(f"latest_read_through-{self.user_id}-{self.book_id}")
self.user.update_active_date()
# an active readthrough must have an unset finish date # an active readthrough must have an unset finish date
if self.finish_date or self.stopped_date: if self.finish_date or self.stopped_date:
self.is_active = False self.is_active = False
super().save(*args, **kwargs) super().save(*args, **kwargs)
cache.delete(f"latest_read_through-{self.user_id}-{self.book_id}")
self.user.update_active_date()
def create_update(self): def create_update(self):
"""add update to the readthrough""" """add update to the readthrough"""
if self.progress: if self.progress:

View file

@ -38,14 +38,16 @@ class UserRelationship(BookWyrmModel):
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
"""clear the template cache""" """clear the template cache"""
clear_cache(self.user_subject, self.user_object)
super().save(*args, **kwargs) super().save(*args, **kwargs)
clear_cache(self.user_subject, self.user_object)
def delete(self, *args, **kwargs): def delete(self, *args, **kwargs):
"""clear the template cache""" """clear the template cache"""
clear_cache(self.user_subject, self.user_object)
super().delete(*args, **kwargs) super().delete(*args, **kwargs)
clear_cache(self.user_subject, self.user_object)
class Meta: class Meta:
"""relationships should be unique""" """relationships should be unique"""

View file

@ -44,6 +44,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
"""set the identifier""" """set the identifier"""
super().save(*args, priority=priority, **kwargs) super().save(*args, priority=priority, **kwargs)
if not self.identifier: if not self.identifier:
# this needs the auto increment ID from the save() above
self.identifier = self.get_identifier() self.identifier = self.get_identifier()
super().save(*args, **kwargs, broadcast=False) super().save(*args, **kwargs, broadcast=False)
@ -103,7 +104,11 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
def save(self, *args, priority=BROADCAST, **kwargs): def save(self, *args, priority=BROADCAST, **kwargs):
if not self.user: if not self.user:
self.user = self.shelf.user self.user = self.shelf.user
if self.id and self.user.local:
is_update = self.id is not None
super().save(*args, priority=priority, **kwargs)
if is_update and self.user.local:
# remove all caches related to all editions of this book # remove all caches related to all editions of this book
cache.delete_many( cache.delete_many(
[ [
@ -111,7 +116,6 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
for book in self.book.parent_work.editions.all() for book in self.book.parent_work.editions.all()
] ]
) )
super().save(*args, priority=priority, **kwargs)
def delete(self, *args, **kwargs): def delete(self, *args, **kwargs):
if self.id and self.user.local: if self.id and self.user.local:

View file

@ -139,13 +139,15 @@ class SiteSettings(SiteModel):
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
"""if require_confirm_email is disabled, make sure no users are pending, """if require_confirm_email is disabled, make sure no users are pending,
if enabled, make sure invite_question_text is not empty""" 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?"
super().save(*args, **kwargs)
if not self.require_confirm_email: if not self.require_confirm_email:
User.objects.filter(is_active=False, deactivation_reason="pending").update( User.objects.filter(is_active=False, deactivation_reason="pending").update(
is_active=True, deactivation_reason=None is_active=True, deactivation_reason=None
) )
if not self.invite_question_text:
self.invite_question_text = "What is your favourite book?"
super().save(*args, **kwargs)
class Theme(SiteModel): class Theme(SiteModel):

View file

@ -459,9 +459,10 @@ class Review(BookStatus):
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
"""clear rating caches""" """clear rating caches"""
super().save(*args, **kwargs)
if self.book.parent_work: if self.book.parent_work:
cache.delete(f"book-rating-{self.book.parent_work.id}") cache.delete(f"book-rating-{self.book.parent_work.id}")
super().save(*args, **kwargs)
class ReviewRating(Review): class ReviewRating(Review):