Repository: spdx/tools-python

Issue:
Finish package validation
part of #307 

- [x] check if verification code is required when `files_analyzed = True` and make check for it
- [x] write tests for package verification code validation
- [x] write tests for "no `CONTAINS` or `CONTAINED_BY` relationships if `files_analyzed = False`"

Discussion:

Patch:
diff --git a/src/spdx/validation/checksum_validator.py b/src/spdx/validation/checksum_validator.py
index b5f3f80..b684ec7 100644
--- a/src/spdx/validation/checksum_validator.py
+++ b/src/spdx/validation/checksum_validator.py
@@ -60,7 +60,7 @@ def validate_checksum(checksum: Checksum, parent_id: str) -> List[ValidationMess
             length = algorithm_length[algorithm]
         validation_messages.append(
             ValidationMessage(
-                f"value of {algorithm} must consist of {length} hexadecimal digits, but is: {checksum.value} (length: {len(checksum.value)} digits)",
+                f"value of {algorithm} must consist of {length} lowercase hexadecimal digits, but is: {checksum.value} (length: {len(checksum.value)} digits)",
                 context)
         )
 
diff --git a/src/spdx/validation/document_validator.py b/src/spdx/validation/document_validator.py
index a3fa007..fe6422a 100644
--- a/src/spdx/validation/document_validator.py
+++ b/src/spdx/validation/document_validator.py
@@ -21,6 +21,7 @@ from spdx.validation.file_validator import validate_files
 from spdx.validation.package_validator import validate_packages
 from spdx.validation.relationship_validator import validate_relationships
 from spdx.validation.snippet_validator import validate_snippets
+from spdx.validation.spdx_id_validators import get_list_of_all_spdx_ids
 from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
 
 
@@ -74,4 +75,16 @@ def validate_full_spdx_document(document: Document, spdx_version: str = None) ->
                 ValidationContext(spdx_id=document_id,
                                   element_type=SpdxElementType.DOCUMENT)))
 
+    all_spdx_ids: List[str] = get_list_of_all_spdx_ids(document)
+    auxiliary_set = set()
+    duplicated_spdx_ids = set(
+        spdx_id for spdx_id in all_spdx_ids if spdx_id in auxiliary_set or auxiliary_set.add(spdx_id))
+
+    if duplicated_spdx_ids:
+        validation_messages.append(
+            ValidationMessage(
+                f"every spdx_id must be unique within the document, but found the following duplicates: {sorted(duplicated_spdx_ids)}",
+                context)
+        )
+
     return validation_messages
diff --git a/src/spdx/validation/package_validator.py b/src/spdx/validation/package_validator.py
index 5131d39..f73f42e 100644
--- a/src/spdx/validation/package_validator.py
+++ b/src/spdx/validation/package_validator.py
@@ -13,7 +13,8 @@ from typing import List, Optional
 
 from spdx.model.document import Document
 from spdx.model.package import Package
-from spdx.model.relationship import RelationshipType
+from spdx.model.relationship import RelationshipType, Relationship
+from spdx.model.relationship_filters import filter_by_type_and_origin, filter_by_type_and_target
 from spdx.validation.checksum_validator import validate_checksums
 from spdx.validation.external_package_ref_validator import validate_external_package_refs
 from spdx.validation.license_expression_validator import validate_license_expression, validate_license_expressions
@@ -43,23 +44,18 @@ def validate_package_within_document(package: Package, document: Document) -> Li
     for message in validate_spdx_id(package.spdx_id, document):
         validation_messages.append(ValidationMessage(message, context))
 
-    # TODO: make test for this (https://github.com/spdx/tools-python/issues/386)
     if not package.files_analyzed:
-        package_contains_relationships = [relationship for relationship in document.relationships if
-                                          relationship.relationship_type == RelationshipType.CONTAINS and relationship.spdx_element_id == package.spdx_id]
-        if package_contains_relationships:
-            validation_messages.append(
-                ValidationMessage(
-                    f"package must contain no elements if files_analyzed is False, but found {package_contains_relationships}",
-                    context)
-            )
+        package_contains_relationships = filter_by_type_and_origin(document.relationships, RelationshipType.CONTAINS,
+                                                                   package.spdx_id)
+        contained_in_package_relationships = filter_by_type_and_target(document.relationships,
+                                                                       RelationshipType.CONTAINED_BY, package.spdx_id)
+
+        combined_relationships: List[Relationship] = package_contains_relationships + contained_in_package_relationships
 
-        contained_in_package_relationships = [relationship for relationship in document.relationships if
-                                              relationship.relationship_type == RelationshipType.CONTAINED_BY and relationship.related_spdx_element_id == package.spdx_id]
-        if contained_in_package_relationships:
+        if combined_relationships:
             validation_messages.append(
                 ValidationMessage(
-                    f"package must contain no elements if files_analyzed is False, but found {contained_in_package_relationships}",
+                    f"package must contain no elements if files_analyzed is False, but found {combined_relationships}",
                     context)
             )
 
@@ -83,7 +79,6 @@ def validate_package(package: Package, context: Optional[ValidationContext] = No
         for message in validate_url(homepage):
             validation_messages.append(ValidationMessage("homepage " + message, context))
 
-    # TODO: is verification_code required if files_analyzed=True? (https://github.com/spdx/tools-python/issues/386)
     verification_code = package.verification_code
     if verification_code:
         if not package.files_analyzed:
diff --git a/src/spdx/validation/package_verification_code_validator.py b/src/spdx/validation/package_verification_code_validator.py
index 2b86520..de6abbe 100644
--- a/src/spdx/validation/package_verification_code_validator.py
+++ b/src/spdx/validation/package_verification_code_validator.py
@@ -16,24 +16,23 @@ from spdx.model.package import PackageVerificationCode
 from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
 
 
-# TODO: make test for this (https://github.com/spdx/tools-python/issues/386)
 def validate_verification_code(verification_code: PackageVerificationCode, parent_id: str) -> List[ValidationMessage]:
     validation_messages: List[ValidationMessage] = []
     context = ValidationContext(parent_id=parent_id, element_type=SpdxElementType.PACKAGE_VERIFICATION_CODE,
                                 full_element=verification_code)
 
     for file in verification_code.excluded_files:
-        if not file.startswith("./"):
+        if file.startswith("/"):
             validation_messages.append(
                 ValidationMessage(
-                    f'file name must be a relative path to the file, starting with "./", but is: {file}', context)
+                    f'file name must not be an absolute path starting with "/", but is: {file}', context)
             )
 
     value: str = verification_code.value
     if not re.match("^[0-9a-f]{40}$", value):
         validation_messages.append(
             ValidationMessage(
-                f"value of verification_code must consist of 40 hexadecimal digits, but is: {value} (length: {len(value)} digits)",
+                f"value of verification_code must consist of 40 lowercase hexadecimal digits, but is: {value} (length: {len(value)} digits)",
                 context)
         )
 
diff --git a/src/spdx/validation/spdx_id_validators.py b/src/spdx/validation/spdx_id_validators.py
index dfe25f4..cdf8f0c 100644
--- a/src/spdx/validation/spdx_id_validators.py
+++ b/src/spdx/validation/spdx_id_validators.py
@@ -53,8 +53,7 @@ def validate_spdx_id(spdx_id: str, document: Document, check_document: bool = Fa
     str]:
     """ Test that the given spdx_id (and a potential DocumentRef to an external document) is valid
     and, if it is a reference, actually exists in the document. Optionally checks files or the whole document
-    for the existence of the spdx_id (i.e. if it is used as a reference). Returns a list of validation messages,
-    and the external document ref part and id part of the provided spdx_id. """
+    for the existence of the spdx_id (i.e. if it is used as a reference). Returns a list of validation messages. """
 
     validation_messages: List[str] = []
     split_id: List[str] = spdx_id.split(":")
@@ -74,7 +73,7 @@ def validate_spdx_id(spdx_id: str, document: Document, check_document: bool = Fa
                 f'the internal SPDX id part of spdx_id must only contain letters, numbers, "." and "-" and must begin with "SPDXRef-", but is: {split_id[1]}')
         if not is_external_doc_ref_present_in_document(split_id[0], document):
             validation_messages.append(
-                f"did not find the external document reference {split_id[0]} in the SPDX document")
+                f'did not find the external document reference "{split_id[0]}" in the SPDX document')
 
         return validation_messages
 
@@ -85,10 +84,10 @@ def validate_spdx_id(spdx_id: str, document: Document, check_document: bool = Fa
 
     if check_document:
         if not is_spdx_id_present_in_document(spdx_id, document):
-            validation_messages.append(f"did not find the referenced spdx_id {spdx_id} in the SPDX document")
+            validation_messages.append(f'did not find the referenced spdx_id "{spdx_id}" in the SPDX document')
 
     if check_files:
         if not is_spdx_id_present_in_files(spdx_id, document.files):
-            validation_messages.append(f"did not find the referenced spdx_id {spdx_id} in the SPDX document's files")
+            validation_messages.append(f'did not find the referenced spdx_id "{spdx_id}" in the SPDX document\'s files')
 
     return validation_messages


Test patch:
diff --git a/tests/spdx/validation/test_annotation_validator.py b/tests/spdx/validation/test_annotation_validator.py
index b37a686..608de5b 100644
--- a/tests/spdx/validation/test_annotation_validator.py
+++ b/tests/spdx/validation/test_annotation_validator.py
@@ -28,7 +28,7 @@ def test_valid_annotation():
 
 @pytest.mark.parametrize("annotation_id, file_id, expected_message",
                          [("SPDXRef-File", "SPDXRef-hiddenFile",
-                           "did not find the referenced spdx_id SPDXRef-File in the SPDX document")
+                           'did not find the referenced spdx_id "SPDXRef-File" in the SPDX document')
                           ])
 def test_invalid_annotation(annotation_id, file_id, expected_message):
     annotation: Annotation = annotation_fixture(spdx_id=annotation_id)
diff --git a/tests/spdx/validation/test_checksum_validator.py b/tests/spdx/validation/test_checksum_validator.py
index 99a127d..7fd5f56 100644
--- a/tests/spdx/validation/test_checksum_validator.py
+++ b/tests/spdx/validation/test_checksum_validator.py
@@ -58,40 +58,42 @@ def test_valid_checksum(checksum):
 
 @pytest.mark.parametrize("checksum, expected_message",
                          [(Checksum(ChecksumAlgorithm.SHA1, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA1 must consist of 40 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA1 must consist of 40 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA224, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA224 must consist of 56 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA224 must consist of 56 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA256, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA384, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA512, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA3_256, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA3_256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA3_256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA3_384, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA3_384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA3_384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.SHA3_512, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.SHA3_512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.SHA3_512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.BLAKE2B_256, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.BLAKE2B_256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.BLAKE2B_256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.BLAKE2B_384, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.BLAKE2B_384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.BLAKE2B_384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.BLAKE2B_512, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.BLAKE2B_512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.BLAKE2B_512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.BLAKE3, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.BLAKE3 must consist of at least 256 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.BLAKE3 must consist of at least 256 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
                           (Checksum(ChecksumAlgorithm.MD2, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
-                           "value of ChecksumAlgorithm.MD2 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
+                           "value of ChecksumAlgorithm.MD2 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
                           (Checksum(ChecksumAlgorithm.MD4, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
-                           "value of ChecksumAlgorithm.MD4 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
+                           "value of ChecksumAlgorithm.MD4 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
                           (Checksum(ChecksumAlgorithm.MD5, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
-                           "value of ChecksumAlgorithm.MD5 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
+                           "value of ChecksumAlgorithm.MD5 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
                           (Checksum(ChecksumAlgorithm.MD6,
                                     "a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5"),
-                           "value of ChecksumAlgorithm.MD6 must consist of between 0 and 512 hexadecimal digits, but is: a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5 (length: 513 digits)"),
+                           "value of ChecksumAlgorithm.MD6 must consist of between 0 and 512 lowercase hexadecimal digits, but is: a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5 (length: 513 digits)"),
                           (Checksum(ChecksumAlgorithm.ADLER32, "af1eec2a1b18886c3f3cc244349d91d8"),
-                           "value of ChecksumAlgorithm.ADLER32 must consist of 8 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                           "value of ChecksumAlgorithm.ADLER32 must consist of 8 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
+                          (Checksum(ChecksumAlgorithm.SHA1, "CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4"),
+                           "value of ChecksumAlgorithm.SHA1 must consist of 40 lowercase hexadecimal digits, but is: CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4 (length: 40 digits)"),
                           ])
 def test_invalid_checksum(checksum, expected_message):
     parent_id = "parent_id"
diff --git a/tests/spdx/validation/test_document_validator.py b/tests/spdx/validation/test_document_validator.py
index c3b8055..944f307 100644
--- a/tests/spdx/validation/test_document_validator.py
+++ b/tests/spdx/validation/test_document_validator.py
@@ -16,7 +16,7 @@ import pytest
 from spdx.model.document import Document, CreationInfo
 from spdx.validation.document_validator import validate_full_spdx_document
 from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
-from tests.spdx.fixtures import document_fixture, creation_info_fixture
+from tests.spdx.fixtures import document_fixture, creation_info_fixture, file_fixture, package_fixture, snippet_fixture
 
 
 def test_valid_document():
@@ -56,3 +56,18 @@ def test_spdx_version_handling(creation_info: CreationInfo, version_input: str,
     assert validation_messages == expected
 
     # TODO: https://github.com/spdx/tools-python/issues/375
+
+
+def test_duplicated_spdx_ids():
+    document = document_fixture(
+        files=[file_fixture(spdx_id="SPDXRef-File"), file_fixture(spdx_id="SPDXRef-2"), file_fixture(spdx_id="SPDXRef-3")],
+        packages=[package_fixture(spdx_id="SPDXRef-2"), package_fixture(spdx_id="SPDXRef-DOCUMENT")],
+        snippets=[snippet_fixture(spdx_id="SPDXRef-2"), snippet_fixture(spdx_id="SPDXRef-3")])
+
+    context = ValidationContext(spdx_id=document.creation_info.spdx_id, element_type=SpdxElementType.DOCUMENT)
+
+    validation_messages: List[ValidationMessage] = validate_full_spdx_document(document)
+
+    assert validation_messages == [ValidationMessage(
+        "every spdx_id must be unique within the document, but found the following duplicates: ['SPDXRef-2', 'SPDXRef-3', 'SPDXRef-DOCUMENT']",
+        context)]
diff --git a/tests/spdx/validation/test_package_validator.py b/tests/spdx/validation/test_package_validator.py
index 3a41338..fa79239 100644
--- a/tests/spdx/validation/test_package_validator.py
+++ b/tests/spdx/validation/test_package_validator.py
@@ -14,11 +14,12 @@ from typing import List
 import pytest
 from license_expression import Licensing
 
+from spdx.model.relationship import Relationship, RelationshipType
 from spdx.model.spdx_no_assertion import SpdxNoAssertion
 from spdx.model.spdx_none import SpdxNone
 from spdx.validation.package_validator import validate_package_within_document
 from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
-from tests.spdx.fixtures import package_fixture, package_verification_code_fixture, document_fixture
+from tests.spdx.fixtures import package_fixture, package_verification_code_fixture, document_fixture, file_fixture
 
 
 def test_valid_package():
@@ -42,7 +43,7 @@ def test_valid_package():
                                            license_info_from_files=[Licensing().parse("some_license")],
                                            verification_code=None),
                            "license_info_from_files must be None if files_analyzed is False, but is: [LicenseSymbol('some_license', "
-                                      "is_exception=False)]")
+                           "is_exception=False)]")
                           ])
 def test_invalid_package(package_input, expected_message):
     validation_messages: List[ValidationMessage] = validate_package_within_document(package_input,
@@ -54,3 +55,25 @@ def test_invalid_package(package_input, expected_message):
                                                    full_element=package_input))
 
     assert validation_messages == [expected]
+
+
+@pytest.mark.parametrize("relationships",
+                         [[Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "SPDXRef-File1")],
+                          [Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "DocumentRef-external:SPDXRef-File")],
+                          [Relationship("SPDXRef-File2", RelationshipType.CONTAINED_BY, "SPDXRef-Package")],
+                          [Relationship("DocumentRef-external:SPDXRef-File", RelationshipType.CONTAINED_BY, "SPDXRef-Package")],
+                          [Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "SPDXRef-File2"),
+                           Relationship("SPDXRef-File1", RelationshipType.CONTAINED_BY, "SPDXRef-Package")]])
+def test_invalid_package_with_contains(relationships):
+    document = document_fixture(relationships=relationships,
+                                files=[file_fixture(spdx_id="SPDXRef-File1"), file_fixture(spdx_id="SPDXRef-File2")])
+    package = package_fixture(files_analyzed=False, verification_code=None, license_info_from_files=[])
+    context = ValidationContext(spdx_id=package.spdx_id, parent_id=document.creation_info.spdx_id,
+                                element_type=SpdxElementType.PACKAGE,
+                                full_element=package)
+
+    validation_messages: List[ValidationMessage] = validate_package_within_document(package, document)
+
+    assert validation_messages == [
+        ValidationMessage(f"package must contain no elements if files_analyzed is False, but found {relationships}",
+                          context)]
diff --git a/tests/spdx/validation/test_package_verification_code_validator.py b/tests/spdx/validation/test_package_verification_code_validator.py
new file mode 100644
index 0000000..5ed3ae4
--- /dev/null
+++ b/tests/spdx/validation/test_package_verification_code_validator.py
@@ -0,0 +1,40 @@
+# Copyright (c) 2022 spdx contributors
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#     http://www.apache.org/licenses/LICENSE-2.0
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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 pytest
+
+from spdx.model.package import PackageVerificationCode
+from spdx.validation.package_verification_code_validator import validate_verification_code
+from spdx.validation.validation_message import ValidationContext, SpdxElementType, ValidationMessage
+
+
+def test_valid_package_verification_code():
+    code = PackageVerificationCode("71c4025dd9897b364f3ebbb42c484ff43d00791c", ["./excluded_file", "another.file"])
+    validation_messages = validate_verification_code(code, "SPDXRef-Package")
+
+    assert validation_messages == []
+
+
+@pytest.mark.parametrize("code, expected_message",
+                         [(PackageVerificationCode("71c4025dd9897b364f3ebbb42c484ff43d00791cab", []),
+                           "value of verification_code must consist of 40 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791cab (length: 42 digits)"),
+                          (PackageVerificationCode("CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4", []),
+                           "value of verification_code must consist of 40 lowercase hexadecimal digits, but is: CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4 (length: 40 digits)"),
+                          (PackageVerificationCode("71c4025dd9897b364f3ebbb42c484ff43d00791c", ["/invalid/excluded/file"]),
+                           'file name must not be an absolute path starting with "/", but is: /invalid/excluded/file')
+                          ])
+def test_invalid_package_verification_code(code, expected_message):
+    parent_id = "SPDXRef-Package"
+    context = ValidationContext(parent_id=parent_id, element_type=SpdxElementType.PACKAGE_VERIFICATION_CODE,
+                                full_element=code)
+    validation_messages = validate_verification_code(code, parent_id)
+
+    assert validation_messages == [ValidationMessage(expected_message, context)]
diff --git a/tests/spdx/validation/test_relationship_validator.py b/tests/spdx/validation/test_relationship_validator.py
index c93c2e2..89ea088 100644
--- a/tests/spdx/validation/test_relationship_validator.py
+++ b/tests/spdx/validation/test_relationship_validator.py
@@ -33,9 +33,9 @@ def test_valid_relationship(related_spdx_element):
 
 @pytest.mark.parametrize("spdx_element_id, related_spdx_element_id, expected_message",
                          [("SPDXRef-unknownFile", "SPDXRef-File",
-                           'did not find the referenced spdx_id SPDXRef-unknownFile in the SPDX document'),
+                           'did not find the referenced spdx_id "SPDXRef-unknownFile" in the SPDX document'),
                           ("SPDXRef-File", "SPDXRef-unknownFile",
-                           'did not find the referenced spdx_id SPDXRef-unknownFile in the SPDX document'),
+                           'did not find the referenced spdx_id "SPDXRef-unknownFile" in the SPDX document'),
                           ])
 def test_unknown_spdx_id(spdx_element_id, related_spdx_element_id, expected_message):
     relationship: Relationship = relationship_fixture(spdx_element_id=spdx_element_id,
diff --git a/tests/spdx/validation/test_spdx_id_validators.py b/tests/spdx/validation/test_spdx_id_validators.py
index 4c00169..ee8536c 100644
--- a/tests/spdx/validation/test_spdx_id_validators.py
+++ b/tests/spdx/validation/test_spdx_id_validators.py
@@ -8,5 +8,115 @@
 # 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.
+from unittest import TestCase
+
+import pytest
+
+from spdx.validation.spdx_id_validators import is_valid_internal_spdx_id, is_valid_external_doc_ref_id, \
+    get_list_of_all_spdx_ids, is_spdx_id_present_in_document, is_external_doc_ref_present_in_document, validate_spdx_id
+from tests.spdx.fixtures import document_fixture, file_fixture, package_fixture, snippet_fixture, creation_info_fixture, \
+    external_document_ref_fixture
+
+DOCUMENT = document_fixture(files=[file_fixture(spdx_id="SPDXRef-File1"),
+                                   file_fixture(spdx_id="SPDXRef-File2")],
+                            packages=[package_fixture(spdx_id="SPDXRef-Package1"),
+                                      package_fixture(spdx_id="SPDXRef-Package2")],
+                            snippets=[snippet_fixture(spdx_id="SPDXRef-Snippet1"),
+                                      snippet_fixture(spdx_id="SPDXRef-Snippet2")],
+                            creation_info=creation_info_fixture(
+                                external_document_refs=[external_document_ref_fixture(document_ref_id="DocumentRef-external"),
+                                                        external_document_ref_fixture(document_ref_id="DocumentRef-1.2-ext")]))
+
+
+@pytest.mark.parametrize("spdx_id", ["SPDXRef-DOCUMENT", "SPDXRef-File1", "SPDXRef-1.3-3.7"])
+def test_valid_internal_spdx_ids(spdx_id):
+    assert is_valid_internal_spdx_id(spdx_id)
+
+
+@pytest.mark.parametrize("spdx_id",
+                         ["spdxId", "spdxRef-DOCUMENT", "SPDXRef.File", "SPDXRef#Snippet", "SPDXRef-1.3_3.7"])
+def test_invalid_internal_spdx_ids(spdx_id):
+    assert not is_valid_internal_spdx_id(spdx_id)
+
+
+@pytest.mark.parametrize("doc_ref_id", ["DocumentRef-external", "DocumentRef-...+", "DocumentRef-v0.4.2-alpha"])
+def test_valid_external_doc_ref_ids(doc_ref_id):
+    assert is_valid_external_doc_ref_id(doc_ref_id)
+
+
+@pytest.mark.parametrize("doc_ref_id",
+                         ["external-ref", "Documentref-external", "DocumentRef-...#", "DocumentRef-v0_4_2-alpha"])
+def test_invalid_external_doc_ref_ids(doc_ref_id):
+    assert not is_valid_external_doc_ref_id(doc_ref_id)
+
+
+def test_is_spdx_id_present_in_document():
+    assert is_spdx_id_present_in_document("SPDXRef-File1", DOCUMENT)
+    assert is_spdx_id_present_in_document("SPDXRef-Package2", DOCUMENT)
+    assert is_spdx_id_present_in_document("SPDXRef-Snippet1", DOCUMENT)
+    assert is_spdx_id_present_in_document("SPDXRef-DOCUMENT", DOCUMENT)
+    assert not is_spdx_id_present_in_document("SPDXRef-file2", DOCUMENT)
+
+
+def test_is_external_doc_ref_present_in_document():
+    assert is_external_doc_ref_present_in_document("DocumentRef-1.2-ext", DOCUMENT)
+    assert not is_external_doc_ref_present_in_document("DocumentRef-External1", DOCUMENT)
+
+def test_list_of_all_spdx_ids():
+    TestCase().assertCountEqual(get_list_of_all_spdx_ids(DOCUMENT),
+                                ["SPDXRef-DOCUMENT", "SPDXRef-File1", "SPDXRef-File2", "SPDXRef-Package1",
+                                 "SPDXRef-Package2", "SPDXRef-Snippet1", "SPDXRef-Snippet2"])
+
+
+@pytest.mark.parametrize("spdx_id",
+                         ["DocumentRef-external:SPDXRef-File", "SPDXRef-Package"])
+def test_valid_spdx_id(spdx_id):
+    validation_messages = validate_spdx_id(spdx_id, DOCUMENT)
+
+    assert validation_messages == []
+
+
+@pytest.mark.parametrize("spdx_id, expected_messages",
+                         [("DocumentRef-external:extern:SPDXRef-File",
+                           [f"spdx_id must not contain more than one colon in order to separate the external document reference id from the internal SPDX id, but is: DocumentRef-external:extern:SPDXRef-File"]),
+                          ("DocumentRef external:SPDXRef-File",
+                           ['the external document reference part of spdx_id must only contain letters, numbers, ".", "-" and "+" and must begin with "DocumentRef-", but is: DocumentRef external',
+                            'did not find the external document reference "DocumentRef external" in the SPDX document']),
+                          ("DocRef-ext:SPDXRef-File_2",
+                           ['the external document reference part of spdx_id must only contain letters, numbers, ".", "-" and "+" and must begin with "DocumentRef-", but is: DocRef-ext',
+                            'the internal SPDX id part of spdx_id must only contain letters, numbers, "." and "-" and must begin with "SPDXRef-", but is: SPDXRef-File_2',
+                            'did not find the external document reference "DocRef-ext" in the SPDX document']),
+                          ("DocumentRef-external:SPDXRef-File_2",
+                           ['the internal SPDX id part of spdx_id must only contain letters, numbers, "." and "-" and must begin with "SPDXRef-", but is: SPDXRef-File_2']),
+                          ("SPDXRef-42+",
+                           ['spdx_id must only contain letters, numbers, "." and "-" and must begin with "SPDXRef-", but is: SPDXRef-42+'])
+                          ])
+def test_invalid_spdx_id(spdx_id, expected_messages):
+    validation_messages = validate_spdx_id(spdx_id, DOCUMENT)
+
+    TestCase().assertCountEqual(validation_messages, expected_messages)
+
+
+@pytest.mark.parametrize("spdx_id",
+                         ["DocumentRef-external:SPDXRef-File", "SPDXRef-DOCUMENT", "SPDXRef-File1", "SPDXRef-Package1", "SPDXRef-Snippet1"])
+def test_valid_spdx_id_with_check_document(spdx_id):
+    validation_messages = validate_spdx_id(spdx_id, DOCUMENT, check_document=True)
+    assert validation_messages == []
+
+
+def test_invalid_spdx_id_with_check_document():
+    validation_messages = validate_spdx_id("SPDXRef-Filet", DOCUMENT, check_document=True)
+    assert validation_messages == ['did not find the referenced spdx_id "SPDXRef-Filet" in the SPDX document']
+
+
+@pytest.mark.parametrize("spdx_id",
+                         ["DocumentRef-external:SPDXRef-File", "SPDXRef-File1"])
+def test_valid_spdx_id_with_check_files(spdx_id):
+    validation_messages = validate_spdx_id(spdx_id, DOCUMENT, check_files=True)
+    assert validation_messages == []
+
+
+def test_invalid_spdx_id_with_check_files():
+    validation_messages = validate_spdx_id("SPDXRef-Package1", DOCUMENT, check_files=True)
+    assert validation_messages == ['did not find the referenced spdx_id "SPDXRef-Package1" in the SPDX document\'s files']
 
-# TODO: https://github.com/spdx/tools-python/issues/376