From 886854fc4a27541eb53dd0e8c78918d1f8626e7d Mon Sep 17 00:00:00 2001
From: Enhao Cui <ecui@vmware.com>
Date: Tue, 18 Feb 2020 14:52:12 -0800
Subject: [PATCH] Fix group def in Policy API

When updating group with empty conditions list, "expression" list
should be explicitly setting to empty list instead of ignored,
so that group gets updated properly with empty expressions.

Change-Id: I779dca3587721f7d9b0da83385a243e3a1132f7c
---
 .../tests/unit/v3/policy/test_resources.py     | 18 ++++++++++++++++++
 .../tests/unit/v3/policy/test_transaction.py   | 10 +++++-----
 vmware_nsxlib/v3/policy/core_defs.py           | 10 ++++++----
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
index 8120c26e..48fb849c 100644
--- a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
+++ b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
@@ -297,6 +297,24 @@ class TestPolicyGroup(NsxPolicyLibTestCase):
             self.assert_called_with_def(api_call, expected_def)
             self.assertIsNotNone(result)
 
+    def test_create_with_empty_condition(self):
+        domain_id = '111'
+        name = 'g1'
+        description = 'desc'
+        with mock.patch.object(self.policy_api,
+                               "create_or_update") as api_call:
+            result = self.resourceApi.create_or_overwrite(
+                name, domain_id, description=description,
+                tenant=TEST_TENANT)
+            expected_def = core_defs.GroupDef(domain_id=domain_id,
+                                              group_id=mock.ANY,
+                                              name=name,
+                                              description=description,
+                                              conditions=[],
+                                              tenant=TEST_TENANT)
+            self.assert_called_with_def(api_call, expected_def)
+            self.assertIsNotNone(result)
+
     def test_create_with_simple_condition(self):
         domain_id = '111'
         name = 'g1'
diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py b/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py
index 091ca9b1..2e8af0ad 100644
--- a/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py
+++ b/vmware_nsxlib/tests/unit/v3/policy/test_transaction.py
@@ -77,15 +77,15 @@ class TestPolicyTransaction(policy_testcase.TestPolicyApi):
         g1 = {'resource_type': 'Group', 'id': 'group1',
               'display_name': 'g1',
               'description': 'first group',
-              'tags': None}
+              'tags': None, 'expression': []}
         g2 = {'resource_type': 'Group', 'id': 'group2',
               'description': 'second group',
               'display_name': 'g2',
-              'tags': tags}
+              'tags': tags, 'expression': []}
         g3 = {'resource_type': 'Group', 'id': 'group3',
               'display_name': 'g3',
               'description': 'third group',
-              'tags': None}
+              'tags': None, 'expression': []}
         d1 = {'resource_type': 'Domain', 'id': 'domain1',
               'display_name': 'd1', 'description': 'first domain',
               'tags': tags}
@@ -168,10 +168,10 @@ class TestPolicyTransaction(policy_testcase.TestPolicyApi):
 
         g1 = {'resource_type': 'Group', 'id': 'group1',
               'display_name': 'g1',
-              'description': 'first group'}
+              'description': 'first group', 'expression': []}
         g2 = {'resource_type': 'Group', 'id': 'group2',
               'description': 'second group',
-              'display_name': 'g2'}
+              'display_name': 'g2', 'expression': []}
         d1 = {'resource_type': 'Domain', 'id': 'domain1'}
 
         d2 = {'resource_type': 'Domain', 'id': 'domain2'}
diff --git a/vmware_nsxlib/v3/policy/core_defs.py b/vmware_nsxlib/v3/policy/core_defs.py
index 7f25e914..736cd319 100644
--- a/vmware_nsxlib/v3/policy/core_defs.py
+++ b/vmware_nsxlib/v3/policy/core_defs.py
@@ -1516,11 +1516,13 @@ class GroupDef(ResourceDef):
     def get_obj_dict(self):
         body = super(GroupDef, self).get_obj_dict()
         conds = self.get_attr('conditions')
-        if conds:
+        # If conditions were IGNORE, conds would be None here.
+        # Otherwise, conds could be an empty list which denotes
+        # updating group expression to empty list.
+        if conds is not None:
             conds = conds if isinstance(conds, list) else [conds]
-            if conds:
-                body['expression'] = [condition.get_obj_dict()
-                                      for condition in conds]
+            body['expression'] = [condition.get_obj_dict()
+                                  for condition in conds]
         return body