Repository: spdx/tools-python

Issue:
Remove `documentDescribes` and `hasFiles` from output
`documentDescribes` and `hasFiles` have now been officially deprecated, see here: https://github.com/spdx/spdx-spec/pull/841
We can now remove these from the json writer output.

Discussion:

Patch:
diff --git a/README.md b/README.md
index 41f2580..19ae4e3 100644
--- a/README.md
+++ b/README.md
@@ -102,7 +102,7 @@ instead of `bin`.
     * Note: in-place manipulations like `list.append(item)` will circumvent the type checking (a `TypeError` will still be raised when reading `list` again). We recommend using `list = list + [item]` instead.
   * The main entry point of an SPDX document is the `Document` class, which links to all other classes.
   * For license handling, the [license_expression](https://github.com/nexB/license-expression) library is used.
-  * Note on `documentDescribes` and `hasFiles`: These fields will be converted to relationships in the internal data model. During serialization, they will be written again where appropriate.
+  * Note on `documentDescribes` and `hasFiles`: These fields will be converted to relationships in the internal data model. As they are deprecated, these fields will not be written in the output.
 2. **PARSING**
   * Use `parse_file(file_name)` from the `parse_anything.py` module to parse an arbitrary file with one of the supported file endings.
   * Successful parsing will return a `Document` instance. Unsuccessful parsing will raise `SPDXParsingError` with a list of all encountered problems.
diff --git a/src/spdx/jsonschema/document_converter.py b/src/spdx/jsonschema/document_converter.py
index 56bba28..2b9967d 100644
--- a/src/spdx/jsonschema/document_converter.py
+++ b/src/spdx/jsonschema/document_converter.py
@@ -16,13 +16,6 @@ from spdx.jsonschema.package_converter import PackageConverter
 from spdx.jsonschema.relationship_converter import RelationshipConverter
 from spdx.jsonschema.snippet_converter import SnippetConverter
 from spdx.model.document import Document
-from spdx.model.relationship import RelationshipType
-from spdx.model.relationship_filters import (
-    filter_by_type_and_origin,
-    filter_by_type_and_target,
-    find_file_contained_by_package_relationships,
-    find_package_contains_file_relationships,
-)
 
 
 class DocumentConverter(TypedConverter[Document]):
@@ -90,20 +83,6 @@ class DocumentConverter(TypedConverter[Document]):
             return document.creation_info.spdx_version
         elif document_property == DocumentProperty.DOCUMENT_NAMESPACE:
             return document.creation_info.document_namespace
-        elif document_property == DocumentProperty.DOCUMENT_DESCRIBES:
-            describes_ids = [
-                relationship.related_spdx_element_id
-                for relationship in filter_by_type_and_origin(
-                    document.relationships, RelationshipType.DESCRIBES, document.creation_info.spdx_id
-                )
-            ]
-            described_by_ids = [
-                relationship.spdx_element_id
-                for relationship in filter_by_type_and_target(
-                    document.relationships, RelationshipType.DESCRIBED_BY, document.creation_info.spdx_id
-                )
-            ]
-            return describes_ids + described_by_ids or None
         elif document_property == DocumentProperty.PACKAGES:
             return [self.package_converter.convert(package, document) for package in document.packages] or None
         elif document_property == DocumentProperty.FILES:
@@ -111,22 +90,6 @@ class DocumentConverter(TypedConverter[Document]):
         elif document_property == DocumentProperty.SNIPPETS:
             return [self.snippet_converter.convert(snippet, document) for snippet in document.snippets] or None
         elif document_property == DocumentProperty.RELATIONSHIPS:
-            already_covered_relationships = filter_by_type_and_origin(
-                document.relationships, RelationshipType.DESCRIBES, document.creation_info.spdx_id
-            )
-            already_covered_relationships.extend(
-                filter_by_type_and_target(
-                    document.relationships, RelationshipType.DESCRIBED_BY, document.creation_info.spdx_id
-                )
-            )
-            for package in document.packages:
-                already_covered_relationships.extend(find_package_contains_file_relationships(document, package))
-                already_covered_relationships.extend(find_file_contained_by_package_relationships(document, package))
-            relationships_to_ignore = [
-                relationship for relationship in already_covered_relationships if relationship.comment is None
-            ]
             return [
-                self.relationship_converter.convert(relationship)
-                for relationship in document.relationships
-                if relationship not in relationships_to_ignore
+                self.relationship_converter.convert(relationship) for relationship in document.relationships
             ] or None
diff --git a/src/spdx/jsonschema/document_properties.py b/src/spdx/jsonschema/document_properties.py
index 6fc244a..045ecb7 100644
--- a/src/spdx/jsonschema/document_properties.py
+++ b/src/spdx/jsonschema/document_properties.py
@@ -17,7 +17,6 @@ class DocumentProperty(JsonProperty):
     NAME = auto()
     SPDX_VERSION = auto()
     DOCUMENT_NAMESPACE = auto()
-    DOCUMENT_DESCRIBES = auto()
     PACKAGES = auto()
     FILES = auto()
     SNIPPETS = auto()
diff --git a/src/spdx/jsonschema/package_converter.py b/src/spdx/jsonschema/package_converter.py
index 0141289..0019b80 100644
--- a/src/spdx/jsonschema/package_converter.py
+++ b/src/spdx/jsonschema/package_converter.py
@@ -15,10 +15,6 @@ from spdx.jsonschema.package_verification_code_converter import PackageVerificat
 from spdx.model.actor import Actor
 from spdx.model.document import Document
 from spdx.model.package import Package
-from spdx.model.relationship_filters import (
-    find_file_contained_by_package_relationships,
-    find_package_contains_file_relationships,
-)
 
 
 class PackageConverter(TypedConverter[Package]):
@@ -71,16 +67,6 @@ class PackageConverter(TypedConverter[Package]):
             ] or None
         elif package_property == PackageProperty.FILES_ANALYZED:
             return package.files_analyzed
-        elif package_property == PackageProperty.HAS_FILES:
-            package_contains_file_ids = [
-                relationship.related_spdx_element_id
-                for relationship in find_package_contains_file_relationships(document, package)
-            ]
-            file_contained_in_package_ids = [
-                relationship.spdx_element_id
-                for relationship in find_file_contained_by_package_relationships(document, package)
-            ]
-            return package_contains_file_ids + file_contained_in_package_ids or None
         elif package_property == PackageProperty.HOMEPAGE:
             return apply_if_present(str, package.homepage)
         elif package_property == PackageProperty.LICENSE_COMMENTS:
diff --git a/src/spdx/jsonschema/package_properties.py b/src/spdx/jsonschema/package_properties.py
index 5b3b42e..60ba0ff 100644
--- a/src/spdx/jsonschema/package_properties.py
+++ b/src/spdx/jsonschema/package_properties.py
@@ -18,7 +18,6 @@ class PackageProperty(JsonProperty):
     DOWNLOAD_LOCATION = auto()
     EXTERNAL_REFS = auto()
     FILES_ANALYZED = auto()
-    HAS_FILES = auto()
     HOMEPAGE = auto()
     LICENSE_COMMENTS = auto()
     LICENSE_CONCLUDED = auto()


Test patch:
diff --git a/tests/spdx/jsonschema/test_document_converter.py b/tests/spdx/jsonschema/test_document_converter.py
index 149eca9..ba964e7 100644
--- a/tests/spdx/jsonschema/test_document_converter.py
+++ b/tests/spdx/jsonschema/test_document_converter.py
@@ -11,7 +11,6 @@ import pytest
 from spdx.jsonschema.annotation_converter import AnnotationConverter
 from spdx.jsonschema.document_converter import DocumentConverter
 from spdx.jsonschema.document_properties import DocumentProperty
-from spdx.jsonschema.relationship_converter import RelationshipConverter
 from spdx.model.actor import Actor, ActorType
 from spdx.model.annotation import Annotation, AnnotationType
 from spdx.model.document import Document
@@ -20,14 +19,12 @@ from spdx.model.relationship import Relationship, RelationshipType
 from tests.spdx.fixtures import (
     annotation_fixture,
     creation_info_fixture,
-    document_fixture,
     external_document_ref_fixture,
     file_fixture,
     package_fixture,
-    relationship_fixture,
     snippet_fixture,
 )
-from tests.spdx.mock_utils import assert_mock_method_called_with_arguments, assert_no_mock_methods_called
+from tests.spdx.mock_utils import assert_mock_method_called_with_arguments
 
 
 @pytest.fixture
@@ -139,11 +136,13 @@ def test_successful_conversion(converter: DocumentConverter):
         converter.json_property_name(DocumentProperty.NAME): "name",
         converter.json_property_name(DocumentProperty.SPDX_VERSION): "spdxVersion",
         converter.json_property_name(DocumentProperty.DOCUMENT_NAMESPACE): "namespace",
-        converter.json_property_name(DocumentProperty.DOCUMENT_DESCRIBES): ["describedElementId"],
         converter.json_property_name(DocumentProperty.PACKAGES): ["mock_converted_package"],
         converter.json_property_name(DocumentProperty.FILES): ["mock_converted_file"],
         converter.json_property_name(DocumentProperty.SNIPPETS): ["mock_converted_snippet"],
-        converter.json_property_name(DocumentProperty.RELATIONSHIPS): ["mock_converted_relationship"],
+        converter.json_property_name(DocumentProperty.RELATIONSHIPS): [
+            "mock_converted_relationship",
+            "mock_converted_relationship",
+        ],
     }
 
 
@@ -163,7 +162,6 @@ def test_null_values(converter: DocumentConverter):
     assert converter.json_property_name(DocumentProperty.ANNOTATIONS) not in converted_dict
     assert converter.json_property_name(DocumentProperty.EXTERNAL_DOCUMENT_REFS) not in converted_dict
     assert converter.json_property_name(DocumentProperty.HAS_EXTRACTED_LICENSING_INFOS) not in converted_dict
-    assert converter.json_property_name(DocumentProperty.DOCUMENT_DESCRIBES) not in converted_dict
     assert converter.json_property_name(DocumentProperty.PACKAGES) not in converted_dict
     assert converter.json_property_name(DocumentProperty.FILES) not in converted_dict
     assert converter.json_property_name(DocumentProperty.SNIPPETS) not in converted_dict
@@ -205,88 +203,3 @@ def test_document_annotations(converter: DocumentConverter):
     assert_mock_method_called_with_arguments(annotation_converter, "convert", document_annotation, other_annotation)
     converted_document_annotations = converted_dict.get(converter.json_property_name(DocumentProperty.ANNOTATIONS))
     assert converted_document_annotations == ["mock_converted_annotation", "mock_converted_annotation"]
-
-
-def test_document_describes(converter: DocumentConverter):
-    document = document_fixture()
-    document_id = document.creation_info.spdx_id
-    document_describes_relationship = relationship_fixture(
-        spdx_element_id=document_id,
-        relationship_type=RelationshipType.DESCRIBES,
-        related_spdx_element_id="describesId",
-    )
-    described_by_document_relationship = relationship_fixture(
-        related_spdx_element_id=document_id,
-        relationship_type=RelationshipType.DESCRIBED_BY,
-        spdx_element_id="describedById",
-    )
-    other_describes_relationship = relationship_fixture(
-        spdx_element_id="DocumentRef-external", relationship_type=RelationshipType.DESCRIBES
-    )
-    other_relationship = relationship_fixture(spdx_element_id=document_id, relationship_type=RelationshipType.CONTAINS)
-    document.relationships = [
-        document_describes_relationship,
-        described_by_document_relationship,
-        other_describes_relationship,
-        other_relationship,
-    ]
-
-    converted_dict = converter.convert(document)
-
-    document_describes = converted_dict.get(converter.json_property_name(DocumentProperty.DOCUMENT_DESCRIBES))
-    assert document_describes == [
-        document_describes_relationship.related_spdx_element_id,
-        described_by_document_relationship.spdx_element_id,
-    ]
-
-
-DOCUMENT_ID = "docConverterTestDocumentId"
-PACKAGE_ID = "docConverterTestPackageId"
-FILE_ID = "docConverterTestFileId"
-
-
-@pytest.mark.parametrize(
-    "relationship,should_be_written",
-    [
-        (relationship_fixture(DOCUMENT_ID, RelationshipType.DESCRIBES), True),
-        (relationship_fixture(DOCUMENT_ID, RelationshipType.DESCRIBES, comment=None), False),
-        (
-            relationship_fixture(relationship_type=RelationshipType.DESCRIBED_BY, related_spdx_element_id=DOCUMENT_ID),
-            True,
-        ),
-        (
-            relationship_fixture(
-                relationship_type=RelationshipType.DESCRIBED_BY, related_spdx_element_id=DOCUMENT_ID, comment=None
-            ),
-            False,
-        ),
-        (relationship_fixture(DOCUMENT_ID, RelationshipType.AMENDS, comment=None), True),
-        (relationship_fixture(PACKAGE_ID, RelationshipType.CONTAINS, FILE_ID), True),
-        (relationship_fixture(PACKAGE_ID, RelationshipType.CONTAINS, FILE_ID, comment=None), False),
-        (relationship_fixture(FILE_ID, RelationshipType.CONTAINED_BY, PACKAGE_ID), True),
-        (relationship_fixture(FILE_ID, RelationshipType.CONTAINED_BY, PACKAGE_ID, comment=None), False),
-        (relationship_fixture(PACKAGE_ID, RelationshipType.CONTAINS, comment=None), True),
-        (relationship_fixture(PACKAGE_ID, RelationshipType.COPY_OF, FILE_ID, comment=None), True),
-    ],
-)
-def test_document_relationships(converter: DocumentConverter, relationship: Relationship, should_be_written: bool):
-    package = package_fixture(spdx_id=PACKAGE_ID)
-    file = file_fixture(spdx_id=FILE_ID)
-    document = document_fixture(
-        creation_info_fixture(spdx_id=DOCUMENT_ID), packages=[package], files=[file], relationships=[relationship]
-    )
-
-    # Weird type hint to make warnings about unresolved references from the mock class disappear
-    relationship_converter: Union[RelationshipConverter, NonCallableMagicMock] = converter.relationship_converter
-    relationship_converter.convert.return_value = "mock_converted_relationship"
-
-    converted_dict = converter.convert(document)
-
-    relationships = converted_dict.get(converter.json_property_name(DocumentProperty.RELATIONSHIPS))
-
-    if should_be_written:
-        assert_mock_method_called_with_arguments(relationship_converter, "convert", relationship)
-        assert relationships == ["mock_converted_relationship"]
-    else:
-        assert_no_mock_methods_called(relationship_converter)
-        assert relationships is None
diff --git a/tests/spdx/jsonschema/test_package_converter.py b/tests/spdx/jsonschema/test_package_converter.py
index cb8466e..d352365 100644
--- a/tests/spdx/jsonschema/test_package_converter.py
+++ b/tests/spdx/jsonschema/test_package_converter.py
@@ -17,7 +17,6 @@ from spdx.model.annotation import Annotation, AnnotationType
 from spdx.model.checksum import Checksum, ChecksumAlgorithm
 from spdx.model.document import Document
 from spdx.model.package import Package, PackagePurpose, PackageVerificationCode
-from spdx.model.relationship import RelationshipType
 from spdx.model.spdx_no_assertion import SPDX_NO_ASSERTION_STRING, SpdxNoAssertion
 from spdx.model.spdx_none import SPDX_NONE_STRING, SpdxNone
 from tests.spdx.fixtures import (
@@ -25,10 +24,7 @@ from tests.spdx.fixtures import (
     creation_info_fixture,
     document_fixture,
     external_package_ref_fixture,
-    file_fixture,
     package_fixture,
-    relationship_fixture,
-    snippet_fixture,
 )
 from tests.spdx.mock_utils import assert_mock_method_called_with_arguments
 
@@ -66,7 +62,6 @@ def converter(
         (PackageProperty.DOWNLOAD_LOCATION, "downloadLocation"),
         (PackageProperty.EXTERNAL_REFS, "externalRefs"),
         (PackageProperty.FILES_ANALYZED, "filesAnalyzed"),
-        (PackageProperty.HAS_FILES, "hasFiles"),
         (PackageProperty.HOMEPAGE, "homepage"),
         (PackageProperty.LICENSE_COMMENTS, "licenseComments"),
         (PackageProperty.LICENSE_CONCLUDED, "licenseConcluded"),
@@ -230,7 +225,6 @@ def test_null_values(converter: PackageConverter):
     assert converter.json_property_name(PackageProperty.ATTRIBUTION_TEXTS) not in converted_dict
     assert converter.json_property_name(PackageProperty.CHECKSUMS) not in converted_dict
     assert converter.json_property_name(PackageProperty.EXTERNAL_REFS) not in converted_dict
-    assert converter.json_property_name(PackageProperty.HAS_FILES) not in converted_dict
     assert converter.json_property_name(PackageProperty.LICENSE_INFO_FROM_FILES) not in converted_dict
 
 
@@ -314,45 +308,3 @@ def test_package_annotations(converter: PackageConverter):
     )
     converted_file_annotations = converted_dict.get(converter.json_property_name(PackageProperty.ANNOTATIONS))
     assert converted_file_annotations == ["mock_converted_annotation", "mock_converted_annotation"]
-
-
-def test_has_files(converter: PackageConverter):
-    package = package_fixture()
-    first_contained_file = file_fixture(spdx_id="firstFileId")
-    second_contained_file = file_fixture(spdx_id="secondFileId")
-    non_contained_file = file_fixture(spdx_id="otherFileId")
-    snippet = snippet_fixture()
-    document = document_fixture(
-        packages=[package], files=[first_contained_file, second_contained_file, non_contained_file], snippets=[snippet]
-    )
-    package_contains_file_relationship = relationship_fixture(
-        spdx_element_id=package.spdx_id,
-        relationship_type=RelationshipType.CONTAINS,
-        related_spdx_element_id=first_contained_file.spdx_id,
-    )
-    file_contained_in_package_relationship = relationship_fixture(
-        spdx_element_id=second_contained_file.spdx_id,
-        relationship_type=RelationshipType.CONTAINED_BY,
-        related_spdx_element_id=package.spdx_id,
-    )
-    package_contains_snippet_relationship = relationship_fixture(
-        spdx_element_id=package.spdx_id,
-        relationship_type=RelationshipType.CONTAINS,
-        related_spdx_element_id=snippet.spdx_id,
-    )
-    package_describes_file_relationship = relationship_fixture(
-        spdx_element_id=package.spdx_id,
-        relationship_type=RelationshipType.DESCRIBES,
-        related_spdx_element_id=non_contained_file.spdx_id,
-    )
-    document.relationships = [
-        package_contains_file_relationship,
-        file_contained_in_package_relationship,
-        package_contains_snippet_relationship,
-        package_describes_file_relationship,
-    ]
-
-    converted_dict = converter.convert(package, document)
-
-    has_files = converted_dict.get(converter.json_property_name(PackageProperty.HAS_FILES))
-    assert has_files == [first_contained_file.spdx_id, second_contained_file.spdx_id]
diff --git a/tests/spdx/writer/json/expected_results/expected.json b/tests/spdx/writer/json/expected_results/expected.json
index 0d444ba..264cf04 100644
--- a/tests/spdx/writer/json/expected_results/expected.json
+++ b/tests/spdx/writer/json/expected_results/expected.json
@@ -10,9 +10,6 @@
         "licenseListVersion": "3.19"
     },
     "dataLicense": "CC0-1.0",
-    "documentDescribes": [
-        "SPDXRef-File"
-    ],
     "documentNamespace": "https://some.namespace",
     "externalDocumentRefs": [
         {