[issue-403] restructure spdx version validation

Signed-off-by: Armin Tänzer <armin.taenzer@tngtech.com>

Author: armintaenzertng

src/validation/creation_info_validator.py

@@ -19,24 +19,11 @@
 from src.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
 
 
-def validate_creation_info(creation_info: CreationInfo, spdx_version: str) -> List[ValidationMessage]:
+def validate_creation_info(creation_info: CreationInfo) -> List[ValidationMessage]:
     validation_messages: List[ValidationMessage] = []
 
     context = ValidationContext(spdx_id=creation_info.spdx_id, element_type=SpdxElementType.DOCUMENT)
 
-    if not re.match(r"^SPDX-\d+.\d+$", creation_info.spdx_version):
-        validation_messages.append(
-            ValidationMessage(
-                f'spdx_version must be of the form "SPDX-[major].[minor]" but is: {creation_info.spdx_version}',
-                context
-            )
-        )
-    elif spdx_version != creation_info.spdx_version:
-        validation_messages.append(
-            ValidationMessage(f"provided SPDX version {spdx_version} does not match "
-                              f"the document's SPDX version {creation_info.spdx_version}", context)
-        )
-
     if creation_info.spdx_id != "SPDXRef-DOCUMENT":
         validation_messages.append(
             ValidationMessage(

src/validation/document_validator.py

@@ -8,7 +8,7 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-
+import re
 from typing import List
 
 from src.model.document import Document
@@ -27,10 +27,32 @@
 def validate_full_spdx_document(document: Document, spdx_version: str = None) -> List[ValidationMessage]:
     validation_messages: List[ValidationMessage] = []
 
+    # SPDX version validation has to happen here because subsequent validators rely on it
+    document_version: str = document.creation_info.spdx_version
+    context = ValidationContext(spdx_id=document.creation_info.spdx_id, element_type=SpdxElementType.DOCUMENT)
     if not spdx_version:
-        spdx_version = document.creation_info.spdx_version
+        spdx_version = document_version
+
+    if not re.match(r"^SPDX-\d+.\d+$", document_version):
+        validation_messages.append(
+            ValidationMessage(
+                f'the document\'s spdx_version must be of the form "SPDX-[major].[minor]" but is: {document_version}',
+                context
+            )
+        )
+    elif spdx_version != document_version:
+        validation_messages.append(
+            ValidationMessage(f"provided SPDX version {spdx_version} does not match "
+                              f"the document's SPDX version {document_version}", context)
+        )
+
+    if validation_messages:
+        validation_messages.append(ValidationMessage("There are issues concerning the SPDX version of the document. "
+                                                     "As subsequent validation relies on the correct version, "
+                                                     "the validation process has been cancelled.", context))
+        return validation_messages
 
-    validation_messages.extend(validate_creation_info(document.creation_info, spdx_version))
+    validation_messages.extend(validate_creation_info(document.creation_info))
     validation_messages.extend(validate_packages(document.packages, document))
     validation_messages.extend(validate_files(document.files, document))
     validation_messages.extend(validate_snippets(document.snippets, document))

tests/validation/test_creation_info_validator.py

@@ -27,9 +27,7 @@ def test_valid_creation_info():
 
 @pytest.mark.parametrize \
         ("creation_info_input, spdx_id, expected_message",
-         [(creation_info_fixture(spdx_version="version-2.3"), "SPDXRef-DOCUMENT",
-           'spdx_version must be of the form "SPDX-[major].[minor]" but is: version-2.3'),
-          (creation_info_fixture(spdx_id="SPDXRef-doc"), "SPDXRef-doc",
+         [(creation_info_fixture(spdx_id="SPDXRef-doc"), "SPDXRef-doc",
            'spdx_id must be "SPDXRef-DOCUMENT", but is: SPDXRef-doc'),
           (creation_info_fixture(data_license="MIT"), "SPDXRef-DOCUMENT",
            'data_license must be "CC0-1.0", but is: MIT'),

tests/validation/test_document_validator.py

@@ -9,11 +9,14 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-from typing import List
+from typing import List, Optional
 
+import pytest
+
+from src.model.document import Document, CreationInfo
 from src.validation.document_validator import validate_full_spdx_document
-from src.validation.validation_message import ValidationMessage
-from tests.fixtures import document_fixture
+from src.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
+from tests.fixtures import document_fixture, creation_info_fixture
 
 
 def test_valid_document():
@@ -22,4 +25,34 @@ def test_valid_document():
 
     assert validation_messages == []
 
-# TODO: https://github.com/spdx/tools-python/issues/375
+
+@pytest.mark.parametrize("creation_info, version_input, expected_message",
+                         [(creation_info_fixture(spdx_version="SPDX-2.3"), "SPDX-2.3", None),
+                          (creation_info_fixture(spdx_version="SPDX-2.3"), None, None),
+                          (creation_info_fixture(spdx_version="SPDX-2.3"), "SPDX-2.2",
+                           "provided SPDX version SPDX-2.2 does not match the document's SPDX version SPDX-2.3"),
+                          (creation_info_fixture(spdx_version="SPDX-2.3"), "SPDX2.3",
+                           "provided SPDX version SPDX2.3 does not match the document's SPDX version SPDX-2.3"),
+                          (creation_info_fixture(spdx_version="SPDX2.3"), "SPDX-2.3",
+                           'the document\'s spdx_version must be of the form "SPDX-[major].[minor]" but is: SPDX2.3'),
+                          (creation_info_fixture(spdx_version="SPDX2.3"), None,
+                           'the document\'s spdx_version must be of the form "SPDX-[major].[minor]" but is: SPDX2.3'),
+                          (creation_info_fixture(spdx_version="SPDX2.3"), "SPDX2.3",
+                           'the document\'s spdx_version must be of the form "SPDX-[major].[minor]" but is: SPDX2.3'),
+                          ])
+def test_spdx_version_handling(creation_info: CreationInfo, version_input: str, expected_message: Optional[str]):
+    document: Document = document_fixture(creation_info=creation_info)
+    validation_messages: List[ValidationMessage] = validate_full_spdx_document(document, version_input)
+
+    context = ValidationContext(spdx_id=creation_info.spdx_id, element_type=SpdxElementType.DOCUMENT)
+    expected: List[ValidationMessage] = []
+
+    if expected_message:
+        expected.append(ValidationMessage(expected_message, context))
+        expected.append(ValidationMessage("There are issues concerning the SPDX version of the document. "
+                                          "As subsequent validation relies on the correct version, "
+                                          "the validation process has been cancelled.", context))
+
+    assert validation_messages == expected
+
+    # TODO: https://github.com/spdx/tools-python/issues/375