DaanHoogland commented on a change in pull request #5307:
URL: https://github.com/apache/cloudstack/pull/5307#discussion_r692853931
##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -346,14 +345,17 @@
private DiskOfferingJoinDao _diskOfferingJoinDao;
@Inject
- private DiskOfferingDetailsDao diskOfferingDetailsDao;
+ private DiskOfferingDetailsDao _diskOfferingDetailsDao;
Review comment:
why add the '_'? we are trying to get away from that convention.
##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -3064,39 +3033,72 @@
sc.addAnd("cpuspeedconstraints", SearchCriteria.Op.SC,
cpuSpeedSearchCriteria);
}
- Pair<List<ServiceOfferingJoinVO>, Integer> result =
_srvOfferingJoinDao.searchAndCount(sc, searchFilter);
-
- //Couldn't figure out a smart way to filter offerings based on tags in
sql so doing it in Java.
- List<ServiceOfferingJoinVO> filteredOfferings =
filterOfferingsOnCurrentTags(result.first(), currentVmOffering);
- // Remove offerings that are not associated with caller's domain
- if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN &&
CollectionUtils.isNotEmpty(filteredOfferings)) {
- ListIterator<ServiceOfferingJoinVO> it =
filteredOfferings.listIterator();
- while (it.hasNext()) {
- ServiceOfferingJoinVO offering = it.next();
- if(!Strings.isNullOrEmpty(offering.getDomainId())) {
- boolean toRemove = true;
- String[] domainIdsArray =
offering.getDomainId().split(",");
- for (String domainIdString : domainIdsArray) {
- Long dId = Long.valueOf(domainIdString.trim());
- if (isRecursive) {
- if (_domainDao.isChildDomain(caller.getDomainId(),
dId)) {
- toRemove = false;
- break;
- }
- } else {
- if (_domainDao.isChildDomain(dId,
caller.getDomainId())) {
- toRemove = false;
- break;
- }
- }
- }
- if (toRemove) {
- it.remove();
- }
+ // Filter offerings that are not associated with caller's domain
Review comment:
once again, a this comment is a good method name to extract the code
below to.
##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -2824,75 +2826,42 @@
sc.addAnd("zoneId", SearchCriteria.Op.SC, zoneSC);
}
- // FIXME: disk offerings should search back up the hierarchy for
- // available disk offerings...
- /*
- * sb.addAnd("domainId", sb.entity().getDomainId(),
- * SearchCriteria.Op.EQ); if (domainId != null) {
- * SearchBuilder<DomainVO> domainSearch =
- * _domainDao.createSearchBuilder(); domainSearch.addAnd("path",
- * domainSearch.entity().getPath(), SearchCriteria.Op.LIKE);
- * sb.join("domainSearch", domainSearch, sb.entity().getDomainId(),
- * domainSearch.entity().getId()); }
- */
-
- // FIXME: disk offerings should search back up the hierarchy for
- // available disk offerings...
- /*
- * if (domainId != null) { sc.setParameters("domainId", domainId); //
- * //DomainVO domain = _domainDao.findById((Long)domainId); // // I
want
- * to join on user_vm.domain_id = domain.id where domain.path like
- * 'foo%' //sc.setJoinParameters("domainSearch", "path",
- * domain.getPath() + "%"); // }
- */
-
- Pair<List<DiskOfferingJoinVO>, Integer> result =
_diskOfferingJoinDao.searchAndCount(sc, searchFilter);
- // Remove offerings that are not associated with caller's domain
- if (account.getType() != Account.ACCOUNT_TYPE_ADMIN &&
CollectionUtils.isNotEmpty(result.first())) {
- ListIterator<DiskOfferingJoinVO> it =
result.first().listIterator();
- while (it.hasNext()) {
- DiskOfferingJoinVO offering = it.next();
- if(!Strings.isNullOrEmpty(offering.getDomainId())) {
- boolean toRemove = true;
- String[] domainIdsArray =
offering.getDomainId().split(",");
- for (String domainIdString : domainIdsArray) {
- Long dId = Long.valueOf(domainIdString.trim());
- if (isRecursive) {
- if
(_domainDao.isChildDomain(account.getDomainId(), dId)) {
- toRemove = false;
- break;
- }
- } else {
- if (_domainDao.isChildDomain(dId,
account.getDomainId())) {
- toRemove = false;
- break;
- }
- }
- }
- if (toRemove) {
- it.remove();
- }
- }
+ // Filter offerings that are not associated with caller's domain
Review comment:
good method name ;) maybe factor out the below.
##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -346,14 +345,17 @@
private DiskOfferingJoinDao _diskOfferingJoinDao;
@Inject
- private DiskOfferingDetailsDao diskOfferingDetailsDao;
+ private DiskOfferingDetailsDao _diskOfferingDetailsDao;
@Inject
private ServiceOfferingJoinDao _srvOfferingJoinDao;
@Inject
private ServiceOfferingDao _srvOfferingDao;
+ @Inject
+ private ServiceOfferingDetailsDao _srvOfferingDetailsDao;
Review comment:
```suggestion
private ServiceOfferingDetailsDao serviceOfferingDetailsDao;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
