From 388f32f6bb1015c8a1c532cbfcee3b9dd47b5c62 Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Fri, 6 Nov 2020 12:34:33 +0100 Subject: [PATCH 1/2] Consider other conditions while listing templates with id Many of the searching conditions are not considered when template id is passed but they are considered only when template id is not passed. Move other conditions out of else part so that they will be considered when id is passed. --- .../com/cloud/api/query/QueryManagerImpl.java | 117 +++++++++--------- 1 file changed, 56 insertions(+), 61 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 e8161bc4c652..4bf15292579c 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3434,11 +3434,6 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP domain = _domainDao.findById(Domain.ROOT_DOMAIN); } - // List hypers = null; - // if (!isIso) { - // hypers = _resourceMgr.listAvailHypervisorInZone(null, null); - // } - setIdsListToSearchCriteria(sc, ids); // add criteria for project or not @@ -3486,17 +3481,6 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP } } - if (!isIso) { - // add hypervisor criteria for template case - if (hypers != null && !hypers.isEmpty()) { - String[] relatedHypers = new String[hypers.size()]; - for (int i = 0; i < hypers.size(); i++) { - relatedHypers[i] = hypers.get(i).toString(); - } - sc.addAnd("hypervisorType", SearchCriteria.Op.IN, relatedHypers); - } - } - // control different template filters if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) { sc.addAnd("publicTemplate", SearchCriteria.Op.EQ, true); @@ -3539,62 +3523,73 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP } sc.addAnd("publicTemplate", SearchCriteria.Op.SC, scc); } + } - // add tags criteria - if (tags != null && !tags.isEmpty()) { - SearchCriteria scc = _templateJoinDao.createSearchCriteria(); - for (Map.Entry entry : tags.entrySet()) { - SearchCriteria scTag = _templateJoinDao.createSearchCriteria(); - scTag.addAnd("tagKey", SearchCriteria.Op.EQ, entry.getKey()); - scTag.addAnd("tagValue", SearchCriteria.Op.EQ, entry.getValue()); - if (isIso) { - scTag.addAnd("tagResourceType", SearchCriteria.Op.EQ, ResourceObjectType.ISO); - } else { - scTag.addAnd("tagResourceType", SearchCriteria.Op.EQ, ResourceObjectType.Template); - } - scc.addOr("tagKey", SearchCriteria.Op.SC, scTag); + if (!isIso) { + // add hypervisor criteria for template case + if (hypers != null && !hypers.isEmpty()) { + String[] relatedHypers = new String[hypers.size()]; + for (int i = 0; i < hypers.size(); i++) { + relatedHypers[i] = hypers.get(i).toString(); } - sc.addAnd("tagKey", SearchCriteria.Op.SC, scc); + sc.addAnd("hypervisorType", SearchCriteria.Op.IN, relatedHypers); } + } - // other criteria - - if (keyword != null) { - sc.addAnd("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); - } else if (name != null) { - sc.addAnd("name", SearchCriteria.Op.EQ, name); + // add tags criteria + if (tags != null && !tags.isEmpty()) { + SearchCriteria scc = _templateJoinDao.createSearchCriteria(); + for (Map.Entry entry : tags.entrySet()) { + SearchCriteria scTag = _templateJoinDao.createSearchCriteria(); + scTag.addAnd("tagKey", SearchCriteria.Op.EQ, entry.getKey()); + scTag.addAnd("tagValue", SearchCriteria.Op.EQ, entry.getValue()); + if (isIso) { + scTag.addAnd("tagResourceType", SearchCriteria.Op.EQ, ResourceObjectType.ISO); + } else { + scTag.addAnd("tagResourceType", SearchCriteria.Op.EQ, ResourceObjectType.Template); + } + scc.addOr("tagKey", SearchCriteria.Op.SC, scTag); } + sc.addAnd("tagKey", SearchCriteria.Op.SC, scc); + } - if (isIso) { - sc.addAnd("format", SearchCriteria.Op.EQ, "ISO"); + // other criteria - } else { - sc.addAnd("format", SearchCriteria.Op.NEQ, "ISO"); - } + if (keyword != null) { + sc.addAnd("name", SearchCriteria.Op.LIKE, "%" + keyword + "%"); + } else if (name != null) { + sc.addAnd("name", SearchCriteria.Op.EQ, name); + } - if (!hyperType.equals(HypervisorType.None)) { - sc.addAnd("hypervisorType", SearchCriteria.Op.EQ, hyperType); - } + if (isIso) { + sc.addAnd("format", SearchCriteria.Op.EQ, "ISO"); - if (bootable != null) { - sc.addAnd("bootable", SearchCriteria.Op.EQ, bootable); - } + } else { + sc.addAnd("format", SearchCriteria.Op.NEQ, "ISO"); + } - if (onlyReady) { - SearchCriteria readySc = _templateJoinDao.createSearchCriteria(); - readySc.addOr("state", SearchCriteria.Op.EQ, TemplateState.Ready); - readySc.addOr("format", SearchCriteria.Op.EQ, ImageFormat.BAREMETAL); - SearchCriteria isoPerhostSc = _templateJoinDao.createSearchCriteria(); - isoPerhostSc.addAnd("format", SearchCriteria.Op.EQ, ImageFormat.ISO); - isoPerhostSc.addAnd("templateType", SearchCriteria.Op.EQ, TemplateType.PERHOST); - readySc.addOr("templateType", SearchCriteria.Op.SC, isoPerhostSc); - sc.addAnd("state", SearchCriteria.Op.SC, readySc); - } + if (!hyperType.equals(HypervisorType.None)) { + sc.addAnd("hypervisorType", SearchCriteria.Op.EQ, hyperType); + } - if (!showDomr) { - // excluding system template - sc.addAnd("templateType", SearchCriteria.Op.NEQ, Storage.TemplateType.SYSTEM); - } + if (bootable != null) { + sc.addAnd("bootable", SearchCriteria.Op.EQ, bootable); + } + + if (onlyReady) { + SearchCriteria readySc = _templateJoinDao.createSearchCriteria(); + readySc.addOr("state", SearchCriteria.Op.EQ, TemplateState.Ready); + readySc.addOr("format", SearchCriteria.Op.EQ, ImageFormat.BAREMETAL); + SearchCriteria isoPerhostSc = _templateJoinDao.createSearchCriteria(); + isoPerhostSc.addAnd("format", SearchCriteria.Op.EQ, ImageFormat.ISO); + isoPerhostSc.addAnd("templateType", SearchCriteria.Op.EQ, TemplateType.PERHOST); + readySc.addOr("templateType", SearchCriteria.Op.SC, isoPerhostSc); + sc.addAnd("state", SearchCriteria.Op.SC, readySc); + } + + if (!showDomr) { + // excluding system template + sc.addAnd("templateType", SearchCriteria.Op.NEQ, Storage.TemplateType.SYSTEM); } if (zoneId != null) { From 1a47099e5f2c0dc00ee2f6ab3a30bbe14b1a2435 Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Fri, 6 Nov 2020 16:26:21 +0100 Subject: [PATCH 2/2] Move to functions --- .../com/cloud/api/query/QueryManagerImpl.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 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 4bf15292579c..d488bbab407b 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3525,6 +3525,15 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP } } + return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, + showRemovedTmpl, parentTemplateId, showUnique, searchFilter, sc); + + } + + private Pair, Integer> templateChecks(boolean isIso, List hypers, Map tags, String name, String keyword, + HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, + boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique, + Filter searchFilter, SearchCriteria sc) { if (!isIso) { // add hypervisor criteria for template case if (hypers != null && !hypers.isEmpty()) { @@ -3561,12 +3570,8 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP sc.addAnd("name", SearchCriteria.Op.EQ, name); } - if (isIso) { - sc.addAnd("format", SearchCriteria.Op.EQ, "ISO"); - - } else { - sc.addAnd("format", SearchCriteria.Op.NEQ, "ISO"); - } + SearchCriteria.Op op = isIso ? Op.EQ : Op.NEQ; + sc.addAnd("format", op, "ISO"); if (!hyperType.equals(HypervisorType.None)) { sc.addAnd("hypervisorType", SearchCriteria.Op.EQ, hyperType); @@ -3634,7 +3639,6 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.ACCOUNT_TYP // VMTemplateDaoImpl.searchForTemplates and understand why we need to // specially handle ISO. The original logic is very twisted and no idea // about what the code was doing. - } // findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair