Skip to content

NRC migration to mapster - #204

Open
ragaumon wants to merge 15 commits into
mainfrom
feature/mapster-nrc-migration
Open

NRC migration to mapster#204
ragaumon wants to merge 15 commits into
mainfrom
feature/mapster-nrc-migration

Conversation

@ragaumon

@ragaumon ragaumon commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

NRC migration from automapper to mapster

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation

Testing

  • Tests pass
  • Manual testing completed

Checklist

  • Self-review completed
  • Documentation updated (if needed)

ragaumon and others added 13 commits August 5, 2026 14:39
@ragaumon @claude
feat(nrc): add Mapster DomainToResponseRegister and convert its tests
e963c11 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
feat(nrc): add Mapster RequestToDomainRegister and convert its tests
691f4d1 
Port the request-to-domain mapping profile to a Mapster IRegister,
including the first AfterMapping use (Kanaal set from source Naam) and
the dictionary->list Filters conversion. AutoMapper profile stays in
place; nothing wires the register into production yet.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
test(nrc): assert nested-element AfterMapping fires for AbonnementKan…
edda9a5 
…alen collection

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
test(nrc): add temporary AutoMapper-vs-Mapster JSON parity guard
96a34bc 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
test(nrc): verify AddZgwMapster discovers NRC's Mapster registers
6baae20 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
docs(nrc): correct factually-wrong comment about Kanaal.Url in wiring…
ba644d9 
… test

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
feat(nrc): route controllers through MapsterMapper.IMapper
4c78ece 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon @claude
refactor(nrc): remove AutoMapper profiles now that NRC runs on Mapster
1b06cc1 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ragaumon
Added mapster extenssion
4abf36f
@ragaumon @claude
fix(nrc): enable Mapster in the wiring test so it exercises the seam
204279f 
AddZgwMapster's enable parameter defaults to false, so the one-argument call
registered nothing and GetRequiredService<IMapper>() threw. Also drop a comment
reference to an AC wiring test that does not exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ragaumon @claude
test(nrc): guard the PATCH merge contract and the controller's merger…
dcf3b5e 
… dependency

The per-register mapping tests build an isolated TypeAdapterConfig and cannot
see the PATCH merge path, so they stayed green while PATCH threw at runtime.

Two facts for two failure modes: the merge fact proves the Mapster register
still serves Abonnement -> AbonnementRequestDto, and the reflection fact proves
the controller depends on the Mapster-backed merger. The latter fails at this
commit, which is the defect it exists to catch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ragaumon @claude
fix(nrc): merge PATCH bodies through the Mapster-backed request merger
fd9faa9 
The AutoMapper-backed IRequestMerger needs an Abonnement -> AbonnementRequestDto
map, which no longer exists now that the profiles are gone, so every PATCH on an
abonnement threw AutoMapperMappingException. IZgwRequestMerger resolves the same
pair from the Mapster register.

IRequestMerger stays in the constructor because ZGWControllerBase still requires
it. The other two controllers call no merger and are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ragaumon @claude
test(nrc): name the mapping tests after registers and assert Kenmerke…
ea137ee 
…n values

No AutoMapper profile exists any more, so *ProfileTests was misleading; other
migrated services already use *RegisterTests.

The second Kenmerken assertion was a verbatim duplicate of the key comparison
and was dropped during the migration. Restore it as the value comparison it was
meant to be.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 6, 2026 06:58
@ragaumon ragaumon added Notificaties API Related to Notificaties (NRC) component Improvement labels Aug 6, 2026
Copilot AI reviewed Aug 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the NRC (Notificaties) service’s runtime mapping from AutoMapper profiles to Mapster registers by enabling the Mapster seam in AddZGWApi, updating controllers to use the DI-provided Mapster mapper, and adding/adjusting unit tests to validate wiring and PATCH-merge behavior.

Changes:

  • Enable Mapster for NRC via ApiServiceSettings.EnableMapster = true and introduce Mapster IRegister mapping configurations.
  • Switch NRC controllers to map request/response models using MapsterMapper.IMapper and route PATCH merging through IZgwRequestMerger.
  • Update/add unit tests to cover register-level mapping behavior, Mapster DI wiring, and the PATCH merge contract path.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/OneGround.ZGW.Notificaties.Web/Startup.cs Enables Mapster seam for NRC via service settings.
src/OneGround.ZGW.Notificaties.Web/MappingProfiles/v1/RequestToDomainRegister.cs Adds Mapster register for request DTO → domain mappings.
src/OneGround.ZGW.Notificaties.Web/MappingProfiles/v1/RequestToDomainProfile.cs Removes AutoMapper profile replaced by Mapster register.
src/OneGround.ZGW.Notificaties.Web/MappingProfiles/v1/DomainToResponseRegister.cs Adds Mapster register for domain → response DTO (and PATCH merge support) mappings.
src/OneGround.ZGW.Notificaties.Web/MappingProfiles/v1/DomainToResponseProfile.cs Removes AutoMapper profile replaced by Mapster register.
src/OneGround.ZGW.Notificaties.Web/Controllers/v1/NotificatiesController.cs Uses Mapster mapper for request DTO → domain mapping.
src/OneGround.ZGW.Notificaties.Web/Controllers/v1/KanaalController.cs Uses Mapster mapper for domain ↔ DTO mappings.
src/OneGround.ZGW.Notificaties.Web/Controllers/v1/AbonnementController.cs Uses Mapster mapper for mappings and IZgwRequestMerger for PATCH merge flow.
src/Tests/OneGround.ZGW.Notificaties.WebApi.UnitTests/MappingTests/RequestToDomainRegisterTests.cs Updates mapping tests to Mapster and strengthens nested mapping assertions.
src/Tests/OneGround.ZGW.Notificaties.WebApi.UnitTests/MappingTests/DomainToResponseRegisterTests.cs Updates mapping tests to Mapster and wires DI to support URL resolution via MapContext.
src/Tests/OneGround.ZGW.Notificaties.WebApi.UnitTests/MappingTests/NrcMapsterWiringTests.cs Adds test ensuring AddZgwMapster discovers NRC registers and resolves URLs via DI.
src/Tests/OneGround.ZGW.Notificaties.WebApi.UnitTests/MappingTests/NrcMapperContractTests.cs Adds contract tests guarding the Mapster-backed mapper/merger wiring for PATCH merge.
Suppressed comments (1)

src/Tests/OneGround.ZGW.Notificaties.WebApi.UnitTests/MappingTests/RequestToDomainRegisterTests.cs:93

  • The Kenmerken assertions compare dictionary enumeration order (Select(k => k.Key/Value)), which is not guaranteed and can make the test flaky if source/destination dictionaries enumerate differently. Prefer an order-independent check that verifies every expected key/value exists in the mapped result.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings August 6, 2026 11:52
Copilot AI reviewed Aug 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 6, 2026 12:49
Copilot AI reviewed Aug 6, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Improvement Notificaties API Related to Notificaties (NRC) component

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ragaumon