feat: migrate TradeConfirmationService from direct invocation (v1) to REST client interface (v2)

[devin-ai-integration]

Summary

Migrates trade confirmation delivery from direct method invocation (TradeConfirmationService) to a TradeConfirmationClient interface backed by a resilient REST client with automatic fallback — following the Strangler Fig pattern.

Before (v1)

• CounterpartyController branched on the use.confirmation.service toggle to choose between TradeConfirmationService (local logger) and TradeConfirmationMicroserviceClient (REST).
• RFQExecutionSaga did not send any trade confirmation after execution.
• Toggle logic lived inside the controller.

After (v2)

• New TradeConfirmationClient interface in the client/ package.
• RestTradeConfirmationClient — calls the external confirmation microservice via REST.
• FallbackTradeConfirmationClient — logs locally when REST is unavailable.
• ResilientTradeConfirmationClient (@Primary) — respects the toggle; tries REST first, falls back on failure.
• CounterpartyController injects TradeConfirmationClient — no branching logic in callers.
• RFQExecutionSaga publishes a TradeExecutedEvent; a TradeExecutedEventListener annotated with @TransactionalEventListener(phase = AFTER_COMMIT) sends the confirmation only after the transaction commits — preventing phantom confirmations for rolled-back trades.
• Old TradeConfirmationService and TradeConfirmationMicroserviceClient marked @Deprecated.
• MIGRATION_CHECKLIST.md provides a repeatable playbook for migrating remaining modules.

Files changed

File	Change
client/TradeConfirmationClient.java	New interface
client/RestTradeConfirmationClient.java	New REST implementation
client/FallbackTradeConfirmationClient.java	New fallback implementation
client/ResilientTradeConfirmationClient.java	New resilient decorator (@Primary)
event/TradeExecutedEvent.java	New domain event for post-commit confirmation
event/TradeExecutedEventListener.java	Listener fires confirmation after transaction commits
controller/CounterpartyController.java	Replaced v1 toggle + dual injection with single TradeConfirmationClient
saga/RFQExecutionSaga.java	Publishes TradeExecutedEvent instead of calling client directly
service/TradeConfirmationService.java	Marked @Deprecated
restclient/TradeConfirmationMicroserviceClient.java	Marked @Deprecated
client/ResilientTradeConfirmationClientTest.java	Tests: REST path, fallback on failure
client/FallbackModeTest.java	Test: toggle off skips REST
MIGRATION_CHECKLIST.md	Repeatable migration checklist for remaining modules

Review & Testing Checklist for Human

• Verify fallback behaviour: Set use.confirmation.service=true and confirm the app starts; PATCH a counterparty credit without the external microservice running — logs should show the FALLBACK warning instead of a 500 error.
• Verify RFQ execution sends confirmation after commit: POST an RFQ and check logs for the trade confirmation message from TradeExecutedEventListener.
• Regression check: Run ./mvnw clean test — all 10 tests should pass (7 existing + 3 new).
• Review MIGRATION_CHECKLIST.md: Confirm the checklist steps match your team's migration workflow expectations.

Recommended test plan:

1. cd monolith && ./mvnw spring-boot:run
2. curl -X PATCH http://localhost:8080/counterparties/1 -H 'Content-Type: application/json' -d '{"amount":"1000000","operation":"ADD"}'
3. Observe logs for fallback confirmation message (since the external microservice is not running).
4. POST a new RFQ and confirm the saga sends a confirmation after commit.

Notes

• The use.confirmation.service toggle defaults to false in application.properties, preserving v1 behaviour until the external microservice is deployed and the toggle is flipped.
• Trade confirmation is treated as fire-and-forget: a REST failure does not roll back the trade transaction.
• The @TransactionalEventListener(AFTER_COMMIT) pattern ensures confirmations are only sent for durably committed trades — no phantom confirmations on rollback.
• The deprecated v1 classes are retained for rollback safety and can be removed once the v2 path is stable.

Link to Devin session: https://app.devin.ai/sessions/2f870b8314bc4da08344f8d2c0609f33
Requested by: @achalc

---

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[devin-ai-integration]

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

[devin-ai-integration]

🔴 Trade confirmation sent inside @transactional boundary, before commit

The sendConfirmation call at lines 53-54 executes inside the @Transactional method executeRfq. The transaction only commits after the method returns (line 56), so when the REST path is enabled (use.confirmation.service=true), an HTTP POST is dispatched to the external confirmation microservice before the database transaction commits. If the commit subsequently fails (e.g., constraint violation detected at flush, DB connectivity loss), the confirmation has already been irrevocably sent for a trade that was rolled back — creating a phantom confirmation in the external service.

This directly violates the PR's own MIGRATION_CHECKLIST.md Phase 5, bullet 3: "If the module participates in a saga, verify the confirmation is sent after the transaction commits."

The fix is to move the confirmation outside the transactional boundary — e.g., using TransactionSynchronizationManager.registerSynchronization() with an afterCommit() callback, or a @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) pattern.

Prompt for agents

The trade confirmation REST call at RFQExecutionSaga.java:53-54 is made inside the @Transactional method executeRfq(). The transaction commits only after the method returns, so the confirmation is sent before the DB changes are committed. If the commit fails, a phantom confirmation has been sent to the external microservice.

To fix this, the confirmation should only be sent after the transaction successfully commits. Two common approaches:

1. Use TransactionSynchronizationManager: Register a synchronization callback inside executeRfq() that sends the confirmation in afterCommit(). This keeps the logic co-located.

2. Use @TransactionalEventListener: Publish a domain event (e.g., TradeExecutedEvent) from executeRfq(), and have a separate listener annotated with @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) that calls tradeConfirmationClient.sendConfirmation().

Either approach ensures the REST call only fires after the trade data is durably committed. The second approach is more aligned with Spring best practices and the eventual move to event-driven choreography mentioned in MIGRATION_CHECKLIST.md.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Good catch. Fixed in d45c235 — the saga now publishes a TradeExecutedEvent via ApplicationEventPublisher, and a separate TradeExecutedEventListener annotated with @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) sends the confirmation only after the trade data is durably committed. This prevents phantom confirmations on rollback and aligns with the eventual event-driven choreography path noted in MIGRATION_CHECKLIST.md.