Improve performance to fetch hostname for changes (#304)

Make changes page load multiple times faster by avoiding to fetch hostnames in extra queries.

serveradmin/serverdb/models.py

@@ -4,14 +4,6 @@
 """
 
 import re
-import json
-import netfields
-
-from typing import Union
-
-from django.core.serializers.json import DjangoJSONEncoder
-from django.db.models import Q
-from netaddr import EUI
 from distutils.util import strtobool
 from ipaddress import (
     IPv4Address,
@@ -23,18 +15,22 @@
     IPv4Network,
     IPv6Network,
 )
+from typing import Union
 
+import netfields
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
+from django.core.serializers.json import DjangoJSONEncoder
 from django.core.validators import RegexValidator
 from django.db import models
+from django.db.models import Q
 from django.utils.timezone import now
 from django.utils.translation import gettext as _
+from netaddr import EUI
 
 from adminapi.datatype import STR_BASED_DATATYPES
 from serveradmin.apps.models import Application
 
-
 ATTRIBUTE_TYPES = {
     'string': str,
     'boolean': lambda x: bool(strtobool(x)),
@@ -800,26 +796,3 @@ def default(self, obj):
 
     class Meta:
         app_label = 'serverdb'
-
-    def hostname(self):
-        """Get last hostname for object of Change
-
-        Get the current hostname if the object still exists or the last known
-        one when deleted.
-
-        :return:
-        """
-
-        if self.change_type == Change.Type.DELETE:
-            return self.change_json['hostname']
-        else:
-            try:
-                return Server.objects.filter(
-                    pk=self.object_id).only('hostname').get().hostname
-            except Server.DoesNotExist:
-                # We seem to have re-used the same object_id in the past in
-                # some exceptions - in such as case just pick the latest.
-                return Change.objects.filter(
-                    object_id=self.object_id,
-                    change_type=Change.Type.DELETE
-                ).order_by('-id').first().change_json['hostname']

serveradmin/serverdb/templates/serverdb/changes.html

@@ -81,12 +81,11 @@ <h3>
                         <td>{{ commit.change_on|timesince }}</td>
                         <td>{{ commit.app|default:"Servershell" }}</td>
                         <td>{{ commit.user }}</td>
-                        {# TODO: Find a way to avoid a SQL query for each Change to get the hostname by object_id #}
                         <td>
                             <ul>
                             {% for change in commit.change_set.get_queryset %}
                                 {% if change.change_type == 'create' %}
-                                    <li><a href="{% url 'serverdb_history' %}?commit_id={{ commit.id }}&object_id={{ change.object_id }}">View history of {{ change.hostname }}</a></li>
+                                    <li><a href="{% url 'serverdb_history' %}?commit_id={{ commit.id }}&object_id={{ change.object_id }}">History for {{ change.hostname }}</a></li>
                                 {% endif %}
                             {% endfor %}
                             </ul>
@@ -95,7 +94,7 @@ <h3>
                             <ul>
                             {% for change in commit.change_set.get_queryset %}
                                 {% if change.change_type == 'change' %}
-                                    <li><a href="{% url 'serverdb_history' %}?commit_id={{ commit.id }}&object_id={{ change.object_id }}">View history of {{ change.hostname }}</a></li>
+                                    <li><a href="{% url 'serverdb_history' %}?commit_id={{ commit.id }}&object_id={{ change.object_id }}">History for {{ change.hostname }}</a></li>
                                 {% endif %}
                             {% endfor %}
                             </ul>
@@ -104,7 +103,7 @@ <h3>
                             <ul>
                             {% for change in commit.change_set.get_queryset %}
                                 {% if change.change_type == 'delete' %}
-                                    <li><a href="{% url 'serverdb_restore' change.id %}">Restore object {{ change.hostname }}</a></li>
+                                    <li><a href="{% url 'serverdb_restore' change.id %}">Recreate {{ change.hostname }}</a></li>
                                 {% endif %}
                             {% endfor %}
                             </ul>

serveradmin/serverdb/views.py

@@ -8,7 +8,9 @@
 from django.contrib.auth.decorators import login_required
 from django.core.exceptions import ValidationError
 from django.core.paginator import Paginator
-from django.db.models import Q
+from django.db.models import Q, Subquery, OuterRef, Prefetch, CharField
+from django.db.models.fields.json import KeyTextTransform
+from django.db.models.functions import Coalesce, Cast
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -19,8 +21,7 @@
     ChangeCommit,
     Server,
     ServertypeAttribute,
-    Change, Attribute,
-)
+    Change, )
 from serveradmin.serverdb.query_committer import CommitError, commit_query
 
 
@@ -64,7 +65,23 @@ def changes(request):
             Q(app__name=f_user_or_app) | Q(user__username=f_user_or_app))
 
     commits = commits.select_related('app', 'user')
-    commits = commits.prefetch_related('change_set')
+
+    # This complex statement is just here to be able to prefetch the changes'
+    # relation including the hostname fetched either from the server table
+    # or from the change entry where the object was deleted.
+    server_hostname = Server.objects.filter(
+        server_id=OuterRef('object_id')).values('hostname')
+    change_hostname = Change.objects.filter(
+        id=OuterRef('id'),
+        change_type=Change.Type.DELETE).order_by('-id').values('change_json')
+    commits = commits.prefetch_related(Prefetch(
+        'change_set',
+        queryset=Change.objects.all().annotate(hostname=Coalesce(
+            Subquery(server_hostname),
+            Cast(
+                KeyTextTransform('hostname', Subquery(change_hostname)),
+                output_field=CharField())
+    ))))
 
     class NoCountPaginator(Paginator):
         @cached_property