Damans227 commented on code in PR #11443:
URL: https://github.com/apache/cloudstack/pull/11443#discussion_r4272717121


##########
server/src/main/java/com/cloud/server/ManagementServerImpl.java:
##########
@@ -1225,57 +1224,64 @@ protected void checkPortParameters(final String 
publicPort, final String private
     }
 
     @Override
-    public boolean archiveEvents(final ArchiveEventsCmd cmd) {
-        final Account caller = getCaller();
-        final List<Long> ids = cmd.getIds();
-        boolean result = true;
-        List<Long> permittedAccountIds = new ArrayList<>();
+    public boolean archiveEvents(ArchiveEventsCmd cmd) {
+        List<Long> ids = cmd.getIds();
+        String type = cmd.getType();
+        Date startDate = cmd.getStartDate();
+        Date endDate = cmd.getEndDate();
+
+        validateEventOperationParameters(ids, type, endDate, startDate);
+
+        Long accountId = null;
+        List<Long> domainIds = null;
+        Account caller = getCaller();
+        Account.Type callerType = caller.getType();
 
-        if (_accountService.isNormalUser(caller.getId()) || caller.getType() 
== Account.Type.PROJECT) {
-            permittedAccountIds.add(caller.getId());
+        if (callerType == Account.Type.NORMAL || callerType == 
Account.Type.PROJECT) {
+            accountId = caller.getId();
         } else {
-            final DomainVO domain = _domainDao.findById(caller.getDomainId());
-            final List<Long> permittedDomainIds = 
_domainDao.getDomainChildrenIds(domain.getPath());
-            permittedAccountIds = 
_accountDao.getAccountIdsForDomains(permittedDomainIds);
+            domainIds = 
_domainDao.getDomainAndChildrenIds(caller.getDomainId());
         }
 
-        final List<EventVO> events = 
_eventDao.listToArchiveOrDeleteEvents(ids, cmd.getType(), cmd.getStartDate(), 
cmd.getEndDate(), permittedAccountIds);
-        final ControlledEntity[] sameOwnerEvents = events.toArray(new 
ControlledEntity[events.size()]);
-        _accountMgr.checkAccess(CallContext.current().getCallingAccount(), 
null, false, sameOwnerEvents);
+        long totalArchived = _eventDao.archiveEvents(ids, type, startDate, 
endDate, accountId, domainIds,
+                ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value());
 
-        if (ids != null && events.size() < ids.size()) {
-            return false;
-        }
-        _eventDao.archiveEvents(events);
-        return result;
+        return totalArchived > 0;

Review Comment:
   does this still need the change you mentioned earlier? if i pass three ids 
and only one is mine, it archives that one and says success. before it failed 
and changed nothing, same for delete below



##########
server/src/main/java/com/cloud/server/ManagementServerImpl.java:
##########
@@ -1225,57 +1224,64 @@ protected void checkPortParameters(final String 
publicPort, final String private
     }
 
     @Override
-    public boolean archiveEvents(final ArchiveEventsCmd cmd) {
-        final Account caller = getCaller();
-        final List<Long> ids = cmd.getIds();
-        boolean result = true;
-        List<Long> permittedAccountIds = new ArrayList<>();
+    public boolean archiveEvents(ArchiveEventsCmd cmd) {
+        List<Long> ids = cmd.getIds();
+        String type = cmd.getType();
+        Date startDate = cmd.getStartDate();
+        Date endDate = cmd.getEndDate();
+
+        validateEventOperationParameters(ids, type, endDate, startDate);
+
+        Long accountId = null;
+        List<Long> domainIds = null;
+        Account caller = getCaller();
+        Account.Type callerType = caller.getType();
 
-        if (_accountService.isNormalUser(caller.getId()) || caller.getType() 
== Account.Type.PROJECT) {
-            permittedAccountIds.add(caller.getId());
+        if (callerType == Account.Type.NORMAL || callerType == 
Account.Type.PROJECT) {
+            accountId = caller.getId();
         } else {
-            final DomainVO domain = _domainDao.findById(caller.getDomainId());
-            final List<Long> permittedDomainIds = 
_domainDao.getDomainChildrenIds(domain.getPath());
-            permittedAccountIds = 
_accountDao.getAccountIdsForDomains(permittedDomainIds);
+            domainIds = 
_domainDao.getDomainAndChildrenIds(caller.getDomainId());

Review Comment:
   can we skip the domain filter for the root admin? right now it puts every 
domain id in the query, which gets big on clouds with lots of domains



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to