Dedup conditions before build union condition

NSX will reject expression list of a group if defined like follows:
---
"expression":
  [{"expressions": [
      {"member_type": "LogicalPort", "operator": "EQUALS", "value": "tag1|x"},
      {"conjunction_operator": "AND", "resource_type": "ConjunctionOperator"},
      {member_type": "LogicalPort", "operator": "EQUALS", "value": "tag2|y"}
    ], "resource_type": "NestedExpression"},
   {"conjunction_operator": "OR", "resource_type": "ConjunctionOperator"},
   {"expressions": [
       {"member_type": "LogicalPort", "operator": "EQUALS", "value": "tag1|x"},
       {"conjunction_operator": "AND", "resource_type": "ConjunctionOperator"},
       {member_type": "LogicalPort", "operator": "EQUALS", "value": "tag2|y"}
    ], "resource_type": "NestedExpression"},
   "resource_type": "NestedExpression"}
 ]
---
Because the two NestedExpressions are identical. If patch a group with the
spec above, 'Duplicate expressions specified' error will be returned. This
patch ensures that before a union condition is built, all conditions are
first dedupped.

Change-Id: I0d2d93f6ade992582ad931b3622354e5b9398a1d
Signed-off-by: Yang Ding <dingyang@vmware.com>

vmware_nsxlib/tests/unit/v3/policy/test_resources.py

@@ -384,8 +384,16 @@ class TestPolicyGroup(NsxPolicyLibTestCase):
             cond_op=cond_op,
             cond_member_type=cond_member_type,
             cond_key=cond_key)
-        union_cond = self.resourceApi.build_union_condition(
+        cond1_dup = self.resourceApi.build_condition(
+            cond_val=cond_val1,
+            cond_op=cond_op,
+            cond_member_type=cond_member_type,
+            cond_key=cond_key)
+
+        union_cond_no_dup = self.resourceApi.build_union_condition(
             conditions=[cond1, cond2])
+        union_cond_dup = self.resourceApi.build_union_condition(
+            conditions=[cond1, cond1_dup])
 
         exp_cond1 = core_defs.Condition(value=cond_val1,
                                         key=cond_key,
@@ -397,8 +405,10 @@ class TestPolicyGroup(NsxPolicyLibTestCase):
                                         member_type=cond_member_type)
         or_cond = core_defs.ConjunctionOperator(
             operator=constants.CONDITION_OP_OR)
-        exp_cond = [exp_cond1, or_cond, exp_cond2]
+        exp_cond = list(set([exp_cond1, exp_cond2]))
-        self._test_create_with_condition(union_cond, exp_cond)
+        exp_cond.insert(1, or_cond)
+        self._test_create_with_condition(union_cond_no_dup, exp_cond)
+        self._test_create_with_condition(union_cond_dup, [exp_cond1])
 
     def test_create_with_nested_condition(self):
         cond_val1 = '123'
@@ -429,10 +439,49 @@ class TestPolicyGroup(NsxPolicyLibTestCase):
                                         operator=cond_op,
                                         member_type=cond_member_type)
         and_cond = core_defs.ConjunctionOperator()
-        exp_cond = core_defs.NestedExpression(
+        expressions = list(set([exp_cond1, exp_cond2]))
-            expressions=[exp_cond1, and_cond, exp_cond2])
+        expressions.insert(1, and_cond)
+        exp_cond = core_defs.NestedExpression(expressions=expressions)
         self._test_create_with_condition(nested, exp_cond)
 
+    def test_create_with_dup_nested_condition(self):
+        cond_val1 = '123'
+        cond_val2 = '456'
+        cond_op = constants.CONDITION_OP_EQUALS
+        cond_member_type = constants.CONDITION_MEMBER_VM
+        cond_key = constants.CONDITION_KEY_TAG
+
+        cond1 = self.resourceApi.build_condition(
+            cond_val=cond_val1,
+            cond_op=cond_op,
+            cond_member_type=cond_member_type,
+            cond_key=cond_key)
+        cond2 = self.resourceApi.build_condition(
+            cond_val=cond_val2,
+            cond_op=cond_op,
+            cond_member_type=cond_member_type,
+            cond_key=cond_key)
+        nested = self.resourceApi.build_nested_condition(
+            conditions=[cond1, cond2])
+
+        cond1_dup = self.resourceApi.build_condition(
+            cond_val=cond_val1,
+            cond_op=cond_op,
+            cond_member_type=cond_member_type,
+            cond_key=cond_key)
+        cond2_dup = self.resourceApi.build_condition(
+            cond_val=cond_val2,
+            cond_op=cond_op,
+            cond_member_type=cond_member_type,
+            cond_key=cond_key)
+        nested_dup = self.resourceApi.build_nested_condition(
+            conditions=[cond1_dup, cond2_dup])
+
+        double_nested = self.resourceApi.build_nested_condition(
+            conditions=[nested, nested_dup])
+        exp_cond = core_defs.NestedExpression(expressions=[nested])
+        self._test_create_with_condition(double_nested, exp_cond)
+
     def test_create_with_ip_expression(self):
         domain_id = '111'
         name = 'g1'

vmware_nsxlib/v3/policy/core_defs.py

@@ -1485,6 +1485,14 @@ class Condition(object):
                 'value': self.value,
                 'operator': self.operator}
 
+    def __eq__(self, other):
+        if isinstance(other, Condition):
+            return self.get_obj_dict() == other.get_obj_dict()
+        return False
+
+    def __hash__(self):
+        return hash(tuple(self.get_obj_dict().values()))
+
 
 class IPAddressExpression(object):
     def __init__(self, ip_addresses):
@@ -1521,6 +1529,46 @@ class NestedExpression(object):
         return {'resource_type': 'NestedExpression',
                 'expressions': [ex.get_obj_dict() for ex in self.expressions]}
 
+    def _get_unique_expressions(self):
+        """Only AND operator is supported in a nested expression.
+
+        When comparing two nested expressions, only checking the Conditions
+        in the expression list will suffice since all ConjunctionOperators
+        will be the same.
+        """
+        expr_list = self.get_obj_dict()['expressions']
+        unique_exprs = [expr for expr in expr_list
+                        if expr.get('resource_type') != 'ConjunctionOperator']
+        return unique_exprs
+
+    def __eq__(self, other):
+
+        def expr_identifier(expr):
+            return expr.get('member_type'), expr.get('value')
+
+        if not isinstance(other, NestedExpression):
+            return False
+        self_expr_list = self._get_unique_expressions()
+        other_expr_list = other._get_unique_expressions()
+        if len(self_expr_list) != len(other_expr_list):
+            return False
+        # Each expression dict in the list should be uniquely identified by
+        # (expr.member_type, expr.value pair). In case of segment expressions,
+        # the identifier will be of format ("Segment", "tag_scope|tag_value")
+        self_sorted = sorted(self_expr_list, key=expr_identifier)
+        other_sorted = sorted(other_expr_list, key=expr_identifier)
+        for i in range(len(self_sorted)):
+            if not all(item in other_sorted[i].items()
+                       for item in self_sorted[i].items()):
+                return False
+        return True
+
+    def __hash__(self):
+        # List is not hashable in python
+        value_list = [tuple(expr.values())
+                      for expr in self._get_unique_expressions()]
+        return hash(tuple(value_list))
+
 
 class GroupDef(ResourceDef):
 

vmware_nsxlib/v3/policy/core_resources.py

@@ -498,12 +498,19 @@ class NsxPolicyGroupApi(NsxPolicyResourceBase):
 
     def build_union_condition(self, operator=constants.CONDITION_OP_OR,
                               conditions=None):
+        # NSX don't allow duplicate expressions in expression list
+        # of a group -> (ERROR: Duplicate expressions specified)
+        # Members of input conditions is either instance of Condition
+        # or NestedExpression class.
         expressions = []
-        for cond in conditions:
+        if conditions:
-            if len(expressions):
+            conditions = list(set(conditions))
-                expressions.append(core_defs.ConjunctionOperator(
+            expressions = []
-                    operator=operator))
+            for cond in conditions:
-            expressions.append(cond)
+                if len(expressions):
+                    expressions.append(core_defs.ConjunctionOperator(
+                        operator=operator))
+                expressions.append(cond)
         return expressions
 
     def build_nested_condition(