This is an automated email from the ASF dual-hosted git repository. Pearl1594 pushed a commit to branch improve-listtemplates-perf in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 3fd789bbad52aee18052a9d4e3dcd4dd50202121 Author: aniishadas <[email protected]> AuthorDate: Thu May 21 17:54:03 2026 -0700 Bypass logic to create template_pair from 6 tables, with fallback * added bypass logic to create template_pair from 6 tables, fallback for hard filters * Removed batching, checking against per pair * follow try-with-resources design * unit tests for the bypass logic * Fix cross-zone template lookup in listTemplates Phase 2 When temp_zone_pair has dcId=0 (cross-zone template with no data_center), use IS NULL predicate instead of broken EQ/IN with literal 0 which never matches NULL rows. * Add filter for non-root domain-admin users --------- Co-authored-by: anishadas <[email protected]> Co-authored-by: Aaron Chung <[email protected]> --- .../org/apache/cloudstack/query/QueryService.java | 5 + .../java/com/cloud/api/query/QueryManagerImpl.java | 159 ++++++++- .../com/cloud/api/query/dao/TemplateJoinDao.java | 15 + .../cloud/api/query/dao/TemplateJoinDaoImpl.java | 368 ++++++++++++++++++--- .../cloud/api/query/dao/TemplateListFilter.java | 225 +++++++++++++ .../dao/TemplateJoinDaoImplBypassExplainTest.java | 266 +++++++++++++++ 6 files changed, 988 insertions(+), 50 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/query/QueryService.java b/api/src/main/java/org/apache/cloudstack/query/QueryService.java index 6a20c2fa248..c950d20850b 100644 --- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java +++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java @@ -140,6 +140,11 @@ public interface QueryService { ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "true", "Determines whether VM stats should be returned when details are not explicitly specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global); + ConfigKey<Boolean> BypassTemplateView = new ConfigKey<>("Advanced", Boolean.class, "template.list.bypass.view", "false", + "If true, listTemplates Phase 1 (DISTINCT temp_zone_pair) bypasses template_view and runs a hand-tuned SQL " + + "against the underlying tables. Falls back to the view automatically when the request uses tags, " + + "sharedAccountIds, domainPath, or featured/community-domain filters. Toggleable at runtime; flip back " + + "to false to revert without a deploy if a parity bug surfaces.", true, ConfigKey.Scope.Global); ListResponse<UserResponse> searchForUsers(ResponseObject.ResponseView responseView, ListUsersCmd cmd) throws PermissionDeniedException; 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 e1dc73a3225..4ec3542ba0a 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -45,6 +45,14 @@ import com.cloud.server.ManagementService; import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.cluster.ManagementServerHostPeerJoinVO; +import com.cloud.template.VirtualMachineTemplate; +import com.cloud.user.AccountVO; +import com.cloud.user.SSHKeyPairVO; +import com.cloud.user.dao.SSHKeyPairDao; +import com.cloud.vm.InstanceGroupVMMapVO; +import com.cloud.vm.NicVO; +import com.cloud.vm.dao.InstanceGroupVMMapDao; +import com.cloud.vm.dao.NicDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker; @@ -207,6 +215,7 @@ import com.cloud.api.query.dao.ServiceOfferingJoinDao; import com.cloud.api.query.dao.SnapshotJoinDao; import com.cloud.api.query.dao.StoragePoolJoinDao; import com.cloud.api.query.dao.TemplateJoinDao; +import com.cloud.api.query.dao.TemplateListFilter; import com.cloud.api.query.dao.UserAccountJoinDao; import com.cloud.api.query.dao.UserVmJoinDao; import com.cloud.api.query.dao.VolumeJoinDao; @@ -332,12 +341,9 @@ import com.cloud.template.VirtualMachineTemplate.State; import com.cloud.template.VirtualMachineTemplate.TemplateFilter; import com.cloud.user.Account; import com.cloud.user.AccountManager; -import com.cloud.user.AccountVO; import com.cloud.user.DomainManager; -import com.cloud.user.SSHKeyPairVO; import com.cloud.user.User; import com.cloud.user.dao.AccountDao; -import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.user.dao.UserDao; import com.cloud.utils.DateUtil; import com.cloud.utils.NumbersUtil; @@ -353,8 +359,6 @@ import com.cloud.utils.db.SearchCriteria.Func; import com.cloud.utils.db.SearchCriteria.Op; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DomainRouterVO; -import com.cloud.vm.InstanceGroupVMMapVO; -import com.cloud.vm.NicVO; import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.VMInstanceVO; @@ -362,8 +366,6 @@ import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.DomainRouterDao; -import com.cloud.vm.dao.InstanceGroupVMMapDao; -import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.dao.VMInstanceDetailsDao; @@ -5054,7 +5056,11 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q applyPublicTemplateSharingRestrictions(sc, caller); return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller, - showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc); + showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc, + buildTemplateListFilter(templateId, ids, name, keyword, templateFilter, isIso, bootable, + pageSize, startIndex, zoneId, hyperType, hypers, showDomr, onlyReady, + permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl, + parentTemplateId, showUnique)); } /** @@ -5109,7 +5115,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword, HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller, boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique, String templateType, Boolean isVnf, Boolean forCks, - Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) { + Filter searchFilter, SearchCriteria<TemplateJoinVO> sc, + TemplateListFilter listFilter) { if (!isIso) { // add hypervisor criteria for template case if (hypers != null && !hypers.isEmpty()) { @@ -5216,6 +5223,14 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q // search unique templates and find details by Ids Pair<List<TemplateJoinVO>, Integer> uniqueTmplPair; + + // Only attempted when the bypass DAO supports the filter set; falls + // back to the SearchBuilder/template_view path below otherwise. + if (!showRemovedTmpl && BypassTemplateView.value() && listFilter != null && listFilter.canBypass()) { + uniqueTmplPair = _templateJoinDao.findDistinctTempZonePairs(listFilter); + return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique != null && showUnique, caller); + } + if (showRemovedTmpl) { uniqueTmplPair = _templateJoinDao.searchIncludingRemovedAndCount(sc, searchFilter); } else { @@ -5237,6 +5252,119 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q // about what the code was doing. } + /** + * Build the immutable filter the bypass-the-view DAO consumes. Mirrors the + * SearchCriteria construction in {@link #searchForTemplatesInternal}; the + * SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds, + * domainPathLike, domainIdsForFeaturedCommunity, tags) drive + * {@link TemplateListFilter#canBypass()} — when any are populated, + * {@code templateChecks} routes to the legacy view-based path instead. + */ + private TemplateListFilter buildTemplateListFilter(Long templateId, List<Long> ids, String name, String keyword, + TemplateFilter templateFilter, boolean isIso, Boolean bootable, + Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, + List<HypervisorType> hypers, boolean showDomr, boolean onlyReady, + List<Account> permittedAccounts, Account caller, + ListProjectResourcesCriteria listProjectResourcesCriteria, + Map<String, String> tags, boolean showRemovedTmpl, Long parentTemplateId, + Boolean showUnique) { + TemplateListFilter.Builder b = TemplateListFilter.builder() + .templateId(templateId) + .ids(ids == null ? null : new ArrayList<>(ids)) + .name(name) + .keyword(keyword) + .hypervisorType(hyperType != null && !HypervisorType.None.equals(hyperType) ? hyperType : null) + .availableHypervisors(hypers) + .format(ImageFormat.ISO) // bypass DAO uses isIso flag to flip between EQ and NEQ + .isIso(isIso) + .bootable(bootable) + .parentTemplateId(parentTemplateId) + .zoneId(zoneId) + .onlyReady(onlyReady) + .excludeSystemTemplates(!showDomr) + .showUnique(showUnique != null && showUnique) + .startIndex(startIndex) + .pageSize(pageSize) + .sortAscending(SortKeyAscending.value()) + .showRemoved(showRemovedTmpl) + .tags(tags); + + if (!showRemovedTmpl) { + b.templateStates(Arrays.asList(VirtualMachineTemplate.State.Active, + VirtualMachineTemplate.State.UploadAbandoned, + VirtualMachineTemplate.State.UploadError, + VirtualMachineTemplate.State.NotUploaded, + VirtualMachineTemplate.State.UploadInProgress)); + } + + // accountType filter (Project vs non-Project), applied only when no specific templateId. + if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) { + b.accountTypeNeq(Account.Type.PROJECT); + } else if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) { + b.accountTypeEq(Account.Type.PROJECT); + } + + // ACL handling — mirrors the templateFilter switch in searchForTemplatesInternal. + // For "hard" templateFilter values, set requiresViewFallback so canBypass() returns + // false and the dispatcher falls back to the legacy view-based path. + if (templateId != null) { + return b.build(); + } + + List<Long> permittedAccountIds = new ArrayList<>(); + for (Account account : permittedAccounts) { + permittedAccountIds.add(account.getId()); + } + + if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) { + // Walks the domain hierarchy and ORs domainId IS NULL — not yet modeled in bypass SQL. + b.publicTemplate(Boolean.TRUE); + b.featured(templateFilter == TemplateFilter.featured ? Boolean.TRUE : Boolean.FALSE); + b.requiresViewFallback(true); + } else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) { + if (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { + // Match the existing path's domain resolution (see searchForTemplatesInternal:4498-4502): + // scope to the queried account's domain when one was specified, else the caller's. + Long domainIdForScope = !permittedAccounts.isEmpty() + ? permittedAccounts.get(0).getDomainId() + : caller.getDomainId(); + DomainVO scopeDomain = _domainDao.findById(domainIdForScope); + if (scopeDomain != null) { + b.domainPathLike(scopeDomain.getPath() + "%"); + } + } + if (!permittedAccountIds.isEmpty()) { + b.accountIds(permittedAccountIds); + b.accountIdRequiredForFilter(true); + } + } else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) { + // launch_permission join — not modeled in bypass SQL. + b.requiresViewFallback(true); + } else if (templateFilter == TemplateFilter.executable) { + b.publicOrAccountIdComposite(true); + if (!permittedAccountIds.isEmpty()) { + b.accountIds(permittedAccountIds); + } + } else if (templateFilter == TemplateFilter.all && caller.getType() != Account.Type.ADMIN) { + // Non-admin "all" composite splits on listProjectResourcesCriteria: + // SkipProjectResources → (public OR domainPath LIKE) — bypass-eligible + // else → (public OR account_id IN OR sharedAccountId IN) — needs launch_permission, falls back + if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) { + DomainVO callerDomain = _domainDao.findById(caller.getDomainId()); + if (callerDomain != null) { + b.publicOrDomainPathComposite(true); + b.domainPathLike(callerDomain.getPath() + "%"); + } else { + b.requiresViewFallback(true); + } + } else { + b.requiresViewFallback(true); + } + } + + return b.build(); + } + // findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair, boolean showRemoved, boolean showUnique, Account caller) { @@ -6239,6 +6367,17 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Override public ConfigKey<?>[] getConfigKeys() { return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending, - AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList}; + AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, + ReturnVmStatsOnVmList, BypassTemplateView }; + } + + @Override + public EntityManager getEntityManager() { + return entityManager; + } + + @Override + public AccountManager getAccountManager() { + return accountMgr; } } diff --git a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java index a7b82e47265..ff0be5933b9 100644 --- a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java @@ -52,5 +52,20 @@ public interface TemplateJoinDao extends GenericDao<TemplateJoinVO, Long> { Pair<List<TemplateJoinVO>, Integer> searchIncludingRemovedAndCount(final SearchCriteria<TemplateJoinVO> sc, final Filter filter); + /** + * Bypass-the-view Phase 1 implementation. Issues a hand-tuned SQL query + * directly against the underlying tables (vm_template, account, + * template_store_ref, image_store, template_zone_ref, data_center) instead + * of going through {@code template_view}. Caller must check + * {@link TemplateListFilter#canBypass()} first; this method does not handle + * tags, sharedAccountIds, domainPath, or featured/community-style domainId + * filters and will throw IllegalArgumentException if those are populated. + * + * Returns TemplateJoinVO objects with only {@code id} (when + * {@code filter.showUnique}) or {@code tempZonePair} populated, plus the + * total distinct count for pagination. + */ + Pair<List<TemplateJoinVO>, Integer> findDistinctTempZonePairs(TemplateListFilter filter); + List<TemplateJoinVO> findByDistinctIds(Long... ids); } 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 672d83a27b3..feb071a8beb 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 @@ -16,6 +16,10 @@ // under the License. package com.cloud.api.query.dao; +import java.lang.reflect.Field; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; @@ -79,6 +83,8 @@ import com.cloud.utils.Pair; import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; +import com.cloud.utils.db.TransactionLegacy; +import com.cloud.utils.exception.CloudRuntimeException; @Component @@ -114,6 +120,8 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa private final SearchBuilder<TemplateJoinVO> tmpltIdPairSearch; + private final SearchBuilder<TemplateJoinVO> tmpltIdPairCrossZoneSearch; + private final SearchBuilder<TemplateJoinVO> tmpltIdSearch; private final SearchBuilder<TemplateJoinVO> tmpltIdsSearch; @@ -131,6 +139,11 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa tmpltIdPairSearch.and("tempZonePairIN", tmpltIdPairSearch.entity().getTempZonePair(), SearchCriteria.Op.IN); tmpltIdPairSearch.done(); + tmpltIdPairCrossZoneSearch = createSearchBuilder(); + tmpltIdPairCrossZoneSearch.and("template_dc_pair_templateid", tmpltIdPairCrossZoneSearch.entity().getId(), SearchCriteria.Op.EQ); + tmpltIdPairCrossZoneSearch.and("template_dc_pair_dcid", tmpltIdPairCrossZoneSearch.entity().getDataCenterId(), SearchCriteria.Op.NULL); + tmpltIdPairCrossZoneSearch.done(); + tmpltIdSearch = createSearchBuilder(); tmpltIdSearch.and("id", tmpltIdSearch.entity().getId(), SearchCriteria.Op.EQ); tmpltIdSearch.done(); @@ -624,55 +637,48 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa @Override public List<TemplateJoinVO> searchByTemplateZonePair(Boolean showRemoved, String... idPairs) { - // 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 Filter searchFilter = new Filter(TemplateJoinVO.class, "sortKey", QueryService.SortKeyAscending.value(), null, null); searchFilter.addOrderBy(TemplateJoinVO.class, "tempZonePair", QueryService.SortKeyAscending.value()); + List<TemplateJoinVO> uvList = new ArrayList<TemplateJoinVO>(); - // query details by batches - int curr_index = 0; - if (idPairs.length > DETAILS_BATCH_SIZE) { - while ((curr_index + DETAILS_BATCH_SIZE) <= idPairs.length) { - String[] labels = new String[DETAILS_BATCH_SIZE]; - for (int k = 0, j = curr_index; j < curr_index + DETAILS_BATCH_SIZE; j++, k++) { - labels[k] = idPairs[j]; - } - SearchCriteria<TemplateJoinVO> sc = tmpltIdPairSearch.create(); - if (!showRemoved) { - sc.setParameters("templateState", VirtualMachineTemplate.State.Active); - } - sc.setParameters("tempZonePairIN", labels); - List<TemplateJoinVO> vms = searchIncludingRemoved(sc, searchFilter, null, false); - if (vms != null) { - uvList.addAll(vms); - } - curr_index += DETAILS_BATCH_SIZE; - } + if (idPairs == null || idPairs.length == 0) { + return uvList; } - if (curr_index < idPairs.length) { - int batch_size = (idPairs.length - curr_index); - String[] labels = new String[batch_size]; - for (int k = 0, j = curr_index; j < curr_index + batch_size; j++, k++) { - labels[k] = idPairs[j]; - } - SearchCriteria<TemplateJoinVO> sc = tmpltIdPairSearch.create(); - if (!showRemoved) { - sc.setParameters("templateState", VirtualMachineTemplate.State.Active, VirtualMachineTemplate.State.UploadAbandoned, VirtualMachineTemplate.State.UploadError ,VirtualMachineTemplate.State.NotUploaded, VirtualMachineTemplate.State.UploadInProgress); - } - sc.setParameters("tempZonePairIN", labels); - List<TemplateJoinVO> vms = searchIncludingRemoved(sc, searchFilter, null, false); - if (vms != null) { - uvList.addAll(vms); + + for (String idPair : idPairs) { + SearchCriteria<TemplateJoinVO> sc = applyPairWhereClauseHackyWorkaround(idPair); + List<TemplateJoinVO> rows = searchIncludingRemoved(sc, searchFilter, null, false); + if (rows != null) { + uvList.addAll(rows); } } return uvList; } + private SearchCriteria<TemplateJoinVO> applyPairWhereClauseHackyWorkaround(String idPair) { + if (idPair == null || idPair.isEmpty()) { + throw new IllegalArgumentException("template zone pair id is null or empty"); + } + // eg "3124_3" → templateId=3124, dcId=3 + String[] parts = idPair.split("_"); + if (parts.length != 2) { + throw new IllegalArgumentException("unexpected template zone pair id format: " + idPair); + } + long templateId = Long.parseLong(parts[0]); + long dcId = Long.parseLong(parts[1]); + + SearchCriteria<TemplateJoinVO> sc; + if (dcId == 0) { + sc = tmpltIdPairCrossZoneSearch.create(); + sc.setParameters("template_dc_pair_templateid", templateId); + } else { + sc = tmpltIdPairSearch.create(); + sc.setParameters("template_dc_pair_templateid", templateId); + sc.setParameters("template_dc_pair_dcid", dcId); + } + return sc; + } + @Override public List<TemplateJoinVO> listActiveTemplates(long storeId) { SearchCriteria<TemplateJoinVO> sc = activeTmpltSearch.create(); @@ -696,6 +702,288 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa return new Pair<List<TemplateJoinVO>, Integer>(objects, count); } + // ============================================================================ + // The standard Phase 1 path runs `SELECT DISTINCT temp_zone_pair FROM + // template_view WHERE ...` against a 13-table view. + // + // This bypass path issues hand-tuned SQL against only the 6 tables needed + // to compute the (template_id, data_center_id) pair: vm_template, account, + // template_store_ref, image_store, template_zone_ref, data_center. The OR + // join is replaced with COALESCE. + // + // Hard filters (tags, sharedAccountIds, domainPath, featured/community + // domain hierarchy) are not implemented here — TemplateListFilter#canBypass() + // returns false in those cases and the dispatcher falls back to the + // SearchBuilder path. + + private static final Field TEMPLATE_JOIN_ID_FIELD; + private static final Field TEMPLATE_JOIN_PAIR_FIELD; + + static { + try { + TEMPLATE_JOIN_ID_FIELD = findFieldUpHierarchy(TemplateJoinVO.class, "id"); + TEMPLATE_JOIN_PAIR_FIELD = findFieldUpHierarchy(TemplateJoinVO.class, "tempZonePair"); + TEMPLATE_JOIN_ID_FIELD.setAccessible(true); + TEMPLATE_JOIN_PAIR_FIELD.setAccessible(true); + } catch (NoSuchFieldException e) { + throw new ExceptionInInitializerError(e); + } + } + + private static Field findFieldUpHierarchy(Class<?> clazz, String name) throws NoSuchFieldException { + Class<?> c = clazz; + while (c != null) { + try { + return c.getDeclaredField(name); + } catch (NoSuchFieldException ignored) { + c = c.getSuperclass(); + } + } + throw new NoSuchFieldException(name + " on " + clazz); + } + + @Override + public Pair<List<TemplateJoinVO>, Integer> findDistinctTempZonePairs(TemplateListFilter filter) { + if (!filter.canBypass()) { + throw new IllegalArgumentException( + "findDistinctTempZonePairs called with unsupported filter (tags / sharedAccountIds / domainPath / domainIds populated). Caller should fall back to searchAndDistinctCount."); + } + + List<Object> params = new ArrayList<>(); + StringBuilder where = new StringBuilder(" WHERE 1=1"); + appendCommonWhere(where, params, filter); + + String fromClause = buildFromClause(filter); + + String selectExpr = filter.showUnique + ? "vt.id" + : "CONCAT(vt.id, '_', IFNULL(dc.id, 0))"; + + String dataSql = "SELECT DISTINCT " + selectExpr + " AS distinct_key, vt.sort_key" + + " FROM " + fromClause + + where + + " ORDER BY vt.sort_key " + (filter.sortAscending ? "ASC" : "DESC") + + ", distinct_key " + (filter.sortAscending ? "ASC" : "DESC") + + buildLimitClause(filter); + + String countSql = "SELECT COUNT(DISTINCT " + selectExpr + ")" + + " FROM " + fromClause + + where; + + List<TemplateJoinVO> rows = executeDistinctQuery(dataSql, params, filter.showUnique); + int count = executeCountQuery(countSql, params); + return new Pair<>(rows, count); + } + + /** + * Build the FROM clause. The 6-table base is always present; conditional + * joins are added when the filter actually needs them. Today only + * `domain` is conditional (used for {@code domainPathLike} predicates); + * `launch_permission` and `resource_tags` are added in later iterations. + */ + private String buildFromClause(TemplateListFilter filter) { + StringBuilder from = new StringBuilder() + .append("cloud.vm_template vt") + .append(" JOIN cloud.account a ON a.id = vt.account_id") + .append(" LEFT JOIN cloud.template_store_ref tsr") + .append(" ON tsr.template_id = vt.id AND tsr.store_role = 'Image' AND tsr.destroyed = 0") + .append(" LEFT JOIN cloud.image_store img") + .append(" ON img.id = tsr.store_id AND img.removed IS NULL") + .append(" LEFT JOIN cloud.template_zone_ref tzr") + .append(" ON tzr.template_id = vt.id AND tsr.store_id IS NULL AND tzr.removed IS NULL") + .append(" LEFT JOIN cloud.data_center dc") + .append(" ON dc.id = COALESCE(img.data_center_id, tzr.zone_id)"); + + if (filter.domainPathLike != null) { + from.append(" JOIN cloud.domain d ON d.id = a.domain_id"); + } + return from.toString(); + } + + private void appendCommonWhere(StringBuilder where, List<Object> params, TemplateListFilter filter) { + if (filter.templateId != null) { + where.append(" AND vt.id = ?"); + params.add(filter.templateId); + } + + if (!filter.ids.isEmpty()) { + where.append(" AND vt.id IN ").append(inClausePlaceholders(filter.ids.size())); + params.addAll(filter.ids); + } + + if (filter.keyword != null) { + where.append(" AND vt.name LIKE ?"); + params.add("%" + filter.keyword + "%"); + } else if (filter.name != null) { + where.append(" AND vt.name = ?"); + params.add(filter.name); + } + + if (filter.format != null) { + // searchForTemplatesInternal: format = 'ISO' for isos, format != 'ISO' otherwise. + where.append(filter.isIso ? " AND vt.format = ?" : " AND vt.format != ?"); + params.add(filter.format.toString()); + } + + if (filter.hypervisorType != null) { + where.append(" AND vt.hypervisor_type = ?"); + params.add(filter.hypervisorType.toString()); + } + + if (!filter.availableHypervisors.isEmpty()) { + where.append(" AND vt.hypervisor_type IN ").append(inClausePlaceholders(filter.availableHypervisors.size())); + for (HypervisorType h : filter.availableHypervisors) { + params.add(h.toString()); + } + } + + if (filter.publicTemplate != null && !filter.publicOrAccountIdComposite) { + where.append(" AND vt.public = ?"); + params.add(filter.publicTemplate ? 1 : 0); + } + + if (filter.featured != null) { + where.append(" AND vt.featured = ?"); + params.add(filter.featured ? 1 : 0); + } + + if (filter.bootable != null) { + where.append(" AND vt.bootable = ?"); + params.add(filter.bootable ? 1 : 0); + } + + if (filter.parentTemplateId != null) { + where.append(" AND vt.parent_template_id = ?"); + params.add(filter.parentTemplateId); + } + + if (filter.excludeSystemTemplates) { + where.append(" AND vt.type != 'SYSTEM'"); + } + + if (filter.accountTypeNeq != null) { + where.append(" AND a.type != ?"); + params.add(filter.accountTypeNeq.ordinal()); + } + if (filter.accountTypeEq != null) { + where.append(" AND a.type = ?"); + params.add(filter.accountTypeEq.ordinal()); + } + + // ACL — accountIds: either pure IN, or composite OR with public=true. + if (filter.publicOrAccountIdComposite) { + where.append(" AND (vt.public = 1"); + if (!filter.accountIds.isEmpty()) { + where.append(" OR vt.account_id IN ").append(inClausePlaceholders(filter.accountIds.size())); + params.addAll(filter.accountIds); + } + where.append(")"); + } else if (filter.publicOrDomainPathComposite) { + // all + non-admin + SkipProjectResources: (public OR domain.path LIKE) + where.append(" AND (vt.public = 1"); + if (filter.domainPathLike != null) { + where.append(" OR d.path LIKE ?"); + params.add(filter.domainPathLike); + } + where.append(")"); + } else if (filter.accountIdRequiredForFilter && !filter.accountIds.isEmpty()) { + where.append(" AND vt.account_id IN ").append(inClausePlaceholders(filter.accountIds.size())); + params.addAll(filter.accountIds); + } + + // Standalone domain_path predicate — used when domainPathLike is set + // outside any composite (self/selfexecutable + DOMAIN_ADMIN scoping). + if (filter.domainPathLike != null + && !filter.publicOrDomainPathComposite) { + where.append(" AND d.path LIKE ?"); + params.add(filter.domainPathLike); + } + + if (filter.zoneId != null) { + // mirrors templateChecks(): zone match OR REGION-scoped store OR (ISO + PERHOST) + where.append(" AND (dc.id = ? OR img.scope = 'REGION' OR (vt.format = 'ISO' AND vt.type = 'PERHOST'))"); + params.add(filter.zoneId); + } + + if (filter.onlyReady) { + // mirrors templateChecks(): tsr Ready OR BAREMETAL format OR (ISO + PERHOST) + where.append(" AND (tsr.state = 'Ready' OR vt.format = 'BAREMETAL' OR (vt.format = 'ISO' AND vt.type = 'PERHOST'))"); + } + + if (!filter.showRemoved) { + where.append(" AND vt.removed IS NULL"); + if (!filter.templateStates.isEmpty()) { + where.append(" AND vt.state IN ").append(inClausePlaceholders(filter.templateStates.size())); + for (VirtualMachineTemplate.State s : filter.templateStates) { + params.add(s.toString()); + } + } + } + } + + private String buildLimitClause(TemplateListFilter filter) { + if (filter.pageSize == null) { + return ""; + } + long start = filter.startIndex == null ? 0L : filter.startIndex; + return " LIMIT " + start + ", " + filter.pageSize; + } + + private static String inClausePlaceholders(int n) { + StringBuilder sb = new StringBuilder("("); + for (int i = 0; i < n; i++) { + if (i > 0) { + sb.append(','); + } + sb.append('?'); + } + sb.append(')'); + return sb.toString(); + } + + private List<TemplateJoinVO> executeDistinctQuery(String sql, List<Object> params, boolean showUnique) { + List<TemplateJoinVO> out = new ArrayList<>(); + try (TransactionLegacy txn = TransactionLegacy.open("TemplateJoinDao"); + PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) { + bindParams(pstmt, params); + try (ResultSet rs = pstmt.executeQuery()) { + while (rs.next()) { + TemplateJoinVO vo = new TemplateJoinVO(); + if (showUnique) { + TEMPLATE_JOIN_ID_FIELD.setLong(vo, rs.getLong(1)); + } else { + TEMPLATE_JOIN_PAIR_FIELD.set(vo, rs.getString(1)); + } + out.add(vo); + } + } + } catch (SQLException | IllegalAccessException e) { + throw new CloudRuntimeException("findDistinctTempZonePairs data query failed: " + sql, e); + } + return out; + } + + private int executeCountQuery(String sql, List<Object> params) { + try (TransactionLegacy txn = TransactionLegacy.open("TemplateJoinDao"); + PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) { + bindParams(pstmt, params); + try (ResultSet rs = pstmt.executeQuery()) { + if (rs.next()) { + return rs.getInt(1); + } + return 0; + } + } catch (SQLException e) { + throw new CloudRuntimeException("findDistinctTempZonePairs count query failed: " + sql, e); + } + } + + private static void bindParams(PreparedStatement pstmt, List<Object> params) throws SQLException { + for (int i = 0; i < params.size(); i++) { + pstmt.setObject(i + 1, params.get(i)); + } + } + @Override public List<TemplateJoinVO> findByDistinctIds(Long... ids) { if (ids == null || ids.length == 0) { diff --git a/server/src/main/java/com/cloud/api/query/dao/TemplateListFilter.java b/server/src/main/java/com/cloud/api/query/dao/TemplateListFilter.java new file mode 100644 index 00000000000..7e1dcd3099a --- /dev/null +++ b/server/src/main/java/com/cloud/api/query/dao/TemplateListFilter.java @@ -0,0 +1,225 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api.query.dao; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.Storage.ImageFormat; +import com.cloud.template.VirtualMachineTemplate.State; +import com.cloud.user.Account; + +/** + * Immutable filter parameters for a listTemplates / listIsos call. + * + * Built once from the cmd parameters in QueryManagerImpl.searchForTemplatesInternal, + * then either consumed by the SearchBuilder path (existing behavior) or by the + * bypass-the-view path (TemplateJoinDaoImpl.findDistinctTempZonePairs). + * + * {@link #canBypass()} reports whether the bypass implementation supports this + * filter combination. The dispatcher must fall back to the SearchBuilder path + * when canBypass() returns false. + */ +public final class TemplateListFilter { + + // identity + public final Long templateId; // exact id match + public final List<Long> ids; // id IN list + + // text search + public final String name; // name EQ when keyword is null + public final String keyword; // name LIKE %keyword% (overrides name) + + // attributes + public final HypervisorType hypervisorType; // EQ when present and not None + public final List<HypervisorType> availableHypervisors; // IN list (from listAvailHypervisorInZone) + public final ImageFormat format; // EQ for ISO, NEQ for non-ISO + public final boolean isIso; + public final boolean excludeSystemTemplates; // templateType NEQ SYSTEM + public final Boolean publicTemplate; // EQ when non-null (forced by featured/community/all-non-admin paths) + public final Boolean featured; // EQ when non-null (true=featured, false=community) + public final Boolean bootable; // EQ when non-null + public final Long parentTemplateId; + public final boolean onlyReady; // composite state-Ready filter + + // ACL + public final Account.Type accountTypeNeq; // accountType NEQ (typically !=Project) + public final Account.Type accountTypeEq; // accountType EQ (typically ==Project) + public final List<Long> accountIds; // EQ-OR-IN over account_id + public final boolean accountIdRequiredForFilter; // when templateFilter forces it (self/selfexecutable) + public final boolean publicOrAccountIdComposite; // executable composite: (public OR account_id IN) + public final boolean publicOrDomainPathComposite; // all-non-admin SkipProjectResources composite: (public OR domain.path LIKE) + + // location + public final Long zoneId; // composite dataCenterId filter + + // state + public final boolean showRemoved; // when true, no templateState filter applied + public final List<State> templateStates; // states IN list when showRemoved is false + + // pagination & shape + public final boolean showUnique; + public final Long startIndex; + public final Long pageSize; + public final boolean sortAscending; + + // hard filters — presence forces fallback to the SearchBuilder path + public final List<Long> sharedAccountIds; // sharedexecutable / shared / all-non-admin + public final String domainPathLike; // domain admin scoping + public final List<Long> domainIdsForFeaturedCommunity; // featured/community related-domain hierarchy + public final Map<String, String> tags; + public final boolean requiresViewFallback; // catch-all flag for templateFilter combinations the bypass SQL doesn't model + + private TemplateListFilter(Builder b) { + this.templateId = b.templateId; + this.ids = nullSafe(b.ids); + this.name = b.name; + this.keyword = b.keyword; + this.hypervisorType = b.hypervisorType; + this.availableHypervisors = nullSafe(b.availableHypervisors); + this.format = b.format; + this.isIso = b.isIso; + this.excludeSystemTemplates = b.excludeSystemTemplates; + this.publicTemplate = b.publicTemplate; + this.featured = b.featured; + this.bootable = b.bootable; + this.parentTemplateId = b.parentTemplateId; + this.onlyReady = b.onlyReady; + this.accountTypeNeq = b.accountTypeNeq; + this.accountTypeEq = b.accountTypeEq; + this.accountIds = nullSafe(b.accountIds); + this.accountIdRequiredForFilter = b.accountIdRequiredForFilter; + this.publicOrAccountIdComposite = b.publicOrAccountIdComposite; + this.publicOrDomainPathComposite = b.publicOrDomainPathComposite; + this.zoneId = b.zoneId; + this.showRemoved = b.showRemoved; + this.templateStates = nullSafe(b.templateStates); + this.showUnique = b.showUnique; + this.startIndex = b.startIndex; + this.pageSize = b.pageSize; + this.sortAscending = b.sortAscending; + this.sharedAccountIds = nullSafe(b.sharedAccountIds); + this.domainPathLike = b.domainPathLike; + this.domainIdsForFeaturedCommunity = nullSafe(b.domainIdsForFeaturedCommunity); + this.tags = b.tags == null ? Collections.emptyMap() : Collections.unmodifiableMap(b.tags); + this.requiresViewFallback = b.requiresViewFallback; + } + + /** + * True if the bypass-the-view DAO path can handle this filter set. False + * if any "hard" filter is set that requires extra joins beyond the base 6 + * tables (vm_template, account, template_store_ref, image_store, + * template_zone_ref, data_center). + */ + public boolean canBypass() { + if (requiresViewFallback) { + return false; + } + if (!sharedAccountIds.isEmpty()) { + return false; + } + if (!domainIdsForFeaturedCommunity.isEmpty()) { + return false; + } + if (!tags.isEmpty()) { + return false; + } + return true; + } + + private static <T> List<T> nullSafe(List<T> in) { + return in == null ? Collections.<T>emptyList() : Collections.unmodifiableList(in); + } + + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + private Long templateId; + private List<Long> ids; + private String name; + private String keyword; + private HypervisorType hypervisorType; + private List<HypervisorType> availableHypervisors; + private ImageFormat format; + private boolean isIso; + private boolean excludeSystemTemplates; + private Boolean publicTemplate; + private Boolean featured; + private Boolean bootable; + private Long parentTemplateId; + private boolean onlyReady; + private Account.Type accountTypeNeq; + private Account.Type accountTypeEq; + private List<Long> accountIds; + private boolean accountIdRequiredForFilter; + private boolean publicOrAccountIdComposite; + private boolean publicOrDomainPathComposite; + private Long zoneId; + private boolean showRemoved; + private List<State> templateStates; + private boolean showUnique; + private Long startIndex; + private Long pageSize; + private boolean sortAscending = true; + private List<Long> sharedAccountIds; + private String domainPathLike; + private List<Long> domainIdsForFeaturedCommunity; + private Map<String, String> tags; + private boolean requiresViewFallback; + + public Builder templateId(Long v) { this.templateId = v; return this; } + public Builder ids(List<Long> v) { this.ids = v; return this; } + public Builder name(String v) { this.name = v; return this; } + public Builder keyword(String v) { this.keyword = v; return this; } + public Builder hypervisorType(HypervisorType v) { this.hypervisorType = v; return this; } + public Builder availableHypervisors(List<HypervisorType> v) { this.availableHypervisors = v; return this; } + public Builder format(ImageFormat v) { this.format = v; return this; } + public Builder isIso(boolean v) { this.isIso = v; return this; } + public Builder excludeSystemTemplates(boolean v) { this.excludeSystemTemplates = v; return this; } + public Builder publicTemplate(Boolean v) { this.publicTemplate = v; return this; } + public Builder featured(Boolean v) { this.featured = v; return this; } + public Builder bootable(Boolean v) { this.bootable = v; return this; } + public Builder parentTemplateId(Long v) { this.parentTemplateId = v; return this; } + public Builder onlyReady(boolean v) { this.onlyReady = v; return this; } + public Builder accountTypeNeq(Account.Type v) { this.accountTypeNeq = v; return this; } + public Builder accountTypeEq(Account.Type v) { this.accountTypeEq = v; return this; } + public Builder accountIds(List<Long> v) { this.accountIds = v; return this; } + public Builder accountIdRequiredForFilter(boolean v) { this.accountIdRequiredForFilter = v; return this; } + public Builder publicOrAccountIdComposite(boolean v) { this.publicOrAccountIdComposite = v; return this; } + public Builder publicOrDomainPathComposite(boolean v) { this.publicOrDomainPathComposite = v; return this; } + public Builder zoneId(Long v) { this.zoneId = v; return this; } + public Builder showRemoved(boolean v) { this.showRemoved = v; return this; } + public Builder templateStates(List<State> v) { this.templateStates = v; return this; } + public Builder showUnique(boolean v) { this.showUnique = v; return this; } + public Builder startIndex(Long v) { this.startIndex = v; return this; } + public Builder pageSize(Long v) { this.pageSize = v; return this; } + public Builder sortAscending(boolean v) { this.sortAscending = v; return this; } + public Builder sharedAccountIds(List<Long> v) { this.sharedAccountIds = v; return this; } + public Builder domainPathLike(String v) { this.domainPathLike = v; return this; } + public Builder domainIdsForFeaturedCommunity(List<Long> v) { this.domainIdsForFeaturedCommunity = v; return this; } + public Builder tags(Map<String, String> v) { this.tags = v; return this; } + public Builder requiresViewFallback(boolean v) { this.requiresViewFallback = v; return this; } + + public TemplateListFilter build() { + return new TemplateListFilter(this); + } + } +} diff --git a/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java b/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java new file mode 100644 index 00000000000..2fa3a53d0c3 --- /dev/null +++ b/server/src/test/java/com/cloud/api/query/dao/TemplateJoinDaoImplBypassExplainTest.java @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.api.query.dao; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.Statement; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.apache.log4j.Logger; +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +/** + * Integration test that EXPLAINs the Phase-1 bypass SQL versus the legacy + * template_view SQL, and asserts that the bypass plan no longer trips the + * "Range checked for each record" path on data_center. + * + * Skipped unless a MySQL URL is provided via system properties: + * -Dtest.cloudstack.mysql.url=jdbc:mysql://host:3306/cloud + * -Dtest.cloudstack.mysql.user=root + * -Dtest.cloudstack.mysql.password= + * + */ +public class TemplateJoinDaoImplBypassExplainTest { + + private static final Logger LOG = Logger.getLogger(TemplateJoinDaoImplBypassExplainTest.class); + + private static final String BYPASS_SQL = + "SELECT DISTINCT CONCAT(vt.id, '_', IFNULL(dc.id, 0)) AS pair " + + "FROM cloud.vm_template vt " + + "JOIN cloud.account a ON a.id = vt.account_id " + + "LEFT JOIN cloud.template_store_ref tsr " + + " ON tsr.template_id = vt.id AND tsr.store_role = 'Image' AND tsr.destroyed = 0 " + + "LEFT JOIN cloud.image_store img " + + " ON img.id = tsr.store_id AND img.removed IS NULL " + + "LEFT JOIN cloud.template_zone_ref tzr " + + " ON tzr.template_id = vt.id AND tsr.store_id IS NULL AND tzr.removed IS NULL " + + "LEFT JOIN cloud.data_center dc " + + " ON dc.id = COALESCE(img.data_center_id, tzr.zone_id) " + + "WHERE vt.removed IS NULL " + + " AND vt.state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress')"; + + /** + * Bypass SQL with the conditional `JOIN cloud.domain` added. + * Triggered when the request needs domain.path scoping (DA self/selfexecutable + * or non-admin all + SkipProjectResources). Should still use eq_ref / PRIMARY + * on data_center; domain itself is small. + */ + private static final String BYPASS_SQL_WITH_DOMAIN = + "SELECT DISTINCT CONCAT(vt.id, '_', IFNULL(dc.id, 0)) AS pair " + + "FROM cloud.vm_template vt " + + "JOIN cloud.account a ON a.id = vt.account_id " + + "LEFT JOIN cloud.template_store_ref tsr " + + " ON tsr.template_id = vt.id AND tsr.store_role = 'Image' AND tsr.destroyed = 0 " + + "LEFT JOIN cloud.image_store img " + + " ON img.id = tsr.store_id AND img.removed IS NULL " + + "LEFT JOIN cloud.template_zone_ref tzr " + + " ON tzr.template_id = vt.id AND tsr.store_id IS NULL AND tzr.removed IS NULL " + + "LEFT JOIN cloud.data_center dc " + + " ON dc.id = COALESCE(img.data_center_id, tzr.zone_id) " + + "JOIN cloud.domain d ON d.id = a.domain_id " + + "WHERE vt.removed IS NULL " + + " AND vt.state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress') " + + " AND d.path LIKE '/%' "; + + private static final String VIEW_SQL = + "SELECT DISTINCT temp_zone_pair FROM cloud.template_view " + + "WHERE template_state IN ('Active','UploadAbandoned','UploadError','NotUploaded','UploadInProgress')"; + + private Connection conn; + + @BeforeClass + public static void logQueriesUnderTest() { + // Print the before/after SQL once per test class run so logs make it + // obvious what shape the bypass replaces. + LOG.info("===== Phase 1 query — BEFORE (legacy template_view path) =====\n" + VIEW_SQL); + LOG.info("===== Phase 1 query — AFTER (bypass; 6 tables, COALESCE) =====\n" + BYPASS_SQL); + LOG.info("===== Phase 1 query — AFTER (bypass + domain join) ===========\n" + BYPASS_SQL_WITH_DOMAIN); + } + + @Before + public void setUp() throws Exception { + String url = System.getProperty("test.cloudstack.mysql.url"); + String user = System.getProperty("test.cloudstack.mysql.user", "root"); + String pwd = System.getProperty("test.cloudstack.mysql.password", ""); + Assume.assumeNotNull("test.cloudstack.mysql.url not set; skipping EXPLAIN integration test", url); + conn = DriverManager.getConnection(url, user, pwd); + } + + @After + public void tearDown() throws Exception { + if (conn != null) { + conn.close(); + } + } + + @Test + public void bypass_dataCenter_isPkLookup_notRangeChecked() throws Exception { + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL); + Map<String, String> dc = rowsByTable.get("dc"); + assertNotNull("EXPLAIN row for `dc` (data_center) missing", dc); + + String type = dc.get("type"); + String key = dc.get("key"); + String extra = dc.get("Extra") == null ? "" : dc.get("Extra"); + + // dc.id is the PK; COALESCE on the other side. Optimizer should pick eq_ref / ref / const. + assertTrue("dc access type is " + type + ", expected eq_ref/ref/const", + "eq_ref".equals(type) || "ref".equals(type) || "const".equals(type)); + assertEquals("dc should use PRIMARY", "PRIMARY", key); + assertFalse("dc should not use 'Range checked for each record'; Extra=" + extra, + extra.contains("Range checked for each record")); + } + + /** + * Asserts the bypass plan trims the join graph the legacy view drags along. + * + * Portable across CIB and prod + * + */ + @Test + public void view_joinsMoreTablesThanBypass() throws Exception { + Map<String, Map<String, String>> view = explain(VIEW_SQL); + Map<String, Map<String, String>> bypass = explain(BYPASS_SQL); + assertTrue( + "expected legacy view plan (" + view.size() + " tables) to join more than bypass (" + bypass.size() + ")", + view.size() > bypass.size()); + // Row-multiplier tables that the bypass intentionally drops. + assertTrue("expected view plan to include vm_template_details", + view.containsKey("vm_template_details")); + assertTrue("expected view plan to include resource_tags", + view.containsKey("resource_tags")); + } + + @Test + public void bypass_doesNotJoin_resourceTags_or_vmTemplateDetails() throws Exception { + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL); + assertFalse("bypass must not join resource_tags", rowsByTable.containsKey("resource_tags")); + assertFalse("bypass must not join vm_template_details", rowsByTable.containsKey("vm_template_details")); + assertFalse("bypass must not join launch_permission", rowsByTable.containsKey("launch_permission")); + } + + // ============================================================================ + // Bypass + conditional JOIN domain + // ============================================================================ + + @Test + public void bypassWithDomain_joinsDomainTable() throws Exception { + // The bypass-with-domain SQL must include a row for the `domain` table, + // since we add the join when domain.path scoping is requested. + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN); + assertTrue("bypass+domain plan must include domain table", rowsByTable.containsKey("d")); + } + + @Test + public void bypassWithDomain_domain_isPkLookup() throws Exception { + // domain.id is the PK; the join is on a.domain_id. Optimizer should pick eq_ref / ref / const. + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN); + Map<String, String> d = rowsByTable.get("d"); + assertNotNull("EXPLAIN row for `d` (domain) missing", d); + + String type = d.get("type"); + String key = d.get("key"); + String extra = d.get("Extra") == null ? "" : d.get("Extra"); + + assertTrue("domain access type is " + type + ", expected eq_ref/ref/const", + "eq_ref".equals(type) || "ref".equals(type) || "const".equals(type)); + assertEquals("domain should use PRIMARY", "PRIMARY", key); + assertFalse("domain should not be 'Range checked for each record'; Extra=" + extra, + extra.contains("Range checked for each record")); + } + + @Test + public void bypassWithDomain_dataCenter_stillPkLookup() throws Exception { + // Adding the domain join must not regress the data_center PK-lookup gain. + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN); + Map<String, String> dc = rowsByTable.get("dc"); + assertNotNull("EXPLAIN row for `dc` missing in bypass+domain plan", dc); + String type = dc.get("type"); + assertTrue("dc access type with domain join is " + type + ", expected eq_ref/ref/const", + "eq_ref".equals(type) || "ref".equals(type) || "const".equals(type)); + assertEquals("PRIMARY", dc.get("key")); + } + + @Test + public void bypassWithDomain_stillFewerTablesThanView() throws Exception { + // Even with the conditional domain join, the bypass plan is still smaller than the legacy view. + Map<String, Map<String, String>> bypass = explain(BYPASS_SQL_WITH_DOMAIN); + Map<String, Map<String, String>> view = explain(VIEW_SQL); + assertTrue( + "bypass+domain plan (" + bypass.size() + " tables) should still join fewer than the view (" + view.size() + ")", + bypass.size() < view.size()); + } + + @Test + public void bypassWithDomain_stillNoRowMultipliers() throws Exception { + // Critical: adding the domain join must not pull in the row multipliers the bypass dropped. + Map<String, Map<String, String>> rowsByTable = explain(BYPASS_SQL_WITH_DOMAIN); + assertFalse("bypass+domain must not join resource_tags", rowsByTable.containsKey("resource_tags")); + assertFalse("bypass+domain must not join vm_template_details", rowsByTable.containsKey("vm_template_details")); + assertFalse("bypass+domain must not join launch_permission", rowsByTable.containsKey("launch_permission")); + } + + /** + * Run EXPLAIN and return one map per table alias (or table name) → column→value. + * Uses traditional EXPLAIN format so column names match across MySQL versions. + * Also logs the raw EXPLAIN rows so test output makes plan differences visible. + */ + private Map<String, Map<String, String>> explain(String query) throws Exception { + Map<String, Map<String, String>> out = new LinkedHashMap<>(); + try (Statement st = conn.createStatement(); + ResultSet rs = st.executeQuery("EXPLAIN " + query)) { + ResultSetMetaData md = rs.getMetaData(); + int colCount = md.getColumnCount(); + + StringBuilder log = new StringBuilder("\nEXPLAIN result for query:\n ").append(query).append('\n'); + // header + for (int i = 1; i <= colCount; i++) { + log.append(md.getColumnLabel(i)).append('\t'); + } + log.append('\n'); + + while (rs.next()) { + Map<String, String> row = new LinkedHashMap<>(); + for (int i = 1; i <= colCount; i++) { + String val = rs.getString(i); + row.put(md.getColumnLabel(i), val); + log.append(val == null ? "NULL" : val).append('\t'); + } + log.append('\n'); + String tableKey = row.get("table"); + if (tableKey != null) { + out.put(tableKey, row); + } + } + LOG.info(log.toString()); + } + return out; + } +}
