Address reviews

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/CreateSecondaryStorageSelectorCmd.java

@@ -30,7 +30,7 @@
 
 import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
 
-@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", responseObject =
+@APICommand(name = "createSecondaryStorageSelector", description = "Creates a secondary storage selector, described by the heuristic rule.", since = "4.19.0", responseObject =
         SecondaryStorageHeuristicsResponse.class, entityType = {Heuristic.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
 public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
 
@@ -49,7 +49,8 @@ public class CreateSecondaryStorageSelectorCmd extends BaseCmd{
     private String purpose;
 
     @Parameter(name = ApiConstants.HEURISTIC_RULE, required = true, type = BaseCmd.CommandType.STRING, description = "The heuristic rule, in JavaScript language. It is required " +
-            "that it returns the UUID of a secondary storage pool.", length = 65535)
+            "that it returns the UUID of a secondary storage pool. An example of a rule is `if (snapshot.hypervisorType === 'KVM') { '7832f261-c602-4e8e-8580-2496ffbbc45d'; " +
+            "}` would allocate all snapshots with the KVM hypervisor to the specified secondary storage UUID.", length = 65535)
     private String heuristicRule;
 
     public String getName() {

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/ListSecondaryStorageSelectorsCmd.java

@@ -28,8 +28,8 @@
 
 import static org.apache.cloudstack.api.ApiConstants.HEURISTIC_PURPOSE_VALID_OPTIONS;
 
-@APICommand(name = "listSecondaryStorageSelectors", description = "Lists the secondary storage selectors and their rules.", responseObject = SecondaryStorageHeuristicsResponse.class,
-        requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+@APICommand(name = "listSecondaryStorageSelectors", description = "Lists the secondary storage selectors and their rules.", since = "4.19.0", responseObject =
+        SecondaryStorageHeuristicsResponse.class, requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
 public class ListSecondaryStorageSelectorsCmd extends BaseListCmd {
 
     @Parameter(name = ApiConstants.ZONE_ID, required = true, entityType = ZoneResponse.class, type = CommandType.UUID, description = "The zone ID to be used in the search filter.")

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/RemoveSecondaryStorageSelectorCmd.java

@@ -26,8 +26,8 @@
 import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.secstorage.heuristics.Heuristic;
 
-@APICommand(name = "removeSecondaryStorageSelector", description = "Removes an existing secondary storage selector.", responseObject = SecondaryStorageHeuristicsResponse.class,
-        requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+@APICommand(name = "removeSecondaryStorageSelector", description = "Removes an existing secondary storage selector.", since = "4.19.0", responseObject =
+        SecondaryStorageHeuristicsResponse.class, requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
 public class RemoveSecondaryStorageSelectorCmd extends BaseCmd {
     @Parameter(name = ApiConstants.ID, type = BaseCmd.CommandType.UUID, entityType = SecondaryStorageHeuristicsResponse.class, required = true,
             description = "The unique identifier of the secondary storage selector to be removed.")

api/src/main/java/org/apache/cloudstack/api/command/admin/storage/heuristics/UpdateSecondaryStorageSelectorCmd.java

@@ -27,15 +27,16 @@
 import org.apache.cloudstack.api.response.SecondaryStorageHeuristicsResponse;
 import org.apache.cloudstack.secstorage.heuristics.Heuristic;
 
-@APICommand(name = "updateSecondaryStorageSelector", description = "Updates an existing secondary storage selector.", responseObject = SecondaryStorageHeuristicsResponse.class,
-        requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
+@APICommand(name = "updateSecondaryStorageSelector", description = "Updates an existing secondary storage selector.", since = "4.19.0", responseObject =
+        SecondaryStorageHeuristicsResponse.class, requestHasSensitiveInfo = false, entityType = {Heuristic.class}, responseHasSensitiveInfo = false, authorized = {RoleType.Admin})
 public class UpdateSecondaryStorageSelectorCmd extends BaseCmd {
     @Parameter(name = ApiConstants.ID, type = BaseCmd.CommandType.UUID, entityType = SecondaryStorageHeuristicsResponse.class, required = true,
             description = "The unique identifier of the secondary storage selector.")
     private Long id;
 
     @Parameter(name = ApiConstants.HEURISTIC_RULE, required = true, type = BaseCmd.CommandType.STRING, description = "The heuristic rule, in JavaScript language. It is required " +
-            "that it returns the UUID of a secondary storage pool.", length = 65535)
+            "that it returns the UUID of a secondary storage pool. An example of a rule is `if (snapshot.hypervisorType === 'KVM') { '7832f261-c602-4e8e-8580-2496ffbbc45d'; " +
+            "}` would allocate all snapshots with the KVM hypervisor to the specified secondary storage UUID.", length = 65535)
     private String heuristicRule;
 
     public Long getId() {

api/src/main/java/org/apache/cloudstack/api/response/SecondaryStorageHeuristicsResponse.java

@@ -62,8 +62,17 @@ public class SecondaryStorageHeuristicsResponse extends BaseResponse {
     @Param(description = "When the heuristic was removed.")
     private Date removed;
 
-    public SecondaryStorageHeuristicsResponse() {
+
+    public SecondaryStorageHeuristicsResponse(String id, String name, String description, String zoneId, String purpose, String heuristicRule, Date created, Date removed) {
         super("heuristics");
+        this.id = id;
+        this.name = name;
+        this.description = description;
+        this.zoneId = zoneId;
+        this.purpose = purpose;
+        this.heuristicRule = heuristicRule;
+        this.created = created;
+        this.removed = removed;
     }
 
     public String getId() {

engine/schema/src/main/java/org/apache/cloudstack/secstorage/HeuristicVO.java

@@ -68,14 +68,12 @@ public class HeuristicVO implements Heuristic {
     public HeuristicVO() {
     }
 
-    public HeuristicVO(String name, String description, Long zoneId, String purpose, String heuristicRule, Date created, Date removed) {
+    public HeuristicVO(String name, String description, Long zoneId, String purpose, String heuristicRule) {
         this.name = name;
         this.description = description;
         this.zoneId = zoneId;
         this.purpose = purpose;
         this.heuristicRule = heuristicRule;
-        this.created = created;
-        this.removed = removed;
     }
 
     @Override

engine/schema/src/main/java/org/apache/cloudstack/secstorage/dao/SecondaryStorageHeuristicDaoImpl.java

@@ -45,6 +45,6 @@ public HeuristicVO findByZoneIdAndPurpose(long zoneId, HeuristicPurpose purpose)
         searchCriteria.setParameters("purpose", purpose.toString());
         final Filter filter = new Filter(HeuristicVO.class, "created", false);
 
-        return findOneIncludingRemovedBy(searchCriteria, filter);
+        return findOneBy(searchCriteria, filter);
     }
 }

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -203,7 +203,7 @@ public ImageStoreVO findOneByZoneAndProtocol(long dataCenterId, String protocol)
     @Override
     public List<ImageStoreVO> listImageStoresByZoneIds(Long... zoneIds) {
         SearchCriteria<ImageStoreVO> sc = zonesInSearch.create();
-        sc.setParametersIfNotNull("zonesIn" ,zoneIds);
+        sc.setParametersIfNotNull("zonesIn", zoneIds);
         return listBy(sc);
     }
 }

framework/db/src/main/java/com/cloud/utils/db/GenericDao.java

@@ -258,7 +258,7 @@ public interface GenericDao<T, ID extends Serializable> {
 
     public T findOneBy(final SearchCriteria<T> sc);
 
-    T findOneIncludingRemovedBy(SearchCriteria<T> sc, Filter filter);
+    T findOneBy(SearchCriteria<T> sc, Filter filter);
 
     /**
      * @return

framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java

@@ -909,7 +909,7 @@ public T findOneBy(SearchCriteria<T> sc) {
 
     @Override
     @DB()
-    public T findOneIncludingRemovedBy(SearchCriteria<T> sc, final Filter filter) {
+    public T findOneBy(SearchCriteria<T> sc, final Filter filter) {
         sc = checkAndSetRemovedIsNull(sc);
         filter.setLimit(1L);
         List<T> results = searchIncludingRemoved(sc, filter, null, false);

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -5093,17 +5093,9 @@ public FirewallResponse createIpv6FirewallRuleResponse(FirewallRule fwRule) {
 
     @Override
     public SecondaryStorageHeuristicsResponse createSecondaryStorageSelectorResponse(Heuristic heuristic) {
-        SecondaryStorageHeuristicsResponse secondaryStorageHeuristicsResponse = new SecondaryStorageHeuristicsResponse();
         String zoneUuid = ApiDBUtils.findZoneById(heuristic.getZoneId()).getUuid();
-
-        secondaryStorageHeuristicsResponse.setId(heuristic.getUuid());
-        secondaryStorageHeuristicsResponse.setName(heuristic.getName());
-        secondaryStorageHeuristicsResponse.setDescription(heuristic.getDescription());
-        secondaryStorageHeuristicsResponse.setZoneId(zoneUuid);
-        secondaryStorageHeuristicsResponse.setPurpose(heuristic.getPurpose());
-        secondaryStorageHeuristicsResponse.setHeuristicRule(heuristic.getHeuristicRule());
-        secondaryStorageHeuristicsResponse.setCreated(heuristic.getCreated());
-        secondaryStorageHeuristicsResponse.setRemoved(heuristic.getRemoved());
+        SecondaryStorageHeuristicsResponse secondaryStorageHeuristicsResponse = new SecondaryStorageHeuristicsResponse(heuristic.getUuid(), heuristic.getName(),
+                heuristic.getDescription(), zoneUuid, heuristic.getPurpose(), heuristic.getHeuristicRule(), heuristic.getCreated(), heuristic.getRemoved());
         secondaryStorageHeuristicsResponse.setResponseName("secondarystorageheuristics");
 
         return secondaryStorageHeuristicsResponse;

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

@@ -1924,7 +1924,7 @@ public Heuristic createSecondaryStorageHeuristic(CreateSecondaryStorageSelectorC
 
         validateHeuristicRule(heuristicRule);
 
-        HeuristicVO heuristicVO = new HeuristicVO(name, description, zoneId, formattedPurpose.toString(), heuristicRule, new Date(), null);
+        HeuristicVO heuristicVO = new HeuristicVO(name, description, zoneId, formattedPurpose.toString(), heuristicRule);
         return secondaryStorageHeuristicDao.persist(heuristicVO);
     }
 
@@ -1948,7 +1948,7 @@ public void removeSecondaryStorageHeuristic(RemoveSecondaryStorageSelectorCmd cm
         if (heuristicVO != null) {
             secondaryStorageHeuristicDao.remove(heuristicId);
         } else {
-            throw new CloudRuntimeException(String.format("Unable to find an active heuristic with the specified UUID."));
+            throw new CloudRuntimeException("Unable to find an active heuristic with the specified UUID.");
         }
     }
 

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java

@@ -16,7 +16,7 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics.presetvariables;
 
-public class Account extends GenericHeuristicPresetVariable{
+public class Account extends GenericHeuristicPresetVariable {
     private String id;
 
     private Domain domain;

server/src/test/java/com/cloud/user/MockUsageEventDao.java

@@ -262,7 +262,7 @@ public UsageEventVO findOneBy(SearchCriteria<UsageEventVO> sc) {
     }
 
     @Override
-    public UsageEventVO findOneIncludingRemovedBy(SearchCriteria<UsageEventVO> sc, Filter filter) {
+    public UsageEventVO findOneBy(SearchCriteria<UsageEventVO> sc, Filter filter) {
         return null;
     }