Fix PR review bevindingen: error handling, type-validatie en tests#6
Open
ericwout-overheid wants to merge 1 commit intofeature/multi-magazijn-aggregatiefrom
Open
Fix PR review bevindingen: error handling, type-validatie en tests#6ericwout-overheid wants to merge 1 commit intofeature/multi-magazijn-aggregatiefrom
ericwout-overheid wants to merge 1 commit intofeature/multi-magazijn-aggregatiefrom
Conversation
…ng en tests Kritieke fixes: - getBerichtById: onderscheid WebApplicationException(404) van ProcessingException zodat 404 correct doorkomt i.p.v. 502 - BerichtenCache.store: verwijder oude cache-data bij lege berichten-lijst om stale data te voorkomen bij re-fetch - BerichtenOphalenResource: log cache-fout bij initStream i.p.v. stil slikken - BerichtenlijstService: gebruik sealed BerichtLookupResult i.p.v. WebApplicationException in de service-laag (CLAUDE.md: error handling in resource) Belangrijke verbeteringen: - zoekBerichten: voeg 409-guard toe (was inconsistent met getBerichten) - HAL pagination links: URL-encode ontvanger parameter - TimeoutException: vang af en retourneer 503 Problem JSON - compose.yaml + application.properties: voeg magazijn-b toe - MagazijnStatusEvent: init-validatie per event type - AggregationStatus: require(totaalMagazijnen >= 0) - BerichtenPage: init-validatie (page >= 0, pageSize > 0) - MagazijnClientFactory: valideer URLs bij startup - Log backend-failures als ERROR i.p.v. WARN - Gebruik generieke foutmelding i.p.v. class name in SSE events - MockMagazijnClientFactory: gebruik ProcessingException, verwijder ongebruikte alias Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
Geadresseerde review-bevindingen
Kritiek (must-fix)
getBerichtByIdvangt nuWebApplicationException(404)apart op — retourneert correct 404 i.p.v. 502BerichtenCache.store()verwijdert oude cache-data bij lege lijst — voorkomt stale data bij re-fetchinitStreamlogt cache-fouten voordatrecoverWithCompletion()ze opvangt — geen stille failures meerBerichtenlijstServiceretourneertBerichtLookupResultsealed class i.p.v.WebApplicationException— error handling in resource, niet service (CLAUDE.md)Belangrijk
zoekBerichtenheeft nu dezelfde 409-guard alsgetBerichten— consistente lifecycle checkontvangerparameter — voorkomt URI-injectieawait().atMost()vangtTimeoutExceptionaf en retourneert 503 Problem JSONcompose.yaml+application.propertiesconfigureren nu magazijn-a én magazijn-bMagazijnStatusEventvalideert verplichte velden per event type viainitAggregationStatuscontroleerttotaalMagazijnen >= 0BerichtenPagevalideertpage >= 0,pageSize > 0,totalElements >= 0MagazijnClientFactoryvalideert URLs bij startupProcessingExceptioni.p.v.RuntimeException— matcht echt REST client gedragTest plan
docker compose up+./mvnw quarkus:dev🤖 Generated with Claude Code