Skip to content

Conversation

@dcarello
Copy link
Contributor

@dcarello dcarello commented Nov 10, 2025

Connected the batches/{id} endpoint to the database. Tested that it works using Postman. Updated Unit Tests also

Summary by CodeRabbit

  • New Features

    • Batch management API now uses UUID identifiers instead of numeric IDs.
    • Batch response data includes schedule ID, start time, last run time, and active status.
    • Enhanced error handling with dedicated batch not found exception.
  • Tests

    • Updated test coverage for new batch API structure and exception scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

Connected the batches/{id} endpoint to the database. Tested that it works using Postman. Updated Unit Tests also
@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../vsp/endpointinsightsapi/service/BatchService.java 93.75% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

dcarello and others added 2 commits November 16, 2025 18:02
Put DTO in its own class and used mapStruct so it can auto generated the mapping.
@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Walkthrough

Added MapStruct integration with Lombok binding and refactored Batch API to use UUID identifiers, DTOs, repository patterns, and a dedicated mapper. Updated controller, service, and tests to use new BatchResponseDTO with revised fields and explicit exception handling.

Changes

Cohort / File(s) Summary
Build Configuration
endpoint-insights-api/pom.xml
Added MapStruct (1.5.5.Final), mapstruct-processor (1.5.5.Final), and lombok-mapstruct-binding (0.2.0) dependencies; registered annotation processors for compile-time code generation.
Exception Handling
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java
Introduced new BatchNotFoundException extending CustomException with NOT_FOUND status and "BATCH_NOT_FOUND" error code.
Data Transfer Object
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java
Refactored fields: id (Long → UUID), name → batchName, status → scheduleId (Long); added startTime, lastTimeRun (LocalDate), active (Boolean); added @Builder annotation for fluent construction.
Mapper
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java
Introduced MapStruct mapper interface with toDto(TestBatch) → BatchResponseDTO method; includes @Mapping to convert batch\_id field to id.
Repository
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/repository/TestBatchRepository.java
Added Spring Data JPA repository extending JpaRepository<TestBatch, UUID> for CRUD and query operations.
Controller
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java
Changed all path variable types from Long to UUID; refactored listBatches to build DTOs dynamically; delegated get/update/delete operations to BatchService; updated to use BatchResponseDTO in responses.
Service
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java
Updated constructor to inject TestBatchRepository and BatchMapper; added getBatchById(UUID) → BatchResponseDTO and deleteBatchById(UUID) methods with exception handling; migrated internal repository references from batchRepository to testBatchRepository.
Unit Tests
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java, endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java
Updated controller tests to verify DTO fields (batchName, scheduleId, active, id) and use BatchNotFoundException mocks; refactored service tests to Mockito annotation-driven pattern (@Mock, @InjectMocks) with explicit exception and mapper interaction verification.

Sequence Diagram

sequenceDiagram
    autonumber
    actor Client
    participant Controller as BatchesController
    participant Service as BatchService
    participant Repository as TestBatchRepository
    participant Mapper as BatchMapper
    participant DB as Database

    rect rgb(200, 220, 240)
    Note over Client,DB: GET /api/batches/{id} - Fetch Single Batch
    Client->>Controller: getBatch(UUID id)
    Controller->>Service: getBatchById(id)
    Service->>Repository: findById(id)
    Repository->>DB: SELECT * FROM test_batch WHERE id=?
    DB-->>Repository: TestBatch entity
    alt Found
        Repository-->>Service: Optional<TestBatch>
        Service->>Mapper: toDto(entity)
        Mapper-->>Service: BatchResponseDTO
        Service-->>Controller: BatchResponseDTO
        Controller-->>Client: 200 OK + DTO
    else Not Found
        Service->>Service: throw BatchNotFoundException
        Service-->>Controller: BatchNotFoundException
        Controller-->>Client: 404 NOT_FOUND
    end
    end

    rect rgb(240, 220, 200)
    Note over Client,DB: DELETE /api/batches/{id} - Delete Batch
    Client->>Controller: deleteBatch(UUID id)
    Controller->>Service: deleteBatchById(id)
    Service->>Repository: existsById(id)
    Repository->>DB: SELECT EXISTS(*)
    DB-->>Repository: boolean
    alt Exists
        Repository-->>Service: true
        Service->>Repository: deleteById(id)
        Repository->>DB: DELETE FROM test_batch WHERE id=?
        Service-->>Controller: void
        Controller-->>Client: 204 NO_CONTENT
    else Not Found
        Repository-->>Service: false
        Service->>Service: throw BatchNotFoundException
        Service-->>Controller: BatchNotFoundException
        Controller-->>Client: 404 NOT_FOUND
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Type migration: UUID/Long identifier change across controller, service, and repository layers requires verification of consistency and null handling.
  • DTO field mapping: Verify MapStruct mapper correctly handles batch\_id → id conversion and all required field transformations in BatchResponseDTO.
  • Exception handling: New BatchNotFoundException usage in service and controller; ensure proper propagation and HTTP status code mapping.
  • Test coverage: Confirm unit tests adequately cover new exception scenarios (getBatchById\_notFound, delete non-existent batch) and DTO field validations.

Possibly related PRs

  • EI-12 GET endpoint /batch #52: Modifies the same Batch controller, service, and repository integration for UUID-based getBatch/getBatchById operations using TestBatchRepository, suggesting overlapping architectural changes or dependencies.

Poem

🐰 With MapStruct's spell and UUIDs bright,
The Batch API dances with elegant might,
DTOs bloom where old fields once lay,
Repositories spring forth—hip-hip-hooray!
Our refactor hops forward, clean and lightweight. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.10% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: implementing the GET batches/{id} endpoint. It is concise, specific, and directly reflects the main objective described in the PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/EI-11

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (1)

25-48: Stubbed list/create/update endpoints should eventually use the service/DB

listBatches, createBatch, and updateBatch still construct BatchResponseDTO instances in memory with hard‑coded or calculated values and UUID.randomUUID() instead of going through BatchService / the database. Prior reviewers have already called this out, and you’ve noted it’s out of scope for this sprint.

Once the backing persistence is ready, it would be good to:

  • Have listBatches query via a service/repository instead of returning literals.
  • Route createBatch and updateBatch through BatchService, letting the DB assign the UUID and persist startTime / lastTimeRun consistently.
  • Remove now‑unused imports like Collections and commented jobs fields when that work is done.

Also applies to: 56-84

🧹 Nitpick comments (4)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchRequestDTO.java (1)

16-19: DTO shape looks consistent; consider name vs batchName alignment

The added fields (scheduleId, startTime, active) look consistent with BatchResponseDTO and the new service layer usage. One optional cleanup is to consider aligning name here with batchName in BatchResponseDTO (or vice versa), or ensure the mapper/service explicitly handles the difference so you don’t get silent mapping issues later.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java (1)

7-11: Verify MapStruct field mappings, especially ID and name fields

The mapper definition is fine, but with a bare toDto(TestBatch entity) MapStruct will only map by matching property names. In the tests you’re calling entity.setBatch_id(id) while BatchResponseDTO exposes id, and you also have batchName/name differences elsewhere. If the entity’s property names don’t exactly match the DTO, you’ll get nulls or warnings at build time.

Consider adding explicit mappings, e.g. @Mapping(source = "batch_id", target = "id"), and any others that differ, to make the mapping contract explicit and future‑proof.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1)

29-49: Strengthen assertions and interaction checks in service tests (optional)

A couple of small improvements would make these tests more robust:

  • In getBatchById_returnsDto, you already set startTime and lastTimeRun on the entity; consider also asserting them on the returned DTO to detect future mapping regressions.
  • In deleteBatchById_NotFound, you can optionally verify that deleteById is never called when existsById is false, to pin down the intended behavior.

For example:

@@     void getBatchById_returnsDto() {
-        assertThat(dto.getId()).isEqualTo(id);
-        assertThat(dto.getBatchName()).isEqualTo("Example");
-        assertThat(dto.getScheduleId()).isEqualTo(1001L);
-        assertThat(dto.getActive()).isTrue();
+        assertThat(dto.getId()).isEqualTo(id);
+        assertThat(dto.getBatchName()).isEqualTo("Example");
+        assertThat(dto.getScheduleId()).isEqualTo(1001L);
+        assertThat(dto.getStartTime()).isEqualTo(LocalDate.parse("2025-11-08"));
+        assertThat(dto.getLastTimeRun()).isEqualTo(LocalDate.parse("2025-11-09"));
+        assertThat(dto.getActive()).isTrue();
@@     void deleteBatchById_NotFound() {
-        assertThrows(BatchNotFoundException.class, () -> batchService.deleteBatchById(id));
+        assertThrows(BatchNotFoundException.class, () -> batchService.deleteBatchById(id));
+        verify(testBatchRepository, org.mockito.Mockito.never()).deleteById(id);

These are optional but help lock in the intended contract of BatchService.

Also applies to: 59-75

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java (1)

11-22: DTO shape matches usage; consider naming consistency with BatchRequestDTO

BatchResponseDTO’s fields (id, batchName, scheduleId, startTime, lastTimeRun, active) line up with how the controller and tests are using the response. The only minor inconsistency is that the request DTO uses name while the response uses batchName, which can be slightly confusing for API consumers.

If you don’t have external clients depending on these names yet, you might consider aligning them (either name everywhere or batchName everywhere) in a follow‑up change.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df5d3ee and c572d43.

📒 Files selected for processing (10)
  • endpoint-insights-api/pom.xml (2 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchRequestDTO.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/repository/TestBatchRepository.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java (1 hunks)
  • endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (2 hunks)
  • endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/JobsController.java (1)
  • RestController (25-157)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchRequestDTO.java (1)
  • Getter (11-20)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchRequestDTO.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java (1)
  • Getter (11-23)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (3)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/JobControllerUnitTest.java (1)
  • TestPropertySource (29-103)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/EndpointInsightsApiApplicationTests.java (1)
  • ActiveProfiles (8-18)
🪛 GitHub Actions: Codecov Backend
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/repository/TestBatchRepository.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java

[error] 43-43: NullPointerException encountered in test: Cannot invoke "com.vsp.endpointinsightsapi.mapper.BatchMapper.toDto(...)" because "this.batchMapper" is null


[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchResponseDTO.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java

[error] 34-34: NullPointerException: Cannot invoke "com.vsp.endpointinsightsapi.mapper.BatchMapper.toDto(com.vsp.endpointinsightsapi.model.TestBatch)" because "this.batchMapper" is null


[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/dto/BatchRequestDTO.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/pom.xml

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java

[error] 1-1: Maven Surefire test suite failed. See target/surefire-reports for details.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Jenkins / Linting and Unit Tests
🔇 Additional comments (5)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/repository/TestBatchRepository.java (1)

9-10: Standard Spring Data repository – looks good

TestBatchRepository cleanly extends JpaRepository<TestBatch, UUID> with no custom methods. This is idiomatic Spring Data JPA and should integrate fine with BatchService.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)

5-13: Exception design and error payload look appropriate

Error code, HTTP status, and message are clear and match the not‑found semantics used in the service and controller tests. This should give a consistent API error contract for missing batches.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (1)

76-83: Good coverage of not‑found scenarios for GET and DELETE

The tests for getBatchById_notFound_shouldReturn404 and shouldReturn404WhenDeletingNonexistentBatch correctly exercise the BatchNotFoundException path and assert 404 responses, matching the new service behavior.

Also applies to: 136-145

endpoint-insights-api/pom.xml (1)

103-120: MapStruct + Lombok wiring in the POM looks consistent

The added mapstruct, mapstruct-processor (as provided), and lombok-mapstruct-binding dependencies, along with the corresponding annotationProcessorPaths, form a coherent setup for MapStruct + Lombok code generation.

Also applies to: 186-195

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (1)

19-23: GET and DELETE by ID correctly delegate to BatchService

The controller now uses constructor injection for BatchService, and both getBatch and deleteBatch cleanly delegate to batchService.getBatchById(id) / batchService.deleteBatchById(id), returning 200 OK and 204 No Content respectively. This aligns with the new service behavior and the updated tests.

Also applies to: 50-54, 86-90

Mocked the mapper in the testing
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1)

31-60: Tighten happy‑path assertions and remove scaffolding comment

  • Line 34’s // adjust getters/setters to your model looks like leftover scaffolding; it can be removed to avoid confusion.
  • You currently assert only id and batchName. Since you build a full BatchResponseDTO (scheduleId, startTime, lastTimeRun, active), consider asserting those fields too (or comparing the whole DTO) to catch mapping regressions.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c572d43 and 09ed898.

📒 Files selected for processing (1)
  • endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Jenkins / Linting and Unit Tests
🔇 Additional comments (4)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (4)

26-28: Mocks and injection wiring look correct

Mocking both TestBatchRepository and BatchMapper and using @InjectMocks for BatchService correctly wires constructor injection and prevents the previous NPE on batchMapper. This setup is solid for isolating service behavior.


62-68: Not‑found getBatchById test is clear and focused

The getBatchById_notFound_throws test cleanly stubs findById to Optional.empty() and asserts BatchNotFoundException, which matches the intended error path and keeps the behavior well specified.


70-78: Delete existing batch test covers the main behavior

deleteBatchById_Exists correctly stubs existsById and verifies deleteById(id) is called, which is the critical side effect for this path. This gives good coverage of the successful delete flow.


80-86: Delete not‑found test correctly guards the error path

deleteBatchById_NotFound stubs existsById to false and asserts BatchNotFoundException, matching the service contract for missing batches and rounding out the negative‑path coverage.

Code rabbit suggestions inputted
Fixed small bug with mapping for getting batch by id. And fixed unit test problems
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java (1)

8-13: LGTM! MapStruct mapper properly configured.

The componentModel = "spring" ensures Spring will manage this mapper as a bean, and the explicit @Mapping for batch_id → id is necessary given the entity's field naming.

Optional: Consider renaming TestBatch.batch_id to camelCase.

The source field batch_id uses snake_case, which violates Java naming conventions. While this is a pre-existing issue in the TestBatch entity (not introduced by this PR), refactoring to batchId would align with Java standards and eliminate the need for explicit field mapping.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (2)

41-41: Remove commented-out code.

Lines 41 and 50 contain commented .jobs(Collections.emptyList()) calls. Commented code should be removed to improve readability.

Apply this diff:

                 .lastTimeRun(LocalDate.now())
                 .active(true)
-//                        .jobs(Collections.emptyList())
                 .build(),
                 .lastTimeRun(LocalDate.now().minusDays(3))
                 .active(false)
-//                        .jobs(Collections.emptyList())
                 .build()

Also applies to: 50-50


65-67: Consider returning BatchResponseDTO for consistency.

The createBatch method returns a TestBatch entity, while other endpoints (getBatch, updateBatch, listBatches) return BatchResponseDTO. This inconsistency exposes internal entity structure in the API response.

For consistency across the API, consider mapping the saved entity to BatchResponseDTO before returning:

 @PostMapping
-public ResponseEntity<TestBatch> createBatch(@RequestBody BatchRequestDTO request) {
+public ResponseEntity<BatchResponseDTO> createBatch(@RequestBody BatchRequestDTO request) {
     TestBatch batch = batchService.createBatch(request);
-    return ResponseEntity.status(HttpStatus.CREATED).body(batch);
+    BatchResponseDTO dto = batchMapper.toDto(batch);
+    return ResponseEntity.status(HttpStatus.CREATED).body(dto);
 }

Note: This would require injecting BatchMapper into the controller or adding a createBatch method to BatchService that returns the DTO.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c0b2a0 and 34ea568.

📒 Files selected for processing (5)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (2 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/mapper/BatchMapper.java (1 hunks)
  • endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java (3 hunks)
  • endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (4 hunks)
  • endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/JobsController.java (1)
  • RestController (25-141)
endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (1)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/exception/BatchNotFoundException.java (1)
  • BatchNotFoundException (8-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Jenkins / Linting and Unit Tests
🔇 Additional comments (11)
endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/service/BatchService.java (3)

27-31: LGTM! Constructor properly enforces non-null dependencies.

The use of Objects.requireNonNull for all three dependencies ensures fail-fast behavior if any dependency is null, which addresses the NPE issues mentioned in previous reviews.


34-41: LGTM! Clean implementation of batch retrieval.

The method correctly:

  • Uses Optional.orElseThrow for explicit exception handling
  • Logs at the appropriate DEBUG level before throwing
  • Delegates DTO mapping to the dedicated mapper

44-53: LGTM! Delete operation with proper existence check.

The existence check before deletion prevents unnecessary exceptions and provides clear error messaging. The INFO-level logging on successful deletion aids observability.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/service/BatchServiceTest.java (3)

24-30: LGTM! Proper Mockito setup with all dependencies mocked.

The addition of @Mock BatchMapper alongside the repository mocks ensures all constructor dependencies are satisfied, preventing the NPE issues from previous commits.


33-62: LGTM! Comprehensive test with proper mocking and verification.

The test correctly:

  • Mocks both repository and mapper behavior
  • Verifies the returned DTO fields
  • Uses verify() to confirm interactions with both dependencies

64-88: LGTM! Thorough error-case coverage.

Both not-found scenarios (getBatchById and deleteBatchById) are properly tested with appropriate exception assertions.

endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java (2)

58-61: LGTM! GET endpoint properly connected to service and database.

The implementation correctly delegates to BatchService.getBatchById, which fetches from the repository and returns a DTO. This fulfills the PR objective.


84-87: LGTM! DELETE endpoint properly connected to service and database.

The delegation to BatchService.deleteBatchById ensures existence checks and exception handling are performed at the service layer, returning the appropriate 204 No Content on success.

endpoint-insights-api/src/test/java/com/vsp/endpointinsightsapi/controller/BatchesControllerUnitTest.java (3)

51-71: LGTM! Test properly validates GET endpoint with DTO response.

The test correctly:

  • Mocks BatchService.getBatchById to return a BatchResponseDTO
  • Verifies all DTO fields in the JSON response
  • Uses the UUID path variable as expected

74-80: LGTM! Proper 404 handling for not-found scenario.

The test verifies that BatchNotFoundException thrown by the service results in a 404 HTTP status, confirming correct exception handling.


113-132: LGTM! Comprehensive DELETE endpoint testing.

Both success (204 No Content) and error (404 Not Found) cases are properly tested with appropriate service mocking.

@tmains515 tmains515 enabled auto-merge (squash) November 26, 2025 06:34
@dcarello dcarello dismissed Crowley723’s stale review December 1, 2025 02:05

Resolved the changes

@tmains515 tmains515 merged commit 087440b into develop Dec 1, 2025
7 checks passed
@tmains515 tmains515 deleted the feature/EI-11 branch December 1, 2025 02:05
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.

4 participants