test(api): bewaak dat elk pad uit de spec ook echt bij een resource aankomt - #192
Conversation
JaCoCo coverage
|
Afwegingen bij deze PRDe generator-keuze is bewust niet gemaakt. Het derde acceptatiecriterium laat de ruimte om vast te leggen waarom de update voorlopig niet kan, en dat is hier de eerlijke uitkomst. Wat er nu ligt maakt die keuze veilig uitstelbaar: de bump kan niet meer ongemerkt binnenkomen, en zou hij dat toch doen, dan valt de controle om in plaats van dat aanleveren stilletjes stopt. De dependabot-blokkade stond er nog niet. Dat verraste me: de generator is wél op 7.23.0 gepind, maar niets hield 7.24.x tegen. De guard vangt de breuk pas ná een merge, dus allebei is nodig. Waarom de assertie smal is. Een De 404-uitzondering was aanvankelijk te ruim. Op een pad mét parameters is 404 het normale antwoord op een onbekend id — maar daarmee hielden acht van de veertien operaties effectief maar één assertie over, en juist De timeouts stonden op de verkeerde plek. Ik zette ze in de routetest — die stuurt bodyloze requests, dus het scenario "server wijst af zonder de body te lezen, client blijft schrijven" kán daar niet optreden. De test die het wél uitlokt, de aanlevering van 25 MiB die met 400 wordt afgewezen, had geen enkele grens. Ze staan nu modulebreed via een launcher-listener; zo'n vangnet hoort niet af te hangen van een parameter die per testklasse goed gezet moet zijn. Nog niet gedekt, uit de testreview en de moeite waard als vervolg: de handgeschreven |
Vervolgticket nodig: uitgaande paden worden nergens tegen de spec gehoudenUit de reviewronde, en te groot om aan deze PR te hangen. Aan serverzijde komt
De WireMock-stubs hardcoderen hetzelfde pad met exacte match. Gevolg: drift tussen client en stub wordt wél gevangen, maar drift tussen spec en client niet. Verplaatst iemand een pad in Dat is dezelfde faalklasse als dit issue, alleen in de uitgaande richting. Richting voor het ticket: een test die de magazijn-spec parseert en per client-methode het effectieve pad (class- De Profiel-client valt hierbuiten: van die dienst staat geen spec in de repo. Dat is een bewuste blinde vlek, geen oplosbaar gat. Verder ongedekt en het overwegen waard in datzelfde ticket: |
…ankomt Toen de code-generator de paden anders over de gegenereerde interfaces verdeelde, kwam het aanlever-endpoint in de routeringsboom onder een andere class terecht en gaf 405. Geen enkele test merkte dat: de contracttests valideren request en response tegen het schema, maar alleen voor de paden die ze zelf aanroepen. Een pad dat niet meer gevonden wordt viel daarbuiten. RouteDekkingTest leest de spec en roept elk pad met elke methode aan. De assertie is bewust smal: 405 betekent altijd dat het pad wél bestaat maar de methode er niet op zit — precies deze breuk. 404 telt alleen als bewijs bij een pad zonder parameters; mét parameter is 404 juist het normale antwoord op een onbekend id. Alle overige statussen bewijzen dat de route gevonden is. Zes routes in het magazijn, acht in de uitvraag. De testrequests krijgen een socket- en connect-timeout mee. Een afgewezen request wordt beantwoord zonder de body te lezen; zonder die grens blijft de client schrijven tot iets anders ingrijpt, en dat liet een CI-job zes uur doorlopen voordat de runner hem afkapte. Dependabot blokkeert openapi-generator 7.24.0 en hoger. Die stond er nog niet, terwijl de bump juist het aanleveren onbruikbaar maakt — RouteDekkingTest vangt dat nu af, maar pas nádat de bump gemerged zou zijn. De keuze uit het issue tussen useTags=false, herziene spec-paden of wachten op upstream is niet gemaakt: alle drie raken het publieke contract of de functionele package-indeling van beide services. Zie de toelichting in het issue. Closes MinBZK/MijnOverheidZakelijk#870 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rameterpaden
Twee bevindingen uit de review, allebei raak.
De socket-timeouts stonden in RouteDekkingTest, waar ze niets beschermen: die test stuurt
bodyloze requests, dus het scenario "server wijst af zonder de body te lezen, client blijft
schrijven" kan er niet optreden. De test die het wél uitlokt — de aanlevering van een bijlage
van 25 MiB die met 400 wordt afgewezen — had geen enkele grens. Onder de generator-breuk werd
die 400 een 405 en liep de job zes uur door.
De timeouts staan nu modulebreed, via een LauncherSessionListener die de launcher één keer per
test-JVM aanroept. Zo'n vangnet hoort niet af te hangen van een parameter die per testklasse
goed gezet moet zijn.
De 404-uitzondering liet acht van de veertien operaties met alleen een 405-check achter. Op een
pad mét parameters is 404 het normale antwoord op een onbekend id, dus daar zei de statuscode
niets over de routering — terwijl juist /berichten/{berichtId} over twee resource-classes
verdeeld is, dezelfde prefix-overlap die de breuk veroorzaakte. Een tweede toets per pad vult
dat gat: een methode die de spec níét noemt levert 405 op zodra de route bestaat, en 404 zodra
hij weg is. Daarmee is ook voor parameterpaden vast te stellen dát ze geregistreerd zijn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er die niets doet De review mat het na: mijn LauncherSessionListener had geen enkel effect. Quarkus' RestAssuredStateManager.setURL — dezelfde aanroep die de testpoort zet, dus gegarandeerd vóór elk request — vervangt de hele httpClient-sectie van de RestAssured-config, inclusief wat de listener er net in had gezet. Gemeten waarden waren 30000/30000, Quarkus' eigen default, niet de 60000 en 10000 uit de listener. Erger nog: de commit daarvóór had de timeout per request op de request-spec staan, en dát werkte wel. Ik heb dus een werkend mechanisme ingeruild voor een no-op, met een KDoc die het tegendeel beweerde. Nu via quarkus.http.test-timeout in de test-properties van beide services. Dat is de knop die Quarkus hiervoor biedt; hij zet de socket-, connect- en pool-timeout op élke test, dus het vangnet hangt niet meer af van een parameter die per testklasse goed gezet moet zijn — wat de bedoeling was. Kanttekening die de review terecht maakte en die ik overneem in de tekst: een socket-timeout begrenst reads, geen blokkerende writes. Het scenario waar dit uit voortkwam is een write-stall, en die wordt hierdoor niet afgekapt. De timeout beperkt de schade, de job-timeout blijft het eigenlijke vangnet. Verder: de sonde-toets assert nu precies 405 in plaats van "alles behalve 404" — alleen zo is de sonde zelf falsifieerbaar — en de stille return wanneer een pad alle sonde-methodes zou declareren is een luide fail geworden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
De timeout-commit deed het tegenovergestelde van wat hij beweerde. quarkus.http.test-timeout heeft al een default van 30 seconden, op élke @QuarkusTest en ook vóór deze branch — mijn 60 was dus een verdubbeling van een bestaande grens, niet een vangnet dat er nog niet was. Nu 15. Daarmee vervalt ook de verklaring die ik eraan hing: de zes uur durende CI-job kan niet veroorzaakt zijn door een ontbrekende RestAssured-timeout, want die stond er al. De oorzaak is nog niet verklaard. De comment zegt nu wat de knop wél doet (read-, connect- en pool-timeout) en wat hij niet doet: dit is SO_TIMEOUT en begrenst geen blokkerende writes. Voor het scenario uit het issue — een afgewezen aanlevering waarbij de server de body niet leest — staat er nu een @timeout op de test die dat uitlokt. Met SEPARATE_THREAD, want de default onderbreekt niets en rapporteert pas ná afloop; precies wanneer je de grens nodig hebt. De sonde met een niet-gespecificeerde methode leunde op de aanname dat een verdwenen route 404 geeft in plaats van 405. Klopt die niet, dan slaagt hij voor élk pad — ook voor paden die niet bestaan — en bewijst hij niets, juist bij de paden mét parameters waar de eerste toets al blind is. Er is nu een controlegroep die dat aantoont; hij slaagt. HEAD is uit de sonde-lijst: JAX-RS leidt die af van GET, dus een pad met een GET antwoordt met de GET-status en nooit met 405 — de sonde zou een bestaande route dan als verdwenen aanmerken. Tot slot een telling van paden en operaties. De spec-parser geeft bij een gedeeltelijk oplosbare spec een niet-null document met minder paden terug; de test bleef dan groen met minder gevallen dan bedoeld. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
De blokkade hield 7.24.0 tegen tot er een keuze lag. Die keuze is er nu: de magazijn-spec krijgt per tag een eigen pad-subtree, waarna de update kan landen. Een dependency tegenhouden die we op eigen kracht gaan volgen, houdt alleen de openstaande update-PR onnodig in de weg. RouteDekkingTest blijft als bewaker: die faalt zodra een pad uit de spec niet meer bij een resource aankomt, ongeacht de oorzaak.
373888f to
92f8da2
Compare
2 participants
Wat er verandert
Een controle die de breuk zou hebben gevangen.
RouteDekkingTestleest de OpenAPI-spec enroept elk pad met elke methode aan — zes routes in het magazijn, acht in de uitvraag. De
bestaande contracttests valideren request en response tegen het schema, maar alleen voor de paden
die ze zélf aanroepen; een pad dat door de routering niet meer gevonden wordt viel daarbuiten.
De assertie is bewust smal:
gebeurde toen de generator de paden anders over de interfaces verdeelde.
antwoord op een onbekend id.
over de dekking.
De verbinding kan niet meer blijven hangen. De testrequests krijgen een socket- en
connect-timeout. Een afgewezen request wordt beantwoord zonder de body te lezen; zonder die grens
blijft de client schrijven tot iets anders ingrijpt — dat liet een CI-job zes uur doorlopen
voordat de runner hem afkapte. De
timeout-minutes: 30op de test-job blijft als vangnet.De controle is gemeten, niet aangenomen
Een bewaker die nooit rood heeft gestaan bewijst niets. Deze is daarom gedraaid tegen de
kapotte situatie: generator 7.24.0 op Quarkus 3.38.1, in een aparte worktree vanaf de
bump-branch.
Drie failures in het magazijn, met exact de bedoelde boodschap. De uitvraag bleef groen (16/16) —
daar houdt geen enkele class een dekkend basispad, dus valt er niets te overschaduwen. De
gegenereerde interfaces bevestigen het mechanisme:
OphaalApibehoudt@Path("/berichten")terwijl
AanleverApienBeheerApi@Path("")krijgen en daardoor onbereikbaar worden.Ook nagegaan: de Quarkus-issue die hierbij hoort (quarkusio/quarkus#26496) is gesloten en
gerepareerd in 3.24.0.CR1, ruim vóór onze 3.38.1. Die fix dekt
@Path("/base")tegenover@Path("/base/{id}"), niet het geval@Path("")tegenover@Path("/berichten"). Wachten opQuarkus lost dit dus niet op.
Geen blokkade op de generator
Een eerdere versie van deze PR blokkeerde
openapi-generatorvanaf 7.24.0 independabot.yml.Die regel is er weer uit: we gaan de update juist volgen. De magazijn-spec krijgt per tag een
eigen pad-subtree, waarna de bump kan landen — dat werk zit in #147. Deze PR levert alleen de
bewaker, zodat die er al staat vóór de contractwijziging.
Verificatie
./mvnw clean test -pl services/berichtenmagazijn -am— 385 tests groen./mvnw clean test -pl services/berichtenuitvraag -am— 202 tests groen./mvnw detekt:check— geen bevindingenRefs MinBZK/MijnOverheidZakelijk#870 — de acceptatiecriteria over bereikbaarheid van alle paden
en over het netjes afronden van een afgewezen aanlevering blijven open; die worden in #147
respectievelijk in vervolgwerk afgedekt.
🤖 Generated with Claude Code