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/21 00:33:13 UTC
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #4213: Search vm snapshots using tags
GabrielBrascher commented on a change in pull request #4213:
URL: https://github.com/apache/cloudstack/pull/4213#discussion_r457766176
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -229,11 +236,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 (tags != null && !tags.isEmpty()) {
+ SearchBuilder<ResourceTagVO> tagSearch = _resourceTagDao.createSearchBuilder();
+ for (int count = 0; count < tags.size(); count++) {
+ tagSearch.or().op("key" + String.valueOf(count), tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
Review comment:
A few values can be extracted to constants, especially considering that they are being used more than once, for instance `"key"`.
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -229,11 +236,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 (tags != null && !tags.isEmpty()) {
Review comment:
Can you please use [MapUtils.isNotEmpty(tags);](https://commons.apache.org/proper/commons-collections/javadocs/api-4.4/org/apache/commons/collections4/MapUtils.html#isNotEmpty-java.util.Map-)?
`MapUtils.isNotEmpty` is a null-safe check that verifies if the specified map is not empty.
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -229,11 +236,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 (tags != null && !tags.isEmpty()) {
+ SearchBuilder<ResourceTagVO> tagSearch = _resourceTagDao.createSearchBuilder();
+ for (int count = 0; count < tags.size(); count++) {
+ tagSearch.or().op("key" + String.valueOf(count), tagSearch.entity().getKey(), SearchCriteria.Op.EQ);
+ tagSearch.and("value" + String.valueOf(count), tagSearch.entity().getValue(), SearchCriteria.Op.EQ);
+ tagSearch.cp();
+ }
+ tagSearch.and("resourceType", 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 (tags != null && !tags.isEmpty()) {
Review comment:
`MapUtils.isNotEmpty` would fit nicely here
----------------------------------------------------------------
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