diff --git a/api/README.md b/api/README.md index 7eaea37c..dc88ad92 100644 --- a/api/README.md +++ b/api/README.md @@ -116,6 +116,20 @@ Most resources follow a standard pattern: - **Create:** `POST /api/{resource}/` - Create new item (requires editor or admin) - **Update:** `PUT/PATCH /api/{resource}/{id}/` - Update item (requires editor or admin) +### Attribution + +Detail responses for arcs, characters, creators, imprints, issues, publishers, series, teams, and +universes include a read-only `attribution` list naming where descriptive content (such as `desc`) +was adapted from. If you reuse that content, credit these sources, since some licenses require it +(Wikipedia's CC BY-SA, for example). List responses don't include it. + +```json +"attribution": [ + {"source": "Wikipedia", "url": "https://en.wikipedia.org/wiki/Spider-Man"}, + {"source": "Marvel", "url": "https://www.marvel.com/characters/spider-man-peter-parker"} +] +``` + ### Nested Resources Some resources include nested actions: @@ -175,6 +189,7 @@ Story arcs that span multiple issues, often across different series. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -232,6 +247,7 @@ Comic book characters appearing in issues. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -286,6 +302,7 @@ Writers, artists, and other comic book creators. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -426,6 +443,7 @@ The Issue endpoint supports the most comprehensive filtering options: - `cover_hash` - Perceptual image hash - `foc_date` - Final order cutoff date - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Cover Price (Write):** @@ -535,6 +553,7 @@ Comic book publishers. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Country (Write):** @@ -599,6 +618,7 @@ Publisher imprints (e.g., Vertigo, MAX). - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -708,6 +728,7 @@ Comic book series. - `genres` - Array of genre objects - `associated` - Associated series - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Series Status Choices:** @@ -813,6 +834,7 @@ Superhero teams and groups. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -864,6 +886,7 @@ Comic book universes (e.g., Earth-616, Earth-1). - `desc` - Description - `image` - Universe image URL - `resource_url` - Link to web UI + - `attribution` - Sources the descriptive content was adapted from, as a list of `{"source", "url"}` objects (empty if none). `source` is one of `Wikipedia`, `Marvel`, `DC`, or `Grand Comics Database` **Example:** @@ -2548,6 +2571,11 @@ For questions, issues, or feature requests: ## Changelog +### Version 1.12 + +- Added read-only `attribution` field to the Arc, Character, Creator, Imprint, Issue, Publisher, Series, Team, and Universe detail endpoints: a list of `{"source", "url"}` objects naming where descriptive content was adapted from +- Added `cover_date_range_after` and `cover_date_range_before` filters to the Issue endpoint + ### Version 1.11 - Added `publisher` (object with `id`/`name`) and `series_type` (object with `id`/`name`) fields to the Series list endpoint diff --git a/api/cache.py b/api/cache.py index 9410b9d6..a4fabea3 100644 --- a/api/cache.py +++ b/api/cache.py @@ -23,6 +23,10 @@ DETAIL_CACHE_TTL = 60 * 60 * 24 * 3 # 3 days; live keys self-invalidate on write. LIST_CACHE_TTL = 60 * 2 # 2min; bounds staleness from nested-object changes we don't chase. +# Bump when a detail response's shape changes (e.g. a field is added), so entries cached in the +# old shape aren't served for up to DETAIL_CACHE_TTL after deploy. List keys don't need this: +# LIST_CACHE_TTL is short enough to ride out. +DETAIL_CACHE_SCHEMA_VERSION = 2 _VERSION_KEY_PREFIX = "cachever" @@ -112,7 +116,12 @@ def detail_cache_key( `request` supplies origin and query params -- see `_request_digest()`. """ - key = f"api:detail:{model_label}:{action}:{pk}:{modified.timestamp()}" + # The schema version goes after the model label, which audit_response_cache + # reads as the key's category. + key = ( + f"api:detail:{model_label}:v{DETAIL_CACHE_SCHEMA_VERSION}:{action}:{pk}:" + f"{modified.timestamp()}" + ) if dependent_labels: version_map = get_model_versions(dependent_labels) versions = "-".join(str(version_map[lbl]) for lbl in dependent_labels) diff --git a/api/v1_0/serializers/__init__.py b/api/v1_0/serializers/__init__.py index f2554366..3b275b60 100644 --- a/api/v1_0/serializers/__init__.py +++ b/api/v1_0/serializers/__init__.py @@ -1,9 +1,11 @@ from api.v1_0.serializers.publisher import ( # noqa: I001 BasicPublisherSerializer, PublisherListSerializer, + PublisherReadSerializer, PublisherSerializer, ) -from api.v1_0.serializers.arc import ArcListSerializer, ArcSerializer +from api.v1_0.serializers.attribution import AttributionSerializer +from api.v1_0.serializers.arc import ArcListSerializer, ArcReadSerializer, ArcSerializer from api.v1_0.serializers.character import ( CharacterListSerializer, CharacterReadSerializer, @@ -11,6 +13,7 @@ ) from api.v1_0.serializers.creator import ( CreatorListSerializer, + CreatorReadSerializer, CreatorSerializer, CreditReadSerializer, CreditSerializer, @@ -65,8 +68,10 @@ __all__ = [ "ArcListSerializer", + "ArcReadSerializer", "ArcSerializer", "AssociatedSeriesSerializer", + "AttributionSerializer", "BasicImprintSerializer", "BasicPublisherSerializer", "CharacterListSerializer", @@ -77,6 +82,7 @@ "CollectionRatingUpdateSerializer", "CollectionReadSerializer", "CreatorListSerializer", + "CreatorReadSerializer", "CreatorSerializer", "CreditReadSerializer", "CreditSerializer", @@ -92,6 +98,7 @@ "MissingIssueSerializer", "MissingSeriesSerializer", "PublisherListSerializer", + "PublisherReadSerializer", "PublisherSerializer", "RatingSerializer", "ReadingListItemSerializer", diff --git a/api/v1_0/serializers/arc.py b/api/v1_0/serializers/arc.py index 16b80eaa..5035d938 100644 --- a/api/v1_0/serializers/arc.py +++ b/api/v1_0/serializers/arc.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from comicsdb.models import Arc @@ -27,3 +28,10 @@ def update(self, instance: Arc, validated_data): class Meta: model = Arc fields = ("id", "name", "desc", "image", "cv_id", "gcd_id", "resource_url", "modified") + + +class ArcReadSerializer(ArcSerializer): + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(ArcSerializer.Meta): + fields = (*ArcSerializer.Meta.fields, "attribution") diff --git a/api/v1_0/serializers/attribution.py b/api/v1_0/serializers/attribution.py new file mode 100644 index 00000000..caf77b73 --- /dev/null +++ b/api/v1_0/serializers/attribution.py @@ -0,0 +1,28 @@ +from drf_spectacular.utils import extend_schema_field +from rest_framework import serializers + +from comicsdb.models import Attribution + +# Fixed, untranslated names so API output doesn't vary with the request's language (and can be +# cached per object), unlike the model's translatable choice labels. +SOURCE_NAMES = { + Attribution.Source.MARVEL: "Marvel", + Attribution.Source.WIKIPEDIA: "Wikipedia", + Attribution.Source.GCD: "Grand Comics Database", + Attribution.Source.DC: "DC", +} +# Referenced by SPECTACULAR_SETTINGS["ENUM_NAME_OVERRIDES"]. +SOURCE_CHOICES = list(SOURCE_NAMES.values()) + + +class AttributionSerializer(serializers.ModelSerializer): + source = serializers.SerializerMethodField() + + class Meta: + model = Attribution + fields = ("source", "url") + + @extend_schema_field(serializers.ChoiceField(choices=SOURCE_CHOICES)) + def get_source(self, obj: Attribution) -> str: + # Fall back to the stored code rather than 500 on a value outside the choices. + return SOURCE_NAMES.get(obj.source, obj.source) diff --git a/api/v1_0/serializers/character.py b/api/v1_0/serializers/character.py index ad18cc8d..dfa25f88 100644 --- a/api/v1_0/serializers/character.py +++ b/api/v1_0/serializers/character.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from api.v1_0.serializers.creator import CreatorListSerializer from api.v1_0.serializers.team import TeamListSerializer from api.v1_0.serializers.universe import UniverseListSerializer @@ -77,3 +78,7 @@ class CharacterReadSerializer(CharacterSerializer): creators = CreatorListSerializer(many=True, read_only=True) teams = TeamListSerializer(many=True, read_only=True) universes = UniverseListSerializer(many=True, read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(CharacterSerializer.Meta): + fields = (*CharacterSerializer.Meta.fields, "attribution") diff --git a/api/v1_0/serializers/creator.py b/api/v1_0/serializers/creator.py index 1fe560f3..10a04daf 100644 --- a/api/v1_0/serializers/creator.py +++ b/api/v1_0/serializers/creator.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from comicsdb.models import Creator, Credits, Role @@ -41,6 +42,13 @@ class Meta: ) +class CreatorReadSerializer(CreatorSerializer): + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(CreatorSerializer.Meta): + fields = (*CreatorSerializer.Meta.fields, "attribution") + + class RoleSerializer(serializers.ModelSerializer): class Meta: model = Role diff --git a/api/v1_0/serializers/imprint.py b/api/v1_0/serializers/imprint.py index f58eb00b..c02db2a6 100644 --- a/api/v1_0/serializers/imprint.py +++ b/api/v1_0/serializers/imprint.py @@ -1,6 +1,7 @@ from rest_framework import serializers from api.v1_0.serializers import BasicPublisherSerializer +from api.v1_0.serializers.attribution import AttributionSerializer from comicsdb.models import Imprint @@ -40,6 +41,10 @@ class Meta: class ImprintReadSerializer(ImprintSerializer): publisher = BasicPublisherSerializer(read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(ImprintSerializer.Meta): + fields = (*ImprintSerializer.Meta.fields, "attribution") class BasicImprintSerializer(serializers.ModelSerializer): diff --git a/api/v1_0/serializers/issue.py b/api/v1_0/serializers/issue.py index 1eb57573..b33bd187 100644 --- a/api/v1_0/serializers/issue.py +++ b/api/v1_0/serializers/issue.py @@ -8,6 +8,7 @@ from api.v1_0.serializers import CreditReadSerializer from api.v1_0.serializers.arc import ArcListSerializer +from api.v1_0.serializers.attribution import AttributionSerializer from api.v1_0.serializers.character import CharacterListSerializer from api.v1_0.serializers.genre import GenreSerializer from api.v1_0.serializers.imprint import BasicImprintSerializer @@ -331,6 +332,7 @@ class IssueReadSerializer(serializers.ModelSerializer): series = IssueSeriesSerializer(read_only=True) reprints = ReprintSerializer(many=True, read_only=True) rating = RatingSerializer(read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) average_rating = serializers.DecimalField( max_digits=3, decimal_places=2, coerce_to_string=False, read_only=True ) @@ -384,4 +386,5 @@ class Meta: "gcd_id", "resource_url", "modified", + "attribution", ) diff --git a/api/v1_0/serializers/publisher.py b/api/v1_0/serializers/publisher.py index 67f4c41d..81c174a5 100644 --- a/api/v1_0/serializers/publisher.py +++ b/api/v1_0/serializers/publisher.py @@ -1,6 +1,7 @@ from django_countries.serializers import CountryFieldMixin from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from comicsdb.models import Publisher @@ -42,6 +43,13 @@ class Meta: ) +class PublisherReadSerializer(PublisherSerializer): + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(PublisherSerializer.Meta): + fields = (*PublisherSerializer.Meta.fields, "attribution") + + class BasicPublisherSerializer(serializers.ModelSerializer): class Meta: model = Publisher diff --git a/api/v1_0/serializers/series.py b/api/v1_0/serializers/series.py index 3fe34e78..ffa028a0 100644 --- a/api/v1_0/serializers/series.py +++ b/api/v1_0/serializers/series.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from api.v1_0.serializers.genre import GenreSerializer from api.v1_0.serializers.imprint import BasicImprintSerializer from api.v1_0.serializers.publisher import BasicPublisherSerializer @@ -116,3 +117,7 @@ class SeriesReadSerializer(SeriesSerializer): issue_count = serializers.IntegerField(source="num_issues", read_only=True) associated = AssociatedSeriesSerializer(many=True, read_only=True) genres = GenreSerializer(many=True, read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(SeriesSerializer.Meta): + fields = (*SeriesSerializer.Meta.fields, "attribution") diff --git a/api/v1_0/serializers/team.py b/api/v1_0/serializers/team.py index 318469dc..71bd79ad 100644 --- a/api/v1_0/serializers/team.py +++ b/api/v1_0/serializers/team.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from api.v1_0.serializers.attribution import AttributionSerializer from api.v1_0.serializers.creator import CreatorListSerializer from api.v1_0.serializers.universe import UniverseListSerializer from comicsdb.models import Team @@ -67,3 +68,7 @@ class Meta: class TeamReadSerializer(TeamSerializer): creators = CreatorListSerializer(many=True, read_only=True) universes = UniverseListSerializer(many=True, read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(TeamSerializer.Meta): + fields = (*TeamSerializer.Meta.fields, "attribution") diff --git a/api/v1_0/serializers/universe.py b/api/v1_0/serializers/universe.py index 6343bcd6..50ad25ef 100644 --- a/api/v1_0/serializers/universe.py +++ b/api/v1_0/serializers/universe.py @@ -1,6 +1,7 @@ from rest_framework import serializers from api.v1_0.serializers import BasicPublisherSerializer +from api.v1_0.serializers.attribution import AttributionSerializer from comicsdb.models import Universe @@ -42,3 +43,7 @@ class Meta: class UniverseReadSerializer(UniverseSerializer): publisher = BasicPublisherSerializer(read_only=True) + attribution = AttributionSerializer(many=True, read_only=True) + + class Meta(UniverseSerializer.Meta): + fields = (*UniverseSerializer.Meta.fields, "attribution") diff --git a/api/views.py b/api/views.py index bd2b5065..a5a42a24 100644 --- a/api/views.py +++ b/api/views.py @@ -35,6 +35,7 @@ ) from api.v1_0.serializers import ( ArcListSerializer, + ArcReadSerializer, ArcSerializer, CharacterListSerializer, CharacterReadSerializer, @@ -44,6 +45,7 @@ CollectionRatingUpdateSerializer, CollectionReadSerializer, CreatorListSerializer, + CreatorReadSerializer, CreatorSerializer, CreditSerializer, ImprintListSerializer, @@ -55,6 +57,7 @@ MissingIssueSerializer, MissingSeriesSerializer, PublisherListSerializer, + PublisherReadSerializer, PublisherSerializer, ReadingListItemSerializer, ReadingListListSerializer, @@ -410,6 +413,8 @@ def get_serializer_class(self): return ArcListSerializer case "issue_list": return IssueListSerializer + case "retrieve": + return ArcReadSerializer case _: return ArcSerializer @@ -501,6 +506,8 @@ def get_serializer_class(self): match self.action: case "list": return CreatorListSerializer + case "retrieve": + return CreatorReadSerializer case _: return CreatorSerializer @@ -722,6 +729,8 @@ def get_serializer_class(self): return PublisherListSerializer case "series_list": return SeriesListSerializer + case "retrieve": + return PublisherReadSerializer case _: return PublisherSerializer diff --git a/comicsdb/apps.py b/comicsdb/apps.py index 412bdab7..f5f4fd4e 100644 --- a/comicsdb/apps.py +++ b/comicsdb/apps.py @@ -20,6 +20,8 @@ update_issue_modified_on_reprint_change, update_issue_modified_on_universe_change, update_issue_modified_on_variant_change, + update_parent_modified_on_attribution_delete, + update_parent_modified_on_attribution_save, update_series_modified_on_associated_change, update_series_modified_on_genre_change, update_series_modified_on_issue_delete, @@ -142,6 +144,18 @@ def ready(self): dispatch_uid="post_delete_variant_issue_modified", ) + attribution = self.get_model("Attribution") + post_save.connect( + update_parent_modified_on_attribution_save, + sender=attribution, + dispatch_uid="post_save_attribution_parent_modified", + ) + post_delete.connect( + update_parent_modified_on_attribution_delete, + sender=attribution, + dispatch_uid="post_delete_attribution_parent_modified", + ) + credits_ = self.get_model("Credits") pre_delete.connect(pre_delete_credit, sender=credits_, dispatch_uid="pre_delete_credits") post_save.connect( diff --git a/comicsdb/management/commands/move_pub_to_imprint.py b/comicsdb/management/commands/move_pub_to_imprint.py deleted file mode 100644 index cacddaec..00000000 --- a/comicsdb/management/commands/move_pub_to_imprint.py +++ /dev/null @@ -1,131 +0,0 @@ -from typing import TYPE_CHECKING - -from django.contrib.contenttypes.models import ContentType -from django.core.management import BaseCommand - -if TYPE_CHECKING: - from django.db.models import QuerySet - -from comicsdb.models import Attribution, Imprint, Publisher, Series - - -class Command(BaseCommand): - help = "Moves publisher to imprint" - - def _delete_original(self, orig_pub: Publisher) -> None: - """ - Deletes the original publisher. - - Args: - orig_pub: The original Publisher object to be deleted. - - Returns: - None - """ - name = orig_pub.name - num, _ = orig_pub.delete() - if num > 0: - self.stdout.write(self.style.SUCCESS(f"Deleted Publisher: '{name}'")) - else: - self.stdout.write(self.style.ERROR(f"Failed to Delete Publisher: '{name}'")) - - def _update_attribution(self, orig_pub: Publisher, imprint: Imprint) -> None: - """ - Updates the attribution of the original publisher to the new imprint. - - Args: - orig_pub: The original Publisher object whose attribution needs to be updated. - imprint: The new Imprint object to which the attribution is updated. - - Returns: - None - """ - imprint_type = ContentType.objects.get_for_model(Imprint) - orig_attributions: QuerySet[Attribution] = orig_pub.attribution.all() - for attribution in orig_attributions: - attribution.content_type = imprint_type - attribution.object_id = imprint.pk - - res = Attribution.objects.bulk_update(orig_attributions, ["content_type", "object_id"]) - self.stdout.write( - self.style.SUCCESS(f"Moved {res} attribution records from the publisher to imprint") - ) - - def _create_imprint(self, original: Publisher, parent: Publisher) -> Imprint: - """ - Creates a new Imprint based on the information from the original Publisher. - - Args: - original: The original Publisher object from which the Imprint is created. - parent: The parent Publisher object for the new Imprint. - - Returns: - The newly created Imprint object. - """ - # TODO: Add image from original publisher - imprint, create = Imprint.objects.get_or_create( - name=original.name, - slug=original.slug, - desc=original.desc, - cv_id=original.cv_id, - publisher=parent, - founded=original.founded, - ) - if create: - self.stdout.write(self.style.SUCCESS(f"Added Imprint: {imprint}")) - else: - self.stdout.write(self.style.WARNING(f"Retrieved Existing Imprint: {imprint}")) - return imprint - - def _update_series(self, imprint: Imprint, series_qs) -> None: - """ - Updates the series objects with the new imprint and publisher. - - Args: - imprint: The new Imprint object to be assigned to the series. - series_qs: QuerySet of series objects to be updated. - - Returns: - None - """ - for series in series_qs: - series.imprint = imprint - series.publisher = imprint.publisher - changed = Series.objects.bulk_update(series_qs, ["imprint", "publisher"]) - self.stdout.write(self.style.SUCCESS(f"Updated {changed} series")) - - def add_arguments(self, parser): - """ - Adds command line arguments for the move_pub_to_imprint management command. - - Args: - parser: The parser object to which the arguments are added. - - Returns: - None - """ - parser.add_argument("--publisher", type=int, required=True) - parser.add_argument("--parent", type=int, required=True) - - def handle(self, *args, **options): - """ - Handles the process of moving a publisher to a new imprint by creating the imprint, - updating series and attribution, and deleting the original publisher. - - Args: - *args: Variable length argument list. - **options: Keyword arguments. - - Returns: - None - """ - - orig_pub = Publisher.objects.get(pk=options["publisher"]) - parent_pub = Publisher.objects.get(pk=options["parent"]) - - new_imprint = self._create_imprint(orig_pub, parent_pub) - series_qs = orig_pub.series.all() - self._update_series(new_imprint, series_qs) - self._update_attribution(orig_pub, new_imprint) - # remove the original publisher - self._delete_original(orig_pub) diff --git a/comicsdb/migrations/0060_attribution_ordering.py b/comicsdb/migrations/0060_attribution_ordering.py new file mode 100644 index 00000000..8fed06a2 --- /dev/null +++ b/comicsdb/migrations/0060_attribution_ordering.py @@ -0,0 +1,16 @@ +# Generated by Django 6.1.2 on 2026-10-16 13:18 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("comicsdb", "0059_issue_desc_is_solicitation"), + ] + + operations = [ + migrations.AlterModelOptions( + name="attribution", + options={"ordering": ["content_type", "object_id", "id"]}, + ), + ] diff --git a/comicsdb/models/attribution.py b/comicsdb/models/attribution.py index 22606763..9b91a269 100644 --- a/comicsdb/models/attribution.py +++ b/comicsdb/models/attribution.py @@ -20,7 +20,7 @@ class Source(models.TextChoices): class Meta: indexes = [models.Index(fields=["content_type", "object_id"], name="ct_obj_id_idx")] - ordering = ["content_type", "object_id"] + ordering = ["content_type", "object_id", "id"] def __str__(self) -> str: return f"{self.source} Attribution" diff --git a/comicsdb/signals.py b/comicsdb/signals.py index 95e562c1..c68fc4d9 100644 --- a/comicsdb/signals.py +++ b/comicsdb/signals.py @@ -1,5 +1,7 @@ import logging +from django.core.exceptions import FieldDoesNotExist +from django.db.models import QuerySet from django.utils import timezone from sorl.thumbnail import delete @@ -269,3 +271,51 @@ def bump_cache_on_name_change(label, sender, instance, **kwargs): pre_save flag is missing, so an unexpected path errs toward freshness.""" if instance.__dict__.pop("_cache_name_changed", True): bump_model_version(label) + + +def _touch_attribution_parent(content_type_id, object_id): + """Attribution is a generic relation, so saving or deleting one doesn't + touch its parent row. Bump the parent's `modified` explicitly so its + cached detail response (which embeds `attribution`) self-invalidates, + and its cache version, since its list response embeds `modified`.""" + from django.contrib.contenttypes.models import ContentType # noqa: PLC0415 + + # get_for_id() is served from ContentType's in-process cache. + model = ContentType.objects.get_for_id(content_type_id).model_class() + if model is None: + return + # The generic FK accepts any model; only bump ones that track `modified`. + try: + model._meta.get_field("modified") + except FieldDoesNotExist: + return + model.objects.filter(pk=object_id).update(modified=timezone.now()) + try: + label = ModelLabel(model._meta.model_name) + except ValueError: + return + bump_model_version(label) + + +def update_parent_modified_on_attribution_save(sender, instance, raw=False, **kwargs): + # Fixture loading may run before the ContentType/parent rows exist. + if raw: + return + # Only the current parent is bumped: nothing reassigns an attribution to a + # different parent, so code that does must bump the old parent itself. + _touch_attribution_parent(instance.content_type_id, instance.object_id) + + +def update_parent_modified_on_attribution_delete(sender, instance, origin=None, **kwargs): + # Attributions are only deleted on their own (instance or queryset + # delete) or by cascade when their parent is deleted. In the cascade + # case the parent is going away too, so there's nothing to bump. + parent = (instance.content_type_id, instance.object_id) + if isinstance(origin, sender): + _touch_attribution_parent(*parent) + elif isinstance(origin, QuerySet) and origin.model is sender: + # A queryset delete sends post_delete per row; bump each parent once. + touched = origin.__dict__.setdefault("_touched_attribution_parents", set()) + if parent not in touched: + touched.add(parent) + _touch_attribution_parent(*parent) diff --git a/comicsdb/views/issue.py b/comicsdb/views/issue.py index 7929a0d2..0da95e8f 100644 --- a/comicsdb/views/issue.py +++ b/comicsdb/views/issue.py @@ -296,7 +296,8 @@ def form_valid(self, form): "It may have been created by another user.", ) return self.form_invalid(form) - return super().form_valid(form) + # Not super().form_valid(): it would call form.save() a second time. + return HttpResponseRedirect(self.get_success_url()) class IssueUpdate(LoginRequiredMixin, UpdateView): diff --git a/comicsdb/views/mixins.py b/comicsdb/views/mixins.py index dd4a4d28..b6fd3ba0 100644 --- a/comicsdb/views/mixins.py +++ b/comicsdb/views/mixins.py @@ -8,6 +8,7 @@ from django.db import transaction from django.db.models import Q +from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import reverse from django.views import View @@ -74,7 +75,8 @@ def form_valid(self, form): return super().form_invalid(form) LOGGER.info(self.get_log_message(), form.instance.name, self.request.user) - return super().form_valid(form) + # Not super().form_valid(): it would call form.save() a second time. + return HttpResponseRedirect(self.get_success_url()) class AttributionUpdateMixin: @@ -144,14 +146,16 @@ def form_valid(self, form): with transaction.atomic(): form.instance.edited_by = self.request.user if attribution_form.is_valid(): - self.object = form.save(commit=False) + self.object = form.save() attribution_form.instance = self.object attribution_form.save() else: return super().form_invalid(form) LOGGER.info(self.get_log_message(), form.instance.name, self.request.user) - return super().form_valid(form) + # Not super().form_valid(): it would call form.save() a second time, outside the + # transaction. + return HttpResponseRedirect(self.get_success_url()) class NavigationMixin: diff --git a/metron/settings.py b/metron/settings.py index 451ecb2c..c71971fc 100644 --- a/metron/settings.py +++ b/metron/settings.py @@ -344,6 +344,7 @@ "url": "https://creativecommons.org/licenses/by-sa/4.0/", }, "ENUM_NAME_OVERRIDES": { + "AttributionSourceEnum": "api.v1_0.serializers.attribution.SOURCE_CHOICES", "CurrencyEnum": "metron.choices.CURRENCY_CHOICES", "DesiredGradeEnum": "user_collection.models.GRADE_CHOICES", }, diff --git a/tests/api/test_audit_response_cache.py b/tests/api/test_audit_response_cache.py index 37b405cc..07aeafcc 100644 --- a/tests/api/test_audit_response_cache.py +++ b/tests/api/test_audit_response_cache.py @@ -20,6 +20,11 @@ def test_categorize_detail_key_with_dependent_labels_still_extracts_model(): assert Command._categorize(key) == "api:detail:series" +def test_categorize_detail_key_with_schema_version_still_extracts_model(): + key = ":1:api:detail:arc:v2:retrieve:7:1704085200.0:abcdef0123456789" + assert Command._categorize(key) == "api:detail:arc" + + def test_categorize_list_key_extracts_model_label(): key = ":1:api:list:arc::4-2:abcdef0123456789" assert Command._categorize(key) == "api:list:arc" diff --git a/tests/comicsdb/test_api_attribution.py b/tests/comicsdb/test_api_attribution.py new file mode 100644 index 00000000..383b1bf8 --- /dev/null +++ b/tests/comicsdb/test_api_attribution.py @@ -0,0 +1,265 @@ +import uuid +from datetime import timedelta +from unittest.mock import patch + +import pytest +from django.core.cache.backends.locmem import LocMemCache +from django.db import connection +from django.test.utils import CaptureQueriesContext +from django.urls import reverse +from django.utils import timezone +from drf_spectacular.generators import SchemaGenerator +from rest_framework import status + +from api.cache import ModelLabel, get_model_version +from api.v1_0.serializers.attribution import SOURCE_NAMES +from comicsdb.models import Attribution, Character, Publisher, Rating + +# (fixture name, API basename) for every resource whose detail response embeds attribution. +ATTRIBUTED_RESOURCES = [ + ("wwh_arc", "arc"), + ("batman", "character"), + ("john_byrne", "creator"), + ("vertigo_imprint", "imprint"), + ("basic_issue", "issue"), + ("dc_comics", "publisher"), + ("fc_series", "series"), + ("teen_titans", "team"), + ("earth_2_universe", "universe"), +] + +WIKI_URL = "https://en.wikipedia.org/wiki/Example" +MARVEL_URL = "https://www.marvel.com/characters/example" + + +@pytest.fixture +def local_cache(): + """Isolate the response cache from concurrent xdist workers sharing Redis.""" + test_cache = LocMemCache(f"test-api-attribution-{uuid.uuid4()}", {}) + with patch("api.views.cache", test_cache), patch("api.cache.cache", test_cache): + yield test_cache + + +@pytest.mark.parametrize(("fixture_name", "basename"), ATTRIBUTED_RESOURCES) +def test_detail_attribution_is_empty_list_by_default( + request, api_client_with_credentials, local_cache, fixture_name, basename +): + obj = request.getfixturevalue(fixture_name) + resp = api_client_with_credentials.get(reverse(f"api:{basename}-detail", args=(obj.pk,))) + assert resp.status_code == status.HTTP_200_OK + assert resp.json()["attribution"] == [] + + +@pytest.mark.parametrize(("fixture_name", "basename"), ATTRIBUTED_RESOURCES) +def test_detail_attribution_returns_source_label_and_url( + request, api_client_with_credentials, local_cache, fixture_name, basename +): + obj = request.getfixturevalue(fixture_name) + obj.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + obj.attribution.create(source=Attribution.Source.MARVEL, url=MARVEL_URL) + + resp = api_client_with_credentials.get(reverse(f"api:{basename}-detail", args=(obj.pk,))) + assert resp.status_code == status.HTTP_200_OK + # Ordered by creation within one object, so the response is stable across cache misses. + assert resp.json()["attribution"] == [ + {"source": "Wikipedia", "url": WIKI_URL}, + {"source": "Marvel", "url": MARVEL_URL}, + ] + + +def test_every_attribution_source_has_an_api_name(): + assert set(SOURCE_NAMES) == set(Attribution.Source) + + +@pytest.mark.parametrize(("fixture_name", "basename"), ATTRIBUTED_RESOURCES) +def test_list_does_not_include_attribution( + request, api_client_with_credentials, local_cache, fixture_name, basename +): + obj = request.getfixturevalue(fixture_name) + obj.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + + resp = api_client_with_credentials.get(reverse(f"api:{basename}-list")) + assert resp.status_code == status.HTTP_200_OK + assert "attribution" not in resp.json()["results"][0] + + +def test_attribution_add_busts_detail_cache(api_client_with_credentials, batman, local_cache): + url = reverse("api:character-detail", args=(batman.pk,)) + assert api_client_with_credentials.get(url).json()["attribution"] == [] + + batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + + resp = api_client_with_credentials.get(url) + assert resp.json()["attribution"] == [{"source": "Wikipedia", "url": WIKI_URL}] + + +def test_attribution_edit_busts_detail_cache(api_client_with_credentials, batman, local_cache): + attribution = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + url = reverse("api:character-detail", args=(batman.pk,)) + assert api_client_with_credentials.get(url).json()["attribution"][0]["url"] == WIKI_URL + + attribution.source = Attribution.Source.MARVEL + attribution.url = MARVEL_URL + attribution.save() + + resp = api_client_with_credentials.get(url) + assert resp.json()["attribution"] == [{"source": "Marvel", "url": MARVEL_URL}] + + +def test_attribution_delete_busts_detail_cache(api_client_with_credentials, batman, local_cache): + attribution = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + url = reverse("api:character-detail", args=(batman.pk,)) + assert len(api_client_with_credentials.get(url).json()["attribution"]) == 1 + + attribution.delete() + + assert api_client_with_credentials.get(url).json()["attribution"] == [] + + +def test_attribution_change_updates_last_modified(api_client_with_credentials, batman, local_cache): + # Last-Modified has one-second resolution; backdate so the bump is visible. + Character.objects.filter(pk=batman.pk).update(modified=timezone.now() - timedelta(days=1)) + url = reverse("api:character-detail", args=(batman.pk,)) + last_modified = api_client_with_credentials.get(url)["Last-Modified"] + stale = api_client_with_credentials.get(url, HTTP_IF_MODIFIED_SINCE=last_modified) + assert stale.status_code == status.HTTP_304_NOT_MODIFIED + + batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + + resp = api_client_with_credentials.get(url, HTTP_IF_MODIFIED_SINCE=last_modified) + assert resp.status_code == status.HTTP_200_OK + assert len(resp.json()["attribution"]) == 1 + + +def test_attribution_change_bumps_parent_modified_and_cache_version(batman, local_cache): + past = timezone.now() - timedelta(days=1) + Character.objects.filter(pk=batman.pk).update(modified=past) + before = get_model_version(ModelLabel.CHARACTER) + + attribution = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + batman.refresh_from_db() + assert batman.modified > past + assert get_model_version(ModelLabel.CHARACTER) == before + 1 + + Character.objects.filter(pk=batman.pk).update(modified=past) + attribution.delete() + batman.refresh_from_db() + assert batman.modified > past + assert get_model_version(ModelLabel.CHARACTER) == before + 2 + + +def test_attribution_save_does_not_reread_its_row(batman, local_cache): + created = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + attribution = Attribution.objects.get(pk=created.pk) + attribution.url = MARVEL_URL + + with CaptureQueriesContext(connection) as ctx: + attribution.save() + + assert not [ + q + for q in ctx.captured_queries + if q["sql"].startswith("SELECT") and "comicsdb_attribution" in q["sql"] + ] + + +def test_attribution_save_does_not_query_content_type(batman, local_cache): + attribution = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + attribution = Attribution.objects.get(pk=attribution.pk) # fresh instance, no cached FK + + with CaptureQueriesContext(connection) as ctx: + attribution.save() + attribution.delete() + + assert not [q for q in ctx.captured_queries if "django_content_type" in q["sql"]] + + +def test_attribution_raw_save_skips_parent_bump(batman, local_cache): + attribution = Attribution( + source=Attribution.Source.WIKIPEDIA, url=WIKI_URL, content_object=batman + ) + with patch("comicsdb.signals._touch_attribution_parent") as touch: + attribution.save_base(raw=True) + touch.assert_not_called() + + +def test_parent_delete_skips_attribution_parent_bump(batman, local_cache): + batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + batman.attribution.create(source=Attribution.Source.MARVEL, url=MARVEL_URL) + + with patch("comicsdb.signals._touch_attribution_parent") as touch: + batman.delete() + + touch.assert_not_called() + assert not Attribution.objects.exists() + + +def test_attribution_queryset_delete_bumps_parent(batman, local_cache): + batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + past = timezone.now() - timedelta(days=1) + Character.objects.filter(pk=batman.pk).update(modified=past) + + batman.attribution.all().delete() + + batman.refresh_from_db() + assert batman.modified > past + + +def test_attribution_on_model_without_modified_saves(db, local_cache): + rating = Rating.objects.create(name="Test Rating") + + attribution = Attribution.objects.create( + source=Attribution.Source.WIKIPEDIA, url=WIKI_URL, content_object=rating + ) + attribution.delete() + + +def test_create_view_saves_parent_once(auto_login_user, local_cache): + client, _ = auto_login_user() + data = { + "name": "Ahoy Comics", + "country": "US", + "comicsdb-attribution-content_type-object_id-TOTAL_FORMS": 1, + "comicsdb-attribution-content_type-object_id-INITIAL_FORMS": 0, + "comicsdb-attribution-content_type-object_id-0-source": "W", + "comicsdb-attribution-content_type-object_id-0-url": WIKI_URL, + } + + resp = client.post(reverse("publisher:create"), data=data) + + assert resp.status_code == status.HTTP_302_FOUND + ahoy = Publisher.objects.get(name="Ahoy Comics") + assert resp.url == ahoy.get_absolute_url() + assert ahoy.attribution.count() == 1 + # One history row per save. + assert ahoy.history.count() == 1 + + +def test_attribution_queryset_delete_bumps_each_parent_once(batman, superman, local_cache): + batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + batman.attribution.create(source=Attribution.Source.MARVEL, url=MARVEL_URL) + superman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + + with patch("comicsdb.signals._touch_attribution_parent") as touch: + Attribution.objects.all().delete() + + assert touch.call_count == 2 + + +def test_schema_names_attribution_source_enum(db): + schemas = SchemaGenerator().get_schema(request=None, public=True)["components"]["schemas"] + assert schemas["AttributionSourceEnum"]["enum"] == list(SOURCE_NAMES.values()) + assert "SourceEnum" not in schemas + + +def test_unknown_source_code_falls_back_to_stored_value( + api_client_with_credentials, batman, local_cache +): + # The model only enforces choices in forms, so a raw write can store anything. + attribution = batman.attribution.create(source=Attribution.Source.WIKIPEDIA, url=WIKI_URL) + Attribution.objects.filter(pk=attribution.pk).update(source="X") + + resp = api_client_with_credentials.get(reverse("api:character-detail", args=(batman.pk,))) + + assert resp.status_code == status.HTTP_200_OK + assert resp.json()["attribution"] == [{"source": "X", "url": WIKI_URL}] diff --git a/tests/comicsdb/test_api_response_caching.py b/tests/comicsdb/test_api_response_caching.py index ba967adb..f9d62113 100644 --- a/tests/comicsdb/test_api_response_caching.py +++ b/tests/comicsdb/test_api_response_caching.py @@ -11,7 +11,7 @@ from rest_framework.pagination import PageNumberPagination from rest_framework.request import Request -from api import views as api_views +from api import cache as api_cache, views as api_views from api.cache import ModelLabel, get_model_version from api.views import CollectionViewSet, PullListViewSet, WishListViewSet from comicsdb.models import Credits, Issue, Variant @@ -839,6 +839,20 @@ def test_arc_retrieve_does_not_poison_issue_list_cache( assert body["results"][0]["number"] == "1" +def test_detail_schema_version_bump_orphans_cached_responses( + api_client_with_credentials, fc_arc, monkeypatch, local_cache +): + """Entries cached before a detail response's shape changed must not be + served after DETAIL_CACHE_SCHEMA_VERSION is bumped.""" + url = reverse("api:arc-detail", kwargs={"pk": fc_arc.pk}) + assert api_client_with_credentials.get(url)["X-Cache"] == "MISS" + assert api_client_with_credentials.get(url)["X-Cache"] == "HIT" + + monkeypatch.setattr(api_cache, "DETAIL_CACHE_SCHEMA_VERSION", 3) + + assert api_client_with_credentials.get(url)["X-Cache"] == "MISS" + + def test_arc_issue_list_pagination_is_not_poisoned_across_pages( api_client_with_credentials, issue_with_arc, fc_arc, fc_series, monkeypatch, local_cache ): diff --git a/tests/comicsdb/test_issue_views.py b/tests/comicsdb/test_issue_views.py index cf94755d..f4040f69 100644 --- a/tests/comicsdb/test_issue_views.py +++ b/tests/comicsdb/test_issue_views.py @@ -2199,6 +2199,29 @@ def test_issue_create_checks_variant_upc_against_cover_date( assert b'href="#section-variants"' in resp.content +def test_issue_create_saves_issue_once(auto_login_user, fc_series): + client, _ = auto_login_user() + data = { + "series": fc_series.pk, + "number": "1", + "cover_date": "2023-01-01", + "rating": Rating.objects.create(name="Test Rating").pk, + **{ + f"{prefix}-{field}": "0" + for prefix in ("credits", "variants", "attribution") + for field in ("TOTAL_FORMS", "INITIAL_FORMS") + }, + } + + resp = client.post(reverse("issue:create"), data=data) + + issue = Issue.objects.get(series=fc_series, number="1") + assert resp.status_code == HTML_REDIRECT_CODE + assert resp.url == issue.get_absolute_url() + # One history row per save. + assert issue.history.count() == 1 + + def test_issue_detail_shows_solicitation_tag(solicited_issue, auto_login_user): client, _ = auto_login_user() resp = client.get(reverse("issue:detail", kwargs={"slug": solicited_issue.slug})) diff --git a/tests/users/test_views.py b/tests/users/test_views.py index ba9cf571..f788b750 100644 --- a/tests/users/test_views.py +++ b/tests/users/test_views.py @@ -29,7 +29,7 @@ def test_view_url_exists_at_desired_location(auto_login_user, url): assert resp.status_code == HTML_OK_CODE -# @pytest.mark.parametrize("url", ["/accounts/update", "/accounts/password", "/accounts/signup"]) # noqa: E501 +# @pytest.mark.parametrize("url", ["/accounts/update", "/accounts/password", "/accounts/signup"]) # def test_view_url_exists_at_desired_location_redirected(auto_login_user, url): # client, _ = auto_login_user() # resp = client.get(url)