Fixed #324

 * Prompts error and stop generating the output if multiple keys are
detected.

Signed-off-by: Chin Yeung Li <tli@nexb.com>

Author: chinyeungli

src/attributecode/cmd.py

@@ -157,9 +157,22 @@ def inventory(location, output, mapping, mapping_file, quiet, format, verbose):
 
     errors, abouts = model.collect_inventory(location, use_mapping=mapping, mapping_file=mapping_file)
 
-    write_errors = model.write_output(abouts, output, format)
-    for err in write_errors:
-        errors.append(err)
+    # Do not write the output if one of the ABOUT files has duplicated key names
+    dup_error_msg = u'Duplicated key name(s)'
+    halt_output = False
+    for err in errors:
+        if dup_error_msg in err.message:
+            halt_output = True
+            break
+
+    if not halt_output:
+        write_errors = model.write_output(abouts, output, format)
+        for err in write_errors:
+            errors.append(err)
+    else:
+        msg = u'Duplicated key names are not supported.\n' + \
+                        'Please correct and re-run.'
+        print(msg)
 
     finalized_errors = update_severity_level_about_resource_path_not_exist_error(errors)
 

src/attributecode/model.py

@@ -60,6 +60,7 @@
 from attributecode import util
 from attributecode.util import add_unc
 from attributecode.util import copy_license_notice_files
+from attributecode.util import check_duplicate_keys_about_file
 from attributecode.util import on_windows
 from attributecode.util import UNC_PREFIX
 from attributecode.util import UNC_PREFIX_POSIX
@@ -983,18 +984,23 @@ def load(self, location, use_mapping=False, mapping_file=None):
             loc = add_unc(loc)
             with codecs.open(loc, encoding='utf-8') as txt:
                 input_text = txt.read()
-            """
-            The running_inventory defines if the current process is 'inventory' or not.
-            This is used for the validation of the about_resource_path.
-            In the 'inventory' command, the code will use the parent of the about_file_path
-            location and join with the 'about_resource_path' for the validation.
-            On the other hand, in the 'gen' command, the code will use the
-            generated location (aka base_dir) along with the parent of the about_file_path
-            and then join with the 'about_resource_path'
-            """
-            running_inventory = True
-            errs = self.load_dict(saneyaml.load(input_text), base_dir, running_inventory, use_mapping, mapping_file)
-            errors.extend(errs)
+            dup_keys = check_duplicate_keys_about_file(input_text)
+            if dup_keys:
+                msg = ('Duplicated key name(s): %(dup_keys)s' % locals())
+                errors.append(Error(ERROR, msg % locals()))
+            else:
+                """
+                The running_inventory defines if the current process is 'inventory' or not.
+                This is used for the validation of the about_resource_path.
+                In the 'inventory' command, the code will use the parent of the about_file_path
+                location and join with the 'about_resource_path' for the validation.
+                On the other hand, in the 'gen' command, the code will use the
+                generated location (aka base_dir) along with the parent of the about_file_path
+                and then join with the 'about_resource_path'
+                """
+                running_inventory = True
+                errs = self.load_dict(saneyaml.load(input_text), base_dir, running_inventory, use_mapping, mapping_file)
+                errors.extend(errs)
         except Exception as e:
             msg = 'Cannot load invalid ABOUT file: %(location)r: %(e)r\n' + str(e)
             errors.append(Error(CRITICAL, msg % locals()))

src/attributecode/util.py

@@ -124,6 +124,23 @@ def check_file_names(paths):
     return errors
 
 
+def check_duplicate_keys_about_file(context):
+    keys = []
+    dup_keys = []
+    for line in context.splitlines():
+        """
+        Ignore all the continuation string, string block and empty line
+        """
+        if not line.startswith(' ') and not len(line.strip()) == 0 :
+            # Get the key name
+            key = line.partition(':')[0]
+            if key in keys:
+                dup_keys.append(key)
+            else:
+                keys.append(key)
+    return dup_keys
+
+
 def get_absolute(location):
     """
     Return an absolute normalized location.

tests/test_util.py

@@ -444,3 +444,30 @@ def test_update_severity_level_about_resource_path_not_exist_error(self):
         input_err = [Error(ERROR, 'Field about_resource_path: test.tar.gz does not exist')]
         expected_err = [Error(INFO, 'Field about_resource_path: test.tar.gz does not exist')]
         assert util.update_severity_level_about_resource_path_not_exist_error(input_err) == expected_err
+
+    def test_check_duplicate_keys_about_file(self):
+        test = '''
+name: test
+notes: some notes
+
+license_expression: mit
+notes: dup key here
+            '''
+        expected = ['notes']
+        assert expected == util.check_duplicate_keys_about_file(test)
+
+    def test_check_duplicate_keys_about_file_with_multiline(self):
+        test = '''
+name: test
+owner: test
+notes: |
+    String block here
+license_expression: mit
+owner: test1
+notes: continuation
+ line
+description: sample
+            '''
+        expected = ['owner', 'notes']
+        assert expected == util.check_duplicate_keys_about_file(test)
+