CLOUDSTACK-9574: Redesign storage tags and remove details from listImageStores response and UI

api/src/org/apache/cloudstack/api/response/ImageStoreResponse.java

@@ -16,9 +16,6 @@
 // under the License.
 package org.apache.cloudstack.api.response;
 
-import java.util.LinkedHashSet;
-import java.util.Set;
-
 import com.google.gson.annotations.SerializedName;
 
 import org.apache.cloudstack.api.ApiConstants;
@@ -63,12 +60,7 @@ public class ImageStoreResponse extends BaseResponse {
     @Param(description = "the scope of the image store")
     private ScopeType scope;
 
-    @SerializedName("details")
-    @Param(description = "the details of the image store")
-    private Set<ImageStoreDetailResponse> details;
-
     public ImageStoreResponse() {
-        this.details = new LinkedHashSet<ImageStoreDetailResponse>();
     }
 
     @Override
@@ -140,16 +132,4 @@ public class ImageStoreResponse extends BaseResponse {
         this.protocol = protocol;
     }
 
-    public Set<ImageStoreDetailResponse> getDetails() {
-        return details;
-    }
-
-    public void setDetails(Set<ImageStoreDetailResponse> details) {
-        this.details = details;
-    }
-
-    public void addDetail(ImageStoreDetailResponse detail) {
-        this.details.add(detail);
-    }
-
 }

engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml

@@ -266,6 +266,7 @@
   <bean id="storageNetworkIpRangeDaoImpl" class="com.cloud.dc.dao.StorageNetworkIpRangeDaoImpl" />
   <bean id="storagePoolDetailsDaoImpl" class="com.cloud.storage.dao.StoragePoolDetailsDaoImpl" />
   <bean id="storagePoolJoinDaoImpl" class="com.cloud.api.query.dao.StoragePoolJoinDaoImpl" />
+  <bean id="storagePoolTagsDaoImpl" class="com.cloud.storage.dao.StoragePoolTagsDaoImpl" />
   <bean id="storageTagDaoImpl" class="com.cloud.api.query.dao.StorageTagDaoImpl" />
   <bean id="hostTagDaoImpl" class="com.cloud.api.query.dao.HostTagDaoImpl" />
   <bean id="storagePoolWorkDaoImpl" class="com.cloud.storage.dao.StoragePoolWorkDaoImpl" />

engine/schema/src/com/cloud/storage/StoragePoolTagVO.java

@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.storage;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+
+import org.apache.cloudstack.api.InternalIdentity;
+
+@Entity
+@Table(name = "storage_pool_tags")
+public class StoragePoolTagVO implements InternalIdentity {
+
+    protected StoragePoolTagVO() {
+    }
+
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    @Column(name = "id")
+    private long id;
+
+    @Column(name = "pool_id")
+    private long poolId;
+
+    @Column(name = "tag")
+    private String tag;
+
+    public StoragePoolTagVO(long poolId, String tag) {
+        this.poolId = poolId;
+        this.tag = tag;
+    }
+
+    @Override
+    public long getId() {
+        return this.id;
+    }
+
+    public long getPoolId() {
+        return poolId;
+    }
+
+    public String getTag() {
+        return tag;
+    }
+
+}

engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDao.java

@@ -0,0 +1,30 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.storage.dao;
+
+import java.util.List;
+
+import com.cloud.storage.StoragePoolTagVO;
+import com.cloud.utils.db.GenericDao;
+
+public interface StoragePoolTagsDao extends GenericDao<StoragePoolTagVO, Long> {
+
+    void persist(long poolId, List<String> storagePoolTags);
+    List<String> getStoragePoolTags(long poolId);
+    void deleteTags(long poolId);
+
+}

engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java

@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.storage.dao;
+
+import java.util.ArrayList;
+import java.util.List;
+import com.cloud.storage.StoragePoolTagVO;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.db.TransactionLegacy;
+
+public class StoragePoolTagsDaoImpl extends GenericDaoBase<StoragePoolTagVO, Long> implements StoragePoolTagsDao {
+
+    protected final SearchBuilder<StoragePoolTagVO> StoragePoolSearch;
+
+    public StoragePoolTagsDaoImpl() {
+        StoragePoolSearch = createSearchBuilder();
+        StoragePoolSearch.and("poolId", StoragePoolSearch.entity().getPoolId(), SearchCriteria.Op.EQ);
+        StoragePoolSearch.done();
+    }
+
+    @Override
+    public void persist(long poolId, List<String> storagePoolTags) {
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+
+        txn.start();
+        SearchCriteria<StoragePoolTagVO> sc = StoragePoolSearch.create();
+        sc.setParameters("poolId", poolId);
+        expunge(sc);
+
+        for (String tag : storagePoolTags) {
+            tag = tag.trim();
+            if (tag.length() > 0) {
+                StoragePoolTagVO vo = new StoragePoolTagVO(poolId, tag);
+                persist(vo);
+            }
+        }
+        txn.commit();
+    }
+
+    @Override
+    public List<String> getStoragePoolTags(long poolId) {
+        SearchCriteria<StoragePoolTagVO> sc = StoragePoolSearch.create();
+        sc.setParameters("poolId", poolId);
+
+        List<StoragePoolTagVO> results = search(sc, null);
+        List<String> storagePoolTags = new ArrayList<String>(results.size());
+        for (StoragePoolTagVO result : results) {
+            storagePoolTags.add(result.getTag());
+        }
+
+        return storagePoolTags;
+    }
+
+    @Override
+    public void deleteTags(long poolId) {
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        txn.start();
+        SearchCriteria<StoragePoolTagVO> sc = StoragePoolSearch.create();
+        sc.setParameters("poolId", poolId);
+        expunge(sc);
+        txn.commit();
+    }
+
+}

server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java

@@ -24,8 +24,6 @@ import javax.inject.Inject;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
-import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.response.ImageStoreDetailResponse;
 import org.apache.cloudstack.api.response.ImageStoreResponse;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 
@@ -77,32 +75,12 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase<ImageStoreJoinVO, Long
         osResponse.setZoneId(ids.getZoneUuid());
         osResponse.setZoneName(ids.getZoneName());
 
-        String detailName = ids.getDetailName();
-        if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) {
-            String detailValue = ids.getDetailValue();
-            if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) {
-                // ALWAYS return an empty value for the S3 secret key since that key is managed by Amazon and not CloudStack
-                detailValue = "";
-            }
-            ImageStoreDetailResponse osdResponse = new ImageStoreDetailResponse(detailName, detailValue);
-            osResponse.addDetail(osdResponse);
-        }
         osResponse.setObjectName("imagestore");
         return osResponse;
     }
 
     @Override
     public ImageStoreResponse setImageStoreResponse(ImageStoreResponse response, ImageStoreJoinVO ids) {
-        String detailName = ids.getDetailName();
-        if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) {
-            String detailValue = ids.getDetailValue();
-            if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) {
-                // ALWAYS return an empty value for the S3 secret key since that key is managed by Amazon and not CloudStack
-                detailValue = "";
-            }
-            ImageStoreDetailResponse osdResponse = new ImageStoreDetailResponse(detailName, detailValue);
-            response.addDetail(osdResponse);
-        }
         return response;
     }
 

server/src/com/cloud/api/query/vo/ImageStoreJoinVO.java

@@ -76,12 +76,6 @@ public class ImageStoreJoinVO extends BaseViewVO implements InternalIdentity, Id
     @Column(name = "data_center_name")
     private String zoneName;
 
-    @Column(name = "detail_name")
-    private String detailName;
-
-    @Column(name = "detail_value")
-    private String detailValue;
-
     @Column(name = GenericDao.REMOVED_COLUMN)
     private Date removed;
 
@@ -127,14 +121,6 @@ public class ImageStoreJoinVO extends BaseViewVO implements InternalIdentity, Id
         return scope;
     }
 
-    public String getDetailName() {
-        return detailName;
-    }
-
-    public String getDetailValue() {
-        return detailValue;
-    }
-
     public DataStoreRole getRole() {
         return role;
     }

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

@@ -31,7 +31,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
-import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -157,6 +156,7 @@ import com.cloud.storage.Volume.Type;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.StoragePoolTagsDao;
 import com.cloud.storage.dao.StoragePoolWorkDao;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VMTemplatePoolDao;
@@ -290,6 +290,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     ResourceLimitService _resourceLimitMgr;
     @Inject
     EntityManager _entityMgr;
+    @Inject
+    StoragePoolTagsDao _storagePoolTagsDao;
 
     protected List<StoragePoolDiscoverer> _discoverers;
 
@@ -776,51 +778,18 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws IllegalArgumentException {
         // Input validation
         Long id = cmd.getId();
-        List<String> tags = cmd.getTags();
 
         StoragePoolVO pool = _storagePoolDao.findById(id);
         if (pool == null) {
             throw new IllegalArgumentException("Unable to find storage pool with ID: " + id);
         }
 
-        Map<String, String> updatedDetails = new HashMap<String, String>();
-
-        if (tags != null) {
-            Map<String, String> existingDetails = _storagePoolDetailsDao.listDetailsKeyPairs(id);
-            Set<String> existingKeys = existingDetails.keySet();
-
-            Map<String, String> existingDetailsToKeep = new HashMap<String, String>();
-
-            for (String existingKey : existingKeys) {
-                String existingValue = existingDetails.get(existingKey);
-
-                if (!Boolean.TRUE.toString().equalsIgnoreCase(existingValue)) {
-                    existingDetailsToKeep.put(existingKey, existingValue);
+        final List<String> storagePoolTags = cmd.getTags();
+        if (storagePoolTags != null) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Updating Storage Pool Tags to :" + storagePoolTags);
             }
-            }
-
-            Map<String, String> details = new HashMap<String, String>();
-            for (String tag : tags) {
-                tag = tag.trim();
-                if (tag.length() > 0 && !details.containsKey(tag)) {
-                    details.put(tag, "true");
-                }
-            }
-
-            Set<String> existingKeysToKeep = existingDetailsToKeep.keySet();
-
-            for (String existingKeyToKeep : existingKeysToKeep) {
-                String existingValueToKeep = existingDetailsToKeep.get(existingKeyToKeep);
-
-                if (details.containsKey(existingKeyToKeep)) {
-                    throw new CloudRuntimeException("Storage tag '" + existingKeyToKeep +
-                            "' conflicts with a stored property of this primary storage. No changes were made.");
-                }
-
-                details.put(existingKeyToKeep, existingValueToKeep);
-            }
-
-            updatedDetails.putAll(details);
+            _storagePoolTagsDao.persist(pool.getId(), storagePoolTags);
         }
 
         Long updatedCapacityBytes = null;
@@ -863,8 +832,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
             } else {
                 disablePrimaryStoragePool(pool);
             }
-        } else if (updatedDetails.size() >= 0) {
-            _storagePoolDao.updateDetails(id, updatedDetails);
         }
 
         if (updatedCapacityBytes != null) {

setup/db/db/schema-4920to41000.sql

@@ -62,3 +62,84 @@ INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`,
 INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 3, 'createSnapshotFromVMSnapshot', 'ALLOW', 302) ON DUPLICATE KEY UPDATE rule=rule;
 INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'createSnapshotFromVMSnapshot', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule;
 
+-- Create table storage_pool_tags
+CREATE TABLE `cloud`.`storage_pool_tags` (
+  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
+  `pool_id` bigint(20) unsigned NOT NULL COMMENT "pool id",
+  `tag` varchar(255) NOT NULL,
+  PRIMARY KEY (`id`),
+  KEY `fk_storage_pool_tags__pool_id` (`pool_id`),
+  CONSTRAINT `fk_storage_pool_tags__pool_id` FOREIGN KEY (`pool_id`) REFERENCES `storage_pool` (`id`) ON DELETE CASCADE
+) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8;
+
+-- Insert storage tags from storage_pool_details
+INSERT INTO `cloud`.`storage_pool_tags` (pool_id, tag) SELECT pool_id, 
+name FROM `cloud`.`storage_pool_details` WHERE value = 'true';
+
+-- Alter view storage_pool_view
+CREATE OR REPLACE
+VIEW `storage_pool_view` AS
+    SELECT
+        `storage_pool`.`id` AS `id`,
+        `storage_pool`.`uuid` AS `uuid`,
+        `storage_pool`.`name` AS `name`,
+        `storage_pool`.`status` AS `status`,
+        `storage_pool`.`path` AS `path`,
+        `storage_pool`.`pool_type` AS `pool_type`,
+        `storage_pool`.`host_address` AS `host_address`,
+        `storage_pool`.`created` AS `created`,
+        `storage_pool`.`removed` AS `removed`,
+        `storage_pool`.`capacity_bytes` AS `capacity_bytes`,
+        `storage_pool`.`capacity_iops` AS `capacity_iops`,
+        `storage_pool`.`scope` AS `scope`,
+        `storage_pool`.`hypervisor` AS `hypervisor`,
+        `storage_pool`.`storage_provider_name` AS `storage_provider_name`,
+        `cluster`.`id` AS `cluster_id`,
+        `cluster`.`uuid` AS `cluster_uuid`,
+        `cluster`.`name` AS `cluster_name`,
+        `cluster`.`cluster_type` AS `cluster_type`,
+        `data_center`.`id` AS `data_center_id`,
+        `data_center`.`uuid` AS `data_center_uuid`,
+        `data_center`.`name` AS `data_center_name`,
+        `data_center`.`networktype` AS `data_center_type`,
+        `host_pod_ref`.`id` AS `pod_id`,
+        `host_pod_ref`.`uuid` AS `pod_uuid`,
+        `host_pod_ref`.`name` AS `pod_name`,
+        `storage_pool_tags`.`tag` AS `tag`,
+        `op_host_capacity`.`used_capacity` AS `disk_used_capacity`,
+        `op_host_capacity`.`reserved_capacity` AS `disk_reserved_capacity`,
+        `async_job`.`id` AS `job_id`,
+        `async_job`.`uuid` AS `job_uuid`,
+        `async_job`.`job_status` AS `job_status`,
+        `async_job`.`account_id` AS `job_account_id`
+    FROM
+        ((((((`storage_pool`
+        LEFT JOIN `cluster` ON ((`storage_pool`.`cluster_id` = `cluster`.`id`)))
+        LEFT JOIN `data_center` ON ((`storage_pool`.`data_center_id` = `data_center`.`id`)))
+        LEFT JOIN `host_pod_ref` ON ((`storage_pool`.`pod_id` = `host_pod_ref`.`id`)))
+        LEFT JOIN `storage_pool_tags` ON (((`storage_pool_tags`.`pool_id` = `storage_pool`.`id`))))
+        LEFT JOIN `op_host_capacity` ON (((`storage_pool`.`id` = `op_host_capacity`.`host_id`)
+            AND (`op_host_capacity`.`capacity_type` IN (3 , 9)))))
+        LEFT JOIN `async_job` ON (((`async_job`.`instance_id` = `storage_pool`.`id`)
+            AND (`async_job`.`instance_type` = 'StoragePool')
+            AND (`async_job`.`job_status` = 0))));
+
+-- Alter view image_store_view
+CREATE OR REPLACE
+VIEW `image_store_view` AS
+    SELECT
+        `image_store`.`id` AS `id`,
+        `image_store`.`uuid` AS `uuid`,
+        `image_store`.`name` AS `name`,
+        `image_store`.`image_provider_name` AS `image_provider_name`,
+        `image_store`.`protocol` AS `protocol`,
+        `image_store`.`url` AS `url`,
+        `image_store`.`scope` AS `scope`,
+        `image_store`.`role` AS `role`,
+        `image_store`.`removed` AS `removed`,
+        `data_center`.`id` AS `data_center_id`,
+        `data_center`.`uuid` AS `data_center_uuid`,
+        `data_center`.`name` AS `data_center_name`
+    FROM
+        (`image_store`
+        LEFT JOIN `data_center` ON ((`image_store`.`data_center_id` = `data_center`.`id`)));

ui/scripts/system.js

@@ -19973,21 +19973,6 @@
                                             zonename: {
                                                 label: 'label.zone'
                                             },
-                                            details: {
-                                                label: 'label.details',
-                                                converter: function (array1) {
-                                                    var string1 = '';
-                                                    if (array1 != null) {
-                                                        for (var i = 0; i < array1.length; i++) {
-                                                            if (i > 0)
-                                                            string1 += ', ';
-
-                                                            string1 += array1[i].name + ': ' + array1[i].value;
-                                                        }
-                                                    }
-                                                    return string1;
-                                                }
-                                            },
                                             id: {
                                                 label: 'label.id'
                                             }