Fix review comments.

pdc/apps/compose/tests.py

@@ -201,26 +201,6 @@ def test_get_for_release_with_latest(self):
         self.assertEqual(response.data,
                          [{'compose': 'compose-3', 'packages': ['bash-0:5.6.7-8.x86_64.rpm']}])
 
-    def test_get_for_release_to_dict(self):
-        url = reverse('findcomposebyrr-list', kwargs={'rpm_name': 'bash', 'release_id': 'release-1.0'})
-        response = self.client.get(url, {'to_dict': True})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        expected = [
-            {'compose': u'compose-1', 'packages': [
-                {'name': u'bash', 'version': u'1.2.3', 'epoch': 0, 'release': u'4.b1',
-                 'arch': u'x86_64', 'srpm_name': u'bash', 'srpm_nevra': u'bash-0:1.2.3-4.b1.src',
-                 'filename': 'bash-1.2.3-4.b1.x86_64.rpm'}]},
-            {'compose': u'compose-2', 'packages': [
-                {'name': u'bash', 'version': u'1.2.3', 'epoch': 0, 'release': u'4.b1',
-                 'arch': u'x86_64', 'srpm_name': u'bash', 'srpm_nevra': u'bash-0:1.2.3-4.b1.src',
-                 'filename': 'bash-1.2.3-4.b1.x86_64.rpm'}]},
-            {'compose': u'compose-3', 'packages': [
-                {'name': u'bash', 'version': u'5.6.7', 'epoch': 0, 'release': u'8',
-                 'arch': u'x86_64', 'srpm_name': u'bash', 'srpm_nevra': None,
-                 'filename': 'bash-5.6.7-8.x86_64.rpm'}]}
-        ]
-        self.assertEqual(response.data, expected)
-
     def test_get_for_excluded_compose_type(self):
         url = reverse('findcomposebyrr-list', kwargs={'rpm_name': 'bash', 'release_id': 'release-1.0'})
         response = self.client.get(url, {'excluded_compose_type': 'production'})

pdc/apps/compose/views.py

@@ -1110,42 +1110,44 @@ def list(self, request):
 
 class FindComposeMixin(object):
 
+    TO_DICT_DEFAULT_VALUE = False
+
     def _filter_by_compose_type(self, qs):
         if self.included_compose_type:
             qs = qs.filter(compose_type__name=self.included_compose_type)
         if self.excluded_compose_type:
             qs = qs.exclude(compose_type__name=self.excluded_compose_type)
         return qs
 
-    def _get_compose_for_release(self):
+    def _get_compose_for_release(self, to_dict=TO_DICT_DEFAULT_VALUE):
         result = []
         composes = Compose.objects.filter(release__release_id=self.release_id)
         composes = self._filter_by_compose_type(composes)
-        result = self._get_result(composes, result)
+        result = self._get_result(composes, result, to_dict)
         return result
 
-    def _get_result(self, composes, result):
+    def _get_result(self, composes, result, to_dict):
         if self.latest:
             compose = max(composes) if composes else None
             if compose:
-                self._construct_result(compose, result)
+                self._construct_result(compose, result, to_dict)
         else:
             for compose in sorted(composes):
-                self._construct_result(compose, result)
+                self._construct_result(compose, result, to_dict)
         return result
 
-    def _construct_result(self, compose, result):
+    def _construct_result(self, compose, result, to_dict):
         rpms = compose.get_rpms(self.rpm_name)
         result.append({'compose': compose.compose_id,
-                       'packages': self._packages_output(rpms)})
+                       'packages': self._packages_output(rpms, to_dict)})
         return result
 
-    def _packages_output(self, rpms):
+    def _packages_output(self, rpms, to_dict):
         """
         Output packages with unicode or dict
         """
         packages = [unicode(rpm) for rpm in rpms]
-        return packages if not self.to_dict else [RPMSerializer(rpm).data for rpm in rpms]
+        return packages if not to_dict else [RPMSerializer(rpm).data for rpm in rpms]
 
     def _get_query_param_or_false(self, request, query_str):
         value = request.query_params.get(query_str)
@@ -1155,7 +1157,7 @@ def _get_query_param_or_false(self, request, query_str):
             value = False
         setattr(self, query_str, value)
 
-    def _get_older_compose(self):
+    def _get_older_compose(self, to_dict=TO_DICT_DEFAULT_VALUE):
         compose = get_object_or_404(Compose, compose_id=self.compose_id)
         current_rpms = set(r.sort_key for r in compose.get_rpms(self.rpm_name))
         # Find older composes for same release (not including this one)
@@ -1177,7 +1179,7 @@ def _get_older_compose(self):
             raise Http404('No older compose with earlier version of RPM')
         return {
             'compose': latest.compose_id,
-            'packages': self._packages_output(rpms)
+            'packages': self._packages_output(rpms, to_dict)
         }
 
 
@@ -1187,20 +1189,16 @@ class FindComposeByReleaseRPMViewSet(StrictQueryParamMixin, FindComposeMixin, vi
     (and include its version) for a given release and srpm_name
     """
     queryset = ComposeRPM.objects.none()    # Required for permissions
-    extra_query_params = ('to_dict', 'included_compose_type', 'excluded_compose_type', 'latest')
+    extra_query_params = ('included_compose_type', 'excluded_compose_type', 'latest')
 
     def list(self, request, **kwargs):
         """
         This method allows listing all (compose, package) pairs for a given
         release and RPM name.
 
         The ordering of composes is performed by the *productmd* library. It
-        first compares compose date, then respin and lastly compose type
-        (`test` < `nightly` < `production`).
-
-        `to_dict` is optional parameter, accepted values (True, 'true', 't', 'True', '1'),
-        or (False, 'false', 'f', 'False', '0'). If it is provided, and the value is True,
-        packages' format will be as a dict.
+        first compares compose date, then compose type
+        (`test` < `nightly` < `production`) and lastly respin.
 
         `latest` is optional parameter. If it is provided, and the value is True, it will
         return a single pair with the latest compose and its version of the packages.
@@ -1214,7 +1212,6 @@ def list(self, request, **kwargs):
 
         The RPM name and release id are always required.
 
-         * `to_dict`: optional
          * `included_compose_type`: optional
          * `excluded_compose_type`: optional
          * `latest`: optional
@@ -1233,7 +1230,6 @@ def list(self, request, **kwargs):
         """
         self.included_compose_type = request.query_params.get('included_compose_type')
         self.excluded_compose_type = request.query_params.get('excluded_compose_type')
-        self._get_query_param_or_false(request, 'to_dict')
         self._get_query_param_or_false(request, 'latest')
         self.release_id = kwargs.get('release_id')
         self.rpm_name = kwargs.get('rpm_name')
@@ -1250,17 +1246,14 @@ class FindLatestComposeByComposeRPMViewSet(StrictQueryParamMixin, FindComposeMix
 
     def list(self, request, **kwargs):
         """
-        This method is to given a compose C and a package, then find the latest
-        compose older than C which contains a different version of the package.
+        This method is to find the latest compose older than specified compose
+        which contains a different version of the specified package when given a
+        compose and a package.
 
         The ordering of composes is performed by the *productmd* library. It
-        first compares compose date, then respin and lastly compose type
-        (`test` < `nightly` < `production`). This method will find the latest
-        one according to the sequence.
-
-        `to_dict` is optional parameter, accepted values (True, 'true', 't', 'True', '1'),
-        or (False, 'false', 'f', 'False', '0'). If it is provided, and the value is True,
-        packages' format will be as a dict.
+        first compares compose date, then compose type
+        (`test` < `nightly` < `production`) and lastly respin.
+        This method will find the latest one according to above sequence.
 
         `latest` is optional parameter. If it is provided, and the value is True, it will
         return a single pair with the latest compose and its version of the packages.
@@ -1274,7 +1267,6 @@ def list(self, request, **kwargs):
 
         The RPM name and release id are always required.
 
-         * `to_dict`: optional
          * `included_compose_type`: optional
          * `excluded_compose_type`: optional
          * `latest`: optional
@@ -1320,9 +1312,15 @@ def list(self, request):
         with the newest compose older than the given one that has a different
         version of the package.
 
+        Above 2 functions in this endpoint are deprecated. Please use
+        rpc/find-compose-by-release-rpm/{release_id}/{rpm_name>}
+        and
+        rpc/find-latest-compose-by-compose-rpm/{compose_id}/{rpm_name}
+        instead.
+
         The ordering of composes is performed by the *productmd* library. It
-        first compares compose date, then compose type (`test` < `nightly` <
-        `production`) and lastly respin.
+        first compares compose date, then compose type
+        (`test` < `nightly` < `production`) and lastly respin.
 
         `to_dict` is optional parameter, accepted values (True, 'true', 't', 'True', '1'),
         or (False, 'false', 'f', 'False', '0'). If it is provided, and the value is True,
@@ -1378,9 +1376,9 @@ def list(self, request):
             return Response(status=status.HTTP_400_BAD_REQUEST,
                             data={'detail': 'The rpm_name is required.'})
         if self.release_id:
-            return Response(self._get_compose_for_release())
+            return Response(self._get_compose_for_release(self.to_dict))
         if self.compose_id:
-            return Response(self._get_older_compose())
+            return Response(self._get_older_compose(self.to_dict))
         if self.product_version:
             return Response(self._get_compose_for_product_version())
         return Response(status=status.HTTP_400_BAD_REQUEST,
@@ -1394,5 +1392,5 @@ def _get_compose_for_product_version(self):
             composes = Compose.objects.filter(release=release)
             composes = self._filter_by_compose_type(composes)
             all_composes.extend(composes)
-        result = self._get_result(all_composes, result)
+        result = self._get_result(all_composes, result, self.to_dict)
         return result

pdc/routers.py

@@ -101,11 +101,11 @@
 router.register(r'rpc/where-to-file-bugs', compose_views.FilterBugzillaProductsAndComponents,
                 base_name='bugzilla')
 
-router.register('rpc/find_compose_by_release_rpm/(?P<release_id>[^/]+)/(?P<rpm_name>[^/]+)',
+router.register('rpc/find-compose-by-release-rpm/(?P<release_id>[^/]+)/(?P<rpm_name>[^/]+)',
                 compose_views.FindComposeByReleaseRPMViewSet,
                 base_name='findcomposebyrr')
 
-router.register('rpc/find_latest_compose_by_compose_rpm/(?P<compose_id>[^/]+)/(?P<rpm_name>[^/]+)',
+router.register('rpc/find-latest-compose-by-compose-rpm/(?P<compose_id>[^/]+)/(?P<rpm_name>[^/]+)',
                 compose_views.FindLatestComposeByComposeRPMViewSet,
                 base_name='findlatestcomposebycr')