[GitHub] [cloudstack] rhtyd commented on a change in pull request #4140: Adding showunique parameter to list templates and isos

2020-06-16 Thread GitBox 


rhtyd commented on a change in pull request #4140:
URL: https://github.com/apache/cloudstack/pull/4140#discussion_r441239286



##
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##
@@ -3423,12 +3432,22 @@ else if (!template.isPublicTemplate() && 
caller.getType() != Account.ACCOUNT_TYP
 return uniqueTmplPair;
 }
 List uniqueTmpls = uniqueTmplPair.first();
-String[] tzIds = new String[uniqueTmpls.size()];
 int i = 0;
-for (TemplateJoinVO v : uniqueTmpls) {
-tzIds[i++] = v.getTempZonePair();
+List vrs = null;
+if (showUnique) {
+Long[] tzIds = new Long[uniqueTmpls.size()];

Review comment:
   @davidjumani nit - let's refactor and use a more modern form of writing 
code, the current code looks less like Java, more like C/C++; can be written as:
   ```
   Long[] templateIds = uniqueTmpls.stream().map(template -> 
template.getId()).toArray(Long[]::new);
   ```





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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #4140: Adding showunique parameter to list templates and isos

2020-06-16 Thread GitBox 


rhtyd commented on a change in pull request #4140:
URL: https://github.com/apache/cloudstack/pull/4140#discussion_r441234854



##
File path: server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java
##
@@ -481,4 +488,14 @@ public TemplateResponse newIsoResponse(TemplateJoinVO iso) 
{
 return new Pair, Integer>(objects, count);
 }
 
+@Override
+public List findByDistinctIds(Long... ids) {
+if (ids.length == 0) {

Review comment:
   @davidjumani make it defensive, for example do a null check on ids





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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #4140: Adding showunique parameter to list templates and isos

2020-06-14 Thread GitBox 


rhtyd commented on a change in pull request #4140:
URL: https://github.com/apache/cloudstack/pull/4140#discussion_r439908149



##
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##
@@ -3423,12 +3432,32 @@ else if (!template.isPublicTemplate() && 
caller.getType() != Account.ACCOUNT_TYP
 return uniqueTmplPair;
 }
 List uniqueTmpls = uniqueTmplPair.first();
-String[] tzIds = new String[uniqueTmpls.size()];
 int i = 0;
-for (TemplateJoinVO v : uniqueTmpls) {
-tzIds[i++] = v.getTempZonePair();
+List vrs = null;
+if (showUnique) {
+Long[] tzIds = new Long[uniqueTmpls.size()];
+for (TemplateJoinVO v : uniqueTmpls) {
+tzIds[i++] = v.getId();
+}
+vrs = _templateJoinDao.findByIds(tzIds);
+
+// Get only unique id rows
+Long lastId = null;
+for(i = vrs.size() - 1; i >= 0; i--) {
+if (new Long(vrs.get(i).getId()).equals(lastId)) {
+vrs.remove(i);

Review comment:
   Please explore if you can simplify the code





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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #4140: Adding showunique parameter to list templates and isos

2020-06-14 Thread GitBox 


rhtyd commented on a change in pull request #4140:
URL: https://github.com/apache/cloudstack/pull/4140#discussion_r439908027



##
File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsosCmd.java
##
@@ -118,6 +121,10 @@ public Boolean getShowRemoved() {
 return (showRemoved != null ? showRemoved : false);
 }
 
+public Boolean getShowUnique() {
+return (showUnique != null ? showUnique : false);

Review comment:
   Can be simplified as `return showUnique != null && showUnique;`





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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #4140: Adding showunique parameter to list templates and isos

2020-06-14 Thread GitBox 


rhtyd commented on a change in pull request #4140:
URL: https://github.com/apache/cloudstack/pull/4140#discussion_r439907843



##
File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatesCmd.java
##
@@ -78,6 +78,9 @@
 @Parameter(name = ApiConstants.PARENT_TEMPLATE_ID, type = 
CommandType.UUID, entityType = TemplateResponse.class, description = "list 
datadisk templates by parent template id", since = "4.4")
 private Long parentTemplateId;
 
+@Parameter(name = ApiConstants.SHOW_UNIQUE, type = CommandType.BOOLEAN, 
description = "If set to true, list only unique templates across zones")

Review comment:
   @davidjumani add version `4.15`





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