Skip to content

Add GOPACSHandler day-ahead UFTP flow tests#42

Open
Miggets7 wants to merge 10 commits into
mainfrom
test/gopacs-handler-dayahead-flows
Open

Add GOPACSHandler day-ahead UFTP flow tests#42
Miggets7 wants to merge 10 commits into
mainfrom
test/gopacs-handler-dayahead-flows

Conversation

@Miggets7
Copy link
Copy Markdown
Contributor

@Miggets7 Miggets7 commented Jun 3, 2026

Summary

Adds test coverage for the GOPACSHandler day-ahead capacity-steering message flow on the AGR (EMS) side, closing the gap left by the UFTP V2→V3 participant migration (#40). The ems module 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 (DSO nilsgrid.net → AGR openremote.io, v3.0.0, attribute-style). A small test-support constructor on GOPACSHandler skips remote config (OAuth client / private-key file) and JAX-RS deployment; a RecordingGOPACSHandler subclass captures outbound messages (both the library-generated responses and the handler-built FlexOffer); scheduled work runs inline for deterministic assertions.

Covered flows:

  • FlexRequest → asset updated from request ISPs (powerMaximumFlexRequest = importMax, powerMinimumFlexRequest = exportMax); replies with FlexRequestResponse (Accepted) then FlexOffer. The emitted FlexRequestResponse is asserted to match the reference message shape (result, referenced request id, swapped domains, version, conversation id).
  • FlexOfferResponse (Accepted and Rejected) → handled with no asset mutation and no reply.
  • FlexOrder → asset updated from order ISPs (currentPowerFlexRequest, plus the offtake powerLimitMaximumProfileFlexOrder / feed-in powerLimitMinimumProfileFlexOrder profile); replies with FlexOrderResponse (Accepted).
  • Out-of-scope drop → a validly-signed FlexRequest whose CongestionPoint differs from the contracted EAN is dropped: no asset mutation, no reply (the V3 re-scoping).

@Miggets7 Miggets7 added the Test Development of missing or unstable test label Jun 3, 2026
@Miggets7 Miggets7 requested a review from a team June 3, 2026 14:41
@wborn wborn requested a review from Copilot June 3, 2026 16:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 GOPACSHandlerTest covering FlexRequest → (FlexRequestResponse + FlexOffer), FlexOfferResponse handling, FlexOrder flows, and out-of-scope congestion point dropping.
  • Add a protected test-support constructor on GOPACSHandler to 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.

Comment on lines +246 to +250
this.client = null;
this.gopacsAddressBookResource = null;
this.gopacsAuthResource = null;
this.gopacsServerResource = null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This path already degrades gracefully — no NPE is thrown.

  1. fetchBearerToken() wraps the gopacsAuthResource call in try/catch (Exception e). A null gopacsAuthResource raises an NPE, which is caught and the method returns "".

  2. getParticipantInformation() checks the blank token before touching the address book:

String authorization = fetchBearerToken();
if (authorization.isBlank()) {
    LOG.warning("Skipping participant lookup for " + domain + ": no OAuth2 bearer token available");
    return Optional.empty();
}
try (Response response = gopacsAddressBookResource.fetchParticipantByDomain(authorization, domain)) {

So even with participants unseeded: caught NPE → blank token → Optional.empty(), and gopacsAddressBookResource is never dereferenced. Additionally, the covered flows pre-seed participants, so the participants.containsKey(domain) cache returns first and fetchBearerToken() is not called at all.

Leaving the constructor as-is; no-op resource implementations would be unused plumbing.

@wborn wborn mentioned this pull request Jun 4, 2026
4 tasks
@wborn
Copy link
Copy Markdown
Member

wborn commented Jun 4, 2026

Will these tests also find any regressions that might occur due to the changes in #43?

@Miggets7
Copy link
Copy Markdown
Contributor Author

Miggets7 commented Jun 4, 2026

@wborn Not really, and that's intentional. These tests stub the OpenRemote collaborators (AssetProcessingService, AssetPredictedDatapointService, TimerService) and skip deploy(), so they pin the GOPACS/UFTP message behavior — sign/verify, deserialize, ISP mapping, scoping, outbound shapes — against Shapeshifter 3.2.2, which #43 doesn't move. #43's only handler change is the getStandardProviders signature inside deploy(), which the test-support constructor bypasses, and the deps it bumps (resteasy/jackson/BC) aren't on the path these tests exercise. So they won't catch an OR-upgrade regression; once #43 is merged they'll at least confirm the handler still binds and the message flow still passes under the new deps. The deployment path itself needs the usual compile + manual/integration check.

Copy link
Copy Markdown
Member

@richturner richturner left a comment

Choose a reason for hiding this comment

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

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:

  1. POST /gopacs/message with signed XML and application/xml returns success and triggers the same flow as the direct processRawMessage test.
  2. Malformed XML or invalid signature returns 400, covering the UftpConnectorException branch.
  3. A valid out-of-scope message returns success but causes no asset mutation and no outbound HTTP POST.
  4. Outbound FlexRequestResponse/FlexOffer/FlexOrderResponse produces actual broker POSTs to /shapeshifter/api/v3/message with signed XML and Authorization: Bearer ....
  5. OAuth/address-book failure cases: token 500, invalid token JSON, participant 404, and successful lookup caching.

@richturner
Copy link
Copy Markdown
Member

@wborn @Miggets7 see my comment also; happy to see tests for the message payloads but more integration level tests would be appropriate an inline with what we do generally.

@wborn
Copy link
Copy Markdown
Member

wborn commented Jun 4, 2026

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.

Miggets7 added 2 commits June 4, 2026 14:39
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).
@Miggets7
Copy link
Copy Markdown
Contributor Author

Miggets7 commented Jun 4, 2026

Added GOPACSHandlerHttpTest — a full-transport integration test alongside the existing unit test (commits 086187d, f69c2c9).

It POSTs real signed messages to the deployed /gopacs/{realm}/message endpoint and verifies the outbound broker delivery, OAuth2 and address-book calls against a WireMock server. Covers all five scenarios: inbound success, malformed/bad-signature → 400, out-of-scope drop, broker POST (signed XML + Bearer), and the OAuth/address-book failure + caching cases.

Note: the broker send uses java.net.http.HttpClient (via shapeshifter), which a JAX-RS ClientRequestFilter can't intercept — hence WireMock, same as shapeshifter's own UftpSendMessageServiceTest. Like EntsoeProtocolTest it's @IgnoreIf(GITHUB_ACTIONS) (runs locally, not CI).

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

Labels

Test Development of missing or unstable test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants