Fixed #451

 * Add support for '@' as filename
 * Update SPEC
 * Add tests
 * improve efficiency

Author: chinyeungli

SPECIFICATION.rst

@@ -1,4 +1,4 @@
-ABOUT File Specification v3.2.0
+ABOUT File Specification v3.2.1
 
 
 Purpose
@@ -74,7 +74,7 @@ A file name can contain only these US-ASCII characters:
 
 - digits from 0 to 9
 - uppercase and lowercase letters from A to Z
-- the following symbols: "_", "-", "+", ".", "(", ")", "~", "[", "]", "{", "}"
+- the following symbols: "_", "-", "+", ".", "(", ")", "~", "[", "]", "{", "}", "@"
 
 - The case of a file name is not significant. On case-sensitive file systems
   (such as on Linux), a tool must report an error if two ABOUT files stored in

src/attributecode/gen.py

@@ -37,6 +37,7 @@
 from attributecode.util import add_unc
 from attributecode.util import csv
 from attributecode.util import file_fields
+from attributecode.util import invalid_chars
 from attributecode.util import to_posix
 from attributecode.util import UNC_PREFIX_POSIX
 from attributecode.util import unique
@@ -77,42 +78,42 @@ def check_duplicated_columns(location):
         errors.append(Error(ERROR, msg))
     return unique(errors)
 
-
-def check_duplicated_about_resource(inventory_dict):
+def check_duplicated_about_resource(arp, arp_list):
     """
     Return a list of errors for duplicated about_resource in a CSV file at location.
     """
-    arp_list = []
-    errors = []
-    for component in inventory_dict:
-        # Ignore all the empty path
-        if component['about_resource']:
-            if component['about_resource'] in arp_list:
-                msg = ("The input has duplicated values in 'about_resource' "
-                       "field: " + component['about_resource'])
-                errors.append(Error(CRITICAL, msg))
-            else:
-                arp_list.append(component['about_resource'])
-    return errors
+    if arp in arp_list:
+        msg = ("The input has duplicated values in 'about_resource' "
+               "field: " + arp)
+        return Error(CRITICAL, msg)
+    return ''
 
-
-def check_newline_in_file_field(inventory_dict):
+def check_newline_in_file_field(component):
     """
     Return a list of errors for newline characters detected in *_file fields.
     """
     errors = []
-    for component in inventory_dict:
-        for k in component.keys():
-            if k in file_fields:
-                try:
-                    if '\n' in component[k]:
-                        msg = ("New line character detected in '%s' for '%s' which is not supported."
-                                "\nPlease use ',' to declare multiple files.") % (k, component['about_resource'])
-                        errors.append(Error(CRITICAL, msg))
-                except:
-                    pass
+    for k in component.keys():
+        if k in file_fields:
+            try:
+                if '\n' in component[k]:
+                    msg = ("New line character detected in '%s' for '%s' which is not supported."
+                            "\nPlease use ',' to declare multiple files.") % (k, component['about_resource'])
+                    errors.append(Error(CRITICAL, msg))
+            except:
+                pass
     return errors
 
+def check_about_resource_filename(arp):
+    """
+    Return error for invalid/non-support about_resource's filename or
+    empty string if no error is found. 
+    """
+    if invalid_chars(arp):
+        msg = ("Invalid characters present in 'about_resource' "
+                   "field: " + arp)
+        return (Error(CRITICAL, msg))
+    return ''
 
 # TODO: this should be either the CSV or the ABOUT files but not both???
 def load_inventory(location, base_dir, reference_dir=None):
@@ -139,15 +140,26 @@ def load_inventory(location, base_dir, reference_dir=None):
         inventory = util.load_json(location)
 
     try:
-        # FIXME: this should not be done here.
-        dup_about_resource_err = check_duplicated_about_resource(inventory)
-        if dup_about_resource_err:
-            errors.extend(dup_about_resource_err)
-            return errors, abouts
-        newline_in_file = check_newline_in_file_field(inventory)
-        if newline_in_file:
-            errors.extend(newline_in_file)
+        arp_list = []
+        errors = []
+        for component in inventory:
+            arp = component['about_resource']
+            dup_err = check_duplicated_about_resource(arp, arp_list)
+            if dup_err:
+                errors.append(dup_err)
+            else:
+                arp_list.append(arp)
+
+            newline_in_file_err = check_newline_in_file_field(component)
+            for err in newline_in_file_err:
+                errors.append(err)
+
+            invalid_about_filename = check_about_resource_filename(arp)
+            if invalid_about_filename:
+                errors.append(invalid_about_filename)
+        if errors:
             return errors, abouts
+
     except Exception as e:
         # TODO: why catch ALL Exception
         msg = "The essential field 'about_resource' is not found in the <input>"

src/attributecode/util.py

@@ -70,7 +70,7 @@ def to_posix(path):
 UNC_PREFIX_POSIX = to_posix(UNC_PREFIX)
 UNC_PREFIXES = (UNC_PREFIX_POSIX, UNC_PREFIX,)
 
-valid_file_chars = string.digits + string.ascii_letters + '_-.+()~[]{}|' + ' '
+valid_file_chars = string.digits + string.ascii_letters + '_-.+()~[]{}|@' + ' '
 
 
 def invalid_chars(path):

tests/test_gen.py

@@ -46,27 +46,38 @@ def test_check_duplicated_columns_handles_lower_upper_case(self):
         assert expected == result
 
     def test_check_duplicated_about_resource(self):
-        test_dict = [
-            {'about_resource': '/test/test.c', 'version': '1.03', 'name': 'test.c'},
-            {'about_resource': '/test/abc/', 'version': '1.0', 'name': 'abc'},
-            {'about_resource': '/test/test.c', 'version': '1.04', 'name': 'test1.c'}]
-        expected = [
-            Error(CRITICAL,
-                  "The input has duplicated values in 'about_resource' field: /test/test.c")]
-        result = gen.check_duplicated_about_resource(test_dict)
-        assert expected == result
+        arp_list = ['/test/test.c', 'test/test1.h']
+        arp1 = '/test/test.c'
+        arp2 = '/test/tmp/test.c'
+        expected = Error(CRITICAL,
+                  "The input has duplicated values in 'about_resource' field: " + arp1)
+        result1 = gen.check_duplicated_about_resource(arp1, arp_list)
+        result2 = gen.check_duplicated_about_resource(arp2, arp_list)
+        assert result1 == expected
+        assert result2 == ''
 
     def test_check_newline_in_file_field(self):
-        test_dict = [
-            {'about_resource': '/test/test.c', 'name': 'test.c', 'notice_file': 'NOTICE\nNOTICE2'},
-            {'about_resource': '/test/abc/', 'version': '1.0', 'name': 'abc'},
-            {'about_resource': '/test/test.c', 'version': '1.04', 'name': 'test1.c'}]
+        test_dict1 = {'about_resource': '/test/test.c', 'name': 'test.c', 'notice_file': 'NOTICE\nNOTICE2'}
+        test_dict2 = {'about_resource': '/test/test.c', 'name': 'test.c', 'notice_file': 'NOTICE, NOTICE2'}
         expected = [
             Error(CRITICAL,
                   "New line character detected in 'notice_file' for '/test/test.c' which is not supported."
                   "\nPlease use ',' to declare multiple files.")]
-        result = gen.check_newline_in_file_field(test_dict)
-        assert expected == result
+        result1 = gen.check_newline_in_file_field(test_dict1)
+        result2 = gen.check_newline_in_file_field(test_dict2)
+        assert result1 == expected
+        assert result2 == []
+
+    def test_check_about_resource_filename(self):
+        arp1 = '/test/[email protected]'
+        arp2 = '/test/t!est.c'
+        msg = ("Invalid characters present in 'about_resource' "
+                   "field: " + arp2)
+        expected2 = Error(CRITICAL, msg)
+        result1 = gen.check_about_resource_filename(arp1)
+        result2 = gen.check_about_resource_filename(arp2)
+        assert result1 == ''
+        assert result2 == expected2
 
     def test_load_inventory(self):
         location = get_test_loc('test_gen/inv.csv')