Java migration foundation + HISTLD00 reference vertical slice - #267
eashansinha wants to merge 7 commits into
Conversation
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
Original prompt from Eashan
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
🧪 Tested locally (Java 17, Maven 3.6.3, embedded H2)
Adversarial check: missing control record (date 20990101)Run with an unknown process date fails cleanly with |
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 DB2 compatibility remains untested
Hibernate-generated H2 schemas cannot validate the supplied DB2 identifiers and constraints. Add DB2-compatible schema tests and document their validation steps.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
🔍 Performance impact remains undocumented
Each accepted row performs an existence query and save. The required performance analysis does not measure query count, batching, or representative throughput.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
🔍 Source record contract conflicts
HISTLD00 references fields absent from its copied HISTREC layout. Correct the source fixture or document which conflicting definition governs the migration.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| if (isNullOrNegativeRequired(item.getQuantity()) | ||
| || isNullOrNegativeRequired(item.getPrice()) |
There was a problem hiding this comment.
🟡 Signed transactions are discarded
When quantity or price is negative, isNullOrNegativeRequired rejects a value the source load accepts. The transaction disappears from position history.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| @Column(name = "LOG_TIMESTAMP", nullable = false) | ||
| private LocalDateTime timestamp; |
There was a problem hiding this comment.
🟡 Return-code persistence targets no column
When ReturnCodeLog uses the supplied RTNCODES schema, its LOG_TIMESTAMP mapping references no column. Every return-code read or write fails.
Prompt for agents
Map ReturnCodeLog.Key.timestamp to the actual TIMESTAMP column declared in src/database/db2/RTNCODES.sql. Handle the reserved identifier using the JPA/Hibernate quoting mechanism supported by both DB2 and H2, and add an integration test against a schema created from the supplied DDL rather than Hibernate-generated names.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| if (repository.existsById(item.getKey())) { | ||
| continue; | ||
| } | ||
| repository.save(item); |
There was a problem hiding this comment.
🔍 Duplicate handling relies on serialization
existsById and save are separate operations. Concurrent loaders can both pass the check, leaving one to fail at flush instead of skipping.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private String validate(TransactionHistoryFileRecord item) { | ||
| if (item.getKey() == null | ||
| || item.getKey().getTransDate() == null | ||
| || item.getKey().getTransTime() == null | ||
| || isBlank(item.getKey().getPortfolioId())) { | ||
| return "Missing transaction key fields"; | ||
| } | ||
| if (isBlank(item.getAccountNo())) { | ||
| return "Missing account number"; | ||
| } | ||
| if (item.getTransType() == null || !VALID_TRANS_TYPES.contains(item.getTransType())) { | ||
| return "Invalid transaction type: " + item.getTransType(); | ||
| } | ||
| if (isBlank(item.getSecurityId())) { | ||
| return "Missing security ID"; | ||
| } | ||
| if (isNullOrNegativeRequired(item.getQuantity()) | ||
| || isNullOrNegativeRequired(item.getPrice()) | ||
| || item.getAmount() == null | ||
| || item.getTotalAmount() == null | ||
| || item.getCostBasis() == null | ||
| || item.getGainLoss() == null) { | ||
| return "Missing or invalid numeric fields"; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🔍 Validation absent from the COBOL is added in Java
The COBOL 2200-LOAD-TO-DB2 performs no validation and only counts DB2 insert failures toward WS-ERROR-COUNT. HistoryItemProcessor.validate rejects records with unknown transaction type, blank security id, or negative quantity/price, counts them as errors, filters them, and can abort at >100. Records the COBOL would load are dropped or can abort the job in Java. Confirm this divergence is acceptable for benchmark fidelity.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct — this is an intentional divergence. In COBOL, malformed records surface as DB2 insert failures (constraint/type errors) that feed WS-ERROR-COUNT via DB2-ERROR-ROUTINE; in Java those same records would fail at JPA/H2 constraint level inside a chunk and roll back the whole chunk. Validating in the processor moves that per-record error detection before the insert so error counting stays per-record like the COBOL. The rules (types BU/SL/TR/FE, non-negative qty/price, required fields) mirror the POSHIST DDL constraints and PORTVAL ranges. Happy to relax/remove validation if strict load-what-COBOL-loads fidelity is preferred — flagging for the maintainers to confirm.
| if (errors > HistoryLoadStats.MAX_ERRORS) { | ||
| throw new ErrorLimitExceededException(errors); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
📝 Info: Abort rolls back the current chunk's inserts
The COBOL commits every 1000 records, so valid inserts before an abort persist. In Java, ErrorLimitExceededException from the processor fails the chunk transaction and rolls back any valid rows written in that same chunk. With valid records interspersed in the final chunk, their inserts are lost on abort, unlike the COBOL. Prior committed chunks still persist.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — known trade-off of chunk-based commits: on the >100-error abort, valid inserts in the in-flight chunk roll back, whereas the COBOL would have committed any that happened to cross the 1000-record threshold. Prior chunks persist identically in both. Since the abort path means the run failed and will be rerun (and reruns skip duplicates like -803), the end state after a successful rerun is the same. Leaving as-is unless stricter mid-chunk fidelity is required.
…pe reader Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
| T result = transactionTemplate.execute(callback); | ||
| commitCount.incrementAndGet(); | ||
| return result; |
There was a problem hiding this comment.
🟡 Rollback-only work counts as committed
When a callback calls setRollbackOnly, transactionTemplate.execute can return normally. commitCount then increases although the transaction rolled back.
Prompt for agents
Track the actual transaction completion outcome rather than treating every normal callback return as a commit. Use transaction synchronization or inspect transaction status in a supported way so rollback-only completion increments rollbackCount, not commitCount. Add a test whose callback calls status.setRollbackOnly() and returns normally.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| .listener(new ChunkListener() { | ||
| @Override | ||
| public void afterChunk(ChunkContext context) { | ||
| JobParameters params = context.getStepContext() | ||
| .getStepExecution().getJobParameters(); | ||
| batchControlService.updateCheckpoint( | ||
| PROGRAM_ID, | ||
| params.getString("processDate"), | ||
| stats.getRecordsRead(), | ||
| stats.getRecordsWritten()); | ||
| } | ||
| }) |
There was a problem hiding this comment.
🔍 Checkpoints cannot resume reading
updateCheckpoint stores only counters, while the reader always starts at the first record. Restarts rescan the entire input instead of resuming.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| url: jdbc:h2:mem:portfolio;DB_CLOSE_DELAY=-1 | ||
| driver-class-name: org.h2.Driver | ||
| username: sa | ||
| password: "" |
There was a problem hiding this comment.
🟨 Default database has no password
The default sa account uses a blank password. Any deployment that retains these settings leaves the application database without credential protection.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
| @Component | ||
| public class HistoryLoadStats { | ||
|
|
||
| /** WS-ERROR-COUNT limit: HISTLD00 stops processing when the count exceeds 100. */ | ||
| public static final int MAX_ERRORS = 100; | ||
|
|
||
| private final AtomicLong recordsRead = new AtomicLong(); | ||
| private final AtomicLong recordsWritten = new AtomicLong(); | ||
| private final AtomicLong errorCount = new AtomicLong(); | ||
|
|
||
| public void reset() { | ||
| recordsRead.set(0); | ||
| recordsWritten.set(0); | ||
| errorCount.set(0); |
There was a problem hiding this comment.
🔍 Statistics are not execution-scoped
Concurrent job executions reset and share one HistoryLoadStats instance. Counters can mix despite using atomic fields.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| public PositionHistory process(TransactionHistoryFileRecord item) { | ||
| stats.incrementRecordsRead(); | ||
|
|
||
| String validationError = validate(item); | ||
| if (validationError != null) { | ||
| long errors = stats.incrementErrorCount(); | ||
| errorHandlingService.logError("HISTLD00", "V", 2, "HIST0001", | ||
| validationError, String.valueOf(item.getKey() == null ? null | ||
| : item.getKey().getPortfolioId() + "/" + item.getKey().getSequenceNo())); | ||
| if (errors > HistoryLoadStats.MAX_ERRORS) { | ||
| throw new ErrorLimitExceededException(errors); |
There was a problem hiding this comment.
🔍 Counter semantics remain ambiguous
Read and error counters survive chunk rollback, while output rows do not. Batch-control totals can describe attempts instead of committed processing.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| @Override | ||
| public int getExitCode() { | ||
| return (int) Math.min(stats.getErrorCount(), 255); | ||
| } |
There was a problem hiding this comment.
🔍 Exit code ignores FAILED status but matches COBOL
getExitCode returns the error count regardless of job status, so a job that fails with zero validation errors exits 0. This mirrors the COBOL, which moves WS-ERROR-COUNT to RETURN-CODE unconditionally, so it is not flagged. Consider whether the port should surface a FAILED job as a non-zero exit for JCL orchestration.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional for now — it mirrors the COBOL MOVE WS-ERROR-COUNT TO RETURN-CODE exactly. I've asked the maintainer whether a FAILED job should surface a non-zero exit for scheduler orchestration; will adjust based on their answer.
| /** Composite primary key (ACCOUNT_NO, PORTFOLIO_ID, TRANS_DATE, TRANS_TIME). */ | ||
| @Embeddable | ||
| public static class Key implements Serializable { | ||
|
|
||
| /** PH-ACCOUNT-NO PIC X(8) / ACCOUNT_NO CHAR(8). */ | ||
| @Column(name = "ACCOUNT_NO", length = 8, nullable = false) | ||
| private String accountNo; | ||
|
|
||
| /** PH-PORTFOLIO-ID PIC X(10) / PORTFOLIO_ID CHAR(10). */ | ||
| @Column(name = "PORTFOLIO_ID", length = 10, nullable = false) | ||
| private String portfolioId; | ||
|
|
||
| /** PH-TRANS-DATE PIC X(10) / TRANS_DATE DATE. */ | ||
| @Column(name = "TRANS_DATE", nullable = false) | ||
| private LocalDate transDate; | ||
|
|
||
| /** PH-TRANS-TIME PIC X(8) / TRANS_TIME TIME. */ | ||
| @Column(name = "TRANS_TIME", nullable = false) | ||
| private LocalTime transTime; | ||
|
|
||
| public Key() {} | ||
|
|
||
| public Key(String accountNo, String portfolioId, LocalDate transDate, LocalTime transTime) { | ||
| this.accountNo = accountNo; | ||
| this.portfolioId = portfolioId; | ||
| this.transDate = transDate; | ||
| this.transTime = transTime; | ||
| } | ||
|
|
||
| public String getAccountNo() { return accountNo; } | ||
| public void setAccountNo(String accountNo) { this.accountNo = accountNo; } | ||
| public String getPortfolioId() { return portfolioId; } | ||
| public void setPortfolioId(String portfolioId) { this.portfolioId = portfolioId; } | ||
| public LocalDate getTransDate() { return transDate; } | ||
| public void setTransDate(LocalDate transDate) { this.transDate = transDate; } | ||
| public LocalTime getTransTime() { return transTime; } | ||
| public void setTransTime(LocalTime transTime) { this.transTime = transTime; } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof Key key)) return false; | ||
| return Objects.equals(accountNo, key.accountNo) | ||
| && Objects.equals(portfolioId, key.portfolioId) | ||
| && Objects.equals(transDate, key.transDate) | ||
| && Objects.equals(transTime, key.transTime); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(accountNo, portfolioId, transDate, transTime); | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: POSHIST key omits sequence, collapsing same-second transactions
PositionHistory.Key matches POSHIST's primary key (account, portfolio, date, time) but omits the sequence number that the TRANHIST input key carries. Two input rows differing only by sequence collapse to one POSHIST key, and the writer skips the second as a duplicate. This faithfully mirrors the DDL and the COBOL -803 handling, but silently drops distinct transactions sharing a second.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct and intentional: the key mirrors the POSHIST DDL primary key (ACCOUNT_NO, PORTFOLIO_ID, TRANS_DATE, TRANS_TIME), and the collapse of same-second transactions is exactly what the COBOL's -803 CONTINUE produces against that table. Keeping DDL fidelity here; a wider key would change the target schema.
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
| /** | ||
| * End-to-end tests of the HISTLD00 Spring Batch migration against embedded H2. | ||
| */ | ||
| @SpringBootTest |
There was a problem hiding this comment.
🔍 Production contracts lack coverage
Tests use generated H2 tables only. Add coverage for DB2-compatible DDL, failed-job exit codes, fixed-width usernames, and non-destructive production startup.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fair points for hardening beyond this reference slice. Since 52291b0 the base profile uses ddl-auto: validate, so production startup is non-destructive by default; DB2-compatible DDL verification and failed-job exit-code semantics are noted as follow-ups (the exit-code question is open with the maintainer — current behavior intentionally mirrors COBOL MOVE WS-ERROR-COUNT TO RETURN-CODE).
| if (repository.existsById(item.getKey())) { | ||
| continue; | ||
| } | ||
| repository.saveAndFlush(item); |
There was a problem hiding this comment.
🟡 Bulk loader runs two queries per row and cannot batch inserts
Each record hits existsById and then saveAndFlush. Because the entity has an assigned @EmbeddedId, save routes to merge, which runs its own SELECT before the insert. Every new row therefore costs two SELECTs plus an insert, and flushing per item blocks JDBC batching, throttling what is meant to be a high-volume load.
Prompt for agents
HistoryItemWriter.write does existsById(key) followed by saveAndFlush(item) for every record. Two issues compound: (1) PositionHistory uses an assigned @EmbeddedId that is always non-null, so Spring Data's SimpleJpaRepository.save() considers the entity not-new and calls EntityManager.merge(), which performs a SELECT to load the row before inserting/updating; combined with the explicit existsById this is two SELECTs per new row. (2) Flushing each item individually prevents Hibernate JDBC batch inserts. For a bulk POSHIST load this is a significant throughput regression versus the COBOL EXEC SQL INSERT. Consider persisting via EntityManager.persist (or making the entity implement Persistable so save() uses persist), relying on the primary-key/duplicate exception (SQLCODE -803 equivalent) or a single existence strategy rather than existsById+merge, and enabling hibernate.jdbc.batch_size with a batched flush per chunk while still being able to attribute a failure to the offending record.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged as a deliberate trade-off for the reference slice: the per-item existsById + saveAndFlush buys exact -803 semantics and per-record error attribution at the cost of throughput. For a true high-volume load the right shape is persist (via Persistable) with hibernate.jdbc.batch_size and duplicate handling via the constraint exception — noting that as a follow-up optimization rather than reworking it in this PR, since correctness/fidelity is the priority for the reference pattern. Happy to do it here if maintainers prefer.
|
|
||
| @Override | ||
| public PositionHistory process(TransactionHistoryFileRecord item) { | ||
| stats.incrementRecordsRead(); |
There was a problem hiding this comment.
📝 Info: Read/error counts depend on processorNonTransactional()
recordsRead and validation errors are counted inside the processor, not at read time. This stays correct only because the step sets processorNonTransactional(), so the fault-tolerant scan reuses cached output instead of re-running process() on retry. Dropping that setting in a future slice built from this pattern would silently double-count reads and errors.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — added a code comment in 52291b0 marking processorNonTransactional() as required because the processor owns the read/error counters, so future slices copying this pattern don't drop it silently.
| control.setStatus(jobFailed || returnCode > HistoryLoadStats.MAX_ERRORS | ||
| ? BatchControlConstants.STAT_ERROR | ||
| : BatchControlConstants.STAT_DONE); |
There was a problem hiding this comment.
📝 Info: Terminal DONE/ERROR status is new behavior vs COBOL
markComplete writes a terminal DONE/ERROR status at job end. COBOL HISTLD00 sets ACTIVE at init and never writes a terminal status, so it leaves the control record ACTIVE. This is arguably an improvement, but it is not a faithful reproduction and should be a conscious choice for the reference pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional deviation: COBOL HISTLD00 indeed leaves the control record ACTIVE, but that makes success indistinguishable from a crash mid-run. Since BCHCON already defines D/E, writing a terminal status gives operators (and the restart-count logic) a reliable signal. It's documented as a conscious improvement; can revert to strict fidelity if preferred.
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
There was a problem hiding this comment.
🔍 Required verification is undocumented
The contribution rules require performance analysis and sample translation-tool testing. The PR records neither result, leaving mandatory validation incomplete.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| String validationError = validate(item); | ||
| if (validationError != null) { | ||
| long errors = stats.incrementErrorCount(); | ||
| errorHandlingService.logError("HISTLD00", "V", 2, "HIST0001", |
There was a problem hiding this comment.
🟡 Validation logs use undefined category
For validation failures, logError stores V although the table contract defines only system, application, or data codes. Reports receive an undefined category.
Prompt for agents
Map HISTLD00 validation failures to one of the ERRLOG.ERROR_TYPE values defined by src/database/db2/ERRLOG.sql and ErrorLog (S, A, or D), or formally extend the database contract and all consumers if a distinct validation category is required. Align ErrorHandlingService documentation with the chosen database values and add an assertion for the persisted category.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| .listener(new ChunkListener() { | ||
| @Override | ||
| public void afterChunk(ChunkContext context) { | ||
| JobParameters params = context.getStepContext() | ||
| .getStepExecution().getJobParameters(); | ||
| batchControlService.updateCheckpoint( | ||
| PROGRAM_ID, | ||
| params.getString("processDate"), | ||
| stats.getRecordsRead(), | ||
| stats.getRecordsWritten()); | ||
| } | ||
| }) |
There was a problem hiding this comment.
🔍 Checkpoints lag committed data
afterChunk updates control data in a separate transaction after item commit. An interruption can leave durable rows ahead of the recorded checkpoint.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
| /** | ||
| * Composite primary key = VSAM TRANHIST record key: | ||
| * Transaction Date (8) + Transaction Time (6) + Portfolio ID (8) + Sequence No (6). | ||
| */ |
There was a problem hiding this comment.
🔍 TRANHIST key layout needs resolution
The source declares a 20-byte key but lists components totaling 28 bytes. The entity adopts all four, leaving physical VSAM migration behavior ambiguous.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| private BatchControl find(String jobName, String processDate) { | ||
| return repository.findById(new BatchControl.Key(jobName, processDate, 1)) | ||
| .orElseThrow(() -> new FileProcessingException( | ||
| "Control record not found for job " + jobName | ||
| + " date " + processDate, "23")); | ||
| } |
There was a problem hiding this comment.
🔍 Control-record lookup key differs from COBOL
COBOL 1300-INIT-CHECKPOINTS reads the control record by job name alone (date and sequence left as spaces). find instead looks up (jobName, processDate, 1) with a hardcoded sequence of 1. Sample data and tests seed exactly this key, but a control record keyed differently is not found and the job fails. Worth confirming the intended BCHCTL key convention.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional convention: BCHCTL's RECORD KEY is (job name, process date, sequence), and the COBOL read-by-job-name-only works because the benchmark's VSAM file holds a single record per job. Keying the lookup on (jobName, processDate, 1) makes the control table hold one row per job per business date, which the checkpoint/restart design needs once multiple dates coexist in a table. Sequence is fixed at 1 because HISTLD00 is a single-step job; multi-step slices would pass their step sequence. Documented convention — happy to adjust if maintainers prefer strict job-name-only lookup.
|
|
||
| @Override | ||
| public PositionHistory process(TransactionHistoryFileRecord item) { | ||
| stats.incrementRecordsRead(); |
There was a problem hiding this comment.
📝 Info: recordsRead overstates on the insert-error abort path
The processor runs for every item in a chunk before the writer, so recordsRead reflects the whole chunk even when the writes then fail and the job aborts. COBOL increments its read counter record-by-record and stops earlier, so the migrated stat can overstate on the abort path. Not asserted by tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct observation — on the abort path recordsRead can reflect items processed ahead of the failing write, since chunk processing runs the processor for the whole chunk before writing. This is inherent to the chunk model; the counter is exact on all non-abort paths and the discrepancy is bounded by one chunk. Leaving as-is for the reference slice rather than instrumenting the reader, since the abort path already reports FAILED with status E.
| public void write(Chunk<? extends PositionHistory> chunk) { | ||
| long written = 0; | ||
| for (PositionHistory item : chunk) { | ||
| if (repository.existsById(item.getKey())) { | ||
| continue; | ||
| } | ||
| repository.saveAndFlush(item); | ||
| written++; | ||
| } | ||
| final long delta = written; | ||
| TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { | ||
| @Override | ||
| public void afterCommit() { | ||
| stats.addRecordsWritten(delta); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
📝 Info: afterCommit write counting handles rollback correctly
HistoryItemWriter adds the written delta only in afterCommit, so a chunk that inserts then rolls back is not counted, and the fault-tolerant per-item rescan re-counts committed items. Combined with the existsById duplicate skip this correctly mirrors the COBOL -803 CONTINUE without double-counting.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
| private String validate(TransactionHistoryFileRecord item) { | ||
| if (item.getKey() == null | ||
| || item.getKey().getTransDate() == null | ||
| || item.getKey().getTransTime() == null | ||
| || isBlank(item.getKey().getPortfolioId())) { | ||
| return "Missing transaction key fields"; | ||
| } | ||
| if (isBlank(item.getAccountNo())) { | ||
| return "Missing account number"; | ||
| } | ||
| if (item.getTransType() == null || !VALID_TRANS_TYPES.contains(item.getTransType())) { | ||
| return "Invalid transaction type: " + item.getTransType(); | ||
| } | ||
| if (isBlank(item.getSecurityId())) { | ||
| return "Missing security ID"; | ||
| } | ||
| if (isNullOrNegativeRequired(item.getQuantity()) | ||
| || isNullOrNegativeRequired(item.getPrice()) | ||
| || item.getAmount() == null | ||
| || item.getTotalAmount() == null | ||
| || item.getCostBasis() == null | ||
| || item.getGainLoss() == null) { | ||
| return "Missing or invalid numeric fields"; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🔍 Validation changes source behavior
validate filters records before insertion, while HISTLD00 directly moves every record. Define expected parity for records the database accepts.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| jpa: | ||
| hibernate: | ||
| # Safe default for a real (DB2) datasource; the histld00 sample profile | ||
| # and tests override this to create the embedded H2 schema. | ||
| ddl-auto: validate | ||
| open-in-view: false |
There was a problem hiding this comment.
🔍 Default profile cannot start against its own H2 datasource
The default datasource is embedded H2, but ddl-auto: validate with sql.init.mode: never creates no business schema, so Hibernate validation fails against the empty database and mvn spring-boot:run with no profile aborts startup. Only the histld00 profile or a DB2 URL override boots. Confirm the default is meant to be unusable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional: with no profile the app has nothing to do anyway (batch job launch is disabled; HISTLD00 only runs under the histld00 profile, which creates the H2 schema, and tests create it via test properties). The base profile exists as the safe template to repoint at DB2, where validate is the right default. If a usable no-profile H2 boot is wanted, I can move the schema creation to the base profile instead — but that reintroduces the destructive-default risk flagged earlier.
| .skipPolicy((throwable, skipCount) -> | ||
| throwable instanceof DataAccessException | ||
| && stats.getErrorCount() + skipCount < HistoryLoadStats.MAX_ERRORS) | ||
| .listener(new SkipListener<TransactionHistoryFileRecord, PositionHistory>() { | ||
| @Override | ||
| public void onSkipInWrite(PositionHistory item, Throwable t) { | ||
| errorHandlingService.logError(PROGRAM_ID, "S", 3, "HIST0002", | ||
| "POSHIST insert failed: " + t.getMessage(), | ||
| String.valueOf(item.getKey().getAccountNo() + "/" | ||
| + item.getKey().getPortfolioId())); | ||
| } | ||
| }) | ||
| .listener(new StepExecutionListener() { | ||
| @Override | ||
| public ExitStatus afterStep(StepExecution stepExecution) { | ||
| stats.addErrorCount(stepExecution.getWriteSkipCount()); | ||
| // The insert failure that breaches the limit is not | ||
| // skipped (it aborts the step); count and log it once | ||
| // here so RETURN-CODE is 101 like the validation path. | ||
| if (stepExecution.getStatus() == BatchStatus.FAILED | ||
| && stepExecution.getFailureExceptions().stream() | ||
| .anyMatch(HistoryLoadJobConfig::causedByDataAccess)) { | ||
| stats.incrementErrorCount(); | ||
| errorHandlingService.logError(PROGRAM_ID, "S", 3, "HIST0002", | ||
| "POSHIST insert failed: error limit exceeded", PROGRAM_ID); | ||
| } | ||
| return stepExecution.getExitStatus(); | ||
| } |
There was a problem hiding this comment.
📝 Info: Error accounting depends on skip-callback flush timing
The insert-abort path relies on Spring Batch flushing onSkipInWrite (which writes ERRLOG in REQUIRES_NEW) only on successful chunk completion, while skip counts still accumulate. When a whole chunk fails mid-scan, no per-item ERRLOG rows persist and only the afterStep row remains, matching the test. Correct but tightly coupled to Spring Batch internals and version-sensitive.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed on the coupling. The behavior is pinned by tests (abortsWithSameReturnCodeWhenInsertErrorsExceedOneHundred asserts both the RC and the ERRLOG contents on abort), so a Spring Batch upgrade that changes skip-callback timing would surface as a test failure rather than silent drift. Noting it as a known sensitivity for future slices built from this pattern.
1 participant
Summary
Adds a new
java/Maven project (Java 17, Spring Boot 3, Spring Batch, Spring Data JPA, H2) as the migration foundation for the COBOL Portfolio Management System, and migrates the transaction-history batch load (HISTLD00) end-to-end as the reference pattern. COBOL source undersrc/is untouched. The CICS online layer (src/programs/online,src/maps) is deliberately NOT migrated — flagged injava/MIGRATION.mdas a separate CICS→REST redesign.Conventions established (documented in
java/MIGRATION.md):model/copybook/): one POJO per01layout, original PIC clauses in Javadoc; COMP-3 financial fields →BigDecimal,PIC X(n)→String, level-88/constant copybooks →static finalconstants,OCCURS→List<>.domain/): DB2 tables (POSHIST→PositionHistory,ERRLOG→ErrorLog,RTNCODES,PORTFOLIO_MASTER,INVESTMENT_POSITIONS,TRANSACTION_HISTORY) map 1:1; VSAM KSDS files becomeVSAM_*tables keyed on their COBOL RECORD KEY via@EmbeddedId(TRANHIST→TransactionHistoryFileRecord,BCHCTL→BatchControl).common/):DB2CONN→DataSourceConfig(HikariCP pool replaces connect/retry),DB2CMT→TransactionHelper,ERRPROC→ErrorHandlingService.logError(...)writing ERRLOG in REQUIRES_NEW. FILE STATUS / SQLCA checks becomeFileProcessingException/SqlProcessingException.batch/):RepositoryItemReaderoverVSAM_TRANHISTin RECORD KEY orderHistoryItemProcessor— validation, TH-→PH- mapping, error counting; throwsErrorLimitExceededExceptionwhenWS-ERROR-COUNT > 100HistoryItemWriter— POSHIST insert, duplicates skipped like SQLCODE -803CONTINUEWS-COMMIT-THRESHOLD;afterChunkupdates the batch control record (2310-UPDATE-CHECKPOINT)HistoryLoadJobRunner(profilehistld00) returns the error count as the JVM exit code (MOVE WS-ERROR-COUNT TO RETURN-CODE)Verification
mvn test: 6 tests pass on embedded H2 — happy-path load, batch-control checkpoint/status, -803 duplicate skip, validation-error logging to ERRLOG, >100-error abort (status E, RC 101), missing control record failure.mvn spring-boot:run -Dspring-boot.run.profiles=histld00 -Dspring-boot.run.arguments=20240320→ 5 read / 5 written / 0 errors, COMPLETED.Link to Devin session: https://app.devin.ai/sessions/4f4cdf336e56491b9b6e92b6ef42d6d3
Requested by: @eashansinha
Devin Review
ed7214a