Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the generated XML model usage for ISO18626 (and a small NCIP sample/test) to stop relying on utils.PrefixAttr-based namespace/prefix handling and instead use namespaced codegen output directly across the broker + illmock codepaths.
Changes:
- Switch from
Iso18626MessageNS/NewIso18626MessageNS()toISO18626Message/NewISO18626Message()across broker, illmock, shims, and tests. - Update ISO18626 schema generation settings (xsd2goxsl v1.5.1 +
namespaced=yes, explicit root, schemaLocation). - Adjust NCIP sample XML and marshal test to include a scheme attribute on
ToAgencyId/AgencyId.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ncip/testdata/ncip_sample.xml | Updates sample XML to include a ncip:Scheme attribute on ToAgencyId/AgencyId. |
| ncip/ncip_gen_test.go | Updates marshal test to populate ToAgencyId/AgencyId scheme+text and match sample output formatting. |
| iso18626/models_ns.go | Removes the custom namespace wrapper and introduces NewISO18626Message() returning the generated root type. |
| iso18626/models_ns_test.go | Reworks namespace/version assertions and JSON roundtrip checks for the new generated type/prefixing. |
| iso18626/Makefile | Updates xsd2goxsl version and enables namespaced/root/schemaLocation generation options. |
| illmock/reqform/reqform_test.go | Updates ISO18626 message unmarshal type used by reqform tests. |
| illmock/flows/flows.go | Updates FlowMessage to embed ISO18626Message rather than the old wrapper type. |
| illmock/flows/flows_test.go | Updates tests to construct messages via NewISO18626Message(). |
| illmock/app/supplier.go | Updates supplier handlers/helpers to use the new ISO18626 message type. |
| illmock/app/requester.go | Updates requester handlers/helpers to use the new ISO18626 message type. |
| illmock/app/app.go | Updates core illmock handler plumbing and removes now-obsolete iso18626.InitNs() call. |
| illmock/app/app_test.go | Updates helper signatures and test message construction to the new message type. |
| broker/test/service/supplierlocator_test.go | Updates tests to construct messages via NewISO18626Message(). |
| broker/test/service/e2e_test.go | Updates XML unmarshalling targets to ISO18626Message. |
| broker/test/handler/iso18626-handler_test.go | Updates handler test helper + unmarshal target type. |
| broker/test/client/client_test.go | Updates client tests to use ISO18626Message and adjusts mock XML root namespace. |
| broker/test/api/sse_broker_test.go | Updates SSE test message creation to the new ISO18626 message type. |
| broker/shim/shim.go | Updates shim interfaces and implementations to accept/return the new message type. |
| broker/shim/shim_test.go | Updates shim tests to build/unmarshal ISO18626Message. |
| broker/service/workflow.go | Updates helper signature to accept the new ISO18626 message type. |
| broker/service/workflow_test.go | Updates workflow tests to construct messages via NewISO18626Message(). |
| broker/patron_request/service/message-handler.go | Updates message handler signatures and response construction to the new message type. |
| broker/patron_request/service/message-handler_test.go | Updates message handler tests to use the new message constructor/type. |
| broker/patron_request/service/action.go | Updates action service message creation/capture to the new message type. |
| broker/patron_request/service/action_test.go | Updates mock ISO handler and test responses to the new message type. |
| broker/handler/iso18626-handler.go | Updates HTTP handler plumbing, response creation, and shim application to the new message type. |
| broker/handler/iso18626-handler_test.go | Updates handler unit tests to use NewISO18626Message() and new type assertions. |
| broker/events/eventmodels.go | Updates event payload message pointers to *ISO18626Message. |
| broker/client/client.go | Updates client send/receive logic and type assertions to ISO18626Message. |
| broker/client/client_test.go | Updates client unit tests to use NewISO18626Message(). |
| broker/api/sse_broker.go | Updates SSE event payload to embed ISO18626Message as the data model. |
adamdickmeiss
approved these changes
Apr 9, 2026
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
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.
No description provided.