Ensure that an "or later" id can be used #27

 * the key change is in rdf.py
 * also add tests for doc validate() and write_document() for rdf and
   tag/values
 * streamline and refine validation code and add several new todo/fixme
 * bump version

Signed-off-by: Philippe Ombredanne <[email protected]>

setup.py

@@ -13,7 +13,7 @@ def test_suite():
 
 setup(
     name='spdx-tools',
-    version='0.5.3',
+    version='0.5.4',
     description='SPDX parser and tools.',
     packages=['spdx', 'spdx.parsers', 'spdx.writers', 'spdx.parsers.lexers'],
     package_data={'spdx': ['spdx_licenselist.csv']},
@@ -30,6 +30,9 @@ def test_suite():
             'spdx-tv2rdf = spdx.tv_to_rdf:main',
         ],
     },
+    tests_require=[
+        'xmltodict',
+    ],
 
     author='Ahmed H. Ismail',
     author_email='[email protected]',

spdx/creationinfo.py

@@ -54,10 +54,10 @@ def __init__(self, name, email):
 
     # FIXME: do not overrride eq and not hash
     def __eq__(self, other):
-        return isinstance(other, Organization) and (self.name, self.email) == (other.name,  other.email)
+        return isinstance(other, Organization) and (self.name, self.email) == (other.name, other.email)
 
     def __lt__(self, other):
-        return isinstance(other, Organization) and (self.name, self.email) < (other.name,  other.email)
+        return isinstance(other, Organization) and (self.name, self.email) < (other.name, other.email)
 
     def to_value(self):
         if self.email:
@@ -84,10 +84,10 @@ def __init__(self, name, email):
 
     # FIXME: do not overrride eq and not hash
     def __eq__(self, other):
-        return isinstance(other, Person) and (self.name, self.email) == (other.name,  other.email)
+        return isinstance(other, Person) and (self.name, self.email) == (other.name, other.email)
 
     def __lt__(self, other):
-        return isinstance(other, Person) and (self.name, self.email) < (other.name,  other.email)
+        return isinstance(other, Person) and (self.name, self.email) < (other.name, other.email)
 
     def to_value(self):
         if self.email is not None:
@@ -152,21 +152,30 @@ def created_iso_format(self):
     def has_comment(self):
         return self.comment is not None
 
-    def validate(self, messages):
+    def validate(self, messages=None):
         """Returns True if the fields are valid according to the SPDX standard.
         Appends user friendly messages to the messages parameter.
         """
-        return (self.validate_creators(messages) and
-                self.validate_created(messages))
+        # FIXME: messages should be returned
+        messages = messages if messages is not None else []
+
+        return (self.validate_creators(messages)
+            and self.validate_created(messages))
+
+    def validate_creators(self, messages=None):
+        # FIXME: messages should be returned
+        messages = messages if messages is not None else []
 
-    def validate_creators(self, messages):
         if len(self.creators) != 0:
             return True
         else:
             messages.append('No creators defined, must have at least one.')
             return False
 
-    def validate_created(self, messages):
+    def validate_created(self, messages=None):
+        # FIXME: messages should be returned
+        messages = messages if messages is not None else []
+
         if self.created is not None:
             return True
         else:

spdx/document.py

@@ -175,8 +175,8 @@ def add_xref(self, ref):
 
     def validate(self, messages=None):
         # FIXME: messages should be returned
-        if messages is None:
-            messages = []
+        messages = messages if messages is not None else []
+
         if self.text is None:
             messages.append('ExtractedLicense text can not be None')
             return False
@@ -234,18 +234,19 @@ def validate(self, messages=None):
         """
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
-        return (
-            self.validate_version(messages) and
-            self.validate_data_lics(messages) and
-            self.validate_creation_info(messages) and
-            self.validate_package(messages) and
-            self.validate_extracted_licenses(messages) and
-            self.validate_reviews(messages)
+
+        return (self.validate_version(messages)
+            and self.validate_data_lics(messages)
+            and self.validate_creation_info(messages)
+            and self.validate_package(messages)
+            and self.validate_extracted_licenses(messages)
+            and self.validate_reviews(messages)
         )
 
     def validate_version(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
+
         if self.version is None:
             messages.append('Document has no version.')
             return False
@@ -255,27 +256,31 @@ def validate_version(self, messages=None):
     def validate_data_lics(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
-        if self.data_license is not None:
-            if self.data_license.identifier == 'CC0-1.0':
-                return True
-            else:
-                messages.append('Document data license must be CC0-1.0.')
-                return False
-        else:
+
+        if self.data_license is None:
             messages.append('Document has no data license.')
             return False
 
+        if self.data_license.identifier == 'CC0-1.0':
+            return True
+        else:
+            # FIXME: REALLY? what if someone wants to use something else?
+            messages.append('Document data license must be CC0-1.0.')
+            return False
+
     def validate_reviews(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
-        status = True
+
+        valid = True
         for review in self.reviews:
-            status = status and review.validate(messages)
-        return status
+            valid = review.validate(messages) and valid
+        return valid
 
     def validate_creation_info(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
+
         if self.creation_info is not None:
             return self.creation_info.validate(messages)
         else:
@@ -285,6 +290,7 @@ def validate_creation_info(self, messages=None):
     def validate_package(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
+
         if self.package is not None:
             return self.package.validate(messages)
         else:
@@ -294,12 +300,14 @@ def validate_package(self, messages=None):
     def validate_extracted_licenses(self, messages=None):
         # FIXME: messages should be returned
         messages = messages if messages is not None else []
-        status = True
+
+        valid = True
         for lic in self.extracted_licenses:
             if isinstance(lic, ExtractedLicense):
-                status = status and lic.validate(messages)
+                valid = lic.validate(messages) and valid
             else:
-                messages.append('Document extracted licenses must be of type' +
-                    'spdx.document.ExtractedLicense')
-                return False
-        return status
+                messages.append(
+                    'Document extracted licenses must be of type '
+                    'spdx.document.ExtractedLicense and not ' + type(lic))
+                valid = False
+        return valid

spdx/file.py

@@ -98,27 +98,31 @@ def validate(self, messages=None):
         to messages parameter if there are errors.
         """
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
-        return (self.validate_lic_conc(messages) and
-                self.validate_type(messages) and
-                self.validate_chksum(messages) and
-                self.validate_licenses_in_file(messages) and
-                self.validate_copyright(messages) and
-                self.validate_artifacts(messages))
+        messages = messages if messages is not None else []
+
+        return (self.validate_concluded_license(messages)
+            and self.validate_type(messages)
+            and self.validate_chksum(messages)
+            and self.validate_licenses_in_file(messages)
+            and self.validate_copyright(messages)
+            and self.validate_artifacts(messages))
 
     def validate_copyright(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
-        if isinstance(self.copyright, (six.string_types, six.text_type,
-                                       utils.NoAssert, utils.SPDXNone)):
+        messages = messages if messages is not None else []
+
+        if isinstance(
+            self.copyright,
+            (six.string_types, six.text_type, utils.NoAssert, utils.SPDXNone)):
             return True
         else:
             messages.append('File copyright must be str or unicode or utils.NoAssert or utils.SPDXNone')
             return False
 
     def validate_artifacts(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
+        messages = messages if messages is not None else []
+
         if (len(self.artifact_of_project_home) >=
             max(len(self.artifact_of_project_uri), len(self.artifact_of_project_name))):
             return True
@@ -128,36 +132,43 @@ def validate_artifacts(self, messages=None):
 
     def validate_licenses_in_file(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
+        messages = messages if messages is not None else []
+
+        # FIXME: what are we testing the length of a list? or?
         if len(self.licenses_in_file) == 0:
             messages.append('File must have at least one license in file.')
             return False
         else:
             return True
 
-    def validate_lic_conc(self, messages=None):
+    def validate_concluded_license(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
-        if type(self.conc_lics) in [utils.NoAssert,
-            utils.SPDXNone] or isinstance(self.conc_lics, document.License):
+        messages = messages if messages is not None else []
+
+        # FIXME: use isinstance instead??
+        if (type(self.conc_lics) in [utils.NoAssert, utils.SPDXNone]
+        or isinstance(self.conc_lics, document.License)):
             return True
         else:
-            messages.append('File concluded license must be one of document.License, utils.NoAssert or utils.SPDXNone')
+            messages.append(
+                'File concluded license must be one of '
+                'document.License, utils.NoAssert or utils.SPDXNone')
             return False
 
     def validate_type(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
-        if self.type in [None, FileType.SOURCE, FileType.OTHER, FileType.BINARY,
-            FileType.ARCHIVE]:
+        messages = messages if messages is not None else []
+
+        if self.type in [None, FileType.SOURCE, FileType.OTHER, FileType.BINARY, FileType.ARCHIVE]:
             return True
         else:
             messages.append('File type must be one of the constants defined in class spdx.file.FileType')
             return False
 
     def validate_chksum(self, messages=None):
         # FIXME: messages should be returned
-        messages = messages if messages is None else []
+        messages = messages if messages is not None else []
+
         if isinstance(self.chk_sum, checksum.Algorithm):
             if self.chk_sum.identifier == 'SHA1':
                 return True