Skip to content

Commit d14a0dd

Browse files
vishesh92DaanHoogland
authored and
dhslove
committed
Fixup alerting and logging error in BGPServiceImpl (apache#10252)
* Fixup alerting and logging error in BGPServiceImpl * Update unit tests * Apply suggestions from code review Co-authored-by: dahn <[email protected]> --------- Co-authored-by: dahn <[email protected]>
1 parent 2a776c1 commit d14a0dd

File tree

3 files changed

+65
-17
lines changed

3 files changed

+65
-17
lines changed

‎server/src/main/java/com/cloud/alert/AlertManagerImpl.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -790,14 +790,15 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust
790790
AlertVO alert = null;
791791
Long clusterId = cluster == null ? null : cluster.getId();
792792
Long podId = pod == null ? null : pod.getId();
793+
long dcId = dataCenter == null ? 0L : dataCenter.getId();
793794
if ((alertType != AlertManager.AlertType.ALERT_TYPE_HOST) && (alertType != AlertManager.AlertType.ALERT_TYPE_USERVM)
794795
&& (alertType != AlertManager.AlertType.ALERT_TYPE_DOMAIN_ROUTER) && (alertType != AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY)
795796
&& (alertType != AlertManager.AlertType.ALERT_TYPE_SSVM) && (alertType != AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC)
796797
&& (alertType != AlertManager.AlertType.ALERT_TYPE_MANAGEMENT_NODE) && (alertType != AlertManager.AlertType.ALERT_TYPE_RESOURCE_LIMIT_EXCEEDED)
797798
&& (alertType != AlertManager.AlertType.ALERT_TYPE_UPLOAD_FAILED) && (alertType != AlertManager.AlertType.ALERT_TYPE_OOBM_AUTH_ERROR)
798799
&& (alertType != AlertManager.AlertType.ALERT_TYPE_HA_ACTION) && (alertType != AlertManager.AlertType.ALERT_TYPE_CA_CERT)
799800
&& (alertType != AlertManager.AlertType.EVENT_USER_SESSION_BLOCK)) {
800-
alert = _alertDao.getLastAlert(alertType.getType(), dataCenter.getId(), podId, clusterId);
801+
alert = _alertDao.getLastAlert(alertType.getType(), dcId, podId, clusterId);
801802
}
802803

803804
if (alert == null) {
@@ -807,7 +808,7 @@ public void sendAlert(AlertType alertType, DataCenter dataCenter, Pod pod, Clust
807808
newAlert.setContent(content);
808809
newAlert.setClusterId(clusterId);
809810
newAlert.setPodId(podId);
810-
newAlert.setDataCenterId(dataCenter.getId());
811+
newAlert.setDataCenterId(dcId);
811812
newAlert.setSentCount(1);
812813
newAlert.setLastSent(new Date());
813814
newAlert.setName(alertType.getName());

‎server/src/main/java/com/cloud/bgp/BGPServiceImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,9 @@ public boolean allocateASNumber(long zoneId, Long asNumber, Long networkId, Long
255255
netName = network.getName();
256256
}
257257

258-
LOGGER.debug("Allocating the AS Number {} to {} on zone {}", asNumber::toString,
259-
(Objects.nonNull(vpcId) ? "VPC " + vpc : "network " + network)::toString,
260-
() -> dataCenterDao.findById(zoneId));
258+
String networkName = Objects.nonNull(vpcId) ? ("VPC " + vpc) : ("network " + network);
259+
LOGGER.debug("Allocating the AS Number {} to {} on zone {}", asNumberVO::toString,
260+
networkName::toString, () -> dataCenterDao.findById(zoneId));
261261
asNumberVO.setAllocated(true);
262262
asNumberVO.setAllocatedTime(new Date());
263263
if (Objects.nonNull(vpcId)) {

‎server/src/test/java/com/cloud/alert/AlertManagerImplTest.java‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,10 @@
2626
import org.apache.cloudstack.utils.mailing.SMTPMailSender;
2727
import org.apache.logging.log4j.Logger;
2828
import org.junit.Assert;
29+
import org.junit.Before;
2930
import org.junit.Test;
3031
import org.junit.runner.RunWith;
32+
import org.mockito.ArgumentCaptor;
3133
import org.mockito.InjectMocks;
3234
import org.mockito.Mock;
3335
import org.mockito.Mockito;
@@ -48,6 +50,12 @@
4850
import com.cloud.host.dao.HostDao;
4951
import com.cloud.storage.StorageManager;
5052

53+
import static org.junit.Assert.assertEquals;
54+
import static org.junit.Assert.assertNotNull;
55+
import static org.junit.Assert.assertNull;
56+
import static org.mockito.ArgumentMatchers.any;
57+
import static org.mockito.Mockito.verify;
58+
5159
@RunWith(MockitoJUnitRunner.class)
5260
public class AlertManagerImplTest {
5361

@@ -56,7 +64,7 @@ public class AlertManagerImplTest {
5664
AlertManagerImpl alertManagerImplMock;
5765

5866
@Mock
59-
AlertDao alertDaoMock;
67+
AlertDao _alertDao;
6068

6169
@Mock
6270
private DataCenterDao _dcDao;
@@ -88,7 +96,16 @@ public class AlertManagerImplTest {
8896
@Mock
8997
SMTPMailSender mailSenderMock;
9098

91-
private void sendMessage (){
99+
private final String[] recipients = new String[]{"[email protected]"};
100+
private final String senderAddress = "[email protected]";
101+
102+
@Before
103+
public void setUp() {
104+
alertManagerImplMock.recipients = recipients;
105+
alertManagerImplMock.senderAddress = senderAddress;
106+
}
107+
108+
private void sendMessage() {
92109
try {
93110
DataCenterVO zone = Mockito.mock(DataCenterVO.class);
94111
Mockito.when(zone.getId()).thenReturn(0L);
@@ -100,47 +117,77 @@ private void sendMessage (){
100117
Mockito.when(cluster.getId()).thenReturn(1L);
101118
Mockito.when(_clusterDao.findById(1L)).thenReturn(cluster);
102119

103-
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1l, 1l, "", "");
120+
alertManagerImplMock.sendAlert(AlertManager.AlertType.ALERT_TYPE_CPU, 0, 1L, 1L, "", "");
104121
} catch (UnsupportedEncodingException | MessagingException e) {
105122
Assert.fail();
106123
}
107124
}
108125

109126
@Test
110127
public void sendAlertTestSendMail() {
111-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
128+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
112129
Mockito.anyLong(), Mockito.anyLong());
113-
Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
114-
alertManagerImplMock.recipients = new String [] {""};
130+
Mockito.doReturn(null).when(_alertDao).persist(any());
131+
alertManagerImplMock.recipients = new String[]{""};
115132

116133
sendMessage();
117134

118-
Mockito.verify(alertManagerImplMock).sendMessage(Mockito.any());
135+
Mockito.verify(alertManagerImplMock).sendMessage(any());
119136
}
120137

121138
@Test
122139
public void sendAlertTestDebugLogging() {
123140
Mockito.doReturn(0).when(alertVOMock).getSentCount();
124-
Mockito.doReturn(alertVOMock).when(alertDaoMock).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
141+
Mockito.doReturn(alertVOMock).when(_alertDao).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
125142
Mockito.anyLong(), Mockito.anyLong());
126143

127144
sendMessage();
128145

129146
Mockito.verify(alertManagerImplMock.logger).debug(Mockito.anyString());
130-
Mockito.verify(alertManagerImplMock, Mockito.never()).sendMessage(Mockito.any());
147+
Mockito.verify(alertManagerImplMock, Mockito.never()).sendMessage(any());
131148
}
132149

133150
@Test
134151
public void sendAlertTestWarnLogging() {
135-
Mockito.doReturn(null).when(alertDaoMock).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
152+
Mockito.doReturn(null).when(_alertDao).getLastAlert(Mockito.anyShort(), Mockito.anyLong(),
136153
Mockito.anyLong(), Mockito.anyLong());
137-
Mockito.doReturn(null).when(alertDaoMock).persist(Mockito.any());
154+
Mockito.doReturn(null).when(_alertDao).persist(Mockito.any());
138155
alertManagerImplMock.recipients = null;
139156

140157
sendMessage();
141158

142159
Mockito.verify(alertManagerImplMock.logger, Mockito.times(2)).warn(Mockito.anyString());
143-
Mockito.verify(alertManagerImplMock, Mockito.never()).sendMessage(Mockito.any());
160+
Mockito.verify(alertManagerImplMock, Mockito.never()).sendMessage(any());
161+
}
162+
163+
@Test
164+
public void testSendAlertWithNullParameters() throws MessagingException, UnsupportedEncodingException {
165+
// Given
166+
String subject = "Test Subject";
167+
String content = "Test Content";
168+
AlertManager.AlertType alertType = AlertManager.AlertType.ALERT_TYPE_MEMORY;
169+
170+
// When
171+
alertManagerImplMock.sendAlert(alertType, null, null, null, subject, content);
172+
173+
// Then
174+
ArgumentCaptor<AlertVO> alertCaptor = ArgumentCaptor.forClass(AlertVO.class);
175+
verify(_alertDao).persist(alertCaptor.capture());
176+
177+
AlertVO capturedAlert = alertCaptor.getValue();
178+
assertNotNull("Captured alert should not be null", capturedAlert);
179+
assertEquals(0L, capturedAlert.getDataCenterId());
180+
assertNull(capturedAlert.getPodId());
181+
assertNull(capturedAlert.getClusterId());
182+
assertEquals(subject, capturedAlert.getSubject());
183+
assertEquals(content, capturedAlert.getContent());
184+
assertEquals(alertType.getType(), capturedAlert.getType());
185+
}
186+
187+
@Test(expected = NullPointerException.class)
188+
public void testSendAlertWithNullAlertType() throws MessagingException, UnsupportedEncodingException {
189+
// When
190+
alertManagerImplMock.sendAlert(null, 0, 1L, 1L, "subject", "content");
144191
}
145192

146193
@Test

0 commit comments

Comments
 (0)