Merge branch '542_strip_empty_spaces_empty_newline' into develop

Author: chinyeungli

src/attributecode/gen.py

@@ -33,6 +33,7 @@
 from attributecode.util import to_posix
 from attributecode.util import UNC_PREFIX_POSIX
 from attributecode.util import load_scancode_json, load_csv, load_json, load_excel
+from attributecode.util import strip_inventory_value
 
 
 def check_duplicated_columns(location):
@@ -93,8 +94,12 @@ def check_newline_in_file_field(component):
         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'])
+                    if k == u'about_resource':
+                        msg = (
+                            "Multiple lines detected in 'about_resource' for '%s' which is not supported.") % component['about_resource']
+                    else:
+                        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
@@ -124,6 +129,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
     """
     errors = []
     abouts = []
+    is_spreadsheet = False
 
     if base_dir:
         base_dir = util.to_posix(base_dir)
@@ -136,8 +142,10 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
                 errors.extend(dup_cols_err)
                 return errors, abouts
             inventory = load_csv(location)
+            is_spreadsheet = True
         elif location.endswith('.xlsx'):
             dup_cols_err, inventory = load_excel(location, worksheet)
+            is_spreadsheet = True
             if dup_cols_err:
                 errors.extend(dup_cols_err)
                 return errors, abouts
@@ -147,7 +155,14 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
     try:
         arp_list = []
         errors = []
-        for component in inventory:
+
+        if is_spreadsheet:
+            # Only the .csv and .xlsx may have newline issue
+            stripped_inv = strip_inventory_value(inventory)
+        else:
+            stripped_inv = inventory
+
+        for component in stripped_inv:
             if not from_attrib:
                 arp = component['about_resource']
                 dup_err = check_duplicated_about_resource(arp, arp_list)
@@ -164,6 +179,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
             newline_in_file_err = check_newline_in_file_field(component)
             if newline_in_file_err:
                 errors.extend(newline_in_file_err)
+
         if errors:
             return errors, abouts
     except Exception as e:
@@ -173,7 +189,7 @@ def load_inventory(location, from_attrib=False, base_dir=None, scancode=False, r
         return errors, abouts
 
     custom_fields_list = []
-    for fields in inventory:
+    for fields in stripped_inv:
         # check does the input contains the required fields
         required_fields = model.About.required_fields
 

src/attributecode/model.py

@@ -798,12 +798,12 @@ def set_standard_fields(self):
             ('package_url', PackageUrlField()),
             ('notes', StringField()),
 
-            ('license_expression', StringField()),
+            ('license_expression', SingleLineField()),
             ('license_key', ListField()),
             ('license_name', ListField()),
             ('license_file', FileTextField()),
             ('license_url', UrlListField()),
-            ('spdx_license_expression', StringField()),
+            ('spdx_license_expression', SingleLineField()),
             ('spdx_license_key', ListField()),
             ('copyright', StringField()),
             ('notice_file', FileTextField()),

src/attributecode/util.py

@@ -803,6 +803,20 @@ def write_licenses(lic_dict, location):
     return errors
 
 
+def strip_inventory_value(inventory):
+    """
+    The inventory is a list of dictionaries. This function will strip the value
+    of the dictionary and return the stripped dictionary to a list
+    """
+    stripped_inventory = []
+    for component in inventory:
+        comp_dict = {}
+        for key in component:
+            comp_dict[key] = component[key].strip()
+        stripped_inventory.append(comp_dict)
+    return stripped_inventory
+
+
 """
 Return True if a string s  name is safe to use as an attribute name.
 """

tests/test_attrib.py

@@ -245,6 +245,8 @@ def test_scancode_input_dup_lic_match(self):
         test_file = get_test_loc(
             'test_attrib/scancode_input/sc-dup-lic-match.json')
         errors, abouts = gen.load_inventory(test_file, scancode=True)
+        print("############################")
+        print(errors)
         # Check if there is error's level > INFO
         result = [(level, e) for level, e in errors if level > INFO]
         assert result == []

tests/test_util.py

@@ -162,7 +162,7 @@ def test_check_file_names_with_dupes_return_errors(self):
             Error(
                 CRITICAL,
                 "Duplicate files: 'some/PAth' and 'some/path' have the same case-insensitive file name")
-            ]
+        ]
         assert expected == result
 
     def test_check_file_names_without_dupes_return_no_error(self):
@@ -196,9 +196,11 @@ def test_check_file_names_with_invalid_chars_return_errors(self):
         ]
         import sys
         if sys.version_info[0] < 3:  # python2
-            expected = [Error(CRITICAL, b"Invalid characters '\xe9\xe8' in file name at: 'Accessibilit\xe9/ p\xe9rim\xe8tre'")]
+            expected = [Error(
+                CRITICAL, b"Invalid characters '\xe9\xe8' in file name at: 'Accessibilit\xe9/ p\xe9rim\xe8tre'")]
         else:
-            expected = [Error(CRITICAL, "Invalid characters ':' in file name at: 'locations/in:valid'")]
+            expected = [
+                Error(CRITICAL, "Invalid characters ':' in file name at: 'locations/in:valid'")]
         result = util.check_file_names(paths)
 
         assert expected[0].message == result[0].message
@@ -272,7 +274,8 @@ def test_get_about_locations(self):
         assert expected == result
 
     def test_get_locations_can_yield_a_single_file(self):
-        test_file = get_test_loc('test_util/about_locations/file with_spaces.ABOUT')
+        test_file = get_test_loc(
+            'test_util/about_locations/file with_spaces.ABOUT')
         result = list(util.get_locations(test_file))
         assert 1 == len(result)
 
@@ -351,13 +354,15 @@ def test_format_about_dict_output(self):
 
     def test_load_csv_microsoft_utf_8(self):
         test_file = get_test_loc('test_util/csv/test_ms_utf8.csv')
-        expected = [dict([(u'about_resource', u'/myFile'), (u'name', u'myName')])]
+        expected = [
+            dict([(u'about_resource', u'/myFile'), (u'name', u'myName')])]
         result = util.load_csv(test_file)
         assert expected == result
 
     def test_load_csv_utf_8(self):
         test_file = get_test_loc('test_util/csv/test_utf8.csv')
-        expected = [dict([(u'about_resource', u'/myFile'), (u'name', u'\u540d')])]
+        expected = [
+            dict([(u'about_resource', u'/myFile'), (u'name', u'\u540d')])]
         result = util.load_csv(test_file)
         assert expected == result
 
@@ -409,7 +414,7 @@ def test_load_non_list_json(self):
             'about_resource': '.',
             'name': 'AboutCode',
             'version': '0.11.0'
-            }]
+        }]
         result = util.load_json(test_file)
         assert expected == result
 
@@ -420,7 +425,7 @@ def test_load_non_list_json2(self):
             'about_resource': '.',
             'name': 'AboutCode',
             'version': '0.11.0'
-            }]
+        }]
         result = util.load_json(test_file)
         assert expected == result
 
@@ -450,7 +455,7 @@ def test_load_json_from_scancode(self):
             'dirs_count': 0,
             'size_count': 0,
             'scan_errors': []
-            }]
+        }]
         result = util.load_scancode_json(test_file)
         assert expected == result
 
@@ -497,7 +502,7 @@ def test_load_yaml_about_file_raise_exception_on__duplicate(self):
         try:
             saneyaml.load(test, allow_duplicate_keys=False)
             self.fail('Exception not raised')
-        except saneyaml.UnsupportedYamlFeatureError as e :
+        except saneyaml.UnsupportedYamlFeatureError as e:
             assert 'Duplicate key in YAML source: notes' == str(e)
 
     def test_load_yaml_about_file_raise_exception_on_invalid_yaml_ignore_non_key_line(self):
@@ -532,7 +537,7 @@ def test_load_yaml_about_file_with_multiline(self):
         try:
             saneyaml.load(test, allow_duplicate_keys=False)
             self.fail('Exception not raised')
-        except saneyaml.UnsupportedYamlFeatureError as e :
+        except saneyaml.UnsupportedYamlFeatureError as e:
             # notes: exceptio is rasied only for the first dupe
             assert 'Duplicate key in YAML source: owner' == str(e)
 
@@ -558,7 +563,8 @@ def test_ungroup_licenses(self):
             u'https://enterprise.dejacode.com/urn/?urn=urn:dje:license:mit',
             u'https://enterprise.dejacode.com/urn/?urn=urn:dje:license:bsd-new']
         expected_spdx = [u'MIT', u'BSD-3-Clause']
-        lic_key, lic_name, lic_file, lic_url, spdx_lic_key, lic_score, _matched_text = util.ungroup_licenses(about)
+        lic_key, lic_name, lic_file, lic_url, spdx_lic_key, lic_score, _matched_text = util.ungroup_licenses(
+            about)
         assert expected_lic_key == lic_key
         assert expected_lic_name == lic_name
         assert expected_lic_file == lic_file
@@ -648,3 +654,17 @@ def test_copy_file_with_dir(self):
         assert len(licenses) == len(files_list)
         for license in licenses:
             assert license in files_list
+
+    def test_strip_inventory_value(self):
+        test = [{'about_resource': 'empty_newlines.rpm\n\n', 'name': 'empty_newlines.rpm'},
+                {'about_resource': 'spaces_after.rpm   ',
+                    'name': 'spaces_after.rpm   '},
+                {'about_resource': 'value_after_newline\n123.rpm   ',
+                    'name': 'value_after'}]
+        expected = [{'about_resource': 'empty_newlines.rpm', 'name': 'empty_newlines.rpm'},
+                    {'about_resource': 'spaces_after.rpm',
+                        'name': 'spaces_after.rpm'},
+                    {'about_resource': 'value_after_newline\n123.rpm',
+                    'name': 'value_after'}]
+        stripped_result = util.strip_inventory_value(test)
+        assert stripped_result == expected