fix(M5): corrections d'audit — transport HTTP, statuts d'erreur, cache atomique
Corrige les 5 points de l'audit FIXME_M5 : 1. (Bloquant) Sépare transport HTTP et parsing : utilise requests.get() avec timeout explicite et en-têtes conditionnels, puis transmet le contenu à feedparser.parse() — supprime le paramètre inexistant request_timeout qui faisait échouer toute récupération réelle. 2. Rejette les statuts HTTP 4xx/5xx via raise_for_status() avant le parsing. 3. Préserve les validateurs de cache (etag, last_modified) d'entrée sur les chemins d'échec (exception, bozo) au lieu de les écraser à None. 4. Sauvegarde atomique de BlogRSSState : écrit dans un .tmp puis Path.replace() pour éviter la corruption sur interruption. 5. Déduplication normale silencieuse : les GUID déjà connus sont ignorés sans warning ; seuls les doublons intra-flux génèrent un avertissement. Tests : 49 tests (32 client + 17 state) dont 11 nouveaux couvrant transport HTTP réel, statuts 401/404/500, préservation des validateurs, en-têtes conditionnels, doublons intra-flux et sauvegarde atomique. Guide : §5 bis.7.1 aligné avec le nouveau pattern transport/parsing. Co-authored-by: opencode/coder <coder@agents.invalid> Co-authored-by: opencode/test-engineer <test-engineer@agents.invalid>
This commit is contained in:
@@ -1,8 +1,9 @@
|
||||
"""Client de récupération et de parsing du flux RSS du blog du collège.
|
||||
|
||||
Ce module définit :class:`BlogRSSClient`, un client sans état qui
|
||||
télécharge le flux RSS du blog via ``feedparser``, déduplique les entrées
|
||||
par GUID et les convertit en :class:`~pronote_sync.models.blog.BlogArticle`.
|
||||
télécharge le flux RSS du blog via ``requests``, le parse via
|
||||
``feedparser``, déduplique les entrées par GUID et les convertit en
|
||||
:class:`~pronote_sync.models.blog.BlogArticle`.
|
||||
|
||||
Le résultat d'une récupération est un
|
||||
:class:`~pronote_sync.sources.blog.result.BlogRSSFetchResult` : les
|
||||
@@ -21,6 +22,7 @@ from datetime import UTC, datetime
|
||||
from html import unescape
|
||||
|
||||
import feedparser # type: ignore[import-untyped]
|
||||
import requests
|
||||
from bs4 import BeautifulSoup
|
||||
|
||||
from pronote_sync.models.blog import BlogArticle
|
||||
@@ -63,9 +65,10 @@ class BlogRSSClient:
|
||||
) -> BlogRSSFetchResult:
|
||||
"""Télécharge et parse le flux RSS du blog en nouveaux articles.
|
||||
|
||||
Le flux est téléchargé par ``feedparser`` avec les en-têtes de
|
||||
requête conditionnelle fournis (``ETag``/``Last-Modified``). Si le
|
||||
serveur répond ``304 Not Modified``, le résultat est vide avec
|
||||
Le flux est téléchargé par ``requests`` avec les en-têtes de
|
||||
requête conditionnelle fournis (``ETag``/``Last-Modified``), puis
|
||||
parsé par ``feedparser``. Si le serveur répond ``304 Not Modified``,
|
||||
le résultat est vide avec
|
||||
``not_modified=True`` et les en-têtes passés en entrée sont
|
||||
restitués tels quels. Chaque entrée est dédupliquée par GUID,
|
||||
convertie en :class:`~pronote_sync.models.blog.BlogArticle`, puis
|
||||
@@ -87,16 +90,19 @@ class BlogRSSClient:
|
||||
:rtype: :class:`~pronote_sync.sources.blog.result.BlogRSSFetchResult`
|
||||
"""
|
||||
try:
|
||||
feed = feedparser.parse(
|
||||
self.rss_url,
|
||||
etag=etag,
|
||||
modified=last_modified,
|
||||
request_timeout=self.timeout,
|
||||
)
|
||||
# Téléchargement HTTP explicite via requests : feedparser 6.x
|
||||
# n'accepte aucun paramètre de transport ; les requêtes
|
||||
# conditionnelles sont gérées avec les en-têtes HTTP standards.
|
||||
headers: dict[str, str] = {"user-agent": "pronote-sync"}
|
||||
if etag is not None:
|
||||
headers["If-None-Match"] = etag
|
||||
if last_modified is not None:
|
||||
headers["If-Modified-Since"] = last_modified
|
||||
response = requests.get(self.rss_url, headers=headers, timeout=self.timeout)
|
||||
|
||||
# Réponse 304 Not Modified : rien n'a changé, on restitue les
|
||||
# en-têtes mémorisés tels quels pour les conserver.
|
||||
if getattr(feed, "status", None) == 304:
|
||||
if response.status_code == 304:
|
||||
return BlogRSSFetchResult(
|
||||
articles=(),
|
||||
etag=etag,
|
||||
@@ -104,16 +110,24 @@ class BlogRSSClient:
|
||||
not_modified=True,
|
||||
)
|
||||
|
||||
response_etag: str | None = getattr(feed, "etag", None)
|
||||
response_last_modified: str | None = getattr(feed, "modified", None)
|
||||
if response_last_modified is None:
|
||||
headers = getattr(feed, "headers", None)
|
||||
if headers is not None:
|
||||
# Les clés des en-têtes sont en minuscules côté feedparser.
|
||||
response_last_modified = headers.get("last-modified") or None
|
||||
# Les statuts 4xx/5xx lèvent une exception HTTP, attrapée par le
|
||||
# gestionnaire général et dégradée en résultat vide.
|
||||
response.raise_for_status()
|
||||
|
||||
# Flux invalide (erreur HTTP, XML malformé, etc.) : avertissement
|
||||
# puis résultat vide, sans propager l'exception brute.
|
||||
response_etag: str | None = response.headers.get("ETag", None)
|
||||
if response_etag is None:
|
||||
response_etag = response.headers.get("etag", None)
|
||||
response_last_modified: str | None = response.headers.get("Last-Modified", None)
|
||||
if response_last_modified is None:
|
||||
response_last_modified = response.headers.get("last-modified", None)
|
||||
|
||||
# feedparser ne reçoit que le contenu brut de la réponse.
|
||||
feed = feedparser.parse(response.content)
|
||||
|
||||
# Flux invalide (XML malformé, etc.) : avertissement puis résultat
|
||||
# vide, sans propager l'exception brute. Les validateurs de cache
|
||||
# d'entrée sont conservés : on ne fait pas confiance aux en-têtes
|
||||
# d'une réponse au contenu invalide.
|
||||
if getattr(feed, "bozo", None):
|
||||
bozo_exception = getattr(feed, "bozo_exception", None)
|
||||
if bozo_exception is not None:
|
||||
@@ -129,13 +143,16 @@ class BlogRSSClient:
|
||||
)
|
||||
return BlogRSSFetchResult(
|
||||
articles=(),
|
||||
etag=response_etag,
|
||||
last_modified=response_last_modified,
|
||||
etag=etag,
|
||||
last_modified=last_modified,
|
||||
not_modified=False,
|
||||
)
|
||||
|
||||
articles: list[BlogArticle] = []
|
||||
seen_guids: set[str] = set(known_guids) if known_guids is not None else set()
|
||||
# Déduplication silencieuse des GUID déjà connus (exécutions
|
||||
# précédentes) et détection des doublons au sein de la réponse.
|
||||
known_set = set(known_guids) if known_guids is not None else None
|
||||
seen_in_feed: set[str] = set()
|
||||
for entry in getattr(feed, "entries", []):
|
||||
guid_source = entry.get("id") or entry.get("link")
|
||||
if not guid_source:
|
||||
@@ -146,13 +163,20 @@ class BlogRSSClient:
|
||||
continue
|
||||
guid = str(guid_source)
|
||||
|
||||
if guid in seen_guids:
|
||||
if known_set is not None and guid in known_set:
|
||||
# Déduplication normale (GUID connu d'une exécution
|
||||
# précédente) : aucun journal n'est nécessaire.
|
||||
continue
|
||||
|
||||
if guid in seen_in_feed:
|
||||
logger.warning(
|
||||
"Entrée RSS déjà traitée ou en double, ignorée : %s",
|
||||
"Entrée RSS en double dans le flux, ignorée : %s",
|
||||
redact_url(self.rss_url),
|
||||
)
|
||||
continue
|
||||
|
||||
seen_in_feed.add(guid)
|
||||
|
||||
published_at = self._parse_date(
|
||||
entry.get("published_parsed") or entry.get("pubdate_parsed")
|
||||
)
|
||||
@@ -197,8 +221,6 @@ class BlogRSSClient:
|
||||
)
|
||||
)
|
||||
|
||||
seen_guids.add(guid)
|
||||
|
||||
# Tri stable : d'abord par identifiant croissant, puis par date de
|
||||
# publication décroissante ; l'ordre par identifiant est conservé
|
||||
# entre articles de même date.
|
||||
@@ -217,7 +239,12 @@ class BlogRSSClient:
|
||||
redact_url(self.rss_url),
|
||||
redact_exception(exc),
|
||||
)
|
||||
return BlogRSSFetchResult(articles=(), not_modified=False)
|
||||
return BlogRSSFetchResult(
|
||||
articles=(),
|
||||
etag=etag,
|
||||
last_modified=last_modified,
|
||||
not_modified=False,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _parse_date(date_tuple: tuple[int, ...] | None) -> datetime | None:
|
||||
|
||||
@@ -88,11 +88,15 @@ class BlogRSSState:
|
||||
)
|
||||
|
||||
def _save(self) -> None:
|
||||
"""Sauvegarde l'état dans le fichier JSON.
|
||||
"""Sauvegarde l'état dans le fichier JSON de manière atomique.
|
||||
|
||||
La sortie est déterministe : ``known_guids`` est trié
|
||||
alphabétiquement et le champ ``version`` vaut 1. En cas d'erreur
|
||||
d'écriture, une erreur est journalisée sans être propagée.
|
||||
alphabétiquement et le champ ``version`` vaut 1. Le JSON est
|
||||
d'abord écrit dans un fichier temporaire du même répertoire, puis
|
||||
remplacé atomiquement par :meth:`~pathlib.Path.replace` afin de ne
|
||||
jamais laisser un fichier partiel en cas d'interruption. En cas
|
||||
d'erreur d'écriture, une erreur est journalisée sans être
|
||||
propagée et le fichier temporaire est supprimé.
|
||||
"""
|
||||
payload = {
|
||||
"version": _STATE_VERSION,
|
||||
@@ -100,15 +104,24 @@ class BlogRSSState:
|
||||
"etag": self._etag,
|
||||
"last_modified": self._last_modified,
|
||||
}
|
||||
tmp_file = self._state_file.with_suffix(".tmp")
|
||||
try:
|
||||
with self._state_file.open("w", encoding="utf-8") as handle:
|
||||
with open(tmp_file, "w", encoding="utf-8") as handle:
|
||||
json.dump(payload, handle, indent=2)
|
||||
tmp_file.replace(self._state_file)
|
||||
except Exception as exc:
|
||||
logger.error(
|
||||
"Impossible d'écrire le fichier d'état blog RSS %s : %s.",
|
||||
redact_secrets(str(self._state_file)),
|
||||
redact_exception(exc),
|
||||
)
|
||||
try:
|
||||
tmp_file.unlink(missing_ok=True)
|
||||
except Exception as cleanup_exc:
|
||||
logger.debug(
|
||||
"Nettoyage du fichier temporaire échoué : %s",
|
||||
redact_exception(cleanup_exc),
|
||||
)
|
||||
|
||||
def get_known_guids(self) -> frozenset[str]:
|
||||
"""Renvoie une copie immuable des GUID d'articles déjà connus.
|
||||
|
||||
Reference in New Issue
Block a user