fix: LicenseChoiceFactory.make_from_string() prioritize SPDX id over expression (#427)

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

Author: jkowalleck

cyclonedx/factory/license.py

@@ -57,11 +57,19 @@ 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."""
+        """Make a :class:`cyclonedx.model.LicenseChoice` from a string.
+
+        Priority: SPDX license ID, SPDX license expression, named license
+        """
+        try:
+            return LicenseChoice(license=self.license_factory.make_with_id(expression_or_name_or_spdx))
+        except InvalidSpdxLicenseException:
+            pass
         try:
             return self.make_with_compound_expression(expression_or_name_or_spdx)
         except InvalidLicenseExpressionException:
-            return self.make_with_license(expression_or_name_or_spdx)
+            pass
+        return LicenseChoice(license=self.license_factory.make_with_name(expression_or_name_or_spdx))
 
     def make_with_compound_expression(self, compound_expression: str) -> LicenseChoice:
         """Make a :class:`cyclonedx.model.LicenseChoice` with a compound expression.

tests/test_factory_license.py

@@ -72,28 +72,46 @@ def test_make_with_name(self) -> None:
 
 class TestFactoryLicenseChoice(unittest.TestCase):
 
+    def test_make_from_string_with_license_id(self) -> None:
+        license_ = unittest.mock.NonCallableMock(spec=License)
+        expected = LicenseChoice(license=license_)
+        license_factory = unittest.mock.MagicMock(spec=LicenseFactory)
+        license_factory.make_with_id.return_value = license_
+        factory = LicenseChoiceFactory(license_factory=license_factory)
+
+        actual = factory.make_from_string('foo')
+
+        self.assertEqual(expected, actual)
+        self.assertIs(license_, actual.license)
+        license_factory.make_with_id.assert_called_once_with('foo')
+
     def test_make_from_string_with_compound_expression(self) -> None:
         expected = LicenseChoice(expression='foo')
-        factory = LicenseChoiceFactory(license_factory=unittest.mock.MagicMock(spec=LicenseFactory))
+        license_factory = unittest.mock.MagicMock(spec=LicenseFactory)
+        license_factory.make_with_id.side_effect = InvalidSpdxLicenseException('foo')
+        factory = LicenseChoiceFactory(license_factory=license_factory)
 
         with unittest.mock.patch('cyclonedx.factory.license.is_spdx_compound_expression', return_value=True):
             actual = factory.make_from_string('foo')
 
         self.assertEqual(expected, actual)
+        license_factory.make_with_id.assert_called_once_with('foo')
 
-    def test_make_from_string_with_license(self) -> None:
+    def test_make_from_string_with_license_name(self) -> None:
         license_ = unittest.mock.NonCallableMock(spec=License)
         expected = LicenseChoice(license=license_)
         license_factory = unittest.mock.MagicMock(spec=LicenseFactory)
-        license_factory.make_from_string.return_value = license_
+        license_factory.make_with_id.side_effect = InvalidSpdxLicenseException('foo')
+        license_factory.make_with_name.return_value = license_
         factory = LicenseChoiceFactory(license_factory=license_factory)
 
         with unittest.mock.patch('cyclonedx.factory.license.is_spdx_compound_expression', return_value=False):
             actual = factory.make_from_string('foo')
 
         self.assertEqual(expected, actual)
         self.assertIs(license_, actual.license)
-        license_factory.make_from_string.assert_called_once_with('foo', license_text=None, license_url=None)
+        license_factory.make_with_id.assert_called_once_with('foo')
+        license_factory.make_with_name.assert_called_once_with('foo')
 
     def test_make_with_compound_expression(self) -> None:
         expected = LicenseChoice(expression='foo')
@@ -119,8 +137,7 @@ def test_make_with_license(self) -> None:
         license_factory.make_from_string.return_value = license_
         factory = LicenseChoiceFactory(license_factory=license_factory)
 
-        with unittest.mock.patch('cyclonedx.factory.license.is_spdx_compound_expression', return_value=False):
-            actual = factory.make_with_license('foo', license_text=text, license_url=url)
+        actual = factory.make_with_license('foo', license_text=text, license_url=url)
 
         self.assertEqual(expected, actual)
         self.assertIs(license_, actual.license)