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

Proposed by Mike Pontillo on 2016-06-15
Status: Rejected
Rejected by: MAAS Lander on 2017-06-22
Proposed branch: lp:~mpontillo/maas/node-statistics-bug-1584926
Merge into: lp:maas/trunk
Diff against target: 765 lines (+564/-23)
7 files modified
src/maasserver/api/devices.py (+34/-1)
src/maasserver/api/machines.py (+86/-0)
src/maasserver/api/nodes.py (+86/-9)
src/maasserver/api/support.py (+83/-0)
src/maasserver/api/tests/test_machines.py (+40/-5)
src/maasserver/api/tests/test_nodes.py (+234/-7)
src/maasserver/testing/testclient.py (+1/-1)
To merge this branch: bzr merge lp:~mpontillo/maas/node-statistics-bug-1584926
Reviewer Review Type Date Requested Status
MAAS Maintainers 2016-06-15 Pending
Review via email: mp+297430@code.launchpad.net

Commit message

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

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

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) wrote :

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.

Jeffrey C Jones (trapnine) :
Mike Pontillo (mpontillo) wrote :

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.

5140. By Mike Pontillo on 2016-06-16

Fix lint, review comments.

MAAS Lander (maas-lander) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/devices.py'
2--- src/maasserver/api/devices.py 2016-05-12 19:07:37 +0000
3+++ src/maasserver/api/devices.py 2016-06-16 19:51:56 +0000
4@@ -6,6 +6,7 @@
5 "DevicesHandler",
6 ]
7
8+from formencode.validators import StringBool
9 from maasserver.api.logger import maaslog
10 from maasserver.api.nodes import (
11 NodeHandler,
12@@ -13,13 +14,17 @@
13 OwnerDataMixin,
14 )
15 from maasserver.api.support import operation
16+from maasserver.api.utils import get_optional_param
17 from maasserver.enum import NODE_PERMISSION
18 from maasserver.exceptions import MAASAPIValidationError
19 from maasserver.forms import (
20 DeviceForm,
21 DeviceWithMACsForm,
22 )
23-from maasserver.models.node import Device
24+from maasserver.models.node import (
25+ Device,
26+ Node,
27+)
28 from maasserver.utils.orm import reload_object
29 from piston3.utils import rc
30
31@@ -190,6 +195,34 @@
32 else:
33 raise MAASAPIValidationError(form.errors)
34
35+ @operation(idempotent=True)
36+ def by_parent(self, request):
37+ """Fetches the list of devices by parent.
38+
39+ Optional arguments:
40+ summary: If True, prints a summary of each node instead of the complete
41+ object.
42+ count: If True, prints a count of the number of nodes allocated to each
43+ user instead of any node details.
44+ by_system_id: If True, prints the parent node system_id values instead
45+ of hostnames.
46+
47+ Also accepts any filter parameter the 'read' operation accepts.
48+ """
49+ summary = get_optional_param(
50+ request.GET, 'summary', default=False, validator=StringBool)
51+ count = get_optional_param(
52+ request.GET, 'count', default=False, validator=StringBool)
53+ by_system_id = get_optional_param(
54+ request.GET, 'by_system_id', default=False, validator=StringBool)
55+ return {
56+ node.system_id if by_system_id else node.hostname:
57+ self.prepare_results(
58+ self._query(request).filter(parent=node),
59+ count=count, summary=summary)
60+ for node in Node.objects.filter(children__isnull=False)
61+ }
62+
63 @classmethod
64 def resource_uri(cls, *args, **kwargs):
65 return ('devices_handler', [])
66
67=== modified file 'src/maasserver/api/machines.py'
68--- src/maasserver/api/machines.py 2016-06-10 16:43:12 +0000
69+++ src/maasserver/api/machines.py 2016-06-16 19:51:56 +0000
70@@ -45,6 +45,7 @@
71 from maasserver.enum import (
72 NODE_PERMISSION,
73 NODE_STATUS,
74+ NODE_STATUS_CHOICES,
75 )
76 from maasserver.exceptions import (
77 MAASAPIBadRequest,
78@@ -68,6 +69,8 @@
79 Filesystem,
80 Machine,
81 RackController,
82+ User,
83+ Zone,
84 )
85 from maasserver.models.node import RELEASABLE_STATUSES
86 from maasserver.node_constraint_filter_forms import AcquireNodeForm
87@@ -1358,6 +1361,89 @@
88 rack.hostname, hostname),
89 content_type=("text/plain; charset=%s" % settings.DEFAULT_CHARSET))
90
91+ @operation(idempotent=True)
92+ def get_status_descriptions(self, request):
93+ """Returns the mapping of stauts IDs to status labels."""
94+ return {choice[0]: choice[1] for choice in NODE_STATUS_CHOICES}
95+
96+ @operation(idempotent=True)
97+ def by_user(self, request):
98+ """Fetches the list of nodes allocated to each user.
99+
100+ Optional arguments:
101+ summary: If True, prints a summary of each node instead of the complete
102+ object.
103+ count: If True, prints a count of the number of nodes allocated to each
104+ user instead of any node details.
105+
106+ Also accepts any filter parameter the 'read' operation accepts.
107+ """
108+ summary = get_optional_param(
109+ request.GET, 'summary', default=False, validator=StringBool)
110+ count = get_optional_param(
111+ request.GET, 'count', default=False, validator=StringBool)
112+ return {
113+ user.username:
114+ self.prepare_results(
115+ self._query(request).filter(owner=user),
116+ count=count, summary=summary)
117+ for user in User.objects.filter(is_active=True)
118+ }
119+
120+ @operation(idempotent=True)
121+ def by_status(self, request):
122+ """Fetches the list of nodes grouped by node status.
123+
124+ Optional arguments:
125+ summary: If True, prints a summary of each node instead of the complete
126+ object.
127+ count: If True, prints a count of the number of nodes with each status
128+ instead of any node details.
129+ numeric: If True, prints numeric status values as the key instead of
130+ friendly descriptions.
131+
132+ Also accepts any filter parameter the 'read' operation accepts.
133+ """
134+ summary = get_optional_param(
135+ request.GET, 'summary', default=False, validator=StringBool)
136+ count = get_optional_param(
137+ request.GET, 'count', default=False, validator=StringBool)
138+ numeric = get_optional_param(
139+ request.GET, 'numeric', default=False, validator=StringBool)
140+ return {
141+ # NODE_STATUS_CHOICES[0] will be the numeric value; [1] is the
142+ # description.
143+ status_choice[0 if numeric else 1]:
144+ self.prepare_results(
145+ self._query(request).filter(status=status_choice[0]),
146+ count=count, summary=summary)
147+ for status_choice in NODE_STATUS_CHOICES
148+ }
149+
150+ @operation(idempotent=True)
151+ def by_zone(self, request):
152+ """Fetches the list of nodes grouped by zone.
153+
154+ Optional arguments:
155+ summary: If True, prints a summary of each node instead of the complete
156+ object.
157+ count: If True, prints a count of the number of nodes in each zone
158+ instead of any node details.
159+
160+ Also accepts any filter parameter the 'read' operation accepts.
161+ """
162+ summary = get_optional_param(
163+ request.GET, 'summary', default=False, validator=StringBool)
164+ count = get_optional_param(
165+ request.GET, 'count', default=False, validator=StringBool)
166+ return {
167+ zone.name:
168+ self.prepare_results(
169+ self._query(request).filter(zone=zone),
170+ count=count, summary=summary)
171+ for zone in Zone.objects.all()
172+ }
173+
174 @classmethod
175 def resource_uri(cls, *args, **kwargs):
176 return ('machines_handler', [])
177
178=== modified file 'src/maasserver/api/nodes.py'
179--- src/maasserver/api/nodes.py 2016-05-24 13:50:54 +0000
180+++ src/maasserver/api/nodes.py 2016-06-16 19:51:56 +0000
181@@ -15,6 +15,7 @@
182 import bson
183 from django.http import HttpResponse
184 from django.shortcuts import get_object_or_404
185+from formencode.validators import StringBool
186 from maasserver.api.support import (
187 admin_method,
188 AnonymousOperationsHandler,
189@@ -47,7 +48,13 @@
190 Node,
191 OwnerData,
192 )
193-from maasserver.models.node import typecast_to_node_type
194+from maasserver.models.node import (
195+ Device,
196+ Machine,
197+ RackController,
198+ RegionController,
199+ typecast_to_node_type,
200+)
201 from maasserver.models.nodeprobeddetails import get_single_probed_details
202 from piston3.utils import rc
203 from provisioningserver.power.schema import UNKNOWN_POWER_TYPE
204@@ -281,6 +288,21 @@
205 anonymous = AnonNodesHandler
206 base_model = Node
207
208+ def prefetch(self, nodes):
209+ nodes = nodes.prefetch_related('interface_set__node')
210+ nodes = nodes.prefetch_related('interface_set__ip_addresses')
211+ nodes = nodes.prefetch_related('tags')
212+ nodes = nodes.prefetch_related('zone')
213+ return nodes
214+
215+ def _query(self, request):
216+ """Helper method to filter the list of machines.
217+
218+ Based on the request, the list will be filtered based on the currently
219+ logged in user, and whatever search criteria are supplied.
220+ """
221+ return filtered_nodes_list_from_request(request, model=self.base_model)
222+
223 def read(self, request):
224 """List Nodes visible to the user, optionally filtered by criteria.
225
226@@ -311,27 +333,42 @@
227 :param agent_name: An optional agent name. Only nodes relating to the
228 nodes with matching agent names will be returned.
229 :type agent_name: unicode
230+
231+ :param summary: If True, prints a summary of each node instead of the
232+ complete object.
233+ :type summary: bool
234+
235+ :param count: If True, prints a count of nodes instead of any
236+ node details.
237+ :type count: bool
238 """
239+ summary = get_optional_param(
240+ request.GET, 'summary', default=False, validator=StringBool)
241+ count = get_optional_param(
242+ request.GET, 'count', default=False, validator=StringBool)
243+ return self.prepare_results(
244+ self._read(
245+ request, summary=summary, count=count),
246+ count=count, summary=summary)
247
248- if self.base_model == Node:
249+ def _read(self, request, summary=False, count=False):
250+ """List nodes visible to the user."""
251+ if self.base_model == Node and not summary and not count:
252 # Avoid circular dependencies
253 from maasserver.api.devices import DevicesHandler
254 from maasserver.api.machines import MachinesHandler
255 from maasserver.api.rackcontrollers import RackControllersHandler
256 nodes = list(chain(
257- DevicesHandler().read(request).order_by("id"),
258- MachinesHandler().read(request).order_by("id"),
259- RackControllersHandler().read(request).order_by("id"),
260+ DevicesHandler().read(request),
261+ MachinesHandler().read(request),
262+ RackControllersHandler().read(request),
263 ))
264 return nodes
265 else:
266 nodes = filtered_nodes_list_from_request(request, self.base_model)
267 # Prefetch related objects that are needed for rendering the
268 # result.
269- nodes = nodes.prefetch_related('interface_set__node')
270- nodes = nodes.prefetch_related('interface_set__ip_addresses')
271- nodes = nodes.prefetch_related('tags')
272- nodes = nodes.prefetch_related('zone')
273+ nodes = self.prefetch(nodes)
274 return nodes.order_by('id')
275
276 @admin_method
277@@ -356,6 +393,46 @@
278 raise MAASAPIValidationError(form.errors)
279 form.save()
280
281+ def summary(self, node):
282+ """Returns a summary a node."""
283+ return {
284+ 'system_id': node.system_id,
285+ 'hostname': node.hostname
286+ }
287+
288+ @operation(idempotent=True)
289+ def by_type(self, request):
290+ """Prints a listing of nodes grouped by their type.
291+
292+ Optional arguments:
293+ summary: If True, prints a summary of each node instead of the complete
294+ object.
295+ count: If True, prints a count of each node type instead of any
296+ node details.
297+
298+ Also accepts any filter parameter the 'read' operation accepts.
299+ """
300+ summary = get_optional_param(
301+ request.GET, 'summary', default=False, validator=StringBool)
302+ count = get_optional_param(
303+ request.GET, 'count', default=False, validator=StringBool)
304+ label_to_model = {
305+ "machines": Machine,
306+ "devices": Device,
307+ "rack-controllers": RackController,
308+ "region-controllers": RegionController
309+ }
310+ if count:
311+ # We don't have a definition for the displayed fields of a plain
312+ # node, so only include that if it's a summary or a count.
313+ label_to_model["nodes"] = Node
314+ return {
315+ label: self.prepare_results(
316+ filtered_nodes_list_from_request(request, model=model),
317+ count=count, summary=summary)
318+ for label, model in label_to_model.items()
319+ }
320+
321 @classmethod
322 def resource_uri(cls, *args, **kwargs):
323 return ('nodes_handler', [])
324
325=== modified file 'src/maasserver/api/support.py'
326--- src/maasserver/api/support.py 2016-05-20 14:31:34 +0000
327+++ src/maasserver/api/support.py 2016-06-16 19:51:56 +0000
328@@ -13,6 +13,7 @@
329 from functools import wraps
330
331 from django.core.exceptions import PermissionDenied
332+from django.db.models.query import QuerySet
333 from django.http import Http404
334 from maasserver.api.doc import get_api_description_hash
335 from maasserver.exceptions import MAASAPIBadRequest
336@@ -260,6 +261,88 @@
337 else:
338 return function(self, request, *args, **kwargs)
339
340+ def prefetch(self, query):
341+ """Pre-fetches the specified query.
342+
343+ This method should pre-fetch anything anticipated to be recursively
344+ queried for in the non-summary version of the output.
345+
346+ By default, does no prefetching.
347+
348+ Override this in a subclass to do prefetching in case result_summary()
349+ in case non-summary output is requested.
350+ """
351+ return query
352+
353+ def summary(self, obj):
354+ """Return a summarized version of the specified model object.
355+
356+ This method is intended to return the object in a condensed format,
357+ with a minimal amount of useful identifying information.
358+
359+ Override this in a subclass to allow result_summary() to work
360+ correctly.
361+ """
362+ raise NotImplementedError
363+
364+ def result_summary(self, result, summary=False, order_by='id'):
365+ """Utility method to provide a shorter, summary output for the objects
366+ in the specified query, if desired.
367+
368+ :param result: must be either a QuerySet, or an integer.
369+ If an integer is passed in, it is simply returned. (This means
370+ the user requested a count instead of the query results.)
371+ :param summary: if True, returns a smaller object containing only
372+ essential information, rather than the complete object.
373+ :param order_by: optional field to order by (defaults to 'id').
374+ """
375+ if isinstance(result, int):
376+ # The user chose to count the objects in the query, instead of
377+ # returning the results of the query. So just give them the result.
378+ return result
379+ elif isinstance(result, QuerySet):
380+ # If this is a QuerySet, we need to ensure we both prefetch related
381+ # objects, and return it in a consistent order.
382+ result = self.prefetch(result).order_by(order_by)
383+ if not summary:
384+ # Pass this through to Piston if we're not in summary mode.
385+ return result
386+ else:
387+ # It would be nice if we could return a generator expression
388+ # here, but that doesn't seem to be supported by Piston.
389+ return [
390+ self.summary(obj) for obj in result
391+ ]
392+ else:
393+ # We're not sure what this is. So just let Piston do whatever
394+ # it was going to do with it.
395+ return result
396+
397+ def query_or_count(self, filter, count=False):
398+ """Returns either the results of the filter, or a count.
399+
400+ :param count: if True, calls count() on the specified filter and
401+ returns that. Otherwise, returns the filter unchanged.
402+ """
403+ if count:
404+ return filter.count()
405+ else:
406+ return filter
407+
408+ def prepare_results(self, query, count=False, summary=False):
409+ """Prepares the result set for the specified query.
410+
411+ Calls the 'summary' and 'prefetch' methods on subclasses, as
412+ appropriate.
413+
414+ :param count: If True, return a count of the objects that would be
415+ returned, instead of the objects themselves.
416+ :param summary: If True, return summarized objects, instead of the
417+ objects themselves.
418+ """
419+ return self.result_summary(
420+ self.query_or_count(query, count=count), summary=summary)
421+
422 @classmethod
423 def decorate(cls, func):
424 """Decorate all exported operations with the given function.
425
426=== modified file 'src/maasserver/api/tests/test_machines.py'
427--- src/maasserver/api/tests/test_machines.py 2016-06-09 23:39:53 +0000
428+++ src/maasserver/api/tests/test_machines.py 2016-06-16 19:51:56 +0000
429@@ -208,14 +208,49 @@
430 machine1 = factory.make_Node()
431 machine2 = factory.make_Node(
432 status=NODE_STATUS.ALLOCATED, owner=self.user)
433+ # This device should be ignored.
434+ factory.make_Device()
435 response = self.client.get(reverse('machines_handler'))
436 parsed_result = json.loads(
437 response.content.decode(settings.DEFAULT_CHARSET))
438-
439- self.assertEqual(http.client.OK, response.status_code)
440- self.assertItemsEqual(
441- [machine1.system_id, machine2.system_id],
442- extract_system_ids(parsed_result))
443+ self.assertEqual(http.client.OK, response.status_code)
444+ self.assertItemsEqual(
445+ [machine1.system_id, machine2.system_id],
446+ extract_system_ids(parsed_result))
447+
448+ def test_GET_lists_machines_with_summary(self):
449+ # The api allows for fetching the list of Machines.
450+ machine1 = factory.make_Node()
451+ machine2 = factory.make_Node(
452+ status=NODE_STATUS.ALLOCATED, owner=self.user)
453+ # This device should be ignored.
454+ factory.make_Device()
455+ response = self.client.get(reverse('machines_handler'), {
456+ 'summary': True
457+ })
458+ parsed_result = json.loads(
459+ response.content.decode(settings.DEFAULT_CHARSET))
460+ self.assertEqual(http.client.OK, response.status_code)
461+ self.assertItemsEqual(
462+ [machine1.system_id, machine2.system_id],
463+ extract_system_ids(parsed_result))
464+ # Ensure each result is a summary (only contains two items)
465+ self.expectThat(len(parsed_result[0]), Equals(2))
466+
467+ def test_GET_lists_machines_with_count(self):
468+ # The api allows for fetching the list of Machines.
469+ num_hosts = random.choice([3, 5, 7, 9])
470+ for _ in range(num_hosts):
471+ factory.make_Node()
472+ # This device should be ignored.
473+ factory.make_Device()
474+ response = self.client.get(reverse('machines_handler'), {
475+ 'count': True
476+ })
477+ parsed_result = json.loads(
478+ response.content.decode(settings.DEFAULT_CHARSET))
479+ self.assertEqual(http.client.OK, response.status_code)
480+ self.expectThat(parsed_result, Equals(num_hosts))
481
482 def test_GET_machines_issues_constant_number_of_queries(self):
483 # XXX: GavinPanella 2014-10-03 bug=1377335
484
485=== modified file 'src/maasserver/api/tests/test_nodes.py'
486--- src/maasserver/api/tests/test_nodes.py 2016-05-24 22:05:45 +0000
487+++ src/maasserver/api/tests/test_nodes.py 2016-06-16 19:51:56 +0000
488@@ -25,6 +25,7 @@
489 from maasserver.utils import ignore_unused
490 from maasserver.utils.orm import reload_object
491 from maastesting.djangotestcase import count_queries
492+from testtools.matchers import Equals
493
494
495 class AnonymousIsRegisteredAPITest(APITestCase.ForAnonymous):
496@@ -272,8 +273,109 @@
497 extract_system_ids_from_nodes(node_list))
498
499
500-class TestNodesAPI(APITestCase.ForUser):
501- """Tests for /api/2.0/nodes/."""
502+class TestAdminNodesAPI(APITestCase.ForAdmin):
503+ """Tests for /api/2.0/nodes/. (administrative users)"""
504+
505+ def test_GET_by_type_groups_by_type(self):
506+ # The api allows for fetching the list of Machines.
507+ machine = factory.make_Node()
508+ device = factory.make_Device()
509+ regionrack = factory.make_RegionRackController()
510+ rack = factory.make_RackController()
511+ region = factory.make_RegionController()
512+ response = self.client.get(reverse('nodes_handler'), {
513+ 'op': "by_type",
514+ 'summary': False,
515+ })
516+ result = json.loads(
517+ response.content.decode(settings.DEFAULT_CHARSET))
518+ self.assertEqual(http.client.OK, response.status_code)
519+ # Since we're testing as a normal user...
520+ self.assertItemsEqual(
521+ [
522+ machine.system_id,
523+ ],
524+ extract_system_ids(result["machines"]))
525+ self.assertItemsEqual(
526+ [
527+ device.system_id,
528+ ],
529+ extract_system_ids(result["devices"]))
530+ self.assertItemsEqual(
531+ [
532+ regionrack.system_id,
533+ region.system_id
534+ ],
535+ extract_system_ids(result["region-controllers"]))
536+ self.assertItemsEqual(
537+ [
538+ regionrack.system_id,
539+ rack.system_id,
540+ ],
541+ extract_system_ids(result["rack-controllers"]))
542+
543+ def test_GET_by_type_with_summary_groups_by_type(self):
544+ # The api allows for fetching the list of Machines.
545+ machine = factory.make_Node()
546+ device = factory.make_Device()
547+ regionrack = factory.make_RegionRackController()
548+ rack = factory.make_RackController()
549+ region = factory.make_RegionController()
550+ response = self.client.get(reverse('nodes_handler'), {
551+ 'op': "by_type",
552+ 'summary': False,
553+ })
554+ result = json.loads(
555+ response.content.decode(settings.DEFAULT_CHARSET))
556+ self.assertEqual(http.client.OK, response.status_code)
557+ # Since we're testing as a normal user...
558+ self.assertItemsEqual(
559+ [
560+ machine.system_id,
561+ ],
562+ extract_system_ids(result["machines"]))
563+ self.assertItemsEqual(
564+ [
565+ device.system_id,
566+ ],
567+ extract_system_ids(result["devices"]))
568+ self.assertItemsEqual(
569+ [
570+ regionrack.system_id,
571+ region.system_id
572+ ],
573+ extract_system_ids(result["region-controllers"]))
574+ self.assertItemsEqual(
575+ [
576+ regionrack.system_id,
577+ rack.system_id,
578+ ],
579+ extract_system_ids(result["rack-controllers"]))
580+
581+ def test_GET_by_type_with_count_groups_by_type(self):
582+ # The api allows for fetching the list of Machines.
583+ factory.make_Node()
584+ factory.make_Device()
585+ factory.make_RegionRackController()
586+ factory.make_RackController()
587+ factory.make_RegionController()
588+ response = self.client.get(reverse('nodes_handler'), {
589+ 'op': "by_type",
590+ 'count': True,
591+ })
592+ result = json.loads(
593+ response.content.decode(settings.DEFAULT_CHARSET))
594+ self.assertEqual(http.client.OK, response.status_code)
595+ # Since we're testing as an admin, we can see everything.
596+ self.assertThat(result["nodes"], Equals(5))
597+ self.assertThat(result["machines"], Equals(1))
598+ self.assertThat(result["devices"], Equals(1))
599+ self.assertThat(result["rack-controllers"], Equals(2))
600+ self.assertThat(result["region-controllers"], Equals(2))
601+
602+
603+class TestUserNodesAPI(APITestCase.ForUser):
604+ """Tests for /api/2.0/nodes/. (normal users)"""
605
606 def test_handler_path(self):
607 self.assertEqual(
608@@ -284,14 +386,139 @@
609 node1 = factory.make_Node()
610 node2 = factory.make_Node(
611 status=NODE_STATUS.ALLOCATED, owner=self.user)
612+ device = factory.make_Device()
613 response = self.client.get(reverse('nodes_handler'))
614 parsed_result = json.loads(
615 response.content.decode(settings.DEFAULT_CHARSET))
616-
617- self.assertEqual(http.client.OK, response.status_code)
618- self.assertItemsEqual(
619- [node1.system_id, node2.system_id],
620- extract_system_ids(parsed_result))
621+ self.assertEqual(http.client.OK, response.status_code)
622+ self.assertItemsEqual(
623+ [node1.system_id, node2.system_id, device.system_id],
624+ extract_system_ids(parsed_result))
625+
626+ def test_GET_lists_nodes_with_summary(self):
627+ # The api allows for fetching the list of Machines.
628+ machine1 = factory.make_Node()
629+ machine2 = factory.make_Node(
630+ status=NODE_STATUS.ALLOCATED, owner=self.user)
631+ device = factory.make_Device()
632+ response = self.client.get(reverse('nodes_handler'), {
633+ 'summary': True
634+ })
635+ parsed_result = json.loads(
636+ response.content.decode(settings.DEFAULT_CHARSET))
637+
638+ self.assertEqual(http.client.OK, response.status_code)
639+ self.assertItemsEqual(
640+ [machine1.system_id, machine2.system_id, device.system_id],
641+ extract_system_ids(parsed_result))
642+ # Ensure each result is a summary (only contains two items)
643+ self.expectThat(len(parsed_result[0]), Equals(2))
644+
645+ def test_GET_lists_nodes_with_count(self):
646+ # The api allows for fetching the list of Machines.
647+ num_hosts = random.choice([3, 5, 7, 9])
648+ for _ in range(num_hosts):
649+ factory.make_Node()
650+ factory.make_Device()
651+ response = self.client.get(reverse('nodes_handler'), {
652+ 'count': True
653+ })
654+ parsed_result = json.loads(
655+ response.content.decode(settings.DEFAULT_CHARSET))
656+ self.assertEqual(http.client.OK, response.status_code)
657+ self.expectThat(parsed_result, Equals(num_hosts * 2))
658+
659+ def test_GET_by_type_groups_by_type(self):
660+ # The api allows for fetching the list of Machines.
661+ machine = factory.make_Node()
662+ device = factory.make_Device()
663+ # These should be ignored since we're a normal user.
664+ factory.make_RegionRackController()
665+ factory.make_RackController()
666+ factory.make_RegionController()
667+ response = self.client.get(reverse('nodes_handler'), {
668+ 'op': "by_type",
669+ 'summary': False,
670+ })
671+ result = json.loads(
672+ response.content.decode(settings.DEFAULT_CHARSET))
673+ self.assertEqual(http.client.OK, response.status_code)
674+ self.assertItemsEqual(
675+ [
676+ machine.system_id,
677+ ],
678+ extract_system_ids(result["machines"]))
679+ self.assertItemsEqual(
680+ [
681+ device.system_id,
682+ ],
683+ extract_system_ids(result["devices"]))
684+ # Since we're testing as a normal user, we won't be able to see
685+ # controllers.
686+ self.assertItemsEqual(
687+ [
688+ ],
689+ extract_system_ids(result["region-controllers"]))
690+ self.assertItemsEqual(
691+ [
692+ ],
693+ extract_system_ids(result["rack-controllers"]))
694+
695+ def test_GET_by_type_with_summary_groups_by_type(self):
696+ # The api allows for fetching the list of Machines.
697+ machine = factory.make_Node()
698+ device = factory.make_Device()
699+ factory.make_RegionRackController()
700+ factory.make_RackController()
701+ factory.make_RegionController()
702+ response = self.client.get(reverse('nodes_handler'), {
703+ 'op': "by_type",
704+ 'summary': False,
705+ })
706+ result = json.loads(
707+ response.content.decode(settings.DEFAULT_CHARSET))
708+ self.assertEqual(http.client.OK, response.status_code)
709+ # Since we're testing as a normal user...
710+ self.assertItemsEqual(
711+ [
712+ machine.system_id,
713+ ],
714+ extract_system_ids(result["machines"]))
715+ self.assertItemsEqual(
716+ [
717+ device.system_id,
718+ ],
719+ extract_system_ids(result["devices"]))
720+ self.assertItemsEqual(
721+ [
722+ ],
723+ extract_system_ids(result["region-controllers"]))
724+ self.assertItemsEqual(
725+ [
726+ ],
727+ extract_system_ids(result["rack-controllers"]))
728+
729+ def test_GET_by_type_with_count_groups_by_type(self):
730+ # The api allows for fetching the list of Machines.
731+ factory.make_Node()
732+ factory.make_Device()
733+ factory.make_RegionRackController()
734+ factory.make_RackController()
735+ factory.make_RegionController()
736+ response = self.client.get(reverse('nodes_handler'), {
737+ 'op': "by_type",
738+ 'count': True,
739+ })
740+ result = json.loads(
741+ response.content.decode(settings.DEFAULT_CHARSET))
742+ self.assertEqual(http.client.OK, response.status_code)
743+ # Since we're testing as a normal user, they can only see the nodes
744+ # and devices.
745+ self.assertThat(result["nodes"], Equals(2))
746+ self.assertThat(result["machines"], Equals(1))
747+ self.assertThat(result["devices"], Equals(1))
748+ self.assertThat(result["rack-controllers"], Equals(0))
749+ self.assertThat(result["region-controllers"], Equals(0))
750
751 def create_nodes(self, nodegroup, nb):
752 for _ in range(nb):
753
754=== modified file 'src/maasserver/testing/testclient.py'
755--- src/maasserver/testing/testclient.py 2016-05-24 14:53:56 +0000
756+++ src/maasserver/testing/testclient.py 2016-06-16 19:51:56 +0000
757@@ -30,7 +30,7 @@
758 """A derivative of Django's test client specially for MAAS.
759
760 This ensures that requests are performed in a transaction, and that
761- post-commit hooks are alway fired or reset.
762+ post-commit hooks are always fired or reset.
763
764 It also permits logging-in using just a user object. A password will be
765 configured and used to log-in.