Refactor `listTemplates` and `listIsos` workflows

[bernardodemarco]

The workflows of the listIsos and listTemplates APIs, particularly the com.cloud.api.query.QueryManagerImpl#searchForTemplatesInternal method, could greatly benefit from refactoring.

This method accepts over 20 parameters, spans approximately 200 lines, performs DAO operations directly from the business/service layer, and lacks modularization, logging, and unit tests. These issues make troubleshooting and maintenance challenging.

cloudstack/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

Lines 4802 to 5007 in f52e058

	private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long templateId, String name, String keyword,
	TemplateFilter templateFilter, boolean isIso, Boolean bootable, Long pageSize,
	Long startIndex, Long zoneId, Long storagePoolId, Long imageStoreId, HypervisorType hyperType,
	boolean showDomr, boolean onlyReady, List<Account> permittedAccounts, Account caller,
	ListProjectResourcesCriteria listProjectResourcesCriteria, Map<String, String> tags,
	boolean showRemovedTmpl, List<Long> ids, Long parentTemplateId, Boolean showUnique, String templateType,
	Boolean isVnf, CPU.CPUArch arch, Long osCategoryId, Boolean forCks) {
	// check if zone is configured, if not, just return empty list
	List<HypervisorType> hypers = null;
	if (!isIso) {
	hypers = _resourceMgr.listAvailHypervisorInZone(null);
	if (hypers == null || hypers.isEmpty()) {
	return new Pair<>(new ArrayList<>(), 0);
	}
	}
	VMTemplateVO template;
	Filter searchFilter = new Filter(TemplateJoinVO.class, "sortKey", SortKeyAscending.value(), startIndex, pageSize);
	searchFilter.addOrderBy(TemplateJoinVO.class, "tempZonePair", SortKeyAscending.value());
	SearchBuilder<TemplateJoinVO> sb = _templateJoinDao.createSearchBuilder();
	if (showUnique) {
	sb.select(null, Func.DISTINCT, sb.entity().getId()); // select distinct templateId
	} else {
	sb.select(null, Func.DISTINCT, sb.entity().getTempZonePair()); // select distinct (templateId, zoneId) pair
	}
	if (ids != null && !ids.isEmpty()) {
	sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
	}
	if (storagePoolId != null) {
	SearchBuilder<VMTemplateStoragePoolVO> storagePoolSb = templatePoolDao.createSearchBuilder();
	sb.join("storagePool", storagePoolSb, storagePoolSb.entity().getTemplateId(), sb.entity().getId(), JoinBuilder.JoinType.INNER);
	}
	if (osCategoryId != null) {
	sb.and("guestOsIdIN", sb.entity().getGuestOSId(), Op.IN);
	}
	SearchCriteria<TemplateJoinVO> sc = sb.create();
	if (imageStoreId != null) {
	sc.addAnd("dataStoreId", SearchCriteria.Op.EQ, imageStoreId);
	}
	if (arch != null) {
	sc.addAnd("arch", SearchCriteria.Op.EQ, arch);
	}
	if (storagePoolId != null) {
	sc.setJoinParameters("storagePool", "pool_id", storagePoolId);
	}
	if (osCategoryId != null) {
	List<Long> guestOsIds = guestOSDao.listIdsByCategoryId(osCategoryId);
	if (CollectionUtils.isNotEmpty(guestOsIds)) {
	sc.setParameters("guestOsIdIN", guestOsIds.toArray());
	} else {
	return new Pair<>(new ArrayList<>(), 0);
	}
	}
	// verify templateId parameter and specially handle it
	if (templateId != null) {
	template = _templateDao.findByIdIncludingRemoved(templateId); // Done for backward compatibility - Bug-5221
	if (template == null) {
	throw new InvalidParameterValueException("Please specify a valid template ID.");
	}// If ISO requested then it should be ISO.
	if (isIso && template.getFormat() != ImageFormat.ISO) {
	logger.error("Template {} is not an ISO", template);
	InvalidParameterValueException ex = new InvalidParameterValueException("Specified Template Id is not an ISO");
	ex.addProxyObject(template.getUuid(), "templateId");
	throw ex;
	}// If ISO not requested then it shouldn't be an ISO.
	if (!isIso && template.getFormat() == ImageFormat.ISO) {
	logger.error("Incorrect format of the template: {}", template);
	InvalidParameterValueException ex = new InvalidParameterValueException("Incorrect format " + template.getFormat() + " of the specified template id");
	ex.addProxyObject(template.getUuid(), "templateId");
	throw ex;
	}
	if (!template.isPublicTemplate() && caller.getType() == Account.Type.DOMAIN_ADMIN) {
	Account template_acc = accountMgr.getAccount(template.getAccountId());
	DomainVO domain = _domainDao.findById(template_acc.getDomainId());
	accountMgr.checkAccess(caller, domain);
	}
	// if template is not public, perform permission check here
	else if (!template.isPublicTemplate() && caller.getType() != Account.Type.ADMIN) {
	accountMgr.checkAccess(caller, null, false, template);
	} else if (template.isPublicTemplate()) {
	accountMgr.checkAccess(caller, null, false, template);
	}
	// if templateId is specified, then we will just use the id to
	// search and ignore other query parameters
	sc.addAnd("id", SearchCriteria.Op.EQ, templateId);
	} else {
	DomainVO domain;
	if (!permittedAccounts.isEmpty()) {
	domain = _domainDao.findById(permittedAccounts.get(0).getDomainId());
	} else {
	domain = _domainDao.findById(caller.getDomainId());
	}
	setIdsListToSearchCriteria(sc, ids);
	// add criteria for project or not
	if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
	sc.addAnd("accountType", SearchCriteria.Op.NEQ, Account.Type.PROJECT);
	} else if (listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) {
	sc.addAnd("accountType", SearchCriteria.Op.EQ, Account.Type.PROJECT);
	}
	// add criteria for domain path in case of domain admin
	if ((templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable)
	&& (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN)) {
	sc.addAnd("domainPath", SearchCriteria.Op.LIKE, domain.getPath() + "%");
	}
	List<Long> relatedDomainIds = new ArrayList<>();
	List<Long> permittedAccountIds = new ArrayList<>();
	if (!permittedAccounts.isEmpty()) {
	for (Account account : permittedAccounts) {
	permittedAccountIds.add(account.getId());
	boolean publicTemplates = (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community);
	// get all parent domain ID's all the way till root domain
	DomainVO domainTreeNode;
	//if template filter is featured, or community, all child domains should be included in search
	if (publicTemplates) {
	domainTreeNode = _domainDao.findById(Domain.ROOT_DOMAIN);
	} else {
	domainTreeNode = _domainDao.findById(account.getDomainId());
	}
	relatedDomainIds.add(domainTreeNode.getId());
	while (domainTreeNode.getParent() != null) {
	domainTreeNode = _domainDao.findById(domainTreeNode.getParent());
	relatedDomainIds.add(domainTreeNode.getId());
	}
	// get all child domain ID's
	if (accountMgr.isAdmin(account.getId()) || publicTemplates) {
	List<DomainVO> allChildDomains = _domainDao.findAllChildren(domainTreeNode.getPath(), domainTreeNode.getId());
	for (DomainVO childDomain : allChildDomains) {
	relatedDomainIds.add(childDomain.getId());
	}
	}
	}
	}
	// control different template filters
	if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) {
	sc.addAnd("publicTemplate", SearchCriteria.Op.EQ, true);
	if (templateFilter == TemplateFilter.featured) {
	sc.addAnd("featured", SearchCriteria.Op.EQ, true);
	} else {
	sc.addAnd("featured", SearchCriteria.Op.EQ, false);
	}
	if (!permittedAccounts.isEmpty()) {
	SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
	scc.addOr("domainId", SearchCriteria.Op.IN, relatedDomainIds.toArray());
	scc.addOr("domainId", SearchCriteria.Op.NULL);
	sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
	}
	} else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) {
	if (!permittedAccounts.isEmpty()) {
	sc.addAnd("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
	}
	} else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) {
	// only show templates shared by others
	if (permittedAccounts.isEmpty()) {
	return new Pair<>(new ArrayList<>(), 0);
	}
	sc.addAnd("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
	} else if (templateFilter == TemplateFilter.executable) {
	SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
	scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
	if (!permittedAccounts.isEmpty()) {
	scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
	}
	sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc);
	} else if (templateFilter == TemplateFilter.all && caller.getType() != Account.Type.ADMIN) {
	SearchCriteria<TemplateJoinVO> scc = _templateJoinDao.createSearchCriteria();
	scc.addOr("publicTemplate", SearchCriteria.Op.EQ, true);
	if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
	scc.addOr("domainPath", SearchCriteria.Op.LIKE, _domainDao.findById(caller.getDomainId()).getPath() + "%");
	} else {
	if (!permittedAccounts.isEmpty()) {
	scc.addOr("accountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
	scc.addOr("sharedAccountId", SearchCriteria.Op.IN, permittedAccountIds.toArray());
	}
	}
	sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc);
	}
	}
	applyPublicTemplateSharingRestrictions(sc, caller);
	return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
	showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc);
	}

[DaanHoogland]

this is one of the methods that suffers this quality. I am now working on listDataCentersInternal. it fortunately takes the cmd object but is 200 lines as well. I am refactorring it to be able to troubleshoot.