From a38c18220cf28c122aeed8a15638f36b4fc5ac1b Mon Sep 17 00:00:00 2001 From: nit Date: Mon, 6 Dec 2010 19:47:33 +0530 Subject: [PATCH] bug 7410 : listTemplate - Correcting the pagesize use. Putting all the filters in the DB query rather than after execution so that pagesize restriction happens at the end. --- api/src/com/cloud/api/ResponseGenerator.java | 2 +- .../cloud/api/commands/ListTemplatesCmd.java | 8 +---- .../src/com/cloud/api/ApiResponseHelper.java | 5 +-- .../cloud/server/ManagementServerImpl.java | 26 +++++++++++--- .../com/cloud/storage/dao/VMTemplateDao.java | 3 +- .../cloud/storage/dao/VMTemplateDaoImpl.java | 35 +++++++++++++++---- 6 files changed, 55 insertions(+), 24 deletions(-) diff --git a/api/src/com/cloud/api/ResponseGenerator.java b/api/src/com/cloud/api/ResponseGenerator.java index 6f2f6ad2ef7..ef81d130863 100644 --- a/api/src/com/cloud/api/ResponseGenerator.java +++ b/api/src/com/cloud/api/ResponseGenerator.java @@ -168,7 +168,7 @@ public interface ResponseGenerator { RemoteAccessVpnResponse createRemoteAccessVpnResponse(RemoteAccessVpn vpn); - void createTemplateResponse(List responses, VirtualMachineTemplate template, boolean onlyReady, Long zoneId, boolean isAdmin, + void createTemplateResponse(List responses, VirtualMachineTemplate template, Long zoneId, boolean isAdmin, Account account); ListResponse createTemplateResponse2(VirtualMachineTemplate template, Long zoneId); diff --git a/api/src/com/cloud/api/commands/ListTemplatesCmd.java b/api/src/com/cloud/api/commands/ListTemplatesCmd.java index b9776370728..b02c1fbbeee 100644 --- a/api/src/com/cloud/api/commands/ListTemplatesCmd.java +++ b/api/src/com/cloud/api/commands/ListTemplatesCmd.java @@ -137,12 +137,6 @@ public class ListTemplatesCmd extends BaseListCmd { } } - boolean onlyReady = (templateFilterObj == TemplateFilter.featured) || - (templateFilterObj == TemplateFilter.selfexecutable) || - (templateFilterObj == TemplateFilter.sharedexecutable) || - (templateFilterObj == TemplateFilter.executable && isAccountSpecific) || - (templateFilterObj == TemplateFilter.community); - boolean showDomr = (templateFilterObj != TemplateFilter.selfexecutable); ListResponse response = new ListResponse(); @@ -152,7 +146,7 @@ public class ListTemplatesCmd extends BaseListCmd { if (!showDomr && template.getTemplateType() == Storage.TemplateType.SYSTEM) { continue; } - _responseGenerator.createTemplateResponse(templateResponses, template, onlyReady, zoneId, isAdmin, account); + _responseGenerator.createTemplateResponse(templateResponses, template, zoneId, isAdmin, account); } response.setResponses(templateResponses); diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index 67595f6ee02..e5806c93014 100644 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -1455,13 +1455,10 @@ public class ApiResponseHelper implements ResponseGenerator { } @Override - public void createTemplateResponse(List responses, VirtualMachineTemplate template, boolean onlyReady, Long zoneId, boolean isAdmin, Account account) { + public void createTemplateResponse(List responses, VirtualMachineTemplate template, Long zoneId, boolean isAdmin, Account account) { List templateHostRefsForTemplate = ApiDBUtils.listTemplateHostBy(template.getId(), zoneId); for (VMTemplateHostVO templateHostRef : templateHostRefsForTemplate) { - if (onlyReady && templateHostRef.getDownloadState() != Status.DOWNLOADED) { - continue; - } TemplateResponse templateResponse = new TemplateResponse(); templateResponse.setId(template.getId()); diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 0652385eadd..f8f1ed59dbc 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -1582,8 +1582,13 @@ public class ManagementServerImpl implements ManagementServer { accountId = account.getId(); } + //It is account specific if account is admin type and domainId and accountName are not null + boolean isAccountSpecific = (account == null || isAdmin(account.getType())) + && (accountName != null) + && (domainId != null); + HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor()); - return listTemplates(cmd.getId(), cmd.getIsoName(), cmd.getKeyword(), isoFilter, true, cmd.isBootable(), accountId, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType); + return listTemplates(cmd.getId(), cmd.getIsoName(), cmd.getKeyword(), isoFilter, true, cmd.isBootable(), accountId, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType, isAccountSpecific, true); } @Override @@ -1613,11 +1618,17 @@ public class ManagementServerImpl implements ManagementServer { accountId = account.getId(); } + //It is account specific if account is admin type and domainId and accountName are not null + boolean isAccountSpecific = (account == null || isAdmin(account.getType())) + && (accountName != null) + && (domainId != null); + boolean showDomr = (templateFilter != TemplateFilter.selfexecutable); HypervisorType hypervisorType = HypervisorType.getType(cmd.getHypervisor()); - return listTemplates(cmd.getId(), cmd.getTemplateName(), cmd.getKeyword(), templateFilter, false, null, accountId, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType); + + return listTemplates(cmd.getId(), cmd.getTemplateName(), cmd.getKeyword(), templateFilter, false, null, accountId, cmd.getPageSizeVal(), cmd.getStartIndex(), cmd.getZoneId(), hypervisorType, isAccountSpecific, showDomr); } - private List listTemplates(Long templateId, String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Long accountId, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType) throws InvalidParameterValueException { + private List listTemplates(Long templateId, String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Long accountId, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, boolean isAccountSpecific, boolean showDomr) throws InvalidParameterValueException { VMTemplateVO template = null; if (templateId != null) { template = _templateDao.findById(templateId); @@ -1634,6 +1645,13 @@ public class ManagementServerImpl implements ManagementServer { } } + // Show only those that are downloaded. + boolean onlyReady = (templateFilter == TemplateFilter.featured) || + (templateFilter == TemplateFilter.selfexecutable) || + (templateFilter == TemplateFilter.sharedexecutable) || + (templateFilter == TemplateFilter.executable && isAccountSpecific) || + (templateFilter == TemplateFilter.community); + Account account = null; DomainVO domain = null; if (accountId != null) { @@ -1646,7 +1664,7 @@ public class ManagementServerImpl implements ManagementServer { List templates = new ArrayList(); if (template == null) { - templates = _templateDao.searchTemplates(name, keyword, templateFilter, isIso, bootable, account, domain, pageSize, startIndex, zoneId, hyperType); + templates = _templateDao.searchTemplates(name, keyword, templateFilter, isIso, bootable, account, domain, pageSize, startIndex, zoneId, hyperType, onlyReady, showDomr); } else { templates = new ArrayList(); templates.add(template); diff --git a/server/src/com/cloud/storage/dao/VMTemplateDao.java b/server/src/com/cloud/storage/dao/VMTemplateDao.java index d3499984e36..ae7302d0fac 100644 --- a/server/src/com/cloud/storage/dao/VMTemplateDao.java +++ b/server/src/com/cloud/storage/dao/VMTemplateDao.java @@ -56,7 +56,8 @@ public interface VMTemplateDao extends GenericDao { public List findIsosByIdAndPath(Long domainId, Long accountId, String path); public List listReadyTemplates(); public List listByAccountId(long accountId); - public List searchTemplates(String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Account account, DomainVO domain, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType); + public List searchTemplates(String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, + Account account, DomainVO domain, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, boolean onlyReady, boolean showDomr); public long addTemplateToZone(VMTemplateVO tmplt, long zoneId); public List listAllInZone(long dataCenterId); diff --git a/server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java b/server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java index 610f25c0a27..9c244340100 100755 --- a/server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java +++ b/server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java @@ -38,6 +38,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Storage.TemplateType; +import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; import com.cloud.template.VirtualMachineTemplate.TemplateFilter; import com.cloud.user.Account; import com.cloud.utils.component.Inject; @@ -233,7 +234,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem } @Override - public List searchTemplates(String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Account account, DomainVO domain, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType) { + public List searchTemplates(String name, String keyword, TemplateFilter templateFilter, boolean isIso, Boolean bootable, Account account, DomainVO domain, Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, boolean onlyReady,boolean showDomr) { Transaction txn = Transaction.currentTxn(); txn.start(); List templates = new ArrayList(); @@ -249,12 +250,22 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem accountType = Account.ACCOUNT_TYPE_ADMIN; } - String guestOSJoin = ""; + String guestOSJoin = ""; + StringBuilder templateHostRefJoin = new StringBuilder(); + String templateZoneRef = ""; if (isIso && !hyperType.equals(HypervisorType.None)) { guestOSJoin = " INNER JOIN guest_os guestOS on (guestOS.id = t.guest_os_id) INNER JOIN guest_os_hypervisor goh on ( goh.guest_os_id = guestOS.id) "; } + if (onlyReady){ + templateHostRefJoin.append(" INNER JOIN template_host_ref thr on (t.id = thr.template_id) "); + if(zoneId != null){ + templateHostRefJoin.append("INNER JOIN host h on (thr.host_id = h.id)"); + } + }else if (zoneId != null){ + templateZoneRef = " INNER JOIN template_zone_ref tzr on (t.id = tzr.template_id) "; + } - String sql = SELECT_ALL + guestOSJoin; + String sql = SELECT_ALL + guestOSJoin + templateHostRefJoin + templateZoneRef; String whereClause = ""; if (templateFilter == TemplateFilter.featured) { @@ -289,7 +300,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem whereClause += " AND "; } - sql += whereClause + getExtrasWhere(templateFilter, name, keyword, isIso, bootable, hyperType) + getOrderByLimit(pageSize, startIndex); + sql += whereClause + getExtrasWhere(templateFilter, name, keyword, isIso, bootable, hyperType, zoneId, onlyReady, showDomr) + getOrderByLimit(pageSize, startIndex); pstmt = txn.prepareStatement(sql); rs = pstmt.executeQuery(); @@ -328,7 +339,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem return templates; } - private String getExtrasWhere(TemplateFilter templateFilter, String name, String keyword, boolean isIso, Boolean bootable, HypervisorType hyperType) { + private String getExtrasWhere(TemplateFilter templateFilter, String name, String keyword, boolean isIso, Boolean bootable, HypervisorType hyperType, Long zoneId, boolean onlyReady, boolean showDomr) { String sql = ""; if (keyword != null) { sql += " t.name LIKE \"%" + keyword + "%\" AND"; @@ -351,8 +362,18 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem if (bootable != null) { sql += " AND t.bootable = " + bootable; } - - + + if (onlyReady){ + sql += " AND thr.download_state = '" +Status.DOWNLOADED.toString() + "'"; + if (zoneId != null){ + sql += " AND h.data_center_id = " +zoneId; + } + }else if (zoneId != null){ + sql += " AND tzr.zone_id = " +zoneId; + } + if (!showDomr){ + sql += " AND t.type = '" +Storage.TemplateType.SYSTEM.toString() + "'"; + } sql += " AND t.removed IS NULL";