Skip to content

Commit a4e88c7

Browse files
author
Ryan Winkler
committed
Harden RSS item identity and parent titles
- Distinguish valid library variants and encode GUID components. - Select the matching stored parent title in one local query. - Cover grouped anime, orphan rows, offline use, and query growth. Refs #48 Related: FuzzyGrim#714
1 parent 8ded080 commit a4e88c7

2 files changed

Lines changed: 287 additions & 28 deletions

File tree

‎src/lists/feeds.py‎

Lines changed: 96 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
from urllib.parse import quote
2+
13
from django.apps import apps
24
from django.contrib.auth.decorators import login_not_required
35
from django.contrib.syndication.views import Feed
6+
from django.db.models import Q
47
from django.http import Http404, JsonResponse
58
from django.urls import reverse
69
from django.utils import timezone
@@ -89,29 +92,83 @@ def _attach_owner_media_statuses(self, list_items, owner):
8992
list_item.feed_status = status_by_item_id.get(list_item.item_id, "")
9093
list_item.feed_description = self._build_item_description(list_item.item)
9194

95+
@staticmethod
96+
def _show_title(media_item, shows):
97+
"""Return the closest stored series title for a season or episode."""
98+
if not shows:
99+
return None
100+
101+
library_media_type = media_item.library_media_type or media_item.media_type
102+
show = min(
103+
shows,
104+
key=lambda candidate: (
105+
candidate.library_media_type != library_media_type,
106+
candidate.media_type != library_media_type,
107+
candidate.media_type != MediaTypes.TV.value,
108+
candidate.pk,
109+
),
110+
)
111+
return show.title
112+
92113
def _attach_show_titles(self, list_items):
93-
"""Attach the parent show's title to episode/season list items."""
94-
show_keys = {
95-
(list_item.item.source, list_item.item.media_id)
96-
for list_item in list_items
97-
if list_item.item.media_type
98-
in (MediaTypes.EPISODE.value, MediaTypes.SEASON.value)
99-
}
100-
if not show_keys:
114+
"""Attach stored parent titles with one local query."""
115+
child_ids_by_source = {}
116+
117+
for list_item in list_items:
118+
media_item = list_item.item
119+
if media_item.media_type not in {
120+
MediaTypes.SEASON.value,
121+
MediaTypes.EPISODE.value,
122+
}:
123+
continue
124+
list_item.feed_show_title = None
125+
child_ids_by_source.setdefault(media_item.source, set()).add(
126+
media_item.media_id
127+
)
128+
129+
if not child_ids_by_source:
101130
return
102131

103-
sources = {source for source, _media_id in show_keys}
104-
media_ids = {media_id for _source, media_id in show_keys}
105-
shows = Item.objects.filter(
106-
media_type=MediaTypes.TV.value,
107-
source__in=sources,
108-
media_id__in=media_ids,
132+
identity_filter = Q()
133+
for source, media_ids in child_ids_by_source.items():
134+
identity_filter |= Q(source=source, media_id__in=media_ids)
135+
136+
shows = (
137+
Item.objects.filter(identity_filter)
138+
.filter(
139+
media_type__in=[MediaTypes.TV.value, MediaTypes.ANIME.value],
140+
season_number__isnull=True,
141+
episode_number__isnull=True,
142+
)
143+
.only(
144+
"media_id",
145+
"source",
146+
"media_type",
147+
"library_media_type",
148+
"title",
149+
)
150+
.order_by("pk")
109151
)
110-
title_by_key = {(show.source, show.media_id): show.title for show in shows}
152+
153+
shows_by_identity = {}
154+
for show in shows:
155+
shows_by_identity.setdefault(
156+
(show.source, show.media_id),
157+
[],
158+
).append(show)
111159

112160
for list_item in list_items:
113-
key = (list_item.item.source, list_item.item.media_id)
114-
list_item.feed_show_title = title_by_key.get(key)
161+
media_item = list_item.item
162+
if media_item.media_type not in {
163+
MediaTypes.SEASON.value,
164+
MediaTypes.EPISODE.value,
165+
}:
166+
continue
167+
candidates = shows_by_identity.get(
168+
(media_item.source, media_item.media_id),
169+
[],
170+
)
171+
list_item.feed_show_title = self._show_title(media_item, candidates)
115172

116173
def _build_item_description(self, item):
117174
"""Return a local feed description without provider lookups."""
@@ -162,11 +219,16 @@ def items(self, obj):
162219
return list_items
163220

164221
def item_title(self, item):
165-
"""Return the item title with S01E02/year markers for automation tools."""
222+
"""Return a title that automation tools can match."""
166223
media_item = item.item
167-
title = getattr(item, "feed_show_title", None) or media_item.title
224+
title = media_item.title
168225

169226
if media_item.season_number is not None:
227+
title = (
228+
getattr(item, "feed_show_title", None)
229+
or media_item.series_name
230+
or title
231+
)
170232
title += f" S{media_item.season_number:02d}"
171233
if media_item.episode_number is not None:
172234
title += f"E{media_item.episode_number:02d}"
@@ -186,19 +248,25 @@ def item_link(self, item):
186248
return self.request.build_absolute_uri(media_url(item.item))
187249

188250
def item_guid(self, item):
189-
"""Return a guid from the item's own immutable natural key."""
251+
"""Return a stable GUID from the stored item identity."""
190252
media_item = item.item
191-
guid = f"{media_item.source}:{media_item.media_type}:{media_item.media_id}"
192-
253+
media_type = media_item.media_type
254+
library_media_type = media_item.library_media_type or media_type
255+
parts = [
256+
quote(media_item.source, safe=""),
257+
quote(media_type, safe=""),
258+
quote(str(media_item.media_id), safe=""),
259+
]
260+
if library_media_type != media_type:
261+
parts.extend(["library", quote(library_media_type, safe="")])
193262
if media_item.season_number is not None:
194-
guid += f":s{media_item.season_number}"
195-
if media_item.episode_number is not None:
196-
guid += f":e{media_item.episode_number}"
197-
198-
return guid
263+
parts.append(f"s{media_item.season_number}")
264+
if media_item.episode_number is not None:
265+
parts.append(f"e{media_item.episode_number}")
266+
return ":".join(parts)
199267

200268
def item_guid_is_permalink(self, item):
201-
"""Return whether the guid is a dereferenceable URL."""
269+
"""Mark the identity GUID as a non-permalink."""
202270
return False
203271

204272
def item_categories(self, item):

‎src/lists/tests/test_rss_feed.py‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import xml.etree.ElementTree as ET
2+
from unittest.mock import patch
3+
4+
from django.contrib.auth import get_user_model
5+
from django.test import TestCase
6+
from django.urls import reverse
7+
8+
from app.models import Item, MediaTypes, Sources
9+
from lists.feeds import PublicListFeed
10+
from lists.models import CustomList, CustomListItem
11+
12+
13+
class PublicListRssIdentityTests(TestCase):
14+
"""Cover RSS identity, parent titles, and offline operation."""
15+
16+
def setUp(self):
17+
self.user = get_user_model().objects.create_user(
18+
username="rss-identity-user",
19+
password="testpassword",
20+
)
21+
self.custom_list = CustomList.objects.create(
22+
name="RSS Identity",
23+
description="RSS identity regression tests",
24+
owner=self.user,
25+
visibility="public",
26+
)
27+
28+
def _create_list_item(self, **item_fields):
29+
item = Item.objects.create(**item_fields)
30+
CustomListItem.objects.create(custom_list=self.custom_list, item=item)
31+
return item
32+
33+
def _feed_items(self):
34+
response = self.client.get(
35+
reverse("list_rss", args=[self.custom_list.id]),
36+
)
37+
self.assertEqual(response.status_code, 200)
38+
root = ET.fromstring(response.content)
39+
return root.findall("./channel/item")
40+
41+
def test_grouped_anime_uses_matching_library_parent(self):
42+
"""A grouped anime episode uses the title from its library parent."""
43+
Item.objects.create(
44+
media_id="grouped-anime",
45+
source=Sources.TMDB.value,
46+
media_type=MediaTypes.TV.value,
47+
library_media_type=MediaTypes.ANIME.value,
48+
title="Anime Library Title",
49+
)
50+
Item.objects.create(
51+
media_id="grouped-anime",
52+
source=Sources.TMDB.value,
53+
media_type=MediaTypes.TV.value,
54+
library_media_type=MediaTypes.TV.value,
55+
title="TV Library Title",
56+
)
57+
self._create_list_item(
58+
media_id="grouped-anime",
59+
source=Sources.TMDB.value,
60+
media_type=MediaTypes.EPISODE.value,
61+
library_media_type=MediaTypes.ANIME.value,
62+
title="Episode Name",
63+
season_number=1,
64+
episode_number=3,
65+
)
66+
67+
titles = [item.findtext("title") for item in self._feed_items()]
68+
69+
self.assertEqual(titles, ["Anime Library Title S01E03"])
70+
71+
def test_series_name_is_used_when_parent_is_not_stored(self):
72+
"""An orphan episode remains matchable from its stored series name."""
73+
self._create_list_item(
74+
media_id="orphan-episode",
75+
source=Sources.TMDB.value,
76+
media_type=MediaTypes.EPISODE.value,
77+
title="Pilot",
78+
series_name="Orphan Series",
79+
season_number=1,
80+
episode_number=1,
81+
)
82+
83+
titles = [item.findtext("title") for item in self._feed_items()]
84+
85+
self.assertEqual(titles, ["Orphan Series S01E01"])
86+
87+
def test_season_uses_stored_parent_title(self):
88+
"""A season entry uses its parent title and season marker."""
89+
Item.objects.create(
90+
media_id="season-show",
91+
source=Sources.TMDB.value,
92+
media_type=MediaTypes.TV.value,
93+
title="Season Show",
94+
)
95+
self._create_list_item(
96+
media_id="season-show",
97+
source=Sources.TMDB.value,
98+
media_type=MediaTypes.SEASON.value,
99+
title="Season One",
100+
season_number=1,
101+
)
102+
103+
titles = [item.findtext("title") for item in self._feed_items()]
104+
105+
self.assertEqual(titles, ["Season Show S01"])
106+
107+
def test_guid_distinguishes_library_variants(self):
108+
"""Two library rows for one provider item have distinct GUIDs."""
109+
self._create_list_item(
110+
media_id="shared-show",
111+
source=Sources.TMDB.value,
112+
media_type=MediaTypes.TV.value,
113+
library_media_type=MediaTypes.TV.value,
114+
title="TV Library Row",
115+
)
116+
self._create_list_item(
117+
media_id="shared-show",
118+
source=Sources.TMDB.value,
119+
media_type=MediaTypes.TV.value,
120+
library_media_type=MediaTypes.ANIME.value,
121+
title="Anime Library Row",
122+
)
123+
124+
guids = {item.findtext("guid") for item in self._feed_items()}
125+
126+
self.assertEqual(
127+
guids,
128+
{
129+
"tmdb:tv:shared-show",
130+
"tmdb:tv:shared-show:library:anime",
131+
},
132+
)
133+
134+
def test_guid_encodes_reserved_identity_characters(self):
135+
"""Reserved characters cannot make GUID components ambiguous."""
136+
self._create_list_item(
137+
media_id="custom:item/one",
138+
source=Sources.MANUAL.value,
139+
media_type=MediaTypes.MOVIE.value,
140+
title="Manual Movie",
141+
)
142+
143+
guid = self._feed_items()[0].findtext("guid")
144+
145+
self.assertEqual(guid, "manual:movie:custom%3Aitem%2Fone")
146+
147+
def test_parent_title_lookup_uses_one_query_for_multiple_children(self):
148+
"""Parent lookup query count does not grow with the item count."""
149+
Item.objects.create(
150+
media_id="query-show",
151+
source=Sources.TMDB.value,
152+
media_type=MediaTypes.TV.value,
153+
title="Query Show",
154+
)
155+
for episode_number in (1, 2):
156+
self._create_list_item(
157+
media_id="query-show",
158+
source=Sources.TMDB.value,
159+
media_type=MediaTypes.EPISODE.value,
160+
title=f"Episode {episode_number}",
161+
season_number=1,
162+
episode_number=episode_number,
163+
)
164+
list_items = list(
165+
CustomListIitem.objects.filter(custom_list=self.custom_list)
166+
.select_related("item")
167+
.order_by("pk")
168+
)
169+
170+
with self.assertNumQueries(1):
171+
PublicListFeed()._attach_show_titles(list_items)
172+
173+
self.assertEqual(
174+
[item.feed_show_title for item in list_items],
175+
["Query Show", "Query Show"],
176+
)
177+
178+
@patch("lists.feeds.tmdb.tv")
179+
def test_rss_generation_does_not_call_external_providers(self, tmdb_tv):
180+
"""RSS generation remains usable when providers are offline."""
181+
self._create_list_item(
182+
media_id="offline-movie",
183+
source=Sources.TMDB.value,
184+
media_type=MediaTypes.MOVIE.value,
185+
title="Offline Movie",
186+
)
187+
188+
items = self._feed_items()
189+
190+
self.assertEqual(len(items), 1)
191+
tmdb_tv.assert_not_called()

0 commit comments

Comments
 (0)