Conversation
Implement bult managemant
Getting changes from main
There was a problem hiding this comment.
Pull request overview
Adds bullet stock tracking and integrates bullet issue/return details into the existing weapon issue flow, expanding the service/controller layer to support CRUD for bullet types and exposing new endpoints/DTO fields.
Changes:
- Introduce
Bulletentity + repository/service/controller for bullet stock management. - Extend weapon issue/return logic to validate, decrement, and increment bullet magazine stock; persist bullet details on
WeaponIssue. - Extend weapon detail responses (
WeaponResponseDTO) and request DTOs to include bullet-related fields.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/crimeLink/analyzer/service/impl/WeaponServiceImpl.java | Adds transaction boundaries and includes issued bullet info in weapon detail DTOs. |
| src/main/java/com/crimeLink/analyzer/service/impl/WeaponIssueServiceImpl.java | Integrates bullet stock decrement/increment into issue/return flows and stores bullet details on WeaponIssue. |
| src/main/java/com/crimeLink/analyzer/service/impl/BulletserviceImpl.java | Implements bullet CRUD/service logic (naming currently inconsistent with other *ServiceImpl classes). |
| src/main/java/com/crimeLink/analyzer/service/WeaponIssueService.java | Minor interface reordering; no functional changes. |
| src/main/java/com/crimeLink/analyzer/service/UserService.java | Adds getAllOfficers() for new controller endpoint. |
| src/main/java/com/crimeLink/analyzer/service/BulletService.java | New bullet service interface. |
| src/main/java/com/crimeLink/analyzer/repository/BulletRepository.java | New bullet repository; currently only offers case-sensitive type lookup. |
| src/main/java/com/crimeLink/analyzer/entity/WeaponIssue.java | Adds bullet tracking columns (type, issued/returned mags, condition/remarks, etc.). |
| src/main/java/com/crimeLink/analyzer/entity/Bullet.java | New Bullet stock entity with timestamps. |
| src/main/java/com/crimeLink/analyzer/dto/WeaponReturnResponseDTO.java | Removes unused DTO. |
| src/main/java/com/crimeLink/analyzer/dto/WeaponResponseDTO.java | Adds issued bullet type + magazine count fields. |
| src/main/java/com/crimeLink/analyzer/dto/ReturnWeaponRequestDTO.java | Adds bullet return fields (returned mags, condition, remarks, etc.). |
| src/main/java/com/crimeLink/analyzer/dto/IssueWeaponRequestDTO.java | Adds bullet issue fields (type, number of magazines, remarks). |
| src/main/java/com/crimeLink/analyzer/dto/BulletUpdateDTO.java | New DTO for bullet updates. |
| src/main/java/com/crimeLink/analyzer/dto/BulletResponseDTO.java | New response DTO for bullet listing with formatted register date. |
| src/main/java/com/crimeLink/analyzer/dto/BulletAddDTO.java | New DTO for bullet creation. |
| src/main/java/com/crimeLink/analyzer/controller/UserController.java | Adds /all-officers endpoint but unintentionally drops mapping for field officers. |
| src/main/java/com/crimeLink/analyzer/controller/BulletController.java | New REST API endpoints for bullet CRUD and listing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| .body(createErrorResponse("Failed to fetch bullets with details: " + e.getMessage())); | ||
| } |
There was a problem hiding this comment.
The exception handler calls e.printStackTrace() and also returns e.getMessage() to clients. This can leak implementation details and makes logs inconsistent with the rest of the app; prefer structured logging (e.g., SLF4J logger) and return a generic error message while logging the full exception server-side.
|
|
||
| @GetMapping("/field-officers") | ||
| public List<User> getFieldOfficers() { | ||
| return service.getFieldOfficers(); | ||
| } |
There was a problem hiding this comment.
getFieldOfficers() no longer has any Spring mapping annotation, so /api/users/field-officers (or any route) won’t be exposed and the method becomes dead code. Re-add the intended @GetMapping (or remove the method entirely if it’s intentionally deprecated).
| stockBullet = bulletRepository.findByBulletType(bulletType.trim()) | ||
| .orElseThrow(() -> new RuntimeException("Bullet type not found: " + bulletType)); | ||
|
|
There was a problem hiding this comment.
Bullet lookup is case-sensitive (findByBulletType(bulletType.trim())) but bullet types are treated as case-insensitive elsewhere (e.g., existsByBulletTypeIgnoreCase). This can cause issuing/returning to fail when request casing differs from stored casing. Use a case-insensitive lookup (e.g., repository method findByBulletTypeIgnoreCase) or normalize bulletType consistently on both write and read.
| int available = stockBullet.getNumberOfMagazines() != null ? stockBullet.getNumberOfMagazines() : 0; | ||
|
|
||
| if (magsToIssue > available) { | ||
| throw new RuntimeException( | ||
| "Not enough magazines. Available: " + available + ", Requested: " + magsToIssue); | ||
| } | ||
|
|
||
| stockBullet.setNumberOfMagazines(available - magsToIssue); | ||
| bulletRepository.save(stockBullet); |
There was a problem hiding this comment.
Bullet stock decrement is vulnerable to lost updates under concurrency: two transactions can read the same available and both save, allowing oversubscription/incorrect stock. Consider adding optimistic locking (@Version) on Bullet, or using a pessimistic write lock / atomic update query that decrements stock only when sufficient magazines remain.
| Bullet stockBullet = bulletRepository.findByBulletType(normalizedBulletType) | ||
| .orElseThrow( | ||
| () -> new RuntimeException("Bullet stock not found for type: " + normalizedBulletType)); | ||
|
|
||
| // Add returned magazines back to stock | ||
| int currentStock = stockBullet.getNumberOfMagazines() != null ? stockBullet.getNumberOfMagazines() : 0; | ||
| stockBullet.setNumberOfMagazines(currentStock + returnedMags); | ||
| bulletRepository.save(stockBullet); |
There was a problem hiding this comment.
Bullet stock increment on return has the same lost-update risk as issuing: concurrent returns/issues can overwrite numberOfMagazines because it’s read-modify-write without locking/versioning. Use optimistic/pessimistic locking or an atomic update statement to make the adjustment concurrency-safe.
| public interface BulletRepository extends JpaRepository<Bullet, Integer> { | ||
|
|
||
| Optional<Bullet> findByBulletType(String bulletType); | ||
|
|
||
| boolean existsByBulletTypeIgnoreCase(String bulletType); |
There was a problem hiding this comment.
Repository provides only a case-sensitive findByBulletType(...), but service logic treats bullet types as case-insensitive (see existsByBulletTypeIgnoreCase). Add a case-insensitive finder (e.g., findByBulletTypeIgnoreCase) and prefer it in issue/return flows to avoid casing-related failures.
| @Service | ||
| @RequiredArgsConstructor | ||
| public class BulletserviceImpl implements BulletService { | ||
|
|
There was a problem hiding this comment.
Class name BulletserviceImpl doesn’t follow the project’s existing *ServiceImpl naming pattern (e.g., WeaponServiceImpl) and standard Java PascalCase for word boundaries. Renaming to BulletServiceImpl (and matching filename) will make it easier to find and keep naming consistent.
No description provided.