Merge pull request #2331 from nexB/1532-add-package-extra-data-field

Add extra_data field to Package model #1532

Author: pombredanne

CHANGELOG.rst

@@ -1,8 +1,8 @@
 Changelog
 =========
 
-v21.x.x (next)
---------------
+v21.x.x (next, future)
+-----------------------
 
 Breaking API changes:
 ~~~~~~~~~~~~~~~~~~~~~
@@ -24,7 +24,7 @@ Breaking API changes:
    multiple manifests for a single package instance.
 
 
-v21.6.2
+v21.6.6
 --------
 
 Breaking API changes:
@@ -36,6 +36,7 @@ Breaking API changes:
    has been removed. Use the PYTHON_EXECUTABLE enviroment variable to point to
    alternative non-default Python executable and this on all OSes.
 
+
 Security updates:
 ~~~~~~~~~~~~~~~~~
 
@@ -47,14 +48,18 @@ Security updates:
      - pkg:pypi/lxml: (low severity, likely no impact) CVE-2021-28957
      - pkg:pypi/nltk: (low severity, likely no impact) CVE-2019-14751
      - pkg:pypi/jinja2: (low severity, likely no impact) CVE-2020-28493, CVE-2019-10906
-     - pkg:pypi/pycryptodome: (high severity) CVE-2018-15560 (dropped since no longer used)
+     - pkg:pypi/pycryptodome: (high severity) CVE-2018-15560 (dropped since no
+       longer used by pdfminer)
 
 
 Ouputs:
 ~~~~~~~
 
  - Add new YAML-formatted output. This is exactly the same data structure as for
    the JSON output
+ - The JSON output packages section has a new "extra_data" attributes which is
+   a JSON object that can contain arbitrary data that are specific to a package
+   type.
 
 
 License scanning:

src/packagedcode/__init__.py

@@ -7,6 +7,8 @@
 # See https://aboutcode.org for more information about nexB OSS projects.
 #
 
+import attr
+
 from packagedcode import about
 from packagedcode import bower
 from packagedcode import build
@@ -126,20 +128,111 @@ def get_package_class(scan_data, default=models.Package):
     return ptype_class or default
 
 
-_props = frozenset([
-    'api_data_url',
-    'repository_download_url',
-    'purl',
-    'repository_homepage_url']
-)
+def get_package_instance(scan_data):
+    """
+    Return a Package instance re-built from a mapping of ``scan_data`` native
+    Python data that has the structure of a scan. Known attributes that store a
+    list of objects are also "rehydrated" (such as models.Party).
 
+    The Package instance will use the Package subclass that supports the
+    provided package "type" when possible or the base Package class otherwise.
 
-def get_package_instance(scan_data, properties=_props):
-    """
-    Given a `scan_data` native Python mapping representing a Package, return a
-    Package object instance.
+    Unknown attributes provided in ``scan_data`` that do not exist as fields in
+    the Package class are kept as items in the Package.extra_data mapping.
+    An Exception is raised if an "unknown attribute" name already exists as
+    a Package.extra_data key.
     """
-    # remove computed properties from attributes
-    scan_data = {k: v for k, v in scan_data.items() if k not in properties}
+    # TODO: consider using a proper library for this such as cattrs,
+    # marshmallow, etc. or use the field type that we declare.
+
+    # Each of these are lists of class instances tracked here, which are stored
+    # as a list of mappings in scanc_data
+    list_field_types_by_name = {
+        'parties': models.Party,
+        'dependencies': models.DependentPackage,
+        'installed_files': models.PackageFile,
+    }
+
+    # these are computed attributes serialized on a package
+    # that should not be recreated when serializing
+    computed_attributes = set([
+        'purl',
+        'repository_homepage_url',
+        'repository_download_url',
+        'api_data_url'
+    ])
+
+    # re-hydrate lists of typed objects
     klas = get_package_class(scan_data)
-    return klas(**scan_data)
+    existing_fields = attr.fields_dict(klas)
+
+    extra_data = scan_data.get('extra_data')
+    package_data = {}
+
+    for key, value in scan_data.items():
+        if not value or key in computed_attributes:
+            continue
+
+        field = existing_fields.get(key)
+
+        if not field:
+            if key not in extra_data:
+                # keep unknown field as extra data
+                extra_data[key] = value
+                continue
+            else:
+                raise Exception(
+                    f'Invalid scan_data with duplicated key: {key}={value!r} '
+                    f'present both as attribute AND as extra_data: '
+                    f'{key}={extra_data[key]!r}'
+                )
+
+        list_field_type = list_field_types_by_name.get(key)
+        if not list_field_type:
+            # this is a plain known field
+            package_data[key] = value
+            continue
+
+        # Since we have a list_field_type, value must be a list of mappings:
+        # we transform it in a list of objects.
+
+        if not isinstance(value, list):
+            raise Exception(
+                f'Invalid scan_data with unknown data structure. '
+                f'Expected the value to be a list of dicts and not a '
+                f'{type(value)!r} for {key}={value!r}'
+            )
+
+        objects = list(_build_objects_list(values=value, klass=list_field_type))
+        package_data[key] = objects
+
+    return klas(**package_data)
+
+
+def _build_objects_list(values, klass):
+    """
+    Yield ``klass`` objects built from a ``values`` list of mappings.
+    """
+    # Since we have a list_field_type, value must be a list of mappings:
+    # we transform it in a list of objects.
+
+    if not isinstance(values, list):
+        raise Exception(
+            f'Invalid scan_data with unknown data structure. '
+            f'Expected the value to be a list of dicts and not a '
+            f'{type(values)!r} for {values!r}'
+        )
+
+    for val in values:
+        if not val:
+            continue
+
+        if not isinstance(val, dict):
+            raise Exception(
+                f'Invalid scan_data with unknown data structure. '
+                f'Expected the value to be a mapping for and not a '
+                f'{type(val)!r} for {values!r}'
+            )
+
+        yield klass.create(**val)
+

src/packagedcode/about.py

@@ -37,12 +37,8 @@ class AboutPackage(models.Package):
     def recognize(cls, location):
         yield parse(location)
 
-    @classmethod
-    def get_package_root(cls, manifest_resource, codebase):
-        # FIXME: this should have been already stored with the Package itself as extra_data
-        with io.open(manifest_resource.location, encoding='utf-8') as loc:
-            package_data = saneyaml.load(loc.read())
-        about_resource = package_data.get('about_resource')
+    def get_package_root(self, manifest_resource, codebase):
+        about_resource = self.extra_data.get('about_resource')
         if about_resource:
             manifest_resource_parent = manifest_resource.parent(codebase)
             for child in manifest_resource_parent.children(codebase):
@@ -89,7 +85,7 @@ def build_package(package_data):
         owner = repr(owner)
     parties = [models.Party(type=models.party_person, name=owner, role='owner')]
 
-    return AboutPackage(
+    about_package = AboutPackage(
         type='about',
         name=name,
         version=version,
@@ -99,3 +95,5 @@ def build_package(package_data):
         homepage_url=homepage_url,
         download_url=download_url,
     )
+    about_package.extra_data['about_resource'] = package_data.get('about_resource')
+    return about_package

src/packagedcode/alpine.py

@@ -106,7 +106,7 @@ def build_package(package_fields):
             converted_fields.update(converted)
 
     # construct the package: we ignore unknown as we added a few technical fields
-    package = AlpinePackage.create(ignore_unknown=True, **converted_fields)
+    package = AlpinePackage.create(**converted_fields)
     return package
 
 # Note handlers MUST accept **kwargs as they also receive the current data

src/packagedcode/debian.py

@@ -15,7 +15,6 @@
 from packageurl import PackageURL
 
 from commoncode import filetype
-from commoncode.datautils import String
 from packagedcode import models
 
 """
@@ -40,24 +39,16 @@ class DebianPackage(models.Package):
     mimetypes = ('application/x-archive', 'application/vnd.debian.binary-package',)
     default_type = 'deb'
 
-    multi_arch = String(
-        label='Multi-Arch',
-        help='Multi-Arch value from status file')
-
-    def to_dict(self, _detailed=False, **kwargs):
-        data = super().to_dict(_detailed=_detailed, **kwargs)
-        if _detailed:
-            #################################################
-            # populate temporary fields
-            data['multi_arch'] = self.multi_arch
-            #################################################
-        else:
-            #################################################
-            # remove temporary fields
-            data.pop('multi_arch', None)
-            #################################################
+    @property
+    def multi_arch(self):
+        """
+        Multi-Arch value from a status or spec file.
+        """
+        return self.extra_data.get('multi_arch')
 
-        return data
+    def set_multi_arch(self, value):
+        if value:
+            self.extra_data['multi_arch'] = value
 
     def populate_installed_files(self, var_lib_dpkg_info_dir):
         """
@@ -235,6 +226,8 @@ def build_package(package_data, distro='debian'):
         ('arch', package_data.get('architecture')),
     ])
 
+    package.set_multi_arch(package_data.get('multi-arch'))
+
     # mapping of top level `status` file items to the Package object field name
     plain_fields = [
         ('description', 'description'),
@@ -243,7 +236,6 @@ def build_package(package_data, distro='debian'):
         ('package', 'name'),
         ('version', 'version'),
         ('maintainer', 'maintainer'),
-        ('multi-arch', 'multi_arch'),
     ]
 
     for source, target in plain_fields: