[Patchew-devel] [PATCH 04/12] models: move Result log_url from constructor to renderer

Paolo Bonzini pbonzini at redhat.com
Tue May 22 06:57:32 UTC 2018


The log_url is not going to be stored in the database.  In preparation for
changing Result to a model, retrieve it through a method in the renderer.
(The constructor and the renderer are actually always the same object
currently :) but there will be no constructor concept will also disappear
once Results will be fetched from the database).

This has other benefits too.  It also removes the request argument
parameter from the Result constructor (which would not have any
equivalent for database-stored Results) and from the rest_results_hook;
and it makes log_url a read-only field in the REST API, which will be
the right thing once PUT of Results will be supported.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 api/models.py   | 16 +++++++++++-----
 api/rest.py     | 11 +++++++----
 mods/git.py     | 15 ++++++++-------
 mods/testing.py | 19 +++++++++++--------
 www/views.py    |  4 ++--
 5 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/api/models.py b/api/models.py
index 8f7ca19..0f25f85 100644
--- a/api/models.py
+++ b/api/models.py
@@ -614,7 +614,7 @@ class Module(models.Model):
     def __str__(self):
         return self.name
 
-class Result(namedtuple("Result", "name status log log_url obj data renderer")):
+class Result(namedtuple("Result", "name status log obj data renderer")):
     __slots__ = ()
     PENDING = 'pending'
     SUCCESS = 'success'
@@ -622,12 +622,10 @@ class Result(namedtuple("Result", "name status log log_url obj data renderer")):
     RUNNING = 'running'
     VALID_STATUSES = (PENDING, SUCCESS, FAILURE, RUNNING)
 
-    def __new__(cls, name, status, obj, log=None, log_url=None, data=None, request=None, renderer=None):
-        if log_url is not None and request is not None:
-            log_url = request.build_absolute_uri(log_url)
+    def __new__(cls, name, status, obj, log=None, data=None, renderer=None):
         if status not in cls.VALID_STATUSES:
             raise ValueError("invalid value '%s' for status field" % status)
-        return super(cls, Result).__new__(cls, status=status, log=log, log_url=log_url,
+        return super(cls, Result).__new__(cls, status=status, log=log,
                                           obj=obj, data=data, name=name, renderer=renderer)
 
     def is_success(self):
@@ -649,3 +647,11 @@ class Result(namedtuple("Result", "name status log log_url obj data renderer")):
         if self.renderer is None:
             return None
         return self.renderer.render_result(self)
+
+    def get_log_url(self, request=None):
+        if not self.is_completed() or self.renderer is None:
+            return None
+        log_url = self.renderer.get_result_log_url(self.obj, self.name)
+        if log_url is not None and request is not None:
+            log_url = request.build_absolute_uri(log_url)
+        return log_url
diff --git a/api/rest.py b/api/rest.py
index fa6ca3f..f36cf28 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -19,7 +19,7 @@ from .search import SearchEngine
 from rest_framework import (permissions, serializers, viewsets, filters,
     mixins, generics, renderers, status)
 from rest_framework.decorators import detail_route
-from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField
+from rest_framework.fields import SerializerMethodField, CharField, JSONField, EmailField, SkipField
 from rest_framework.relations import HyperlinkedIdentityField
 from rest_framework.response import Response
 import rest_framework
@@ -429,9 +429,13 @@ class ResultSerializer(serializers.Serializer):
     resource_uri = HyperlinkedResultField(view_name='results-detail')
     name = CharField()
     status = CharField() # one of 'failure', 'success', 'pending', 'running'
-    log_url = CharField(required=False)
+    log_url = SerializerMethodField(required=False)
     data = JSONField(required=False)
 
+    def get_log_url(self, obj):
+        request = self.context['request']
+        return obj.get_log_url(request)
+
 class ResultSerializerFull(ResultSerializer):
     log = CharField(required=False)
 
@@ -452,8 +456,7 @@ class ResultsViewSet(viewsets.ViewSet, generics.GenericAPIView):
         except IndexError:
             raise Http404
         results = []
-        dispatch_module_hook("rest_results_hook", request=self.request,
-                             obj=obj, results=results,
+        dispatch_module_hook("rest_results_hook", obj=obj, results=results,
                              detailed=detailed)
         return {x.name: x for x in results}
 
diff --git a/mods/git.py b/mods/git.py
index fe10423..a216ea2 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -126,7 +126,7 @@ class GitModule(PatchewModule):
     def rest_series_fields_hook(self, request, fields, detailed):
         fields['based_on'] = PluginMethodField(obj=self, required=False)
 
-    def rest_results_hook(self, request, obj, results, detailed=False):
+    def rest_results_hook(self, obj, results, detailed=False):
         if not isinstance(obj, Message):
             return
         log = obj.get_property("git.apply-log")
@@ -148,13 +148,10 @@ class GitModule(PatchewModule):
                     if git_base:
                         data['base'] = git_base
                 status = Result.SUCCESS
-            log_url = reverse("git-log", kwargs={'series': obj.message_id})
         else:
             status = Result.PENDING
-            log_url = None
         results.append(Result(name='git', obj=obj, status=status,
-                              log=log, log_url=log_url, data=data,
-                              request=request, renderer=self))
+                              log=log, data=data, renderer=self))
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
@@ -199,9 +196,10 @@ class GitModule(PatchewModule):
         if not result.is_completed():
             return None
 
-        html_log_url = result.log_url + "?html=1"
+        log_url = result.get_log_url()
+        html_log_url = log_url + "?html=1"
         colorbox_a = format_html('<a class="cbox-log" data-link="{}" href="{}">apply log</a>',
-                                 html_log_url, result.log_url)
+                                 html_log_url, log_url)
         if result.is_failure():
             return format_html('Failed in applying to current master ({})', colorbox_a)
         else:
@@ -218,6 +216,9 @@ class GitModule(PatchewModule):
                 s += format_html('<br/><samp>git fetch {} {}</samp>', git_repo, git_tag)
             return s
 
+    def get_result_log_url(self, obj, name):
+        return reverse("git-log", kwargs={'series': obj.message_id})
+
     def prepare_project_hook(self, request, project):
         if not project.maintained_by(request.user):
             return
diff --git a/mods/testing.py b/mods/testing.py
index 824df15..2d379cb 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -165,8 +165,8 @@ class TestingModule(PatchewModule):
                                       "testing_name": test}) + "?type=project"
         if html:
             log_url += "&html=1"
-        # Generate a full URL, including the host and port, for use in email
-        # notifications and REST API responses.
+        # Generate a full URL, including the host and port, for use in
+        # email notifications
         if request:
             log_url = request.build_absolute_uri(log_url)
         return log_url
@@ -251,7 +251,7 @@ class TestingModule(PatchewModule):
                         })
         return ret
 
-    def rest_results_hook(self, request, obj, results, detailed=False):
+    def rest_results_hook(self, obj, results, detailed=False):
         all_tests = set([k for k, v in _instance.get_tests(obj).items() if v["enabled"]])
         for pn, p in obj.get_properties().items():
             if not pn.startswith("testing.report."):
@@ -262,7 +262,6 @@ class TestingModule(PatchewModule):
             except:
                 pass
             failed = not p["passed"]
-            log_url = self.reverse_testing_log(obj, tn, request=request, html=False)
             passed_str = Result.FAILURE if failed else Result.SUCCESS
             if detailed:
                 log = obj.get_property("testing.log." + tn)
@@ -272,8 +271,7 @@ class TestingModule(PatchewModule):
             data = p.copy()
             del data['passed']
             results.append(Result(name='testing.' + tn, obj=obj, status=passed_str,
-                                  log=log, log_url=log_url, request=request, data=data,
-                                  renderer=self))
+                                  log=log, data=data, renderer=self))
 
         if obj.get_property("testing.ready"):
             for tn in all_tests:
@@ -305,15 +303,20 @@ class TestingModule(PatchewModule):
                 "char": "T",
                 })
 
+    def get_result_log_url(self, obj, name):
+        tn = name[len("testing."):]
+        return self.reverse_testing_log(obj, tn, html=False)
+
     def render_result(self, result):
         if not result.is_completed():
             return None
         pn = result.name
         tn = pn[len("testing."):]
-        html_log_url = result.log_url + '&html=1'
+        log_url = result.get_log_url()
+        html_log_url = log_url + '&html=1'
         passed_str = "failed" if result.is_failure() else "passed"
         return format_html('Test <b>{}</b> <a class="cbox-log" data-link="{}" href="{}">{}</a>',
-                           tn, html_log_url, result.log_url, passed_str)
+                           tn, html_log_url, log_url, passed_str)
 
     def check_active_testers(self, project):
         at = []
diff --git a/www/views.py b/www/views.py
index 1eb6c3c..c2010c5 100644
--- a/www/views.py
+++ b/www/views.py
@@ -93,8 +93,8 @@ def prepare_series(request, s, skip_patches=False):
 
 def prepare_results(request, obj):
     results = []
-    dispatch_module_hook("rest_results_hook", request=request,
-                         obj=obj, results=results, detailed=False)
+    dispatch_module_hook("rest_results_hook", obj=obj,
+                         results=results, detailed=False)
 
     results_dicts = []
     for result in results:
-- 
2.17.0





More information about the Patchew-devel mailing list