feat(batch): assemble csvImportStep and csvToJpaJob + run endpoint#54
feat(batch): assemble csvImportStep and csvToJpaJob + run endpoint#54
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the end-to-end CSV to JPA batch job processing system, assembling the previously built components into a complete working solution. The implementation includes the job configuration, step configuration, a REST endpoint for job execution, and comprehensive end-to-end testing.
Key changes:
- Assembles the CSV import step and job with proper dependency injection and parameter validation
- Adds a REST controller for executing the batch job with configurable parameters
- Implements comprehensive E2E testing using Testcontainers and PostgreSQL
- Updates database schema and configuration for batch processing support
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
CsvToJpaJobConfig.java |
Defines the main batch job with listeners, validators, and incrementers |
CsvImportStepConfig.java |
Configures the chunk-oriented step with reader, processor, and writer |
JobRunController.java |
REST endpoint for triggering batch job execution |
CsvToJpaJobE2ETest.java |
End-to-end test with Testcontainers and PostgreSQL |
ImportRecordProcessor.java |
Adds default constructor for @StepScope compatibility |
V2__batch_schema.sql |
Updates Spring Batch schema to newer format |
V4__create_import_errors.sql |
Creates table for tracking import errors |
application.yml & application-test.yml |
Adds batch configuration properties |
README.md |
Comprehensive documentation of the implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .addString("storagePath", req.storagePath()) | ||
| .addString("delimiter", req.delimiter() == null ? "," : req.delimiter()) | ||
| .addLong("chunkSize", req.chunkSize() == null ? 500L : req.chunkSize().longValue()) | ||
| .addString("requestTime", (req.requestTime() == null ? Instant.now() : req.requestTime()).toString()) |
There was a problem hiding this comment.
Converting Instant to String for job parameters may cause issues with date parsing and timezone handling. Consider using .addDate() with Date.from() or keeping it as a proper temporal type for better type safety.
| .addString("requestTime", (req.requestTime() == null ? Instant.now() : req.requestTime()).toString()) | |
| .addDate("requestTime", Date.from(req.requestTime() == null ? Instant.now() : req.requestTime())) |
| -- | ||
|
|
||
| CREATE TABLE BATCH_JOB_INSTANCE ( | ||
| -- V0__spring_batch_metadata_v5.sql (ESQUEMA NUEVO) |
There was a problem hiding this comment.
The comment references 'V0__spring_batch_metadata_v5.sql' but this file is named 'V2__batch_schema.sql'. This inconsistency could cause confusion about file versioning.
| -- V0__spring_batch_metadata_v5.sql (ESQUEMA NUEVO) | |
| -- V2__batch_schema.sql (ESQUEMA NUEVO) |
| row_num BIGINT NULL, | ||
| external_id VARCHAR(128) NULL, | ||
| reason TEXT NOT NULL, | ||
| raw_line TEXT NULL, |
There was a problem hiding this comment.
[nitpick] The column alignment is inconsistent with the other columns above. All other columns have proper spacing alignment while 'reason' has extra spaces.
| raw_line TEXT NULL, | |
| reason TEXT NOT NULL, | |
| raw_line TEXT NULL, |
| properties = { | ||
| "spring.batch.job.enabled=false", | ||
| "spring.flyway.enabled=true", | ||
| "spring.flyway.locations=classpath:db/migration", // solo tus tablas de negocio |
There was a problem hiding this comment.
The Spanish comment 'solo tus tablas de negocio' should be in English to maintain consistency with the codebase language. Consider translating to 'only business tables' or similar.
| "spring.flyway.locations=classpath:db/migration", // solo tus tablas de negocio | |
| "spring.flyway.locations=classpath:db/migration", // only business tables |
| "spring.batch.job.enabled=false", | ||
| "spring.flyway.enabled=true", | ||
| "spring.flyway.locations=classpath:db/migration", // solo tus tablas de negocio | ||
| "spring.batch.jdbc.initialize-schema=never", // lo hacemos manual |
There was a problem hiding this comment.
The Spanish comment 'lo hacemos manual' should be in English. Consider translating to 'we do it manually' or 'manual initialization'.
| "spring.batch.jdbc.initialize-schema=never", // lo hacemos manual | |
| "spring.flyway.locations=classpath:db/migration", // only your business tables | |
| "spring.batch.jdbc.initialize-schema=never", // we do it manually |
|
|
||
| @BeforeAll | ||
| void initBatchSchema() { | ||
| // Drop + Create de las tablas BATCH_* correctas para PostgreSQL |
There was a problem hiding this comment.
The Spanish comment should be in English. Consider translating to 'Drop + Create the correct BATCH_* tables for PostgreSQL'.
| // Drop + Create de las tablas BATCH_* correctas para PostgreSQL | |
| // Drop + Create the correct BATCH_* tables for PostgreSQL |
No description provided.