From 12f9ef8d54fdd4eaf73a2f521632964bf5a84571 Mon Sep 17 00:00:00 2001
From: Travis Tripp <travis.tripp@hpe.com>
Date: Wed, 27 Jul 2016 22:42:36 -0600
Subject: [PATCH] Remove array metadata when no items + fix case

Array metadata appends an operator automatically, which
means the value will still contain something and it
doesn't get detected for removal.

In addition, bad APIs like glance v1 will store a property
with mixed case, but then return only lower case when
listing them. However, it will not remove the property
when you request to remove it unless it matches the original
case (sad panda).  So, this patch will reset the proper
casing for properties defined as metadata definitions before
making the request to save the metadata.

Change-Id: I1e5a6fde35b7be6118f21ac46ad0aea088280215
Closes-Bug: 1606988
---
 .../widgets/metadata/tree/tree.service.js     |  8 +++--
 .../core/metadata/modal/modal.controller.js   | 13 ++++++++
 .../metadata/modal/modal.controller.spec.js   | 30 ++++++++++++++++---
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/horizon/static/framework/widgets/metadata/tree/tree.service.js b/horizon/static/framework/widgets/metadata/tree/tree.service.js
index 065797c19e..c4d86d87e3 100644
--- a/horizon/static/framework/widgets/metadata/tree/tree.service.js
+++ b/horizon/static/framework/widgets/metadata/tree/tree.service.js
@@ -82,7 +82,7 @@
           break;
         case 'array':
           var data = /^(<.*?>) (.*)$/.exec(value);
-          if (data) {
+          if (data && data.length > 2) {
             this.operator = data[1];
             this.value = data[2].split(',');
           }
@@ -440,7 +440,11 @@
       var existing = {};
       angular.forEach(this.flatTree, function (item) {
         if (item.added && item.leaf) {
-          existing[item.leaf.name] = item.getLeafValue();
+          if (item.leaf.type === 'array' && item.leaf.value.length === 0) {
+            // Do nothing, the user has removed or not selected any array values
+          } else {
+            existing[item.leaf.name] = item.getLeafValue();
+          }
         }
       });
       return existing;
diff --git a/openstack_dashboard/static/app/core/metadata/modal/modal.controller.js b/openstack_dashboard/static/app/core/metadata/modal/modal.controller.js
index a69a19826f..cf884c6534 100644
--- a/openstack_dashboard/static/app/core/metadata/modal/modal.controller.js
+++ b/openstack_dashboard/static/app/core/metadata/modal/modal.controller.js
@@ -53,6 +53,19 @@
       ctrl.saving = true;
       var updated = ctrl.tree.getExisting();
       var removed = angular.copy(existing.data);
+
+      // Glance v1 changes metadata property casing in the get request
+      // but to remove you still need to send back in using the proper original case.
+      // See https://bugs.launchpad.net/horizon/+bug/1606988
+      angular.forEach(removed, function bug1606988(value, removedKey) {
+        angular.forEach(ctrl.tree.flatTree, function compareToDefinitions(item) {
+          if (item.leaf && removedKey.toLocaleLowerCase() === item.leaf.name.toLocaleLowerCase()) {
+            removed[item.leaf.name] = value;
+            delete removed[removedKey];
+          }
+        });
+      });
+
       angular.forEach(updated, function(value, key) {
         delete removed[key];
       });
diff --git a/openstack_dashboard/static/app/core/metadata/modal/modal.controller.spec.js b/openstack_dashboard/static/app/core/metadata/modal/modal.controller.spec.js
index 4ac32e8c14..081c029115 100644
--- a/openstack_dashboard/static/app/core/metadata/modal/modal.controller.spec.js
+++ b/openstack_dashboard/static/app/core/metadata/modal/modal.controller.spec.js
@@ -24,6 +24,27 @@
       editMetadata: function() {}
     };
 
+    var availableItem = {
+      'namespace': 'bug1606988',
+      'properties': {
+        'UPPER_lower': {
+          'items': {
+            'enum': [
+              'foo',
+              'bar'
+            ],
+            'type': 'string'
+          },
+          'type': 'array'
+        }
+      }
+    };
+
+    var existing = {
+      'upper_lower': 'foo',
+      'custom': 'bar'
+    };
+
     beforeEach(function() {
       modalInstance = {
         dismiss: jasmine.createSpy(),
@@ -62,7 +83,7 @@
 
       expect(modalInstance.close).toHaveBeenCalled();
       expect(metadataService.editMetadata)
-        .toHaveBeenCalledWith('aggregate', '123', {someProperty: 'someValue'}, []);
+        .toHaveBeenCalledWith('aggregate', '123', {'custom': 'bar'}, ['UPPER_lower']);
     });
 
     it('should clear saving flag on failed save', function() {
@@ -81,16 +102,17 @@
 
       expect(modalInstance.close).not.toHaveBeenCalled();
       expect(metadataService.editMetadata)
-        .toHaveBeenCalledWith('aggregate', '123', {someProperty: 'someValue'}, []);
+        .toHaveBeenCalledWith('aggregate', '123', {'custom': 'bar'}, ['UPPER_lower']);
     });
 
     function createController() {
+      //Purposely use different cases in available and existing.
       return $controller('MetadataModalController', {
         '$modalInstance': modalInstance,
         'horizon.framework.widgets.metadata.tree.service': treeService,
         'horizon.app.core.metadata.service': metadataService,
-        'available': {data: {}},
-        'existing': {data: {someProperty: 'someValue'}},
+        'available': {data: {items: [availableItem]}},
+        'existing': {data: existing},
         'params': {resource: 'aggregate', id: '123'}
       });
     }