diff --git a/docs/deweloper/p4-error-record-unifikacja-spec.md b/docs/deweloper/p4-error-record-unifikacja-spec.md new file mode 100644 index 000000000..eba075651 --- /dev/null +++ b/docs/deweloper/p4-error-record-unifikacja-spec.md @@ -0,0 +1,236 @@ +# P4 — Unifikacja parsowania błędów PBN przez `ErrorRecord` + +**Status:** Stage 1 (reader-first). Writery NIE zmieniane. +**Gałąź:** `feat/pbn-p4-reader` (stack na `feat/pbn-shims` / #609). +**Zakres wybrany przez użytkownika:** pełna **unifikacja** (nie minimalny wariant). + +--- + +## 1. Problem + +Błędy wysyłki do PBN są zapisywane w bazie jako **surowe stringi** w dwóch +polach: + +- `pbn_api.SentData.exception` (`TextField`, max 65535) — zapis przez + `str(exception)` w `SentData.objects.mark_as_failed()` oraz + `publication_sync.py` (`exception=str(e)`). +- `pbn_export_queue.PBN_Export_Queue.komunikat` (`TextField`) — zapis przez + `traceback.format_exc()` w `_handle_pbn_exception()`. + +Odczyt (display) jest **rozproszony w 4 funkcjach**, każda z własną, kruchą +logiką parsowania tego samego stringa: + +| Funkcja | Plik | Zwraca | Parser | +|---|---|---|---| +| `format_pbn_error(value, rodzaj_bledu)` | `pbn_export_queue/templatetags/pbn_queue_extras.py` | HTML (`mark_safe`) | regex na krotkę + `json.loads` | +| `parse_pbn_api_error(text)` | `pbn_export_queue/views/utils.py` | `dict` | `ast.literal_eval` + `json.loads` | +| `parse_error_details(sent_data)` | `pbn_export_queue/views/utils.py` | `dict` | `ast.literal_eval` + `json.loads` | +| `extract_pbn_error_from_komunikat(komunikat)` | `pbn_export_queue/views/utils.py` | `str \| None` | skan linii traceback | +| `exception_details(obj)` (admin) | `pbn_api/admin/sentdata.py` | `str` | hack `split('"details":')[1][:-3]` | + +Każdy hack pęka przy innym kształcie danych; admin-owy `split(...)[1][:-3]` +jest szczególnie kruchy. + +## 2. Formaty legacy (korpus wejściowy) + +Empirycznie potwierdzone kształty stringów w bazie: + +1. **Goła krotka** (z `SentData.exception`, bo `str(HttpException)` = + `str(self.args)` = tuple-repr): + `(400, '/api/v1/publications', '{"message":"...","details":{...}}')` +2. **Linia z prefiksem** (ostatnia linia tracebacku w `komunikat`; Python + formatuje wyjątek jako `moduł.Klasa: str(wyjątek)`): + `pbn_api.exceptions.HttpException: (400, '/url', '{...}')` + lub `pbn_client.exceptions.PBNValidationError: (400, '/url', '{...}')` + (obie ścieżki importu muszą być rozpoznawane). +3. **Pełny traceback** (`komunikat`): wieloliniowy, kończy się linią (2). +4. **Payload JSON = lista**: `(400, '/url', '[{...},{...}]')`. +5. **Payload zdegenerowany**: pusta lista `[]`, string `"..."`, liczba `42`. +6. **Prosty wyjątek bez krotki**: `pbn_api.exceptions.StatementsMissing: msg`. +7. **Nie-PBN plaintext**: `Some random error message`. +8. **Puste**: `None`, `""`. +9. **Payload z HTML/XSS** w `message`/`description`/`details` (musi być + escapowany w HTML-owej ścieżce). + +## 3. Nowy format v1 (wire format — pisany dopiero w Stage 2) + +Jednoliniowy JSON, sanity-markers `v` i `kind` **oba wymagane** do rozpoznania: + +```json +{"v": 1, "kind": "http", "source": "sentdata", + "exception_class": "pbn_client.exceptions.PBNValidationError", + "status_code": 400, "url": "/api/v1/publications", + "content": "{...raw body...}", "message": "...", "traceback": "...", + "truncated": false} +``` + +- `kind` ∈ {`http`, `generic`}. +- Limity rozmiarów (mieszczą się w 65535 pola `exception`): + `content` 10k, `traceback` 20k (trzymany od końca), `message` 2k, + `url` 512, cały blob ≤ 60k. Przekroczenie → `truncated=True`. +- **W Stage 1 nikt nie pisze v1.** `serialize()` istnieje i jest + przetestowany, ale nie jest podpięty do writerów. Readery MUSZĄ już + rozumieć v1 (reader-first) — inaczej deploy-race Stage 2 wywali stary + proces na nowym blobie. + +## 4. `ErrorRecord` + `parse()` + +**Dom modułu: pakiet `pbn-client`** (`pbn_client.error_record`, wydany jako +`pbn-client` 0.2.1). To czysta, framework-niezależna wiedza o protokole PBN — +należy do pakietu, nie do monolitu. BPP importuje `from pbn_client.error_record +import parse, serialize, ErrorRecord`; w BPP zostają wyłącznie Django-owe +adaptery display (§5). Do czasu publikacji 0.2.1 na PyPI, BPP pinuje pakiet +przez `[tool.uv.sources]` (git rev) — do usunięcia po release. + +### 4.1. `ErrorRecord` (frozen dataclass) + +Pola wystarczające do odtworzenia outputu KAŻDEGO renderera: + +| Pole | Typ | Znaczenie | +|---|---|---| +| `kind` | `str` | `"http"` \| `"generic"` | +| `source` | `str \| None` | `"sentdata"` \| `"queue"` \| `None` | +| `exception_class` | `str \| None` | pełna nazwa (`pbn_api.exceptions.HttpException`) | +| `exception_type` | `str \| None` | krótka nazwa (`HttpException`) — ostatni segment | +| `status_code` | `int \| None` | kod HTTP | +| `url` | `str \| None` | endpoint | +| `content` | `str \| None` | surowy body odpowiedzi (string JSON lub inny) | +| `content_json` | `dict \| list \| str \| int \| None` | sparsowany `content` (None gdy niepoprawny JSON) | +| `message` | `str \| None` | komunikat wyjątku / opis | +| `traceback` | `str \| None` | pełny traceback (gdy był) | +| `raw` | `str` | oryginalny wejściowy string (fallback) | +| `is_pbn_api_error` | `bool` | czy rozpoznano strukturę PBN (krotka lub prefiks PBN) | +| `wire` | `str` | proweniencja: `"v1"` \| `"legacy"` \| `"empty"` | +| `content_json_valid` | `bool` | czy `content` był poprawnym JSON-em (odróżnia `null` od błędu) | +| `truncated` | `bool` | czy przycięto przy serializacji | + +`wire` jest kluczowe dla reader-first: adaptery renderują blob v1 WPROST ze +strukturalnych pól (bez `exception_line`/krotki), inaczej pokazałyby surowy +JSON. `content_json_valid` odróżnia poprawny JSON `null` (`content_json is +None`, `valid=True`) od niepoprawnego body (`valid=False`). + +### 4.2. `parse(stored: str | None) -> ErrorRecord` + +**Gwarancja: NIGDY nie rzuca.** Drabina rozpoznania (pierwszy match wygrywa): + +1. `None`/blank → `ErrorRecord(kind="generic", raw="", is_pbn_api_error=False)`. +2. **v1 JSON**: `json.loads` daje `dict` z `v == 1` (int) i `kind` ∈ {http, + generic} → zbuduj z pól. (Oba markery wymagane — chroni przed kolizją z + legacy payloadem który przypadkiem jest dict-em.) +3. **Traceback**: wieloliniowy string zawierający `Traceback (most recent` + → wyłuskaj ostatnią linię z `pbn_api.exceptions`/`pbn_client.exceptions`; + zapamiętaj `traceback=stored`; parsuj tę linię dalej jak (4)/(5). +4. **Linia z prefiksem PBN**: `moduł.Klasa: ` gdzie moduł ∈ + {pbn_api.exceptions, pbn_client.exceptions} → `exception_class`, + `exception_type`; `` parsuj jak (5). +5. **Krotka**: `(code, 'url', 'json_str')` przez `ast.literal_eval` + (≥3 elementy) → `status_code`, `url`, `content`; `content_json = + json.loads(content)` (None gdy błąd). `is_pbn_api_error=True`, + `kind="http"`. +6. **Prosty wyjątek**: prefiks PBN + brak krotki → `message = `, + `kind="generic"`, `is_pbn_api_error=True`. +7. **Plaintext fallback**: cokolwiek innego → `raw=stored`, `message=stored`, + `is_pbn_api_error=False`, `kind="generic"`. + +**DoS guard**: część-message dłuższa niż 512 znaków w ścieżce krotki → +zachowanie jak w obecnym `parse_pbn_api_error` (flaga + skrócony komunikat). + +### 4.3. `serialize(rec: ErrorRecord) -> str` + +v1 JSON, jednoliniowy, z limitami rozmiaru z §3. Round-trip: +`parse(serialize(rec))` zwraca rekord równoważny na polach v1. **Nie podpięty +do writerów w Stage 1.** + +## 5. Renderery (podpięte pod `ErrorRecord`) + +Każda z 4 funkcji display zostaje **przepisana jako cienki renderer nad +`parse()`**, zachowując **identyczny podpis i kontrakt outputu** (pinowane +testami charakteryzacyjnymi z §6): + +- `format_pbn_error(value, rodzaj_bledu=None)` → `parse(value)` → + `_render_html(rec, rodzaj_bledu)`. Ta sama logika MERYT/TECH (ukrywanie + nagłówka), te same klasy CSS, **KAŻDA** dynamiczna wartość przez + `escape()` (stored-XSS). Fallback: `
…
`. +- `parse_pbn_api_error(text)` → `parse(text)` → `_render_dict(rec)` z tymi + samymi kluczami (`is_pbn_api_error`, `error_code`, `error_endpoint`, + `error_message`, `error_description`, `error_details_json`, + `exception_type`, `raw_error`). +- `parse_error_details(sent_data)` → `parse(sent_data.exception)` → + `error_code`/`error_endpoint`/`error_details` + fallback na + `api_response_status`. +- `extract_pbn_error_from_komunikat(komunikat)` → `parse(komunikat)`; + zwraca zrekonstruowaną „linię wyjątku" (`exception_class: content-repr`) + lub `None` gdy brak — kontrakt jak dziś (używana potem jako wejście do + `parse_pbn_api_error`, więc round-trip musi się zgadzać). +- `exception_details(obj)` (admin) → `parse(obj.exception)` → czytelny + opis `details` z `content_json` (koniec hacka `split`). To jest + **zamierzone ulepszenie** (brak testów pinujących stary hack; nowy output + jest nadzbiorem informacyjnym). Udokumentowane jako świadoma zmiana. + +## 6. Testy (TDD, byte-identyczność) + +### 6.1. Testy charakteryzacyjne (siatka bezpieczeństwa) +Przed refaktorem: `test_error_record_golden.py` zdejmuje **aktualny** output +wszystkich funkcji na pełnym korpusie z §2 i asertuje **dokładną** równość. +Po refaktorze te same asercje muszą przejść → dowód byte-identyczności dla +legacy (poza świadomie zmienionym adminem, §5). + +### 6.2. Testy jednostkowe `ErrorRecord`/`parse`/`serialize` +- `parse()` nigdy nie rzuca (fuzz: losowe/wrogie/binarne stringi — wszystkie + dają `ErrorRecord`, żaden wyjątek). +- Rozpoznanie każdej gałęzi drabiny (§4.2) osobno. +- Brak kolizji legacy↔v1: legacy dict-payload BEZ `v==1` NIE łapie się jako + v1; string `{"v":1,...}` w treści legacy nie myli parsera. +- `serialize()` respektuje limity; round-trip `parse(serialize(x))`. + +### 6.3. Istniejące testy (muszą zostać zielone bez zmian) +`test_template_filters.py`, `test_utils.py` — obecny kontrakt. Nie ruszamy +asercji; służą jako dodatkowe piny. + +## 7. Rollout (przypomnienie) + +- **Stage 1 (ten PR):** moduł + readery rozumieją legacy ORAZ v1. Writery + bez zmian. `serialize()` istnieje, nie podpięty. +- **Stage 2 (osobny PR, DOPIERO po wdrożeniu Stage 1 na całej flocie + + restarcie web+celery):** writery → `serialize()`. +- **Stage 3 (później):** usunięcie parsowania legacy + backfill. + +## 8. Poza zakresem Stage 1 + +- Zmiana writerów / migracja danych / backfill. +- Zmiana schematu bazy (pole `exception`/`komunikat` bez zmian). +- Zmiana szablonów poza tym, co wynika z identycznego HTML z `format_pbn_error`. + +## 9. Rozliczenie recenzji adwersaryjnych (2× Fable) + +Dwie niezależne recenzje Fable (poprawność/byte-identyczność + bezpieczeństwo). +Findingi i ich rozwiązania: + +**Naprawione przed mergem (krytyczne):** +- **Reader-first zepsuty dla v1** (parse_pbn_api_error klasyfikował blob v1 + >512 jako „nie-PBN", format_pbn_error pokazywał surowy JSON): dodano jawną + gałąź `wire == "v1"` w obu adapterach + guard >512 stosowany TYLKO do + legacy. Pinowane `test_error_record_v1_reader.py`. +- **Totalność `parse()`**: `_try_json`/`_try_v1` łapią teraz też + `RecursionError`/`MemoryError` (głęboki JSON z wrogiego body), a + `_try_tuple` — `OverflowError` (`int(1e999)`). Pinowane w testach + jednostkowych pakietu. +- **Konflacja „niepoprawny JSON" ↔ „JSON `null`"**: pole `content_json_valid` + odróżnia oba; `_render_pbn_dict`/`parse_error_details` renderują `null` + jak legacy („Nieoczekiwany typ odpowiedzi PBN API"). +- **serialize() bez limitu na `exception_class`/`source`**: dodano capy → + blob dowodliwie < 65535. + +**Zaakceptowane jako świadome, drobne zmiany zachowania** (nietypowe wejścia, +neutralne-lub-lepsze, poza golden): +- prefiks + puste body `(400,'/x','')`: nowe `HttpException: HTTP 400 - ` + zamiast surowej krotki (regex legacy wymagał niepustego body). +- linia PBN bez `:` (wyjątek bez argumentów): `exception_type` = realna klasa + (nie domyślne „HttpException"), `error_message` = pusty (downstream i tak + ratuje się `raw_error`). +- body z escapowanym apostrofem/cudzysłowem: renderowane strukturalnie + (legacy gubił je w zepsutym unescape). + +**Czyste (bez findingów):** stored-XSS (wszystkie ścieżki `escape()`), +kolizja legacy↔v1 (markery `v==1`+`kind` szczelne), rozdział reader/writer +(`serialize()` niepodpięty), byte-identyczność realistycznego legacy. diff --git a/pyproject.toml b/pyproject.toml index ffc1fc29a..cf94f189d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,7 @@ dependencies = [ "Django>=5.2.16,<5.3", # Reużywalne pakiety PBN wydzielone na PyPI (github.com/iplweb/*). "django-pbn-client>=0.2,<0.3", - "pbn-client>=0.2,<0.3", + "pbn-client>=0.2.1,<0.3", "django-polish-inflection>=0.1,<0.2", "django-axes>=7.0,<9", "arrow>=1.3,<2", @@ -314,6 +314,13 @@ default = true # WYCOFAĆ gdy upstream wyda release z #2156 → wróć na PyPI `>=4.5`. Tracking: #280. [tool.uv.sources] django-import-export = { git = "https://github.com/mpasternak/django-import-export.git", rev = "d6ee0d39194fee31437affbdc6fee5ce549b4b8f" } +# TYMCZASOWO: pbn-client 0.2.1 (ErrorRecord) nie jest jeszcze na PyPI. Pin do +# commita gałęzi feat/error-record, żeby CI/lokalnie rozwiązać zależność. +# USUŃ po wydaniu pbn-client 0.2.1 na PyPI (wtedy sam pin >=0.2.1,<0.3 wystarczy). +pbn-client = { git = "https://github.com/iplweb/pbn-client.git", rev = "84503cf41b308bc86edc6f8709b71383a8676cac" } +# TYMCZASOWO: django-pbn-client 0.2.1 (sync_dictionary, D3) nie jest jeszcze na +# PyPI. Pin do commita gałęzi feat/sync-dictionary. USUŃ po wydaniu 0.2.1. +django-pbn-client = { git = "https://github.com/iplweb/django-pbn-client.git", rev = "9b2164f889861db9292b91508debf0732211bb39" } # Konfiguracja `pytest-testcontainers-django` — pluginu pytest, ktory # startuje kontenery PG/Redis przed importem Django settings i wstrzykuje diff --git a/src/bpp/migrations/0475_merge_20260724_1726.py b/src/bpp/migrations/0475_merge_20260724_1726.py new file mode 100644 index 000000000..5b5b5273e --- /dev/null +++ b/src/bpp/migrations/0475_merge_20260724_1726.py @@ -0,0 +1,13 @@ +# Generated by Django 5.2.16 on 2026-07-24 15:26 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("bpp", "0474_constraint_autor_jednostka_okresy_bez_nakladan"), + ("bpp", "0474_merge_20260724_1635"), + ] + + operations = [] diff --git a/src/bpp/newsfragments/pbn-d3-sync-dictionary.bugfix.rst b/src/bpp/newsfragments/pbn-d3-sync-dictionary.bugfix.rst new file mode 100644 index 000000000..e1ba00cb6 --- /dev/null +++ b/src/bpp/newsfragments/pbn-d3-sync-dictionary.bugfix.rst @@ -0,0 +1,5 @@ +Synchronizacja słownika dyscyplin z PBN nie trzyma już otwartej transakcji +bazodanowej przez cały czas pobierania danych z PBN. Pobranie (remote) wykonuje +się teraz przed otwarciem transakcji (wzorzec ``sync_dictionary`` z pakietu +``django-pbn-client``), a zapis leci atomowo — dłuższa niedostępność PBN nie +blokuje już połączenia bazodanowego. diff --git a/src/bpp/newsfragments/pbn-error-record.feature.rst b/src/bpp/newsfragments/pbn-error-record.feature.rst new file mode 100644 index 000000000..5cc8a37d6 --- /dev/null +++ b/src/bpp/newsfragments/pbn-error-record.feature.rst @@ -0,0 +1,7 @@ +Ujednolicono parsowanie błędów wysyłki do PBN. Czysty, wersjonowany kontrakt +błędów (``ErrorRecord`` + ``parse``/``serialize``) trafił do pakietu +``pbn-client`` (0.2.1), a wszystkie miejsca wyświetlania w BPP (kolejka +eksportu, widoki detalu, panel admina ``SentData``) korzystają teraz z niego +jako cienkie adaptery — znika kilka kruchych, rozjeżdżających się parserów. +Poprawia to m.in. błąd wyświetlania błędów o payloadzie liczbowym oraz kruchy +skrót w panelu admina; readery rozumieją już nowy format v1 (reader-first). diff --git a/src/pbn_api/admin/sentdata.py b/src/pbn_api/admin/sentdata.py index dfbc48c09..1d6a4caeb 100644 --- a/src/pbn_api/admin/sentdata.py +++ b/src/pbn_api/admin/sentdata.py @@ -1,16 +1,12 @@ -import logging - from django.contrib import admin +from pbn_client.error_record import parse from bpp.admin.helpers.pbn_api.gui import sprobuj_wyslac_do_pbn_gui from bpp.admin.helpers.site_filtered import SiteFilteredAdminMixin -from bpp.util import zaloguj_polkniety_wyjatek from pbn_api.admin.base import BasePBNAPIAdminNoReadonly from pbn_api.admin.widgets import JSONWithActionsWidget from pbn_api.models import SentData -logger = logging.getLogger(__name__) - @admin.register(SentData) class SentDataAdmin(SiteFilteredAdminMixin, BasePBNAPIAdminNoReadonly): @@ -89,17 +85,22 @@ def formfield_for_dbfield(self, db_field, request, **kwargs): return super().formfield_for_dbfield(db_field, request, **kwargs) def exception_details(self, obj): - if obj.exception: - try: - return obj.exception.split('"details":')[1][:-3] - except Exception: - zaloguj_polkniety_wyjatek( - f"Nie udało się wyłuskać 'details' z pola exception " - f"obiektu SentData pk={obj.pk} — pokazuję surowy tekst", - logger=logger, - do_rollbar=True, - ) - return obj.exception + """Czytelny opis błędu z ``SentData.exception``. + + Zunifikowane parsowanie przez ``pbn_client.error_record.parse`` zastąpiło + kruchy hack ``obj.exception.split('"details":')[1][:-3]`` (zostawiał + m.in. wiszące ``}}`` na tracebackach i milkł na innych kształtach). + Pokazujemy skondensowane komunikaty walidacyjne PBN, a w razie ich + braku — komunikat wyjątku lub surowy tekst. + """ + if not obj.exception: + return None + rec = parse(obj.exception) + if rec.messages: + return "; ".join(rec.messages) + if rec.message: + return rec.message + return obj.exception exception_details.short_description = "Opis problemu" exception_details.admin_order_field = "exception" diff --git a/src/pbn_api/client/disciplines.py b/src/pbn_api/client/disciplines.py index e19ba3462..237570acd 100644 --- a/src/pbn_api/client/disciplines.py +++ b/src/pbn_api/client/disciplines.py @@ -1,6 +1,13 @@ -"""Disciplines synchronization mixin for PBN API client.""" +"""Disciplines synchronization mixin for PBN API client. + +Pobranie słownika z PBN korzysta z ``django_pbn_client.sync_dictionary`` +(materialize-before-atomic): remote-fetch wykonuje się PRZED transakcją, a +upsert do lokalnych modeli — w świeżym bloku atomic. Wcześniej +``@transaction.atomic`` obejmował cały remote-call. +""" from django.db import IntegrityError, transaction +from django_pbn_client import sync_dictionary from import_common.core import ( matchuj_aktualna_dyscypline_pbn, @@ -40,11 +47,21 @@ def _update_or_create_odporne_na_wyscig(manager, defaults, **lookup): class DisciplinesMixin: """Mixin providing discipline synchronization methods.""" - @transaction.atomic def download_disciplines(self): - """Zapisuje słownik dyscyplin z API PBN do lokalnej bazy""" + """Pobierz słownik dyscyplin z API PBN i zapisz do lokalnej bazy. + + Remote-fetch (``get_disciplines``) wykonuje się poza transakcją; sam + zapis leci atomowo (patrz ``sync_dictionary``). + """ + sync_dictionary(self.get_disciplines, self._upsert_disciplines) + + def _upsert_disciplines(self, elems): + """Upsert pobranego słownika do ``DisciplineGroup``/``Discipline``. - for elem in self.get_disciplines(): + Wołane WEWNĄTRZ transakcji otwartej przez ``sync_dictionary`` — bez + własnego ``@transaction.atomic``. + """ + for elem in elems: validityDateFrom = elem.get("validityDateFrom", None) validityDateTo = elem.get("validityDateTo", None) uuid = elem["uuid"] @@ -71,9 +88,25 @@ def download_disciplines(self): ), ) - @transaction.atomic def sync_disciplines(self): + """Pobierz słownik i zsynchronizuj tłumaczenia dyscyplin BPP. + + Remote-fetch (``download_disciplines``) jest transakcyjnie bezpieczny; + dopasowanie do modeli BPP leci w OSOBNEJ transakcji + (``_sync_discipline_translations``). Remote-call NIE jest już + obejmowany transakcją (wcześniejszy ``@transaction.atomic`` na całej + metodzie trzymał ją otwartą przez czas pobierania z PBN). + """ self.download_disciplines() + self._sync_discipline_translations() + + @transaction.atomic + def _sync_discipline_translations(self): + """Dopasuj aktualny słownik PBN do modeli BPP. + + Matching (``Dyscyplina_Naukowa``/``TlumaczDyscyplin``) jest BPP-specific + i celowo pozostaje w BPP (nie w pakiecie). + """ try: cur_dg = DisciplineGroup.objects.get_current() except DisciplineGroup.DoesNotExist as e: @@ -104,9 +137,7 @@ def sync_disciplines(self): wpis_tlumacza.save() for discipline in cur_dg.discipline_set.all(): - if discipline.name == "weterynaria": - pass - # Każda dyscyplina z aktualnego słownika powinna być wpisana do systemu BPP + # Każda dyscyplina z aktualnego słownika powinna być wpisana do BPP try: TlumaczDyscyplin.objects.get(pbn_2024_now=discipline) except TlumaczDyscyplin.DoesNotExist: diff --git a/src/pbn_api/tests/data/error_record_golden.json b/src/pbn_api/tests/data/error_record_golden.json new file mode 100644 index 000000000..e5e86e6ca --- /dev/null +++ b/src/pbn_api/tests/data/error_record_golden.json @@ -0,0 +1,237 @@ +{ + "extract_from_komunikat": { + "bare_tuple": null, + "empty": null, + "empty_list": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '[]')", + "list_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '[{\"error\": \"First error\"}, {\"error\": \"Second error\"}]')", + "none": null, + "number_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '42')", + "plaintext": null, + "prefix_http": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"message\":\"Bad Request\",\"description\":\"Validation failed.\",\"details\":{\"isbn\":\"Publikacja o identycznym ISBN już istnieje!\"}}')", + "prefix_validation": "pbn_client.exceptions.PBNValidationError: (400, '/api/v1/publications', '{\"details\":{\"doi\":\"Duplicate\"}}')", + "simple_exc": "pbn_api.exceptions.StatementsMissing: czegoś brakuje", + "string_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '\"Just a string\"')", + "traceback": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"message\":\"Bad Request\",\"description\":\"Validation failed.\",\"details\":{\"isbn\":\"Publikacja o identycznym ISBN już istnieje!\"}}')", + "tuple_dict": null, + "xss_fallback": null, + "xss_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"description\":\"\",\"details\":{\"isbn\":\"\"}}')" + }, + "format_meryt": { + "bare_tuple": "
(400, "/api/v1/publications", '{"message": "Error", "code": 400}')
", + "empty": "", + "empty_list": "
Endpoint: /api/v1/publications
", + "list_payload": "
Endpoint: /api/v1/publications
", + "none": "", + "plaintext": "
Some random error message
", + "prefix_http": "
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "prefix_validation": "
Szczegóły:
\n
• doi: Duplicate
\n
Endpoint: /api/v1/publications
", + "simple_exc": "
StatementsMissing: czegoś brakuje
", + "string_payload": "
Endpoint: /api/v1/publications
", + "traceback": "
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "tuple_dict": "
(400, '/api/v1/publications', '{"code":400,"message":"Bad Request","description":"Validation failed.","details":{"isbn":"Publikacja o identycznym ISBN już istnieje!"}}')
", + "xss_fallback": "
<script>alert('xss')</script>
", + "xss_payload": "
Szczegóły:
\n
• isbn: <img src=x onerror=alert(2)>
\n
Endpoint: /api/v1/publications
" + }, + "format_none": { + "bare_tuple": "
(400, "/api/v1/publications", '{"message": "Error", "code": 400}')
", + "empty": "", + "empty_list": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "list_payload": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "none": "", + "plaintext": "
Some random error message
", + "prefix_http": "
HttpException: HTTP 400
\n
Wiadomość: Bad Request
\n
Opis: Validation failed.
\n
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "prefix_validation": "
PBNValidationError: HTTP 400
\n
Szczegóły:
\n
• doi: Duplicate
\n
Endpoint: /api/v1/publications
", + "simple_exc": "
StatementsMissing: czegoś brakuje
", + "string_payload": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "traceback": "
HttpException: HTTP 400
\n
Wiadomość: Bad Request
\n
Opis: Validation failed.
\n
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "tuple_dict": "
(400, '/api/v1/publications', '{"code":400,"message":"Bad Request","description":"Validation failed.","details":{"isbn":"Publikacja o identycznym ISBN już istnieje!"}}')
", + "xss_fallback": "
<script>alert('xss')</script>
", + "xss_payload": "
HttpException: HTTP 400
\n
Opis: <script>alert(1)</script>
\n
Szczegóły:
\n
• isbn: <img src=x onerror=alert(2)>
\n
Endpoint: /api/v1/publications
" + }, + "format_tech": { + "bare_tuple": "
(400, "/api/v1/publications", '{"message": "Error", "code": 400}')
", + "empty": "", + "empty_list": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "list_payload": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "none": "", + "plaintext": "
Some random error message
", + "prefix_http": "
HttpException: HTTP 400
\n
Wiadomość: Bad Request
\n
Opis: Validation failed.
\n
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "prefix_validation": "
PBNValidationError: HTTP 400
\n
Szczegóły:
\n
• doi: Duplicate
\n
Endpoint: /api/v1/publications
", + "simple_exc": "
StatementsMissing: czegoś brakuje
", + "string_payload": "
HttpException: HTTP 400
\n
Endpoint: /api/v1/publications
", + "traceback": "
HttpException: HTTP 400
\n
Wiadomość: Bad Request
\n
Opis: Validation failed.
\n
Szczegóły:
\n
• isbn: Publikacja o identycznym ISBN już istnieje!
\n
Endpoint: /api/v1/publications
", + "tuple_dict": "
(400, '/api/v1/publications', '{"code":400,"message":"Bad Request","description":"Validation failed.","details":{"isbn":"Publikacja o identycznym ISBN już istnieje!"}}')
", + "xss_fallback": "
<script>alert('xss')</script>
", + "xss_payload": "
HttpException: HTTP 400
\n
Opis: <script>alert(1)</script>
\n
Szczegóły:
\n
• isbn: <img src=x onerror=alert(2)>
\n
Endpoint: /api/v1/publications
" + }, + "inputs": { + "bare_tuple": "(400, \"/api/v1/publications\", '{\"message\": \"Error\", \"code\": 400}')", + "empty": "", + "empty_list": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '[]')", + "list_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '[{\"error\": \"First error\"}, {\"error\": \"Second error\"}]')", + "none": null, + "number_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '42')", + "plaintext": "Some random error message", + "prefix_http": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"message\":\"Bad Request\",\"description\":\"Validation failed.\",\"details\":{\"isbn\":\"Publikacja o identycznym ISBN już istnieje!\"}}')", + "prefix_validation": "pbn_client.exceptions.PBNValidationError: (400, '/api/v1/publications', '{\"details\":{\"doi\":\"Duplicate\"}}')", + "simple_exc": "pbn_api.exceptions.StatementsMissing: czegoś brakuje", + "string_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '\"Just a string\"')", + "traceback": "Traceback (most recent call last):\n File \"/app/src/pbn_export_queue/models.py\", line 358, in send_to_pbn\npbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"message\":\"Bad Request\",\"description\":\"Validation failed.\",\"details\":{\"isbn\":\"Publikacja o identycznym ISBN już istnieje!\"}}')\n", + "tuple_dict": "(400, '/api/v1/publications', '{\"code\":400,\"message\":\"Bad Request\",\"description\":\"Validation failed.\",\"details\":{\"isbn\":\"Publikacja o identycznym ISBN już istnieje!\"}}')", + "xss_fallback": "", + "xss_payload": "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '{\"code\":400,\"description\":\"\",\"details\":{\"isbn\":\"\"}}')" + }, + "parse_error_details": { + "bare_tuple": { + "error_code": 400, + "error_details": "{\n \"message\": \"Error\",\n \"code\": 400\n}", + "error_endpoint": "/api/v1/publications" + }, + "empty": { + "error_code": "Brak kodu błędu", + "error_details": "Brak szczegółów błędu", + "error_endpoint": "Nieznany endpoint" + }, + "none": { + "error_code": "Brak kodu błędu", + "error_details": "Brak szczegółów błędu", + "error_endpoint": "Nieznany endpoint" + }, + "plaintext": { + "error_code": "Brak kodu błędu", + "error_details": "Some random error message", + "error_endpoint": "Nieznany endpoint" + }, + "tuple_dict": { + "error_code": 400, + "error_details": "{\n \"code\": 400,\n \"message\": \"Bad Request\",\n \"description\": \"Validation failed.\",\n \"details\": {\n \"isbn\": \"Publikacja o identycznym ISBN już istnieje!\"\n }\n}", + "error_endpoint": "/api/v1/publications" + } + }, + "parse_error_details_with_status": { + "bare_tuple": { + "error_code": 400, + "error_details": "{\n \"message\": \"Error\",\n \"code\": 400\n}", + "error_endpoint": "/api/v1/publications" + }, + "empty": { + "error_code": 404, + "error_details": "Brak szczegółów błędu", + "error_endpoint": "Nieznany endpoint" + }, + "none": { + "error_code": 404, + "error_details": "Brak szczegółów błędu", + "error_endpoint": "Nieznany endpoint" + }, + "plaintext": { + "error_code": 404, + "error_details": "Some random error message", + "error_endpoint": "Nieznany endpoint" + }, + "tuple_dict": { + "error_code": 400, + "error_details": "{\n \"code\": 400,\n \"message\": \"Bad Request\",\n \"description\": \"Validation failed.\",\n \"details\": {\n \"isbn\": \"Publikacja o identycznym ISBN już istnieje!\"\n }\n}", + "error_endpoint": "/api/v1/publications" + } + }, + "parse_pbn_api_error": { + "bare_tuple": { + "error_code": 400, + "error_details_json": "{\n \"message\": \"Error\",\n \"code\": 400\n}", + "error_endpoint": "/api/v1/publications", + "error_message": "Error", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "empty": { + "is_pbn_api_error": false, + "raw_error": "Brak szczegółów błędu" + }, + "empty_list": { + "error_code": 400, + "error_details_json": "[]", + "error_endpoint": "/api/v1/publications", + "error_message": "PBN API zwróciło listę błędów", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "list_payload": { + "error_code": 400, + "error_details_json": "[\n {\n \"error\": \"First error\"\n },\n {\n \"error\": \"Second error\"\n }\n]", + "error_endpoint": "/api/v1/publications", + "error_message": "PBN API zwróciło listę błędów", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "none": { + "is_pbn_api_error": false, + "raw_error": "Brak szczegółów błędu" + }, + "number_payload": { + "error_code": 400, + "error_details_json": "42", + "error_endpoint": "/api/v1/publications", + "error_message": "Nieoczekiwany typ odpowiedzi PBN API", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "plaintext": { + "is_pbn_api_error": false, + "raw_error": "Some random error message" + }, + "prefix_http": { + "error_code": 400, + "error_description": "Validation failed.", + "error_details_json": "{\n \"isbn\": \"Publikacja o identycznym ISBN już istnieje!\"\n}", + "error_endpoint": "/api/v1/publications", + "error_message": "Bad Request", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "prefix_validation": { + "error_code": 400, + "error_details_json": "{\n \"doi\": \"Duplicate\"\n}", + "error_endpoint": "/api/v1/publications", + "error_message": "", + "exception_type": "PBNValidationError", + "is_pbn_api_error": true + }, + "simple_exc": { + "error_message": "czegoś brakuje", + "exception_type": "StatementsMissing", + "is_pbn_api_error": true, + "raw_error": "pbn_api.exceptions.StatementsMissing: czegoś brakuje" + }, + "string_payload": { + "error_code": 400, + "error_details_json": "\"Just a string\"", + "error_endpoint": "/api/v1/publications", + "error_message": "Nieoczekiwany typ odpowiedzi PBN API", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "tuple_dict": { + "error_code": 400, + "error_description": "Validation failed.", + "error_details_json": "{\n \"isbn\": \"Publikacja o identycznym ISBN już istnieje!\"\n}", + "error_endpoint": "/api/v1/publications", + "error_message": "Bad Request", + "exception_type": "HttpException", + "is_pbn_api_error": true + }, + "xss_fallback": { + "is_pbn_api_error": false, + "raw_error": "" + }, + "xss_payload": { + "error_code": 400, + "error_description": "", + "error_details_json": "{\n \"isbn\": \"\"\n}", + "error_endpoint": "/api/v1/publications", + "error_message": "", + "exception_type": "HttpException", + "is_pbn_api_error": true + } + } +} \ No newline at end of file diff --git a/src/pbn_api/tests/test_client_disciplines.py b/src/pbn_api/tests/test_client_disciplines.py index e6d186f53..53b64267b 100644 --- a/src/pbn_api/tests/test_client_disciplines.py +++ b/src/pbn_api/tests/test_client_disciplines.py @@ -9,6 +9,7 @@ from pathlib import Path import pytest +from django.db import connection from pbn_client.const import PBN_GET_DISCIPLINES_URL from bpp.decorators import json @@ -17,23 +18,23 @@ from pbn_api.models.discipline import Discipline -def test_get_disciplines(pbn_client): +def _load_disciplines_fixture(pbn_client): fixture_path = Path(__file__).parent / "fixture_test_get_disciplines.json" with open(fixture_path, "rb") as f: pbn_client.transport.return_values[PBN_GET_DISCIPLINES_URL] = json.loads( f.read() ) + + +def test_get_disciplines(pbn_client): + _load_disciplines_fixture(pbn_client) ret = pbn_client.get_disciplines() assert "validityDateFrom" in ret[0] @pytest.mark.django_db def test_download_disciplines(pbn_client): - fixture_path = Path(__file__).parent / "fixture_test_get_disciplines.json" - with open(fixture_path, "rb") as f: - pbn_client.transport.return_values[PBN_GET_DISCIPLINES_URL] = json.loads( - f.read() - ) + _load_disciplines_fixture(pbn_client) assert Discipline.objects.count() == 0 pbn_client.download_disciplines() @@ -42,11 +43,7 @@ def test_download_disciplines(pbn_client): @pytest.mark.django_db def test_sync_disciplines(pbn_client): - fixture_path = Path(__file__).parent / "fixture_test_get_disciplines.json" - with open(fixture_path, "rb") as f: - pbn_client.transport.return_values[PBN_GET_DISCIPLINES_URL] = json.loads( - f.read() - ) + _load_disciplines_fixture(pbn_client) d1 = Dyscyplina_Naukowa.objects.create(kod="5.1", nazwa="ekonomia i finanse") d2 = Dyscyplina_Naukowa.objects.create(kod="1.6", nazwa="nauka o kulturze") @@ -60,3 +57,36 @@ def test_sync_disciplines(pbn_client): assert TlumaczDyscyplin.objects.przetlumacz_dyscypline(d1, 2024) is not None assert TlumaczDyscyplin.objects.przetlumacz_dyscypline(d2, 2024) is not None + + +@pytest.mark.django_db(transaction=True) +def test_download_disciplines_fetches_outside_transaction(pbn_client): + """D3/bugfix: remote-fetch (get_disciplines) NIE może dziać się w otwartej + transakcji (wcześniej @transaction.atomic obejmował cały remote-call). + + ``transaction=True`` sprawia, że sam test nie owija się w atomic, więc + ``connection.in_atomic_block`` w trakcie fetchu wiarygodnie odzwierciedla + brak transakcji; upsert leci już wewnątrz transakcji sync_dictionary. + """ + _load_disciplines_fixture(pbn_client) + + seen = {} + original_get = pbn_client.get_disciplines + + def spying_get_disciplines(): + seen["fetch_in_atomic"] = connection.in_atomic_block + return original_get() + + original_upsert = pbn_client._upsert_disciplines + + def spying_upsert(elems): + seen["upsert_in_atomic"] = connection.in_atomic_block + return original_upsert(elems) + + pbn_client.get_disciplines = spying_get_disciplines + pbn_client._upsert_disciplines = spying_upsert + + pbn_client.download_disciplines() + + assert seen["fetch_in_atomic"] is False # remote POZA transakcją + assert seen["upsert_in_atomic"] is True # zapis WEWNĄTRZ transakcji diff --git a/src/pbn_api/tests/test_error_record_golden.py b/src/pbn_api/tests/test_error_record_golden.py new file mode 100644 index 000000000..0f74b8ffa --- /dev/null +++ b/src/pbn_api/tests/test_error_record_golden.py @@ -0,0 +1,74 @@ +"""Golden (charakteryzacyjne) testy byte-identyczności display-funkcji błędów. + +Fixture ``data/error_record_golden.json`` zdjęto z kodu SPRZED unifikacji +(P4). Po przepisaniu display-funkcji na ``ErrorRecord``/``parse()`` te same +asercje muszą przejść → dowód, że unifikacja NIE zmienia widocznego outputu +dla realistycznych danych legacy. + +Domeny wejścia pinujemy per-funkcja (patrz ``scratch_build_fixture.py`` / +spec §6): każda funkcja tylko na kształtach, którymi realnie jest wołana. +Świadome ulepszenia (crash na payload-liczbie, hack admina) są WYKLUCZONE +z tej siatki i testowane w ``test_error_record_improvements.py``. +""" + +import json +from pathlib import Path + +import pytest + +from pbn_export_queue.templatetags.pbn_queue_extras import format_pbn_error +from pbn_export_queue.views.utils import ( + extract_pbn_error_from_komunikat, + parse_error_details, + parse_pbn_api_error, +) + +_GOLDEN = json.loads( + (Path(__file__).parent / "data" / "error_record_golden.json").read_text("utf-8") +) +_INPUTS = _GOLDEN["inputs"] + + +class _FakeSent: + def __init__(self, exception, api_response_status=None): + self.exception = exception + self.api_response_status = api_response_status + + +def _cases(fn_key): + return [(k, _GOLDEN[fn_key][k]) for k in sorted(_GOLDEN[fn_key])] + + +@pytest.mark.parametrize("case,expected", _cases("format_none")) +def test_golden_format_pbn_error_no_rodzaj(case, expected): + assert str(format_pbn_error(_INPUTS[case] or "")) == expected + + +@pytest.mark.parametrize("case,expected", _cases("format_meryt")) +def test_golden_format_pbn_error_meryt(case, expected): + assert str(format_pbn_error(_INPUTS[case] or "", "MERYT")) == expected + + +@pytest.mark.parametrize("case,expected", _cases("format_tech")) +def test_golden_format_pbn_error_tech(case, expected): + assert str(format_pbn_error(_INPUTS[case] or "", "TECH")) == expected + + +@pytest.mark.parametrize("case,expected", _cases("parse_pbn_api_error")) +def test_golden_parse_pbn_api_error(case, expected): + assert parse_pbn_api_error(_INPUTS[case]) == expected + + +@pytest.mark.parametrize("case,expected", _cases("extract_from_komunikat")) +def test_golden_extract_from_komunikat(case, expected): + assert extract_pbn_error_from_komunikat(_INPUTS[case]) == expected + + +@pytest.mark.parametrize("case,expected", _cases("parse_error_details")) +def test_golden_parse_error_details(case, expected): + assert parse_error_details(_FakeSent(_INPUTS[case], None)) == expected + + +@pytest.mark.parametrize("case,expected", _cases("parse_error_details_with_status")) +def test_golden_parse_error_details_with_status(case, expected): + assert parse_error_details(_FakeSent(_INPUTS[case], 404)) == expected diff --git a/src/pbn_api/tests/test_error_record_improvements.py b/src/pbn_api/tests/test_error_record_improvements.py new file mode 100644 index 000000000..7a1d911ee --- /dev/null +++ b/src/pbn_api/tests/test_error_record_improvements.py @@ -0,0 +1,78 @@ +"""Świadome ulepszenia unifikacji P4 (wykluczone z golden byte-identyczności). + +1. ``format_pbn_error`` nie może już rzucać ``TypeError`` na payloadzie-liczbie + (stary bug: ``"message" in 42``). +2. Admin ``exception_details`` przestaje używać hacka + ``split('"details":')[1][:-3]`` — zwraca czytelny komunikat z ``ErrorRecord``. +""" + +import pytest +from django.contrib import admin + +from pbn_api.admin.sentdata import SentDataAdmin +from pbn_api.models import SentData +from pbn_export_queue.templatetags.pbn_queue_extras import format_pbn_error + + +class _FakeSent: + def __init__(self, exception, pk=1): + self.exception = exception + self.pk = pk + + +NUMBER_PAYLOAD = "pbn_api.exceptions.HttpException: (400, '/api/v1/publications', '42')" + + +@pytest.mark.django_db +def test_format_pbn_error_number_payload_does_not_raise(): + # Stary kod rzucał TypeError; nowy renderuje bezpieczny HTML. + result = format_pbn_error(NUMBER_PAYLOAD) + assert isinstance(str(result), str) + assert "