Repository: spdx/tools-python

Issue:
TV parser doesn't deduplicate files
Starting from a valid spdx document, serialising it as tv and parsing it again leads to an invalid document.

To reproduce (using version 0.8.0a2):

```
from datetime import datetime

from spdx_tools.spdx.model.actor import Actor, ActorType
from spdx_tools.spdx.model.checksum import Checksum, ChecksumAlgorithm
from spdx_tools.spdx.model.document import CreationInfo, Document
from spdx_tools.spdx.model.file import File
from spdx_tools.spdx.model.package import Package
from spdx_tools.spdx.model.relationship import Relationship, RelationshipType
from spdx_tools.spdx.writer.write_anything import write_file
from spdx_tools.spdx.parser.parse_anything import parse_file
from spdx_tools.spdx.validation.document_validator import validate_full_spdx_document




def _create_minimal_document() -> Document:
    """This is a helper method to create a minimal document that describes two
    packages which both contain the same file."""
    creation_info = CreationInfo(
        spdx_version="SPDX-2.3",
        spdx_id="SPDXRef-DOCUMENT",
        data_license="CC0-1.0",
        name="SPDX Lite Document",
        document_namespace="https://test.namespace.com",
        creators=[Actor(ActorType.PERSON, "John Doe")],
        created=datetime(2023, 3, 14, 8, 49),
    )
    package_a = Package(
        name="Example package A",
        spdx_id="SPDXRef-Package-A",
        download_location="https://download.com",
    )
    package_b = Package(
        name="Example package B",
        spdx_id="SPDXRef-Package-B",
        download_location="https://download.com",
    )
    file = File(
        name="Example file",
        spdx_id="SPDXRef-File",
        checksums=[
            Checksum(ChecksumAlgorithm.SHA1, "2fd4e1c67a2d28fced849ee1bb76e7391b93eb12")
        ],
    )

    relationships = [
        Relationship(
            "SPDXRef-DOCUMENT", RelationshipType.DESCRIBES, "SPDXRef-Package-A"
        ),
        Relationship(
            "SPDXRef-DOCUMENT", RelationshipType.DESCRIBES, "SPDXRef-Package-B"
        ),
        Relationship("SPDXRef-Package-A", RelationshipType.CONTAINS, "SPDXRef-File"),
        Relationship("SPDXRef-Package-B", RelationshipType.CONTAINS, "SPDXRef-File"),
    ]
    document = Document(
        creation_info=creation_info,
        packages=[package_a, package_b],
        files=[file],
        relationships=relationships,
    )

    return document

if __name__ == "__main__":
    document = _create_minimal_document()
    validation_messages_before_writing = validate_full_spdx_document(document)
    write_file(document, "test_file.spdx", False)
    document = parse_file("test_file.spdx")
    validation_messages_after_writing_and_parsing = validate_full_spdx_document(document)

    assert validation_messages_before_writing == validation_messages_after_writing_and_parsing
```






Discussion:

Patch:
diff --git a/src/spdx_tools/spdx/writer/tagvalue/tagvalue_writer.py b/src/spdx_tools/spdx/writer/tagvalue/tagvalue_writer.py
index d82a121..af58b8a 100644
--- a/src/spdx_tools/spdx/writer/tagvalue/tagvalue_writer.py
+++ b/src/spdx_tools/spdx/writer/tagvalue/tagvalue_writer.py
@@ -11,7 +11,7 @@
 from beartype.typing import List, TextIO
 
 from spdx_tools.spdx.document_utils import create_document_without_duplicates
-from spdx_tools.spdx.model import Document
+from spdx_tools.spdx.model import Document, Relationship, RelationshipType
 from spdx_tools.spdx.validation.document_validator import validate_full_spdx_document
 from spdx_tools.spdx.validation.validation_message import ValidationMessage
 from spdx_tools.spdx.writer.tagvalue.annotation_writer import write_annotation
@@ -70,17 +70,27 @@ def write_document(document: Document, text_output: TextIO):
                     file_ids_with_contained_snippets[file.spdx_id], write_snippet, text_output, with_separator=True
                 )
 
+    already_written_file_ids = []  # a file can belong to multiple packages but must appear only once
     for package in document.packages:
         write_package(package, text_output)
         write_separator(text_output)
         if package.spdx_id in contained_files_by_package_id:
             for file in contained_files_by_package_id[package.spdx_id]:
-                write_file(file, text_output)
-                write_separator(text_output)
-                if file.spdx_id in file_ids_with_contained_snippets:
-                    write_list_of_elements(
-                        file_ids_with_contained_snippets[file.spdx_id], write_snippet, text_output, with_separator=True
+                if file.spdx_id in already_written_file_ids:
+                    relationships_to_write.append(
+                        Relationship(package.spdx_id, RelationshipType.CONTAINS, file.spdx_id)
                     )
+                else:
+                    write_file(file, text_output)
+                    write_separator(text_output)
+                    if file.spdx_id in file_ids_with_contained_snippets:
+                        write_list_of_elements(
+                            file_ids_with_contained_snippets[file.spdx_id],
+                            write_snippet,
+                            text_output,
+                            with_separator=True,
+                        )
+                    already_written_file_ids.append(file.spdx_id)
 
     write_optional_heading(document.extracted_licensing_info, "## License Information\n", text_output)
     write_list_of_elements(


Test patch:
diff --git a/tests/spdx/writer/tagvalue/test_tagvalue_writer.py b/tests/spdx/writer/tagvalue/test_tagvalue_writer.py
index 995687c..86e7a2f 100644
--- a/tests/spdx/writer/tagvalue/test_tagvalue_writer.py
+++ b/tests/spdx/writer/tagvalue/test_tagvalue_writer.py
@@ -3,11 +3,24 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import os
+from datetime import datetime
 from unittest.mock import MagicMock, call, mock_open, patch
 
 import pytest
 
-from spdx_tools.spdx.model import File, Package, Relationship, RelationshipType, Snippet
+from spdx_tools.spdx.model import (
+    Actor,
+    ActorType,
+    Checksum,
+    ChecksumAlgorithm,
+    CreationInfo,
+    Document,
+    File,
+    Package,
+    Relationship,
+    RelationshipType,
+    Snippet,
+)
 from spdx_tools.spdx.parser.tagvalue import tagvalue_parser
 from spdx_tools.spdx.writer.tagvalue.tagvalue_writer import write_document, write_document_to_file
 from tests.spdx.fixtures import checksum_fixture, document_fixture
@@ -122,3 +135,87 @@ def test_correct_order_of_elements():
             call("\n"),
         ]
     )
+
+
+def test_same_file_in_multiple_packages():
+    creation_info = CreationInfo(
+        spdx_version="SPDX-2.3",
+        spdx_id="SPDXRef-DOCUMENT",
+        data_license="CC0-1.0",
+        name="SPDX Lite Document",
+        document_namespace="https://test.namespace.com",
+        creators=[Actor(ActorType.PERSON, "John Doe")],
+        created=datetime(2023, 3, 14, 8, 49),
+    )
+    package_a = Package(
+        name="Example package A",
+        spdx_id="SPDXRef-Package-A",
+        download_location="https://download.com",
+    )
+    package_b = Package(
+        name="Example package B",
+        spdx_id="SPDXRef-Package-B",
+        download_location="https://download.com",
+    )
+    file = File(
+        name="Example file",
+        spdx_id="SPDXRef-File",
+        checksums=[Checksum(ChecksumAlgorithm.SHA1, "2fd4e1c67a2d28fced849ee1bb76e7391b93eb12")],
+    )
+
+    relationships = [
+        Relationship("SPDXRef-DOCUMENT", RelationshipType.DESCRIBES, "SPDXRef-Package-A"),
+        Relationship("SPDXRef-DOCUMENT", RelationshipType.DESCRIBES, "SPDXRef-Package-B"),
+        Relationship("SPDXRef-Package-A", RelationshipType.CONTAINS, "SPDXRef-File"),
+        Relationship("SPDXRef-Package-B", RelationshipType.CONTAINS, "SPDXRef-File"),
+    ]
+    document = Document(
+        creation_info=creation_info,
+        packages=[package_a, package_b],
+        files=[file],
+        relationships=relationships,
+    )
+    mock: MagicMock = mock_open()
+    with patch(f"{__name__}.open", mock, create=True):
+        with open("foo", "w") as file:
+            write_document(document, file)
+
+    mock.assert_called_once_with("foo", "w")
+    handle = mock()
+    handle.write.assert_has_calls(
+        [
+            call("## Document Information\n"),
+            call("SPDXVersion: SPDX-2.3\n"),
+            call("DataLicense: CC0-1.0\n"),
+            call("SPDXID: SPDXRef-DOCUMENT\n"),
+            call("DocumentName: SPDX Lite Document\n"),
+            call("DocumentNamespace: https://test.namespace.com\n"),
+            call("\n"),
+            call("## Creation Information\n"),
+            call("Creator: Person: John Doe\n"),
+            call("Created: 2023-03-14T08:49:00Z\n"),
+            call("\n"),
+            call("## Package Information\n"),
+            call("PackageName: Example package A\n"),
+            call("SPDXID: SPDXRef-Package-A\n"),
+            call("PackageDownloadLocation: https://download.com\n"),
+            call("FilesAnalyzed: True\n"),
+            call("\n"),
+            call("## File Information\n"),
+            call("FileName: Example file\n"),
+            call("SPDXID: SPDXRef-File\n"),
+            call("FileChecksum: SHA1: 2fd4e1c67a2d28fced849ee1bb76e7391b93eb12\n"),
+            call("\n"),
+            call("## Package Information\n"),
+            call("PackageName: Example package B\n"),
+            call("SPDXID: SPDXRef-Package-B\n"),
+            call("PackageDownloadLocation: https://download.com\n"),
+            call("FilesAnalyzed: True\n"),
+            call("\n"),
+            call("## Relationships\n"),
+            call("Relationship: SPDXRef-DOCUMENT DESCRIBES SPDXRef-Package-A\n"),
+            call("Relationship: SPDXRef-DOCUMENT DESCRIBES SPDXRef-Package-B\n"),
+            call("Relationship: SPDXRef-Package-B CONTAINS SPDXRef-File\n"),
+            call("\n"),
+        ]
+    )