Skip to content

Commit 6acbdd0

Browse files
davidjumanidavidjumani
committed
Filter disk / service offerings by domain at DB level
1 parent 98d3231 commit 6acbdd0

File tree

7 files changed

+137
-92
lines changed

7 files changed

+137
-92
lines changed

‎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: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@
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;
2526
import org.springframework.stereotype.Component;
2627

2728
import com.cloud.service.ServiceOfferingDetailsVO;
29+
import com.cloud.utils.db.GenericSearchBuilder;
30+
import com.cloud.utils.db.SearchCriteria;
31+
import com.cloud.utils.db.SearchCriteria.Op;
2832

2933
@Component
3034
public class ServiceOfferingDetailsDaoImpl extends ResourceDetailsDaoBase<ServiceOfferingDetailsVO> implements ServiceOfferingDetailsDao {
@@ -67,4 +71,23 @@ public String getDetail(Long serviceOfferingId, String key) {
6771
}
6872
return detailValue;
6973
}
74+
75+
@Override
76+
public List<Long> findOfferingIdsByDomainIds(List<Long> domainIds) {
77+
Object[] dIds = domainIds.stream().map(s -> String.valueOf(s)).collect(Collectors.toList()).toArray();
78+
79+
GenericSearchBuilder<ServiceOfferingDetailsVO, Long> sb = createSearchBuilder(Long.class);
80+
sb.selectFields(sb.entity().getResourceId());
81+
sb.and("name", sb.entity().getName(), Op.EQ);
82+
sb.and().op("value", sb.entity().getValue(), Op.IN);
83+
sb.or("valueNull", sb.entity().getValue(), Op.NULL);
84+
sb.cp();
85+
sb.done();
86+
87+
SearchCriteria<Long> sc = sb.create();
88+
sc.setParameters("name", "domainid");
89+
sc.setParameters("value", dIds);
90+
91+
return customSearch(sc, null);
92+
}
7093
}

‎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: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919

2020
import java.util.ArrayList;
2121
import java.util.List;
22+
import java.util.stream.Collectors;
23+
24+
import com.cloud.utils.db.GenericSearchBuilder;
25+
import com.cloud.utils.db.SearchCriteria;
26+
import com.cloud.utils.db.SearchCriteria.Op;
2227

2328
import org.apache.cloudstack.api.ApiConstants;
2429
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
@@ -66,4 +71,25 @@ public String getDetail(Long diskOfferingId, String key) {
6671
}
6772
return detailValue;
6873
}
69-
}
74+
75+
@Override
76+
public List<Long> findOfferingIdsByDomainIds(List<Long> domainIds) {
77+
Object[] dIds = domainIds.stream().map(s -> String.valueOf(s)).collect(Collectors.toList()).toArray();
78+
79+
GenericSearchBuilder<DiskOfferingDetailVO, Long> sb = createSearchBuilder(Long.class);
80+
sb.selectFields(sb.entity().getResourceId());
81+
sb.and("name", sb.entity().getName(), Op.EQ);
82+
sb.and().op("value", sb.entity().getValue(), Op.IN);
83+
sb.or("valueNull", sb.entity().getValue(), Op.NULL);
84+
sb.cp();
85+
sb.done();
86+
87+
SearchCriteria<Long> sc = sb.create();
88+
sc.setParameters("name", "domainid");
89+
sc.setParameters("value", dIds);
90+
91+
return customSearch(sc, null);
92+
}
93+
94+
}
95+

‎engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -708,26 +708,26 @@ CREATE PROCEDURE `cloud`.`ADD_GUEST_OS_AND_HYPERVISOR_MAPPING` (
708708
IN guest_os_hypervisor_hypervisor_version VARCHAR(32),
709709
IN guest_os_hypervisor_guest_os_name VARCHAR(255)
710710
)
711-
BEGIN
712-
INSERT INTO cloud.guest_os (uuid, category_id, display_name, created)
711+
BEGIN
712+
INSERT INTO cloud.guest_os (uuid, category_id, display_name, created)
713713
SELECT UUID(), guest_os_category_id, guest_os_display_name, now()
714714
FROM DUAL
715-
WHERE not exists( SELECT 1
715+
WHERE not exists( SELECT 1
716716
FROM cloud.guest_os
717717
WHERE cloud.guest_os.category_id = guest_os_category_id
718-
AND cloud.guest_os.display_name = guest_os_display_name)
719-
720-
; INSERT INTO cloud.guest_os_hypervisor (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created)
718+
AND cloud.guest_os.display_name = guest_os_display_name)
719+
720+
; INSERT INTO cloud.guest_os_hypervisor (uuid, hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created)
721721
SELECT UUID(), guest_os_hypervisor_hypervisor_type, guest_os_hypervisor_hypervisor_version, guest_os_hypervisor_guest_os_name, guest_os.id, now()
722722
FROM cloud.guest_os
723723
WHERE guest_os.category_id = guest_os_category_id
724724
AND guest_os.display_name = guest_os_display_name
725-
AND NOT EXISTS (SELECT 1
725+
AND NOT EXISTS (SELECT 1
726726
FROM cloud.guest_os_hypervisor as hypervisor
727-
WHERE hypervisor_type = guest_os_hypervisor_hypervisor_type
727+
WHERE hypervisor_type = guest_os_hypervisor_hypervisor_type
728728
AND hypervisor_version = guest_os_hypervisor_hypervisor_version
729729
AND hypervisor.guest_os_id = guest_os.id
730-
AND hypervisor.guest_os_name = guest_os_hypervisor_guest_os_name)
730+
AND hypervisor.guest_os_name = guest_os_hypervisor_guest_os_name)
731731
;END;
732732

733733
-- PR#4699 Call procedure `ADD_GUEST_OS_AND_HYPERVISOR_MAPPING` to add new data to guest_os and guest_os_hypervisor.
@@ -757,3 +757,4 @@ UPDATE cloud.user_vm_deploy_as_is_details SET value='' WHERE value IS NULL;
757757
ALTER TABLE cloud.user_vm_deploy_as_is_details MODIFY value text NOT NULL;
758758
UPDATE cloud.user_vm_details SET value='' WHERE value IS NULL;
759759
ALTER TABLE cloud.user_vm_details MODIFY value varchar(5120) NOT NULL;
760+

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

Lines changed: 71 additions & 79 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;
@@ -121,7 +120,6 @@
121120
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
122121
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
123122
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
124-
import org.apache.commons.collections.CollectionUtils;
125123
import org.apache.log4j.Logger;
126124
import org.springframework.stereotype.Component;
127125

@@ -207,6 +205,7 @@
207205
import com.cloud.server.TaggedResourceService;
208206
import com.cloud.service.ServiceOfferingVO;
209207
import com.cloud.service.dao.ServiceOfferingDao;
208+
import com.cloud.service.dao.ServiceOfferingDetailsDao;
210209
import com.cloud.storage.DataStoreRole;
211210
import com.cloud.storage.DiskOfferingVO;
212211
import com.cloud.storage.ScopeType;
@@ -350,14 +349,17 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
350349
private DiskOfferingJoinDao _diskOfferingJoinDao;
351350

352351
@Inject
353-
private DiskOfferingDetailsDao diskOfferingDetailsDao;
352+
private DiskOfferingDetailsDao _diskOfferingDetailsDao;
354353

355354
@Inject
356355
private ServiceOfferingJoinDao _srvOfferingJoinDao;
357356

358357
@Inject
359358
private ServiceOfferingDao _srvOfferingDao;
360359

360+
@Inject
361+
private ServiceOfferingDetailsDao _srvOfferingDetailsDao;
362+
361363
@Inject
362364
private DataCenterJoinDao _dcJoinDao;
363365

@@ -2868,57 +2870,41 @@ private Pair<List<DiskOfferingJoinVO>, Integer> searchForDiskOfferingsInternal(L
28682870
sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC);
28692871
}
28702872

2871-
// FIXME: disk offerings should search back up the hierarchy for
2872-
// available disk offerings...
2873-
/*
2874-
* sb.addAnd("domainId", sb.entity().getDomainId(),
2875-
* SearchCriteria.Op.EQ); if (domainId != null) {
2876-
* SearchBuilder<DomainVO> domainSearch =
2877-
* _domainDao.createSearchBuilder(); domainSearch.addAnd("path",
2878-
* domainSearch.entity().getPath(), SearchCriteria.Op.LIKE);
2879-
* sb.join("domainSearch", domainSearch, sb.entity().getDomainId(),
2880-
* domainSearch.entity().getId()); }
2881-
*/
2882-
2883-
// FIXME: disk offerings should search back up the hierarchy for
2884-
// available disk offerings...
2885-
/*
2886-
* if (domainId != null) { sc.setParameters("domainId", domainId); //
2887-
* //DomainVO domain = _domainDao.findById((Long)domainId); // // I want
2888-
* to join on user_vm.domain_id = domain.id where domain.path like
2889-
* 'foo%' //sc.setJoinParameters("domainSearch", "path",
2890-
* domain.getPath() + "%"); // }
2891-
*/
2873+
// Filter offerings that are not associated with caller's domain
2874+
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
2875+
Account caller = CallContext.current().getCallingAccount();
2876+
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
2877+
Domain callerDomain = _domainDao.findById(caller.getDomainId());
2878+
List<Long> domainIds = _domainDao.getDomainParentIds(callerDomain.getId())
2879+
.stream().collect(Collectors.toList());
2880+
if (isRecursive) {
2881+
List<Long> childrenIds = _domainDao.getDomainChildrenIds(callerDomain.getPath());
2882+
if (childrenIds != null && !childrenIds.isEmpty())
2883+
domainIds.addAll(childrenIds);
2884+
}
28922885

2893-
Pair<List<DiskOfferingJoinVO>, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter);
2894-
// Remove offerings that are not associated with caller's domain
2895-
if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(result.first())) {
2896-
ListIterator<DiskOfferingJoinVO> it = result.first().listIterator();
2897-
while (it.hasNext()) {
2898-
DiskOfferingJoinVO offering = it.next();
2899-
if(!Strings.isNullOrEmpty(offering.getDomainId())) {
2900-
boolean toRemove = true;
2901-
String[] domainIdsArray = offering.getDomainId().split(",");
2902-
for (String domainIdString : domainIdsArray) {
2903-
Long dId = Long.valueOf(domainIdString.trim());
2904-
if (isRecursive) {
2905-
if (_domainDao.isChildDomain(account.getDomainId(), dId)) {
2906-
toRemove = false;
2907-
break;
2908-
}
2909-
} else {
2910-
if (_domainDao.isChildDomain(dId, account.getDomainId())) {
2911-
toRemove = false;
2912-
break;
2913-
}
2914-
}
2915-
}
2916-
if (toRemove) {
2917-
it.remove();
2918-
}
2919-
}
2886+
List<Long> ids = _diskOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
2887+
2888+
if (ids == null || ids.isEmpty()) {
2889+
SearchBuilder<DiskOfferingJoinVO> sb = _diskOfferingJoinDao.createSearchBuilder();
2890+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
2891+
sb.done();
2892+
2893+
SearchCriteria<DiskOfferingJoinVO> scc = sb.create();
2894+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
2895+
} else {
2896+
SearchBuilder<DiskOfferingJoinVO> sb = _diskOfferingJoinDao.createSearchBuilder();
2897+
sb.and("id", sb.entity().getId(), Op.IN);
2898+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
2899+
sb.done();
2900+
2901+
SearchCriteria<DiskOfferingJoinVO> scc = sb.create();
2902+
scc.setParameters("id", ids.toArray());
2903+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
29202904
}
29212905
}
2906+
2907+
Pair<List<DiskOfferingJoinVO>, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter);
29222908
return new Pair<>(result.first(), result.second());
29232909
}
29242910

@@ -3109,38 +3095,44 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
31093095
sc.addAnd("cpuspeedconstraints", SearchCriteria.Op.SC, cpuSpeedSearchCriteria);
31103096
}
31113097

3098+
// Filter offerings that are not associated with caller's domain
3099+
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
3100+
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
3101+
Domain callerDomain = _domainDao.findById(caller.getDomainId());
3102+
List<Long> domainIds = _domainDao.getDomainParentIds(callerDomain.getId())
3103+
.stream().collect(Collectors.toList());
3104+
if (isRecursive) {
3105+
List<Long> childrenIds = _domainDao.getDomainChildrenIds(callerDomain.getPath());
3106+
if (childrenIds != null && !childrenIds.isEmpty())
3107+
domainIds.addAll(childrenIds);
3108+
}
3109+
3110+
List<Long> ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
3111+
3112+
if (ids == null || ids.isEmpty()) {
3113+
SearchBuilder<ServiceOfferingJoinVO> sb = _srvOfferingJoinDao.createSearchBuilder();
3114+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
3115+
sb.done();
3116+
3117+
SearchCriteria<ServiceOfferingJoinVO> scc = sb.create();
3118+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
3119+
} else {
3120+
SearchBuilder<ServiceOfferingJoinVO> sb = _srvOfferingJoinDao.createSearchBuilder();
3121+
sb.and("id", sb.entity().getId(), Op.IN);
3122+
sb.or("domainId", sb.entity().getDomainId(), Op.NULL);
3123+
sb.done();
3124+
3125+
SearchCriteria<ServiceOfferingJoinVO> scc = sb.create();
3126+
scc.setParameters("id", ids.toArray());
3127+
sc.addAnd("domainId", SearchCriteria.Op.SC, scc);
3128+
}
3129+
}
3130+
31123131
Pair<List<ServiceOfferingJoinVO>, Integer> result = _srvOfferingJoinDao.searchAndCount(sc, searchFilter);
31133132

31143133
//Couldn't figure out a smart way to filter offerings based on tags in sql so doing it in Java.
31153134
List<ServiceOfferingJoinVO> filteredOfferings = filterOfferingsOnCurrentTags(result.first(), currentVmOffering);
3116-
// Remove offerings that are not associated with caller's domain
3117-
if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN && CollectionUtils.isNotEmpty(filteredOfferings)) {
3118-
ListIterator<ServiceOfferingJoinVO> it = filteredOfferings.listIterator();
3119-
while (it.hasNext()) {
3120-
ServiceOfferingJoinVO offering = it.next();
3121-
if(!Strings.isNullOrEmpty(offering.getDomainId())) {
3122-
boolean toRemove = true;
3123-
String[] domainIdsArray = offering.getDomainId().split(",");
3124-
for (String domainIdString : domainIdsArray) {
3125-
Long dId = Long.valueOf(domainIdString.trim());
3126-
if (isRecursive) {
3127-
if (_domainDao.isChildDomain(caller.getDomainId(), dId)) {
3128-
toRemove = false;
3129-
break;
3130-
}
3131-
} else {
3132-
if (_domainDao.isChildDomain(dId, caller.getDomainId())) {
3133-
toRemove = false;
3134-
break;
3135-
}
3136-
}
3137-
}
3138-
if (toRemove) {
3139-
it.remove();
3140-
}
3141-
}
3142-
}
3143-
}
3135+
31443136
return new Pair<>(filteredOfferings, result.second());
31453137
}
31463138

‎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
@@ -145,5 +145,4 @@ public ServiceOfferingJoinVO newServiceOfferingView(ServiceOffering offering) {
145145
assert offerings != null && offerings.size() == 1 : "No service offering found for offering id " + offering.getId();
146146
return offerings.get(0);
147147
}
148-
149148
}

0 commit comments

Comments
 (0)