Add option to get enhanced package data in API #157

TG1999 · 2023-08-02T08:59:31Z

No description provided.

tdruez · 2023-08-03T16:08:22Z

packagedb/serializers.py

+    def to_representation(self, instance):
+        data = super().to_representation(instance)
+        request = self.context["request"]
+        if hasattr(request, "query_params") and request.query_params.get("enhanced", False):
+            from packagedb.api import get_enhanced_package
+            data["enhanced_package"] = get_enhanced_package(instance)
+        return data


This looks like a hack. Why not use a proper field on the serializer?

JonoYang

@TG1999

Please take a look at the new API endpoint on packages: https://github.com/nexB/purldb/blob/main/packagedb/api.py#L470

We will want to add the function to enhance package data there, rather than overriding to_representation().

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

JonoYang

@TG1999 I recommended some changes. I noticed that when the enhanced package option is enabled in filter_by_checksums, that packages that are not enhanced are skipped and not returned. I think it may be good for us to have get_enhanced_package to return package data , even if it was not enhanced.

@pombredanne Does this sound like a good idea?

JonoYang · 2023-08-08T00:02:04Z

packagedb/tests/test_api.py

+            'sha1': 'testsha1-4',
+            'md5': 'testmd5-3',
+            'size': 100,
+            'package_content': 5,


Import PackageContentType and use PackageContentType.BINARY instead of 5.

JonoYang · 2023-08-08T00:09:59Z

packagedb/tests/test_api.py

@@ -265,14 +265,31 @@ def setUp(self):
        self.package3 = Package.objects.create(**self.package_data3)
        self.package3.refresh_from_db()

+        self.package_data4= {


I think you should add two new maven packages, one of them a binary package, the other a source_archive package. These packages should have the same purl fields (type, namespace, name, version) with the qualifiers being different. On the binary package, leave out license information, and on the source archive package, have license information. We want to see the enhanced package data returned for the binary package in the test results when we enable the option.

JonoYang · 2023-08-08T00:11:24Z

packagedb/api.py

@@ -522,6 +522,12 @@ def filter_by_checksums(self, request, *args, **kwargs):

        qs = Package.objects.filter(q)
        paginated_qs = self.paginate_queryset(qs)
+        get_enhanced_package_data = request.query_params.get('get_enhanced_package_data', False)


Lets use enhance_package_data as the option name.

pombredanne · 2023-08-08T09:17:53Z

@pombredanne Does this sound like a good idea?

@TG1999 I recommended some changes. I noticed that when the enhanced package option is enabled in filter_by_checksums, that packages that are not enhanced are skipped and not returned. I think it may be good for us to have get_enhanced_package to return package data , even if it was not enhanced.

@pombredanne Does this sound like a good idea?

This is a must. By default we should always return something, enhanced or not. And we should never silently not return things that were requested.

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

JonoYang

@TG1999 There are some things left to change. Also, I think we are missing the changes to https://github.com/nexB/purldb/blob/main/packagedb/models.py#L1061

JonoYang · 2023-08-08T23:38:19Z

packagedb/api.py

@@ -522,6 +522,17 @@ def filter_by_checksums(self, request, *args, **kwargs):

        qs = Package.objects.filter(q)
        paginated_qs = self.paginate_queryset(qs)
+        enhance_package_data = request.query_params.get('enhance_package_data', False)


I think the enhance_package_data flag should be sent along with the data in the post request, rather than using query_params here

Suggested change

enhance_package_data = request.query_params.get('enhance_package_data', False)

enhance_package_data = data.get('enhance_package_data', False)

JonoYang · 2023-08-08T23:38:46Z

packagedb/tests/test_api.py

        expected = self.get_test_loc('api/package-filter_by_checksums-expected.json')
-        self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources"], regen=False)
+        self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)


Suggested change

self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)

self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=False)

JonoYang · 2023-08-08T23:38:56Z

packagedb/tests/test_api.py

+        enhanced_response = self.client.post('/api/packages/filter_by_checksums/?enhance_package_data=true', data=data)
+        self.assertEqual(5, len(enhanced_response.data['results']))
+        expected = self.get_test_loc('api/package-filter_by_checksums-enhanced-package-data-expected.json')
+        self.check_expected_results(enhanced_response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)


Suggested change

self.check_expected_results(enhanced_response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)

self.check_expected_results(enhanced_response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=False)

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

JonoYang

@TG1999 Thanks for addressing the review comments. I have a last thing for you to do on the PR.

JonoYang · 2023-08-09T17:02:43Z

packagedb/api.py

@@ -517,11 +517,24 @@ def filter_by_checksums(self, request, *args, **kwargs):
        for field, value in data.items():


I think it would be better to pop enhance_package_data from data before iterating through it, then use enhance_package_data afterwards.

Suggested change

for field, value in data.items():

enhance_package_data = data.pop('enhance_package_data', False)

for field, value in data.items():

JonoYang · 2023-08-09T17:02:57Z

packagedb/api.py

@@ -517,11 +517,24 @@ def filter_by_checksums(self, request, *args, **kwargs):
        for field, value in data.items():
            # We create this intermediate dictionary so we can modify the field
            # name to have __in at the end
+            if field in ["enhance_package_data"]:


Suggested change

if field in ["enhance_package_data"]:

JonoYang · 2023-08-09T17:03:23Z

packagedb/api.py

@@ -517,11 +517,24 @@ def filter_by_checksums(self, request, *args, **kwargs):
        for field, value in data.items():
            # We create this intermediate dictionary so we can modify the field
            # name to have __in at the end
+            if field in ["enhance_package_data"]:
+                continue


Suggested change

continue

JonoYang · 2023-08-09T17:09:08Z

packagedb/api.py

            d = {f'{field}__in': value}
            q |= Q(**d)

        qs = Package.objects.filter(q)
        paginated_qs = self.paginate_queryset(qs)
+        enhance_package_data = data.get("enhance_package_data", False)


Remove this, since we're going to pop enhance_package_data from data above.

Suggested change

enhance_package_data = data.get("enhance_package_data", False)

* Return unenhanced package data for packages not in a package set or if they do not have package content * Refactor code in filter_by_checksums() Signed-off-by: Jono Yang <jyang@nexb.com>

JonoYang · 2023-08-09T20:41:59Z

@TG1999 I've made the changes and committed them to your branch, and I will merge it into main. Thank you again for your PR!

TG1999 changed the title ~~Add option to enhanced package data in API~~ Add option to get enhanced package data in API Aug 2, 2023

TG1999 requested review from pombredanne and JonoYang August 3, 2023 12:28

tdruez reviewed Aug 3, 2023

View reviewed changes

JonoYang requested changes Aug 3, 2023

View reviewed changes

TG1999 added 2 commits August 4, 2023 14:56

Add option to enhanced package data in API

0937a62

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Address review comments

b34edf8

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

TG1999 force-pushed the add_enhanced_package_data_in_api branch from d8155cb to b34edf8 Compare August 7, 2023 14:21

Change regen to False

d96a719

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

TG1999 requested a review from JonoYang August 7, 2023 17:13

JonoYang requested changes Aug 8, 2023

View reviewed changes

Change get_enhanced_package logic and add tests

d510cd7

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

JonoYang requested changes Aug 8, 2023

View reviewed changes

Address review comments

4815a74

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

JonoYang requested changes Aug 9, 2023

View reviewed changes

Update get_enhanced_package

5cc124c

* Return unenhanced package data for packages not in a package set or if they do not have package content * Refactor code in filter_by_checksums() Signed-off-by: Jono Yang <jyang@nexb.com>

JonoYang force-pushed the add_enhanced_package_data_in_api branch from ea2d591 to 5cc124c Compare August 9, 2023 20:16

JonoYang merged commit 7a9e4f9 into main Aug 9, 2023
8 checks passed

JonoYang deleted the add_enhanced_package_data_in_api branch August 9, 2023 22:46

TG1999 mentioned this pull request Aug 11, 2023

Use enhanced PurlDB "package set" lookup in d2d pipeline aboutcode-org/scancode.io#829

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to get enhanced package data in API #157

Add option to get enhanced package data in API #157

TG1999 commented Aug 2, 2023

tdruez Aug 3, 2023

JonoYang left a comment

JonoYang left a comment

JonoYang Aug 8, 2023

JonoYang Aug 8, 2023

JonoYang Aug 8, 2023

pombredanne commented Aug 8, 2023

JonoYang left a comment

JonoYang Aug 8, 2023

JonoYang Aug 8, 2023

JonoYang Aug 8, 2023

JonoYang left a comment

JonoYang Aug 9, 2023

JonoYang Aug 9, 2023

JonoYang Aug 9, 2023

JonoYang Aug 9, 2023

JonoYang commented Aug 9, 2023

	enhance_package_data = request.query_params.get('enhance_package_data', False)
	enhance_package_data = data.get('enhance_package_data', False)

	self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)
	self.check_expected_results(response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=False)

	self.check_expected_results(enhanced_response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=True)
	self.check_expected_results(enhanced_response.data['results'], expected, fields_to_remove=["url", "uuid", "resources", "package_sets",], regen=False)

		@@ -517,11 +517,24 @@ def filter_by_checksums(self, request, args, *kwargs):
		for field, value in data.items():

	for field, value in data.items():
	enhance_package_data = data.pop('enhance_package_data', False)
	for field, value in data.items():

Add option to get enhanced package data in API #157

Add option to get enhanced package data in API #157

Conversation

TG1999 commented Aug 2, 2023

Choose a reason for hiding this comment

JonoYang left a comment

Choose a reason for hiding this comment

JonoYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pombredanne commented Aug 8, 2023

JonoYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonoYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonoYang commented Aug 9, 2023