From b13aff4e7c452bdf437a8b7057ae0b8298b72c6b Mon Sep 17 00:00:00 2001 From: EranTimothy-dev Date: Fri, 20 Jun 2025 23:58:02 +0530 Subject: [PATCH 1/2] refactor: removed unused lines and objects --- .../shiftsl/backend/unittests/Service/LeaveServiceTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/test/java/com/shiftsl/backend/unittests/Service/LeaveServiceTest.java b/src/test/java/com/shiftsl/backend/unittests/Service/LeaveServiceTest.java index ecc13c4..1fa204b 100644 --- a/src/test/java/com/shiftsl/backend/unittests/Service/LeaveServiceTest.java +++ b/src/test/java/com/shiftsl/backend/unittests/Service/LeaveServiceTest.java @@ -48,7 +48,6 @@ public class LeaveServiceTest { @Mock private LeaveDTO testLeaveDTO; -// private ArgumentCaptor captor = ArgumentCaptor.forClass(Leave.class); private ArgumentCaptor captor; private static final Long ID = 123456L; @@ -130,7 +129,7 @@ void requestLeaveTest() { when(shiftService.getShiftByID(ID)).thenReturn(testShift); when(leaveRepo.save(any(Leave.class))).thenReturn(testLeave); - Leave result = underTest.requestLeave(testLeaveDTO); + underTest.requestLeave(testLeaveDTO); verify(leaveRepo).save(captor.capture()); verify(userService).getUserById(ID); @@ -163,7 +162,6 @@ void getLeaveTest() { @Test void rejectTest() { -// ArgumentCaptor captor = ArgumentCaptor.forClass(Leave.class); when(leaveRepo.findById(ID)).thenReturn(Optional.of(testLeave)); when(leaveRepo.save(any(Leave.class))).thenReturn(testLeave); String result = underTest.reject(ID); @@ -188,7 +186,6 @@ void rejectExceptionTest() { @Order(2) void approveTest() { testShift = Mockito.mock(Shift.class); -// ArgumentCaptor captor = ArgumentCaptor.forClass(Leave.class); when(shiftService.getShiftWithLock(2L)).thenReturn(testShift); when(testShift.getDoctors()).thenReturn(testDoctors); when(shiftService.updateShiftByID(testShift)).thenReturn(testShift); From 5ef6d09d43d8f87fa6a6dda16b2df9144b3e17d0 Mon Sep 17 00:00:00 2001 From: EranTimothy-dev Date: Sat, 21 Jun 2025 05:14:41 +0530 Subject: [PATCH 2/2] updated exception handling and log message levels --- .../shiftsl/backend/Service/ShiftService.java | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/shiftsl/backend/Service/ShiftService.java b/src/main/java/com/shiftsl/backend/Service/ShiftService.java index 647d0f1..508c7b7 100644 --- a/src/main/java/com/shiftsl/backend/Service/ShiftService.java +++ b/src/main/java/com/shiftsl/backend/Service/ShiftService.java @@ -65,7 +65,7 @@ public List getAvailableShifts() { logger.info("Getting available shifts"); return shiftRepo.findAvailableShifts(); } catch (Exception e) { - logger.error("Unable to retrieve available shifts."); + logger.warn("Unable to retrieve available shifts."); throw new ShiftRetrievalException("Error occurred while trying to retrieve available shifts from database"); } } @@ -73,6 +73,7 @@ public List getAvailableShifts() { // Doctor claims a shift from the shift pool @Transactional public void claimShift(Long doctorId, Long shiftId) { + String lockErrorMessage = null; try { logger.info("Claiming shift " + shiftId); Shift shift = getShiftWithLock(shiftId); @@ -92,10 +93,22 @@ public void claimShift(Long doctorId, Long shiftId) { shiftRepo.save(shift); } catch (LockTimeoutException | PessimisticLockException e) { logger.error("Too many threads are trying to claim shift."); - throw new ShiftClaimFailedException("System is experiencing high load. Please try again later."+ e); + lockErrorMessage = "System is experiencing high load. Please try again later. " + e.getMessage(); + throw new ShiftClaimFailedException(lockErrorMessage); } catch (Exception e) { logger.error("Error occurred while trying to store shift {} for doctor {} in database", shiftId, doctorId); - throw new ShiftClaimFailedException(String.format("Unable to claim shift %d for doctor %d", shiftId, doctorId)); + + String fullMessage = String.format( + "Unable to claim shiftId: %d for doctorId: %d", shiftId, doctorId + ); + + if (lockErrorMessage != null) { + fullMessage += ". " + lockErrorMessage; + } else { + fullMessage += ". " + e.getMessage(); + } + + throw new ShiftClaimFailedException(fullMessage); } } @@ -103,7 +116,7 @@ public void claimShift(Long doctorId, Long shiftId) { public Shift getShiftByID(Long shiftID){ logger.info("Retrieving Shift '{}' from database", shiftID); return shiftRepo.findById(shiftID).orElseThrow(() -> { - logger.error("Unable to find shift with ID " + shiftID); + logger.warn("Unable to find shift with ID {}", shiftID); return new ShiftNotFoundException("Shift ID - (" + shiftID + ") not found."); }); } @@ -112,7 +125,7 @@ public Shift getShiftByID(Long shiftID){ public Shift getShiftWithLock(Long shiftID){ logger.info("Retrieving Shift {} with pessimistic lock", shiftID); return shiftRepo.findShiftWithLock(shiftID).orElseThrow(() -> { - logger.error("Unable to find shift with locking implemented for ID " + shiftID); + logger.warn("Unable to find shift with locking implemented for ID {}", shiftID); return new ShiftNotFoundException("Shift ID - (" + shiftID + ") not found."); }); } @@ -123,7 +136,7 @@ public List getShiftsForDoctor(Long doctorId) { userService.getUserById(doctorId); //check whether the doctor exists or else throws UserNotFoundException List shifts = shiftRepo.findByDoctors_Id(doctorId); if (shifts.isEmpty()) { - logger.info("No shift found for doctor " + doctorId); + logger.warn("No shift found for doctor {}", doctorId); throw new ShiftsNotFoundException("No shifts found for doctor with ID " + doctorId); } return shifts; @@ -185,7 +198,7 @@ public List getRoster(int month) { return shiftRepo.findByStartTimeBetween(startDateTime, endDateTime); } catch (Exception e) { - logger.error("Error occurred while trying to retrieve roster for month " + month); + logger.error("Error occurred while trying to retrieve roster for month {}", month); throw new ShiftsNotFoundException("Unable to retrieve shifts for the given month from database"); }