From 362b16b0aaff01cb3f8503208043450efd56a263 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 31 Oct 2023 13:38:24 -0700 Subject: [PATCH 1/2] Remove search field and API endpoint * Do not use lowercase fields for purl fields on Package Signed-off-by: Jono Yang --- packagedb/api.py | 14 -------------- packagedb/models.py | 21 --------------------- packagedb/signals.py | 21 --------------------- packagedb/tests/test_api.py | 26 -------------------------- 4 files changed, 82 deletions(-) delete mode 100644 packagedb/signals.py diff --git a/packagedb/api.py b/packagedb/api.py index 3dd17d7e..b7ad9d6b 100644 --- a/packagedb/api.py +++ b/packagedb/api.py @@ -193,19 +193,6 @@ def filter(self, qs, value): return qs.filter(q) -class PackageSearchFilter(Filter): - def filter(self, qs, value): - try: - request = self.parent.request - except AttributeError: - return None - - if not value: - return qs - - return Package.objects.filter(search_vector=value) - - class PackageFilter(FilterSet): type = django_filters.CharFilter( lookup_expr="iexact", @@ -229,7 +216,6 @@ class PackageFilter(FilterSet): help_text="Exact SHA1. Multi-value supported.", ) purl = MultiplePackageURLFilter(label='Package URL') - search = PackageSearchFilter(label='Search') sort = OrderingFilter(fields=[ 'type', diff --git a/packagedb/models.py b/packagedb/models.py index 23588273..d628d21c 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -14,8 +14,6 @@ import uuid from django.contrib.postgres.fields import ArrayField -from django.contrib.postgres.indexes import GinIndex -from django.contrib.postgres.search import SearchVectorField from django.core.paginator import Paginator from django.db import models from django.utils import timezone @@ -465,21 +463,6 @@ class Package( uuid = models.UUIDField( verbose_name=_("UUID"), default=uuid.uuid4, unique=True, editable=False ) - type = LowerCaseField( - max_length=16, - ) - namespace = LowerCaseField( - max_length=255, - ) - name = LowerCaseField( - max_length=100, - ) - qualifiers = LowerCaseField( - max_length=1024, - ) - subpath = LowerCaseField( - max_length=200, - ) mining_level = models.PositiveIntegerField( default=0, help_text=_('A numeric indication of the highest depth and breadth ' @@ -546,8 +529,6 @@ class Package( ), ) - search_vector = SearchVectorField(null=True) - objects = PackageQuerySet.as_manager() # TODO: Think about ordering, unique together, indexes, etc. @@ -565,8 +546,6 @@ class Meta: ) ] indexes = [ - # GIN index for search performance increase - GinIndex(fields=['search_vector']), # multicolumn index for search on a whole `purl` models.Index(fields=[ 'type', 'namespace', 'name', 'version', 'qualifiers', 'subpath' diff --git a/packagedb/signals.py b/packagedb/signals.py deleted file mode 100644 index b8f375f2..00000000 --- a/packagedb/signals.py +++ /dev/null @@ -1,21 +0,0 @@ -# -# Copyright (c) nexB Inc. and others. All rights reserved. -# purldb is a trademark of nexB Inc. -# SPDX-License-Identifier: Apache-2.0 -# See http://www.apache.org/licenses/LICENSE-2.0 for the license text. -# See https://github.com/nexB/purldb for support or download. -# See https://aboutcode.org for more information about nexB OSS projects. -# - -from django.contrib.postgres.search import SearchVector -from django.db.models.signals import post_save -from django.dispatch import receiver - -from packagedb.models import Package - - -@receiver(post_save, sender=Package) -def update_search_vector(sender, instance, **kwargs): - Package.objects.filter(pk=instance.pk).update( - search_vector=SearchVector('namespace', 'name', 'version', 'download_url') - ) diff --git a/packagedb/tests/test_api.py b/packagedb/tests/test_api.py index a0c93d93..e40b055a 100644 --- a/packagedb/tests/test_api.py +++ b/packagedb/tests/test_api.py @@ -11,7 +11,6 @@ import json import os -from django.contrib.postgres.search import SearchVector from django.test import TestCase from django.urls import reverse from django.utils import timezone @@ -374,31 +373,6 @@ def test_package_api_list_endpoint_filter_by_purl_fields_ignores_case(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(1, response.data.get('count')) - def test_package_api_list_endpoint_search(self): - # Populate the search vector field. This is done via Django signals - # outside of the tests. - Package.objects.filter(uuid=self.package.uuid).update( - search_vector=SearchVector('namespace', 'name', 'version', 'download_url') - ) - - # Create a dummy package to verify search filter works. - Package.objects.create( - type='generic', - namespace='dummy-namespace', - name='dummy-name', - version='12.35', - download_url='https://dummy.com/dummy' - ) - - for key, value in self.package_data.items(): - # Skip since we only search on one field - if key not in ['namespace', 'name', 'version', 'download_url']: - continue - - response = self.client.get('/api/packages/?search={}'.format(value)) - assert response.status_code == status.HTTP_200_OK - assert response.data.get('count') == 1 - def test_package_api_retrieve_endpoint(self): response = self.client.get('/api/packages/{}/'.format(self.package.uuid)) self.assertEqual(response.status_code, status.HTTP_200_OK) From eb1dccb6eb17467a62d33e5533cdc19937e097be Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 31 Oct 2023 18:14:54 -0700 Subject: [PATCH 2/2] Update test expectations Signed-off-by: Jono Yang --- .../declared_license_search_expected.json | 6 +- .../housekeeping/example_expected.json | 6 +- .../ignore_upper_case_search_expected.json | 6 +- .../license_expression_search_expected.json | 6 +- ...packagedb_p_search__8d33bb_gin_and_more.py | 63 +++++++++++++++++++ packagedb/tests/test_api.py | 8 +-- packagedb/tests/test_models.py | 12 ---- ...cksums-enhanced-package-data-expected.json | 24 +++---- .../package-filter_by_checksums-expected.json | 24 +++---- 9 files changed, 103 insertions(+), 52 deletions(-) create mode 100644 packagedb/migrations/0079_remove_package_packagedb_p_search__8d33bb_gin_and_more.py diff --git a/minecode/tests/testfiles/housekeeping/declared_license_search_expected.json b/minecode/tests/testfiles/housekeeping/declared_license_search_expected.json index 3f6e5105..b428f472 100644 --- a/minecode/tests/testfiles/housekeeping/declared_license_search_expected.json +++ b/minecode/tests/testfiles/housekeeping/declared_license_search_expected.json @@ -2,7 +2,7 @@ { "type":"maven", "namespace":"", - "name":"foo", + "name":"Foo", "version":"", "qualifiers":"", "subpath":"", @@ -36,9 +36,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:maven/foo?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:maven/Foo?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:maven/foo", + "purl":"pkg:maven/Foo", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, diff --git a/minecode/tests/testfiles/housekeeping/example_expected.json b/minecode/tests/testfiles/housekeeping/example_expected.json index 3f6e5105..b428f472 100644 --- a/minecode/tests/testfiles/housekeeping/example_expected.json +++ b/minecode/tests/testfiles/housekeeping/example_expected.json @@ -2,7 +2,7 @@ { "type":"maven", "namespace":"", - "name":"foo", + "name":"Foo", "version":"", "qualifiers":"", "subpath":"", @@ -36,9 +36,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:maven/foo?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:maven/Foo?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:maven/foo", + "purl":"pkg:maven/Foo", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, diff --git a/minecode/tests/testfiles/housekeeping/ignore_upper_case_search_expected.json b/minecode/tests/testfiles/housekeeping/ignore_upper_case_search_expected.json index 5d00d875..192cc5b3 100644 --- a/minecode/tests/testfiles/housekeeping/ignore_upper_case_search_expected.json +++ b/minecode/tests/testfiles/housekeeping/ignore_upper_case_search_expected.json @@ -2,7 +2,7 @@ { "type":"maven", "namespace":"", - "name":"foo", + "name":"Foo", "version":"", "qualifiers":"", "subpath":"", @@ -36,9 +36,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:maven/foo?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:maven/Foo?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:maven/foo", + "purl":"pkg:maven/Foo", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, diff --git a/minecode/tests/testfiles/housekeeping/license_expression_search_expected.json b/minecode/tests/testfiles/housekeeping/license_expression_search_expected.json index 3f6e5105..b428f472 100644 --- a/minecode/tests/testfiles/housekeeping/license_expression_search_expected.json +++ b/minecode/tests/testfiles/housekeeping/license_expression_search_expected.json @@ -2,7 +2,7 @@ { "type":"maven", "namespace":"", - "name":"foo", + "name":"Foo", "version":"", "qualifiers":"", "subpath":"", @@ -36,9 +36,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:maven/foo?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:maven/Foo?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:maven/foo", + "purl":"pkg:maven/Foo", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, diff --git a/packagedb/migrations/0079_remove_package_packagedb_p_search__8d33bb_gin_and_more.py b/packagedb/migrations/0079_remove_package_packagedb_p_search__8d33bb_gin_and_more.py new file mode 100644 index 00000000..9fb7450c --- /dev/null +++ b/packagedb/migrations/0079_remove_package_packagedb_p_search__8d33bb_gin_and_more.py @@ -0,0 +1,63 @@ +# Generated by Django 4.1.2 on 2023-10-31 20:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("packagedb", "0078_alter_package_release_date"), + ] + + operations = [ + migrations.RemoveIndex( + model_name="package", + name="packagedb_p_search__8d33bb_gin", + ), + migrations.RemoveField( + model_name="package", + name="search_vector", + ), + migrations.AlterField( + model_name="package", + name="name", + field=models.CharField( + blank=True, help_text="Name of the package.", max_length=100 + ), + ), + migrations.AlterField( + model_name="package", + name="namespace", + field=models.CharField( + blank=True, + help_text="Package name prefix, such as Maven groupid, Docker image owner, GitHub user or organization, etc.", + max_length=255, + ), + ), + migrations.AlterField( + model_name="package", + name="qualifiers", + field=models.CharField( + blank=True, + help_text="Extra qualifying data for a package such as the name of an OS, architecture, distro, etc.", + max_length=1024, + ), + ), + migrations.AlterField( + model_name="package", + name="subpath", + field=models.CharField( + blank=True, + help_text="Extra subpath within a package, relative to the package root.", + max_length=200, + ), + ), + migrations.AlterField( + model_name="package", + name="type", + field=models.CharField( + blank=True, + help_text="A short code to identify the type of this package. For example: gem for a Rubygem, docker for a container, pypi for a Python Wheel or Egg, maven for a Maven Jar, deb for a Debian package, etc.", + max_length=16, + ), + ), + ] diff --git a/packagedb/tests/test_api.py b/packagedb/tests/test_api.py index e40b055a..9d58c7da 100644 --- a/packagedb/tests/test_api.py +++ b/packagedb/tests/test_api.py @@ -362,7 +362,7 @@ def test_package_api_list_endpoint_filter(self): def test_package_api_list_endpoint_filter_by_purl_fields_ignores_case(self): for key, value in self.package_data.items(): # Skip non-purl fields - if key not in ['type', 'namespace', 'name', 'version', 'qualifiers', 'subpath']: + if key not in ['type', 'namespace', 'name']: continue response = self.client.get('/api/packages/?{}={}'.format(key, value.lower())) @@ -384,7 +384,7 @@ def test_package_api_retrieve_endpoint(self): continue if key in ['type', 'namespace', 'name', 'version', 'qualifiers', 'subpath']: - self.assertEqual(value.lower(), getattr(self.package, key)) + self.assertEqual(value, getattr(self.package, key)) continue if key == 'history': @@ -640,7 +640,7 @@ def test_reindex_package(self): self.assertEqual('error', self.scannableuri.index_error) self.assertEqual(self.scan_request_date, self.scannableuri.scan_request_date) response = self.client.get(f'/api/packages/{self.package.uuid}/reindex_package/') - self.assertEqual('pkg:maven/sample/baz@90.12 has been queued for reindexing', response.data['status']) + self.assertEqual('pkg:maven/sample/Baz@90.12 has been queued for reindexing', response.data['status']) self.scannableuri.refresh_from_db() self.assertEqual(True, self.scannableuri.rescan_uri) self.assertEqual(100, self.scannableuri.priority) @@ -664,7 +664,7 @@ def test_reindex_packages_basic(self): self.assertEqual('error', self.scannableuri2.index_error) self.assertEqual(self.scan_request_date2, self.scannableuri2.scan_request_date) existing_purls = [ - 'pkg:maven/sample/baz@90.12', + 'pkg:maven/sample/Baz@90.12', 'pkg:npm/example/bar@56.78', ] nonexistent_purls = [ diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index 6d3a32dd..74bd261e 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -108,18 +108,6 @@ def setUp(self): self.created_package = Package.objects.create(**self.created_package_data) self.inserted_package = Package.objects.insert(**self.inserted_package_data) - def test_package_creation(self): - test_package = Package.objects.get(download_url=self.created_package_download_url) - self.assertIsNotNone(test_package) - for key, val in self.created_package_data.items(): - self.assertEqual(val.lower(), getattr(test_package, key)) - - def test_package_insertion(self): - test_package = Package.objects.get(download_url=self.inserted_package_download_url) - self.assertIsNotNone(test_package) - for key, val in self.inserted_package_data.items(): - self.assertEqual(val.lower(), getattr(test_package, key)) - def test_package_download_url_is_unique(self): self.assertIsNone(Package.objects.insert(download_url=self.created_package_download_url)) self.assertIsNone(Package.objects.insert(download_url=self.inserted_package_download_url)) diff --git a/packagedb/tests/testfiles/api/package-filter_by_checksums-enhanced-package-data-expected.json b/packagedb/tests/testfiles/api/package-filter_by_checksums-enhanced-package-data-expected.json index 33bc5d68..e5066387 100644 --- a/packagedb/tests/testfiles/api/package-filter_by_checksums-enhanced-package-data-expected.json +++ b/packagedb/tests/testfiles/api/package-filter_by_checksums-enhanced-package-data-expected.json @@ -2,7 +2,7 @@ { "type":"generic", "namespace":"generic", - "name":"foo", + "name":"Foo", "version":"12.34", "qualifiers":"test_qual=qual", "subpath":"test_subpath", @@ -35,9 +35,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:generic/generic/foo@12.34?test_qual=qual&uuid=fixed-uid-done-for-testing-5642512d1758#test_subpath", + "package_uid":"pkg:generic/generic/Foo@12.34?test_qual=qual&uuid=fixed-uid-done-for-testing-5642512d1758#test_subpath", "datasource_id":null, - "purl":"pkg:generic/generic/foo@12.34?test_qual=qual#test_subpath", + "purl":"pkg:generic/generic/Foo@12.34?test_qual=qual#test_subpath", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, @@ -46,7 +46,7 @@ { "type":"npm", "namespace":"example", - "name":"bar", + "name":"Bar", "version":"56.78", "qualifiers":"", "subpath":"", @@ -79,9 +79,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:npm/example/bar@56.78?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:npm/example/Bar@56.78?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:npm/example/bar@56.78", + "purl":"pkg:npm/example/Bar@56.78", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, @@ -90,7 +90,7 @@ { "type":"jar", "namespace":"sample", - "name":"baz", + "name":"Baz", "version":"90.12", "qualifiers":"", "subpath":"", @@ -123,9 +123,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:jar/sample/baz@90.12?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:jar/sample/Baz@90.12?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:jar/sample/baz@90.12", + "purl":"pkg:jar/sample/Baz@90.12", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, @@ -134,7 +134,7 @@ { "type":"jar", "namespace":"sample", - "name":"baz", + "name":"Baz", "version":"90.123", "qualifiers":"", "subpath":"", @@ -167,9 +167,9 @@ "source_packages":[], "extra_data":{}, "dependencies":[], - "package_uid":"pkg:jar/sample/baz@90.123?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:jar/sample/Baz@90.123?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, - "purl":"pkg:jar/sample/baz@90.123", + "purl":"pkg:jar/sample/Baz@90.123", "repository_homepage_url":null, "repository_download_url":null, "api_data_url":null, diff --git a/packagedb/tests/testfiles/api/package-filter_by_checksums-expected.json b/packagedb/tests/testfiles/api/package-filter_by_checksums-expected.json index 4fd07d81..92337a85 100644 --- a/packagedb/tests/testfiles/api/package-filter_by_checksums-expected.json +++ b/packagedb/tests/testfiles/api/package-filter_by_checksums-expected.json @@ -2,10 +2,10 @@ { "filename":"Foo.zip", "package_content":null, - "purl":"pkg:generic/generic/foo@12.34?test_qual=qual#test_subpath", + "purl":"pkg:generic/generic/Foo@12.34?test_qual=qual#test_subpath", "type":"generic", "namespace":"generic", - "name":"foo", + "name":"Foo", "version":"12.34", "qualifiers":"test_qual=qual", "subpath":"test_subpath", @@ -39,7 +39,7 @@ "notice_text":null, "source_packages":[], "extra_data":{}, - "package_uid":"pkg:generic/generic/foo@12.34?test_qual=qual&uuid=fixed-uid-done-for-testing-5642512d1758#test_subpath", + "package_uid":"pkg:generic/generic/Foo@12.34?test_qual=qual&uuid=fixed-uid-done-for-testing-5642512d1758#test_subpath", "datasource_id":null, "file_references":[], "dependencies":[] @@ -47,10 +47,10 @@ { "filename":"Bar.zip", "package_content":null, - "purl":"pkg:npm/example/bar@56.78", + "purl":"pkg:npm/example/Bar@56.78", "type":"npm", "namespace":"example", - "name":"bar", + "name":"Bar", "version":"56.78", "qualifiers":"", "subpath":"", @@ -84,7 +84,7 @@ "notice_text":null, "source_packages":[], "extra_data":{}, - "package_uid":"pkg:npm/example/bar@56.78?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:npm/example/Bar@56.78?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, "file_references":[], "dependencies":[] @@ -92,10 +92,10 @@ { "filename":"Baz.zip", "package_content":null, - "purl":"pkg:jar/sample/baz@90.12", + "purl":"pkg:jar/sample/Baz@90.12", "type":"jar", "namespace":"sample", - "name":"baz", + "name":"Baz", "version":"90.12", "qualifiers":"", "subpath":"", @@ -129,7 +129,7 @@ "notice_text":null, "source_packages":[], "extra_data":{}, - "package_uid":"pkg:jar/sample/baz@90.12?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:jar/sample/Baz@90.12?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, "file_references":[], "dependencies":[] @@ -137,10 +137,10 @@ { "filename":"Baz.zip", "package_content":"binary", - "purl":"pkg:jar/sample/baz@90.123", + "purl":"pkg:jar/sample/Baz@90.123", "type":"jar", "namespace":"sample", - "name":"baz", + "name":"Baz", "version":"90.123", "qualifiers":"", "subpath":"", @@ -174,7 +174,7 @@ "notice_text":null, "source_packages":[], "extra_data":{}, - "package_uid":"pkg:jar/sample/baz@90.123?uuid=fixed-uid-done-for-testing-5642512d1758", + "package_uid":"pkg:jar/sample/Baz@90.123?uuid=fixed-uid-done-for-testing-5642512d1758", "datasource_id":null, "file_references":[], "dependencies":[]