feature/INT-1675 - BACS Direct Debit notifications + refactor - #371

[david-ruiz-cko]

Breaking changes (check at the end )

This pull request introduces Bacs Direct Debit support to the SDK, enabling sending pre-notification (advance notice) events and full instrument lifecycle management for Bacs payment methods. The changes include new client interfaces and implementations, updates to serialization logic, and enhancements to the API surface. The most important changes are grouped below.

Bacs Direct Debit Client and API Enhancements:

• Added a new BacsClient interface and its implementation BacsClientImpl to support sending Bacs Direct Debit pre-notification events, with both asynchronous and synchronous methods. This client is configured to use secret key authorization only. [1] [2]
• Integrated the new BacsClient into the main API (AbstractCheckoutApmApi and CheckoutApmApi), making it accessible as bacsClient() alongside other APM clients. [1] [2] [3] [4]

Serialization and Instrument Support:

• Updated the GsonSerializer to handle new Bacs-related classes, including request and response types for Bacs sources and instruments, ensuring correct (de)serialization of Bacs payment flows. This includes registering new instrument response subtypes for Bacs and updating the runtime type adapter factories to support polymorphic deserialization. [1] [2] [3] [4]

Documentation:

• Added comprehensive documentation to README.md demonstrating how to use the new Bacs Direct Debit features, including sending pre-notifications, creating, updating, and retrieving Bacs instruments, and taking payments using stored Bacs instruments.

Breaking changes

Property moved off a base class

GetInstrumentResponse.accountHolder removed from the base, now on each concrete variant. Forced by Gson: two fields mapping to the same JSON name throw at runtime, and the spec gives all five retrieve variants a different account-holder shape. Base-typed getAccountHolder() no longer compiles and needs a cast to the concrete variant.

Changed field types

instruments.create.InstrumentData.paymentType: payments.PaymentType → SepaPaymentType. The old enum serialized "Recurring"/"Regular"; StoreSepaInstrumentRequest pins lowercase recurring/regular, so SEPA store could not send a valid value. Callers passing PaymentType.RECURRING must switch enum — though no such call can currently succeed against the API.
GetSepaInstrumentResponse.instrumentData: InstrumentData → GetSepaInstrumentData, which adds the mandate type (Core/B2B) the old type omitted.

Wire value changed

UpdateInstrumentBankAccountRequest sent "type":"token", now sends "type":"bank_account". token is absent from the update discriminator mapping entirely, so PATCH /instruments/{id} could not be validated as a bank-account update. No test referenced the class before this PR.

Deserialization behaviour changed (throw → object)

type: "bacs" and type: "ach" on the create and get instrument responses previously threw JsonParseException, because the three RuntimeTypeAdapterFactory registrations have no default subtype. They now deserialize to the concrete type. Anyone catching that exception as control flow is affected. The factories remain strict for unmodelled types — deliberately, per decision A2.

Deprecated, not removed (non-breaking)

PaymentSourceType.SEPA — zero callers; use ID for the previous platform, SEPAV4 for the current one
CreateInstrumentResponse.customer — moves to the token and bank_account variants in the next major
Correction to something I told you earlier
previous.RequestSepaSource switching from PaymentSourceType.SEPA to PaymentSourceType.ID is wire-neutral, not a breaking change. Both constants already carried @SerializedName("id") on master, so the bytes are identical. I listed it as a wire change twice, including in the ledger — now fixed there too.

[agent-wall-e]

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

• exceeds_bounded_scope:3346>200

Operational gates

• ✅ jira_ticket (INT-1675)
• ✅ independent_review

Files analysed: 100

---

_{wall-e 2026.06.19-02 · policy 376219bc71e6…}

[agent-wall-e]

🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code	Kind	Clause	Meaning
exceeds_bounded_scope — 3346>200	classifying	§2.1 M8	More than 200 non-test, non-doc, non-lockfile lines changed.

Kinds:

• classifying — this rule contributed to the chosen tier.
• informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

_{wall-e 2026.06.19-02 · debug}

[agent-wall-e]

🔵 Advisory review: Sound, but needs your judgement

This PR needs a human approval. The code itself reads as correct; whether it should land depends on context I don't have.

This PR adds Bacs Direct Debit support (new BacsClient, instrument types, serialization wiring) and refactors several existing instrument classes. The changes look technically coherent in the visible portion, but the PR explicitly documents multiple breaking changes that require a human to assess blast radius and versioning decisions.

For you to decide

• The PR explicitly removes CustomerResponse customer from CreateInstrumentResponse base class — any caller using response.getCustomer() on the base type will no longer compile; this is a real breaking change requiring a major version bump decision.
• The identifier() → serializedName() switch for four SourceType entries (PAYMENT_GET_RESPONSE_GIROPAY_SOURCE, PAYMENT_GET_RESPONSE_KLARNA_SOURCE, PAYMENT_GET_RESPONSE_SEPAV4_SOURCE, PAYMENT_RESPONSE_SOURCE) changes which string is used as the discriminator key in RuntimeTypeAdapterFactory; if identifier() and serializedName() return different values for these constants the deserialization of those response types silently breaks, and this cannot be verified from the truncated diff alone.
• The AbstractSource factory now passes true (unknown-type-tolerant) as the third argument — the PR description says the factory remains strict for unmodelled types, but the true flag typically means the opposite; this contradiction needs confirmation that the RuntimeTypeAdapterFactory semantics match the described intent.
• The BacsClientImpl is wired with SdkAuthorizationType.SECRET_KEY only, which the PR says is correct per the spec, but the OAuth-only client variant (CheckoutApi) will expose bacsClient() through the shared AbstractCheckoutApmApi — a reviewer should confirm OAuth clients either can't reach this method or get a clear runtime failure rather than a silent auth error.
• CreateCustomerInstrumentRequest.defaultInstrument changed from primitive boolean to Boolean (boxed), which changes the default value from false to null; any serialization that previously omitted a false value may now serialize a null, depending on Gson config — needs verification against the wire format.
• The diff is explicitly truncated (60 files omitted); correctness of the serializer registrations for Bacs/ACH instrument create and get response subtypes, the GetBacsInstrumentResponse shape, and the RequestBacsSource/BacsResponseSource payment-side types cannot be assessed from what is shown.
• No new integration or unit tests are visible in the diff; the PR description notes 'no test referenced the class before this PR' for one changed class, suggesting test coverage of new paths is absent or not shown — a reviewer should confirm test coverage exists in the omitted files.

⚠️ The diff was too large to read in full, so this review covers only part of the change.

---

_{This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02}

[sonarqubecloud]

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.0% Duplication on New Code

See analysis details on SonarQube Cloud