Merge ~newell-jensen/maas:lp1749017 into maas:master

[Newell Jensen]

LP: #1749017 -- Fixes event query error for null node or user.

This branch fixes the bug and also adds username to the information returned by the events query API endpoint as this information is important in audit events to track the user's the event is associated with.

[Blake Rouse (blake-rouse)]

Change looks good. I have a suggestion that should improve performance for this API.

review: Needs Fixing

ec95cb7... by Newell Jensen on 2018-02-12

Use select_related instead of prefetch_related to reduce query count.

[Blake Rouse (blake-rouse)]

Looks good. Awesome!

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1749017 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: faca47962fefa17ab39c51e75c522af9d19ff61d

review: Approve

diff --git a/src/maasserver/api/events.py b/src/maasserver/api/events.py
index 72ff4f1..eee116d 100644
--- a/src/maasserver/api/events.py
+++ b/src/maasserver/api/events.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -37,8 +37,15 @@ DEFAULT_EVENT_LOG_LIMIT = 100
 def event_to_dict(event):
     """Convert `Event` to a dictionary."""
     return dict(
-        node=event.node.system_id,
-        hostname=event.node.hostname,
+        username=(
+            event.user.username
+            if event.user is not None else event.username),
+        node=(
+            event.node.system_id
+            if event.node is not None else None),
+        hostname=(
+            event.node.hostname
+            if event.node is not None else event.node_hostname),
         id=event.id,
         level=event.type.level_str,
         created=event.created.strftime('%a, %d %b. %Y %H:%M:%S'),
@@ -133,20 +140,21 @@ class EventsHandler(OperationsHandler):
         # Check first for AUDIT level.
         if level == LOGGING_LEVELS[AUDIT]:
             events = Event.objects.filter(type__level=AUDIT)
-            events = (events.all().prefetch_related('type'))
         elif level in LOGGING_LEVELS_BY_NAME:
             events = Event.objects.filter(node__in=nodes)
             # Eliminate logs below the requested level.
             events = events.exclude(
                 type__level__lt=LOGGING_LEVELS_BY_NAME[level])
-            events = (
-                events.all()
-                .prefetch_related('type')
-                .prefetch_related('node'))
         elif level is not None:
             raise MAASAPIBadRequest(
                 "Unrecognised log level: %s" % level)
 
+        events = (
+            events.all()
+            .select_related('type')
+            .select_related('node')
+            .select_related('user'))
+
         # Filter events for owner.
         if owner is not None:
             events = events.filter(user__username=owner)
diff --git a/src/maasserver/api/tests/test_events.py b/src/maasserver/api/tests/test_events.py
index 2cbbf12..24ecff1 100644
--- a/src/maasserver/api/tests/test_events.py
+++ b/src/maasserver/api/tests/test_events.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the events API."""
@@ -20,6 +20,7 @@ from urllib.parse import (
 
 from django.conf import settings
 from maasserver.api import events as events_module
+from maasserver.api.events import event_to_dict
 from maasserver.api.tests.test_nodes import RequestFixture
 from maasserver.enum import NODE_TYPE
 from maasserver.testing.api import APITestCase
@@ -36,6 +37,7 @@ from testtools.matchers import (
     Equals,
     HasLength,
     Is,
+    MatchesDict,
     MatchesStructure,
     Not,
 )
@@ -71,6 +73,54 @@ def AfterBeingDecoded(matcher):
         matcher)
 
 
+class TestEventToDict(APITestCase.ForUser):
+    """Test for `event_to_dict` function."""
+
+    def test__node_not_None(self):
+        event = factory.make_Event()
+        self.assertThat(event_to_dict(event), MatchesDict({
+            "username": Equals(event.user.username),
+            "node": Equals(event.node.system_id),
+            "hostname": Equals(event.node.hostname),
+            "id": Equals(event.id),
+            "level": Equals(event.type.level_str),
+            "created": Equals(
+                event.created.strftime('%a, %d %b. %Y %H:%M:%S')),
+            "type": Equals(event.type.description),
+            "description": Equals(event.description),
+            }))
+
+    def test__node_and_user_is_None(self):
+        event = factory.make_Event()
+        event.node = None
+        event.user = None
+        self.assertThat(event_to_dict(event), MatchesDict({
+            "username": Equals(event.username),
+            "node": Equals(None),
+            "hostname": Equals(event.node_hostname),
+            "id": Equals(event.id),
+            "level": Equals(event.type.level_str),
+            "created": Equals(
+                event.created.strftime('%a, %d %b. %Y %H:%M:%S')),
+            "type": Equals(event.type.description),
+            "description": Equals(event.description),
+            }))
+
+    def test__type_level_AUDIT(self):
+        event = factory.make_Event()
+        self.assertThat(event_to_dict(event), MatchesDict({
+            "username": Equals(event.user.username),
+            "node": Equals(event.node.system_id),
+            "hostname": Equals(event.node.hostname),
+            "id": Equals(event.id),
+            "level": Equals(event.type.level_str),
+            "created": Equals(
+                event.created.strftime('%a, %d %b. %Y %H:%M:%S')),
+            "type": Equals(event.type.description),
+            "description": Equals(event.render_audit_description),
+            }))
+
+
 class TestEventsAPI(APITestCase.ForUser):
     """Tests for /api/2.0/events/."""
 
@@ -588,10 +638,8 @@ class TestEventsAPI(APITestCase.ForUser):
             make_events(number_events, node=node)
 
     def test_query_num_queries_is_independent_of_num_nodes_and_events(self):
-        # 1 query for select event +
-        # 1 query to prefetch eventtype +
-        # 1 query to prefetch node details
-        expected_queries = 3
+        # 1 query for all the select_related's.
+        expected_queries = 1
         events_per_node = 5
         num_nodes_per_group = 5
         events_per_group = num_nodes_per_group * events_per_node