In the REST API, each series can have one or more results.  Right now
results are synthesized from message properties which are set by
the applier-report and tester-report API.  In order to simplify the
conversion to REST of those APIs (e.g. using POST, PUT or PATCH
requests), move the results out of the SeriesSerializer and into
their own collection.

For now, the ViewSet is not model-based, but I am including
paginator-like fields so that it should be possible to change it
to a ModelViewSet in the future.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 api/rest.py           | 75 +++++++++++++++++++++++++++++++++++++++++----------
 api/urls.py           |  5 ++++
 mods/git.py           |  4 +--
 mods/testing.py       |  4 +--
 tests/test_git.py     | 40 +++++++++++++++------------
 tests/test_rest.py    |  6 +++++
 tests/test_testing.py | 28 +++++++++----------
 7 files changed, 111 insertions(+), 51 deletions(-)

diff --git a/api/rest.py b/api/rest.py
index 7df7618..3d22de8 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -8,19 +8,22 @@
 # This work is licensed under the MIT License.  Please see the LICENSE file or
 # http://opensource.org/licenses/MIT.
 
-from collections import namedtuple
+from collections import namedtuple, OrderedDict
 
 from django.contrib.auth.models import User
+from django.http import Http404
 from django.template import loader
 
 from mod import dispatch_module_hook
 from .models import Project, Message
 from .search import SearchEngine
-from rest_framework import permissions, serializers, viewsets, filters, mixins, renderers
+from rest_framework import (permissions, serializers, viewsets, filters,
+    mixins, generics, renderers)
 from rest_framework.decorators import detail_route
-from rest_framework.fields import SerializerMethodField
+from rest_framework.fields import SerializerMethodField, CharField, JSONField
 from rest_framework.relations import HyperlinkedIdentityField
 from rest_framework.response import Response
+import rest_framework
 
 SEARCH_PARAM = 'q'
 
@@ -143,16 +146,16 @@ class ProjectMessagesViewSetMixin(mixins.RetrieveModelMixin):
     def get_queryset(self):
         return self.queryset.filter(project=self.kwargs['projects_pk'])
 
-class Result(namedtuple("Result", "name status log_url data")):
+class Result(namedtuple("Result", "name status log_url message data")):
     __slots__ = ()
 
-    def __new__(cls, name, status, log_url=None, data=None, request=None):
+    def __new__(cls, name, status, message, log_url=None, data=None, request=None):
         if log_url is not None and request is not None:
             log_url = request.build_absolute_uri(log_url)
         if status not in ('pending', 'success', 'failure'):
             raise ValueError("invalid value '%s' for status field" % status)
         return super(cls, Result).__new__(cls, status=status, log_url=log_url,
-                                          data=data, name=name)
+                                          message=message, data=data, name=name)
 
 # Series
 
@@ -176,8 +179,8 @@ class SeriesSerializer(BaseMessageSerializer):
 
     resource_uri = HyperlinkedMessageField(view_name='series-detail')
     message = HyperlinkedMessageField(view_name='messages-detail')
+    results = HyperlinkedMessageField(view_name='results-list', lookup_field='series_message_id')
     total_patches = SerializerMethodField()
-    results = SerializerMethodField()
 
     def __init__(self, *args, **kwargs):
         self.detailed = kwargs.pop('detailed', False)
@@ -190,13 +193,6 @@ class SeriesSerializer(BaseMessageSerializer):
                              fields=fields, detailed=self.detailed)
         return fields
 
-    def get_results(self, message):
-        results = []
-        request = self.context['request']
-        dispatch_module_hook("rest_results_hook", request=request,
-                             message=message, results=results)
-        return {x.name: x._asdict() for x in results}
-
     def get_total_patches(self, obj):
         return obj.get_total_patches()
 
@@ -329,3 +325,54 @@ class MessagesViewSet(ProjectMessagesViewSetMixin,
         serializer = BaseMessageSerializer(page, many=True,
                                            context=self.get_serializer_context())
         return self.get_paginated_response(serializer.data)
+
+# Results
+
+class HyperlinkedResultField(HyperlinkedIdentityField):
+    def get_url(self, obj, view_name, request, format):
+        message = obj.message
+        kwargs = {'projects_pk': message.project_id, 'series_message_id': message.message_id,
+                  'name': obj.name}
+        return self.reverse(view_name, kwargs=kwargs, request=request, format=format)
+
+class ResultSerializer(serializers.Serializer):
+    resource_uri = HyperlinkedResultField(view_name='results-detail')
+    name = CharField()
+    status = CharField() # one of 'failure', 'success', 'pending'
+    log_url = CharField(required=False)
+    data = JSONField(required=False)
+
+class SeriesResultsViewSet(viewsets.ViewSet, generics.GenericAPIView):
+    serializer_class = ResultSerializer
+    lookup_field = 'name'
+    lookup_value_regex = '[^/]+'
+
+    def get_queryset(self):
+        return Message.objects.filter(project=self.kwargs['projects_pk'],
+                                      message_id=self.kwargs['series_message_id'])
+
+    def get_results(self):
+        message = self.get_queryset()[0]
+        results = []
+        dispatch_module_hook("rest_results_hook", request=self.request,
+                             message=message, results=results)
+        return {x.name: x for x in results}
+
+    def list(self, request, *args, **kwargs):
+        results = self.get_results().values()
+        serializer = self.get_serializer(results, many=True)
+        # Fake paginator response for forwards-compatibility, in case
+        # this ViewSet becomes model-based
+        return Response(OrderedDict([
+            ('count', len(results)),
+            ('results', serializer.data)
+        ]))
+
+    def retrieve(self, request, name, *args, **kwargs):
+        results = self.get_results()
+        try:
+            result = results[name]
+        except KeyError:
+            raise Http404
+        serializer = self.get_serializer(result)
+        return Response(serializer.data)
diff --git a/api/urls.py b/api/urls.py
index 0a447f0..becabeb 100644
--- a/api/urls.py
+++ b/api/urls.py
@@ -37,11 +37,16 @@ projects_router.include_format_suffixes = False
 projects_router.register('series', rest.ProjectSeriesViewSet, base_name='series')
 projects_router.register('messages', rest.MessagesViewSet, base_name='messages')
 
+results_router = NestedDefaultRouter(projects_router, 'series', lookup='series', trailing_slash=True)
+results_router.include_format_suffixes = False
+results_router.register('results', rest.SeriesResultsViewSet, base_name='results')
+
 schema_view = get_schema_view(title='API schema')
 
 urlpatterns = _build_urls() + [
         url(r"v1/", include(router.urls)),
         url(r"v1/", include(projects_router.urls)),
+        url(r"v1/", include(results_router.urls)),
         url(r'^v1/schema/$', schema_view),
         # Use the base class's handler by default
         url(r".*", views.APIView.as_view())
diff --git a/mods/git.py b/mods/git.py
index fc71174..8164854 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -129,8 +129,8 @@ class GitModule(PatchewModule):
         else:
             status = 'pending'
             log_url = None
-        results.append(Result(name='git', status=status, log_url=log_url, data=data,
-                              request=request))
+        results.append(Result(name='git', message=message, status=status,
+                              log_url=log_url, data=data, request=request))
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
diff --git a/mods/testing.py b/mods/testing.py
index 970256d..6f7d1ae 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -290,8 +290,8 @@ class TestingModule(PatchewModule):
             failed = not p["passed"]
             log_url = self.reverse_testing_log(message, tn, request=request, html=False)
             passed_str = "failure" if failed else "success"
-            results.append(Result(name='testing.' + tn, status=passed_str, log_url=log_url,
-                                  request=request))
+            results.append(Result(name='testing.' + tn, message=message, status=passed_str,
+                                  log_url=log_url, request=request))
 
     def prepare_message_hook(self, request, message, detailed):
         if not message.is_series_head:
diff --git a/tests/test_git.py b/tests/test_git.py
index 4744ef4..fdf6b67 100755
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -103,31 +103,37 @@ class GitTest(PatchewTestCase):
                          self.repo + " patchew/20160628014747.20971-2-famz@redhat.com")
 
     def test_rest_need_apply(self):
-        resp = self.apply_and_retrieve("0013-foo-patch.mbox.gz", self.p.id,
-                                       "20160628014747.20971-1-famz@redhat.com")
-        self.assertEqual(resp.data['results']['git']['status'], 'pending')
+        self.cli_import("0013-foo-patch.mbox.gz")
+        MESSAGE_ID = '20160628014747.20971-1-famz@redhat.com'
+        resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+        self.assertEqual(resp.data['status'], 'pending')
+        self.assertEqual(resp.data['log_url'], None)
 
     def test_rest_apply_failure(self):
         self.cli_import("0014-bar-patch.mbox.gz")
         self.do_apply()
-        resp = self.api_client.get(self.REST_BASE + 'series/?q=project:QEMU')
-        self.assertEqual(resp.data['results'][0]['is_complete'], True)
-        self.assertEqual(resp.data['results'][0]['results']['git']['status'], 'failure')
-        self.assertEqual('repo' in resp.data['results'][0]['results']['git'], False)
-        self.assertEqual('tag' in resp.data['results'][0]['results']['git'], False)
-        log = self.client.get(resp.data['results'][0]['results']['git']['log_url'])
-        self.assertEquals(log.status_code, 200)
+        MESSAGE_ID = '20160628014747.20971-2-famz@redhat.com'
+        resp = self.api_client.get('%sseries/%s/' % (self.PROJECT_BASE, MESSAGE_ID))
+        self.assertEqual(resp.data['is_complete'], True)
+        resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+        self.assertEqual(resp.data['status'], 'failure')
+        self.assertEqual('repo' in resp.data, False)
+        self.assertEqual('tag' in resp.data, False)
+        log = self.client.get(resp.data['log_url'])
+        self.assertEqual(log.status_code, 200)
 
     def test_rest_apply_success(self):
         self.cli_import("0013-foo-patch.mbox.gz")
         self.do_apply()
-        resp = self.api_client.get(self.REST_BASE + 'series/?q=project:QEMU')
-        self.assertEqual(resp.data['results'][0]['is_complete'], True)
-        self.assertEqual(resp.data['results'][0]['results']['git']['status'], 'success')
-        self.assertEqual(resp.data['results'][0]['results']['git']['data']['repo'], self.repo)
-        self.assertEqual(resp.data['results'][0]['results']['git']['data']['tag'], "refs/tags/patchew/20160628014747.20971-1-famz@redhat.com")
-        log = self.client.get(resp.data['results'][0]['results']['git']['log_url'])
-        self.assertEquals(log.status_code, 200)
+        MESSAGE_ID = '20160628014747.20971-1-famz@redhat.com'
+        resp = self.api_client.get('%sseries/%s/' % (self.PROJECT_BASE, MESSAGE_ID))
+        self.assertEqual(resp.data['is_complete'], True)
+        resp = self.api_client.get('%sseries/%s/results/git/' % (self.PROJECT_BASE, MESSAGE_ID))
+        self.assertEqual(resp.data['status'], 'success')
+        self.assertEqual(resp.data['data']['repo'], self.repo)
+        self.assertEqual(resp.data['data']['tag'], "refs/tags/patchew/20160628014747.20971-1-famz@redhat.com")
+        log = self.client.get(resp.data['log_url'])
+        self.assertEqual(log.status_code, 200)
 
 if __name__ == '__main__':
     main()
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 1a7285b..38fd6bf 100755
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -193,6 +193,12 @@ class RestTest(PatchewTestCase):
         resp = self.api_client.get(self.REST_BASE + 'projects/12345/series/')
         self.assertEqual(resp.data['count'], 0)
 
+    def test_series_results_list(self):
+        resp1 = self.apply_and_retrieve('0001-simple-patch.mbox.gz',
+                                       self.p.id, '20160628014747.20971-1-famz@redhat.com')
+        resp = self.api_client.get(resp1.data['results'])
+        self.assertEqual(resp.data['count'], len(resp.data['results']))
+
     def test_series_search(self):
         resp1 = self.apply_and_retrieve('0004-multiple-patch-reviewed.mbox.gz',
                                         self.p.id, '1469192015-16487-1-git-send-email-berrange@redhat.com')
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 0d7e316..9bc03ab 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -94,41 +94,37 @@ class TestingTest(PatchewTestCase):
                            capabilities=[])
         self.assertFalse(td)
 
-    def test_rest_basic(self):
-        resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
-        self.assertEquals('testing.tests' in resp.data['results'], False)
-
     def test_rest_done_success(self):
         self.msg_testing_done(log='everything good!', passed=True)
-        resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
-        self.assertEquals(resp.data['results']['testing.tests']['status'], 'success')
-        log = self.client.get(resp.data['results']['testing.tests']['log_url'])
+        resp = self.api_client.get('%sseries/%s/results/testing.tests/' % (self.PROJECT_BASE, self.msg.message_id))
+        self.assertEquals(resp.data['status'], 'success')
+        log = self.client.get(resp.data['log_url'])
         self.assertEquals(log.status_code, 200)
         self.assertEquals(log.content, b'everything good!')
 
     def test_rest_done_failure(self):
         self.msg_testing_done(log='sorry no good', passed=False)
-        resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
-        self.assertEquals(resp.data['results']['testing.tests']['status'], 'failure')
-        log = self.client.get(resp.data['results']['testing.tests']['log_url'])
+        resp = self.api_client.get('%sseries/%s/results/testing.tests/' % (self.PROJECT_BASE, self.msg.message_id))
+        self.assertEquals(resp.data['status'], 'failure')
+        log = self.client.get(resp.data['log_url'])
         self.assertEquals(log.status_code, 200)
         self.assertEquals(log.content, b'sorry no good')
 
     def test_api_report_success(self):
         self.api_login()
         msgid = self.msg_testing_report(log='everything good!', passed=True)
-        resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
-        self.assertEquals(resp.data['results']['testing.a']['status'], 'success')
-        log = self.client.get(resp.data['results']['testing.a']['log_url'])
+        resp = self.api_client.get('%sseries/%s/results/testing.a/' % (self.PROJECT_BASE, self.msg.message_id))
+        self.assertEquals(resp.data['status'], 'success')
+        log = self.client.get(resp.data['log_url'])
         self.assertEquals(log.status_code, 200)
         self.assertEquals(log.content, b'everything good!')
 
     def test_api_report_failure(self):
         self.api_login()
         msgid = self.msg_testing_report(log='sorry no good', passed=False)
-        resp = self.api_client.get(self.PROJECT_BASE + 'series/' + self.msg.message_id + '/')
-        self.assertEquals(resp.data['results']['testing.a']['status'], 'failure')
-        log = self.client.get(resp.data['results']['testing.a']['log_url'])
+        resp = self.api_client.get('%sseries/%s/results/testing.a/' % (self.PROJECT_BASE, self.msg.message_id))
+        self.assertEquals(resp.data['status'], 'failure')
+        log = self.client.get(resp.data['log_url'])
         self.assertEquals(log.status_code, 200)
         self.assertEquals(log.content, b'sorry no good')
 
-- 
2.16.2


_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

On Sat, 03/17 15:47, Paolo Bonzini wrote:
> In the REST API, each series can have one or more results.  Right now
> results are synthesized from message properties which are set by
> the applier-report and tester-report API.  In order to simplify the
> conversion to REST of those APIs (e.g. using POST, PUT or PATCH
> requests), move the results out of the SeriesSerializer and into
> their own collection.
> 
> For now, the ViewSet is not model-based, but I am including
> paginator-like fields so that it should be possible to change it
> to a ModelViewSet in the future.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  api/rest.py           | 75 +++++++++++++++++++++++++++++++++++++++++----------
>  api/urls.py           |  5 ++++
>  mods/git.py           |  4 +--
>  mods/testing.py       |  4 +--
>  tests/test_git.py     | 40 +++++++++++++++------------
>  tests/test_rest.py    |  6 +++++
>  tests/test_testing.py | 28 +++++++++----------
>  7 files changed, 111 insertions(+), 51 deletions(-)
> 
> diff --git a/api/rest.py b/api/rest.py
> index 7df7618..3d22de8 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -8,19 +8,22 @@
>  # This work is licensed under the MIT License.  Please see the LICENSE file or
>  # http://opensource.org/licenses/MIT.
>  
> -from collections import namedtuple
> +from collections import namedtuple, OrderedDict
>  
>  from django.contrib.auth.models import User
> +from django.http import Http404
>  from django.template import loader
>  
>  from mod import dispatch_module_hook
>  from .models import Project, Message
>  from .search import SearchEngine
> -from rest_framework import permissions, serializers, viewsets, filters, mixins, renderers
> +from rest_framework import (permissions, serializers, viewsets, filters,
> +    mixins, generics, renderers)
>  from rest_framework.decorators import detail_route
> -from rest_framework.fields import SerializerMethodField
> +from rest_framework.fields import SerializerMethodField, CharField, JSONField
>  from rest_framework.relations import HyperlinkedIdentityField
>  from rest_framework.response import Response
> +import rest_framework
>  
>  SEARCH_PARAM = 'q'
>  
> @@ -143,16 +146,16 @@ class ProjectMessagesViewSetMixin(mixins.RetrieveModelMixin):
>      def get_queryset(self):
>          return self.queryset.filter(project=self.kwargs['projects_pk'])
>  
> -class Result(namedtuple("Result", "name status log_url data")):
> +class Result(namedtuple("Result", "name status log_url message data")):
>      __slots__ = ()
>  
> -    def __new__(cls, name, status, log_url=None, data=None, request=None):
> +    def __new__(cls, name, status, message, log_url=None, data=None, request=None):
>          if log_url is not None and request is not None:
>              log_url = request.build_absolute_uri(log_url)
>          if status not in ('pending', 'success', 'failure'):
>              raise ValueError("invalid value '%s' for status field" % status)
>          return super(cls, Result).__new__(cls, status=status, log_url=log_url,
> -                                          data=data, name=name)
> +                                          message=message, data=data, name=name)

Result model is useful for Project as well (we want to test project master too?
It will be important when we add this:

https://github.com/patchew-project/patchew/issues/32
(Test base branch as well when testing on topic failed, and use it as a
reference)

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

On 21/03/2018 03:33, Fam Zheng wrote:
>> -    def __new__(cls, name, status, log_url=None, data=None, request=None):
>> +    def __new__(cls, name, status, message, log_url=None, data=None, request=None):
>>          if log_url is not None and request is not None:
>>              log_url = request.build_absolute_uri(log_url)
>>          if status not in ('pending', 'success', 'failure'):
>>              raise ValueError("invalid value '%s' for status field" % status)
>>          return super(cls, Result).__new__(cls, status=status, log_url=log_url,
>> -                                          data=data, name=name)
>> +                                          message=message, data=data, name=name)
> Result model is useful for Project as well (we want to test project master too?
> It will be important when we add this:
> 
> https://github.com/patchew-project/patchew/issues/32
> (Test base branch as well when testing on topic failed, and use it as a
> reference)

I agree, and we need to add it for a REST equivalent of
/api/testing-report.  For this series I only looked at ensuring that
future changes to the REST API keep backwards-compatibility, but
/projects/{id}/results/ is certainly on the roadmap.

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel