feat(spring-batch-file-example): implement Spring Batch file processi… #155
Conversation
…ng with custom readers for small and large Excel files; refactor services and tests
WalkthroughRestructures a Spring Batch file-reading project from a single async Excel pipeline to support multiple file reader strategies. Removes async configuration, introduces separate small and large Excel readers with distinct implementations, reorganizes job definitions to support dynamic job selection via execution type parameter, and renames service/controller components for clarity (TestService/TestController → StudentService/StudentController). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
spring-batch-file-examples/README.md (1)
1-56: Consider fixing markdown formatting for consistency.The documentation updates accurately reflect the new architecture. However, there are minor markdown formatting issues flagged by the linter:
- List indentation (Lines 25-26, 29-32): Nested list items use 4-space indentation, but the markdown standard expects 2 spaces.
- Trailing newline (Line 56): File should end with a single newline character.
♻️ Proposed fix for list indentation
- **Small Excel files (`.xlsx`)** - - Suitable for files that fit comfortably in memory - - Simple and fast processing + - Suitable for files that fit comfortably in memory + - Simple and fast processing - **Large Excel files (`.xlsx`)** - - Streaming-based reader - - Designed for large datasets - - Low memory footprint - - Handles empty rows gracefully + - Streaming-based reader + - Designed for large datasets + - Low memory footprint + - Handles empty rows gracefullyAnd add a newline at the end of the file (after line 56).
spring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.java (1)
51-56: Consider parameterizing the file path.The
filePathis hardcoded to"files/students.xlsx". If future use cases require different files per job type or request, consider accepting the file path viaExecutionDtoor making it configurable per job type.spring-batch-file-examples/src/test/java/com/io/example/controller/TestControllerStudents.java (1)
43-60: Consider using@EnumSourcefor cleaner parameterization.Using
@EnumSource(JobsType.class)would be more type-safe and automatically include any future job types added to the enum.Suggested change
@ParameterizedTest - @ValueSource(strings = { - "SMALL_EXCEL", - "LARGE_EXCEL" - }) + @EnumSource(JobsType.class) @DisplayName("POST /job/process → should return job ID when service runs successfully") - void shouldReturnJobIdWhenProcessJobIsCalled(String jobName) throws Exception { - var dto = new ExecutionDto(JobsType.valueOf(jobName)); + void shouldReturnJobIdWhenProcessJobIsCalled(JobsType jobType) throws Exception { + var dto = new ExecutionDto(jobType);
🤖 Fix all issues with AI agents
In
@spring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.java:
- Around line 34-38: The inline row mapper in LargeExcelReadBatchConfig that
creates StudentDto from row.getCell(...).getStringCellValue() and
LocalDate.parse(...) is fragile; replace it with reuse of the existing
StudentMapper from SmallExcelReadBatchConfig or implement equivalent defensive
logic: check for null cells and cell types (use fallback to cell.toString() or
empty string), catch DateTimeParseException around LocalDate.parse and apply a
safe parse or default value, and ensure StudentDto construction uses the
sanitized values; locate the lambda that constructs new StudentDto and either
call new StudentMapper().map(row) or add the null/type checks and try/catch
around date parsing before creating StudentDto.
In
@spring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.java:
- Around line 38-41: Remove the unnecessary identity ItemProcessor bean in
SmallExcelReadBatchConfig by deleting the smallExcelProcessor() method and
update the step configuration that currently references it to build the step
without a processor (i.e., use .<chunk/reader/writer> configuration without
.processor(...)); apply the same change to LargeExcelReadBatchConfig for
consistency so both configs omit the identity processor bean and steps run
directly from reader to writer.
In
@spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java:
- Around line 60-75: The read() method wrongly increments currentRow before
checking isRowEmpty, causing skipped rows on restart; change read() so you only
advance currentRow when you actually return a non-empty row (i.e., move the
currentRow increment to after the isRowEmpty check or increment it just before
returning the mapped value), and ensure the value currentRow represents the last
returned row so open()’s seek logic remains consistent with currentRow, using
the rowIterator, read(), isRowEmpty(Row) and open() methods to locate and update
the state.
- Around line 39-58: The open() method hides IOExceptions and miscalculates
restart skipping: remove the Lombok @SneakyThrows and declare
open(ExecutionContext) throws IOException so IO errors propagate, and change the
skip calculation from Math.max(linesToSkip, currentRow) to skip = linesToSkip +
currentRow (so header + all processed rows are skipped on restart); keep using
resource.getInputStream(), StreamingReader.builder() and rowIterator, and ensure
the for-loop advances rowIterator skip times as before.
- Around line 19-22: The class StreamingExcelItemReader<T> is a generic reader
annotated with @Component which can cause a single bean to be created and lead
to type conflicts; remove the @Component annotation from
StreamingExcelItemReader<T>, keep @StepScope, and instead define explicit @Bean
factory methods in your configuration (e.g., LargeExcelReadBatchConfig) that
return concrete parameterized types of StreamingExcelItemReader with @Bean and
@StepScope (or @StepScope on the factory) so each step gets its own
correctly-typed reader instance.
- Around line 82-88: Remove the Lombok @SneakyThrows from the
StreamingExcelItemReader.close() method and implement explicit IOException
handling: replace the sneaky throws with a normal method that wraps
workbook.close() in a try/catch, catch IOException, and log the cleanup failure
using the class logger (or add a logger if none exists) so resource-close errors
are recorded without propagating unexpected checked exceptions. Ensure the
method still attempts to close only when workbook != null and does not rethrow
the IOException.
In
@spring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.java:
- Around line 25-28: In runJob(ExecutionDto dto) the call
jobs.get(dto.getType().getJobName()) can return null and leads to an NPE when
invoking job.getName(); add null-safe validation: ensure dto and dto.getType()
are not null, retrieve the job into a local variable (job) and if job == null
either throw an IllegalArgumentException/CustomException with a clear message
including the requested jobName or return a failure/optional result per your
method contract, and only call job.getName() after that check; reference the
runJob method and the jobs map/get call to locate where to add the validation
and error handling.
In
@spring-batch-file-examples/src/test/java/com/io/example/controller/TestControllerStudents.java:
- Around line 29-32: Rename the test class TestControllerStudents to
StudentControllerTest and update the @DisplayName value to reflect
"StudentController - Unit tests with MockMvc"; keep the
@WebMvcTest(StudentController.class) annotation and
@Import(GlobalHandlerException.class) as-is so the controller under test and
exception handler remain unchanged.
In
@spring-batch-file-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.java:
- Around line 49-55: The jobs map in setUp() uses string keys "SMALL_JOB" and
"LARGE_JOB" which don't match the JobsType enum (SMALL_EXCEL, LARGE_EXCEL);
update the map to use JobsType enum values (e.g., JobsType.SMALL_EXCEL and
JobsType.LARGE_EXCEL) or use JobsType.name() strings to keep consistency, and
adjust any tests that reference the keys (such as the @ValueSource in the
parameterized test) to use the enum values or their names accordingly; look for
setUp(), jobs, smallExcelJob, largeExcelJob, JobsType and the @ValueSource usage
to make the changes.
In
@spring-batch-file-examples/src/test/java/com/io/example/service/FileBatchServiceImplTest.java:
- Around line 52-53: Replace the mocked ExecutionDto field with a real instance:
remove the @Mock on ExecutionDto and in the shouldRunJobSuccessfully test (or
test setup) create a new ExecutionDto(...) with the same sample values used in
shouldThrowBusinessExceptionWhenJobFails so both tests use a concrete DTO;
update any stubbing or verify calls that relied on the mock to use the real
object's state (e.g., use the real instance when calling fileBatchService.runJob
or asserting interactions).
In
@spring-batch-file-examples/src/test/java/com/io/example/service/StudentServiceImplStudent.java:
- Line 8: Rename the test class from StudentServiceImplStudent to
StudentServiceImplTest and update the class declaration (class
StudentServiceImplTest {) to follow JUnit naming conventions; also rename the
file from StudentServiceImplStudent.java to StudentServiceImplTest.java and
update any references/imports or test suite entries that reference
StudentServiceImplStudent to the new class name.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
spring-batch-file-examples/README.mdspring-batch-file-examples/pom.xmlspring-batch-file-examples/src/main/java/com/io/example/README.mdspring-batch-file-examples/src/main/java/com/io/example/config/ExcelBatchConfig.javaspring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.javaspring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.javaspring-batch-file-examples/src/main/java/com/io/example/controller/StudentController.javaspring-batch-file-examples/src/main/java/com/io/example/dto/ExecutionDto.javaspring-batch-file-examples/src/main/java/com/io/example/enums/JobsType.javaspring-batch-file-examples/src/main/java/com/io/example/job/FilesJob.javaspring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.javaspring-batch-file-examples/src/main/java/com/io/example/service/FileBatchService.javaspring-batch-file-examples/src/main/java/com/io/example/service/StudentService.javaspring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.javaspring-batch-file-examples/src/main/java/com/io/example/service/impl/StudentServiceImpl.javaspring-batch-file-examples/src/test/java/com/io/example/controller/TestControllerStudents.javaspring-batch-file-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.javaspring-batch-file-examples/src/test/java/com/io/example/service/FileBatchServiceImplTest.javaspring-batch-file-examples/src/test/java/com/io/example/service/StudentServiceImplStudent.java
💤 Files with no reviewable changes (2)
- spring-batch-file-examples/src/main/java/com/io/example/config/ExcelBatchConfig.java
- spring-batch-file-examples/src/main/java/com/io/example/README.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: igorcampos-dev
Repo: igorcampos-dev/spring-integrations-examples PR: 40
File: spring-kafka-example/src/main/java/com/io/example/consumer/KafkaConsumerServiceImpl.java:18-22
Timestamp: 2025-07-27T19:57:31.204Z
Learning: In the spring-integrations-examples project, user igorcampos-dev prefers to keep simple example implementations minimal without additional error handling complexity, as the examples are for demonstration purposes rather than production use.
Learnt from: igorcampos-dev
Repo: igorcampos-dev/spring-integrations-examples PR: 21
File: spring-kafka-example/README.md:12-17
Timestamp: 2025-07-22T01:35:41.406Z
Learning: User igorcampos-dev prefers minimal, high-level documentation in README files without detailed version specifications, choosing to keep tech stack information simple rather than adding precise version numbers.
Learnt from: igorcampos-dev
Repo: igorcampos-dev/spring-integrations-examples PR: 21
File: spring-kafka-example/compose.yaml:30-32
Timestamp: 2025-07-22T01:30:57.054Z
Learning: User igorcampos-dev prefers to maintain environment parity between local development and production, avoiding development-specific configurations like ALLOW_PLAINTEXT_LISTENER even when they might provide clearer intent or safety nets.
Learnt from: igorcampos-dev
Repo: igorcampos-dev/spring-integrations-examples PR: 26
File: spring-kafka-example/compose.yaml:3-4
Timestamp: 2025-07-24T01:40:49.337Z
Learning: User igorcampos-dev requires hard-coded container names in Docker Compose files (like `container_name: app`) for CI pipeline validation purposes, which is a valid operational requirement that overrides the general best practice of avoiding hard-coded container names.
🧬 Code graph analysis (11)
spring-batch-file-examples/src/main/java/com/io/example/controller/StudentController.java (1)
spring-batch-file-examples/src/main/java/com/io/example/controller/TestController.java (2)
RestController(12-31)GetMapping(19-23)
spring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.java (3)
spring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.java (1)
Configuration(20-63)spring-batch-file-examples/src/main/java/com/io/example/config/ExcelBatchConfig.java (7)
Bean(76-89)Bean(34-45)Bean(66-69)Configuration(28-91)Bean(57-64)Bean(71-74)Bean(47-55)spring-batch-db-examples/src/main/java/com/io/example/config/BatchConfig.java (1)
Configuration(37-124)
spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java (1)
spring-batch-file-examples/src/main/java/com/io/example/config/ExcelBatchConfig.java (6)
Bean(34-45)Bean(76-89)Bean(71-74)Bean(66-69)Bean(47-55)Bean(57-64)
spring-batch-file-examples/README.md (2)
spring-batch-db-examples/src/main/java/com/io/example/config/BatchConfig.java (1)
Configuration(37-124)spring-batch-file-examples/src/main/java/com/io/example/config/ExcelBatchConfig.java (5)
Configuration(28-91)Bean(71-74)Bean(76-89)Bean(47-55)Bean(57-64)
spring-batch-file-examples/src/test/java/com/io/example/service/StudentServiceImplStudent.java (3)
spring-batch-file-examples/src/test/java/com/io/example/service/TestServiceImplTest.java (2)
TestServiceImplTest(8-18)Test(12-16)spring-batch-file-examples/src/main/java/com/io/example/service/impl/TestServiceImpl.java (2)
Slf4j(8-17)Override(12-15)spring-batch-file-examples/src/main/java/com/io/example/service/TestService.java (2)
TestService(5-7)
spring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.java (2)
spring-batch-file-examples/src/main/java/com/io/example/mapper/StudentMapper.java (1)
StudentMapper(9-21)spring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.java (1)
Configuration(21-69)
spring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.java (1)
spring-batch-db-examples/src/main/java/com/io/example/service/impl/DBBatchServiceImpl.java (1)
Slf4j(12-51)
spring-batch-file-examples/src/main/java/com/io/example/service/impl/StudentServiceImpl.java (1)
spring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.java (1)
Slf4j(15-58)
spring-batch-file-examples/src/test/java/com/io/example/controller/TestControllerStudents.java (1)
spring-batch-db-examples/src/test/java/com/io/example/controller/TestControllerTest.java (1)
DisplayName(22-71)
spring-batch-file-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.java (2)
spring-batch-db-examples/src/test/java/com/io/example/controller/TestControllerTest.java (1)
DisplayName(22-71)spring-batch-db-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.java (1)
SpringBootTest(25-86)
spring-batch-file-examples/src/test/java/com/io/example/service/FileBatchServiceImplTest.java (2)
spring-batch-db-examples/src/main/java/com/io/example/exception/BusinessException.java (1)
BusinessException(3-13)spring-batch-db-examples/src/test/java/com/io/example/service/DBBatchServiceImplTest.java (1)
ExtendWith(23-93)
🪛 markdownlint-cli2 (0.18.1)
spring-batch-file-examples/README.md
25-25: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (21)
spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java (2)
77-80: LGTM!The state persistence logic correctly saves the current position to the
ExecutionContext, enabling proper restart capability.
90-101: LGTM!The empty row detection logic is correct and handles edge cases properly (null rows, blank cells).
spring-batch-file-examples/src/main/java/com/io/example/service/FileBatchService.java (1)
1-9: LGTM!The interface update to accept
ExecutionDtoenables dynamic job selection, which aligns well with the PR's objective of supporting multiple file reader strategies.spring-batch-file-examples/src/main/java/com/io/example/service/StudentService.java (1)
1-7: LGTM!Renaming
TestServicetoStudentServiceimproves code clarity and better reflects the purpose of the interface.spring-batch-file-examples/pom.xml (2)
92-110: LGTM!The dependency additions are appropriate:
- The
excel-streaming-readerdependency supports the newStreamingExcelItemReaderimplementation- Explicit versioning of test dependencies improves build reproducibility and maintainability
31-31: Use excel-streaming-reader version 5.1.1 instead of 5.2.0.Version 5.2.0 does not exist on Maven Central Repository. The latest available version is 5.1.1 (released July 2025). No known CVEs are reported against the 5.x line.
Likely an incorrect or invalid review comment.
spring-batch-file-examples/src/main/java/com/io/example/dto/ExecutionDto.java (1)
1-12: LGTM! Clean DTO design.The DTO is well-structured with appropriate Lombok annotations for a simple data carrier. The single-field design keeps it focused on its purpose of specifying the job execution type.
spring-batch-file-examples/src/main/java/com/io/example/controller/StudentController.java (2)
13-13: Good refactoring: Controller renamed to reflect domain.The rename from
TestControllertoStudentControllerimproves code clarity and aligns with the Student-focused domain model.
17-20: Appropriate change from GET to POST for request body.The switch from
@GetMappingto@PostMappingwith@RequestBody ExecutionDto dtois semantically correct. The endpoint now properly accepts a payload to specify the job type.spring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.java (2)
26-36: Well-configured step-scoped reader.The
PoiItemReaderis properly configured with:
- Step-scoped bean for job parameter binding
- Header row skipping via
setLinesToSkip(1)- Custom
StudentMapperfor row mapping
48-62: Step configuration looks correct.The step is properly configured with chunk-oriented processing, injecting the required Spring Batch infrastructure components and using the configured chunk size from properties.
spring-batch-file-examples/src/main/java/com/io/example/enums/JobsType.java (1)
1-14: LGTM! Clean enum design.The enum effectively maps execution types to their corresponding Spring Batch job names. Lombok annotations are appropriately used for the getter and constructor generation.
spring-batch-file-examples/src/test/java/com/io/example/service/StudentServiceImplStudent.java (1)
4-10: Service import and field updated correctly.The import and field declaration properly reference the renamed
StudentServiceImplclass, maintaining consistency with the service layer refactoring.spring-batch-file-examples/src/main/java/com/io/example/service/impl/StudentServiceImpl.java (1)
1-17: LGTM!Clean rename from
TestServiceImpltoStudentServiceImplthat better reflects the domain. The implementation is appropriately minimal for a demonstration project.spring-batch-file-examples/src/main/java/com/io/example/job/FilesJob.java (1)
11-27: LGTM!Good separation of job definitions for small and large Excel processing. The dual-job approach cleanly supports the new dynamic job selection via
ExecutionDto.Note:
@RequiredArgsConstructoron line 12 is unused since there are no instance fields, but it's harmless.spring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.java (1)
52-67: LGTM!The step configuration properly wires the reader, processor, and writer with chunk-based processing. Using
@Valuefor chunk-size from properties is a good practice for configurability.spring-batch-file-examples/src/test/java/com/io/example/service/FileBatchServiceImplTest.java (2)
69-86: LGTM on parameterized test structure.Good use of
@MethodSourceto cover both job types. The verification of the correct job being passed to the launcher is appropriate.
128-133: LGTM!The
scenarios()method cleanly provides test data for both job types, ensuring coverage of the multi-job execution paths.spring-batch-file-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.java (3)
57-73: LGTM!The parameterized integration test properly exercises both job types with real file reading. Using
atLeastOnce()for verification is appropriate since chunk size determines exact call count.
75-90: LGTM!Good failure scenario testing using a dedicated error file (
students-error.xlsx). Properly verifies that the service is never called when the job fails.
92-109: LGTM!Empty data scenario is well tested, verifying job completion without any service interactions.
| row -> new StudentDto( | ||
| row.getCell(0).getStringCellValue(), | ||
| row.getCell(1).getStringCellValue(), | ||
| LocalDate.parse(row.getCell(2).getStringCellValue()) | ||
| ) |
There was a problem hiding this comment.
Add defensive checks for cell values and date parsing.
The inline row mapper is fragile:
getStringCellValue()will throw if a cell is null or not a string typeLocalDate.parse()assumes ISO-8601 format and will throwDateTimeParseExceptionon invalid input
The SmallExcelReadBatchConfig uses a dedicated StudentMapper class, which likely handles these edge cases. Consider reusing that mapper or adding similar defensive logic here.
Proposed defensive implementation
- row -> new StudentDto(
- row.getCell(0).getStringCellValue(),
- row.getCell(1).getStringCellValue(),
- LocalDate.parse(row.getCell(2).getStringCellValue())
- )
+ row -> {
+ var cell0 = row.getCell(0);
+ var cell1 = row.getCell(1);
+ var cell2 = row.getCell(2);
+ return new StudentDto(
+ cell0 != null ? cell0.getStringCellValue() : null,
+ cell1 != null ? cell1.getStringCellValue() : null,
+ cell2 != null ? LocalDate.parse(cell2.getStringCellValue()) : null
+ );
+ }Based on learnings, this is a demonstration project, so minimal error handling may be acceptable if intentional.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| row -> new StudentDto( | |
| row.getCell(0).getStringCellValue(), | |
| row.getCell(1).getStringCellValue(), | |
| LocalDate.parse(row.getCell(2).getStringCellValue()) | |
| ) | |
| row -> { | |
| var cell0 = row.getCell(0); | |
| var cell1 = row.getCell(1); | |
| var cell2 = row.getCell(2); | |
| return new StudentDto( | |
| cell0 != null ? cell0.getStringCellValue() : null, | |
| cell1 != null ? cell1.getStringCellValue() : null, | |
| cell2 != null ? LocalDate.parse(cell2.getStringCellValue()) : null | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/config/LargeExcelReadBatchConfig.java
around lines 34 - 38, The inline row mapper in LargeExcelReadBatchConfig that
creates StudentDto from row.getCell(...).getStringCellValue() and
LocalDate.parse(...) is fragile; replace it with reuse of the existing
StudentMapper from SmallExcelReadBatchConfig or implement equivalent defensive
logic: check for null cells and cell types (use fallback to cell.toString() or
empty string), catch DateTimeParseException around LocalDate.parse and apply a
safe parse or default value, and ensure StudentDto construction uses the
sanitized values; locate the lambda that constructs new StudentDto and either
call new StudentMapper().map(row) or add the null/type checks and try/catch
around date parsing before creating StudentDto.
| @Bean | ||
| public ItemProcessor<StudentDto, StudentDto> smallExcelProcessor() { | ||
| return student -> student; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing the identity processor.
The processor returns the input unchanged (student -> student). While this pattern is consistent with LargeExcelReadBatchConfig, you could simplify the step configuration by omitting the processor entirely if no transformation is needed.
♻️ Simplification option
If no processing is required, remove the processor bean and update the step configuration:
- @Bean
- public ItemProcessor<StudentDto, StudentDto> smallExcelProcessor() {
- return student -> student;
- }
-
@Bean
public Step smallExcelStep(JobRepository jobRepository,
PlatformTransactionManager transactionManager,
PoiItemReader<StudentDto> smallExcelReader,
- ItemProcessor<StudentDto, StudentDto> smallExcelProcessor,
ItemWriter<StudentDto> smallExcelWriter,
@Value("${spring.batch.chunk-size}") int chunkSize) {
return new StepBuilder("smallExcelStep", jobRepository)
.<StudentDto, StudentDto>chunk(chunkSize, transactionManager)
.reader(smallExcelReader)
- .processor(smallExcelProcessor)
.writer(smallExcelWriter)
.build();
}Note: Apply the same simplification to LargeExcelReadBatchConfig for consistency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/config/SmallExcelReadBatchConfig.java
around lines 38 - 41, Remove the unnecessary identity ItemProcessor bean in
SmallExcelReadBatchConfig by deleting the smallExcelProcessor() method and
update the step configuration that currently references it to build the step
without a processor (i.e., use .<chunk/reader/writer> configuration without
.processor(...)); apply the same change to LargeExcelReadBatchConfig for
consistency so both configs omit the identity processor bean and steps run
directly from reader to writer.
| @Component | ||
| @StepScope | ||
| public class StreamingExcelItemReader<T> | ||
| implements ItemStreamReader<T> { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing @component from this generic class.
Since StreamingExcelItemReader<T> is a generic class, marking it with @Component may cause Spring to create a single bean instance, which could lead to type conflicts when multiple reader instances with different generic types are needed. The class is already annotated with @StepScope, which is appropriate for step-level lifecycle management.
Consider removing @Component and creating explicit @Bean methods in your configuration classes (e.g., LargeExcelReadBatchConfig) where you can specify the concrete type and scope.
♻️ Proposed fix
-@Component
@StepScope
public class StreamingExcelItemReader<T>
implements ItemStreamReader<T> {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java
around lines 19 - 22, The class StreamingExcelItemReader<T> is a generic reader
annotated with @Component which can cause a single bean to be created and lead
to type conflicts; remove the @Component annotation from
StreamingExcelItemReader<T>, keep @StepScope, and instead define explicit @Bean
factory methods in your configuration (e.g., LargeExcelReadBatchConfig) that
return concrete parameterized types of StreamingExcelItemReader with @Bean and
@StepScope (or @StepScope on the factory) so each step gets its own
correctly-typed reader instance.
| @Override | ||
| @SneakyThrows(IOException.class) | ||
| public void open(ExecutionContext context) { | ||
| this.currentRow = context.getInt("current.row", 0); | ||
|
|
||
| InputStream is = resource.getInputStream(); | ||
| this.workbook = StreamingReader.builder() | ||
| .rowCacheSize(100) | ||
| .bufferSize(4096) | ||
| .open(is); | ||
|
|
||
| Sheet sheet = workbook.getSheetAt(0); | ||
| this.rowIterator = sheet.iterator(); | ||
|
|
||
| int linesToSkip = 1; | ||
| int skip = Math.max(linesToSkip, currentRow); | ||
| for (int i = 0; i < skip && rowIterator.hasNext(); i++) { | ||
| rowIterator.next(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Fix restart logic and exception handling.
Two critical issues in the open() method:
-
Incorrect restart logic (Lines 53-57): The skip calculation
Math.max(linesToSkip, currentRow)is incorrect for restart scenarios. When restarting atcurrentRow=5, this only skips 5 rows total (header + rows 1-4), but you need to skip the header PLUS all processed rows (6 total) to resume at row 5. -
Hidden exceptions (Line 40): Using
@SneakyThrowssuppressesIOException, preventing proper batch job failure handling and restart capabilities. Spring Batch needs to detect failures to trigger restarts.
🐛 Proposed fix
@Override
-@SneakyThrows(IOException.class)
-public void open(ExecutionContext context) {
+public void open(ExecutionContext context) throws IOException {
this.currentRow = context.getInt("current.row", 0);
InputStream is = resource.getInputStream();
this.workbook = StreamingReader.builder()
.rowCacheSize(100)
.bufferSize(4096)
.open(is);
Sheet sheet = workbook.getSheetAt(0);
this.rowIterator = sheet.iterator();
int linesToSkip = 1;
- int skip = Math.max(linesToSkip, currentRow);
+ int skip = linesToSkip + currentRow;
for (int i = 0; i < skip && rowIterator.hasNext(); i++) {
rowIterator.next();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| @SneakyThrows(IOException.class) | |
| public void open(ExecutionContext context) { | |
| this.currentRow = context.getInt("current.row", 0); | |
| InputStream is = resource.getInputStream(); | |
| this.workbook = StreamingReader.builder() | |
| .rowCacheSize(100) | |
| .bufferSize(4096) | |
| .open(is); | |
| Sheet sheet = workbook.getSheetAt(0); | |
| this.rowIterator = sheet.iterator(); | |
| int linesToSkip = 1; | |
| int skip = Math.max(linesToSkip, currentRow); | |
| for (int i = 0; i < skip && rowIterator.hasNext(); i++) { | |
| rowIterator.next(); | |
| } | |
| } | |
| @Override | |
| public void open(ExecutionContext context) throws IOException { | |
| this.currentRow = context.getInt("current.row", 0); | |
| InputStream is = resource.getInputStream(); | |
| this.workbook = StreamingReader.builder() | |
| .rowCacheSize(100) | |
| .bufferSize(4096) | |
| .open(is); | |
| Sheet sheet = workbook.getSheetAt(0); | |
| this.rowIterator = sheet.iterator(); | |
| int linesToSkip = 1; | |
| int skip = linesToSkip + currentRow; | |
| for (int i = 0; i < skip && rowIterator.hasNext(); i++) { | |
| rowIterator.next(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java
around lines 39 - 58, The open() method hides IOExceptions and miscalculates
restart skipping: remove the Lombok @SneakyThrows and declare
open(ExecutionContext) throws IOException so IO errors propagate, and change the
skip calculation from Math.max(linesToSkip, currentRow) to skip = linesToSkip +
currentRow (so header + all processed rows are skipped on restart); keep using
resource.getInputStream(), StreamingReader.builder() and rowIterator, and ensure
the for-loop advances rowIterator skip times as before.
| @Override | ||
| public T read() { | ||
| while (rowIterator.hasNext()) { | ||
|
|
||
| Row row = rowIterator.next(); | ||
| currentRow++; | ||
|
|
||
| if (isRowEmpty(row)) { | ||
| continue; | ||
| } | ||
|
|
||
| return rowMapper.apply(row); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Fix state tracking to prevent data loss on restart.
The currentRow is incremented (Line 65) before checking if the row is empty (Line 67), which causes a critical flaw in restart scenarios:
- If row 5 is empty and skipped,
currentRowis still incremented to 5 - On restart,
open()skips to row 5, causing row 6 (which may contain data) to be skipped - This results in data loss
🐛 Proposed fix
@Override
public T read() {
while (rowIterator.hasNext()) {
Row row = rowIterator.next();
- currentRow++;
if (isRowEmpty(row)) {
continue;
}
+ currentRow++;
return rowMapper.apply(row);
}
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public T read() { | |
| while (rowIterator.hasNext()) { | |
| Row row = rowIterator.next(); | |
| currentRow++; | |
| if (isRowEmpty(row)) { | |
| continue; | |
| } | |
| return rowMapper.apply(row); | |
| } | |
| return null; | |
| } | |
| @Override | |
| public T read() { | |
| while (rowIterator.hasNext()) { | |
| Row row = rowIterator.next(); | |
| if (isRowEmpty(row)) { | |
| continue; | |
| } | |
| currentRow++; | |
| return rowMapper.apply(row); | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/reader/StreamingExcelItemReader.java
around lines 60 - 75, The read() method wrongly increments currentRow before
checking isRowEmpty, causing skipped rows on restart; change read() so you only
advance currentRow when you actually return a non-empty row (i.e., move the
currentRow increment to after the isRowEmpty check or increment it just before
returning the mapped value), and ensure the value currentRow represents the last
returned row so open()’s seek logic remains consistent with currentRow, using
the rowIterator, read(), isRowEmpty(Row) and open() methods to locate and update
the state.
| public Long runJob(ExecutionDto dto) { | ||
| Job job = jobs.get(dto.getType().getJobName()); | ||
|
|
||
| String jobName = job.getName(); |
There was a problem hiding this comment.
Handle missing job type gracefully.
If dto.getType().getJobName() returns a key not present in the jobs map, jobs.get() returns null, causing an NPE on line 28 when calling job.getName().
Proposed fix with validation
public Long runJob(ExecutionDto dto) {
Job job = jobs.get(dto.getType().getJobName());
+ if (job == null) {
+ throw new BusinessException("Unknown job type: " + dto.getType());
+ }
String jobName = job.getName();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Long runJob(ExecutionDto dto) { | |
| Job job = jobs.get(dto.getType().getJobName()); | |
| String jobName = job.getName(); | |
| public Long runJob(ExecutionDto dto) { | |
| Job job = jobs.get(dto.getType().getJobName()); | |
| if (job == null) { | |
| throw new BusinessException("Unknown job type: " + dto.getType()); | |
| } | |
| String jobName = job.getName(); |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/main/java/com/io/example/service/impl/FileBatchServiceImpl.java
around lines 25 - 28, In runJob(ExecutionDto dto) the call
jobs.get(dto.getType().getJobName()) can return null and leads to an NPE when
invoking job.getName(); add null-safe validation: ensure dto and dto.getType()
are not null, retrieve the job into a local variable (job) and if job == null
either throw an IllegalArgumentException/CustomException with a clear message
including the requested jobName or return a failure/optional result per your
method contract, and only call job.getName() after that check; reference the
runJob method and the jobs map/get call to locate where to add the validation
and error handling.
| @WebMvcTest(StudentController.class) | ||
| @DisplayName("TestController - Unit tests with MockMvc") | ||
| @Import(GlobalHandlerException.class) | ||
| class TestControllerTest { | ||
| class TestControllerStudents { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent class and display naming.
The test class is named TestControllerStudents but tests StudentController. Consider renaming to StudentControllerTest for consistency. Also, the @DisplayName still references "TestController" rather than "StudentController".
Suggested naming fix
@WebMvcTest(StudentController.class)
-@DisplayName("TestController - Unit tests with MockMvc")
+@DisplayName("StudentController - Unit tests with MockMvc")
@Import(GlobalHandlerException.class)
-class TestControllerStudents {
+class StudentControllerTest {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @WebMvcTest(StudentController.class) | |
| @DisplayName("TestController - Unit tests with MockMvc") | |
| @Import(GlobalHandlerException.class) | |
| class TestControllerTest { | |
| class TestControllerStudents { | |
| @WebMvcTest(StudentController.class) | |
| @DisplayName("StudentController - Unit tests with MockMvc") | |
| @Import(GlobalHandlerException.class) | |
| class StudentControllerTest { |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/test/java/com/io/example/controller/TestControllerStudents.java
around lines 29 - 32, Rename the test class TestControllerStudents to
StudentControllerTest and update the @DisplayName value to reflect
"StudentController - Unit tests with MockMvc"; keep the
@WebMvcTest(StudentController.class) annotation and
@Import(GlobalHandlerException.class) as-is so the controller under test and
exception handler remain unchanged.
| @BeforeEach | ||
| void setUp() { | ||
| jobs = Map.of( | ||
| "SMALL_JOB", smallExcelJob, | ||
| "LARGE_JOB", largeExcelJob | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider aligning map keys with JobsType enum values.
The map uses "SMALL_JOB" and "LARGE_JOB" as keys, but the JobsType enum uses SMALL_EXCEL and LARGE_EXCEL. Using consistent naming (or the enum directly) would reduce confusion.
Alternative using enum values
@BeforeEach
void setUp() {
jobs = Map.of(
- "SMALL_JOB", smallExcelJob,
- "LARGE_JOB", largeExcelJob
+ JobsType.SMALL_EXCEL.name(), smallExcelJob,
+ JobsType.LARGE_EXCEL.name(), largeExcelJob
);
}Then update @ValueSource accordingly:
@ValueSource(strings = {
- "SMALL_JOB",
- "LARGE_JOB"
+ "SMALL_EXCEL",
+ "LARGE_EXCEL"
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @BeforeEach | |
| void setUp() { | |
| jobs = Map.of( | |
| "SMALL_JOB", smallExcelJob, | |
| "LARGE_JOB", largeExcelJob | |
| ); | |
| } | |
| @BeforeEach | |
| void setUp() { | |
| jobs = Map.of( | |
| JobsType.SMALL_EXCEL.name(), smallExcelJob, | |
| JobsType.LARGE_EXCEL.name(), largeExcelJob | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/test/java/com/io/example/job/BatchJobIntegrationTest.java
around lines 49 - 55, The jobs map in setUp() uses string keys "SMALL_JOB" and
"LARGE_JOB" which don't match the JobsType enum (SMALL_EXCEL, LARGE_EXCEL);
update the map to use JobsType enum values (e.g., JobsType.SMALL_EXCEL and
JobsType.LARGE_EXCEL) or use JobsType.name() strings to keep consistency, and
adjust any tests that reference the keys (such as the @ValueSource in the
parameterized test) to use the enum values or their names accordingly; look for
setUp(), jobs, smallExcelJob, largeExcelJob, JobsType and the @ValueSource usage
to make the changes.
| @Mock | ||
| private ExecutionDto executionDto; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid mocking simple DTOs.
ExecutionDto is a simple data class; mocking it is unnecessary and adds complexity. The other test method (shouldThrowBusinessExceptionWhenJobFails) correctly uses new ExecutionDto(...) instead.
Suggested consistent approach
- @Mock
- private ExecutionDto executionDto;Then update shouldRunJobSuccessfully:
void shouldRunJobSuccessfully(String jobType, String jobName) throws Exception {
+ var executionDto = new ExecutionDto(JobsType.valueOf(jobType));
- when(executionDto.getType()).thenReturn(JobsType.valueOf(jobType));
when(job.getName()).thenReturn(jobName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Mock | |
| private ExecutionDto executionDto; |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/test/java/com/io/example/service/FileBatchServiceImplTest.java
around lines 52 - 53, Replace the mocked ExecutionDto field with a real
instance: remove the @Mock on ExecutionDto and in the shouldRunJobSuccessfully
test (or test setup) create a new ExecutionDto(...) with the same sample values
used in shouldThrowBusinessExceptionWhenJobFails so both tests use a concrete
DTO; update any stubbing or verify calls that relied on the mock to use the real
object's state (e.g., use the real instance when calling fileBatchService.runJob
or asserting interactions).
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class TestServiceImplTest { | ||
| class StudentServiceImplStudent { |
There was a problem hiding this comment.
Fix the test class naming convention.
The test class is named StudentServiceImplStudent but should follow the standard JUnit naming convention with a Test suffix: StudentServiceImplTest.
📝 Proposed fix
-class StudentServiceImplStudent {
+class StudentServiceImplTest {Don't forget to rename the file from StudentServiceImplStudent.java to StudentServiceImplTest.java as well.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class StudentServiceImplStudent { | |
| class StudentServiceImplTest { |
🤖 Prompt for AI Agents
In
@spring-batch-file-examples/src/test/java/com/io/example/service/StudentServiceImplStudent.java
at line 8, Rename the test class from StudentServiceImplStudent to
StudentServiceImplTest and update the class declaration (class
StudentServiceImplTest {) to follow JUnit naming conventions; also rename the
file from StudentServiceImplStudent.java to StudentServiceImplTest.java and
update any references/imports or test suite entries that reference
StudentServiceImplStudent to the new class name.
…ng with custom readers for small and large Excel files; refactor services and tests
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.