#331 - Multiple licenses support

 * Remove the loads and load_lines as as we are not reading the ABOUT
file line by line by strings, we are not using yaml.load instead.
 * Add a yaml class to check for duplicated keys
 * Using yaml dump to write files instead of string object

Signed-off-by: Chin Yeung Li <[email protected]>

Author: chinyeungli

src/attributecode/model.py

@@ -34,16 +34,20 @@
 import os
 import posixpath
 from posixpath import dirname
+
+import yaml
 import re
 import sys
 
 if sys.version_info[0] < 3:  # Python 2
     import backports.csv as csv
+    from itertools import izip_longest as zip_longest
     from urlparse import urljoin, urlparse
     from urllib2 import urlopen, Request, HTTPError
 else:  # Python 3
     basestring = str
     import csv
+    from itertools import zip_longest
     from urllib.parse import urljoin, urlparse
     from urllib.request import urlopen, Request
     from urllib.error import HTTPError
@@ -60,7 +64,6 @@
 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
@@ -107,11 +110,6 @@ def validate(self, *args, **kwargs):
                 msg = u'Field %(name)s is required'
                 errors.append(Error(CRITICAL, msg % locals()))
                 return errors
-            """else:
-                # no error for not present non required fields
-                # FIXME: should we add an info?
-                # CY: I don't think so.
-                pass"""
         else:
             # present fields should have content ...
             if not self.has_content:
@@ -986,51 +984,34 @@ 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()
+            # Check for duplicated key
+            yaml.load(input_text, Loader=util.NoDuplicateLoader)
+            '''
             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)
+            '''
+            """
+            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()))
 
         self.errors = errors
         return errors
 
-    def loads(self, string, base_dir):
-        """
-        Load the ABOUT file from string. Return a list of errors.
-        """
-        lines = string.splitlines(True)
-        errors = self.load_lines(lines, base_dir)
-        self.errors = errors
-        return errors
-
-    def load_lines(self, lines, base_dir):
-        """
-        Load the ABOUT file from a lines list. Return a list of errors.
-        """
-        errors = []
-        parse_errors, fields = parse(lines)
-        errors.extend(parse_errors)
-        process_errors = self.process(fields, base_dir)
-        errors.extend(process_errors)
-        self.errors = errors
-        return errors
-
     def load_dict(self, fields_dict, base_dir, running_inventory=False,
                   use_mapping=False, mapping_file=None,
                   license_notice_text_location=None, with_empty=True):
@@ -1050,17 +1031,52 @@ def load_dict(self, fields_dict, base_dir, running_inventory=False,
         self.errors = errors
         return errors
 
+
     def dumps(self, with_absent=False, with_empty=True):
         """
         Return self as a formatted ABOUT string.
         If with_absent, include absent (not present) fields.
         If with_empty, include empty fields.
         """
-        serialized = []
+        about_data = {}
+        # Group the same license information (name, url, file) together
+        license_key = []
+        license_name = []
+        license_file = []
+        license_url = []
+
         for field in self.all_fields(with_absent, with_empty):
-            serialized.append(field.serialize())
-        # always end with a new line
-        return u'\n'.join(serialized) + u'\n'
+            if field.name == 'license_key':
+                license_key = field.value
+            elif field.name == 'license_name':
+                license_name = field.value
+            elif field.name == 'license_file':
+                license_file = field.value.keys()
+            elif field.name == 'license_url':
+                license_url = field.value
+            # No multiple 'about_resource' and 'about_resource_path' reference supported.
+            # Take the first element (should only be one) in the list for the
+            # value of 'about_resource' and 'about_resource_path'
+            elif field.name == 'about_resource':
+                about_data[field.name] = field.value[0]
+            elif field.name == 'about_resource_path':
+                about_data[field.name] = field.value.keys()[0]
+            else:
+                about_data[field.name] = field.value
+        # Group the same license information in a list
+        license_group = list(zip_longest(license_key, license_name, license_file, license_url))
+        for lic_group in license_group:
+            lic_dict = {}
+            if lic_group[0] or with_empty:
+                lic_dict['key'] = lic_group[0]
+            if lic_group[1] or with_empty:
+                lic_dict['name'] = lic_group[1]
+            if lic_group[2] or with_empty:
+                lic_dict['file'] = lic_group[2]
+            if lic_group[3] or with_empty:
+                lic_dict['url'] = lic_group[3] 
+            about_data.setdefault('licenses', []).append(lic_dict)
+        return saneyaml.dump(about_data)
 
     def dump(self, location, with_absent=False, with_empty=True):
         """

src/attributecode/util.py

@@ -17,6 +17,7 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import collections
 from collections import OrderedDict
 import codecs
 import errno
@@ -32,6 +33,15 @@
 import string
 import sys
 
+import yaml
+from yaml.reader import Reader
+from yaml.scanner import Scanner
+from yaml.parser import Parser
+from yaml.composer import Composer
+from yaml.constructor import Constructor, ConstructorError
+from yaml.resolver import Resolver
+from yaml.nodes import MappingNode
+
 if sys.version_info[0] < 3:
     # Python 2
     import backports.csv as csv
@@ -635,4 +645,53 @@ def update_about_dictionary_keys(about_dictionary_list, mapping_output):
             if not update_key:
                 updated_ordered_dict[about_key] = value
         updated_dict_list.append(updated_ordered_dict)
-    return updated_dict_list
+    return updated_dict_list
+
+class NoDuplicateConstructor(Constructor):
+    def construct_mapping(self, node, deep=False):
+        if not isinstance(node, MappingNode):
+            raise ConstructorError(
+                None, None,
+                "expected a mapping node, but found %s" % node.id,
+                node.start_mark)
+        mapping = {}
+        for key_node, value_node in node.value:
+            # keys can be list -> deep
+            key = self.construct_object(key_node, deep=True)
+            # lists are not hashable, but tuples are
+            if not isinstance(key, collections.Hashable):
+                if isinstance(key, list):
+                    key = tuple(key)
+
+            if sys.version_info.major == 2:
+                try:
+                    hash(key)
+                except TypeError as exc:
+                    raise ConstructorError(
+                        "while constructing a mapping", node.start_mark,
+                        "found unacceptable key (%s)" %
+                        exc, key_node.start_mark)
+            else:
+                if not isinstance(key, collections.Hashable):
+                    raise ConstructorError(
+                        "while constructing a mapping", node.start_mark,
+                        "found unhashable key", key_node.start_mark)
+
+            value = self.construct_object(value_node, deep=deep)
+
+            # Actually do the check.
+            if key in mapping:
+                raise KeyError("Got duplicate key: {!r}".format(key))
+
+            mapping[key] = value
+        return mapping
+
+
+class NoDuplicateLoader(Reader, Scanner, Parser, Composer, NoDuplicateConstructor, Resolver):
+    def __init__(self, stream):
+        Reader.__init__(self, stream)
+        Scanner.__init__(self)
+        Parser.__init__(self)
+        Composer.__init__(self)
+        NoDuplicateConstructor.__init__(self)
+        Resolver.__init__(self)

tests/test_model.py

@@ -940,34 +940,6 @@ def test_About_as_dict_with_nothing(self):
         # FIXME: why converting back to dict?
         assert expected == dict(result)
 
-    def test_loads_dumps_is_idempotent(self):
-        test1 = u'''about_resource: .
-name: AboutCode
-version: 0.11.0
-copyright: |
-    multi
-    line
-'''
-
-        test2 = u'''about_resource: .
-name: AboutCode
-version: 0.11.0
-copyright: >
-    multi
-    line
-'''
-        a = model.About()
-        base_dir = 'some_dir'
-        a.loads(test1, base_dir)
-        dumped = a.dumps(with_absent=False, with_empty=False)
-        assert test1 == dumped
-
-        b = model.About()
-        base_dir = 'some_dir'
-        b.loads(test2, base_dir)
-        dumped = b.dumps(with_absent=False, with_empty=False)
-        assert test2 == dumped
-
     def test_load_dump_is_idempotent(self):
         test_file = get_test_loc('load/this.ABOUT')
         a = model.About()
@@ -1075,22 +1047,6 @@ def test_write_output_json(self):
         expected = get_test_loc('load/expected.json')
         check_json(expected, result)
 
-    def test_colon_in_value(self):
-        test = u'''about_resource: .
-name: AboutCode
-version: v: 0.11.0
-'''
-        expected = u'''about_resource: .
-name: AboutCode
-version: |
-    v: 0.11.0
-'''
-        a = model.About()
-        base_dir = 'some_dir'
-        a.loads(test, base_dir)
-        dumped = a.dumps(with_absent=False, with_empty=False)
-        assert dumped == expected
-
 class CollectorTest(unittest.TestCase):
 
     def test_collect_inventory_in_directory_with_correct_about_file_path(self):