Add test for sort on /api/collect #508

Signed-off-by: Jono Yang <[email protected]>

packagedb/api.py

@@ -829,6 +829,7 @@ def list(self, request, format=None):
 
         validated_data = serializer.validated_data
         purl = validated_data.get('purl')
+        sort = validated_data.get('sort', [])
 
         kwargs = dict()
         # We want this request to have high priority since the user knows the
@@ -841,7 +842,8 @@ def list(self, request, format=None):
         if addon_pipelines := validated_data.get('addon_pipelines', []):
             kwargs["addon_pipelines"] = addon_pipelines
 
-        packages = Package.objects.filter(**lookups)
+        lookups = purl_to_lookups(purl)
+        packages = Package.objects.filter(**lookups).order_by(*sort)
         if packages.count() == 0:
             try:
                 errors = priority_router.process(purl, **kwargs)
@@ -852,7 +854,6 @@ def list(self, request, format=None):
                 return Response(message, status=status.HTTP_400_BAD_REQUEST)
 
             lookups = purl_to_lookups(purl)
-            sort = validated_data.get('sort', [])
             packages = Package.objects.filter(**lookups).order_by(*sort)
             if packages.count() == 0:
                 message = {}
@@ -861,6 +862,7 @@ def list(self, request, format=None):
                         'status': f'error(s) occurred when fetching metadata for {purl}: {errors}'
                     }
                 return Response(message, status=status.HTTP_400_BAD_REQUEST)
+
         for package in packages:
             get_source_package_and_add_to_package_set(package)
 

packagedb/serializers.py

@@ -428,21 +428,16 @@ def validate_source_purl(self, value):
         return value
 
     def validate_addon_pipelines(self, value):
-        invalid_pipelines = [
-            pipe for pipe in value if not is_supported_addon_pipeline(pipe)]
-        if invalid_pipelines:
-            raise ValidationError(
-                f'Error unsupported addon pipelines: {",".join(invalid_pipelines)}')
-
+        if invalid_pipelines := [pipe for pipe in value if not is_supported_addon_pipeline(pipe)]:
+            raise ValidationError(f'Error unsupported addon pipelines: {",".join(invalid_pipelines)}')
         return value
 
     def validate_sort(self, value):
-        invalid_sort_fields = [field for field in value if not is_supported_sort_field(field)]
-        if invalid_sort_fields:
+        if invalid_sort_fields := [field for field in value if not is_supported_sort_field(field)]:
             raise ValidationError(f'Error unsupported sort fields: {",".join(invalid_sort_fields)}')
-
         return value
 
+
 class PackageVersSerializer(Serializer):
     purl = CharField()
     vers = CharField(required=False)
@@ -544,4 +539,5 @@ def is_supported_addon_pipeline(addon_pipeline):
 
 def is_supported_sort_field(field):
     from packagedb.api import PACKAGE_FILTER_SORT_FIELDS
-    return field in PACKAGE_FILTER_SORT_FIELDS
+    # A field could have a leading `-`
+    return field.lstrip('-') in PACKAGE_FILTER_SORT_FIELDS

packagedb/tests/test_api.py

@@ -878,7 +878,6 @@ def setUp(self):
             'size': 100,
         }
         self.package = Package.objects.create(**self.package_data)
-        self.package.refresh_from_db()
         self.scannableuri = ScannableURI.objects.create(
             package=self.package,
             uri=self.package_download_url,
@@ -906,7 +905,6 @@ def setUp(self):
             'size': 100,
         }
         self.package2 = Package.objects.create(**self.package_data2)
-        self.package2.refresh_from_db()
         self.scannableuri2 = ScannableURI.objects.create(
             package=self.package2,
             uri=self.package_download_url2,
@@ -919,6 +917,55 @@ def setUp(self):
         self.scan_request_date2 = timezone.now()
         self.scannableuri2.scan_request_date = self.scan_request_date2
 
+        self.package_download_url3 = 'http://clone.org/clone1.zip'
+        self.package_data3 = {
+            'type': 'pypi',
+            'namespace': '',
+            'name': 'clone',
+            'version': '1',
+            'qualifiers': '',
+            'subpath': '',
+            'download_url': self.package_download_url3,
+            'filename': 'clone1.zip',
+            'sha1': 'clone1',
+            'md5': '',
+            'size': 100,
+        }
+        self.package3 = Package.objects.create(**self.package_data3)
+
+        self.package_download_url4 = 'http://clone.org/clone1-src.zip'
+        self.package_data4 = {
+            'type': 'pypi',
+            'namespace': '',
+            'name': 'clone',
+            'version': '1',
+            'qualifiers': 'package=src',
+            'subpath': '',
+            'download_url': self.package_download_url4,
+            'filename': 'clone1-src.zip',
+            'sha1': 'clone1-src',
+            'md5': '',
+            'size': 50,
+        }
+        self.package4 = Package.objects.create(**self.package_data4)
+
+        self.package_download_url5 = 'http://clone.org/clone1-all.zip'
+        self.package_data5 = {
+            'type': 'pypi',
+            'namespace': '',
+            'name': 'clone',
+            'version': '1',
+            'qualifiers': 'package=all',
+            'subpath': '',
+            'download_url': self.package_download_url5,
+            'filename': 'clone1-all.zip',
+            'sha1': 'clone1-all',
+            'md5': '',
+            'size': 25,
+        }
+        self.package5 = Package.objects.create(**self.package_data5)
+
+
     def test_package_live(self):
         purl_str = 'pkg:maven/org.apache.twill/[email protected]'
         download_url = 'https://repo1.maven.org/maven2/org/apache/twill/twill-core/0.12.0/twill-core-0.12.0.jar'
@@ -993,6 +1040,23 @@ def test_package_live_works_with_purl2vcs(self):
         self.check_expected_results(
             result, expected, fields_to_remove=fields_to_remove, regen=FIXTURES_REGEN)
 
+    def test_collect_sort(self):
+        purl_str = 'pkg:pypi/clone@1'
+        response = self.client.get(f'/api/collect/?purl={purl_str}&sort=size')
+        for i, package_data in enumerate(response.data[1:], start=1):
+            prev_package_data = response.data[i-1]
+            self.assertTrue(prev_package_data['size'] < package_data['size'])
+
+        response = self.client.get(f'/api/collect/?purl={purl_str}&sort=-size')
+        for i, package_data in enumerate(response.data[1:], start=1):
+            prev_package_data = response.data[i-1]
+            self.assertTrue(prev_package_data['size'] > package_data['size'])
+
+        response = self.client.get(f'/api/collect/?purl={purl_str}&sort=-size')
+        for i, package_data in enumerate(response.data[1:], start=1):
+            prev_package_data = response.data[i-1]
+            self.assertTrue(prev_package_data['size'] > package_data['size'])
+
     def test_package_api_index_packages_endpoint(self):
         priority_resource_uris_count = PriorityResourceURI.objects.all().count()
         self.assertEqual(0, priority_resource_uris_count)