From c599c24f16bad206975ea9a17524e1cd68cdefc4 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 7 Nov 2023 15:01:20 +0530 Subject: [PATCH 01/28] Use join instead of views for filtering volumes --- .../com/cloud/api/query/QueryManagerImpl.java | 184 +++++++++++------- 1 file changed, 111 insertions(+), 73 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index c3807fdb8d7e..fd793e763486 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -58,6 +58,7 @@ import com.cloud.vm.dao.InstanceGroupVMMapDao; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDetailsDao; +import com.cloud.storage.VolumeVO; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker; @@ -176,6 +177,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -2331,6 +2333,19 @@ public ListResponse searchForVolumes(ListVolumesCmd cmd) { } private Pair, Integer> searchForVolumesInternal(ListVolumesCmd cmd) { + Pair, Integer> volumeIdPage = searchForVolumeIdsAndCount(cmd); + + Integer count = volumeIdPage.second(); + Long[] idArray = volumeIdPage.first().toArray(new Long[0]); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List vms = _volumeJoinDao.searchByIds( idArray); + return new Pair<>(vms, count); + } + private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) { Account caller = CallContext.current().getCallingAccount(); List permittedAccounts = new ArrayList(); @@ -2358,61 +2373,97 @@ private Pair, Integer> searchForVolumesInternal(ListVolumesCm Long domainId = domainIdRecursiveListProject.first(); Boolean isRecursive = domainIdRecursiveListProject.second(); ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); - Filter searchFilter = new Filter(VolumeJoinVO.class, "created", false, cmd.getStartIndex(), cmd.getPageSizeVal()); + Filter searchFilter = new Filter(VolumeVO.class, "created", false, cmd.getStartIndex(), cmd.getPageSizeVal()); - // hack for now, this should be done better but due to needing a join I - // opted to - // do this quickly and worry about making it pretty later - SearchBuilder sb = _volumeJoinDao.createSearchBuilder(); - sb.select(null, Func.DISTINCT, sb.entity().getId()); // select distinct - // ids to get - // number of - // records with - // pagination - accountMgr.buildACLViewSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchBuilder volumeSearchBuilder = volumeDao.createSearchBuilder(); + volumeSearchBuilder.select(null, Func.DISTINCT, volumeSearchBuilder.entity().getId()); // select distinct + accountMgr.buildACLSearchBuilder(volumeSearchBuilder, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); - sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN); - sb.and("volumeType", sb.entity().getVolumeType(), SearchCriteria.Op.LIKE); - sb.and("uuid", sb.entity().getUuid(), SearchCriteria.Op.NNULL); - sb.and("instanceId", sb.entity().getVmId(), SearchCriteria.Op.EQ); - sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ); - sb.and("podId", sb.entity().getPodId(), SearchCriteria.Op.EQ); + if (CollectionUtils.isNotEmpty(ids)) { + volumeSearchBuilder.and("idIN", volumeSearchBuilder.entity().getId(), SearchCriteria.Op.IN); + } + + volumeSearchBuilder.and("name", volumeSearchBuilder.entity().getName(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("volumeType", volumeSearchBuilder.entity().getVolumeType(), SearchCriteria.Op.LIKE); + volumeSearchBuilder.and("uuid", volumeSearchBuilder.entity().getUuid(), SearchCriteria.Op.NNULL); + volumeSearchBuilder.and("instanceId", volumeSearchBuilder.entity().getInstanceId(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("dataCenterId", volumeSearchBuilder.entity().getDataCenterId(), SearchCriteria.Op.EQ); + + if (keyword != null) { + volumeSearchBuilder.and().op("keywordName", volumeSearchBuilder.entity().getName(), SearchCriteria.Op.LIKE); + volumeSearchBuilder.or("keywordVolumeType", volumeSearchBuilder.entity().getVolumeType(), SearchCriteria.Op.LIKE); + volumeSearchBuilder.or("keywordState", volumeSearchBuilder.entity().getState(), SearchCriteria.Op.LIKE); + volumeSearchBuilder.cp(); + } + + StoragePoolVO poolVO = null; if (storageId != null) { - StoragePoolVO poolVO = storagePoolDao.findByUuid(storageId); - if (poolVO.getPoolType() == Storage.StoragePoolType.DatastoreCluster) { - sb.and("storageId", sb.entity().getPoolUuid(), SearchCriteria.Op.IN); + poolVO = storagePoolDao.findByUuid(storageId); + if (poolVO == null) { + throw new InvalidParameterValueException("Unable to find storage pool by uuid " + storageId); + } else if (poolVO.getPoolType() == Storage.StoragePoolType.DatastoreCluster) { + volumeSearchBuilder.and("storageId", volumeSearchBuilder.entity().getPoolId(), SearchCriteria.Op.IN); } else { - sb.and("storageId", sb.entity().getPoolUuid(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("storageId", volumeSearchBuilder.entity().getPoolId(), SearchCriteria.Op.EQ); } } - sb.and("diskOfferingId", sb.entity().getDiskOfferingId(), SearchCriteria.Op.EQ); - sb.and("display", sb.entity().isDisplayVolume(), SearchCriteria.Op.EQ); - sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); - sb.and("stateNEQ", sb.entity().getState(), SearchCriteria.Op.NEQ); + if (clusterId != null || podId != null) { + SearchBuilder storagePoolSearch = storagePoolDao.createSearchBuilder(); + storagePoolSearch.and("clusterId", storagePoolSearch.entity().getClusterId(), SearchCriteria.Op.EQ); + storagePoolSearch.and("podId", storagePoolSearch.entity().getPodId(), SearchCriteria.Op.EQ); + volumeSearchBuilder.join("storagePoolSearch", storagePoolSearch, storagePoolSearch.entity().getId(), volumeSearchBuilder.entity().getPoolId(), JoinBuilder.JoinType.INNER); + } + + volumeSearchBuilder.and("diskOfferingId", volumeSearchBuilder.entity().getDiskOfferingId(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("display", volumeSearchBuilder.entity().isDisplayVolume(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("state", volumeSearchBuilder.entity().getState(), SearchCriteria.Op.EQ); + volumeSearchBuilder.and("stateNEQ", volumeSearchBuilder.entity().getState(), SearchCriteria.Op.NEQ); + + // Need to test thoroughly if (!shouldListSystemVms) { - sb.and().op("systemUse", sb.entity().isSystemUse(), SearchCriteria.Op.NEQ); - sb.or("nulltype", sb.entity().isSystemUse(), SearchCriteria.Op.NULL); - sb.cp(); + SearchBuilder vmSearch = _vmInstanceDao.createSearchBuilder(); + SearchBuilder serviceOfferingSearch = _srvOfferingDao.createSearchBuilder(); + vmSearch.and().op("svmType", vmSearch.entity().getType(), SearchCriteria.Op.NIN); + vmSearch.or("nulltype", vmSearch.entity().getType(), SearchCriteria.Op.NULL); + vmSearch.cp(); + + serviceOfferingSearch.and().op("systemUse", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NEQ); + serviceOfferingSearch.or("nulltype", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NULL); + serviceOfferingSearch.cp(); + + vmSearch.join("serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); + + volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); - sb.and().op("type", sb.entity().getVmType(), SearchCriteria.Op.NIN); - sb.or("nulltype", sb.entity().getVmType(), SearchCriteria.Op.NULL); - sb.cp(); + } + + if (MapUtils.isNotEmpty(tags)) { + SearchBuilder resourceTagSearch = resourceTagDao.createSearchBuilder(); + resourceTagSearch.and("resourceType", resourceTagSearch.entity().getResourceType(), Op.EQ); + resourceTagSearch.and().op(); + for (int count = 0; count < tags.size(); count++) { + if (count == 0) { + resourceTagSearch.op("tagKey" + count, resourceTagSearch.entity().getKey(), Op.EQ); + } else { + resourceTagSearch.or().op("tagKey" + count, resourceTagSearch.entity().getKey(), Op.EQ); + } + resourceTagSearch.and("tagValue" + count, resourceTagSearch.entity().getValue(), Op.EQ); + resourceTagSearch.cp(); + } + resourceTagSearch.cp(); + + volumeSearchBuilder.join("tags", resourceTagSearch, resourceTagSearch.entity().getResourceId(), volumeSearchBuilder.entity().getId(), JoinBuilder.JoinType.INNER); } // now set the SC criteria... - SearchCriteria sc = sb.create(); - accountMgr.buildACLViewSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + SearchCriteria sc = volumeSearchBuilder.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); if (keyword != null) { - SearchCriteria ssc = _volumeJoinDao.createSearchCriteria(); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("volumeType", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("state", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - - sc.addAnd("name", SearchCriteria.Op.SC, ssc); + sc.setParameters("keywordName", "%" + keyword + "%"); + sc.setParameters("keywordVolumeType", "%" + keyword + "%"); + sc.setParameters("keywordState", "%" + keyword + "%"); } if (name != null) { @@ -2426,19 +2477,18 @@ private Pair, Integer> searchForVolumesInternal(ListVolumesCm setIdsListToSearchCriteria(sc, ids); if (!shouldListSystemVms) { - sc.setParameters("systemUse", 1); - sc.setParameters("type", VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.DomainRouter); + sc.setJoinParameters("vmSearch", "svmType", VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.DomainRouter); + sc.setJoinParameters("serviceOfferingSearch", "systemUse", 1); } - if (tags != null && !tags.isEmpty()) { - SearchCriteria tagSc = _volumeJoinDao.createSearchCriteria(); - for (String key : tags.keySet()) { - SearchCriteria tsc = _volumeJoinDao.createSearchCriteria(); - tsc.addAnd("tagKey", SearchCriteria.Op.EQ, key); - tsc.addAnd("tagValue", SearchCriteria.Op.EQ, tags.get(key)); - tagSc.addOr("tagKey", SearchCriteria.Op.SC, tsc); + if (MapUtils.isNotEmpty(tags)) { + int count = 0; + sc.setJoinParameters("tags", "resourceType", ResourceObjectType.Volume); + for (Map.Entry entry : tags.entrySet()) { + sc.setJoinParameters("tags", "tagKey" + count, entry.getKey()); + sc.setJoinParameters("tags", "tagValue" + count, entry.getValue()); + count++; } - sc.addAnd("tagKey", SearchCriteria.Op.SC, tagSc); } if (diskOffId != null) { @@ -2458,23 +2508,21 @@ private Pair, Integer> searchForVolumesInternal(ListVolumesCm if (zoneId != null) { sc.setParameters("dataCenterId", zoneId); } - if (podId != null) { - sc.setParameters("podId", podId); - } if (storageId != null) { - StoragePoolVO poolVO = storagePoolDao.findByUuid(storageId); if (poolVO.getPoolType() == Storage.StoragePoolType.DatastoreCluster) { - List childDatastores = storagePoolDao.listChildStoragePoolsInDatastoreCluster(poolVO.getId()); - List childDatastoreIds = childDatastores.stream().map(mo -> mo.getUuid()).collect(Collectors.toList()); - sc.setParameters("storageId", childDatastoreIds.toArray()); + List childDataStores = storagePoolDao.listChildStoragePoolsInDatastoreCluster(poolVO.getId()); + sc.setParameters("storageId", childDataStores.stream().map(StoragePoolVO::getId).toArray()); } else { - sc.setParameters("storageId", storageId); + sc.setParameters("storageId", poolVO.getId()); } } if (clusterId != null) { - sc.setParameters("clusterId", clusterId); + sc.setJoinParameters("storagePoolSearch", "clusterId", clusterId); + } + if (podId != null) { + sc.setJoinParameters("storagePoolSearch", "podId", podId); } if (state != null) { @@ -2484,20 +2532,10 @@ private Pair, Integer> searchForVolumesInternal(ListVolumesCm } // search Volume details by ids - Pair, Integer> uniqueVolPair = _volumeJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueVolPair = volumeDao.searchAndCount(sc, searchFilter); Integer count = uniqueVolPair.second(); - if (count.intValue() == 0) { - // empty result - return uniqueVolPair; - } - List uniqueVols = uniqueVolPair.first(); - Long[] vrIds = new Long[uniqueVols.size()]; - int i = 0; - for (VolumeJoinVO v : uniqueVols) { - vrIds[i++] = v.getId(); - } - List vrs = _volumeJoinDao.searchByIds(vrIds); - return new Pair, Integer>(vrs, count); + List vmIds = uniqueVolPair.first().stream().map(VolumeVO::getId).collect(Collectors.toList()); + return new Pair<>(vmIds, count); } private boolean shouldListSystemVms(ListVolumesCmd cmd, Long callerId) { From 116243b78a63c93765280c466df522035fb7e35c Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 8 Nov 2023 01:34:21 +0530 Subject: [PATCH 02/28] Use join instead of views for filtering events --- .../com/cloud/api/query/QueryManagerImpl.java | 87 ++++++++++++------- 1 file changed, 57 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index fd793e763486..8c79ffc3fd4e 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -36,6 +36,8 @@ import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; +import com.cloud.event.EventVO; +import com.cloud.event.dao.EventDao; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplatePoolDao; @@ -353,6 +355,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private UserAccountJoinDao _userAccountJoinDao; + @Inject + private EventDao eventDao; + @Inject private EventJoinDao _eventJoinDao; @@ -790,9 +795,23 @@ public ListResponse searchForEvents(ListEventsCmd cmd) { } private Pair, Integer> searchForEventsInternal(ListEventsCmd cmd) { + Pair, Integer> eventIdPage = searchForEventIdsAndCount(cmd); + + Integer count = eventIdPage.second(); + Long[] idArray = eventIdPage.first().toArray(new Long[0]); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List events = _eventJoinDao.searchByIds( idArray); + return new Pair<>(events, count); + } + + private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); boolean isRootAdmin = accountMgr.isRootAdmin(caller.getId()); - List permittedAccounts = new ArrayList(); + List permittedAccounts = new ArrayList<>(); Long id = cmd.getId(); String type = cmd.getType(); @@ -841,32 +860,39 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c Boolean isRecursive = domainIdRecursiveListProject.second(); ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); - Filter searchFilter = new Filter(EventJoinVO.class, "createDate", false, cmd.getStartIndex(), cmd.getPageSizeVal()); + Filter searchFilter = new Filter(EventVO.class, "createDate", false, cmd.getStartIndex(), cmd.getPageSizeVal()); // additional order by since createdDate does not have milliseconds // and two events, created within one second can be incorrectly ordered (for example VM.CREATE Completed before Scheduled) - searchFilter.addOrderBy(EventJoinVO.class, "id", false); + searchFilter.addOrderBy(EventVO.class, "id", false); + + SearchBuilder eventSearchBuilder = eventDao.createSearchBuilder(); + accountMgr.buildACLSearchBuilder(eventSearchBuilder, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + + eventSearchBuilder.and("id", eventSearchBuilder.entity().getId(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("levelL", eventSearchBuilder.entity().getLevel(), SearchCriteria.Op.LIKE); + eventSearchBuilder.and("levelEQ", eventSearchBuilder.entity().getLevel(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("type", eventSearchBuilder.entity().getType(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("createDateB", eventSearchBuilder.entity().getCreateDate(), SearchCriteria.Op.BETWEEN); + eventSearchBuilder.and("createDateG", eventSearchBuilder.entity().getCreateDate(), SearchCriteria.Op.GTEQ); + eventSearchBuilder.and("createDateL", eventSearchBuilder.entity().getCreateDate(), SearchCriteria.Op.LTEQ); + eventSearchBuilder.and("state", eventSearchBuilder.entity().getState(), SearchCriteria.Op.NEQ); + eventSearchBuilder.or("startId", eventSearchBuilder.entity().getStartId(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("createDate", eventSearchBuilder.entity().getCreateDate(), SearchCriteria.Op.BETWEEN); + eventSearchBuilder.and("displayEvent", eventSearchBuilder.entity().isDisplay(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("archived", eventSearchBuilder.entity().getArchived(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("resourceId", eventSearchBuilder.entity().getResourceId(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("resourceType", eventSearchBuilder.entity().getResourceType(), SearchCriteria.Op.EQ); - SearchBuilder sb = _eventJoinDao.createSearchBuilder(); - accountMgr.buildACLViewSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); - - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("levelL", sb.entity().getLevel(), SearchCriteria.Op.LIKE); - sb.and("levelEQ", sb.entity().getLevel(), SearchCriteria.Op.EQ); - sb.and("type", sb.entity().getType(), SearchCriteria.Op.EQ); - sb.and("createDateB", sb.entity().getCreateDate(), SearchCriteria.Op.BETWEEN); - sb.and("createDateG", sb.entity().getCreateDate(), SearchCriteria.Op.GTEQ); - sb.and("createDateL", sb.entity().getCreateDate(), SearchCriteria.Op.LTEQ); - sb.and("state", sb.entity().getState(), SearchCriteria.Op.NEQ); - sb.or("startId", sb.entity().getStartId(), SearchCriteria.Op.EQ); - sb.and("createDate", sb.entity().getCreateDate(), SearchCriteria.Op.BETWEEN); - sb.and("displayEvent", sb.entity().getDisplay(), SearchCriteria.Op.EQ); - sb.and("archived", sb.entity().getArchived(), SearchCriteria.Op.EQ); - sb.and("resourceId", sb.entity().getResourceId(), SearchCriteria.Op.EQ); - sb.and("resourceType", sb.entity().getResourceType(), SearchCriteria.Op.EQ); + if (keyword != null) { + eventSearchBuilder.and().op("keywordType", eventSearchBuilder.entity().getType(), SearchCriteria.Op.LIKE); + eventSearchBuilder.or("keywordDescription", eventSearchBuilder.entity().getDescription(), SearchCriteria.Op.LIKE); + eventSearchBuilder.or("keywordLevel", eventSearchBuilder.entity().getLevel(), SearchCriteria.Op.LIKE); + eventSearchBuilder.cp(); + } - SearchCriteria sc = sb.create(); + SearchCriteria sc = eventSearchBuilder.create(); // building ACL condition - accountMgr.buildACLViewSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); // For end users display only enabled events if (!accountMgr.isRootAdmin(caller.getId())) { @@ -885,11 +911,9 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c } if (keyword != null) { - SearchCriteria ssc = _eventJoinDao.createSearchCriteria(); - ssc.addOr("type", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("description", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("level", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - sc.addAnd("level", SearchCriteria.Op.SC, ssc); + sc.setParameters("keywordType", "%" + keyword + "%"); + sc.setParameters("keywordDescription", "%" + keyword + "%"); + sc.setParameters("keywordLevel", "%" + keyword + "%"); } if (level != null) { @@ -920,7 +944,7 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c sc.setParameters("archived", cmd.getArchived()); } - Pair, Integer> eventPair = null; + Pair, Integer> eventPair = null; // event_view will not have duplicate rows for each event, so // searchAndCount should be good enough. if ((entryTime != null) && (duration != null)) { @@ -944,11 +968,14 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c * _eventDao.findCompletedEvent(event.getId()); if (completedEvent * == null) { pendingEvents.add(event); } } return pendingEvents; */ + eventPair = new Pair<>(new ArrayList<>(), 0); } else { - eventPair = _eventJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueEventPair = eventDao.searchAndCount(sc, searchFilter); + Integer count = uniqueEventPair.second(); + List eventIds = uniqueEventPair.first().stream().map(EventVO::getId).collect(Collectors.toList()); + eventPair = new Pair<>(eventIds, count); } return eventPair; - } @Override From 8671c666b69b3820c17de439996a310dc572156a Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 8 Nov 2023 14:01:36 +0530 Subject: [PATCH 03/28] Use join instead of views for filtering accounts --- .../com/cloud/api/query/QueryManagerImpl.java | 71 +++++++++++++------ .../cloud/api/query/dao/AccountJoinDao.java | 2 + .../api/query/dao/AccountJoinDaoImpl.java | 53 ++++++++++++++ 3 files changed, 104 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 8c79ffc3fd4e..d1c0820472da 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -52,6 +52,7 @@ import com.cloud.network.PublicIpQuarantine; import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.AccountVO; import com.cloud.user.SSHKeyPairVO; import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.vm.InstanceGroupVMMapVO; @@ -804,7 +805,7 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c return new Pair<>(new ArrayList<>(), count); } - List events = _eventJoinDao.searchByIds( idArray); + List events = _eventJoinDao.searchByIds(idArray); return new Pair<>(events, count); } @@ -866,6 +867,7 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { searchFilter.addOrderBy(EventVO.class, "id", false); SearchBuilder eventSearchBuilder = eventDao.createSearchBuilder(); + eventSearchBuilder.select(null, Func.DISTINCT, eventSearchBuilder.entity().getId()); accountMgr.buildACLSearchBuilder(eventSearchBuilder, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); eventSearchBuilder.and("id", eventSearchBuilder.entity().getId(), SearchCriteria.Op.EQ); @@ -1160,7 +1162,7 @@ private Pair, Integer> searchForUserVMsInternal(ListVMsCmd cm } // search vm details by ids - List vms = _userVmJoinDao.searchByIds( idArray); + List vms = _userVmJoinDao.searchByIds(idArray); return new Pair<>(vms, count); } @@ -2369,7 +2371,7 @@ private Pair, Integer> searchForVolumesInternal(ListVolumesCm return new Pair<>(new ArrayList<>(), count); } - List vms = _volumeJoinDao.searchByIds( idArray); + List vms = _volumeJoinDao.searchByIds(idArray); return new Pair<>(vms, count); } private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) { @@ -2667,6 +2669,21 @@ public ListResponse searchForAccounts(ListAccountsCmd cmd) { } private Pair, Integer> searchForAccountsInternal(ListAccountsCmd cmd) { + + Pair, Integer> accountIdPage = searchForAccountIdsAndCount(cmd); + + Integer count = accountIdPage.second(); + Long[] idArray = accountIdPage.first().toArray(new Long[0]); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List accounts = _accountJoinDao.searchByIds(idArray); + return new Pair<>(accounts, count); + } + + private Pair, Integer> searchForAccountIdsAndCount(ListAccountsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); Long domainId = cmd.getDomainId(); Long accountId = cmd.getId(); @@ -2722,29 +2739,38 @@ private Pair, Integer> searchForAccountsInternal(ListAccount accountMgr.checkAccess(caller, null, true, account); } - Filter searchFilter = new Filter(AccountJoinVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); + Filter searchFilter = new Filter(AccountVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); Object type = cmd.getAccountType(); Object state = cmd.getState(); Object isCleanupRequired = cmd.isCleanupRequired(); Object keyword = cmd.getKeyword(); - SearchBuilder sb = _accountJoinDao.createSearchBuilder(); - sb.and("accountName", sb.entity().getAccountName(), SearchCriteria.Op.EQ); - sb.and("domainId", sb.entity().getDomainId(), SearchCriteria.Op.EQ); - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("type", sb.entity().getType(), SearchCriteria.Op.EQ); - sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); - sb.and("needsCleanup", sb.entity().isNeedsCleanup(), SearchCriteria.Op.EQ); - sb.and("typeNEQ", sb.entity().getType(), SearchCriteria.Op.NEQ); - sb.and("idNEQ", sb.entity().getId(), SearchCriteria.Op.NEQ); - sb.and("type2NEQ", sb.entity().getType(), SearchCriteria.Op.NEQ); + SearchBuilder accountSearchBuilder = _accountDao.createSearchBuilder(); + accountSearchBuilder.select(null, Func.DISTINCT, accountSearchBuilder.entity().getId()); // select distinct + accountSearchBuilder.and("accountName", accountSearchBuilder.entity().getAccountName(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("domainId", accountSearchBuilder.entity().getDomainId(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("id", accountSearchBuilder.entity().getId(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("type", accountSearchBuilder.entity().getType(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("state", accountSearchBuilder.entity().getState(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("needsCleanup", accountSearchBuilder.entity().getNeedsCleanup(), SearchCriteria.Op.EQ); + accountSearchBuilder.and("typeNEQ", accountSearchBuilder.entity().getType(), SearchCriteria.Op.NEQ); + accountSearchBuilder.and("idNEQ", accountSearchBuilder.entity().getId(), SearchCriteria.Op.NEQ); + accountSearchBuilder.and("type2NEQ", accountSearchBuilder.entity().getType(), SearchCriteria.Op.NEQ); if (domainId != null && isRecursive) { - sb.and("path", sb.entity().getDomainPath(), SearchCriteria.Op.LIKE); + SearchBuilder domainSearch = _domainDao.createSearchBuilder(); + domainSearch.and("path", domainSearch.entity().getPath(), SearchCriteria.Op.LIKE); + accountSearchBuilder.join("domainSearch", domainSearch, domainSearch.entity().getId(), accountSearchBuilder.entity().getDomainId(), JoinBuilder.JoinType.INNER); } - SearchCriteria sc = sb.create(); + if (keyword != null) { + accountSearchBuilder.and().op("keywordAccountName", accountSearchBuilder.entity().getAccountName(), SearchCriteria.Op.LIKE); + accountSearchBuilder.or("keywordState", accountSearchBuilder.entity().getState(), SearchCriteria.Op.LIKE); + accountSearchBuilder.cp(); + } + + SearchCriteria sc = accountSearchBuilder.create(); // don't return account of type project to the end user sc.setParameters("typeNEQ", Account.Type.PROJECT); @@ -2758,10 +2784,8 @@ private Pair, Integer> searchForAccountsInternal(ListAccount } if (keyword != null) { - SearchCriteria ssc = _accountJoinDao.createSearchCriteria(); - ssc.addOr("accountName", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("state", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - sc.addAnd("accountName", SearchCriteria.Op.SC, ssc); + sc.setParameters("keywordAccountName", "%" + keyword + "%"); + sc.setParameters("keywordState", "%" + keyword + "%"); } if (type != null) { @@ -2790,13 +2814,16 @@ private Pair, Integer> searchForAccountsInternal(ListAccount if (domain == null) { domain = _domainDao.findById(domainId); } - sc.setParameters("path", domain.getPath() + "%"); + sc.setJoinParameters("domainSearch", "path", domain.getPath() + "%"); } else { sc.setParameters("domainId", domainId); } } - return _accountJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueAccountPair = _accountDao.searchAndCount(sc, searchFilter); + Integer count = uniqueAccountPair.second(); + List accountIds = uniqueAccountPair.first().stream().map(AccountVO::getId).collect(Collectors.toList()); + return new Pair<>(accountIds, count); } @Override diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDao.java index 42842f568b40..16bda5b4aa91 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDao.java @@ -17,6 +17,7 @@ package com.cloud.api.query.dao; import java.util.EnumSet; +import java.util.List; import org.apache.cloudstack.api.ApiConstants.DomainDetails; import org.apache.cloudstack.api.ResponseObject.ResponseView; @@ -35,4 +36,5 @@ public interface AccountJoinDao extends GenericDao { void setResourceLimits(AccountJoinVO account, boolean accountIsAdmin, ResourceLimitAndCountResponse response); + List searchByIds(Long... ids); } diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java index 790758c627fd..d03b6db9b0d6 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java @@ -16,11 +16,13 @@ // under the License. package com.cloud.api.query.dao; +import java.util.ArrayList; import java.util.EnumSet; import java.util.List; import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -46,12 +48,19 @@ public class AccountJoinDaoImpl extends GenericDaoBase implements AccountJoinDao { public static final Logger s_logger = Logger.getLogger(AccountJoinDaoImpl.class); + @Inject + private ConfigurationDao configDao; private final SearchBuilder acctIdSearch; + private final SearchBuilder volSearch; @Inject AccountManager _acctMgr; protected AccountJoinDaoImpl() { + volSearch = createSearchBuilder(); + volSearch.and("idIN", volSearch.entity().getId(), SearchCriteria.Op.IN); + volSearch.done(); + acctIdSearch = createSearchBuilder(); acctIdSearch.and("id", acctIdSearch.entity().getId(), SearchCriteria.Op.EQ); acctIdSearch.done(); @@ -232,6 +241,50 @@ public void setResourceLimits(AccountJoinVO account, boolean fullView, ResourceL response.setSecondaryStorageAvailable(secondaryStorageAvail); } + @Override + public List searchByIds(Long... accountIds) { + // set detail batch query size + int DETAILS_BATCH_SIZE = 2000; + String batchCfg = configDao.getValue("detail.batch.query.size"); + if (batchCfg != null) { + DETAILS_BATCH_SIZE = Integer.parseInt(batchCfg); + } + // query details by batches + List uvList = new ArrayList<>(); + // query details by batches + int curr_index = 0; + if (accountIds.length > DETAILS_BATCH_SIZE) { + while ((curr_index + DETAILS_BATCH_SIZE) <= accountIds.length) { + Long[] ids = new Long[DETAILS_BATCH_SIZE]; + for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { + ids[k] = accountIds[j]; + } + SearchCriteria sc = volSearch.create(); + sc.setParameters("idIN", ids); + List vms = searchIncludingRemoved(sc, null, null, false); + if (vms != null) { + uvList.addAll(vms); + } + curr_index += DETAILS_BATCH_SIZE; + } + } + if (curr_index < accountIds.length) { + int batch_size = (accountIds.length - curr_index); + // set the ids value + Long[] ids = new Long[batch_size]; + for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { + ids[k] = accountIds[j]; + } + SearchCriteria sc = volSearch.create(); + sc.setParameters("idIN", ids); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); + } + } + return uvList; + } + @Override public AccountJoinVO newAccountView(Account acct) { SearchCriteria sc = acctIdSearch.create(); From 7ec0165535771f40f921b4e738cb4511e3331c07 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 8 Nov 2023 14:29:53 +0530 Subject: [PATCH 04/28] Use join instead of views for filtering domains --- .../com/cloud/api/query/QueryManagerImpl.java | 44 ++++++++++----- .../api/query/dao/AccountJoinDaoImpl.java | 20 +++---- .../cloud/api/query/dao/DomainJoinDao.java | 2 + .../api/query/dao/DomainJoinDaoImpl.java | 53 +++++++++++++++++++ 4 files changed, 97 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d1c0820472da..e817f2aeb777 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2587,6 +2587,20 @@ public ListResponse searchForDomains(ListDomainsCmd cmd) { } private Pair, Integer> searchForDomainsInternal(ListDomainsCmd cmd) { + Pair, Integer> domainIdPage = searchForDomainIdsAndCount(cmd); + + Integer count = domainIdPage.second(); + Long[] idArray = domainIdPage.first().toArray(new Long[0]); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List domains = _domainJoinDao.searchByIds(idArray); + return new Pair<>(domains, count); + } + + private Pair, Integer> searchForDomainIdsAndCount(ListDomainsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); Long domainId = cmd.getId(); boolean listAll = cmd.listAll(); @@ -2608,24 +2622,27 @@ private Pair, Integer> searchForDomainsInternal(ListDomainsCm } } - Filter searchFilter = new Filter(DomainJoinVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); + Filter searchFilter = new Filter(DomainVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); String domainName = cmd.getDomainName(); Integer level = cmd.getLevel(); Object keyword = cmd.getKeyword(); - SearchBuilder sb = _domainJoinDao.createSearchBuilder(); - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); - sb.and("level", sb.entity().getLevel(), SearchCriteria.Op.EQ); - sb.and("path", sb.entity().getPath(), SearchCriteria.Op.LIKE); - sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + SearchBuilder domainSearchBuilder = _domainDao.createSearchBuilder(); + domainSearchBuilder.select(null, Func.DISTINCT, domainSearchBuilder.entity().getId()); // select distinct + domainSearchBuilder.and("id", domainSearchBuilder.entity().getId(), SearchCriteria.Op.EQ); + domainSearchBuilder.and("name", domainSearchBuilder.entity().getName(), SearchCriteria.Op.EQ); + domainSearchBuilder.and("level", domainSearchBuilder.entity().getLevel(), SearchCriteria.Op.EQ); + domainSearchBuilder.and("path", domainSearchBuilder.entity().getPath(), SearchCriteria.Op.LIKE); + domainSearchBuilder.and("state", domainSearchBuilder.entity().getState(), SearchCriteria.Op.EQ); + + if (keyword != null) { + domainSearchBuilder.and("keywordName", domainSearchBuilder.entity().getName(), SearchCriteria.Op.LIKE); + } - SearchCriteria sc = sb.create(); + SearchCriteria sc = domainSearchBuilder.create(); if (keyword != null) { - SearchCriteria ssc = _domainJoinDao.createSearchCriteria(); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - sc.addAnd("name", SearchCriteria.Op.SC, ssc); + sc.setParameters("keywordName", "%" + keyword + "%"); } if (domainName != null) { @@ -2650,7 +2667,10 @@ private Pair, Integer> searchForDomainsInternal(ListDomainsCm // return only Active domains to the API sc.setParameters("state", Domain.State.Active); - return _domainJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniqueDomainPair = _domainDao.searchAndCount(sc, searchFilter); + Integer count = uniqueDomainPair.second(); + List domainIds = uniqueDomainPair.first().stream().map(DomainVO::getId).collect(Collectors.toList()); + return new Pair<>(domainIds, count); } @Override diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java index d03b6db9b0d6..2daa411e4fbd 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java @@ -51,15 +51,15 @@ public class AccountJoinDaoImpl extends GenericDaoBase impl @Inject private ConfigurationDao configDao; private final SearchBuilder acctIdSearch; - private final SearchBuilder volSearch; + private final SearchBuilder domainSearch; @Inject AccountManager _acctMgr; protected AccountJoinDaoImpl() { - volSearch = createSearchBuilder(); - volSearch.and("idIN", volSearch.entity().getId(), SearchCriteria.Op.IN); - volSearch.done(); + domainSearch = createSearchBuilder(); + domainSearch.and("idIN", domainSearch.entity().getId(), SearchCriteria.Op.IN); + domainSearch.done(); acctIdSearch = createSearchBuilder(); acctIdSearch.and("id", acctIdSearch.entity().getId(), SearchCriteria.Op.EQ); @@ -249,7 +249,7 @@ public List searchByIds(Long... accountIds) { if (batchCfg != null) { DETAILS_BATCH_SIZE = Integer.parseInt(batchCfg); } - // query details by batches + List uvList = new ArrayList<>(); // query details by batches int curr_index = 0; @@ -259,11 +259,11 @@ public List searchByIds(Long... accountIds) { for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { ids[k] = accountIds[j]; } - SearchCriteria sc = volSearch.create(); + SearchCriteria sc = domainSearch.create(); sc.setParameters("idIN", ids); - List vms = searchIncludingRemoved(sc, null, null, false); - if (vms != null) { - uvList.addAll(vms); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); } curr_index += DETAILS_BATCH_SIZE; } @@ -275,7 +275,7 @@ public List searchByIds(Long... accountIds) { for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { ids[k] = accountIds[j]; } - SearchCriteria sc = volSearch.create(); + SearchCriteria sc = domainSearch.create(); sc.setParameters("idIN", ids); List accounts = searchIncludingRemoved(sc, null, null, false); if (accounts != null) { diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java index 94efc28d2d48..a023371a818b 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java @@ -17,6 +17,7 @@ package com.cloud.api.query.dao; import java.util.EnumSet; +import java.util.List; import org.apache.cloudstack.api.ApiConstants.DomainDetails; import org.apache.cloudstack.api.ResponseObject.ResponseView; @@ -35,4 +36,5 @@ public interface DomainJoinDao extends GenericDao { void setResourceLimits(DomainJoinVO domain, boolean isRootDomain, ResourceLimitAndCountResponse response); + List searchByIds(Long[] domainIds); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java index 56f5417da3fa..42d9f13eb47e 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.api.query.dao; +import java.util.ArrayList; import java.util.EnumSet; import java.util.List; @@ -29,6 +30,7 @@ import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.ResourceLimitAndCountResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -47,10 +49,13 @@ public class DomainJoinDaoImpl extends GenericDaoBase implem public static final Logger s_logger = Logger.getLogger(DomainJoinDaoImpl.class); private SearchBuilder domainIdSearch; + private SearchBuilder domainSearch; @Inject private AnnotationDao annotationDao; @Inject + private ConfigurationDao configDao; + @Inject private AccountManager accountManager; protected DomainJoinDaoImpl() { @@ -59,6 +64,10 @@ protected DomainJoinDaoImpl() { domainIdSearch.and("id", domainIdSearch.entity().getId(), SearchCriteria.Op.EQ); domainIdSearch.done(); + domainSearch = createSearchBuilder(); + domainSearch.and("idIN", domainSearch.entity().getId(), SearchCriteria.Op.IN); + domainSearch.done(); + this._count = "select count(distinct id) from domain_view WHERE "; } @@ -207,6 +216,50 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim response.setSecondaryStorageAvailable(secondaryStorageAvail); } + @Override + public List searchByIds(Long[] domainIds) { + // set detail batch query size + int DETAILS_BATCH_SIZE = 2000; + String batchCfg = configDao.getValue("detail.batch.query.size"); + if (batchCfg != null) { + DETAILS_BATCH_SIZE = Integer.parseInt(batchCfg); + } + + List uvList = new ArrayList<>(); + // query details by batches + int curr_index = 0; + if (domainIds.length > DETAILS_BATCH_SIZE) { + while ((curr_index + DETAILS_BATCH_SIZE) <= domainIds.length) { + Long[] ids = new Long[DETAILS_BATCH_SIZE]; + for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { + ids[k] = domainIds[j]; + } + SearchCriteria sc = domainSearch.create(); + sc.setParameters("idIN", ids); + List domains = searchIncludingRemoved(sc, null, null, false); + if (domains != null) { + uvList.addAll(domains); + } + curr_index += DETAILS_BATCH_SIZE; + } + } + if (curr_index < domainIds.length) { + int batch_size = (domainIds.length - curr_index); + // set the ids value + Long[] ids = new Long[batch_size]; + for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { + ids[k] = domainIds[j]; + } + SearchCriteria sc = domainSearch.create(); + sc.setParameters("idIN", ids); + List domains = searchIncludingRemoved(sc, null, null, false); + if (domains != null) { + uvList.addAll(domains); + } + } + return uvList; + } + @Override public DomainJoinVO newDomainView(Domain domain) { SearchCriteria sc = domainIdSearch.create(); From 1cca89a09bf381dca7e8306fffa3558cdb0fca78 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 8 Nov 2023 16:33:15 +0530 Subject: [PATCH 05/28] Use join instead of views for filtering hosts --- .../com/cloud/api/query/QueryManagerImpl.java | 106 ++++++++++-------- 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index e817f2aeb777..d6603d241188 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -38,6 +38,7 @@ import com.cloud.storage.StoragePoolHostVO; import com.cloud.event.EventVO; import com.cloud.event.dao.EventDao; +import com.cloud.host.HostVO; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplatePoolDao; @@ -165,6 +166,8 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.outofbandmanagement.OutOfBandManagementVO; +import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.secstorage.HeuristicVO; @@ -547,6 +550,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private HostDao hostDao; + @Inject + private OutOfBandManagementDao outOfBandManagementDao; + @Inject private InstanceGroupVMMapDao instanceGroupVMMapDao; @@ -2197,7 +2203,19 @@ public ListResponse searchForServers(ListHostsCmd cmd) { } public Pair, Integer> searchForServersInternal(ListHostsCmd cmd) { + Pair, Integer> serverIdPage = searchForServerIdsAndCount(cmd); + + Integer count = serverIdPage.second(); + Long[] idArray = serverIdPage.first().toArray(new Long[0]); + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List servers = hostJoinDao.searchByIds(idArray); + return new Pair<>(servers, count); + } + public Pair, Integer> searchForServerIdsAndCount(ListHostsCmd cmd) { Long zoneId = accountMgr.checkAccessAndSpecifyAuthority(CallContext.current().getCallingAccount(), cmd.getZoneId()); Object name = cmd.getHostName(); Object type = cmd.getType(); @@ -2214,44 +2232,55 @@ public Pair, Integer> searchForServersInternal(ListHostsCmd cmd Long pageSize = cmd.getPageSizeVal(); Hypervisor.HypervisorType hypervisorType = cmd.getHypervisor(); - Filter searchFilter = new Filter(HostJoinVO.class, "id", Boolean.TRUE, startIndex, pageSize); + Filter searchFilter = new Filter(HostVO.class, "id", Boolean.TRUE, startIndex, pageSize); - SearchBuilder sb = hostJoinDao.createSearchBuilder(); - sb.select(null, Func.DISTINCT, sb.entity().getId()); // select distinct + SearchBuilder hostSearchBuilder = hostDao.createSearchBuilder(); + hostSearchBuilder.select(null, Func.DISTINCT, hostSearchBuilder.entity().getId()); // select distinct // ids - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); - sb.and("type", sb.entity().getType(), SearchCriteria.Op.LIKE); - sb.and("status", sb.entity().getStatus(), SearchCriteria.Op.EQ); - sb.and("dataCenterId", sb.entity().getZoneId(), SearchCriteria.Op.EQ); - sb.and("podId", sb.entity().getPodId(), SearchCriteria.Op.EQ); - sb.and("clusterId", sb.entity().getClusterId(), SearchCriteria.Op.EQ); - sb.and("oobmEnabled", sb.entity().isOutOfBandManagementEnabled(), SearchCriteria.Op.EQ); - sb.and("powerState", sb.entity().getOutOfBandManagementPowerState(), SearchCriteria.Op.EQ); - sb.and("resourceState", sb.entity().getResourceState(), SearchCriteria.Op.EQ); - sb.and("hypervisor_type", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("id", hostSearchBuilder.entity().getId(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("name", hostSearchBuilder.entity().getName(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("type", hostSearchBuilder.entity().getType(), SearchCriteria.Op.LIKE); + hostSearchBuilder.and("status", hostSearchBuilder.entity().getStatus(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("dataCenterId", hostSearchBuilder.entity().getDataCenterId(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("podId", hostSearchBuilder.entity().getPodId(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("clusterId", hostSearchBuilder.entity().getClusterId(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("resourceState", hostSearchBuilder.entity().getResourceState(), SearchCriteria.Op.EQ); + hostSearchBuilder.and("hypervisor_type", hostSearchBuilder.entity().getHypervisorType(), SearchCriteria.Op.EQ); + + if (keyword != null) { + hostSearchBuilder.and().op("keywordName", hostSearchBuilder.entity().getName(), SearchCriteria.Op.LIKE); + hostSearchBuilder.or("keywordStatus", hostSearchBuilder.entity().getStatus(), SearchCriteria.Op.LIKE); + hostSearchBuilder.or("keywordType", hostSearchBuilder.entity().getType(), SearchCriteria.Op.LIKE); + hostSearchBuilder.cp(); + } + + if (outOfBandManagementEnabled != null || powerState != null) { + SearchBuilder oobmSearch = outOfBandManagementDao.createSearchBuilder(); + oobmSearch.and("oobmEnabled", oobmSearch.entity().isEnabled(), SearchCriteria.Op.EQ); + oobmSearch.and("powerState", oobmSearch.entity().getPowerState(), SearchCriteria.Op.EQ); + + hostSearchBuilder.join("oobmSearch", oobmSearch, hostSearchBuilder.entity().getId(), oobmSearch.entity().getHostId(), JoinBuilder.JoinType.INNER); + } String haTag = _haMgr.getHaTag(); if (haHosts != null && haTag != null && !haTag.isEmpty()) { + SearchBuilder hostTagSearchBuilder = _hostTagDao.createSearchBuilder(); if ((Boolean)haHosts) { - sb.and("tag", sb.entity().getTag(), SearchCriteria.Op.EQ); + hostTagSearchBuilder.and("tag", hostTagSearchBuilder.entity().getName(), SearchCriteria.Op.EQ); } else { - sb.and().op("tag", sb.entity().getTag(), SearchCriteria.Op.NEQ); - sb.or("tagNull", sb.entity().getTag(), SearchCriteria.Op.NULL); - sb.cp(); + hostTagSearchBuilder.and().op("tag", hostTagSearchBuilder.entity().getName(), Op.NEQ); + hostTagSearchBuilder.or("tagNull", hostTagSearchBuilder.entity().getName(), Op.NULL); + hostTagSearchBuilder.cp(); } - + hostSearchBuilder.join("hostTagSearch", hostTagSearchBuilder, hostSearchBuilder.entity().getId(), hostTagSearchBuilder.entity().getHostId(), JoinBuilder.JoinType.LEFT); } - SearchCriteria sc = sb.create(); + SearchCriteria sc = hostSearchBuilder.create(); if (keyword != null) { - SearchCriteria ssc = hostJoinDao.createSearchCriteria(); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("status", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("type", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - - sc.addAnd("name", SearchCriteria.Op.SC, ssc); + sc.setParameters("keywordName", "%" + keyword + "%"); + sc.setParameters("keywordStatus", "%" + keyword + "%"); + sc.setParameters("keywordType", "%" + keyword + "%"); } if (id != null) { @@ -2278,11 +2307,11 @@ public Pair, Integer> searchForServersInternal(ListHostsCmd cmd } if (outOfBandManagementEnabled != null) { - sc.setParameters("oobmEnabled", outOfBandManagementEnabled); + sc.setJoinParameters("oobmSearch", "oobmEnabled", outOfBandManagementEnabled); } if (powerState != null) { - sc.setParameters("powerState", powerState); + sc.setJoinParameters("oobmSearch", "powerState", powerState); } if (resourceState != null) { @@ -2290,28 +2319,17 @@ public Pair, Integer> searchForServersInternal(ListHostsCmd cmd } if (haHosts != null && haTag != null && !haTag.isEmpty()) { - sc.setParameters("tag", haTag); + sc.setJoinParameters("hostTagSearch", "tag", haTag); } if (hypervisorType != HypervisorType.None && hypervisorType != HypervisorType.Any) { sc.setParameters("hypervisor_type", hypervisorType); } - // search host details by ids - Pair, Integer> uniqueHostPair = hostJoinDao.searchAndCount(sc, searchFilter); - Integer count = uniqueHostPair.second(); - if (count.intValue() == 0) { - // handle empty result cases - return uniqueHostPair; - } - List uniqueHosts = uniqueHostPair.first(); - Long[] hostIds = new Long[uniqueHosts.size()]; - int i = 0; - for (HostJoinVO v : uniqueHosts) { - hostIds[i++] = v.getId(); - } - List hosts = hostJoinDao.searchByIds(hostIds); - return new Pair, Integer>(hosts, count); + Pair, Integer> uniqueHostPair = hostDao.searchAndCount(sc, searchFilter); + Integer count = uniqueHostPair.second(); + List hostIds = uniqueHostPair.first().stream().map(HostVO::getId).collect(Collectors.toList()); + return new Pair<>(hostIds, count); } @Override From 863cffa03e33b32b636fe29f018328d48114836a Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 9 Nov 2023 11:42:53 +0530 Subject: [PATCH 06/28] Use join instead of views for filtering storage pools --- .../datastore/db/PrimaryDataStoreDao.java | 6 ++ .../datastore/db/PrimaryDataStoreDaoImpl.java | 81 +++++++++++++++++++ .../com/cloud/api/query/QueryManagerImpl.java | 22 ++--- .../api/query/dao/StoragePoolJoinDao.java | 4 +- .../api/query/dao/StoragePoolJoinDaoImpl.java | 4 +- 5 files changed, 98 insertions(+), 19 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java index 8f77b4ba63e9..cf3e35ce4aa1 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java @@ -22,6 +22,8 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.ScopeType; import com.cloud.storage.StoragePoolStatus; +import com.cloud.utils.Pair; +import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDao; /** @@ -133,4 +135,8 @@ public interface PrimaryDataStoreDao extends GenericDao { List findPoolsByStorageType(String storageType); List listStoragePoolsWithActiveVolumesByOfferingId(long offeringid); + + Pair, Integer> searchForIdsAndCount(Long storagePoolId, String storagePoolName, Long zoneId, + String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, + String keyword, Filter searchFilter); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java index af7dbdc02259..3ba24e96774d 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java @@ -22,10 +22,13 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.utils.Pair; +import com.cloud.utils.db.Filter; import org.apache.commons.collections.CollectionUtils; import com.cloud.host.Status; @@ -645,4 +648,82 @@ public List listStoragePoolsWithActiveVolumesByOfferingId(long of throw new CloudRuntimeException("Caught: " + sql, e); } } + + @Override + public Pair, Integer> searchForIdsAndCount(Long storagePoolId, String storagePoolName, Long zoneId, + String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, + String keyword, Filter searchFilter) { + SearchCriteria sc = createStoragePoolSearchCriteria(storagePoolId, storagePoolName, zoneId, path, podId, clusterId, address, scopeType, status, keyword); + Pair, Integer> uniquePair = searchAndCount(sc, searchFilter); + List idList = uniquePair.first().stream().map(StoragePoolVO::getId).collect(Collectors.toList()); + return new Pair<>(idList, uniquePair.second()); + } + + private SearchCriteria createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName, + Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, + StoragePoolStatus status, String keyword) { + SearchBuilder sb = createSearchBuilder(); + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct + // ids + sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); + sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); + sb.and("path", sb.entity().getPath(), SearchCriteria.Op.EQ); + sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ); + sb.and("podId", sb.entity().getPodId(), SearchCriteria.Op.EQ); + sb.and("clusterId", sb.entity().getClusterId(), SearchCriteria.Op.EQ); + sb.and("hostAddress", sb.entity().getHostAddress(), SearchCriteria.Op.EQ); + sb.and("scope", sb.entity().getScope(), SearchCriteria.Op.EQ); + sb.and("status", sb.entity().getStatus(), SearchCriteria.Op.EQ); + sb.and("parent", sb.entity().getParent(), SearchCriteria.Op.EQ); + + SearchCriteria sc = sb.create(); + + if (keyword != null) { + SearchCriteria ssc = createSearchCriteria(); + ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + ssc.addOr("poolType", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + + sc.addAnd("name", SearchCriteria.Op.SC, ssc); + } + + if (storagePoolId != null) { + sc.setParameters("id", storagePoolId); + } + + if (storagePoolName != null) { + sc.setParameters("name", storagePoolName); + } + + if (path != null) { + sc.setParameters("path", path); + } + if (zoneId != null) { + sc.setParameters("dataCenterId", zoneId); + } + if (podId != null) { + SearchCriteria ssc = createSearchCriteria(); + ssc.addOr("podId", SearchCriteria.Op.EQ, podId); + ssc.addOr("podId", SearchCriteria.Op.NULL); + + sc.addAnd("podId", SearchCriteria.Op.SC, ssc); + } + if (address != null) { + sc.setParameters("hostAddress", address); + } + if (clusterId != null) { + SearchCriteria ssc = createSearchCriteria(); + ssc.addOr("clusterId", SearchCriteria.Op.EQ, clusterId); + ssc.addOr("clusterId", SearchCriteria.Op.NULL); + + sc.addAnd("clusterId", SearchCriteria.Op.SC, ssc); + } + if (scopeType != null) { + sc.setParameters("scope", scopeType.toString()); + } + if (status != null) { + sc.setParameters("status", status.toString()); + } + sc.setParameters("parent", 0); + return sc; + } } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d6603d241188..ce1a5225952f 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3015,26 +3015,14 @@ private Pair, Integer> searchForStoragePoolsInternal(Lis Long startIndex = cmd.getStartIndex(); Long pageSize = cmd.getPageSizeVal(); - Filter searchFilter = new Filter(StoragePoolJoinVO.class, "id", Boolean.TRUE, startIndex, pageSize); + Filter searchFilter = new Filter(StoragePoolVO.class, "id", Boolean.TRUE, startIndex, pageSize); - // search & count Pool details by ids - Pair, Integer> uniquePoolPair = _poolJoinDao.searchAndCount(id, name, zoneId, path, pod, + Pair, Integer> uniquePoolPair = storagePoolDao.searchForIdsAndCount(id, name, zoneId, path, pod, cluster, address, scopeType, status, keyword, searchFilter); - Integer count = uniquePoolPair.second(); - if (count.intValue() == 0) { - // empty result - return uniquePoolPair; - } - List uniquePools = uniquePoolPair.first(); - Long[] vrIds = new Long[uniquePools.size()]; - int i = 0; - for (StoragePoolJoinVO v : uniquePools) { - vrIds[i++] = v.getId(); - } - List vrs = _poolJoinDao.searchByIds(vrIds); - return new Pair, Integer>(vrs, count); + List storagePools = _poolJoinDao.searchByIds(uniquePoolPair.first().toArray(new Long[0])); + return new Pair<>(storagePools, uniquePoolPair.second()); } @Override @@ -3685,7 +3673,7 @@ private Pair, Integer> searchForServiceOfferingsInte if (cpuSpeed != null) { SearchCriteria cpuSpeedSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - cpuSpeedSearchCriteria.addOr("speed", Op.NULL); + cpuSpeedSearchCriteria.addOr("speed", Op.NULL); cpuSpeedSearchCriteria.addOr("speed", Op.GTEQ, cpuSpeed); sc.addAnd("cpuspeedconstraints", SearchCriteria.Op.SC, cpuSpeedSearchCriteria); } diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java index 9028f3418b3e..b9860cffd33f 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java @@ -43,7 +43,9 @@ public interface StoragePoolJoinDao extends GenericDao List searchByIds(Long... spIds); - Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword, Filter searchFilter); + Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, Long zoneId, + String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, + String keyword, Filter searchFilter); List findStoragePoolByScopeAndRuleTags(Long datacenterId, Long podId, Long clusterId, ScopeType scopeType, List tags); diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java index e75e86108c70..3b31ac7180cb 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java @@ -312,7 +312,9 @@ public List searchByIds(Long... spIds) { } @Override - public Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword, Filter searchFilter) { + public Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, + Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, + StoragePoolStatus status, String keyword, Filter searchFilter) { SearchCriteria sc = createStoragePoolSearchCriteria(storagePoolId, storagePoolName, zoneId, path, podId, clusterId, address, scopeType, status, keyword); return searchAndCount(sc, searchFilter); } From 6a90395d3f306071d5bb216f768f4776ba1ea146 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 16 Nov 2023 17:59:31 +0530 Subject: [PATCH 07/28] Use join instead of views for filtering service offerings --- .../com/cloud/utils/db/GenericDaoBase.java | 20 +- .../cloud/utils/db/GenericSearchBuilder.java | 22 + .../java/com/cloud/utils/db/JoinBuilder.java | 8 +- .../java/com/cloud/utils/db/SearchBase.java | 39 +- .../com/cloud/utils/db/SearchCriteria.java | 28 +- .../cloud/utils/db/GenericDaoBaseTest.java | 10 +- .../com/cloud/api/query/QueryManagerImpl.java | 446 +++++++++++++----- .../api/query/dao/ServiceOfferingJoinDao.java | 1 + .../query/dao/ServiceOfferingJoinDaoImpl.java | 55 ++- 9 files changed, 504 insertions(+), 125 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 6bf36df09414..e020e8242d57 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1289,7 +1289,7 @@ protected void addJoins(StringBuilder str, Collection> join : joins) { String joinTableName = join.getSecondAttribute().table; - String joinTableAlias = findNextJoinTableName(joinTableName, joinedTableNames); + String joinTableAlias = StringUtils.isNotEmpty(join.getName()) ? join.getName() : findNextJoinTableName(joinTableName, joinedTableNames); StringBuilder onClause = new StringBuilder(); onClause.append(" ") .append(join.getType().getName()) @@ -1312,7 +1312,7 @@ protected void addJoins(StringBuilder str, Collection and(String name, Object field, Op op) { return this; } + public GenericSearchBuilder and(String joinName, String name, Object field, Op op) { + SearchBase join = _joins.get(joinName).getT(); + constructCondition(joinName, name, " AND ", join._specifiedAttrs.get(0), op); + return this; + } + /** * Adds an AND condition. Some prefer this method because it looks like * the actual SQL query. @@ -134,6 +140,12 @@ protected GenericSearchBuilder left(Object field, Op op, String name) { return this; } + protected GenericSearchBuilder left(String joinName, Object field, Op op, String name) { + SearchBase joinSb = _joins.get(joinName).getT(); + constructCondition(joinName, name, " ( ", joinSb._specifiedAttrs.get(0), op); + return this; + } + protected Preset left(Object field, Op op) { Condition condition = constructCondition(UUID.randomUUID().toString(), " ( ", _specifiedAttrs.get(0), op); return new Preset(this, condition); @@ -169,6 +181,10 @@ public GenericSearchBuilder op(String name, Object field, Op op) { return left(field, op, name); } + public GenericSearchBuilder op(String joinName, String name, Object field, Op op) { + return left(joinName, field, op, name); + } + /** * Adds an OR condition to the SearchBuilder. * @@ -182,6 +198,12 @@ public GenericSearchBuilder or(String name, Object field, Op op) { return this; } + public GenericSearchBuilder or(String joinName, String name, Object field, Op op) { + SearchBase join = _joins.get(joinName).getT(); + constructCondition(joinName, name, " OR ", join._specifiedAttrs.get(0), op); + return this; + } + /** * Adds an OR condition * diff --git a/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java b/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java index 60e392170be9..8d83cf3d38ed 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java +++ b/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java @@ -33,17 +33,23 @@ public String getName() { } private final T t; + private final String name; private JoinType type; private Attribute firstAttribute; private Attribute secondAttribute; - public JoinBuilder(T t, Attribute firstAttribute, Attribute secondAttribute, JoinType type) { + public JoinBuilder(String name, T t, Attribute firstAttribute, Attribute secondAttribute, JoinType type) { + this.name = name; this.t = t; this.firstAttribute = firstAttribute; this.secondAttribute = secondAttribute; this.type = type; } + public String getName() { + return name; + } + public T getT() { return t; } diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index 3d41a62b43ca..dbd6084d225a 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -36,6 +36,7 @@ import net.sf.cglib.proxy.Factory; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; +import org.apache.commons.lang3.StringUtils; /** * SearchBase contains the methods that are used to build up search @@ -70,6 +71,14 @@ public abstract class SearchBase, T, K> { init(entityType, resultType); } + public SearchBase getJoinSB(String name) { + JoinBuilder> jb = null; + if (_joins != null) { + jb = _joins.get(name); + } + return jb == null ? null : jb.getT(); + } + protected void init(final Class entityType, final Class resultType) { _dao = (GenericDaoBase)GenericDaoBase.getDao(entityType); if (_dao == null) { @@ -202,7 +211,7 @@ public J join(final String name, final SearchBase builder, final Object assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; assert builder != this : "You can't add yourself, can you? Really think about it!"; - final JoinBuilder> t = new JoinBuilder>(builder, _specifiedAttrs.get(0), builder._specifiedAttrs.get(0), joinType); + final JoinBuilder> t = new JoinBuilder>(name, builder, _specifiedAttrs.get(0), builder._specifiedAttrs.get(0), joinType); if (_joins == null) { _joins = new HashMap>>(); } @@ -242,17 +251,26 @@ protected List getSpecifiedAttributes() { return _specifiedAttrs; } - protected Condition constructCondition(final String conditionName, final String cond, final Attribute attr, final Op op) { + protected Condition constructCondition(final String joinName, final String conditionName, final String cond, final Attribute attr, final Op op) { assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; assert op == null || _specifiedAttrs.size() == 1 : "You didn't select the attribute."; assert op != Op.SC : "Call join"; final Condition condition = new Condition(conditionName, cond, attr, op); + if (StringUtils.isNotEmpty(joinName)) { + condition.setJoinName(joinName); + } _conditions.add(condition); _specifiedAttrs.clear(); return condition; } + + protected Condition constructCondition(final String conditionName, final String cond, final Attribute attr, final Op op) { + return constructCondition(null, conditionName, cond, attr, op); + } + + /** * creates the SearchCriteria so the actual values can be filled in. * @@ -364,6 +382,7 @@ protected synchronized void finalize() { protected static class Condition { protected final String name; protected final String cond; + protected String joinName; protected final Op op; protected final Attribute attr; protected Object[] presets; @@ -388,11 +407,25 @@ public void setPresets(final Object... presets) { this.presets = presets; } + public void setJoinName(final String joinName) { + this.joinName = joinName; + } + public Object[] getPresets() { return presets; } public void toSql(final StringBuilder sql, final Object[] params, final int count) { + String tableAlias = null; + if (joinName != null) { + tableAlias = joinName; + } else if (attr != null) { + tableAlias = attr.table; + } + toSql(sql, tableAlias, params, count); + } + + public void toSql(final StringBuilder sql, final String tableAlias, final Object[] params, final int count) { if (count > 0) { sql.append(cond); } @@ -414,7 +447,7 @@ public void toSql(final StringBuilder sql, final Object[] params, final int coun sql.append(" FIND_IN_SET(?, "); } - sql.append(attr.table).append(".").append(attr.columnName).append(op.toString()); + sql.append(tableAlias).append(".").append(attr.columnName).append(op.toString()); if (op == Op.IN && params.length == 1) { sql.delete(sql.length() - op.toString().length(), sql.length()); sql.append("=?"); diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java index 6f90d2391e64..9467a476bd2a 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java @@ -106,7 +106,7 @@ protected SearchCriteria(SearchBase sb) { for (Map.Entry>> entry : sb._joins.entrySet()) { JoinBuilder> value = entry.getValue(); _joins.put(entry.getKey(), - new JoinBuilder>(value.getT().create(), value.getFirstAttribute(), value.getSecondAttribute(), value.getType())); + new JoinBuilder>(entry.getKey(), value.getT().create(), value.getFirstAttribute(), value.getSecondAttribute(), value.getType())); } } _selects = sb._selects; @@ -250,6 +250,32 @@ protected void addCondition(String conditionName, String cond, Attribute attr, O _additionals.add(condition); } + public String getWhereClause(String tableAlias) { + StringBuilder sql = new StringBuilder(); + int i = 0; + for (Condition condition : _conditions) { + if (condition.isPreset()) { + _params.put(condition.name, condition.presets); + } + Object[] params = _params.get(condition.name); + if ((condition.op == null || condition.op.params == 0) || (params != null)) { + condition.toSql(sql, tableAlias, params, i++); + } + } + + for (Condition condition : _additionals) { + if (condition.isPreset()) { + _params.put(condition.name, condition.presets); + } + Object[] params = _params.get(condition.name); + if ((condition.op.params == 0) || (params != null)) { + condition.toSql(sql, tableAlias, params, i++); + } + } + + return sql.toString(); + } + public String getWhereClause() { StringBuilder sql = new StringBuilder(); int i = 0; diff --git a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java index b950501337b3..3853569af36a 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java @@ -230,8 +230,8 @@ public void addJoinsTest() { Attribute attr3 = new Attribute("table3", "column1"); Attribute attr4 = new Attribute("table4", "column2"); - joins.add(new JoinBuilder<>(dbTestDao.createSearchCriteria(), attr1, attr2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>(dbTestDao.createSearchCriteria(), attr3, attr4, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), attr1, attr2, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), attr3, attr4, JoinBuilder.JoinType.INNER)); dbTestDao.addJoins(joinString, joins); Assert.assertEquals(" INNER JOIN table2 ON table1.column1=table2.column2 INNER JOIN table4 ON table3.column1=table4.column2 ", joinString.toString()); @@ -249,9 +249,9 @@ public void multiJoinSameTableTest() { Attribute tCc3 = new Attribute("tableC", "column3"); Attribute tDc4 = new Attribute("tableD", "column4"); - joins.add(new JoinBuilder<>(dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>(dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>(dbTestDao.createSearchCriteria(), tDc4, tAc3, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tDc4, tAc3, JoinBuilder.JoinType.INNER)); dbTestDao.addJoins(joinString, joins); Assert.assertEquals(" INNER JOIN tableA ON tableB.column2=tableA.column1 INNER JOIN tableA tableA1 ON tableC.column3=tableA1.column2 INNER JOIN tableA tableA2 ON tableD.column4=tableA2.column3 ", joinString.toString()); diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index ce1a5225952f..7cc1b2ea7f57 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -39,6 +39,8 @@ import com.cloud.event.EventVO; import com.cloud.event.dao.EventDao; import com.cloud.host.HostVO; +import com.cloud.offering.ServiceOffering; +import com.cloud.service.ServiceOfferingDetailsVO; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplatePoolDao; @@ -262,7 +264,6 @@ import com.cloud.network.security.SecurityGroupVMMapVO; import com.cloud.network.security.dao.SecurityGroupVMMapDao; import com.cloud.offering.DiskOffering; -import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.projects.Project; import com.cloud.projects.Project.ListProjectResourcesCriteria; @@ -295,7 +296,6 @@ import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiServiceImpl; -import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.BucketDao; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.StoragePoolTagsDao; @@ -3475,6 +3475,20 @@ public ListResponse searchForServiceOfferings(ListServi } private Pair, Integer> searchForServiceOfferingsInternal(ListServiceOfferingsCmd cmd) { + Pair, Integer> offeringIdPage = searchForServiceOfferingIdsAndCount(cmd); + + Integer count = offeringIdPage.second(); + Long[] idArray = offeringIdPage.first().toArray(new Long[0]); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List srvOfferings = _srvOfferingJoinDao.searchByIds(idArray); + return new Pair<>(srvOfferings, count); + } + + private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServiceOfferingsCmd cmd) { // Note // The filteredOfferings method for offerings is being modified in accordance with // discussion with Will/Kevin @@ -3484,9 +3498,6 @@ private Pair, Integer> searchForServiceOfferingsInte // their domains+parent domains ... all the way // till // root - Filter searchFilter = new Filter(ServiceOfferingJoinVO.class, "sortKey", SortKeyAscending.value(), cmd.getStartIndex(), cmd.getPageSizeVal()); - searchFilter.addOrderBy(ServiceOfferingJoinVO.class, "id", true); - Account caller = CallContext.current().getCallingAccount(); Long projectId = cmd.getProjectId(); String accountName = cmd.getAccountName(); @@ -3498,6 +3509,7 @@ private Pair, Integer> searchForServiceOfferingsInte Boolean isSystem = cmd.getIsSystem(); String vmTypeStr = cmd.getSystemVmType(); ServiceOfferingVO currentVmOffering = null; + DiskOfferingVO diskOffering = null; Boolean isRecursive = cmd.isRecursive(); Long zoneId = cmd.getZoneId(); Integer cpuNumber = cmd.getCpuNumber(); @@ -3507,9 +3519,9 @@ private Pair, Integer> searchForServiceOfferingsInte String storageType = cmd.getStorageType(); final Account owner = accountMgr.finalizeOwner(caller, accountName, domainId, projectId); - SearchCriteria sc = _srvOfferingJoinDao.createSearchCriteria(); + if (!accountMgr.isRootAdmin(caller.getId()) && isSystem) { - throw new InvalidParameterValueException("Only ROOT admins can access system's offering"); + throw new InvalidParameterValueException("Only ROOT admins can access system offerings."); } // Keeping this logic consistent with domain specific zones @@ -3523,34 +3535,37 @@ private Pair, Integer> searchForServiceOfferingsInte } } + VMInstanceVO vmInstance = null; if (vmId != null) { - VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId); + vmInstance = _vmInstanceDao.findById(vmId); if ((vmInstance == null) || (vmInstance.getRemoved() != null)) { InvalidParameterValueException ex = new InvalidParameterValueException("unable to find a virtual machine with specified id"); ex.addProxyObject(vmId.toString(), "vmId"); throw ex; } + accountMgr.checkAccess(owner, null, true, vmInstance); + } + + Filter searchFilter = new Filter(ServiceOfferingVO.class, "sortKey", SortKeyAscending.value(), cmd.getStartIndex(), cmd.getPageSizeVal()); + searchFilter.addOrderBy(ServiceOfferingVO.class, "id", true); - accountMgr.checkAccess(caller, null, true, vmInstance); + SearchBuilder serviceOfferingSearch = _srvOfferingDao.createSearchBuilder(); + serviceOfferingSearch.select(null, Func.DISTINCT, serviceOfferingSearch.entity().getId()); // select distinct + serviceOfferingSearch.and("activeState", serviceOfferingSearch.entity().getState(), Op.EQ); + if (vmId != null) { currentVmOffering = _srvOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); - if (! currentVmOffering.isDynamic()) { - sc.addAnd("id", SearchCriteria.Op.NEQ, currentVmOffering.getId()); + diskOffering = _diskOfferingDao.findByIdIncludingRemoved(currentVmOffering.getDiskOfferingId()); + if (!currentVmOffering.isDynamic()) { + serviceOfferingSearch.and("idNEQ", serviceOfferingSearch.entity().getId(), SearchCriteria.Op.NEQ); } if (currentVmOffering.getDiskOfferingStrictness()) { - sc.addAnd("diskOfferingId", Op.EQ, currentVmOffering.getDiskOfferingId()); - sc.addAnd("diskOfferingStrictness", Op.EQ, true); - } else { - sc.addAnd("diskOfferingStrictness", Op.EQ, false); + serviceOfferingSearch.and("diskOfferingId", serviceOfferingSearch.entity().getDiskOfferingId(), SearchCriteria.Op.EQ); } + serviceOfferingSearch.and("diskOfferingStrictness", serviceOfferingSearch.entity().getDiskOfferingStrictness(), SearchCriteria.Op.EQ); - boolean isRootVolumeUsingLocalStorage = virtualMachineManager.isRootVolumeOnLocalStorage(vmId); - - // 1. Only return offerings with the same storage type than the storage pool where the VM's root volume is allocated - sc.addAnd("useLocalStorage", SearchCriteria.Op.EQ, isRootVolumeUsingLocalStorage); - - // 2.In case vm is running return only offerings greater than equal to current offering compute and offering's dynamic scalability should match + // In case vm is running return only offerings greater than equal to current offering compute and offering's dynamic scalability should match if (vmInstance.getState() == VirtualMachine.State.Running) { Integer vmCpu = currentVmOffering.getCpu(); Integer vmMemory = currentVmOffering.getRamSize(); @@ -3566,20 +3581,64 @@ private Pair, Integer> searchForServiceOfferingsInte vmMemory = NumbersUtil.parseInt(details.get(ApiConstants.MEMORY), 0); } if (vmCpu != null && vmCpu > 0) { - sc.addAnd("cpu", Op.SC, getMinimumCpuServiceOfferingJoinSearchCriteria(vmCpu)); + /* + (service_offering.cpu >= ?) + OR ( + service_offering.cpu IS NULL + AND ( + maxComputeDetailsSearch.value IS NULL OR maxComputeDetailsSearch.value >= ? + ) + ) + */ + SearchBuilder maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + maxComputeDetailsSearch.and().op("name", maxComputeDetailsSearch.entity().getName(), Op.EQ); + maxComputeDetailsSearch.or("idNull", maxComputeDetailsSearch.entity().getId(), Op.NULL); + maxComputeDetailsSearch.cp(); + + serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, + serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + + serviceOfferingSearch.and().op("vmCpu", serviceOfferingSearch.entity().getCpu(), Op.GTEQ); + serviceOfferingSearch.or().op("vmCpuNull", serviceOfferingSearch.entity().getCpu(), Op.NULL); + serviceOfferingSearch.and().op("maxComputeDetailsSearch", "vmMaxComputeNull", maxComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.or("maxComputeDetailsSearch", "vmMaxComputeGTEQ", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + + serviceOfferingSearch.cp().cp(); + } if (vmSpeed != null && vmSpeed > 0) { - sc.addAnd("speed", Op.SC, getMinimumCpuSpeedServiceOfferingJoinSearchCriteria(vmSpeed)); + serviceOfferingSearch.and().op("speedNULL", serviceOfferingSearch.entity().getSpeed(), Op.NULL); + serviceOfferingSearch.or("speedGTEQ", serviceOfferingSearch.entity().getSpeed(), Op.GTEQ); + serviceOfferingSearch.cp(); } if (vmMemory != null && vmMemory > 0) { - sc.addAnd("ramSize", Op.SC, getMinimumMemoryServiceOfferingJoinSearchCriteria(vmMemory)); + /* + (service_offering.ram_size >= ?) + OR ( + service_offering.ram_size IS NULL + AND (max_memory_details.value IS NULL OR max_memory_details.value >= ?) + ) + */ + SearchBuilder maxMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + maxMemoryDetailsSearch.and().op("name", maxMemoryDetailsSearch.entity().getName(), Op.EQ); + maxMemoryDetailsSearch.or("idNull", maxMemoryDetailsSearch.entity().getId(), Op.NULL); + maxMemoryDetailsSearch.cp(); + + serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, + serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + + serviceOfferingSearch.and().op("vmMemory", serviceOfferingSearch.entity().getRamSize(), Op.GTEQ); + serviceOfferingSearch.or().op("vmMemoryNull", serviceOfferingSearch.entity().getRamSize(), Op.NULL); + serviceOfferingSearch.and().op("maxMemoryDetailsSearch", "vmMaxMemoryNull", maxMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("maxMemoryDetailsSearch", "vmMaxMemoryGTEQ", maxMemoryDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + + serviceOfferingSearch.cp().cp(); } - sc.addAnd("dynamicScalingEnabled", Op.EQ, currentVmOffering.isDynamicScalingEnabled()); + serviceOfferingSearch.and("dynamicScalingEnabled", serviceOfferingSearch.entity().isDynamicScalingEnabled(), SearchCriteria.Op.EQ); } } - // boolean includePublicOfferings = false; - if ((accountMgr.isNormalUser(caller.getId()) || accountMgr.isDomainAdmin(caller.getId())) || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { + if ((accountMgr.isNormalUser(owner.getId()) || accountMgr.isDomainAdmin(owner.getId())) || owner.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { // For non-root users. if (isSystem) { throw new InvalidParameterValueException("Only root admins can access system's offering"); @@ -3592,90 +3651,284 @@ private Pair, Integer> searchForServiceOfferingsInte } else { // for root users if (owner.getDomainId() != 1 && isSystem) { // NON ROOT admin - throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering."); + throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering"); } if (domainId != null && accountName == null) { - sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId)); + SearchBuilder srvOffrDomainDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); + srvOffrDomainDetailSearch.and("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); + srvOffrDomainDetailSearch.and("value", srvOffrDomainDetailSearch.entity().getValue(), Op.EQ); + serviceOfferingSearch.join("domainDetailSearch", srvOffrDomainDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); } } if (keyword != null) { - SearchCriteria ssc = _srvOfferingJoinDao.createSearchCriteria(); - ssc.addOr("displayText", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + serviceOfferingSearch.and().op("keywordName", serviceOfferingSearch.entity().getName(), SearchCriteria.Op.LIKE); + serviceOfferingSearch.or("keywordDisplayText", serviceOfferingSearch.entity().getDisplayText(), SearchCriteria.Op.LIKE); + serviceOfferingSearch.cp(); + } - sc.addAnd("name", SearchCriteria.Op.SC, ssc); + if (id != null) { + serviceOfferingSearch.and("id", serviceOfferingSearch.entity().getId(), SearchCriteria.Op.EQ); + } + + if (isSystem != null) { + // note that for non-root users, isSystem is always false when + // control comes to here + serviceOfferingSearch.and("systemUse", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.EQ); + } + + if (name != null) { + serviceOfferingSearch.and("name", serviceOfferingSearch.entity().getName(), SearchCriteria.Op.EQ); + } + + if (vmTypeStr != null) { + serviceOfferingSearch.and("vmType", serviceOfferingSearch.entity().getSystemVmType(), SearchCriteria.Op.EQ); + } + DataCenterJoinVO zone = null; + if (zoneId != null) { + SearchBuilder srvOffrZoneDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); + srvOffrZoneDetailSearch.and("name", srvOffrZoneDetailSearch.entity().getName(), Op.EQ); + srvOffrZoneDetailSearch.and().op("value", srvOffrZoneDetailSearch.entity().getValue(), Op.EQ); + srvOffrZoneDetailSearch.and().or("valueNull", srvOffrZoneDetailSearch.entity().getValue(), Op.NULL); + srvOffrZoneDetailSearch.cp(); + serviceOfferingSearch.join("ZoneDetailSearch", srvOffrZoneDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrZoneDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + zone = _dcJoinDao.findById(zoneId); + } + + if (encryptRoot != null || vmId != null || (zone != null && DataCenter.Type.Edge.equals(zone.getType()))) { + SearchBuilder diskOfferingSearch = _diskOfferingDao.createSearchBuilder(); + diskOfferingSearch.and("useLocalStorage", diskOfferingSearch.entity().isUseLocalStorage(), SearchCriteria.Op.EQ); + diskOfferingSearch.and("encrypt", diskOfferingSearch.entity().getEncrypt(), SearchCriteria.Op.EQ); + + if (diskOffering != null) { + List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); + if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { + for (String tag : storageTags) { + diskOfferingSearch.and(tag, diskOfferingSearch.entity().getTags(), Op.FIND_IN_SET); + } + diskOfferingSearch.done(); + } + } + + serviceOfferingSearch.join("diskOfferingSearch", diskOfferingSearch, serviceOfferingSearch.entity().getDiskOfferingId(), diskOfferingSearch.entity().getId(), JoinBuilder.JoinType.INNER); + } + + if (cpuNumber != null) { + SearchBuilder maxComputeDetailsSearch = (SearchBuilder) serviceOfferingSearch.getJoinSB("maxComputeDetailsSearch"); + if (maxComputeDetailsSearch == null) { + maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + maxComputeDetailsSearch.and().op("name", maxComputeDetailsSearch.entity().getName(), Op.EQ); + maxComputeDetailsSearch.or("idNull", maxComputeDetailsSearch.entity().getId(), Op.NULL); + maxComputeDetailsSearch.cp(); + serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, + serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + } + + SearchBuilder minComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + minComputeDetailsSearch.and().op("name", minComputeDetailsSearch.entity().getName(), Op.EQ); + minComputeDetailsSearch.or("idNull", minComputeDetailsSearch.entity().getId(), Op.NULL); + minComputeDetailsSearch.cp(); + + serviceOfferingSearch.join("minComputeDetailsSearch", minComputeDetailsSearch, + serviceOfferingSearch.entity().getId(), minComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + + /* + (min_cpu IS NULL AND cpu IS NULL AND max_cpu IS NULL) + OR (cpu = X) + OR (min_cpu <= X AND max_cpu >= X) + */ + serviceOfferingSearch.and().op().op("minComputeDetailsSearch", "cpuConstraintMinComputeNull", minComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("maxComputeDetailsSearch", "cpuConstraintMaxComputeNull", maxComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("cpuConstraintNull", serviceOfferingSearch.entity().getCpu(), Op.NULL).cp(); + + serviceOfferingSearch.or("cpuConstraintEQ", serviceOfferingSearch.entity().getCpu(), Op.EQ); + + serviceOfferingSearch.or().op("minComputeDetailsSearch", "cpuConstraintMinComputeLTEQ", minComputeDetailsSearch.entity().getValue(), Op.LTEQ); + serviceOfferingSearch.and("maxComputeDetailsSearch", "cpuConstraintMaxComputeGTEQ", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + serviceOfferingSearch.cp(); + } + + if (memory != null) { + SearchBuilder maxMemoryDetailsSearch = (SearchBuilder) serviceOfferingSearch.getJoinSB("maxMemoryDetailsSearch"); + if (maxMemoryDetailsSearch == null) { + maxMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + maxMemoryDetailsSearch.and().op("name", maxMemoryDetailsSearch.entity().getName(), Op.EQ); + maxMemoryDetailsSearch.or("idNull", maxMemoryDetailsSearch.entity().getId(), Op.NULL); + maxMemoryDetailsSearch.cp(); + serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, + serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + } + + SearchBuilder minMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); + minMemoryDetailsSearch.and().op("name", minMemoryDetailsSearch.entity().getName(), Op.EQ); + minMemoryDetailsSearch.or("idNull", minMemoryDetailsSearch.entity().getId(), Op.NULL); + minMemoryDetailsSearch.cp(); + + serviceOfferingSearch.join("minMemoryDetailsSearch", minMemoryDetailsSearch, + serviceOfferingSearch.entity().getId(), minMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + + /* + (min_ram_size IS NULL AND ram_size IS NULL AND max_ram_size IS NULL) + OR (ram_size = X) + OR (min_ram_size <= X AND max_ram_size >= X) + */ + serviceOfferingSearch.and().op().op("minMemoryDetailsSearch", "memoryConstraintMinMemoryNull", minMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("maxMemoryDetailsSearch", "memoryConstraintMaxMemoryNull", maxMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("memoryConstraintNull", serviceOfferingSearch.entity().getRamSize(), Op.NULL).cp(); + + serviceOfferingSearch.or("memoryConstraintEQ", serviceOfferingSearch.entity().getRamSize(), Op.EQ); + + serviceOfferingSearch.or().op("minMemoryDetailsSearch", "memoryConstraintMinMemoryLTEQ", minMemoryDetailsSearch.entity().getValue(), Op.LTEQ); + serviceOfferingSearch.and("maxMemoryDetailsSearch", "memoryConstraintMaxMemoryGTEQ", maxMemoryDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + serviceOfferingSearch.cp(); + } + + if (cpuSpeed != null) { + serviceOfferingSearch.and().op("speedNull", serviceOfferingSearch.entity().getSpeed(), Op.NULL); + serviceOfferingSearch.or("speedGTEQ", serviceOfferingSearch.entity().getSpeed(), Op.GTEQ); + serviceOfferingSearch.cp(); + } + + // Filter offerings that are not associated with caller's domain + // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! + if (owner.getType() != Account.Type.ADMIN) { + SearchBuilder srvOffrDomainDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); + srvOffrDomainDetailSearch.and("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); + srvOffrDomainDetailSearch.and().op("value", srvOffrDomainDetailSearch.entity().getValue(), Op.IN); + srvOffrDomainDetailSearch.or("value", srvOffrDomainDetailSearch.entity().getValue(), Op.NULL); + srvOffrDomainDetailSearch.cp().done(); + serviceOfferingSearch.join("domainDetailSearchNormalUser", srvOffrDomainDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + } + + if (currentVmOffering != null) { + List hostTags = com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()); + if (!hostTags.isEmpty()) { + + serviceOfferingSearch.and().op("hostTag", serviceOfferingSearch.entity().getHostTag(), Op.NULL); + serviceOfferingSearch.or().op(); + + for(String tag : hostTags) { + serviceOfferingSearch.and(tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET); + } + serviceOfferingSearch.cp().cp().done(); + } + } + + SearchCriteria sc = serviceOfferingSearch.create(); + sc.setParameters("activeState", ServiceOffering.State.Active); + + if (vmId != null) { + if (!currentVmOffering.isDynamic()) { + sc.setParameters("idNEQ", currentVmOffering.getId()); + } + + if (currentVmOffering.getDiskOfferingStrictness()) { + sc.setParameters("diskOfferingId", currentVmOffering.getDiskOfferingId()); + sc.setParameters("diskOfferingStrictness", true); + } else { + sc.setParameters("diskOfferingStrictness", false); + } + + boolean isRootVolumeUsingLocalStorage = virtualMachineManager.isRootVolumeOnLocalStorage(vmId); + + // 1. Only return offerings with the same storage type than the storage pool where the VM's root volume is allocated + sc.setJoinParameters("diskOfferingSearch", "useLocalStorage", isRootVolumeUsingLocalStorage); + + // 2.In case vm is running return only offerings greater than equal to current offering compute and offering's dynamic scalability should match + if (vmInstance.getState() == VirtualMachine.State.Running) { + Integer vmCpu = currentVmOffering.getCpu(); + Integer vmMemory = currentVmOffering.getRamSize(); + Integer vmSpeed = currentVmOffering.getSpeed(); + if ((vmCpu == null || vmMemory == null || vmSpeed == null) && VirtualMachine.Type.User.equals(vmInstance.getType())) { + UserVmVO userVmVO = userVmDao.findById(vmId); + userVmDao.loadDetails(userVmVO); + Map details = userVmVO.getDetails(); + vmCpu = NumbersUtil.parseInt(details.get(ApiConstants.CPU_NUMBER), 0); + if (vmSpeed == null) { + vmSpeed = NumbersUtil.parseInt(details.get(ApiConstants.CPU_SPEED), 0); + } + vmMemory = NumbersUtil.parseInt(details.get(ApiConstants.MEMORY), 0); + } + if (vmCpu != null && vmCpu > 0) { + sc.setJoinParameters("maxComputeDetailsSearch", "name", "maxcpunumber"); + sc.setParameters("vmCpu", vmCpu); + sc.setParameters("vmMaxComputeGTEQ", vmCpu); + } + if (vmSpeed != null && vmSpeed > 0) { + sc.setParameters("speedGTEQ", vmSpeed); + } + if (vmMemory != null && vmMemory > 0) { + sc.setJoinParameters("maxMemoryDetailsSearch", "name", "maxmemory"); + sc.setParameters("vmMemory", vmMemory); + sc.setParameters("vmMaxMemoryGTEQ", vmMemory); + } + sc.setParameters("dynamicScalingEnabled", currentVmOffering.isDynamicScalingEnabled()); + } + } + + if ((!accountMgr.isNormalUser(caller.getId()) && !accountMgr.isDomainAdmin(caller.getId())) && caller.getType() != Account.Type.RESOURCE_DOMAIN_ADMIN) { + if (domainId != null && accountName == null) { + sc.setParameters("domainDetailSearch", "name", "domainid"); + sc.setParameters("domainDetailSearch", "value", domainId); + } + } + + if (keyword != null) { + sc.setParameters("keywordName", "%" + keyword + "%"); + sc.setParameters("keywordDisplayText", "%" + keyword + "%"); } if (id != null) { - sc.addAnd("id", SearchCriteria.Op.EQ, id); + sc.setParameters("id", id); } if (isSystem != null) { // note that for non-root users, isSystem is always false when // control comes to here - sc.addAnd("systemUse", SearchCriteria.Op.EQ, isSystem); + sc.setParameters("systemUse", isSystem); } if (encryptRoot != null) { - sc.addAnd("encryptRoot", SearchCriteria.Op.EQ, encryptRoot); + sc.setJoinParameters("diskOfferingSearch", "encrypt", encryptRoot); } if (name != null) { - sc.addAnd("name", SearchCriteria.Op.EQ, name); + sc.setParameters("name", name); } if (vmTypeStr != null) { - sc.addAnd("vmType", SearchCriteria.Op.EQ, vmTypeStr); + sc.setParameters("vmType", vmTypeStr); } useStorageType(sc, storageType); if (zoneId != null) { - SearchBuilder sb = _srvOfferingJoinDao.createSearchBuilder(); - sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); - sb.or("zId", sb.entity().getZoneId(), Op.NULL); - sb.done(); - SearchCriteria zoneSC = sb.create(); - zoneSC.setParameters("zoneId", String.valueOf(zoneId)); - sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC); - DataCenterJoinVO zone = _dcJoinDao.findById(zoneId); + sc.setJoinParameters("ZoneDetailSearch", "name", "zoneid"); + sc.setJoinParameters("ZoneDetailSearch", "value", zoneId); + if (DataCenter.Type.Edge.equals(zone.getType())) { - sc.addAnd("useLocalStorage", Op.EQ, true); + sc.setJoinParameters("diskOfferingSearch", "useLocalStorage", true); } } if (cpuNumber != null) { - SearchCriteria cpuConstraintSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - cpuConstraintSearchCriteria.addAnd("minCpu", Op.LTEQ, cpuNumber); - cpuConstraintSearchCriteria.addAnd("maxCpu", Op.GTEQ, cpuNumber); - - SearchCriteria cpuSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - cpuSearchCriteria.addOr("minCpu", Op.NULL); - cpuSearchCriteria.addOr("constraints", Op.SC, cpuConstraintSearchCriteria); - cpuSearchCriteria.addOr("minCpu", Op.GTEQ, cpuNumber); - - sc.addAnd("cpuConstraints", SearchCriteria.Op.SC, cpuSearchCriteria); + sc.setJoinParameters("maxComputeDetailsSearch", "name", "maxcpunumber"); + sc.setJoinParameters("minComputeDetailsSearch", "name", "mincpunumber"); + sc.setParameters("cpuConstraintEQ", cpuNumber); + sc.setParameters("cpuConstraintMinComputeLTEQ", cpuNumber); + sc.setParameters("cpuConstraintMaxComputeGTEQ", cpuNumber); } if (memory != null) { - SearchCriteria memoryConstraintSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - memoryConstraintSearchCriteria.addAnd("minMemory", Op.LTEQ, memory); - memoryConstraintSearchCriteria.addAnd("maxMemory", Op.GTEQ, memory); - - SearchCriteria memSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - memSearchCriteria.addOr("minMemory", Op.NULL); - memSearchCriteria.addOr("memconstraints", Op.SC, memoryConstraintSearchCriteria); - memSearchCriteria.addOr("minMemory", Op.GTEQ, memory); - - sc.addAnd("memoryConstraints", SearchCriteria.Op.SC, memSearchCriteria); + sc.setJoinParameters("maxMemoryDetailsSearch", "name", "maxmemory"); + sc.setJoinParameters("minMemoryDetailsSearch", "name", "minmemory"); + sc.setParameters("memoryConstraintEQ", memory); + sc.setParameters("memoryConstraintMinMemoryLTEQ", memory); + sc.setParameters("memoryConstraintMaxMemoryGTEQ", memory); } if (cpuSpeed != null) { - SearchCriteria cpuSpeedSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - cpuSpeedSearchCriteria.addOr("speed", Op.NULL); - cpuSpeedSearchCriteria.addOr("speed", Op.GTEQ, cpuSpeed); - sc.addAnd("cpuspeedconstraints", SearchCriteria.Op.SC, cpuSpeedSearchCriteria); + sc.setParameters("speedGTEQ", cpuSpeed); } // Filter offerings that are not associated with caller's domain @@ -3685,59 +3938,34 @@ private Pair, Integer> searchForServiceOfferingsInte List domainIds = findRelatedDomainIds(callerDomain, isRecursive); List ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds); - SearchBuilder sb = _srvOfferingJoinDao.createSearchBuilder(); - if (ids != null && !ids.isEmpty()) { - sb.and("id", sb.entity().getId(), Op.IN); - } - sb.or("domainId", sb.entity().getDomainId(), Op.NULL); - sb.done(); - SearchCriteria scc = sb.create(); - if (ids != null && !ids.isEmpty()) { - scc.setParameters("id", ids.toArray()); - } - sc.addAnd("domainId", SearchCriteria.Op.SC, scc); + sc.setJoinParameters("domainDetailSearchNormalUser", "name", "domainid"); + sc.setJoinParameters("domainDetailSearchNormalUser", "value", ids.toArray()); } if (currentVmOffering != null) { - DiskOfferingVO diskOffering = _diskOfferingDao.findByIdIncludingRemoved(currentVmOffering.getDiskOfferingId()); - List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); - if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { - SearchBuilder sb = _srvOfferingJoinDao.createSearchBuilder(); - for(String tag : storageTags) { - sb.and(tag, sb.entity().getTags(), Op.FIND_IN_SET); - } - sb.done(); - SearchCriteria scc = sb.create(); - for(String tag : storageTags) { - scc.setParameters(tag, tag); + if (diskOffering != null) { + List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); + if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { + for(String tag : storageTags) { + sc.setJoinParameters("diskOfferingSearch", tag, tag); + } } - sc.addAnd("storageTags", SearchCriteria.Op.SC, scc); } List hostTags = com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()); if (!hostTags.isEmpty()) { - SearchBuilder hostTagsSearchBuilder = _srvOfferingJoinDao.createSearchBuilder(); for(String tag : hostTags) { - hostTagsSearchBuilder.and(tag, hostTagsSearchBuilder.entity().getHostTag(), Op.FIND_IN_SET); + sc.setParameters(tag, tag); } - hostTagsSearchBuilder.done(); - - SearchCriteria hostTagsSearchCriteria = hostTagsSearchBuilder.create(); - for(String tag : hostTags) { - hostTagsSearchCriteria.setParameters(tag, tag); - } - - SearchCriteria finalHostTagsSearchCriteria = _srvOfferingJoinDao.createSearchCriteria(); - finalHostTagsSearchCriteria.addOr("hostTag", Op.NULL); - finalHostTagsSearchCriteria.addOr("hostTag", Op.SC, hostTagsSearchCriteria); - - sc.addAnd("hostTagsConstraint", SearchCriteria.Op.SC, finalHostTagsSearchCriteria); } } - return _srvOfferingJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> uniquePair = _srvOfferingDao.searchAndCount(sc, searchFilter); + Integer count = uniquePair.second(); + List offeringIds = uniquePair.first().stream().map(ServiceOfferingVO::getId).collect(Collectors.toList()); + return new Pair<>(offeringIds, count); } @Override diff --git a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDao.java index 973e4017529b..d28b9fc7e88f 100644 --- a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDao.java @@ -34,4 +34,5 @@ public interface ServiceOfferingJoinDao extends GenericDao> listDomainsOfServiceOfferingsUsedByDomainPath(String domainPath); + List searchByIds(Long... id); } diff --git a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java index 4a81cc8bfeec..92175568cd9a 100644 --- a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java @@ -21,6 +21,7 @@ import java.sql.SQLException; import java.util.Arrays; import java.util.HashMap; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -34,6 +35,7 @@ import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.response.ServiceOfferingResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -56,10 +58,14 @@ public class ServiceOfferingJoinDaoImpl extends GenericDaoBase sofIdSearch; + private SearchBuilder srvOfferingSearch; + /** * Constant used to convert GB into Bytes (or the other way around). * GB * MB * KB = Bytes // @@ -85,6 +91,10 @@ protected ServiceOfferingJoinDaoImpl() { sofIdSearch.and("id", sofIdSearch.entity().getId(), SearchCriteria.Op.EQ); sofIdSearch.done(); + srvOfferingSearch = createSearchBuilder(); + srvOfferingSearch.and("idIN", srvOfferingSearch.entity().getId(), SearchCriteria.Op.IN); + srvOfferingSearch.done(); + this._count = "select count(distinct service_offering_view.id) from service_offering_view WHERE "; } @@ -184,7 +194,6 @@ public ServiceOfferingJoinVO newServiceOfferingView(ServiceOffering offering) { return offerings.get(0); } - @Override public Map> listDomainsOfServiceOfferingsUsedByDomainPath(String domainPath) { s_logger.debug(String.format("Retrieving the domains of the service offerings used by domain with path [%s].", domainPath)); @@ -217,4 +226,48 @@ public Map> listDomainsOfServiceOfferingsUsedByDomainPath(Str return new HashMap<>(); } } + + @Override + public List searchByIds(Long... offeringIds) { + // set detail batch query size + int DETAILS_BATCH_SIZE = 2000; + String batchCfg = configDao.getValue("detail.batch.query.size"); + if (batchCfg != null) { + DETAILS_BATCH_SIZE = Integer.parseInt(batchCfg); + } + + List uvList = new ArrayList<>(); + // query details by batches + int curr_index = 0; + if (offeringIds.length > DETAILS_BATCH_SIZE) { + while ((curr_index + DETAILS_BATCH_SIZE) <= offeringIds.length) { + Long[] ids = new Long[DETAILS_BATCH_SIZE]; + for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { + ids[k] = offeringIds[j]; + } + SearchCriteria sc = srvOfferingSearch.create(); + sc.setParameters("idIN", ids); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); + } + curr_index += DETAILS_BATCH_SIZE; + } + } + if (curr_index < offeringIds.length) { + int batch_size = (offeringIds.length - curr_index); + // set the ids value + Long[] ids = new Long[batch_size]; + for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { + ids[k] = offeringIds[j]; + } + SearchCriteria sc = srvOfferingSearch.create(); + sc.setParameters("idIN", ids); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); + } + } + return uvList; + } } From 3a18a800cbf2fe6be5ba97100694cd076113f11e Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 21 Nov 2023 15:39:27 +0530 Subject: [PATCH 08/28] Use join instead of views for filtering disk offerings --- .../com/cloud/api/query/QueryManagerImpl.java | 208 ++++++++++++------ .../api/query/dao/DiskOfferingJoinDao.java | 2 + .../query/dao/DiskOfferingJoinDaoImpl.java | 54 +++++ 3 files changed, 200 insertions(+), 64 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 7cc1b2ea7f57..b908afa2ae9d 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -171,6 +171,7 @@ import org.apache.cloudstack.outofbandmanagement.OutOfBandManagementVO; import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao; import org.apache.cloudstack.query.QueryService; +import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.secstorage.HeuristicVO; import org.apache.cloudstack.secstorage.dao.SecondaryStorageHeuristicDao; @@ -3280,6 +3281,33 @@ public ListResponse searchForDiskOfferings(ListDiskOfferin } private Pair, Integer> searchForDiskOfferingsInternal(ListDiskOfferingsCmd cmd) { + Ternary, Integer, String[]> diskOfferingIdPage = searchForDiskOfferingsIdsAndCount(cmd); + + Integer count = diskOfferingIdPage.second(); + Long[] idArray = diskOfferingIdPage.first().toArray(new Long[0]); + String[] requiredTagsArray = diskOfferingIdPage.third(); + + if (count == 0) { + return new Pair<>(new ArrayList<>(), count); + } + + List diskOfferings = _diskOfferingJoinDao.searchByIds(idArray); + + if (requiredTagsArray.length != 0) { + ListIterator iteratorForTagsChecking = diskOfferings.listIterator(); + while (iteratorForTagsChecking.hasNext()) { + DiskOfferingJoinVO offering = iteratorForTagsChecking.next(); + String offeringTags = offering.getTags(); + String[] offeringTagsArray = (offeringTags == null || offeringTags.isEmpty()) ? new String[0] : offeringTags.split(","); + if (!CollectionUtils.isSubCollection(Arrays.asList(requiredTagsArray), Arrays.asList(offeringTagsArray))) { + iteratorForTagsChecking.remove(); + } + } + } + return new Pair<>(diskOfferings, count); + } + + private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount(ListDiskOfferingsCmd cmd) { // Note // The list method for offerings is being modified in accordance with // discussion with Will/Kevin @@ -3290,11 +3318,6 @@ private Pair, Integer> searchForDiskOfferingsInternal(L // till // root - Filter searchFilter = new Filter(DiskOfferingJoinVO.class, "sortKey", SortKeyAscending.value(), cmd.getStartIndex(), cmd.getPageSizeVal()); - searchFilter.addOrderBy(DiskOfferingJoinVO.class, "id", true); - SearchCriteria sc = _diskOfferingJoinDao.createSearchCriteria(); - sc.addAnd("computeOnly", Op.EQ, false); - Account account = CallContext.current().getCallingAccount(); Object name = cmd.getDiskOfferingName(); Object id = cmd.getId(); @@ -3310,18 +3333,39 @@ private Pair, Integer> searchForDiskOfferingsInternal(L Boolean encrypt = cmd.getEncrypt(); String storageType = cmd.getStorageType(); + Filter searchFilter = new Filter(DiskOfferingVO.class, "sortKey", SortKeyAscending.value(), cmd.getStartIndex(), cmd.getPageSizeVal()); + searchFilter.addOrderBy(DiskOfferingVO.class, "id", true); + SearchBuilder diskOfferingSearch = _diskOfferingDao.createSearchBuilder(); + diskOfferingSearch.select(null, Func.DISTINCT, diskOfferingSearch.entity().getId()); // select distinct + + diskOfferingSearch.and("computeOnly", diskOfferingSearch.entity().isComputeOnly(), Op.EQ); + // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the disk offering // associated with this domain - if (domainId != null) { + if (domainId != null && accountName == null) { if (accountMgr.isRootAdmin(account.getId()) || isPermissible(account.getDomainId(), domainId)) { // check if the user's domain == do's domain || user's domain is // a child of so's domain for non-root users - sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId)); + SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); + domainDetailsSearch.and("name", domainDetailsSearch.entity().getName(), Op.EQ); + domainDetailsSearch.and("domainId", domainDetailsSearch.entity().getValue(), Op.EQ); + + diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + if (!isRootAdmin) { - sc.addAnd("displayOffering", SearchCriteria.Op.EQ, 1); + diskOfferingSearch.and("displayOffering", diskOfferingSearch.entity().getDisplayOffering(), Op.EQ); } - return _diskOfferingJoinDao.searchAndCount(sc, searchFilter); + + SearchCriteria sc = diskOfferingSearch.create(); + sc.setParameters("computeOnly", false); + + sc.setJoinParameters("domainDetailsSearch", "name", "domainid"); + sc.setJoinParameters("domainDetailsSearch", "domainId", domainId); + + Pair, Integer> uniquePairs = _diskOfferingDao.searchAndCount(sc, searchFilter); + List idsArray = uniquePairs.first().stream().map(DiskOfferingVO::getId).collect(Collectors.toList()); + return new Ternary<>(idsArray, uniquePairs.second(), new String[0]); } else { throw new PermissionDeniedException("The account:" + account.getAccountName() + " does not fall in the same domain hierarchy as the disk offering"); } @@ -3342,104 +3386,140 @@ private Pair, Integer> searchForDiskOfferingsInternal(L } if (keyword != null) { - SearchCriteria ssc = _diskOfferingJoinDao.createSearchCriteria(); - ssc.addOr("displayText", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - - sc.addAnd("name", SearchCriteria.Op.SC, ssc); + diskOfferingSearch.and().op("keywordDisplayText", diskOfferingSearch.entity().getDisplayText(), Op.LIKE); + diskOfferingSearch.or("keywordName", diskOfferingSearch.entity().getName(), Op.LIKE); + diskOfferingSearch.cp(); } if (id != null) { - sc.addAnd("id", SearchCriteria.Op.EQ, id); + diskOfferingSearch.and("id", diskOfferingSearch.entity().getId(), Op.EQ); } if (name != null) { - sc.addAnd("name", SearchCriteria.Op.EQ, name); + diskOfferingSearch.and("name", diskOfferingSearch.entity().getName(), Op.EQ); } if (encrypt != null) { - sc.addAnd("encrypt", SearchCriteria.Op.EQ, encrypt); + diskOfferingSearch.and("encrypt", diskOfferingSearch.entity().getEncrypt(), Op.EQ); } - useStorageType(sc, storageType); + if (storageType != null || zoneId != null) { + diskOfferingSearch.and("useLocalStorage", diskOfferingSearch.entity().isUseLocalStorage(), Op.EQ); + } if (zoneId != null) { - SearchBuilder sb = _diskOfferingJoinDao.createSearchBuilder(); - sb.and("zoneId", sb.entity().getZoneId(), Op.FIND_IN_SET); - sb.or("zId", sb.entity().getZoneId(), Op.NULL); - sb.done(); - SearchCriteria zoneSC = sb.create(); - zoneSC.setParameters("zoneId", String.valueOf(zoneId)); - sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC); - DataCenterJoinVO zone = _dcJoinDao.findById(zoneId); - if (DataCenter.Type.Edge.equals(zone.getType())) { - sc.addAnd("useLocalStorage", Op.EQ, true); - } + SearchBuilder zoneDetailSearch = _diskOfferingDetailsDao.createSearchBuilder(); + zoneDetailSearch.and().op("name", zoneDetailSearch.entity().getName(), Op.EQ); // zoneid + zoneDetailSearch.or("nameNull", zoneDetailSearch.entity().getName(), Op.NULL); + + zoneDetailSearch.and().op("zoneId", zoneDetailSearch.entity().getValue(), Op.EQ); + zoneDetailSearch.or("zoneIdNull", zoneDetailSearch.entity().getValue(), Op.NULL); + zoneDetailSearch.cp(); + + diskOfferingSearch.join("zoneDetailSearch", zoneDetailSearch, diskOfferingSearch.entity().getId(), zoneDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); } DiskOffering currentDiskOffering = null; + Volume volume = null; if (volumeId != null) { - Volume volume = volumeDao.findById(volumeId); + volume = volumeDao.findById(volumeId); if (volume == null) { throw new InvalidParameterValueException(String.format("Unable to find a volume with specified id %s", volumeId)); } currentDiskOffering = _diskOfferingDao.findByIdIncludingRemoved(volume.getDiskOfferingId()); if (!currentDiskOffering.isComputeOnly() && currentDiskOffering.getDiskSizeStrictness()) { - SearchCriteria ssc = _diskOfferingJoinDao.createSearchCriteria(); - ssc.addOr("diskSize", Op.EQ, volume.getSize()); - ssc.addOr("customized", SearchCriteria.Op.EQ, true); - sc.addAnd("diskSizeOrCustomized", SearchCriteria.Op.SC, ssc); + diskOfferingSearch.and().op("diskSize", diskOfferingSearch.entity().getDiskSize(), Op.EQ); + diskOfferingSearch.or("customized", diskOfferingSearch.entity().isCustomized(), Op.EQ); + diskOfferingSearch.cp(); } - sc.addAnd("id", SearchCriteria.Op.NEQ, currentDiskOffering.getId()); - sc.addAnd("diskSizeStrictness", Op.EQ, currentDiskOffering.getDiskSizeStrictness()); + diskOfferingSearch.and("idNEQ", diskOfferingSearch.entity().getId(), Op.NEQ); + diskOfferingSearch.and("diskSizeStrictness", diskOfferingSearch.entity().getDiskSizeStrictness(), Op.EQ); } // Filter offerings that are not associated with caller's domain // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! account = accountMgr.finalizeOwner(account, accountName, domainId, projectId); if (!Account.Type.ADMIN.equals(account.getType())) { - Domain callerDomain = _domainDao.findById(account.getDomainId()); - List domainIds = findRelatedDomainIds(callerDomain, isRecursive); + SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); + domainDetailsSearch.and().op("name", domainDetailsSearch.entity().getName(), Op.EQ); + domainDetailsSearch.or("nameNull", domainDetailsSearch.entity().getName(), Op.EQ); + domainDetailsSearch.cp(); + domainDetailsSearch.and().op("valueIn", domainDetailsSearch.entity().getValue(), Op.IN); + domainDetailsSearch.or("valueNull", domainDetailsSearch.entity().getValue(), Op.NULL); + + diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + } + + SearchCriteria sc = diskOfferingSearch.create(); + + sc.setParameters("computeOnly", false); - List ids = _diskOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds); - SearchBuilder sb = _diskOfferingJoinDao.createSearchBuilder(); - if (ids != null && !ids.isEmpty()) { - sb.and("id", sb.entity().getId(), Op.IN); + if (keyword != null) { + sc.setParameters("keywordDisplayText", "%" + keyword + "%"); + sc.setParameters("keywordName", "%" + keyword + "%"); + } + + if (id != null) { + sc.setParameters("id", id); + } + + if (name != null) { + sc.setParameters("name", name); + } + + if (encrypt != null) { + sc.setParameters("encrypt", encrypt); + } + + if (storageType != null) { + if (storageType.equalsIgnoreCase(ServiceOffering.StorageType.local.toString())) { + sc.setParameters("useLocalStorage", true); + + } else if (storageType.equalsIgnoreCase(ServiceOffering.StorageType.shared.toString())) { + sc.setParameters("useLocalStorage", false); } - sb.or("domainId", sb.entity().getDomainId(), Op.NULL); - sb.done(); + } - SearchCriteria scc = sb.create(); - if (ids != null && !ids.isEmpty()) { - scc.setParameters("id", ids.toArray()); + if (zoneId != null) { + sc.setJoinParameters("zoneDetailSearch", "name", "zoneid"); + sc.setJoinParameters("zoneDetailSearch", "zoneId", zoneId); + + DataCenterJoinVO zone = _dcJoinDao.findById(zoneId); + if (DataCenter.Type.Edge.equals(zone.getType())) { + sc.setParameters("useLocalStorage", true); + } + } + + if (volumeId != null) { + if (!currentDiskOffering.isComputeOnly() && currentDiskOffering.getDiskSizeStrictness()) { + sc.setParameters("diskSize", volume.getSize()); + sc.setParameters("customized", true); } - sc.addAnd("domainId", SearchCriteria.Op.SC, scc); + sc.setParameters("idNEQ", currentDiskOffering.getId()); + sc.setParameters("diskSizeStrictness", currentDiskOffering.getDiskSizeStrictness()); } - Pair, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter); + // Filter offerings that are not associated with caller's domain + if (!Account.Type.ADMIN.equals(account.getType())) { + Domain callerDomain = _domainDao.findById(account.getDomainId()); + List domainIds = findRelatedDomainIds(callerDomain, isRecursive); + + sc.setJoinParameters("domainDetailsSearch", "name", "domainid"); + sc.setJoinParameters("domainDetailsSearch", "valueIn", domainIds.toArray()); + } + + Pair, Integer> uniquePairs = _diskOfferingDao.searchAndCount(sc, searchFilter); String[] requiredTagsArray = new String[0]; - if (CollectionUtils.isNotEmpty(result.first()) && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.valueIn(zoneId)) { + if (CollectionUtils.isNotEmpty(uniquePairs.first()) && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.valueIn(zoneId)) { if (volumeId != null) { - Volume volume = volumeDao.findById(volumeId); - currentDiskOffering = _diskOfferingDao.findByIdIncludingRemoved(volume.getDiskOfferingId()); requiredTagsArray = currentDiskOffering.getTagsArray(); } else if (storagePoolId != null) { requiredTagsArray = _storageTagDao.getStoragePoolTags(storagePoolId).toArray(new String[0]); } } - if (requiredTagsArray.length != 0) { - ListIterator iteratorForTagsChecking = result.first().listIterator(); - while (iteratorForTagsChecking.hasNext()) { - DiskOfferingJoinVO offering = iteratorForTagsChecking.next(); - String offeringTags = offering.getTags(); - String[] offeringTagsArray = (offeringTags == null || offeringTags.isEmpty()) ? new String[0] : offeringTags.split(","); - if (!CollectionUtils.isSubCollection(Arrays.asList(requiredTagsArray), Arrays.asList(offeringTagsArray))) { - iteratorForTagsChecking.remove(); - } - } - } + List idsArray = uniquePairs.first().stream().map(DiskOfferingVO::getId).collect(Collectors.toList()); - return new Pair<>(result.first(), result.second()); + return new Ternary<>(idsArray, uniquePairs.second(), requiredTagsArray); } private void useStorageType(SearchCriteria sc, String storageType) { diff --git a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDao.java index 3d612c63d382..3b38a562d580 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDao.java @@ -33,4 +33,6 @@ public interface DiskOfferingJoinDao extends GenericDao searchByIds(Long... idArray); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java index 9592986151fc..5341b3b56d76 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.api.query.dao; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -26,6 +27,7 @@ import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -51,9 +53,12 @@ public class DiskOfferingJoinDaoImpl extends GenericDaoBase dofIdSearch; + private SearchBuilder diskOfferingSearch; private final Attribute _typeAttr; protected DiskOfferingJoinDaoImpl() { @@ -62,6 +67,11 @@ protected DiskOfferingJoinDaoImpl() { dofIdSearch.and("id", dofIdSearch.entity().getId(), SearchCriteria.Op.EQ); dofIdSearch.done(); + diskOfferingSearch = createSearchBuilder(); + diskOfferingSearch.and("idIN", diskOfferingSearch.entity().getId(), SearchCriteria.Op.IN); + diskOfferingSearch.done(); + + _typeAttr = _allAttributes.get("type"); _count = "select count(distinct id) from disk_offering_view WHERE "; @@ -154,4 +164,48 @@ public DiskOfferingJoinVO newDiskOfferingView(DiskOffering offering) { assert offerings != null && offerings.size() == 1 : "No disk offering found for offering id " + offering.getId(); return offerings.get(0); } + + @Override + public List searchByIds(Long... offeringIds) { + // set detail batch query size + int DETAILS_BATCH_SIZE = 2000; + String batchCfg = configDao.getValue("detail.batch.query.size"); + if (batchCfg != null) { + DETAILS_BATCH_SIZE = Integer.parseInt(batchCfg); + } + + List uvList = new ArrayList<>(); + // query details by batches + int curr_index = 0; + if (offeringIds.length > DETAILS_BATCH_SIZE) { + while ((curr_index + DETAILS_BATCH_SIZE) <= offeringIds.length) { + Long[] ids = new Long[DETAILS_BATCH_SIZE]; + for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { + ids[k] = offeringIds[j]; + } + SearchCriteria sc = diskOfferingSearch.create(); + sc.setParameters("idIN", ids); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); + } + curr_index += DETAILS_BATCH_SIZE; + } + } + if (curr_index < offeringIds.length) { + int batch_size = (offeringIds.length - curr_index); + // set the ids value + Long[] ids = new Long[batch_size]; + for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { + ids[k] = offeringIds[j]; + } + SearchCriteria sc = diskOfferingSearch.create(); + sc.setParameters("idIN", ids); + List accounts = searchIncludingRemoved(sc, null, null, false); + if (accounts != null) { + uvList.addAll(accounts); + } + } + return uvList; + } } From d36e44059af2e647df019e4b671ae884f9e533e4 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 6 Dec 2023 22:23:33 +0530 Subject: [PATCH 09/28] Remove unused code --- .../api/query/dao/StoragePoolJoinDao.java | 7 -- .../api/query/dao/StoragePoolJoinDaoImpl.java | 76 ------------------- 2 files changed, 83 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java index b9860cffd33f..26ee3f01789b 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java @@ -19,9 +19,6 @@ import java.util.List; import com.cloud.storage.ScopeType; -import com.cloud.storage.StoragePoolStatus; -import com.cloud.utils.Pair; -import com.cloud.utils.db.Filter; import org.apache.cloudstack.api.response.StoragePoolResponse; import com.cloud.api.query.vo.StoragePoolJoinVO; @@ -43,10 +40,6 @@ public interface StoragePoolJoinDao extends GenericDao List searchByIds(Long... spIds); - Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, Long zoneId, - String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, - String keyword, Filter searchFilter); - List findStoragePoolByScopeAndRuleTags(Long datacenterId, Long podId, Long clusterId, ScopeType scopeType, List tags); } diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java index 3b31ac7180cb..f3b832d10424 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java @@ -23,13 +23,10 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.StorageStats; import com.cloud.storage.VolumeApiServiceImpl; import com.cloud.user.AccountManager; -import com.cloud.utils.Pair; import com.cloud.utils.StringUtils; -import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; @@ -311,79 +308,6 @@ public List searchByIds(Long... spIds) { return uvList; } - @Override - public Pair, Integer> searchAndCount(Long storagePoolId, String storagePoolName, - Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, - StoragePoolStatus status, String keyword, Filter searchFilter) { - SearchCriteria sc = createStoragePoolSearchCriteria(storagePoolId, storagePoolName, zoneId, path, podId, clusterId, address, scopeType, status, keyword); - return searchAndCount(sc, searchFilter); - } - - private SearchCriteria createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword) { - SearchBuilder sb = createSearchBuilder(); - sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); // select distinct - // ids - sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ); - sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); - sb.and("path", sb.entity().getPath(), SearchCriteria.Op.EQ); - sb.and("dataCenterId", sb.entity().getZoneId(), SearchCriteria.Op.EQ); - sb.and("podId", sb.entity().getPodId(), SearchCriteria.Op.EQ); - sb.and("clusterId", sb.entity().getClusterId(), SearchCriteria.Op.EQ); - sb.and("hostAddress", sb.entity().getHostAddress(), SearchCriteria.Op.EQ); - sb.and("scope", sb.entity().getScope(), SearchCriteria.Op.EQ); - sb.and("status", sb.entity().getStatus(), SearchCriteria.Op.EQ); - sb.and("parent", sb.entity().getParent(), SearchCriteria.Op.EQ); - - SearchCriteria sc = sb.create(); - - if (keyword != null) { - SearchCriteria ssc = createSearchCriteria(); - ssc.addOr("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - ssc.addOr("poolType", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - - sc.addAnd("name", SearchCriteria.Op.SC, ssc); - } - - if (storagePoolId != null) { - sc.setParameters("id", storagePoolId); - } - - if (storagePoolName != null) { - sc.setParameters("name", storagePoolName); - } - - if (path != null) { - sc.setParameters("path", path); - } - if (zoneId != null) { - sc.setParameters("dataCenterId", zoneId); - } - if (podId != null) { - SearchCriteria ssc = createSearchCriteria(); - ssc.addOr("podId", SearchCriteria.Op.EQ, podId); - ssc.addOr("podId", SearchCriteria.Op.NULL); - - sc.addAnd("podId", SearchCriteria.Op.SC, ssc); - } - if (address != null) { - sc.setParameters("hostAddress", address); - } - if (clusterId != null) { - SearchCriteria ssc = createSearchCriteria(); - ssc.addOr("clusterId", SearchCriteria.Op.EQ, clusterId); - ssc.addOr("clusterId", SearchCriteria.Op.NULL); - - sc.addAnd("clusterId", SearchCriteria.Op.SC, ssc); - } - if (scopeType != null) { - sc.setParameters("scope", scopeType.toString()); - } - if (status != null) { - sc.setParameters("status", status.toString()); - } - sc.setParameters("parent", 0); - return sc; - } @Override public List findStoragePoolByScopeAndRuleTags(Long datacenterId, Long podId, Long clusterId, ScopeType scopeType, List tags) { SearchCriteria sc = findByDatacenterAndScopeSb.create(); From 519bd47068b98c45e0c68bc3c40fc041d9792c91 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 7 Dec 2023 00:56:52 +0530 Subject: [PATCH 10/28] Fix unit test --- .../cloud/api/query/QueryManagerImplTest.java | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java index 4af84cfa814f..be8978e799bd 100644 --- a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java +++ b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java @@ -20,6 +20,8 @@ import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.api.query.vo.EventJoinVO; import com.cloud.api.query.vo.TemplateJoinVO; +import com.cloud.event.EventVO; +import com.cloud.event.dao.EventDao; import com.cloud.event.dao.EventJoinDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; @@ -92,6 +94,9 @@ public class QueryManagerImplTest { @Mock AccountManager accountManager; + @Mock + EventDao eventDao; + @Mock EventJoinDao eventJoinDao; @@ -128,12 +133,12 @@ private void setupCommonMocks() { UUID.randomUUID().toString(), User.Source.UNKNOWN); CallContext.register(user, account); Mockito.when(accountManager.isRootAdmin(account.getId())).thenReturn(false); - final SearchBuilder searchBuilder = Mockito.mock(SearchBuilder.class); - final SearchCriteria searchCriteria = Mockito.mock(SearchCriteria.class); - final EventJoinVO eventJoinVO = Mockito.mock(EventJoinVO.class); - when(searchBuilder.entity()).thenReturn(eventJoinVO); - when(searchBuilder.create()).thenReturn(searchCriteria); - Mockito.when(eventJoinDao.createSearchBuilder()).thenReturn(searchBuilder); + final SearchBuilder eventSearchBuilder = Mockito.mock(SearchBuilder.class); + final SearchCriteria eventSearchCriteria = Mockito.mock(SearchCriteria.class); + final EventVO eventVO = Mockito.mock(EventVO.class); + when(eventSearchBuilder.entity()).thenReturn(eventVO); + when(eventSearchBuilder.create()).thenReturn(eventSearchCriteria); + Mockito.when(eventDao.createSearchBuilder()).thenReturn(eventSearchBuilder); } private ListEventsCmd setupMockListEventsCmd() { @@ -149,19 +154,26 @@ public void searchForEventsSuccess() { String uuid = UUID.randomUUID().toString(); Mockito.when(cmd.getResourceId()).thenReturn(uuid); Mockito.when(cmd.getResourceType()).thenReturn(ApiCommandResourceType.Network.toString()); - List events = new ArrayList<>(); - events.add(Mockito.mock(EventJoinVO.class)); - events.add(Mockito.mock(EventJoinVO.class)); - events.add(Mockito.mock(EventJoinVO.class)); - Pair, Integer> pair = new Pair<>(events, events.size()); + List events = new ArrayList<>(); + events.add(Mockito.mock(EventVO.class)); + events.add(Mockito.mock(EventVO.class)); + events.add(Mockito.mock(EventVO.class)); + Pair, Integer> pair = new Pair<>(events, events.size()); + + List eventJoins = new ArrayList<>(); + eventJoins.add(Mockito.mock(EventJoinVO.class)); + eventJoins.add(Mockito.mock(EventJoinVO.class)); + eventJoins.add(Mockito.mock(EventJoinVO.class)); + NetworkVO network = Mockito.mock(NetworkVO.class); Mockito.when(network.getId()).thenReturn(1L); Mockito.when(network.getAccountId()).thenReturn(account.getId()); Mockito.when(entityManager.findByUuidIncludingRemoved(Network.class, uuid)).thenReturn(network); Mockito.doNothing().when(accountManager).checkAccess(account, SecurityChecker.AccessType.ListEntry, true, network); - Mockito.when(eventJoinDao.searchAndCount(Mockito.any(), Mockito.any(Filter.class))).thenReturn(pair); + Mockito.when(eventDao.searchAndCount(Mockito.any(), Mockito.any(Filter.class))).thenReturn(pair); + Mockito.when(eventJoinDao.searchByIds(Mockito.any())).thenReturn(eventJoins); List respList = new ArrayList(); - for (EventJoinVO vt : events) { + for (EventJoinVO vt : eventJoins) { respList.add(eventJoinDao.newEventResponse(vt)); } try (MockedStatic ignored = Mockito.mockStatic(ViewResponseHelper.class)) { From 35a226f57c4ae75ea403ae1e228fee39978efe11 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 7 Dec 2023 01:05:04 +0530 Subject: [PATCH 11/28] Use disk_offering instead of disk_offering_view in service_offering_view --- .../resources/META-INF/db/views/cloud.service_offering_view.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql index e859af482b48..63b0b044f7bf 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql @@ -84,7 +84,7 @@ SELECT FROM `cloud`.`service_offering` INNER JOIN - `cloud`.`disk_offering_view` AS `disk_offering` ON service_offering.disk_offering_id = disk_offering.id + `cloud`.`disk_offering` AS `disk_offering` ON service_offering.disk_offering_id = disk_offering.id LEFT JOIN `cloud`.`service_offering_details` AS `domain_details` ON `domain_details`.`service_offering_id` = `service_offering`.`id` AND `domain_details`.`name`='domainid' LEFT JOIN From a77fe22efeeafb118233ce6b010a51c349664ec3 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 7 Dec 2023 01:38:19 +0530 Subject: [PATCH 12/28] Fixup --- .../java/com/cloud/utils/db/SearchBase.java | 13 +++++++++++ .../com/cloud/api/query/QueryManagerImpl.java | 23 +++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index dbd6084d225a..440910a37951 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -222,6 +222,19 @@ public J join(final String name, final SearchBase builder, final Object return (J)this; } + + @SuppressWarnings("unchecked") + public J join(final String tableAliasName, final String name, final SearchBase builder, final Object joinField1, final Object joinField2, final JoinBuilder.JoinType joinType) { + assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; + assert _specifiedAttrs.size() == 1 : "You didn't select the attribute."; + assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; + assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; + assert builder != this : "You can't add yourself, can you? Really think about it!"; + + _specifiedAttrs.get(0).table = tableAliasName; + return join(name, builder, joinField1, joinField2, joinType); + } + public SelectType getSelectType() { return _selectType; } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index b908afa2ae9d..e297d963ce99 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2473,14 +2473,14 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) SearchBuilder vmSearch = _vmInstanceDao.createSearchBuilder(); SearchBuilder serviceOfferingSearch = _srvOfferingDao.createSearchBuilder(); vmSearch.and().op("svmType", vmSearch.entity().getType(), SearchCriteria.Op.NIN); - vmSearch.or("nulltype", vmSearch.entity().getType(), SearchCriteria.Op.NULL); + vmSearch.or("vmSearchNulltype", vmSearch.entity().getType(), SearchCriteria.Op.NULL); vmSearch.cp(); serviceOfferingSearch.and().op("systemUse", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NEQ); - serviceOfferingSearch.or("nulltype", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NULL); + serviceOfferingSearch.or("serviceOfferingSearchNulltype", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NULL); serviceOfferingSearch.cp(); - vmSearch.join("serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); + vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); @@ -3409,8 +3409,8 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount if (zoneId != null) { SearchBuilder zoneDetailSearch = _diskOfferingDetailsDao.createSearchBuilder(); - zoneDetailSearch.and().op("name", zoneDetailSearch.entity().getName(), Op.EQ); // zoneid - zoneDetailSearch.or("nameNull", zoneDetailSearch.entity().getName(), Op.NULL); + zoneDetailSearch.and().op("name", zoneDetailSearch.entity().getName(), Op.EQ); + zoneDetailSearch.or("nameNull", zoneDetailSearch.entity().getName(), Op.NULL).cp(); zoneDetailSearch.and().op("zoneId", zoneDetailSearch.entity().getValue(), Op.EQ); zoneDetailSearch.or("zoneIdNull", zoneDetailSearch.entity().getValue(), Op.NULL); @@ -3442,10 +3442,11 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount if (!Account.Type.ADMIN.equals(account.getType())) { SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); domainDetailsSearch.and().op("name", domainDetailsSearch.entity().getName(), Op.EQ); - domainDetailsSearch.or("nameNull", domainDetailsSearch.entity().getName(), Op.EQ); + domainDetailsSearch.or("nameNull", domainDetailsSearch.entity().getName(), Op.NULL); domainDetailsSearch.cp(); domainDetailsSearch.and().op("valueIn", domainDetailsSearch.entity().getValue(), Op.IN); domainDetailsSearch.or("valueNull", domainDetailsSearch.entity().getValue(), Op.NULL); + domainDetailsSearch.cp(); diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); } @@ -3767,7 +3768,8 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic DataCenterJoinVO zone = null; if (zoneId != null) { SearchBuilder srvOffrZoneDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); - srvOffrZoneDetailSearch.and("name", srvOffrZoneDetailSearch.entity().getName(), Op.EQ); + srvOffrZoneDetailSearch.and().op("name", srvOffrZoneDetailSearch.entity().getName(), Op.EQ); + srvOffrZoneDetailSearch.or("nameNull", srvOffrZoneDetailSearch.entity().getName(), Op.NULL).cp(); srvOffrZoneDetailSearch.and().op("value", srvOffrZoneDetailSearch.entity().getValue(), Op.EQ); srvOffrZoneDetailSearch.and().or("valueNull", srvOffrZoneDetailSearch.entity().getValue(), Op.NULL); srvOffrZoneDetailSearch.cp(); @@ -3784,7 +3786,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { for (String tag : storageTags) { - diskOfferingSearch.and(tag, diskOfferingSearch.entity().getTags(), Op.FIND_IN_SET); + diskOfferingSearch.and(tag, diskOfferingSearch.entity().getTags(), Op.EQ); } diskOfferingSearch.done(); } @@ -3873,7 +3875,8 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! if (owner.getType() != Account.Type.ADMIN) { SearchBuilder srvOffrDomainDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); - srvOffrDomainDetailSearch.and("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); + srvOffrDomainDetailSearch.and().op("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); + srvOffrDomainDetailSearch.or("nameNull", srvOffrDomainDetailSearch.entity().getName(), Op.NULL).cp(); srvOffrDomainDetailSearch.and().op("value", srvOffrDomainDetailSearch.entity().getValue(), Op.IN); srvOffrDomainDetailSearch.or("value", srvOffrDomainDetailSearch.entity().getValue(), Op.NULL); srvOffrDomainDetailSearch.cp().done(); @@ -3888,7 +3891,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic serviceOfferingSearch.or().op(); for(String tag : hostTags) { - serviceOfferingSearch.and(tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET); + serviceOfferingSearch.and(tag, serviceOfferingSearch.entity().getHostTag(), Op.EQ); } serviceOfferingSearch.cp().cp().done(); } From 15de70e26c9e229724aac369751e0ec36e237990 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 12 Dec 2023 09:20:44 +0530 Subject: [PATCH 13/28] Fix listing of diskoffering & serviceoffering --- .../cloudstack/api/InternalIdentity.java | 14 ++ .../com/cloud/service/ServiceOfferingVO.java | 14 +- .../resourcedetail/DiskOfferingDetailVO.java | 4 + .../java/com/cloud/utils/db/Attribute.java | 18 ++ .../com/cloud/utils/db/GenericDaoBase.java | 39 ++-- .../java/com/cloud/utils/db/JoinBuilder.java | 56 ++++- .../java/com/cloud/utils/db/SearchBase.java | 61 +++++- .../com/cloud/utils/db/SearchCriteria.java | 2 +- .../cloud/utils/db/GenericDaoBaseTest.java | 6 +- .../com/cloud/api/query/QueryManagerImpl.java | 203 +++++++++--------- 10 files changed, 282 insertions(+), 135 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/InternalIdentity.java b/api/src/main/java/org/apache/cloudstack/api/InternalIdentity.java index 4149dd1c8465..ce214e05b4f1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/InternalIdentity.java +++ b/api/src/main/java/org/apache/cloudstack/api/InternalIdentity.java @@ -25,4 +25,18 @@ public interface InternalIdentity extends Serializable { long getId(); + + /* + Helper method to add conditions in joins where some column name is equal to a string value + */ + default Object setString(String str) { + return null; + } + + /* + Helper method to add conditions in joins where some column name is equal to a long value + */ + default Object setLong(Long l) { + return null; + } } diff --git a/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java b/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java index 31e4b073c139..125458650096 100644 --- a/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java @@ -110,7 +110,7 @@ public class ServiceOfferingVO implements ServiceOffering { private boolean defaultUse; @Column(name = "vm_type") - private String vmType; + private String systemVmType; @Column(name = "sort_key") int sortKey; @@ -140,7 +140,7 @@ protected ServiceOfferingVO() { } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, String displayText, - boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { + boolean systemUse, VirtualMachine.Type systemVmType, boolean defaultUse) { this.cpu = cpu; this.ramSize = ramSize; this.speed = speed; @@ -150,7 +150,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee limitCpuUse = false; volatileVm = false; this.defaultUse = defaultUse; - this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); + this.systemVmType = systemVmType == null ? null : systemVmType.toString().toLowerCase(); uuid = UUID.randomUUID().toString(); this.systemUse = systemUse; this.name = name; @@ -159,7 +159,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, boolean volatileVm, String displayText, boolean systemUse, - VirtualMachine.Type vmType, String hostTag, String deploymentPlanner, boolean dynamicScalingEnabled, boolean isCustomized) { + VirtualMachine.Type systemVmType, String hostTag, String deploymentPlanner, boolean dynamicScalingEnabled, boolean isCustomized) { this.cpu = cpu; this.ramSize = ramSize; this.speed = speed; @@ -168,7 +168,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee this.offerHA = offerHA; this.limitCpuUse = limitResourceUse; this.volatileVm = volatileVm; - this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); + this.systemVmType = systemVmType == null ? null : systemVmType.toString().toLowerCase(); this.hostTag = hostTag; this.deploymentPlanner = deploymentPlanner; uuid = UUID.randomUUID().toString(); @@ -194,7 +194,7 @@ public ServiceOfferingVO(ServiceOfferingVO offering) { limitCpuUse = offering.getLimitCpuUse(); volatileVm = offering.isVolatileVm(); hostTag = offering.getHostTag(); - vmType = offering.getSystemVmType(); + systemVmType = offering.getSystemVmType(); systemUse = offering.isSystemUse(); dynamicScalingEnabled = offering.isDynamicScalingEnabled(); diskOfferingStrictness = offering.diskOfferingStrictness; @@ -279,7 +279,7 @@ public String getHostTag() { @Override public String getSystemVmType() { - return vmType; + return systemVmType; } @Override diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/DiskOfferingDetailVO.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/DiskOfferingDetailVO.java index f4d98c3fb7fa..7b0500680b95 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/DiskOfferingDetailVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/DiskOfferingDetailVO.java @@ -65,6 +65,10 @@ public String getName() { return name; } + public void setName(String name) { + this.name = name; + } + @Override public String getValue() { return value; diff --git a/framework/db/src/main/java/com/cloud/utils/db/Attribute.java b/framework/db/src/main/java/com/cloud/utils/db/Attribute.java index 82c2bdbaa79b..92f4e19cf50f 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/Attribute.java +++ b/framework/db/src/main/java/com/cloud/utils/db/Attribute.java @@ -81,6 +81,7 @@ public int setFalse(int value) { protected String table; protected String columnName; + protected Object value; protected Field field; protected int flags; protected Column column; @@ -100,6 +101,10 @@ public Attribute(String table, String columnName) { this.column = null; } + public Attribute(Object value) { + this.value = value; + } + protected void setupColumnInfo(Class clazz, AttributeOverride[] overrides, String tableName, boolean isEmbedded, boolean isId) { flags = Flag.Selectable.setTrue(flags); GeneratedValue gv = field.getAnnotation(GeneratedValue.class); @@ -214,6 +219,19 @@ public Field getField() { return field; } + public Object getValue() { + return value; + } + + public String getValueToSql() { + if (value instanceof String) { + return "'" + value + "'"; + } else if (value instanceof Long) { + return value.toString(); + } + throw new IllegalArgumentException("Unsupported type " + value.getClass()); + } + public Object get(Object entity) { try { return field.get(entity); diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index e020e8242d57..2974896d5959 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1288,7 +1288,7 @@ protected void addJoins(StringBuilder str, Collection> join : joins) { - String joinTableName = join.getSecondAttribute().table; + String joinTableName = join.getSecondAttribute()[0].table; String joinTableAlias = StringUtils.isNotEmpty(join.getName()) ? join.getName() : findNextJoinTableName(joinTableName, joinedTableNames); StringBuilder onClause = new StringBuilder(); onClause.append(" ") @@ -1298,19 +1298,32 @@ protected void addJoins(StringBuilder str, Collection 0) { + onClause.append(join.getCondition().getName()); + } + if (join.getFirstAttributes()[i].getValue() != null) { + onClause.append(join.getFirstAttributes()[i].getValueToSql()); + } else { + onClause.append(join.getFirstAttributes()[i].table) + .append(".") + .append(join.getFirstAttributes()[i].columnName); + } + onClause.append("="); + if (join.getSecondAttribute()[i].getValue() != null) { + onClause.append(join.getSecondAttribute()[i].getValueToSql()); + } else { + if(!joinTableAlias.equals(joinTableName)) { + onClause.append(joinTableAlias); + } else { + onClause.append(joinTableName); + } + onClause.append(".") + .append(join.getSecondAttribute()[i].columnName); + } } - onClause.append(".") - .append(join.getSecondAttribute().columnName) - .append(" "); + onClause.append(" "); str.insert(fromIndex, onClause); String whereClause = join.getT().getWhereClause(joinTableAlias); if (StringUtils.isNotEmpty(whereClause)) { diff --git a/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java b/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java index 8d83cf3d38ed..f0a3ad825960 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java +++ b/framework/db/src/main/java/com/cloud/utils/db/JoinBuilder.java @@ -31,19 +31,51 @@ public String getName() { return _name; } } + public enum JoinCondition { + AND(" AND "), OR(" OR "); + + private final String name; + + JoinCondition(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } private final T t; private final String name; private JoinType type; - private Attribute firstAttribute; - private Attribute secondAttribute; - public JoinBuilder(String name, T t, Attribute firstAttribute, Attribute secondAttribute, JoinType type) { + private JoinCondition condition; + private Attribute[] firstAttributes; + private Attribute[] secondAttribute; + + public JoinBuilder(String name, T t, Attribute firstAttributes, Attribute secondAttribute, JoinType type) { this.name = name; this.t = t; - this.firstAttribute = firstAttribute; + this.firstAttributes = new Attribute[]{firstAttributes}; + this.secondAttribute = new Attribute[]{secondAttribute}; + this.type = type; + } + + public JoinBuilder(String name, T t, Attribute[] firstAttributes, Attribute[] secondAttribute, JoinType type) { + this.name = name; + this.t = t; + this.firstAttributes = firstAttributes; + this.secondAttribute = secondAttribute; + this.type = type; + } + + public JoinBuilder(String name, T t, Attribute[] firstAttributes, Attribute[] secondAttribute, JoinType type, JoinCondition condition) { + this.name = name; + this.t = t; + this.firstAttributes = firstAttributes; this.secondAttribute = secondAttribute; this.type = type; + this.condition = condition; } public String getName() { @@ -62,19 +94,23 @@ public void setType(JoinType type) { this.type = type; } - public Attribute getFirstAttribute() { - return firstAttribute; + public JoinCondition getCondition() { + return condition; + } + + public Attribute[] getFirstAttributes() { + return firstAttributes; } - public void setFirstAttribute(Attribute firstAttribute) { - this.firstAttribute = firstAttribute; + public void setFirstAttributes(Attribute[] firstAttributes) { + this.firstAttributes = firstAttributes; } - public Attribute getSecondAttribute() { + public Attribute[] getSecondAttribute() { return secondAttribute; } - public void setSecondAttribute(Attribute secondAttribute) { + public void setSecondAttribute(Attribute[] secondAttribute) { this.secondAttribute = secondAttribute; } diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index 440910a37951..e2ded86bca97 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -211,7 +211,7 @@ public J join(final String name, final SearchBase builder, final Object assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; assert builder != this : "You can't add yourself, can you? Really think about it!"; - final JoinBuilder> t = new JoinBuilder>(name, builder, _specifiedAttrs.get(0), builder._specifiedAttrs.get(0), joinType); + final JoinBuilder> t = new JoinBuilder>(name, builder, new Attribute[]{_specifiedAttrs.get(0)}, new Attribute[]{builder._specifiedAttrs.get(0)}, joinType); if (_joins == null) { _joins = new HashMap>>(); } @@ -223,16 +223,57 @@ public J join(final String name, final SearchBase builder, final Object } + public J join(final String name, final SearchBase builder, final JoinBuilder.JoinType joinType, final + JoinBuilder.JoinCondition condition, final Object... joinFields) { + assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; + assert !_specifiedAttrs.isEmpty() : "You didn't select the attribute."; + assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; + assert !builder._specifiedAttrs.isEmpty() : "You didn't select the attribute."; + assert builder != this : "You can't add yourself, can you? Really think about it!"; + assert _specifiedAttrs.size() == builder._specifiedAttrs.size() : "You didn't select the same number of attributes."; + + final JoinBuilder> t = new JoinBuilder<>(name, builder, _specifiedAttrs.toArray(new Attribute[0]), + builder._specifiedAttrs.toArray(new Attribute[0]), joinType, condition); + if (_joins == null) { + _joins = new HashMap>>(); + } + _joins.put(name, t); + + builder._specifiedAttrs.clear(); + _specifiedAttrs.clear(); + return (J)this; + } + + + /** + * This method is used to set alias for the table name when joining with another table. + * Allows to set alias for the table name which is further used to generate the condition for nested join. + * e.g. vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); + * + * volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); + * + * In the above example, vmSearch is the alias for the table name vm_instance and the query generated is + * FROM volume + * LEFT JOIN vm_instance vmSearch ON vmSearch.id = volume.instance_id + * LEFT JOIN service_offering serviceOfferingSearch ON serviceOfferingSearch.id = vmSearch.service_offering_id + */ @SuppressWarnings("unchecked") - public J join(final String tableAliasName, final String name, final SearchBase builder, final Object joinField1, final Object joinField2, final JoinBuilder.JoinType joinType) { + public J join( + final String tableAliasName, final String name, final SearchBase builder, + final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields + ) { assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; assert _specifiedAttrs.size() == 1 : "You didn't select the attribute."; assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; assert builder != this : "You can't add yourself, can you? Really think about it!"; - _specifiedAttrs.get(0).table = tableAliasName; - return join(name, builder, joinField1, joinField2, joinType); + for (final Attribute attr : _specifiedAttrs) { + if (attr.table != null) { + attr.table = tableAliasName; + } + } + return join(name, builder, joinType, condition, joinFields); } public SelectType getSelectType() { @@ -245,6 +286,16 @@ protected void set(final String name) { _specifiedAttrs.add(attr); } + /* + Allows to set conditions in join where one entity is equivalent to a string or a long + e.g. join("vm", vmSearch, VmDetailVO.class, entity.getName(), "vm.id", SearchCriteria.Op.EQ); + will create a condition 'vm.name = "vm.id"' + */ + protected void setAttr(final Object obj) { + final Attribute attr = new Attribute(obj); + _specifiedAttrs.add(attr); + } + /** * @return entity object. This allows the caller to use the entity return * to specify the field to be selected in many of the search parameters. @@ -531,6 +582,8 @@ public Object intercept(final Object object, final Method method, final Object[] final String fieldName = Character.toLowerCase(name.charAt(2)) + name.substring(3); set(fieldName); return null; + } else if (name.equals("setLong") || name.equals("setString")) { + setAttr(args[0]); } else { final Column ann = method.getAnnotation(Column.class); if (ann != null) { diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java index 9467a476bd2a..72dc7996860a 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java @@ -106,7 +106,7 @@ protected SearchCriteria(SearchBase sb) { for (Map.Entry>> entry : sb._joins.entrySet()) { JoinBuilder> value = entry.getValue(); _joins.put(entry.getKey(), - new JoinBuilder>(entry.getKey(), value.getT().create(), value.getFirstAttribute(), value.getSecondAttribute(), value.getType())); + new JoinBuilder>(entry.getKey(), value.getT().create(), value.getFirstAttributes(), value.getSecondAttribute(), value.getType(), value.getCondition())); } } _selects = sb._selects; diff --git a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java index 3853569af36a..15d6c8901591 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java @@ -251,10 +251,12 @@ public void multiJoinSameTableTest() { joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tDc4, tAc3, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("tableAlias", dbTestDao.createSearchCriteria(), tDc4, tAc3, JoinBuilder.JoinType.INNER)); dbTestDao.addJoins(joinString, joins); - Assert.assertEquals(" INNER JOIN tableA ON tableB.column2=tableA.column1 INNER JOIN tableA tableA1 ON tableC.column3=tableA1.column2 INNER JOIN tableA tableA2 ON tableD.column4=tableA2.column3 ", joinString.toString()); + Assert.assertEquals(" INNER JOIN tableA ON tableB.column2=tableA.column1 " + + " INNER JOIN tableA tableA1 ON tableC.column3=tableA1.column2 " + + " INNER JOIN tableA tableAlias ON tableD.column4=tableAlias.column3 ", joinString.toString()); } @Test diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index e297d963ce99..4e0ec0e7c1f9 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2480,7 +2480,7 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) serviceOfferingSearch.or("serviceOfferingSearchNulltype", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NULL); serviceOfferingSearch.cp(); - vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); + vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, JoinBuilder.JoinType.LEFT, null, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId()); volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); @@ -3339,6 +3339,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount diskOfferingSearch.select(null, Func.DISTINCT, diskOfferingSearch.entity().getId()); // select distinct diskOfferingSearch.and("computeOnly", diskOfferingSearch.entity().isComputeOnly(), Op.EQ); + diskOfferingSearch.and("activeState", diskOfferingSearch.entity().getState(), Op.EQ); // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the disk offering @@ -3348,10 +3349,11 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount // check if the user's domain == do's domain || user's domain is // a child of so's domain for non-root users SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); - domainDetailsSearch.and("name", domainDetailsSearch.entity().getName(), Op.EQ); domainDetailsSearch.and("domainId", domainDetailsSearch.entity().getValue(), Op.EQ); - diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), + domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString("domainid")); if (!isRootAdmin) { diskOfferingSearch.and("displayOffering", diskOfferingSearch.entity().getDisplayOffering(), Op.EQ); @@ -3359,8 +3361,8 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount SearchCriteria sc = diskOfferingSearch.create(); sc.setParameters("computeOnly", false); + sc.setParameters("activeState", DiskOffering.State.Active); - sc.setJoinParameters("domainDetailsSearch", "name", "domainid"); sc.setJoinParameters("domainDetailsSearch", "domainId", domainId); Pair, Integer> uniquePairs = _diskOfferingDao.searchAndCount(sc, searchFilter); @@ -3409,14 +3411,13 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount if (zoneId != null) { SearchBuilder zoneDetailSearch = _diskOfferingDetailsDao.createSearchBuilder(); - zoneDetailSearch.and().op("name", zoneDetailSearch.entity().getName(), Op.EQ); - zoneDetailSearch.or("nameNull", zoneDetailSearch.entity().getName(), Op.NULL).cp(); - zoneDetailSearch.and().op("zoneId", zoneDetailSearch.entity().getValue(), Op.EQ); - zoneDetailSearch.or("zoneIdNull", zoneDetailSearch.entity().getValue(), Op.NULL); + zoneDetailSearch.or("zoneIdNull", zoneDetailSearch.entity().getId(), Op.NULL); zoneDetailSearch.cp(); - diskOfferingSearch.join("zoneDetailSearch", zoneDetailSearch, diskOfferingSearch.entity().getId(), zoneDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + diskOfferingSearch.join("zoneDetailSearch", zoneDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + diskOfferingSearch.entity().getId(), zoneDetailSearch.entity().getResourceId(), + zoneDetailSearch.entity().getName(), diskOfferingSearch.entity().setString("zoneid")); } DiskOffering currentDiskOffering = null; @@ -3436,24 +3437,22 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount diskOfferingSearch.and("diskSizeStrictness", diskOfferingSearch.entity().getDiskSizeStrictness(), Op.EQ); } - // Filter offerings that are not associated with caller's domain - // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! account = accountMgr.finalizeOwner(account, accountName, domainId, projectId); if (!Account.Type.ADMIN.equals(account.getType())) { SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); - domainDetailsSearch.and().op("name", domainDetailsSearch.entity().getName(), Op.EQ); - domainDetailsSearch.or("nameNull", domainDetailsSearch.entity().getName(), Op.NULL); - domainDetailsSearch.cp(); - domainDetailsSearch.and().op("valueIn", domainDetailsSearch.entity().getValue(), Op.IN); - domainDetailsSearch.or("valueNull", domainDetailsSearch.entity().getValue(), Op.NULL); + domainDetailsSearch.and().op("domainIdIN", domainDetailsSearch.entity().getValue(), Op.IN); + domainDetailsSearch.or("domainIdNull", domainDetailsSearch.entity().getId(), Op.NULL); domainDetailsSearch.cp(); - diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), + domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString("domainid")); } SearchCriteria sc = diskOfferingSearch.create(); sc.setParameters("computeOnly", false); + sc.setParameters("activeState", DiskOffering.State.Active); if (keyword != null) { sc.setParameters("keywordDisplayText", "%" + keyword + "%"); @@ -3482,7 +3481,6 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount } if (zoneId != null) { - sc.setJoinParameters("zoneDetailSearch", "name", "zoneid"); sc.setJoinParameters("zoneDetailSearch", "zoneId", zoneId); DataCenterJoinVO zone = _dcJoinDao.findById(zoneId); @@ -3505,8 +3503,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount Domain callerDomain = _domainDao.findById(account.getDomainId()); List domainIds = findRelatedDomainIds(callerDomain, isRecursive); - sc.setJoinParameters("domainDetailsSearch", "name", "domainid"); - sc.setJoinParameters("domainDetailsSearch", "valueIn", domainIds.toArray()); + sc.setJoinParameters("domainDetailsSearch", "domainIdIN", domainIds.toArray()); } Pair, Integer> uniquePairs = _diskOfferingDao.searchAndCount(sc, searchFilter); @@ -3671,13 +3668,12 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic ) ) */ + // TODO: Fix this SearchBuilder maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - maxComputeDetailsSearch.and().op("name", maxComputeDetailsSearch.entity().getName(), Op.EQ); - maxComputeDetailsSearch.or("idNull", maxComputeDetailsSearch.entity().getId(), Op.NULL); - maxComputeDetailsSearch.cp(); - serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, - serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), + maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxcpunumber")); serviceOfferingSearch.and().op("vmCpu", serviceOfferingSearch.entity().getCpu(), Op.GTEQ); serviceOfferingSearch.or().op("vmCpuNull", serviceOfferingSearch.entity().getCpu(), Op.NULL); @@ -3701,12 +3697,10 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic ) */ SearchBuilder maxMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - maxMemoryDetailsSearch.and().op("name", maxMemoryDetailsSearch.entity().getName(), Op.EQ); - maxMemoryDetailsSearch.or("idNull", maxMemoryDetailsSearch.entity().getId(), Op.NULL); - maxMemoryDetailsSearch.cp(); - serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, - serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), + maxMemoryDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxmemory")); serviceOfferingSearch.and().op("vmMemory", serviceOfferingSearch.entity().getRamSize(), Op.GTEQ); serviceOfferingSearch.or().op("vmMemoryNull", serviceOfferingSearch.entity().getRamSize(), Op.NULL); @@ -3736,9 +3730,10 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (domainId != null && accountName == null) { SearchBuilder srvOffrDomainDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); - srvOffrDomainDetailSearch.and("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); - srvOffrDomainDetailSearch.and("value", srvOffrDomainDetailSearch.entity().getValue(), Op.EQ); - serviceOfferingSearch.join("domainDetailSearch", srvOffrDomainDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + srvOffrDomainDetailSearch.and("domainId", srvOffrDomainDetailSearch.entity().getValue(), Op.EQ); + serviceOfferingSearch.join("domainDetailSearch", srvOffrDomainDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), + srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("domainid")); } } @@ -3763,17 +3758,18 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (vmTypeStr != null) { - serviceOfferingSearch.and("vmType", serviceOfferingSearch.entity().getSystemVmType(), SearchCriteria.Op.EQ); + serviceOfferingSearch.and("svmType", serviceOfferingSearch.entity().getSystemVmType(), SearchCriteria.Op.EQ); } DataCenterJoinVO zone = null; if (zoneId != null) { SearchBuilder srvOffrZoneDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); - srvOffrZoneDetailSearch.and().op("name", srvOffrZoneDetailSearch.entity().getName(), Op.EQ); - srvOffrZoneDetailSearch.or("nameNull", srvOffrZoneDetailSearch.entity().getName(), Op.NULL).cp(); - srvOffrZoneDetailSearch.and().op("value", srvOffrZoneDetailSearch.entity().getValue(), Op.EQ); - srvOffrZoneDetailSearch.and().or("valueNull", srvOffrZoneDetailSearch.entity().getValue(), Op.NULL); + srvOffrZoneDetailSearch.and().op("zoneId", srvOffrZoneDetailSearch.entity().getValue(), Op.EQ); + srvOffrZoneDetailSearch.or("idNull", srvOffrZoneDetailSearch.entity().getId(), Op.NULL); srvOffrZoneDetailSearch.cp(); - serviceOfferingSearch.join("ZoneDetailSearch", srvOffrZoneDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrZoneDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + + serviceOfferingSearch.join("ZoneDetailSearch", srvOffrZoneDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), srvOffrZoneDetailSearch.entity().getResourceId(), + srvOffrZoneDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("zoneid")); zone = _dcJoinDao.findById(zoneId); } @@ -3799,70 +3795,85 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic SearchBuilder maxComputeDetailsSearch = (SearchBuilder) serviceOfferingSearch.getJoinSB("maxComputeDetailsSearch"); if (maxComputeDetailsSearch == null) { maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - maxComputeDetailsSearch.and().op("name", maxComputeDetailsSearch.entity().getName(), Op.EQ); - maxComputeDetailsSearch.or("idNull", maxComputeDetailsSearch.entity().getId(), Op.NULL); - maxComputeDetailsSearch.cp(); - serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, - serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), + maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxcpunumber")); } SearchBuilder minComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - minComputeDetailsSearch.and().op("name", minComputeDetailsSearch.entity().getName(), Op.EQ); - minComputeDetailsSearch.or("idNull", minComputeDetailsSearch.entity().getId(), Op.NULL); - minComputeDetailsSearch.cp(); - serviceOfferingSearch.join("minComputeDetailsSearch", minComputeDetailsSearch, - serviceOfferingSearch.entity().getId(), minComputeDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("minComputeDetailsSearch", minComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), minComputeDetailsSearch.entity().getResourceId(), + minComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("mincpunumber")); /* (min_cpu IS NULL AND cpu IS NULL AND max_cpu IS NULL) OR (cpu = X) OR (min_cpu <= X AND max_cpu >= X) + + AND ( + (min_compute_details.value is NULL AND cpu is NULL) + OR (min_compute_details.value is NULL AND cpu >= X) + OR min_compute_details.value >= X + OR ( + ((min_compute_details.value is NULL AND cpu <= X) OR min_compute_details.value <= X) + AND ((max_compute_details.value is NULL AND cpu >= X) OR max_compute_details.value >= X) + ) + ) */ serviceOfferingSearch.and().op().op("minComputeDetailsSearch", "cpuConstraintMinComputeNull", minComputeDetailsSearch.entity().getValue(), Op.NULL); - serviceOfferingSearch.and("maxComputeDetailsSearch", "cpuConstraintMaxComputeNull", maxComputeDetailsSearch.entity().getValue(), Op.NULL); serviceOfferingSearch.and("cpuConstraintNull", serviceOfferingSearch.entity().getCpu(), Op.NULL).cp(); - serviceOfferingSearch.or("cpuConstraintEQ", serviceOfferingSearch.entity().getCpu(), Op.EQ); + serviceOfferingSearch.or().op("minComputeDetailsSearch", "cpuConstraintMinComputeNull", minComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("cpuNumber", serviceOfferingSearch.entity().getCpu(), Op.GTEQ).cp(); + serviceOfferingSearch.or("cpuNumber", serviceOfferingSearch.entity().getCpu(), Op.GTEQ); - serviceOfferingSearch.or().op("minComputeDetailsSearch", "cpuConstraintMinComputeLTEQ", minComputeDetailsSearch.entity().getValue(), Op.LTEQ); - serviceOfferingSearch.and("maxComputeDetailsSearch", "cpuConstraintMaxComputeGTEQ", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); - serviceOfferingSearch.cp(); + serviceOfferingSearch.or().op().op(); + serviceOfferingSearch.op("minComputeDetailsSearch", "cpuConstraintMinComputeNull", minComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("cpuNumber", serviceOfferingSearch.entity().getCpu(), Op.LTEQ).cp(); + serviceOfferingSearch.or("minComputeDetailsSearch", "cpuNumber", minComputeDetailsSearch.entity().getValue(), Op.LTEQ).cp(); + serviceOfferingSearch.and().op().op("maxComputeDetailsSearch", "cpuConstraintMaxComputeNull", maxComputeDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("cpuNumber", serviceOfferingSearch.entity().getCpu(), Op.GTEQ).cp(); + serviceOfferingSearch.or("maxComputeDetailsSearch", "cpuNumber", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + serviceOfferingSearch.cp().cp(); } if (memory != null) { SearchBuilder maxMemoryDetailsSearch = (SearchBuilder) serviceOfferingSearch.getJoinSB("maxMemoryDetailsSearch"); if (maxMemoryDetailsSearch == null) { maxMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - maxMemoryDetailsSearch.and().op("name", maxMemoryDetailsSearch.entity().getName(), Op.EQ); - maxMemoryDetailsSearch.or("idNull", maxMemoryDetailsSearch.entity().getId(), Op.NULL); - maxMemoryDetailsSearch.cp(); - serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, - serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("maxMemoryDetailsSearch", maxMemoryDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), maxMemoryDetailsSearch.entity().getResourceId(), + maxMemoryDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxmemory")); } SearchBuilder minMemoryDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); - minMemoryDetailsSearch.and().op("name", minMemoryDetailsSearch.entity().getName(), Op.EQ); - minMemoryDetailsSearch.or("idNull", minMemoryDetailsSearch.entity().getId(), Op.NULL); - minMemoryDetailsSearch.cp(); - serviceOfferingSearch.join("minMemoryDetailsSearch", minMemoryDetailsSearch, - serviceOfferingSearch.entity().getId(), minMemoryDetailsSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + serviceOfferingSearch.join("minMemoryDetailsSearch", minMemoryDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), minMemoryDetailsSearch.entity().getResourceId(), + minMemoryDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("minmemory")); /* (min_ram_size IS NULL AND ram_size IS NULL AND max_ram_size IS NULL) OR (ram_size = X) OR (min_ram_size <= X AND max_ram_size >= X) */ + serviceOfferingSearch.and().op().op("minMemoryDetailsSearch", "memoryConstraintMinMemoryNull", minMemoryDetailsSearch.entity().getValue(), Op.NULL); - serviceOfferingSearch.and("maxMemoryDetailsSearch", "memoryConstraintMaxMemoryNull", maxMemoryDetailsSearch.entity().getValue(), Op.NULL); serviceOfferingSearch.and("memoryConstraintNull", serviceOfferingSearch.entity().getRamSize(), Op.NULL).cp(); - serviceOfferingSearch.or("memoryConstraintEQ", serviceOfferingSearch.entity().getRamSize(), Op.EQ); + serviceOfferingSearch.or().op("minMemoryDetailsSearch", "memoryConstraintMinMemoryNull", minMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("memory", serviceOfferingSearch.entity().getRamSize(), Op.GTEQ).cp(); + serviceOfferingSearch.or("memory", serviceOfferingSearch.entity().getRamSize(), Op.GTEQ); - serviceOfferingSearch.or().op("minMemoryDetailsSearch", "memoryConstraintMinMemoryLTEQ", minMemoryDetailsSearch.entity().getValue(), Op.LTEQ); - serviceOfferingSearch.and("maxMemoryDetailsSearch", "memoryConstraintMaxMemoryGTEQ", maxMemoryDetailsSearch.entity().getValue(), Op.GTEQ).cp(); - serviceOfferingSearch.cp(); + serviceOfferingSearch.or().op().op(); + serviceOfferingSearch.op("minMemoryDetailsSearch", "memoryConstraintMinMemoryNull", minMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("memory", serviceOfferingSearch.entity().getRamSize(), Op.LTEQ).cp(); + serviceOfferingSearch.or("minMemoryDetailsSearch", "memory", minMemoryDetailsSearch.entity().getValue(), Op.LTEQ).cp(); + serviceOfferingSearch.and().op().op("maxMemoryDetailsSearch", "memoryConstraintMaxMemoryNull", maxMemoryDetailsSearch.entity().getValue(), Op.NULL); + serviceOfferingSearch.and("memory", serviceOfferingSearch.entity().getRamSize(), Op.GTEQ).cp(); + serviceOfferingSearch.or("maxMemoryDetailsSearch", "memory", maxMemoryDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + serviceOfferingSearch.cp().cp(); } if (cpuSpeed != null) { @@ -3875,12 +3886,12 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! if (owner.getType() != Account.Type.ADMIN) { SearchBuilder srvOffrDomainDetailSearch = _srvOfferingDetailsDao.createSearchBuilder(); - srvOffrDomainDetailSearch.and().op("name", srvOffrDomainDetailSearch.entity().getName(), Op.EQ); - srvOffrDomainDetailSearch.or("nameNull", srvOffrDomainDetailSearch.entity().getName(), Op.NULL).cp(); - srvOffrDomainDetailSearch.and().op("value", srvOffrDomainDetailSearch.entity().getValue(), Op.IN); - srvOffrDomainDetailSearch.or("value", srvOffrDomainDetailSearch.entity().getValue(), Op.NULL); - srvOffrDomainDetailSearch.cp().done(); - serviceOfferingSearch.join("domainDetailSearchNormalUser", srvOffrDomainDetailSearch, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), JoinBuilder.JoinType.LEFT); + srvOffrDomainDetailSearch.and().op("domainIdIN", srvOffrDomainDetailSearch.entity().getValue(), Op.IN); + srvOffrDomainDetailSearch.or("idNull", srvOffrDomainDetailSearch.entity().getValue(), Op.NULL); + srvOffrDomainDetailSearch.cp(); + serviceOfferingSearch.join("domainDetailSearchNormalUser", srvOffrDomainDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), + srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("domainid")); } if (currentVmOffering != null) { @@ -3933,7 +3944,6 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic vmMemory = NumbersUtil.parseInt(details.get(ApiConstants.MEMORY), 0); } if (vmCpu != null && vmCpu > 0) { - sc.setJoinParameters("maxComputeDetailsSearch", "name", "maxcpunumber"); sc.setParameters("vmCpu", vmCpu); sc.setParameters("vmMaxComputeGTEQ", vmCpu); } @@ -3941,7 +3951,6 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic sc.setParameters("speedGTEQ", vmSpeed); } if (vmMemory != null && vmMemory > 0) { - sc.setJoinParameters("maxMemoryDetailsSearch", "name", "maxmemory"); sc.setParameters("vmMemory", vmMemory); sc.setParameters("vmMaxMemoryGTEQ", vmMemory); } @@ -3951,8 +3960,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic if ((!accountMgr.isNormalUser(caller.getId()) && !accountMgr.isDomainAdmin(caller.getId())) && caller.getType() != Account.Type.RESOURCE_DOMAIN_ADMIN) { if (domainId != null && accountName == null) { - sc.setParameters("domainDetailSearch", "name", "domainid"); - sc.setParameters("domainDetailSearch", "value", domainId); + sc.setJoinParameters("domainDetailSearch", "domainId", domainId); } } @@ -3980,14 +3988,13 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (vmTypeStr != null) { - sc.setParameters("vmType", vmTypeStr); + sc.setParameters("svmType", vmTypeStr); } useStorageType(sc, storageType); if (zoneId != null) { - sc.setJoinParameters("ZoneDetailSearch", "name", "zoneid"); - sc.setJoinParameters("ZoneDetailSearch", "value", zoneId); + sc.setJoinParameters("ZoneDetailSearch", "zoneId", zoneId); if (DataCenter.Type.Edge.equals(zone.getType())) { sc.setJoinParameters("diskOfferingSearch", "useLocalStorage", true); @@ -3995,35 +4002,35 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (cpuNumber != null) { - sc.setJoinParameters("maxComputeDetailsSearch", "name", "maxcpunumber"); - sc.setJoinParameters("minComputeDetailsSearch", "name", "mincpunumber"); - sc.setParameters("cpuConstraintEQ", cpuNumber); - sc.setParameters("cpuConstraintMinComputeLTEQ", cpuNumber); - sc.setParameters("cpuConstraintMaxComputeGTEQ", cpuNumber); + /* + and("cpuConstraintMinComputeGTEQ", entity().getCpu(), Op.GTEQ).cp(); + or("cpuConstraintGTEQ", entity().getCpu(), Op.GTEQ); + + or().op().op(); + + and("cpuConstraintMinComputeLTEQ", entity().getCpu(), Op.LTEQ).cp(); + or("minComputeDetailsSearch", "cpuConstraintMinComputeLTEQ", minComputeDetailsSearch.entity().getValue(), Op.LTEQ).cp(); + + and("cpuConstraintMaxComputeGTEQ", entity().getCpu(), Op.GTEQ).cp(); + or("maxComputeDetailsSearch", "cpuConstraintMaxComputeGTEQ", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); + cp().cp(); + */ + sc.setParameters("cpuNumber", cpuNumber); } if (memory != null) { - sc.setJoinParameters("maxMemoryDetailsSearch", "name", "maxmemory"); - sc.setJoinParameters("minMemoryDetailsSearch", "name", "minmemory"); - sc.setParameters("memoryConstraintEQ", memory); - sc.setParameters("memoryConstraintMinMemoryLTEQ", memory); - sc.setParameters("memoryConstraintMaxMemoryGTEQ", memory); + sc.setParameters("memory", memory); } if (cpuSpeed != null) { sc.setParameters("speedGTEQ", cpuSpeed); } - // Filter offerings that are not associated with caller's domain - // Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet! if (owner.getType() != Account.Type.ADMIN) { Domain callerDomain = _domainDao.findById(owner.getDomainId()); List domainIds = findRelatedDomainIds(callerDomain, isRecursive); - List ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds); - - sc.setJoinParameters("domainDetailSearchNormalUser", "name", "domainid"); - sc.setJoinParameters("domainDetailSearchNormalUser", "value", ids.toArray()); + sc.setJoinParameters("domainDetailSearchNormalUser", "domainIdIN", domainIds.toArray()); } if (currentVmOffering != null) { From 600e2703f85f16481ac1e462e8749028e170da36 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 13 Dec 2023 14:19:43 +0530 Subject: [PATCH 14/28] fix service_offering_view --- .../resources/META-INF/db/views/cloud.service_offering_view.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql index 63b0b044f7bf..da5172e39ccf 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql @@ -84,7 +84,7 @@ SELECT FROM `cloud`.`service_offering` INNER JOIN - `cloud`.`disk_offering` AS `disk_offering` ON service_offering.disk_offering_id = disk_offering.id + `cloud`.`disk_offering` ON service_offering.disk_offering_id = disk_offering.id AND `disk_offering`.`state`='Active' LEFT JOIN `cloud`.`service_offering_details` AS `domain_details` ON `domain_details`.`service_offering_id` = `service_offering`.`id` AND `domain_details`.`name`='domainid' LEFT JOIN From b36ff1ad34a6e3ee0ab71dd1ee29bb98f111caab Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 13 Dec 2023 14:30:55 +0530 Subject: [PATCH 15/28] Use constants instead of strings --- .../com/cloud/api/query/QueryManagerImpl.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 4e0ec0e7c1f9..a097310edc8e 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3353,7 +3353,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), - domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString("domainid")); + domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString(ApiConstants.DOMAIN_ID)); if (!isRootAdmin) { diskOfferingSearch.and("displayOffering", diskOfferingSearch.entity().getDisplayOffering(), Op.EQ); @@ -3417,7 +3417,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount diskOfferingSearch.join("zoneDetailSearch", zoneDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, diskOfferingSearch.entity().getId(), zoneDetailSearch.entity().getResourceId(), - zoneDetailSearch.entity().getName(), diskOfferingSearch.entity().setString("zoneid")); + zoneDetailSearch.entity().getName(), diskOfferingSearch.entity().setString(ApiConstants.ZONE_ID)); } DiskOffering currentDiskOffering = null; @@ -3446,7 +3446,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount diskOfferingSearch.join("domainDetailsSearch", domainDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, diskOfferingSearch.entity().getId(), domainDetailsSearch.entity().getResourceId(), - domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString("domainid")); + domainDetailsSearch.entity().getName(), diskOfferingSearch.entity().setString(ApiConstants.DOMAIN_ID)); } SearchCriteria sc = diskOfferingSearch.create(); @@ -3673,7 +3673,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), - maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxcpunumber")); + maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.MAX_CPU_NUMBER)); serviceOfferingSearch.and().op("vmCpu", serviceOfferingSearch.entity().getCpu(), Op.GTEQ); serviceOfferingSearch.or().op("vmCpuNull", serviceOfferingSearch.entity().getCpu(), Op.NULL); @@ -3733,7 +3733,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic srvOffrDomainDetailSearch.and("domainId", srvOffrDomainDetailSearch.entity().getValue(), Op.EQ); serviceOfferingSearch.join("domainDetailSearch", srvOffrDomainDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), - srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("domainid")); + srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.DOMAIN_ID)); } } @@ -3769,7 +3769,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic serviceOfferingSearch.join("ZoneDetailSearch", srvOffrZoneDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), srvOffrZoneDetailSearch.entity().getResourceId(), - srvOffrZoneDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("zoneid")); + srvOffrZoneDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.ZONE_ID)); zone = _dcJoinDao.findById(zoneId); } @@ -3797,14 +3797,14 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), maxComputeDetailsSearch.entity().getResourceId(), - maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("maxcpunumber")); + maxComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.MAX_CPU_NUMBER)); } SearchBuilder minComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); serviceOfferingSearch.join("minComputeDetailsSearch", minComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), minComputeDetailsSearch.entity().getResourceId(), - minComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString("mincpunumber")); + minComputeDetailsSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.MIN_CPU_NUMBER)); /* (min_cpu IS NULL AND cpu IS NULL AND max_cpu IS NULL) @@ -3891,7 +3891,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic srvOffrDomainDetailSearch.cp(); serviceOfferingSearch.join("domainDetailSearchNormalUser", srvOffrDomainDetailSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, serviceOfferingSearch.entity().getId(), srvOffrDomainDetailSearch.entity().getResourceId(), - srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString("domainid")); + srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.DOMAIN_ID)); } if (currentVmOffering != null) { From d8f25eba36a83b94a304ae57ad0fca64fb1ec801 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 13 Dec 2023 15:07:00 +0530 Subject: [PATCH 16/28] Make changes to prevent sql injection --- .../java/com/cloud/utils/db/Attribute.java | 9 --- .../com/cloud/utils/db/GenericDaoBase.java | 73 ++++++++++++++++--- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/Attribute.java b/framework/db/src/main/java/com/cloud/utils/db/Attribute.java index 92f4e19cf50f..3e5128d97b45 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/Attribute.java +++ b/framework/db/src/main/java/com/cloud/utils/db/Attribute.java @@ -223,15 +223,6 @@ public Object getValue() { return value; } - public String getValueToSql() { - if (value instanceof String) { - return "'" + value + "'"; - } else if (value instanceof Long) { - return value.toString(); - } - throw new IllegalArgumentException("Unsupported type " + value.getClass()); - } - public Object get(Object entity) { try { return field.get(entity); diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 2974896d5959..3cee788ea984 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -56,6 +56,7 @@ import javax.persistence.Table; import javax.persistence.TableGenerator; +import com.amazonaws.util.CollectionUtils; import org.apache.log4j.Logger; import com.cloud.utils.DateUtil; @@ -373,10 +374,11 @@ public List searchIncludingRemoved(SearchCriteria sc, final Filter filter, } Collection>> joins = null; + List joinAttrList = null; if (sc != null) { joins = sc.getJoins(); if (joins != null) { - addJoins(str, joins); + joinAttrList = addJoins(str, joins); } } @@ -396,6 +398,13 @@ public List searchIncludingRemoved(SearchCriteria sc, final Filter filter, try { pstmt = txn.prepareAutoCloseStatement(sql); int i = 1; + + if (!CollectionUtils.isNullOrEmpty(joinAttrList)) { + for (Attribute attr : joinAttrList) { + prepareAttribute(i++, pstmt, attr, null); + } + } + if (clause != null) { for (final Pair value : sc.getValues()) { prepareAttribute(i++, pstmt, value.first(), value.second()); @@ -448,8 +457,9 @@ public List customSearchIncludingRemoved(SearchCriteria sc, final Filt Collection>> joins = null; joins = sc.getJoins(); + List joinAttrList = null; if (joins != null) { - addJoins(str, joins); + joinAttrList = addJoins(str, joins); } List groupByValues = addGroupBy(str, sc); @@ -462,6 +472,13 @@ public List customSearchIncludingRemoved(SearchCriteria sc, final Filt try { pstmt = txn.prepareAutoCloseStatement(sql); int i = 1; + + if (!CollectionUtils.isNullOrEmpty(joinAttrList)) { + for (Attribute attr : joinAttrList) { + prepareAttribute(i++, pstmt, attr, null); + } + } + if (clause != null) { for (final Pair value : sc.getValues()) { prepareAttribute(i++, pstmt, value.first(), value.second()); @@ -1272,12 +1289,13 @@ protected StringBuilder createPartialSelectSql(SearchCriteria sc, final boole } @DB() - protected void addJoins(StringBuilder str, Collection>> joins) { - addJoins(str, joins, new HashMap<>()); + protected List addJoins(StringBuilder str, Collection>> joins) { + return addJoins(str, joins, new HashMap<>()); } @DB() - protected void addJoins(StringBuilder str, Collection>> joins, Map joinedTableNames) { + protected List addJoins(StringBuilder str, Collection>> joins, Map joinedTableNames) { + List joinAttrList = new ArrayList<>(); boolean hasWhereClause = true; int fromIndex = str.lastIndexOf("WHERE"); if (fromIndex == -1) { @@ -1304,7 +1322,8 @@ protected void addJoins(StringBuilder str, Collection> join : joins) { if (join.getT().getJoins() != null) { - addJoins(str, join.getT().getJoins(), joinedTableNames); + joinAttrList.addAll(addJoins(str, join.getT().getJoins(), joinedTableNames)); } } + return joinAttrList; } protected static String findNextJoinTableName(String tableName, Map usedTableNames) { @@ -1608,7 +1629,11 @@ protected void prepareAttribute(final int j, final PreparedStatement pstmt, fina return; } } - if (attr.field.getType() == String.class) { + if (attr.getValue() != null && attr.getValue() instanceof String) { + pstmt.setString(j, (String)attr.getValue()); + } else if (attr.getValue() != null && attr.getValue() instanceof Long) { + pstmt.setLong(j, (Long)attr.getValue()); + } else if (attr.field.getType() == String.class) { final String str; try { str = (String) value; @@ -2041,10 +2066,11 @@ public Integer getDistinctCountIncludingRemoved(SearchCriteria sc) { } Collection>> joins = null; + List joinAttrList = null; if (sc != null) { joins = sc.getJoins(); if (joins != null) { - addJoins(str, joins); + joinAttrList = addJoins(str, joins); } } @@ -2057,6 +2083,13 @@ public Integer getDistinctCountIncludingRemoved(SearchCriteria sc) { try { pstmt = txn.prepareAutoCloseStatement(sql); int i = 1; + + if (!CollectionUtils.isNullOrEmpty(joinAttrList)) { + for (Attribute attr : joinAttrList) { + prepareAttribute(i++, pstmt, attr, null); + } + } + if (clause != null) { for (final Pair value : sc.getValues()) { prepareAttribute(i++, pstmt, value.first(), value.second()); @@ -2104,10 +2137,11 @@ public Integer getDistinctCountIncludingRemoved(SearchCriteria sc, String[] d } Collection>> joins = null; + List joinAttrList = null; if (sc != null) { joins = sc.getJoins(); if (joins != null) { - addJoins(str, joins); + joinAttrList = addJoins(str, joins); } } @@ -2116,6 +2150,13 @@ public Integer getDistinctCountIncludingRemoved(SearchCriteria sc, String[] d try (PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) { int i = 1; + + if (!CollectionUtils.isNullOrEmpty(joinAttrList)) { + for (Attribute attr : joinAttrList) { + prepareAttribute(i++, pstmt, attr, null); + } + } + if (clause != null) { for (final Pair value : sc.getValues()) { prepareAttribute(i++, pstmt, value.first(), value.second()); @@ -2159,10 +2200,11 @@ public Integer getCountIncludingRemoved(SearchCriteria sc) { } Collection>> joins = null; + List joinAttrList = null; if (sc != null) { joins = sc.getJoins(); if (joins != null) { - addJoins(str, joins); + joinAttrList = addJoins(str, joins); } } @@ -2173,6 +2215,13 @@ public Integer getCountIncludingRemoved(SearchCriteria sc) { try { pstmt = txn.prepareAutoCloseStatement(sql); int i = 1; + + if (!CollectionUtils.isNullOrEmpty(joinAttrList)) { + for (Attribute attr : joinAttrList) { + prepareAttribute(i++, pstmt, attr, null); + } + } + if (clause != null) { for (final Pair value : sc.getValues()) { prepareAttribute(i++, pstmt, value.first(), value.second()); From 48c3325e5e41644858ec494185892f64485c328e Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 13 Dec 2023 16:03:38 +0530 Subject: [PATCH 17/28] Remove commented code --- .../java/com/cloud/api/query/QueryManagerImpl.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index a097310edc8e..d94acfb1b573 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -4002,19 +4002,6 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (cpuNumber != null) { - /* - and("cpuConstraintMinComputeGTEQ", entity().getCpu(), Op.GTEQ).cp(); - or("cpuConstraintGTEQ", entity().getCpu(), Op.GTEQ); - - or().op().op(); - - and("cpuConstraintMinComputeLTEQ", entity().getCpu(), Op.LTEQ).cp(); - or("minComputeDetailsSearch", "cpuConstraintMinComputeLTEQ", minComputeDetailsSearch.entity().getValue(), Op.LTEQ).cp(); - - and("cpuConstraintMaxComputeGTEQ", entity().getCpu(), Op.GTEQ).cp(); - or("maxComputeDetailsSearch", "cpuConstraintMaxComputeGTEQ", maxComputeDetailsSearch.entity().getValue(), Op.GTEQ).cp(); - cp().cp(); - */ sc.setParameters("cpuNumber", cpuNumber); } From ec7a9e0f766599d37242a6e4eff53e2096c7c469 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 14 Dec 2023 11:25:47 +0530 Subject: [PATCH 18/28] Prevent n+1 queries for template's response --- .../storage/datastore/db/ImageStoreDao.java | 2 ++ .../datastore/db/ImageStoreDaoImpl.java | 17 +++++++++++++++++ .../datastore/db/PrimaryDataStoreDao.java | 2 ++ .../datastore/db/PrimaryDataStoreDaoImpl.java | 18 ++++++++++++++++++ .../api/query/dao/TemplateJoinDaoImpl.java | 11 +++++++++-- 5 files changed, 48 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java index 1c31b3e0cc4b..7aab5bbf7b31 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java @@ -51,4 +51,6 @@ public interface ImageStoreDao extends GenericDao { ImageStoreVO findOneByZoneAndProtocol(long zoneId, String protocol); List listImageStoresByZoneIds(Long... zoneIds); + + List listByIds(List ids); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index a4827a1beae4..dd41869d9580 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -18,12 +18,15 @@ */ package org.apache.cloudstack.storage.datastore.db; +import java.util.Collections; import java.util.List; import java.util.Map; import javax.naming.ConfigurationException; import com.cloud.utils.db.Filter; +import com.cloud.utils.db.SearchBase; +import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; @@ -44,6 +47,7 @@ public class ImageStoreDaoImpl extends GenericDaoBase implem private SearchBuilder zoneProtocolSearch; private SearchBuilder zonesInSearch; + private SearchBuilder IdsSearch; public ImageStoreDaoImpl() { super(); @@ -62,6 +66,9 @@ public ImageStoreDaoImpl() { zonesInSearch.and("zonesIn", zonesInSearch.entity().getDcId(), SearchCriteria.Op.IN); zonesInSearch.done(); + IdsSearch = createSearchBuilder(); + IdsSearch.and("ids", IdsSearch.entity().getId(), SearchCriteria.Op.IN); + IdsSearch.done(); } @Override public boolean configure(String name, Map params) throws ConfigurationException { @@ -206,4 +213,14 @@ public List listImageStoresByZoneIds(Long... zoneIds) { sc.setParametersIfNotNull("zonesIn", zoneIds); return listBy(sc); } + + @Override + public List listByIds(List ids) { + if (CollectionUtils.isEmpty(ids)) { + return Collections.emptyList(); + } + SearchCriteria sc = IdsSearch.create(); + sc.setParameters("ids", ids.toArray()); + return listBy(sc); + } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java index cf3e35ce4aa1..3c4c54bceb94 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java @@ -139,4 +139,6 @@ public interface PrimaryDataStoreDao extends GenericDao { Pair, Integer> searchForIdsAndCount(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword, Filter searchFilter); + + List listByIds(List ids); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java index 3ba24e96774d..9a02ca87fc92 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java @@ -20,6 +20,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -29,6 +30,7 @@ import com.cloud.utils.Pair; import com.cloud.utils.db.Filter; +import com.cloud.utils.db.GenericQueryBuilder; import org.apache.commons.collections.CollectionUtils; import com.cloud.host.Status; @@ -60,6 +62,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase private final SearchBuilder DcLocalStorageSearch; private final GenericSearchBuilder StatusCountSearch; private final SearchBuilder ClustersSearch; + private final SearchBuilder IdsSearch; @Inject private StoragePoolDetailsDao _detailsDao; @@ -146,6 +149,11 @@ public PrimaryDataStoreDaoImpl() { ClustersSearch = createSearchBuilder(); ClustersSearch.and("clusterIds", ClustersSearch.entity().getClusterId(), Op.IN); ClustersSearch.and("status", ClustersSearch.entity().getStatus(), Op.EQ); + ClustersSearch.done(); + + IdsSearch = createSearchBuilder(); + IdsSearch.and("ids", IdsSearch.entity().getId(), SearchCriteria.Op.IN); + IdsSearch.done(); } @@ -659,6 +667,16 @@ public Pair, Integer> searchForIdsAndCount(Long storagePoolId, String return new Pair<>(idList, uniquePair.second()); } + @Override + public List listByIds(List ids) { + if (CollectionUtils.isEmpty(ids)) { + return Collections.emptyList(); + } + SearchCriteria sc = IdsSearch.create(); + sc.setParameters("ids", ids.toArray()); + return listBy(sc); + } + private SearchCriteria createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName, Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType, StoragePoolStatus status, String keyword) { diff --git a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java index 501d413f117b..5a0c19955bda 100644 --- a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java @@ -198,11 +198,15 @@ public TemplateResponse newTemplateResponse(EnumSet List storesInZone = dataStoreDao.listStoresByZoneId(template.getDataCenterId()); Long[] storeIds = storesInZone.stream().map(ImageStoreVO::getId).toArray(Long[]::new); List templatesInStore = _templateStoreDao.listByTemplateNotBypassed(template.getId(), storeIds); + + List dataStoreIdList = templatesInStore.stream().map(TemplateDataStoreVO::getDataStoreId).collect(Collectors.toList()); + Map imageStoreMap = dataStoreDao.listByIds(dataStoreIdList).stream().collect(Collectors.toMap(ImageStoreVO::getId, imageStore -> imageStore)); + List> downloadProgressDetails = new ArrayList<>(); HashMap downloadDetailInImageStores = null; for (TemplateDataStoreVO templateInStore : templatesInStore) { downloadDetailInImageStores = new HashMap<>(); - ImageStoreVO imageStore = dataStoreDao.findById(templateInStore.getDataStoreId()); + ImageStoreVO imageStore = imageStoreMap.get(templateInStore.getDataStoreId()); if (imageStore != null) { downloadDetailInImageStores.put("datastore", imageStore.getName()); if (view.equals(ResponseView.Full)) { @@ -219,9 +223,12 @@ public TemplateResponse newTemplateResponse(EnumSet List poolIds = poolsInZone.stream().map(StoragePoolVO::getId).collect(Collectors.toList()); List templatesInPool = templatePoolDao.listByTemplateId(template.getId(), poolIds); + dataStoreIdList = templatesInStore.stream().map(TemplateDataStoreVO::getDataStoreId).collect(Collectors.toList()); + Map storagePoolMap = primaryDataStoreDao.listByIds(dataStoreIdList).stream().collect(Collectors.toMap(StoragePoolVO::getId, store -> store)); + for (VMTemplateStoragePoolVO templateInPool : templatesInPool) { downloadDetailInImageStores = new HashMap<>(); - StoragePoolVO storagePool = primaryDataStoreDao.findById(templateInPool.getDataStoreId()); + StoragePoolVO storagePool = storagePoolMap.get(templateInPool.getDataStoreId()); if (storagePool != null) { downloadDetailInImageStores.put("datastore", storagePool.getName()); if (view.equals(ResponseView.Full)) { From 5ccd87cc07448b7cf42dcbe1f060c3e36384aeb9 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 15 Dec 2023 15:02:36 +0530 Subject: [PATCH 19/28] remove unused import --- .../cloudstack/storage/datastore/db/ImageStoreDaoImpl.java | 1 - .../cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java | 1 - 2 files changed, 2 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index dd41869d9580..84b88c215ca6 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -25,7 +25,6 @@ import javax.naming.ConfigurationException; import com.cloud.utils.db.Filter; -import com.cloud.utils.db.SearchBase; import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java index 9a02ca87fc92..c7a2a4d6d69f 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java @@ -30,7 +30,6 @@ import com.cloud.utils.Pair; import com.cloud.utils.db.Filter; -import com.cloud.utils.db.GenericQueryBuilder; import org.apache.commons.collections.CollectionUtils; import com.cloud.host.Status; From 616f934429a98a33852ca7dd03625ddb9b14db7a Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 15 Dec 2023 17:26:59 +0530 Subject: [PATCH 20/28] refactor some code --- .../com/cloud/offering/ServiceOffering.java | 2 +- .../com/cloud/service/ServiceOfferingVO.java | 16 ++-- .../java/com/cloud/utils/db/SearchBase.java | 83 ++++++++++--------- .../com/cloud/utils/db/SearchCriteria.java | 24 +----- .../cloud/utils/db/GenericDaoBaseTest.java | 15 +++- .../com/cloud/api/query/QueryManagerImpl.java | 2 +- .../ConfigurationManagerImpl.java | 2 +- .../com/cloud/network/NetworkServiceImpl.java | 4 +- .../cloud/network/NetworkServiceImplTest.java | 4 +- 9 files changed, 69 insertions(+), 83 deletions(-) diff --git a/api/src/main/java/com/cloud/offering/ServiceOffering.java b/api/src/main/java/com/cloud/offering/ServiceOffering.java index 278acbe231ef..58c7b0dbaf96 100644 --- a/api/src/main/java/com/cloud/offering/ServiceOffering.java +++ b/api/src/main/java/com/cloud/offering/ServiceOffering.java @@ -102,7 +102,7 @@ enum StorageType { boolean getDefaultUse(); - String getSystemVmType(); + String getVmType(); String getDeploymentPlanner(); diff --git a/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java b/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java index 125458650096..7f5c1a7afa19 100644 --- a/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java @@ -110,7 +110,7 @@ public class ServiceOfferingVO implements ServiceOffering { private boolean defaultUse; @Column(name = "vm_type") - private String systemVmType; + private String vmType; @Column(name = "sort_key") int sortKey; @@ -140,7 +140,7 @@ protected ServiceOfferingVO() { } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, String displayText, - boolean systemUse, VirtualMachine.Type systemVmType, boolean defaultUse) { + boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { this.cpu = cpu; this.ramSize = ramSize; this.speed = speed; @@ -150,7 +150,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee limitCpuUse = false; volatileVm = false; this.defaultUse = defaultUse; - this.systemVmType = systemVmType == null ? null : systemVmType.toString().toLowerCase(); + this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); uuid = UUID.randomUUID().toString(); this.systemUse = systemUse; this.name = name; @@ -159,7 +159,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, boolean volatileVm, String displayText, boolean systemUse, - VirtualMachine.Type systemVmType, String hostTag, String deploymentPlanner, boolean dynamicScalingEnabled, boolean isCustomized) { + VirtualMachine.Type vmType, String hostTag, String deploymentPlanner, boolean dynamicScalingEnabled, boolean isCustomized) { this.cpu = cpu; this.ramSize = ramSize; this.speed = speed; @@ -168,7 +168,7 @@ public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer spee this.offerHA = offerHA; this.limitCpuUse = limitResourceUse; this.volatileVm = volatileVm; - this.systemVmType = systemVmType == null ? null : systemVmType.toString().toLowerCase(); + this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); this.hostTag = hostTag; this.deploymentPlanner = deploymentPlanner; uuid = UUID.randomUUID().toString(); @@ -194,7 +194,7 @@ public ServiceOfferingVO(ServiceOfferingVO offering) { limitCpuUse = offering.getLimitCpuUse(); volatileVm = offering.isVolatileVm(); hostTag = offering.getHostTag(); - systemVmType = offering.getSystemVmType(); + vmType = offering.getVmType(); systemUse = offering.isSystemUse(); dynamicScalingEnabled = offering.isDynamicScalingEnabled(); diskOfferingStrictness = offering.diskOfferingStrictness; @@ -278,8 +278,8 @@ public String getHostTag() { } @Override - public String getSystemVmType() { - return systemVmType; + public String getVmType() { + return vmType; } @Override diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index e2ded86bca97..150d9457d3f4 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -205,32 +205,29 @@ public J selectFields(final Object... fields) { */ @SuppressWarnings("unchecked") public J join(final String name, final SearchBase builder, final Object joinField1, final Object joinField2, final JoinBuilder.JoinType joinType) { - assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert _specifiedAttrs.size() == 1 : "You didn't select the attribute."; - assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; - assert builder != this : "You can't add yourself, can you? Really think about it!"; - - final JoinBuilder> t = new JoinBuilder>(name, builder, new Attribute[]{_specifiedAttrs.get(0)}, new Attribute[]{builder._specifiedAttrs.get(0)}, joinType); - if (_joins == null) { - _joins = new HashMap>>(); - } - _joins.put(name, t); + if (_specifiedAttrs.size() != 1) + throw new CloudRuntimeException("You didn't select the attribute."); + if (builder._specifiedAttrs.size() != 1) + throw new CloudRuntimeException("You didn't select the attribute."); - builder._specifiedAttrs.clear(); - _specifiedAttrs.clear(); - return (J)this; + return join(name, builder, joinType, null, joinField1, joinField2); } public J join(final String name, final SearchBase builder, final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields) { - assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert !_specifiedAttrs.isEmpty() : "You didn't select the attribute."; - assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert !builder._specifiedAttrs.isEmpty() : "You didn't select the attribute."; - assert builder != this : "You can't add yourself, can you? Really think about it!"; - assert _specifiedAttrs.size() == builder._specifiedAttrs.size() : "You didn't select the same number of attributes."; + if (_entity == null) + throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); + if (_specifiedAttrs.isEmpty()) + throw new CloudRuntimeException("Attribute not specified."); + if (builder._entity == null) + throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); + if (builder._specifiedAttrs.isEmpty()) + throw new CloudRuntimeException("Attribute not specified."); + if (builder == this) + throw new CloudRuntimeException("Can't join with itself. Create a new SearchBuilder for the same entity and use that."); + if (_specifiedAttrs.size() != builder._specifiedAttrs.size()) + throw new CloudRuntimeException("Number of attributes to join on must be the same."); final JoinBuilder> t = new JoinBuilder<>(name, builder, _specifiedAttrs.toArray(new Attribute[0]), builder._specifiedAttrs.toArray(new Attribute[0]), joinType, condition); @@ -248,13 +245,14 @@ public J join(final String name, final SearchBase builder, final JoinBu /** * This method is used to set alias for the table name when joining with another table. * Allows to set alias for the table name which is further used to generate the condition for nested join. + *
* e.g. vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); - * + *
* volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); - * - * In the above example, vmSearch is the alias for the table name vm_instance and the query generated is - * FROM volume - * LEFT JOIN vm_instance vmSearch ON vmSearch.id = volume.instance_id + *
+ * In the above example, vmSearch is the alias for the table name vm_instance and the query generated is
+ * FROM volume
+ * LEFT JOIN vm_instance vmSearch ON vmSearch.id = volume.instance_id
* LEFT JOIN service_offering serviceOfferingSearch ON serviceOfferingSearch.id = vmSearch.service_offering_id */ @SuppressWarnings("unchecked") @@ -262,11 +260,16 @@ public J join( final String tableAliasName, final String name, final SearchBase builder, final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields ) { - assert _entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert _specifiedAttrs.size() == 1 : "You didn't select the attribute."; - assert builder._entity != null : "SearchBuilder cannot be modified once it has been setup"; - assert builder._specifiedAttrs.size() == 1 : "You didn't select the attribute."; - assert builder != this : "You can't add yourself, can you? Really think about it!"; + if (_entity == null) + throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); + if (_specifiedAttrs.size() != 1) + throw new CloudRuntimeException("Attribute not specified."); + if (builder._entity == null) + throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); + if (builder._specifiedAttrs.size() != 1) + throw new CloudRuntimeException("Attribute not specified."); + if (builder == this) + throw new CloudRuntimeException("Can't join with itself. Create a new SearchBuilder for the same entity and use that."); for (final Attribute attr : _specifiedAttrs) { if (attr.table != null) { @@ -479,17 +482,7 @@ public Object[] getPresets() { return presets; } - public void toSql(final StringBuilder sql, final Object[] params, final int count) { - String tableAlias = null; - if (joinName != null) { - tableAlias = joinName; - } else if (attr != null) { - tableAlias = attr.table; - } - toSql(sql, tableAlias, params, count); - } - - public void toSql(final StringBuilder sql, final String tableAlias, final Object[] params, final int count) { + public void toSql(final StringBuilder sql, String tableAlias, final Object[] params, final int count) { if (count > 0) { sql.append(cond); } @@ -511,6 +504,14 @@ public void toSql(final StringBuilder sql, final String tableAlias, final Object sql.append(" FIND_IN_SET(?, "); } + if (tableAlias == null) { + if (joinName != null) { + tableAlias = joinName; + } else { + tableAlias = attr.table; + } + } + sql.append(tableAlias).append(".").append(attr.columnName).append(op.toString()); if (op == Op.IN && params.length == 1) { sql.delete(sql.length() - op.toString().length(), sql.length()); diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java index 72dc7996860a..8affbd5300aa 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchCriteria.java @@ -277,29 +277,7 @@ public String getWhereClause(String tableAlias) { } public String getWhereClause() { - StringBuilder sql = new StringBuilder(); - int i = 0; - for (Condition condition : _conditions) { - if (condition.isPreset()) { - _params.put(condition.name, condition.presets); - } - Object[] params = _params.get(condition.name); - if ((condition.op == null || condition.op.params == 0) || (params != null)) { - condition.toSql(sql, params, i++); - } - } - - for (Condition condition : _additionals) { - if (condition.isPreset()) { - _params.put(condition.name, condition.presets); - } - Object[] params = _params.get(condition.name); - if ((condition.op.params == 0) || (params != null)) { - condition.toSql(sql, params, i++); - } - } - - return sql.toString(); + return getWhereClause(null); } public List> getValues() { diff --git a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java index 15d6c8901591..11983a7a2064 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java @@ -229,12 +229,16 @@ public void addJoinsTest() { Attribute attr2 = new Attribute("table2", "column2"); Attribute attr3 = new Attribute("table3", "column1"); Attribute attr4 = new Attribute("table4", "column2"); + Attribute attr5 = new Attribute("table3", "column1"); + Attribute attr6 = new Attribute("XYZ"); joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), attr1, attr2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), attr3, attr4, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), + new Attribute[]{attr3, attr5}, new Attribute[]{attr4, attr6}, JoinBuilder.JoinType.INNER, JoinBuilder.JoinCondition.OR)); dbTestDao.addJoins(joinString, joins); - Assert.assertEquals(" INNER JOIN table2 ON table1.column1=table2.column2 INNER JOIN table4 ON table3.column1=table4.column2 ", joinString.toString()); + Assert.assertEquals(" INNER JOIN table2 ON table1.column1=table2.column2 " + + " INNER JOIN table4 ON table3.column1=table4.column2 OR table3.column1=? ", joinString.toString()); } @Test @@ -248,15 +252,18 @@ public void multiJoinSameTableTest() { Attribute tBc2 = new Attribute("tableB", "column2"); Attribute tCc3 = new Attribute("tableC", "column3"); Attribute tDc4 = new Attribute("tableD", "column4"); + Attribute tDc5 = new Attribute("tableD", "column5"); + Attribute attr = new Attribute(123); joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>("tableAlias", dbTestDao.createSearchCriteria(), tDc4, tAc3, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("tableAlias", dbTestDao.createSearchCriteria(), + new Attribute[]{tDc4, tDc5}, new Attribute[]{tAc3, attr}, JoinBuilder.JoinType.INNER, JoinBuilder.JoinCondition.AND)); dbTestDao.addJoins(joinString, joins); Assert.assertEquals(" INNER JOIN tableA ON tableB.column2=tableA.column1 " + " INNER JOIN tableA tableA1 ON tableC.column3=tableA1.column2 " + - " INNER JOIN tableA tableAlias ON tableD.column4=tableAlias.column3 ", joinString.toString()); + " INNER JOIN tableA tableAlias ON tableD.column4=tableAlias.column3 AND tableD.column5=? ", joinString.toString()); } @Test diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d94acfb1b573..65be4174704c 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3758,7 +3758,7 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } if (vmTypeStr != null) { - serviceOfferingSearch.and("svmType", serviceOfferingSearch.entity().getSystemVmType(), SearchCriteria.Op.EQ); + serviceOfferingSearch.and("svmType", serviceOfferingSearch.entity().getVmType(), SearchCriteria.Op.EQ); } DataCenterJoinVO zone = null; if (zoneId != null) { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 94d992f86369..5ad1e7549173 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -7436,7 +7436,7 @@ public Integer getServiceOfferingNetworkRate(final long serviceOfferingId, final networkRate = offering.getRateMbps(); } else { // for domain router service offering, get network rate from - if (offering.getSystemVmType() != null && offering.getSystemVmType().equalsIgnoreCase(VirtualMachine.Type.DomainRouter.toString())) { + if (offering.getVmType() != null && offering.getVmType().equalsIgnoreCase(VirtualMachine.Type.DomainRouter.toString())) { networkRate = NetworkOrchestrationService.NetworkThrottlingRate.valueIn(dataCenterId); } else { networkRate = Integer.parseInt(_configDao.getValue(Config.VmNetworkThrottlingRate.key())); diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 18f98e6f99f1..ca1967a07e13 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -4286,9 +4286,9 @@ public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(final } final String virtualMachineDomainRouterType = VirtualMachine.Type.DomainRouter.toString(); - if (!virtualMachineDomainRouterType.equalsIgnoreCase(serviceOffering.getSystemVmType())) { + if (!virtualMachineDomainRouterType.equalsIgnoreCase(serviceOffering.getVmType())) { throw new InvalidParameterValueException(String.format("The specified service offering [%s] is of type [%s]. Virtual routers can only be created with service offering " - + "of type [%s].", serviceOffering, serviceOffering.getSystemVmType(), virtualMachineDomainRouterType.toLowerCase())); + + "of type [%s].", serviceOffering, serviceOffering.getVmType(), virtualMachineDomainRouterType.toLowerCase())); } } diff --git a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java index c993f7b7095e..c1e95874d73f 100644 --- a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java +++ b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java @@ -789,10 +789,10 @@ public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMu public void validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMustThrowInvalidParameterValueExceptionWhenSystemVmTypeIsNotDomainRouter() { doReturn(serviceOfferingVoMock).when(serviceOfferingDaoMock).findById(anyLong()); doReturn(ServiceOffering.State.Active).when(serviceOfferingVoMock).getState(); - doReturn(VirtualMachine.Type.ElasticLoadBalancerVm.toString()).when(serviceOfferingVoMock).getSystemVmType(); + doReturn(VirtualMachine.Type.ElasticLoadBalancerVm.toString()).when(serviceOfferingVoMock).getVmType(); String expectedMessage = String.format("The specified service offering [%s] is of type [%s]. Virtual routers can only be created with service offering of type [%s].", - serviceOfferingVoMock, serviceOfferingVoMock.getSystemVmType(), VirtualMachine.Type.DomainRouter.toString().toLowerCase()); + serviceOfferingVoMock, serviceOfferingVoMock.getVmType(), VirtualMachine.Type.DomainRouter.toString().toLowerCase()); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedException, () -> { service.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l); }); From 8669e701348bacb80a73e9ceb9992dc6620e864d Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 18 Dec 2023 15:41:53 +0530 Subject: [PATCH 21/28] Add missing check for service offering's join with disk offering --- .../main/java/com/cloud/utils/db/SearchBase.java | 14 ++++++++++++-- .../java/com/cloud/api/query/QueryManagerImpl.java | 4 +++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index 150d9457d3f4..012e61aa1fc7 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -203,7 +203,6 @@ public J selectFields(final Object... fields) { * @param joinType type of join * @return itself */ - @SuppressWarnings("unchecked") public J join(final String name, final SearchBase builder, final Object joinField1, final Object joinField2, final JoinBuilder.JoinType joinType) { if (_specifiedAttrs.size() != 1) throw new CloudRuntimeException("You didn't select the attribute."); @@ -214,6 +213,18 @@ public J join(final String name, final SearchBase builder, final Object } + /** + * joins this search with another search with multiple conditions in the join clause + * + * @param name name given to the other search. used for setJoinParameters. + * @param builder The other search + * @param joinType type of join + * @param condition condition to be used for multiple conditions in the join clause + * @param joinFields fields the first and second table used to perform the join. + * The fields should be in the order of the checks between the two tables. + * + * @return + */ public J join(final String name, final SearchBase builder, final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields) { if (_entity == null) @@ -255,7 +266,6 @@ public J join(final String name, final SearchBase builder, final JoinBu * LEFT JOIN vm_instance vmSearch ON vmSearch.id = volume.instance_id
* LEFT JOIN service_offering serviceOfferingSearch ON serviceOfferingSearch.id = vmSearch.service_offering_id */ - @SuppressWarnings("unchecked") public J join( final String tableAliasName, final String name, final SearchBase builder, final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 65be4174704c..537cc4b09c1c 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3788,7 +3788,9 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic } } - serviceOfferingSearch.join("diskOfferingSearch", diskOfferingSearch, serviceOfferingSearch.entity().getDiskOfferingId(), diskOfferingSearch.entity().getId(), JoinBuilder.JoinType.INNER); + serviceOfferingSearch.join("diskOfferingSearch", diskOfferingSearch, JoinBuilder.JoinType.INNER, JoinBuilder.JoinCondition.AND, + serviceOfferingSearch.entity().getDiskOfferingId(), diskOfferingSearch.entity().getId(), + serviceOfferingSearch.entity().setString("Active"), diskOfferingSearch.entity().getState()); } if (cpuNumber != null) { From 5f09c54d62744238000ac667eded22507c18c32d Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 21 Dec 2023 21:03:14 +0530 Subject: [PATCH 22/28] Fix n+1 queries for stoage pool metrics --- .../db/src/main/java/com/cloud/utils/db/GenericDao.java | 2 ++ .../src/main/java/com/cloud/utils/db/GenericDaoBase.java | 7 +++++++ .../apache/cloudstack/metrics/MetricsServiceImpl.java | 9 ++++----- .../main/java/com/cloud/api/query/QueryManagerImpl.java | 4 ++-- .../src/test/java/com/cloud/user/MockUsageEventDao.java | 5 +++++ 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index b2a9e6f733aa..2fc02301cb7a 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -286,4 +286,6 @@ public interface GenericDao { Pair, Integer> searchAndDistinctCount(final SearchCriteria sc, final Filter filter, final String[] distinctColumns); Integer countAll(); + + List findByUuids(String... uuidArray); } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 3cee788ea984..fc9b2c7cfb56 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -2183,6 +2183,13 @@ public Integer countAll() { return getCount(null); } + @Override + public List findByUuids(String... uuidArray) { + SearchCriteria sc = createSearchCriteria(); + sc.addAnd("uuid", SearchCriteria.Op.IN, uuidArray); + return search(sc, null); + } + public Integer getCount(SearchCriteria sc) { sc = checkAndSetRemovedIsNull(sc); return getCountIncludingRemoved(sc); diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java index 51c020fc2375..7b222a0dc546 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java @@ -27,9 +27,11 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.stream.Collectors; import javax.inject.Inject; +import com.cloud.dc.ClusterVO; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ListClustersMetricsCmd; import org.apache.cloudstack.api.ListDbMetricsCmd; @@ -631,6 +633,7 @@ public List listVmMetrics(List vmResponses) { @Override public List listStoragePoolMetrics(List poolResponses) { final List metricsResponses = new ArrayList<>(); + Map clusterUuidToIdMap = clusterDao.findByUuids(poolResponses.stream().map(StoragePoolResponse::getClusterId).toArray(String[]::new)).stream().collect(Collectors.toMap(ClusterVO::getUuid, ClusterVO::getId)); for (final StoragePoolResponse poolResponse: poolResponses) { StoragePoolMetricsResponse metricsResponse = new StoragePoolMetricsResponse(); @@ -640,11 +643,7 @@ public List listStoragePoolMetrics(List createStoragesPoolResponse(Pair response = new ListResponse<>(); List poolResponses = ViewResponseHelper.createStoragePoolResponse(storagePools.first().toArray(new StoragePoolJoinVO[storagePools.first().size()])); + Map poolUuidToIdMap = storagePools.first().stream().collect(Collectors.toMap(StoragePoolJoinVO::getUuid, StoragePoolJoinVO::getId)); for (StoragePoolResponse poolResponse : poolResponses) { DataStore store = dataStoreManager.getPrimaryDataStore(poolResponse.getId()); if (store != null) { @@ -2985,8 +2986,7 @@ private ListResponse createStoragesPoolResponse(Pair caps = driver.getCapabilities(); if (Storage.StoragePoolType.NetworkFilesystem.toString().equals(poolResponse.getType()) && HypervisorType.VMware.toString().equals(poolResponse.getHypervisor())) { - StoragePoolVO pool = storagePoolDao.findPoolByUUID(poolResponse.getId()); - StoragePoolDetailVO detail = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + StoragePoolDetailVO detail = _storagePoolDetailsDao.findDetail(poolUuidToIdMap.get(poolResponse.getId()), Storage.Capability.HARDWARE_ACCELERATION.toString()); if (detail != null) { caps.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), detail.getValue()); } diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java index 4639c5092490..52e1b1a02b49 100644 --- a/server/src/test/java/com/cloud/user/MockUsageEventDao.java +++ b/server/src/test/java/com/cloud/user/MockUsageEventDao.java @@ -297,6 +297,11 @@ public Integer countAll() { return null; } + @Override + public List findByUuids(String... uuids) { + return null; + } + @Override public List listLatestEvents(Date endDate) { return null; From a9edb5e6d0c495cfc383ff3f45aee267c6782527 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 22 Dec 2023 11:58:06 +0530 Subject: [PATCH 23/28] Remove n+1 queries from list accounts --- .../apache/cloudstack/acl/RoleService.java | 2 ++ .../apache/cloudstack/acl/dao/RoleDao.java | 2 ++ .../cloudstack/acl/dao/RoleDaoImpl.java | 19 ++++++++++++++ .../main/java/com/cloud/api/ApiDBUtils.java | 26 +++++++++++++++++++ .../cloud/api/query/ViewResponseHelper.java | 6 +---- .../cloudstack/acl/RoleManagerImpl.java | 23 ++++++++++++++++ 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java index 9becce643d40..c45152c4de99 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java +++ b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java @@ -40,6 +40,8 @@ public interface RoleService { */ Role findRole(Long id, boolean removePrivateRoles); + List findRoles(List ids, boolean removePrivateRoles); + Role findRole(Long id); Role createRole(String name, RoleType roleType, String description, boolean publicRole); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java index a776f7b6fe66..2d4151afc7db 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java @@ -37,4 +37,6 @@ public interface RoleDao extends GenericDao { Pair, Integer> findAllByRoleType(RoleType type, Long offset, Long limit, boolean showPrivateRole); Pair, Integer> listAllRoles(Long startIndex, Long limit, boolean showPrivateRole); + + List searchByIds(Long... ids); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java index 06d3108076aa..42e67365ec56 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java @@ -25,13 +25,18 @@ import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.RoleVO; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; +import java.util.Arrays; +import java.util.Collections; import java.util.List; @Component public class RoleDaoImpl extends GenericDaoBase implements RoleDao { + + private final SearchBuilder RoleByIdsSearch; private final SearchBuilder RoleByNameSearch; private final SearchBuilder RoleByTypeSearch; private final SearchBuilder RoleByNameAndTypeSearch; @@ -40,6 +45,10 @@ public class RoleDaoImpl extends GenericDaoBase implements RoleDao public RoleDaoImpl() { super(); + RoleByIdsSearch = createSearchBuilder(); + RoleByIdsSearch.and("idIN", RoleByIdsSearch.entity().getId(), SearchCriteria.Op.IN); + RoleByIdsSearch.done(); + RoleByNameSearch = createSearchBuilder(); RoleByNameSearch.and("roleName", RoleByNameSearch.entity().getName(), SearchCriteria.Op.LIKE); RoleByNameSearch.and("isPublicRole", RoleByNameSearch.entity().isPublicRole(), SearchCriteria.Op.EQ); @@ -116,6 +125,16 @@ public Pair, Integer> listAllRoles(Long startIndex, Long limit, boo return searchAndCount(sc, new Filter(RoleVO.class, "id", true, startIndex, limit)); } + @Override + public List searchByIds(Long... ids) { + if (ids == null || ids.length == 0) { + return Collections.emptyList(); + } + SearchCriteria sc = RoleByIdsSearch.create(); + sc.setParameters("idIN", ids); + return listBy(sc); + } + public void filterPrivateRolesIfNeeded(SearchCriteria sc, boolean showPrivateRole) { if (!showPrivateRole) { sc.setParameters("isPublicRole", true); diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 3cade046c747..97ecd982f53c 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -17,12 +17,15 @@ package com.cloud.api; import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.inject.Inject; @@ -2082,6 +2085,29 @@ public static AccountResponse newAccountResponse(ResponseView view, EnumSet newAccountResponses(ResponseView view, EnumSet details, AccountJoinVO... accounts) { + List responseList = new ArrayList<>(); + + List roleIdList = Arrays.stream(accounts).map(AccountJoinVO::getRoleId).collect(Collectors.toList()); + Map roleIdMap = s_roleService.findRoles(roleIdList, false).stream().collect(Collectors.toMap(Role::getId, Function.identity())); + + for (AccountJoinVO account : accounts) { + AccountResponse response = s_accountJoinDao.newAccountResponse(view, details, account); + // Populate account role information + if (account.getRoleId() != null) { + Role role = roleIdMap.get(account.getRoleId()); + if (role != null) { + response.setRoleId(role.getUuid()); + response.setRoleType(role.getRoleType()); + response.setRoleName(role.getName()); + } + } + responseList.add(response); + } + + return responseList; + } + public static AccountJoinVO newAccountView(Account e) { return s_accountJoinDao.newAccountView(e); } diff --git a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java index 44096a799b7a..623ba436d0e0 100644 --- a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java +++ b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java @@ -541,11 +541,7 @@ private static void setParentResourceLimitIfNeeded(Map createAccountResponse(ResponseView view, EnumSet details, AccountJoinVO... accounts) { - List respList = new ArrayList(); - for (AccountJoinVO vt : accounts){ - respList.add(ApiDBUtils.newAccountResponse(view, details, vt)); - } - return respList; + return ApiDBUtils.newAccountResponses(view, details, accounts); } public static List createAsyncJobResponse(AsyncJobJoinVO... jobs) { diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java index ff97d7ecf6fb..4caa1378e992 100644 --- a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java @@ -111,6 +111,29 @@ public Role findRole(Long id, boolean removePrivateRoles) { return role; } + @Override + public List findRoles(List ids, boolean removePrivateRoles) { + List result = new ArrayList<>(); + if (CollectionUtils.isEmpty(ids)) { + logger.trace(String.format("Role IDs are invalid [%s]", ids)); + return result; + } + + List roles = roleDao.searchByIds(ids.toArray(new Long[0])); + if (CollectionUtils.isEmpty(roles)) { + logger.trace(String.format("Roles not found [ids=%s]", ids)); + return result; + } + for (Role role : roles) { + if (!isCallerRootAdmin() && (RoleType.Admin == role.getRoleType() || (!role.isPublicRole() && removePrivateRoles))) { + logger.debug(String.format("Role [id=%s, name=%s] is either of 'Admin' type or is private and is only visible to 'Root admins'.", role.getId(), role.getName())); + continue; + } + result.add(role); + } + return result; + } + @Override public Role findRole(Long id) { return findRole(id, false); From de2a5b24574aa03193c657cae23f23c1a1b5fe1a Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 3 Jan 2024 12:00:31 +0530 Subject: [PATCH 24/28] Remove unused imports --- .../main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java index 42e67365ec56..2e8fdd5fcc20 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java @@ -25,11 +25,9 @@ import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.RoleVO; -import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; -import java.util.Arrays; import java.util.Collections; import java.util.List; From 7d89d4c21ff3458c60f3bb3b7c53ce44a2de41d7 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 4 Jan 2024 12:17:13 +0530 Subject: [PATCH 25/28] remove todo --- .../src/main/java/com/cloud/api/query/QueryManagerImpl.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 00be6cc5bfeb..04e6364ccea9 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3663,12 +3663,9 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic (service_offering.cpu >= ?) OR ( service_offering.cpu IS NULL - AND ( - maxComputeDetailsSearch.value IS NULL OR maxComputeDetailsSearch.value >= ? - ) + AND (maxComputeDetailsSearch.value IS NULL OR maxComputeDetailsSearch.value >= ?) ) */ - // TODO: Fix this SearchBuilder maxComputeDetailsSearch = _srvOfferingDetailsDao.createSearchBuilder(); serviceOfferingSearch.join("maxComputeDetailsSearch", maxComputeDetailsSearch, JoinBuilder.JoinType.LEFT, JoinBuilder.JoinCondition.AND, From cfc59c24dbc5f1c00c3b939bddf2d6267e6513b4 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 10 Jan 2024 13:14:17 +0530 Subject: [PATCH 26/28] Fixup query generation for nested joins --- .../com/cloud/utils/db/GenericDaoBase.java | 23 +++++++----------- .../cloud/utils/db/GenericDaoBaseTest.java | 24 +++++-------------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index fc9b2c7cfb56..4e2960ee9f8f 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1294,7 +1294,7 @@ protected List addJoins(StringBuilder str, Collection addJoins(StringBuilder str, Collection>> joins, Map joinedTableNames) { + protected List addJoins(StringBuilder str, Collection>> joins, Map joinedTableNames) { List joinAttrList = new ArrayList<>(); boolean hasWhereClause = true; int fromIndex = str.lastIndexOf("WHERE"); @@ -1307,7 +1307,13 @@ protected List addJoins(StringBuilder str, Collection> join : joins) { String joinTableName = join.getSecondAttribute()[0].table; - String joinTableAlias = StringUtils.isNotEmpty(join.getName()) ? join.getName() : findNextJoinTableName(joinTableName, joinedTableNames); + String joinTableAlias; + if (StringUtils.isNotEmpty(join.getName())) { + joinTableAlias = join.getName(); + joinedTableNames.put(joinTableName, joinTableAlias); + } else { + joinTableAlias = joinedTableNames.getOrDefault(joinTableName, joinTableName); + } StringBuilder onClause = new StringBuilder(); onClause.append(" ") .append(join.getType().getName()) @@ -1325,7 +1331,7 @@ protected List addJoins(StringBuilder str, Collection addJoins(StringBuilder str, Collection usedTableNames) { - if (usedTableNames.containsKey(tableName)) { - Integer tableCounter = usedTableNames.get(tableName); - usedTableNames.put(tableName, ++tableCounter); - tableName = tableName + tableCounter; - } else { - usedTableNames.put(tableName, 0); - } - return tableName; - } - private void removeAndClause(StringBuilder sql) { sql.delete(sql.length() - 4, sql.length()); } diff --git a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java index 11983a7a2064..308600341c3f 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java @@ -20,8 +20,6 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; -import java.util.Map; import org.junit.Assert; import org.junit.Before; @@ -255,24 +253,14 @@ public void multiJoinSameTableTest() { Attribute tDc5 = new Attribute("tableD", "column5"); Attribute attr = new Attribute(123); - joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>("", dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); - joins.add(new JoinBuilder<>("tableAlias", dbTestDao.createSearchCriteria(), + joins.add(new JoinBuilder<>("tableA1Alias", dbTestDao.createSearchCriteria(), tBc2, tAc1, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("tableA2Alias", dbTestDao.createSearchCriteria(), tCc3, tAc2, JoinBuilder.JoinType.INNER)); + joins.add(new JoinBuilder<>("tableA3Alias", dbTestDao.createSearchCriteria(), new Attribute[]{tDc4, tDc5}, new Attribute[]{tAc3, attr}, JoinBuilder.JoinType.INNER, JoinBuilder.JoinCondition.AND)); dbTestDao.addJoins(joinString, joins); - Assert.assertEquals(" INNER JOIN tableA ON tableB.column2=tableA.column1 " + - " INNER JOIN tableA tableA1 ON tableC.column3=tableA1.column2 " + - " INNER JOIN tableA tableAlias ON tableD.column4=tableAlias.column3 AND tableD.column5=? ", joinString.toString()); - } - - @Test - public void findNextTableNameTest() { - Map usedTables = new HashMap<>(); - - Assert.assertEquals("tableA", GenericDaoBase.findNextJoinTableName("tableA", usedTables)); - Assert.assertEquals("tableA1", GenericDaoBase.findNextJoinTableName("tableA", usedTables)); - Assert.assertEquals("tableA2", GenericDaoBase.findNextJoinTableName("tableA", usedTables)); - Assert.assertEquals("tableA3", GenericDaoBase.findNextJoinTableName("tableA", usedTables)); + Assert.assertEquals(" INNER JOIN tableA tableA1Alias ON tableB.column2=tableA1Alias.column1 " + + " INNER JOIN tableA tableA2Alias ON tableC.column3=tableA2Alias.column2 " + + " INNER JOIN tableA tableA3Alias ON tableD.column4=tableA3Alias.column3 AND tableD.column5=? ", joinString.toString()); } } From b4d3268773d35ee8b0fdd9155e7a6ce9f5de2c14 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 10 Jan 2024 14:24:46 +0530 Subject: [PATCH 27/28] Fixups --- .../main/java/org/apache/cloudstack/acl/RoleService.java | 4 ++-- .../main/java/com/cloud/api/query/dao/DomainJoinDao.java | 2 +- .../java/com/cloud/api/query/dao/DomainJoinDaoImpl.java | 2 +- .../java/org/apache/cloudstack/acl/RoleManagerImpl.java | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java index c45152c4de99..07f62a7e7f8f 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java +++ b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java @@ -38,9 +38,9 @@ public interface RoleService { * Moreover, we will check if the requested role is of 'Admin' type; roles with 'Admin' type should only be visible to 'root admins'. * Therefore, if a non-'root admin' user tries to search for an 'Admin' role, this method will return null. */ - Role findRole(Long id, boolean removePrivateRoles); + Role findRole(Long id, boolean ignorePrivateRoles); - List findRoles(List ids, boolean removePrivateRoles); + List findRoles(List ids, boolean ignorePrivateRoles); Role findRole(Long id); diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java index a023371a818b..c7960fd1110e 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDao.java @@ -36,5 +36,5 @@ public interface DomainJoinDao extends GenericDao { void setResourceLimits(DomainJoinVO domain, boolean isRootDomain, ResourceLimitAndCountResponse response); - List searchByIds(Long[] domainIds); + List searchByIds(Long... domainIds); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java index 42d9f13eb47e..24200fa84f95 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java @@ -217,7 +217,7 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim } @Override - public List searchByIds(Long[] domainIds) { + public List searchByIds(Long... domainIds) { // set detail batch query size int DETAILS_BATCH_SIZE = 2000; String batchCfg = configDao.getValue("detail.batch.query.size"); diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java index 4caa1378e992..eeaff6139136 100644 --- a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java @@ -94,7 +94,7 @@ public boolean isEnabled() { } @Override - public Role findRole(Long id, boolean removePrivateRoles) { + public Role findRole(Long id, boolean ignorePrivateRoles) { if (id == null || id < 1L) { logger.trace(String.format("Role ID is invalid [%s]", id)); return null; @@ -104,7 +104,7 @@ public Role findRole(Long id, boolean removePrivateRoles) { logger.trace(String.format("Role not found [id=%s]", id)); return null; } - if (!isCallerRootAdmin() && (RoleType.Admin == role.getRoleType() || (!role.isPublicRole() && removePrivateRoles))) { + if (!isCallerRootAdmin() && (RoleType.Admin == role.getRoleType() || (!role.isPublicRole() && ignorePrivateRoles))) { logger.debug(String.format("Role [id=%s, name=%s] is either of 'Admin' type or is private and is only visible to 'Root admins'.", id, role.getName())); return null; } @@ -112,7 +112,7 @@ public Role findRole(Long id, boolean removePrivateRoles) { } @Override - public List findRoles(List ids, boolean removePrivateRoles) { + public List findRoles(List ids, boolean ignorePrivateRoles) { List result = new ArrayList<>(); if (CollectionUtils.isEmpty(ids)) { logger.trace(String.format("Role IDs are invalid [%s]", ids)); @@ -125,7 +125,7 @@ public List findRoles(List ids, boolean removePrivateRoles) { return result; } for (Role role : roles) { - if (!isCallerRootAdmin() && (RoleType.Admin == role.getRoleType() || (!role.isPublicRole() && removePrivateRoles))) { + if (!isCallerRootAdmin() && (RoleType.Admin == role.getRoleType() || (!role.isPublicRole() && ignorePrivateRoles))) { logger.debug(String.format("Role [id=%s, name=%s] is either of 'Admin' type or is private and is only visible to 'Root admins'.", role.getId(), role.getName())); continue; } From 6df3c332743637a0983e9fa8bbffa1b19d840434 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 12 Jan 2024 13:45:46 +0530 Subject: [PATCH 28/28] Fix DB exception on ClientPreparedStatement --- .../java/com/cloud/utils/db/SearchBase.java | 37 ------------------- .../com/cloud/api/query/QueryManagerImpl.java | 4 +- 2 files changed, 2 insertions(+), 39 deletions(-) diff --git a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java index 012e61aa1fc7..fcc9ded684d3 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/SearchBase.java @@ -252,43 +252,6 @@ public J join(final String name, final SearchBase builder, final JoinBu return (J)this; } - - /** - * This method is used to set alias for the table name when joining with another table. - * Allows to set alias for the table name which is further used to generate the condition for nested join. - *
- * e.g. vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); - *
- * volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); - *
- * In the above example, vmSearch is the alias for the table name vm_instance and the query generated is
- * FROM volume
- * LEFT JOIN vm_instance vmSearch ON vmSearch.id = volume.instance_id
- * LEFT JOIN service_offering serviceOfferingSearch ON serviceOfferingSearch.id = vmSearch.service_offering_id - */ - public J join( - final String tableAliasName, final String name, final SearchBase builder, - final JoinBuilder.JoinType joinType, final JoinBuilder.JoinCondition condition, final Object... joinFields - ) { - if (_entity == null) - throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); - if (_specifiedAttrs.size() != 1) - throw new CloudRuntimeException("Attribute not specified."); - if (builder._entity == null) - throw new CloudRuntimeException("SearchBuilder cannot be modified once it has been setup"); - if (builder._specifiedAttrs.size() != 1) - throw new CloudRuntimeException("Attribute not specified."); - if (builder == this) - throw new CloudRuntimeException("Can't join with itself. Create a new SearchBuilder for the same entity and use that."); - - for (final Attribute attr : _specifiedAttrs) { - if (attr.table != null) { - attr.table = tableAliasName; - } - } - return join(name, builder, joinType, condition, joinFields); - } - public SelectType getSelectType() { return _selectType; } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 04e6364ccea9..db462373eede 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2480,7 +2480,7 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) serviceOfferingSearch.or("serviceOfferingSearchNulltype", serviceOfferingSearch.entity().isSystemUse(), SearchCriteria.Op.NULL); serviceOfferingSearch.cp(); - vmSearch.join("vmSearch", "serviceOfferingSearch", serviceOfferingSearch, JoinBuilder.JoinType.LEFT, null, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId()); + vmSearch.join("serviceOfferingSearch", serviceOfferingSearch, serviceOfferingSearch.entity().getId(), vmSearch.entity().getServiceOfferingId(), JoinBuilder.JoinType.LEFT); volumeSearchBuilder.join("vmSearch", vmSearch, vmSearch.entity().getId(), volumeSearchBuilder.entity().getInstanceId(), JoinBuilder.JoinType.LEFT); @@ -2526,7 +2526,7 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) if (!shouldListSystemVms) { sc.setJoinParameters("vmSearch", "svmType", VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.DomainRouter); - sc.setJoinParameters("serviceOfferingSearch", "systemUse", 1); + sc.getJoin("vmSearch").setJoinParameters("serviceOfferingSearch", "systemUse", 1); } if (MapUtils.isNotEmpty(tags)) {