Skip to content

Commit a5e7e08

Browse files
davidjumanidavidjumani
authored
Filter disk / service offerings by domain at DB level (#5307)
* Filter disk / service offerings by domain at DB level * Search for tags in the db * Update search to include host tags * Differenciate between tags * Refactor
1 parent cf6dc66 commit a5e7e08

8 files changed

Lines changed: 126 additions & 99 deletions

File tree

‎engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDao.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,6 @@ public interface ServiceOfferingDetailsDao extends GenericDao<ServiceOfferingDet
2727
List<Long> findDomainIds(final long resourceId);
2828
List<Long> findZoneIds(final long resourceId);
2929
String getDetail(Long diskOfferingId, String key);
30-
}
30+
List<Long> findOfferingIdsByDomainIds(List<Long> domainIds);
31+
}
32+

‎engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDetailsDaoImpl.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.util.ArrayList;
2121
import java.util.List;
22+
import java.util.stream.Collectors;
2223

2324
import org.apache.cloudstack.api.ApiConstants;
2425
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
@@ -67,4 +68,10 @@ public String getDetail(Long serviceOfferingId, String key) {
6768
}
6869
return detailValue;
6970
}
71+
72+
@Override
73+
public List<Long> findOfferingIdsByDomainIds(List<Long> domainIds) {
74+
Object[] dIds = domainIds.stream().map(s -> String.valueOf(s)).collect(Collectors.toList()).toArray();
75+
return findResouceIdsByNameAndValueIn("domainid", dIds);
76+
}
7077
}

‎engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,5 @@ public interface ResourceDetailsDao<R extends ResourceDetail> extends GenericDao
9696

9797
public void addDetail(long resourceId, String key, String value, boolean display);
9898

99+
public List<Long> findResouceIdsByNameAndValueIn(String name, Object[] values);
99100
}

‎engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@
2323
import org.apache.cloudstack.api.ResourceDetail;
2424

2525
import com.cloud.utils.db.GenericDaoBase;
26+
import com.cloud.utils.db.GenericSearchBuilder;
2627
import com.cloud.utils.db.SearchBuilder;
2728
import com.cloud.utils.db.SearchCriteria;
2829
import com.cloud.utils.db.TransactionLegacy;
30+
import com.cloud.utils.db.SearchCriteria.Op;
2931

3032
public abstract class ResourceDetailsDaoBase<R extends ResourceDetail> extends GenericDaoBase<R, Long> implements ResourceDetailsDao<R> {
3133
private SearchBuilder<R> AllFieldsSearch;
@@ -182,4 +184,21 @@ public List<R> listDetails(long resourceId, boolean forDisplay) {
182184
List<R> results = search(sc, null);
183185
return results;
184186
}
187+
188+
@Override
189+
public List<Long> findResouceIdsByNameAndValueIn(String name, Object[] values) {
190+
GenericSearchBuilder<R, Long> sb = createSearchBuilder(Long.class);
191+
sb.selectFields(sb.entity().getResourceId());
192+
sb.and("name", sb.entity().getName(), Op.EQ);
193+
sb.and().op("value", sb.entity().getValue(), Op.IN);
194+
sb.or("valueNull", sb.entity().getValue(), Op.NULL);
195+
sb.cp();
196+
sb.done();
197+
198+
SearchCriteria<Long> sc = sb.create();
199+
sc.setParameters("name", name);
200+
sc.setParameters("value", values);
201+
202+
return customSearch(sc, null);
203+
}
185204
}

‎engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDao.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,6 @@ public interface DiskOfferingDetailsDao extends GenericDao<DiskOfferingDetailVO,
2727
List<Long> findDomainIds(final long resourceId);
2828
List<Long> findZoneIds(final long resourceId);
2929
String getDetail(Long diskOfferingId, String key);
30-
}
30+
List<Long> findOfferingIdsByDomainIds(List<Long> domainIds);
31+
}
32+

‎engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/dao/DiskOfferingDetailsDaoImpl.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.util.ArrayList;
2121
import java.util.List;
22+
import java.util.stream.Collectors;
2223

2324
import org.apache.cloudstack.api.ApiConstants;
2425
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
@@ -66,4 +67,11 @@ public String getDetail(Long diskOfferingId, String key) {
6667
}
6768
return detailValue;
6869
}
69-
}
70+
71+
@Override
72+
public List<Long> findOfferingIdsByDomainIds(List<Long> domainIds) {
73+
Object[] dIds = domainIds.stream().map(s -> String.valueOf(s)).collect(Collectors.toList()).toArray();
74+
return findResouceIdsByNameAndValueIn("domainid", dIds);
75+
}
76+
}
77+

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

Lines changed: 84 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.HashMap;
2424
import java.util.HashSet;
2525
import java.util.List;
26-
import java.util.ListIterator;
2726
import java.util.Map;
2827
import java.util.Set;
2928
import java.util.stream.Collectors;
@@ -119,7 +118,6 @@
119118
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
120119
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
121120
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
122-
import org.apache.commons.collections.CollectionUtils;
123121
import org.apache.log4j.Logger;
124122
import org.springframework.stereotype.Component;
125123

@@ -204,6 +202,7 @@
204202
import com.cloud.server.TaggedResourceService;
205203
import com.cloud.service.ServiceOfferingVO;
206204
import com.cloud.service.dao.ServiceOfferingDao;
205+
import com.cloud.service.dao.ServiceOfferingDetailsDao;
207206
import com.cloud.storage.DataStoreRole;
208207
import com.cloud.storage.DiskOfferingVO;
209208
import com.cloud.storage.ScopeType;
@@ -346,14 +345,17 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
346345
private DiskOfferingJoinDao _diskOfferingJoinDao;
347346

348347
@Inject
349-
private DiskOfferingDetailsDao diskOfferingDetailsDao;
348+
private DiskOfferingDetailsDao _diskOfferingDetailsDao;
350349

351350
@Inject
352351
private ServiceOfferingJoinDao _srvOfferingJoinDao;
353352

354353
@Inject
355354
private ServiceOfferingDao _srvOfferingDao;
356355

356+
@Inject
357+
private ServiceOfferingDetailsDao _srvOfferingDetailsDao;
358+
357359
@Inject
358360
private DataCenterJoinDao _dcJoinDao;
359361

@@ -2824,75 +2826,41 @@ private Pair<List<DiskOfferingJoinVO>, Integer> searchForDiskOfferingsInternal(L
28242826
sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC);
28252827
}
28262828

2827-
// FIXME: disk offerings should search back up the hierarchy for
2828-
// available disk offerings...
2829-
/*
2830-
* sb.addAnd("domainId", sb.entity().getDomainId(),
2831-
* SearchCriteria.Op.EQ); if (domainId != null) {
2832-
* SearchBuilder<DomainVO> domainSearch =
2833-
* _domainDao.createSearchBuilder(); domainSearch.addAnd("path",
2834-
* domainSearch.entity().getPath(), SearchCriteria.Op.LIKE);
2835-
* sb.join("domainSearch", domainSearch, sb.entity().getDomainId(),
2836-
* domainSearch.entity().getId()); }
2837-
*/
2838-
2839-
// FIXME: disk offerings should search back up the hierarchy for
2840-
// available disk offerings...
2841-
/*
2842-
* if (domainId != null) { sc.setParameters("domainId", domainId); //
2843-
* //DomainVO domain = _domainDao.findById((Long)domainId); // // I want
2844-
* to join on user_vm.domain_id = domain.id where domain.path like
2845-
* 'foo%' //sc.setJoinParameters("domainSearch", "path",
2846-
* domain.getPath() + "%"); // }
2847-
*/
2829+
// Filter offerings that are not associated with caller's domain
2830+
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
2831+
Account caller = CallContext.current().getCallingAccount();
2832+
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
2833+
Domain callerDomain = _domainDao.findById(caller.getDomainId());
2834+
List<Long> domainIds = findRelatedDomainIds(callerDomain, isRecursive);
28482835

2849-
Pair<List<DiskOfferingJoinVO>, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter);
2850-
// Remove offerings that are not associated with caller's domain
2851-
if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(result.first())) {
2852-
ListIterator<DiskOfferingJoinVO> it = result.first().listIterator();
2853-
while (it.hasNext()) {
2854-
DiskOfferingJoinVO offering = it.next();
2855-
if(!Strings.isNullOrEmpty(offering.getDomainId())) {
2856-
boolean toRemove = true;
2857-
String[] domainIdsArray = offering.getDomainId().split(",");
2858-
for (String domainIdString : domainIdsArray) {
2859-
Long dId = Long.valueOf(domainIdString.trim());
2860-
if (isRecursive) {
2861-
if (_domainDao.isChildDomain(account.getDomainId(), dId)) {
2862-
toRemove = false;
2863-
break;
2864-
}
2865-
} else {
2866-
if (_domainDao.isChildDomain(dId, account.getDomainId())) {
2867-
toRemove = false;
2868-
break;
2869-
}
2870-
}
2871-
}
2872-
if (toRemove) {
2873-
it.remove();
2874-
}
2875-
}
2836+
List<Long> ids = _diskOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
2837+
SearchBuilder<DiskOfferingJoinVO> sb = _diskOfferingJoinDao.createSearchBuilder();
2838+
if (ids != null && !ids.isEmpty()) {
2839+
sb.and("id", sb.entity().getId(), Op.IN);
28762840
}
2841+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
2842+
sb.done();
2843+
2844+
SearchCriteria<DiskOfferingJoinVO> scc = sb.create();
2845+
if (ids != null && !ids.isEmpty()) {
2846+
scc.setParameters("id", ids.toArray());
2847+
}
2848+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
28772849
}
2850+
2851+
Pair<List<DiskOfferingJoinVO>, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter);
28782852
return new Pair<>(result.first(), result.second());
28792853
}
28802854

2881-
private List<ServiceOfferingJoinVO> filterOfferingsOnCurrentTags(List<ServiceOfferingJoinVO> offerings, ServiceOfferingVO currentVmOffering) {
2882-
if (currentVmOffering == null) {
2883-
return offerings;
2855+
private List<Long> findRelatedDomainIds(Domain domain, boolean isRecursive) {
2856+
List<Long> domainIds = _domainDao.getDomainParentIds(domain.getId())
2857+
.stream().collect(Collectors.toList());
2858+
if (isRecursive) {
2859+
List<Long> childrenIds = _domainDao.getDomainChildrenIds(domain.getPath());
2860+
if (childrenIds != null && !childrenIds.isEmpty())
2861+
domainIds.addAll(childrenIds);
28842862
}
2885-
List<String> currentTagsList = StringUtils.csvTagsToList(currentVmOffering.getTags());
2886-
2887-
// New service offering should have all the tags of the current service offering.
2888-
List<ServiceOfferingJoinVO> filteredOfferings = new ArrayList<>();
2889-
for (ServiceOfferingJoinVO offering : offerings) {
2890-
List<String> newTagsList = StringUtils.csvTagsToList(offering.getTags());
2891-
if (newTagsList.containsAll(currentTagsList)) {
2892-
filteredOfferings.add(offering);
2893-
}
2894-
}
2895-
return filteredOfferings;
2863+
return domainIds;
28962864
}
28972865

28982866
@Override
@@ -3064,39 +3032,60 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
30643032
sc.addAnd("cpuspeedconstraints", SearchCriteria.Op.SC, cpuSpeedSearchCriteria);
30653033
}
30663034

3067-
Pair<List<ServiceOfferingJoinVO>, Integer> result = _srvOfferingJoinDao.searchAndCount(sc, searchFilter);
3068-
3069-
//Couldn't figure out a smart way to filter offerings based on tags in sql so doing it in Java.
3070-
List<ServiceOfferingJoinVO> filteredOfferings = filterOfferingsOnCurrentTags(result.first(), currentVmOffering);
3071-
// Remove offerings that are not associated with caller's domain
3072-
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(filteredOfferings)) {
3073-
ListIterator<ServiceOfferingJoinVO> it = filteredOfferings.listIterator();
3074-
while (it.hasNext()) {
3075-
ServiceOfferingJoinVO offering = it.next();
3076-
if(!Strings.isNullOrEmpty(offering.getDomainId())) {
3077-
boolean toRemove = true;
3078-
String[] domainIdsArray = offering.getDomainId().split(",");
3079-
for (String domainIdString : domainIdsArray) {
3080-
Long dId = Long.valueOf(domainIdString.trim());
3081-
if (isRecursive) {
3082-
if (_domainDao.isChildDomain(caller.getDomainId(), dId)) {
3083-
toRemove = false;
3084-
break;
3085-
}
3086-
} else {
3087-
if (_domainDao.isChildDomain(dId, caller.getDomainId())) {
3088-
toRemove = false;
3089-
break;
3090-
}
3091-
}
3092-
}
3093-
if (toRemove) {
3094-
it.remove();
3095-
}
3035+
// Filter offerings that are not associated with caller's domain
3036+
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
3037+
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
3038+
Domain callerDomain = _domainDao.findById(caller.getDomainId());
3039+
List<Long> domainIds = findRelatedDomainIds(callerDomain, isRecursive);
3040+
3041+
List<Long> ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
3042+
SearchBuilder<ServiceOfferingJoinVO> sb = _srvOfferingJoinDao.createSearchBuilder();
3043+
if (ids != null && !ids.isEmpty()) {
3044+
sb.and("id", sb.entity().getId(), Op.IN);
3045+
}
3046+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
3047+
sb.done();
3048+
3049+
SearchCriteria<ServiceOfferingJoinVO> scc = sb.create();
3050+
if (ids != null && !ids.isEmpty()) {
3051+
scc.setParameters("id", ids.toArray());
3052+
}
3053+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
3054+
}
3055+
3056+
if (currentVmOffering != null) {
3057+
List<String> storageTags = StringUtils.csvTagsToList(currentVmOffering.getTags());
3058+
if (!storageTags.isEmpty()) {
3059+
SearchBuilder<ServiceOfferingJoinVO> sb = _srvOfferingJoinDao.createSearchBuilder();
3060+
for(String tag : storageTags) {
3061+
sb.and(tag, sb.entity().getTags(), Op.FIND_IN_SET);
3062+
}
3063+
sb.done();
3064+
3065+
SearchCriteria<ServiceOfferingJoinVO> scc = sb.create();
3066+
for(String tag : storageTags) {
3067+
scc.setParameters(tag, tag);
30963068
}
3069+
sc.addAnd("storageTags", SearchCriteria.Op.SC, scc);
3070+
}
3071+
3072+
List<String> hostTags = StringUtils.csvTagsToList(currentVmOffering.getHostTag());
3073+
if (!hostTags.isEmpty()) {
3074+
SearchBuilder<ServiceOfferingJoinVO> sb = _srvOfferingJoinDao.createSearchBuilder();
3075+
for(String tag : hostTags) {
3076+
sb.and(tag, sb.entity().getHostTag(), Op.FIND_IN_SET);
3077+
}
3078+
sb.done();
3079+
3080+
SearchCriteria<ServiceOfferingJoinVO> scc = sb.create();
3081+
for(String tag : hostTags) {
3082+
scc.setParameters(tag, tag);
3083+
}
3084+
sc.addAnd("hostTags", SearchCriteria.Op.SC, scc);
30973085
}
30983086
}
3099-
return new Pair<>(filteredOfferings, result.second());
3087+
3088+
return _srvOfferingJoinDao.searchAndCount(sc, searchFilter);
31003089
}
31013090

31023091
@Override

‎server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,5 +144,4 @@ public ServiceOfferingJoinVO newServiceOfferingView(ServiceOffering offering) {
144144
assert offerings != null && offerings.size() == 1 : "No service offering found for offering id " + offering.getId();
145145
return offerings.get(0);
146146
}
147-
148147
}

0 commit comments

Comments
 (0)