Merge lp:~mpontillo/maas/fix-1517097-1.9 into lp:maas/1.9

[Mike Pontillo]

Merge fix for #1517097 from trunk revision 4512.

[Mike Pontillo (mpontillo)]

Self-approving approved backported fix.

review: Approve

=== modified file 'src/maasserver/node_constraint_filter_forms.py'
--- src/maasserver/node_constraint_filter_forms.py	2015-11-03 02:37:13 +0000
+++ src/maasserver/node_constraint_filter_forms.py	2015-11-21 00:08:55 +0000
@@ -398,19 +398,12 @@
     node_ids = None
     label_map = {}
     for label in interfaces_label_map:
-        specifiers = []
         constraints = interfaces_label_map[label]
-        for specifier_type in constraints.iterkeys():
-            # Build constraints string.
-            specifiers.extend([
-                specifier_type + ":" + item
-                for item in constraints[specifier_type]
-            ])
         if node_ids is None:
             # The first time through the filter, build the list
             # of candidate nodes.
             node_ids, node_map = Interface.objects.get_matching_node_map(
-                specifiers)
+                constraints)
             label_map[label] = node_map
         else:
             # For subsequent labels, only match nodes that already matched a
@@ -421,7 +414,7 @@
             # to filter the nodes starting from an 'id__in' filter using the
             # current 'node_ids' set.
             new_node_ids, node_map = Interface.objects.get_matching_node_map(
-                specifiers)
+                constraints)
             label_map[label] = node_map
             node_ids &= new_node_ids
     return node_ids, label_map

=== modified file 'src/maasserver/tests/test_node_constraint_filter_forms.py'
--- src/maasserver/tests/test_node_constraint_filter_forms.py	2015-11-03 02:37:13 +0000
+++ src/maasserver/tests/test_node_constraint_filter_forms.py	2015-11-21 00:08:55 +0000
@@ -1026,7 +1026,7 @@
         filtered_nodes, _, _ = form.filter_nodes(Node.nodes)
         self.assertItemsEqual([node2], filtered_nodes)
 
-    def test_interfaces_filters_multiples_treated_as_OR_operation(self):
+    def test_interfaces_filters_same_key_treated_as_OR_operation(self):
         fabric1 = factory.make_Fabric(class_type="1g")
         fabric2 = factory.make_Fabric(class_type="10g")
         vlan1 = factory.make_VLAN(vid=1, fabric=fabric1)
@@ -1050,6 +1050,30 @@
         filtered_nodes, _, _ = form.filter_nodes(Node.nodes)
         self.assertItemsEqual([node2], filtered_nodes)
 
+    def test_interfaces_filters_different_key_treated_as_AND_operation(self):
+        fabric1 = factory.make_Fabric(class_type="1g")
+        fabric2 = factory.make_Fabric(class_type="10g")
+        vlan1 = factory.make_VLAN(vid=1, fabric=fabric1)
+        vlan2 = factory.make_VLAN(vid=2, fabric=fabric2)
+        node1 = factory.make_Node_with_Interface_on_Subnet(
+            fabric=fabric1, vlan=vlan1)
+        node2 = factory.make_Node_with_Interface_on_Subnet(
+            fabric=fabric2, vlan=vlan2)
+
+        form = AcquireNodeForm({
+            u'interfaces':
+            u'none:fabric_class=1g,vid=2'})
+        self.assertTrue(form.is_valid(), dict(form.errors))
+        filtered_nodes, _, _ = form.filter_nodes(Node.nodes)
+        self.assertItemsEqual([], filtered_nodes)
+
+        form = AcquireNodeForm({
+            u'interfaces':
+            u'any:fabric_class=10g,fabric_class=1g,vid=1,vid=2'})
+        self.assertTrue(form.is_valid(), dict(form.errors))
+        filtered_nodes, _, _ = form.filter_nodes(Node.nodes)
+        self.assertItemsEqual([node1, node2], filtered_nodes)
+
     def test_combined_constraints(self):
         tag_big = factory.make_Tag(name='big')
         arch = '%s/generic' % factory.make_name('arch')

=== modified file 'src/maasserver/utils/orm.py'
--- src/maasserver/utils/orm.py	2015-11-07 00:56:58 +0000
+++ src/maasserver/utils/orm.py	2015-11-21 00:08:55 +0000
@@ -801,6 +801,12 @@
         inferred that the user would like a set of queries joined with
         logical AND operators.
 
+        If the list of specifiers is given as a dict, it is inferred that each
+        key is a specifier type, and each value is a list of specifier values.
+        The specifier values inside each list will be joined with logical OR
+        operators. The lists for each key will be joined with logical AND
+        operators.
+
         For example, 'name:eth0,hostname:tasty-buscuits' might match interface
         eth0 on node 'tasty-biscuits'; that is, both constraints are required.
         """
@@ -810,6 +816,8 @@
             return [
                 '&' + specifier.strip() for specifier in specifiers.split(',')
             ]
+        elif isinstance(specifiers, dict):
+            return specifiers
         else:
             return list(flatten(specifiers))
 
@@ -894,13 +902,28 @@
         if specifier_types is None:
             raise NotImplementedError("Subclass must specify specifier_types.")
         current_q = Q()
-        for item in specifiers:
-            item, op = parse_item_operation(item)
-            item, specifier_type = parse_item_specifier_type(
-                item, spec_types=specifier_types, separator=separator)
-            query = self.get_filter_function(
-                specifier_type, specifier_types, item, separator=separator)
-            current_q = query(current_q, op, item)
+        if isinstance(specifiers, dict):
+            # If we got a dictionary, treat it as one of the entries in a
+            # LabeledConstraintMap. That is, each key is a specifier, and
+            # each value is a list of values (which must be OR'd together).
+            for key in specifiers.iterkeys():
+                assert isinstance(specifiers[key], list)
+                constraints = [
+                    key + separator + value
+                    for value in specifiers[key]
+                ]
+                # Leave off specifier_types here because this recursion
+                # will go back to the subclass to get the types filled in.
+                current_q &= self.get_specifiers_q(
+                    constraints, separator=separator)
+        else:
+            for item in specifiers:
+                item, op = parse_item_operation(item)
+                item, specifier_type = parse_item_specifier_type(
+                    item, spec_types=specifier_types, separator=separator)
+                query = self.get_filter_function(
+                    specifier_type, specifier_types, item, separator=separator)
+                current_q = query(current_q, op, item)
         if len(kwargs) > 0:
             current_q &= Q(**kwargs)
         return current_q