diff --git a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java index 2a16f39ab3..bfbed03191 100644 --- a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java +++ b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java @@ -185,13 +185,18 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine * The rest api returns receivers with masked secret fields, so a receiver submitted * from the ui may carry the mask placeholder instead of the real secret. * Restore such fields from the stored entity before using the receiver. + * @throws IllegalArgumentException if the receiver carries an id but no stored receiver + * exists for it: without the stored entity the mask cannot + * be resolved, and saving would persist the placeholder */ private void resolveMaskedSecrets(NoticeReceiver noticeReceiver) { if (noticeReceiver == null || noticeReceiver.getId() == null) { return; } - noticeReceiverDao.findById(noticeReceiver.getId()) - .ifPresent(existing -> NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing)); + NoticeReceiver existing = noticeReceiverDao.findById(noticeReceiver.getId()) + .orElseThrow(() -> new IllegalArgumentException( + "The receiver with id " + noticeReceiver.getId() + " does not exist.")); + NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing); } @Override diff --git a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java index 9b2780ba1e..72f21bc3c8 100644 --- a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java +++ b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java @@ -52,10 +52,12 @@ import java.util.Map; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -213,7 +215,9 @@ class NoticeConfigServiceTest { @Test void editReceiver() { - final NoticeReceiver noticeReceiver = mock(NoticeReceiver.class); + final NoticeReceiver noticeReceiver = new NoticeReceiver(); + noticeReceiver.setId(5L); + when(noticeReceiverDao.findById(5L)).thenReturn(Optional.of(noticeReceiver)); noticeConfigService.editReceiver(noticeReceiver); verify(noticeReceiverDao, times(1)).save(noticeReceiver); } @@ -235,6 +239,18 @@ class NoticeConfigServiceTest { verify(noticeReceiverDao, times(1)).save(incoming); } + @Test + void editReceiverRejectsUnknownId() { + when(noticeReceiverDao.findById(5L)).thenReturn(Optional.empty()); + + final NoticeReceiver incoming = new NoticeReceiver(); + incoming.setId(5L); + incoming.setTgBotToken(NoticeReceiverMaskUtil.SECRET_MASK + "voJM"); + + assertThrows(IllegalArgumentException.class, () -> noticeConfigService.editReceiver(incoming)); + verify(noticeReceiverDao, never()).save(any()); + } + @Test void sendTestMsgResolvesMaskedSecret() { final NoticeReceiver stored = new NoticeReceiver(); @@ -251,6 +267,18 @@ class NoticeConfigServiceTest { assertEquals("1499012345:AAEOB_wEYS-DZyPM3h5NzI8voJM", incoming.getTgBotToken()); } + @Test + void sendTestMsgRejectsUnknownId() { + when(noticeReceiverDao.findById(5L)).thenReturn(Optional.empty()); + + final NoticeReceiver incoming = new NoticeReceiver(); + incoming.setId(5L); + incoming.setTgBotToken(NoticeReceiverMaskUtil.SECRET_MASK + "voJM"); + + assertThrows(IllegalArgumentException.class, () -> noticeConfigService.sendTestMsg(incoming)); + verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any()); + } + @Test void deleteReceiver() { final Long receiverId = 23342525L; @@ -323,7 +351,7 @@ class NoticeConfigServiceTest { @Test void sendTestMsg() { - final NoticeReceiver noticeReceiver = mock(NoticeReceiver.class); + final NoticeReceiver noticeReceiver = new NoticeReceiver(); final NoticeTemplate noticeTemplate = null; noticeConfigService.sendTestMsg(noticeReceiver); verify(dispatcherAlarm, times(1)).sendNoticeMsg(eq(noticeReceiver), eq(noticeTemplate), any(GroupAlert.class));