Skip to content

feature: stored-value gift card API (issue, balance, idempotent redeem, ledger) - #295

Open
vanessasalas-cog wants to merge 9 commits into
DevOpsfrom
devin/1787596568-stored-value-api
Open

feature: stored-value gift card API (issue, balance, idempotent redeem, ledger)#295
vanessasalas-cog wants to merge 9 commits into
DevOpsfrom
devin/1787596568-stored-value-api

Conversation

@vanessasalas-cog

@vanessasalas-cog vanessasalas-cog commented Aug 24, 2026

Copy link
Copy Markdown

Summary

Adds a partner-facing stored-value / gift-card surface under /api/v1/stored-value, written spec-first: the OpenAPI 3.0 contract and the invariant table live in docs/stored-value-api.md, and the code implements exactly that contract.

Endpoints (stateless HTTP Basic, own SecurityFilterChain at @Order(1); the Thymeleaf/form-login chain is untouched):

Method Path Notes
POST /api/v1/stored-value/cards issue; returns opaque cardToken + fee/expiry disclosure
GET /api/v1/stored-value/cards/{token}/balance inquiry; reports EXPIRED once past expiresAt
POST /api/v1/stored-value/cards/{token}/redeem partial/full, requires Idempotency-Key
GET /api/v1/stored-value/cards/{token}/transactions ledger, oldest first

Invariants enforced (and tested)

  • No double-spend. Redemption reads the card with a row lock inside one transaction:
    @Transactional(isolation = READ_COMMITTED)
    redeem(token, amount, key):
        card = cardRepository.findByCardTokenForUpdate(token)   // select ... for update
        replay = txnRepo.findByCardIdAndIdempotencyKey(card.id, key)
        if replay != null: return replay (amount mismatch -> IDEMPOTENCY_KEY_CONFLICT)
        if card expired -> CARD_EXPIRED ; if amount > balance -> INSUFFICIENT_BALANCE
        card.balance -= amount ; if zero -> DEPLETED ; append ledger row
  • At-most-once per key. Backed by unique key uk_stored_value_txn_idempotency (card_id, idempotency_key), not just the application check; replays return the original entry with replayed: true and move no money. Same key with a different amount is a 409 rather than a silent re-price. The header is bounded (@Size(max = 128)) so an oversized key is a 400, never a column-overflow 500.
  • Amount bounds belong to the domain. Positivity, minor-unit scale and the 10,000.00 ceiling are all re-checked in StoredValueService.issueCard, so the invariant holds for direct service callers, not only for requests that pass through IssueCardRequest bean validation.
  • Token hygiene. The PAN-equivalent card_reference is persisted but never serialised in any DTO and never logged; log lines carry ****<last4> of the token only.
  • Disclosure. Card responses embed disclosure { feesAssessed:false, feePolicy, expiryPolicy, expiresAt }.
  • No HTML in the API. Both missing and invalid credentials return 401 with the same {code, message, details, timestamp} body as every other error — the entry point serialises a real ErrorResponse through the injected ObjectMapper. httpBasic gets that same entry point as exceptionHandling, so a bad password can't fall through to the form-login redirect — the live run below caught it returning 302 /login plus a JSESSIONID on a chain declared STATELESS.

Other errors share that one shape via a @RestControllerAdvice scoped to this controller, so existing MVC error behaviour is unchanged. Dispatch-level failures (unsupported method/media type) are raised before a handler resolves and so never reach the advice's catch-all — Spring answers those with the correct 4xx itself, asserted by test.

Persistence follows the repo's existing approach — JPA entities under ddl-auto=update, plus a checked-in DDL script alongside the existing one at src/main/resources/db/stored_value_schema.sql (deliberately not under static/, which Spring Boot serves unauthenticated).

Also sets the executable bit on mvnw so the documented ./mvnw clean test runs as written.

Test evidence

./mvnw clean test against MySQL 8 at jdbc:mysql://localhost:3306/bankappdb: 30 tests, 0 failures, 0 errors.

  • StoredValueConcurrencyTest — 20 threads each redeeming 10.00 from a 100.00 card: exactly 10 succeed, 10 get INSUFFICIENT_BALANCE, balance lands on 0.00 with 11 ledger rows; a second case fires 20 concurrent requests with one shared key and asserts a single 10.00 debit.
  • StoredValueApiIntegrationTest — full HTTP flow over the real DB: issue → balance → partial + full redeem → ledger, idempotent replay, key conflict, over-redemption, expired-card redeem and inquiry, validation/missing-header/oversized-header errors, unsupported method, 401 (full error body) for both missing and wrong credentials, and that no response exposes cardReference.
  • StoredValueServiceTest — unit coverage of expiry, over-redemption, replay, conflict, amount validation including the issuance cap, token masking.
  • Live end-to-end run of all four endpoints against a booted app, with screenshots and log-masking checks: feature: stored-value gift card API (issue, balance, idempotent redeem, ledger) #295 (comment)

The repo has no GitHub Actions workflow; CI is the Jenkins pipeline, which runs the same Maven build.

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/5091c8eaef664e4c875bda220fd137a6
Requested by: @vanessasalas-cog


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Devin Review (Staging)
Open in Devin Review

…edeem, ledger)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown

🤖 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, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

… of login redirect

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown

End-to-end run against a live app

Booted the app (./mvnw spring-boot:run + MySQL bankappdb), registered a partner account, and drove all four endpoints over HTTP Basic.

check result
POST /cards → 201, cardToken, balance 50.00, ACTIVE, disclosure block, no cardReference
GET /cards/{token}/balance → 50.00 / ACTIVE
POST /redeem Idempotency-Key: order-4711 20.00 → remainingBalance 30.00, replayed:false
identical replay → same transactionId, remainingBalance 30.00, replayed:true, balance still 30.00
same key, different amount → 409 IDEMPOTENCY_KEY_CONFLICT
over-redemption (500 > 30) → 409 INSUFFICIENT_BALANCE, balance unchanged
GET /transactions → ISSUE 50.00/50.00 + single REDEMPTION 20.00/30.00
logs: 0 hits for the full token and for the PAN-equivalent card_reference; only token=****c9bc
regression: web form login → dashboard still renders
wrong credentials → 302 redirect to /login with a JSESSIONID ❌ → fixed in 705a8c4

Issue card
Redeem and idempotent replay
409 conflicts, balance unchanged
Ledger
Log masking

The one failure was the bad-credentials path flagged in review: BasicAuthenticationFilter was falling through to the form-login entry point, so a partner client got an HTML redirect (and a session cookie on a chain declared STATELESS) instead of a 401. 705a8c4 gives httpBasic the same JSON entry point and adds a regression test. ./mvnw clean test now: 25 tests, 0 failures.

devin-ai-integration[bot]

This comment was marked as resolved.

…idempotency keys (feature hardening)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 4 new potential issues.

Open in Devin Review

Comment thread pom.xml
Comment thread src/main/java/com/example/bankapp/controller/StoredValueController.java Outdated
Comment thread src/main/java/com/example/bankapp/service/StoredValueService.java
.securityMatcher("/api/v1/stored-value/**")
.csrf(csrf -> csrf.disable())
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.authorizeHttpRequests(authz -> authz.anyRequest().authenticated())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 Any authenticated user can redeem any card

The stored-value filter chain authorizes only anyRequest().authenticated(), and cards carry no owner. Any registered bank account that learns a card token can query balance, redeem funds, and read the ledger for a card it did not issue, because redeem and getCard in StoredValueService never check the principal against a card owner.

Open in Devin Review

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Accurate description of the behaviour, and it's a deliberate design point rather than an oversight — but it does need a product decision, so I'm not changing it unilaterally.

The card is a bearer instrument: possession of the token is the entitlement, exactly like a physical gift card, and the caller is a partner integration authenticating to the platform (not the cardholder). Cards therefore have no owner, and the token is the unguessable secret (UUIDv4, never logged in full, never returned as the PAN-equivalent reference).

If BHN wants per-partner tenancy instead — an issuer/owner column on stored_value_card, scoping every lookup to the authenticated principal, and a 404 (not 403) on cross-tenant access to avoid token probing — that's a small follow-up, but it changes the contract in docs/stored-value-api.md, so it should be an explicit decision. Flagging it to the requester.

…le (stored-value feature)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

…tus on idempotent replay (stored-value feature)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

…t instead of skewing counters (feature tests)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

…d-value feature)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

…esources (feature hardening)

Co-Authored-By: vanessa.salas <vanessa.salas@cognition.ai>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 0 new potential issues.

Open in Devin Review

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant