Repository navigation
Add GOPACSHandler day-ahead UFTP flow tests - #42
Conversation
…he example message format
There was a problem hiding this comment.
Pull request overview
Adds isolated Spock-based test coverage for the GOPACS UFTP day-ahead message flow on the EMS/AGR side by driving real signed UFTP v3.0.0 XML payloads through GOPACSHandler.processRawMessage, and introduces a test-support constructor to wire collaborators without container/OAuth/JAX-RS setup.
Changes:
- Add
GOPACSHandlerTestcovering FlexRequest → (FlexRequestResponse + FlexOffer), FlexOfferResponse handling, FlexOrder flows, and out-of-scope congestion point dropping. - Add a protected test-support constructor on
GOPACSHandlerto allow exercising message handling without container wiring and remote configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ems/src/test/groovy/org/openremote/extension/ems/manager/gopacs/GOPACSHandlerTest.groovy |
New Spock spec with signed-message fixtures and assertions for day-ahead GOPACS/UFTP flows. |
ems/src/main/java/org/openremote/extension/ems/manager/gopacs/GOPACSHandler.java |
Adds a test-support constructor to instantiate the handler with injected services/executor/private key and without deployment/remote config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Will these tests also find any regressions that might occur due to the changes in #43? |
|
@wborn Not really, and that's intentional. These tests stub the OpenRemote collaborators ( |
richturner
left a comment
richturner
left a comment
There was a problem hiding this comment.
I have concerns that this doesn't verify any of the HTTP request/response chain, it should at least follow the same structure as other HTTP tests including the ENTSOE one in this repo; I asked Codex for thoughts on this also:
Findings
- Medium: inbound HTTP endpoint is not covered. The tests call handler.processRawMessage(...) directly in GOPACSHandlerTest.groovy, while production traffic enters via POST /gopacs/message through GOPACSHandler.java and GOPACSServerResource.java. This leaves the deployed path, accepted media types, JAX-RS delegation, and WebApplicationException status mapping in GOPACSHandler.java untested. A path/annotation regression could pass the current suite while real GOPACS POSTs fail.
- Medium: outbound HTTP delivery is bypassed. The production sender goes through uftpSendMessageService.attemptToSendMessage(...) in GOPACSHandler.java, but the test subclass overrides notifyNewOutgoingMessage and only records payload objects in GOPACSHandlerTest.groovy. That means no test asserts that signed XML is actually posted to the broker endpoint, that the recipient role/domain is resolved correctly, or that the Authorization header is attached.
- Medium: OAuth and address-book HTTP calls are not exercised. The test constructor sets gopacsAuthResource, gopacsAddressBookResource, and client to null in GOPACSHandler.java, and the test pre-seeds participants in GOPACSHandlerTest.groovy. So fetchBearerToken() and getParticipantInformation() in GOPACSHandler.java / GOPACSHandler.java are unprotected: form params, bearer parsing, 404 handling, lookup failures, participant caching, and synthesized broker URL are all outside this PR’s tests.
Suggested Improvement
Add a focused HTTP-level test class alongside the current message-flow unit test. The current tests are useful for UFTP semantics; the missing layer is the actual HTTP contract.
A pragmatic route is to use the existing RESTEasy filter style already used in EntsoeProtocolTest.groovy, or add a small stub server dependency such as WireMock/MockWebServer.
Minimum scenarios I’d add:
- POST /gopacs/message with signed XML and application/xml returns success and triggers the same flow as the direct processRawMessage test.
- Malformed XML or invalid signature returns 400, covering the UftpConnectorException branch.
- A valid out-of-scope message returns success but causes no asset mutation and no outbound HTTP POST.
- Outbound FlexRequestResponse/FlexOffer/FlexOrderResponse produces actual broker POSTs to /shapeshifter/api/v3/message with signed XML and Authorization: Bearer ....
- OAuth/address-book failure cases: token 500, invalid token JSON, participant 404, and successful lookup caching.
|
Yes I agree it would be nicer if there are more integration like tests using real container services instead of mocks that mimic the current behavior. |
The GOPACS broker delivery goes through java.net.http.HttpClient (via shapeshifter's UftpSendMessageService), which a JAX-RS ClientRequestFilter cannot intercept. WireMock (standalone, shaded to avoid clashing with the manager's web stack) provides a local HTTP server to stub and verify the broker, OAuth2 token and address-book endpoints on the wire -- the same approach shapeshifter uses to test UftpSendMessageService.
Complements the in-process GOPACSHandlerTest (which calls processRawMessage directly and records outgoing payload objects) by exercising the real transport chain end to end, addressing the HTTP coverage gap raised in review: - Inbound: real POST to the deployed /gopacs/message JAX-RS resource, asserting accepted media type, JAX-RS delegation and the WebApplicationException status mapping (success vs 400). - Outbound: real broker delivery via UftpSendMessageService (over java.net.http.HttpClient), plus the OAuth2 token and address-book lookups via the handler's RESTEasy client. All three are pointed at one WireMock server, so the signed XML, synthesised broker endpoint and 'Authorization: Bearer' header are verified on the wire. Covers: in-scope FlexRequest -> FlexRequestResponse + FlexOffer broker delivery; FlexOrder -> FlexOrderResponse; out-of-scope drop (accepted, no delivery); malformed XML and signature-mismatch -> 400; OAuth2 500 / invalid token JSON and address-book 404 -> 400; and participant-lookup caching. Like the other manager-container integration tests in this repo (e.g. EntsoeProtocolTest) it requires a running OpenRemote dev stack and is skipped on CI via @IgnoreIf(GITHUB_ACTIONS).
|
Added It POSTs real signed messages to the deployed Note: the broker send uses |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GOPACS flex messages carry the congestion point with an "ean." scheme prefix (e.g. "ean.265987182507322951"), while the contracted EAN is configured on the asset as the bare code. The exact equals() comparison in isWithinContractedScope therefore never matched, so in-scope flex messages were rejected as out-of-scope and silently dropped. Normalise both sides by stripping the optional "ean." prefix before comparing, making the scope check agnostic to whether either value includes it.
GOPACS flex messages carry the congestion point in the canonical format
ean.[0-9]{18} (e.g. "ean.265987182507322951"), while the contracted EAN
is configured on the asset as the bare code. The exact equals() comparison
in isWithinContractedScope therefore never matched, so in-scope flex
messages were rejected as out-of-scope and silently dropped.
Canonicalise both sides to the documented ean.<code> congestion-point
format before comparing, so the scope check is correct whether or not the
contracted EAN is configured with the prefix.
Brings the toCongestionPoint EAN canonicalisation so it can be unit tested.
Covers toCongestionPoint directly (prefix present/absent, mixed case, surrounding whitespace, empty and null), and adds an end-to-end scope test proving a FlexRequest is accepted when the contracted EAN is configured without the "ean." prefix while the message carries the canonical ean.<code> congestion point. Extracts a newHandler() helper so a second handler with a different contracted EAN can be built from the shared keypair.
|
@richturner @wborn ready to be reviewed again |
|
@Miggets7 according to @igorrutka GOPACS is no longer working so I guess this urgently needs to be deployed? |
|
@richturner this is a separate PR for only the tests, not the fix needed |
Summary
Adds test coverage for the
GOPACSHandlerday-ahead capacity-steering message flow on the AGR (EMS) side, closing the gap left by the UFTP V2→V3 participant migration (#40). Theemsmodule previously had no tests for the GOPACS handler.The spec drives real signed UFTP messages through the handler's actual entry point (
processRawMessage) using libsodium crypto — no database, no container, no network. Inbound payloads are embedded as XML fixtures matching the real wire format (DSOnilsgrid.net→ AGRopenremote.io, v3.0.0, attribute-style). A small test-support constructor onGOPACSHandlerskips remote config (OAuth client / private-key file) and JAX-RS deployment; aRecordingGOPACSHandlersubclass captures outbound messages (both the library-generated responses and the handler-builtFlexOffer); scheduled work runs inline for deterministic assertions.Covered flows:
powerMaximumFlexRequest= importMax,powerMinimumFlexRequest= exportMax); replies withFlexRequestResponse(Accepted) thenFlexOffer. The emittedFlexRequestResponseis asserted to match the reference message shape (result, referenced request id, swapped domains, version, conversation id).currentPowerFlexRequest, plus the offtakepowerLimitMaximumProfileFlexOrder/ feed-inpowerLimitMinimumProfileFlexOrderprofile); replies withFlexOrderResponse(Accepted).CongestionPointdiffers from the contracted EAN is dropped: no asset mutation, no reply (the V3 re-scoping).