From 811fc3165d8d46b924985ced778d131cb6a7bfc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 14 Jul 2026 21:24:00 +0200 Subject: [PATCH 1/2] Adoptuj pbn_client.normalize_author_name w BPP (2 site'y) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dwa miejsca normalizowały dane osobowe autora z PBN własnymi, rozjeżdżającymi się łańcuchami preferencji: - pbn_api/models/publication.py (_normalizuj_autora): lastName + firstName/givenNames/name, brak familyName; - pbn_import/.../pbn_importuj_uid.py: familyName/lastName + givenNames/name, brak firstName, brak guardu na nie-dict. Oba delegują teraz do jednego źródła prawdy — pbn_client.normalize_author_name (unia kluczy: familyName→lastName, firstName→givenNames→name; nie-dict → brak danych). Paczka używa None jako sentinela pustki; oba site'y koerują go do "" (szablon rekordu i str(autorzy) w logach zakładają puste stringi, a display-line importu składa f-string). Zysk: rozpoznanie familyName (publication) i firstName (import) tam, gdzie wcześniej wypadało puste; bezpieczeństwo na nie-dict w komendzie importu. Zachowany zamrożony ""-kontrakt (test goły-UID) + nowy test familyName. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pbn-normalize-author.feature.rst | 4 ++++ src/pbn_api/models/publication.py | 24 +++++++------------ src/pbn_api/tests/test_publication_autorzy.py | 13 ++++++++++ .../management/commands/pbn_importuj_uid.py | 11 ++++++--- 4 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 src/bpp/newsfragments/pbn-normalize-author.feature.rst diff --git a/src/bpp/newsfragments/pbn-normalize-author.feature.rst b/src/bpp/newsfragments/pbn-normalize-author.feature.rst new file mode 100644 index 000000000..cbce2bae8 --- /dev/null +++ b/src/bpp/newsfragments/pbn-normalize-author.feature.rst @@ -0,0 +1,4 @@ +Normalizacja danych osobowych autora z PBN (``lastName``/``familyName`` oraz +``firstName``/``givenNames``/``name``) korzysta teraz z ``pbn_client.normalize_author_name`` +— jedno źródło prawdy dla niespójnych kształtów PBN, obejmujące także pole +``familyName``, którego wcześniejsza normalizacja rekordu PBN nie rozpoznawała. diff --git a/src/pbn_api/models/publication.py b/src/pbn_api/models/publication.py index b5d266903..334869f84 100644 --- a/src/pbn_api/models/publication.py +++ b/src/pbn_api/models/publication.py @@ -1,5 +1,6 @@ from django.db import models from django.utils.functional import cached_property +from pbn_client import normalize_author_name from bpp import const from bpp.models.abstract import LinkDoPBNMixin @@ -104,22 +105,15 @@ def policz_autorow(self): def _normalizuj_autora(autor): """Sprowadza pojedynczego autora z PBN do ``{lastName, firstName}``. - PBN podaje imię raz jako ``firstName``, raz jako ``givenNames``, - a w danych zaciągniętych z API instytucji jako ``name``. Czasem - zamiast słownika dostajemy goły UID (string) — wtedy nie mamy - danych osobowych i zwracamy puste pola (zamiast wysadzać szablon). + Deleguje wybór pól do ``pbn_client.normalize_author_name`` (jedno + źródło prawdy dla niespójnych kształtów PBN: ``lastName``/``familyName`` + dla nazwiska, ``firstName``/``givenNames``/``name`` dla imienia; goły + UID lub inny nie-dict → brak danych). Paczka używa ``None`` jako + sentinela pustki — tu koerujemy go do ``""``, bo szablon rekordu i + ``str(autorzy)`` (logi) zakładają puste stringi, nie ``None``. """ - if not isinstance(autor, dict): - return {"lastName": "", "firstName": ""} - return { - "lastName": autor.get("lastName") or "", - "firstName": ( - autor.get("firstName") - or autor.get("givenNames") - or autor.get("name") - or "" - ), - } + normalized = normalize_author_name(autor) + return {klucz: (wartosc or "") for klucz, wartosc in normalized.items()} @cached_property def autorzy(self): diff --git a/src/pbn_api/tests/test_publication_autorzy.py b/src/pbn_api/tests/test_publication_autorzy.py index 480f4af51..6305e525f 100644 --- a/src/pbn_api/tests/test_publication_autorzy.py +++ b/src/pbn_api/tests/test_publication_autorzy.py @@ -67,6 +67,19 @@ def test_policz_autorow_dziala_dla_dict_kluczowanego_uidem(): assert pub.policz_autorow() == 2 +@pytest.mark.django_db +def test_autorzy_nazwisko_z_familyName(): + """Adopcja ``pbn_client.normalize_author_name``: nazwisko bywa podane + + jako ``familyName`` (nie ``lastName``) — delegacja do paczki pokrywa + ten kształt, którego lokalna normalizacja wcześniej nie łapała. + """ + pub = _publikacja({"authors": [{"familyName": "Abacki", "givenNames": "Ewa"}]}) + autorzy = pub.autorzy["authors"] + assert autorzy[0]["lastName"] == "Abacki" + assert autorzy[0]["firstName"] == "Ewa" + + @pytest.mark.django_db def test_autorzy_goly_uid_bez_danych_osobowych_nie_wybucha(): """Defensywnie: gdyby PBN podał listę samych UID-ów (stringów), diff --git a/src/pbn_import/management/commands/pbn_importuj_uid.py b/src/pbn_import/management/commands/pbn_importuj_uid.py index c0262e98b..0dd616c11 100644 --- a/src/pbn_import/management/commands/pbn_importuj_uid.py +++ b/src/pbn_import/management/commands/pbn_importuj_uid.py @@ -1,5 +1,7 @@ """Import publikacji z PBN do BPP po PBN UID.""" +from pbn_client import normalize_author_name + from pbn_api.exceptions import HttpException from pbn_api.management.commands.util import PBNBaseCommand from pbn_import.utils.command_helpers import ( @@ -86,9 +88,12 @@ def _dry_run_single_publication(self, pbn_uid, client): ) names = [] for p in persons_list[:5]: - # PBN uses familyName/givenNames or lastName/name - last = p.get("familyName") or p.get("lastName", "") - first = p.get("givenNames") or p.get("name", "") + # Jedno źródło prawdy dla niespójnych kształtów PBN + # (familyName/lastName, firstName/givenNames/name); paczka + # zwraca None dla braku — koerujemy do "" na potrzeby f-stringa. + autor = normalize_author_name(p) + last = autor["lastName"] or "" + first = autor["firstName"] or "" names.append(f"{last} {first}".strip()) if len(persons_list) > 5: names.append(f"... (+{len(persons_list) - 5})") From 3b763af942dd080a75aeada4bc5c59fb27846c1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Tue, 14 Jul 2026 21:43:36 +0200 Subject: [PATCH 2/2] =?UTF-8?q?Adoptuj=20pakietowe=20helpery=20o=C5=9Bwiad?= =?UTF-8?q?cze=C5=84=20w=20pbn=5Ftest=5Fwysylka=5Finteraktywna?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interaktywne narzędzie diagnostyczne trzymało własne kopie logiki, którą mamy teraz w pbn-client. Podmienione na jedno źródło prawdy: - _extract_object_id → pbn_client.decode_publication_object_id (mapowanie endpoint_choice → bez_oswiadczen: "publications"=all-in-one → False, "repositorium" → True). Paczka przy niejednoznacznej odpowiedzi (lista != 1 element, zły typ) RZUCA zamiast po cichu zwracać None — narzędzie łapie to głośno, pokazuje surową odpowiedź i pyta usera czy wyjść czy jechać dalej z objectId=None (decyzja usera: rzadko używane, niech krzyczy i pyta). - lokalny _key → pakietowe statement_key_pbn / statement_key_intended; różnice liczy diff_statements, część wspólną (której paczka nie zwraca) liczymy lokalnie na tych samych kluczach. Testy: nowy test integracyjny (niejednoznaczna odpowiedź → głośny błąd + przerwanie pod --yes-all) + unit gałęzi "kontynuuj" (zwraca None). 16 passed w suicie komendy, 339 w całym pbn_api. ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pbn-adopt-statement-helpers.feature.rst | 6 ++ .../commands/pbn_test_wysylka_interaktywna.py | 77 ++++++++++--------- .../test_pbn_test_wysylka_interaktywna.py | 59 ++++++++++++++ 3 files changed, 105 insertions(+), 37 deletions(-) create mode 100644 src/bpp/newsfragments/pbn-adopt-statement-helpers.feature.rst diff --git a/src/bpp/newsfragments/pbn-adopt-statement-helpers.feature.rst b/src/bpp/newsfragments/pbn-adopt-statement-helpers.feature.rst new file mode 100644 index 000000000..60ee004c2 --- /dev/null +++ b/src/bpp/newsfragments/pbn-adopt-statement-helpers.feature.rst @@ -0,0 +1,6 @@ +Narzędzie diagnostyczne ``pbn_test_wysylka_interaktywna`` korzysta teraz z +pakietowych helperów ``pbn_client`` do dekodowania identyfikatora publikacji +(``decode_publication_object_id``) oraz porównywania oświadczeń +(``diff_statements`` + ``statement_key_*``). Przy niejednoznacznej odpowiedzi +PBN (lista różna od jednego elementu) narzędzie głośno sygnalizuje błąd i pyta +o kontynuację zamiast po cichu jechać dalej. diff --git a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py index 95d2c2340..655bdb6c4 100644 --- a/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/management/commands/pbn_test_wysylka_interaktywna.py @@ -24,6 +24,7 @@ from typing import Any from django.core.management.base import CommandError +from pbn_client import decode_publication_object_id from bpp.models import Wydawnictwo_Ciagle, Wydawnictwo_Zwarte from bpp.util import zaloguj_polkniety_wyjatek @@ -156,7 +157,9 @@ def _run_flow(self, pbn_client, publication): return pbn_statements = self._step_get_pbn_statements(pbn_client, object_id) - identyczne = self._step_compare_statements(publication, pbn_statements) + identyczne = self._step_compare_statements( + pbn_client, publication, pbn_statements + ) # Zawsze pytamy osobno o DELETE i POST — nawet gdy identyczne. # Default zależy od wyniku porównania (False dla identycznych, @@ -363,7 +366,7 @@ def _step_get_pbn_statements(self, pbn_client, object_id): self._prompt_enter() return result - def _step_compare_statements(self, publication, pbn_statements): + def _step_compare_statements(self, pbn_client, publication, pbn_statements): self._header("KROK 6/8 — Porównanie: intencja BPP (live) vs PBN") # Intencja BPP: co wygenerowałby adapter GDYBY teraz wysłać — czyli @@ -395,33 +398,16 @@ def _step_compare_statements(self, publication, pbn_statements): self._prompt_enter() return False # traktujemy jak "różne" - def _key(stmt): - """Klucz porównania: (person-mongoId, disciplineNumer). - - Mapowanie między formatami: - - PBN GET response (``/page/statements``): ``personId`` (mongoId), - ``area`` (string, numerek dyscypliny MNiSW np. "301"). - - Adapter ``pbn_get_json_statements()`` (przed konwersją): - ``personObjectId`` (mongoId), ``disciplineId`` (int, numerek - dyscypliny MNiSW). - Oba oznaczają to samo. - """ - if not isinstance(stmt, dict): - return (None, None) - person = stmt.get("personId") or stmt.get("personObjectId") - discipline = stmt.get("area") - if discipline is None: - discipline = stmt.get("disciplineId") - return ( - str(person) if person else None, - str(discipline) if discipline is not None else "", - ) - - intended_keys = {_key(x) for x in intended} - pbn_keys = {_key(x) for x in pbn_statements} + # Klucze porównania liczą pakietowe helpery klienta (jedno źródło + # prawdy): ``statement_key_pbn`` czyta format PBN GET (``personId``/ + # ``area``), ``statement_key_intended`` — format adaptera + # (``personObjectId``/``disciplineId``). ``diff_statements`` zwraca + # (tylko-w-PBN, tylko-w-intencji); część wspólną liczymy tu lokalnie + # (paczka jej nie zwraca) na tych samych kluczach. + intended_keys = {pbn_client.statement_key_intended(x) for x in intended} + pbn_keys = {pbn_client.statement_key_pbn(x) for x in pbn_statements} - only_intended = intended_keys - pbn_keys - only_pbn = pbn_keys - intended_keys + only_pbn, only_intended = pbn_client.diff_statements(pbn_statements, intended) common = intended_keys & pbn_keys self._info(f"Intencja BPP (live): {len(intended_keys)}") @@ -536,15 +522,32 @@ def _step_post_statements(self, pbn_client, publication): # ------------------------- helpers ------------------------- def _extract_object_id(self, response, endpoint_choice): - if endpoint_choice == "publications": - if isinstance(response, dict): - return response.get("objectId") - return None - if isinstance(response, list) and len(response) == 1: - item = response[0] - if isinstance(item, dict): - return item.get("id") or item.get("objectId") - return None + """Dekoduje objectId z odpowiedzi POST-a przez pakietową + ``pbn_client.decode_publication_object_id``. + + Mapowanie endpointa na tryb paczki: + - ``"publications"`` (all-in-one) → ``bez_oswiadczen=False`` + (odpowiedź: ``{"objectId": ...}``), + - ``"repositorium"`` → ``bez_oswiadczen=True`` + (odpowiedź: ``[{"id": ...}]``). + + Przy niejednoznacznej odpowiedzi (lista != 1 element, zły typ, brak + klucza) paczka **rzuca wyjątkiem** zamiast po cichu zwracać None — + w tym rzadko używanym narzędziu diagnostycznym łapiemy to głośno, + pokazujemy surową odpowiedź i pytamy usera: wyjść czy jechać dalej + z ``objectId=None``. + """ + bez_oswiadczen = endpoint_choice != "publications" + try: + return decode_publication_object_id(response, bez_oswiadczen=bez_oswiadczen) + except Exception as e: # noqa: BLE001 — narzędzie debug: głośno + pytanie + self._err(f"Nie mogę zdekodować objectId z odpowiedzi PBN: {e}") + self.stdout.write(_json_truncated(response, max_len=800)) + if self._prompt_yes_no( + "Kontynuować flow mimo to (objectId=None)?", default=False + ): + return None + raise UserAbort() from e def _print_http_request(self, method, url, body, label=""): self._info(f"Wywołanie: {label}" if label else "Żądanie HTTP:") diff --git a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py index 017800c41..7c50e38fc 100644 --- a/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py +++ b/src/pbn_api/tests/test_pbn_test_wysylka_interaktywna.py @@ -627,3 +627,62 @@ def test_json_truncated_nie_obcina_krotkiego_tekstu(): result = cmd_mod._json_truncated(small, max_len=100) assert "obcięto" not in result assert '"a": 1' in result + + +@pytest.mark.django_db +def test_niejednoznaczny_objectId_glosno_krzyczy_i_przerywa( + pbn_client, + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina, + pbn_publication, + monkeypatch, +): + """Repozytorium zwraca listę != 1 element → pakietowe + ``decode_publication_object_id`` rzuca zamiast po cichu zwracać None. + + Narzędzie łapie to głośno (pokazuje błąd + surową odpowiedź) i pyta czy + kontynuować. Pod ``--yes-all`` pytanie idzie na default (False = nie) → + flow przerywany, zamiast jechać dalej z ``objectId=None``. + """ + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pbn_uid = pbn_publication + pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.save() + + _patch_get_client(monkeypatch, pbn_client) + _patch_intended_statements(monkeypatch, []) + # Dwa elementy — sytuacja niejednoznaczna (spodziewamy się dokładnie 1): + pbn_client.transport.return_values[PBN_POST_PUBLICATION_NO_STATEMENTS_URL] = [ + {"id": pbn_publication.pk}, + {"id": pbn_publication.pk}, + ] + _patch_input(monkeypatch, ["2"]) # endpoint repozytoryjny + + out = StringIO() + call_command( + "pbn_test_wysylka_interaktywna", + "--wydawnictwo-zwarte", + str(pbn_wydawnictwo_zwarte_z_autorem_z_dyscyplina.pk), + "--yes-all", + stdout=out, + ) + output = out.getvalue() + assert "Nie mogę zdekodować objectId" in output + assert "Przerwano przez użytkownika." in output + # Nie dobił do GET oświadczeń (KROK 5) — przerwał już na dekodowaniu: + assert not any( + k.startswith(PBN_GET_INSTITUTION_STATEMENTS) + for k in pbn_client.transport.input_values + ) + + +def test_extract_object_id_niejednoznaczny_kontynuacja_zwraca_none(monkeypatch): + """Gałąź „kontynuuj mimo błędu": user zgadza się jechać dalej → + ``_extract_object_id`` zwraca None (zamiast rzucać UserAbort).""" + from django.core.management.base import OutputWrapper + + cmd = cmd_mod.Command() + cmd.stdout = OutputWrapper(StringIO()) + monkeypatch.setattr(cmd, "_prompt_yes_no", lambda *args, **kwargs: True) + + result = cmd._extract_object_id( + [{"id": 1}, {"id": 2}], endpoint_choice="repositorium" + ) + assert result is None