feat(uitvraag): geef de berichtenlijst de naam van de afzender - #285
Open
ericwout-overheid wants to merge 7 commits into
Open
feat(uitvraag): geef de berichtenlijst de naam van de afzender#285ericwout-overheid wants to merge 7 commits into
ericwout-overheid wants to merge 7 commits into
Conversation
De lijst droeg van de afzender alleen een nummer van twintig cijfers: `magazijnId` en `afzender` bevatten allebei de afzender-OIN. De leesbare organisatienaam kwam uitsluitend voorbij in de voortgangsmeldingen van een ophaalronde, dus moest een afnemer eerst álle organisaties laten bevragen om een leesbare lijst te kunnen tonen — en een bericht dat via de aanmeld-webhook binnenkwam van een organisatie die niet meedeed hield een rij cijfers, die een schermlezer cijfer voor cijfer voorleest. `BerichtSamenvatting` en `Bericht` dragen nu `afzenderNaam`, door de nieuwe `Afzendernamen`-bean per bericht opgezocht in het magazijnregister bij het `magazijnId`. Omdat het register de bron is en niet de ophaalronde, draagt een aangemeld bericht dezelfde naam als een opgehaald bericht en hoeft een afnemer niets tussen aanroepen door te onthouden. Het veld is optioneel: kent het register geen naam, dan ontbreekt het in het antwoord. Daaraan herkent een afnemer dát er geen naam is. Terugvallen op het `magazijnId` zou het probleem onzichtbaar maken in plaats van oplossen. BREAKING CHANGE: `afzender` vervalt in `BerichtSamenvatting` en `Bericht` van de uitvraag-API. Het droeg dezelfde OIN als `magazijnId` en was daarmee precies het nummer dat zich als naam voordoet; afnemers lezen voortaan `afzenderNaam` voor de naam en `magazijnId` voor het nummer. Het `afzender`-veld in `AangemeldBerichtData` blijft ongewijzigd — dat is het inkomende magazijn-contract, waar het veld een OIN hoort te zijn. Closes MinBZK/MijnOverheidZakelijk#1065 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
Open
6 tasks
Contributor
JaCoCo coverage
Files
|
Vijf review-agents op PR #285. De inhoudelijke punten: Een blanco naam kon als weergavenaam de keten in. `magazijnen."<OIN>".naam= ` leverde een lege string die met serialization-inclusion=non_null gewoon als `"afzenderNaam": ""` op de lijn kwam — precies de toestand die het contract als onmogelijk beschrijft, en die een afnemer niet als "geen naam" kan herkennen. `Magazijninschrijving` eist nu, net als bij `grantHash`, een niet-blanco naam; `ConfigMagazijnregister` trimt de configwaarde en leest blanco als afwezig. Geen fail-fast: geen naam is geldige configuratie. `Afzendernamen` liet register-drift spoorloos verdwijnen. Een `magazijnId` dat niet (meer) in het register staat en een `magazijnId` dat geen geldige OIN is zijn nu twee onderscheiden takken met een debug-log; de OIN gaat voluit mee (publiek, geen PII), de niet-OIN-waarde bewust niet — die haalde de validatie niet en kan een logregel vervalsen. Debug en niet warn omdat de lookup per bericht gebeurt; waar drift de gebruiker écht blokkeert escaleert MagazijnRouter hem al naar error + 502. Comments die een mechanisme beschreven dat de code niet heeft, zijn gecorrigeerd: de catch verklaarde een register-miss in plaats van een onparseerbare waarde, de mapper-KDoc suggereerde een afzender→naam-conversie die er niet is, en "directe register-hit" sprak de nullable lookup eronder tegen. De call-site-KDoc in BerichtenlijstService herhaalde de KDoc van Afzendernamen en is weg. De spec-description dekte maar één van de drie afwezigheid-oorzaken en claimde een patroon dat het schema niet afdwingt; `minLength: 1` maakt het non-blank-contract nu wél expliciet. Tests: de lege-registercase deed nul assertions en slaagde daarmee altijd (nu een eigen test), de catch-tak was alleen op unit-niveau bewezen en niet als HTTP-gedrag, de PATCH-respons droeg nooit een echte naam, de contract-validator zag het veld nooit gevuld, en er was geen bewijs dat het veld per bericht wordt gezet in plaats van per lijst. Toegevoegd, plus een E2E die berichten uit een échte ophaalronde langs twee magazijnen haalt — mét en zónder naam — en een guard dat `afzender` uit de responses verdwenen is. Bij het langslopen van de aangrenzende regels bleek een bestaand comment in berichtenbox.js te beweren dat het detail-endpoint `magazijnId` niet teruggeeft, terwijl het veld daar `required` is; gecorrigeerd naar de werkelijke reden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
Een deelnemende organisatie zonder leesbare naam is geen geldige inschrijving, en een berichtenlijst zonder afzendernaam geen bruikbare lijst. Het veld was optioneel met een expliciet afwezig-geval; dat legde de vraag "en wat toon ik dan?" bij elke afnemer neer, terwijl het antwoord in de configuratie hoort. `naam` is nu verplicht in het magazijnregister: `Magazijninschrijving` eist een niet-blanco waarde en `ConfigMagazijnregister` faalt fail-fast bij het opstarten, net als bij een ontbrekende of ongeldige `url`. Alle bestaande configuraties voldoen daar al aan, inclusief de regels die `demo/genereer-magazijnen.py` uitschrijft. Daarnaast krijgt elk bericht de naam mee op het moment dat het in de sessie wordt opgeslagen — zowel op het aggregatie-pad als via de aanmeld-webhook. Zo laat een organisatie die later uit het register verdwijnt haar berichten niet naamloos achter. `afzenderNaam` is daarmee `required` in de uitvraag-API en nooit leeg; een afnemer hoeft geen terugval te bouwen. Bij het lezen wint het register alsnog van de meegeschreven naam wanneer het de organisatie kent: dan werkt een hernoeming meteen door in plaats van pas na het verlopen van de sessie. De meegeschreven naam is het vangnet, niet de bron. De sessiecache-sleutels gaan van `v1` naar `v2`. Een oude entry mist het nieuwe veld en zou als corrupt gelezen worden; met een eigen prefix verlopen die entries via hun eigen TTL. Bij uitrol vraagt dat één keer de schema-bump-procedure uit docs/operations/redisearch-schema-bump.md — pre-productie mag de cache leeglopen. De SSE-events `magazijn-bevraging-gestart`/`-voltooid` dragen `naam` nu ook verplicht: dezelfde invariant, en het weglaat-gedrag had geen bron meer die null kon leveren. Op verzoek zijn de docs-blokken op de Bruno-requests weer verwijderd: te specifiek voor een collectie die de naam vanzelf in het antwoord laat zien. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
…dleiding De weergavenaam per magazijn is nu een harde boot-eis. Wie een magazijn toevoegt zonder `naam` krijgt een config-bindingsfout te zien zonder dat de handleiding vertelde dat het veld verplicht was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
Vijf review-agents op de aangescherpte PR. Twee bevindingen waren stille defecten die de wijziging zelf had geïntroduceerd. De prefix-bump nam de RediSearch-index niet mee. De bootstrap laat een bestaande index bewust ongemoeid, dus `berichten-idx` zou op `bericht:v1:` blijven filteren terwijl alle nieuwe hashes onder `bericht:v2:` landen: `_zoeken` en de gefilterde lijst zouden stil nul resultaten geven — HTTP 200 met een lege lijst, geen fout, geen logregel. Draaien er tegelijk nog oude pods, dan komen hún hashes wél uit de index en missen die het nieuwe veld, wat een 500 oplevert die naar datacorruptie wijst in plaats van naar een overgeslagen schema-bump. De index-naam draagt nu dezelfde versie als de prefix (`berichten-idx-v2`), waarmee elke nieuwe pod zijn eigen index aanmaakt en de handmatige drop bij een prefix-wijziging vervalt. Het demo-bedieningspaneel wist sessie-keys met een eigen patroon dat op `:v1:` stond. Na de bump vond dat niets meer en meldde de knop tóch "0 sessie-keys gewist" — een succesmelding zonder effect, met een test die de oude waarde pinde en dus groen bleef. Het patroon is nu versieloos. Verder: - De drift-logs stonden op debug, en `nl.rijksoverheid.moz` staat buiten dev/test op INFO: in productie produceerden ze niets. Ze staan nu op warn, gededupliceerd per magazijnId zodat één gedrift magazijn één regel oplevert in plaats van één per bericht. - `Afzendernamen.naamVoor` nam twee `String`-parameters naast elkaar. Verwisseld gaf de functie het `magazijnId` als naam terug — precies het nummer-als-naam dat deze wijziging wegneemt. De kern is nu private met overloads op `Bericht` en `BerichtSamenvatting`, waarmee de fout niet meer te maken is. - De KDoc claimde dat het `magazijnId` altijd tegen het register gehouden is. Dat geldt voor het aggregatiepad, niet voor het aanmeld-pad: daar is het afgeleid van `data.afzender` uit de payload. Gecorrigeerd. - `MagazijnClientFactory` hield twee parallelle maps (clients en namen) met een `getNaam` die op divergentie een `IllegalStateException` gooide — buiten de lock-cleanup, dus dat zou de hele ophaalronde slopen en de ontvanger daarna twee minuten lang een 409 geven over een niet-bestaande ronde. Client en naam reizen nu samen in één map; de accessor en zijn exception zijn weg. - Een geweigerde aanmelding door eigen config-drift ging als 400 naar de peer zonder enige logregel. Nu een errorf met de OIN. - Testfixture voor magazijn B droeg de naam van magazijn A, waardoor geen enkele assertie een verwisseling kon zien. Tests erbij voor de gaten die de review vond: `afzenderNaam` wordt nu op alle drie de leespaden uit Redis teruggelezen (list-blob, hash, RediSearch-projectie) met per bericht een unieke naam, de projectie-guard noemt het veld, een achtergebleven v1-entry geeft aantoonbaar geen leesfout, de register-miss loopt via een echte HTTP-test, het aanmeld-pad bewijst de geschreven naam in plaats van de leestijd-resolutie, en een E2E pint vast dat een magazijn zich niet als een andere organisatie kan presenteren. `AfzenderNaamContractTest` leest de spec zelf, zodat `required` en `minLength` niet ongemerkt versoepeld kunnen worden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
…in-berichtenlijst # Conflicts: # demo/demo-console/src/main/resources/META-INF/resources/berichtenbox.js # libraries/fbs-berichtensessiecache/src/main/kotlin/nl/rijksoverheid/moz/fbs/berichtensessiecache/berichten/BerichtensessiecacheService.kt # libraries/fbs-berichtensessiecache/src/main/kotlin/nl/rijksoverheid/moz/fbs/berichtensessiecache/berichten/MagazijnEvent.kt # libraries/fbs-berichtensessiecache/src/test/kotlin/nl/rijksoverheid/moz/fbs/berichtensessiecache/berichten/BerichtensessiecacheServiceTest.kt # libraries/fbs-berichtensessiecache/src/test/kotlin/nl/rijksoverheid/moz/fbs/berichtensessiecache/berichten/MagazijnEventTest.kt # libraries/fbs-berichtensessiecache/src/test/kotlin/nl/rijksoverheid/moz/fbs/berichtensessiecache/magazijn/MagazijnCircuitBreakerTest.kt
Het wire-type dat een magazijn levert draagt geen weergavenaam — die komt uit het register. De bulk-aanpassing van de fixtures zette het veld er per ongeluk toch op, en de merge-commit legde die staat vast omdat er vóór de correctie al gestaged was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694
Contributor
🚀 Preview DeploymentDe preview-omgeving van deze PR: Demo
Berichtenuitvraag
Deze preview wordt opgeruimd zodra de PR sluit. |
ericwout-overheid
marked this pull request as ready for review
September 4, 2026 12:31
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.
De berichtenlijst gaf van de afzender alleen een nummer van twintig cijfers terug:
magazijnIdenafzenderbevatten allebei de afzender-OIN. De leesbare organisatienaam kwam uitsluitend voorbij in de voortgangsmeldingen van een ophaalronde. Een afnemer moest dus eerst álle aangesloten organisaties laten bevragen om een leesbare lijst te kunnen tonen, en een bericht dat via de aanmeld-webhook binnenkwam van een organisatie die niet meedeed hield een rij cijfers — die een schermlezer cijfer voor cijfer voorleest.Wat er verandert
BerichtSamenvattingenBerichtdragen nuafzenderNaam. De nieuweAfzendernamen-bean zoekt die per bericht op in het magazijnregister bij hetmagazijnId. Omdat het register de bron is en niet de ophaalronde, draagt een aangemeld bericht dezelfde naam als een opgehaald bericht en hoeft een afnemer niets tussen aanroepen door te onthouden.Het veld is optioneel. Kent het register geen naam voor die organisatie, dan ontbreekt
afzenderNaamin het antwoord; daaraan herkent een afnemer dát er geen naam is en bepaalt hij zelf wat hij toont. Terugvallen op hetmagazijnIdzou het probleem onzichtbaar maken in plaats van oplossen.De demo-berichtenbox leest het veld nu rechtstreeks uit de lijst; de per-zitting bewaarde namen-map uit de ophaal-events is daarmee overbodig en verwijderd. Ontbreekt de naam, dan toont de box "Onbekende organisatie" in plaats van de OIN.
Breaking change voor afnemers
afzendervervalt inBerichtSamenvattingenBerichtvan de uitvraag-API. Dat veld droeg dezelfde OIN alsmagazijnIden was daarmee precies het nummer dat zich als naam voordoet. Afnemers lezen voortaanafzenderNaamvoor de naam enmagazijnIdvoor het nummer. Voor de berichtenbox uit de proeftuin betekent dat: het veld tonen zoals het binnenkomt, en bij afwezigheid zelf bepalen wat er in de kolom komt.Het
afzender-veld inAangemeldBerichtDatablijft ongewijzigd — dat is het inkomende magazijn-contract, waar het veld een OIN hoort te zijn en als zodanig gedocumenteerd staat.Ontwerpkeuze
Twee richtingen lagen open: de naam als eigen veld naast
magazijnId, of een eigen endpoint dat het register uitleest. Het veld wint op alle vier de gedragseisen uit het issue: geen extra ronde vóór de eerste lijst, meteen goed voor een aangemeld bericht van een nog niet bevraagde organisatie, en geen namen die de afnemer zelf moet onthouden. De kosten — de naam staat per bericht in het antwoord — zijn een paar tientallen bytes op een bericht dat in de praktijk kilobytes groot is. De afweging staat uitgeschreven indocs/plans/2026-09-04-afzendernaam-in-de-berichtenlijst.md.Tests
AfzendernamenTest: per organisatie de eigen naam (parameterized over leeg/één/meerdere inschrijvingen, zodat de lookup aantoonbaar discrimineert), ingeschreven zonder naam, niet-ingeschreven, en eenmagazijnIddat geen geldige OIN is.UitvraagDtoMapperTestenBerichtenlijstServiceTest: mét en zónder bekende naam; twee berichten uit verschillende magazijnen in één lijst bewijzen dat de naam per bericht wordt opgezocht.ServiceCoverageTest: lijst én detail over HTTP, met naam en met een ontbrekend veld.AanmeldResourceTest: een via de webhook aangemeld bericht draagt in de lijst de naam van zijn organisatie, zonder dat er een ophaalronde is gedraaid../mvnw clean verify -pl services/berichtenuitvraag -amis groen (267 tests), JaCoCo-gate gehaald, detekt zonder bevindingen, geen nieuwe build-warnings../mvnw clean test -pl demo/demo-console -amgroen. De Spectral ADR-lint geeft alleen de twee bevindingen die al opmainstonden.Closes MinBZK/MijnOverheidZakelijk#1065
🤖 Generated with Claude Code
https://claude.ai/code/session_01RC1C3fjqKGSKAzVHQko694