RodrigoDLopez commented on a change in pull request #4213:
URL: https://github.com/apache/cloudstack/pull/4213#discussion_r461837337
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -229,11 +238,32 @@ public boolean stop() {
sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
sb.and("display_name", sb.entity().getDisplayName(),
SearchCriteria.Op.EQ);
sb.and("account_id", sb.entity().getAccountId(), SearchCriteria.Op.EQ);
- sb.done();
+
+ if (MapUtils.isNotEmpty(tags)) {
+ SearchBuilder<ResourceTagVO> tagSearch =
_resourceTagDao.createSearchBuilder();
+ for (int count = 0; count < tags.size(); count++) {
+ tagSearch.or().op(ApiConstants.KEY + String.valueOf(count),
tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
+ tagSearch.and(ApiConstants.VALUE + String.valueOf(count),
tagSearch.entity().getValue(), SearchCriteria.Op.EQ);
+ tagSearch.cp();
+ }
+ tagSearch.and(ApiConstants.RESOURCE_TYPE,
tagSearch.entity().getResourceType(), SearchCriteria.Op.EQ);
+ sb.groupBy(sb.entity().getId());
+ sb.join("tagSearch", tagSearch, sb.entity().getId(),
tagSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER);
Review comment:
Could you please extract this to a method, add documentation to what
this method do and why.
Also can you please create unit tests to cover those method
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -229,11 +238,32 @@ public boolean stop() {
sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
sb.and("display_name", sb.entity().getDisplayName(),
SearchCriteria.Op.EQ);
sb.and("account_id", sb.entity().getAccountId(), SearchCriteria.Op.EQ);
- sb.done();
+
+ if (MapUtils.isNotEmpty(tags)) {
+ SearchBuilder<ResourceTagVO> tagSearch =
_resourceTagDao.createSearchBuilder();
+ for (int count = 0; count < tags.size(); count++) {
+ tagSearch.or().op(ApiConstants.KEY + String.valueOf(count),
tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
+ tagSearch.and(ApiConstants.VALUE + String.valueOf(count),
tagSearch.entity().getValue(), SearchCriteria.Op.EQ);
+ tagSearch.cp();
+ }
+ tagSearch.and(ApiConstants.RESOURCE_TYPE,
tagSearch.entity().getResourceType(), SearchCriteria.Op.EQ);
+ sb.groupBy(sb.entity().getId());
+ sb.join("tagSearch", tagSearch, sb.entity().getId(),
tagSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER);
+ }
SearchCriteria<VMSnapshotVO> sc = sb.create();
_accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive,
permittedAccounts, listProjectResourcesCriteria);
+ if (MapUtils.isNotEmpty(tags)) {
+ int count = 0;
+ sc.setJoinParameters("tagSearch", ApiConstants.RESOURCE_TYPE,
ResourceTag.ResourceObjectType.VMSnapshot.toString());
+ for (String key : tags.keySet()) {
+ sc.setJoinParameters("tagSearch", ApiConstants.KEY +
String.valueOf(count), key);
+ sc.setJoinParameters("tagSearch", ApiConstants.VALUE +
String.valueOf(count), tags.get(key));
+ count++;
+ }
Review comment:
cloud you do the same here please,
extract to a new method, add documentation and unit tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
