DaanHoogland commented on code in PR #10252:
URL: https://github.com/apache/cloudstack/pull/10252#discussion_r1954800697
##########
server/src/main/java/com/cloud/bgp/BGPServiceImpl.java:
##########
@@ -255,9 +255,9 @@ public boolean allocateASNumber(long zoneId, Long asNumber,
Long networkId, Long
netName = network.getName();
}
- LOGGER.debug("Allocating the AS Number {} to {} on zone {}",
asNumber::toString,
- (Objects.nonNull(vpcId) ? "VPC " + vpc : "network " +
network)::toString,
- () -> dataCenterDao.findById(zoneId));
+ String logMsg = Objects.nonNull(vpcId) ? ("VPC " + vpc) : ("network "
+ network);
+ LOGGER.debug("Allocating the AS Number {} to {} on zone {}",
asNumberVO::toString,
+ logMsg::toString, () -> dataCenterDao.findById(zoneId));
Review Comment:
```suggestion
String networkName = Objects.nonNull(vpcId) ? ("VPC " + vpc) :
("network " + network);
LOGGER.debug("Allocating the AS Number {} to {} on zone {}",
asNumberVO::toString,
networkName::toString, () -> dataCenterDao.findById(zoneId));
```
##########
server/src/test/java/com/cloud/alert/AlertManagerImplTest.java:
##########
@@ -45,7 +53,7 @@ public class AlertManagerImplTest {
AlertManagerImpl alertManagerImplMock;
@Mock
- AlertDao alertDaoMock;
+ AlertDao _alertDao;
Review Comment:
do we need the old name convention?
##########
server/src/test/java/com/cloud/alert/AlertManagerImplTest.java:
##########
@@ -77,46 +94,78 @@ private void sendMessage (){
Mockito.when(cluster.getId()).thenReturn(1L);
Mockito.when(_clusterDao.findById(1L)).thenReturn(cluster);
-
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1l,
1l, "", "");
+
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1L,
1L, "", "");
} catch (UnsupportedEncodingException | MessagingException e) {
Assert.fail();
}
}
@Test
public void sendAlertTestSendMail() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
- alertManagerImplMock.recipients = new String [] {""};
+ Mockito.doReturn(null).when(_alertDao).persist(any());
+ alertManagerImplMock.recipients = new String[]{""};
sendMessage();
- Mockito.verify(alertManagerImplMock).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock).sendMessage(any());
}
@Test
public void sendAlertTestDebugLogging() {
Mockito.doReturn(0).when(alertVOMock).getSentCount();
-
Mockito.doReturn(alertVOMock).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(alertVOMock).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
sendMessage();
Mockito.verify(alertManagerImplMock.logger).debug(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
}
@Test
public void sendAlertTestWarnLogging() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
+ Mockito.doReturn(null).when(_alertDao).persist(Mockito.any());
alertManagerImplMock.recipients = null;
sendMessage();
Mockito.verify(alertManagerImplMock.logger,
Mockito.times(2)).warn(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
+ }
+
Review Comment:
```suggestion
```
##########
server/src/test/java/com/cloud/alert/AlertManagerImplTest.java:
##########
@@ -77,46 +94,78 @@ private void sendMessage (){
Mockito.when(cluster.getId()).thenReturn(1L);
Mockito.when(_clusterDao.findById(1L)).thenReturn(cluster);
-
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1l,
1l, "", "");
+
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1L,
1L, "", "");
} catch (UnsupportedEncodingException | MessagingException e) {
Assert.fail();
}
}
@Test
public void sendAlertTestSendMail() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
- alertManagerImplMock.recipients = new String [] {""};
+ Mockito.doReturn(null).when(_alertDao).persist(any());
+ alertManagerImplMock.recipients = new String[]{""};
sendMessage();
- Mockito.verify(alertManagerImplMock).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock).sendMessage(any());
}
@Test
public void sendAlertTestDebugLogging() {
Mockito.doReturn(0).when(alertVOMock).getSentCount();
-
Mockito.doReturn(alertVOMock).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(alertVOMock).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
sendMessage();
Mockito.verify(alertManagerImplMock.logger).debug(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
}
@Test
public void sendAlertTestWarnLogging() {
-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(),
Mockito.anyLong(),
Mockito.anyLong(), Mockito.anyLong());
- Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
+ Mockito.doReturn(null).when(_alertDao).persist(Mockito.any());
alertManagerImplMock.recipients = null;
sendMessage();
Mockito.verify(alertManagerImplMock.logger,
Mockito.times(2)).warn(Mockito.anyString());
- Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(Mockito.any());
+ Mockito.verify(alertManagerImplMock,
Mockito.never()).sendMessage(any());
+ }
+
+
+ @Test
+ public void testSendAlertWithNullParameters() throws MessagingException,
UnsupportedEncodingException {
+ // Given
+ String subject = "Test Subject";
+ String content = "Test Content";
+ AlertManager.AlertType alertType =
AlertManager.AlertType.ALERT_TYPE_MEMORY;
+
+ // When
+ alertManagerImplMock.sendAlert(alertType, null, null, null, subject,
content);
+
+ // Then
+ ArgumentCaptor<AlertVO> alertCaptor =
ArgumentCaptor.forClass(AlertVO.class);
+ verify(_alertDao).persist(alertCaptor.capture());
+
+ AlertVO capturedAlert = alertCaptor.getValue();
+ assertNotNull("Captured alert should not be null", capturedAlert);
+ assertEquals(0L, capturedAlert.getDataCenterId());
+ assertNull(capturedAlert.getPodId());
+ assertNull(capturedAlert.getClusterId());
+ assertEquals(subject, capturedAlert.getSubject());
+ assertEquals(content, capturedAlert.getContent());
+ assertEquals(alertType.getType(), capturedAlert.getType());
+ }
+
+
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
