From b2501452faeb473b355abc8ba9cc318b493285a9 Mon Sep 17 00:00:00 2001 From: davidjumani Date: Wed, 10 Jun 2020 16:34:24 +0530 Subject: [PATCH 01/10] Adding showunique parameter to list templates and isos --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../api/command/user/iso/ListIsosCmd.java | 8 +++++++ .../user/template/ListTemplatesCmd.java | 8 +++++++ .../com/cloud/api/query/QueryManagerImpl.java | 24 +++++++++++++++---- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index e8595e677459..83ec10a9e2d7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -300,6 +300,7 @@ public class ApiConstants { public static final String SESSIONKEY = "sessionkey"; public static final String SHOW_CAPACITIES = "showcapacities"; public static final String SHOW_REMOVED = "showremoved"; + public static final String SHOW_UNIQUE = "showunique"; public static final String SIGNATURE = "signature"; public static final String SIGNATURE_VERSION = "signatureversion"; public static final String SIZE = "size"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java index dee60f4f8d53..919285451db3 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java @@ -78,6 +78,10 @@ public class ListIsosCmd extends BaseListTaggedResourcesCmd { @Parameter(name=ApiConstants.SHOW_REMOVED, type=CommandType.BOOLEAN, description="show removed ISOs as well") private Boolean showRemoved; + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, " + + "list only unique resources despite being accross multiple zones") + private Boolean showUnique; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -118,6 +122,10 @@ public Boolean getShowRemoved() { return (showRemoved != null ? showRemoved : false); } + public Boolean getShowUnique() { + return (showUnique != null ? showUnique : false); + } + public boolean listInReadyState() { Account account = CallContext.current().getCallingAccount(); // It is account specific if account is admin type and domainId and accountName are not null diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java index e7d328495ed6..1da7b243753c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java @@ -78,6 +78,10 @@ public class ListTemplatesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.PARENT_TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list datadisk templates by parent template id", since = "4.4") private Long parentTemplateId; + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, " + + "list only unique resources despite being accross multiple zones") + private Boolean showUnique; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -147,4 +151,8 @@ public void execute() { public List getIds() { return ids; } + + public Boolean getShowUnique() { + return (showUnique != null ? showUnique : false); + } } 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 fae76122f076..f6f75f15005d 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3154,12 +3154,12 @@ private Pair, Integer> searchForTemplatesInternal(ListTempl HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor()); return searchForTemplatesInternal(id, cmd.getTemplateName(), cmd.getKeyword(), templateFilter, false, null, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType, - showDomr, cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl, cmd.getIds(), parentTemplateId); + showDomr, cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl, cmd.getIds(), parentTemplateId, cmd.getShowUnique()); } private Pair, Integer> searchForTemplatesInternal(Long templateId, String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, boolean showDomr, boolean onlyReady, List permittedAccounts, Account caller, - ListProjectResourcesCriteria listProjectResourcesCriteria, Map tags, boolean showRemovedTmpl, List ids, Long parentTemplateId) { + ListProjectResourcesCriteria listProjectResourcesCriteria, Map tags, boolean showRemovedTmpl, List ids, Long parentTemplateId, Boolean showUnique) { // check if zone is configured, if not, just return empty list List hypers = null; @@ -3425,8 +3425,22 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP List uniqueTmpls = uniqueTmplPair.first(); String[] tzIds = new String[uniqueTmpls.size()]; int i = 0; - for (TemplateJoinVO v : uniqueTmpls) { - tzIds[i++] = v.getTempZonePair(); + + if (showUnique) { + // Get only one tempZonePair per template + Set tzSet = new HashSet<>(); + for (TemplateJoinVO v : uniqueTmpls) { + Long tzId = Long.parseLong(v.getTempZonePair().split("_")[0]); + if (!tzSet.contains(tzId)) { + tzSet.add(tzId); + tzIds[i++] = v.getTempZonePair(); + } + } + count = tzSet.size(); + } else { + for (TemplateJoinVO v : uniqueTmpls) { + tzIds[i++] = v.getTempZonePair(); + } } List vrs = _templateJoinDao.searchByTemplateZonePair(showRemovedTmpl, tzIds); return new Pair, Integer>(vrs, count); @@ -3480,7 +3494,7 @@ private Pair, Integer> searchForIsosInternal(ListIsosCmd cm HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor()); return searchForTemplatesInternal(cmd.getId(), cmd.getIsoName(), cmd.getKeyword(), isoFilter, true, cmd.isBootable(), cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), - hypervisorType, true, cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedISO, null, null); + hypervisorType, true, cmd.listInReadyState(), permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedISO, null, null, cmd.getShowUnique()); } @Override From 775e4872b104fd9238555025960de8108d3320a0 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 11 Jun 2020 15:00:27 +0530 Subject: [PATCH 02/10] Update ListIsosCmd.java --- .../org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java index 919285451db3..31c62843fd48 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java @@ -79,7 +79,7 @@ public class ListIsosCmd extends BaseListTaggedResourcesCmd { private Boolean showRemoved; @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, " - + "list only unique resources despite being accross multiple zones") + + "list only unique isos across zones") private Boolean showUnique; ///////////////////////////////////////////////////// From 77ba5a873a97046cfa1ab4ad32a438d2894d1cd3 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 11 Jun 2020 15:01:31 +0530 Subject: [PATCH 03/10] Update ListTemplatesCmd.java --- .../cloudstack/api/command/user/template/ListTemplatesCmd.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java index 1da7b243753c..fd0bbd273be4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java @@ -78,8 +78,7 @@ public class ListTemplatesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.PARENT_TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list datadisk templates by parent template id", since = "4.4") private Long parentTemplateId; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, " - + "list only unique resources despite being accross multiple zones") + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones") private Boolean showUnique; ///////////////////////////////////////////////////// From e9f48ff2d20d2b5dfb35a80341ef4cf9c4ac5cf6 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 11 Jun 2020 15:02:33 +0530 Subject: [PATCH 04/10] Update ListIsosCmd.java --- .../apache/cloudstack/api/command/user/iso/ListIsosCmd.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java index 31c62843fd48..d37af57cd49f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java @@ -78,8 +78,7 @@ public class ListIsosCmd extends BaseListTaggedResourcesCmd { @Parameter(name=ApiConstants.SHOW_REMOVED, type=CommandType.BOOLEAN, description="show removed ISOs as well") private Boolean showRemoved; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, " - + "list only unique isos across zones") + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique isos across zones") private Boolean showUnique; ///////////////////////////////////////////////////// From 1997abf22c2d694e11ed6bd50e7a5ffabeb277ee Mon Sep 17 00:00:00 2001 From: davidjumani Date: Fri, 12 Jun 2020 11:27:18 +0530 Subject: [PATCH 05/10] Fixing count bug --- .../com/cloud/api/query/QueryManagerImpl.java | 41 +++++++++++++------ .../cloud/api/query/dao/TemplateJoinDao.java | 2 + .../api/query/dao/TemplateJoinDaoImpl.java | 16 ++++++++ 3 files changed, 46 insertions(+), 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 f6f75f15005d..03a96a767234 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3176,7 +3176,11 @@ private Pair, Integer> searchForTemplatesInternal(Long temp searchFilter.addOrderBy(TemplateJoinVO.class, "tempZonePair", SortKeyAscending.value()); SearchBuilder sb = _templateJoinDao.createSearchBuilder(); - sb.select(null, Func.DISTINCT, sb.entity().getTempZonePair()); // select distinct (templateId, zoneId) pair + 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); } @@ -3413,8 +3417,13 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP uniqueTmplPair = _templateJoinDao.searchIncludingRemovedAndCount(sc, searchFilter); } else { sc.addAnd("templateState", SearchCriteria.Op.IN, new State[] {State.Active, State.UploadAbandoned, State.UploadError, State.NotUploaded, State.UploadInProgress}); - final String[] distinctColumns = {"temp_zone_pair"}; - uniqueTmplPair = _templateJoinDao.searchAndDistinctCount(sc, searchFilter, distinctColumns); + if (showUnique) { + final String[] distinctColumns = {"id"}; + uniqueTmplPair = _templateJoinDao.searchAndDistinctCount(sc, searchFilter, distinctColumns); + } else { + final String[] distinctColumns = {"temp_zone_pair"}; + uniqueTmplPair = _templateJoinDao.searchAndDistinctCount(sc, searchFilter, distinctColumns); + } } Integer count = uniqueTmplPair.second(); @@ -3423,26 +3432,32 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP return uniqueTmplPair; } List uniqueTmpls = uniqueTmplPair.first(); - String[] tzIds = new String[uniqueTmpls.size()]; int i = 0; - + List vrs = null; if (showUnique) { - // Get only one tempZonePair per template - Set tzSet = new HashSet<>(); + Long[] tzIds = new Long[uniqueTmpls.size()]; for (TemplateJoinVO v : uniqueTmpls) { - Long tzId = Long.parseLong(v.getTempZonePair().split("_")[0]); - if (!tzSet.contains(tzId)) { - tzSet.add(tzId); - tzIds[i++] = v.getTempZonePair(); + tzIds[i++] = v.getId(); + } + vrs = _templateJoinDao.findByIds(tzIds); + + // Get only unique id rows + Long lastId = null; + for(i = vrs.size() - 1; i >= 0; i--) { + if (new Long(vrs.get(i).getId()).equals(lastId)) { + vrs.remove(i); + continue; } + lastId = vrs.get(i).getId(); } - count = tzSet.size(); } else { + String[] tzIds = new String[uniqueTmpls.size()]; for (TemplateJoinVO v : uniqueTmpls) { tzIds[i++] = v.getTempZonePair(); } + vrs = _templateJoinDao.searchByTemplateZonePair(showRemovedTmpl, tzIds); } - List vrs = _templateJoinDao.searchByTemplateZonePair(showRemovedTmpl, tzIds); + return new Pair, Integer>(vrs, count); // TODO: revisit the special logic for iso search in 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 298be4d6f01f..dcba560121dc 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 @@ -48,4 +48,6 @@ public interface TemplateJoinDao extends GenericDao { Pair, Integer> searchIncludingRemovedAndCount(final SearchCriteria sc, final Filter filter); + List findByIds(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 54686f73df2a..8053667d5bae 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 @@ -73,6 +73,8 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation tmpltIdSearch; + private final SearchBuilder tmpltIdsSearch; + private final SearchBuilder tmpltZoneSearch; private final SearchBuilder activeTmpltSearch; @@ -88,6 +90,10 @@ protected TemplateJoinDaoImpl() { tmpltIdSearch.and("id", tmpltIdSearch.entity().getId(), SearchCriteria.Op.EQ); tmpltIdSearch.done(); + tmpltIdsSearch = createSearchBuilder(); + tmpltIdsSearch.and("idsIN", tmpltIdsSearch.entity().getId(), SearchCriteria.Op.IN); + tmpltIdsSearch.done(); + tmpltZoneSearch = createSearchBuilder(); tmpltZoneSearch.and("id", tmpltZoneSearch.entity().getId(), SearchCriteria.Op.EQ); tmpltZoneSearch.and("dataCenterId", tmpltZoneSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ); @@ -481,4 +487,14 @@ public Pair, Integer> searchIncludingRemovedAndCount(final return new Pair, Integer>(objects, count); } + @Override + public List findByIds(Long[] ids) { + if (ids.length == 0) { + return new ArrayList(); + } + SearchCriteria sc = tmpltIdsSearch.create(); + sc.setParameters("idsIN", ids); + return searchIncludingRemoved(sc, null, null, false); + } + } From 8ccdfd190ff50d5c98fdec3ec9e3e59e5668aa36 Mon Sep 17 00:00:00 2001 From: davidjumani Date: Mon, 15 Jun 2020 11:19:24 +0530 Subject: [PATCH 06/10] Simplifying --- .../cloudstack/api/command/user/iso/ListIsosCmd.java | 6 +++--- .../api/command/user/template/ListTemplatesCmd.java | 12 ++++++------ .../java/com/cloud/api/query/QueryManagerImpl.java | 12 +----------- .../com/cloud/api/query/dao/TemplateJoinDao.java | 3 +-- .../com/cloud/api/query/dao/TemplateJoinDaoImpl.java | 3 ++- 5 files changed, 13 insertions(+), 23 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java index d37af57cd49f..ed079c05ed55 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java @@ -78,7 +78,7 @@ public class ListIsosCmd extends BaseListTaggedResourcesCmd { @Parameter(name=ApiConstants.SHOW_REMOVED, type=CommandType.BOOLEAN, description="show removed ISOs as well") private Boolean showRemoved; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique isos across zones") + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique isos across zones", since = "4.15") private Boolean showUnique; ///////////////////////////////////////////////////// @@ -118,11 +118,11 @@ public Long getZoneId() { } public Boolean getShowRemoved() { - return (showRemoved != null ? showRemoved : false); + return showRemoved != null && showRemoved; } public Boolean getShowUnique() { - return (showUnique != null ? showUnique : false); + return showUnique != null && showUnique; } public boolean listInReadyState() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java index fd0bbd273be4..ac3dacba5f0a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java @@ -78,7 +78,7 @@ public class ListTemplatesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.PARENT_TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list datadisk templates by parent template id", since = "4.4") private Long parentTemplateId; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones") + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones", since = "4.15") private Boolean showUnique; ///////////////////////////////////////////////////// @@ -106,7 +106,11 @@ public Long getZoneId() { } public Boolean getShowRemoved() { - return (showRemoved != null ? showRemoved : false); + return showRemoved != null && showRemoved; + } + + public Boolean getShowUnique() { + return showUnique != null && showUnique; } public Long getParentTemplateId() { @@ -150,8 +154,4 @@ public void execute() { public List getIds() { return ids; } - - public Boolean getShowUnique() { - return (showUnique != null ? showUnique : false); - } } 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 03a96a767234..57868c53aa64 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3439,17 +3439,7 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP for (TemplateJoinVO v : uniqueTmpls) { tzIds[i++] = v.getId(); } - vrs = _templateJoinDao.findByIds(tzIds); - - // Get only unique id rows - Long lastId = null; - for(i = vrs.size() - 1; i >= 0; i--) { - if (new Long(vrs.get(i).getId()).equals(lastId)) { - vrs.remove(i); - continue; - } - lastId = vrs.get(i).getId(); - } + vrs = _templateJoinDao.findByDistinctIds(tzIds); } else { String[] tzIds = new String[uniqueTmpls.size()]; for (TemplateJoinVO v : uniqueTmpls) { 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 dcba560121dc..c9d7eba48b2f 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 @@ -48,6 +48,5 @@ public interface TemplateJoinDao extends GenericDao { Pair, Integer> searchIncludingRemovedAndCount(final SearchCriteria sc, final Filter filter); - List findByIds(Long[] ids); - + List 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 8053667d5bae..75ba3ca74633 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 @@ -92,6 +92,7 @@ protected TemplateJoinDaoImpl() { tmpltIdsSearch = createSearchBuilder(); tmpltIdsSearch.and("idsIN", tmpltIdsSearch.entity().getId(), SearchCriteria.Op.IN); + tmpltIdsSearch.groupBy(tmpltIdsSearch.entity().getId()); tmpltIdsSearch.done(); tmpltZoneSearch = createSearchBuilder(); @@ -488,7 +489,7 @@ public Pair, Integer> searchIncludingRemovedAndCount(final } @Override - public List findByIds(Long[] ids) { + public List findByDistinctIds(Long... ids) { if (ids.length == 0) { return new ArrayList(); } From 94963dffc16583a4dc8805c70fa11cf41c7f56b5 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 16 Jun 2020 09:52:14 +0530 Subject: [PATCH 07/10] Update ListTemplatesCmd.java --- .../api/command/user/template/ListTemplatesCmd.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java index ac3dacba5f0a..6b225831f54a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java @@ -74,13 +74,13 @@ public class ListTemplatesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.SHOW_REMOVED, type = CommandType.BOOLEAN, description = "show removed templates as well") private Boolean showRemoved; + + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones", since = "4.13.2") + private Boolean showUnique; @Parameter(name = ApiConstants.PARENT_TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list datadisk templates by parent template id", since = "4.4") private Long parentTemplateId; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones", since = "4.15") - private Boolean showUnique; - ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// From b83a4c47046d782114d74f32056150a89bd0190c Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 16 Jun 2020 09:52:31 +0530 Subject: [PATCH 08/10] Update ListIsosCmd.java --- .../org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java index ed079c05ed55..d45c8cd37e9b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java @@ -78,7 +78,7 @@ public class ListIsosCmd extends BaseListTaggedResourcesCmd { @Parameter(name=ApiConstants.SHOW_REMOVED, type=CommandType.BOOLEAN, description="show removed ISOs as well") private Boolean showRemoved; - @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique isos across zones", since = "4.15") + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique isos across zones", since = "4.13.2") private Boolean showUnique; ///////////////////////////////////////////////////// From 83bd4177373f131c64107983b90a806aacb46c5f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 16 Jun 2020 11:29:22 +0530 Subject: [PATCH 09/10] fix trailing white space - remind me to not use Github editor Signed-off-by: Rohit Yadav --- .../cloudstack/api/command/user/template/ListTemplatesCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java index 6b225831f54a..481cfd13406d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java @@ -74,7 +74,7 @@ public class ListTemplatesCmd extends BaseListTaggedResourcesCmd { @Parameter(name = ApiConstants.SHOW_REMOVED, type = CommandType.BOOLEAN, description = "show removed templates as well") private Boolean showRemoved; - + @Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, description = "If set to true, list only unique templates across zones", since = "4.13.2") private Boolean showUnique; From 28137f6283093793c66497e97c3e101b12062429 Mon Sep 17 00:00:00 2001 From: davidjumani Date: Wed, 17 Jun 2020 10:10:37 +0530 Subject: [PATCH 10/10] Refactoring for showUnique templates --- .../com/cloud/api/query/QueryManagerImpl.java | 43 +++++++++---------- .../api/query/dao/TemplateJoinDaoImpl.java | 2 +- 2 files changed, 21 insertions(+), 24 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 57868c53aa64..127ac9037a1c 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3426,29 +3426,7 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP } } - Integer count = uniqueTmplPair.second(); - if (count.intValue() == 0) { - // empty result - return uniqueTmplPair; - } - List uniqueTmpls = uniqueTmplPair.first(); - int i = 0; - List vrs = null; - if (showUnique) { - Long[] tzIds = new Long[uniqueTmpls.size()]; - for (TemplateJoinVO v : uniqueTmpls) { - tzIds[i++] = v.getId(); - } - vrs = _templateJoinDao.findByDistinctIds(tzIds); - } else { - String[] tzIds = new String[uniqueTmpls.size()]; - for (TemplateJoinVO v : uniqueTmpls) { - tzIds[i++] = v.getTempZonePair(); - } - vrs = _templateJoinDao.searchByTemplateZonePair(showRemovedTmpl, tzIds); - } - - return new Pair, Integer>(vrs, count); + return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique); // TODO: revisit the special logic for iso search in // VMTemplateDaoImpl.searchForTemplates and understand why we need to @@ -3457,6 +3435,25 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP } + // findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair + private Pair, Integer> findTemplatesByIdOrTempZonePair(Pair, Integer> templateDataPair, boolean showRemoved, boolean showUnique) { + Integer count = templateDataPair.second(); + if (count.intValue() == 0) { + // empty result + return templateDataPair; + } + List templateData = templateDataPair.first(); + List templates = null; + if (showUnique) { + Long[] templateIds = templateData.stream().map(template -> template.getId()).toArray(Long[]::new); + templates = _templateJoinDao.findByDistinctIds(templateIds); + } else { + String[] templateZonePairs = templateData.stream().map(template -> template.getTempZonePair()).toArray(String[]::new); + templates = _templateJoinDao.searchByTemplateZonePair(showRemoved, templateZonePairs); + } + return new Pair, Integer>(templates, count); + } + @Override public ListResponse listIsos(ListIsosCmd cmd) { Pair, Integer> result = searchForIsosInternal(cmd); 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 75ba3ca74633..27380ffaa936 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 @@ -490,7 +490,7 @@ public Pair, Integer> searchIncludingRemovedAndCount(final @Override public List findByDistinctIds(Long... ids) { - if (ids.length == 0) { + if (ids == null || ids.length == 0) { return new ArrayList(); } SearchCriteria sc = tmpltIdsSearch.create();