Merge lp:~mpontillo/maas/node-statistics-bug-1584926 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Node API usability enhancements.

 * Refactor APIs to support summaries and counts more easily.
   - All new APIs in this commit support a 'count' parameter, which will
     print the number of results instead of the results themselves.
   - All new APIs in this commit support a 'summary' parameter, which will
     print a subset of node details (enough to identify the node).
   - <nodes|machines|devices|region-controllers|rack-controllers> read:
     now supports the 'count' and 'summary' parameters.
 * Add new APIs to fetch machines using a "group by":
   - nodes by-type
   - machines by-status [numeric=<bool>]
   - machines by-user
   - machines by-zone
   - devices by-parent [by_system_id=<bool>]
 * Add new API to map node status IDs to labels:
   - machines get-status-descriptions

[Mike Pontillo (mpontillo)]

This branch isn't quite done, but I wanted to put it up for review since it's getting quite large.

I've done the tests for the "nodes list" enhancements and "nodes by-type" operation.

Testing still left to do:
   - machines by-status [numeric=<bool>]
   - machines by-user
   - machines by-zone
   - devices by-parent [by_system_id=<bool>]

I'd hate to write all these tests if there is an issue with the API design, though.

I've got some examples here of how the API works:

    https://paste.ubuntu.com/17370652/

[Blake Rouse (blake-rouse)]

The API looks good. This is a very large branch and you still haven't finished the tests. I would split it apart to help get quicker reviews.

[Mike Pontillo (mpontillo)]

Thanks for the comments. I'll fix these up and then break this branch apart into the nodes API changes, machines API changes, and then devices API change.

[lp:~mpontillo/maas/node-statistics-bug-1584926]

5140. By Mike Pontillo on 2016-06-16

Fix lint, review comments.

[MAAS Lander (maas-lander)]

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5140. By Mike Pontillo on 2016-06-16

Fix lint, review comments.

5139. By Mike Pontillo on 2016-06-16

More tests.

5138. By Mike Pontillo on 2016-06-16

Start adding test cases.

5137. By Mike Pontillo on 2016-06-15

Format imports.

5136. By Mike Pontillo on 2016-06-15

Add 'devices by-parent [by_system_id=<bool>'.

5135. By Mike Pontillo on 2016-06-15

Refactor result_summary() to be a little more defensive.

5134. By Mike Pontillo on 2016-06-15

Clean up API docs.

5133. By Mike Pontillo on 2016-06-15

Missed an occurance of ModelCountMixin.

5132. By Mike Pontillo on 2016-06-15

Remove ModelCountMixin. (using read with a parameter seems to make more sense)

5131. By Mike Pontillo on 2016-06-15

Refactor summary code to be more generic.

=== modified file 'src/maasserver/api/devices.py'
--- src/maasserver/api/devices.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/api/devices.py	2016-06-16 19:51:56 +0000
@@ -6,6 +6,7 @@
     "DevicesHandler",
     ]
 
+from formencode.validators import StringBool
 from maasserver.api.logger import maaslog
 from maasserver.api.nodes import (
     NodeHandler,
@@ -13,13 +14,17 @@
     OwnerDataMixin,
 )
 from maasserver.api.support import operation
+from maasserver.api.utils import get_optional_param
 from maasserver.enum import NODE_PERMISSION
 from maasserver.exceptions import MAASAPIValidationError
 from maasserver.forms import (
     DeviceForm,
     DeviceWithMACsForm,
 )
-from maasserver.models.node import Device
+from maasserver.models.node import (
+    Device,
+    Node,
+)
 from maasserver.utils.orm import reload_object
 from piston3.utils import rc
 
@@ -190,6 +195,34 @@
         else:
             raise MAASAPIValidationError(form.errors)
 
+    @operation(idempotent=True)
+    def by_parent(self, request):
+        """Fetches the list of devices by parent.
+
+        Optional arguments:
+        summary: If True, prints a summary of each node instead of the complete
+        object.
+        count: If True, prints a count of the number of nodes allocated to each
+        user instead of any node details.
+        by_system_id: If True, prints the parent node system_id values instead
+        of hostnames.
+
+        Also accepts any filter parameter the 'read' operation accepts.
+        """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        by_system_id = get_optional_param(
+            request.GET, 'by_system_id', default=False, validator=StringBool)
+        return {
+            node.system_id if by_system_id else node.hostname:
+                self.prepare_results(
+                    self._query(request).filter(parent=node),
+                    count=count, summary=summary)
+            for node in Node.objects.filter(children__isnull=False)
+        }
+
     @classmethod
     def resource_uri(cls, *args, **kwargs):
         return ('devices_handler', [])

=== modified file 'src/maasserver/api/machines.py'
--- src/maasserver/api/machines.py	2016-06-10 16:43:12 +0000
+++ src/maasserver/api/machines.py	2016-06-16 19:51:56 +0000
@@ -45,6 +45,7 @@
 from maasserver.enum import (
     NODE_PERMISSION,
     NODE_STATUS,
+    NODE_STATUS_CHOICES,
 )
 from maasserver.exceptions import (
     MAASAPIBadRequest,
@@ -68,6 +69,8 @@
     Filesystem,
     Machine,
     RackController,
+    User,
+    Zone,
 )
 from maasserver.models.node import RELEASABLE_STATUSES
 from maasserver.node_constraint_filter_forms import AcquireNodeForm
@@ -1358,6 +1361,89 @@
                 rack.hostname, hostname),
             content_type=("text/plain; charset=%s" % settings.DEFAULT_CHARSET))
 
+    @operation(idempotent=True)
+    def get_status_descriptions(self, request):
+        """Returns the mapping of stauts IDs to status labels."""
+        return {choice[0]: choice[1] for choice in NODE_STATUS_CHOICES}
+
+    @operation(idempotent=True)
+    def by_user(self, request):
+        """Fetches the list of nodes allocated to each user.
+
+        Optional arguments:
+        summary: If True, prints a summary of each node instead of the complete
+        object.
+        count: If True, prints a count of the number of nodes allocated to each
+        user instead of any node details.
+
+        Also accepts any filter parameter the 'read' operation accepts.
+        """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        return {
+            user.username:
+                self.prepare_results(
+                    self._query(request).filter(owner=user),
+                    count=count, summary=summary)
+            for user in User.objects.filter(is_active=True)
+        }
+
+    @operation(idempotent=True)
+    def by_status(self, request):
+        """Fetches the list of nodes grouped by node status.
+
+        Optional arguments:
+        summary: If True, prints a summary of each node instead of the complete
+        object.
+        count: If True, prints a count of the number of nodes with each status
+        instead of any node details.
+        numeric: If True, prints numeric status values as the key instead of
+        friendly descriptions.
+
+        Also accepts any filter parameter the 'read' operation accepts.
+        """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        numeric = get_optional_param(
+            request.GET, 'numeric', default=False, validator=StringBool)
+        return {
+            # NODE_STATUS_CHOICES[0] will be the numeric value; [1] is the
+            # description.
+            status_choice[0 if numeric else 1]:
+                self.prepare_results(
+                    self._query(request).filter(status=status_choice[0]),
+                    count=count, summary=summary)
+            for status_choice in NODE_STATUS_CHOICES
+            }
+
+    @operation(idempotent=True)
+    def by_zone(self, request):
+        """Fetches the list of nodes grouped by zone.
+
+        Optional arguments:
+        summary: If True, prints a summary of each node instead of the complete
+        object.
+        count: If True, prints a count of the number of nodes in each zone
+        instead of any node details.
+
+        Also accepts any filter parameter the 'read' operation accepts.
+        """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        return {
+            zone.name:
+                self.prepare_results(
+                    self._query(request).filter(zone=zone),
+                    count=count, summary=summary)
+            for zone in Zone.objects.all()
+            }
+
     @classmethod
     def resource_uri(cls, *args, **kwargs):
         return ('machines_handler', [])

=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py	2016-05-24 13:50:54 +0000
+++ src/maasserver/api/nodes.py	2016-06-16 19:51:56 +0000
@@ -15,6 +15,7 @@
 import bson
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404
+from formencode.validators import StringBool
 from maasserver.api.support import (
     admin_method,
     AnonymousOperationsHandler,
@@ -47,7 +48,13 @@
     Node,
     OwnerData,
 )
-from maasserver.models.node import typecast_to_node_type
+from maasserver.models.node import (
+    Device,
+    Machine,
+    RackController,
+    RegionController,
+    typecast_to_node_type,
+)
 from maasserver.models.nodeprobeddetails import get_single_probed_details
 from piston3.utils import rc
 from provisioningserver.power.schema import UNKNOWN_POWER_TYPE
@@ -281,6 +288,21 @@
     anonymous = AnonNodesHandler
     base_model = Node
 
+    def prefetch(self, nodes):
+        nodes = nodes.prefetch_related('interface_set__node')
+        nodes = nodes.prefetch_related('interface_set__ip_addresses')
+        nodes = nodes.prefetch_related('tags')
+        nodes = nodes.prefetch_related('zone')
+        return nodes
+
+    def _query(self, request):
+        """Helper method to filter the list of machines.
+
+        Based on the request, the list will be filtered based on the currently
+        logged in user, and whatever search criteria are supplied.
+        """
+        return filtered_nodes_list_from_request(request, model=self.base_model)
+
     def read(self, request):
         """List Nodes visible to the user, optionally filtered by criteria.
 
@@ -311,27 +333,42 @@
         :param agent_name: An optional agent name.  Only nodes relating to the
             nodes with matching agent names will be returned.
         :type agent_name: unicode
+
+        :param summary: If True, prints a summary of each node instead of the
+            complete object.
+        :type summary: bool
+
+        :param count: If True, prints a count of nodes instead of any
+            node details.
+        :type count: bool
         """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        return self.prepare_results(
+            self._read(
+                request, summary=summary, count=count),
+            count=count, summary=summary)
 
-        if self.base_model == Node:
+    def _read(self, request, summary=False, count=False):
+        """List nodes visible to the user."""
+        if self.base_model == Node and not summary and not count:
             # Avoid circular dependencies
             from maasserver.api.devices import DevicesHandler
             from maasserver.api.machines import MachinesHandler
             from maasserver.api.rackcontrollers import RackControllersHandler
             nodes = list(chain(
-                DevicesHandler().read(request).order_by("id"),
-                MachinesHandler().read(request).order_by("id"),
-                RackControllersHandler().read(request).order_by("id"),
+                DevicesHandler().read(request),
+                MachinesHandler().read(request),
+                RackControllersHandler().read(request),
             ))
             return nodes
         else:
             nodes = filtered_nodes_list_from_request(request, self.base_model)
             # Prefetch related objects that are needed for rendering the
             # result.
-            nodes = nodes.prefetch_related('interface_set__node')
-            nodes = nodes.prefetch_related('interface_set__ip_addresses')
-            nodes = nodes.prefetch_related('tags')
-            nodes = nodes.prefetch_related('zone')
+            nodes = self.prefetch(nodes)
             return nodes.order_by('id')
 
     @admin_method
@@ -356,6 +393,46 @@
             raise MAASAPIValidationError(form.errors)
         form.save()
 
+    def summary(self, node):
+        """Returns a summary a node."""
+        return {
+            'system_id': node.system_id,
+            'hostname': node.hostname
+        }
+
+    @operation(idempotent=True)
+    def by_type(self, request):
+        """Prints a listing of nodes grouped by their type.
+
+        Optional arguments:
+        summary: If True, prints a summary of each node instead of the complete
+        object.
+        count: If True, prints a count of each node type instead of any
+        node details.
+
+        Also accepts any filter parameter the 'read' operation accepts.
+        """
+        summary = get_optional_param(
+            request.GET, 'summary', default=False, validator=StringBool)
+        count = get_optional_param(
+            request.GET, 'count', default=False, validator=StringBool)
+        label_to_model = {
+            "machines": Machine,
+            "devices": Device,
+            "rack-controllers": RackController,
+            "region-controllers": RegionController
+        }
+        if count:
+            # We don't have a definition for the displayed fields of a plain
+            # node, so only include that if it's a summary or a count.
+            label_to_model["nodes"] = Node
+        return {
+            label: self.prepare_results(
+                filtered_nodes_list_from_request(request, model=model),
+                count=count, summary=summary)
+            for label, model in label_to_model.items()
+        }
+
     @classmethod
     def resource_uri(cls, *args, **kwargs):
         return ('nodes_handler', [])

=== modified file 'src/maasserver/api/support.py'
--- src/maasserver/api/support.py	2016-05-20 14:31:34 +0000
+++ src/maasserver/api/support.py	2016-06-16 19:51:56 +0000
@@ -13,6 +13,7 @@
 from functools import wraps
 
 from django.core.exceptions import PermissionDenied
+from django.db.models.query import QuerySet
 from django.http import Http404
 from maasserver.api.doc import get_api_description_hash
 from maasserver.exceptions import MAASAPIBadRequest
@@ -260,6 +261,88 @@
         else:
             return function(self, request, *args, **kwargs)
 
+    def prefetch(self, query):
+        """Pre-fetches the specified query.
+
+        This method should pre-fetch anything anticipated to be recursively
+        queried for in the non-summary version of the output.
+
+        By default, does no prefetching.
+
+        Override this in a subclass to do prefetching in case result_summary()
+        in case non-summary output is requested.
+        """
+        return query
+
+    def summary(self, obj):
+        """Return a summarized version of the specified model object.
+
+        This method is intended to return the object in a condensed format,
+        with a minimal amount of useful identifying information.
+
+        Override this in a subclass to allow result_summary() to work
+        correctly.
+        """
+        raise NotImplementedError
+
+    def result_summary(self, result, summary=False, order_by='id'):
+        """Utility method to provide a shorter, summary output for the objects
+        in the specified query, if desired.
+
+        :param result: must be either a QuerySet, or an integer.
+            If an integer is passed in, it is simply returned. (This means
+            the user requested a count instead of the query results.)
+        :param summary: if True, returns a smaller object containing only
+            essential information, rather than the complete object.
+        :param order_by: optional field to order by (defaults to 'id').
+        """
+        if isinstance(result, int):
+            # The user chose to count the objects in the query, instead of
+            # returning the results of the query. So just give them the result.
+            return result
+        elif isinstance(result, QuerySet):
+            # If this is a QuerySet, we need to ensure we both prefetch related
+            # objects, and return it in a consistent order.
+            result = self.prefetch(result).order_by(order_by)
+            if not summary:
+                # Pass this through to Piston if we're not in summary mode.
+                return result
+            else:
+                # It would be nice if we could return a generator expression
+                # here, but that doesn't seem to be supported by Piston.
+                return [
+                    self.summary(obj) for obj in result
+                ]
+        else:
+            # We're not sure what this is. So just let Piston do whatever
+            # it was going to do with it.
+            return result
+
+    def query_or_count(self, filter, count=False):
+        """Returns either the results of the filter, or a count.
+
+        :param count: if True, calls count() on the specified filter and
+            returns that. Otherwise, returns the filter unchanged.
+        """
+        if count:
+            return filter.count()
+        else:
+            return filter
+
+    def prepare_results(self, query, count=False, summary=False):
+        """Prepares the result set for the specified query.
+
+        Calls the 'summary' and 'prefetch' methods on subclasses, as
+        appropriate.
+
+        :param count: If True, return a count of the objects that would be
+            returned, instead of the objects themselves.
+        :param summary: If True, return summarized objects, instead of the
+            objects themselves.
+        """
+        return self.result_summary(
+            self.query_or_count(query, count=count), summary=summary)
+
     @classmethod
     def decorate(cls, func):
         """Decorate all exported operations with the given function.

=== modified file 'src/maasserver/api/tests/test_machines.py'
--- src/maasserver/api/tests/test_machines.py	2016-06-09 23:39:53 +0000
+++ src/maasserver/api/tests/test_machines.py	2016-06-16 19:51:56 +0000
@@ -208,14 +208,49 @@
         machine1 = factory.make_Node()
         machine2 = factory.make_Node(
             status=NODE_STATUS.ALLOCATED, owner=self.user)
+        # This device should be ignored.
+        factory.make_Device()
         response = self.client.get(reverse('machines_handler'))
         parsed_result = json.loads(
             response.content.decode(settings.DEFAULT_CHARSET))
-
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertItemsEqual(
-            [machine1.system_id, machine2.system_id],
-            extract_system_ids(parsed_result))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertItemsEqual(
+            [machine1.system_id, machine2.system_id],
+            extract_system_ids(parsed_result))
+
+    def test_GET_lists_machines_with_summary(self):
+        # The api allows for fetching the list of Machines.
+        machine1 = factory.make_Node()
+        machine2 = factory.make_Node(
+            status=NODE_STATUS.ALLOCATED, owner=self.user)
+        # This device should be ignored.
+        factory.make_Device()
+        response = self.client.get(reverse('machines_handler'), {
+            'summary': True
+        })
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertItemsEqual(
+            [machine1.system_id, machine2.system_id],
+            extract_system_ids(parsed_result))
+        # Ensure each result is a summary (only contains two items)
+        self.expectThat(len(parsed_result[0]), Equals(2))
+
+    def test_GET_lists_machines_with_count(self):
+        # The api allows for fetching the list of Machines.
+        num_hosts = random.choice([3, 5, 7, 9])
+        for _ in range(num_hosts):
+            factory.make_Node()
+        # This device should be ignored.
+        factory.make_Device()
+        response = self.client.get(reverse('machines_handler'), {
+            'count': True
+        })
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.expectThat(parsed_result, Equals(num_hosts))
 
     def test_GET_machines_issues_constant_number_of_queries(self):
         # XXX: GavinPanella 2014-10-03 bug=1377335

=== modified file 'src/maasserver/api/tests/test_nodes.py'
--- src/maasserver/api/tests/test_nodes.py	2016-05-24 22:05:45 +0000
+++ src/maasserver/api/tests/test_nodes.py	2016-06-16 19:51:56 +0000
@@ -25,6 +25,7 @@
 from maasserver.utils import ignore_unused
 from maasserver.utils.orm import reload_object
 from maastesting.djangotestcase import count_queries
+from testtools.matchers import Equals
 
 
 class AnonymousIsRegisteredAPITest(APITestCase.ForAnonymous):
@@ -272,8 +273,109 @@
             extract_system_ids_from_nodes(node_list))
 
 
-class TestNodesAPI(APITestCase.ForUser):
-    """Tests for /api/2.0/nodes/."""
+class TestAdminNodesAPI(APITestCase.ForAdmin):
+    """Tests for /api/2.0/nodes/. (administrative users)"""
+
+    def test_GET_by_type_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        machine = factory.make_Node()
+        device = factory.make_Device()
+        regionrack = factory.make_RegionRackController()
+        rack = factory.make_RackController()
+        region = factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'summary': False,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        # Since we're testing as a normal user...
+        self.assertItemsEqual(
+            [
+                machine.system_id,
+            ],
+            extract_system_ids(result["machines"]))
+        self.assertItemsEqual(
+            [
+                device.system_id,
+            ],
+            extract_system_ids(result["devices"]))
+        self.assertItemsEqual(
+            [
+                regionrack.system_id,
+                region.system_id
+            ],
+            extract_system_ids(result["region-controllers"]))
+        self.assertItemsEqual(
+            [
+                regionrack.system_id,
+                rack.system_id,
+            ],
+            extract_system_ids(result["rack-controllers"]))
+
+    def test_GET_by_type_with_summary_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        machine = factory.make_Node()
+        device = factory.make_Device()
+        regionrack = factory.make_RegionRackController()
+        rack = factory.make_RackController()
+        region = factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'summary': False,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        # Since we're testing as a normal user...
+        self.assertItemsEqual(
+            [
+                machine.system_id,
+            ],
+            extract_system_ids(result["machines"]))
+        self.assertItemsEqual(
+            [
+                device.system_id,
+            ],
+            extract_system_ids(result["devices"]))
+        self.assertItemsEqual(
+            [
+                regionrack.system_id,
+                region.system_id
+            ],
+            extract_system_ids(result["region-controllers"]))
+        self.assertItemsEqual(
+            [
+                regionrack.system_id,
+                rack.system_id,
+            ],
+            extract_system_ids(result["rack-controllers"]))
+
+    def test_GET_by_type_with_count_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        factory.make_Node()
+        factory.make_Device()
+        factory.make_RegionRackController()
+        factory.make_RackController()
+        factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'count': True,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        # Since we're testing as an admin, we can see everything.
+        self.assertThat(result["nodes"], Equals(5))
+        self.assertThat(result["machines"], Equals(1))
+        self.assertThat(result["devices"], Equals(1))
+        self.assertThat(result["rack-controllers"], Equals(2))
+        self.assertThat(result["region-controllers"], Equals(2))
+
+
+class TestUserNodesAPI(APITestCase.ForUser):
+    """Tests for /api/2.0/nodes/. (normal users)"""
 
     def test_handler_path(self):
         self.assertEqual(
@@ -284,14 +386,139 @@
         node1 = factory.make_Node()
         node2 = factory.make_Node(
             status=NODE_STATUS.ALLOCATED, owner=self.user)
+        device = factory.make_Device()
         response = self.client.get(reverse('nodes_handler'))
         parsed_result = json.loads(
             response.content.decode(settings.DEFAULT_CHARSET))
-
-        self.assertEqual(http.client.OK, response.status_code)
-        self.assertItemsEqual(
-            [node1.system_id, node2.system_id],
-            extract_system_ids(parsed_result))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertItemsEqual(
+            [node1.system_id, node2.system_id, device.system_id],
+            extract_system_ids(parsed_result))
+
+    def test_GET_lists_nodes_with_summary(self):
+        # The api allows for fetching the list of Machines.
+        machine1 = factory.make_Node()
+        machine2 = factory.make_Node(
+            status=NODE_STATUS.ALLOCATED, owner=self.user)
+        device = factory.make_Device()
+        response = self.client.get(reverse('nodes_handler'), {
+            'summary': True
+        })
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertItemsEqual(
+            [machine1.system_id, machine2.system_id, device.system_id],
+            extract_system_ids(parsed_result))
+        # Ensure each result is a summary (only contains two items)
+        self.expectThat(len(parsed_result[0]), Equals(2))
+
+    def test_GET_lists_nodes_with_count(self):
+        # The api allows for fetching the list of Machines.
+        num_hosts = random.choice([3, 5, 7, 9])
+        for _ in range(num_hosts):
+            factory.make_Node()
+            factory.make_Device()
+        response = self.client.get(reverse('nodes_handler'), {
+            'count': True
+        })
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.expectThat(parsed_result, Equals(num_hosts * 2))
+
+    def test_GET_by_type_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        machine = factory.make_Node()
+        device = factory.make_Device()
+        # These should be ignored since we're a normal user.
+        factory.make_RegionRackController()
+        factory.make_RackController()
+        factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'summary': False,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertItemsEqual(
+            [
+                machine.system_id,
+            ],
+            extract_system_ids(result["machines"]))
+        self.assertItemsEqual(
+            [
+                device.system_id,
+            ],
+            extract_system_ids(result["devices"]))
+        # Since we're testing as a normal user, we won't be able to see
+        # controllers.
+        self.assertItemsEqual(
+            [
+            ],
+            extract_system_ids(result["region-controllers"]))
+        self.assertItemsEqual(
+            [
+            ],
+            extract_system_ids(result["rack-controllers"]))
+
+    def test_GET_by_type_with_summary_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        machine = factory.make_Node()
+        device = factory.make_Device()
+        factory.make_RegionRackController()
+        factory.make_RackController()
+        factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'summary': False,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        # Since we're testing as a normal user...
+        self.assertItemsEqual(
+            [
+                machine.system_id,
+            ],
+            extract_system_ids(result["machines"]))
+        self.assertItemsEqual(
+            [
+                device.system_id,
+            ],
+            extract_system_ids(result["devices"]))
+        self.assertItemsEqual(
+            [
+            ],
+            extract_system_ids(result["region-controllers"]))
+        self.assertItemsEqual(
+            [
+            ],
+            extract_system_ids(result["rack-controllers"]))
+
+    def test_GET_by_type_with_count_groups_by_type(self):
+        # The api allows for fetching the list of Machines.
+        factory.make_Node()
+        factory.make_Device()
+        factory.make_RegionRackController()
+        factory.make_RackController()
+        factory.make_RegionController()
+        response = self.client.get(reverse('nodes_handler'), {
+            'op': "by_type",
+            'count': True,
+        })
+        result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        self.assertEqual(http.client.OK, response.status_code)
+        # Since we're testing as a normal user, they can only see the nodes
+        # and devices.
+        self.assertThat(result["nodes"], Equals(2))
+        self.assertThat(result["machines"], Equals(1))
+        self.assertThat(result["devices"], Equals(1))
+        self.assertThat(result["rack-controllers"], Equals(0))
+        self.assertThat(result["region-controllers"], Equals(0))
 
     def create_nodes(self, nodegroup, nb):
         for _ in range(nb):

=== modified file 'src/maasserver/testing/testclient.py'
--- src/maasserver/testing/testclient.py	2016-05-24 14:53:56 +0000
+++ src/maasserver/testing/testclient.py	2016-06-16 19:51:56 +0000
@@ -30,7 +30,7 @@
     """A derivative of Django's test client specially for MAAS.
 
     This ensures that requests are performed in a transaction, and that
-    post-commit hooks are alway fired or reset.
+    post-commit hooks are always fired or reset.
 
     It also permits logging-in using just a user object. A password will be
     configured and used to log-in.