Annotate SEPAAccounts with holder name, product name, currency and type from the UPD

[Cosnavel]

Problem

Applications need the per-account information from the HIUPD segments — holder name, product name, currency, account type — to label the accounts that GetSEPAAccounts returns (see #467, #573). FinTs::$upd is private, so the only way was reflection:

$ref = new \ReflectionProperty($fints, 'upd');
$upd = $ref->getValue($fints);

Both of our applications do exactly that in production.

Change

Following the review discussion below, the UPD stays internal. Instead, GetSEPAAccounts keeps the UPD it receives in createRequest() and, in processResponse(), copies the descriptive fields of the matching HIUPD (UPD::findHiupd()) onto each SEPAAccount:

$account->getName();        // "Mustermann Max" (name1 + name2)
$account->getProductName(); // "Girokonto Komfort"
$account->getCurrency();    // "EUR"
$account->getAccountType(); // 1 (Kontokorrent), null before HIUPD v6

These are the same fields CreditCardAccount got in #574, so both account kinds read alike; the name join is HIUPD::getAccountHolderName() (implemented once for v4 and v6) and used by both actions. Accounts without a matching HIUPD keep null in these fields, and a SEPAAccount an application constructs itself is unaffected.

Some banks require a TAN for HKSPA and applications serialize the action while waiting for it. The FinTs instance that completes the TAN may have been restored from persist(true), which leaves out BPD and UPD and cannot fetch them again while the dialog is open, so the action keeps its own copy of the UPD and serializes it (__serialize()/__unserialize()). Actions serialized by earlier versions (PaginateableAction state only) still unserialize.

Tests

• Tests/Unit/Integration/DKB/GetSEPAAccountsTest.php: the recorded DKB dialog now also asserts name, product name, currency and type from the login response's HIUPD.
• Tests/Unit/Action/GetSEPAAccountsTest.php (on ActionTestCase from Keep the XML fallback of GetStatementOfAccount across serialization (#553) #591): annotation via IBAN and via Kontonummer (HIUPD without IBAN), accounts without HIUPD, no UPD at all, UPD surviving the serialization round trip, and unserialization of a blob recorded with dd2a7f2.
• Tests/Unit/Segment/HIUPDTest.php: getAccountHolderName() for v4 (single name field) and v6 (both fields).

Note on authorship

Developed with AI assistance (Cursor) and reviewed by the submitting human; the vouching statement per #586 will be added by the author once the review is complete.

[Cosnavel]

@nemiah PR ist ready für's Review

[nemiah]

It probably won't break something and doesn't look too verbose for me. I'll merge it if your human takes responsibility

[Philipp91]

which is more than {@link \Fhp\Action\GetSEPAAccounts} exposes.

Have you considered augmenting what GetSEPAAccounts exposes? It returns Fhp\Model\... classes, which have more purposefully designed and more ergonomic APIs than UPD. I'm not sure we want to expose the UPD API as a whole, as users might start depending on weird quirks deep inside of it, which would prevent us from changing it easily in the future.

[Cosnavel]

@nemiah I have manually reviewed the pr and adjusted minor things.

Only when fully ready and me taking responsibility I write the comment: "@nemiah PR ist ready für's Review"

[Cosnavel]

Good point, agreed. Exposing the whole UPD (segment classes, wire-level field names) as public API would let callers depend on exactly the internals that should stay movable, and it is inconsistent with #574, which reads the UPD internally and returns a purpose-built CreditCardAccount.

I'll rework this PR along those lines: GetSEPAAccounts keeps the UPD it receives in createRequest() and annotates each SEPAAccount in processResponse() with what applications actually need from the HIUPD — holder name (name1 + name2), product name, currency and account type — via UPD::findHiupd(), mirroring the fields CreditCardAccount already has. FinTs::getUpd() goes away.

One subtlety: HKSPA is TAN-gated at some banks, so the UPD has to survive the action's serialization round trip while the application waits for the TAN. I'll serialize it with the action (backwards compatible with previously serialized actions) and cover that with a test. If someone later needs the permitted business transactions per account, they can be added the same way as a plain list of segment names.

@nemiah please hold the merge until the rework is pushed.

[Cosnavel]

Reworked as discussed (force-pushed, single commit):

• FinTs::getUpd() is gone.
• GetSEPAAccounts keeps the UPD from createRequest() and annotates each SEPAAccount in processResponse() with getName() (name1 + name2), getProductName(), getCurrency() and getAccountType() — the same fields CreditCardAccount has since Add credit card transaction retrieval (DKKKU) #574. The name join is now shared via UPD::accountHolderName().
• The UPD is serialized with the action so a TAN-gated HKSPA still yields annotated accounts after the round trip; pre-change blobs (PaginateableAction state only) still unserialize.
• Tests: the recorded DKB dialog asserts the new fields from its HIUPD, plus unit tests for the Kontonummer fallback, missing HIUPD, missing UPD, the serialization round trip and the legacy blob.

Description updated accordingly. @Philipp91 does that match what you had in mind?

[Philipp91]

Should this be a member function of HIUPD instead?

[Cosnavel]

Yes, that's the better home for it. It's now HIUPD::getAccountHolderName(), implemented once in AccountHolderNameTrait for v4 and v6 (the way FindRueckmeldungTrait implements RueckmeldungContainer). UPD::accountHolderName() is gone, both actions call it on the segment, and HIUPDTest covers both versions. (300ae4e)

[Philipp91]

Any reason this is a separate function and not just if ($hiupd = $this->upd?->findHiupd($account)) { ... } in the function above?

[Cosnavel]

No good reason – inlined as you suggested. (300ae4e)

[Philipp91]

Just thinking out loud, another solution would be to pass BPD and ?UPD to processResponse() as well. Then we wouldn't need this whole serialization business here, we'd always get the freshest data and the serialized action would be smaller. It would mean that all action implementations would need to have their signature updated and most of them would then receive an argument they never use. But that seems fine (it's not like ?UPD especially is used in all createRequest() implementations either).

[Cosnavel]

I looked into it, and I think it would lose the data in exactly the case the serialization is there for. persist(true) is documented for completing an outstanding TAN and leaves out BPD and UPD. After FinTs::new(..., $minimalPersist), submitTan()/checkDecoupledSubmission() still work (the resolved TanMode is persisted, and buildMessage() doesn't need the BPD), but $this->bpd and $this->upd are null when processActionResponse() runs, and neither can be fetched again while the dialog is open: the UPD only arrives with the dialog initialization, and ensureBpdAvailable() throws Cannot init another dialog. So a TAN-gated HKSPA would come back unannotated for applications that use the minimal persist. As far as I can tell, that's also why GetStatementOfAccount captures $bankName from the BPD in createRequest() and serializes it, rather than reading the BPD later.

Changing BaseAction::processResponse() would also break applications that subclass an action and override processResponse(Message $response) – PHP refuses to load an override with fewer parameters than the parent.

So I kept the copy and put the reason into the comment on $upd (300ae4e). Size-wise, the serialized UPD of the banks recorded in Tests/Unit/Integration ranges from 0.6 KB (ING) to 3.3 KB (GLS).

[Cosnavel]

Addressed in 300ae4e (rebased onto master first, so the test can use ActionTestCase from #591):

• HIUPD::getAccountHolderName() instead of the static helper on UPD.
• The annotation is inlined in processResponse().
• On passing BPD/UPD to processResponse(): see my reply in that thread. With persist(true) the FinTs instance that completes the TAN has neither, so the action keeps its own copy.
• GetSEPAAccountsTest uses nextRequest()/createBpd() and a blob recorded with dd2a7f2.

@Philipp91 ready for another look.