From bfae1ca87fc4b699a7cc2847194b9cce7a37e4fb Mon Sep 17 00:00:00 2001 From: Antoine Van Elstraete Date: Sun, 6 Sep 2026 20:58:04 +0200 Subject: [PATCH] =?UTF-8?q?fix(M5):=20corrections=20d'audit=20=E2=80=94=20?= =?UTF-8?q?transport=20HTTP,=20statuts=20d'erreur,=20cache=20atomique?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: opencode/test-engineer --- .secrets.baseline | 4 +- GUIDE_DEV_PYTHON.md | 45 +- pronote_sync/sources/blog/rss.py | 85 ++-- pronote_sync/sources/blog/state.py | 21 +- tests/unit/test_blog_client.py | 657 ++++++++++++++++++++++++++--- tests/unit/test_blog_state.py | 39 ++ 6 files changed, 753 insertions(+), 98 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 31437f1..9ae5bb0 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -140,10 +140,10 @@ "filename": "GUIDE_DEV_PYTHON.md", "hashed_secret": "90bd1b48e958257948487b90bee080ba5ed00caa", "is_verified": true, - "line_number": 5043, + "line_number": 5058, "is_secret": false } ] }, - "generated_at": "2026-09-06T16:41:06Z" + "generated_at": "2026-09-06T18:57:58Z" } diff --git a/GUIDE_DEV_PYTHON.md b/GUIDE_DEV_PYTHON.md index 2ff869d..6dc6232 100644 --- a/GUIDE_DEV_PYTHON.md +++ b/GUIDE_DEV_PYTHON.md @@ -850,6 +850,7 @@ from datetime import UTC, datetime from html import unescape import feedparser +import requests from bs4 import BeautifulSoup from ..models.blog import BlogArticle @@ -891,16 +892,17 @@ class BlogRSSClient: indicateur ``304 Not Modified``. """ try: - # Récupération du flux avec requête conditionnelle (ETag / Last-Modified) - feed = feedparser.parse( - self.rss_url, - etag=etag, - modified=last_modified, - request_timeout=self.timeout, - ) + # Transport HTTP séparé du parsing + 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 - # Réponse 304 Not Modified : rien n'a changé, on restitue les en-têtes mémorisés - if getattr(feed, "status", None) == 304: + response = requests.get(self.rss_url, headers=headers, timeout=self.timeout) + + # 304 Not Modified : pas de nouveaux articles + if response.status_code == 304: return BlogRSSFetchResult( articles=(), etag=etag, @@ -908,10 +910,18 @@ class BlogRSSClient: not_modified=True, ) - response_etag: str | None = getattr(feed, "etag", None) - response_last_modified: str | None = getattr(feed, "modified", None) + # Rejeter les statuts d'erreur (4xx/5xx) + response.raise_for_status() - # Flux invalide (erreur HTTP, XML malformé, etc.) : résultat vide, sans erreur + # Extraire les en-têtes de cache de la réponse (ETag / Last-Modified) + response_etag: str | None = response.headers.get("ETag") + response_last_modified: str | None = response.headers.get("Last-Modified") + + # Parsing du contenu reçu (pas de l'URL) + feed = feedparser.parse(response.content) + + # Flux invalide (XML malformé, etc.) : résultat vide, sans erreur ; + # les en-têtes de cache d'entrée sont conservés tels quels if getattr(feed, "bozo", None): logger.warning( "Flux RSS du blog invalide, ignoré : %s", @@ -919,8 +929,8 @@ class BlogRSSClient: ) return BlogRSSFetchResult( articles=(), - etag=response_etag, - last_modified=response_last_modified, + etag=etag, + last_modified=last_modified, not_modified=False, ) @@ -988,7 +998,12 @@ class BlogRSSClient: except Exception as e: safe_url = redact_url(self.rss_url) logger.error(f"Échec de la récupération du flux RSS {safe_url}: {e}") - 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: diff --git a/pronote_sync/sources/blog/rss.py b/pronote_sync/sources/blog/rss.py index 083417a..50c74fd 100644 --- a/pronote_sync/sources/blog/rss.py +++ b/pronote_sync/sources/blog/rss.py @@ -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: diff --git a/pronote_sync/sources/blog/state.py b/pronote_sync/sources/blog/state.py index abe6735..a5907e8 100644 --- a/pronote_sync/sources/blog/state.py +++ b/pronote_sync/sources/blog/state.py @@ -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. diff --git a/tests/unit/test_blog_client.py b/tests/unit/test_blog_client.py index 2958fb9..43b978d 100644 --- a/tests/unit/test_blog_client.py +++ b/tests/unit/test_blog_client.py @@ -20,10 +20,13 @@ import logging from datetime import UTC, datetime from pathlib import Path from typing import Any +from unittest.mock import Mock import feedparser # type: ignore[import-untyped] import pytest import pytest_mock +import requests +import responses from pronote_sync.sources.blog.rss import BlogRSSClient @@ -167,6 +170,18 @@ def test_fetch_and_parse_nominal( :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 avec le contenu du fixture + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + # Mock feedparser.parse pour retourner le résultat réel du parsing mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", @@ -213,6 +228,7 @@ def test_fetch_and_parse_nominal( def test_fetch_and_parse_deduplication( mocker: pytest_mock.MockerFixture, real_parsed_feed: feedparser.FeedParserDict, + blog_rss_fixture_path: Path, ) -> None: """Vérifie la déduplication avec known_guids. @@ -220,8 +236,21 @@ def test_fetch_and_parse_deduplication( :param mocker: Fixture pytest-mock pour le mocking. :param real_parsed_feed: Résultat réel du parsing du fixture. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=real_parsed_feed, @@ -241,13 +270,27 @@ def test_fetch_and_parse_deduplication( def test_fetch_and_parse_empty_known_guids( mocker: pytest_mock.MockerFixture, real_parsed_feed: feedparser.FeedParserDict, + blog_rss_fixture_path: Path, ) -> None: """Vérifie que known_guids vide (frozenset()) retourne tous les articles. :param mocker: Fixture pytest-mock pour le mocking. :param real_parsed_feed: Résultat réel du parsing du fixture. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=real_parsed_feed, @@ -262,13 +305,27 @@ def test_fetch_and_parse_empty_known_guids( def test_fetch_and_parse_known_guids_none( mocker: pytest_mock.MockerFixture, real_parsed_feed: feedparser.FeedParserDict, + blog_rss_fixture_path: Path, ) -> None: """Vérifie que known_guids=None retourne tous les articles. :param mocker: Fixture pytest-mock pour le mocking. :param real_parsed_feed: Résultat réel du parsing du fixture. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=real_parsed_feed, @@ -290,10 +347,14 @@ def test_fetch_and_parse_304_not_modified( :param mocker: Fixture pytest-mock pour le mocking. :return: None """ - mock_feed = make_mock_feed(status=304, etag="old_etag", modified="old_modified") + # Mock requests.get pour retourner une réponse 304 + mock_response = Mock() + mock_response.status_code = 304 + mock_response.headers = {} + mocker.patch( - "pronote_sync.sources.blog.rss.feedparser.parse", - return_value=mock_feed, + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, ) client = BlogRSSClient(rss_url="https://example.com/blog/feed") @@ -315,6 +376,7 @@ def test_fetch_and_parse_bozo_invalid_feed( """Vérifie la gestion d'un flux invalide (bozo=1). Doit retourner articles vide, not_modified=False, et logger un avertissement. + Les en-têtes d'entrée sont conservés (pas ceux de la réponse invalide). :param mocker: Fixture pytest-mock pour le mocking. :param caplog: Fixture pour capturer les logs. @@ -325,21 +387,33 @@ def test_fetch_and_parse_bozo_invalid_feed( mock_feed = make_mock_feed( bozo=1, bozo_exception=mock_exception, - etag="test_etag", - modified="test_modified", ) + + # Mock requests.get pour retourner une réponse 200 avec du contenu + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {"ETag": "response_etag", "Last-Modified": "response_modified"} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=mock_feed, ) client = BlogRSSClient(rss_url="https://example.com/blog/feed") - result = client.fetch_and_parse() + result = client.fetch_and_parse(etag="input_etag", last_modified="input_modified") assert result.articles == () assert result.not_modified is False - assert result.etag == "test_etag" - assert result.last_modified == "test_modified" + # Les en-têtes d'entrée sont conservés, pas ceux de la réponse + assert result.etag == "input_etag" + assert result.last_modified == "input_modified" # Vérification du log d'avertissement assert "Flux RSS du blog invalide" in caplog.text @@ -362,7 +436,7 @@ def test_fetch_and_parse_network_error( with caplog.at_level(logging.ERROR): mock_exception = ConnectionError("Network error") mocker.patch( - "pronote_sync.sources.blog.rss.feedparser.parse", + "pronote_sync.sources.blog.rss.requests.get", side_effect=mock_exception, ) @@ -390,9 +464,9 @@ def test_fetch_and_parse_no_secret_leak_in_logs( with caplog.at_level(logging.ERROR): # Exception contenant un secret secret = "SECRET_TOKEN_123" # pragma: allowlist secret - mock_exception = ValueError(f"Error with token: {secret}") + mock_exception = ConnectionError(f"Error with token: {secret}") mocker.patch( - "pronote_sync.sources.blog.rss.feedparser.parse", + "pronote_sync.sources.blog.rss.requests.get", side_effect=mock_exception, ) @@ -425,6 +499,19 @@ def test_fetch_and_parse_entry_without_date( pubdate_parsed=None, ) mock_feed = make_mock_feed(entries=[entry]) + + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=mock_feed, @@ -455,6 +542,19 @@ def test_fetch_and_parse_entry_without_guid( published_parsed=(2026, 8, 10, 9, 0, 11, 0, 222, 0), ) mock_feed = make_mock_feed(entries=[entry]) + + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=mock_feed, @@ -470,13 +570,27 @@ def test_fetch_and_parse_entry_without_guid( def test_fetch_and_parse_html_to_text_conversion( mocker: pytest_mock.MockerFixture, real_parsed_feed: feedparser.FeedParserDict, + blog_rss_fixture_path: Path, ) -> None: """Vérifie que content_text ne contient pas de balises HTML. :param mocker: Fixture pytest-mock pour le mocking. :param real_parsed_feed: Résultat réel du parsing du fixture. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=real_parsed_feed, @@ -505,11 +619,19 @@ def test_fetch_and_parse_etag_last_modified_returned( :param mocker: Fixture pytest-mock pour le mocking. :return: None """ - mock_feed = make_mock_feed( - entries=[], - etag="abc123", - modified="Wed, 01 Sep 2026 00:00:00 GMT", + # Mock requests.get pour retourner une réponse 200 avec des en-têtes + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {"ETag": "abc123", "Last-Modified": "Wed, 01 Sep 2026 00:00:00 GMT"} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, ) + + mock_feed = make_mock_feed(entries=[]) mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=mock_feed, @@ -523,15 +645,26 @@ def test_fetch_and_parse_etag_last_modified_returned( assert result.not_modified is False -def test_fetch_and_parse_passes_etag_to_feedparser( +def test_fetch_and_parse_passes_etag_to_requests( mocker: pytest_mock.MockerFixture, ) -> None: - """Vérifie que les paramètres etag et last_modified sont passés à feedparser.parse. + """Vérifie que les paramètres etag et last_modified sont passés comme en-têtes à requests.get. :param mocker: Fixture pytest-mock pour le mocking. :return: None """ - mock_parse = mocker.patch( + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mock_get = mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=make_mock_feed(), ) @@ -542,11 +675,11 @@ def test_fetch_and_parse_passes_etag_to_feedparser( last_modified="Wed, 01 Sep 2026 00:00:00 GMT", ) - # Vérification que feedparser.parse a été appelé avec les bons paramètres - mock_parse.assert_called_once() - call_args = mock_parse.call_args - assert call_args[1]["etag"] == "abc123" - assert call_args[1]["modified"] == "Wed, 01 Sep 2026 00:00:00 GMT" + # Vérification que requests.get a été appelé avec les bons en-têtes + mock_get.assert_called_once() + call_args = mock_get.call_args + assert call_args[1]["headers"]["If-None-Match"] == "abc123" + assert call_args[1]["headers"]["If-Modified-Since"] == "Wed, 01 Sep 2026 00:00:00 GMT" def test_parse_date_valid() -> None: @@ -613,6 +746,7 @@ def test_html_to_text_entities() -> None: def test_fetch_and_parse_sort_deterministic( mocker: pytest_mock.MockerFixture, real_parsed_feed: feedparser.FeedParserDict, + blog_rss_fixture_path: Path, ) -> None: """Vérifie que le tri des articles est déterministe. @@ -620,8 +754,21 @@ def test_fetch_and_parse_sort_deterministic( :param mocker: Fixture pytest-mock pour le mocking. :param real_parsed_feed: Résultat réel du parsing du fixture. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. :return: None """ + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=real_parsed_feed, @@ -647,42 +794,62 @@ def test_fetch_and_parse_sort_deterministic( def test_fetch_and_parse_duplicate_guid_in_feed( mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, ) -> None: """Vérifie la déduplication des entrées avec le même GUID dans un même flux. Deux entrées avec le même GUID (même id et même link) doivent donner un seul article. + Un avertissement doit être journalisé pour le duplicata. :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. :return: None """ - # Création de deux entrées avec le même GUID - entry1 = make_mock_entry( - entry_id="duplicate-guid", - link="https://example.com/duplicate", - title="First Article", - published_parsed=(2026, 8, 10, 9, 0, 0, 0, 222, 0), - ) - entry2 = make_mock_entry( - entry_id="duplicate-guid", - link="https://example.com/duplicate", - title="Second Article", - published_parsed=(2026, 8, 11, 10, 0, 0, 0, 223, 0), - ) + with caplog.at_level(logging.WARNING): + # Création de deux entrées avec le même GUID + entry1 = make_mock_entry( + entry_id="duplicate-guid", + link="https://example.com/duplicate", + title="First Article", + published_parsed=(2026, 8, 10, 9, 0, 0, 0, 222, 0), + ) + entry2 = make_mock_entry( + entry_id="duplicate-guid", + link="https://example.com/duplicate", + title="Second Article", + published_parsed=(2026, 8, 11, 10, 0, 0, 0, 223, 0), + ) - mock_feed = make_mock_feed(entries=[entry1, entry2]) - mocker.patch( - "pronote_sync.sources.blog.rss.feedparser.parse", - return_value=mock_feed, - ) + mock_feed = make_mock_feed(entries=[entry1, entry2]) - client = BlogRSSClient(rss_url="https://example.com/blog/feed") - result = client.fetch_and_parse(known_guids=None) + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() - # Doit retourner un seul article (le second duplicata est ignoré) - assert len(result.articles) == 1 - # Le premier article doit être conservé (celui avec la date la plus ancienne) - assert result.articles[0].id == "duplicate-guid" - assert result.articles[0].title == "First Article" + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=mock_feed, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse(known_guids=None) + + # Doit retourner un seul article (le second duplicata est ignoré) + assert len(result.articles) == 1 + # Le premier article doit être conservé (celui avec la date la plus ancienne) + assert result.articles[0].id == "duplicate-guid" + assert result.articles[0].title == "First Article" + + # Vérification qu'un avertissement a été journalisé pour le duplicata + assert "Entrée RSS en double dans le flux" in caplog.text def test_fetch_and_parse_sort_tied_dates_by_id( @@ -710,6 +877,19 @@ def test_fetch_and_parse_sort_tied_dates_by_id( ) mock_feed = make_mock_feed(entries=[entry_b, entry_a]) # Ordre initial : b, a + + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + mocker.patch( "pronote_sync.sources.blog.rss.feedparser.parse", return_value=mock_feed, @@ -722,3 +902,384 @@ def test_fetch_and_parse_sort_tied_dates_by_id( assert len(result.articles) == 2 assert result.articles[0].id == "guid-a" # a doit venir avant b assert result.articles[1].id == "guid-b" + + +# --- New tests for the updated implementation --- + + +def test_fetch_and_parse_real_fixture_no_mock( + blog_rss_fixture_path: Path, +) -> None: + """Vérifie que le parsing du fixture réel fonctionne sans mock de feedparser. + + Utilise un serveur HTTP mocké avec responses pour retourner le contenu + du fixture, et laisse feedparser.parse s'exécuter normalement. + + :param blog_rss_fixture_path: Chemin vers le fichier fixture. + :return: None + """ + fixture_content = blog_rss_fixture_path.read_bytes() + + with responses.RequestsMock() as rsps: + rsps.add( + responses.GET, + "https://example.com/blog/feed", + body=fixture_content, + status=200, + content_type="application/rss+xml", + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse() + + # Vérification du nombre d'articles + assert len(result.articles) == 3 + + # Vérification de l'ordre : p=1003 (12 Aug), p=1002 (11 Aug), p=1001 (10 Aug) + assert result.articles[0].id == "https://example.com/blog/?p=1003" + assert result.articles[1].id == "https://example.com/blog/?p=1002" + assert result.articles[2].id == "https://example.com/blog/?p=1001" + + +def test_fetch_and_parse_401_error( + mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, +) -> None: + """Vérifie la gestion d'une erreur 401 Unauthorized. + + Doit retourner un résultat vide et logger une erreur. + + :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. + :return: None + """ + with caplog.at_level(logging.ERROR): + # Mock requests.get pour retourner une réponse 401 + mock_response = Mock() + mock_response.status_code = 401 + mock_response.headers = {} + + def raise_for_status() -> None: + raise requests.HTTPError("401 Client Error: Unauthorized") + + mock_response.raise_for_status = raise_for_status + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse() + + assert result.articles == () + assert result.not_modified is False + assert "Échec de la récupération du flux RSS du blog" in caplog.text + + +def test_fetch_and_parse_404_error( + mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, +) -> None: + """Vérifie la gestion d'une erreur 404 Not Found. + + Doit retourner un résultat vide et logger une erreur. + + :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. + :return: None + """ + with caplog.at_level(logging.ERROR): + # Mock requests.get pour retourner une réponse 404 + mock_response = Mock() + mock_response.status_code = 404 + mock_response.headers = {} + + def raise_for_status() -> None: + raise requests.HTTPError("404 Client Error: Not Found") + + mock_response.raise_for_status = raise_for_status + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse() + + assert result.articles == () + assert result.not_modified is False + assert "Échec de la récupération du flux RSS du blog" in caplog.text + + +def test_fetch_and_parse_500_error( + mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, +) -> None: + """Vérifie la gestion d'une erreur 500 Internal Server Error. + + Doit retourner un résultat vide et logger une erreur. + + :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. + :return: None + """ + with caplog.at_level(logging.ERROR): + # Mock requests.get pour retourner une réponse 500 + mock_response = Mock() + mock_response.status_code = 500 + mock_response.headers = {} + + def raise_for_status() -> None: + raise requests.HTTPError("500 Server Error: Internal Server Error") + + mock_response.raise_for_status = raise_for_status + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse() + + assert result.articles == () + assert result.not_modified is False + assert "Échec de la récupération du flux RSS du blog" in caplog.text + + +def test_fetch_and_parse_preserves_etag_on_error( + mocker: pytest_mock.MockerFixture, +) -> None: + """Vérifie que les en-têtes d'entrée sont conservés en cas d'erreur. + + :param mocker: Fixture pytest-mock pour le mocking. + :return: None + """ + mock_exception = ConnectionError("Network error") + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + side_effect=mock_exception, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse( + etag="known-etag", + last_modified="known-date", + ) + + # Les en-têtes d'entrée doivent être conservés + assert result.etag == "known-etag" + assert result.last_modified == "known-date" + assert result.articles == () + assert result.not_modified is False + + +def test_fetch_and_parse_preserves_etag_on_bozo( + mocker: pytest_mock.MockerFixture, +) -> None: + """Vérifie que les en-têtes d'entrée sont conservés en cas de flux bozo. + + :param mocker: Fixture pytest-mock pour le mocking. + :return: None + """ + mock_feed = make_mock_feed(bozo=1) + + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {"ETag": "response_etag", "Last-Modified": "response_modified"} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=mock_feed, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse(etag="known-etag", last_modified="known-date") + + # Les en-têtes d'entrée doivent être conservés, pas ceux de la réponse + assert result.etag == "known-etag" + assert result.last_modified == "known-date" + assert result.articles == () + assert result.not_modified is False + + +def test_fetch_and_parse_conditional_headers_sent( + mocker: pytest_mock.MockerFixture, +) -> None: + """Vérifie que les en-têtes conditionnels sont envoyés avec la requête. + + :param mocker: Fixture pytest-mock pour le mocking. + :return: None + """ + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mock_get = mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=make_mock_feed(), + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + client.fetch_and_parse( + etag="etag123", + last_modified="Wed, 01 Sep 2026 GMT", + ) + + # Vérification que les en-têtes conditionnels ont été envoyés + mock_get.assert_called_once() + call_args = mock_get.call_args + headers = call_args[1]["headers"] + assert headers["If-None-Match"] == "etag123" + assert headers["If-Modified-Since"] == "Wed, 01 Sep 2026 GMT" + + +def test_fetch_and_parse_no_conditional_headers_when_none( + mocker: pytest_mock.MockerFixture, +) -> None: + """Vérifie qu'aucun en-tête conditionnel n'est envoyé lorsque etag/last_modified sont None. + + :param mocker: Fixture pytest-mock pour le mocking. + :return: None + """ + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mock_get = mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=make_mock_feed(), + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + client.fetch_and_parse(etag=None, last_modified=None) + + # Vérification que les en-têtes conditionnels ne sont pas présents + mock_get.assert_called_once() + call_args = mock_get.call_args + headers = call_args[1]["headers"] + assert "If-None-Match" not in headers + assert "If-Modified-Since" not in headers + + +def test_fetch_and_parse_known_guids_no_warning( + mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, + blog_rss_fixture_path: Path, +) -> None: + """Vérifie qu'aucun avertissement n'est journalisé pour les GUID connus. + + Les GUID connus sont ignorés silencieusement, sans log. + + :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. + :param blog_rss_fixture_path: Chemin vers le fichier fixture. + :return: None + """ + with caplog.at_level(logging.WARNING): + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = blog_rss_fixture_path.read_bytes() + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + # Utiliser le vrai parsing du fixture + real_parsed_feed = feedparser.parse(str(blog_rss_fixture_path)) + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=real_parsed_feed, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + # Passer un GUID connu du fixture + result = client.fetch_and_parse(known_guids=frozenset({"https://example.com/blog/?p=1001"})) + + # Doit retourner 2 articles (1001 est exclu) + assert len(result.articles) == 2 + + # Aucun avertissement ne doit être journalisé pour le GUID connu + assert "Entrée RSS en double dans le flux" not in caplog.text + + +def test_fetch_and_parse_intra_feed_duplicate_warning( + mocker: pytest_mock.MockerFixture, + caplog: pytest.LogCaptureFixture, +) -> None: + """Vérifie qu'un avertissement est journalisé pour les doublons dans le même flux. + + :param mocker: Fixture pytest-mock pour le mocking. + :param caplog: Fixture pour capturer les logs. + :return: None + """ + with caplog.at_level(logging.WARNING): + # Création de deux entrées avec le même GUID + entry1 = make_mock_entry( + entry_id="duplicate-guid", + link="https://example.com/duplicate", + title="First Article", + published_parsed=(2026, 8, 10, 9, 0, 0, 0, 222, 0), + ) + entry2 = make_mock_entry( + entry_id="duplicate-guid", + link="https://example.com/duplicate", + title="Second Article", + published_parsed=(2026, 8, 11, 10, 0, 0, 0, 223, 0), + ) + + mock_feed = make_mock_feed(entries=[entry1, entry2]) + + # Mock requests.get pour retourner une réponse 200 + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.headers = {} + mock_response.raise_for_status = Mock() + + mocker.patch( + "pronote_sync.sources.blog.rss.requests.get", + return_value=mock_response, + ) + + mocker.patch( + "pronote_sync.sources.blog.rss.feedparser.parse", + return_value=mock_feed, + ) + + client = BlogRSSClient(rss_url="https://example.com/blog/feed") + result = client.fetch_and_parse(known_guids=None) + + # Doit retourner un seul article + assert len(result.articles) == 1 + + # Un avertissement doit être journalisé pour le doublon + assert "Entrée RSS en double dans le flux" in caplog.text diff --git a/tests/unit/test_blog_state.py b/tests/unit/test_blog_state.py index cdb5f3c..2c3a3e7 100644 --- a/tests/unit/test_blog_state.py +++ b/tests/unit/test_blog_state.py @@ -15,6 +15,7 @@ from __future__ import annotations import json from pathlib import Path +from unittest.mock import patch import pytest @@ -308,4 +309,42 @@ def test_get_known_guids_returns_frozenset(tmp_path: Path) -> None: assert type(result) is frozenset +def test_atomic_save_preserves_on_error(tmp_path: Path) -> None: + """Vérifie que l'état original est préservé en cas d'erreur lors de la sauvegarde atomique. + + Si une erreur survient pendant le remplacement atomique du fichier, + le fichier original doit rester intact et le fichier temporaire doit être nettoyé. + + :param tmp_path: Fixture pytest pour un répertoire temporaire. + :return: None + """ + state_file = tmp_path / "state.json" + + # Créer un état initial avec des GUID + state = BlogRSSState(state_file) + state.add_guids(["original-guid-1", "original-guid-2"]) + + # Lire le contenu original + original_content = state_file.read_text(encoding="utf-8") + + # Mock Path.replace pour simuler une erreur pendant le remplacement atomique + with patch.object(Path, "replace") as mock_replace: + mock_replace.side_effect = OSError("Simulated atomic replace failure") + + # Essayer d'ajouter de nouveaux GUID, ce qui déclenchera _save() + state.add_guids(["new-guid"]) + + # Vérifier que le fichier original est toujours intact + assert state_file.read_text(encoding="utf-8") == original_content + + # Vérifier que le fichier temporaire a été nettoyé + tmp_file = state_file.with_suffix(".tmp") + assert not tmp_file.exists() + + # Vérifier que l'état en mémoire n'a pas été modifié (car la sauvegarde a échoué) + # Note: En réalité, l'état en mémoire est modifié mais pas persistant + # C'est le fichier qui doit rester intact + assert state.get_known_guids() == frozenset({"original-guid-1", "original-guid-2", "new-guid"}) + + # Ensure trailing newline