soft-delete faza 05a — wycofanie oświadczeń z PBN - #755
Open
mpasternak wants to merge 16 commits into
Open
Conversation
Plan fazy 05 powstal 2026-06-04; jego rewizje (08-06, 08-07) sprawdzaly kolejke i klienta PBN, ale zadna nie sprawdzila, co faza 02 dopisala do check_if_record_still_exists(). A to przesadza o wykonalnosci Taska 05.3. Bloker #1: send_to_pbn() wola guard PRZED _zajmij_atomowo(), a guard od commita 2e3b386 odrzuca rekordy z koszem (deleted_at IS NOT NULL). Wycofanie zlecamy wylacznie dla rekordow soft-skasowanych, wiec galaz WYCOFANIE z planu bylaby martwym kodem — kazdy wpis konczylby sie FINISHED_ERROR z komunikatem "Rekord zostal usuniety nim wysylka byla mozliwa". Bloker #2 (cichy, gorszy): kolejka_wyczysc_wpisy_bez_rekordow() ma ten sam guard za kryterium i delete()-uje wpisy, dla ktorych zwroci False. Zlecenie wycofania znikalo z kolejki, oswiadczenia zostawaly w PBN, sladu brak. Rozstrzygniecie (opcja A, decyzja wlasciciela): guard staje sie swiadomy operacji — odrzucenie soft-deleted obowiazuje tylko dla WYSYLKA. Naprawia oba blokery jedna zmiana, bo sprzataczka wola te sama metode na instancji. Wariant z wczesnym rozgalezieniem w send_to_pbn() leczylby tylko #1. Poza tym: nagrobki (OAI-PMH/CERIF/REST) wychodza do osobnej fazy 05b — mialy zero taskow, a lacza je z wycofaniem tylko motyw, nie kod. Termin (przed faza 07) bez zmian. Zmiany w dokumencie: sekcja rewizji, korekta numerow linii w Stanie zastanym, krok "guard swiadomy operacji" w Tasku 05.3 przed rozgalezieniem, poprawka testu 05.3 (musi soft-delete'owac rekord, inaczej nie odtwarza blokera), NOWY Task 05.3a z regresja sprzataczki.
…mark_as_successful
Cala logika wycofania oswiadczen dyscyplin z profilu instytucji PBN mieszka w wolnostojacej funkcji, zeby wejscie asynchroniczne (kolejka) i synchroniczne (poza kolejka) zostawialy identyczny stan SentData i identycznie rozumialy sukces. Podzial odpowiedzialnosci (niezmiennik §4.2 specu): prymityw odpowiada za semantyke "co znaczy sukces" i za stan SentData; klasyfikacja wyjatkow i polityka ponawiania naleza do wolajacego. Dlatego wyjatki PBN propaguja stad nietkniete — inaczej kolejka i sciezka synchroniczna mialyby dwie rozjezdzajace sie tabele decyzji. Jedyny lapany wyjatek to CannotDeleteStatementsException, bo to pytanie o semantyke sukcesu, nie o retry: "nie bylo czego usuwac" znaczy, ze stan docelowy jest osiagniety. Uwaga na pulapke — pbn-client w _delete_statements_with_retry PONAWIA na tym samym wyjatku, bo tam kasowanie poprzedza wysylke i brak oswiadczen jest przeszkoda. Obiektu publikacji w PBN nie kasujemy — jest wspoldzielony miedzy instytucjami; usuwamy wylacznie oswiadczenia naszej.
…_pbn Trzy elementy, z ktorych pierwszy byl w planie pominiety i bez niego reszta jest martwym kodem. 1) check_if_record_still_exists() staje sie swiadomy operacji. Guard z fazy 02 odrzucal rekordy z deleted_at IS NOT NULL — czyli DOKLADNIE te, dla ktorych wycofanie istnieje. Dla WYSYLKI przeslanka "operator usunal rekord" znaczy "nie pchaj do PBN"; dla WYCOFANIA ta sama przeslanka znaczy "wlasnie teraz wycofaj oswiadczenia". Ten sam fakt, przeciwne wnioski — stad warunek na operacji. Rekord skasowany TWARDO nadal daje False dla obu operacji: bez wiersza nie odczytamy pbn_uid. 2) _pozyskaj_klienta_pbn() — uczelnia z FK wpisu, nigdy "pierwsza z brzegu". Dla wpisow legacy (uczelnia_id NULL) fallback jedyna-albo-glosny-blad. 3) withdraw_from_pbn() jako CIENKI wrapper na prymityw wycofaj_oswiadczenia: pozyskanie klienta, wywolanie, tlumaczenie na SendStatus. Klasyfikacja wyjatkow idzie przez wspolne _handle_pbn_exception, wiec wycofanie dziedziczy CALA tabele decyzji wysylki, zamiast budowac druga. Rozgalezienie wstawione PO _zajmij_atomowo(), wiec wycofanie dziedziczy za darmo licznik prob, wysylke_podjeto i ochrone przed dwoma workerami. Test odtwarza realny scenariusz: rekord jest w koszu, bo bez tego przeszedlby takze przed poprawka guardu i niczego by nie pilnowal. Regresja: 174 passed (caly pbn_export_queue + guard multi-hosted).
Bloker #2 z rewizji planu 2026-08-10 — cichszy i grozniejszy od #1. kolejka_wyczysc_wpisy_bez_rekordow() iteruje po CALEJ kolejce i delete()-uje wpisy, dla ktorych check_if_record_still_exists() zwroci False. Zanim guard poznal operacje, zlecenie WYCOFANIA (rekord z definicji w koszu) padalo jej ofiara: znikalo z kolejki, oswiadczenia zostawaly w PBN, sladu brak. Wyscig miedzy beatem sprzatajacym a workerem celery rozstrzygal sie losowo. Roznica wobec blokera #1: tam zostawal przynajmniej wpis FINISHED_ERROR z komunikatem, tu nie zostawalo NIC. Poprawka guardu z poprzedniego commita rozbraja to automatycznie (sprzataczka wola te sama metode na instancji), wiec ten commit dokłada wylacznie test — zeby przyszla zmiana guardu nie wskrzesila cichej utraty zlecen. Test asertuje ROZNICE miedzy operacjami, nie samo przetrwanie wpisu: zachowanie dla WYSYLKI (faza 02, rekord z kosza ma znikac) musi zostac. Mutacja potwierdzajaca (cofniecie warunku na operacji w guardzie) wywala oba testy naraz — #1 jako FINISHED_ERROR zamiast FINISHED_OKAY, #2 jako skasowane zlecenie. Jedna przyczyna, dwa objawy.
…rd WYSYLKI sprobuj_utowrzyc_wpis lapalo KAZDY IntegrityError i przemianowywalo go na AlreadyEnqueuedError, bo spodziewalo sie wylacznie kolizji czesciowego unikatu. Systemowy soft-delete publikacji (sygnal, celery, scalanie — bez request.user) trafia jednak w NOT NULL na zamowil i dostawal "ten rekord jest juz w kolejce": zakolejkuj_wycofanie zwrocilo by None, oswiadczenia zostalyby w PBN, a operator widzialby komunikat sugerujacy, ze wszystko gra. Teraz tlumaczymy tylko naruszenie nazwanego unikatu (pbn_export_queue_jeden_aktywny_wpis_na_rekord), rozpoznawane po exc.__cause__.diag.constraint_name z fallbackiem na tekst wyjatku. Reszta leci w gore. Spec §4.2 przypisywal konto techniczne do "fazy 05/06", plan 05 pisal "rozwiazuje faza 06/07", plan 06 pisal "to dlug fazy 05" — czyli nikt tego nie robil. Ta czesc (rozroznienie wyjatku) jest tu; pobierz_konto_techniczne przychodzi razem z operacje.py w nastepnym kroku. Dodatkowo guard regresyjny: WYSYLKA nie moze wpasc w galaz wycofania. Blad w warunku albo w defaulcie pola zamienilby zalegla kolejke wysylek w kolejke wycofan i skasowal oswiadczenia rekordow, ktorych nikt nie usuwal. Regresja wyscigu na unikacie nadal zielona (3 testy alreadyenqueued).
…to techniczne Publiczny kontrakt dla fazy 06 (receivery sygnalow): FUNKCJE MODULOWE w pbn_export_queue/operacje.py, nie metody managera. Faza 06 sprawdza dokladnie ten import, wiec metody na managerze wymusilyby shim i dwie rozbiezne implementacje. Konsekwencja: faza 06 pomija swoj Task 5. Gate na pbn_uid ma TYLKO wycofanie. zakolejkuj_wysylke celowo go NIE ma — rekord, ktory nigdy nie poszedl do PBN, po przywroceniu i tak ma prawo pojechac, bo wysylka dopiero nadaje PBN UID. Kontrakt planu fazy 06 opisywal "None gdy brak pbn_uid" dla obu funkcji; to swiadome odejscie, udokumentowane w docstringu, zeby nikt go nie "naprawil" pod tamten opis. pobierz_konto_techniczne() domyka dlug, ktory spec §4.2 i plany 05/06 przerzucaly miedzy soba: zamowil jest NOT NULL, a soft-delete z sygnalu nie ma requestu. Konto jest nieaktywne i bez hasla — ma istniec jako podmiot audytu, nie jako sposob logowania. Konto nie ma tokenu PBN, wiec samo z siebie nic nie wysle: konczy sie WillNotExportError i FINISHED_ERROR z bledem MERYTORYCZNYM. To akceptowalne WYLACZNIE dlatego, ze jest glosne — test to przypina. Obejscie produkcyjne bez zmiany kodu: administrator ustawia kontu przedstawiaj_w_pbn_jako na konto z waznym tokenem (get_pbn_user() samo podmieni uzytkownika). sprobuj_utowrzyc_wpis dostaje opcjonalny kwarg operacja — minimalne rozszerzenie, savepoint i tlumaczenie kolizji unikatu bez zmian, wszystkie dotychczasowe wywolania dzialaja i dostaja default modelu.
Oba testy e2e migracji cofaly baze MigrationExecutorem i przywracaly ja do
ZASZYTEGO numeru migracji. Cofniecie do stanu "PRZED" odapplikowuje wszystko
powyzej, wiec powrot do stalej zostawia baze o tyle migracji w tyle, ile ich
od napisania testu przybylo.
Objaw: dodanie 0080 (SentData.withdrawn_at) wywrocilo Playwrightowy test
admina SentData bledem UndefinedColumn w miejscu bez zwiazku z przyczyna.
Reprodukcja deterministyczna na dwoch plikach:
pytest test_migracja_publikacja_instytucji_e2e.py test_playwright/test_admin.py
To nie byl blad fazy 05 — to mina, na ktora nadepnelaby dowolna przyszla
migracja pbn_api. test_migracja_dyscypliny_uuid_e2e.py byl gorszy: nie mial
finally W OGOLE i konczyl na 0077, czyli juz dzis zostawial baze trzy
migracje w tyle; nie bylo tego widac tylko dlatego, ze sasiedni test
przypadkiem sprzatal po nim, dociagajac do owczesnego czubka.
Intencja oryginalnego komentarza byla trafna ("inaczej worker zostaje na
0077 i psuje WSZYSTKIE kolejne testy kaskada niezrozumialych bledow") —
utrwalono tylko NUMER zamiast POJECIA "czubek grafu migracji". Teraz
leaf_nodes("pbn_api"), wiec kolejna migracja niczego nie wywroci.
Regresja: caly src/pbn_api/tests/ — 466 passed.
…encji Testy 05.6 sprawdzaja zachowanie pochodzace z DWOCH roznych warstw i to jest ich sens: CannotDeleteStatementsException obsluguje prymityw (sukces BRAK_OSWIADCZEN), a ResourceLocked/PraceSerwisowe klasyfikuje wspolne _handle_pbn_exception. Gdyby ktorys padl, poprawiac nalezy TE warstwe, a nie dokladac druga drabinke except w withdraw_from_pbn. Test retry jest sparametryzowany dwoma wyjatkami o roznych politykach (RETRY_LATER vs RETRY_MUCH_LATER) — dowodzi, ze wycofanie dziedziczy CALA tabele decyzji wysylki, a nie dwa wybrane przypadki. Przy porazce wpis zostaje niezakonczony, a SentData nietkniete: oswiadczenia nadal sa w PBN. Uwaga dla przyszlych testow: ResourceLockedException dziedziczy po HttpException i wymaga (status_code, url, content), nie samego komunikatu. Admin: kolumna + filtr + readonly na "operacja", zeby superuser odroznil w kolejce wycofanie od wysylki. Regresja: caly pbn_export_queue — 186 passed.
…ania Plan kazal NIE zgadywac, tylko ustalic realny zbior sciezek synchronicznych. Ustalone: nie ma czego podpinac, i to nie jest przeoczenie. synchronizuj_publikacje to wsadowy UPLOADER, nie sciezka kasowania. Decyzja #16 byla trafna co do faktu (rekord bywa wysylany poza kolejka), ale wysylka synchroniczna nie rodzi potrzeby wycofania synchronicznego: wycofanie wyzwala soft-delete, a ten zawsze idzie przez kolejke (receivery fazy 06). Dodatkowo wydawnictwa_zwarte_do_synchronizacji() filtruje przez .objects, czyli menedzer pomijajacy kosz od fazy 02 — rekord soft-skasowany po prostu wypada z wsadu. Co faza faktycznie dostarcza dla wejscia synchronicznego: prymityw przyjmuje klienta OD WYWOLUJACEGO, wiec jest gotowym kontraktem dla dowolnej przyszlej sciezki poza kolejka. test_wycofanie.py wola go dokladnie tak (wlasny klient, bez kolejki), czyli testuje wlasnie ksztalt wejscia synchronicznego. Rownowaznosc obu wejsc jest zagwarantowana konstrukcyjnie: kolejka nie ma wlasnej implementacji, tylko cienki wrapper. Grep kontrolny 05.9.4: w produkcji doszla DOKLADNIE jedna linia (pbn_api/wycofanie.py:68). Trzy baseline'owe wystapienia bez zmian, zero w pbn_export_queue/ i pbn_integrator/ poza asercjami na mocku w testach.
Uzupelnienie poprzedniej poprawki, ktora byla dobra co do kierunku, ale za waska co do zakresu. Szeroka suita (9584 passed) pokazala dwie porazki w pbn_integrator/tests/test_mongodb_ops.py: "ProgrammingError: relacja pbn_integrator_rekordprzy... nie istnieje". Przyczyna: migracje innych aplikacji zaleza od pbn_api — pbn_integrator/0002_indeks_content_type_object_id wymaga pbn_api/0079. Gdy test e2e cofa pbn_api do 0077, Django odapplikowuje RAZEM z nim migracje zalezne z innych aplikacji. Przywracanie samego pbn_api zostawialo wiec pbn_integrator bez tabel. leaf_nodes() bez argumentu zwraca czubki calego grafu, wiec baza wraca dokladnie tam, gdzie byla przed testem — niezaleznie od tego, ile aplikacji zostalo po drodze cofnietych. Helper przemianowany: przywroc_czubek_migracji_pbn_api -> przywroc_czubek_migracji (nazwa sugerowala wlasnie ten za waski zakres).
…i fazy 04 makemigrations --check byl czerwony: niezmigrowana zmiana stanu. Faza 04 przestawila autor na PROTECT w klasie abstrakcyjnej BazaModeluOdpowiedzialnosciAutorow, ale migracja bpp/0501 objela wylacznie modele z aplikacji bpp (Patent_Autor, Praca_Doktorska, Wydawnictwo_Ciagle_Autor, Wydawnictwo_Zwarte_Autor). Handoff fazy 04 mowil "dziedzicza 3 modele *_Autor" — dziedziczy CZWARTY, Zgloszenie_Publikacji_Autor, i mieszka w innej aplikacji. Ochrona sama w sobie DZIALALA juz wczesniej: on_delete jest regula kolektora Django, zyjaca w Pythonie, wiec pole zachowywalo sie jak PROTECT od chwili zmiany abstraktu. Brakowalo wylacznie ksiegowosci stanu — i to ona wywracala bramke braku driftu migracji. Migracja state-only, z tego samego powodu co bpp/0501: on_delete nie ma odpowiednika w schemacie, wiec autogenerowany AlterField wygenerowalby DROP + ADD CONSTRAINT (ACCESS EXCLUSIVE) dla constraintu identycznego z istniejacym. Wniosek na przyszlosc: zmiana on_delete na modelu ABSTRAKCYJNYM rozlewa sie na wszystkie aplikacje, ktore go dziedzicza, a makemigrations zglasza to per-aplikacja. Szukanie dziedziczacych tylko w bpp/ jest niewystarczajace. Po tej migracji: zero driftu w src/ (pozostale zgloszenia dotycza pakietow zewnetrznych w site-packages: favicon, flexible_reports, siteblog).
Member
Author
Weryfikacja lokalna (2026-08-12)CI nie biegnie na PR-ach do gałęzi
Wynik Mutacje potwierdzające, że testy pilnują
Uwagi o środowiskuHost był w trakcie sesji współdzielony z równoległymi przebiegami innych gałęzi ( |
Member
Author
Playwright — uzupełnienie
Komplet bramek fazy jest zielony:
Dla porównania stan wejściowy fazy 04 (handoff): 9562 / 157 / 81. Przyrost po stronie Pythona to testy tej fazy. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Faza 05a soft-delete: soft-delete publikacji wysłanej do PBN wycofuje jej
oświadczenia dyscyplin z profilu instytucji; przywrócenie wysyła je ponownie.
Stacked na #745 (faza 04). Bazuje na
feat/soft-delete-04.✂️ Zakres rozdzielony względem planu
Plan fazy 05 zawierał dwa niezależne podsystemy: wycofanie z PBN (plan
drobiazgowy, 1286 linii) i nagrobki OAI-PMH/CERIF/REST (baner zakresu
z 2026-08-08, zero tasków). Decyzją właściciela nagrobki wychodzą do
fazy 05b z własnym cyklem brainstorming → spec → plan → PR; termin (przed
fazą 07) bez zmian. Punkt startowy dla 05b jest rozpoznany i zapisany
w handoffie.
Plan powstał 2026-06-04; jego rewizje sprawdzały kolejkę i klienta PBN, ale
żadna nie sprawdziła, co faza 02 dopisała do
check_if_record_still_exists()(commit
2e3b38611):Wycofanie zlecamy wyłącznie dla rekordów w koszu, więc guard odrzucał
dokładnie te wpisy, które ta faza tworzy.
send_to_pbn()woła guard przed_zajmij_atomowo()FINISHED_ERRORkolejka_wyczysc_wpisy_bez_rekordow()ma ten sam guard za kryteriumRozwiązanie: guard świadomy operacji — odrzucenie soft-deleted obowiązuje
tylko dla
WYSYLKA. Jedna zmiana naprawia oba, bo sprzątaczka woła tę samąmetodę na instancji. Rekord skasowany twardo nadal daje
Falsedla obuoperacji (bez wiersza nie ma
pbn_uid).Mutacja potwierdzająca (cofnięcie warunku) wywala oba testy naraz.
Co dochodzi
wycofaj_oswiadczenia(publikacja, client, uczelnia=None)pbn_api/wycofanie.py— JEDYNE miejsce wołającedelete_all_publication_statementsw kontekście soft-deletePBN_Export_Queue.Operacja(WYSYLKA/WYCOFANIE) +withdraw_from_pbn()pbn_export_queue/models.pyzakolejkuj_wycofanie/zakolejkuj_wysylke/pobierz_konto_technicznepbn_export_queue/operacje.py— kontrakt PINNED dla fazy 06SentData.withdrawn_at+mark_as_withdrawnpbn_api/models/sentdata.py, per-uczelniaoperacjapbn_export_queue/admin.pyMigracje:
pbn_export_queue/0011,pbn_api/0080,zglos_publikacje/0028.Obiektu publikacji w PBN nie kasujemy — jest współdzielony między
instytucjami; usuwamy wyłącznie oświadczenia naszej.
Rozstrzygnięcia warte uwagi recenzenta
zakolejkuj_wysylkeNIE ma gate'u napbn_uid, choć kontrakt planufazy 06 pisał „None gdy brak pbn_uid" dla obu funkcji. Rekord, który nigdy
nie poszedł do PBN, po przywróceniu i tak ma prawo pojechać — wysyłka
dopiero nadaje PBN UID. Udokumentowane w docstringu.
CannotDeleteStatementsException= sukces, nie retry. Pakietpbn-clientponawia na tym samym wyjątku, bo tam kasowanie poprzedzawysyłkę; u nas „nie było czego usuwać" to stan docelowy.
żeby kolejka i przyszła ścieżka synchroniczna nie miały dwóch
rozjeżdżających się tabel decyzji o ponawianiu.
FINISHED_ERROR(błądMERYTORYCZNY) — akceptowalne wyłącznie dlatego, że głośne; test to
przypina. Wdrożeniowo: administrator ustawia mu
przedstawiaj_w_pbn_jakona konto z ważnym tokenem.
Naprawione przy okazji (nie były długiem tej fazy)
odapplikowuje wszystko powyżej, więc powrót do stałej zostawiał bazę w tyle
— dodanie
pbn_api/0080wywróciło Playwrightowy test adminaSentDatabłędem
UndefinedColumnw miejscu bez związku z przyczyną. Terazleaf_nodes()całego grafu: przywracanie samegopbn_apibyło zawąskie, bo
pbn_integrator/0002zależy odpbn_api/0079i Djangoodapplikowuje migracje zależne z innych aplikacji.
makemigrations --checkbył czerwony:
zglos_publikacje.Zgloszenie_Publikacji_AutordziedziczyBazaModeluOdpowiedzialnosciAutorow, alebpp/0501objęła tylko modelez aplikacji
bpp. Ochrona działała (on_delete żyje w Pythonie);brakowało księgowości stanu. Migracja state-only, wzorzec
bpp/0501.Weryfikacja
Wyniki przebiegu lokalnego są w komentarzu pod PR-em.
feat/soft-delete*— zielonycheck tutaj nie jest dowodem.
baseline-sql/) celowo NIE odświeżany — zgodnie z CLAUDE.mdrobi się to raz, przy scalaniu całego
feat/soft-deletedodev.Czego ta faza NIE domyka (świadomie)
nie odczytamy
pbn_uid— kończy się głośnymFINISHED_ERROR. Właściwerozwiązanie to
SoftDeleteLogfazy 06.i funkcje, nie podpina ich do
post_soft_delete/post_restore.Handoff:
docs/superpowers/HANDOFF-soft-delete-faza-06.md