Repository: spdx/tools-python

Issue:
Incorrect RDF generated in 1 document with 2 packages
I am trying to generate a simple document with two packages. I am not sure I use the library or SPDX properly so any help would be appreciated.

My code creates an SPDX document and then adds two packages to it. Then I try to parse the file with another script. Two packages are indeed detected, but some of the properties are misplaced (they are parsed in the wrong package).
Taking a look at the generated XML I can see only 1 package element and all attributes are mixed up inside of it.

Am I making a mistake or it is a bug in the library?

Here is the generating code:

```python
import codecs
import logging
import os

from spdx.writers.rdf import write_document, InvalidDocumentError
from spdx.parsers.loggers import ErrorMessages

def write_spdx_to_file(doc, path: str):
    with open(path, 'wb') as out:
        try:
            write_document(doc, out)
            logging.info("The document has been saved to: '%s'", path)
        except InvalidDocumentError:
            logging.error('Document is Invalid')
            messages = ErrorMessages()
            doc.validate(messages)
            logging.error('\n'.join(messages))
            # Error handling can be customized here, e.g. mail sending

def generate_sample_spdx_3():
    """
    A more specific example of SPDX document with 2 package to illustrate python SPDX tools problems to the community.
    """
    doc = Document()
    doc.spdx_id = "Abc-SPDXRef-DOCUMENT"
    doc.version = Version(2, 2)
    doc.data_license = License.from_identifier('CC0-1.0')
    doc.namespace = "com.example"
    doc.name = "http://example.com/documents/abc"
    doc.comment = 'Example Document'
    
    doc.creation_info.add_creator(Person('John Smith', 'john@example.com'))
    doc.creation_info.add_creator(Organization("Example Ltd.", "info@example.com"))
    doc.creation_info.add_creator(Tool("def"))
    doc.creation_info.set_created_now()

    # Package 1
    package = Package()
    package.name = 'TagWriteTest'
    package.spdx_id = "TagWriteTest"
    package.version = '1.2'
    package.file_name = 'twt.jar'
    package.download_location = 'http://www.tagwritetest.test/download'
    package.homepage = SPDXNone()
    package.verif_code = '4e3211c67a2d28fced849ee1bb76e7391b93feba'
    license_set = LicenseConjunction(License.from_identifier('Apache-2.0'), License.from_identifier('BSD-2-Clause'))
    package.conc_lics = license_set
    package.license_declared = license_set
    package.add_lics_from_file(License.from_identifier('Apache-2.0'))
    package.add_lics_from_file(License.from_identifier('BSD-2-Clause'))
    package.cr_text = NoAssert()
    package.summary = 'Simple package 1.'
    package.description = 'Really simple package 1.'
    doc.add_package(package)

    # Package 2
    package = Package()
    package.name = 'FooTest'
    package.spdx_id = "FooTest"
    package.version = '3.4'
    package.file_name = 'foo.jar'
    package.download_location = 'http://www.footest.test/download'
    package.homepage = SPDXNone()
    package.verif_code = '4e3211c67a2d28fced849ee1bb76e7391b93feb1'
    license_set = LicenseConjunction(License.from_identifier('Apache-2.0'), License.from_identifier('BSD-2-Clause'))
    package.conc_lics = license_set
    package.license_declared = license_set
    package.add_lics_from_file(License.from_identifier('Apache-2.0'))
    package.add_lics_from_file(License.from_identifier('BSD-2-Clause'))
    package.cr_text = NoAssert()
    package.summary = 'Simple package 2.'
    package.description = 'Really simple package 2.'
    doc.add_package(package)

    write_spdx_to_file(doc, "/tmp/abc.rdf")

    
if __name__ == '__main__':
    logging.basicConfig(level=logging.DEBUG)
    generate_sample_spdx_3()
```

The parsing is done like so:

```python
import logging
import spdx

from spdx.parsers.parse_anything import parse_file

def test_parse_spdx(input_file: str):

    doc, errors = parse_file(input_file)

    if errors:
        logging.warning("Errors while parsing SPDX document: errors=%s, file='%s'", errors, input_file)

    logging.debug("SPDX document: %s", doc.name)
    
    print(len(doc.packages))

    for package in doc.packages:
        print(package.name)
        print(package.version)
        print(package.summary)
        print(package.description)

if __name__ == '__main__':
    logging.basicConfig(level=logging.DEBUG)
    test_parse_spdx("/tmp/abc.rdf")
```

This produces the following output (notice the first package version, summary and descriptions are None and the second ones belong to the first package and are showed in the wrong place):

```
More than one Package version info defined.
More than one Package file name defined.
More than one Package download location defined.
More than one package verification code defined.
More than one package http://spdx.org/rdf/terms#licenseConcluded defined.
More than one package http://spdx.org/rdf/terms#licenseDeclared defined.
More than one package summary defined.
More than one package description defined.
WARNING:root:Errors while parsing SPDX document: errors=True, file='/tmp/abc.rdf'
DEBUG:root:SPDX document: http://example.com/documents/abc
2
TagWriteTest
None
None
None
FooTest
1.2
Simple package 2.
Really simple package 1.
```

And finally, here is the generated XML:

```xml
<?xml version="1.0" encoding="utf-8"?>
<rdf:RDF
  xmlns:ns1="http://spdx.org/rdf/terms#"
  xmlns:ns2="http://usefulinc.com/ns/doap#"
  xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>
  <ns1:SpdxDocument rdf:about="http://www.spdx.org/tools#SPDXRef-DOCUMENT">
    <ns1:specVersion>SPDX-2.2</ns1:specVersion>
    <ns1:name rdf:resource="http://example.com/documents/abc"/>
    <ns1:dataLicense rdf:resource="http://spdx.org/licenses/CC0-1.0"/>
    <ns1:describesPackage>
      <ns1:Package rdf:about="http://www.spdx.org/tools#SPDXRef-Package">
        <ns2:homepage rdf:resource="http://spdx.org/rdf/terms#none"/>
        <ns1:licenseDeclared>
          <ns1:ConjunctiveLicenseSet rdf:nodeID="N803aaddeb84a40c194f448b61db20a18">
            <ns1:member rdf:resource="http://spdx.org/licenses/BSD-2-Clause"/>
            <ns1:member rdf:resource="http://spdx.org/licenses/Apache-2.0"/>
          </ns1:ConjunctiveLicenseSet>
        </ns1:licenseDeclared>
        <ns1:licenseDeclared>
          <ns1:ConjunctiveLicenseSet rdf:nodeID="Nbc56a48570a34e11af70d50c96a650b6">
            <ns1:member rdf:resource="http://spdx.org/licenses/Apache-2.0"/>
            <ns1:member rdf:resource="http://spdx.org/licenses/BSD-2-Clause"/>
          </ns1:ConjunctiveLicenseSet>
        </ns1:licenseDeclared>
        <ns1:name>TagWriteTest</ns1:name>
        <ns1:name>FooTest</ns1:name>
        <ns1:licenseInfoFromFiles rdf:resource="http://spdx.org/licenses/Apache-2.0"/>
        <ns1:licenseInfoFromFiles rdf:resource="http://spdx.org/licenses/BSD-2-Clause"/>
        <ns1:packageVerificationCode>
          <ns1:PackageVerificationCode rdf:nodeID="N6ce231f395d749fbb38fbe4db70c55ea">
            <ns1:packageVerificationCodeValue>4e3211c67a2d28fced849ee1bb76e7391b93feba</ns1:packageVerificationCodeValue>
          </ns1:PackageVerificationCode>
        </ns1:packageVerificationCode>
        <ns1:packageVerificationCode>
          <ns1:PackageVerificationCode rdf:nodeID="N8759686ff5f344d7a7efa06c2565377d">
            <ns1:packageVerificationCodeValue>4e3211c67a2d28fced849ee1bb76e7391b93feb1</ns1:packageVerificationCodeValue>
          </ns1:PackageVerificationCode>
        </ns1:packageVerificationCode>
        <ns1:summary>Simple package 2.</ns1:summary>
        <ns1:summary>Simple package 1.</ns1:summary>
        <ns1:packageFileName>twt.jar</ns1:packageFileName>
        <ns1:packageFileName>foo.jar</ns1:packageFileName>
        <ns1:versionInfo>1.2</ns1:versionInfo>
        <ns1:versionInfo>3.4</ns1:versionInfo>
        <ns1:downloadLocation>http://www.footest.test/download</ns1:downloadLocation>
        <ns1:downloadLocation>http://www.tagwritetest.test/download</ns1:downloadLocation>
        <ns1:description>Really simple package 1.</ns1:description>
        <ns1:description>Really simple package 2.</ns1:description>
        <ns1:licenseConcluded>
          <ns1:ConjunctiveLicenseSet rdf:nodeID="N091584493e0e4624bd54e640cc5bb3ed">
            <ns1:member rdf:resource="http://spdx.org/licenses/BSD-2-Clause"/>
            <ns1:member rdf:resource="http://spdx.org/licenses/Apache-2.0"/>
          </ns1:ConjunctiveLicenseSet>
        </ns1:licenseConcluded>
        <ns1:licenseConcluded>
          <ns1:ConjunctiveLicenseSet rdf:nodeID="N35334f5df81f4e6facf7fe32a49936c2">
            <ns1:member rdf:resource="http://spdx.org/licenses/Apache-2.0"/>
            <ns1:member rdf:resource="http://spdx.org/licenses/BSD-2-Clause"/>
          </ns1:ConjunctiveLicenseSet>
        </ns1:licenseConcluded>
        <ns1:Package rdf:resource="TagWriteTest"/>
        <ns1:Package rdf:resource="FooTest"/>
        <ns1:copyrightText rdf:resource="http://spdx.org/rdf/terms#noassertion"/>
      </ns1:Package>
    </ns1:describesPackage>
    <ns1:creationInfo>
      <ns1:CreationInfo rdf:nodeID="N38c8160ed5124b269ed54d8cf045f6c0">
        <ns1:creator>Organization: Example Ltd. (info@example.com)</ns1:creator>
        <ns1:creator>Person: John Smith (john@example.com)</ns1:creator>
        <ns1:creator>Tool: def</ns1:creator>
        <ns1:created>2023-03-09T20:45:07Z</ns1:created>
      </ns1:CreationInfo>
    </ns1:creationInfo>
  </ns1:SpdxDocument>
</rdf:RDF>
```

There is only one Package and all attributes are mixed up in it. I expect two Package elements to be generated. What causes this issue?

Discussion:

Patch:
diff --git a/spdx/writers/rdf.py b/spdx/writers/rdf.py
index 6622072..c64b36b 100644
--- a/spdx/writers/rdf.py
+++ b/spdx/writers/rdf.py
@@ -793,18 +793,9 @@ class PackageWriter(LicenseWriter):
         Return a Node representing the package.
         Files must have been added to the graph before this method is called.
         """
-        package_node = URIRef("http://www.spdx.org/tools#SPDXRef-Package")
+        package_node = URIRef(f"http://www.spdx.org/tools#{package.spdx_id}")
         type_triple = (package_node, RDF.type, self.spdx_namespace.Package)
         self.graph.add(type_triple)
-        # Package SPDXID
-        if package.spdx_id:
-            pkg_spdx_id = URIRef(package.spdx_id)
-            pkg_spdx_id_triple = (
-                package_node,
-                self.spdx_namespace.Package,
-                pkg_spdx_id,
-            )
-            self.graph.add(pkg_spdx_id_triple)
         # Handle optional fields:
         self.handle_pkg_optional_fields(package, package_node)
         # package name


Test patch:
diff --git a/tests/data/doc_write/rdf-simple-plus.json b/tests/data/doc_write/rdf-simple-plus.json
index d2b65ae..9a50011 100644
--- a/tests/data/doc_write/rdf-simple-plus.json
+++ b/tests/data/doc_write/rdf-simple-plus.json
@@ -6,9 +6,6 @@
       "ns1:describesPackage": {
         "ns1:Package": {
           "@rdf:about": "http://www.spdx.org/tools#SPDXRef-Package",
-          "ns1:Package": {
-            "@rdf:resource": "SPDXRef-Package"
-          },
           "ns1:hasFile": {
             "@rdf:resource": "http://www.spdx.org/files#SPDXRef-File"
           },
diff --git a/tests/data/doc_write/rdf-simple.json b/tests/data/doc_write/rdf-simple.json
index 00064a3..6b1ac2d 100644
--- a/tests/data/doc_write/rdf-simple.json
+++ b/tests/data/doc_write/rdf-simple.json
@@ -6,9 +6,6 @@
       "ns1:describesPackage": {
         "ns1:Package": {
           "@rdf:about": "http://www.spdx.org/tools#SPDXRef-Package",
-          "ns1:Package": {
-            "@rdf:resource": "SPDXRef-Package"
-          },
           "ns1:hasFile": {
             "@rdf:resource": "http://www.spdx.org/files#SPDXRef-File"
           },
diff --git a/tests/test_rdf_writer.py b/tests/test_rdf_writer.py
index 9153dba..65b0347 100644
--- a/tests/test_rdf_writer.py
+++ b/tests/test_rdf_writer.py
@@ -1,4 +1,6 @@
 import os
+from typing import Optional
+from unittest import TestCase
 
 import pytest
 from rdflib import URIRef
@@ -62,6 +64,35 @@ def test_external_package_references(temporary_file_path) -> None:
     assert second_ref.category in parsed_reference_categories
 
 
+# This test is really clunky since it's hard to isolate features of the rdf writer to test. Should be improved when
+# that part is refactored.
+def test_multiple_packages_in_one_document(temporary_file_path) -> None:
+    doc_node = URIRef("http://www.spdx.org/tools#SPDXRef-DOCUMENT")
+    document = Document()
+    document.creation_info.set_created_now()
+    package = Package()
+    package.spdx_id = "SPDXRef-Package"
+    package.version = "2.1"
+    document.add_package(package)
+    package2 = Package()
+    package2.spdx_id = "SPDXRef-Another-Package"
+    package2.version = "2.3"
+    document.add_package(package2)
+
+    with open(temporary_file_path, "wb") as out:
+        writer = Writer(document, out)
+        writer.write(doc_node)
+    parser = Parser(Builder(), StandardLogger())
+    with open(temporary_file_path, "r") as file:
+        parsed_document: Document = parser.parse(file)[0]
+
+    assert len(parsed_document.packages) == 2
+    first_package = get_package_by_spdx_id("SPDXRef-Package", document)
+    assert first_package.version == "2.1"
+    second_package = get_package_by_spdx_id("SPDXRef-Another-Package", document)
+    assert second_package.version == "2.3"
+
+
 def minimal_document_with_package() -> Document:
     document = Document(data_license=License.from_identifier('CC0-1.0'))
     document.creation_info.set_created_now()
@@ -72,7 +103,15 @@ def minimal_document_with_package() -> Document:
 
 def minimal_package() -> Package:
     package = Package()
+    package.spdx_id = "SPDXRef-Package"
     package.conc_lics = NoAssert()
     package.license_declared = NoAssert()
     package.add_lics_from_file(NoAssert())
     return package
+
+
+def get_package_by_spdx_id(package_spdx_id: str, document: Document) -> Optional[Package]:
+    for package in document.packages:
+        if package.spdx_id == package_spdx_id:
+            return package
+    return None