Skip to content

Commit eaf2c19

Browse files
winterhazelwinterhazel
committed
fix a few regressions and bugs
1 parent 750ede0 commit eaf2c19

5 files changed

Lines changed: 68 additions & 22 deletions

File tree

‎engine/schema/src/main/java/com/cloud/event/dao/EventDao.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public interface EventDao extends GenericDao<EventVO, Long> {
3939
long archiveEvents(List<Long> ids, String type, Date startDate, Date endDate, Long accountId, List<Long> domainIds,
4040
long limitPerQuery);
4141

42-
long purgeAll(List<Long> ids, Date startDate, Date endDate, Date limitDate, String type, Long accountId, List<Long> domainIds,
43-
long limitPerQuery);
42+
long deleteEvents(List<Long> ids, Date startDate, Date endDate, Date limitDate, String type, Long accountId, List<Long> domainIds,
43+
long limitPerQuery);
4444

4545
}

‎engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ public class EventDaoImpl extends GenericDaoBase<EventVO, Long> implements Event
3939

4040
public EventDaoImpl() {
4141
ToArchiveOrDeleteEventSearch = createSearchBuilder();
42-
ToArchiveOrDeleteEventSearch.select("id", SearchCriteria.Func.NATIVE, ToArchiveOrDeleteEventSearch.entity().getId());
4342
ToArchiveOrDeleteEventSearch.and("id", ToArchiveOrDeleteEventSearch.entity().getId(), Op.IN);
4443
ToArchiveOrDeleteEventSearch.and("type", ToArchiveOrDeleteEventSearch.entity().getType(), Op.EQ);
4544
ToArchiveOrDeleteEventSearch.and("accountId", ToArchiveOrDeleteEventSearch.entity().getAccountId(), Op.EQ);
4645
ToArchiveOrDeleteEventSearch.and("domainIds", ToArchiveOrDeleteEventSearch.entity().getDomainId(), Op.IN);
4746
ToArchiveOrDeleteEventSearch.and("createdDateB", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.BETWEEN);
47+
ToArchiveOrDeleteEventSearch.and("createdDateG", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.GTEQ);
4848
ToArchiveOrDeleteEventSearch.and("createdDateL", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.LTEQ);
4949
ToArchiveOrDeleteEventSearch.and("createdDateLT", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.LT);
5050
ToArchiveOrDeleteEventSearch.and("archived", ToArchiveOrDeleteEventSearch.entity().getArchived(), Op.EQ);
@@ -84,6 +84,8 @@ private SearchCriteria<EventVO> createEventSearchCriteria(List<Long> ids, String
8484
sc.setParameters("createdDateB", startDate, endDate);
8585
} else if (endDate != null) {
8686
sc.setParameters("createdDateL", endDate);
87+
} else if (startDate != null) {
88+
sc.setParameters("createdDateG", startDate);
8789
}
8890
sc.setParametersIfNotNull("accountId", accountId);
8991
sc.setParametersIfNotNull("createdDateLT", limitDate);
@@ -119,8 +121,8 @@ public long archiveEvents(List<Long> ids, String type, Date startDate, Date endD
119121
}
120122

121123
@Override
122-
public long purgeAll(List<Long> ids, Date startDate, Date endDate, Date limitDate, String type, Long accountId,
123-
List<Long> domainIds, long limitPerQuery) {
124+
public long deleteEvents(List<Long> ids, Date startDate, Date endDate, Date limitDate, String type, Long accountId,
125+
List<Long> domainIds, long limitPerQuery) {
124126
SearchCriteria<EventVO> sc = createEventSearchCriteria(ids, type, startDate, endDate, limitDate, accountId, domainIds);
125127
if (!allIdsMatch(ids, sc)) {
126128
return 0L;

‎engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42300to2400.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,14 @@ private void addIndexes(Connection conn) {
4848
}
4949

5050
private void dropOldIndexes(Connection conn) {
51-
DbUpgradeUtils.dropIndexIfExists(conn, "event", "archived");
52-
DbUpgradeUtils.dropIndexIfExists(conn, "event", "i_event__account_id");
53-
DbUpgradeUtils.dropIndexIfExists(conn, "event", "i_event__resource_type__resource_id");
51+
if (DbUpgradeUtils.indexExists(conn, "event", "i_event__archived__created")) {
52+
DbUpgradeUtils.dropIndexIfExists(conn, "event", "archived");
53+
}
54+
if (DbUpgradeUtils.indexExists(conn, "event", "i_event__account_id__archived__created")) {
55+
DbUpgradeUtils.dropIndexIfExists(conn, "event", "i_event__account_id");
56+
}
57+
if (DbUpgradeUtils.indexExists(conn, "event", "i_event__resource_type__resource_id__archived__created")) {
58+
DbUpgradeUtils.dropIndexIfExists(conn, "event", "i_event__resource_type__resource_id");
59+
}
5460
}
5561
}

‎server/src/main/java/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,7 +1230,7 @@ public boolean archiveEvents(ArchiveEventsCmd cmd) {
12301230
Date startDate = cmd.getStartDate();
12311231
Date endDate = cmd.getEndDate();
12321232

1233-
validateEventOperationParameters(ids, type, endDate, startDate);
1233+
validateEventOperationParameters(ids, type, startDate, endDate);
12341234

12351235
Long accountId = null;
12361236
List<Long> domainIds = null;
@@ -1241,6 +1241,9 @@ public boolean archiveEvents(ArchiveEventsCmd cmd) {
12411241
accountId = caller.getId();
12421242
} else if (callerType != Account.Type.ADMIN) {
12431243
domainIds = _domainDao.getDomainAndChildrenIds(caller.getDomainId());
1244+
if (CollectionUtils.isEmpty(domainIds)) {
1245+
throw new CloudRuntimeException(String.format("Unable to determine the domains of account %s", caller.getUuid()));
1246+
}
12441247
}
12451248

12461249
long totalArchived = _eventDao.archiveEvents(ids, type, startDate, endDate, accountId, domainIds,
@@ -1256,7 +1259,7 @@ public boolean deleteEvents(DeleteEventsCmd cmd) {
12561259
Date startDate = cmd.getStartDate();
12571260
Date endDate = cmd.getEndDate();
12581261

1259-
validateEventOperationParameters(ids, type, endDate, startDate);
1262+
validateEventOperationParameters(ids, type, startDate, endDate);
12601263

12611264
Long accountId = null;
12621265
List<Long> domainIds = null;
@@ -1267,15 +1270,21 @@ public boolean deleteEvents(DeleteEventsCmd cmd) {
12671270
accountId = caller.getId();
12681271
} else if (callerType != Account.Type.ADMIN) {
12691272
domainIds = _domainDao.getDomainAndChildrenIds(caller.getDomainId());
1273+
if (CollectionUtils.isEmpty(domainIds)) {
1274+
throw new CloudRuntimeException(String.format("Unable to determine the domains of account %s", caller.getUuid()));
1275+
}
12701276
}
12711277

1272-
long totalRemoved = _eventDao.purgeAll(ids, startDate, endDate, null, type, accountId, domainIds,
1278+
long totalRemoved = _eventDao.deleteEvents(ids, startDate, endDate, null, type, accountId, domainIds,
12731279
ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
12741280

12751281
return CollectionUtils.isEmpty(ids) || totalRemoved > 0;
12761282
}
12771283

1278-
protected void validateEventOperationParameters(List<Long> ids, String type, Date endDate, Date startDate) {
1284+
protected void validateEventOperationParameters(List<Long> ids, String type, Date startDate, Date endDate) {
1285+
if (ids != null && ids.isEmpty()) {
1286+
throw new InvalidParameterValueException("'ids' must not be empty when provided.");
1287+
}
12791288
if (CollectionUtils.isEmpty(ids) && ObjectUtils.allNull(type, endDate)) {
12801289
throw new InvalidParameterValueException("Either 'ids', 'type' or 'enddate' must be specified.");
12811290
}
@@ -4475,7 +4484,7 @@ protected void runInContext() {
44754484
purgeCal.add(Calendar.DAY_OF_YEAR, -_purgeDelay);
44764485
final Date purgeTime = purgeCal.getTime();
44774486
logger.debug("Starting to purge all event rows older than [{}].", purgeTime);
4478-
long totalRemoved = _eventDao.purgeAll(null, null, null, purgeTime, null, null, null,
4487+
long totalRemoved = _eventDao.deleteEvents(null, null, null, purgeTime, null, null, null,
44794488
ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
44804489
logger.info("Purged a total of [{}] event rows older than [{}].", totalRemoved, purgeTime);
44814490
} catch (final Exception e) {

‎server/src/test/java/com/cloud/server/ManagementServerImplTest.java‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,12 +1154,17 @@ public void testGetExternalVmConsole() {
11541154

11551155
@Test(expected = InvalidParameterValueException.class)
11561156
public void validateEventOperationParametersTestIdsAndTypeAndEndDateIsNullThrowInvalidParameterValueException() {
1157-
spy.validateEventOperationParameters(null, null, null, new Date());
1157+
spy.validateEventOperationParameters(null, null, new Date(), null);
11581158
}
11591159

11601160
@Test(expected = InvalidParameterValueException.class)
11611161
public void validateEventOperationParametersTestStartDateIsNotNullAndEndDateIsNullThrowInvalidParameterValueException() {
1162-
spy.validateEventOperationParameters(List.of(1L), null, null, new Date());
1162+
spy.validateEventOperationParameters(List.of(1L), null, new Date(), null);
1163+
}
1164+
1165+
@Test(expected = InvalidParameterValueException.class)
1166+
public void validateEventOperationParametersTestEmptyIdsThrowInvalidParameterValueException() {
1167+
spy.validateEventOperationParameters(Collections.emptyList(), "type", null, null);
11631168
}
11641169

11651170
private ArchiveEventsCmd createArchiveEventsCmdForTests() {
@@ -1183,7 +1188,7 @@ public void archiveEventsTestArchivesOnlyOwnEventsWhenCallerIsNormalUser() {
11831188

11841189
spy.archiveEvents(cmd);
11851190

1186-
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getEndDate(), cmd.getStartDate());
1191+
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate());
11871192
Mockito.verify(eventDao).archiveEvents(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate(), callerId,
11881193
null, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
11891194
}
@@ -1201,11 +1206,23 @@ public void archiveEventsTestArchivesOnlyEventsOfOwnDomainWhenCallerIsAdmin() {
12011206

12021207
spy.archiveEvents(cmd);
12031208

1204-
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getEndDate(), cmd.getStartDate());
1209+
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate());
12051210
Mockito.verify(eventDao).archiveEvents(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate(), null,
12061211
domainIds, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
12071212
}
12081213

1214+
@Test(expected = CloudRuntimeException.class)
1215+
public void archiveEventsTestFailsWhenCallerDomainIsNotFound() {
1216+
ArchiveEventsCmd cmd = createArchiveEventsCmdForTests();
1217+
1218+
Mockito.doReturn(Account.Type.DOMAIN_ADMIN).when(account).getType();
1219+
Mockito.doReturn(6L).when(account).getDomainId();
1220+
Mockito.doReturn(account).when(spy).getCaller();
1221+
Mockito.doReturn(Collections.emptyList()).when(domainDao).getDomainAndChildrenIds(6L);
1222+
1223+
spy.archiveEvents(cmd);
1224+
}
1225+
12091226
@Test
12101227
public void archiveEventsTestDoesNotFilterDomainsWhenCallerIsRootAdmin() {
12111228
ArchiveEventsCmd cmd = createArchiveEventsCmdForTests();
@@ -1241,8 +1258,8 @@ public void deleteEventsTestDeletesOnlyOwnEventsWhenCallerIsNormalUser() {
12411258

12421259
spy.deleteEvents(cmd);
12431260

1244-
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getEndDate(), cmd.getStartDate());
1245-
Mockito.verify(eventDao).purgeAll(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
1261+
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate());
1262+
Mockito.verify(eventDao).deleteEvents(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
12461263
callerId, null, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
12471264
}
12481265

@@ -1259,11 +1276,23 @@ public void deleteEventsTestDeletesOnlyEventsOfOwnDomainCallerIsAdmin() {
12591276

12601277
spy.deleteEvents(cmd);
12611278

1262-
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getEndDate(), cmd.getStartDate());
1263-
Mockito.verify(eventDao).purgeAll(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
1279+
Mockito.verify(spy).validateEventOperationParameters(cmd.getIds(), cmd.getType(), cmd.getStartDate(), cmd.getEndDate());
1280+
Mockito.verify(eventDao).deleteEvents(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
12641281
null, domainIds, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
12651282
}
12661283

1284+
@Test(expected = CloudRuntimeException.class)
1285+
public void deleteEventsTestFailsWhenCallerDomainIsNotFound() {
1286+
DeleteEventsCmd cmd = createDeleteEventsCmdForTests();
1287+
1288+
Mockito.doReturn(Account.Type.DOMAIN_ADMIN).when(account).getType();
1289+
Mockito.doReturn(6L).when(account).getDomainId();
1290+
Mockito.doReturn(account).when(spy).getCaller();
1291+
Mockito.doReturn(Collections.emptyList()).when(domainDao).getDomainAndChildrenIds(6L);
1292+
1293+
spy.deleteEvents(cmd);
1294+
}
1295+
12671296
@Test
12681297
public void deleteEventsTestDoesNotFilterDomainsWhenCallerIsRootAdmin() {
12691298
DeleteEventsCmd cmd = createDeleteEventsCmdForTests();
@@ -1274,7 +1303,7 @@ public void deleteEventsTestDoesNotFilterDomainsWhenCallerIsRootAdmin() {
12741303
spy.deleteEvents(cmd);
12751304

12761305
Mockito.verify(domainDao, Mockito.never()).getDomainAndChildrenIds(Mockito.anyLong());
1277-
Mockito.verify(eventDao).purgeAll(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
1306+
Mockito.verify(eventDao).deleteEvents(cmd.getIds(), cmd.getStartDate(), cmd.getEndDate(), null, cmd.getType(),
12781307
null, null, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
12791308
}
12801309

0 commit comments

Comments
 (0)