Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Address the missing date validation, Swagger annotations, resolver tests, and catalogus expansion N+1 query path.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds ZTC 1.3.3 catalogus expansion for zaaktypen, besluittypen, and informatieobjecttypen, including propagation to Documenten v1.7.
Changes:
- Added expand-aware contracts, validation, mappings, controllers, and version metadata.
- Added catalogus resolution and service-agent forwarding.
- Updated nested expands, caching, and shared expand utilities.
File summaries
| File | Summary |
|---|---|
src/OneGround.ZGW.Documenten.Web/Startup.cs |
Registers the shared Catalogi expand decorator. |
src/OneGround.ZGW.Documenten.Web/Expands/v1/7/InformatieObjectTypeResolver.cs |
Forwards catalogus expansion to ZTC. |
src/OneGround.ZGW.Documenten.Web/Expands/v1/7/InformatieObjectTypeCatalogusResolver.cs |
Reads embedded catalogus data. |
src/OneGround.ZGW.Documenten.Web/Expands/v1/7/ICatalogiServiceAgentDecorator.cs |
Removes the obsolete local decorator contract. |
src/OneGround.ZGW.Documenten.Web/Expands/v1/7/ExpandsServiceCollectionExtensions.cs |
Updates resolver registrations. |
src/OneGround.ZGW.Documenten.Web/Expands/v1/7/EnkelvoudigInformatieObject_InformatieObjectType_Resolver.cs |
Forwards nested catalogus expansion. |
src/OneGround.ZGW.Documenten.Web/Controllers/v1/7/VerzendingenController.cs |
Updates the exception namespace. |
src/OneGround.ZGW.Documenten.Web/Controllers/v1/7/ObjectInformatieObjectenController.cs |
Updates the exception namespace. |
src/OneGround.ZGW.Documenten.Web/Controllers/v1/7/GebruiksRechtenController.cs |
Updates the exception namespace. |
src/OneGround.ZGW.Documenten.Web/Controllers/v1/7/EnkelvoudigInformatieObjectenController.cs |
Updates the exception namespace. |
src/OneGround.ZGW.Common.ServiceAgent/Expands/ExpandExternalServiceException.cs |
Moves the shared exception. |
src/OneGround.ZGW.Common.ServiceAgent/Caching/CachingConfiguration.cs |
Includes query strings in cache keys. |
src/OneGround.ZGW.Common.Contracts/Extensions/IExpandableExtensions.cs |
Converts embedded expand values. |
src/OneGround.ZGW.Catalogi.Web/Validators/v1/3/Queries/ZaakTypenQueryParametersValidator.cs |
Moderate (1 vote): DatumGeldigheid is not validated, allowing invalid values to fall back to today’s date. |
src/OneGround.ZGW.Catalogi.Web/Validators/v1/3/Queries/InformatieObjectTypenQueryParametersValidator.cs |
Adds v1.3 query validation. |
src/OneGround.ZGW.Catalogi.Web/Startup.cs |
Registers catalogus expand services. |
src/OneGround.ZGW.Catalogi.Web/MappingProfiles/v1/3/RequestToDomainRegister.cs |
Adds query-to-domain mappings. |
src/OneGround.ZGW.Catalogi.Web/MappingProfiles/v1/3/DomainToResponseRegister.cs |
Prevents mapping expand state. |
src/OneGround.ZGW.Catalogi.Web/Extensions/QueryParameterValidationExtensions.cs |
Registers v1.3 validation filters. |
src/OneGround.ZGW.Catalogi.Web/Expands/v1/3/ExpandsServiceCollectionExtensions.cs |
Registers expand engines and caches. |
src/OneGround.ZGW.Catalogi.Web/Expands/v1/3/Catalogus_Resolver.cs |
Moderate (1 vote): list expansion can cause an N+1 query pattern. Nit (1 vote): add focused mapping, error, and cache-reuse tests. |
src/OneGround.ZGW.Catalogi.Web/Controllers/v1/3/ZaakTypeController.cs |
Nit (2 votes): add [Expand] so Swagger documents expand for the relevant actions. |
src/OneGround.ZGW.Catalogi.Web/Controllers/v1/3/InformatieObjectTypeController.cs |
Nit (2 votes): add [Expand] so Swagger documents expand for the relevant actions. |
src/OneGround.ZGW.Catalogi.Web/Controllers/v1/3/BesluitTypeController.cs |
Nit (2 votes): add [Expand] to the HEAD action for Swagger documentation. |
src/OneGround.ZGW.Catalogi.Web/Controllers/Api.cs |
Adds version 1.3.3 support. |
src/OneGround.ZGW.Catalogi.ServiceAgent/v1/3/ICatalogiServiceAgent.cs |
Adds expand parameters to resource calls. |
src/OneGround.ZGW.Catalogi.ServiceAgent/v1/3/Expands/ICatalogiServiceAgentDecorator.cs |
Adds the shared decorator interface. |
src/OneGround.ZGW.Catalogi.ServiceAgent/v1/3/Expands/CatalogiServiceAgentDecorator.cs |
Adds expand forwarding. |
src/OneGround.ZGW.Catalogi.ServiceAgent/v1/3/CatalogiServiceAgent.cs |
Sends expand query parameters. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Responses/ZaakTypeResponseDto.cs |
Adds expandable response state. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Responses/InformatieObjectTypeResponseDto.cs |
Updates expand documentation. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Responses/BesluitTypeResponseDto.cs |
Adds expandable response state. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetZaakTypeQueryParameters.cs |
Adds single-resource expand parameters. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetInformatieObjectTypeQueryParameters.cs |
Adds single-resource expand parameters. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetBesluitTypeQueryParameters.cs |
Adds single-resource expand parameters. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetAllZaakTypenQueryParameters.cs |
Adds zaaktypen filters and expands. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetAllInformatieObjectTypenQueryParameters.cs |
Adds informatieobjecttypen filters and expands. |
src/OneGround.ZGW.Catalogi.Contracts/v1/3/Queries/GetAllBesluitTypenQueryParameters.cs |
Adds besluittypen expand support. |
Review details
Suppressed comments (3)
src/OneGround.ZGW.Catalogi.Web/Expands/v1/3/Catalogus_Resolver.cs:68
- This is the core path for all three new
catalogusexpands, but there is no test covering the successful mapping, aNotFound/Forbiddenresult, or reuse of the per-request cache. A regression in the nested scope/query or DTO mapping would therefore only surface through an endpoint call; add focused resolver tests for those cases before relying on this behavior.
private async Task<object> ResolveCatalogusAsync(string catalogusUrl) =>
await _catalogusCache.GetOrCacheAndGetAsync(
$"key_{catalogusUrl}",
async () =>
{
src/OneGround.ZGW.Catalogi.Web/Expands/v1/3/Catalogus_Resolver.cs:68
- For list endpoints this resolver is invoked once per returned type, and the cache only deduplicates identical URLs; a page containing many distinct catalogi therefore opens a scope and executes a separate
GetCatalogusQuery(including three navigation collections) for each item. That creates an N+1 query pattern for the newexpand=cataloguspath; consider preloading the distinct catalogi in one query or adding a batch resolver before resolving the list.
private async Task<object> ResolveCatalogusAsync(string catalogusUrl) =>
await _catalogusCache.GetOrCacheAndGetAsync(
$"key_{catalogusUrl}",
async () =>
{
src/OneGround.ZGW.Catalogi.Web/Validators/v1/3/Queries/ZaakTypenQueryParametersValidator.cs:13
GetAllZaakTypenQueryParametersnow exposesDatumGeldigheidand the mapping parses it, but this validator never validates it. An invalid value is accepted, mapped to null, and the handler then falls back to today's date, silently returning a different result instead of 400. Add the same optional date rule used by the other v1.3 list validators.
CascadeRuleFor(p => p.Catalogus).IsUri();
CascadeRuleFor(p => p.Status).IsEnumName(typeof(ConceptStatus));
- Files reviewed: 38/38 changed files
- Comments generated: 3
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pull Request
Description
Type of Change
Related Issues
Testing
Checklist