Skip to content

feature: migrate COBOL portfolio TRANSACTION entity to a Java 21 / Spring Boot 3 service - #258

Open
devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
devin/1785431168-cobol-transaction-java-migration
Open

devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
devin/1785431168-cobol-transaction-java-migration

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 30, 2026

Copy link
Copy Markdown

Summary

Migrates one legacy entity — the portfolio transaction (TRNREC.cpy, VSAM KSDS TRANHIST) — into a self-contained Java 21 / Spring Boot 3 service under java-migration/. Purely additive: no COBOL file is touched, so the before/after sits side by side in the repo.

What the COBOL did. PORTTRAN.cbl reads transactions, validates them (2100-series: portfolio exists, type in BU/SL/TR/FE, quantity/price/amount positive), and — in 22102240 — applies the position update per type (buy adds units and cost, sell checks available units then subtracts, transfer is unimplemented, fee subtracts cost only), writing an audit record whose action depends on the type. PORTVALD.cbl is the field-level validation subroutine (PORT + 4 digits, investment types, amount bounds, numeric return codes 04). PRCSEQ00.cbl supplies the 1-up sequence-numbering convention.

What the service does now. The same rules, each tagged with where it came from:

@CobolOrigin(program = "PORTTRAN", paragraph = "2220-PROCESS-SELL", rules = {"BR-10"})
public PortfolioPostingEffect processSell(PortfolioTransaction t, BigDecimal availableUnits) {
    if (availableUnits.compareTo(t.getTrnQuantity()) < 0)
        throw new TransactionProcessingException("Insufficient units for sale", "BR-10", …);
    return new PortfolioPostingEffect(t.getTrnQuantity().negate(), t.getTrnAmount().negate(), "DELETE");
}

grep -r "2220-PROCESS-SELL" java-migration/src lands on the Java that replaces it — that is the traceability contract for this PR, applied to every migrated type and method.

Three findings a reviewer should know about (all in MIGRATION-NOTES.md §4, nothing guessed silently):

  • 2200-UPDATE-POSITIONS is unreachable in PORTTRAN.cbl — no paragraph performs it; 2100-VALIDATE-TRANSACTION only bumps counters. Both readings are preserved rather than picked: runBatch() is the literal batch (validate + count, no posting, nothing written back — the file is opened INPUT), while process() wires validation to the position update (the evident intent) and is what the REST API exposes. (OQ-6)
  • Nothing in the supplied programs computes TRN-AMOUNT — the batch only validates it. Amount derivation is implemented as the most literal reading of a COMPUTE without ROUNDED: quantity × price truncated to 2 dp (RoundingMode.DOWN), and flagged as derived rule BR-22. (OQ-1)
  • vsam-definitions.txt and the copybook disagree: the KSDS declares RECORD LENGTH 300 / KEY LENGTH 20, the copybook is 152 bytes with a 28-byte key group. The copybook wins (it is the layout source of truth); the key is modelled as 28 bytes. (OQ-4)

2130-CHECK-AMOUNTS exempts TR from the price and amount checks but not from the quantity check — reproduced literally, with a test pinning it, and raised as OQ-3.

What's in the module

Entity/domain PortfolioTransaction with an @EmbeddedId TransactionKey; every field carries Javadoc naming its COBOL field and PIC clause; 88-levels become TransactionType (BU/SL/TR/FE) and TransactionStatus (P/D/F/R) enums persisted as the literal COBOL codes
Decimals BigDecimal only — DECIMAL(15,4) for TRN-QUANTITY/TRN-PRICE (S9(11)V9(4) COMP-3), DECIMAL(15,2) for TRN-AMOUNT (S9(13)V9(2) COMP-3); a test round-trips full 15-digit values
Flyway V1__create_portfolio_transaction.sql with the composite key, exact precision/scale and CHECK constraints on the 88-level code sets; ddl-auto: validate keeps entity and schema in lockstep
API keyed read, key-ordered paged browse, insert, rewrite, status transition, process — OpenAPI 3, contract committed at docs/openapi.yaml, Swagger UI at /swagger-ui.html
Docs MIGRATION-NOTES.md: 23 numbered rules → paragraph → Java → test, the field mapping table with byte offsets, and the open questions

The 28-byte VSAM key is the API resource id, so keyed reads read like the COBOL: GET /api/v1/transactions/20240320093015PORT0001000001.

Evidence of equivalence

87 tests, one or more per numbered rule (MIGRATION-NOTES.md maps rule → test):

$ cd java-migration && mvn clean test
Tests run: 17, Failures: 0 -- PortfolioFieldValidatorTest        (BR-15..BR-19, PORTVALD return codes)
Tests run: 10, Failures: 0 -- TransactionValidatorTest           (BR-01..BR-07, exact COBOL error text)
Tests run: 19, Failures: 0 -- PortfolioTransactionServiceTest    (CRUD, BR-08 counters/limit, BR-20, BR-23, OQ-6)
Tests run:  7, Failures: 0 -- TransactionPostingServiceTest      (BR-09..BR-13)
Tests run:  6, Failures: 0 -- TransactionAmountCalculatorTest    (BR-22 truncation and S9(13)V9(2) capacity)
Tests run: 10, Failures: 0 -- PortfolioTransactionControllerTest (API contract + OpenAPI publication)
Tests run:  6, Failures: 0 -- PortfolioTransactionRepositoryTest (BR-21 key order, browse filters, 15-digit precision)
Tests run:  5, Failures: 0 -- TransactionKeyTest                 (28-byte key, VSAM ordering, PIC X tolerance)
Tests run:  4, Failures: 0 -- TransactionStatusTest              (BR-23)
Tests run:  3, Failures: 0 -- SeedDataTest                       (representative records reconcile)

Tests run: 87, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Error text is asserted verbatim against the COBOL (Insufficient units for sale, Transfer processing not implemented, Invalid Transaction Type: XX, …), so a divergence in wording fails the build.

Test data

There is no ASCII extract of TRANHIST in this repo — only src/database/vsam/vsam-definitions.txt, which describes the KSDS but holds no records. Nothing was invented: the fixtures and the 8 rows in db/seed/V900__…sql (loaded only under the seed profile) are derived from the copybook layout plus the repo's own generators — PORTTEST.cbl 2100-GENERATE-KEY ('PORT' + WS-RECORD-COUNT) and TSTGEN00.cbl 2300-GEN-TRANSACTION. They are representative, not a production sample, and are labelled as such in the SQL, the tests and the notes. If an extract appears later, the remaining work is the COMP-3 decode for the three packed fields; the rest are display characters.

Checks

  • No COBOL, copybook or data file modified — the diff is java-migration/ only.
  • GnuCOBOL behaviour unchanged (nothing to break): cobc on the base commit compiles PORTVALD.cbl clean and already fails on PORTTRAN.cbl/PORTTEST.cbl/PRCSEQ00.cbl (FUNCTION 'USER-ID' unknown, PORT-ACCOUNT-NO not defined, missing PROGRAM-ID); this PR neither fixes nor worsens that.
  • No credentials: DB_URL/DB_USERNAME/DB_PASSWORD are environment-substituted, default H2 in-memory.

Review follow-up (dd4a62f, 13f10b3)

The automated review found real gaps; each is fixed and pinned by a test, and each thread has the detail:

Fix Behaviour now
BR-23 enforced on process() a D/F/R record is refused instead of being silently re-posted
Sequence assignment read and write share one transaction and retry on collision, so concurrent creates no longer collide
Audit on failed postings a rejected posting keeps its AUD-ACTION with AUD-STATUS = FAIL (the 2300 path is unconditional); a 2100 rejection branches to 9000 and stays unaudited
Invalid Transaction Type: ZZ the rejected code is echoed, matching the job log
Missing PORT-TOTAL-UNITS on a sell 400 caller error, record stays P, instead of a bogus Insufficient units for sale failure
Amount overflow a product beyond S9(13)V9(2) is refused rather than corrupted or left to fail in the database (deviation recorded as OQ-10)
Browse portability the :param is null idiom is replaced by derived queries, so the query no longer depends on the database typing a null bind parameter
Typed key views getTransactionDate()/getTransactionTime() return Optional under ResolverStyle.STRICT, so PIC X values VSAM accepts cannot throw on a response path
Rewrite the payload is validated before the managed record is mutated
Placeholder portfolio check selected by portfolio.reference-validator instead of @ConditionalOnMissingBean, so a deployment override cannot be silently ignored

Not changed, deliberately: the module ships no authentication — the legacy access control is RACF/CICS and the target architecture puts Apigee in front, so the README now states explicitly that the service must sit behind the gateway. Say the word if you would rather it fail closed on its own with a deny-by-default SecurityFilterChain.

What remains

The 9 open questions in MIGRATION-NOTES.md §4 need a legacy owner: chiefly the amount-derivation semantics (OQ-1), the unreachable position update (OQ-6), and the status lifecycle (OQ-5). Portfolio existence (BR-02) is behind a PortfolioReferenceValidator interface whose default implementation only checks the PORTVALD id format, because the portfolio master is a separate entity — the next migration slice supplies the real bean (OQ-2).

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


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)

…ng Boot 3 service

Co-Authored-By: Dana Gajewski <danagajewski2018@gmail.com>
@devin-ai-integration

devin-ai-integration Bot commented Jul 30, 2026

Copy link
Copy Markdown
Author

🤖 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 devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 13 potential issues.

Open in Devin Review

Comment on lines +162 to +175
public BatchRunSummary runBatch(List<PortfolioTransaction> transactions) {
List<TransactionProcessingResult> results = new ArrayList<>();
int readCount = 0;
int processCount = 0;
int errorCount = 0;
boolean aborted = false;

for (PortfolioTransaction transaction : sequenceService.inKeySequence(transactions)) {
if (errorCount > MAX_ERRORS) {
aborted = true;
break;
}
readCount++;
ValidationOutcome outcome = validator.validate(transaction);

@devin-ai-integration devin-ai-integration Bot Jul 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Batch driver ordering is imposed by the service rather than by the input file

runBatch sorts its input with sequenceService.inKeySequence(...) before iterating. The COBOL driver simply performs sequential READs on TRANSACTION-FILE, which is declared ORGANIZATION IS SEQUENTIAL (see OQ-7), so the physical order of the extract governs both the processing order and, crucially, which records are left unread when the 100-error limit trips. Re-sorting means the abort point of the Java run can differ from the mainframe run for the same extract. If the intent is a literal port of 0000-MAIN, preserving the caller's order (and documenting that key order is only assumed) would be closer to the legacy behaviour.

Open in Devin Review

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deliberate, and worth stating in the notes rather than changing — thanks for flagging the abort-point consequence.

TRANHIST is a KSDS, and a sequential read of a KSDS is a read in key order, so the sort reproduces the order the batch sees when it is fed from the file. The divergence you describe is real only if the extract is re-ordered before the job runs, which is exactly the ambiguity already recorded as OQ-7 (ORGANIZATION IS SEQUENTIAL in PORTTRAN.cbl vs. the KSDS definition). Since runBatch takes a List, a caller that wants literal file order can pass records already in that order, and for a key-ordered extract the sort is a no-op.

Left as is because dropping the sort would make the 100-error abort point depend on the caller's list order, which is less reproducible than the key order the KSDS guarantees. BR-21/OQ-7 in MIGRATION-NOTES carry the reasoning.

devin-ai-integration Bot and others added 2 commits July 30, 2026 17:36
…inal-status processing, audit metadata on failed postings, raw TRN-TYPE in errors, collision-safe sequence assignment, portable browse)

Co-Authored-By: Dana Gajewski <danagajewski2018@gmail.com>
…d document the missing gateway-level auth

Co-Authored-By: Dana Gajewski <danagajewski2018@gmail.com>
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.

0 participants