fix: multiple licenses issue #365 (#466)

breaking changes
------------------
* Reworked license related models and collections
* API
  * Removed class `factory.license.LicenseChoiceFactory`  
    The old functionality was integrated into `factory.license.LicenseFactory`.
  * Method `factory.license.LicenseFactory.make_from_string()`'s parameter `name_or_spdx` was renamed to `value`
  * Method `factory.license.LicenseFactory.make_from_string()`'s return value can also be a `LicenseExpression`
    The behavior imitates the old `factory.license.LicenseChoiceFactory.make_from_string()`
  * Renamed class `module.License` to `module.license.DisjunctliveLicense`
  * Removed class `module.LicenseChoice`
    Use dedicated classes `module.license.DisjunctliveLicense` and `module.license.LicenseExpression` instead
  * All occurrences of `models.LicenseChoice` were replaced by `models.licenses.License`
  * All occurrences of `SortedSet[LicenseChoice]` were specialized to `models.license.LicenseRepository`


fixes
------------------
* serialization of multy-licenses #365

added
------------------
* API
  * Method `factory.license.LicenseFactory.make_with_expression()`
  * Class `model.license.DisjunctiveLicense`
  * Class `model.license.LicenseExpression`
  * Class `model.license.LicenseRepository`
  * Class `serialization.LicenseRepositoryHelper`

tests
------------------
* added regression test for bug #365

misc
------------------
* raised dependency `py-serializable@^9.15`



----

fixes #365

~~BLOCKED by a feature request to serializer: <https://github.com/madpah/serializable/pull/32>~~


---------

Signed-off-by: Jan Kowalleck <[email protected]>

Author: jkowalleck

cyclonedx/factory/license.py

@@ -13,76 +13,59 @@
 # SPDX-License-Identifier: Apache-2.0
 # Copyright (c) OWASP Foundation. All Rights Reserved.
 
-from typing import Optional
+from typing import TYPE_CHECKING, Optional
 
 from ..exception.factory import InvalidLicenseExpressionException, InvalidSpdxLicenseException
-from ..model import AttachedText, License, LicenseChoice, XsUri
+from ..model.license import DisjunctiveLicense, LicenseExpression
 from ..spdx import fixup_id as spdx_fixup, is_compound_expression as is_spdx_compound_expression
 
+if TYPE_CHECKING:  # pragma: no cover
+    from ..model import AttachedText, XsUri
+    from ..model.license import License
 
-class LicenseFactory:
-    """Factory for :class:`cyclonedx.model.License`."""
-
-    def make_from_string(self, name_or_spdx: str, *,
-                         license_text: Optional[AttachedText] = None,
-                         license_url: Optional[XsUri] = None) -> License:
-        """Make a :class:`cyclonedx.model.License` from a string."""
-        try:
-            return self.make_with_id(name_or_spdx, text=license_text, url=license_url)
-        except InvalidSpdxLicenseException:
-            return self.make_with_name(name_or_spdx, text=license_text, url=license_url)
-
-    def make_with_id(self, spdx_id: str, *, text: Optional[AttachedText] = None,
-                     url: Optional[XsUri] = None) -> License:
-        """Make a :class:`cyclonedx.model.License` from an SPDX-ID.
-
-        :raises InvalidSpdxLicenseException: if `spdx_id` was not known/supported SPDX-ID
-        """
-        spdx_license_id = spdx_fixup(spdx_id)
-        if spdx_license_id is None:
-            raise InvalidSpdxLicenseException(spdx_id)
-        return License(id=spdx_license_id, text=text, url=url)
-
-    def make_with_name(self, name: str, *, text: Optional[AttachedText] = None, url: Optional[XsUri] = None) -> License:
-        """Make a :class:`cyclonedx.model.License` with a name."""
-        return License(name=name, text=text, url=url)
 
+class LicenseFactory:
+    """Factory for :class:`cyclonedx.model.license.License`."""
 
-class LicenseChoiceFactory:
-    """Factory for :class:`cyclonedx.model.LicenseChoice`."""
-
-    def __init__(self, *, license_factory: LicenseFactory) -> None:
-        self.license_factory = license_factory
-
-    def make_from_string(self, expression_or_name_or_spdx: str) -> LicenseChoice:
-        """Make a :class:`cyclonedx.model.LicenseChoice` from a string.
-
-        Priority: SPDX license ID, SPDX license expression, named license
-        """
+    def make_from_string(self, value: str, *,
+                         license_text: Optional['AttachedText'] = None,
+                         license_url: Optional['XsUri'] = None) -> 'License':
+        """Make a :class:`cyclonedx.model.license.License` from a string."""
         try:
-            return LicenseChoice(license=self.license_factory.make_with_id(expression_or_name_or_spdx))
+            return self.make_with_id(value, text=license_text, url=license_url)
         except InvalidSpdxLicenseException:
             pass
         try:
-            return self.make_with_compound_expression(expression_or_name_or_spdx)
+            return self.make_with_expression(value)
         except InvalidLicenseExpressionException:
             pass
-        return LicenseChoice(license=self.license_factory.make_with_name(expression_or_name_or_spdx))
+        return self.make_with_name(value, text=license_text, url=license_url)
 
-    def make_with_compound_expression(self, compound_expression: str) -> LicenseChoice:
-        """Make a :class:`cyclonedx.model.LicenseChoice` with a compound expression.
+    def make_with_expression(self, expression: str) -> LicenseExpression:
+        """Make a :class:`cyclonedx.model.license.LicenseExpression` with a compound expression.
 
         Utilizes :func:`cyclonedx.spdx.is_compound_expression`.
 
-        :raises InvalidLicenseExpressionException: if `expression` is not known/supported license expression
+        :raises InvalidLicenseExpressionException: if param `value` is not known/supported license expression
         """
-        if is_spdx_compound_expression(compound_expression):
-            return LicenseChoice(expression=compound_expression)
-        raise InvalidLicenseExpressionException(compound_expression)
+        if is_spdx_compound_expression(expression):
+            return LicenseExpression(expression)
+        raise InvalidLicenseExpressionException(expression)
+
+    def make_with_id(self, spdx_id: str, *,
+                     text: Optional['AttachedText'] = None,
+                     url: Optional['XsUri'] = None) -> DisjunctiveLicense:
+        """Make a :class:`cyclonedx.model.license.DisjunctiveLicense` from an SPDX-ID.
+
+        :raises InvalidSpdxLicenseException: if param `spdx_id` was not known/supported SPDX-ID
+        """
+        spdx_license_id = spdx_fixup(spdx_id)
+        if spdx_license_id is None:
+            raise InvalidSpdxLicenseException(spdx_id)
+        return DisjunctiveLicense(id=spdx_license_id, text=text, url=url)
 
-    def make_with_license(self, name_or_spdx: str, *,
-                          license_text: Optional[AttachedText] = None,
-                          license_url: Optional[XsUri] = None) -> LicenseChoice:
-        """Make a :class:`cyclonedx.model.LicenseChoice` with a license (name or SPDX-ID)."""
-        return LicenseChoice(license=self.license_factory.make_from_string(
-            name_or_spdx, license_text=license_text, license_url=license_url))
+    def make_with_name(self, name: str, *,
+                       text: Optional['AttachedText'] = None,
+                       url: Optional['XsUri'] = None) -> DisjunctiveLicense:
+        """Make a :class:`cyclonedx.model.license.DisjunctiveLicense` with a name."""
+        return DisjunctiveLicense(name=name, text=text, url=url)

cyclonedx/model/__init__.py

@@ -15,7 +15,6 @@
 
 import hashlib
 import re
-import warnings
 from datetime import datetime, timezone
 from enum import Enum
 from itertools import zip_longest
@@ -28,7 +27,6 @@
 from ..exception.model import (
     InvalidLocaleTypeException,
     InvalidUriException,
-    MutuallyExclusivePropertiesException,
     NoPropertiesProvidedException,
     UnknownHashTypeException,
 )
@@ -444,20 +442,19 @@ def uri(self) -> str:
         return self._uri
 
     @classmethod
-    def serialize(cls, o: object) -> str:
+    def serialize(cls, o: Any) -> str:
         if isinstance(o, XsUri):
             return str(o)
-
         raise ValueError(f'Attempt to serialize a non-XsUri: {o.__class__}')
 
     @classmethod
-    def deserialize(cls, o: object) -> 'XsUri':
+    def deserialize(cls, o: Any) -> 'XsUri':
         try:
             return XsUri(uri=str(o))
         except ValueError:
             raise ValueError(f'XsUri string supplied ({o}) does not parse!')
 
-    def __eq__(self, other: object) -> bool:
+    def __eq__(self, other: Any) -> bool:
         if isinstance(other, XsUri):
             return hash(other) == hash(self)
         return False
@@ -579,181 +576,6 @@ def __repr__(self) -> str:
         return f'<ExternalReference {self.type.name}, {self.url}>'
 
 
-@serializable.serializable_class
-class License:
-    """
-    This is our internal representation of `licenseType` complex type that can be used in multiple places within
-    a CycloneDX BOM document.
-
-    .. note::
-        See the CycloneDX Schema definition: https://cyclonedx.org/docs/1.4/xml/#type_licenseType
-    """
-
-    def __init__(self, *, id: Optional[str] = None, name: Optional[str] = None,
-                 text: Optional[AttachedText] = None, url: Optional[XsUri] = None) -> None:
-        if not id and not name:
-            raise MutuallyExclusivePropertiesException('Either `id` or `name` MUST be supplied')
-        if id and name:
-            warnings.warn(
-                'Both `id` and `name` have been supplied - `name` will be ignored!',
-                RuntimeWarning
-            )
-        self.id = id
-        if not id:
-            self.name = name
-        else:
-            self.name = None
-        self.text = text
-        self.url = url
-
-    @property
-    def id(self) -> Optional[str]:
-        """
-        A valid SPDX license ID
-
-        Returns:
-            `str` or `None`
-        """
-        return self._id
-
-    @id.setter
-    def id(self, id: Optional[str]) -> None:
-        self._id = id
-
-    @property
-    def name(self) -> Optional[str]:
-        """
-        If SPDX does not define the license used, this field may be used to provide the license name.
-
-        Returns:
-            `str` or `None`
-        """
-        return self._name
-
-    @name.setter
-    def name(self, name: Optional[str]) -> None:
-        self._name = name
-
-    @property
-    def text(self) -> Optional[AttachedText]:
-        """
-        Specifies the optional full text of the attachment
-
-        Returns:
-            `AttachedText` else `None`
-        """
-        return self._text
-
-    @text.setter
-    def text(self, text: Optional[AttachedText]) -> None:
-        self._text = text
-
-    @property
-    def url(self) -> Optional[XsUri]:
-        """
-        The URL to the attachment file. If the attachment is a license or BOM, an externalReference should also be
-        specified for completeness.
-
-        Returns:
-            `XsUri` or `None`
-        """
-        return self._url
-
-    @url.setter
-    def url(self, url: Optional[XsUri]) -> None:
-        self._url = url
-
-    def __eq__(self, other: object) -> bool:
-        if isinstance(other, License):
-            return hash(other) == hash(self)
-        return False
-
-    def __lt__(self, other: Any) -> bool:
-        if isinstance(other, License):
-            return ComparableTuple((self.id, self.name)) < ComparableTuple((other.id, other.name))
-        return NotImplemented
-
-    def __hash__(self) -> int:
-        return hash((self.id, self.name, self.text, self.url))
-
-    def __repr__(self) -> str:
-        return f'<License id={self.id}, name={self.name}>'
-
-
-@serializable.serializable_class
-class LicenseChoice:
-    """
-    This is our internal representation of `licenseChoiceType` complex type that can be used in multiple places within
-    a CycloneDX BOM document.
-
-    .. note::
-        See the CycloneDX Schema definition: https://cyclonedx.org/docs/1.4/xml/#type_licenseChoiceType
-    """
-
-    def __init__(self, *, license: Optional[License] = None, expression: Optional[str] = None) -> None:
-        if not license and not expression:
-            raise NoPropertiesProvidedException(
-                'One of `license` or `expression` must be supplied - neither supplied'
-            )
-        if license and expression:
-            warnings.warn(
-                'Both `license` and `expression` have been supplied - `license` will take precedence',
-                RuntimeWarning
-            )
-        self.license = license
-        if not license:
-            self.expression = expression
-        else:
-            self.expression = None
-
-    @property
-    def license(self) -> Optional[License]:
-        """
-        License definition
-
-        Returns:
-            `License` or `None`
-        """
-        return self._license
-
-    @license.setter
-    def license(self, license: Optional[License]) -> None:
-        self._license = license
-
-    @property
-    def expression(self) -> Optional[str]:
-        """
-        A valid SPDX license expression (not enforced).
-
-        Refer to https://spdx.org/specifications for syntax requirements.
-
-        Returns:
-            `str` or `None`
-        """
-        return self._expression
-
-    @expression.setter
-    def expression(self, expression: Optional[str]) -> None:
-        self._expression = expression
-
-    def __eq__(self, other: object) -> bool:
-        if isinstance(other, LicenseChoice):
-            return hash(other) == hash(self)
-        return False
-
-    def __lt__(self, other: Any) -> bool:
-        if isinstance(other, LicenseChoice):
-            return ComparableTuple((self.license, self.expression)) < ComparableTuple(
-                (other.license, other.expression))
-        return NotImplemented
-
-    def __hash__(self) -> int:
-        return hash((self.license, self.expression))
-
-    def __repr__(self) -> str:
-        return f'<LicenseChoice license={self.license}, expression={self.expression}>'
-
-
 @serializable.serializable_class
 class Property:
     """