From d50cf71357497c9b1e47531475a72d8002e26324 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 13 Nov 2023 12:48:41 -0800 Subject: [PATCH 1/8] Create methods to update package fields Signed-off-by: Jono Yang --- packagedb/models.py | 94 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packagedb/models.py b/packagedb/models.py index 3e9a8b1c..fb5514eb 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -26,6 +26,8 @@ from packageurl.contrib.django.models import PackageURLMixin from packageurl.contrib.django.models import PackageURLQuerySetMixin +from packagedcode.models import normalize_qualifiers +from dateutil.parser import parse as dateutil_parse TRACE = False logger = logging.getLogger(__name__) @@ -625,6 +627,98 @@ def rescan(self): if scannable_uri: scannable_uri.rescan() + def update_field(self, field, value, save=False): + # Ensure the incoming value is of the correct type + if field == 'qualifiers' and isinstance(value, dict): + value = normalize_qualifiers(value, encode=True) + + date_fields = [ + 'created_date', + 'last_indexed_date', + 'last_modified_date', + 'release_date', + ] + if field in date_fields and isinstance(value, str): + value = dateutil_parse(value) + + # Update field value if it differs from the existing value + updated_field = None + history_entries = [] + package_value = getattr(self, field) + if package_value != value: + setattr(self, field, value) + # Update history + if field in date_fields: + p_val = str(p_val) + value = str(value) + entry = dict( + field=field, + old_value=p_val, + new_value=value, + ) + updated_field = field + history_entries.append(entry) + + if updated_field: + data = { + 'updated_fields': history_entries, + } + self.append_to_history( + 'Package field values have been updated.', + data=data, + ) + + if save: + self.save() + + return self, updated_field + + def update_fields(self, save=False, **values_by_fields): + updated_fields = [] + history_entries = [] + for field, value in values_by_fields.items(): + # Ensure the incoming value is of the correct type + if field == 'qualifiers' and isinstance(value, dict): + value = normalize_qualifiers(value, encode=True) + + date_fields = [ + 'created_date', + 'last_indexed_date', + 'last_modified_date', + 'release_date', + ] + if field in date_fields and isinstance(value, str): + value = dateutil_parse(value) + + package_value = getattr(self, field) + if package_value != value: + setattr(self, field, value) + # Update history + if field in date_fields: + p_val = str(p_val) + value = str(value) + entry = dict( + field=field, + old_value=p_val, + new_value=value, + ) + updated_fields.append(field) + history_entries.append(entry) + + if updated_fields: + data = { + 'updated_fields': history_entries, + } + self.append_to_history( + 'Package field values have been updated.', + data=data, + ) + + if save: + self.save() + + return self, updated_fields + party_person = 'person' # often loosely defined From 1a00a3bdcbc16e540e778b3485c227cf036ced65 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 13 Nov 2023 16:02:14 -0800 Subject: [PATCH 2/8] Add tests Signed-off-by: Jono Yang --- packagedb/models.py | 9 ++-- packagedb/tests/test_models.py | 82 ++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/packagedb/models.py b/packagedb/models.py index fb5514eb..48fd9e05 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -76,7 +76,6 @@ def paginated(self, per_page=5000): yield object - VCS_CHOICES = [ ('git', 'git'), ('svn', 'subversion'), @@ -649,11 +648,11 @@ def update_field(self, field, value, save=False): setattr(self, field, value) # Update history if field in date_fields: - p_val = str(p_val) + package_value = str(package_value) value = str(value) entry = dict( field=field, - old_value=p_val, + old_value=package_value, new_value=value, ) updated_field = field @@ -695,11 +694,11 @@ def update_fields(self, save=False, **values_by_fields): setattr(self, field, value) # Update history if field in date_fields: - p_val = str(p_val) + package_value = str(package_value) value = str(value) entry = dict( field=field, - old_value=p_val, + old_value=package_value, new_value=value, ) updated_fields.append(field) diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index 74bd261e..dbb50ad2 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -141,3 +141,85 @@ def test_packagedb_package_model_get_latest_version(self): self.assertEqual(p3, p2.get_latest_version()) self.assertEqual(p3, p3.get_latest_version()) self.assertEqual(p4, p4.get_latest_version()) + + def test_packagedb_package_model_update_field(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + self.assertFalse(p1.history) + self.assertEquals('', p1.namespace) + package, updated_field = p1.update_field(field='namespace', value='test') + self.assertEqual(updated_field, 'namespace') + self.assertEqual('test', p1.namespace) + self.assertEqual(1, len(p1.history)) + expected_history_entry = { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': + [ + { + 'field': 'namespace', + 'old_value': '', + 'new_value': 'test' + } + ] + } + } + history_entry = p1.history[0] + history_entry.pop('timestamp') + self.assertEqual(expected_history_entry, history_entry) + + def test_packagedb_package_model_update_field(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + self.assertFalse(p1.history) + self.assertEquals('', p1.namespace) + package, updated_field = p1.update_field(field='namespace', value='test') + self.assertEqual(updated_field, 'namespace') + self.assertEqual('test', p1.namespace) + self.assertEqual(1, len(p1.history)) + expected_history_entry = { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': + [ + { + 'field': 'namespace', + 'old_value': '', + 'new_value': 'test' + } + ] + } + } + history_entry = p1.history[0] + history_entry.pop('timestamp') + self.assertEqual(expected_history_entry, history_entry) + + def test_packagedb_package_model_update_fields(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + self.assertFalse(p1.history) + self.assertEquals('', p1.namespace) + self.assertEquals(None, p1.homepage_url) + package, updated_fields = p1.update_fields(namespace='test', homepage_url='https://example.com') + self.assertEqual(updated_fields, ['namespace', 'homepage_url']) + self.assertEqual('test', p1.namespace) + self.assertEqual('https://example.com', p1.homepage_url) + self.assertEqual(1, len(p1.history)) + expected_history_entry = { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': + [ + { + 'field': 'namespace', + 'old_value': '', + 'new_value': 'test' + }, + { + 'field': 'homepage_url', + 'old_value': None, + 'new_value': 'https://example.com' + } + ] + } + } + history_entry = p1.history[0] + history_entry.pop('timestamp') + self.assertEqual(expected_history_entry, history_entry) From 23b75fa26bcf80591afc77b1f059929034f3f2ea Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Thu, 16 Nov 2023 16:21:53 -0800 Subject: [PATCH 3/8] Refactor update_fields and related methods * Update test expectations Signed-off-by: Jono Yang --- packagedb/models.py | 99 +++++++++++++++++----------------- packagedb/tests/test_models.py | 7 ++- 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/packagedb/models.py b/packagedb/models.py index 48fd9e05..113dcbc8 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -626,7 +626,12 @@ def rescan(self): if scannable_uri: scannable_uri.rescan() - def update_field(self, field, value, save=False): + def _update_field(self, field, value): + """ + Update `field` of this Package with a value of `value` and return a + 3-tuple containing this Package, a list of fields that were updated, and + a list of history entries. + """ # Ensure the incoming value is of the correct type if field == 'qualifiers' and isinstance(value, dict): value = normalize_qualifiers(value, encode=True) @@ -640,25 +645,32 @@ def update_field(self, field, value, save=False): if field in date_fields and isinstance(value, str): value = dateutil_parse(value) - # Update field value if it differs from the existing value - updated_field = None history_entries = [] + # Update Package value package_value = getattr(self, field) - if package_value != value: - setattr(self, field, value) - # Update history - if field in date_fields: - package_value = str(package_value) - value = str(value) - entry = dict( - field=field, - old_value=package_value, - new_value=value, - ) - updated_field = field - history_entries.append(entry) + setattr(self, field, value) + + # Update history + if field in date_fields: + package_value = str(package_value) + value = str(value) + entry = dict( + field=field, + old_value=package_value, + new_value=value, + ) + updated_fields = [field, 'history'] + history_entries.append(entry) + return self, updated_fields, history_entries + + def update_field(self, field, value, save=False): + """ + Update `field` of this Package with a value of `value` and return a + 2-tuple containing this Package and a list of fields that were updated. + """ + package, updated_fields, history_entries = self._update_field(field=field, value=value) - if updated_field: + if updated_fields: data = { 'updated_fields': history_entries, } @@ -668,41 +680,32 @@ def update_field(self, field, value, save=False): ) if save: - self.save() + package.save() - return self, updated_field + return package, updated_fields def update_fields(self, save=False, **values_by_fields): + """ + Given keyword arguments from `values_by_fields`, where the argument + names passed into the method are the fields of this Package to be + updated and the values are the new values to be set (e.g. + path="/new/path", sha1="newsha1"), update all the fields and return a + 2-tuple containing this Package and a list containing the fields that + were updated. + """ updated_fields = [] history_entries = [] for field, value in values_by_fields.items(): - # Ensure the incoming value is of the correct type - if field == 'qualifiers' and isinstance(value, dict): - value = normalize_qualifiers(value, encode=True) - - date_fields = [ - 'created_date', - 'last_indexed_date', - 'last_modified_date', - 'release_date', - ] - if field in date_fields and isinstance(value, str): - value = dateutil_parse(value) - - package_value = getattr(self, field) - if package_value != value: - setattr(self, field, value) - # Update history - if field in date_fields: - package_value = str(package_value) - value = str(value) - entry = dict( - field=field, - old_value=package_value, - new_value=value, - ) - updated_fields.append(field) - history_entries.append(entry) + ( + package, + uf, + he + ) = self._update_field(field=field, value=value) + updated_fields.extend(uf) + history_entries.extend(he) + + # Deduplicate field names + updated_fields = list(set(updated_fields)) if updated_fields: data = { @@ -714,9 +717,9 @@ def update_fields(self, save=False, **values_by_fields): ) if save: - self.save() + package.save() - return self, updated_fields + return package, updated_fields party_person = 'person' diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index dbb50ad2..29b4de6d 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -172,7 +172,7 @@ def test_packagedb_package_model_update_field(self): self.assertFalse(p1.history) self.assertEquals('', p1.namespace) package, updated_field = p1.update_field(field='namespace', value='test') - self.assertEqual(updated_field, 'namespace') + self.assertEqual(updated_field, ['namespace', 'history']) self.assertEqual('test', p1.namespace) self.assertEqual(1, len(p1.history)) expected_history_entry = { @@ -198,7 +198,10 @@ def test_packagedb_package_model_update_fields(self): self.assertEquals('', p1.namespace) self.assertEquals(None, p1.homepage_url) package, updated_fields = p1.update_fields(namespace='test', homepage_url='https://example.com') - self.assertEqual(updated_fields, ['namespace', 'homepage_url']) + self.assertEqual( + sorted(updated_fields), + sorted(['homepage_url', 'history', 'namespace']) + ) self.assertEqual('test', p1.namespace) self.assertEqual('https://example.com', p1.homepage_url) self.assertEqual(1, len(p1.history)) From 695913f622853c97bc56dceb941e699007178be1 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Thu, 16 Nov 2023 18:21:38 -0800 Subject: [PATCH 4/8] Add test for update_fields special cases Signed-off-by: Jono Yang --- packagedb/tests/test_models.py | 105 +++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index 29b4de6d..b40d6a13 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -7,6 +7,8 @@ # See https://aboutcode.org for more information about nexB OSS projects. # +from dateutil.parser import parse as dateutil_parse + from django.db import IntegrityError from django.test import TransactionTestCase from django.utils import timezone @@ -226,3 +228,106 @@ def test_packagedb_package_model_update_fields(self): history_entry = p1.history[0] history_entry.pop('timestamp') self.assertEqual(expected_history_entry, history_entry) + + def test_packagedb_package_model_update_fields_special_cases(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + # Test dates + date_fields = [ + 'created_date', + 'last_indexed_date', + 'release_date', + ] + for field in date_fields: + value = getattr(p1, field) + self.assertEqual(None, value) + timestamp_str = '2017-03-25T14:39:00+00:00' + package, updated_fields = p1.update_fields( + **{field: timestamp_str for field in date_fields} + ) + timestamp = dateutil_parse(timestamp_str) + for field in date_fields: + value = getattr(package, field) + self.assertEqual(timestamp, value) + self.assertEqual( + sorted(updated_fields), + sorted(date_fields + ['history']) + ) + + # Test qualifiers + self.assertEqual('', p1.qualifiers) + dict_qualifiers1 = { + 'classifier': 'sources', + 'type': 'war', + } + string_qualifiers1='classifier=sources&type=war' + package, updated_fields = p1.update_field('qualifiers', dict_qualifiers1) + self.assertEqual( + sorted(['qualifiers', 'history']), + sorted(updated_fields), + ) + self.assertEqual( + string_qualifiers1, + p1.qualifiers + ) + string_qualifiers2='classifier=somethingelse' + package, updated_fields = p1.update_field('qualifiers', string_qualifiers2) + self.assertEqual( + sorted(['qualifiers', 'history']), + sorted(updated_fields), + ) + self.assertEqual( + string_qualifiers2, + p1.qualifiers, + ) + expected_history = [ + { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': [ + { + 'field': 'created_date', + 'old_value': 'None', + 'new_value': '2017-03-25 14:39:00+00:00' + }, { + 'field': 'last_indexed_date', + 'old_value': 'None', + 'new_value': '2017-03-25 14:39:00+00:00' + }, { + 'field': 'release_date', + 'old_value': 'None', + 'new_value': '2017-03-25 14:39:00+00:00' + } + ] + } + }, + { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': [ + { + 'field': 'qualifiers', + 'old_value': '', + 'new_value': 'classifier=sources&type=war' + } + ] + } + }, + { + 'message': 'Package field values have been updated.', + 'data': { + 'updated_fields': [ + { + 'field': 'qualifiers', + 'old_value': 'classifier=sources&type=war', + 'new_value': 'classifier=somethingelse' + } + ] + } + } + ] + # remove timestamp before comparison + history = [] + for entry in p1.history: + entry.pop('timestamp') + history.append(entry) + self.assertEquals(expected_history, history) From 71c3e4aa540c0a434bb1c94705145ec912b93217 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 20 Nov 2023 16:50:54 -0800 Subject: [PATCH 5/8] Handle related models in update_fields * Remove _update_field and update_field Signed-off-by: Jono Yang --- packagedb/models.py | 245 +++++++++++++++++++++++++++++++------------- 1 file changed, 171 insertions(+), 74 deletions(-) diff --git a/packagedb/models.py b/packagedb/models.py index 113dcbc8..48184695 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -7,6 +7,7 @@ # See https://aboutcode.org for more information about nexB OSS projects. # +from collections import OrderedDict import copy import logging import natsort @@ -14,20 +15,19 @@ 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.db import transaction from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from dateutil.parser import parse as dateutil_parse from licensedcode.cache import build_spdx_license_expression +from packagedcode.models import normalize_qualifiers from packageurl import PackageURL from packageurl.contrib.django.models import PackageURLMixin from packageurl.contrib.django.models import PackageURLQuerySetMixin -from packagedcode.models import normalize_qualifiers -from dateutil.parser import parse as dateutil_parse TRACE = False logger = logging.getLogger(__name__) @@ -455,6 +455,11 @@ class PackageContentType(models.IntegerChoices): DOC = 7, 'doc' +def get_class_name(obj): + """Return a string containing the class name of `obj`""" + return type(obj).__name__ + + # TODO: Figure out what ordering we want for the fields class Package( HistoryMixin, @@ -626,64 +631,6 @@ def rescan(self): if scannable_uri: scannable_uri.rescan() - def _update_field(self, field, value): - """ - Update `field` of this Package with a value of `value` and return a - 3-tuple containing this Package, a list of fields that were updated, and - a list of history entries. - """ - # Ensure the incoming value is of the correct type - if field == 'qualifiers' and isinstance(value, dict): - value = normalize_qualifiers(value, encode=True) - - date_fields = [ - 'created_date', - 'last_indexed_date', - 'last_modified_date', - 'release_date', - ] - if field in date_fields and isinstance(value, str): - value = dateutil_parse(value) - - history_entries = [] - # Update Package value - package_value = getattr(self, field) - setattr(self, field, value) - - # Update history - if field in date_fields: - package_value = str(package_value) - value = str(value) - entry = dict( - field=field, - old_value=package_value, - new_value=value, - ) - updated_fields = [field, 'history'] - history_entries.append(entry) - return self, updated_fields, history_entries - - def update_field(self, field, value, save=False): - """ - Update `field` of this Package with a value of `value` and return a - 2-tuple containing this Package and a list of fields that were updated. - """ - package, updated_fields, history_entries = self._update_field(field=field, value=value) - - if updated_fields: - data = { - 'updated_fields': history_entries, - } - self.append_to_history( - 'Package field values have been updated.', - data=data, - ) - - if save: - package.save() - - return package, updated_fields - def update_fields(self, save=False, **values_by_fields): """ Given keyword arguments from `values_by_fields`, where the argument @@ -692,22 +639,159 @@ def update_fields(self, save=False, **values_by_fields): path="/new/path", sha1="newsha1"), update all the fields and return a 2-tuple containing this Package and a list containing the fields that were updated. + + In the instance where the field to be updated is `dependencies`, + `parties`, or `resources`, then we will delete all existing related + models for that field and then create the new models from the argument + values. """ + class_name = get_class_name(self) + replaced_fields = [] updated_fields = [] history_entries = [] for field, value in values_by_fields.items(): - ( - package, - uf, - he - ) = self._update_field(field=field, value=value) - updated_fields.extend(uf) - history_entries.extend(he) - - # Deduplicate field names - updated_fields = list(set(updated_fields)) + if not hasattr(self, field): + # Raise exception when we we are given a keyword argument that + # doesn't correspond to a Package field + raise AttributeError(f"'{class_name}' has no attribute '{field}'") + + related_model_fields = [ + 'dependencies', + 'parties', + 'resources', + ] + if field in related_model_fields: + unsaved_models = [] + if field == 'dependencies': + for dep_data in value: + if isinstance(dep_data, (dict, OrderedDict)): + dep = DependentPackage( + package=self, + purl=dep_data.get('purl'), + extracted_requirement=dep_data.get('extracted_requirement'), + scope=dep_data.get('scope'), + is_runtime=dep_data.get('is_runtime'), + is_optional=dep_data.get('is_optional'), + is_resolved=dep_data.get('is_resolved'), + ) + elif isinstance(dep_data, DependentPackage): + dep = dep_data + else: + raise ValueError( + f"Cannot save object of type '{get_class_name(dep_data)}' to field '{field}' of type 'DependentPackage'" + ) + unsaved_models.append(dep) + + if field == 'parties': + for party_data in value: + if isinstance(party_data, (dict, OrderedDict)): + party = Party( + package=self, + type=party_data.get('type'), + role=party_data.get('role'), + name=party_data.get('name'), + email=party_data.get('email'), + url=party_data.get('url'), + ) + elif isinstance(party_data, Party): + party = party_data + else: + raise ValueError( + f"Cannot save object of type '{get_class_name(party_data)}' to field '{field}' of type 'Party'" + ) + unsaved_models.append(party) + + if field == 'resources': + for resource_data in value: + if isinstance(resource_data, (dict, OrderedDict)): + resource = Resource( + package=self, + path=resource_data.get('path'), + is_file=resource_data.get('type') == 'file', + name=resource_data.get('name'), + extension=resource_data.get('extension'), + size=resource_data.get('size'), + md5=resource_data.get('md5'), + sha1=resource_data.get('sha1'), + sha256=resource_data.get('sha256'), + mime_type=resource_data.get('mime_type'), + file_type=resource_data.get('file_type'), + programming_language=resource_data.get('programming_language'), + is_binary=resource_data.get('is_binary'), + is_text=resource_data.get('is_text'), + is_archive=resource_data.get('is_archive'), + is_media=resource_data.get('is_media'), + is_key_file=resource_data.get('is_key_file'), + ) + resource.set_scan_results(resource_data) + elif isinstance(resource_data, Resource): + resource = resource_data + else: + raise ValueError( + f"Cannot save object of type '{get_class_name(resource_data)}' to field '{field}' of type 'Resource'" + ) + unsaved_models.append(resource) + + if unsaved_models: + created_models_count = len(unsaved_models) + model_count = 0 + if field == 'dependencies': + model_count = self.dependencies.all().count() + with transaction.atomic(): + self.dependencies.all().delete() + DependentPackage.objects.bulk_create(unsaved_models) + + if field == 'parties': + model_count = self.parties.all().count() + with transaction.atomic(): + self.parties.all().delete() + Party.objects.bulk_create(unsaved_models) + + if field == 'resources': + model_count = self.resources.all().count() + with transaction.atomic(): + self.resources.all().delete() + Resource.objects.bulk_create(unsaved_models) + + msg = f"Replaced {model_count} existing entries of field '{field}' with {created_models_count} new entries." + self.append_to_history(msg) + replaced_fields.append(field) + else: + # Ensure the incoming value is of the correct type + if field == 'qualifiers' and isinstance(value, dict): + value = normalize_qualifiers(value, encode=True) + + date_fields = [ + 'created_date', + 'last_indexed_date', + 'last_modified_date', + 'release_date', + ] + if field in date_fields and isinstance(value, str): + value = dateutil_parse(value) + + # Update field value + package_value = getattr(self, field) + setattr(self, field, value) + + # Cast datetime value to a string for history + if field in date_fields: + package_value = str(package_value) + value = str(value) + + # Create entry for history + entry = dict( + field=field, + old_value=package_value, + new_value=value, + ) + history_entries.append(entry) + updated_fields.append(field) if updated_fields: + updated_fields.append('history') + # Deduplicate field names + updated_fields = list(set(updated_fields)) data = { 'updated_fields': history_entries, } @@ -716,10 +800,13 @@ def update_fields(self, save=False, **values_by_fields): data=data, ) + if replaced_fields: + updated_fields.extend(replaced_fields) + if save: - package.save() + self.save() - return package, updated_fields + return self, updated_fields party_person = 'person' @@ -783,6 +870,11 @@ class Party(models.Model): help_text=_('URL to a primary web page for this party.') ) + def to_dict(self): + from packagedb.serializers import PartySerializer + party_data = PartySerializer(self).data + return party_data + class DependentPackage(models.Model): """ @@ -834,6 +926,11 @@ class DependentPackage(models.Model): 'exact version.') ) + def to_dict(self): + from packagedb.serializers import DependentPackageSerializer + depedent_package_data = DependentPackageSerializer(self).data + return depedent_package_data + class AbstractResource(models.Model): """ From 60c5996782c66154548342662a5d13d442dbd0b3 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 20 Nov 2023 18:28:58 -0800 Subject: [PATCH 6/8] Test updating related models using update_fields Signed-off-by: Jono Yang --- packagedb/tests/test_models.py | 180 +++++++++++++++++++++++---------- 1 file changed, 128 insertions(+), 52 deletions(-) diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index b40d6a13..a83eeb80 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -13,7 +13,9 @@ from django.test import TransactionTestCase from django.utils import timezone +from packagedb.models import DependentPackage from packagedb.models import Package +from packagedb.models import Party from packagedb.models import Resource @@ -144,56 +146,6 @@ def test_packagedb_package_model_get_latest_version(self): self.assertEqual(p3, p3.get_latest_version()) self.assertEqual(p4, p4.get_latest_version()) - def test_packagedb_package_model_update_field(self): - p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') - self.assertFalse(p1.history) - self.assertEquals('', p1.namespace) - package, updated_field = p1.update_field(field='namespace', value='test') - self.assertEqual(updated_field, 'namespace') - self.assertEqual('test', p1.namespace) - self.assertEqual(1, len(p1.history)) - expected_history_entry = { - 'message': 'Package field values have been updated.', - 'data': { - 'updated_fields': - [ - { - 'field': 'namespace', - 'old_value': '', - 'new_value': 'test' - } - ] - } - } - history_entry = p1.history[0] - history_entry.pop('timestamp') - self.assertEqual(expected_history_entry, history_entry) - - def test_packagedb_package_model_update_field(self): - p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') - self.assertFalse(p1.history) - self.assertEquals('', p1.namespace) - package, updated_field = p1.update_field(field='namespace', value='test') - self.assertEqual(updated_field, ['namespace', 'history']) - self.assertEqual('test', p1.namespace) - self.assertEqual(1, len(p1.history)) - expected_history_entry = { - 'message': 'Package field values have been updated.', - 'data': { - 'updated_fields': - [ - { - 'field': 'namespace', - 'old_value': '', - 'new_value': 'test' - } - ] - } - } - history_entry = p1.history[0] - history_entry.pop('timestamp') - self.assertEqual(expected_history_entry, history_entry) - def test_packagedb_package_model_update_fields(self): p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') self.assertFalse(p1.history) @@ -260,7 +212,7 @@ def test_packagedb_package_model_update_fields_special_cases(self): 'type': 'war', } string_qualifiers1='classifier=sources&type=war' - package, updated_fields = p1.update_field('qualifiers', dict_qualifiers1) + package, updated_fields = p1.update_fields(qualifiers=dict_qualifiers1) self.assertEqual( sorted(['qualifiers', 'history']), sorted(updated_fields), @@ -270,7 +222,7 @@ def test_packagedb_package_model_update_fields_special_cases(self): p1.qualifiers ) string_qualifiers2='classifier=somethingelse' - package, updated_fields = p1.update_field('qualifiers', string_qualifiers2) + package, updated_fields = p1.update_fields(qualifiers=string_qualifiers2) self.assertEqual( sorted(['qualifiers', 'history']), sorted(updated_fields), @@ -331,3 +283,127 @@ def test_packagedb_package_model_update_fields_special_cases(self): entry.pop('timestamp') history.append(entry) self.assertEquals(expected_history, history) + + def test_packagedb_package_model_update_fields_related_models(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + path = 'asdf' + resources = [Resource(package=p1, path=path)] + p1.update_fields(resources=resources) + expected_message = "Replaced 0 existing entries of field 'resources' with 1 new entries." + self.assertEqual(1, len(p1.history)) + history_message = p1.history[0]['message'] + self.assertEqual(expected_message, history_message) + + p2 = Package.objects.create(download_url='http://b.b', name='example', version='1.0') + resources = [ + { + "path": "example.jar", + "type": "file", + "name": "example.jar", + "status": "", + "tag": "", + "extension": ".jar", + "size": 20621, + "md5": "9307296944793049edbef60784afb12d", + "sha1": "6f776af78d7eded5a1eb2870f9d81abbf690f8b8", + "sha256": "bb83934ba50c26c093d4bea5f3faead15a3e8c176dc5ec93837d6beeaa1f27e8", + "sha512": "", + "mime_type": "application/java-archive", + "file_type": "Java archive data (JAR)", + "programming_language": "", + "is_binary": True, + "is_text": False, + "is_archive": True, + "is_media": False, + "is_key_file": True, + "detected_license_expression": "", + "detected_license_expression_spdx": "", + "license_detections": [], + "license_clues": [], + "percentage_of_license_text": 0.0, + "compliance_alert": "", + "copyrights": [], + "holders": [], + "authors": [], + "package_data": [], + "for_packages": [ + + ], + "emails": [], + "urls": [], + "extra_data": {} + } + ] + p2.update_fields(resources=resources) + expected_message = "Replaced 0 existing entries of field 'resources' with 1 new entries." + self.assertEqual(1, len(p2.history)) + history_message = p2.history[0]['message'] + self.assertEqual(expected_message, history_message) + + p3 = Package.objects.create(download_url='http://foo', name='foo', version='1.0') + parties = [ + dict( + type='admin', + role='admin', + name='foo', + email='foo@foo.com', + url='foo.com', + ) + ] + p3.update_fields(parties=parties) + expected_message = "Replaced 0 existing entries of field 'parties' with 1 new entries." + self.assertEqual(1, len(p3.history)) + history_message = p3.history[0]['message'] + self.assertEqual(expected_message, history_message) + + p4 = Package.objects.create(download_url='http://bar', name='bar', version='1.0') + parties = [ + Party( + package=p4, + type='admin', + role='admin', + name='bar', + email='bar@bar.com', + url='foo.com', + ) + ] + p4.update_fields(parties=parties) + expected_message = "Replaced 0 existing entries of field 'parties' with 1 new entries." + self.assertEqual(1, len(p4.history)) + history_message = p4.history[0]['message'] + self.assertEqual(expected_message, history_message) + + p5 = Package.objects.create(download_url='http://baz', name='baz', version='1.0') + dependencies = [ + dict( + purl='pkg:baz_dep@1.0', + extracted_requirement='>1', + scope='runtime', + is_runtime=True, + is_optional=False, + is_resolved=True, + ) + ] + p5.update_fields(dependencies=dependencies) + expected_message = "Replaced 0 existing entries of field 'dependencies' with 1 new entries." + self.assertEqual(1, len(p5.history)) + history_message = p5.history[0]['message'] + self.assertEqual(expected_message, history_message) + + p6 = Package.objects.create(download_url='http://qux', name='qux', version='1.0') + dependencies = [ + DependentPackage( + package=p6, + purl='pkg:qux_dep@1.0', + extracted_requirement='>1', + scope='runtime', + is_runtime=True, + is_optional=False, + is_resolved=True, + ) + ] + p6.update_fields(dependencies=dependencies) + expected_message = "Replaced 0 existing entries of field 'dependencies' with 1 new entries." + self.assertEqual(1, len(p6.history)) + history_message = p6.history[0]['message'] + self.assertEqual(expected_message, history_message) From db75be0cbc4648689e73e729b2f678f3099ea12a Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 21 Nov 2023 16:48:17 -0800 Subject: [PATCH 7/8] Update the logic around updated_fields Signed-off-by: Jono Yang --- packagedb/models.py | 12 +++++++----- packagedb/tests/test_models.py | 36 ++++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/packagedb/models.py b/packagedb/models.py index 48184695..6feddd1c 100644 --- a/packagedb/models.py +++ b/packagedb/models.py @@ -755,7 +755,7 @@ def update_fields(self, save=False, **values_by_fields): msg = f"Replaced {model_count} existing entries of field '{field}' with {created_models_count} new entries." self.append_to_history(msg) - replaced_fields.append(field) + replaced_fields.extend([field, 'history']) else: # Ensure the incoming value is of the correct type if field == 'qualifiers' and isinstance(value, dict): @@ -788,10 +788,7 @@ def update_fields(self, save=False, **values_by_fields): history_entries.append(entry) updated_fields.append(field) - if updated_fields: - updated_fields.append('history') - # Deduplicate field names - updated_fields = list(set(updated_fields)) + if updated_fields and history_entries: data = { 'updated_fields': history_entries, } @@ -799,10 +796,15 @@ def update_fields(self, save=False, **values_by_fields): 'Package field values have been updated.', data=data, ) + updated_fields.append('history') if replaced_fields: updated_fields.extend(replaced_fields) + if updated_fields: + # Deduplicate field names + updated_fields = list(set(updated_fields)) + if save: self.save() diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index a83eeb80..5668640a 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -288,7 +288,11 @@ def test_packagedb_package_model_update_fields_related_models(self): p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') path = 'asdf' resources = [Resource(package=p1, path=path)] - p1.update_fields(resources=resources) + _, updated_fields = p1.update_fields(resources=resources) + self.assertEquals( + sorted(['resources', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'resources' with 1 new entries." self.assertEqual(1, len(p1.history)) history_message = p1.history[0]['message'] @@ -334,7 +338,11 @@ def test_packagedb_package_model_update_fields_related_models(self): "extra_data": {} } ] - p2.update_fields(resources=resources) + _, updated_fields = p2.update_fields(resources=resources) + self.assertEquals( + sorted(['resources', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'resources' with 1 new entries." self.assertEqual(1, len(p2.history)) history_message = p2.history[0]['message'] @@ -350,7 +358,11 @@ def test_packagedb_package_model_update_fields_related_models(self): url='foo.com', ) ] - p3.update_fields(parties=parties) + _, updated_fields = p3.update_fields(parties=parties) + self.assertEquals( + sorted(['parties', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'parties' with 1 new entries." self.assertEqual(1, len(p3.history)) history_message = p3.history[0]['message'] @@ -367,7 +379,11 @@ def test_packagedb_package_model_update_fields_related_models(self): url='foo.com', ) ] - p4.update_fields(parties=parties) + _, updated_fields = p4.update_fields(parties=parties) + self.assertEquals( + sorted(['parties', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'parties' with 1 new entries." self.assertEqual(1, len(p4.history)) history_message = p4.history[0]['message'] @@ -384,7 +400,11 @@ def test_packagedb_package_model_update_fields_related_models(self): is_resolved=True, ) ] - p5.update_fields(dependencies=dependencies) + _, updated_fields = p5.update_fields(dependencies=dependencies) + self.assertEquals( + sorted(['dependencies', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'dependencies' with 1 new entries." self.assertEqual(1, len(p5.history)) history_message = p5.history[0]['message'] @@ -402,7 +422,11 @@ def test_packagedb_package_model_update_fields_related_models(self): is_resolved=True, ) ] - p6.update_fields(dependencies=dependencies) + _, updated_fields = p6.update_fields(dependencies=dependencies) + self.assertEquals( + sorted(['dependencies', 'history']), + sorted(updated_fields) + ) expected_message = "Replaced 0 existing entries of field 'dependencies' with 1 new entries." self.assertEqual(1, len(p6.history)) history_message = p6.history[0]['message'] From bd1adf4621636d4ea336a4b97da76b18bdca7ce7 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 21 Nov 2023 17:10:59 -0800 Subject: [PATCH 8/8] Test exceptions Signed-off-by: Jono Yang --- packagedb/tests/test_models.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packagedb/tests/test_models.py b/packagedb/tests/test_models.py index 5668640a..ae432f3a 100644 --- a/packagedb/tests/test_models.py +++ b/packagedb/tests/test_models.py @@ -431,3 +431,17 @@ def test_packagedb_package_model_update_fields_related_models(self): self.assertEqual(1, len(p6.history)) history_message = p6.history[0]['message'] self.assertEqual(expected_message, history_message) + + def test_packagedb_package_model_update_fields_exceptions(self): + p1 = Package.objects.create(download_url='http://a.a', name='name', version='1.0') + with self.assertRaises(AttributeError): + p1.update_fields(asdf=123) + + with self.assertRaises(ValueError): + p1.update_fields(resources=[1]) + + with self.assertRaises(ValueError): + p1.update_fields(dependencies=[1]) + + with self.assertRaises(ValueError): + p1.update_fields(parties=[1])