launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/bug-1027154 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1027154 in MAAS: "Unable to look up a node based on mac address"
  https://bugs.launchpad.net/maas/+bug/1027154

For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1027154/+merge/116241

This branch adds the possibility to filter the nodes given by the (existing) API method 'list' with MAC addresses (only the nodes with the specific MAC addresses will be returned).

= Pre-imp =

Talked with Julian about this.
-- 
https://code.launchpad.net/~rvb/maas/bug-1027154/+merge/116241
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1027154 into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-07-12 12:33:57 +0000
+++ src/maasserver/api.py	2012-07-23 10:39:24 +0000
@@ -321,6 +321,16 @@
         return value
 
 
+def get_optional_list(data, key, default=None):
+    """Get the list from the provided data dict or return a default value.
+    """
+    value = data.getlist(key)
+    if value == []:
+        return default
+    else:
+        return value
+
+
 def extract_oauth_key_from_auth_header(auth_data):
     """Extract the oauth key from auth data in HTTP header.
 
@@ -667,21 +677,30 @@
 
     @api_exported('GET')
     def list(self, request):
-        """List Nodes visible to the user, optionally filtered by id."""
-        match_ids = request.GET.getlist('id')
-        if match_ids == []:
-            match_ids = None
+        """List Nodes visible to the user, optionally filtered by criteria.
+
+        :param mac_addresses: An optional list of MAC addresses.  Only
+            nodes with matching MAC addresses will be returned.
+        :type mac_addresses: iterable
+        :param id: An optional list of system ids.  Only nodes with
+            matching system ids will be returned.
+        :type id: iterable
+        """
+        # Get filters from request.
+        match_ids = get_optional_list(request.GET, 'id')
+        match_macs = get_optional_list(request.GET, 'mac_addresses')
+        # Fetch nodes and apply filters.
         nodes = Node.objects.get_nodes(
             request.user, NODE_PERMISSION.VIEW, ids=match_ids)
+        if match_macs is not None:
+            nodes = nodes.filter(macaddress__mac_address__in=match_macs)
         return nodes.order_by('id')
 
     @api_exported('GET')
     def list_allocated(self, request):
         """Fetch Nodes that were allocated to the User/oauth token."""
         token = get_oauth_token(request)
-        match_ids = request.GET.getlist('id')
-        if match_ids == []:
-            match_ids = None
+        match_ids = get_optional_list(request.GET, 'id')
         nodes = Node.objects.get_allocated_visible_nodes(token, match_ids)
         return nodes.order_by('id')
 

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-07-12 12:33:57 +0000
+++ src/maasserver/tests/test_api.py	2012-07-23 10:39:24 +0000
@@ -1393,6 +1393,20 @@
         self.assertItemsEqual(
             [existing_id], extract_system_ids(parsed_result))
 
+    def test_GET_list_with_macs_returns_matching_nodes(self):
+        # The "list" operation takes optional "mac_addresses" parameters.  Only
+        # nodes with matching MAC addresses will be returned.
+        macs = [factory.make_mac_address() for counter in range(3)]
+        matching_mac = macs[0].mac_address
+        matching_system_id = macs[0].node.system_id
+        response = self.client.get(self.get_uri('nodes/'), {
+            'op': 'list',
+            'mac_addresses': [matching_mac],
+        })
+        parsed_result = json.loads(response.content)
+        self.assertItemsEqual(
+            [matching_system_id], extract_system_ids(parsed_result))
+
     def test_GET_list_allocated_returns_only_allocated_with_user_token(self):
         # If the user's allocated nodes have different session tokens,
         # list_allocated should only return the nodes that have the