Skip to content

Commit

Permalink
Add override_unknown option in update_from_data method #202
Browse files Browse the repository at this point in the history
Signed-off-by: tdruez <[email protected]>
  • Loading branch information
tdruez committed Dec 13, 2024
1 parent 1a4f703 commit e290a82
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ Release notes
Add a VulnerabilityAnalysis REST API endpoint.
https://github.com/aboutcode-org/dejacode/issues/103

- Add `override_unknown` option in `update_from_data` method.
This is enabled in the context of updating Package data from the PurlDB, a Scan, and
an import. Fields with "unknown" values will be override with values available in the
new `data` dict.
https://github.com/aboutcode-org/dejacode/issues/202

### Version 5.2.1

- Fix the models documentation navigation.
Expand Down
7 changes: 6 additions & 1 deletion component_catalog/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2489,7 +2489,12 @@ def update_from_purldb(self, user):
package_data["release_date"] = release_date.split("T")[0]
package_data["license_expression"] = package_data.get("declared_license_expression")

updated_fields = self.update_from_data(user, package_data, override=False)
updated_fields = self.update_from_data(
user,
package_data,
override=False,
override_unknown=True,
)
return updated_fields


Expand Down
23 changes: 21 additions & 2 deletions component_catalog/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,20 @@ def test_package_model_update_from_data(self):
updated_fields = package.update_from_data(user=None, data=new_data, override=True)
self.assertEqual(["filename"], updated_fields)

package.update(declared_license_expression="unknown")
new_data = {"declared_license_expression": "apache-2.0"}
updated_fields = package.update_from_data(
user=None, data=new_data, override=False, override_unknown=False
)
self.assertEqual([], updated_fields)

updated_fields = package.update_from_data(
user=None, data=new_data, override=False, override_unknown=True
)
self.assertEqual(["declared_license_expression"], updated_fields)
package.refresh_from_db()
self.assertEqual("apache-2.0", package.declared_license_expression)

@mock.patch("component_catalog.models.collect_package_data")
def test_package_model_create_from_url(self, mock_collect):
self.assertIsNone(Package.create_from_url(url=" ", user=self.user))
Expand Down Expand Up @@ -2540,11 +2554,16 @@ def test_package_model_update_from_purldb(self, mock_get_purldb_entries):
"sha1": "96ae8d8dd673d4fc92ce2cb2df9cdab6f6fd7d9f",
"sha256": "0a1efde1b685a6c30999ba00902f23613cf5db864c5a1532d2edf3eda7896a37",
"copyright": "(c) Copyright",
"declared_license_expression": "(bsd-simplified AND bsd-new) AND unknown",
"declared_license_expression": "(bsd-simplified AND bsd-new)",
}

mock_get_purldb_entries.return_value = [purldb_entry]
package1 = Package.objects.create(filename="package", dataspace=self.dataspace)
package1 = Package.objects.create(
filename="package",
dataspace=self.dataspace,
# "unknown" values are overrided
declared_license_expression="unknown",
)
updated_fields = package1.update_from_purldb(self.user)
# Note: PURL fields are never updated.
expected = [
Expand Down
7 changes: 6 additions & 1 deletion dejacode_toolkit/scancodeio.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,12 @@ def update_from_scan(self, package, user):
values_from_scan["notice_text"] = notice_text

if values_from_scan:
updated_fields = package.update_from_data(user, values_from_scan)
updated_fields = package.update_from_data(
user,
values_from_scan,
override=False,
override_unknown=True,
)
if updated_fields:
msg = f'Automatically updated {", ".join(updated_fields)} from scan results'
logger.debug(f"{self.label}: {msg}")
Expand Down
7 changes: 6 additions & 1 deletion dje/importers.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,12 @@ def save_form(self, form):
# We need to refresh the instance from the db because form.instance has
# the unsaved form.cleaned_data modification at that stage.
instance.refresh_from_db()
updated_fields = instance.update_from_data(self.user, form.cleaned_data, override=False)
updated_fields = instance.update_from_data(
self.user,
form.cleaned_data,
override=False,
override_unknown=True,
)
if updated_fields:
self.results["modified"].append(instance)
msg = f'Updated {", ".join(updated_fields)} from import'
Expand Down
16 changes: 14 additions & 2 deletions dje/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ def create_from_data(cls, user, data, validate=False):

return instance

def update_from_data(self, user, data, override=False):
def update_from_data(self, user, data, override=False, override_unknown=False):
"""
Update this object instance with the provided `data`.
The `save()` method is called only if at least one field was modified.
Expand All @@ -798,6 +798,13 @@ def update_from_data(self, user, data, override=False):
not associated to a specific user.
We do not want to promote this as the default behavior thus we keep the user
a required parameter.
By default, only empty values will be set on the instance, protecting the
existing values.
When `override` is True, all the values from the `data` dict will be set on the
instance, erasing any existing values.
When `override_unknown` is True, field value currently set as "unknown" will be
replace by the value from the `data` dict.
"""
model_fields = self.model_fields()
updated_fields = []
Expand All @@ -812,7 +819,12 @@ def update_from_data(self, user, data, override=False):
continue

current_value = getattr(self, field_name, None)
if not current_value or (current_value != value and override):
update_conditions = [
not current_value,
current_value != value and override,
current_value == "unknown" and override_unknown,
]
if any(update_conditions):
setattr(self, field_name, value)
updated_fields.append(field_name)

Expand Down
2 changes: 1 addition & 1 deletion product_portfolio/importers.py
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ def import_package(self, package_data):
package_data["license_expression"] = license_expression

if package and self.update_existing:
package.update_from_data(self.user, package_data, override=False)
package.update_from_data(self.user, package_data, override=False, override_unknown=True)

if not package:
try:
Expand Down
4 changes: 3 additions & 1 deletion vulnerabilities/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ def fetch_for_packages(
created_vulnerabilities += 1
elif update:
updated_fields = vulnerability.update_from_data(
user=None, data=vulnerability_data, override=True
user=None,
data=vulnerability_data,
override=True,
)
if updated_fields:
updated_vulnerabilities += 1
Expand Down

0 comments on commit e290a82

Please sign in to comment.