You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:34:57 UTC
[GitHub] [cloudstack] RodrigoDLopez commented on a change in pull request #4213: Search vm snapshots using tags
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:
users@infra.apache.org