Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted bug fixes #263

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion spdx/relationship.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ def __init__(self, relationship=None, relationship_comment=None):
self.relationship = relationship
self.relationship_comment = relationship_comment

def __eq__(self, other):
return (
isinstance(other, Relationship)
and self.relationship == other.relationship
)

@property
def has_comment(self):
return self.relationship_comment is not None
Expand All @@ -103,7 +109,7 @@ def validate(self, messages):
Check that all the fields are valid.
Appends any error messages to messages parameter shall be a ErrorMessages.
"""
self.validate_relationship(messages)
return self.validate_relationship(messages)

def validate_relationship(self, messages):
r_type = self.relationship.split(" ")[1]
Expand All @@ -112,3 +118,5 @@ def validate_relationship(self, messages):
"Relationship type must be one of the constants defined in "
"class spdx.relationship.Relationship"
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use these two functions if the validate method only calls the validate_relationship method? IMHO we should only use the validate_relationship method.


return messages
4 changes: 4 additions & 0 deletions spdx/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ def to_value(self):
def __str__(self):
return self.to_value()

def __repr__(self):
return self.to_value()


class UnKnown(object):
"""
Expand All @@ -91,6 +94,7 @@ def __repr__(self):
def __eq__(self, other):
return self.to_value() == other.to_value()


class SPDXNone(object):
"""
Represent SPDX None value.
Expand Down
10 changes: 9 additions & 1 deletion spdx/writers/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
from spdx.writers.tagvalue import InvalidDocumentError
from spdx.writers.jsonyamlxml import Writer
from spdx.parsers.loggers import ErrorMessages
import datetime


def json_converter(obj):
if isinstance(obj, datetime.datetime):
return str(obj)
else:
raise TypeError("No implementation available to serialize objects of type " + type(obj).__name__)


def write_document(document, out, validate=True):
Expand All @@ -26,4 +34,4 @@ def write_document(document, out, validate=True):

writer = Writer(document)
document_object = writer.create_document()
json.dump(document_object, out, indent=4)
json.dump(document_object, out, indent=4, default=json_converter)
15 changes: 11 additions & 4 deletions spdx/writers/jsonyamlxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,12 @@ def __init__(self, document):

def create_extracted_license(self):
extracted_license_objects = []
extracted_licenses = self.document.extracted_licenses
unique_extracted_licenses = {}
for lic in self.document.extracted_licenses:
if lic.identifier not in unique_extracted_licenses.keys():
unique_extracted_licenses[lic.identifier] = lic

for extracted_license in extracted_licenses:
for extracted_license in unique_extracted_licenses.values():
extracted_license_object = dict()

if isinstance(extracted_license.identifier, Literal):
Expand Down Expand Up @@ -498,9 +501,14 @@ def create_document(self):
document_describes = self.create_document_describes()
self.document_object["documentDescribes"] = document_describes

unique_doc_packages = {}
for doc_package in self.document.packages:
if doc_package.spdx_id not in unique_doc_packages.keys():
unique_doc_packages[doc_package.spdx_id] = doc_package

package_objects = []
file_objects = []
for package in self.document.packages:
for package in unique_doc_packages.values():
package_info_object, files_in_package = self.create_package_info(package)
package_objects.append(package_info_object)
file_objects.extend(file for file in files_in_package if file not in file_objects)
Expand Down Expand Up @@ -533,4 +541,3 @@ def create_document(self):
self.document_object["relationships"] = self.create_relationship_info()

return self.document_object

9 changes: 5 additions & 4 deletions spdx/writers/rdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,11 @@ def write(self, doc_node=None):
for package_node in self.packages():
package_triple = (doc_node, self.spdx_namespace.describesPackage, package_node)
self.graph.add(package_triple)
"""# Add relationship
relate_node = self.relationships()
relate_triple = (doc_node, self.spdx_namespace.relationship, relate_node)
self.graph.add(relate_triple)"""
# Add relationship
for relate_node in self.relationships():
relate_triple = (doc_node, self.spdx_namespace.relationship, relate_node)
self.graph.add(relate_triple)

Comment on lines +1021 to +1025
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the same as in your comment here and still got the same result. I think this should be fixed before merging this PR.

# Add snippet
snippet_nodes = self.snippets()
for snippet in snippet_nodes:
Expand Down
2 changes: 1 addition & 1 deletion spdx/writers/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ def write_document(document, out, validate=True):
writer = Writer(document)
document_object = writer.create_document()

yaml.safe_dump(document_object, out, indent=2, explicit_start=True)
yaml.safe_dump(document_object, out, indent=2, explicit_start=True, encoding='utf-8')
1 change: 1 addition & 0 deletions tests/data/doc_write/json-simple-multi-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,4 @@
}
]
}

2 changes: 1 addition & 1 deletion tests/data/doc_write/json-simple-plus.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@
]
}
]
}
}
2 changes: 1 addition & 1 deletion tests/data/doc_write/json-simple.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@
]
}
]
}
}
7 changes: 5 additions & 2 deletions tests/data/doc_write/rdf-simple-plus.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,11 @@
"@rdf:resource": "http://spdx.org/licenses/LGPL-2.1-or-later"
}
}
},
},
"ns1:relationship": {"ns1:Relationship": {"@rdf:about": "SPDXRef-DOCUMENT",
"ns1:relatedSpdxElement": "SPDXRef-Package",
"ns1:relationshipType": "DESCRIBES"}},
"@rdf:about": "http://www.spdx.org/tools#SPDXRef-DOCUMENT"
}
}
}
}
7 changes: 5 additions & 2 deletions tests/data/doc_write/rdf-simple.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,11 @@
},
"ns1:fileName": "./some/path/tofile"
}
},
},
"ns1:relationship": {"ns1:Relationship": {"@rdf:about": "SPDXRef-DOCUMENT",
"ns1:relatedSpdxElement": "SPDXRef-Package",
"ns1:relationshipType": "DESCRIBES"}},
Comment on lines +65 to +67
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment about these additions:
This structure looks reasonable to me and is apparently what our rdf writer produces at the moment. It may not be what the spec intends, though:

The structure in e.g. this example is different: The related element is usually expanded (making the whole thing completely unreadable), and there doesn't seem to be a reference to the current element (maybe relationships are put on the originating object instead of the top-level document?).

Considering that there are other oddities in this test file as well (e.g. referencesFile and describesPackage doesn't exist in example files from the spec or java tools), and the documentation on the desired state is unclear, I'd leave it at this for now. I will ask on the ML what's going on with rdf...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, elements in rdf files are normally expanded at the first place they appear (which does get quite muddy, yeah).

"@rdf:about": "http://www.spdx.org/tools#SPDXRef-DOCUMENT"
}
}
}
}
2 changes: 1 addition & 1 deletion tests/data/doc_write/xml-simple-multi-package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@
<licenseInfoInFiles>LGPL-2.1-or-later</licenseInfoInFiles>
<copyrightText>NOASSERTION</copyrightText>
</files>
</Document>
</Document>
78 changes: 36 additions & 42 deletions tests/data/doc_write/xml-simple-plus.xml
Original file line number Diff line number Diff line change
@@ -1,43 +1,37 @@
<?xml version="1.0" encoding="utf-8"?>

<Document>
<spdxVersion>SPDX-2.1</spdxVersion>
<dataLicense>CC0-1.0</dataLicense>
<name>Sample_Document-V2.1</name>
<SPDXID>SPDXRef-DOCUMENT</SPDXID>
<documentNamespace>https://spdx.org/spdxdocs/spdx-example-444504E0-4F89-41D3-9A0C-0305E82C3301</documentNamespace>
<documentDescribes>SPDXRef-Package</documentDescribes>
<packages>
<SPDXID>SPDXRef-Package</SPDXID>
<name>some/path</name>
<downloadLocation>NOASSERTION</downloadLocation>
<copyrightText>Some copyrught</copyrightText>
<packageVerificationCode>
<packageVerificationCodeValue>SOME code</packageVerificationCodeValue>
</packageVerificationCode>
<checksums>
<checksumValue>SOME-SHA1</checksumValue>
<algorithm>SHA1</algorithm>
</checksums>
<licenseDeclared>NOASSERTION</licenseDeclared>
<licenseConcluded>NOASSERTION</licenseConcluded>
<licenseInfoFromFiles>LGPL-2.1-or-later</licenseInfoFromFiles>

<hasFiles>SPDXRef-File</hasFiles>
</packages>
<files>

<fileName>./some/path/tofile</fileName>
<SPDXID>SPDXRef-File</SPDXID>
<checksums>
<checksumValue>SOME-SHA1</checksumValue>
<algorithm>SHA1</algorithm>
</checksums>
<licenseConcluded>NOASSERTION</licenseConcluded>
<copyrightText>NOASSERTION</copyrightText>
<licenseInfoInFiles>LGPL-2.1-or-later</licenseInfoInFiles>

</files>


</Document>
<Document>
<spdxVersion>SPDX-2.1</spdxVersion>
<dataLicense>CC0-1.0</dataLicense>
<name>Sample_Document-V2.1</name>
<SPDXID>SPDXRef-DOCUMENT</SPDXID>
<documentNamespace>https://spdx.org/spdxdocs/spdx-example-444504E0-4F89-41D3-9A0C-0305E82C3301</documentNamespace>
<documentDescribes>SPDXRef-Package</documentDescribes>
<packages>
<SPDXID>SPDXRef-Package</SPDXID>
<name>some/path</name>
<downloadLocation>NOASSERTION</downloadLocation>
<copyrightText>Some copyrught</copyrightText>
<packageVerificationCode>
<packageVerificationCodeValue>SOME code</packageVerificationCodeValue>
</packageVerificationCode>
<checksums>
<checksumValue>SOME-SHA1</checksumValue>
<algorithm>SHA1</algorithm>
</checksums>
<licenseDeclared>NOASSERTION</licenseDeclared>
<licenseConcluded>NOASSERTION</licenseConcluded>
<licenseInfoFromFiles>LGPL-2.1-or-later</licenseInfoFromFiles>
<hasFiles>SPDXRef-File</hasFiles>
</packages>
<files>
<fileName>./some/path/tofile</fileName>
<SPDXID>SPDXRef-File</SPDXID>
<checksums>
<checksumValue>SOME-SHA1</checksumValue>
<algorithm>SHA1</algorithm>
</checksums>
<licenseConcluded>NOASSERTION</licenseConcluded>
<copyrightText>NOASSERTION</copyrightText>
<licenseInfoInFiles>LGPL-2.1-or-later</licenseInfoInFiles>
</files>
</Document>
2 changes: 1 addition & 1 deletion tests/data/doc_write/xml-simple.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@
<copyrightText>NOASSERTION</copyrightText>
<licenseInfoInFiles>LGPL-2.1-only</licenseInfoInFiles>
</files>
</Document>
</Document>
2 changes: 1 addition & 1 deletion tests/data/doc_write/yaml-simple-multi-package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ files:
licenseConcluded: NOASSERTION
licenseInfoInFiles:
- LGPL-2.1-or-later
fileName: ./some/path/tofile
fileName: ./some/path/tofile
9 changes: 6 additions & 3 deletions tests/test_jsonyamlxml_parser.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

# Copyright (c) Xavier Figueroa
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -10,15 +9,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from collections import OrderedDict
import io
import json
import unittest
from collections import OrderedDict
from unittest import TestCase

from spdx.parsers import jsonparser, yamlparser, xmlparser
from spdx.parsers.jsonyamlxmlbuilders import Builder
from spdx.parsers.loggers import StandardLogger

from tests import utils_test
from tests.utils_test import TestParserUtils

Expand Down Expand Up @@ -54,6 +53,10 @@ def test_yaml_parser(self):
expected_loc = utils_test.get_test_loc('doc_parse/expected.json')
self.check_document(document, expected_loc)

@unittest.skip(
"This fails currently due to some differing whitespace. While trying to fix this in expected.json, "
"I realized that it is hopelessly out of date and should be replaced completely. "
"https://github.com/spdx/tools-python/issues/264")
def test_xml_parser(self):
parser = xmlparser.Parser(Builder(), StandardLogger())
test_file = utils_test.get_test_loc('formats/SPDXXmlExample.xml')
Expand Down
4 changes: 2 additions & 2 deletions tests/utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def check_rdf_scan(expected_file, result_file, regen=False):
else:
with io.open(expected_file, 'r', encoding='utf-8') as i:
expected = sort_nested(json.load(i))
assert expected == result
assert result == expected


def load_and_clean_tv(location):
Expand Down Expand Up @@ -195,7 +195,7 @@ def check_json_scan(expected_file, result_file, regen=False):
o.write(result)

expected = load_and_clean_json(expected_file)
assert expected == result
assert result == expected


def load_and_clean_yaml(location):
Expand Down